fix(config): route the un-inventoried env-only readers through config.py#3248
Merged
Conversation
Lands the residual polylogue-uu8r-class scope PR #3243 explicitly deferred: 8 genuinely POLYLOGUE_*-namespaced settings that had NO config-inventory entry at all (not merely a bypassing caller for an already-inventoried key). Re-derived the file list live against master rather than trusting the bead's stale 31-file/7-8-file counts. Problem ------- investigations/config-resolution.md's dogfood-2 round-2 config investigation found config.py documents a 5-layer TOML/env precedence system, but 8 settings across 7 files bypassed it entirely because they were never inventoried in the first place: hook-provider detection override, raw-authority repair commit batch size, 4 daemon parse-stage prefetch knobs (worker count, in-flight-bytes budget, cached-tree-bytes budget, warm timeout), and 2 revision-parse dispatch thresholds. TOML site/user configuration could never reach any of them. Solution -------- - Added 8 ConfigInventoryEntry rows to polylogue/config.py plus matching PolylogueConfig properties, `_default_config_values` defaults, and `_INT_CONFIG_KEYS`/`_FLOAT_CONFIG_KEYS` coercion entries, following the existing owner_class/reload_behavior conventions (mirrors `ingest_commit_batch_messages`/`daemon_parse_stage_split` siblings). - Migrated each call site's `os.environ.get(...)` read to `load_polylogue_config().<key>`, preserving each function's existing "override wins, invalid/absent falls back to the module default" semantics exactly (no behavior change for unconfigured deployments). - Documented all 8 new keys in docs/configuration.md (both the TOML-key table and the env-var quick-reference table). Judgment calls (env-only exemptions, NOT routed through config.py): - `POLYLOGUE_DEV_LOOP_RUN_ID` / `POLYLOGUE_DEV_LOOP_LOG_DIR` (daemon/cli.py, daemon/http.py) and `POLYLOGUE_SESSION_REF` (coordination/envelope.py) are launcher/harness-injected correlation metadata for one process invocation, structurally identical to `CODEX_SESSION_ID`/`CLAUDE_SESSION_ID` (already out of scope), not an operator TOML preference. Documented at each call site plus a docs/configuration.md note. - `POLYLOGUE_CONFIG` (cli/commands/embed.py) is the bootstrap variable that selects *which* user TOML file the layered resolver loads, so it cannot itself be resolved through that resolver -- mirrors config.py's own `_user_config_path`/`_site_config_path` bootstrap reads. - `POLYLOGUE_ARCHIVE_ROOT` inside daemon/http.py's `_dev_loop_payload` stays a raw read: that endpoint is a launcher-diagnostic probe reporting whether the daemon's OWN environment has the var set, not a config consumer -- resolving it through the layered reader would mask an unset env var behind the TOML/default fallback and defeat the probe. Subprocess-spawn safety: the 4 daemon parse-stage knobs are read from `DaemonParseStage.__init__` in the daemon's own ThreadPoolExecutor setup (never inside a forked/spawned worker); the 2 revision-parse thresholds and the raw-authority batch size are read in dispatcher/orchestration code before a ProcessPoolExecutor is created, never inside a worker function. `load_polylogue_config()` carries no multiprocessing-spawn hazard at any of these call sites. Verification ------------ - `devtools test tests/unit/core/test_config_resolution_regression.py`: 16 passed (6 pre-existing fd2s/cxlk/uu8r-class + 10 new, one class per newly-inventoried setting family against its real consumer, each with a revert-witness docstring). Live-verified the raw_authority witness by reverting the raw-authority call site to `os.environ.get` and confirming the test fails (20 != 4242) before restoring the fix. - Touched-consumer files, individually: test_config.py (95), test_config_inventory.py (60), test_hooks.py (8), test_repair.py (55), test_revision_backfill.py (44), test_parse_prefetch.py (12), test_envelope.py (26), test_embed.py (18), test_backup.py (23) -- all passed. - `devtools verify --quick`: ruff format/check, mypy --strict, render all --check, topology/layering/closure-matrix, docs-coverage, and the rest of the quick gate all green (exit_code 0). Ref polylogue-uu8r; last open child of polylogue-9gh1. Co-Authored-By: Claude <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 24 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 (11)
✨ 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 |
The lane's repair.py edit was lost to worktree auto-cleanup before commit (the regression test referenced it and failed on the pushed branch); reconstructed by the coordinator, semantics identical to the env escape hatch (<=0 disables batching). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QUsH3Rhq6oAZpYPWcsZqnZ
Owner
Author
|
Coordinator review (CodeRabbit rate-limited): full diff read. Migration patterns match #3243's provenance conventions; the exemption table (dev-loop correlation ids, POLYLOGUE_CONFIG bootstrap, raw diagnostic probe) is correctly reasoned and documented at call sites + docs/configuration.md. NOTE: the lane's repair.py edit was lost to worktree auto-cleanup before commit (its own regression test failed on the pushed branch — caught by coordinator re-run); reconstructed in a766b8a with identical env-escape-hatch semantics, 10/10 regression tests + mypy strict green. The process_pool.py leftover bypass for the already-inventoried parse-workers key is noted as a known residual. Merging; this closes the last open child of polylogue-9gh1. |
Sinity
added a commit
that referenced
this pull request
Jul 21, 2026
#3249) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QUsH3Rhq6oAZpYPWcsZqnZ
Sinity
added a commit
that referenced
this pull request
Jul 21, 2026
…alation pass (polylogue-t93b) (#3256) ## Summary Adds an escalation tier so the daemon converges whale-scale raw-authority components (aggregate payload over the ordinary 64MiB fast-path envelope) instead of leaving them permanently resource-blocked. Live witness: `codex:019f49d8` (788 raws, 6.33GB) has zero index presence because every ordinary daemon pass logs "resource-blocked ... exceed replay limit" and moves on, forever. ## Problem `_RAW_MATERIALIZATION_DAEMON_BLOB_LIMIT_BYTES` (64MiB, `daemon/cli.py:89`) exists to bound writer-hold transaction length and parse memory during the daemon's ordinary raw→index convergence pass. It works correctly as a *fast-path* limit, but a component that exceeds it has no escalation path — the daemon owns raw→index convergence end to end, so a permanent manual/offline requirement for oversized components is a policy bug (operator ruling, automagic-invariants doctrine). The two concerns the limit protects already have productized answers elsewhere in the codebase: stream-record parse dispatch (`is_stream_record_provider`, already routes Codex/Claude Code/Beads/Hermes through `parse_stream_payload`), the `RawParsePrefetchCache` inflight-byte budget, a dedicated whale-residency parsed-tree spill tier (`_ParsedSessionSpill`, polylogue-odm1), and commit-batched replay (`raw_authority_commit_batch_size`, PR #3248). The resource-block gate itself is purely an admission check on `max_payload_bytes` — nothing else in the pipeline needed to change. ## Solution Escalation tier, not a blanket limit raise: 1. **Ordinary fast-path limit unchanged.** The 64MiB envelope still governs every regular daemon pass. 2. **Quiescence-triggered whale pass** (`polylogue/daemon/cli.py`): `_periodic_raw_materialization_convergence` now tracks whether the ordinary trickle conveyor reached genuine quiescence this tick (no remaining candidates, or no progress — never on bulk-rebuild-in-flight or browser-capture-spool-pending breaks, which are deferrals for unrelated reasons). On quiescence, `_maybe_run_raw_materialization_whale_pass` looks up one resource-blocked, entirely stream-safe component via a new read-only selector and runs a dedicated pass for it, scoped by `raw_artifact_id` at a widened envelope (`raw_authority_whale_payload_bytes`, default 8 GiB), through the same writer coordinator. Emits `raw_materialization_whale_pass_started`/`_completed` daemon events. Gated by `daemon_whale_raw_materialization` (**on by default** — the whole point is the daemon self-heals codex:019f49d8 without an operator flipping a switch). 3. **Non-stream-safe components stay blocked, distinctly.** `polylogue/storage/repair.py` adds `_raw_materialization_component_stream_safe` (all members) and `raw_materialization_whale_pass_candidate` (fairness-ordered, read-only `mode=ro`, reuses the existing component/fairness machinery — never selects a component with any non-stream-safe member). New `raw_materialization_non_stream_safe_oversized_count` metric complements the existing `raw_materialization_stream_oversized_count`. `polylogue/sources/revision_backfill.py`'s `record_resource_blocked_revision_census` now records whether the blocked component is stream-safe, so the durable per-raw census detail text says `"escalation-eligible: stream-safe, awaiting a bounded daemon whale pass"` vs `"escalation-blocked: non-stream-safe, requires manual/offline convergence"` — without touching the admission fingerprint identity (re-admission-on-wider-envelope invariant preserved). 4. **Product boundary.** `polylogue/product/raw_authority.py`'s `repair_materialization`/new `whale_pass_candidate` wrappers thread `raw_artifact_id` through to storage, keeping the daemon off `polylogue.storage.repair` internals for this call. 5. **Config surface.** `raw_authority_whale_payload_bytes` (escalation envelope) and `daemon_whale_raw_materialization` (on/off switch) added to the config inventory, TOML/env layering, and `docs/configuration.md`. ### Alternatives rejected - Raising the ordinary 64MiB limit globally — bead explicitly rules this out ("escalation tier, not a blanket limit raise"); it would remove the writer-hold/parse-memory bound for *every* pass, not just whale components. - Building new streaming/memory-bounding machinery — unnecessary; the codebase already has it (see Problem section), so the fix is pure orchestration (when to widen the envelope, for what, gated how). ## Verification ``` devtools test tests/unit/storage/test_repair.py tests/unit/daemon/test_daemon_cli.py \ tests/unit/sources/test_revision_backfill.py tests/unit/core/test_config_inventory.py \ tests/unit/core/test_config.py tests/unit/core/test_config_resolution_regression.py # 382 passed, 1 pre-existing unrelated failure # (test_bulk_rebuild_routing_resumable_transaction_drives_pass_even_below_threshold, # reproduced identically on unmodified master via `git stash`) devtools verify --quick # format/lint/mypy/render-all-check/topology/layering/closure-matrix/manifests/ # ci-workflows/doc-commands/docs-coverage/test-infra-currency/test-clock-hygiene/ # pytest-timeout-overrides/degrade-loudly all green ``` Anti-vacuity (mutated production code, confirmed red, reverted, confirmed green again): - Deleting the daemon's quiescence-triggered whale-pass call (`if quiescent and not ...: await _maybe_run_raw_materialization_whale_pass()`) breaks `test_periodic_raw_materialization_convergence_schedules_whale_pass_on_quiescence` and `test_periodic_raw_materialization_convergence_skips_whale_pass_mid_burst`. - Forcing `raw_materialization_whale_pass_candidate` to always return `None` breaks `test_raw_materialization_whale_pass_candidate_selects_stream_safe_blocked_component` and `test_raw_materialization_whale_pass_converges_blocked_component_to_resolved_head`. New regression tests build a synthetic multi-raw "whale" fixture (`tests/infra/revision_backfill_benchmark.build_revision_chain_corpus` — a growing-file revision chain, the exact shape of the live codex:019f49d8 witness) exceeding a lowered test envelope, and prove: the ordinary pass blocks it; the whale pass converges it to a resolved head (`sessions` row materialized in `index.db`); commit-batch size genuinely controls writer-hold granularity (`test_raw_materialization_whale_pass_commit_batches_bounded`); and a non-stream-safe oversized component gets the distinct typed census reason (`test_raw_materialization_ordinary_pass_census_detail_distinguishes_escalation_eligibility`). Not run: full `devtools verify` (broad local gate) — the touched-file targeted run above plus `--quick` cover the changed surface; a full run was not required for this scope. Live-archive verification of `codex:019f49d8` itself is out of scope for this PR (would touch the live archive, which agent work must never do) — it is the deploy-time acceptance witness per the bead. ## AC matrix (polylogue-t93b) | Acceptance criterion | Status | | --- | --- | | Component whose bytes exceed the daemon limit but is fully stream-safe converges through the daemon (no offline pass) | Satisfied — quiescence-triggered whale pass, same `repair_raw_materialization` entrypoint, `raw_artifact_id`-scoped | | Writer-hold bounded (commit batches) | Satisfied — `raw_authority_commit_batch_size` unchanged, threaded through unmodified; test proves finer batch size yields strictly more commit boundaries | | Memory bounded (streaming parse + inflight budget) | Satisfied — inherited unmodified from existing `is_stream_record_provider` dispatch + `RawParsePrefetchCache`/`_ParsedSessionSpill` whale tier; this PR adds no new parse code | | Non-stream-safe oversized components get a distinct typed blocked reason | Satisfied — new `escalation-eligible`/`escalation-blocked` durable census detail text + `raw_materialization_non_stream_safe_oversized_count` metric | | Regression test with synthetic whale fixture | Satisfied — `tests/unit/storage/test_repair.py` (5 new tests) + `tests/unit/daemon/test_daemon_cli.py` (5 new tests) | | Live witness codex:019f49d8 resolves after deploy | Deferred to deploy — `daemon_whale_raw_materialization` defaults on, so no operator action is needed; not verified against the live archive in this PR by design (agent work never touches the live archive) | | Daemon event receipts recorded | Satisfied — `raw_materialization_whale_pass_started`/`_completed` events via `emit_daemon_event`, tested | Ref polylogue-t93b
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
Lands the residual polylogue-uu8r-class scope PR #3243 explicitly deferred: 8 genuinely
POLYLOGUE_*-namespaced settings that had no config-inventory entry at all (distinct from an already-inventoried key whose caller merely bypassed the resolver — that class was closed by #3243). Re-derived the file list live against master withrg 'os\.environ.*POLYLOGUE_' polylogue/rather than trusting the bead's stale 31-file/7-8-file counts, which had drifted after #3243 and #3079 landed.Problem
investigations/config-resolution.md's dogfood-2 round-2 config investigation foundconfig.pydocuments a 5-layer TOML/env precedence system, but several settings bypassed it entirely because they were never inventoried in the first place — so TOML could never reach them regardless of caller migration:POLYLOGUE_HOOK_PROVIDER(hook-harness detection override)POLYLOGUE_RAW_AUTHORITY_COMMIT_BATCH_SIZE(raw-materialization repair batching)POLYLOGUE_DAEMON_PARSE_STAGE_{WORKERS,MAX_INFLIGHT_BYTES,MAX_CACHED_TREE_BYTES,WARM_TIMEOUT_SECONDS}POLYLOGUE_REVISION_PARSE_{DISPATCH_MAX_BYTES,POOL_MIN_BYTES}Solution
ConfigInventoryEntryrows topolylogue/config.pyplus matchingPolylogueConfigproperties,_default_config_valuesdefaults, and_INT_CONFIG_KEYS/_FLOAT_CONFIG_KEYScoercion entries, following existingowner_class/reload_behaviorconventions (mirrors theingest_commit_batch_messages/daemon_parse_stage_splitsiblings).os.environ.get(...)read toload_polylogue_config().<key>, preserving each function's existing "explicit override wins, invalid/absent falls back to the module default" semantics exactly — no behavior change for unconfigured deployments.docs/configuration.md(TOML-key table + env-var quick-reference table).Per-setting judgment table
hook_providersources.hook_provider)themereference pattern (env wins, falls to TOML otherwise).raw_authority_commit_batch_sizepipeline.raw_authority.commit_batch_size)ingest_commit_batch_messages.daemon_parse_stage_workers/max_inflight_bytes/max_cached_tree_bytes/warm_timeout_secondsdaemon.raw_materialization.parse_stage_*)daemon_parse_stage_split; read fromDaemonParseStage.__init__in-process (ThreadPoolExecutor, never a spawned worker) — no multiprocessing-spawn hazard.revision_parse_dispatch_max_bytes/pool_min_bytespipeline.revision_parse.*)ProcessPoolExecutorcreation, never inside a worker — no spawn hazard.POLYLOGUE_DEV_LOOP_RUN_ID/POLYLOGUE_DEV_LOOP_LOG_DIRCODEX_SESSION_ID/CLAUDE_SESSION_ID(already out of the config domain), not an operator TOML preference. Documented at each call site +docs/configuration.md.POLYLOGUE_SESSION_REFCODEX_THREAD_ID/CODEX_SESSION_ID/CLAUDE_SESSION_IDincoordination/envelope.py.POLYLOGUE_CONFIGconfig.py's own_user_config_path/_site_config_pathbootstrap reads.POLYLOGUE_ARCHIVE_ROOTindaemon/http.py::_dev_loop_payloadarchive_rootitself IS inventoried, but this endpoint reports whether the daemon's own environment has the var set (launcher diagnosis), not a config value; routing through the resolver would mask an unset env var behind the TOML/default fallback and defeat the probe's purpose.Verification
devtools test tests/unit/core/test_config_resolution_regression.py: 16 passed (6 pre-existing fd2s/cxlk/uu8r-class + 10 new — one regression class per newly-inventoried setting family, against its real consumer, each with a revert-witness docstring). Live-verified the raw-authority witness by reverting that call site toos.environ.getand confirming the test fails (20 != 4242) before restoring the fix.test_config.py(95),test_config_inventory.py(60),test_hooks.py(8),test_repair.py(55),test_revision_backfill.py(44),test_parse_prefetch.py(12),test_envelope.py(26),test_embed.py(18),test_backup.py(23) — all passed.devtools verify --quick: ruff format/check, mypy --strict,render all --check(grepped for "out of sync": none), topology/layering/closure-matrix, docs-coverage, and the rest of the quick gate all green (exit_code: 0), both standalone and via the pre-push hook.AC matrix (polylogue-uu8r)
os.environPOLYLOGUE_* read outsideconfig.pyis classifiedrg, 7 files / 8 settings genuinely un-inventoried; 4 settings classified as intentional env-only/bootstrap exemptions with rationale documented at call sites + docs.config.py_CONFIG_INVENTORY(config_inventory_payload,_toml_section_layout,_ENV_CONFIG_KEY_MAPall derive from the inventory list).devtools teston affected files +config.pypassesRef polylogue-uu8r; last open child of polylogue-9gh1.