Repository navigation
Conversation
Replay recording fixtures used `tag: "ui.click"`, a shape the browser SDK never emits. Real user actions arrive as rrweb custom events with `tag: "breadcrumb"`, where the meaning lives in `payload.category`. Because the fixtures matched what the classifier expected rather than what Sentry sends, snapshots looked correct while real output degraded. Rebuild the segment fixtures from the recording spec and the `@sentry-internal/replay` frame types: breadcrumbs carry millisecond timestamps, performance spans carry seconds, and the set covers ui.click, a console error, a navigation breadcrumb, a navigation span, fetches at 200 and 500, options, a script resource, and two ui.slowClickDetected events that differ only by clickCount so rage and dead clicks are separable behaviorally. Reconcile the replay metadata counts with the recording. Upstream sets click_is_dead for both DEAD_CLICK and RAGE_CLICK, and count_dead_clicks sums that column, so one rage click plus one dead click is 2 dead and 1 rage. Add fixtures and handlers for endpoints the tools do not read yet: a multi-page recording served through a Link header cursor, replays-events-meta, and the experimental Seer summarize endpoint. Cover them with contract tests so the shapes cannot drift before the tools arrive. The get_replay_details snapshots are re-baselined against these fixtures and now record known-wrong output: every user action is labeled `breadcrumb`, four of six lines are session-boot noise, and the console error, failed checkout request, rage click, and dead click are dropped by the event cap. The snapshots are annotated as such so the taxonomy fix lands as a visible diff rather than a rewrite. Refs docs/specs/replay-review.md Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Document the map-then-zoom restructuring of replay retrieval: get_replay_details returns session shape instead of a fixed six-event prose sample, a new catalog-only get_replay_activity returns signals for a requested window and grain, and classification follows Sentry's own replay event taxonomy so MCP and Seer agree on what a session contains. Also records the adjacent correctness fixes the work depends on: segment pagination through the Link header cursor, per-event-type timestamp units, truncation reporting, replay sort reconciliation, capability gating on the replays search path, and distinguishing replay-count rate limiting from absence. Section 1 of tasks.md is checked off by the preceding commit, which rebuilt the fixtures from real SDK event shapes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replay events were classified by their rrweb `data.tag`, but the SDK emits every user action with `tag: "breadcrumb"` and puts the meaning in `payload.category`. Clicks, console errors, and rage clicks therefore all rendered under the literal label `breadcrumb`, with their real content demoted into a details blob. Add a shared internal module that ports Sentry's own taxonomy from `replays/usecases/ingest/event_parser.py` and `summarize.py`, so MCP output and Seer's summarizer agree about what a session contains: - Classify breadcrumbs by `payload.category` and spans by `op`, matching `which()`. Unrecognized events are `unknown` rather than guessed at. - Resolve timestamps by event type. Spans and web vitals are seconds; clicks, console, and navigation are milliseconds. The previous magnitude heuristic held only by coincidence of present-day epochs. - Drop the noise upstream refuses to narrate, and skip 2xx requests from the rendered set while still counting them, so `network 58 (2 failed)` means 58 requests of which 2 were narrated. - Prefer the navigation span on web and the navigation breadcrumb on mobile, where no span exists. - Separate dead, rage, and slow clicks behaviorally: all three arrive as `ui.slowClickDetected` and differ only by end reason, target element, stall duration, and click count. - Measure offsets from the replay's `started_at` rather than the first recorded event, so they align with replay metadata and error timestamps. Render at three grains, and label unavailable payload rather than passing over it in silence: bodies the SDK never captured are marked not captured, and only Relay's `[Filtered]` marker is reported as redacted. Client-side SDK masking leaves no marker, so masked values render as delivered instead of asserting a redaction we cannot detect. Widen the recording payload schema to carry the fields classification needs. Parsing stays permissive: recordings span many SDK versions and pass through PII scrubbing, which can replace a number with a marker string, so a malformed field degrades that field rather than dropping the event. Against the rebuilt fixtures this surfaces the failed checkout request, the console error, the rage click, and the dead click — all four of which the previous six-event cap dropped before reaching them. Refs docs/specs/replay-review.md Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The recording segments index was requested with `?download=true`, a parameter that does not exist on that endpoint, while the `Link` header's cursor was ignored. Sentry serves at most 100 segments per page — `per_page` cannot raise that, since its default and maximum are both 100 — so any replay longer than 100 segments was silently truncated to its first page. Follow the cursor, and bound the read twice over. Segments are capped at 150, matching the clamp Sentry's own summarize endpoint applies, and raw JSON at 10MB measured before parsing. The byte ceiling is the real guard: segment sizes vary by orders of magnitude, parsed rrweb objects expand well beyond their serialized size, and mcp-cloudflare runs under a 128MB Workers limit. Both bounds are provisional pending QA against real replays. The caller is told which bound stopped the read, and `get_replay_details` now says so rather than presenting a partial recording as a whole session. Add two endpoints the map will need: - `getReplayErrorEvents` resolves a replay's error ids through `replays-events-meta` in one batched call. Error ids are event ids, and resolving them through `listIssues` returns issues, which carry no event timestamp; this returns issue identity and a millisecond-precision timestamp together, so it also replaces the per-error issue lookups. The endpoint is PRIVATE, so its response is parsed defensively. - `getReplaySummary` reads Seer's replay summary state. Deliberately one GET: no start request, no retry loop. Seer's start route enqueues background work and returns an empty body, so starting would spend an LLM run per call and still report `processing` on the immediate read. Send an explicit `field` allow-list on the replays index, drawn from `VALID_FIELD_SET`. Omitting it returns Sentry's default column set, which is wider than anything rendered. `replay_type` and `ota_updates` appear in responses but are absent from that set and would 400, so they are excluded. Refs docs/specs/replay-review.md Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replay details returned a six-event prose sample, which either truncates a long session or floods a short one, and in neither case tells the reader where to look. The cap was routinely spent on session-boot noise before reaching the failure the user cared about. Return the shape of the session instead: signal counts and time span, the page flow, and a per-kind breakdown carrying error, rage, and dead click counts. Counts come from every classified event including ones that are never rendered, so `network 2 (1 failed)` describes two requests of which one is shown. Truncation is stated rather than implied. Add a suggested next call windowed on the replay's own first resolvable error, so the reader gets a concrete zoom target instead of guessing. Error timestamps come from one batched `replays-events-meta` lookup, which also replaces the per-error `listIssues` calls behind the Related section — up to three sequential requests become one. Related entries are still driven by `replay.error_ids`, so an id that private endpoint cannot resolve is listed by id rather than disappearing. When no timestamp resolves, the suggestion falls back to a whole-session digest and the map is unaffected. Render Sentry's AI summary as an optional Chapters section, read once. It is strictly additive: a 403, a server failure, a still-running or not-started task, an unparseable body, or a non-completed status carrying stale chapter data all omit the section and leave the map intact. Status decides, not the presence of data, so a superseded run cannot present itself as current. Report how many related issues and traces were omitted; both previously stopped at their display limit in silence. Delete the old classifier, which keyed on a `data.tag` shape the SDK never emits. Its snapshots recorded that defect deliberately; they now record correct output, and two tests that pinned the old rendering are rewritten to assert what replaced it. Refs docs/specs/replay-review.md Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The replay map tells an agent where to look but gives it no way to look there. Add a catalog-only tool that returns the signals in a time window at a requested level of detail, completing the map-then-zoom pair: the map's suggested call now resolves. Windowing is by `startMs`/`endMs` measured from the replay's `started_at`, defaulting to the whole session. Bounds are inclusive. A signal whose timestamp could not be resolved is excluded from a windowed read rather than guessed into one, and stays available in a whole-session read. `grain` controls rendering and `kinds` controls inclusion, kept orthogonal so a digest of a filtered window stays consistent with the same window read at standard grain. Paging is by `limit`/`cursor`. Sentry paginates recording segments rather than signals, so there is no server-side signal cursor to pass through: each page re-reads the recording and skips a signal offset. The cursor is therefore synthetic and encodes the window and kind filter alongside that offset, so a continuation resumes the same query rather than a differently-filtered one. It also overrides any window or filter passed beside it, since honouring both would silently change what the caller is paging through. Truncation always reports the cursor needed to continue, and a partial recording read is reported separately from a paged result. Extract replay parameter resolution and the project-constraint check into a shared helper so the two replay tools cannot disagree about what a valid replay reference is, or about which replays a constrained session may read. The direct tool surface is unchanged: this is catalog-only, and `measure-tokens` still reports 5,598 tokens across 9 direct tools. Refs docs/specs/replay-review.md Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ailures Four correctness fixes on the replay paths outside the two replay tools. Replay sorting drifted from Sentry's `sort_config`, which is the authority for both the scalar and aggregated query paths and raises a ParseError for anything absent from it. Add the four sorts it supports that we omitted: `count_screens` and the aliases `browser`, `os`, and `os_name`. A test now asserts exact parity with upstream's 29 keys, in both directions — a sort we advertise but Sentry rejects fails at query time, and one Sentry supports but we omit is simply unreachable. Field discovery had no notion of sortability at all, so it presented every searchable replay field as if a sort on it would work. Most replay fields are filterable but not sortable, which meant an agent could take a sort straight from discovery output and get a UserInputError. Replay fields now carry a `sortable` flag derived from the same allow-list, and the routing prompt tells the agent to respect it. `search_events` served the replays dataset regardless of whether the constrained project had Session Replay, so replay search and replay details disagreed about availability. Reject it at handler time, checking the resolved dataset so an agent-chosen route is caught as well as an explicit request, and drop `replays` from the advertised dataset options so the router cannot pick what would be rejected. The schema narrowing goes through a new general `refineInputSchema` hook rather than putting replay logic in shared infrastructure: `requiredCapabilities` gates a tool as a whole, and search_events serves six datasets of which only one needs replays. `listReplayIdsForIssue` swallowed every failure with `.catch(() => undefined)`. Because `replay-count` is rate limited per organization and `get_issue_details` calls it on every lookup, parallel issue triage silently lost the Session Replay section — a throttled request was indistinguishable from an issue with no replays. Report the lookup as unavailable instead, both when nothing else is known and when an attached replay was found but related ones could not be. Refs docs/specs/replay-review.md Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Section 7 of the replay review change. The gaps that remained were the ones mocks make easy to skip: paths that render nothing, where a passing test and a silently broken one look identical. Cover the two truncation paths end to end. A recording spanning three segment pages is only complete if both `Link` cursors are followed, and the byte budget is tripped with a real oversized page rather than by injecting a bound, so the "Truncated" line is driven by the same code that runs in production. Silence here would present a partial recording as a whole one, turning "no errors in this replay" into a false negative. Extend summary degradation to a connection failure and a non-JSON body. Seer takes tens of seconds on a long replay, and a timeout raises a ConfigurationError that would abort the whole tool if it escaped the chapters lookup. Assert redaction survives to tool output. `<not captured>` and `<redacted>` mean different things — enable networkCaptureBodies, or relax a scrubbing rule — so they appear on one signal where conflating them would fail. Cover capability gating, which no tool tested. Verified by mutation: stubbing the check out fails the replays-absent case. Add a replay eval for map-then-zoom. It executes against the mocks, so the zoom can only pass by reading the window off the map. Also reset MSW handlers between replay detail tests; overrides are not reset automatically here, and a leaked stub changes what a later test measures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running the eval showed both cases scoring 0.5 while the model's answers were correct — it found the 500, the console error, and the uncaptured body sizes. The spec was wrong, not the behavior. Two problems. The eval required routing to the map through search_sentry_tools, but get_sentry_resource is a top-level tool that delegates to get_replay_details, so a pasted replay URL reaches the map without catalog discovery. Both routes are correct, and asserting one scores a routing preference rather than the handoff under test. The first case also asserted a map read with nothing to hand off to, which get_sentry_resource already covers. Assert only the zoom, and let allowExtras absorb whichever route the model takes to the map. The zoom is the real subject: its offsets are milliseconds from the start of the replay and appear nowhere in the prompt, so passing means the map's suggested window was read and followed. Now scores 1.00. Mutating the expected startMs drops it to 0.5, confirming the offsets are load-bearing rather than incidentally matched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ts spec Section 8 of the replay review change. The spec was written before the work and read as though it still described pending behavior, so mark it implemented and record where the shipped code took a different path. Keep the Motivation section in its original present tense. It is the record of what was wrong and why the work was justified; rewriting it to past tense would lose the evidence and leave assertions no one can check. Correct the Map example. Offsets render in a consistent `T+` form rather than mixing bare seconds, and the page count is gone from the Signals line, where a count beside a time span read as something temporal. Add a divergences section covering the seven places the implementation differs: discovery gained a sortability flag rather than losing fields, dataset narrowing uses a general `refineInputSchema` hook instead of replay-specific logic in shared infrastructure, both replay tools share extracted parameter resolution, a cursor overrides arguments passed beside it, unresolvable error ids are listed rather than dropped, `count_dead_clicks` includes rage clicks upstream, and the segment bounds remain unmeasured against real replays. Also correct the two upstream fix references. The descriptions of #120859 and #121765 were swapped in the task notes; verified against the commits in getsentry/sentry and confirmed both behaviors are implemented and tested here. No other doc describes replay tool output. Generated definitions were already current. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Section 9's automated portion. Targeted replay and catalog tests (167 across 9 files), tsc, lint, full suite (1481 in mcp-core), and token measurement all pass. Token cost is unchanged at 5,598 across 9 direct tools, since get_replay_activity is catalog-only. The remaining task, QA against a real organization, needs a Sentry token and a long real replay; it cannot be satisfied from mocks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Out of Scope section ruled out "web visual snapshots" on the grounds that rendering needs a browser. That is right about images and wrong as a blanket statement: reading DOM structure needs no layout engine, and the structure is already in segments this change downloads and discards. Record what is actually reachable. rrweb FullSnapshot (type 2) carries the serialized DOM, and IncrementalSnapshot (type 3, source 0) carries the mutations to replay it forward to a timestamp. Upstream's which() ignores both, so matching it lost nothing for behavioral summarization, but it leaves structure on the floor. Rooting is close to free: click breadcrumbs already carry payload.data.node.id, and rrweb ids are stable within a recording, so the identifier a subtree read needs is one get_replay_activity already surfaces. Bounding boxes and a visible lens stay out, for the same reason images do — both are layout properties, not recorded facts. An interactive lens is decidable from tags and attributes alone. Two constraints are recorded because they shape any implementation: replaying mutations to a timestamp cannot reuse the existing segment reader without blowing the Workers memory ceiling, and a full tree exceeds any reasonable response size, so rooting plus a lens is the default rather than an option. Also note that SDK masking defaults to on and leaves no marker, so tree text must render as delivered rather than be claimed as redacted — consistent with the redaction rule this spec already sets. Verified against rrweb's type definitions and Sentry's pinned EventType enum rather than from memory; both are now cited. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The prior entry established that tree reads are reachable. This works out what a correct one requires, and surfaces two problems the sketch had glossed over. Input values arrive as rrweb source 5 (Input), not as attribute mutations. An implementation applying only source 0 renders every field at its initial value — a stale tree that looks authoritative, which is worse than no tree. Called out explicitly and given a test case, since it would otherwise ship silently. Reconstruction cost is not known to be bounded. rrweb re-snapshots on checkoutEveryNms, and Sentry's SDK treats a recording's first event as a checkout, so a read at time T may only need the nearest preceding snapshot — or, if a recording carries exactly one, may have to apply every mutation in the session. Those two cases have different feature shapes, and building for the wrong one yields a tool that works on short replays and fails on the long ones that matter. Promoted to the first question QA should answer. Also record that element nodes carry no textContent (a label is a child text node, not a property), that reconstruction fidelity must be reported rather than assumed, and that a visible lens is out for the same reason boxes are — both need the cascade resolved. Argue the tool surface three ways and recommend a separate catalog tool: a point-in-time structural read has a different return shape and cost model than a windowed signal list, and only a separate tool can refuse on budget grounds without complicating a contract agents call routinely. Verified against rrweb's type definitions and the Sentry SDK's recording emit handler rather than from memory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Turn the three-way comparison into a decision. get_replay_dom is a separate catalog-only tool: a point-in-time structural read differs from a windowed signal list in return shape, cost model, and failure modes, and it is the only option where a read can refuse on budget grounds without complicating a contract agents call routinely. Keep the rejected options as a short record so the choice is not relitigated from scratch, and note why a third replay tool is affordable: the ≤20 target governs the direct surface, which stands at 9, while catalog tools carry no per-session token overhead. Two consequences are written down because they are easy to drop under implementation pressure. Refusal is a supported outcome rather than a failure, since a partial tree is indistinguishable from a complete one at the point of use. And the tool returns structure only, so it composes with get_replay_activity instead of drifting into a competing account of the same moment. Gating, parameter resolution, and the constraint check follow the existing replay tools; internal/tool-helpers/replay.ts was extracted for this case when the second one landed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
DOM reconstruction folds a whole recording into a running node map and keeps only the result. The existing reader accumulates every segment before returning, so a tree read would hold the raw events and the derived state at once — double the peak for no benefit, under a 128MB Workers ceiling that the byte budget already exists to respect. Add streamReplayRecordingSegments, which invokes a callback per segment in wire order and retains nothing. Returning "stop" ends the read immediately, so a caller that has passed the moment it cares about does not page the rest of the session. Stopping by choice is reported as truncatedBy: null. Conflating it with a budget stop would make a deliberate early exit look like a lost tail. getReplayRecordingSegments becomes a thin wrapper that pushes into an array, so signal extraction is unchanged — its 111 existing tests pass untouched. A test asserts the two agree on segments, count, and bytes, since any drift between them would now be a bug in the wrapper. The stop path is mutation-verified: forcing the branch false fails the early-exit test rather than passing quietly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the fold behind a DOM read: a flat node map built from a FullSnapshot (type 2) and advanced by IncrementalSnapshot (type 3) mutations up to a target time. Semantics come from rrweb's own type definitions rather than from the SDK, which does not re-export them. Input values are applied from source 5, not from attribute mutations. This is the failure worth naming: a source 0-only fold renders every form field at the value the page shipped with, so the tree looks current and reports stale content. The initial value still comes from the attribute, and the two are kept separate. A later FullSnapshot supersedes the map entirely rather than merging. Merging would resurrect nodes the page had already discarded, and it makes the one-snapshot and periodic-checkout cases behave identically without branching on which a recording uses. An add whose parentId is unknown is dropped and counted, never reparented to the root: fabricated structure is indistinguishable from recorded structure once rendered. Removes delete the subtree, so a later add cannot reattach orphans under a parent that no longer exists, and a relocation detaches before inserting so a moved node does not appear twice. Fidelity is reported, not assumed. Drops are counted by reason (unknown-parent, unknown-node, malformed, duplicate-id), a missing snapshot is distinguished from an empty page, and apply() returns past-target so a streaming caller stops paging at the moment it cares about. Subtree deletion is iterative — untrusted DOM depth should not reach the call stack. Five mutations of the risky paths each fail a test: dropping source 5, reparenting unknown parents, skipping subtree deletion, ignoring nextId ordering, and merging snapshots instead of superseding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the rendering half: selector, text, state attributes, and node id per element, with connectors showing structure. Two lenses, and deliberately not a third. `interactive` keeps what a user can act on — interactive tags, plus anything carrying role, href, onclick, or tabindex — and the ancestors needed to place them, so a button is never shown floating without a path. `full` keeps every element. There is no `visible` lens: visibility is a computed style, and a lens that guessed at the cascade would be confidently wrong about the thing a reader most wants to trust. Text comes from immediate text children only. rrweb element nodes carry no textContent, so a renderer that ignores children shows no labels at all, and one that walks descendants attributes a child's label to its container. Values render as recorded. SDK masking leaves no marker, so a masked field is indistinguishable from a real one, and claiming <redacted> would assert something the recording cannot support — consistent with the rule the signal renderer already follows. The current input value is shown rather than the value attribute, since input events supersede it. A rootNodeId that is not in the reconstruction is reported rather than falling back to the document root, which would answer a question the caller did not ask. Mutation-verified. One mutation initially survived: the test for container text relied on directText only walking immediate children rather than on the node-type check. Added a case with an element child carrying textContent, which now fails without the check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verified against the `@sentry-internal/rrweb-snapshot` build Sentry ships:
`serializeNodeWithId` assigns an id to every node including the Document, so a
snapshot's root is a `nodeType: 0` node rather than an element. The renderer
walked from that root and bailed on the first non-element, so a real recording
rendered nothing at all. Resolve the render root by descending to the first
element instead, breadth-first so the doctype sibling cannot lead the search
away from `html`.
The unit fixtures had modelled an id-less document wrapper, which is what hid
this. They now carry the real shape.
Three rendering defects surfaced once the fixtures were honest, each confirmed
against rrweb's own emission code rather than assumed:
- `role` presence was treated as interactivity, so `role="alert"` pulled status
banners into a tree labelled interactive. Match the WAI-ARIA widget roles by
value instead.
- `isChecked` is reported on every input event, not only for checkboxes and
radios, so text fields rendered `[checked=false]` — a claim about a property
they do not have.
- A boolean HTML attribute serializes as the empty string, since
`getAttribute("disabled")` returns `""`. `disabled` rendered as `[disabled=]`.
Each fix is pinned by a test that fails without it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Click breadcrumbs carry `payload.data.node.id`, the rrweb node id, which is stable within a recording. It was parsed but never rendered, so nothing in the output named the element a structural read could focus on — "show me the DOM around what was rage-clicked" had no handle to pass along. Report it at detail grain. Also add the DOM events the recording fixture lacked. It carried only custom events (`type: 5`), so nothing exercised the two types that describe structure. The login and checkout pages are now snapshotted, and the checkout segment carries a `source: 5` input change, an ignored scroll, an added error banner, and a text-plus-attribute mutation that disables the submit button — the last two after the console error, so a read at the error must not show them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes the three-step replay read: the map finds the failure, `get_replay_activity` names the element and reports its node id, and this explains what was around it. Each step hands the next a concrete handle. A separate catalog-only tool rather than a grain or a flag on `get_replay_activity`. A structural read has a different return shape, cost model, and failure mode from a windowed signal list, and it is the only surface where a read can refuse on budget grounds without complicating a contract agents call routinely. Catalog-only keeps the direct-surface budget unchanged at 5,598 tokens across 9 tools. `atMs` is required. Defaulting to either end of the session would answer a different question often enough that guessing is worse than asking. It is an offset from the replay's start, matching the other two tools, so a replay with no parseable `started_at` is refused rather than placed by guess. Refusal is a supported outcome. When the segment budget runs out before the target moment is reached, the tool says so and explains what would help. It does not return a partial tree, because a tree assembled from a truncated mutation history is indistinguishable from a complete one at the point of use. Reads stop paging as soon as an event passes the target, so cost is bounded by how far in the moment is rather than by session length. Fidelity is reported, not assumed: which snapshot it started from, how many mutations it applied, how many operations it dropped and why, and whether the recording ended before the moment asked for. Structure only. Text and input values arrive already masked by the SDK with no marker, so they render as delivered and are never labelled redacted — the tool cannot distinguish masked-at-capture from genuinely-this-text. It answers "was the button disabled", not "what did the user type". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Promote the DOM tree section from Future Work into Design, replace the proposed output with the shipped format, and add a requirement to the delta spec. The checkout-frequency question the section called "the single largest open question" is answered by design rather than measurement: superseding the node map on each snapshot makes the frequent-checkout and single-snapshot cases identical, so it no longer gates the interface. What remains unmeasured is whether the budget is generous enough on a long real replay — and if it is not, the tool refuses, which is a correct answer rather than a wrong tree. Also record the divergences: ARIA roles matched by value rather than `role` presence, node ids inline rather than in a trailing index, and two reported conditions the spec did not name (a recording that ends before `atMs`, and a replay with no parseable `started_at`). The testing list gained a case it was missing, added because it caused a real bug: a fixture's node shape must match what rrweb actually emits. A merely plausible fixture tests the implementation against itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… walk Found by pointing the tool at a real replay: the rendered tree stopped partway through `head` and never reached `body` at all. Two defects, both of which made a real page look far smaller than it was. Depth truncation aborted the whole traversal. `truncated` was a single flag and the walk returned as soon as it was set, so the first branch to exceed the depth limit hid every later sibling — and on a live page `head` is routinely deep enough to swallow `body` outright. Depth pruning is now local: the offending branch is dropped and counted, and its siblings still render. The node budget stays global, since once it is spent there is nothing left to spend. The default `maxDepth` of 12 was also too shallow to be useful. The real page measured 17 levels of elements with its interactive nodes below level 12, so the default clipped exactly what the interactive lens exists to surface. Raise it to 40 and document depth as a poor proxy for output size — component frameworks nest wrappers freely, so `maxNodes` is the real budget. Report the two limits separately. They call for different fixes, and the old single line named `maxNodes` even when depth was the cause, which sent a reader at the wrong dial. Each message now also names the way out, including that every rendered line carries the node id `rootNodeId` accepts. The first test written for this passed with the bug reintroduced: the fixture was not deep enough for one branch to hide another. Mutation testing caught that, and the replacement fails without the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Agents investigating a replay over-rotate onto web vitals and console warnings instead of the DOM. Three causes, all in what the output emphasizes rather than in the model. The chain dead-ended. `get_replay_details` prints an explicit `get_replay_activity(...)` call to run next, but activity pointed nowhere, so the structural read was reachable only by knowing it existed and searching the catalog — while console and vital signals sat in the output already in hand. Activity now prints a rooted `get_replay_dom(...)` call, so the three steps read end to end. The suggestion is conditional. It appears for a rage click, a dead click, or a hydration error — cases where the user acted and the page did not answer, and the explanation is the element rather than a response. It stays silent after an ordinary click or a failed request, because a suggestion on every response is one nobody reads, and silent when the DOM tool is absent from the session, because naming an unreachable tool sends the reader after nothing. A web vital that met its threshold no longer renders as a signal. `good`-rated LCP beside a 500 and a rage click invites investigation of a metric that is already fine; kind counts still include it. The rrweb node id is now carried on the signal rather than only in its rendered detail line, so the handoff does not depend on parsing prose it also produces. Descriptions say structural questions exist and which tool answers them, and that vitals and warnings are context rather than cause. Verified against a real replay: of 16 signals, 8 carry a node id, and this recording has no rage clicks, dead clicks, or hydration errors — so the suggestion correctly stays silent. That means prod has not yet exercised the trigger; the gating is covered by tests, including one that fails if the availability guard is removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…l reads Three findings from manual use. Network requests never reported a duration. The code read `data.duration`, which the SDK does not set — `NetworkRequestData` in `@sentry-internal/replay` has no such field, and real spans carry timing in `startTimestamp`/`endTimestamp` instead. The line was dead in every recording. Derive the elapsed time from the span's own bounds and put it in the summary rather than the detail lines: whether a failure was rejected or timed out is the first question asked of it, so it belongs on the line every grain shows. An absent bound reports nothing rather than 0ms, which would claim the request was instant. A truncated digest was actively misleading. `navigation x2 / click x1` reads as a whole-session rollup while the rage click, dead click, failed request and console error sit on later pages — the counts look authoritative and describe a healthier session than the real one. Say so above the numbers, since the numbers are what gets believed. Continuation is now a callable `get_replay_activity(...)` rather than a bare `cursor='...'` the reader has to rebuild a call around, and it names raising `limit` or windowing as the cheaper alternatives to paging. The structural read went unused on client-side state questions. The handoff only fired for a rage click, dead click, or hydration error, but a message that flashed or a control that never rendered produces no such signal — and an agent with no pointer goes looking for the explanation in application source instead of reconstructing the page. State the capability whenever there is a timeline to aim into, anchored on a failure rather than the first signal, since a leading navigation is a useless moment to reconstruct. A specific signal still earns a specific rooted call; this is the fallback, not a replacement. Two tests here passed against shapes that do not occur: one asserted the nonexistent `data.duration`, and one could not tell a failure-anchored offset from a first-signal one. Both were caught by mutation testing and now fail without the behaviour they cover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An agent asked to confirm the wording shown during checkout read the map, concluded "the replay tooling only exposes metadata/breadcrumbs — not rendered DOM text", and went to read application source at a release SHA. Three separate causes, none of them discovery: `get_replay_dom` already ranks first or second in tool search for phrasings like "rendered DOM text replay". The map named only `get_replay_activity`. The chain runs map to activity to DOM, and the handoff added for the third step lived in activity — which this agent never called, because the map looked sufficient. The map is where a reader stops, so it now names the structural read too, anchored on the same moment it suggests zooming into. The description understated what a tree contains, in the exact direction that mattered. It claimed text and form values are masked, so the tool "answers structural questions, not what a user typed" — which reads as "rendered text is unavailable" and is wrong. `maskAllText` targets user-entered content: on a real recording, 22 of 24 text nodes are intact, including product copy like "Session replay" and "Watch real user sessions to see what went wrong". Only the user's own details were masked. Reading the wording a user was shown is now stated as a primary use, with its own example, and the masking note scoped to what it actually covers. The agent's conclusion was a positive claim about capability, and nothing in the map contradicted it. That is the failure mode being fixed: silence about a capability reads as its absence. Verified against the SDK rather than assumed — `maskAllText` defaults to true at the integration level, and the masking function targets text nodes by selector, which is why static copy survives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The 80-character cap was deliberate when the tree was a structural artifact and text was only a label to identify an element by. It became wrong once confirming the exact wording a user was shown was a documented use of the tool, and nothing flagged the contradiction. Measured against a real onboarding page: product copy runs 60 to 100 characters, so 80 cuts through the middle of the distribution. The longest line there — "Catch breaking changes, automatically root cause issues in production, and fix what you missed." at 95 characters — lost its last two words, which is the worst available outcome. It looks complete enough to quote and is not. Rendered text now allows 400 characters. The node budget is what bounds response size; this only bounds a single pathological node, so it can afford to be generous. Form values keep a tighter 80. Values are usually masked to `***`, and an unmasked one long enough to hit the cap is a payload — a token or a pasted blob — where the length is the informative part rather than the content. The cap had no test at all, which is why the number survived a change in what the tool is for. Both limits are now covered, each verified by a mutation that fails the new tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
| - [ ] 9.6 QA against a real organization with the `mcp-qa` skill, since mocks cannot prove the SDK-shape fix. Confirm on a long real replay that the 150-segment and 10MB bounds hold under the Workers memory ceiling, and adjust them if not. `get_replay_dom` is the read most exposed to those bounds, since it must page to the requested moment rather than sampling; check how often a reconstruction refuses on real sessions. Two of the three DOM observations originally filed here no longer gate anything: snapshot frequency is handled by superseding the node map wholesale, so both cases take one path. Still worth capturing: whether real click breadcrumbs populate `payload.data.node.id` consistently, which decides whether rooting is a reliable entry point or best-effort, and how large a real `FullSnapshot` is. None of these block this change. | ||
|
|
||
| ## 10. DOM Tree Reads | ||
|
|
There was a problem hiding this comment.
DomReconstructor leaks superseded drop counts into final fidelity report
DomReconstructor does not reset dropped counts when a later snapshot supersedes an earlier one, so malformed nodes or unknown-parent drops from the superseded state leak into the final fidelity report and falsely warn that the current tree is incomplete.
Evidence
- Task 10.2 states a later snapshot supersedes wholesale, matching the
ingestSnapshotcomment that pre-snapshot mutations described a state the snapshot already supersedes. ingestSnapshot()inpackages/mcp-core/src/internal/replay-dom.tsresetsnodes,rootId, andmutationsAppliedto enforce this, but does not resetthis.dropped.result()returns{ ...this.dropped }, andformatDomOutput()prints a warning likeDropped 2 operations (2 malformed); the structure below may be incompletewhenevercountDropped(dropped) > 0.- On recordings with
checkoutEveryNms, a clean later snapshot still reports earlier drops in its final output even though those drops do not affect the tree it produced. - The existing test "supersedes an earlier snapshot entirely" asserts
mutationsAppliedis reset but does not assertdroppedis reset, leaving the gap uncovered.
Also found at 1 additional location
packages/mcp-core/src/internal/replay-dom.ts:231
Identified by Warden · code-review · HDK-H8D
Comment on lines
+498
to
+510
| } | ||
| if (typeof data.isChecked === "boolean") { | ||
| node.inputChecked = data.isChecked; | ||
| } | ||
| } | ||
|
|
||
| /** Unlinks a node from its parent without deleting it. */ | ||
| private detach(node: DomNode): void { | ||
| if (node.parentId === null) { | ||
| return; | ||
| } | ||
| const parent = this.nodes.get(node.parentId); | ||
| if (!parent) { |
There was a problem hiding this comment.
Unchecked checkbox still renders [checked] because applyInput leaves stale attributes.checked
When a checkbox that started with checked: true receives an input event with isChecked: false, renderDomTree still emits [checked] from the unchanged attributes.checked.
Evidence
applyInputupdatesnode.inputCheckedbut never touchesnode.attributes.checked.renderDomTreeemits[checked]when eithernode.inputChecked === trueor"checked" in node.attributes.- A snapshot with
<input checked>setsattributes.checked = true. A subsequent uncheck input event setsinputChecked = falsewhileattributes.checkedremainstrue. - The resulting line therefore contains
[checked]even though the control was unchecked by the user.
Identified by Warden · code-review · CAG-TCD
Comment on lines
+892
to
+925
| startId: number, | ||
| ): Set<number> { | ||
| const keep = new Set<number>([startId]); | ||
|
|
||
| const markAncestors = (id: number) => { | ||
| let current = nodes.get(id)?.parentId ?? null; | ||
| while (current !== null && !keep.has(current)) { | ||
| keep.add(current); | ||
| current = nodes.get(current)?.parentId ?? null; | ||
| } | ||
| }; | ||
|
|
||
| // Walk the subtree under startId rather than the whole map, so a rooted | ||
| // render is not influenced by interactive elements elsewhere in the page. | ||
| const stack = [startId]; | ||
| while (stack.length > 0) { | ||
| const id = stack.pop(); | ||
| if (id === undefined) { | ||
| continue; | ||
| } | ||
| const node = nodes.get(id); | ||
| if (!node) { | ||
| continue; | ||
| } | ||
| if (isInteractive(node)) { | ||
| keep.add(id); | ||
| markAncestors(id); | ||
| } | ||
| stack.push(...node.childIds); | ||
| } | ||
|
|
||
| return keep; | ||
| } | ||
|
|
There was a problem hiding this comment.
Interactive lens renders bare root when no interactive elements exist
The interactive lens always includes the render root in keep, so get_replay_dom outputs a single bare root element instead of the "No interactive elements are present here" message when a page has no interactive controls.
Evidence
collectInteractiveAncestryinitializeskeepwith[startId]unconditionally, even when the subtree has no interactive descendants.- When no interactive elements exist,
keepremains{startId}, sowalkrenders exactly the root element and skips all children. renderDomTreereturnslines.length === 1, which prevents the caller inget-replay-dom.ts(line 336) from triggering thetree.lines.length === 0branch that prints'No interactive elements are present here. Try \lens: "full"`.'`- The
DomLenscontract saysinteractivekeeps "interactive elements, plus every ancestor up to the render root so each one has a path"; when there are no interactive elements there are no paths to anchor, so the root should not be kept.
Identified by Warden · code-review · NMJ-VPV
Comment on lines
+196
to
+216
|
|
||
| /** Breadcrumb `payload.category` values, mapped to their event type. */ | ||
| const CATEGORY_TO_TYPE: Record<string, ReplayEventType> = { | ||
| "ui.click": "click", | ||
| "ui.multiClick": "multi-click", | ||
| navigation: "navigation", | ||
| console: "console", | ||
| "ui.blur": "ui-blur", | ||
| "ui.focus": "ui-focus", | ||
| "replay.hydrate-error": "hydration-error", | ||
| "replay.mutations": "mutations", | ||
| "sentry.feedback": "feedback", | ||
| "ui.tap": "tap", | ||
| "device.battery": "device-battery", | ||
| "device.orientation": "device-orientation", | ||
| "device.connectivity": "device-connectivity", | ||
| "ui.scroll": "scroll", | ||
| "ui.swipe": "swipe", | ||
| "app.background": "background", | ||
| "app.foreground": "foreground", | ||
| }; |
There was a problem hiding this comment.
countReplayKinds misses network errors without a numeric >= 400 statusCode
isErrorEvent counts only numeric status codes >= 400 as network failures, but describeNetworkRequest renders any non-2xx request—including CORS failures with no statusCode—as an error. This causes the replay kind breakdown to undercount network errors compared to the rendered signals.
Evidence
TYPE_TO_KINDmapsresource-fetchandresource-xhrtonetworkso both reachcountReplayKindsandextractReplaySignals.describeNetworkRequestsetsisError: truefor every request that does not have a numeric 2xxstatusCode, including CORS failures that carry nostatusCodeat all (explicitly tested as "failed with no response").isErrorEventcheckstypeof status === "number" && status >= 400, so requests with missing or non-numericstatusCodereturnfalseand are excluded from the error count.formatKindBreakdownsurfacesnetwork.errorsfromcountReplayKinds, making the undercount visible in the replay Map output.
Also found at 3 additional locations
packages/mcp-core/src/internal/replay-events.ts:435packages/mcp-core/src/internal/replay-events.ts:582-584packages/mcp-core/src/tools/catalog/get-replay-details.ts:487-489
Identified by Warden · code-review · VJE-EE3
| // A dead or rage click is a failure of the page, not of the request. | ||
| isError: verb !== "Clicked", | ||
| nodeId, | ||
| }; |
There was a problem hiding this comment.
Dead and rage clicks set isError but are not counted by isErrorEvent
describeClick now returns isError: true for dead and rage clicks, but the unchanged isErrorEvent (used by countReplayKinds) does not recognize any click type as an error. This causes renderDigest and countReplayKinds to disagree on the number of click errors.
Evidence
describeClickreturnsisError: verb !== "Clicked", so dead and rage clicks are marked as errors (line ~799).isErrorEventat line 575 was unchanged by the hunk; it only handlesconsole,resource-fetch/xhr, andhydration-error, falling through tofalsefor clicks.renderDigestcounts errors viasignal.isError, whilecountReplayKindscounts them viaisErrorEvent.- The fixture already includes dead and rage clicks (tested at line 657 of
replay-events.test.ts), so the mismatch is reachable immediately.
Identified by Warden · code-review · AXQ-7KK
Comment on lines
+737
to
+757
| const startMs = Math.max(0, anchor.offsetMs - ERROR_WINDOW_PADDING_MS); | ||
| const endMs = anchor.offsetMs + ERROR_WINDOW_PADDING_MS; | ||
|
|
||
| return [ | ||
| `${anchor.label} occurred at ${formatReplayOffset(anchor.offsetMs)}. ${instruction}:`, | ||
| formatToolCall({ | ||
| toolName: "get_replay_activity", | ||
| arguments: { | ||
| organizationSlug, | ||
| replayId, | ||
| startMs, | ||
| endMs, | ||
| grain: "detail", | ||
| }, | ||
| }), | ||
| ...structuralReadLines({ | ||
| organizationSlug, | ||
| replayId, | ||
| atMs: anchor.offsetMs, | ||
| context, | ||
| }), |
There was a problem hiding this comment.
buildNextStepLines generates invalid get_replay_activity and get_replay_dom parameters for errors before session start
When findErrorAnchor returns a negative offsetMs (error timestamp before startedAt), the generated tool suggestions pass unclamped negative values into downstream tools. endMs can become negative if the offset exceeds -ERROR_WINDOW_PADDING_MS, and atMs is always passed raw. Both tools enforce min(0) in their Zod schemas and will reject the generated calls.
Evidence
buildNextStepLinescomputesstartMswithMath.max(0, anchor.offsetMs - ERROR_WINDOW_PADDING_MS)but leavesendMsasanchor.offsetMs + ERROR_WINDOW_PADDING_MSunclamped.findErrorAnchorcomputesoffsetMs = timestampMs - originMswith no guard thattimestampMs >= originMs; an error before session start yields a negative value.- If
anchor.offsetMs < -ERROR_WINDOW_PADDING_MS,endMsis negative, violatingget_replay_activity's schema (endMs: z.number().min(0)) and its explicit validationendMs >= startMs. atMs: anchor.offsetMsis passed directly tostructuralReadLines; any negative value violatesget_replay_dom's schema (atMs: z.number().min(0)).formatReplayOffsetalready clamps toMath.max(0, offsetMs)for display, showing the codebase expects and defends against negative offsets, but the generated tool parameters were not given the same treatment.
Identified by Warden · code-review · BUH-Y58
| maxNodes: 200, | ||
| ...params, | ||
| } as never, | ||
| context, |
There was a problem hiding this comment.
callTool test helper overrides maxDepth, defeating the default-depth regression guard
The test that claims to verify the default depth passes maxDepth: 12 via the callTool helper, so it never exercises the handler’s actual default of 40. If the default ever regresses, this test will still pass and hide the regression.
Evidence
get-replay-dom.tsdefinesmaxDepth: z.number().min(1).max(200).default(40)as the tool default.- The
callToolhelper introduced in the same file hard-codesmaxDepth: 12, overriding the handler default for every invocation. - The test
"renders a deeply nested real-world page at the default depth"invokescallTool({ atMs: AT_CHECKOUT_ERROR }), so it actually runs at depth 12 instead of 40. - Because the fixture tree is shallower than 12, the assertion
not.toContain("deeper than \maxDepth`")` passes even though the test does not validate the intended default. - On a real page whose tree is 17 levels deep, a default of 12 would clip exactly the interactive elements the lens exists to surface, yet this test would still pass and mask that breakage.
Identified by Warden · code-review · D5S-VDQ
| for (const event of segment) { | ||
| if (reconstructor.apply(event) === "past-target") { | ||
| reachedTarget = true; | ||
| return "stop"; |
There was a problem hiding this comment.
Malformed event timestamps bypass the target-time gate and corrupt DOM reconstruction
Malformed replay events whose timestamp is not a number bypass the target-time gate in DomReconstructor.apply and are folded into the DOM even after atMs, producing an incorrect reconstruction.
Evidence
ReplayRecordingEventSchemainpackages/mcp-core/src/api-client/schema.tsdefinestimestampwithz.number().optional().catch(undefined), so a non-numeric timestamp (e.g., a scrubbed marker string) parses successfully withtimestamp: undefined.DomReconstructor.applyinpackages/mcp-core/src/internal/replay-dom.tscheckstypeof event.timestamp === "number"before comparing tothis.atMs; when the field isundefinedthe time gate is skipped and the event is processed.- Because the
streamReplayRecordingSegmentscallback only returns"stop"on"past-target", a post-target mutation or FullSnapshot with a missing timestamp is applied instead of ending the read, silently advancing the DOM state past the requested moment. replay-dom.test.tsonly exercises numeric timestamps, and the fixture segments use well-formed timestamps, so no test guards this path.
Identified by Warden · code-review · N2J-AMX
| if (event.type === RRWEB_FULL_SNAPSHOT) { | ||
| // A later snapshot supersedes everything applied so far: it is a | ||
| // complete state, so replaying earlier mutations onto it would be wrong. | ||
| this.ingestSnapshot(event, timestamp); |
There was a problem hiding this comment.
FullSnapshot with missing timestamp is ingested but reported as missing
A FullSnapshot whose timestamp is missing or not a number is ingested and populates the node map, yet result() sets missingSnapshot: true. This causes get_replay_dom to refuse rendering and falsely tell the user no snapshot exists.
Evidence
applyat lines 167-181 parsesevent.timestampasnullwhen it is missing or non-numeric, then callsingestSnapshotwithout guarding againstnull.ingestSnapshotat line 232 overwritessnapshotTimestampMswithnull, erasing any earlier valid timestamp.result()at line 207 computesmissingSnapshot: this.snapshotTimestampMs === null, so a snapshot that lacked a timestamp is reported as missing.get-replay-dom.tsat line 220 usesmissingSnapshotto refuse rendering with the message "No full DOM snapshot appears…", even thoughreconstruction.nodesis populated.ReplayRecordingEventSchemaat line 498 makestimestampoptional, so a FullSnapshot with a missing timestamp can flow throughstreamReplayRecordingSegmentsunfiltered.
Identified by Warden · code-review · MDD-M2U
Comment on lines
+496
to
+498
| `console ${consoleCount.total}${consoleCount.errors > 0 ? ` (${consoleCount.errors} error)` : ""}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
formatKindBreakdown uses singular "error" for any console error count
formatKindBreakdown prints console X (N error) even when N > 1, producing ungrammatical user-facing output.
Evidence
formatKindBreakdownline 496 appends`(${consoleCount.errors} error)`unconditionally.consoleCount.errorscomes fromcountReplayKinds, which increments for every console event withlevel === "error", so any replay with multiple console errors triggers the bug.- The same file already pluralizes
errorcorrectly foromittedIssues(line 351:error${omittedIssues === 1 ? "" : "s"}), confirming the intended pattern exists nearby.
Identified by Warden · code-review · WGT-BCH
This branch was previously deployed
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
1 participant
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.
No description provided.