fix(reindex): restore source-authoritative acceptance - #3949
Conversation
📝 WalkthroughWalkthroughChangesThe pull request changes managed pytest storage and resource selection, adds cgroup-aware budgeting, improves event tracking, updates stale basetemp cleanup, revises archive replay and logical-head governance, adds source-semantics archive identity, and updates daemon and archive integration paths. Pytest runtime
Archive governance
Archive operations
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
polylogue/maintenance/archive_verification.py (1)
2403-2420: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExclude valid supersession receipts from the convergence backlog.
_check_source_index_coverage_at_index_pathtreats a validated supersession receipt as covered._unindexed_backlog_gapdoes not validate or exclude that receipt type.A valid unindexed byte duplicate will therefore produce a nonzero I6 gap. The convergence check can then return
WARNINGorERROReven though I1 reports no actionable gap.Apply the same receipt, blob, and indexed-twin validation before this query counts a head. Add a regression test that a valid receipt produces
unindexed_backlog_gap == 0.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@polylogue/maintenance/archive_verification.py` around lines 2403 - 2420, Update _unindexed_backlog_gap to validate and exclude valid supersession receipts using the same receipt, blob, and indexed-twin checks as _check_source_index_coverage_at_index_path before counting a head. Preserve counting for genuinely unindexed duplicates, and add a regression test asserting a valid supersession receipt yields unindexed_backlog_gap == 0.tests/unit/devtools/test_verify.py (1)
324-339: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTwo tests exercise the real basetemp placement resolver without isolating it. Both sites let
resolve_pytest_basetemp_rootprobe real host paths and real free space, so the result depends on whether/realm/tmpis mounted and on the actual free space of the referenced directory. The module already provides_patch_basetemp_rootsand_patch_resource_capacityfor exactly this purpose.
tests/unit/devtools/test_verify.py#L324-L339: call_patch_basetemp_roots(monkeypatch, tmp_path, realm_mounted=True)and assertPOLYLOGUE_PYTEST_BASETEMP_ROOTagainst the returned scratch path instead of the module constant.tests/unit/devtools/test_verify.py#L2297-L2316: create thenvme_rootdirectory or patchverify_runs._fs_usageso the configured root clears the headroom preflight and_rundoes not return 125.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/devtools/test_verify.py` around lines 324 - 339, Isolate both tests from real basetemp placement and filesystem capacity. In tests/unit/devtools/test_verify.py lines 324-339, call _patch_basetemp_roots(monkeypatch, tmp_path, realm_mounted=True) and assert the environment value against its returned scratch path rather than verify_runs.DEFAULT_PYTEST_BASETEMP_ROOT; in lines 2297-2316, create the nvme_root directory or patch verify_runs._fs_usage so the configured root passes headroom preflight and _run does not return 125.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 512-519: Update the pytest scratch-space documentation near the
focused-run description to state that bare pytest invocations without
POLYLOGUE_VERIFY_RUN_ID or an explicit basetemp root force
POLYLOGUE_PYTEST_TMPFS=0 and use the NVMe default. Clarify that bounded /dev/shm
tmpfs applies only to managed devtools test/verify runs when the effective
budget permits it.
In `@devtools/verify.py`:
- Around line 2199-2208: Define shared constants for the broad pytest labels and
reuse them in both build_verify_steps and _pytest_uses_full_suite_basetemp.
Replace duplicated label literals, including the labels currently checked by the
startswith tuple, so renaming a broad step updates both resource-admission paths
consistently.
In `@polylogue/daemon_client.py`:
- Around line 100-104: The health probe in polylogue/daemon_client.py lines
100-104 must retain the response status and, when accept_degraded is true,
accept HTTP 503 only if the typed lifecycle field indicates the supported
degraded state; preserve normal HTTP 200 acceptance and reject other 503 states.
In tests/unit/cli/test_daemon_client.py lines 116-164, add a matching
non-degraded 503 fixture and assert that accept_degraded=True rejects it.
In `@tests/unit/devtools/test_run_tests.py`:
- Around line 36-50: Strengthen
test_build_pytest_cmd_forwards_all_xdist_worker_spellings by asserting that the
generated command contains exactly one worker-count flag for each selection,
preventing _worker_args from adding a conflicting default such as “-n 0”. Retain
the existing assertions that every supplied argument is forwarded.
In `@tests/unit/devtools/test_verify.py`:
- Around line 1692-1709: Update
test_production_pytest_commands_reserve_every_xdist_spelling to remove
PYTEST_XDIST_AUTO_NUM_WORKERS from os.environ before exercising the three auto
worker-argument cases, ensuring _pytest_command_concurrency uses the CPU-count
fallback independently of the host environment while leaving the other parameter
cases unchanged.
---
Outside diff comments:
In `@polylogue/maintenance/archive_verification.py`:
- Around line 2403-2420: Update _unindexed_backlog_gap to validate and exclude
valid supersession receipts using the same receipt, blob, and indexed-twin
checks as _check_source_index_coverage_at_index_path before counting a head.
Preserve counting for genuinely unindexed duplicates, and add a regression test
asserting a valid supersession receipt yields unindexed_backlog_gap == 0.
In `@tests/unit/devtools/test_verify.py`:
- Around line 324-339: Isolate both tests from real basetemp placement and
filesystem capacity. In tests/unit/devtools/test_verify.py lines 324-339, call
_patch_basetemp_roots(monkeypatch, tmp_path, realm_mounted=True) and assert the
environment value against its returned scratch path rather than
verify_runs.DEFAULT_PYTEST_BASETEMP_ROOT; in lines 2297-2316, create the
nvme_root directory or patch verify_runs._fs_usage so the configured root passes
headroom preflight and _run does not return 125.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9562f077-520f-4eeb-9bcf-b1ca14b6287b
📒 Files selected for processing (39)
CLAUDE.mdTESTING.mddevtools/campaign_archive_location.pydevtools/run_tests.pydevtools/scale_regression_probe.pydevtools/verify.pydevtools/verify_runs.pydevtools/verify_slos.pypolylogue/core/metrics.pypolylogue/daemon/bulk_rebuild.pypolylogue/daemon_client.pypolylogue/maintenance/archive_verification.pypolylogue/maintenance/reindex_canary.pypolylogue/schemas/sampling_db.pypolylogue/sources/assembly_chatgpt.pypolylogue/sources/emitter.pypolylogue/sources/revision_backfill.pypolylogue/storage/sqlite/archive_tiers/revision_governance.pytests/conftest.pytests/infra/pathology_zoo.pytests/infra/reindex_campaign.pytests/infra/workload_artifacts.pytests/unit/api/test_operation_executor_routes.pytests/unit/cli/test_daemon_client.pytests/unit/core/test_metrics_cgroup_memory_limits.pytests/unit/core/test_schema_generation.pytests/unit/daemon/test_bulk_rebuild_ownership.pytests/unit/devtools/test_index_fast_forward.pytests/unit/devtools/test_run_tests.pytests/unit/devtools/test_slo_catalog.pytests/unit/devtools/test_verify.pytests/unit/infra/test_workload_artifacts.pytests/unit/maintenance/test_archive_verification.pytests/unit/maintenance/test_reindex_campaign.pytests/unit/maintenance/test_reindex_canary.pytests/unit/operations/test_mutation_actuators.pytests/unit/sources/test_source_laws.pytests/unit/storage/test_revision_replay.pytests/unit/test_pytest_temp_policy.py
💤 Files with no reviewable changes (2)
- polylogue/maintenance/reindex_canary.py
- tests/unit/maintenance/test_reindex_canary.py
![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: a6080a5b2e
ℹ️ 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".
a6080a5 to
81da9e9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81da9e9e70
ℹ️ 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".
Route stale multi-tier fixtures through the owned active-root bootstrap. Preserve the fast-forward test generation layout after bootstrap, and allow campaign ownership acquisition to create a documented fresh output directory before its descriptor is locked.
Seed test-owned external evidence and run the schema-inference gate before ownership assertions. The ownership tests isolate source admission because the minimal gate corpus intentionally has no replay census.
Cached real-pipeline archives were keyed by Git HEAD and a short recipe file list. Dirty worktree parser changes could therefore reuse stale archives and produce order-dependent fingerprint failures. Include the actual parser and lowering semantic digest in cache identity and manifest provenance.
Retained-raw replay now reapplies only frozen source-tier enrichment, including every spill fallback, so candidate hashes and titles do not depend on cache order or ambient files. Source coverage, typed failure dispositions, pathology fixtures, degraded daemon discovery, and candidate campaign checks now exercise the same authority boundaries used by the production rebuild.
Problem: convergence freshness and source/index coverage could classify durable revision evidence differently. A retired raw lost its raw-level cohort key, and an unsafe configured tmpfs root remained selected after policy disabled tmpfs.\n\nWhat changed: share the durable logical-cohort and content-bound supersession rules across both verification paths, preserve the sole retained membership identity, reroute unsafe configured tmpfs roots to scratch, and pin the receipt-removal and shared-raw counterexamples.\n\nCompatibility: existing multi-membership raw acquisitions retain their native/path fallback because no single membership identity is authoritative.
81da9e9 to
124989c
Compare
|
@codex review Please review exact head |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…urable-fixtures # Conflicts: # devtools/verify.py
…urable-fixtures # Conflicts: # tests/unit/sources/test_source_laws.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3817f13520
ℹ️ 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".
|
@codex review Please review exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3817f13520
ℹ️ 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".
Bind byte-duplicate supersession to the source coordinate that can affect replay, and share canonical provider/origin cohort identity across verification and schema sampling.
Honor durable non-session evidence during the historical and frozen replay census so malformed terminal artifacts are accounted for without parser dispatch.
Classify generated seed-testmon shards as broad pytest runs and include session-emitter behavior in seeded artifact source semantics.
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
polylogue-9kj24: PR #3949 narrowed plan_byte_duplicate_supersession to require a twin sharing (origin, source_path, source_index), but raw_artifacts carries a UNIQUE index on exactly that triple. The only duplicate class the planner still recognises is one the schema prevents from existing, which leaves four tests red against the pre-#3949 cross-path semantics. Needs a product decision rather than a test edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWcPJJJvuF25CqVwTFgSQC
#3949 narrowed plan_byte_duplicate_supersession's twin match to require an identical (origin, source_path, source_index), which made the class it detects unreachable: raw_artifacts carries a UNIQUE index on exactly that triple for every non-deferred artifact kind, so two raws sharing one coordinate cannot both exist. The duplicates that do exist -- polylogue-vzn6 measures them at ~46% of stored blob bytes -- are identical bytes acquired at different paths, e.g. the same export re-downloaded. Origin equality is kept so a codex raw cannot supersede a chatgpt one. The real safety gate was always the next step, which only counts a twin that is already materialized in index.db. Also fixes the stale-observation refresh case: inspect_raw_artifact keys the observation id on the origin, the test seeded it from the provider, so the refresh missed the seeded row and inserted a second observation for the same source identity -- rejected by that same UNIQUE index. And allowlists _handle_cli_delete_authorize in the token-logging scanner. It returns a single-use authorization to the authenticated CLI caller that must present it back to /api/cli/delete; handing the token to its rightful owner is the endpoint's purpose, the same shape as the pairing-redemption entry above it. Verification: devtools test tests/unit/core/test_verification.py tests/unit/storage/test_raw_byte_duplicate_supersession.py tests/unit/storage/test_raw_authority_artifact_census.py -> 38 passed (5 previously failed). tests/unit/daemon/test_daemon_http_security.py -> 576 passed (1 previously failed). mypy --strict clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWcPJJJvuF25CqVwTFgSQC
1 participant
Summary
Restore source-authoritative reindex acceptance across retained-raw replay, logical source coverage, convergence backlog accounting, daemon maintenance probing, and campaign fixtures. This update also closes the exact-head review findings for durable cohorts, receipt source semantics, terminal frozen replay evidence, seeded artifact identity, and seed-testmon shard policy.
Problem
Source/index coverage recognized valid byte-supersession receipts without binding the receipt to its acquisition coordinate. Retained membership aliases could split a logical cohort across coverage, convergence, and sampling. Frozen replay still dispatched typed terminal non-session evidence, while seeded artifacts omitted emitter behavior and generated seed-testmon shards escaped broad-run workspace policy.
Solution
The authority layer now owns one canonical, membership-aware cohort expression used by coverage, convergence freshness, and schema sampling. Byte-duplicate planning and receipt verification require matching origin, source path, and source index. Replay census rows carry durable terminal non-session evidence so historical and frozen paths account for those raws without parser dispatch. The lowering fingerprint includes
sources/emitter.py, and generated seed-testmon shard labels use the broad-run basetemp policy.Verification
devtools verify --quick: all 25 steps succeeded in 195.73s atedfcf9812.edfcf9812.Anti-vacuity: a receipt with identical bytes but a different source path remains an untyped source error. Provider-wire and public-origin aliases resolve to one retained-membership cohort in both coverage and sampling. Typed terminal corrupt evidence is censused without entering parser replay, an emitter mutation changes the seeded artifact key, and a generated seed-testmon shard receives broad-run policy.
Current exact head:
edfcf9812eac632d4721df63bdc59abd26fa18a8.