Skip to content

feat(admin): correct published match report stats - #269

Open
diese-tech wants to merge 3 commits into
mainfrom
claude/match-report-corrections
Open

feat(admin): correct published match report stats#269
diese-tech wants to merge 3 commits into
mainfrom
claude/match-report-corrections

Conversation

@diese-tech

Copy link
Copy Markdown
Owner

Closes the last gap from the match-report rework: a report that reached done was read-only, so a completed game's stats and screenshots could never be fixed.

What changed

New correction APIsrc/app/api/admin/match-reports/[id]/correct/route.ts calls the correct_match_report_result RPC (shipped in sal-database db-v1.21.1). It requires a non-empty playerId on every row and maps the RPC's error codes onto 409 so the client can distinguish a real conflict from a server fault:

Code Meaning
55000 report changed since it was loaded (stale revision)
23505 correction key already used
23514 roster/division validation failed
23503 unknown player reference

Correction mode in the UIMatchReportClient gains beginCorrection(), which mints a fresh crypto.randomUUID() correction key and unlocks the editors for a published report. Submitting sends expectedRevision, so two admins correcting the same report concurrently cannot silently overwrite each other; the second one is told the report moved.

Completed reports now render their real numberssrc/lib/match-report-published.ts rebuilds the display from player_match_stats rather than extracted_data. extracted_data holds the original AI extraction and never receives admin corrections, so a corrected report used to display its pre-correction values.

Contract bumpdb-contract.lock.json moves db-v1.20.0db-v1.21.1 and vendors the matching generated types, which is what makes correct_match_report_result typecheck.

Why the pin is 4bf9f34e and not the contract-bump merge commit

contract.json and generated/database.types.ts are byte-identical at 9990110a (the bump) and at 4bf9f34e (current main) — the two commits in between changed seed data and tests only. sal-database's release workflow requires the released commit to be main's head, so 4bf9f34e is the commit that can carry the db-v1.21.1 tag.

Verification

  • npx tsc --noEmit — clean; the TS2345 on correct_match_report_result is resolved
  • npm run lint — 0 errors (10 warnings, all pre-existing on main)
  • npx vitest run — 788 passed, including 7 new tests for the correction route
  • Vendored types hash to sha256:4f4564…, matching contract.json's typesSha256

Known blocker — CI will be red until a tag is published

npm run check:db-contract fails with exactly one error:

Error: Unable to fetch generated/database.types.ts at db-v1.21.1: HTTP 404

The verifier resolves the lockfile at both the pinned commit and the release tag, and requires the artifacts to be identical. The commit resolves fine; db-v1.21.1 has not been published yet. In this repo family, contract release tags are created by sal-database's deploy.yml (gh release create) after a production deploy — that workflow requires commit_sha to equal main's head and a restore_evidence_sha256 attested independently from the private restore-evidence bundle, so publishing it is a human step by design.

Once db-v1.21.1 exists at 4bf9f34e, this check passes with no further code changes.

Follow-up, deliberately not in this PR

The approval path (resolve_match_report_review_unpublished) still has the cross-division roster gap that the correction path now closes. It deserves its own reviewed change rather than being folded in here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Bs5rcUV4wybANpywoyaMAB


Generated by Claude Code

diese-tech and others added 2 commits September 1, 2026 14:27
Approval is terminal by design, so a completed report was read-only and a
misread scoreboard could not be repaired from the admin screen.

- POST /api/admin/match-reports/[id]/correct calls the new
  correct_match_report_result RPC. It requires every player to be linked,
  carries the revision the admin loaded and a per-attempt correction key,
  and passes `applied` through so a replayed key is reported as "already
  recorded" rather than a fresh save. A superseded revision or a reused
  key maps onto 409.
- The published view gains a deliberate correction mode: it asks for a
  reason, turns the read-only editors editable against the real roster,
  and re-reads the report on cancel so abandoned edits do not linger.

Does not typecheck until db-contract.lock.json is bumped to the release
carrying the RPC; the tag does not exist yet, so that is the next commit.
Bumps db-contract.lock.json from db-v1.20.0 to db-v1.21.1 and vendors the
matching generated types, which add correct_match_report_result. This is
what the admin correction route calls, so the route now typechecks.

The pinned commit is 4bf9f34e (current sal-database main) rather than the
merge commit of the contract bump itself: contract.json and
generated/database.types.ts are byte-identical at both commits, and the
release workflow requires the released commit to be main's head.

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

vercel Bot commented Sep 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
sal-draft-league Ready Ready Preview Sep 4, 2026 1:45am UTC

@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: da704851c6

ℹ️ 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 src/components/admin/MatchReportClient.tsx Outdated
Comment thread src/components/admin/MatchReportClient.tsx

Copy link
Copy Markdown
Owner Author

CI blocker: db-v1.21.1 is not published yet

lint-and-typecheck fails on its first step, "Verify pinned database contract":

Error: Unable to fetch generated/database.types.ts at db-v1.21.1: HTTP 404
    at fetchText (scripts/db-contract.mjs:51:21)

scripts/db-contract.mjs resolves the lockfile at both the pinned commit and the release tag and requires the artifacts to match. The commit (4bf9f34e) resolves; the tag does not exist. Because the check is the first step, lint and typecheck never execute in CI, and build, integration-tests and e2e-tests are gated on that job, so all three show skipped.

This is not a flake, and re-running will not help

The failure is a deterministic 404 on a ref that does not exist. Re-running produces the identical result, so I have not spent a re-run on it.

What passes

unit-tests ✅ and secret-scan ✅ are green on da70485. Locally on this same commit:

  • npx tsc --noEmit — clean (the TS2345 on correct_match_report_result is resolved by the vendored types)
  • npm run lint — 0 errors, 10 warnings, all pre-existing on main
  • npx vitest run — 788 passed, including 7 new tests for the correction route
  • sha256sum src/types/database.types.ts4f4564…, matching contract.json's typesSha256

What unblocks it

No code change. db-v1.21.1 needs to be published in diese-tech/sal-database at 4bf9f34e0e69651acbfeec159e15f1cdde567010.

That commit — not the contract-bump merge — is the right target for two reasons: contract.json and generated/database.types.ts are byte-identical at 9990110a and 4bf9f34e (the two intervening merges changed seed data and tests only), and sal-database's deploy.yml requires the released commit_sha to equal main's head. Release tags in that repo are created by deploy.yml via gh release create after a production deploy; that workflow also takes a restore_evidence_sha256 attested independently from the private restore-evidence bundle, which makes publishing a deliberate human step.

I'm keeping this PR watched and will confirm CI and merge once the tag lands.


Generated by Claude Code

The server page loader and GET /api/admin/match-reports each kept their own
copy of the match_reports row mapping, and the two had drifted:

- the loader dropped `revision`, so `expectedRevision` fell back to 1 on a
  fresh page load and any report past its first revision — host-reviewed or
  already corrected — failed the RPC's stale-revision check and could never
  be corrected;
- the GET route dropped `publishedGames`, so refreshing after a correction
  emptied the editor for completed reports until a full browser reload;
- the loader also dropped `hostSubmittedAt`.

Both now go through a single mapMatchReportRow, so the shapes cannot drift
apart again. The client no longer substitutes a revision it does not have:
an absent revision means the list is stale, and it says so instead of
sending a guess.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bs5rcUV4wybANpywoyaMAB
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