feat(mecatui): unify bounded scrolling and cursor selection

[jbeda]

Stage

Implementation

Contract

• Plan / Interface PR: plan: bounded mecatui scroll and cursor controls #1593
• Approved baseline: 297817d86ef8173452877d42bc4132d73da4efd5
• Operator-authorized in-place amendment: stable item identity, independent viewport/cursor anchors, caller-owned selected styling, and bounded tiny-width behavior
• Interfaces match approved contract: Yes

Implementation

• adds concrete package-private boundedViewport and composed boundedList; no exported Go interface or protocol surface
• preserves cursor and top-visible anchors by stable item ID across live refresh/reorder, with numeric fallback when a selected item disappears
• keeps physical viewport position independent from logical cursor selection
• supports ANSI-aware multiline wrap/clip, oversized-item paging, caller-owned selected styling, and shared bounded overflow accounting
• migrates unified Agents rosters/details and /models
• routes visible-owner wheel input without moving selection or leaking into hidden conversation scrollback
• adds frame-scoped /models click-to-cursor while Enter remains activation
• deletes superseded Agents/Models windowing and render paths

Acceptance

All 15 ACs in docs/acceptance/mecatui-bounded-scroll-selection.md are implemented and resolve under the targeted strict trace.

Verification

• task lint — passed
• task test — passed
• task test:golden — passed
• task docs — passed
• task site:build — passed
• task api:check — passed
• go run ./cmd/mecademo — passed
• targeted ac-trace --strict --plan docs/acceptance/mecatui-bounded-scroll-selection.md — 15 ACs, 0 failures
• panel — PANEL: ship_blockers=0 important=0 advisory=0 reviewer_failures=0

The branch was intentionally not rebased in the large-context implementation session. Repository-wide task ac-trace-strict failed against this older baseline on unrelated acceptance-plan state; current origin/main includes 0fe7b4739 fix(acceptance): resolve stale ac-trace proofs (#1600). Rebase and rerun the aggregate gates before merge.

Deviations

No implementation deviation from the approved, operator-amended contract.

Relates to #1589

[jhrozek]

Automated panel review (Spec, Standards, Test-adequacy, and a 5-agent Domain panel) against docs/acceptance/mecatui-bounded-scroll-selection.md. Full result: 1 blocker, 4 important, 8 advisory findings across the axes. Posting the actionable subset (1 blocker + 4 important) as inline comments below.

PANEL: ship_blockers=1 important=4 advisory=8 reviewer_failures=0

[jhrozek]

[blocker · test-adequacy] testdata/models_scrolled.golden was regenerated into a degenerate, near-empty ~4-cell box at 100×14 after pressing End on a 30-item list — the pre-diff golden correctly showed a populated "28–30 of 30" window (confirmed via git diff origin/main...HEAD -- cmd/mecatui/ui/testdata/models_scrolled.golden). This looks like the rendered height collapses to near-zero for a frame and the outer modal box gets sized to that collapsed content instead of the offered 100×14 — the opposite failure from what AC2.3 guards against ("no minimum-row rule that can force output beyond offered geometry"; here nothing floors it back up when it transiently collapses). compareGolden only diffs bytes against a self-generated fixture, so this shipped silently under a green gate. Worth tracing rowBudget here plus whatever sizes the outer box against it, and adding a content assertion ("at least one model row visible when width/height are ample") alongside the golden.

[jhrozek]

[important · go-architect] boundedList is copied by value throughout agents_overlay.go (e.g. agentsSelectableList.control). l.items = append(l.items[:0], items...) mutates the shared backing array in place, so a copy's items slice aliases the original's. Call paths that build a copy and discard it — render/boundedView via indicatorAdjustedControl, and agentsNormalBodyFits — silently rewrite persisted roster state (m.subagents.roster, etc.) they never intended to touch. It's masked today because the next real setItems overwrites it again — exactly why it'll be a nasty one to chase down later. Suggest l.items = slices.Clone(items) (or append(l.items[:0:0], items...)).

[jhrozek]

[important · go-architect] This calls configuredControl and then control.move(move) directly. configuredControl sets viewport.height to the full capacity, but boundedListViewWithIndicators (used by the wheel path via scrollList, and by modelsState since it holds a *boundedList) shrinks height by 1–2 rows to reserve the "N lines above/below" indicators. So keyboard PgUp/PgDn here pages by a taller window than what's actually on screen — it skips 1–2 rows of content right at the indicator boundary that the wheel path doesn't skip. Suggest routing this through indicatorAdjustedControl like scrollList does, so all three paths agree on one height.

[jhrozek]

[important · software-architect] cursorID, viewport.valid(), and reveal are read directly as struct fields at two independent call sites (models_surface.go:216,223,236 and agents_overlay.go:1070,1077), each reimplementing the same hadCursor := X.cursorID != "" check. boundedList already has a clean method surface (setGeometry, setItems, setCursor, move, scroll, view…) — worth adding hasCursor() bool / ready() bool accessors and switching both sites to them, so the field layout stays genuinely private and the duplicated one-liner can't drift if the empty-string sentinel convention ever changes.

[jhrozek]

[important · spec] layout.bodyCapacity = height - layout.frameRows - lipgloss.Height(layout.tabStrip) - 2 — this bare -2 sits alongside boundedListViewWithIndicators's own dynamic reservation for the above/below indicator rows: a static offset next to a dynamic one computing roughly the same thing. The plan's "Deferred decisions" section calls this out as a risk ("a second geometry calculation risks pointer disagreement and style leakage"). Could this derive from the same reservation logic boundedListViewWithIndicators uses, or at least get a comment naming exactly what two rows it reserves?

[jbeda]

Thanks for the review! This was a first pass from the agent and I'm going to make sure I give it a careful review :)