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

1. Fix the structural inconsistency between src/tui and crates/lore-tui/src Analysis: The PRD currently defines two different code layouts for the same system. That will cause implementation drift, wrong imports, and duplicated modules. Locking to one canonical layout early prevents execution churn and makes every snippet/action item unambiguous.

@@ 4.1 Module Structure @@
-src/
-  tui/
+crates/lore-tui/src/
     mod.rs
     app.rs
     message.rs
@@
-### 10.5 Dashboard View (FrankenTUI Native)
-// src/tui/view/dashboard.rs
+### 10.5 Dashboard View (FrankenTUI Native)
+// crates/lore-tui/src/view/dashboard.rs
@@
-### 10.6 Sync View
-// src/tui/view/sync.rs
+### 10.6 Sync View
+// crates/lore-tui/src/view/sync.rs

2. Add a small ui_adapter seam to contain FrankenTUI API churn Analysis: You already identified high likelihood of upstream breakage. Pinning a commit helps, but if every screen imports raw ftui_* types directly, churn ripples through dozens of files. A thin adapter layer reduces upgrade cost without introducing the rejected “full portability abstraction”.

@@ 3.1 Risk Matrix @@
 | API breaking changes | High | High (v0.x) | Pin exact git commit; vendor source if needed |
+| API breakage blast radius across app code | High | High | Constrain ftui usage behind `ui_adapter/*` wrappers |

@@ 4.1 Module Structure @@
+    ui_adapter/
+      mod.rs                    # Re-export stable local UI primitives
+      runtime.rs                # App launch/options wrappers
+      widgets.rs                # Table/List/Modal wrapper constructors
+      input.rs                  # Text input + focus helpers

@@ 9.3 Phase 0 — Toolchain Gate @@
+14. `ui_adapter` compile-check: no screen module imports `ftui_*` directly (lint-enforced)

3. Correct search mode behavior and replace sleep-based debounce with cancelable scheduling Analysis: Current plan hardcodes "hybrid" in execute_search, so mode switching is UI-only and incorrect. Also, spawning sleeping tasks per keypress is wasteful under fast typing. Make mode a first-class query parameter and debounce via one cancelable scheduled event per input domain.

@@ 4.4 maybe_debounced_query @@
-std::thread::sleep(std::time::Duration::from_millis(200));
-match crate::tui::action::execute_search(&conn, &query, &filters) {
+// no thread sleep; schedule SearchRequestStarted after 200ms via debounce scheduler
+match crate::tui::action::execute_search(&conn, &query, &filters, mode) {

@@ 10.11 Action Module — Query Bridge @@
-pub fn execute_search(conn: &Connection, query: &str, filters: &SearchCliFilters) -> Result<SearchResponse, LoreError> {
-    let mode_str = "hybrid"; // default; TUI mode selector overrides
+pub fn execute_search(
+    conn: &Connection,
+    query: &str,
+    filters: &SearchCliFilters,
+    mode: SearchMode,
+) -> Result<SearchResponse, LoreError> {
+    let mode_str = match mode {
+        SearchMode::Hybrid => "hybrid",
+        SearchMode::Lexical => "lexical",
+        SearchMode::Semantic => "semantic",
+    };

@@ 9.3 Phase 0 — Toolchain Gate @@
+15. Search mode parity: lexical/hybrid/semantic each return mode-consistent top-N IDs on fixture

4. Guarantee consistent multi-query reads and add query interruption for responsiveness Analysis: Detail screens combine multiple queries that can observe mixed states during sync writes. Wrap each detail fetch in a single read transaction for snapshot consistency. Add cancellation/interrupt checks for long-running queries so UI remains responsive under heavy datasets.

@@ 4.5 Async Action System @@
+All detail fetches (`issue_detail`, `mr_detail`, timeline expansion) run inside one read transaction
+to guarantee snapshot consistency across subqueries.

@@ 10.11 Action Module — Query Bridge @@
+pub fn with_read_snapshot<T>(
+    conn: &Connection,
+    f: impl FnOnce(&rusqlite::Transaction<'_>) -> Result<T, LoreError>,
+) -> Result<T, LoreError> { ... }

+// Long queries register interrupt checks tied to CancelToken
+// to avoid >1s uninterruptible stalls during rapid navigation/filtering.

5. Formalize sync event streaming contract to prevent “stuck” states Analysis: Dropping events on backpressure is acceptable, but completion must never be dropped and event ordering must be explicit. Add a typed SyncUiEvent stream with guaranteed terminal sentinel and progress coalescing to reduce load while preserving correctness.

@@ 4.4 start_sync_task @@
-let (tx, rx) = std::sync::mpsc::sync_channel::<Msg>(1024);
+let (tx, rx) = std::sync::mpsc::sync_channel::<SyncUiEvent>(2048);

-// drop this progress update rather than blocking the sync thread
+// coalesce progress to max 30Hz per lane; never drop terminal events
+// always emit SyncUiEvent::StreamClosed { outcome }

@@ 5.9 Sync @@
-- Log viewer with streaming output
+- Log viewer with streaming output and explicit stream-finalization state
+- UI shows dropped/coalesced event counters for transparency

6. Version and validate session restore payloads Analysis: A raw JSON session file without schema/version checks is fragile across releases and DB switches. Add schema version, DB fingerprint, and safe fallback rules so session restore never blocks startup or applies stale state incorrectly.

@@ 11. Assumptions @@
-12. Minimal TUI state file allowed for session restore only ...
+12. Versioned TUI state file allowed for session restore only:
+    fields include `schema_version`, `app_version`, `db_fingerprint`, `saved_at`, `state`.

@@ 10.1 New Files @@
 crates/lore-tui/src/session.rs         # Lightweight session state persistence
+                                        # + versioning, validation, corruption quarantine

@@ 4.1 Module Structure @@
     session.rs                    # Lightweight session state persistence
+                                  # corrupted file -> `.bad-<timestamp>` and fresh start

7. Harden terminal safety beyond ANSI stripping Analysis: ANSI stripping is necessary but not sufficient. Bidi controls and invisible Unicode controls can still spoof displayed content. URL checks should normalize host/port and disallow deceptive variants. This closes realistic terminal spoofing vectors.

@@ 3.1 Risk Matrix @@
 | Terminal escape/control-sequence injection via issue/note text | High | Medium | Strip ANSI/OSC/control chars via sanitize_for_terminal() ... |
+| Bidi/invisible Unicode spoofing in rendered text | High | Medium | Strip bidi overrides + zero-width controls in untrusted text |

@@ 10.4.1 Terminal Safety — Untrusted Text Sanitization @@
-Strip ANSI escape sequences, OSC commands, and control characters
+Strip ANSI/OSC/control chars, bidi overrides (RLO/LRO/PDF/RLI/LRI/FSI/PDI),
+and zero-width/invisible controls from untrusted text

-pub fn is_safe_url(url: &str, allowed_hosts: &[String]) -> bool {
+pub fn is_safe_url(url: &str, allowed_origins: &[Origin]) -> bool {
+    // normalize host (IDNA), enforce scheme+host+port match

8. Use progressive hydration for detail screens Analysis: Issue/MR detail first-paint can become slow when discussions are large. Split fetch into phases: metadata first, then discussions/file changes, then deep thread content on expand. This improves perceived performance and keeps navigation snappy on large repos.

@@ 5.3 Issue Detail @@
-Data source: `lore issues <iid>` + discussions + cross-references
+Data source (progressive):
+1) metadata/header (first paint)
+2) discussions summary + cross-refs
+3) full thread bodies loaded on demand when expanded

@@ 5.5 MR Detail @@
-Unique features: File changes list, Diff discussions ...
+Unique features (progressive hydration):
+- file change summary in first paint
+- diff discussion bodies loaded lazily per expanded thread

@@ 9.3 Phase 0 — Toolchain Gate @@
+16. Detail first-paint p95 < 75ms on M-tier fixtures (metadata-only phase)

9. Make reliability tests reproducible with deterministic clocks/seeds Analysis: Relative-time rendering and fuzz tests are currently tied to wall clock/randomness, which makes CI flakes hard to diagnose. Introduce a Clock abstraction and deterministic fuzz seeds with failure replay output.

@@ 10.9.1 Non-Snapshot Tests @@
+/// All time-based rendering uses injected `Clock` in tests.
+/// Fuzz failures print deterministic seed for replay.

@@ 9.2 Phase 5.5 — Reliability Test Pack @@
-Event fuzz tests (key/resize/paste):p55g
+Event fuzz tests (key/resize/paste, deterministic seed replay):p55g
+Deterministic clock/render tests:p55i

10. Add an “Actionable Insights” dashboard panel for stronger day-to-day utility Analysis: Current dashboard is informative, but not prioritizing. Adding ranked insights (stale P1s, blocked MRs, discussion hotspots) turns it into a decision surface, not just a metrics screen. This makes the TUI materially more compelling for triage workflows.

@@ 1. Executive Summary @@
 - Dashboard — sync status, project health, counts at a glance
+- Dashboard — sync status, project health, counts, and ranked actionable insights

@@ 5.1 Dashboard (Home Screen) @@
-│  Recent Activity                  │
+│  Recent Activity                  │
+│  Actionable Insights              │
+│  1) 7 opened P1 issues >14d       │
+│  2) 3 MRs blocked by unresolved   │
+│  3) auth/ has +42% note velocity  │

@@ 6. User Flows @@
+### 6.9 Flow: "Risk-first morning sweep"
+Dashboard -> select insight -> jump to pre-filtered list/detail

These 10 changes stay clear of your Rejected Recommendations list and materially improve correctness, operability, and product value without adding speculative architecture.