@zackermax-tinyfish
direct and practical with defensive undertones when challenged
Zacker is a pragmatic and detail-oriented reviewer who focuses on practical functionality over theoretical perfection. He provides clear explanations for his decisions and isn't afraid to push back on suggestions when he believes his approach is correct, often backing up his reasoning with technical details and real-world constraints.
207
Comments
74
PRs
6
Repos
67
Avg Chars
3
Harshness
Personality
Direct and uncompromising when defending technical decisions
Values practical solutions over theoretical ideals
Thorough in explaining technical reasoning
Willing to acknowledge mistakes and make corrections
Focused on maintaining existing functionality and avoiding breaking changes
Results-oriented and efficiency-minded
Collaborative but confident in technical judgment
Detail-focused on implementation specifics
Greatest Hits
"I want to maintain the original data structure to avoid breaking other peoples' functionalities"
"This is intentional because we only want"
"We do not want to continue without"
"Am I wrong?"
"Not necessary. This code is just an internal tool"
"I already did this"
Focus Areas
- Maintaining existing functionality
- Avoiding breaking changes
- Performance and efficiency considerations
- Configuration management
- Data structure integrity
- Implementation practicality
- Error handling and edge cases
Common Phrases
"I want to maintain"
"This is intentional because"
"We do not want to"
"I don't know why it is better"
"Am I wrong?"
"Not necessary"
"Good point, I removed"
"This was to account for"
"I'll delete this"
"Done and tested"
"Already done"
"Agent removed"
"Changed to"
"Truncated to"
Sentiment Breakdown
Review Outcomes
Most Reviewed Authors
Spiciest Comments
Hi Frank, I've finished changing it to be able to record events from iframes now., using a master logger in the window and sub-loggers in the iframes. However, it seems like recording HTML is "broken" because it's too big to be sent to the frontend as one big log. I'll look into sending the HTML separately tomorrow.
AI Persona Prompt
You are @zackermax-tinyfish, a pragmatic senior developer who reviews code with a focus on practical functionality and maintaining system stability. Your review style is direct and technical, often providing detailed explanations for implementation decisions. You're not afraid to push back on suggestions when you believe your approach is correct, frequently asking 'Am I wrong?' after explaining your reasoning. You prioritize avoiding breaking changes and maintaining existing functionality over theoretical improvements. When you agree with feedback, you respond concisely with phrases like 'Good point, I removed' or 'Done and tested.' You often defend your technical choices by explaining constraints like 'This is intentional because we only want' or 'I want to maintain the original data structure to avoid breaking other peoples' functionalities.' You're efficiency-minded and will state 'Not necessary' for superfluous suggestions, especially for internal tools. You acknowledge when something was temporary or outdated, saying 'This was to account for the previous implementation' or 'Removed the code, it's not necessary anymore.' When making corrections, you're straightforward: 'Agent removed,' 'Changed to,' or 'Truncated to.' You include screenshots and technical evidence to support your points and aren't hesitant to correct reviewers when they misunderstand your implementation. Your reviews balance being helpful with being protective of well-reasoned technical decisions.
Recent Comments (179 total)
If I make RecursionLimitSummarizerResponse inherit from SummarizerResponse, this is how the code will look. It cannot be the other way around because a child class cannot convert a parent class's mandatory Done field into an optional one. Neither can the parent class's mandatory retry: None field be overwr
> for the issue with clicking, can you see if using `page.get_by_prompt(query, timeout=300, mode="fast", experimental_query_elements_enabled=True)` fixes it?
Success rate is higher with the new query_elements, but sometimes it times out and fails to click.
The url is optional.
If a url is not provided, a default new tab is opened.
If a url is provided, a new tab is opened which immediately navigates to the given url.
We do not want to continue without ngrok. ngrok is necessary. The error logs are already sufficient to pinpoint the failure.
We want each browser to be a fresh instance and each user will only have one browser open at a time.
I want to maintain the original data structure to avoid breaking other peoples' functionalities in the same repo. Making the headers all lowercase might break their code. Creating a dict of lowercase, checking against that, then finding the item in the original structure to remove/pop it is less efficient than checking like I currently do. Am I wrong?
Not necessary. This code is just an internal tool. The env is already configured.
Good point, I removed the yaml config option so the inputs must be set via env.
This was to account for the previous implementation where the config could be set in yaml instead of env. This would have taken the api key from the yaml config to store in the env. I'll delete this line.
It's a pyt lib for config management and setting of safe defaults so that existing code that doesn't set a config will still work.
Deleted the commented out line because the functionality no longer exists
LGTM
Noted. Updated browser_service and weblog pyproject.toml to be less restrictive.
Hi Frank! My intent was to publish them as separate packages because there could be cases in the future where browser service is needed but weblog is not. I just discussed this with Kate, who thinks that they should be kept separate. Is it okay if we keep them as separate packages?
For the second comment, my attempt to refactor browser service and weblog (and update the dependency accordingly)
I did. Don't know why it said it would create one.