feat(daemon): route bulk-scale rebuild through daemon-owned generation build#3189
Conversation
…n build Problem: the trickle raw-materialization conveyor is sized for steady-state drift; a bulk-scale backlog turns it into a weeks-scale grind (#3145's threshold warning). The correct bulk path already existed as the offline `polylogue ops maintenance rebuild-index` CLI command (resumable transaction, blue-green generation, one census+ replay sweep), but nothing routed to it automatically, and its own resume model silently re-walks the whole corpus when a caller omits `--operation-id` (polylogue-fbte, measured ~2.25h re-verification per resume at 50K raws). Solution: polylogue-gd6v phase (c) of the m6tp convergence redesign. - polylogue/daemon/bulk_rebuild.py (new): daemon-internal orchestrator that resolves/resumes a SINGLE well-known bulk-rebuild transaction per archive (DAEMON_BULK_REBUILD_OPERATION_ID), drives it through `rebuild_index_from_source_sync` -- the SAME engine the offline CLI drives, extracted/shared rather than duplicated -- and retires a terminal (promoted/stale/failed) transaction automatically so the next backlog reuses the same id. Because the daemon always resolves the same operation id (never an operator-supplied one to forget), the fbte class of bug is structurally unreachable here. - polylogue/daemon/cli.py: `_maybe_route_daemon_bulk_rebuild`, gated by a new `daemon_bulk_rebuild_routing` config flag (off by default), called from the existing raw-materialization convergence tick right after the #3145 recommendation. Keeps driving an in-flight build every tick even below the bulk-scale threshold so partial progress is never abandoned. A dedicated `DaemonParseStage` singleton (separate pool from the trickle conveyor's) is shut down at daemon teardown alongside the existing one. - polylogue/daemon/parse_prefetch.py: `DaemonParseStage.warm_raw_ids` extracts `warm`'s dispatch/admit logic to also accept an explicit raw-id list -- the bulk driver already knows its next page's raw ids from the transaction's own cursor, so it pre-warms them in the #3168 thread pool before requesting the writer hold, instead of querying the raw-materialization conveyor's own candidate set. - polylogue/sources/revision_backfill.py, polylogue/maintenance/ replay.py, polylogue/maintenance/rebuild_index.py: thread a new `prefetch_cache` parameter from `RebuildIndexRequest` down through `backfill_historical_revision_evidence`'s census phase, mirroring the #3168 raw-materialization wiring. A prefetch hit flows through the same `spill.add(...)` as a fresh parse, so the replay phase's own `spill.for_raw` lookups see identical warmed content -- census- phase prefetching alone is enough to skip replay-phase reparsing too. Degrades gracefully to the unmodified sequential in-hold parse on any miss or on a GIL build (parallel-parse speedup lands with the 3.14t deploy per m6tp phase (b); this PR is orchestration only). - polylogue/storage/index_generation.py: `IndexGenerationStore. discard_transaction` retires a terminal transaction record so its operation id can be reused (extracted for the daemon driver; the offline CLI path never needed this since it mints a fresh uuid4 per operation). - polylogue/config.py, docs/configuration.md: `daemon_bulk_rebuild_ routing` config flag (off by default), same pattern as `daemon_parse_stage_split`. Not in this PR: the CLI `ops maintenance rebuild-index` command is NOT deleted -- deletion is gated on an archive-scale equivalence receipt (coordinator-owned, post-promote); this PR only proves fixture- scale equivalence and readies the deletion for polylogue-4jsk once that receipt lands. The "second writer connection on the generation's own db" language in gd6v's design note is superseded here by the task's explicit single-writer-process constraint: every bulk-rebuild write, like every other daemon writer actor, is scheduled through the existing `daemon_write_coordinator().run_sync`; responsiveness instead comes from moving parse off that hold (mirroring phase (a)), not a second concurrent writer. Bulk routing does not suppress the trickle conveyor while a build is in flight -- both converge to the same eventual state; the resulting redundant per-raw work during the backlog window is a known, explicitly out-of-scope efficiency gap. Verification: - devtools test tests/unit/daemon/test_bulk_rebuild.py tests/unit/daemon/test_daemon_cli.py tests/unit/daemon/test_parse_prefetch.py tests/unit/daemon/test_raw_materialization_parse_stage_equivalence.py tests/unit/maintenance/test_rebuild_index_bulk_build.py tests/unit/storage/test_incremental_rebuild_equivalence.py tests/unit/storage/test_index_generation.py tests/unit/sources/test_revision_backfill.py -> 176 passed - uv run mypy polylogue --active -> Success: no issues found in 1059 source files - devtools verify --quick -> exit_code 0 (format/lint/mypy/render-all-check/ layering/topology/docs-coverage all clean) - devtools render topology-projection && devtools render topology-status (new polylogue/daemon/bulk_rebuild.py module) Ref polylogue-gd6v Remaining polylogue-gd6v scope: archive-scale equivalence receipt and CLI deletion (polylogue-4jsk), agvo responsiveness p99 gate during a live drain, and suppressing/coordinating the trickle conveyor with an in-flight bulk build -- all coordinator-owned follow-up. fbte requirement satisfied: each bounded pass persists last_raw_id/processed_raw_count via the existing (already-correct) IndexGenerationStore.checkpoint_transaction, and the daemon always resolves the SAME well-known operation id, so a restart mid-build resumes from the persisted cursor by construction -- proven by test_daemon_bulk_rebuild_pass_resumes_without_reprocessing_raw_ids and test_daemon_bulk_rebuild_pass_next_page_excludes_already_scheduled_raws. Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (13)
✨ 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 |
|
CodeRabbit rate-limited (no inline findings); merging on coordinator review (campaign precedent). Reviewed: single-writer preserved via daemon_write_coordinator scheduling; parse-prefetch reuse with graceful sequential fallback; well-known operation id + per-pass cursor checkpoint makes the fbte O(corpus)-resume class structurally unreachable; fixture equivalence vs CLI path asserted on sessions/messages/blocks/links/FTS. CLI deletion correctly gated on the archive-scale receipt (4jsk). 176 focused tests + mypy strict + quick gate green. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0dbc9a2fa9
ℹ️ 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".
| raw_ids = [raw_id for raw_id in candidate_raw_ids if not self.cache.contains(raw_id)] | ||
| return self.warm_raw_ids(config, raw_ids=candidate_raw_ids, max_payload_bytes=max_payload_bytes) | ||
|
|
||
| def warm_raw_ids(self, config: Config, *, raw_ids: Sequence[str], max_payload_bytes: int) -> int: |
There was a problem hiding this comment.
Bound bulk-page parses before submitting them
When bulk routing selects its default 500-row page, this method submits every raw for parsing without using max_payload_bytes; the 64 MiB cache limit is applied only after parsing completes. Because the futures dictionary retains each completed future and its parsed session tree until the method returns, a page containing many large raws can retain the whole page's expanded parse output and OOM the daemon even though nearly all results are rejected from the cache. Limit the dispatched page by the byte budget or release completed futures/results as they are processed.
Useful? React with 👍 / 👎.
| except (OSError, ValueError, TypeError, KeyError) as exc: | ||
| logger.warning( | ||
| "bulk-rebuild: could not load persisted transaction %s; starting a fresh one: %s", | ||
| DAEMON_BULK_REBUILD_OPERATION_ID, | ||
| exc, | ||
| ) | ||
| transaction = None |
There was a problem hiding this comment.
Remove an unreadable transaction before recreating it
If the persisted transaction JSON is truncated, malformed, or incompatible with IndexRebuildTransaction, this handler sets transaction to None but leaves the existing daemon-bulk-rebuild.json in place. The subsequent create_transaction() refuses to reuse an operation ID whose path already exists, so every enabled routing tick fails in the same way and the promised fresh rebuild never starts. Quarantine or remove the unreadable record before recreating the well-known transaction.
Useful? React with 👍 / 👎.
Ref polylogue-gd6v (parse-stage seam performance; surfaced by the resume #4 parallel-warm probe). ## Summary The parse-prefetch inflight budget default becomes adaptive: physical RAM / 16 clamped to [64 MiB, 2 GiB], env override unchanged. ## Problem Measured live on the 50K-raw archive (2026-07-20): under the fixed 64 MiB default, a 2000-raw bulk warm did 139 raws in 376s (0.37 raws/s) — the thread pool was stalled on cache admission, not parsing. With an adequate budget the same host warms 500 raws in 8.8s (56.7 raws/s), ~10x the sequential engine pace. The budget exists to bound transient memory beside a live daemon; a one-size constant starves capable machines and is exactly what made the daemon bulk path (#3189) parallelism theoretical. ## Solution `daemon_parse_stage_max_inflight_bytes()` computes RAM/16 clamped to [64 MiB, 2 GiB]; unknown RAM → 64 MiB floor; `POLYLOGUE_DAEMON_PARSE_STAGE_MAX_INFLIGHT_BYTES` still always wins. ## Verification `devtools test tests/unit/daemon/test_parse_prefetch.py` → 7 passed (new test covers ceiling/floor/proportional/unknown/override). `devtools verify --quick` → exit 0. Live probe numbers above. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Performance** * Improved background session prefetching by adapting its memory usage to the available system RAM. * Added safeguards to prevent prefetch operations from waiting indefinitely. * **Reliability** * If prefetching times out, uncached data is processed normally without interrupting the workflow. * Added support for configuring the prefetch memory limit, including safe fallback limits. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…ht (#3197) ## Summary Closes the two residuals PR #3189 (polylogue-gd6v) explicitly deferred: (1) the trickle raw-materialization conveyor and the daemon-owned bulk-rebuild driver ran every tick regardless of each other, converging on the same backlog with duplicate parse/replay work and needless writer-hold contention; (2) the p99 writer-responsiveness claim for a live bulk drain was never independently measured. ## Problem PR #3189's own body: "No trickle suppression. Bulk routing does not pause or coordinate with the trickle conveyor while a build is in flight...The resulting redundant per-raw work during the backlog window is a known, deliberately out-of-scope efficiency gap" and "agvo responsiveness p99 gate during a live drain -- not independently measured here; rests on the same off-writer-hold parse mechanism phase (a) already established." Both residuals are named directly on polylogue-gd6v (the bead's notes list them as the only two items not shipped in #3189) and block the 3.14t daemon deploy (polylogue-dcz5), which is the next step in the m6tp convergence program. ## Solution **Residual 1 (suppression):** - `polylogue/daemon/cli.py`: new `_daemon_bulk_rebuild_transaction_in_flight()` read-only check, gated by the same `daemon_bulk_rebuild_routing` config flag `_maybe_route_daemon_bulk_rebuild` already uses (flag off => never even check, so a stale transaction record can't wrongly suppress the ONLY mechanism materializing raws). - `_periodic_raw_materialization_convergence`'s burst loop consults this check first. When a bulk-rebuild transaction is in flight it skips the trickle census/drain writer pass for that tick (`_drain_raw_materialization_once`) and drives the bulk pass instead, bursting at the same 1s cadence the trickle conveyor already uses (rather than waiting a full 30s interval between bulk passes, which was the previous effective cadence once trickle's own backlog was exhausted). - **Suppression boundary, justified from the convergence-stage inventory** (`docs/design/convergence-simplification-inventory.md` item 5, "Per-pass candidate requery / resume recompute"): the trickle drain and the bulk engine both walk the exact SAME `raw_sessions` -> index materialization work over the same backlog -- `repair_raw_materialization` and `rebuild_index_from_source_sync`'s census+replay phases are the same pipeline the bulk engine reuses (per #3189's own framing: "the SAME engine the offline CLI rebuild command drives"). The bulk engine's own paged cursor query (`IndexGenerationStore.next_raw_page`) is unfiltered by any fixed snapshot -- it queries live `raw_sessions` ordered by `(acquired_at_ms, raw_id)` each pass -- so it naturally absorbs raws that arrive mid-build; there is no work left for the trickle pass on those same raws while bulk is in flight. What suppression does NOT touch: live-ingest acquisition (a separate watcher pipeline that only appends to `source.db`, which the bulk engine's live query already picks up), hook-spool drain, embedding catch-up, and already-committed session insight convergence (`_periodic_session_insight_convergence_after`) -- all separate periodic loops, untouched, verified by reading their call sites are outside `_periodic_raw_materialization_convergence`. - `_maybe_route_daemon_bulk_rebuild` now returns `bool` (pass genuinely attempted vs. structural no-op / swallowed failure) so a real pass failure during suppression falls back to the slower 30s outer interval instead of retrying every burst second -- matching how a trickle pass failure already escapes to the outer interval via the existing exception handlers. **Residual 2 (responsiveness evidence):** - `tests/unit/daemon/test_daemon_bulk_rebuild_responsiveness.py` (new): drives the REAL `polylogue.daemon.bulk_rebuild.run_daemon_bulk_rebuild_pass` (the exact production pass driver, not a stub) against a 150-raw fixture archive, CONCURRENTLY with 3 small writer-actor coroutines (standing in for live-ingest appends / hook-spool drain writes) sharing the real `DaemonWriteCoordinator` every other daemon writer actor uses, and asserts the small actors' p99 queued-wait time stays under a documented budget throughout the drain. - Budget derivation (not an arbitrary number): manual measurement at this exact fixture shape (150 raws, batch=8, 3 concurrent actors) found the correct bounded-pass implementation gives p99 wait ~0.32s (max single writer hold ~0.34s). Collapsing the SAME 150-raw corpus into one unbounded pass (reproducing a regression that drops per-pass batching) measured p99 wait ~1.26s. The shipped budget (1.0s) sits between those two real measurements -- several times the bounded-pass p99 for headroom against host CPU contention, while staying below what the unbounded-pass regression produces at this same fixture size. This makes it a genuine regression detector at this scale, not a loose ceiling nothing could ever hit (verified manually pre-submission; not shipped as a mutation test since it would require carrying the "bad" implementation variant in the test file). ## Verification ``` devtools test tests/unit/daemon/test_daemon_cli.py \ tests/unit/daemon/test_bulk_rebuild.py \ tests/unit/daemon/test_daemon_bulk_rebuild_responsiveness.py \ tests/unit/daemon/test_write_coordinator.py \ tests/unit/daemon/test_parse_prefetch.py => 134 passed in 11.78s (re-ran 3x for the new responsiveness test alone; stable) uv run mypy --strict polylogue/daemon/cli.py tests/unit/daemon/test_daemon_cli.py \ tests/unit/daemon/test_daemon_bulk_rebuild_responsiveness.py => Success: no issues found in 3 source files devtools verify --quick => exit_code 0 (ruff format/check, mypy, render all --check, layering, topology, docs-coverage, closure-matrix all clean; grepped for "out of sync" -- none found) ``` New tests added to `tests/unit/daemon/test_daemon_cli.py` (6): the in-flight check's flag gate (off never checks, on delegates to `has_resumable_daemon_bulk_rebuild_transaction`), trickle suppression while in flight (asserts the trickle drain's `daemon_write_coordinator().run_sync` is never called), failure fallback to the outer interval, and automatic resumption once the transaction is no longer in flight (trickle drain runs again on the very next tick, no operator action). Also backfilled `daemon_bulk_rebuild_routing = False` onto 4 pre-existing `FakeResolved` config stubs (parse-stage-split tests, #3168) that predate this attribute -- they broke with `AttributeError` once the new suppression check reads `daemon_bulk_rebuild_routing` unconditionally at the top of the burst loop; this is a mechanical fix restoring their original intent (`daemon_bulk_rebuild_routing` off, unrelated to what they test), not a behavior change to those tests. ## Non-obvious decisions - Suppression is per-tick and re-evaluated every burst iteration, not a one-shot decision at loop entry -- this is what makes "resume automatically once the bulk transaction promotes or is abandoned" true within the SAME daemon process run, not only across restarts. - Chose to keep `_maybe_route_daemon_bulk_rebuild`'s counts parameter (rather than adding a separate no-counts overload) and pass an empty `RawMaterializationCounts()` placeholder from the suppression branch -- the function's own guard already treats "resumable transaction exists" as sufficient without needing real counts (see its `not _bulk_scale_raw_materialization_backlog(counts) and not has_resumable(...)` early-return), so the placeholder never needs to carry real data. ## What this does NOT change - The bulk-rebuild engine itself (`polylogue/daemon/bulk_rebuild.py`, `polylogue/maintenance/rebuild_index.py`) -- unmodified. - Archive-scale equivalence receipt and CLI-command deletion (polylogue-4jsk) -- still coordinator-owned follow-up, unaffected by this PR. - The 3.14t daemon deploy itself (polylogue-dcz5) -- this PR removes two named blockers but does not perform the deploy. Ref polylogue-gd6v Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
Summary
Implements polylogue-gd6v (m6tp phase (c)): the daemon now routes a
bulk-scale raw backlog into a resumable, transactional, blue-green index
generation build running in-process, reusing the SAME engine the offline
polylogue ops maintenance rebuild-indexCLI command already drives — noduplicate implementation. Off by default behind a new
daemon_bulk_rebuild_routingconfig flag.Problem
The trickle raw-materialization conveyor is sized for steady-state drift;
a bulk-scale backlog turns it into a weeks-scale grind (#3145's own
threshold-warning evidence). The correct bulk path already existed as the
offline CLI command (resumable transaction, blue-green generation, one
census+replay sweep), but nothing routed to it automatically, and its own
resume model silently re-walks the whole corpus when a caller omits
--operation-id(polylogue-fbte: measured ~2.25h pure re-verification perresume on a ~50K-raw archive, observed on operation
ab5bad1f).Solution
polylogue/daemon/bulk_rebuild.py(new): resolves/resumes a singlewell-known bulk-rebuild transaction per archive
(
DAEMON_BULK_REBUILD_OPERATION_ID), drives it throughrebuild_index_from_source_sync, and retires a terminal(promoted/stale/failed) transaction so the id is reused for the next
backlog. The daemon never has an operator-supplied operation id to
forget, so the fbte bug class is structurally unreachable on this path.
polylogue/daemon/cli.py:_maybe_route_daemon_bulk_rebuild, calledfrom the existing raw-materialization convergence tick right after the
feat(daemon): warn loudly when raw materialization backlog is bulk-scale #3145 recommendation log line. Keeps driving an in-flight build every
tick even if the instantaneous backlog reading dips below threshold, so
partial progress is never abandoned. A dedicated
DaemonParseStagesingleton (separate pool from the trickle conveyor's own) shuts down at
daemon teardown alongside the existing one.
polylogue/daemon/parse_prefetch.py:DaemonParseStage.warm_raw_idsextracts
warm's dispatch/admit logic to also accept an explicit raw-idlist — the bulk driver already knows its next page's raw ids from the
transaction's own paged cursor, so it pre-warms them in the feat(daemon): extract parse stage from writer holds behind config flag #3168
thread pool before ever requesting the writer hold.
polylogue/sources/revision_backfill.py,polylogue/maintenance/replay.py,polylogue/maintenance/rebuild_index.py: thread a newprefetch_cacheparameter from
RebuildIndexRequestdown throughbackfill_historical_revision_evidence's census phase, mirroring thefeat(daemon): extract parse stage from writer holds behind config flag #3168 raw-materialization wiring. A prefetch hit flows through the same
spill.add(...)as a fresh parse, so the replay phase's ownspill.for_rawlookups see identical warmed content — census-phaseprefetching alone is enough to also skip replay-phase reparsing.
Degrades gracefully to the unmodified sequential in-hold parse on any
miss or on a GIL build (real parallel-parse speedup lands with the
3.14t deploy, m6tp phase (b); this PR is orchestration only).
polylogue/storage/index_generation.py:IndexGenerationStore.discard_transactionretires a terminaltransaction record so its operation id can be reused.
polylogue/config.py,docs/configuration.md:daemon_bulk_rebuild_routingconfig flag (off by default), samepattern as
daemon_parse_stage_split.Non-obvious decisions:
writer connection on the generation's own index.db." This PR's explicit
task constraints restate the single-writer invariant more strongly (main
process is the sole SQLite writer; parse workers only produce parsed
payloads, never write), so every bulk-rebuild write — like every other
daemon writer actor — is scheduled through the existing
daemon_write_coordinator().run_sync. Responsiveness instead comes frommoving parse off that hold (mirroring phase (a)'s
daemon_parse_stage_split), not a second concurrent writer thread.with the trickle conveyor while a build is in flight — both converge to
the same eventual state (the bulk build reads a snapshot and replays
into a separate generation file; promotion is atomic). The resulting
redundant per-raw work during the backlog window is a known,
deliberately out-of-scope efficiency gap — m6tp's own design sketch
flags watcher-pause/frozen-snapshot semantics as still-open questions
this PR does not attempt to resolve.
ops maintenance rebuild-indexremains;deletion is gated on an archive-scale equivalence receipt
(coordinator-owned, post-promote) per the gd6v AC. This PR delivers the
fixture-scale half of that gate.
Verification
New fixture-scale tests (
tests/unit/daemon/test_bulk_rebuild.py):test_daemon_bulk_rebuild_equivalent_to_cli_rebuild— builds the same6-raw corpus through the CLI path (single call,
prefetch_cache=None)and the daemon path (multi-pass,
DaemonParseStage-warmedprefetch_cache), asserts identicalsessions/messages/blocks/session_linksrow-level content,messages_ftsrow count, and sessioncount.
test_daemon_bulk_rebuild_pass_resumes_without_reprocessing_raw_ids+test_daemon_bulk_rebuild_pass_next_page_excludes_already_scheduled_raws— the fbte requirement: each bounded pass's transaction cursor
(
last_raw_id/processed_raw_count, written by the existingIndexGenerationStore.checkpoint_transaction) only ever advances; alater page is proven disjoint from every earlier page; a simulated
restart (fresh
DaemonParseStageper pass, mirroring a fresh process)resumes from the persisted cursor instead of re-walking the corpus.
test_resolve_or_start_creates_resumes_and_retires_transaction—transaction lifecycle: create, resume-unchanged, retire-on-terminal,
fresh restart at the same well-known operation id.
tests/unit/daemon/test_daemon_cli.py: 4 new tests for the routing gate(flag off never checks/drives; below-threshold+not-resumable is a
no-op; an in-flight operation keeps being driven below threshold; a
pass failure never propagates to the caller).
Remaining gd6v scope (follow-up, coordinator-owned)
(polylogue-4jsk).
during a real drain (this PR's responsiveness claim rests on parse
moving off the writer hold, same mechanism as phase (a); not
independently measured here).
(explicitly out of scope above).
Ref polylogue-gd6v
Co-Authored-By: Claude noreply@anthropic.com