@colriot - ROAST.dev

Personality

Meticulous attention to detail Strong preference for consistency Constructive but direct communication Pragmatic problem-solver Documentation quality advocate Code organization enthusiast Helpful with specific suggestions Forward-thinking about maintainability

Greatest Hits

"nit: I think we try to keep all imports at the top"

"Consistency in casing"

"pls"

"LGTM"

"might be worth extracting"

"check if you really wanna do this"

"I personally don't like ignoring this"

"this should be in the name ideally"

"lets"

"same for JS"

Focus Areas

consistency in naming and casing
code organization and structure
documentation quality
API design
test naming conventions
import organization
maintainability concerns
user experience considerations

Common Phrases

"nit:" "LGTM" "lets" "pls" "I think" "what is" "should we" "can we" "consistency" "might be worth" "I'm ok" "dont" "this should be" "check if" "same for"

Sentiment Breakdown

neutral

2051

positive

338

constructive

967

questioning

381

critical

11

harsh_questioning

23

very_positive

6

Review Outcomes

APPROVED

720

CHANGES_REQUESTED

263

DISMISSED

43

COMMENTED

40

Most Reviewed Authors

KateZhang98

538

frankfeng98

346

paveldudka

344

jayfish0

280

thakkerurvish

240

gauravreddy08

236

colriot

230

Zechereh

214

nearestnabors

188

EricMulhernTinyfish

136

Spiciest Comments

friday/#748 · pyt/medallion/main.py [view]

And the same thing about output: return text with Result and Steps (and folder of screenshots, but even if this won't work, it's not critical for Medallion), and a wrapper that would save these to files (prob the same that would read from files and pass values to the main API)

friday/#636 · pyt/medallion/main.py [view]

Please, fix ``` if test.friendly_name not in GUARANTEED_FAILURES: ``` after I "fixed" friendly name your change won't work correctly

ux-labs/#1685 [view]

LGTM! Wdym by steaming doesn't work? I.e. browser streaming? It should work. In progress events — yes, we don't have them

ux-labs/#920 [view]

As you mentioned, the only scenario that's added is mixed endpoint case. This is useful, the rest is present. What I'd do is maybe add that mixed scenario as a separate test and that's it. But in the current state this PR shouldn't be merged for sure. I see your point of having parameterization in place to not write a lot of duplicated code. Here are my thoughts about this: - Tests should be small and targeted, scenarios small, ideally testing 1 thing and only. This way when a test breaks, y

ux-labs/#1271 · frontend/app/page.tsx [view]

Tested the root div styling approach (using `fixed inset-0 overflow-y-auto` on the root div) - it doesn't work because the scrollbar gutter is reserved at the `html` element level. The root div sits inside that space, so it can't expand beyond it. The style tag override is the only approach that works without modifying `globals.css`. The alternatives would be: 1. Modify `globals.css` to remove `overflow-y: scroll` globally (risky for modals) 2. Use `scrollbar-gutter: stable` instead (requires b

ux-labs/#941 · backend/app/api/endpoints/deployments.py [view]

I'd remove default value to decrease the chance of providing the wrong value just because there is a default, and make caller expliclitly think about what they are passing

ux-labs/#868 · backend/tests/integration/app/crud/endpoint_test.py [view]

this should be in the name ideally. I know we don't do a good job following this pattern, but I'd like us to adhere to this naming: `unit_under_test__scenario__expected_result`. (when the whole file is devoted to one unit_under_test, then the first part can be skipped). This way when test fails you don't need to go find the test and read it, just by its name you see in console you figure our which scenario is broken and why you can see examples in `test_encryption.py`

ux-labs/#710 [view]

@KateZhang98 update the summary ,pls. Screenshot table is broken

ux-labs/#615 · backend/app/api/mcp_server.py [view]

this breaks the DI. Provide it from above (from mcp endpoint, via Depends) and pass a param in constructor

ux-labs/#537 · backend/mino_api/.env.example [view]

@thakkerurvish why was it localhost at the first place? It doesn't work with Docker, only DNF names are reliable

AI Persona Prompt

You are @colriot, a meticulous code reviewer who catches every detail and cares deeply about consistency and maintainability. Your reviews are thorough and constructive, often containing many 'nit:' comments that improve code quality. You frequently use casual abbreviations like 'pls', 'lets', 'dont', and 'smth' in your comments, giving you a friendly but direct tone. You have a keen eye for consistency issues, especially in naming conventions, casing, import organization, and documentation formatting. You often suggest specific code improvements using GitHub's suggestion feature and aren't afraid to point out when something 'breaks the DI' or violates established patterns. You care about the user experience and maintainability, often asking questions like 'should we', 'can we', 'what is', and 'might be worth' when proposing improvements. You're pragmatic and will approve things with 'LGTM' when they're good enough, but you'll also say things like 'I personally don't like ignoring this' when you have strong opinions. You tend to think ahead about how code will age and be maintained, often commenting on documentation that 'will age really well' or suggesting extractions to reduce duplication. Your reviews feel like a knowledgeable colleague who genuinely wants to improve the codebase, even if it means leaving 15+ comments on a single PR. You balance being thorough with being practical, often saying 'I'm ok' with certain approaches while still suggesting better alternatives.

Sergey Ryabov