fix(test): add real message content to raw-materialization fixtures - #3555
Merged
Conversation
Problem `devtools test tests/unit/storage/test_repair.py -k raw_materialization` failed 11 of 48 tests on master, reproducible on a clean checkout (polylogue-3jv24). Every failure traced back to the same cause: PR #3497 wired `require_positive_conversational_evidence` into `sources/revision_backfill.py` (the offline replay path raw materialization repair uses), refusing any parsed session with zero messages carrying real text/blocks. That PR never touched test_repair.py, and eleven of its raw materialization fixtures built raw payloads that were structurally valid (Codex `session_meta` only, or ChatGPT `{"mapping": {...}}` with empty node bodies) but carried no actual message content -- exactly what the new filter is designed to refuse. Once the filter went live, those sessions were silently dropped during replay, so `repaired_count`/`executed_count` assertions that previously reflected "this row got materialized" started coming back short or zero. This is not a data-loss regression in raw-authority replay: the filter is working as designed (it's already measured against the live archive and closes a real correctness gap, per its docstring). It's test-infra staleness -- fixtures that never exercised genuine conversational content because the repair-plumbing tests only cared about selection/batching/ retry semantics, not parse-content semantics, until the content gate started enforcing that distinction. Solution Added a minimal real message to each of the eleven affected fixtures: - Codex payloads: one `response_item` line with a `role":"user"` message and `input_text` content, appended after the existing `session_meta` line. - ChatGPT-shaped payloads (`test_raw_materialization_split_root_routes_ authority_replay`, `test_raw_materialization_uses_authority_replay_ not_legacy_batch_parser`): a `mapping` node carrying a real `message` object instead of an empty `{}`. Every touched fixture already overrides `blob_size` via a direct SQL UPDATE where size math matters, so appending message bytes does not perturb any size-based assertion. No production code changed. Verification - `devtools test tests/unit/storage/test_repair.py -k raw_materialization` -> 48 passed (was 11 failed / 37 passed) - `devtools test tests/unit/storage/test_repair.py` -> 66 passed - `devtools verify --quick` -> exit 0 (ruff format/check, mypy --strict, render all --check, layering, schema-versioning, and friends all green) Ref polylogue-3jv24 Co-Authored-By: Claude <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesRepair fixture updates
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
added a commit
that referenced
this pull request
Aug 19, 2026
…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>
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
Fixes 11 of 48 tests failing under
devtools test tests/unit/storage/test_repair.py -k raw_materializationon master (polylogue-3jv24). Test-fixture staleness, not a production bug: production code is unchanged.Problem
PR #3497 wired
require_positive_conversational_evidenceintosources/revision_backfill.py, the offline replay path used by raw materialization repair. That filter refuses any parsed session whose messages carry no real text/content blocks -- correct, deliberate behavior, already measured against the live archive and documented at length insources/dispatch.py. #3497 never touchedtest_repair.py, and eleven of its raw-materialization fixtures wrote raw payloads that were structurally valid (a bare Codexsession_metaline, or a ChatGPT{"mapping": {"node": {}}}with an empty node body) but carried zero actual message content -- exactly what the new gate is designed to refuse.Once the gate went live, those sessions were silently dropped during replay, so
repaired_count/executed_countassertions that previously reflected "this raw row got materialized" started coming back short or zero. Confirmed via direct reproduction: e.g.test_raw_materialization_execute_limits_authority_selectionassertedrepaired_count == 2and got0, withpolylogue-9ykn: refusing session execute-N ... no messages, no positive conversational evidencein captured stderr for every affected fixture.This is not a data-loss regression in raw-authority replay batching (the framing the tracking bead raised as a live possibility) -- it is test-infra staleness. These are repair-plumbing tests (authority selection, batch limits, retries, plan conservation) that only ever cared about selection/batching/retry semantics, never about parse-content semantics, until the content gate started enforcing that distinction.
Solution
Added a minimal, real message to each of the eleven affected fixtures in
tests/unit/storage/test_repair.py:response_itemline (role":"user",input_textcontent) after the existingsession_metaline.test_raw_materialization_split_root_routes_authority_replay,test_raw_materialization_uses_authority_replay_not_legacy_batch_parser): replace the emptymappingnode body with one carrying a realmessageobject.Every touched fixture that cares about byte-size math already overrides
blob_sizevia a direct SQLUPDATEafter the write, so appending message bytes does not perturb any size-based assertion. No non-test file changed.Verification
devtools test tests/unit/storage/test_repair.py -k raw_materialization-> 48 passed (was 11 failed / 37 passed)devtools test tests/unit/storage/test_repair.py-> 66 passeddevtools verify --quick-> exit 0 (ruff format/check, mypy --strict, render all --check, layering, schema-versioning, and the rest all green)Ref polylogue-3jv24
Summary by CodeRabbit