docs: retrieval recovery design — Approach A (Python CLI sole owner) recommended

This commit is contained in:
LLLin000 2026-07-10 23:01:57 +08:00
parent e3c41cc07d
commit f52124bcf7
2 changed files with 469 additions and 0 deletions

View file

@ -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.**

View file

@ -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 |