test(term-fidelity): cursor-convention normalization + divergence discriminator#416
Conversation
khaliqgant
added
the
no-agent-relay-review
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
…criminator Two diagnostic improvements to the term-fidelity matrix, from the #403 fixer investigation (lead-endorsed evidence-first method). 1. Cursor-convention normalization (bundle meta). The renderer cursor is xterm's 0-indexed [row,col]; the broker plain-snapshot cursor is 1-indexed (it mirrors the raw CUP coords — a stream ending in ESC[40;3H reports broker [40,3] while xterm reports [39,2] for the SAME cell). Comparing the raw values makes a faithful render look like an off-by-one bug — this misread #403's headline "[39,2] vs [40,3]". Meta now records the raw values WITH their conventions, both normalized to a common 0-indexed base, and a computed `match` bool: a false there on an otherwise content-matching bundle is a REAL cursor divergence, not the convention. 2. Raw-stream divergence discriminator. Tap the raw broker->renderer byte stream (onPtyChunk, post main dedup), and on any divergence — including a telemetry-only reconciler repair — replay those exact bytes through a clean headless xterm at broker dims and diff vs broker and vs the live grid: rawVsBroker==0 -> delivery byte-clean; divergence is RENDERER-INTERNAL rawVsBroker >0 -> bytes lost/reordered before the renderer (DELIVERY-LOSS) Plus a scrollToBottom probe (stranded-viewport vs real content corruption), PEAR_DIAG_PTY per-chunk logging, and renderer-console capture. Writes raw.bin / raw-replay.txt / discriminator.txt / renderer-console.log into the bundle (and the reconciler-telemetry dir). This is what classified #403 as a viewport-pin regression (not byte loss) and #415 as delivery-clean. Test-only; no production/renderer code changed. Refs #403, #412, #415. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
e99eb1c to
849e8ba
Compare
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 849e8ba769
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let discriminator = 'raw-stream unavailable' | ||
| try { | ||
| const raw = await getRawStream(harness.page, agentName) | ||
| const rawReplay = await replayRawToGrid(raw, broker.rows, broker.cols) |
There was a problem hiding this comment.
For the canonical resize-mid-stream workload (tests/term-fidelity/workloads.ts:326-357), PTY dimensions change repeatedly while these bytes are produced, but this replays the entire session once at the final dimensions. Resizes are out-of-band events, and xterm can reflow its buffer differently from the broker emulator during width changes, so even a perfectly delivered stream can produce rawVsBroker > 0 and be mislabeled DELIVERY-LOSS; the cumulative telemetry path repeats the same inference. Record and replay the dimension timeline, or avoid assigning a delivery verdict for sessions containing resizes.
Useful? React with 👍 / 👎.
1 participant
Two diagnostic improvements to the term-fidelity matrix, distilled from the #403 fixer investigation (lead-endorsed evidence-first method). Test-harness only — no production/renderer code changed.
1. Cursor-convention normalization (deliverable)
The renderer cursor comes from xterm's buffer API (0-indexed
[row,col]); the broker plain-snapshot cursor is 1-indexed — it mirrors the raw CUP coordinates. A stream ending inESC[40;3Hreports broker cursor[40,3]while xterm reports[39,2]for the same cell. Comparing the raw values makes a faithful render look like an off-by-one bug — this is exactly what misread #403's headline[39,2]vs[40,3].The bundle meta now records:
rendererRaw0Indexed/brokerRaw1Indexed(raw values, labeled with their conventions),normalized0Indexed(both converted to a common 0-indexed base),match(bool) — afalsehere on an otherwise content-matching bundle is a real cursor divergence, not the convention artifact.2. Raw-stream divergence discriminator
Taps the raw broker→renderer byte stream (
onPtyChunk, post main-process dedup) and, on any divergence — including a telemetry-only reconciler repair — replays those exact bytes through a clean@xterm/headlessat broker dims and diffs vs the broker snapshot and the live grid:rawVsBroker == 0→ delivery is byte-clean; the divergence is RENDERER-INTERNAL.rawVsBroker > 0→ bytes were lost/reordered before the renderer (DELIVERY-LOSS).Plus: a
scrollToBottomprobe (distinguishes a stranded viewport from real content corruption),PEAR_DIAG_PTYper-chunk logging, and renderer-console capture. Writesraw.bin,raw-replay.txt,discriminator.txt,renderer-console.loginto the divergence bundle and thereconciler-telemetrydir.This is the tooling that classified #403 as a viewport-pin regression (not byte loss) and #415 (opencode) as delivery-clean / renderer-internal — turning "infer from aggregates" into "read the actual bytes" for every future divergence.
Verification
npm run typecheckclean.Refs #403 (fixed by #412), #415 (opencode vector, filed with this tooling's evidence).
🤖 Generated with Claude Code