-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Chromie/add page snapshot method #1487
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?
Chromie/add page snapshot method #1487
Conversation
This test validates the new page.snapshot() method that exposes captureHybridSnapshot functionality publicly, allowing users to capture a hybrid DOM + Accessibility snapshot of the page. Tests include: - Basic snapshot with combined tree and maps - Scoped snapshot with CSS focusSelector - Scoped snapshot with XPath focusSelector - Per-frame data availability 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Adds a new public page.snapshot() method that exposes the internal captureHybridSnapshot functionality, allowing users to capture a hybrid DOM + Accessibility snapshot of the page. The method returns a HybridSnapshot containing: - combinedTree: Merged outline across every frame - combinedXpathMap: EncodedId -> absolute XPath mapping - combinedUrlMap: EncodedId -> URL extracted from AX properties - perFrame: Per-frame payloads with original relative data Options include: - focusSelector: Filter to a specific element/subtree (XPath or CSS) - pierceShadow: Pierce shadow DOM (default: true) - experimental: Enable experimental AX traversal tweaks Also exports HybridSnapshot, SnapshotOptions, and PerFrameSnapshot types publicly for TypeScript users. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
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 3 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/tests/page-snapshot.spec.ts">
<violation number="1" location="packages/core/lib/v3/tests/page-snapshot.spec.ts:83">
P2: Test verifies scoped content is present but doesn't verify that out-of-scope content (`#outside`) is excluded. Add a negative assertion to actually test the scoping behavior, otherwise this test provides false confidence.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| expect(scopedSnapshot).toBeDefined(); | ||
| expect(scopedSnapshot.combinedTree).toContain("Main Heading"); | ||
| expect(scopedSnapshot.combinedTree).toContain("Main paragraph"); |
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.
P2: Test verifies scoped content is present but doesn't verify that out-of-scope content (#outside) is excluded. Add a negative assertion to actually test the scoping behavior, otherwise this test provides false confidence.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/tests/page-snapshot.spec.ts, line 83:
<comment>Test verifies scoped content is present but doesn't verify that out-of-scope content (`#outside`) is excluded. Add a negative assertion to actually test the scoping behavior, otherwise this test provides false confidence.</comment>
<file context>
@@ -0,0 +1,138 @@
+
+ expect(scopedSnapshot).toBeDefined();
+ expect(scopedSnapshot.combinedTree).toContain("Main Heading");
+ expect(scopedSnapshot.combinedTree).toContain("Main paragraph");
+ });
+
</file context>
Greptile SummaryExposes the internal Key changes:
This exposes the same snapshot format used internally by Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Page
participant captureHybridSnapshot
participant tryScopedSnapshot
participant buildSessionIndexes
participant collectPerFrameMaps
participant computeFramePrefixes
participant mergeFramesIntoSnapshot
User->>Page: page.snapshot(options?)
Page->>captureHybridSnapshot: captureHybridSnapshot(page, options)
alt focusSelector provided
captureHybridSnapshot->>tryScopedSnapshot: attempt scoped snapshot
alt selector resolves
tryScopedSnapshot-->>captureHybridSnapshot: HybridSnapshot (scoped)
captureHybridSnapshot-->>Page: HybridSnapshot
Page-->>User: snapshot result
else selector fails
tryScopedSnapshot-->>captureHybridSnapshot: null (fallback)
end
end
alt full snapshot (no focusSelector or fallback)
captureHybridSnapshot->>buildSessionIndexes: build DOM indexes per session
buildSessionIndexes-->>captureHybridSnapshot: sessionToIndex map
captureHybridSnapshot->>collectPerFrameMaps: collect per-frame DOM & A11y data
collectPerFrameMaps-->>captureHybridSnapshot: perFrameMaps, perFrameOutlines
captureHybridSnapshot->>computeFramePrefixes: compute absolute XPath prefixes
computeFramePrefixes-->>captureHybridSnapshot: absPrefix, iframeHostEncByChild
captureHybridSnapshot->>mergeFramesIntoSnapshot: merge all frames
mergeFramesIntoSnapshot-->>captureHybridSnapshot: HybridSnapshot
captureHybridSnapshot-->>Page: HybridSnapshot
Page-->>User: snapshot result
end
|
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
|
@chromiebot can we add docs to the relevant section for page.snapshot on the v3 reference docs |
Adds comprehensive documentation for the page.snapshot() method to the v3 page reference docs, including: - Method signature and description - All SnapshotOptions parameters (focusSelector, pierceShadow, experimental) - Return type documentation (HybridSnapshot, PerFrameSnapshot) - Example usage code - Type definitions in the Types section 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
@chromiebot simplify the docs changes for page.snapshot to match the overall level of detail in the v3 page reference docs |
Reduce verbosity of page.snapshot() docs to match the overall level of detail in the v3 page reference documentation: - Shortened method description - Simplified parameter descriptions - Removed experimental parameter documentation - Removed inline interface definitions (kept in Types section) - Removed separate example code block - Simplified type definitions in Types section 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
why
what changed
test plan
Summary by cubic
Adds a public Page.snapshot() to capture a hybrid DOM + Accessibility snapshot. Returns a combined tree with XPath and URL maps, plus per-frame data; includes tests covering scoping and frame payloads.
Written for commit de6bd5f. Summary will update on new commits.