Skip to content

Goldens record no colour: implement the documented ink plane and adopt it across all three render shapes #6223

Description

@Hmbown

The hole

crates/tui/src/tui/golden_harness.rs:27-49 dumps cell.symbol() and nothing else. Its own module doc says:

Goldens are the design contract — a visual change that cannot show as a golden diff did not happen.

That contract is currently false for colour. A screen can go from a gold mark over a blue gradient to uniform grey without moving a single glyph, and all 41 golden buffers pass. The harness already documents the fix at :84-94 — an "ink plane" keying each painted cell to its (fg, bg, modifier) triple with a legend — and it is documented but unimplemented. The comment there notes the founder's standing complaint about the startup screen ("everything in the same dim gray") was, mechanically, invisible to this suite.

This matters right now because the Shoreline re-ink (#6222) changes every colour in the product and no golden can see it. Its correctness rests on a mechanical contrast audit plus one manual terminal capture. So does the next palette change.

Same species as #6193: a claim the repository cannot falsify.

Why this is not a small patch

I implemented the plane (it is ~130 lines and straightforward: row-major first-appearance key assignment, a legend, explicit overflow reporting) and then looked at adoption, which is the part that decides whether it is worth anything. There are 11 assert_matches_golden call sites in three incompatible shapes:

  1. Raw Buffer (5 sites)composer_chrome, phase_strip, notifications, work_surface/panels, work_surface/tideline. These adopt directly: render once, assert {name} and {name}.ink off the same buffer.
  2. Terminal/TestBackend (1 site)work_surface/mod.rs defines its own render_golden_text(app, w, h) that draws through a Terminal and reads terminal_text(&terminal). The buffer is inside the terminal; adopting means reaching terminal.backend().buffer().
  3. Post-processed string (1 site)views/tideline_tests.rs wraps its render in trim_rows, which str::trim_ends every line. Applied to an ink plane that would silently delete trailing unpainted . cells and corrupt the plane. This site needs its own trim, defined on the buffer rather than the string.

Plus theme_picker.rs, history/automation.rs and two sites in views/mod.rs.

Why partial adoption is worse than none

The value of the plane is the invariant "a palette change shows as a golden diff." Adopt it on five surfaces and that invariant is false for the rest, while the doc comment claims it holds — which is exactly the "one caller and a ticket for the rest" outcome AGENTS.md warns about. It should land as one slice covering all three shapes, or not land.

Scope

  • ~130 lines of harness
  • 11 call sites across 3 render shapes
  • ~41 new .ink goldens to bless in the same commit
  • The trim_rows equivalent has to be defined on the buffer, not the text

Not large, but it is a reviewed slice with a big blessed-golden diff, not a mechanical sweep.

Suggested order

  1. Land the plane plus the raw-Buffer adoption (5 sites) and their goldens.
  2. work_surface/mod.rs — hoist its local renderer onto the shared one so there is one render path, not two.
  3. views/tideline_tests.rs — buffer-level trim, then adopt.
  4. Only then update the module doc to claim the contract holds.

Related: #6222 (the re-ink this would defend), #6193 (the same unfalsifiability, for performance).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions