From f52124bcf7a4a84b8ec8244065dac063740ba6f8 Mon Sep 17 00:00:00 2001 From: LLLin000 <809867916@qq.com> Date: Fri, 10 Jul 2026 23:01:57 +0800 Subject: [PATCH] =?UTF-8?q?docs:=20retrieval=20recovery=20design=20?= =?UTF-8?q?=E2=80=94=20Approach=20A=20(Python=20CLI=20sole=20owner)=20reco?= =?UTF-8?q?mmended?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .planning/retrieval-approach-a-python-cli.md | 195 +++++++++++++ .../2026-07-10-retrieval-recovery-design.md | 274 ++++++++++++++++++ 2 files changed, 469 insertions(+) create mode 100644 .planning/retrieval-approach-a-python-cli.md create mode 100644 docs/superpowers/specs/2026-07-10-retrieval-recovery-design.md diff --git a/.planning/retrieval-approach-a-python-cli.md b/.planning/retrieval-approach-a-python-cli.md new file mode 100644 index 00000000..de99767c --- /dev/null +++ b/.planning/retrieval-approach-a-python-cli.md @@ -0,0 +1,195 @@ +# Approach A — Python CLI as Sole Retrieval Owner + +**Date:** 2026-07-10 +**Author:** Main (synthesized from latency probes in failure matrix + architecture drift audit) +**Scope:** Evaluate Approach A for the PaperForge Retrieval Experience: **Python CLI is the only query/build execution owner. Plugin invokes a fresh CLI process for M, @, status, build, and control; sql.js is deleted; SQLite/FTS/sqlite-vec and build_state stay Python-owned.** +**Non-goals:** daemon process; sql.js; native Node SQLite module; Chroma migration. + +--- + +## Verdict + +**Viability: Strongly viable — the simplest correct path.** + +**Recommendation: Implement as the first recovery phase regardless of which long-term architecture is chosen.** Approach A is the only option that can restore retrieval correctness within a single session without introducing new IPC surface or lifecycle complexity. It can later be upgraded to Approach B (add sql.js as read-only cache) or Approach C (add worker) once correctness is proven. + +**Fatal risk:** The only fatal risk is the same as today — contract drift between Python payloads and TypeScript parsers. This is mitigated by a unified PFResult envelope enforced by contract tests. + +--- + +## Exact ownership boundary + +### Python owns everything retrieval (canonical) + +- **Storage:** `paperforge.db` (FTS, vec0, meta, build_state) +- **Schema:** `paperforge/memory/schema.py` +- **M query execution:** `paperforge search --json` +- **@ query execution:** `paperforge retrieve --deep --json` +- **Build execution:** `paperforge embed build|status|stop` +- **Status/health:** `paperforge embed status --json` +- **Result envelope:** one unified `PFResult` across all search/retrieve paths +- **Error taxonomy:** `ok: false` with `error.code` and `error.message` + +### Plugin owns UI only + +- spawn Python CLI per query +- parse one result envelope (`PFResult`) +- render cards, progress, errors +- manage debounce and abortion (kill child process) + +### Explicitly deleted + +- `paperforge/plugin/src/services/db.ts` (sql.js service) +- `paperforge/plugin/sql-wasm.wasm` +- `sql.js` from `package.json` +- All sql.js initialization/fallback branches in dashboard +- `vector-runtime-state.json` as authoritative UI truth (may remain as debug snapshot) +- Legacy Chroma production gates in resume/force/delete/status + +--- + +## Observed latency + +All measurements from the live Literature-hub vault (D:/L/OB/Literature-hub, 868 papers, 88.9 MB paperforge.db): + +| Operation | Time | Notes | +|-----------|------|-------| +| Bare Python child spawn (`python -c pass`) | 73 ms | Floor cost per CLI call | +| `paperforge.cli` import from fresh process | 116 ms | Import chain: config 18ms, db 28ms, search 6ms | +| Full M metadata search spawn (`search knee --json`) | **131 ms** | Cold end-to-end | +| Direct SQLite FTS5 JOIN query (5 results) | 16.4 ms | Query-only, no spawn | +| sqlite-vec extension load | 102.1 ms | One-time per process | + +### User-visible latency under Approach A + +| Interaction | Expected latency | Perceived | +|-------------|-----------------|-----------| +| Debounced M search (200ms timer) | ~130 ms from spawn | **~330 ms total** (200ms debounce + 130ms spawn) | +| Enter-triggered M search | ~130 ms | Acceptable for explicit action | +| @ deep search (cold) | ~200-300 ms (spawn + vec load + API) | Acceptable | +| @ deep search (warm, sequential) | ~130 ms + API | Acceptable | +| Status/health check | ~130 ms | On settings load — fine | +| Build start | ~130 ms | One-time cost | + +### Is ~330ms debounce acceptable? + +Yes. The current debounce is 200ms. Adding 130ms spawn gives ~330ms from last keystroke to results. This is within the 200-500ms range where users perceive "instant." The sql.js path was intended to skip the 130ms spawn, but it never worked (Row 1 — `paper_fts.year` missing), so the real user experience has always been CLI fallback anyway. + +--- + +## P0/P1 matrix resolution + +| Row | How Approach A fixes it | +|-----|------------------------| +| P0 R1: sql.js `paper_fts.year` | **Resolved by deletion.** No JS-owned SQL. Python owns the query next to the schema. | +| P0 R2: missing `--deep` flag | **Resolved.** Plugin passes `--deep` for `@` queries. One flag, one path. | +| P0 R3: `data.chunks` vs `matches` | **Resolved by unification.** Both search and retrieve return `data.matches` with stable field names. | +| P0 R5: delete-after-write bug | **Must fix independently.** Same Python build loop. Requires transaction-level fix. | +| P0 R4: `text` vs `matched_text` | **Resolved by unification.** One field name in the envelope. | +| P1 R6: resume gates on Chroma | **Must fix in build path.** Delete Chroma gating, check vec0 tables. | +| P1 R7: force deletes Chroma dir | **Must fix in build path.** Drop vec0 tables, not legacy dir. | +| P1 R8: dead PID stop unsettled | **Must fix in build path.** Cooperative cancellation + state settlement. | +| P1 R9: JSON vs SQLite split-brain | **Resolved.** Plugin reads `embed status --json` which queries live SQLite. JSON snapshot becomes debug-only. | +| P1 R10: meta-only health | **Must fix in status path.** Add vec0 queryability probe. | + +--- + +## Result envelope — unified PFResult + +All retrieval paths (M search, @ deep) return the same outer shape: + +```json +{ + "ok": true, + "data": { + "query": "knee", + "matches": [ + { + "zotero_key": "...", + "title": "...", + "first_author": "...", + "year": 2024, + "journal": "...", + "domain": "...", + "abstract": "...", + "score": 0.95, + "text": "...", + "heading": "...", + "source": "fulltext" + } + ] + }, + "count": 5, + "warnings": [] +} +``` + +Error envelope: + +```json +{ + "ok": false, + "error": { + "code": "VECTOR_CORRUPTED", + "message": "Vector index is unreadable. Rebuild vectors before retrieving.", + "details": {} + } +} +``` + +Error codes: `VECTOR_NOT_BUILT`, `VECTOR_CORRUPTED`, `MODEL_CHANGED`, `BACKEND_UNAVAILABLE`, `TIMEOUT`, `INTERNAL_ERROR`, `NO_PYTHON`. + +--- + +## Build state flow + +1. Plugin spawns `paperforge embed build --resume` (or `--force`). +2. Python writes `build_state` to SQLite as the only truth. +3. Plugin polls `paperforge embed status --json` periodically during build. +4. `embed status` reads live SQLite `build_state` and vec0 queryability, returns fresh PFResult. +5. Plugin renders progress bar and state from the status response. +6. On stop: plugin spawns `paperforge embed stop --json`. Python kills PID, writes `status=idle` to SQLite, returns settlement confirmation. +7. `vector-runtime-state.json` becomes a debug-only snapshot written by `embed status` for human inspection. Plugin never reads it as control-plane truth. + +--- + +## Minimal implementation sequence + +1. **Unify result envelope.** Make `paperforge search` and `paperforge retrieve` return the same `data.matches` key with the same field names (`first_author`, `text`). Add `--deep` flag plumbing to retrieve. +2. **Fix plugin parser.** Remove `matches`/`results`/`chunks` forking. Parse only `data.matches`. Remove `matched_text` check — use `text`. +3. **Fix plugin spawn.** Pass `--deep` for `@` queries. Delete sql.js service. +4. **Fix build path.** Transactional write-then-delete. Delete Chroma gating from resume/force. Add vec0 queryability check to status. Cooperative stop with state settlement. +5. **Fix plugin state.** Read live `embed status --json` instead of JSON snapshot. Poll during build. +6. **Add contract tests.** E2E: plugin spawn → CLI output → parse → render. Test every error code. + +--- + +## Approximate files affected + +- **Delete:** `paperforge/plugin/src/services/db.ts`, `paperforge/plugin/sql-wasm.wasm` +- **Modify plugin:** `dashboard.ts` (search spawn + parsing + state), `settings.ts` (build state), `memory-state.ts` (delete snapshot-as-truth), `python-bridge.ts` (unified spawn helper) +- **Modify Python:** `search.py` (unified envelope), `retrieve.py` (unified envelope + `--deep`), `embed.py` (fix build loop, Chroma cleanup, cooperative stop, status probe), `build_state.py` (canonical truth), `status.py` (vec0 queryability), `_chroma.py` (delete legacy production gates), `state_snapshot.py` (downgrade to debug-only) +- **New:** contract test file for envelope schema + +--- + +## Comparison to Approaches B and C + +| Dimension | A (CLI only) | B (sql.js cache) | C (worker) | +|-----------|-------------|-------------------|------------| +| Correctness risk | Lowest — one code path | Medium — staleness risk | Medium — IPC/lifecycle risk | +| Implementation complexity | Lowest | Medium | High | +| M search latency | ~330ms (debounce+spawn) | ~200ms (sql.js hit) | ~10-25ms (worker hit) | +| @ search latency | ~200-300ms | ~200-300ms | ~130ms + API | +| New IPC surface | None | None | JSON-lines protocol | +| Ownership clarity | Best — one owner | Good — two readers | Good — two workers | +| Upgrade path | Baseline | From A → B | From A → C | +| Files changed | ~12 | ~16 | ~22 + 4 new | + +--- + +## Final verdict + +**Approach A is the correct first recovery phase.** It restores retrieval correctness with the smallest diff and no new lifecycle complexity. It establishes the unified PFResult envelope and Python-as-canonical-owner contracts that both B and C also require. Once correctness is proven in the live vault with contract tests, the team can decide whether to add Approach B (sql.js read cache) or Approach C (panel-scoped worker) as a latency optimization. + +**Bottom line: Implement Approach A now. Defer B and C until after correctness is restored and contract tests gate the live pipeline.** diff --git a/docs/superpowers/specs/2026-07-10-retrieval-recovery-design.md b/docs/superpowers/specs/2026-07-10-retrieval-recovery-design.md new file mode 100644 index 00000000..35f821e9 --- /dev/null +++ b/docs/superpowers/specs/2026-07-10-retrieval-recovery-design.md @@ -0,0 +1,274 @@ +# PaperForge Retrieval Recovery — Architecture and UX Design + +- **Date:** 2026-07-10 +- **Map:** [Wayfinder: Restore PaperForge retrieval end to end](https://github.com/LLLin000/PaperForge/issues/45) +- **Evidence:** Issues #53 (contract drift), #49 (failure matrix), #47 (deployment parity) +- **Status:** Presenting for design approval before implementation + +--- + +## 1. Problem summary + +The PaperForge Retrieval Experience has **four P0 contract failures** and **ten lifecycle/integrity drifts** that make M metadata search, @ deep search, and vector build controls silently broken or misleading. The root cause is not one bug — it's four independently drifting subsystems (sql.js, Python CLI, build-state JSON, Chroma-legacy control flow) with no unified contract. + +### P0 failures (user-visible breakage) + +| # | Failure | Impact | +|---|---------|--------| +| 1 | sql.js queries `paper_fts.year` — column doesn't exist | sql.js never works; every debounce search falls to CLI | +| 2 | Plugin omits `--deep` for `@` queries | hybrid BM25+vector path unreachable | +| 3 | CLI returns `data.chunks`, plugin expects `data.matches`/`data.results` | Every @ query silently shows "No results found" | +| 4 | Build loop writes then immediately deletes vectors | vec0 tables empty despite build_state=completed | + +### P1 drift (control/integrity) + +5. `--resume` gates on legacy Chroma directory, not vec0 tables +6. `--force` deletes legacy Chroma directory, not vec0 tables +7. Stop writes state after killing PID; dead PID crashes stop +8. Plugin reads stale JSON snapshot, not live SQLite build state +9. embed status counts meta rows, never checks vec0 queryability +10. `text` vs `matched_text`, `first_author` vs `authors` field drift + +--- + +## 2. Design principles + +1. **Source Corpus is never at risk.** Papers, OCR, blocks, metadata, annotations are preserved. +2. **Retrieval Artifacts are disposable.** FTS indexes, embeddings, vector tables may be rebuilt. +3. **One owner, one truth, one envelope.** Python owns storage, schema, queries, build lifecycle. Plugin owns UI rendering only. +4. **States are distinguishable.** "No results" and "backend broken" are never the same view. +5. **Recovery over redesign.** Existing Obsidian visual language and DESIGN.md are binding. This is a functional recovery, not a UI overhaul. + +--- + +## 3. Architecture recommendation: Approach A (Python CLI as sole owner) + +### Decision + +**Implement Approach A as the first recovery phase.** Delete sql.js. Python CLI owns every retrieval and build path. Plugin spawns per-query CLI processes with a unified PFResult envelope. + +### Rationale + +Approach A is the only option that restores correctness with the smallest diff, no new IPC surface, and no lifecycle complexity. Both Approaches B (sql.js read cache) and C (panel-scoped worker) can be added later as latency optimizations once correctness is proven by contract tests. + +### What changes + +| Layer | Action | +|-------|--------| +| **Delete** | sql.js service (`db.ts`), `sql-wasm.wasm`, `sql.js` npm dep, sql.js branches in dashboard | +| **Unify Python envelope** | `search` and `retrieve` both return `data.matches` with stable fields (`first_author`, `text`, `heading`) | +| **Fix plugin spawn** | Pass `--deep` for @ queries. Parse only `data.matches`. Kill child on cancel. | +| **Fix build path** | Transactional write-then-delete. Delete Chroma gating. Add vec0 queryability probe. Cooperative stop with state settlement. | +| **Fix plugin state** | Read live `embed status --json` instead of JSON snapshot. Poll during build. | +| **Downgrade JSON snapshot** | Debug-only; never control-plane truth. | + +### User-visible latency + +| Interaction | Latency | Acceptable? | +|-------------|---------|:---:| +| Debounced M search | ~330ms (200ms debounce + 130ms spawn) | Yes — feels instant | +| Enter M search | ~130ms | Yes | +| @ deep search | ~200-300ms + API | Yes | +| Status check | ~130ms | On settings load — fine | + +### Why not sql.js (Approach B) right now? + +Approach B requires four mandatory controls before sql.js is safe enough: Python-owned M-search contract, staleness invalidation, WAL publication discipline, and deletion of all Chroma ownership leftovers. Adding these controls alongside the build-path fixes creates unnecessary risk. Approach B becomes a straightforward latency optimization (delete sql.js query, replace with Python-owned contract version) once the unified envelope and Python-as-canonical-owner foundation exist. + +### Why not worker (Approach C) right now? + +Approach C is architecturally clean but introduces medium-high lifecycle complexity (IPC protocol, worker restart/crash recovery, stale connection handling, concurrency with build, Windows process semantics) before the simplest correctness fixes are proven. It becomes a viable upgrade path once the unified envelope and build-path fixes are stable. + +--- + +## 4. Unified result envelope (PFResult v1) + +Every retrieval path returns the same outer shape: + +```json +{ + "ok": true, + "data": { + "query": "knee", + "matches": [ + { + "zotero_key": "...", + "title": "Title of the paper", + "first_author": "Author A", + "year": 2024, + "journal": "Journal Name", + "domain": "orthopedics", + "abstract": "Abstract snippet...", + "score": 0.95, + "text": "Matched body text snippet...", + "heading": "Introduction", + "source": "fulltext" + } + ] + }, + "count": 5, + "route_explanation": "FTS match", + "warnings": [] +} +``` + +Error: + +```json +{ + "ok": false, + "error": { + "code": "VECTOR_CORRUPTED", + "message": "Vector index is unreadable. Rebuild vectors before retrieving.", + "details": {} + } +} +``` + +### Error codes + +| Code | UI state | +|------|----------| +| `VECTOR_NOT_BUILT` | Vectors not built — link to build | +| `VECTOR_CORRUPTED` | Corrupted — force rebuild required | +| `MODEL_CHANGED` | Model changed — rebuild recommended | +| `BACKEND_UNAVAILABLE` | Python/CLI not reachable | +| `TIMEOUT` | Search timed out | +| `INTERNAL_ERROR` | Generic failure with stderr detail | +| `NO_PYTHON` | Python runtime missing or incompatible | + +--- + +## 5. Build lifecycle — correctness guarantees + +### Write-then-delete fix + +Current (broken): +```python +write_encoded_payload(vault, payload) # inserts vec0 rows +delete_paper_vectors(vault, paper_id) # deletes ALL rows for paper, including just-written ones +``` +Result: vec0 tables always empty. + +Fixed: delete old vectors BEFORE writing new ones (or wrap in a transaction). + +### Resume/force — target vec0, not Chroma + +- `--resume`: check vec0 meta table row counts, not legacy Chroma directory. +- `--force`: DROP vec0 and companion meta tables, then rebuild. Never touch Chroma paths. +- Delete `get_vector_db_path()` returning legacy `.../indexes/vectors`. + +### Health check — prove vec0 queryability + +`embed status` must additionally: +- Run a trivial vec0 k-NN query (e.g., `SELECT rowid FROM vec_fulltext WHERE fulltext_embedding MATCH ? LIMIT 1` with a zero vector). +- Report `healthy: false` if it fails, even if meta rows exist. + +### Stop — cooperative with state settlement + +1. Plugin spawns `paperforge embed stop --json`. +2. Python sends SIGTERM/CTRL_BREAK_EVENT to build PID. +3. Build process catches signal, marks `build_state.status = "stopping"`, finishes current paper, flushes, then exits. +4. Stop command waits for PID to exit or timeout, then writes `status = "idle"` and returns `{state: "stopped"}`. +5. Plugin never directly kill()s the build process — it always goes through `embed stop`. + +### JSON snapshot — downgrade to debug-only + +`vector-runtime-state.json` is written by `embed status` for human inspection only. Plugin never reads it for control-plane decisions. Plugin reads live `embed status --json` on settings load and polls during build. + +--- + +## 6. UX state model (summary) + +Full design: `local://retrieval-ux-state-design.md` (RecoveryUXDesign agent). + +### Search domain + +| State | Trigger | UI | +|-------|---------|----| +| idle | No query | Input with M/@ mode badge | +| searching | Debounce fired or Enter pressed | Skeleton, input responsive (M) or disabled (@) | +| results | CLI returned matches | Cards with title/author/year/journal/score/snippet | +| empty | 0 matches | "No matching papers found. Try broader terms or @ deep search." | +| vectors not built | Error code `VECTOR_NOT_BUILT` | Warning banner + "Open Vector Settings" | +| backend unavailable | Spawn failed or error code `BACKEND_UNAVAILABLE` | Error card + "Run Doctor" + "Retry" | +| timeout | CLI exceeded 30s | "Search timed out" + Retry | +| model changed | build_state.model ≠ settings model | Warning badge + "Rebuild Vectors" | + +### Build domain + +| State | Trigger | UI | +|-------|---------|----| +| idle | No vectors built | "Vectors: not built" + Build button | +| ready | build_state=completed, chunks>0, healthy | "Chunks: N | model | mode" + Rebuild option | +| building | Process active | Segmented progress bar, "X/Y papers", paper_id, Stop button | +| stopping | Stop requested | Frozen bar, "Stopping..." spinner, disabled Stop | +| failed | Process exited non-zero | Error banner, stderr detail, Retry/Force Rebuild | +| corrupted | healthy=false | Warning: "Vector index corrupted" + Force Rebuild only | +| stale | build_state=running but PID dead | "Previous build interrupted" + Resume/Discard | +| deps missing | deps_installed=false | "Install Dependencies" button | +| runtime mismatch | Plugin version ≠ CLI version | Drift banner across all states | + +### Button hierarchy +1. **Primary CTA:** Build, Rebuild, Retry, Resume +2. **Secondary:** Force Rebuild, Continue anyway +3. **Warning:** Stop, Force Kill +4. **Link-style:** Open Vector Settings, Run Doctor + +### Destructive warnings +- Rebuild (existing chunks): "Rebuilding will replace all existing vectors ({N} chunks). This cannot be undone. Continue?" +- Force Rebuild (healthy data): "Force rebuild will delete all existing vectors and rebuild from scratch. Continue?" +- Force Rebuild (corrupted): No warning — data is already useless. + +--- + +## 7. Implementation plan (Approach A) + +### Phase 1: Unified contract (P0 fixes) + +1. **Unify Python result envelope.** `search.py` and `retrieve.py` return `data.matches` with identical field names. Add `--deep` flag plumbing. +2. **Fix plugin parser.** Delete `matches`/`results`/`chunks` forking. Parse `data.matches`. Use `text` not `matched_text`. Use `first_author` not `authors`. +3. **Fix plugin spawn.** Pass `--deep` for `@`. Delete sql.js service, wasm, npm dep, all sql.js branches. +4. **Contract test.** E2E: spawn CLI → parse JSON → render. Test every error code path. + +### Phase 2: Build path correctness + +5. **Fix write-then-delete.** Delete old vectors before writing new ones (or transaction wrap). +6. **Fix resume/force.** Target vec0/meta tables, delete Chroma gating from `embed.py` and `_chroma.py`. +7. **Fix health probe.** Add vec0 queryability check to `embed status`. +8. **Fix stop.** Cooperative signal → state settlement → `status=idle`. Plugin goes through CLI, never direct kill. + +### Phase 3: Plugin state integrity + +9. **Live status via CLI.** `embed status --json` on settings load. Poll during build. Delete JSON snapshot as control-plane truth. +10. **State polling.** Plugin reads `build_state` via `embed status` every 2s during build. Renders progress from live data. + +### Phase 4: UX recovery + +11. **Implement state matrix.** Every state from Section 6 renders with correct buttons, copy (en/zh), a11y. +12. **Error code routing.** `classifyError(code)` maps `error.code` to UI state components. +13. **i18n.** 33 new keys under `retrieval_*` prefix in both English and Chinese. + +### Phase 5: Verification + +14. **Contract test suite.** Plugin → CLI → PFResult → render for M search, @ search, all error codes. +15. **Build lifecycle test.** Start → progress → stop → resume → force rebuild in disposable test vault. +16. **Literature-hub acceptance.** Smoke test all paths against the real vault. + +--- + +## 8. Acceptance gates + +| Gate | What must pass | +|------|---------------| +| M metadata search | Type "knee" → results with author/year/journal rendered within 400ms | +| @ deep search | Type "@platelet rich plasma" → semantic matches with snippets | +| sql.js deleted | No sql.js import, no wasm asset, no related code paths in bundle | +| Build → vectors exist | `embed build --resume` → vec0 tables have rows → `embed status` reports healthy | +| Stop → idle | Stop build → state settles to idle → Resume works | +| Force rebuild | `embed build --force` → old vectors dropped → new vectors exist | +| Health probe | Corrupted vec0 → status reports `healthy: false` | +| Model change detection | Switch model → warning in UI → rebuild resolves | +| Error distinction | Each error code produces distinct UI, not generic "No results" | +| Live state | Plugin status reflects SQLite truth, not stale JSON snapshot | +| Source Corpus untouched | All 868 papers, OCR, blocks, annotations preserved after rebuild |