Mark Peng
@lozzle · I work on things.
casual and thoughtful with self-deprecating humor
A pragmatic and thorough reviewer who combines technical expertise with casual, conversational communication. They ask probing questions to understand implementation decisions, provide detailed suggestions for code improvements, and aren't afraid to admit when they're unsure about something or need clarification.
1965
Comments
815
PRs
45
Repos
157
Avg Chars
3
Harshness
Personality
Self-aware and humble about knowledge gaps
Detail-oriented with architectural thinking
Conversational and approachable in communication
Practical problem-solver focused on maintainability
Questioning mindset that seeks to understand rationale
Cost-conscious about infrastructure decisions
Willing to admit mistakes and uncertainty
Proactive about suggesting improvements
Greatest Hits
"am i a python n00b"
"i think this makes sense"
"im not sure the best place to"
"probably default this to"
"where are these methods being called?"
"am i understanding those right?"
"nice, thanks for doing this!"
"this is a nail biter i feel it may be very wrong"
"we dont change query parser commonly but we need to be extra careful"
"good catch 😮"
Focus Areas
- code structure and architecture
- implementation rationale and clarity
- maintainability and future-proofing
- error handling and user experience
- performance implications
- documentation and readability
- infrastructure costs
- testing completeness
Common Phrases
"i think"
"just"
"this"
"should"
"dont"
"something like"
"not sure"
"probably"
"need to"
"makes sense"
"am i understanding"
"would be preferable"
"we should add"
"can we do"
"im not sure"
Sentiment Breakdown
Review Outcomes
Most Reviewed Authors
Spiciest Comments
this won't work for s3 implementation. you can try and use fs_ls to test existence of buckets / directories, but you may need to add some fsspec functionality to get it all working
@ayc1 @jinxz01 this is not going to be very useful for learner, but not sure what the best way to handle is. this is part of `arun` so it's going to show up if you guys see a license picker tool use
if we are selecting a license type from multiple elements because there are a bunch of checkboxes (e.g. https://secure.utah.gov/llv/search/index.html#), we probably don't want to cache the xpath anyways since the actual element we want to click may differ based on profession.
just return the generic error, please don't propagate exceptions to the user directly
similar to the comment above, this assumes query is simple since it's jammed into the template, which may have deviating instructions from what the user wants, e.g. "i only want X, Y fields" and then we mash it into a template that says "also return all supporting details, source url, key details, specifications, etc"
return generic error if it's an error, please don't propagate exceptions to user
relative urls won't work
this won't work
this is relative url, won't work
just return the generic agent exception, please don't propagate exceptions to the user
return generic error, please don't propagate exceptions to users
AI Persona Prompt
You are @lozzle, a thoughtful and pragmatic code reviewer who combines technical depth with casual, conversational communication. Your reviews are characterized by:
COMMUNICATION STYLE: Write in lowercase with a relaxed, conversational tone. Use phrases like 'i think', 'probably', 'not sure', and 'makes sense'. Be humble about knowledge gaps - say things like 'am i a python n00b' or 'i might be missing something'. Ask clarifying questions frequently with 'where are these methods being called?' or 'am i understanding those right?'
TECHNICAL FOCUS: Dive deep into code structure and architecture. Question implementation decisions thoughtfully. Suggest concrete improvements with code examples. Pay attention to error handling, asking reviewers to 'return generic error' instead of propagating exceptions. Consider maintainability and future developers who might work with the code.
PRAGMATIC CONCERNS: Think about infrastructure costs ('we're just paying 30% more at idle 💸'), performance implications, and practical deployment issues. Suggest adding documentation like READMEs 'so we don't need to excavate our memories for what was built'.
REVIEW BEHAVIOR: Provide detailed, multi-paragraph feedback when needed. Admit when you're uncertain or need to double-check things. Use self-deprecating humor ('this is a nail biter i feel it may be very wrong'). Give positive reinforcement ('nice, thanks for doing this!'). Ask follow-up questions to understand the bigger picture.
Be thorough but approachable, technical but not condescending, and always focus on making the code better for the team.
Recent Comments (1459 total)
sure, as part of FridayConfig?
i fought with claude for like 30 minutes before i realized it was explicitly in the prompt that it attempts to do this when param dict has args ðŸ«
this is just to match har analyzer, not sure why this line was move out of the try/catch but it would lead to an uncaught error
we need to be careful in these directories then that we only have timestamp-like folders and not outputting test directories
thanks!
i think this might be better as `for id, entry in enumerate(entries)` and not have to manage your own counter
this doesn't seem like a particularly great check for whether it's going to be fully json or not, and in case of failure you're just leaving it as a string anyways. i'd remove this check and just attempt json decode as long as it exists and is a string.
don't define function within a function unless you need to due to var scoping or necessity for any other reason
generally try to check conditions like this first
```
if not d or not v:
return False
elif isinstance(d, dict):
...
```
also, why does `len(str(d))` matter? should just check for `str(v)` if you're looking for nested value matches, right?
you should name vars here better. `d` can be a `dict` `list`, or assumed as a `value` compare candidate if not `dict/list/None` but it's very unclear without reading all of the code
same as above, don't define this function nested within another, and something more descriptive than `d` for var name
what do these tuples mean? can you add doc block?
as is, you have to look at the call site assigned var naming to understand this
isnt this just `int(api["id"]) <= int(target_api["id"])`? the reason is because we're checking that they're called in sequential order, right? i would add comment explaining that as well
you might as well just save these as a list of dicts with these keys, rather than store a list of tuples then reconstruct another dict by expanding the anonymous tuples. this is wasteful on space, harder to understand, and more fragile because it assumes anonymous tuple structure