fix(core): reset style fingerprint at each row boundary in diff renderer#1913
Conversation
📝 WalkthroughWalkthroughIn Diff Renderer Row Boundary Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/core/src/terminal/Renderer.test.ts`:
- Around line 251-263: The regression test in Renderer.test.ts is too weak
because the current reset count can still pass when row 1 does not re-apply
styles after its moveTo call. Tighten the assertion around Renderer.renderNow()
by checking for a reset/style sequence immediately after the row-1 cursor
movement, or by requiring the extra reset that only appears when row 1 correctly
reapplies formatting, so the test fails against the buggy Renderer._flush()
behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c60b33f3-9495-4d02-8de0-4ccdda46cc81
📒 Files selected for processing (2)
packages/core/src/terminal/Renderer.test.tspackages/core/src/terminal/Renderer.ts
|
@Karanjot786 Please review this |
Description
The differential renderer's style optimization (
_lastStyleFingerprint) was only reset once per frame flush, not once per row. When_renderDiffLinerenders spans on multiple rows, the style state bled across row boundaries becausemoveTopositions the cursor at a new row where the terminal has no memory of the previous cell's style. This caused sporadic missing ANSI style sequences, leading to visible color/bold/italic corruption on all rows after the first one.Fix
Moved
this._lastStyleFingerprint = null;inside thefor (let r = 0; r < rows; r++)loop in_flush()(Renderer.ts:139-143) so the fingerprint is reset before each row is processed. This ensures the first cell on every row always emits the full ANSI reset+style sequence.Test
Added
'resets style fingerprint at each row boundary in diff renderer'test that creates two rows with identical boundary styles and asserts the ANSI output contains at least two reset sequences (one per row).Closes #1910
Summary by CodeRabbit