Skip to content

Conversation

@joshka-oai
Copy link
Collaborator

This is a pure refactor only change.

Replace the flattened transcript line metadata from Option<(usize, usize)> to an explicit
TranscriptLineMeta::{CellLine { cell_index, line_in_cell }, Spacer} enum.

This makes spacer rows unambiguous, removes “tuple semantics” from call sites, and keeps the
scroll anchoring model clearer and aligned with the viewport/history design notes.

Changes:

  • Introduce TranscriptLineMeta and update TranscriptScroll helpers to consume it.
  • Update App::build_transcript_lines and downstream consumers (scrolling, row classification, ANSI rendering).
  • Refresh scrolling module docs to describe anchors + spacer semantics in context.
  • Add tests and docs about the behavior

Tests:

  • just fmt
  • cargo test -p codex-tui2 tui::scrolling

Manual testing:

  • Scroll the inline transcript with mouse wheel + PgUp/PgDn/Home/End, then resize the terminal while staying scrolled up; verify the same anchored content stays in view and you don’t jump to bottom unexpectedly.
  • Create a gap case (multiple non-continuation cells) and scroll so a blank spacer row is at/near the top; verify scrolling doesn’t get stuck on spacers and still anchors to nearby real lines.
  • Start a selection while the assistant is streaming; verify the view stops auto-following, the selection stays on the intended content, and subsequent scrolling still behaves normally.
  • Exit the TUI and confirm scrollback rendering still styles user rows as blocks (background padding) and non-user rows as expected.

Move TranscriptScroll into tui::scrolling for isolated tests, shift scroll resolution/anchor logic into methods on TranscriptScroll, and keep explicit ToBottom assignments for intentional snaps while defaults handle initialization.
Add rustdoc for scroll resolution and anchor helpers.
Add unit tests to lock in resolve_top/scrolled_by behavior.
Replace Option<(usize, usize)> transcript line metadata with TranscriptLineMeta::{CellLine, Spacer}.

This makes spacer rows explicit, avoids tuple semantics leaking through call sites, and keeps scroll anchoring documentation aligned with the viewport/history design notes.

Tests: cargo test -p codex-tui2 tui::scrolling
Copy link
Collaborator

@bolinfest bolinfest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ensure CI is green before submitting.

Use a method reference (TranscriptLineMeta::cell_index) to satisfy clippy::redundant-closure-for-method-calls.

Clippy: cargo clippy -p codex-tui2 --all-features --tests --profile dev -- -D warnings
@joshka-oai joshka-oai enabled auto-merge (squash) December 16, 2025 05:13
@joshka-oai joshka-oai merged commit f074e57 into main Dec 16, 2025
26 checks passed
@joshka-oai joshka-oai deleted the joshka/tui2-transcript-line-meta branch December 16, 2025 05:27
@github-actions github-actions bot locked and limited conversation to collaborators Dec 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants