fix(sync): give each pair the review date its own rationale records - #3250
Conversation
…3244) #3183 split `reason` into `divergence`/`divergence_reviewed`/`fingerprint_refreshed` so a mechanical hash bump could not move a review claim, then stamped one blanket date on all twenty pairs - re-creating the false assertion in a machine-readable field. Measured at HEAD e3212f0: divergence_reviewed was 2026-08-23 on 19 of 20, while fingerprint_refreshed had already moved on 12. Each pair's divergence_reviewed now comes from the date its OWN rationale records (13 changed), and the gate checks the property instead of non-emptiness: - divergence_reviewed must equal the review date stated in that pair's prose - no single value may cover half the pairs or more - the two dates may coincide only where the prose names that same date The parser reads a REVIEW VERB followed by a date ("re-reviewed 2026-06-20", "re-baselined 2026-06-30", "updated 2026-08-05"), never a bare date - most of these rationales open with a mechanical event whose own text says the divergence "was NOT re-reviewed". check_template_drift.py now prints each pair's review age beside it and a summary count of coupled bumps. Reported, never enforced: a stale review date must not block a fingerprint refresh, or the gate blocks its own drain. Also swaps two test names so #3183's cited node id test_manifest_issue_references_are_open now belongs to the strict offline check that does the work, rather than to the network-gated probe that only iterates `open:` citations - of which the manifest has zero. pair.11 states no review date and its Non-Goal forbids rewriting the prose, so it keeps its pre-bump date under an explicit cap of one such pair. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 36 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 71 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
ruff check passed but the repo also runs black (line-length 100), which reflows assert-with-message differently. No semantic change; the deliberate-break transcripts in the PR body still reproduce exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b5c6993f8
ℹ️ 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".
Automated Status SummaryHead SHA: c8459fd
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScope#3183 / PR #3203 split the drift allowlist's single #3183 §4 stated the problem: one field carried two different events — a mechanical fingerprint refresh Measured at The two fields are identical for all twenty pairs, and the coupling has already survived one day of The gate cannot see any of this. assert divergence and "Existing reviewed baseline drift" not in divergence
assert reviewed
assert refreshedA blanket Also worth folding in: #3183's named live gate Full trace: Context for AgentRelated Issues/PRsTasks
Acceptance criteria
|
|
Addressed all three active P2 findings in 6e1d516: equal review/refresh dates are reported neutrally rather than treated as suspicious, allowlist-template output now emits the required editable provenance fields, and integrity tests validate calendar dates. Validation: focused allowlist tests 6 passed; full tests/scripts 3,115 passed, 1 token-gated skip; Ruff, Black --fast check, template-drift report, template sync/completeness, and diff check passed. The PR remains in the required seven-minute post-push review window. |
Closer: merge deferred — absent-check gateBefore merge, the lane ran a manual absent-check comparison against merged reference PR #3247. These check names never reported on head
Why merge is paused: repo policy treats absent checks as distinct from green checks. This PR touches only Next safe action: re-dispatch / widen path filters so the missing names report (even as |
Provider Comparison ReportProvider Summary
📋 Full Provider Details (click to expand)openai
anthropic
Agreement
Disagreement
Unique Insights
🔍 LangSmith Traces |
Closes #3244
Automated Status Summary
Scope
#3183 / PR #3203 split the drift allowlist's single
reasonfield intodivergence,divergence_reviewedandfingerprint_refreshed— and then populated the two new fields with thesame blanket date on every pair, which re-creates the exact false assertion the split existed to
remove, now in a machine-readable field.
#3183 §4 stated the problem: one field carried two different events — a mechanical fingerprint refresh
when Renovate moves a pinned SHA, and a judgement that two files SHOULD differ — so "the file asserts
the second whenever the first happens". The remedy was to make a hash refresh unable to touch the
review claim.
Measured at
HEAD a377fd30overconfig/template-drift-allowlist.txt:The two fields are identical for all twenty pairs, and the coupling has already survived one day of
real traffic: the single pair whose fingerprint moved on 2026-08-24 had its
divergence_reviewedmoved with it. Nineteen pairs assert a 2026-08-23 review while their own
divergenceprose dates thejudgement elsewhere —
[pair.3]says "re-reviewed 2026-06-20",[pair.5]"updated 2026-08-05",[pair.10]"re-baselined 2026-06-30",[pair.15]"re-baselined 2026-07-14".The gate cannot see any of this.
tests/scripts/test_template_drift_allowlist.py:12asserts onlynon-emptiness:
A blanket
divergence_reviewed = <today>on every pair passes it, and so would a value that moves inlockstep with
fingerprint_refreshedforever. The field has no drain: nothing decrements it, nothingcan distinguish a real re-review from a copy of the refresh date, and the reader is told the whole
file was re-reviewed today whenever any one SHA moves.
Also worth folding in: #3183's named live gate
tests/scripts/test_sync_manifest_docs.py::test_manifest_issue_references_are_openis currentlyvacuous. It only iterates citations explicitly marked
open:, and.github/sync-manifest.ymlcontains five
resolved:citations and zeroopen:ones, so the node-id the acceptance criterionnames verifies nothing even with a token present. The strict check that does the work lives under a
different name,
test_manifest_issue_citations_are_explicitly_stateful.Full trace:
Code/Audits/Workflows/2026-08-24-implementation-verification.md.Context for Agent
Related Issues/PRs
Tasks
divergence_reviewedinconfig/template-drift-allowlist.txtto the date stated in that pair's owndivergencetext, leavingfingerprint_refreshedat the date the hashes last moved.divergence_reviewedage to the report emitted byscripts/check_template_drift.py, printed beside each allowlisted pair, so a stale review date is visible where the pair is read.scripts/check_template_drift.pycounting pairs whosedivergence_reviewedequals theirfingerprint_refreshed, which is the signature of a coupled bump.tests/scripts/test_template_drift_allowlist.pywith an assertion that fewer than half the pairs share an identicaldivergence_reviewedvalue, so a blanket stamp fails.test_divergence_reviewed_is_not_a_copy_of_fingerprint_refreshedtotests/scripts/test_template_drift_allowlist.pyasserting no pair has the two dates equal unless itsdivergencetext names that same date.tests/scripts/test_template_drift_allowlist.pythat extracts the date from eachdivergencestring and asserts it matches that pair'sdivergence_reviewed.test_manifest_issue_citations_are_explicitly_statefulintests/scripts/test_sync_manifest_docs.pytotest_manifest_issue_references_are_openand give the network-gated live check a name that says it only checksopen:citations.Acceptance criteria
python3 -m pytest tests/scripts/test_template_drift_allowlist.py::test_divergence_reviewed_is_not_a_copy_of_fingerprint_refreshed -qexits 0.python3 -m pytest tests/scripts/test_template_drift_allowlist.py::test_every_pair_states_its_divergence -qexits 0 and its assertions compare the extracted prose date againstdivergence_reviewedrather than testing for a non-empty string.divergence_reviewedinconfig/template-drift-allowlist.txtto today's date;python3 -m pytest tests/scripts/test_template_drift_allowlist.py -qmust FAIL naming both tests. Revert the file; both must pass. Quote both transcripts as raw pytest output in the PR body.fingerprint_refreshedand itsdivergence_reviewedtogether; the named gate must FAIL. Bump onlyfingerprint_refreshed; it must pass. Quote both transcripts as raw pytest output.python3 scripts/check_template_drift.pyexits 0 and reports 20 pairs with 0 unallowlisted, unchanged by the metadata edits.python3 scripts/check_template_drift.pyoutput states how many pairs havedivergence_reviewedequal tofingerprint_refreshed.python3 -m pytest tests/scripts -qexits 0 and the skipped count does not increase.Head SHA: 6e1d516
Latest Runs: ✅ success — Gate
Required: gate: ✅ success