gitlore/plans/tui-prd-v2-frankentui.feedback-1.md

No ## Rejected Recommendations section was present, so these are all net-new improvements.

1. Keep core lore stable; isolate nightly to a TUI crate
   Rationale: the current plan says “whole project nightly” but later assumes TUI is feature-gated. Isolating nightly removes unnecessary risk from non-TUI users, CI, and release cadence.

@@ 3.2 Nightly Rust Strategy
-- The entire gitlore project moves to pinned nightly, not just the TUI feature.
+- Keep core `lore` on stable Rust.
+- Add workspace member `lore-tui` pinned to nightly for FrankenTUI.
+- Ship `lore tui` only when `--features tui` (or separate `lore-tui` binary) is enabled.

@@ 10.1 New Files
+- crates/lore-tui/Cargo.toml
+- crates/lore-tui/src/main.rs

@@ 11. Assumptions
-17. TUI module is feature-gated.
+17. TUI is isolated in a workspace crate and feature-gated in root CLI integration.

2. Add a framework adapter boundary from day 1
   Rationale: the “3-day ratatui escape hatch” is optimistic without a strict interface. A tiny UiRuntime + screen renderer trait makes fallback real, not aspirational.

@@ 4. Architecture
+### 4.9 UI Runtime Abstraction
+Introduce `UiRuntime` trait (`run`, `send`, `subscribe`) and `ScreenRenderer` trait.
+FrankenTUI implementation is default; ratatui adapter can be dropped in with no state/action rewrite.

@@ 3.5 Escape Hatch
-- The migration cost to ratatui is ~3 days
+- Migration cost target is ~3-5 days, validated by one ratatui spike screen in Phase 1.

3. Stop using CLI command modules as the TUI query API
   Rationale: coupling TUI to CLI output-era structs creates long-term friction and accidental regressions. Create a shared domain query layer used by both CLI and TUI.

@@ 10.20 Refactor: Extract Query Functions
-- extract query_* from cli/commands/*
+- introduce `src/domain/query/*` as the canonical read model API.
+- CLI and TUI both depend on domain query layer.
+- CLI modules retain formatting/output only.

@@ 10.2 Modified Files
+- src/domain/query/mod.rs
+- src/domain/query/issues.rs
+- src/domain/query/mrs.rs
+- src/domain/query/search.rs
+- src/domain/query/who.rs

4. Replace single Arc<Mutex<Connection>> with connection manager
   Rationale: one locked connection serializes everything and hurts responsiveness, especially during sync. Use separate read pool + writer connection with WAL and busy timeout.

@@ 4.4 App — Implementing the Model Trait
- pub db: Arc<Mutex<Connection>>,
+ pub db: Arc<DbManager>, // read pool + single writer coordination

@@ 4.5 Async Action System
- Each Cmd::task closure locks the mutex, runs the query, and returns a Msg
+ Reads use pooled read-only connections.
+ Sync/write path uses dedicated writer connection.
+ Enforce WAL, busy_timeout, and retry policy for SQLITE_BUSY.

5. Make debouncing/cancellation explicit and correct
   Rationale: “runtime coalesces rapid keypresses” is not a safe correctness guarantee. Add request IDs and stale-response dropping to prevent flicker and wrong data.

@@ 4.3 Core Types (Msg)
+ SearchRequestStarted { request_id: u64, query: String }
- SearchExecuted(SearchResults),
+ SearchExecuted { request_id: u64, results: SearchResults },

@@ 4.4 maybe_debounced_query()
- runtime coalesces rapid keypresses
+ use explicit 200ms debounce timer + monotonic request_id
+ ignore results whose request_id != current_search_request_id

6. Implement true streaming sync, not batch-at-end pseudo-streaming
   Rationale: the plan promises real-time logs/progress but code currently returns one completion message. This gap will disappoint users and complicate cancellation.

@@ 4.4 start_sync_task()
- Pragmatic approach: run sync synchronously, collect all progress events, return summary.
+ Use event channel subscription for `SyncProgress`/`SyncLogLine` streaming.
+ Keep `SyncCompleted` only as terminal event.
+ Add cooperative cancel token mapped to `Esc` while running.

@@ 5.9 Sync
+ Add "Resume from checkpoint" option for interrupted syncs.

7. Fix entity identity ambiguity across projects
   Rationale: using iid alone is unsafe in multi-project datasets. Navigation and cross-refs should key by (project_id, iid) or global ID.

@@ 4.3 Core Types
- IssueDetail(i64)
- MrDetail(i64)
+ IssueDetail(EntityKey)
+ MrDetail(EntityKey)

+ pub struct EntityKey { pub project_id: i64, pub iid: i64, pub kind: EntityKind }

@@ 10.12.4 Cross-Reference Widget
- parse "group/project#123" -> iid only
+ parse into `{project_path, iid, kind}` then resolve to `project_id` before navigation

8. Resolve keybinding conflicts and formalize keymap precedence
   Rationale: current spec conflicts (Tab sort vs focus filter; gg vs go-prefix). A deterministic keymap contract prevents UX bugs.

@@ 8.2 List Screens
- Tab | Cycle sort column
- f   | Focus filter bar
+ Tab | Focus filter bar
+ S   | Cycle sort column
+ /   | Focus filter bar (alias)

@@ 4.4 interpret_key()
+ Add explicit precedence table:
+ 1) modal/palette
+ 2) focused input
+ 3) global
+ 4) screen-local
+ Add configurable go-prefix timeout (default 500ms) with cancel feedback.

9. Add performance SLOs and DB/index plan
   Rationale: “fast enough” is vague. Add measurable budgets, required indexes, and query-plan gates in CI for predictable performance.

@@ 3.1 Risk Matrix
+ Add risk: "Query latency regressions on large datasets"

@@ 9.3 Phase 0 — Toolchain Gate
+7. p95 list query latency < 75ms on 100k issues synthetic fixture
+8. p95 search latency < 200ms on 1M docs (lexical mode)

@@ 11. Assumptions
-5. SQLite queries are fast enough for interactive use (<50ms for filtered results).
+5. Performance budgets are enforced by benchmark fixtures and query-plan checks.
+6. Required indexes documented and migration-backed before TUI GA.

10. Add reliability/observability model (error classes, retries, tracing)
    Rationale: one string toast is not enough for production debugging. Add typed errors, retry policy, and an in-TUI diagnostics pane.

@@ 4.3 Core Types (Msg)
- Error(String),
+ Error(AppError),

+ pub enum AppError {
+   DbBusy, DbCorruption, NetworkRateLimited, NetworkUnavailable,
+   AuthFailed, ParseError, Internal(String)
+ }

@@ 5.11 Doctor / Stats
+ Add "Diagnostics" tab:
+ - last 100 errors
+ - retry counts
+ - current sync/backoff state
+ - DB contention metrics

11. Add “Saved Views + Watchlist” as high-value product features
    Rationale: this makes the TUI compelling daily, not just navigable. Users can persist filters and monitor critical slices (e.g., “P1 auth issues updated in last 24h”).

@@ 1. Executive Summary
+ - Saved Views (named filters and layouts)
+ - Watchlist panel (tracked queries with delta badges)

@@ 5. Screen Taxonomy
+### 5.12 Saved Views / Watchlist
+Persistent named filters for Issues/MRs/Search.
+Dashboard shows per-watchlist deltas since last session.

@@ 6. User Flows
+### 6.9 Flow: "Run morning watchlist triage"
+Dashboard -> Watchlist -> filtered IssueList/MRList -> detail drilldown

12. Strengthen testing plan with deterministic behavior and chaos cases
    Rationale: snapshot tests alone won’t catch race/staleness/cancellation issues. Add concurrency, cancellation, and flaky terminal behavior tests.

@@ 9.2 Phases
+Phase 5.5 Reliability Test Pack (2d)
+ - stale response drop tests
+ - sync cancel/resume tests
+ - SQLITE_BUSY retry tests
+ - resize storm and rapid key-chord tests

@@ 10.9 Snapshot Test Example
+ Add non-snapshot tests:
+ - property tests for navigation invariants
+ - integration tests for request ordering correctness
+ - benchmark tests for query budgets

If you want, I can produce a consolidated “PRD v2.1 patch” with all of the above merged into one coherent updated document structure.