fix: invalidate merge_cache on scrollback-capacity drain (#405)

[fredclausen]

What

Fixes a visual buffer desync where, during long-running output at full
scrollback (e.g. pre-commit run --all-files after cat-ing a large file),
the terminal window would freeze an old line at the top and drop
genuinely-new content, becoming unusable until the pane was killed.

Root cause

Buffer::merge_cache (the incremental flatten-merge cache from #405 Part C)
is keyed by MergeWindowFp { visible_start, visible_end, auto_detect }, whose
bounds derive purely from rows.len() and height.

Once scrollback is at capacity, each line feed pushes a row at the bottom and
then enforce_scrollback_limit drains a row from the front in the same logical
step — netting rows.len() unchanged. The fingerprint therefore stays
identical across the rotation while every row's identity shifts down by
overflow, so the incremental fast path serves a stale, off-by-overflow
prefix.

This is the same "confined in-place row rotation" bug class already guarded at
the three scroll.rs sites (scroll_slice_up / scroll_slice_down /
scroll_up), each of which nulls merge_cache. enforce_scrollback_limit was
the missed fourth site.

The divergence is silent in release — the debug_verify_against_oracle
cross-check that catches it is #[cfg(debug_assertions)] only.

Fix

• Null merge_cache in enforce_scrollback_limit's overflow > 0 branch
  (never in the rows.len() <= max_rows early return, which drains nothing).
• Correct the merge_cache field doc, which previously (wrongly) claimed this
  site was covered by fingerprint mismatch. erase_scrollback remains
  genuinely fp-covered (always collapses visible_start to 0) and is documented
  as such.

Tradeoff (tracked follow-up: #457)

This is the correctness fix. At steady-state capacity it forces a
full-window re-merge every line feed, giving up the incremental fast path in
that workload. The structural shift-in-place fix (rotate the cached merge
rather than discard it) that buys the performance back is tracked in #457 — it
is a correctness minefield (tag-splitting across the dropped boundary,
url_tag_indices rebasing, overflow > 1) and deliberately deferred so it can
be done carefully. There is no user-facing correctness bug left open.

Tests & benchmarks

• Regression test
  incremental_merge_matches_oracle_after_scrollback_capacity_rotation
  (drives many line feeds past the cap, cross-checks every flatten against the
  independent full-merge oracle). Panics on main without this fix; green with
  it.
• New bench bench_lf_flatten_at_capacity establishes the before/after
  baseline for perf: structural shift of merge_cache on scrollback drain (replace null-on-drain) #457: ~36 µs at window height 24, ~66 µs at height 100 (the cost
  scales with height, confirming it is the full-window re-merge).

Verification

• cargo test --all — green (625 freminal-buffer lib tests + all integration)
• cargo clippy --all-targets --all-features -- -D warnings — clean
• cargo machete — clean
• cargo fmt --all -- --check — clean
• cargo xtask check-windows — clean

Provenance

Diagnosed and fixed via an orchestrated multi-agent pass: read-only exploration
of the flatten/merge-cache and snapshot-windowing paths, a proven headless
reproduction, and an adversarial review of both the diagnosis and the patch
(which caught a iter_batched vs iter_batched_ref benchmark methodology bug,
since fixed).

Summary by CodeRabbit

• Bug Fixes

  • Fixed an issue that could produce stale or incorrect visible buffer content after reaching scrollback capacity.
  • Ensured displayed text and formatting remain accurate as older lines are trimmed and new lines are added.

• Tests

  • Added regression coverage for scrollback rotation and visible content consistency.

• Performance

  • Added benchmarks for flattening buffer content at scrollback capacity across common window sizes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6bc19662-c580-4f67-a6eb-518d34c55bea

📥 Commits

Reviewing files that changed from the base of the PR and between 3cc0137 and b1a904d.

📒 Files selected for processing (4)

• freminal-buffer/benches/buffer_row_bench.rs
• freminal-buffer/src/buffer/flatten.rs
• freminal-buffer/src/buffer/mod.rs
• freminal-buffer/src/buffer/resize_and_alt.rs

---

📝 Walkthrough

Walkthrough

The change invalidates merge_cache during scrollback-capacity trimming, documents related cache behavior, adds regression coverage for incremental flattening, and introduces benchmarks for LF rotation followed by visible flattening.

Changes

Scrollback merge-cache correctness

Layer / File(s)	Summary
Invalidate cache during scrollback trimming freminal-buffer/src/buffer/resize_and_alt.rs, freminal-buffer/src/buffer/mod.rs	enforce_scrollback_limit clears merge_cache after trimming, and documentation describes cache validity across row rotations and visible-window changes.
Validate incremental merge after rotation freminal-buffer/src/buffer/flatten.rs	A regression test compares incremental flattening with a full-merge oracle after repeated capacity rotations and validates extracted visible text.
Benchmark capacity rotation flattening freminal-buffer/benches/buffer_row_bench.rs	Adds and registers benchmarks for LF rotation followed by visible flattening at scrollback capacity for window heights 24 and 100.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• perf: structural shift of merge_cache on scrollback drain (replace null-on-drain) #457 — Addresses the same merge_cache handling during scrollback-capacity rotations in Buffer::enforce_scrollback_limit.

Possibly related PRs

• fredsystems/freminal#389 — Modifies Buffer::enforce_scrollback_limit and overlaps with this PR’s cache invalidation changes.
• fredsystems/freminal#419 — Modifies the same scrollback overflow and rotation bookkeeping path.
• fredsystems/freminal#429 — Modifies incremental flattening and merge_cache invalidation behavior.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the primary fix: invalidating merge_cache when scrollback trimming drains rows.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch task-121/merge-cache-scrollback-rotation-fix

---

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!