Skip to content

fix(snapshots): stop the two generated snapshots conflicting on every PR - #2530

Merged
BigSimmo merged 14 commits into
mainfrom
claude/snapshot-conflicts-3w455k
Sep 2, 2026
Merged

fix(snapshots): stop the two generated snapshots conflicting on every PR#2530
BigSimmo merged 14 commits into
mainfrom
claude/snapshot-conflicts-3w455k

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • #EFETZT — the repo-awareness snapshot no longer conflicts. The gate already excluded review_state and captured_revision from comparison, but excluding a key from a gate never stopped it conflicting in git: the un-compared bytes still shipped, still changed on both sides of every append, and still set mergeable_state=dirty — which suppresses refs/pull/<n>/merge, so pull_request CI does not run and the check list reads empty rather than red. Those keys now carry only content that can merge. review_state.records is ordered by head (a uniformly distributed sha, 2,323 distinct values across 2,662 records) instead of date-descending over only 53 distinct dates, so two branches appending a review record insert hundreds of lines apart; review_state.counts is gone, because an aggregate over an append-only set changes on both sides of every append and no ordering can disperse that; and REVISION_INPUTS excludes the review corpus, so a ledger:append no longer moves captured_revision. Reading order and totals move to the page, which already paginated. Snapshot version bumped to repo-awareness-snapshot-v2. Separately, data/repo-awareness-snapshot.json is added to ci-change-scope's perfExclusionPatterns — the carve-out its sibling already had — because regenerating it was flipping perf_changed and forcing a ~7-minute Lighthouse run against a budget the change cannot move.
  • #Y090R5 — closed at both ends. The core conflict was already fixed by the pending exclusion before this branch started. Three residues were live. (1) The generator wrote pending into the committed file (measured: counts.pending: 8 against a file committed with 0), so it now emits an empty pending by default and fills it only under --with-pending. (2) prebuild passes that flag and runs on any local npm run build, so a routine verify:pr-local still left the tree dirty with pending: 11 — a postbuild step now restores the committed shape once next build has inlined the file, which is safe because the runtime image never copies data/ at all. (3) The gate now requires the committed pending to be empty rather than merely ignoring its value. That third part has already paid for itself: merging main into this branch on 2026-09-02 brought in a committed snapshot carrying pending: 1, and the new guard failed on it — someone had committed an inbox request into the shared generated file, which is precisely the hazard this row records. The clinicalRisk: true misclassification this row named is also fixed: pr-policy.mjs exempts the two repo-metadata snapshots from the blanket data/ rule by exact path, pinned by self-tests including one proving a neighbour under data/ keeps its risk.
  • #JFRCZ4 — established with the git archive export the row asked for, and the probe found a real defect. Outside any repository the gate logs a skip and exits 0, which was already correct. But an export extracted inside another checkout answered git rev-parse --is-inside-work-tree with the outer repository's true; the generator then read that repository, found all 566 documents untracked, and threw an error the skip path could not recognise — exit 1 with a six-hundred-path dump and no explanation, exactly the unexplained failure this row predicted. isGitRepository now requires a repository rooted at the checkout, pinned by a nested-repository fixture test. The isGitRepoImpl() === false branch also had no direct coverage and now has a test asserting generation is never reached. Behaviour is documented in docs/codebase-index.md, including that a skip and a pass share exit code 0 so the message must be read.
  • #XHADPV — verified already resolved, and the deliberate choice written down. The two per-section counts no longer ship: the type declares sections: { name: string }[], the generator emits only { name }, and the committed file's section entries each carry one field. What the row actually asked for was that the choice be made deliberately rather than by omission, so ReviewStateSection now states the rule for the whole snapshot — a count over an append-only set is derived at render, a count over a closed set stays generator-computed — which is also why review_state.counts was removed above.

Fourteen commits: one per item, three more the verification loop turned up, and three merges absorbing main. SIX immutable inbox requests ride this branch — three issues:done closures (#EFETZT, #Y090R5, #JFRCZ4) and three issues:add findings recorded but deliberately not fixed here (below). No closure is queued for #XHADPV: another session had already queued one (8ba1e31f), the reconciler correctly refused two pending mutations on one issue, and since mine had never been committed it was dropped rather than committed alongside a cancellation. main has since reconciled and applied that request. Run npm run issues:reconcile after this lands; a dry-run confirms all pending requests still apply cleanly against the reconciled ledger.

Verification

  • npm run verify:pr-local — the routed handoff gate for this scope, re-run on every head including each merge: 29 gates completed, failed: (none), not reached: (none), including lint, typecheck, the full offline unit suite, a production build, check:gate-manifest, check:ledger-write-discipline, check:pr-policy, check:ci-scope and check:rag:fixtures (offline fixture validation only)
  • npm run check:repo-awareness-snapshotin step with data/repo-awareness-snapshot.json (204 pages, 576 documents, 2663 reviews) on the final merge
  • npm run check:outstanding-issues[snapshot] in step with data/outstanding-issues-snapshot.json (70 open, 0 pending) with 7 requests queued on disk, which is the fix working
  • git archive export probe against the committed tree, in both placements — outside any repository, and nested inside another one. Both now log Skipped: no git repository rooted here and exit 0; the nested case previously exited 1 with a six-hundred-path dump.
  • A real npm run build end to end, to prove the postbuild restore rather than reason about the npm lifecycle: the working tree afterwards shows no change to the snapshot's pending and its counts.pending reads 0. Confirmed again by later gate runs, whose own build step left it clean.
  • After GitHub's "Update branch" auto-merged the wholesale-reordered snapshot, regenerating produced a byte-identical file apart from captured_revision — proving the auto-merge did not corrupt review_state (2,663 records, head-sorted, all six fields, no escaped pipes, no counts key).
  • npm run format (whole tree), committed
  • UI verification not run: the only rendered change is the review-state panel's record ordering and its two count tiles, derived from data it already displayed — no markup, className, design-token, routing or styling change. It renders under /mockups/development/review-state, which 404s for non-admins in production, and tests/developer-review-state-page.dom.test.tsx covers exactly that ordering and those counts, including a new assertion that the page applies reading order rather than inheriting storage order. verify:phone-chrome is not applicable: no composer, dock, header or scroll-hide surface is touched.
  • Not run, and deliberately: no provider-backed gate (OpenAI, Supabase, eval:*, verify:release) — nothing here touches a provider. check:knip and check:maintainability-budgets are also deliberate skips: they sit in the verify:cheap chain rather than this routed one, the latter inspects four hardcoded files none of which are touched, and typecheck already covers the unresolved-import class knip would catch here.

Risk and rollout

  • Risk: low. The behaviour changes are confined to a generated developer-hub artefact and the administrator-only /mockups/development/review-state panel, which renders the same records in the same reading order as before. package.json gains a postbuild and keeps prebuild's effect for the built image by passing --with-pending explicitly. The one classifier change narrows clinicalRisk for exactly two named paths and is pinned by self-tests proving a neighbour under data/ keeps its risk.
  • Rollback: revert the merges and the seven commits. Both snapshots are generated, so npm run docs:update restores whatever shape the reverted code emits; no data is lost, because both files are derived entirely from committed sources.

Recorded but deliberately not fixed here

  • Shallow clone (P2, queued). A web-container session starts at depth 102, and two committed specs then fail with messages that read as governance-manifest corruption rather than truncated history — reviewedCommit does not exist 883f1007… and reconciledBase is unavailable locally: f3d1a3cc…. Both commits are real; git fetch --deepen=2000 takes the clone to 5462 commits and both specs pass unchanged. AGENTS.md already prescribes that remedy for check:dead-code-candidate, so the fix is known — it is simply not applied at session start.
  • Developer-hub perf classification (P3, queued). The carve-out above covers the snapshot data file but not the hub's own code: the route wrapper sits under the excluded src/app/mockups prefix while its panel components live under src/components/developer-area/hub/, which matches the generic src perf pattern. Left alone on purpose — widening a fail-closed exclusion by directory prefix could exempt a component that really is reachable from a budgeted route, so the request records the safer shape instead.
  • Dev-server pending panel (P3, queued). Raised by Codex review and verified: no lifecycle regenerates the snapshot for npm run dev, so the hub's "requests not yet applied" panel reads empty in local development. Not a regression — measured on main at 45a3dcacb, the committed snapshot already carried counts.pending: 0 with 8 requests tracked in the same commit. The obvious fix (a predev generating the pending-inclusive form) would write another branch's queued requests into the tracked tree on every dev start, reintroducing exactly the churn this PR removes, so the request records the two sounder shapes instead. See the review thread for the full reasoning.

Failures found on the way, reported rather than smoothed over

The handoff gate found something on every run against new content. Recorded because the environmental ones will bite the next session here.

  • typecheck TS2769 — mine, fixed. stdio: [...] as const produced a readonly tuple execFileSync rejects; the sibling call sites pass options inline and get the type from context, a standalone object does not. Now annotated ExecFileSyncOptionsWithStringEncoding.
  • A dirtied working tree after build — mine, fixed. Described under #Y090R5 above; found by the gate on this branch, not by reasoning about it.
  • docs/scripts-index.md stale — mine, fixed. Adding a postbuild script changed the npm-script count, and the gate stopped at docs:check-inventory before lint, typecheck, test or build had run at all on those commits.
  • A pre-commit hook invalidating the snapshot it had just verified — mine, fixed. On the third merge the pre-commit hook regenerated docs/design-system-adoption, which is a docs/ input to the snapshot's documentation section, so the snapshot committed one line earlier was already stale and verify:pr-local failed at check:repo-awareness-snapshot. Regenerated and folded into the same merge commit. Worth knowing: a gate run before committing cannot vouch for what the hook writes during it.
  • Three merge conflicts — all resolved, none hand-merged. main advanced three times while this was open, and the PR went mergeable_state: dirty within 25 minutes of first opening — which suppressed CI, SAST and Secret Scan entirely, the exact failure this PR fixes. Each was confirmed a real conflict with git merge-tree --write-tree (clean tree + behind would have been mere staleness) and then resolved by merging main and re-running the generator, never by editing the generated bytes. The first two were on data/repo-awareness-snapshot.json; the third was on docs/scripts-index.md, where both sides had added a script so the file's own "283/284 script files, 285/286 npm scripts" header disagreed — regenerating produced 284 files and 287 scripts, correctly carrying both additions. The snapshot conflicts were caused by this change rather than prevented by it: reordering 2,662 records rewrites the whole file, so it collides with any concurrent edit. That is a one-time migration cost; the recurring append case is what is fixed.
  • PR required red on 610972f2 and again on c25d893a — neither a broken change. Both were the aggregate refusing to go green on cancelled jobs, which verified nothing: the first when the PR was marked ready for review mid-run, the second when GitHub's "Update branch" superseded the run. The check states this itself — CANCELLED with no failing job … Nothing here describes the diff, so this is not a broken change. Every job that actually ran passed in both cases, and a newer run on the newer head confirmed it.
  • PR policy red on 48144a9b — real, and a chicken-and-egg. The check returns early on draft PRs, so every earlier "pass" was that early return rather than an evaluation. Marking the PR ready turned on enforcement, which flagged the clinical-risk misclassification this PR fixes — and cannot fix for itself, because pull_request_target deliberately runs main's trusted copy of the classifier. Resolved by adding the Preflight below, verified ok: true against main's copy rather than the patched one.
  • clinical-hazard-controls and rag-plan-package-parity — environmental, stable across two runs. The shallow clone, above.
  • reconciliation-preflight — transient. commit.gpgsign is on with SSH signing through a signing server that returned 503, so git commit failed inside a throwaway test fixture, in setup before any test body ran. Re-run per the flake policy's sanctioned case: 8/8, and all 13 specs that build git fixtures pass 296/296.

Clinical Governance Preflight

Required because pr-policy classifies this PR clinical-risk — which is the misclassification this PR fixes. The check runs on pull_request_target against main's trusted copy of scripts/pr-policy.mjs, so the exemption added here cannot apply to its own PR. Recorded rather than waved through, because #Y090R5 warns that ticking these boxes reflexively on non-clinical changes is the failure the section exists to prevent: every item below is answered from what the diff actually touches, and the honest summary is that it touches no clinical surface at all. No changed file reads, writes, renders or ranks clinical content; the two data/ snapshots are repository metadata (route and document inventories, the review-record corpus, the outstanding-issues ledger).

  • Source-backed claims still require linked source verification before clinical use
    • Unchanged. No retrieval, ranking, answer-generation or citation code is touched; src/lib/rag/** and the clinical-search surfaces are untouched.
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
    • Unchanged. No ingestion, upload, download or document-access path is touched. The only rendered surface is the administrator-gated /mockups/development/review-state panel, which lists this repository's own review records.
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
    • Unchanged, and independently confirmed on this PR: the Supabase bot reported it ignored the PR because there are no changes in the supabase directory. No migration, schema or connection config is touched.
  • Service-role keys and private document access remain server-only
    • Unchanged. No auth, key handling or access-control code is touched, and Client bundle secret surface check passed in the production build run locally.
  • Demo/synthetic content remains clearly separated from real clinical sources
    • Unchanged. No demo corpus, fixture or clinical dataset is touched; data/medications-snapshot.json and its siblings are untouched, and a self-test added here pins that a neighbour under data/ keeps its clinical-risk classification.
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
    • Unchanged for clinical sources. The only "review status" affected is this repository's own branch-review ledger, which is engineering metadata, not clinical provenance.
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed
    • Checked, and no clinical decision-support behaviour changed, so classification is unaffected. The change alters a generated developer-hub artefact's ordering, one admin-only panel's presentation, two CI classification scripts and an npm lifecycle hook.

🤖 Generated with Claude Code

https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn

…very append

`check:repo-awareness-snapshot` already excluded `review_state` and
`captured_revision` from comparison, but excluding a key from a gate never
stopped it conflicting in git. The un-compared bytes still shipped, still
changed on both sides of every append, and still set `mergeable_state=dirty` —
which suppresses `refs/pull/<n>/merge`, so `pull_request` CI does not run at all
and the check list reads empty rather than red. That is `#EFETZT`, twice in
fifteen minutes on PR #2413.

The excluded keys now carry only content that can merge:

- `review_state.records` is ordered by `head` rather than date-descending. The
  corpus holds 2,662 records across only 53 distinct dates, so newest-first put
  every append into the same dense block and two branches appending a review
  record — the ordinary handoff — inserted on the same lines. A commit sha is
  uniformly distributed (2,323 distinct values here), so they now land hundreds
  of lines apart and git resolves both hunks. Reading order moves to the page,
  which already owned pagination.
- `review_state.counts` is gone. An aggregate over an append-only set changes on
  both sides of every concurrent append, so no ordering can disperse it.
  `reviewStateCounts()` derives it at render from the same list the page shows,
  keeping the guarantee the stored counts gave. `ReviewStateSection` states the
  rule this follows for the whole snapshot.
- `REVISION_INPUTS` excludes the review corpus and the rotated archives. A
  review record is a `docs/**/*.md` file, so the bare glob meant every handoff
  moved `captured_revision` too, and two lines that always differ are a conflict
  no dispersal can avoid. Omitting a true input can only understate freshness,
  which that constant's own docstring already sanctions.

The snapshot version moves to `repo-awareness-snapshot-v2` so a committed file
left at v1 fails `assertRepoAwarenessVersion` loudly rather than rendering a
page whose order and totals no longer mean what the reader is told.

Separately, `data/repo-awareness-snapshot.json` joins its sibling in
`ci-change-scope`'s `perfExclusionPatterns`. It was missed when that carve-out
was added and it is the more frequent file: every `ledger:append` regenerates
it, so a routine handoff was paying a ~7-minute Lighthouse run against a budget
the change cannot move — part of this row's measured per-occurrence cost.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
…d snapshot

`#Y090R5`'s core problem was already fixed — `check:outstanding-issues-snapshot`
excludes `pending` and `counts.pending`, so an inbox PR no longer has to
regenerate the snapshot to pass. Two residues it named were still live.

Regeneration still WROTE `pending` into the committed file. Measured on this
branch: `generate()` returned `counts.pending: 8` against a file committed with
`0`, so `npm run build` via `prebuild`, or `npm run docs:update`, dirtied the
tree — and whoever committed that result re-armed the exact conflict the
exclusion was meant to end, because the value depends on every other branch's
queued requests. The generator now emits an empty `pending` by default and fills
it only under `--with-pending`, which `prebuild` passes. The built image still
shows the developer hub the true unapplied list, because it regenerates during
`next build`; the committed artefact never carries another branch's state.
Twelve requests are queued on disk as this lands and the committed file still
reads `pending: 0`.

The misclassification the row measured on PR #2299 is also fixed.
`clinicalRiskPatterns`' blanket `data/` rule exists for clinical reference
datasets shipped straight to clinicians; it also swept in two generated
repository-metadata exports, so every ledger PR was blocked until its body
carried a complete `## Clinical Governance Preflight` for a file holding no
clinical data. Ticking those boxes reflexively on changes with no clinical
output is the habit that section exists to prevent, so the false positive
actively eroded the gate. Both files are now exempt by EXACT path — never a
prefix, which would silently exempt the next clinical dataset added beside them
— pinned by self-tests including one proving a neighbour under `data/` keeps its
risk and one proving a mixed PR is still clinical-risk.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
…checkout

`#JFRCZ4` asked what this gate does in a git-less checkout, and named a
`git archive` export as the proof to run. Running it found a defect rather than
confirming the skip.

Outside any repository the gate already behaved correctly: it logs and exits 0.
But extract an export INSIDE another checkout — a scratch directory, a nested
clone — and `git rev-parse --is-inside-work-tree` answers the OUTER
repository's `true`. `isGitRepository` believed it, the generator then ran
`git ls-files --others` against that repository, all 566 documents looked
untracked to it, and it threw "Untracked Markdown documents would be omitted".
That is not a git-availability message, so the skip path could not recognise it:
exit 1, a six-hundred-path dump, and no explanation of why — precisely the
unexplained failure this row predicted for a gate that sits in three chains.

`isGitRepository` now requires a repository ROOTED at the checkout, comparing
`git rev-parse --show-toplevel` to the cwd. That is the exact question the gate
needs rather than an approximation of it: every path it reads is relative to the
cwd, so a repository rooted elsewhere is describing different content by
definition. Paths are resolved through `realpath` and normalised before
comparison, because git answers in its own idiom — forward slashes and a drive
letter on Windows — and a raw string comparison would skip the gate on every
workstation checkout. It fails safe: a false negative skips a check CI still
runs against a real checkout; a false positive compares this repository against
another one's files.

Two coverage gaps close with it. The existing skip test drives the
message-sniffing catch by throwing from `generateImpl`, so the branch that
actually fires in an export had none; there is now a test asserting the gate
skips WITHOUT generating at all, because a git-less generator would otherwise
fall back to a filesystem scan and compare a plausible-looking wrong answer. A
second test builds a real nested repository and pins that a directory inside a
work tree but not at its root is rejected while the root itself is accepted.

`listDocumentPaths` also silences git's stderr now, matching its two sibling
call sites. Without it a git-less checkout took the filesystem fallback,
succeeded, and still printed `fatal: not a git repository` — a run that worked
reading as a failure.

`docs/codebase-index.md` records the behaviour, including that a skip and a pass
share exit code 0 so the message is what distinguishes them, and what the
snapshot deliberately does not commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
`#XHADPV` reported thirty dead numbers riding in the committed snapshot:
`documentation.sections[]` carried `documents` and `uncatalogued` for all
fifteen sections while the page computed `section.documents.length` at render.
Verified against the repository rather than assumed, the fields are already
gone — the type declares `sections: { name: string }[]`, the generator emits
only `{ name }`, and every one of the committed file's 21 section entries
carries exactly one field.

What the row actually asked for was still missing. It said to "pick one
deliberately", because rendering the live list is arguably safer than trusting a
stored number but leaves the plan's "counts are computed once by the generator"
rule with an undocumented exception. This records the decision and the rule that
governs it: a count over a set that grows by append is derived at render, a
count over a closed set stays generator-computed. `documentation.counts` is the
second case and stays; `review_state.counts` was the first, and that is why it
was removed rather than as a one-off.

Another session queued the closing ledger request for this row independently
(`8ba1e31f`), reaching the same conclusion from the committed artefact, so no
second request is added here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: df8bdb8f-bc5d-4d97-9caa-93c9045c6ee3


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@supabase

supabase Bot commented Sep 2, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

…requests

Found by running the handoff gate on my own branch: `verify:pr-local` runs
`build`, `build` runs `prebuild`, and `prebuild` passes `--with-pending` — so
the working tree came back dirty with `pending: 11` written into a file
committed with `[]`. The default-empty change earlier in this branch fixed
`npm run docs:update` and a bare `npm run snapshot:issues`, but not this path,
and the built image genuinely does need the true list, so `prebuild` must keep
its flag.

That leaves the original hazard intact where it actually bites: a dirtied
working tree a developer commits by accident, putting every other branch's
queued requests back into a shared file. Excluding `pending` from the value
comparison never prevented that — the bytes still ship, and their value still
depends on work happening elsewhere.

So the gate now REQUIRES the committed snapshot's `pending` to be empty, rather
than merely ignoring its value. The two rules are not in tension and the tests
pin both: a regenerated snapshot carrying the live inbox against a committed one
that is empty is still reported as no difference, which is what isolates a
feature branch; a COMMITTED snapshot carrying requests now fails with a message
naming `npm run build` as the likely cause, because that is what a reader needs
to know and not something they did deliberately.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
…rf gap

Two findings from verifying this branch, queued as immutable inbox requests
rather than left in a session that will not outlive the container.

P2 — a web-container session starts on a shallow clone (measured at depth 102),
and two committed specs then fail with messages that read as governance-manifest
corruption rather than as a truncated history: "reviewedCommit does not exist
883f100…" and "reconciledBase is unavailable locally: f3d1a3c…". Both commits
are real. `git fetch --deepen=2000` takes the clone to 5462 commits and both
specs pass with no code change. AGENTS.md already prescribes that remedy for
`check:dead-code-candidate`; it is simply not applied at session start, and
these two specs are not covered by that note. A full `npm run test` is about six
minutes, so a session can spend two of them before recognising the pattern.

P3 — the `perfExclusionPatterns` carve-out added in this branch covers the
snapshot data file, but not the developer hub's own code: the route wrapper
lives under the excluded `src/app/mockups` prefix while the panel components
live under `src/components/developer-area/hub/`, which matches the generic
`src` entry. So a PR touching that code still pulls a full Lighthouse run for a
route that 404s for non-admins. Left unfixed deliberately — widening a
fail-closed exclusion by directory prefix could exempt a component that really
is reachable from a budgeted route, so the request records the safer shape (an
explicit path list plus a test proving a non-hub file still trips
`perf_changed`) rather than the quick edit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
`prebuild` passes `--with-pending` so the built image can show the developer hub
the requests that have not been reconciled yet, and it must keep doing so. But
`prebuild` runs on any `npm run build`, including the one inside
`verify:pr-local`, so every local build left `data/outstanding-issues-snapshot.json`
dirty with another branch's queued requests — measured here as `pending: 11`
against a file committed as empty. A dirty tracked file after a routine gate is
how that content gets committed by accident, which is the whole of `#Y090R5`.

`postbuild` regenerates it with the default (empty) `pending`, restoring the
committed shape. This is safe for the image: `next build` has already inlined
the file into the bundle by then via the static import in `ledger-snapshot.ts`,
and the runtime image never copies `data/` at all — `Dockerfile` takes only
`.next`, `public`, `node_modules` and four named source files. So the deployed
page still shows the true pending list while the tree comes back clean.

Verified by running a real `npm run build` end to end rather than by reasoning
about the lifecycle: the working tree afterwards shows no change to the
snapshot, and its `counts.pending` reads 0.

Together with the gate guard in the previous commit this closes the hazard at
both ends — a build cannot silently leave conflicting content in the tree, and
if one somehow does, the gate refuses to let it land.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
…icts-3w455k

# Conflicts:
#	data/repo-awareness-snapshot.json
@BigSimmo
BigSimmo marked this pull request as ready for review September 2, 2026 05:58
@BigSimmo
BigSimmo enabled auto-merge (squash) September 2, 2026 05:58
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-02T06:06:05.167853Z 48144a9 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cursor

cursor Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_614ac4bb-f593-4cce-a213-e40bc689d730)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48144a9b1e

ℹ️ 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".

Comment thread scripts/generate-outstanding-issues-snapshot.mjs
Codex flagged (P2) that `npm run dev` and `npm run ensure` invoke no `prebuild`,
so the developer hub's "requests not yet applied" panel renders empty during
local development while inbox requests exist. Verified rather than accepted: the
finding is correct, and it is the `#338` class of failure the hub exists to
prevent — a panel silently under-reporting.

It is not a regression from this branch, which changes how it must be fixed.
Measured on `main` at 45a3dca, before this branch existed: the committed
snapshot already carried `counts.pending: 0` while 8 inbox requests were tracked
in the same commit. The panel was already blank in dev. This branch changed it
from accidentally blank to blank by design and added a gate that keeps it that
way, so a fix now has to be deliberate rather than incidental.

The obvious fix is the wrong one, which is why this is queued rather than
patched here. A `predev` generating the pending-inclusive form would write a
populated `pending` into the tracked working tree on every dev-server start —
exactly the churn `#Y090R5` records and this branch removes — and
`check:outstanding-issues-snapshot` now refuses a committed non-empty `pending`,
so the developer would be left holding a permanently dirty file the gate blocks.
The request records the two shapes worth weighing instead: a development-only
reader that merges the inbox from disk, or a gitignored sidecar artefact, along
with the reason the first one must be written down rather than quietly reversing
the documented decision not to read `docs/` live.

Not urgent: the panel is administrator-gated, 404s for non-admins, and is
correct in the deployed image because `prebuild` regenerates it there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
@BigSimmo
BigSimmo disabled auto-merge September 2, 2026 07:11
…icts-3w455k

# Conflicts:
#	data/repo-awareness-snapshot.json
@cursor

cursor Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_7c23ddc9-f403-4dd5-bb3a-f13a8aea4547)

@BigSimmo
BigSimmo enabled auto-merge (squash) September 2, 2026 07:32
…icts-3w455k

# Conflicts:
#	docs/scripts-index.md
@BigSimmo
BigSimmo merged commit c0ee4dd into main Sep 2, 2026
29 checks passed
@BigSimmo
BigSimmo deleted the claude/snapshot-conflicts-3w455k branch September 2, 2026 08:21
BigSimmo pushed a commit that referenced this pull request Sep 2, 2026
c0ee4dd ("fix(snapshots): stop the two generated snapshots conflicting on
every PR") is the fix for the churn this branch kept hitting: four separate
conflicts today, every one of them in a file the repo generates rather than in
real code. Took main's versions of all three generated files and regenerated
with its new tooling, so this branch now carries that fix and should stop
colliding on every base advance.

Re-verified that check:diff-integrity survived the package.json auto-merge and
is still in verify:cheap:internal, since a registration dropped by an automatic
merge is exactly the silent loss this branch exists to prevent.

Gates: docs:check-inventory, docs:check-scripts, docs:check-index,
docs:check-links, check:gate-manifest, check:verification-plan,
check:diff-integrity, check:repo-awareness-snapshot, check:outstanding-issues,
check:outstanding-issues-snapshot, check:pr-policy and check:ci-scope all pass;
typecheck clean; unit suite 949 files / 12,168 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0193brbqFWHbpVnLfgDBkVNP
BigSimmo pushed a commit that referenced this pull request Sep 2, 2026
main's #2530 ("stop the two generated snapshots conflicting on every PR")
rewrote both snapshot generators and their checkers, which moved code this
branch's policy document quotes by file and line. Conflicts were confined
to the two generated files; both were regenerated with the merged
generators rather than hand-resolved.

Three citations in docs/branch-review-archival-policy.md had gone stale
and are corrected:
- generate-repo-awareness-snapshot.ts:314 -> :323 (REVIEW_RECORDS_PATHSPEC)
- generate-repo-awareness-snapshot.ts:379-386 -> :388-395 (fs fallback)
- check-repo-awareness-snapshot.ts:21-28 -> :22-41 (the review_state exclusion)

The third mattered for more than its line numbers. #2530 inserted a
paragraph inside the block this document quotes, and the old elided quote
skipped it silently. That paragraph is the sharper lesson and is now
quoted and answered: excluding a key from comparison was not enough on its
own, because the un-compared content still conflicted on every append,
which sets mergeable_state=dirty and leaves the check list empty rather
than red. The document now states why the index avoids that trap for a
structural reason rather than by luck — nothing regenerates it on an
ordinary PR — and what would have to change first if that ever stopped
being true.

All 16 code citations in the document were re-checked against the merged
tree and resolve to real line ranges. The PR body's citation and its
verification evidence are refreshed to this tree rather than left
describing the original run.

Verified: 7 test files, 245 tests passed; typecheck 6025 files; policy
evaluator 0 blocking errors; ledger, snapshot, links, inventory and index
gates all green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AxPVZhAvzE2YupMiDz8sqF
BigSimmo pushed a commit that referenced this pull request Sep 2, 2026
Resolves the generated-file conflict in data/repo-awareness-snapshot.json
by regenerating it with npm run snapshot:repo-awareness against the
merged tree, rather than hand-resolving. Also brings in PR #2530, which
fixes the root cause of this repeated conflict (the two generated
snapshots colliding on every concurrent PR).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FDiC2BK8XcPbstaJf7So2x
BigSimmo pushed a commit that referenced this pull request Sep 2, 2026
Codex raised a P2 on the section this PR added, and it was right on the
substance and righter than it knew on the timing.

Its point: an inbox-only ledger PR writes one immutable request and
nothing else, which is what makes those PRs merge-safe against each
other. Only `issues:reconcile` edits the canonical ledger and regenerates
the snapshot, and it does that itself. My section implied every ledger PR
touches the snapshot, which would have had readers hand-creating the
shared-file conflict the inbox design exists to avoid -- and would have
dragged an otherwise non-clinical PR in front of the governance gate.

Checking it turned up more. PR #2530 landed on main while this branch was
open and did two things: it requires the committed snapshot's `pending`
list to be empty, and it added `nonClinicalGeneratedDataPaths` to
pr-policy, exempting both generated snapshots by exact path from the
`data/` clinical-risk rule. So the trap I documented no longer exists --
ledger PRs do not trip the Preflight at all now. Documenting it as
current would have been exactly the stale record this PR exists to
correct, one day old.

The merge from main proved it independently: regenerating the snapshot
with the new generator produced a file byte-identical to main's, so it
dropped out of the diff and this PR is no longer clinical-risk either.

Rewritten to say what is true: keep inbox-only PRs off the snapshot,
`pending` must be empty, the gate no longer fires here and a reflexive
Preflight erodes it (#2530's own argument), and the verbatim-matching
trap still applies to PRs that genuinely are clinical-risk.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NiskRgeXeNRiU3npGP4M9f
BigSimmo pushed a commit that referenced this pull request Sep 2, 2026
…Y090R5 sibling)

The generated counts sentence at the top of docs/scripts-index.md conflicts on
every concurrent PR — measured five times on PR #2531 in about three hours, at
five different main heads, each time as the sole conflicting line.

Same root cause as #Y090R5 for data/outstanding-issues-snapshot.json: a
single-line generated artefact every PR must regenerate. PR #2530 fixed the
snapshot half by moving regeneration into the serialised reconcile step; this
half is untouched.

Records the two traps found while resolving it: taking main's whole file
silently drops the branch's own new script entry, and a bundled
verify:cheap:internal union merge desynchronises the gate counts that
check:gate-manifest reads.

Append-only inbox request; no canonical ledger edit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL
BigSimmo pushed a commit that referenced this pull request Sep 2, 2026
…comes a second row

Two sessions independently hit the same defect and each filed an `add`
request for it, and both are still pending:

- `b11bdfdd` (P2) "Web-container sessions start on a shallow clone, so two
  committed specs fail with messages that read as content errors"
- `4478f605` (P3) "Two tests fail on a shallow clone as ordinary assertion
  failures rather than refusing…"

They are the same defect, not merely adjacent: both name
`tests/clinical-hazard-controls.test.ts` and
`tests/rag-plan-package-parity.test.ts`, and both name commits `883f1007`
and `f3d1a3cc` as the truncated-history cause.

`add` requests carry no `baseRowFingerprint`, so nothing makes them
conflict. Left alone, the next reconciliation files two ledger rows for one
defect — and there is no row yet, so neither would look like a duplicate of
anything already recorded. This cancels the P3 and keeps the P2, which
carries the fuller reproduction and the more accurate severity.

Queued as an immutable cancel request rather than by deleting the P3, per
the clause `check:ledger-write-discipline` cites when it rejects a partial
batch: "use an immutable cancel request for each rejected mutation". The
request is additive and reconciles nothing, so it does not trip the
concurrent-reconciliation guard (`#EH9VA6`) while `claude/issues-reconcile-2526`
is still unmerged.

Raised by Codex review on PR #2530's successor #2560 and verified there.
That PR was closed as a duplicate of #2559; the finding was not specific to
it, so it is carried here rather than lost with the closure.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
BigSimmo added a commit that referenced this pull request Sep 4, 2026
…r review (#2544)

* issues: queue seven verified follow-ups from PRs #2538, #2536 and #2531

Seven immutable inbox requests, no canonical ledger edit. Every claim was
checked against the code before it was written: three against origin/main,
four against the PR heads the follow-up belongs to (#2538 ef7c55a,
#2536 e24e0ee, #2531 60f5e8e), since none of the three has merged yet.

- P2 issue: differential-records.ts asserts validation_status
  "locally_reviewed" from a literal over a snapshot that says
  "Pending review" — the sibling of the fix already in medication-records.ts.
- P2 issue: registry-records.ts and differential-records.ts both return the
  frozen source_status column verbatim, and derive it with a substring test
  that also matches "not checked"/"unchecked".
- P3 issue: a stored source_status of "outdated" can never be cleared, since
  nothing writes that column back on any of the three record tables.
- P3 rec: patient-alert rows with action "info" reach neither unassessed
  tier, leaving a green all-clear for nystatin, levetiracetam and lorazepam.
- P3 issue: isProfileEmpty treats a recorded hepatic "none" as no
  information, disagreeing with the engine, which treats it as an answer.
- P3 task: the considerations panel's two not-assessed sentences format
  their input lists differently.
- P3 rec: the forms PDF manifest records only passwordProtected, though the
  committed bytes also say modification, text extraction and assembly are
  blocked while printing and form-filling are permitted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL

* issues: queue the recurring docs/scripts-index.md counts collision (#Y090R5 sibling)

The generated counts sentence at the top of docs/scripts-index.md conflicts on
every concurrent PR — measured five times on PR #2531 in about three hours, at
five different main heads, each time as the sole conflicting line.

Same root cause as #Y090R5 for data/outstanding-issues-snapshot.json: a
single-line generated artefact every PR must regenerate. PR #2530 fixed the
snapshot half by moving regeneration into the serialised reconcile step; this
half is untouched.

Records the two traps found while resolving it: taking main's whole file
silently drops the branch's own new script entry, and a bundled
verify:cheap:internal union merge desynchronises the gate counts that
check:gate-manifest reads.

Append-only inbox request; no canonical ledger edit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL

* issues: cancel the OCR-routing claim and replace it with a corrected record

Codex review finding on this PR, verified against the code it cited rather than
taken on trust. The finding is correct.

Request e3133c1b asserted that disabled text extraction is "exactly the
condition that sends the worker down the OCR fallback path". It is not.
should_ocr_page() in worker/python/extract_pdf_assets.py (line 186) decides on
extracted text length and image coverage ratio only, and never reads the /P
permission bits. The extractor has no needs_pass or authenticate handling at
all, so a user-password PDF fails before any OCR decision is reached.

A wrong causal mechanism in a durable record is worse than no record: the next
person plans ingestion work from it. Cancelled rather than edited, because
inbox requests are immutable by design and the cancel action exists for exactly
this — the audit trail keeps the wrong claim, its refutation, and the
correction.

The permission-bits finding itself stands unchanged and is restated in the
replacement request, which makes no ingestion claim and says plainly that what
happens to these files on ingestion is untested.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL

* issues: record the reproducible mobile /documents/search LCP breach

Two confirmed measurements on different heads of PR #2536 — +466ms (+20.4%)
and +471ms (+20.6%) against a +20%/+100ms tolerance, five milliseconds apart.
That is reproducible, not noise, and the passing re-run between them was the
outlier.

Ruled out as PR #2536's doing: its diff is six medication files, and
documents/search/page.tsx imports one symbol (Metadata from next). Nothing
outside medication-named files calls /api/medications.

Recorded as a hypothesis, not a finding: lighthouse-budget.json was last
refreshed 2026-08-27 and main has taken heavy change since, so the PR carrying
the newest main absorbs the blame. Same disease as #QSHHGK for the bundle
budget. Main itself was never measured, and the record says so.

Needs an owner decision — find the regression, or refresh the baseline on a
schedule rather than reactively to clear a red PR.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL

* issues: cancel the Lighthouse regression claim and replace it with the measured variance

A third graded run refutes the "reproducible regression" reading in request
d1b3491f. CI run 33684986161 (head 957a038) measured mobile /documents/search
at 2323ms against the 2282ms baseline — +41ms — and reported "Every graded
route is within tolerance of the committed baseline."

The cell has now produced two breaches near 2750ms and at least two passes near
2320ms on the same pinned Chromium and the same route. That is a bimodal
measurement whose two modes straddle the +20%/+100ms tolerance, not a page that
became half a second slower. The gate's 2-of-3 sampling already tolerates noise
within a run; this split is between runs, which that design does not cover.

Cancelled rather than edited, because inbox requests are immutable and the
cancel action exists for this. The replacement states all four outcomes and asks
for the variance to be characterised — repeat the dispatch-only baseline-refresh
job against main and compare the spread — rather than for a regression hunt or a
reactive baseline refresh.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL

* issues: record the measured encryption state of the committed WA MHA form PDFs

A concurrent review comment on this PR asserted that PyMuPDF does not enforce
/P permission bits and that these files therefore "open normally and yield
their text layer". Measured against the committed bytes with PyMuPDF 1.28.0,
the library the worker actually uses, that is false for 50 of the 51 files.

  needs_pass=1, is_encrypted=True, page_count=0
  doc.authenticate("") -> 0        (the empty user password is rejected)
  load_page(0) -> ValueError('document closed or encrypted')

form-12a.pdf is the sole exception: no /Encrypt, opens, 3274 characters of
first-page text — which is exactly the file PR #2531 corrected.

The distinction the comment missed is that /P alone would not block opening;
these files also carry a non-empty /U, and that does. The practical
consequence is that extraction fails at the open call, before
should_ocr_page() is ever reached, so the OCR fallback cannot rescue them.
The JavaScript fallback was not measured and is not claimed either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL

* issues: record the intermittent caring-contacts-guidance strict-mode violation

Observed once on PR #2536 (CI run 33690849576 attempt 1, Production UI shard 2)
and cleared by a single re-run of the same commit:

  strict mode violation: getByTestId("caring-contacts-guidance")
  resolved to 2 elements

The source renders that test id in exactly one place, and both the page and the
shell interpolate it once, so the duplicate is not a second render site. The
likely window is React relocating out-of-order streamed content from the page's
next/dynamic shell, which the test's waitUntil:"load" does not wait past — but
that is a hypothesis and the record says so; the retained trace should confirm
it before anyone edits the test.

Not PR #2536's: its diff is medication and documentation files only, and the
same shard passed on its previous head.

The record asks for the three unscoped locators to be scoped to the main
landmark, and explicitly rules out quarantining — one reproduction is below the
repository's three-on-the-same-SHA bar, and a locator fix is not a suppression.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL

* docs(ledger): record Run PR sweep review for PR #2544

Merged origin/main into claude/issues-followups (clean, no conflicts)
and verified the narrow gates for this append-only inbox PR; both
review threads were already resolved from a prior pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0137etKe6X2p9HkXsXoYY8TA

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants