refactor(storage): remove archive body duplication#1498
Merged
Conversation
Problem: provider_events.payload_json made the archive database behave like a second raw blob store, with live measurements showing roughly 18.6 GiB of inline provider-event payloads. The same issue also surfaced deferred insight catch-up as failed convergence debt even when the daemon was correctly waiting for a hot source to quiet.\n\nWhat changed: bump the fresh schema to v9, remove provider_events.payload_json, add typed detail tables for compactions, turn context, tool calls, reasoning, and ghost snapshots, and wire sync/async provider-event writes through a compact projection helper. ProviderEvent.payload remains reconstructed for current domain readers, but the storage layer no longer copies raw provider bodies into hot event rows. Deferred insight debt is now recorded with status=deferred rather than failed.\n\nVerification: python -m mypy polylogue/storage/sqlite/provider_event_model.py polylogue/storage/sqlite/provider_event_writes.py polylogue/storage/sqlite/queries/provider_events.py polylogue/storage/sqlite/queries/mappers_archive.py polylogue/pipeline/services/ingest_worker.py polylogue/pipeline/services/ingest_batch/_core.py polylogue/sources/live/convergence_debt_store.py polylogue/daemon/convergence_debt_status.py tests/unit/pipeline/test_prepare_records.py tests/unit/daemon/test_convergence_debt_alert.py; pytest -q tests/unit/pipeline/test_prepare_records.py tests/unit/storage/test_repository_lifecycle_laws.py tests/unit/storage/test_raw_retention.py tests/unit/storage/test_session_insight_refresh.py tests/unit/sources/test_parsers_codex.py tests/unit/sources/test_compaction.py tests/unit/sources/test_live_catchup_planning.py tests/unit/daemon/test_convergence_debt_alert.py tests/unit/daemon/test_daemon_cli.py --tb=short.\n\nRef #1486
Problem: messages.text and content_blocks.text both stored full message bodies, while message FTS still assumed the messages row was the canonical search body. That duplicated large tool-result payloads and made the schema cleanup incomplete after the provider-event split. What changed: make content_blocks the canonical full-body storage for structured messages, keep messages.text empty when blocks exist, hydrate aggregate display text from blocks on read, and switch messages_fts to contentless-delete FTS populated from a message/content-block search projection. Search, repair, daemon readiness, and focused tests now join contentless FTS rows back through messages.rowid instead of relying on stored FTS columns. Verification: pytest focused FTS/storage/ingest/daemon/provider-event slice (306 passed). pytest focused post-format FTS/daemon slice (115 passed). python -m mypy focused storage/search/daemon/ingest/test files (success). Ref #1486
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 44 minutes and 4 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (46)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Sinity
marked this pull request as ready for review
May 24, 2026 13:46
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da7b8e4418
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+207
to
+209
| normalized_kind = event_type if stored_kind in {None, "", "provider_native"} else stored_kind | ||
| payload: dict[str, object] = {} | ||
| if normalized_kind == "compaction": |
There was a problem hiding this comment.
When normalized_kind is provider_native (or any event type not handled by the typed branches), this mapper returns an empty payload object, so fields from events like token_count/future Codex event kinds are silently dropped after persistence. That breaks the parser contract that unknown provider events are still surfaced with their data, and it causes irreversible information loss for downstream consumers that read events back from storage.
Useful? React with 👍 / 👎.
Comment on lines
672
to
+674
| has_indexed_rows = exists and bool(conn.execute("SELECT 1 FROM messages_fts_docsize LIMIT 1").fetchone()) | ||
| has_indexable_messages = bool(conn.execute("SELECT 1 FROM messages WHERE text IS NOT NULL LIMIT 1").fetchone()) | ||
| triggers_present = exists and _triggers_present_sync(conn, _MESSAGE_FTS_TRIGGER_NAMES) | ||
| triggers_present = exists and _triggers_present_sync(conn, _message_trigger_names_for_sync(conn)) |
There was a problem hiding this comment.
The fast-path readiness check still treats only messages.text IS NOT NULL as indexable, but this commit now stores many message bodies only in content_blocks with messages.text set to NULL. In that common case, an empty messages_fts can be reported as ready (has_indexable_messages becomes false), so daemon/readiness flows using verify_total_rows=False may incorrectly skip repair or mark the index healthy.
Useful? React with 👍 / 👎.
This was referenced May 26, 2026
Closed
Sinity
added a commit
that referenced
this pull request
May 26, 2026
) (#1647) ## Summary 26 PRs landed between 2026-05-24 and 2026-05-25 chasing I/O symptoms downstream of #1498 (body de-dup). They reshape `daemon/convergence_stages.py`, `daemon/cli.py`, and the FTS lifecycle without a single architectural reference point. Adds `docs/retro/2026-05-24-1498-cascade.md` as that reference. Closes #1605. ## Problem `git log -- polylogue/daemon/convergence_stages.py` shows 10 unrelated-looking commits in 48 hours; each PR body is high-quality on its own but there's no shared narrative. Future agents touching these files have no way to find "this is the cascade, here's the I/O budget we landed on, here are the standing refactor verdicts." ## Solution A single doc with: 1. **Root cause one-liner** — what #1498 changed and why later PRs exist. 2. **PR list in commit order** (26 entries) — each with the one-line symptom it addressed. Future readers searching for a regression pattern can find the precedent quickly. 3. **Architectural review** of the two hottest files (`convergence_stages.py`, `cli.py`) — both have outstanding refactor verdicts before more accretion lands. 4. **CI metric pointer** — refs #1606's pending benchmark. The I/O budget the cascade landed on is not yet a CI assertion; the doc says so out loud. 5. **Outstanding follow-ups** (#1606, #1607, #1614) the cascade did not address. The MEMORY.md update lives in agent-local memory (not in the repo); a one-line pointer was added so future agents read this doc before touching the named files. ## Verification ``` $ devtools verify --quick "exit_code": 0 ``` The doc is referenced from #1605's acceptance criteria #1 (retro doc) and AC #3 (hot-file architectural review). AC #2 (PR dependency chain, ~22 lines) is satisfied by the table; this PR's table has 26 entries because the cascade was longer than the issue's initial count. AC #4 (CI metric or assertion) is **described** in the doc but not implemented — the actual benchmark assertion belongs in #1606's PR. AC #5 (MEMORY.md link) is satisfied via the agent-local memory file, not the repo.
Sinity
added a commit
that referenced
this pull request
Jul 18, 2026
…artup (#3102) ## Summary Fixes the P0 daemon-convergence starvation from the 2026-07-18 poisoned-index incident investigation (bead `polylogue-5jak`): the raw source→index conveyor could only self-heal at 1 row per 30s tick (~25 days for the current 73,311-row backlog), and daemon startup serialized all convergence and the live watcher behind serial Drive network I/O. ## Problem Live evidence from the incident restore (see `.agent/scratch/warroom-2026-07-17/perf-investigation-2026-07-18.md`): - `_RAW_MATERIALIZATION_CONVERGENCE_BATCH_LIMIT = 1` every 30s → 2 rows/min; a rebuilt-but-empty index inherits the entire corpus as backlog and the daemon "heals" it homeopathically. Each tick also paid fixed overhead (blob-ref restore scan, interrupted-frontier recovery, FTS close) amortized over one row. - `startup.drive_catchup` was awaited before every periodic loop and before the LiveWatcher was constructed — measured live at ~4 serial attachment fetches/min; every restart on a Drive-heavy archive blocks all local convergence for tens of minutes to hours. - The conveyor additionally waited on the watcher's full catch-up (`catch_up_complete` gate) despite operating purely on durable local `source.db` rows, which have no acquisition precondition. ## Solution - `_periodic_raw_materialization_convergence` is now ungated and backlog-aware: bounded 16-row passes run back-to-back while `remaining_candidates > 0` and the pass made progress, sleeping 1s between passes so the write coordinator stays responsive to live appends (per the #1498-cascade retro discipline: bounded per-cycle work, no busy loops — a no-progress pass with blocked candidates ends the burst). Pending browser-capture spool files still preempt the loop, including mid-burst. - `RawMaterializationCounts` gains `remaining_candidates`, populated from the repair pass's existing `raw_materialization_remaining_candidate_count` metric. - Interrupted-frontier recovery runs on the first pass of each cycle only (`recover=False` on burst continuations) — it only has work after a crash/restart. - The awaited startup Drive pass is deleted; `_periodic_drive_source_catchup` now runs its first pass immediately as a background task. Nothing gates on it. Insight/embedding/derived-debt loops keep their catch-up gates — they read from the index the catch-up populates, so waiting is correct there. ## Verification - `devtools test tests/unit/daemon/test_daemon_cli.py` — 87 passed, including new tests: burst-through-backlog (multi-pass single cycle, recovery gated to first pass, sleep schedule pinned), burst-stops-without-progress (blocked candidates can't hot-loop), starts-without-catch-up, recover=False skips the frontier recovery scan, and the lifecycle-ordering test now asserts Drive catch-up does NOT precede the watcher. - `mypy polylogue/daemon/cli.py polylogue/product/raw_authority.py` — clean; `ruff` clean; pre-push quick gate green. - Not run: full `devtools verify --all` (testmon-affected scope only). Ref polylogue-5jak 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QUsH3Rhq6oAZpYPWcsZqnZ Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Splits archive storage so large provider-event details and message bodies are no longer duplicated in broad SQLite rows. Provider events now store typed header/detail rows, structured message bodies are canonical in
content_blocks, and message FTS indexes a projected content-block/search-text stream instead of relying on duplicatedmessages.text.Ref #1486.
Problem
The live archive audit showed SQLite carrying multi-GiB duplication for tiny steady-state workload:
provider_eventswas roughly 20.0 GiB on disk with about 18.6 GiB payload text,messageswas roughly 9.8 GiB with about 8.9 GiB text payload,content_blockswas roughly 9.1 GiB with about 8.3 GiB payload, and FTS was roughly 2.7 GiB. That meant catch-up, FTS rebuilds, backups, and diagnostics were scanning repeated bodies rather than a clean canonical model.Solution
provider_events.payload_jsonwith typed provider-event detail tables for compaction, turn context, tool-call, reasoning, and ghost-snapshot details.NULLwhen content blocks carry the canonical body, then hydrate display reads from content blocks.Re-ingest Plan
This is a fresh-first schema change, not an in-place migration. Existing archives with schema version 8 should be moved aside or reset, then reacquired from source with
polylogue reset --database && polylogued run. The daemon convergence path rebuilds archive rows, FTS, and insight products against the version 9 schema. The expected user impact is a one-time re-ingest/rebuild; the steady-state archive should carry much less duplicated text.Verification
pytest -q tests/unit/storage/test_perf_rescue_1314.py::test_search_conversation_hits_falls_back_to_exact_freshness tests/unit/daemon/test_fts_trigger_drift.py tests/unit/storage/test_dead_schema_sweep.py::test_schema_version_is_9 tests/unit/storage/test_no_string_interpolated_sql.py::test_no_unaudited_string_interpolated_sql tests/unit/pipeline/test_resilience.py::test_transform_deduplicates_materialized_message_rows_by_primary_key --tb=short→ 30 passed.pytest -q tests/unit/storage/test_fts_bloat_invariants.py tests/unit/storage/test_fts_trigger_lifecycle.py tests/unit/storage/test_fts5.py tests/unit/storage/test_backend.py tests/unit/storage/test_message_query_reads.py tests/unit/storage/test_retrieval_readiness_laws.py tests/unit/daemon/test_convergence_stages.py tests/unit/daemon/test_daemon_cli.py tests/unit/daemon/test_fts_trigger_drift.py tests/unit/storage/test_perf_rescue_1314.py tests/unit/storage/test_dead_schema_sweep.py tests/unit/storage/test_no_string_interpolated_sql.py tests/unit/pipeline/test_resilience.py --tb=short→ 223 passed.devtools verify --quickatda7b8e44→ passed in 40.44s.POLYLOGUE_PYTEST_WORKERS=8 devtools verifyatda7b8e44→ passed in 265.23s;pytest testmon-globalselected 9,401 tests, with 9,399 passed and 2 xfailed.