feat: snapshot diffing AUTO-40#154
Merged
Merged
Conversation
Contributor
WalkthroughAdds diff-based snapshot rendering with cached prior snapshot state, a ChangesSnapshot Diff Feature
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant browserless_agent
participant agent-format.ts
participant agentSession
browserless_agent->>agent-format.ts: formatSnapshotDiff(snapshot, lastElements)
browserless_agent->>agent-format.ts: formatSnapshotDiffPositional(lastSnapshotElements, snapshot)
agent-format.ts-->>browserless_agent: candidate snapshot texts
browserless_agent->>browserless_agent: choose shortest result or full snapshot
browserless_agent->>agentSession: store lastElements, lastSnapshotElements
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/agent-format.ts`:
- Around line 291-309: The changed-element delta in agent-format.ts is missing
the actual field-level reason for updates detected by elementSignature, so a `~`
line can appear without showing what changed. Update the change rendering in the
delta builder that uses elementSignature, formatElement, and changeLines to
include the previous vs new values for the relevant fields (such as placeholder,
href, ariaLabel, type, and frameId) when an element is marked as changed. Keep
the existing add/remove behavior, but make the `changed` output explicitly
surface the difference so the formatted diff matches the detection logic.
- Around line 241-246: The current indexByIdentity helper uses Map construction,
which overwrites duplicate semantic keys and loses snapshot elements with the
same identity. Update indexByIdentity (and the related comparison logic around
the duplicate handling in agent-format.ts) to preserve all elements for a given
elementKey instead of last-wins, so repeated buttons/links are retained and can
be compared correctly. Use the existing elementKey and
SnapshotResult/SnapshotElement symbols to locate the affected code and adjust
any consumers that assume a single SnapshotElement per key.
- Around line 255-280: The diff snapshot output built by assembleDiff is missing
the frame legend and deep-ref guidance that full snapshots include, so agents
can see [frame#N] markers without the mapping. Update assembleDiff to carry over
the same frame-context metadata used in the full snapshot path, including the
frame legend and deep-ref guidance, and append it alongside the existing counts
and detected challenge lines.
In `@src/tools/agent.ts`:
- Around line 715-742: The snapshot diff logic in agentSession is using shared
caches across tab/target contexts, so peeks like snapshot { targetId } or tab
switches can diff against the wrong prior snapshot and then overwrite it. Update
the cache usage around formatSnapshotDiff/formatSnapshotDiffPositional so
lastElements and lastSnapshotElements are scoped by target/active target (or
bypass diffing and cache updates when the target context changes), and keep the
existing full snapshot fallback in agent.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c31376f-57e5-42b8-8095-c0d9176678df
📒 Files selected for processing (6)
src/@types/types.d.tssrc/lib/agent-format.tssrc/skills/system-prompt.tssrc/tools/agent.tssrc/tools/schemas.tstest/lib/agent-format.diff.spec.ts
…ty keys and include frame legend
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/lib/agent-format.diff.spec.ts (1)
57-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest only checks the summary counts, not the actual removal line.
This test verifies the "1 removed" count but doesn't assert the rendered removal line itself (e.g. that the correct selector shows up in a
- ref=... (removed)line). Since the whole point of this test is that removal isn't hidden by key collision, asserting the actual diff line content would make the test more robust against regressions where counts are right but the wrong element gets flagged.Suggested strengthening
const out = formatSnapshotDiff(snap([el(dup)]), prev); expect(out).to.match(/0 new, 0 changed, 1 removed/); + expect(out).to.match(/- ref=.*\(removed\)/);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/lib/agent-format.diff.spec.ts` around lines 57 - 63, The current test for formatSnapshotDiff only asserts the summary count and can miss regressions where the removed element is not rendered correctly. Strengthen the spec in agent-format.diff.spec.ts around the duplicate-key case by also asserting the actual removal line emitted by formatSnapshotDiff, using the same dup/snap/indexByIdentity setup and checking for the expected “- ref=... (removed)” content tied to the removed element.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/lib/agent-format.diff.spec.ts`:
- Around line 57-63: The current test for formatSnapshotDiff only asserts the
summary count and can miss regressions where the removed element is not rendered
correctly. Strengthen the spec in agent-format.diff.spec.ts around the
duplicate-key case by also asserting the actual removal line emitted by
formatSnapshotDiff, using the same dup/snap/indexByIdentity setup and checking
for the expected “- ref=... (removed)” content tied to the removed element.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6b8a5b3-eeb9-4f46-912e-33aea87d32c8
📒 Files selected for processing (2)
src/lib/agent-format.tstest/lib/agent-format.diff.spec.ts
…rovide detailed change reasons
ahmedAmien
approved these changes
Jul 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds snapshot diffing for save tokens, allowing agents to request a full snapshot if needed
Related issues
Closes AUTO-40
Summary by CodeRabbit
fullparameter to request a complete snapshot when prior context isn’t available.