merge: T118 phase 4.5 docs sweep — Phase 4.5 status + Phase 5 backlog
This commit is contained in:
@@ -322,53 +322,48 @@ Phase 4 polish shipped end-to-end across 15 tasks (T88–T102). Vector retrieval
|
||||
|
||||
### Phase 4.5 / 5 backlog
|
||||
|
||||
New follow-ups discovered during Phase 4 reviews and execution. None are blocking; pick up at any time.
|
||||
All items shipped or deferred to Phase 5 (see "Phase 5 backlog" below). Final schema version: 14.
|
||||
|
||||
#### From T88 review
|
||||
## Phase 4.5 status
|
||||
|
||||
- **`embeddings` FK lacks `ON DELETE CASCADE`**: deindex events are the only deletion path; if memories ever get deleted directly (raw SQL), embedding rows orphan. Defensible since projector model uses explicit deindex events, but worth a comment or `ON DELETE CASCADE` addition.
|
||||
Phase 4.5 cleanup shipped 13 of 14 planned tasks (T103–T117 with T115 deferred; T118 is this docs sweep). Two CLAUDE.md backlogs (Phase 3.6/4, Phase 4.5/5) are now empty; deferred follow-ups discovered during execution are tracked in a new "Phase 5 backlog" section below. Schema baseline advanced from version 13 to **14** (migration 0014: `memories.event_id`). Test count grew from ~413 (Phase 4) to ~457 (+~44 new tests across the wave).
|
||||
|
||||
#### From T89 review
|
||||
- **Wave 1 — trivial polish (parallel)**:
|
||||
- **T103** branches polish — global-branch (`chat_id IS NULL`) leak documented in `list_branches`; branch-switch to nonexistent name now logs a warning.
|
||||
- **T104** `memory.py` DRY — `MAX(id)` helper extracted; `fts_rank=None` contract documented for vector-only rows.
|
||||
- **T105** `snapshots.py` polish — `datetime`/`timezone` imports hoisted to module level; strict `kind` validation in restore/preview (rejects missing); `created_at` from file mtime documented.
|
||||
- **T106** `search.py` polish — `k=50` extracted to module constant; N+1 `get_bot`/`get_chat`/`get_scene` lookups batched.
|
||||
- **T107** `embeddings.py` — `timeout_s` fallback-path warning when non-default model misconfigured.
|
||||
- **Wave 2 — scene-close-on-cancel (single)**:
|
||||
- **T108** strengthened the T74.3 regression test + documented rationale in `turns.py`. **Surfaced a deferred bug**: existing pin only passes because `asyncio` isn't imported in the test module (NameError caught instead of CancelledError). When CancelledError fires for real, `post_turn`'s end-of-function re-raise causes `open_db`'s dependency teardown to skip `conn.commit()`, rolling back ALL post-cancel writes. Documented and deferred to Phase 5 triage.
|
||||
- **Wave 3 — schema 0014 (single)**:
|
||||
- **T109** `memories.event_id` column (foundation for T111 deep-link). FK CASCADE on `embeddings.memory_id` deferred (memories rows are never deleted today; defensive constraint can't fire — saved for broader migration cleanup in Phase 5).
|
||||
- **Wave 4 — drawer Phase 4.5 bundle (single)**:
|
||||
- **T110** `event_id <= 0` guard in `delete_turn` + `html.escape()` on delete-impact modal + Jinja partial extraction + bulk significance re-rate per chat (one `manual_edit` event per memory).
|
||||
- **Wave 5 — search UX (single)**:
|
||||
- **T111** FTS snippet highlighting via `snippet()` + deep-link to turn via `memories.event_id`.
|
||||
- **Wave 6 — real embedding model swap (single)**:
|
||||
- **T112** `LLMClient.embed()` Protocol + Mock impl with `canned_embeddings` + `FeatherlessClient.embed()` (raises `NotImplementedError` — Featherless OAI-compat doesn't expose embeddings, gap documented) + `generate_embedding` routes non-default models through `client.embed()` with fallback + `--re-embed-all` backfill flag.
|
||||
- **Wave 7 — branching read-side filter (single)**:
|
||||
- **T113** `active_branch_event_ids(conn)` helper + applied to `read_recent_dialogue`, `scene_summarize._read_recent_dialogue`, `search_memories`, and `meanwhile._read_recent_meanwhile_dialogue`. Cross-chat search and projector queries deliberately NOT filtered (cross-chat is by design; projectors must see full log). Bootstrap "main" branch (origin=0, head=0) detected as the no-clamp sentinel.
|
||||
- **Wave 8 — regenerate lifecycle rollback (single)**:
|
||||
- **T114** `triggered_by_assistant_turn_id` payload back-reference on `event_started`/`event_completed`/`event_cancelled` + new `event_status_reverted` event kind + projector handler in `chat/state/events.py` + regenerate flow emits revert events for affected lifecycle transitions.
|
||||
- **Wave 9 — final polish + integration (parallel)**:
|
||||
- **T115** sqlite-vec swap — **DEFERRED to Phase 5**. Pre-flight failed: host Python build doesn't expose `sqlite3.Connection.enable_load_extension` (raises `AttributeError`). Requires either Python rebuild with `--enable-loadable-sqlite-extensions` or migration to `apsw`. Phase 4 pure-Python cosine remains in production.
|
||||
- **T116** structured `CannedQueue` test fixture builder + 2–3 POC test migrations (Phase 5 to migrate the rest).
|
||||
- **T117** Phase 4.5 cross-feature integration tests (5 minimum: real embedding swap, branching read-side filter, lifecycle rollback, search deep-link, bulk significance re-rate).
|
||||
- **T118** documentation (this section).
|
||||
|
||||
- **`list_branches(chat_id=...)` filter leaks global branches** (`chat_id IS NULL`) into every chat scope. Intentional? Document.
|
||||
- **Branch-switch to nonexistent silently leaves zero active branches** — log a warning when this would happen.
|
||||
### Phase 5 backlog
|
||||
|
||||
#### From T91 review
|
||||
New follow-ups discovered during Phase 4.5 reviews and execution, plus carry-over deferrals. None are blocking; pick up at any time.
|
||||
|
||||
- **Real embedding model swap**: Phase 4 ships pseudo-embedding (deterministic SHA-256 hash). Phase 4.5+ should swap to a real model (Featherless `bge-small-en-v1.5` if available; or local `sentence-transformers/all-MiniLM-L6-v2`). The 384-dim is hardcoded in `0012_embeddings.sql`; if dim changes, migrate first.
|
||||
- **`timeout_s` unused on pseudo path** — fine, but log when non-default model falls through to fallback so misconfigured callers don't silently degrade.
|
||||
|
||||
#### From T96 review
|
||||
|
||||
- **Duplicate `MAX(id)` lookup** between `_composite_rerank` and the fused-path tail — DRY follow-up.
|
||||
- **`fts_rank=None` for vector-only rows** — document downstream contract.
|
||||
|
||||
#### From T98 review
|
||||
|
||||
- **`event_id <= 0` guard in `delete_turn`** — currently silently rewinds everything if `event_id` is 0. Add `if event_id <= 0: 400`.
|
||||
- **`html.escape()` on `compute_delete_impact` output rendered into the modal** — defense in depth (currently model-controlled strings, but if event payload fields ever appear in descriptions, autoescape needed).
|
||||
- **Extract delete-impact modal HTML to a Jinja partial** — testability + autoescape inheritance.
|
||||
|
||||
#### From T99 review
|
||||
|
||||
- **Hoist `datetime`/`timezone` imports to module level** in `chat/web/snapshots.py`.
|
||||
- **`kind` defaulting in restore/preview** — reject missing `kind` rather than silent 404.
|
||||
- **`created_at` from file mtime** vs filename-encoded timestamp — small drift if files copied; document.
|
||||
|
||||
#### From T100 review
|
||||
|
||||
- **Hardcoded `k=50`** — extract to module constant.
|
||||
- **N+1 lookups (`get_bot`/`get_chat`/`get_scene` per row)** — fine at `k=50`, revisit if `k` grows.
|
||||
- **FTS highlighting via `snippet()`** — Phase 4 skipped this; UX nice-to-have.
|
||||
- **Result links chat-level only** — `memories` table has no `event_id` column; deep-linking to specific turn requires schema addition.
|
||||
|
||||
#### Deferred items
|
||||
|
||||
- **sqlite-vec swap** when host Python supports `enable_load_extension`.
|
||||
- **Real embedding model** with proper semantic similarity.
|
||||
- **Branching read-side filter**: T89 ships data-model + UI but event readers don't yet consult `is_active`. Each branch is metadata-only labeled ranges. Consult-on-read is Phase 4.5+ work.
|
||||
- **Bulk significance re-rate** in drawer (T98.2 deferred — only per-memory edit shipped).
|
||||
- **Vector index optimization** (HNSW) — only relevant if memory counts grow past pure-Python feasibility.
|
||||
- **`scene-close-on-cancel` UX revisit** (Phase 2.5 carry-over).
|
||||
- **Cross-feature canned-queue brittleness fixture builder** (Phase 3 carry-over).
|
||||
- **Full lifecycle-rollback in regenerate** — Phase 3.5 T83.4 shipped a warning log; proper rollback needs schema-level back-references (`triggered_by_assistant_turn_id` payload field).
|
||||
- **T115 sqlite-vec swap** (environmental blocker): host Python's `sqlite3.Connection` does not expose `enable_load_extension` — `python -c "import sqlite3; sqlite3.connect(':memory:').enable_load_extension(True)"` raises `AttributeError`. Fix requires either a Python rebuild with `--enable-loadable-sqlite-extensions` or migration to `apsw`. Pure-Python cosine remains in production until then.
|
||||
- **T108 follow-up: cancel-path commit bug** — `post_turn`'s re-raised `CancelledError` causes `open_db` dependency teardown to skip `conn.commit()`, rolling back all post-cancel writes. The existing T74.3 regression test passes only because `asyncio` isn't imported in the test module (NameError masks the real cancel path). Triage required — either commit before re-raise, or restructure the route to never re-raise after the close-detection branch.
|
||||
- **`embeddings` FK CASCADE on `memory_id`** — deferred from T109; do as part of a broader migration consolidation in Phase 5.
|
||||
- **`CannedQueue` fixture migration** — T116 shipped the builder + POC migrations; remaining tests still use positional canned arrays. Migrate in Phase 5.
|
||||
- **Vector index optimization (HNSW)** — currently scales to a few thousand memories on the flat-index pure-Python cosine path; revisit when counts grow past flat-index feasibility.
|
||||
- **Branch-isolated `event_log`** — each branch has its own physical `event_log` range vs the current shared id space + head filter; full branch isolation is Phase 5+.
|
||||
- **Embedding model swap migration tooling** — T112 added `--re-embed-all`; a more orchestrated swap (drain old worker, re-seed all memories, swap config) is Phase 5+.
|
||||
- **Real-time collaborative branching** (multi-user) — out of scope for v1.
|
||||
- **Avatars / portraits** (multimodality) — deferred indefinitely per design §14.
|
||||
|
||||
@@ -522,6 +522,8 @@ Written per witness when a scene closes. Different details, different interpreta
|
||||
|
||||
**Status: shipped 2026-04-27** (T88–T102, 15 tasks across 8 waves; +70 tests). See "Phase 4 status" in CLAUDE.md for the per-task breakdown. Vector retrieval shipped via pure-Python cosine over a JSON-blob embeddings table (sqlite-vec deferred — host Python lacks loadable extensions); branching is data-model + drawer UI; significance review, hide-from-view soft delete, surgical delete with cascade preview, snapshot UX, and cross-chat search all surface from the drawer or top-bar.
|
||||
|
||||
**Phase 4.5 cleanup: shipped 2026-04-27** (T103–T118, 13 of 14 planned tasks; T115 sqlite-vec swap deferred to Phase 5 due to host Python lacking `enable_load_extension`; +~44 tests; schema baseline now 14). See "Phase 4.5 status" in CLAUDE.md for the per-task breakdown — notable shipped: real embedding model swap path (`LLMClient.embed()` + `--re-embed-all`), branching read-side filter (`active_branch_event_ids`), regenerate lifecycle rollback (`event_status_reverted`), FTS snippet highlighting + deep-link to turn (`memories.event_id`), bulk significance re-rate.
|
||||
|
||||
- Vector retrieval (sqlite-vss or sqlite-vec).
|
||||
- Branching UI.
|
||||
- Drawer-edit on every field.
|
||||
|
||||
Reference in New Issue
Block a user