Below are the strongest revisions I’d make. I intentionally avoided anything in your `## Rejected Recommendations`. 1. **Unify commands/keybindings/help/palette into one registry** Rationale: your plan currently duplicates action definitions across `execute_palette_action`, `ShowCliEquivalent`, help overlay text, and status hints. That will drift quickly and create correctness bugs. A single `CommandRegistry` makes behavior consistent and testable. ```diff diff --git a/PRD.md b/PRD.md @@ 4.1 Module Structure + commands.rs # Single source of truth for actions, keybindings, CLI equivalents @@ 4.4 App — Implementing the Model Trait - fn execute_palette_action(&self, action_id: &str) -> Cmd { ... big match ... } + fn execute_palette_action(&self, action_id: &str) -> Cmd { + if let Some(spec) = self.commands.get(action_id) { + return self.update(spec.to_msg(self.navigation.current())); + } + Cmd::none() + } @@ 8. Keybinding Reference +All keybinding/help/status/palette definitions are generated from `commands.rs`. +No hardcoded duplicate maps in view/state modules. ``` 2. **Replace ad-hoc key flags with explicit input state machine** Rationale: `pending_go` + `go_prefix_instant` is fragile and already inconsistent with documented behavior. A typed `InputMode` removes edge-case bugs and makes prefix timeout deterministic. ```diff diff --git a/PRD.md b/PRD.md @@ 4.4 LoreApp struct - pending_go: bool, - go_prefix_instant: Option, + input_mode: InputMode, // Normal | Text | Palette | GoPrefix { started_at } @@ 8.2 List Screens -| `g` `g` | Jump to top | +| `g` `g` | Jump to top (current list screen) | @@ 4.4 interpret_key - KeyCode::Char('g') => Msg::IssueListScrollToTop + KeyCode::Char('g') => Msg::ScrollToTopCurrentScreen ``` 3. **Fix TaskSupervisor contract and message schema drift** Rationale: the plan mixes `request_id` and `generation`, and `TaskKey::Search { generation }` defeats dedup by making every key unique. This can silently reintroduce stale-result races. ```diff diff --git a/PRD.md b/PRD.md @@ 4.3 Core Types (Msg) - SearchRequestStarted { request_id: u64, query: String }, - SearchExecuted { request_id: u64, results: SearchResults }, + SearchRequestStarted { generation: u64, query: String }, + SearchExecuted { generation: u64, results: SearchResults }, @@ 4.5.1 Task Supervisor - Search { generation: u64 }, + Search, + struct TaskStamp { key: TaskKey, generation: u64 } @@ 10.9.1 Non-Snapshot Tests - Msg::SearchExecuted { request_id: 3, ... } + Msg::SearchExecuted { generation: 3, ... } ``` 4. **Add a `Clock` boundary everywhere time is computed** Rationale: you call `SystemTime::now()` in many query/render paths, causing inconsistent relative-time labels inside one frame and flaky tests. Injected clock gives deterministic rendering and lower per-frame overhead. ```diff diff --git a/PRD.md b/PRD.md @@ 4.1 Module Structure + clock.rs # Clock trait: SystemClock/FakeClock @@ 4.4 LoreApp struct + clock: Arc, @@ 10.11 action.rs - let now_ms = std::time::SystemTime::now()... + let now_ms = clock.now_ms(); @@ 9.3 Phase 0 success criteria +19. Relative-time rendering deterministic under FakeClock across snapshot runs. ``` 5. **Upgrade text truncation to grapheme-safe width handling** Rationale: `unicode-width` alone is not enough for safe truncation; it can split grapheme clusters (emoji ZWJ sequences, skin tones, flags). You need width + grapheme segmentation together. ```diff diff --git a/PRD.md b/PRD.md @@ 10.1 New Files -crates/lore-tui/src/text_width.rs # ... using unicode-width crate +crates/lore-tui/src/text_width.rs # Grapheme-safe width/truncation using unicode-width + unicode-segmentation @@ 10.1 New Files +Cargo.toml (lore-tui): unicode-segmentation = "1" @@ 9.3 Phase 0 success criteria +20. Unicode rendering tests pass for CJK, emoji ZWJ, combining marks, RTL text. ``` 6. **Redact sensitive values in logs and crash dumps** Rationale: current crash/log strategy risks storing tokens/credentials in plain text. This is a serious operational/security gap for local tooling too. ```diff diff --git a/PRD.md b/PRD.md @@ 4.1 Module Structure safety.rs # sanitize_for_terminal(), safe_url_policy() + redact.rs # redact_sensitive() for logs/crash reports @@ 10.3 install_panic_hook_for_tui - let _ = std::fs::write(&crash_path, format!("{panic_info:#?}")); + let report = redact_sensitive(format!("{panic_info:#?}")); + let _ = std::fs::write(&crash_path, report); @@ 9.3 Phase 0 success criteria +21. Redaction tests confirm tokens/Authorization headers never appear in persisted crash/log artifacts. ``` 7. **Add search capability detection and mode fallback UX** Rationale: semantic/hybrid mode should not silently degrade when embeddings are absent/stale. Explicit capability state increases trust and avoids “why are results weird?” confusion. ```diff diff --git a/PRD.md b/PRD.md @@ 5.6 Search +Capability-aware modes: +- If embeddings unavailable/stale, semantic mode is disabled with inline reason. +- Hybrid mode auto-falls back to lexical and shows badge: "semantic unavailable". @@ 4.3 Core Types + SearchCapabilitiesLoaded(SearchCapabilities) @@ 9.3 Phase 0 success criteria +22. Mode availability checks validated: lexical/hybrid/semantic correctly enabled/disabled by fixture capabilities. ``` 8. **Define sync cancel latency SLO and enforce fine-grained checks** Rationale: “check cancel between phases” is too coarse on big projects. Users need fast cancel acknowledgment and bounded stop time. ```diff diff --git a/PRD.md b/PRD.md @@ 5.9 Sync -CANCELLATION: checked between sync phases +CANCELLATION: checked at page boundaries, batch upsert boundaries, and before each network request. +UX target: cancel acknowledged <250ms, sync stop p95 <2s after Esc. @@ 9.3 Phase 0 success criteria +23. Cancel latency test passes: p95 stop time <2s under M-tier fixtures. ``` 9. **Add a “Hotspots” screen for risk/churn triage** Rationale: this is high-value and uses existing data (events, unresolved discussions, stale items). It makes the TUI more compelling without needing new sync tables or rejected features. ```diff diff --git a/PRD.md b/PRD.md @@ 1. Executive Summary +- **Hotspots** — file/path risk ranking by churn × unresolved discussion pressure × staleness @@ 5. Screen Taxonomy +### 5.12 Hotspots +Shows top risky paths with drill-down to related issues/MRs/timeline. @@ 8.1 Global +| `gx` | Go to Hotspots | @@ 10.1 New Files +crates/lore-tui/src/state/hotspots.rs +crates/lore-tui/src/view/hotspots.rs ``` 10. **Add degraded startup mode when compat/schema checks fail** Rationale: hard-exit on mismatch blocks users. A degraded mode that shells to `lore --robot` for read-only summary/doctor keeps the product usable and gives guided recovery. ```diff diff --git a/PRD.md b/PRD.md @@ 3.2 Nightly Rust Strategy - On mismatch: actionable error and exit + On mismatch: actionable error with `--degraded` option. + `--degraded` launches limited TUI (Dashboard/Doctor/Stats via `lore --robot` subprocess calls). @@ 10.3 TuiCli + /// Allow limited mode when schema/compat checks fail + #[arg(long)] + degraded: bool, ``` 11. **Harden query-plan CI checks (don’t rely on `SCAN TABLE` string matching)** Rationale: SQLite planner text varies by version. Parse opcode structure and assert index usage semantically; otherwise CI will be flaky or miss regressions. ```diff diff --git a/PRD.md b/PRD.md @@ 9.3.1 Required Indexes (CI enforcement) - asserts that none show `SCAN TABLE` + parses EXPLAIN QUERY PLAN rows and asserts: + - top-level loop uses expected index families + - no full scan on primary entity tables under default and top filter combos + - join order remains bounded (no accidental cartesian expansions) ``` 12. **Enforce single-instance lock for session/state safety** Rationale: assumption says no concurrent TUI sessions, but accidental double-launch will still happen. Locking prevents state corruption and confusing interleaved sync actions. ```diff diff --git a/PRD.md b/PRD.md @@ 10.1 New Files +crates/lore-tui/src/instance_lock.rs # lock file with stale-lock recovery @@ 11. Assumptions -21. No concurrent TUI sessions. +21. Concurrent sessions unsupported and actively prevented by instance lock (with clear error message). ``` If you want, I can turn this into a consolidated patched PRD (single unified diff) next.