feat(integrity): alert on a completed round that has no scored hole - #1725
Merged
njrini99-code merged 14 commits intoSep 2, 2026
Conversation
Production carries four rounds with status='completed', holes_played=18 and a
total_score, backed by zero golf_holes rows with a score. Nothing watched for
that, so nobody would have known.
The forensics say seeded fixture, not lifecycle failure, and the ids are what
settle it: 0b000000-0000-4000-b000-00000000000{1,2,3,4} — sequential, patterned
literals. Three share created_at to the microsecond (19:01:27.511173+00), every
updated_at equals its created_at, none carries a course_id, and current_hole=1
sits against holes_played=18. No application path emits a uuid like that; they
were inserted directly with the service role, on demo team 6ecdd1a6.
The check ships anyway. save_partial_round_atomic is a REPLACE — it deletes a
round's holes and shots and rebuilds them from the client payload — so a
malformed snapshot arriving after a durable write reaches the same state by a
route that is not seeding. "Completed with nothing scored" is also a state a
player can be shown and analytics will count.
R3, prepared not applied: checks 1-5 are reproduced verbatim from the live
definition (md5 ae683fa1797204f933b261714d3dba84, length 3789, read 2026-09-01)
and the file says to re-check that fingerprint before applying. The new check's
query was validated read-only against production and returned exactly the four
ids above.
It will report fail with count=4 on the first run. That is the intended
reading — the alert is correct and the data is wrong; it goes green when the
fixtures are removed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CeMzi9j5PSzd8GprDapP85
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Updates to Preview Branch (fix/integrity-completed-round-zero-scored-holes) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
…ed-round-zero-scored-holes
njrini99-code
enabled auto-merge (squash)
September 1, 2026 14:01
Audit item 6. Dry-run + apply, following the convention already in scripts/
(backfill-archive-insufficient-sample-insights{,-dryrun}.sql). Owner-executed;
nothing here runs automatically.
The set is eight rows on demo team 6ecdd1a6, and it identifies itself: the
qualifier is literally named "QA — Round Type Verification 2026-08-31", the
five round ids are sequential literals, three share created_at to the
microsecond, every updated_at equals its created_at, and none carries a
course_id.
Blast radius measured against production 2026-09-01: every other team has zero
fixture-shaped rows — Guilford 184 rounds, Demo University Golf (Pat) 90,
Shenandoah 63, UNC Wilmington 50, Lynchburg 37, all clean. The contamination
never left the demo team.
Two properties worth stating, because a production DELETE is the wrong place
to be clever:
- Explicit id lists, never a LIKE pattern. A pattern that widens by accident
takes real rows with it.
- The script refuses itself. A DO block aborts the transaction if any round in
the set carries a scored hole or a shot, so if a genuine round ever holds one
of these ids it survives. A second DO block re-checks the invariant after the
deletes and rolls back if any completed round still reports zero scored
holes — a new one appearing between now and execution should stop the run,
not ride along with it.
The dry run's OUTPUT IS THE EXPORT: it prints every column of every row plus
both assertions. Save it before applying.
Verification after apply: integrity check 6 goes fail/4 -> pass/0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CeMzi9j5PSzd8GprDapP85
…ed-round-zero-scored-holes
The registry's keys must equal the set of open PRs; this row makes the PR IN FLIGHT rather than unclassified while it waits on review. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… baseline Only comment lines and indentation moved; no statement changed. The Review Gate's sql-lint ratchet counts every violation in supabase/migrations against .sqlfluff-baseline.json, so a new file must lint clean under --rules core to land. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner
Author
|
Migration review (db-migration-reviewer, 2026-09-02, run before anything here is merged or applied). Summary for the owner: Migration
Because the PR bundles that script, I am leaving it OPEN for the owner rather than merging it in the queue sweep. — helmv3-ce |
…assertion db-migration-reviewer (2026-09-02): the predecessors (20260701150000, 20260704130000) each carry REVOKE ALL FROM PUBLIC/anon/authenticated, GRANT EXECUTE TO service_role, and a DO block that fails the migration if anon or authenticated can EXECUTE. This file dropped all three. CREATE OR REPLACE preserves the ACL, so nothing was live-exposed; the assertion is the tripwire for the recreate-re-grants-anon class shipping.md §4 records. Lint-clean under the ratchet. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ed-round-zero-scored-holes # Conflicts: # supabase/migrations/HELD.md
This was referenced Sep 2, 2026
…he fixtures Owner instruction 2026-09-02: "Don't delete them." The five patterned QA rounds and their qualifier on the demo team stay in production, so the two owner-run removal scripts leave this PR unrun and #1759 (their fixes) is closed. The HELD.md row now records the consequence: check 6 as written reports fail/4 on those fixtures every run, so the migration is held until the check is amended (exclude the demo team or the fixture ids) or the owner accepts the daily fail. The migration file itself is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner decision 2026-09-02: the four patterned fixture rounds on Demo University Golf (6ecdd1a6-63fe-4beb-b094-00118f334163) are kept, not removed. As written, check 6 would report fail/4 on them at every 07:00 UTC run — an alert about data the owner has already ruled on. The check now excludes that one team, which is QA by construction; every other team stays in scope. Nothing else in the function changed; checks 1–5 and the ACL block are byte-identical to the previous head. Validated read-only against production with the exclusion in place: count=0, sample=[]. Expect pass on first run. Still R3, prepared not applied; the md5 fingerprint check before applying stands (HELD.md row updated to say so). Verified: sqlfluff ratchet, semgrep custom rules on the file, markdown ratchet — all exit 0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner
Author
|
Owner decisions recorded 2026-09-02 (helmv3-ce):
Still R3, prepared not applied. Apply order for the owner: re-check the live md5 fingerprint in the file header, then apply. The HELD.md row says the same. |
…demo team db-migration-reviewer (APPROVE WITH NOTES, 2026-09-02) on the previous amendment: Demo University Golf is not QA by construction — it carries ~100 real demo rounds and the shared demo coach, so excluding the team would blind check 6 to save_partial_round_atomic, the REPLACE path the check exists to catch. Narrowed to the four patterned fixture ids the owner chose to keep; a repeat ad-hoc fixture insert now pages, which is the correct reading. The HELD.md row states one current expectation instead of the stale one plus a correction, and its decided column follows the table's 'X, updated Y' convention. Validated read-only against production with the id exclusion in place: count=0, sample=[]. Checks 1–5 and the ACL block remain byte-identical. sqlfluff ratchet, semgrep custom rules and the markdown ratchet exit 0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner
Author
|
Correction to the note above (helmv3-ce, after db-migration-reviewer): check 6 excludes the four fixture round ids, not the demo team — Demo University Golf holds ~100 real demo rounds and the shared demo coach, so a team-wide exclusion would have blinded the check to the REPLACE save path it exists to catch. Validated read-only against production with the id exclusion: |
…ed-round-zero-scored-holes
…ed-round-zero-scored-holes
…ed-round-zero-scored-holes
njrini99-code
deleted the
fix/integrity-completed-round-zero-scored-holes
branch
September 2, 2026 18:30
njrini99-code
added a commit
that referenced
this pull request
Sep 2, 2026
20260901120000_integrity_completed_round_zero_scored_holes.sql was applied
to production today by the owner's explicit instruction, after the three
recorded in the previous commit. Same route: the repo-local CLI
(`./node_modules/.bin/supabase db query --linked -f <file>`), one
transaction, the header's md5 precondition checked inside it, the
`supabase_migrations.schema_migrations` row inserted
(`created_by` = "owner-authorized direct apply 2026-09-02 (Claude Fable 5.1
session)"), and a post-check.
Precondition held: live md5 ae683fa1797204f933b261714d3dba84, len 3789,
matched the fingerprint the file's header records. Verified after, against
the live catalog: run_integrity_checks md5 f57c6f68206b56f131240d768e00876e,
len 4315; the body carries check 6 and excludes the four QA fixture ids
0b000000-0000-4000-b000-00000000000{1,2,3,4}; anon and authenticated cannot
EXECUTE, service_role can; ledger row
20260901120000:integrity_completed_round_zero_scored_holes present. A
read-only run of check 6's query with the exclusion immediately before the
apply flagged 0 rows, so the next 07:00 UTC run is expected to pass.
Row 38 converted the way rows 34-37 were: status becomes "APPLIED — hold
discharged (catalog-verified 2026-09-02)", a new verification cell is
inserted ending "Historical reasoning follows.", the original reasoning
(main's #1725 text — the branch was cut from that merge commit, so
`git merge origin/main` was already up to date) is kept, and the header
fingerprint is marked STALE with the live value the way rows 35 and 37 do.
No other row touched.
Verified locally, exit codes captured: markdown-lint-ratchet 0 (no
regressions), document-inventory --check 0 (inventory unchanged),
docs:path-drift 0. This commit touches no database.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
njrini99-code
added a commit
that referenced
this pull request
Sep 2, 2026
#1764) * docs(migrations): HELD.md records the three holds discharged 2026-09-02 Three held migrations were applied to production today by the owner's explicit instruction, and this file is the only place that records a deliberate non-application — so it must also record the discharge, or the rows read "still held" for as long as anyone looks (exactly what rows 34-35 did for two days before 2026-09-01). Applied, each through the repo-local CLI (`./node_modules/.bin/supabase db query --linked -f <file>`), each in its own transaction with its precondition checked inside the transaction, the `supabase_migrations.schema_migrations` row inserted (`created_by` = "owner-authorized direct apply 2026-09-02 (Claude Fable 5.1 session)"), and a post-check: - 20260901140000_trace_cannot_claim_success_while_blind.sql — the header's md5 precondition held (5bfaba551f001460e12e6477c663d18e, len 1074). Verified after: helm_debug_finalize_trace now carries status_downgraded_from (md5 338d5f344491586a6ab416ed0798548a, len 2021); anon and authenticated cannot EXECUTE, service_role can; ledger row present. The row now marks the header fingerprint STALE with the live value, the way row 35 does. - 20260819050000_drop_duplicate_baseball_decision_log_index.sql — precondition held (surviving sibling index present, table had 0 rows). Verified after: baseball_decision_log_meeting_item_idx gone, sibling present; ledger row present. - 20260819051000_baseball_fk_covering_indexes_wave_k2.sql — verified after: all three covering indexes present (tables held 9, 2 and 2 rows); ledger row present. Rows converted the way rows 34-35 were: status becomes "APPLIED — hold discharged (catalog-verified 2026-09-02)", a new verification cell is inserted ending "Historical reasoning follows.", and the original reasoning is kept. No other row touched. Verified locally, exit codes captured: markdown-lint-ratchet 0 (no regressions), document-inventory --check 0 (inventory unchanged), docs:path-drift 0. The production verification itself was read-only against the live catalog after the apply; this commit does not touch a database. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(migrations): HELD.md records the fourth hold discharged 2026-09-02 20260901120000_integrity_completed_round_zero_scored_holes.sql was applied to production today by the owner's explicit instruction, after the three recorded in the previous commit. Same route: the repo-local CLI (`./node_modules/.bin/supabase db query --linked -f <file>`), one transaction, the header's md5 precondition checked inside it, the `supabase_migrations.schema_migrations` row inserted (`created_by` = "owner-authorized direct apply 2026-09-02 (Claude Fable 5.1 session)"), and a post-check. Precondition held: live md5 ae683fa1797204f933b261714d3dba84, len 3789, matched the fingerprint the file's header records. Verified after, against the live catalog: run_integrity_checks md5 f57c6f68206b56f131240d768e00876e, len 4315; the body carries check 6 and excludes the four QA fixture ids 0b000000-0000-4000-b000-00000000000{1,2,3,4}; anon and authenticated cannot EXECUTE, service_role can; ledger row 20260901120000:integrity_completed_round_zero_scored_holes present. A read-only run of check 6's query with the exclusion immediately before the apply flagged 0 rows, so the next 07:00 UTC run is expected to pass. Row 38 converted the way rows 34-37 were: status becomes "APPLIED — hold discharged (catalog-verified 2026-09-02)", a new verification cell is inserted ending "Historical reasoning follows.", the original reasoning (main's #1725 text — the branch was cut from that merge commit, so `git merge origin/main` was already up to date) is kept, and the header fingerprint is marked STALE with the live value the way rows 35 and 37 do. No other row touched. Verified locally, exit codes captured: markdown-lint-ratchet 0 (no regressions), document-inventory --check 0 (inventory unchanged), docs:path-drift 0. This commit touches no database. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
njrini99-code
added a commit
that referenced
this pull request
Sep 5, 2026
…e has ended `open-pr-residue` requires the key set to equal the open-PR set, and a row for an in-flight PR lives at that PR's own head rather than on main — so #1836's row rides its own branch. Verified both directions: without it the check names #1836 as unclassified; with it, it does not. Also deletes #1725 and #1738, both MERGED, both past the transitional grace this file's own `$comment` defines — the tool's output says in as many words to delete them "in the PR you are already opening", and that is this one. Leaving them is the exact failure the comment describes: a current-state registry asserting things that stopped being true. **`open-pr-residue` still FAILS, for reasons this PR does not own.** Twelve open PRs carry no row at all — #1834, #1833, #1832, #1831, #1829, #1827, #1759 and five dependabot PRs. That was already true before this commit and is not mine to reconcile; a row is a statement about someone else's work. Recording it here so the red is legible rather than mysterious. Note for merge order: #1835 also edits this file, so whichever lands second resolves that conflict. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019euu9jtJM6WvMVj17coqZ1
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.
Audit item 3. Adds a sixth check to
run_integrity_checks()and answers the forensic question the audit asked.Who wrote the four empty rounds
Not an old deploy, not schema drift, not an import — a direct service-role seed. The ids settle it:
0b000000-0000-4000-b000-00000000000{1,2,3,4}created_at=19:01:27.511173+00updated_at= itscreated_atcourse_id;current_hole=1vsholes_played=186ecdd1a6So items 3 and 6 are one artifact. This PR does not delete them — that is item 6's reviewed production-data plan.
Why the check ships anyway
save_partial_round_atomicis a REPLACE: it deletes a round's holes and shots and rebuilds them from the client payload. A malformed snapshot arriving after a durable write reaches this same state by a route that has nothing to do with seeding. And "completed with nothing scored" is a state a player can be shown and analytics will count.Status — R3, prepared not applied
Checks 1–5 are reproduced verbatim from the live definition (md5
ae683fa1797204f933b261714d3dba84, len 3789, read 2026-09-01). The file instructs re-checking that fingerprint before applying, becauseCREATE OR REPLACEsilently discards anything that moved. Recorded inHELD.md.The new check's query was validated read-only against production and returned exactly those four ids.
It will report
failwith count=4 on first run. That is intended: the alert is right, the data is wrong. It goes green when item 6 removes the fixtures.🤖 Generated with Claude Code
https://claude.ai/code/session_01CeMzi9j5PSzd8GprDapP85