§1: reroute bare rocm/chat → dash chat + parity gate (tui.rs RETAINED)

[michaelroy-amd]

Stack 9/10 · base: supergoal/phase-8-provider-anthropic

• Bare rocm and interactive rocm chat now route to the dash chat (dash::run_chat); --chat-mock + prompt passthrough preserved.
• tui.rs RETAINED (parity then gate) via a single #[allow(dead_code)] retention anchor — deletion is a human step in docs/tui-retirement-checklist.md (NOT performed here).
• Finalizes docs/dash-parity-map.md (30 covered) + adds docs/dash-parity-checklist.md.

Reviewed: rust + maintainability. All cargo gates green.

[rominf]

Approve with minor comments. Reroute + tui.rs retention is well-documented (single dead-code anchor, retirement checklist).

Notes:

• Compat change: interactive rocm chat --provider/--model no longer presets the session (dash auto-detects + /provider switches). Recoverable, but worth a changelog/user-doc line rather than only a code comment. Same for --chat-mock being interactive-only (silently ignored on the render path).
• The parity-gate tests assert on main.rs source text (brace-slicing + .contains) — genuine (fail on revert) but brittle: a rustfmt reformat or rename breaks them with no behavioral regression. Acceptable stopgap given the paths need a TTY; flagging as test debt. assert_ne!(target as usize, 0) is a tautology — the compile-time signature coercion above it is the real guard.

[rominf]

Requesting changes on signing grounds — this is separate from the code review above, which still stands.

Every commit on this PR is unsigned: GitHub reports verification.verified = false (reason unsigned) for all of them. Signed commits are required before this can merge.

To fix, configure commit signing and re-sign the branch history, e.g. with SSH signing:

git config gpg.format ssh
git config user.signingkey <your-signing-key.pub>
git config commit.gpgsign true
git rebase --exec 'git commit --amend --no-edit -S' <base-branch>
git push --force-with-lease

Since this is a stacked PR, re-signing is easiest from the bottom of the stack upward — re-signing a base branch rewrites its SHAs and the dependent branches will need rebasing/re-pushing on top.

Once the commits show as Verified I'll clear this.

[rominf]

Signing gate cleared — all commits on this PR now show Verified (verification.verified = true, reason valid). Re-reviewed the current diff against this PR's own base after the re-sign; it's substantively unchanged from my prior review.

Approving on both grounds: code is sound and commits are signed.

[Copilot]

Pull request overview

Routes the default interactive rocm entrypoint and interactive rocm chat (no --prompt) to the unified dash chat (dash::run_chat focusing ActiveTab::Chat), while intentionally retaining the legacy tui.rs behind a single retention anchor and documenting the human deletion gate once parity is accepted.

Changes:

• Reroutes bare rocm and interactive rocm chat to dash::run_chat, adds --chat-mock passthrough, and pins tui.rs reachability via a retention anchor.
• Adds dash::run_chat and threads initial_tab into the dash async launcher to support chat-first launches.
• Adds/updates parity + retirement documentation (parity map, parity checklist, and a human-only deletion checklist).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
docs/tui-retirement-checklist.md	Adds the human-only go/no-go procedure and explains the retention anchor.
docs/dash-parity-map.md	Updates parity map with Phase 9 routing note and links to the retirement checklist.
docs/dash-parity-checklist.md	Adds the accept/retire gate checklist summarizing parity evidence.
apps/rocm/src/main.rs	Reroutes interactive entrypoints to dash chat, adds --chat-mock, adds retention anchor + routing verification tests.
apps/rocm/src/dash.rs	Adds run_chat entrypoint and plumbs initial_tab into run_async/resolved_args.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rominf]

Re-approving the current head after the cascade rebase onto the updated base — refreshing my prior approval onto this exact commit.

Re-reviewed: reroute bare rocm + interactive rocm chat → dash::run_chat; legacy tui.rs retained behind a single anchor with a human-only deletion gate.

Copilot re-reviewed this same commit (COMMENTED, no blocking findings). No clear issues; LGTM.

[michaelroy-amd]

Reviewed the Copilot suggestions here (simplicity-first / ponytail pass) — dismissing all three; nothing to change on this PR.

• --provider ignored in interactive rocm chat — intentional design. Interactive chat routes to the unified dash where /provider is the switch; --provider is honored on the non-interactive render path. Documented in the code comment at the routing site.
• Routing tests string-match main.rs source — valid smell, but rewriting passing tests mid-merge is churn/risk for little gain. Deferred to a dedicated test-improvement pass, not this stack.
• run_chat duplicates the AppPaths/runtime build — legitimate dedup, but it's a refactor and out of scope for landing this stack. Captured as a follow-up cleanup (a shared runtime-builder helper).

[volen-silo]

Reviewed from the diff — clean, well-documented reroute. One real UX regression to address, plus a test-robustness suggestion. Not blocking.

🟡 --provider is silently ignored on the interactive path

Previously the interactive launch honored it: tui::run(provider.map(provider_name).map(str::to_owned)). Now the branch calls dash::run_chat(chat_mock), whose signature is run_chat(chat_mock: bool) — provider is dropped. So rocm chat --provider anthropic (interactive) starts on auto-detect, not anthropic. clap still accepts the flag, so it's silently ignored — no error, no effect. The code comment documents this intentionally, but a user can't see a comment.

Pick one:

• Thread it through: run_chat(provider, chat_mock) to set the starting backend (the dash already supports /provider to switch afterward), or
• If intentionally deferred, emit a one-line notice when --provider is passed interactively ("ignoring --provider; use /provider in the dash").

Same question for --tools on the interactive branch — confirm it isn't silently dropped too.

🟡 Source-introspection tests are brittle

launch_default_routes_to_dash_run_chat_not_tui et al. read main.rs at test time and hand-roll brace balancing to extract handler bodies, then string-match. Clever given the TTY constraint, but it breaks on any harmless refactor (e.g. extracting the arm into a fn) even when behavior is unchanged, and the brace counter doesn't account for braces inside string/char literals (fine today, latent).

More robust: extract the routing decision into a pure function returning an enum (e.g. chat_route(interactive, prompt, chat_mock) -> ChatRoute), call it from both entrypoints, and unit-test that — assertions key on real logic, not source text. Worth doing before the eventual tui.rs deletion churns these strings.

🟢 Good

• Retention anchor (_RETAINED_TUI_ENTRY) is a clean way to keep the frozen module reachable under -D warnings without a dead_code cascade, and tui_run_is_referenced_only_by_the_retention_anchor locks it to exactly one reference.
• run_chat reuses the same run_async path (replay=None ⇒ live executor + embedded daemon), so chat parity inherits the Phase 2/3 seam correctly.
• The retirement checklist is excellent — compiler-driven pruning, "don't run automatically," rollback section. Right way to stage a risky deletion.

Notes

• CI hasn't reported on this branch yet. With the license-header hawkeye hook now on main (#51), rebase this stack so hawkeye/fmt run clean here too.
• Reviewed from the diff only; I didn't check out the phase-8 stack, so I haven't run its tests or verified the parity-checklist's cross-phase evidence pointers.