-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Refactor screenshot masking to reduce code duplication #1532
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
Extract selectorsToLocators() and DEFAULT_MASK_COLOR constant to shared screenshotUtils.ts. Unify ToolMaskConfig as a type alias for AgentMaskConfig. Extract v3CuaAgentHandler screenshot logic into reusable helper methods (captureScreenshotWithMask and updateScreenshotDimensions) to eliminate duplication between setupAgentClient and captureAndSendScreenshot. Co-Authored-By: Claude Haiku 4.5 <[email protected]>
|
Greptile SummaryThis PR successfully consolidates screenshot masking functionality by extracting shared utilities ( Key improvements:
Minor observation:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Agent as Agent Handler
participant Tool as Vision Tool
participant Handler as Screenshot Handler
participant Utils as Screenshot Utils
participant Page as Browser Page
Agent->>Agent: Set maskConfig from execute options
Agent->>Tool: Pass maskConfig to tool
Tool->>Handler: Call waitAndCaptureScreenshot(page, options)
Handler->>Utils: selectorsToLocators(page, maskConfig.selectors)
Utils-->>Handler: Return Locator[]
Handler->>Utils: applyMaskOverlays(locators, color)
Utils->>Page: Inject mask overlay elements
Utils-->>Handler: Return cleanup function
Handler->>Page: page.screenshot()
Page-->>Handler: Buffer
Handler->>Utils: runScreenshotCleanups(cleanupTasks)
Utils->>Page: Remove mask overlay elements
Handler-->>Tool: base64 screenshot
Tool-->>Agent: Tool result with screenshot
|
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.
15 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.
No issues found across 15 files
- Remove unnecessary ToolMaskConfig type alias, use AgentMaskConfig directly - Remove unused maskConfig parameter from clickAndHold tool - Centralize mask logic into captureWithMask helper in screenshotUtils - Simplify screenshot tool, screenshotHandler, and CUA handler to use shared helper Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Change ScreenshotCaptureOptions to accept { mask: MaskConfig } directly
- Remove verbose { maskSelectors, maskColor } spread pattern from tools
- Reduces boilerplate in click, type, scroll, wait, dragAndDrop, fillFormVision
Co-Authored-By: Claude Opus 4.5 <[email protected]>
captureAndEmitScreenshot in v3AgentHandler was not applying mask overlays, unlike the equivalent in v3CuaAgentHandler. This ensures consistency. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary
Eliminates code duplication in screenshot masking functionality by extracting shared utilities and consolidating helper methods:
selectorsToLocators()andDEFAULT_MASK_COLORto sharedscreenshotUtils.tsToolMaskConfigas a type alias forAgentMaskConfigV3CuaAgentHandlerinto reusable helpersTest Plan
🤖 Generated with Claude Code
Summary by cubic
Unifies screenshot masking behind shared helpers and a single mask config, removing duplicated logic across tools and handlers. Defaults stay the same; masking is optional and off unless provided.
Written for commit c1d9ad6. Summary will update on new commits.