Conversation
|
@claude please review |
This comment was marked as outdated.
This comment was marked as outdated.
| stepMetrics.actionMs = Math.round(actionDuration); | ||
| if (!READ_ONLY_ACTIONS.has(action.type)) { | ||
| markDomSnapshotDirty(page); | ||
| } |
There was a problem hiding this comment.
Bug: Unnecessary DOM Cache Invalidation
The DOM cache is marked dirty immediately after runAction returns, regardless of whether the action succeeded or failed. If an action throws an exception or returns success: false, the cache is still invalidated even though no DOM mutation occurred. This causes unnecessary DOM re-fetches on the next step. The invalidation should only happen when actionOutput.success === true and the action is not read-only.
|
@claude review |
This comment was marked as outdated.
This comment was marked as outdated.
|
@claude can you review |
|
Claude finished @Dingway98's task —— View job Code Review CompleteI've reviewed the CDP layer implementation and related changes. This is an ambitious PR introducing a first-class CDP execution stack. Here's my comprehensive review: SummaryThis PR successfully introduces a robust CDP layer infrastructure that provides direct Chrome DevTools Protocol control alongside Playwright. The architecture is well-designed with proper separation of concerns, pooled sessions, and frame context management. However, there are several issues that should be addressed before merging. Critical Issues1. Listener Overwrite Bug in
|
| ); | ||
| if (domChunks) { | ||
| domState.domState = domChunks; | ||
| } |
There was a problem hiding this comment.
Bug: Incomplete Feature Leaves Dead Code
domChunks is declared as const with value null at line 292 and never reassigned. The conditional at line 355-357 that checks if (domChunks) will always be falsy, making this code unreachable. This appears to be dead code from an incomplete streaming implementation since captureDOMState already handles DOM aggregation internally.
Note
Introduces a full Chrome DevTools Protocol integration for element resolution and interactions, refactors agent flows to use CDP with perf/debug tooling, DOM caching/streaming, updated schemas/docs, and bumps to v1.0.0.
cdp/layer: frame/session management (FrameContextManager), element resolution, interactions, bounding boxes, Playwright adapter.ai()/aiAction()through CDP (performAction); add runtime context init and CDP fallbacks.backendNodeMap, iframe bbox; CDP-based AX/boxes; sync frames/contexts; visual overlay via CDP.actElementnow requireselementId/method/arguments/confidence; shared Playwright/CDP execution.thinking,wait; update prompts/examples; disable some default actions.captureDOMState).docs/cdp-overview.md; remove LangChain migration guide.1.0.0; upgrade@anthropic-ai/sdk; minor scripts and .gitignore tweaks.Written by Cursor Bugbot for commit 225b039. This will update automatically on new commits. Configure here.