Skip to content

Conversation

@seanmcguire12
Copy link
Member

@seanmcguire12 seanmcguire12 commented Dec 23, 2025

why

  • to add more coverage for captureHybridSnapshot() and the functions that it depends on

what changed

  • exported buildFrameContext(), buildSessionIndexes(), and collectPerFrameMaps() so their orchestration logic can be tested directly
  • added snapshot-capture-orchestration.test.ts, which holds tests for the following helpers:
    • buildFrameContext()
      • checks that walking a nested protocol frame tree records the correct DFS-ordered frame list and parent map (root, children, grandchildren)
    • buildSessionIndexes()
      • checks that multiple frames sharing the same CDP session trigger only a single buildSessionDomIndex() call (deduping DOM.getDocument)
      • checks that distinct sessions—including one without an id—each get their own index, with anonymous sessions stored under the "root" key
    • collectPerFrameMaps()
      • checks that when a shared session index exists, the helper reuses it, resolves iframe owners, builds relative xpath/tag/scroll maps, and captures per-frame outlines/URLs
      • checks that missing session indexes are built on demand, memoized, and produce encoded xpath entries rooted at the new frame’s document
    • captureHybridSnapshot()
      • checks that the scoped fast-path (focus selector) resolves DOM/AX data for the owning frame and returns early without touching session indexes
      • checks that the full multi-frame path builds session indexes, collects per-frame maps, computes prefixes, and merges outlines/xpath maps, yielding combined data for parent + child frames

test plan

  • this is it

Summary by cubic

Added unit tests for snapshot orchestration to improve coverage and prevent regressions. Exported buildFrameContext, buildSessionIndexes, and collectPerFrameMaps so their logic can be tested directly.

Written for commit 4785c7f. Summary will update automatically on new commits.

@changeset-bot
Copy link

changeset-bot bot commented Dec 23, 2025

⚠️ No Changeset found

Latest commit: 4785c7f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a 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 2 files

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 23, 2025

Greptile Summary

Exported three internal helper functions (buildFrameContext, buildSessionIndexes, collectPerFrameMaps) to enable direct unit testing of snapshot capture orchestration logic.

  • Added comprehensive test suite covering frame context indexing, session deduplication, per-frame map collection, and both scoped and full snapshot flows
  • Tests verify DFS ordering of frame trees, proper parent-child relationships, session index memoization, and iframe owner resolution
  • Mock infrastructure properly simulates CDP sessions and page interactions without requiring actual browser context
  • All tests use proper spies to verify function call counts and validate orchestration behavior

Confidence Score: 5/5

  • This PR is safe to merge with no risks identified
  • The changes are minimal and focused: only adding export keywords to three existing functions and creating a comprehensive test suite. The exported functions retain their original implementation, and the tests properly mock all dependencies. No logic changes, no breaking changes, and the tests demonstrate thorough understanding of the snapshot capture flow.
  • No files require special attention

Important Files Changed

Filename Overview
packages/core/lib/v3/understudy/a11y/snapshot/capture.ts Exported three helper functions (buildFrameContext, buildSessionIndexes, collectPerFrameMaps) by adding export keywords to enable unit testing
packages/core/tests/snapshot-capture-orchestration.test.ts Added comprehensive test coverage for snapshot capture orchestration, including frame context building, session index deduplication, per-frame map collection, and full snapshot flow with proper mocking

Sequence Diagram

sequenceDiagram
    participant Test as Test Suite
    participant Capture as capture.ts
    participant DomTree as domTree module
    participant A11yTree as a11yTree module
    participant Page as Page Mock
    participant Session as CDP Session
    
    Note over Test,Session: buildFrameContext Test
    Test->>Page: asProtocolFrameTree()
    Page-->>Test: frameTree structure
    Test->>Capture: buildFrameContext(page)
    Capture->>Page: mainFrameId(), listAllFrameIds()
    Capture-->>Test: FrameContext with parentByFrame map
    
    Note over Test,Session: buildSessionIndexes Test
    Test->>Capture: buildSessionIndexes(page, frames, pierce)
    loop for each unique session
        Capture->>Page: getSessionForFrame(frameId)
        Page-->>Capture: session
        Capture->>DomTree: buildSessionDomIndex(session, pierce)
        DomTree-->>Capture: SessionDomIndex
    end
    Capture-->>Test: Map<sessionId, SessionDomIndex>
    
    Note over Test,Session: collectPerFrameMaps Test
    Test->>Capture: collectPerFrameMaps(page, context, sessionToIndex, options, pierce)
    loop for each frame
        Capture->>Page: getSessionForFrame(frameId)
        Capture->>Session: send("DOM.getFrameOwner", {frameId})
        Session-->>Capture: {backendNodeId}
        Capture->>A11yTree: a11yForFrame(session, frameId, options)
        A11yTree-->>Capture: {outline, urlMap}
    end
    Capture-->>Test: {perFrameMaps, perFrameOutlines}
    
    Note over Test,Session: captureHybridSnapshot Test (scoped)
    Test->>Capture: captureHybridSnapshot(page, {focusSelector})
    Capture->>DomTree: domMapsForSession(session, frameId, pierce)
    DomTree-->>Capture: {tagNameMap, xpathMap, scrollableMap}
    Capture->>A11yTree: a11yForFrame(session, frameId, options)
    A11yTree-->>Capture: {outline, urlMap, scopeApplied: true}
    Capture-->>Test: HybridSnapshot (early return)
    
    Note over Test,Session: captureHybridSnapshot Test (full)
    Test->>Capture: captureHybridSnapshot(page)
    Capture->>Capture: buildFrameContext(page)
    Capture->>Capture: buildSessionIndexes(page, frames, pierce)
    Capture->>Capture: collectPerFrameMaps(...)
    Capture->>Capture: mergeFramesIntoSnapshot(...)
    Capture-->>Test: Complete HybridSnapshot with all frames
Loading

@seanmcguire12 seanmcguire12 merged commit 8af65fa into main Dec 24, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants