Context
backfillContributorGateHistory (src/review/contributor-gate-history-backfill.ts:43-99) reconstructs missing contributor_gate_history rows for gate decisions that predate migration 0126. Each candidate row is written with:
await env.DB.prepare(
`INSERT INTO contributor_gate_history (id, login, source, project, target_id, decision, head_sha, created_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
ON CONFLICT(id) DO NOTHING`,
)
.bind(...)
.run();
inserted += 1;
(src/review/contributor-gate-history-backfill.ts:84-92)
inserted is incremented unconditionally after every .run() call that doesn't throw — including when ON CONFLICT(id) DO NOTHING silently no-ops because a row with that deterministic id (contrib:${login}:${source}:${targetId}@${headSha}) already exists. The function's own doc comment says this can happen: the file's header explicitly describes the backfill as safe to call concurrently ("a queue retry, a manual re-trigger"), and two concurrent batches can both SELECT the same not-yet-inserted candidate (the NOT EXISTS guard in the read query only excludes rows already committed at read time) before either has inserted it. In that race, the second writer's INSERT ... ON CONFLICT DO NOTHING performs zero rows, but inserted is still incremented as if a new row was written.
The exact same "insert with ON CONFLICT DO NOTHING, count only real writes" pattern is already handled correctly elsewhere in this codebase — src/ams/ingest.ts:75-89 and src/orb/ingest.ts both check result.meta.changes > 0 before incrementing their own accepted counter after an identical D1 INSERT ... ON CONFLICT pattern. contributor-gate-history-backfill.ts is the one writer that skips this check.
ContributorGateHistoryBackfillResult.inserted is documented as "New contributor_gate_history rows written" (line 24) — under the race above, that claim is false; the returned count can overcount actual new rows.
This table is internal/private-only (never rendered on any public surface, never exported — see the file's own header and contributor-calibration.ts's identical design note), so this is purely an internal-analytics accuracy bug, not a security or gate-disposition issue.
Requirements
backfillContributorGateHistory must only increment inserted when the INSERT actually wrote a new row, using the same result.meta.changes > 0 check already used at src/ams/ingest.ts:89 and src/orb/ingest.ts:179 for the identical INSERT ... ON CONFLICT shape.
- The D1
.run() result must be captured and its meta.changes field inspected instead of assuming success == a new row.
- Behavior must otherwise be unchanged:
scanned, skippedNoAuthor, and hasMore semantics stay exactly as documented today.
Deliverables
Test Coverage Requirements
src/review/** is under the top-level 99% patch coverage gate — the corrected branch and its regression test must both be covered.
Expected Outcome
backfillContributorGateHistory's inserted count accurately reflects only rows genuinely newly written to contributor_gate_history, matching its own documented contract and the identical counting pattern already used correctly by src/ams/ingest.ts and src/orb/ingest.ts.
Links & Resources
src/review/contributor-gate-history-backfill.ts:84-92 (the bug)
src/ams/ingest.ts:75-89 (the correct pattern to mirror — if (result.meta.changes > 0) accepted++;)
src/orb/ingest.ts:179 (the same pattern, second precedent)
src/review/contributor-calibration.ts:45-68 (the live write path this backfill reconstructs history for)
Context
backfillContributorGateHistory(src/review/contributor-gate-history-backfill.ts:43-99) reconstructs missingcontributor_gate_historyrows for gate decisions that predate migration 0126. Each candidate row is written with:(
src/review/contributor-gate-history-backfill.ts:84-92)insertedis incremented unconditionally after every.run()call that doesn't throw — including whenON CONFLICT(id) DO NOTHINGsilently no-ops because a row with that deterministic id (contrib:${login}:${source}:${targetId}@${headSha}) already exists. The function's own doc comment says this can happen: the file's header explicitly describes the backfill as safe to call concurrently ("a queue retry, a manual re-trigger"), and two concurrent batches can bothSELECTthe same not-yet-inserted candidate (theNOT EXISTSguard in the read query only excludes rows already committed at read time) before either has inserted it. In that race, the second writer'sINSERT ... ON CONFLICT DO NOTHINGperforms zero rows, butinsertedis still incremented as if a new row was written.The exact same "insert with
ON CONFLICT DO NOTHING, count only real writes" pattern is already handled correctly elsewhere in this codebase —src/ams/ingest.ts:75-89andsrc/orb/ingest.tsboth checkresult.meta.changes > 0before incrementing their ownacceptedcounter after an identical D1INSERT ... ON CONFLICTpattern.contributor-gate-history-backfill.tsis the one writer that skips this check.ContributorGateHistoryBackfillResult.insertedis documented as "Newcontributor_gate_historyrows written" (line 24) — under the race above, that claim is false; the returned count can overcount actual new rows.This table is internal/private-only (never rendered on any public surface, never exported — see the file's own header and
contributor-calibration.ts's identical design note), so this is purely an internal-analytics accuracy bug, not a security or gate-disposition issue.Requirements
backfillContributorGateHistorymust only incrementinsertedwhen theINSERTactually wrote a new row, using the sameresult.meta.changes > 0check already used atsrc/ams/ingest.ts:89andsrc/orb/ingest.ts:179for the identicalINSERT ... ON CONFLICTshape..run()result must be captured and itsmeta.changesfield inspected instead of assuming success == a new row.scanned,skippedNoAuthor, andhasMoresemantics stay exactly as documented today.Deliverables
src/review/contributor-gate-history-backfill.ts'sinsertedcounter only increments on a genuinemeta.changes > 0write, mirroringsrc/ams/ingest.ts:89's pattern.contributor_gate_history) assertinginserteddoes NOT count that row, whilescannedstill reflects it was examined.Test Coverage Requirements
src/review/**is under the top-level 99% patch coverage gate — the corrected branch and its regression test must both be covered.Expected Outcome
backfillContributorGateHistory'sinsertedcount accurately reflects only rows genuinely newly written tocontributor_gate_history, matching its own documented contract and the identical counting pattern already used correctly bysrc/ams/ingest.tsandsrc/orb/ingest.ts.Links & Resources
src/review/contributor-gate-history-backfill.ts:84-92(the bug)src/ams/ingest.ts:75-89(the correct pattern to mirror —if (result.meta.changes > 0) accepted++;)src/orb/ingest.ts:179(the same pattern, second precedent)src/review/contributor-calibration.ts:45-68(the live write path this backfill reconstructs history for)