Skip to content

Commit fc2b53b

Browse files
committed
fix(calibration): backfill upserts latest-decision-wins instead of silently dropping a changed terminal decision
ORB-review finding on the original INSERT OR IGNORE shape: with targetKey-only deterministic ids, a target whose terminal decision changed between two backfill runs had its second run's row silently dropped. ON CONFLICT(id) DO UPDATE on the decision-bearing columns makes cross-run semantics match the transform's own latest-terminal-wins dedupe; identical re-runs remain effective no-ops, and ids/provenance are unchanged.
1 parent b2e59d1 commit fc2b53b

2 files changed

Lines changed: 15 additions & 6 deletions

File tree

scripts/backfill-calibration-corpus-core.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,12 @@ export function sqlStringLiteral(value: string): string {
152152
}
153153

154154
/**
155-
* Render the synthesized rows as chunked `INSERT OR IGNORE` statements (idempotency comes from the
156-
* deterministic ids: a re-run, or an overlap with a prior partial apply, silently no-ops instead of
157-
* double-writing). Chunked so a statement never grows past what `wrangler d1 execute --command` sanely
155+
* Render the synthesized rows as chunked UPSERT statements. Idempotency comes from the deterministic
156+
* targetKey-derived ids; `ON CONFLICT(id) DO UPDATE` (rather than OR IGNORE) makes the semantics "latest
157+
* decision wins" ACROSS runs too, matching the transform's own latest-terminal-wins dedupe: a target whose
158+
* terminal decision changed between two backfill runs gets its pair UPDATED, never silently dropped
159+
* (ORB-review finding on the original OR IGNORE shape), while a re-run over identical data remains an
160+
* effective no-op. Chunked so a statement never grows past what `wrangler d1 execute --command` sanely
158161
* carries. Returns [] for an empty report.
159162
*/
160163
export function buildBackfillInsertStatements(rows: readonly SynthesizedAuditRow[], chunkSize = 50): string[] {
@@ -169,7 +172,10 @@ export function buildBackfillInsertStatements(rows: readonly SynthesizedAuditRow
169172
.join(", ")})`,
170173
)
171174
.join(", ");
172-
statements.push(`INSERT OR IGNORE INTO audit_events (id, event_type, actor, target_key, outcome, detail, metadata_json, created_at) VALUES ${values}`);
175+
statements.push(
176+
`INSERT INTO audit_events (id, event_type, actor, target_key, outcome, detail, metadata_json, created_at) VALUES ${values} ` +
177+
`ON CONFLICT(id) DO UPDATE SET detail = excluded.detail, metadata_json = excluded.metadata_json, created_at = excluded.created_at`,
178+
);
173179
}
174180
return statements;
175181
}

test/unit/backfill-calibration-corpus-core.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,13 @@ describe("synthesizeBackfillRows (#8157)", () => {
122122
describe("buildBackfillInsertStatements (#8157)", () => {
123123
const report = synthesizeBackfillRows([decisionRow(), decisionRow({ number: 8 }), decisionRow({ number: 9 })]);
124124

125-
it("renders INSERT OR IGNORE with the full audit_events column list and SQL-escaped values", () => {
125+
it("renders latest-decision-wins UPSERTs with the full audit_events column list and SQL-escaped values", () => {
126126
const escaped = synthesizeBackfillRows([decisionRow({ repo: "o'brien/repo" })]);
127127
const [statement] = buildBackfillInsertStatements(escaped.rows);
128-
expect(statement).toMatch(/^INSERT OR IGNORE INTO audit_events \(id, event_type, actor, target_key, outcome, detail, metadata_json, created_at\) VALUES /);
128+
expect(statement).toMatch(/^INSERT INTO audit_events \(id, event_type, actor, target_key, outcome, detail, metadata_json, created_at\) VALUES /);
129+
// ORB-review finding: a target whose terminal decision changed between runs must be UPDATED, never
130+
// silently dropped -- the conflict clause updates exactly the decision-bearing columns.
131+
expect(statement).toContain("ON CONFLICT(id) DO UPDATE SET detail = excluded.detail, metadata_json = excluded.metadata_json, created_at = excluded.created_at");
129132
expect(statement).toContain("o''brien/repo#7");
130133
expect(sqlStringLiteral("it's")).toBe("'it''s'");
131134
});

0 commit comments

Comments
 (0)