Below are my strongest revisions, focused on correctness, reliability, and long-term maintainability, while avoiding all items in your `## Rejected Recommendations`. 1. **Fix the Cargo/toolchain architecture (current plan has a real dependency-cycle risk and shaky per-member toolchain behavior).** Analysis: The current plan has `lore -> lore-tui (optional)` and `lore-tui -> lore`, which creates a package cycle when `tui` is enabled. Also, per-member `rust-toolchain.toml` in a workspace is easy to misapply in CI/dev workflows. The cleanest robust shape is: `lore-tui` is a separate binary crate (nightly), `lore` remains stable and delegates at runtime (`lore tui` shells out to `lore-tui`). ```diff --- a/Gitlore_TUI_PRD_v2.md +++ b/Gitlore_TUI_PRD_v2.md @@ 3.2 Nightly Rust Strategy -- The `lore` binary integrates TUI via `lore tui` subcommand. The `lore-tui` crate is a library dependency feature-gated in the root. +- `lore-tui` is a separate binary crate built on pinned nightly. +- `lore` (stable) does not compile-link `lore-tui`; `lore tui` delegates by spawning `lore-tui`. +- This removes Cargo dependency-cycle risk and keeps stable builds nightly-free. @@ 9.1 Dependency Changes -[features] -tui = ["dep:lore-tui"] -[dependencies] -lore-tui = { path = "crates/lore-tui", optional = true } +[dependencies] +# no compile-time dependency on lore-tui from lore +# runtime delegation keeps toolchains isolated @@ 10.19 CLI Integration -Add Tui match arm that directly calls crate::tui::launch_tui(...) +Add Tui match arm that resolves and spawns `lore-tui` with passthrough args. +If missing, print actionable install/build command. ``` 2. **Make `TaskSupervisor` the *actual* single async path (remove contradictory direct `Cmd::task` usage in state handlers).** Analysis: You declare “direct `Cmd::task` is prohibited outside supervisor,” but later `handle_screen_msg` still launches tasks directly. That contradiction will reintroduce stale-result bugs and race conditions. Make state handlers pure (intent-only); all async launch/cancel/dedup goes through one supervised API. ```diff --- a/Gitlore_TUI_PRD_v2.md +++ b/Gitlore_TUI_PRD_v2.md @@ 4.5.1 Task Supervisor -The supervisor is the ONLY allowed path for background work. +The supervisor is the ONLY allowed path for background work, enforced by architecture: +`AppState` emits intents only; `LoreApp::update` launches tasks via `spawn_task(...)`. @@ 10.10 State Module — Complete -pub fn handle_screen_msg(..., db: &Arc>) -> Cmd +pub fn handle_screen_msg(...) -> ScreenIntent +// no DB access, no Cmd::task in state layer ``` 3. **Enforce `EntityKey` everywhere (remove raw IID navigation paths).** Analysis: Multi-project identity is one of your strongest ideas, but multiple snippets still navigate by bare IID (`document_id`, `EntityRef::Issue(i64)`). That can misroute across projects and create silent correctness bugs. Make all navigation-bearing results carry `EntityKey` end-to-end. ```diff --- a/Gitlore_TUI_PRD_v2.md +++ b/Gitlore_TUI_PRD_v2.md @@ 4.3 Core Types -pub enum EntityRef { Issue(i64), MergeRequest(i64) } +pub enum EntityRef { Issue(EntityKey), MergeRequest(EntityKey) } @@ 10.10 state/search.rs -Some(Msg::NavigateTo(Screen::IssueDetail(r.document_id))) +Some(Msg::NavigateTo(Screen::IssueDetail(r.entity_key.clone()))) @@ 10.11 action.rs -pub fn fetch_issue_detail(conn: &Connection, iid: i64) -> Result +pub fn fetch_issue_detail(conn: &Connection, key: &EntityKey) -> Result ``` 4. **Introduce a shared query boundary inside the existing crate (not a new crate) to decouple TUI from CLI presentation structs.** Analysis: Reusing CLI command modules directly is fast initially, but it ties TUI to output-layer types and command concerns. A minimal internal `core::query::*` module gives a stable data contract used by both CLI and TUI without the overhead of a new crate split. ```diff --- a/Gitlore_TUI_PRD_v2.md +++ b/Gitlore_TUI_PRD_v2.md @@ 10.2 Modified Files -src/cli/commands/list.rs # extract query_issues/query_mrs as pub -src/cli/commands/show.rs # extract query_issue_detail/query_mr_detail as pub +src/core/query/mod.rs +src/core/query/issues.rs +src/core/query/mrs.rs +src/core/query/detail.rs +src/core/query/search.rs +src/core/query/who.rs +src/cli/commands/* now call core::query::* + format output +TUI action.rs calls core::query::* directly ``` 5. **Add terminal-safety sanitization for untrusted text (ANSI/OSC injection hardening).** Analysis: Issue/MR bodies, notes, and logs are untrusted text in a terminal context. Without sanitization, terminal escape/control sequences can spoof UI or trigger unintended behavior. Add explicit sanitization and a strict URL policy before rendering/opening. ```diff --- a/Gitlore_TUI_PRD_v2.md +++ b/Gitlore_TUI_PRD_v2.md @@ 3.1 Risk Matrix +| Terminal escape/control-sequence injection via issue/note text | High | Medium | Strip ANSI/OSC/control chars before render; escape markdown output; allowlist URL scheme+host | @@ 4.1 Module Structure + safety.rs # sanitize_for_terminal(), safe_url_policy() @@ 10.5/10.8/10.14/10.16 +All user-sourced text passes through `sanitize_for_terminal()` before widget rendering. +Disable markdown raw HTML and clickable links unless URL policy passes. ``` 6. **Move resumable sync checkpoints into v1 (lightweight version).** Analysis: You already identify interruption risk as real. Deferring resumability to post-v1 leaves a major reliability gap in exactly the heaviest workflow. A lightweight checkpoint table (resource cursor + updated-at watermark) gives large reliability gain with modest complexity. ```diff --- a/Gitlore_TUI_PRD_v2.md +++ b/Gitlore_TUI_PRD_v2.md @@ 3.1 Risk Matrix -- Resumable checkpoints planned for post-v1 +Resumable checkpoints included in v1 (lightweight cursors per project/resource lane) @@ 9.3 Success Criteria +14. Interrupt-and-resume test: sync resumes from checkpoint and reaches completion without full restart. @@ 9.3.1 Required Indexes (GA Blocker) +CREATE TABLE IF NOT EXISTS sync_checkpoints ( + project_id INTEGER NOT NULL, + lane TEXT NOT NULL, + cursor TEXT, + updated_at_ms INTEGER NOT NULL, + PRIMARY KEY (project_id, lane) +); ``` 7. **Strengthen performance gates with tiered fixtures and memory ceilings.** Analysis: Current thresholds are good, but fixture sizes are too close to mid-scale only. Add S/M/L fixtures and memory budget checks so regressions appear before real-world datasets hit them. This gives much more confidence in long-term scalability. ```diff --- a/Gitlore_TUI_PRD_v2.md +++ b/Gitlore_TUI_PRD_v2.md @@ 9.3 Phase 0 — Toolchain Gate -7. p95 first-paint latency < 50ms ... (100k issues, 50k MRs) -10. p95 search latency < 200ms ... (50k documents) +7. Tiered fixtures: + S: 10k issues / 5k MRs / 50k notes + M: 100k issues / 50k MRs / 500k notes + L: 250k issues / 100k MRs / 1M notes + Enforce p95 targets per tier and memory ceiling (<250MB RSS in M tier). +10. Search SLO validated in S and M tiers, lexical and hybrid modes. ``` 8. **Add session restore (last screen + filters + selection), with explicit `--fresh` opt-out.** Analysis: This is high-value daily UX with low complexity, and it makes the TUI feel materially more “compelling/useful” without feature bloat. It also reduces friction when recovering from crash/restart. ```diff --- a/Gitlore_TUI_PRD_v2.md +++ b/Gitlore_TUI_PRD_v2.md @@ 1. Executive Summary +- **Session restore** — resume last screen, filters, and selection on startup. @@ 4.1 Module Structure + session.rs # persisted UI session state @@ 8.1 Global +| `Ctrl+R` | Reset session state for current screen | @@ 10.19 CLI Integration +`lore tui --fresh` starts without restoring prior session state. @@ 11. Assumptions -12. No TUI-specific configuration initially. +12. Minimal TUI state file is allowed for session restore only. ``` 9. **Add parity tests between TUI data panels and `--robot` outputs.** Analysis: You already have `ShowCliEquivalent`; parity tests make that claim trustworthy and prevent drift between interfaces. This is a strong reliability multiplier and helps future refactors. ```diff --- a/Gitlore_TUI_PRD_v2.md +++ b/Gitlore_TUI_PRD_v2.md @@ 9.2 Phases / 9.3 Success Criteria +Phase 5.6 — CLI/TUI Parity Pack + - Dashboard count parity vs `lore --robot count/status` + - List/detail parity for issues/MRs on sampled entities + - Search result identity parity (top-N ids) for lexical mode +Success criterion: parity suite passes on CI fixtures. ``` If you want, I can produce a single consolidated patch of the PRD text (one unified diff) so you can drop it directly into the next iteration.