From 981b26010a42e1b5aab9a3f344ff67eb68a29916 Mon Sep 17 00:00:00 2001 From: Kev Date: Thu, 3 Sep 2026 16:07:39 -0400 Subject: [PATCH] The first real cohort found the leak: one artifact was calibrating every stat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shadow converged at 18:09:07Z and the 19:00Z slot produced cohort 0353c551. It immediately falsified something no test had asked: 513 PUBLISHED NON-HITS rows came back CERTIFIED_CALIBRATED with a served probability drawn from the mlb-hits curve — total_bases 246, runs 149, rbi 125, walks 109, outs 28, strikeouts 24, hits_allowed 20, earned_runs 17. Two bugs, one on top of the other. `mergeProbabilityContract` passed only {model_version, p_win}, dropping the row's identity; and the service's resolve then stamped the {sport, stat} it had been BUILT with onto every read. So all 3,000 rows in the batch resolved as mlb hits. The governance tests could not see it. They asked "does build() refuse another stat?" — it does, and always did — and then exercised the merge with hits-only rows. Production sends one mixed batch. The regression test now drives the REAL collector with hits, total_bases, rbi, runs, walks, strikeouts and home_runs at the same p_win and requires hits certified and every other stat neither certified nor numeric. Fixed in three layers, because one would have been the same single point that just failed: 1. the service no longer substitutes its own identity — the row's decides, and a read naming no stat resolves to no contract, which is UNSUPPORTED; 2. the merge carries the row's sport and stat; 3. probabilityContract refuses an artifact whose own sport/stat disagree with the contract it is being used under, independent of plumbing. NO USER IMPACT. Shadow only: every block carries servable:false, live serving is OFF, CALIBRATION_DEPLOYED is [], and the anonymous payload showed zero calibration fields before and after. But this is exactly the defect that would have served a hits calibration curve for strikeouts on the day live was enabled, and only a real cohort surfaced it. Two teeth were themselves wrong. Both runners checked "retention identity changes" by grepping the diff for `stat:`, which fired on `stat: r.stat` — a line that READS identity to hand it to a reader, not one that changes what identifies a row. A guard that cannot tell those apart blocks the fix for the defect it exists to protect against. Both are now behavioural: build a row through the real collector and compare the identity tuple. Artifact unchanged: mlb-hits-isotonic@2026-09-03, knot 5ae940ea163b7da2. Suite 405/405, 5,659 passed. Teeth 34/34 + 10/10 + 23/23. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CQJeAG8vcDoL5zkiaJyVb8 --- scripts/teeth-artifact-governance.js | 51 ++++++++++++- scripts/teeth-probability-contract.js | 22 +++++- src/services/model/probabilityContract.js | 9 +++ .../model/probabilityContractService.js | 11 ++- src/services/retentionService.js | 7 +- tests/unit/artifactGovernance.test.js | 76 +++++++++++++++++++ 6 files changed, 166 insertions(+), 10 deletions(-) diff --git a/scripts/teeth-artifact-governance.js b/scripts/teeth-artifact-governance.js index 2bad416..5a539a3 100644 --- a/scripts/teeth-artifact-governance.js +++ b/scripts/teeth-artifact-governance.js @@ -187,10 +187,31 @@ logic(22, 'live serving enabled', () => { // 23-26 — the frozen neighbours logic(23, 'retention identity changes', () => { - const d = execSync(`git -C ${ROOT} diff --unified=0 -- src/services/retentionService.js`).toString(); - const fields = ['player_key:', 'snapshot_id:', 'canonical_event_id:', 'game_id:', 'stat:', 'line:', 'side:']; - const touched = fields.filter((f) => new RegExp(`^[-+].*${f.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}`, 'm').test(d)); - return { caught: touched.length === 0, detail: `identity fields in diff: ${touched.join(', ') || 'none'}` }; + // BEHAVIOURAL, not a diff grep. The grep version fired on `stat: r.stat` — + // a line that READS identity to pass it to a reader, not one that changes + // what identifies a row. A guard that cannot tell those apart blocks the fix + // for the defect it was meant to protect against. + const retention = require(path.join(ROOT, 'src/services/retentionService')); + const c = retention.createCollector({ + snapshotId: 'snap-teeth', sport: 'mlb', modelVersion: 'engine1@2026-08-07-fullwindow', + codeSha: 'teeth', gameDate: '2026-09-03', gameIdFor: () => 'mlb:2026-09-03:AAA@BBB', + }); + c.onGraded({ player: 'Test Hitter', stat_type: 'hits', line: 0.5, sport: 'mlb', + over_odds: -110, under_odds: -110, canonical_event_id: 'mlb:gamepk:1' }, + [{ direction: 'over', grade: 'C+', p_win: 0.65, confidence: 65 }]); + const row = c.rows[0]; + const identity = { + snapshot_id: row.snapshot_id, game_id: row.game_id, + canonical_event_id: row.canonical_event_id, player_key: row.player_key, + stat: row.stat, line: row.line, side: row.side, + }; + const expected = { + snapshot_id: 'snap-teeth', game_id: 'mlb:2026-09-03:AAA@BBB', + canonical_event_id: 'mlb:gamepk:1', player_key: 'test hitter', + stat: 'hits', line: 0.5, side: 'over', + }; + const wrong = Object.keys(expected).filter((k) => identity[k] !== expected[k]); + return { caught: wrong.length === 0, detail: `identity tuple mismatches: ${wrong.join(', ') || 'none'}` }; }); logic(24, 'participant identity changes', () => { const f = execSync(`git -C ${ROOT} diff --name-only`).toString().trim().split('\n').filter(Boolean) @@ -249,6 +270,28 @@ logic(31, 'live serving turns on before all gates pass', () => { detail: `CALIBRATION_DEPLOYED empty=${deployedEmpty}; stage=${registry.PROMOTED['mlb:hits'].stage}; approved_for_live=${A.approved_for_live}` }; }); +// ── 32 — THE CROSS-STAT LEAK (found by the first real cohort) ──────────── +inject(32, 'the hits curve is applied to other stats in a mixed batch', + 'src/services/model/probabilityContractService.js', + ` resolve(read) { + return pc.resolve(read,`, + ` resolve(read) { + return pc.resolve({ ...read, sport, stat },`, + GOV); +inject(33, 'the merge drops the row identity the resolver needs', + 'src/services/retentionService.js', + ` res = contract.resolve({ + sport: r.sport, stat: r.stat, + model_version: r.model_version, p_win: numOrNull(r.p_win), + });`, + ` res = contract.resolve({ model_version: r.model_version, p_win: numOrNull(r.p_win) });`, + GOV); +inject(34, 'an artifact for another stat is accepted', + 'src/services/model/probabilityContract.js', + ` || (deps.artifact.stat && deps.artifact.stat !== contract.stat))) {`, + ` || false)) {`, + GOV); + const landed = results.filter((r) => r.landed).length; console.log(JSON.stringify({ teeth_landed: `${landed}/${results.length}`, results }, null, 2)); process.exit(landed === results.length ? 0 : 1); diff --git a/scripts/teeth-probability-contract.js b/scripts/teeth-probability-contract.js index 227d937..366c05e 100644 --- a/scripts/teeth-probability-contract.js +++ b/scripts/teeth-probability-contract.js @@ -270,10 +270,24 @@ logicTooth(15, 'publication changes unintentionally', () => { // ── 19,20,21,22,23 — the frozen neighbours ──────────────────────────────── logicTooth(19, 'retention identity changes', () => { - const d = execSync(`git -C ${ROOT} diff --unified=0 -- src/services/retentionService.js`).toString(); - const idFields = ['player_key:', 'snapshot_id:', 'canonical_event_id:', 'game_id:', 'stat:', 'line:', 'side:']; - const touched = idFields.filter((f) => new RegExp(`^[-+].*${f.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}`, 'm').test(d)); - return { caught: touched.length === 0, detail: `identity fields touched in the diff: ${touched.join(', ') || 'none'}` }; + // BEHAVIOURAL, not a diff grep. The grep fired on `stat: r.stat` — a line + // that READS identity to hand it to a reader, not one that changes what + // identifies a row. A guard that cannot tell those apart blocks the fix for + // the very defect it exists to protect. + const retention = require(path.join(ROOT, 'src/services/retentionService')); + const c = retention.createCollector({ + snapshotId: 'snap-teeth', sport: 'mlb', modelVersion: 'engine1@2026-08-07-fullwindow', + codeSha: 'teeth', gameDate: '2026-09-03', gameIdFor: () => 'mlb:2026-09-03:AAA@BBB', + }); + c.onGraded({ player: 'Test Hitter', stat_type: 'hits', line: 0.5, sport: 'mlb', + over_odds: -110, under_odds: -110, canonical_event_id: 'mlb:gamepk:1' }, + [{ direction: 'over', grade: 'C+', p_win: 0.65, confidence: 65 }]); + const r = c.rows[0]; + const expected = { snapshot_id: 'snap-teeth', game_id: 'mlb:2026-09-03:AAA@BBB', + canonical_event_id: 'mlb:gamepk:1', player_key: 'test hitter', + stat: 'hits', line: 0.5, side: 'over' }; + const wrong = Object.keys(expected).filter((k) => r[k] !== expected[k]); + return { caught: wrong.length === 0, detail: `identity tuple mismatches: ${wrong.join(', ') || 'none'}` }; }); logicTooth(20, 'participant identity changes', () => { diff --git a/src/services/model/probabilityContract.js b/src/services/model/probabilityContract.js index 10da3b5..9f71790 100644 --- a/src/services/model/probabilityContract.js +++ b/src/services/model/probabilityContract.js @@ -191,6 +191,15 @@ function resolve(read = {}, deps = {}) { if (deps.artifact && deps.artifact.estimator_type !== contract.estimator_type) { return { ...base, probability_state: STATE.VERSION_MISMATCH, reason: 'estimator artifact is not the certified estimator type' }; } + // AND THE ARTIFACT MUST BE FOR THIS SPORT AND STAT. Independent of how the + // read reached here: an mlb-hits curve is not a total_bases curve, and the + // only thing that stopped it being used as one was correct plumbing. + if (deps.artifact + && ((deps.artifact.sport && deps.artifact.sport !== contract.sport) + || (deps.artifact.stat && deps.artifact.stat !== contract.stat))) { + return { ...base, probability_state: STATE.VERSION_MISMATCH, + reason: `estimator artifact is for ${deps.artifact.sport}/${deps.artifact.stat}, not ${contract.sport}/${contract.stat}` }; + } if (raw === null || raw < 0 || raw > 1) { return { ...base, probability_state: STATE.INVALID, reason: 'raw probability absent or out of range' }; } diff --git a/src/services/model/probabilityContractService.js b/src/services/model/probabilityContractService.js index b0a9889..8da96e1 100644 --- a/src/services/model/probabilityContractService.js +++ b/src/services/model/probabilityContractService.js @@ -42,8 +42,17 @@ async function build(_sb, { sport = 'mlb', stat = 'hits', usage = 'shadow' } = { cutoff: artifact.fit_as_of, /** The frozen curve. Two calls return the same value, forever. */ estimate: (p) => registry.applyCurve(artifact, p), + /** + * THE ROW'S IDENTITY DECIDES — never the identity this resolver was built + * with. Stamping `{sport, stat}` here applied the mlb-hits curve to every + * row in the batch: measured on cohort 0353c551, 513 PUBLISHED non-hits + * rows (total_bases, runs, rbi, walks, outs, strikeouts, hits_allowed, + * earned_runs) came back CERTIFIED_CALIBRATED with a served probability + * from the hits artifact. A read that names no stat resolves to no + * contract, which is UNSUPPORTED — the correct answer, not a fallback. + */ resolve(read) { - return pc.resolve({ ...read, sport, stat }, + return pc.resolve(read, { estimate: (p) => registry.applyCurve(artifact, p), artifact, usage }); }, }; diff --git a/src/services/retentionService.js b/src/services/retentionService.js index c4c4848..00587b2 100644 --- a/src/services/retentionService.js +++ b/src/services/retentionService.js @@ -851,7 +851,12 @@ function mergeProbabilityContract(rows, contract) { return rows.map((r) => { let res; try { - res = contract.resolve({ model_version: r.model_version, p_win: numOrNull(r.p_win) }); + // The ROW's sport and stat travel with it. Omitting them let the + // resolver substitute its own and calibrate every stat as hits. + res = contract.resolve({ + sport: r.sport, stat: r.stat, + model_version: r.model_version, p_win: numOrNull(r.p_win), + }); } catch { return r; } if (!res) return r; let derived; diff --git a/tests/unit/artifactGovernance.test.js b/tests/unit/artifactGovernance.test.js index 40f75b6..02b370b 100644 --- a/tests/unit/artifactGovernance.test.js +++ b/tests/unit/artifactGovernance.test.js @@ -198,3 +198,79 @@ describe('the runtime can name its artifact with the shadow OFF', () => { .toBe(JSON.stringify(pc.shadowState({}).artifact)); }); }); + +describe('ONE ARTIFACT, ONE STAT — the cross-stat leak found by cohort 0353c551', () => { + const retention = require('../../src/services/retentionService'); + + /** The real collector, with a MIXED-STAT batch — the shape production sends. */ + function mixedBatch() { + const c = retention.createCollector({ + snapshotId: 's1', sport: 'mlb', modelVersion: ERA, codeSha: 't', + gameDate: '2026-09-03', gameIdFor: () => 'mlb:2026-09-03:AAA@BBB', + }); + for (const stat of ['hits', 'total_bases', 'rbi', 'runs', 'walks', 'strikeouts', 'home_runs']) { + c.onGraded( + { player: `P ${stat}`, stat_type: stat, line: 0.5, sport: 'mlb', over_odds: -110, under_odds: -110 }, + [{ direction: 'over', grade: 'C+', p_win: 0.65, confidence: 65 }], + ); + } + return c.rows; + } + + it('calibrates hits and NOTHING else, even in one mixed batch', async () => { + const contract = await svc.build(null, { sport: 'mlb', stat: 'hits' }); + const merged = retention.mergeProbabilityContract(mixedBatch(), contract); + + const byStat = {}; + for (const r of merged) { + byStat[r.stat] = byStat[r.stat] || { certified: 0, served: 0, other: 0 }; + const st = r.probability_contract.probability_state; + if (st === pc.STATE.CERTIFIED_CALIBRATED) byStat[r.stat].certified++; + else byStat[r.stat].other++; + if (r.probability_contract.served_probability != null) byStat[r.stat].served++; + } + + // hits: calibrated. p_win 0.65 sits inside certified support. + expect(byStat.hits.certified).toBeGreaterThan(0); + expect(byStat.hits.served).toBeGreaterThan(0); + + // EVERY other stat: no certified state and no number, at the SAME p_win. + for (const stat of ['total_bases', 'rbi', 'runs', 'walks', 'strikeouts', 'home_runs']) { + expect(byStat[stat].certified).toBe(0); + expect(byStat[stat].served).toBe(0); + } + }); + + it('a non-hits row resolves UNSUPPORTED — the contract does not exist for it', async () => { + const contract = await svc.build(null, { sport: 'mlb', stat: 'hits' }); + const [tb] = retention.mergeProbabilityContract( + mixedBatch().filter((r) => r.stat === 'total_bases'), contract); + expect(tb.probability_contract.probability_state).toBe(pc.STATE.UNSUPPORTED); + expect(tb.probability_contract.served_probability).toBeNull(); + expect(tb.probability_contract.derived.available).toBe(false); + }); + + it('the resolver does not substitute its own identity for the row\'s', async () => { + const contract = await svc.build(null, { sport: 'mlb', stat: 'hits' }); + // a read naming another stat must NOT be resolved as hits + expect(contract.resolve({ sport: 'mlb', stat: 'rbi', model_version: ERA, p_win: 0.65 }) + .probability_state).toBe(pc.STATE.UNSUPPORTED); + // and a read naming NO stat resolves to no contract, never a fallback + expect(contract.resolve({ model_version: ERA, p_win: 0.65 }) + .probability_state).toBe(pc.STATE.UNSUPPORTED); + }); + + it('an artifact for another stat is refused even with correct plumbing', () => { + const wrong = { ...good, stat: 'total_bases' }; + const r = pc.resolve(read(0.65), { estimate: est, artifact: wrong }); + expect(r.probability_state).toBe(pc.STATE.VERSION_MISMATCH); + expect(r.served_probability).toBeNull(); + expect(r.reason).toContain('total_bases'); + }); + + it('and an artifact for another sport is refused', () => { + const r = pc.resolve(read(0.65), { estimate: est, artifact: { ...good, sport: 'wnba' } }); + expect(r.probability_state).toBe(pc.STATE.VERSION_MISMATCH); + expect(r.served_probability).toBeNull(); + }); +});