fix(devtools): give synthetic codex raw payloads real message text - #3642
Conversation
Problem: tests/unit/devtools/test_raw_authority_scale_proof.py's synthetic codex-session generator wrote a first-revision header carrying only a session_meta record with no message at all, and PR #3497 made require_positive_conversational_evidence refuse any session whose sole message has no real text. Every generated raw payload was refused outright ("no messages, no positive conversational evidence"), breaking 5 scale-proof unit tests and hanging tests/integration/test_raw_authority_daemon_health_proof.py (backlog that can never drain). Solution: extract a shared `_payload_header` helper used by both `_write_payload` and `_row_sizes`'s minimum-row-bytes accounting, so the two can't drift out of sync. The first write for a logical source now always includes a response_item message with non-empty text alongside session_meta. `_row_sizes`'s per-row floor is now the real header byte length (cumulative for chained/non-independent revisions, since each later revision physically copies the previous blob before appending its own header) instead of the old `256 * (revision + 1)` guessed constant, which would otherwise undershoot the true header size and silently inflate the generated corpus past the requested `total_payload_bytes`. Verification: `devtools test tests/unit/devtools/test_raw_authority_scale_proof.py` -- 21 passed (previously 5 failed with "no messages, no positive conversational evidence"). `ruff check`/`ruff format --check`/`mypy` on the touched file are clean. Ref polylogue-h7y0j
|
Warning Review limit reached
Next review available in: 25 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 Plus Run ID: 📒 Files selected for processing (1)
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 |
…gbm-quiet, 7qw4) (#4006) ## Summary Retires the four mechanical rows of the WS-B known-red/false-green ledger (`.agent/campaigns/2026-08-overhaul/ws-b-suite-health.md`): uxrim, bwo2l, vqt48, and 7qw4 were all already satisfied on master before this branch was cut; the one row that needed real work was polylogue-ndgbm's quiet component (Codex query-cardinality oracle's fixture never emitted a `message`-shaped record, so the parser's admission gate refused every payload and the module fixture errored at setup), which this PR fixes and whose two associated xfail markers it removes. ## Problem The ledger recorded five findings as known-red or false-green test debt, each with a RECIPE and file:line anchors, dispatched here as a themed sweep. ## Solution - **uxrim** (`tests/unit/api/test_facade_contracts.py:4789`): the recipe asked to extend the `frozen_clock_modules` marker to also cover `revision_governance`. Already done — commit 8ed4f91 (PR #3595, 2026-08-03) landed exactly that extension. No diff needed. - **7qw4** (`tests/unit/storage/test_store_ops.py`): the recipe asked to rewrite the test to call the production `aggregate_message_stats` async fn and delete a test-local shadow reimplementation. Already done — commits from PR #3729 (call the repository path) and PR #3813 (assert on public `origins`) landed this; no shadow function remains in the file. No diff needed. - **bwo2l + vqt48** (`tests/unit/cli/test_archive_maintenance_cli.py`, `tests/unit/sources/test_live_watcher.py`, `tests/unit/sources/test_live_batch_support.py`): the recipe asked to give refused codex fixtures real conversational content. Already done — the 9ykn positive-evidence-gate fixes landed across several prior PRs (visible in `git log --grep`: #3555, #3572, #3600, #3642, #3841). All 138 + 259 tests pass. - **ndgbm quiet component** (`tests/infra/query_manifest_oracle.py`, `tests/unit/cli/test_query_composition_laws.py`): the actual fix in this PR. `QueryCardinalitySession.wire_records()` emitted only `function_call`/`function_call_output` response_items — zero `message`-shaped records — so `codex.is_supported_session_stream()` (`has_message` never set) refused every payload before ingest and the module-scoped fixture errored at setup before either test could run. Added one planted user/assistant message-pair response_item per session (same wire shape landed for the bwo2l/vqt48 fixtures), which admits the stream; action-identity assertions are unaffected because they only count tool rows. Removing the two xfail markers this unblocks surfaced two further pre-existing issues that had never been reachable before (the tests always errored at setup), fixed in the same commit: - The naive-duplicate-id-join mutation test's hand-rolled `actions` view predates the production view's `result_state` CASE projection (polylogue-cuxz.5, `archive_tiers/index.py:1040`) and crashed with `sqlite3.OperationalError: no such column: a.result_state` instead of producing the comparably-wrong row count the test actually asserts on. Added the same CASE projection to the test's naive view. - `find ... then delete --yes --all` has no non-daemon route — it refuses outright when the daemon does not answer the `/prepare` call. Added a daemon-delete stand-in patch on `_submit_daemon_mutation`, mirroring the existing pattern in `test_verb_cardinality.py::TestDeleteCardinalityLargeNonMocked._daemon_delete_route` (does the real delete through `ArchiveStore`, just skips the daemon process). - `_copy_archive`'s plain `shutil.copytree` left the durable bootstrap marker bound to the *original* archive root path (`durable_identity_digest`), so the first write-mode `ArchiveStore.open_existing` at the copied path raised `DurableChangeTrainError: fresh durable bootstrap marker durable identity mismatch`. Re-bind the marker after copy, mirroring `tests/infra/workload_artifacts.py::clone_seeded_archive`. **ndgbm residual (not closed by this PR):** the bead also names a second, separate symptom — the 64-seed process-pool death only reproduces beside a saturated 10-worker gate (confirmed load-only: quiet re-run 20260818T231534Z was green in 43s). That's a load-robustness question the dispatch prompt asked to coordinate with WS-A's e98k cgroup-mmap-budget findings. I could not type this as a `partial` disposition in the machine-readable pr-scope carrier below: the validator requires a **pre-existing Beads dependency edge** (`blocks`/`discovered-from`/`relates-to`/`supersedes`) between ndgbm and its named successor before it will accept the disposition, and no such edge exists between polylogue-ndgbm and polylogue-e98k today. Per the lane contract I do not invoke `bd` from a worktree, so I did not create that link. **Coordinator action needed:** link `polylogue-ndgbm` → `polylogue-e98k` (or a dedicated new successor) via `bd dep add` (or equivalent) before ndgbm can be typed `partial` in a carrier; until then the bead should stay open with its residual noted in its own notes (already present). ## Verification - `devtools test tests/unit/api/test_facade_contracts.py -k raw_artifacts_read_source_tier` → 1 passed (uxrim, unchanged/confirmed) - `devtools test tests/unit/cli/test_archive_maintenance_cli.py` → 138 passed (bwo2l, unchanged/confirmed) - `devtools test tests/unit/sources/test_live_watcher.py tests/unit/sources/test_live_batch_support.py` → 259 passed (vqt48, unchanged/confirmed) - `devtools test tests/unit/storage/test_store_ops.py -k aggregate_message_stats` → 1 passed (7qw4, unchanged/confirmed); mutation red-check locally re-applied AG1 (role-swap on the unfiltered `role_row` query) and AG2 (`words_approx = 0`) against `polylogue/storage/sqlite/queries/stats.py`, both now fail (`assert 1 == 2`, `assert 0 == 9`), reverted before commit — no diff shipped for this row - `devtools test tests/unit/cli/test_query_composition_laws.py` → 2 passed, 0 xfail (ndgbm quiet component: red→green; both `-p no:testmon` and cached runs confirmed) - `devtools verify --quick` → exit 0 (format + lint + mypy + `render all --check`; no `out of sync` in output) All four rows were verified in the context that was failing: each command above is the exact standalone-context command named in the dispatch recipe, run directly (not nested inside a broader suite run). ## Bead disposition matrix | Bead | Disposition | Evidence | Notes | | --- | --- | --- | --- | | polylogue-uxrim | SATISFIED | test pass + commit 8ed4f91 | already fixed on master, PR #3595 | | polylogue-bwo2l | SATISFIED | test pass (138/138) | already fixed on master | | polylogue-vqt48 | SATISFIED | test pass (259/259) | already fixed on master | | polylogue-7qw4 | SATISFIED | test pass + mutation red-check | already fixed on master (PR #3729, #3813) | | polylogue-ndgbm | PARTIAL | test pass (2/2, no xfail) + diff | quiet component fixed this PR; load-only pool-death residual stays open on the bead — **not represented in the typed carrier below** (see Solution section: needs a coordinator-created Beads link to type as `partial`) | <!-- polylogue-pr-scope:v2 { "assigned_beads": [ "polylogue-uxrim", "polylogue-bwo2l", "polylogue-vqt48", "polylogue-7qw4" ], "dispositions": [ { "bead_id": "polylogue-uxrim", "disposition": "satisfied", "evidence": [ { "kind": "test", "ref": "tests/unit/api/test_facade_contracts.py::test_archive_tiers_api_raw_artifacts_read_source_tier" }, { "kind": "commit", "ref": "8ed4f9101813aa142bc915e9c70ef439b86a100d" } ], "successors": [] }, { "bead_id": "polylogue-bwo2l", "disposition": "satisfied", "evidence": [ { "kind": "test", "ref": "tests/unit/cli/test_archive_maintenance_cli.py" } ], "successors": [] }, { "bead_id": "polylogue-vqt48", "disposition": "satisfied", "evidence": [ { "kind": "test", "ref": "tests/unit/sources/test_live_watcher.py" }, { "kind": "test", "ref": "tests/unit/sources/test_live_batch_support.py" } ], "successors": [] }, { "bead_id": "polylogue-7qw4", "disposition": "satisfied", "evidence": [ { "kind": "test", "ref": "tests/unit/storage/test_store_ops.py::test_aggregate_message_stats_reports_public_contract" } ], "successors": [] } ], "mutated_beads": [], "scope_digest": "e397dd57058877a651382c5cf8f681b4cffabc7aae67e285f7192b2c62c787ad", "scope_kind": "bead", "version": 2 } --> Ref polylogue-uxrim, polylogue-bwo2l, polylogue-vqt48, polylogue-ndgbm, polylogue-7qw4 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01HWcPJJJvuF25CqVwTFgSQC Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Fixes the raw-authority-scale-proof synthetic Codex corpus generator so every generated raw payload carries at least one message with real text, and re-derives its minimum-row-bytes accounting from the real header size instead of a guessed constant.
Problem
devtools/raw_authority_scale_proof.py's synthetic codex-session generator wrote the first revision of each logical source as a JSONL payload containing only asession_metarecord with no message at all. PR #3497 maderequire_positive_conversational_evidence(sources/dispatch.py) refuse any session whose sole message carries no real text, and by extension refuse a session that has no message at all. Every generated raw session was refused outright ("no messages, no positive conversational evidence"), which broke 5 tests intests/unit/devtools/test_raw_authority_scale_proof.pyand madetests/integration/test_raw_authority_daemon_health_proof.py::test_real_daemon_drains_backlog_while_staying_probeablehang for the full 120s timeout, since the synthetic backlog it drives a real daemon against could never actually drain (every candidate refused, forever).Reproduced independently of
polylogue-k2grh'stest_raw_authority_ledger.pyfix (stashed that fix out, identical failure), confirming this is a distinct instance of the same PR #3497 fallout class, aspolylogue-h7y0jdescribes.Solution
_payload_header(native_id, revision, *, first)helper used by both_write_payload(the actual JSONL writer) and_row_sizes(the byte-budget planner), so the two can never drift out of sync again. Thefirst=True(i.e.previous is None) header now always includes aresponse_itemmessage with non-empty text alongsidesession_meta._row_sizes's per-row minimum-byte floor is now the real cumulative header length for that row (each chained/non-independent revision physically copies the previous row's blob before appending its own new header, so its floor must be the previous row's floor plus its own header, not each row's header length in isolation) instead of the old256 * (revision + 1)guessed constant. That constant happened to be generous enough for the old (smaller) header, but undershot the new header size for small scenarios, which silently inflated the generated corpus past the requestedtotal_payload_bytes(SUM(blob_size)from a manual repro measured 8465 vs. a requested 8192 before this fix, exactly right after fixing it).Verification
devtools test tests/unit/devtools/test_raw_authority_scale_proof.py— 21 passed (previously 5 failed withpolylogue-9ykn: refusing session ... no messages, no positive conversational evidence).tests/integration/test_raw_authority_daemon_health_proof.py::test_real_daemon_drains_backlog_while_staying_probeable: with the fix, the synthetic backlog now actually drains (no more refusal warnings in the daemon log, confirmed by grep) instead of hanging for the full 120s timeout as it does on unfixedorigin/master(reproduced: unfixed run hitFailed: Timeout (>120.0s) from pytest-timeout). The test still fails on this host, but on a different, later assertion (/api/status was unresponsive for ~20-29s, exceeding the documented 5.00s bound) — this looks like host-load-driven flakiness rather than a regression from this change: the failure duration varies run to run (20.21s, then 29.29s on a second attempt), the daemon log shows no error for/api/statusat all (just heavy background-thread activity), and this host is currently under substantial contention from concurrent agent worktrees (uptimeload average ~28 on 24 cores, ~15 GiB swapped at the time of testing). Recommend re-verifying this specific integration test on a quieter host/CI; not something a fixture-text fix should paper over by loosening the responsiveness bound.ruff check/ruff format --check/mypy --strictclean on the touched file.devtools verify --quick— exit 0.Ref polylogue-h7y0j