feat(tui): show ←/→ change hint for enum fields

[pedrofuentes]

Settings + Expert-form enum/option fields now show '←/→ change · Enter confirm · Esc cancel' when editing, so changing a value is discoverable (was only ←/→ with no hint); Phase 11 polish — feedback #3

[pedrofuentes]

Status: CONDITIONAL

Sentinel Review Report

Ref: feature/tui-enum-discover → main
PR: #1757 — #1757
Report ID: SENTINEL-1757-0039d33
Reviewed SHA: 0039d33
Sentinel ruleset: v1
Reviewed at: 2026-06-26T10:38:29-07:00
Mode: standard
Review depth: Tier 2 (full)
Verdict: CONDITIONAL
Required action: FILE_ISSUES_AND_MERGE

Phase 1 — TDD / Test Evidence

• Tests exist & meaningful: ✅ Rendered-screen tests drive the real SUT and assert the enum hint: packages/cli/tests/unit/tui/settings-screen.test.tsx:491-494 quotes stdin.write("\r"); // enter edit mode and expect(lastFrame() ?? "").toContain("←/→ change · Enter confirm · Esc cancel");; packages/cli/tests/unit/tui/expert-form-screen.test.tsx:507-510 quotes stdin.write("\u001B[C"); // right arrow and the same rendered hint assertion. Non-enum guard exists at settings-screen.test.tsx:503-506 quoting stdin.write("\r"); // enter edit mode and expect(lastFrame() ?? "").not.toContain("←/→ change");.
• Test-first history verified: ✅ git log --oneline origin/main..origin/feature/tui-enum-discover returned 0ad5ee3 test(tui): add failing tests for discoverable enum change hint before 0039d33 feat(tui): show ←/→ change hint for enum fields. Per-commit diffs verified 0ad5ee3 touched only the two test files; 0039d33 touched only SettingsScreen.tsx and ExpertFormScreen.tsx.
• Red test evidence: ✅ In detached worktree at 0ad5ee3, after npx --yes pnpm@10.30.1 install --frozen-lockfile --ignore-scripts --prefer-offline, command cd packages/cli && npx --yes pnpm@10.30.1 exec vitest run tests/unit/tui/settings-screen.test.tsx tests/unit/tui/expert-form-screen.test.tsx -t "shows ←/→ change hint|does not show" failed as expected: Test Files 2 failed (2) and Tests 2 failed | 1 passed | 26 skipped (29), with assertion output expected ... to contain '←/→ change · Enter confirm · Esc cancel'.
• Full suite green on SHA: ✅ cd packages/cli && npx --yes pnpm@10.30.1 test -- --reporter=dot on reviewed SHA returned Test Files 358 passed | 1 skipped (359) and Tests 4520 passed | 1 skipped (4521).
• Targeted suite green on SHA: ✅ cd packages/cli && npx --yes pnpm@10.30.1 exec vitest run tests/unit/tui returned Test Files 95 passed (95) and Tests 902 passed (902).
• Lint/typecheck: ✅ npx --yes pnpm@10.30.1 lint exited 0 (eslint src tests); npx --yes pnpm@10.30.1 typecheck exited 0 (tsc --noEmit).
• Coverage: N/A (no enforced coverage threshold configured for this PR; not run).
• Base/current-main check: ✅ git merge-base origin/main HEAD returned 4cf1b8c610c6f3f4e83d0cf692624568c4317add, matching origin/main fetched for the review.

Phase 1.5 — Fast-path Evaluation

🔴 count: 0 | LOC: 36 non-test source changed (≤150: Y) | Security paths: Y (TUI terminal rendering surface touched) | New deps: N | Commit types qualify: N (feat)
→ Fast-path eligible: NO → Phase 2

Phase 2 — Execution Log

Dim	Tool Call	Agent ID / Ref	Status
Quick scan	task(agent_type="explore", name="sentinel-quickscan")	N/A (sync task result exposes no ID)	✅ No 🔴 blockers found
A1 Security attacks	multi_tool_use.parallel → task(agent_type="general-purpose", name="sentinel-a1")	N/A (sync task result exposes no ID)	✅ No findings
A2 Security defenses	multi_tool_use.parallel → task(agent_type="general-purpose", name="sentinel-a2")	N/A (sync task result exposes no ID)	✅ No findings
B Resilience	multi_tool_use.parallel → task(agent_type="general-purpose", name="sentinel-b")	N/A (sync task result exposes no ID)	✅ No findings
C Performance/architecture	multi_tool_use.parallel → task(agent_type="general-purpose", name="sentinel-c")	N/A (sync task result exposes no ID)	✅ No findings
D Test quality	multi_tool_use.parallel → task(agent_type="general-purpose", name="sentinel-d")	N/A (sync task result exposes no ID)	✅ No findings from Dim D; orchestrator adopted one cross-dimension coverage concern surfaced during Phase 2
E Dependencies	N/A	N/A	✅ Auto-skip: no package manifests, lockfiles, package-manager config, Dockerfiles, CI/build scripts, or vendored code changed
F Documentation	multi_tool_use.parallel → task(agent_type="general-purpose", name="sentinel-f")	N/A (sync task result exposes no ID)	✅ No in-scope documentation findings; surfaced the cross-dimension coverage concern classified below
Persistence	python3 subprocess.run(["gh", "api", "repos/pedrofuentes/Council/issues/comments/4811933666", "-X", "PATCH", "-f", "body=<report>"])	#1757 (comment)	✅ Full report posted to PR

Additional local verification commands run:

• git fetch origin --quiet && git log --oneline origin/main..origin/feature/tui-enum-discover && git diff origin/main...origin/feature/tui-enum-discover — reviewed 2 commits and 4 changed files.
• git show --name-status --format='%H %s' 0ad5ee3 — only packages/cli/tests/unit/tui/expert-form-screen.test.tsx and packages/cli/tests/unit/tui/settings-screen.test.tsx changed.
• git show --name-status --format='%H %s' 0039d33 — only packages/cli/src/tui/screens/ExpertFormScreen.tsx and packages/cli/src/tui/screens/SettingsScreen.tsx changed.

Findings

• 🔴 CRITICAL: 0
• 🟡 IMPORTANT: 1 new / 0 known
• 🟢 MINOR: 0

Only 🟡 findings remain; no 🔴 blockers were found.

Details (ordered by severity)

1. [🟡 IMPORTANT] Positive regression coverage missing for non-enum edit hints — packages/cli/src/tui/screens/SettingsScreen.tsx:299-302; packages/cli/src/tui/screens/ExpertFormScreen.tsx:343-343
   • Evidence: SettingsScreen.tsx:299-302 adds the boolean/type hint paths:

     if (field.kind === "boolean") {
       return "Enter/Space toggle · Esc cancel";
     }
     return "type · Enter confirm · Esc cancel";

     ExpertFormScreen.tsx:343 adds the text fallback:

     return "type · Enter confirm · Esc cancel";

   • Impact: Trigger: a user edits a boolean, number, or text field. Mechanism: the PR changes those edit-mode hints, but tests only positively assert enum hints and only negatively assert that one string field does not show ←/→ change. Consequence: future regressions that render incorrect boolean/type hint copy can pass the current targeted tests, degrading keyboard discoverability for non-enum edit modes.
   • Remediation: File a follow-up issue to add rendered-screen positive assertions for the boolean hint and the type/text/number hint in Settings, and for the text fallback in ExpertForm, or otherwise constrain the implementation so those branches are covered by existing exact-output assertions.

Follow-ups & Actions

• CONDITIONAL → FILE_ISSUES_AND_MERGE: file an issue for the 🟡 finding above (sentinel:important), link it in PR feat(tui): show ←/→ change hint for enum fields #1757, then merge if policy allows.
• ⚠️ Do NOT fix 🟡/🟢 findings in this PR — file as issues only.

Decision rationale

• TDD choreography is compliant: failing test-only commit precedes implementation-only commit, and red evidence proves the new rendered enum-hint tests failed without the implementation.
• Final reviewed SHA 0039d333c8268ddd8aabe21f76a272e12a126208 passes targeted TUI tests, full CLI tests, lint, and typecheck.
• Enum detection uses the existing discriminant (field.kind === "enum") consistently with the screens' edit behavior; the Settings edit handler still guards actual cycling with selected.options ?? [], so non-enum fields do not receive the arrow-change hint.
• Nav-mode hint remains exactly ↑↓ move · Enter edit · Ctrl+S save · Esc back; new hint strings are trusted static literals, so terminal-output sanitization is not applicable.
• Scope is limited to SettingsScreen.tsx, ExpertFormScreen.tsx, and their two unit test files; no dependencies, public APIs, dynamic terminal rendering, or @github/copilot-sdk imports were introduced.

[pedrofuentes]

Sentinel SENTINEL-1757-0039d33 → CONDITIONAL (0 🔴). Follow-up filed: #1761 (🟡 positive hint coverage). Merging per CONDITIONAL policy.