-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Update screenshot collector to use existing screenshots, handle events automagically & update evals to all use collector #1437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Greptile SummaryOptimizes screenshot collection in hybrid mode by reusing screenshots already taken by the vision model agent, rather than taking redundant screenshots on every step. Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Agent as Agent (Hybrid Mode)
participant Handler as V3AgentHandler
participant Tool as Screenshot Tool
participant Bus as Event Bus
participant Collector as Screenshot Collector
Note over Agent,Collector: Hybrid Mode (Vision Model)
Agent->>Tool: Call screenshot tool
Tool->>Tool: Take screenshot
Tool-->>Agent: Return {base64, timestamp, pageUrl}
Agent->>Handler: Tool execution complete
Handler->>Handler: Intercept screenshot output
Handler->>Bus: Emit agent_screensot_taken_event
Bus->>Collector: Forward screenshot buffer
Note over Agent,Collector: DOM Mode (Non-Vision Model)
Agent->>Handler: Tool execution complete
Handler->>Handler: Check mode === "dom"
Handler->>Handler: captureAndEmitScreenshot()
Handler->>Tool: Take new screenshot
Tool-->>Handler: Return screenshot buffer
Handler->>Bus: Emit agent_screensot_taken_event
Bus->>Collector: Forward screenshot buffer
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, 1 comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 2 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/handlers/v3AgentHandler.ts">
<violation number="1" location="packages/core/lib/v3/handlers/v3AgentHandler.ts:173">
P1: Potential TypeError: `screenshotOutput` could be `undefined` due to the optional chaining, but `.base64` is accessed without a null check. This will crash if `toolResults[i]` or its `output` is missing.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/handlers/v3AgentHandler.ts">
<violation number="1" location="packages/core/lib/v3/handlers/v3AgentHandler.ts:509">
P1: This typo fix is incomplete - the listeners in `webvoyager.ts` and `onlineMind2Web.ts` still subscribe to the old typo `agent_screensot_taken_event`, and `v3CuaAgentHandler.ts` still emits the old typo. This change will silently break screenshot collection for evals using this handler.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
why
what changed
test plan
Summary by cubic
Use agent-provided screenshots in hybrid mode to stop duplicate captures and reduce eval overhead. The screenshot collector now auto-subscribes to agent events, and all evals were updated to use it.
Refactors
Bug Fixes
Written for commit d9d5e26. Summary will update automatically on new commits.