feat(tui): show persisted guidance on rejected policy chunks

[vyncint]

Summary

The reviewer's rejection note is already stored in PolicyChunk.rejection_reason and already reaches the TUI in GetDraftPolicyResponse.chunks, but openshell-tui never read the field — so the note was dropped at the last step and there was no way to recall why a chunk had been rejected. This renders it: truncated on the list row, in full in the detail popup.

Related Issue

Part of #1098 — Definition of Done item "Rejected chunks show persisted guidance."

Deliberately not Fixes/Closes: #1098 has seven DoD items and this PR completes one, so closing the issue would be wrong. The other items are untouched — human_summary (item 1) and the L4/no-method-path scoping warning (item 4) still need a proto field or a design decision; intent_summary (item 2) looks reachable through the existing PolicyChunk.rationale but is a separate change; validation_result (item 3), the ~2s poll cadence (item 6), and the approve/reject hotkeys (item 7) are already implemented on main even though their checkboxes are unticked.

Changes

All in crates/openshell-tui/src/ui/sandbox_draft.rs (+88, −0):

• rejection_guidance(&PolicyChunk) -> Option<&str> — a small pure accessor next to truncate_str, gated on status == "rejected" and trimming the value.
• Detail popup — a Guidance: line after the Review: block, using t.status_err and the existing 12-character label column. Placed high on purpose: popup_height is capped at 22 rows with wrapping but no scrolling, so content after a long endpoint list can fall off the bottom.
• List row — the reason truncated to 32 characters via the existing truncate_str, dimmed with t.muted to match how rejected rows are already greyed, and appended last so it clips harmlessly instead of displacing anything on a narrow terminal.
• Tests — a new #[cfg(test)] mod tests with 7 cases.

The split (truncate in the list, full text in the popup) follows the Long content: truncate + detail popup convention in .agents/skills/tui-development/SKILL.md.

Why the status gate, and not just a non-empty check

Approving a chunk passes None for the reason (grpc/policy.rs:1537, :4760, :5174), and the gateway writes the field only when it is Some (persistence/sqlite.rs:1147-1149). So a chunk that was rejected and later approved — reachable with the [a] Approve hotkey the popup already offers on a rejected chunk — keeps its old note. Reading the field unconditionally would print a stale rejection on an approved rule. Only revert-to-pending clears it, via Some("") at grpc/policy.rs:5376. approved_chunk_hides_stale_reason pins this.

Coordination with #2168

#2168 (draft) rewrites approval_annotation() and deletes validation_issue_summary() in this same file. Nothing here touches that region or calls either function, so the two rebase cleanly in either order.

Testing

• mise run pre-commit passes
• Unit tests added/updated
• E2E tests added/updated (if applicable) — not applicable: this is presentation-only inside openshell-tui, which has no e2e surface

Run locally on Linux x86-64, Rust 1.95.0:

Command	Result
mise run pre-commit	exit 0
mise run ci	exit 0 — Rust 5,646 tests (was 5,639), Python 87, TypeScript SDK 70, 0 failed
cargo test -p openshell-tui	54 passed, 0 failed

The 7 new tests: rejected_chunk_exposes_its_reason, approved_chunk_hides_stale_reason, pending_chunk_has_no_guidance, blank_reason_is_dropped, reason_is_trimmed, long_reason_truncates_for_the_list_row, short_reason_is_not_truncated.

Rendering was checked, not assumed. I rendered draw_detail_popup into a ratatui::backend::TestBackend and read the cell buffer back:

┌ allow-github ────────────────────────────────────────────────────────┐
│ Status:     rejected                                                 │
│ Confidence: 82%                                                      │
│ Guidance:   too broad: allows any port on github.com                 │
│ Denied:     0 connections  (first --:--:-- / last --:--:--)          │
│                                                                      │
│ [a] Approve  [Esc] Close                                             │
│  ... 14 empty rows                                                   │
└──────────────────────────────────────────────────────────────────────┘

That probe is not in the diff — the crate has no rendering-test precedent (every existing test in ui/ and theme.rs covers pure functions, and there are no dev-dependencies), so I kept the shipped tests to the same shape and removed the probe afterwards. Happy to land it as a permanent test if you would rather have coverage of the frame itself.

One limitation I want to be straight about: the list-row span is compile-checked and its truncation is unit-tested, but I did not render that view. draw() takes &mut App, which owns a gRPC client and is not cheaply constructible in a unit test.

Checklist

• Follows Conventional Commits — feat(tui): show persisted guidance on rejected policy chunks
• Commits are signed off (DCO)
• Architecture docs updated (if applicable) — not applicable: no architecture, proto, or API change

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

All contributors have signed the DCO ✍️ ✅
_{Posted by the DCO Assistant Lite bot.}

[vyncint]

I have read the DCO document and I hereby sign the DCO.

[johntmyers]

gator-agent

PR Review Status

This focused TUI change is project-valid under #1098, but the detail view does not keep the full persisted guidance reachable when an accepted rejection reason wraps past the fixed popup height. The direct user-facing TUI behavior also needs a corresponding Fern documentation update before pipeline handoff.

Action required: make long guidance reachable in the detail UI, add the deterministic render coverage described inline, and document the rejected-guidance UX under docs/.

Blocking findings:

• GATOR-30f8375a-01: long persisted rejection guidance is clipped in the fixed, non-scrollable detail popup.

Carried findings:

• None

Non-blocking suggestions:

• None

Gator metadata

• Validation: Project-valid as a focused implementation of the “Rejected chunks show persisted guidance” item in accepted issue #1098.
• Docs: Missing for this direct TUI UX change; update the relevant Fern page under docs/ (navigation only if needed).
• Checks: Branch Checks and Helm Lint have not been dispatched for the current head.
• E2E: Not required for this presentation-only TUI change.
• Head SHA: 30f8375acfd4f41dbb7079b82b83bd93e6dae535
• Base SHA: 905e99aa2a960c17d5672b11cf64f10ff7d18b41
• Merge base SHA: 905e99aa2a960c17d5672b11cf64f10ff7d18b41
• Patch ID: 5a767987002dffe3c12f471538e37f0530e8f858
• Gator payload: 7
• Review mode: initial
• Previous reviewed SHA: none
• Review budget exhausted: no
• Maintainer decision required: no
• Next state: gator:in-review

[vyncint]

Thanks — GATOR-30f8375a-01 reproduces, and it is worse than the summary suggests. I wrote your verify case as a test before changing anything: an 80×24 popup with a 2,037-character reason carrying unique prefix and suffix markers.

Before the fix:

Marker	Visible
PREFIX_MARKER	yes
SUFFIX_MARKER	no
[a] Approve / [Esc] Close	no
Denied: / Rationale: / security note	no

So the tail was lost and every later field plus both action hints were pushed out of the 22-row popup.

Two things I confirmed while verifying:

1. The reason really is unbounded. RejectDraftChunk stores req.reason verbatim — no validation, truncation, or cap anywhere in the server (grpc/policy.rs:4896-4903).
2. The clipping predates this PR. A pending chunk with a long rationale clips identically on main — same lost tail, same missing hints. rationale and security_notes are already unbounded free-form fields in this popup. This PR adds a third and puts it high in the layout, which makes it much easier to hit, so the finding is fair; I mention it only because it made me fix the popup generally rather than special-case guidance.

What changed

Scrollable body with pinned controls. The inner area now splits into a scrolling body and a one-row footer, following the existing pattern in ui/create_provider.rs:653-663. The action and close hints render in that pinned footer, so they are on screen at every scroll position rather than merely reachable by scrolling. Keys are j/k and arrows, PageUp/PageDown, and g/G, clamped to the content. The bottom border carries a position/total indicator, and [j/k] Scroll appears in the footer only when the content actually overflows.

Explicit wrapping instead of Wrap. An honest scroll clamp needs the real rendered row count, and with Wrap the widget's row count is not knowable — Paragraph::line_count exists but is behind ratatui's unstable-rendered-line-info feature, which I did not want to switch on for this. Free-form values are now wrapped to the popup's text width by a small pure wrap_value helper and pushed as pre-fitted rows, so lines.len() is exactly what renders. Continuation rows indent to the label column. Endpoint, binary, rationale and security-note values go through the same helper, so nothing that wrapped before now clips horizontally.

After the fix, at max scroll:

│             must be narrowed; SUFFIX_MARKER                  │
│ Denied:     0 connections  (first --:--:-- / last --:--:--)  │
│ [a] Approve  [j/k] Scroll  [Esc] Close                       │
└─────────────────────────────────────────────────────── 29/45 ┘

Coverage

Deterministic render tests against TestBackend, all at 80×24 with the 2,000-character reason:

• long_guidance_head_and_tail_are_both_reachable — prefix at scroll 0, suffix at max scroll.
• action_hints_stay_visible_at_every_scroll_position — approve, scroll and close hints at both ends.
• scrolling_past_the_end_is_clamped_to_the_last_page — over-scrolling renders identically to the last page.
• long_rationale_tail_is_reachable_on_a_pending_chunk — covers the pre-existing case above.
• short_content_does_not_scroll — no scroll hint when everything fits.

Plus four wrap_value cases: word boundaries, hard-breaking a word longer than the width, empty input, and never exceeding the width.

Docs

Added to the "Review Proposals" section of docs/sandboxes/policy-advisor.mdx, next to openshell rule reject --reason, which previously described only the agent-facing return path. It now also covers the reviewer-facing side: where the guidance appears in openshell term, and the scroll keys. No navigation change, so docs/index.yml is untouched.

Verification

mise run ci passes locally: exit 0, 5,655 Rust tests (up from 5,646 — the nine added here), 87 Python, 70 TypeScript SDK.

The fix is in f18b11b5, left as a separate commit on top of 30f8375a so you can read just the delta. Branch Checks and Helm Lint are still waiting on /ok to test for the new head.

[vyncint]

Self-review of f18b11b5 turned up two regressions I introduced there, both on the horizontal axis rather than the vertical one. Pushing the fix rather than waiting to be told.

Making the scroll clamp exact meant dropping Paragraph's Wrap, and that also removed wrapping from the lines that do not go through push_wrapped. At 70 columns the denial row clipped mid-value:

│ Denied:     3 connections  (first 22:13:20 / last 22 │   70 cols — last-seen lost
│ Denied:     3 connections  (first 22:13:20 /         │   60 cols

And at 60 columns a pending chunk with scrollable content produced [a] Approve [x] Reject [j/k] Scroll [Esc] Close — 50 characters in a 44-column footer, so [Esc] Close fell off the right edge. The key still worked, but that is the same "controls not visible" failure GATOR-30f8375a-01 was about, one axis over.

Fix

The denial row stays on one line while it fits and wraps onto the label indent when it does not, so nothing is lost and the common 80-column case is unchanged. The footer now packs hints into as many rows as they need without ever splitting a single hint, and the footer height is derived from that — body height and hint rows settle together, since adding a hint row shrinks the body and can itself change whether the content scrolls.

At 60 columns:

│ Denied:     3 connections                    │
│             (first 22:13:20 / last 22:15:00) │
│                                              │
│ Rationale:  PREFIX scope this to docs/ paths │
│             only; scope this to docs/ paths  │
│ [a] Approve  [x] Reject  [j/k] Scroll        │
│ [Esc] Close                                  │
└──────────────────────────────────────── 1/19 ┘

Coverage

My earlier tests were all 80×24, which is exactly why they missed this. The new ones sweep widths:

• denied_timestamps_survive_a_narrow_popup — both timestamps present at 60, 70 and 80.
• close_hint_survives_a_narrow_popup_with_scrollable_content — approve, reject and close hints all present at 60, 70 and 80 with overflowing content.
• hints_pack_onto_one_row_when_they_fit and hints_spill_onto_a_second_row_when_they_do_not_fit — direct cover on the packing helper.

67 tests in openshell-tui, mise run ci green.

[johntmyers]

gator-agent

PR Review Status

Thanks @vyncint. I checked the scrollable body, pinned controls, narrow-footer follow-up, deterministic render coverage, and Fern docs you added. Those resolve the original ASCII long-guidance reproducer, but the same reachability obligation remains open for valid double-width Unicode guidance: the manual wrapper counts characters rather than terminal display columns, so CJK text can still be clipped horizontally with no way to scroll to the missing tail.

Action required: measure wrapping and hard breaks by terminal display width, and add an 80×24 render regression with a CJK reason whose suffix must remain reachable.

Blocking findings:

• None newly introduced

Carried findings:

• GATOR-30f8375a-01: partially resolved; vertical overflow and controls are fixed, but double-width Unicode guidance can still be clipped at crates/openshell-tui/src/ui/sandbox_draft.rs:563.

Gator metadata

• Validation: Project-valid as a focused implementation of the “Rejected chunks show persisted guidance” item in accepted issue #1098.
• Docs: Fern documentation updated under docs/sandboxes/policy-advisor.mdx; navigation change not needed.
• Checks: Branch Checks and Helm Lint have not been dispatched for the current head.
• E2E: Not required for this presentation-only TUI change.
• Head SHA: 03c67b18b405fd4c8d8204973efcc1474457f8a8
• Base SHA: 905e99aa2a960c17d5672b11cf64f10ff7d18b41
• Merge base SHA: 905e99aa2a960c17d5672b11cf64f10ff7d18b41
• Patch ID: b112d40f4bdec6b84a9adaa7f47af9388bb9cdee
• Gator payload: 7
• Review mode: follow_up
• Previous reviewed SHA: 30f8375acfd4f41dbb7079b82b83bd93e6dae535
• Review budget exhausted: no
• Maintainer decision required: no
• Next state: gator:in-review

[vyncint]

Confirmed, and thanks — this one was real and I would not have caught it from the ASCII case.

Reproduced first: a CJK reason at 80×24 renders every wrapped row at roughly twice the popup's column width, so the right half of each row is clipped and there is no horizontal scroll to recover it. The cause is exactly as described at sandbox_draft.rs:563 — wrap_value packed by chars().count(), and a CJK glyph is one char but two terminal columns.

Fix

Width is now measured in display columns via Span::width(), which is the same measurement ratatui applies when it lays cells out — so the wrap agrees with the renderer by construction, and it needs no new dependency. That covers word packing, the hard-break path for a word wider than the line, the label indent in push_wrapped, the footer hint packing, and the denial row's fits-on-one-line check.

The list row's shortened copy had the same defect for a different reason: truncate_str counts chars, so a CJK guidance took twice its 32-column budget. I left truncate_str alone since it has two pre-existing callers at sandbox_draft.rs:506-507, and added a truncate_display used only by the guidance row.

Coverage

The render regression you asked for is cjk_guidance_is_fully_reachable_at_80x24. It interleaves 16 markers through the CJK text rather than appending one, because a trailing marker lands on its own short row in both the broken and fixed layouts and therefore proves nothing — I wrote that version first and it passed against the bug. Every marker must be reachable at some scroll offset.

I also checked the tests actually bite. Reverting display_width to chars().count():

test cjk_guidance_is_fully_reachable_at_80x24 ... FAILED
test wrap_value_measures_display_columns_not_chars ... FAILED
test truncate_display_counts_columns_not_chars ... FAILED
test result: FAILED. 67 passed; 3 failed

The first versions of the two unit tests asserted with display_width itself, which made them tautological — they passed against the reverted implementation. They now measure with an independent column oracle in the test module.

70 tests in openshell-tui, mise run ci green.

[johntmyers]

/ok to test b47d01e

[johntmyers]

gator-agent

PR Review Status

Thanks @vyncint. I checked the display-column wrapping, hard-break path, list-row truncation, footer packing, and the independent 80×24 CJK render regression in the latest delta. These changes resolve GATOR-30f8375a-01; the Gator-owned thread is now resolved, and no blocking findings remain.

Blocking findings:

• No blocking findings remain

Carried findings:

• GATOR-30f8375a-01: resolved by the display-width-aware wrapping and deterministic CJK reachability coverage.

Gator metadata

• Validation: Project-valid as a focused implementation of the rejected-guidance item in accepted issue #1098.
• Docs: Fern documentation updated under docs/sandboxes/policy-advisor.mdx; navigation change not needed.
• Checks: Current-head Branch Checks and Helm Lint are in progress after /ok to test; required workflow dispatch is confirmed.
• E2E: Not required for this presentation-only TUI change; a current-head Branch E2E workflow was also dispatched.
• Head SHA: b47d01e00d6972aba3e636edbad107af55564ce7
• Base SHA: 905e99aa2a960c17d5672b11cf64f10ff7d18b41
• Merge base SHA: 905e99aa2a960c17d5672b11cf64f10ff7d18b41
• Patch ID: f6f6b12d89879e1ff19e389dcc5bae9e3db1e819
• Gator payload: 7
• Review mode: follow_up
• Previous reviewed SHA: 03c67b18b405fd4c8d8204973efcc1474457f8a8
• Review budget exhausted: no
• Maintainer decision required: no
• Next state: gator:watch-pipeline