diff --git a/hooks-core/calibration.mjs b/hooks-core/calibration.mjs index 8b9cd0faf..b1ebacf1c 100644 --- a/hooks-core/calibration.mjs +++ b/hooks-core/calibration.mjs @@ -29,7 +29,20 @@ * that cannot support one. */ -import { record, readMetrics } from './metrics.mjs'; +// readBalance, not readMetrics. Both readers used the windowed firehose, which returns at most +// 2,000,000 bytes and the last 5,000 lines -- and forecast events were not among the protected +// kinds, so they lived in a log dominated by per-tool-call records (metrics.mjs's own comment +// cites 4,735 capture events inside one window). +// +// Two failures followed. In observeOutcome, on a busy project the forecast had scrolled past the +// tail by the time compaction fired, so the outcome was discarded with no record, no error and no +// counter. In reliability, outcomes vanished faster than they accumulated, so rows.length could +// never reach MIN_SCORED and calibrate returned 'not yet calibrated (n/8)' permanently -- leaving +// a user unable to distinguish "never had data" from "the data is being thrown away". +// +// 'forecast' and 'forecast-outcome' are now balance kinds, so they are written to balance.jsonl +// as well and read back unwindowed. +import { record, readBalance } from './metrics.mjs'; /** Horizon buckets, in turns. Error behaves differently across these ranges. */ const HORIZONS = [ @@ -51,8 +64,15 @@ function horizonOf(turns) { return HORIZONS.find((h) => turns <= h.max).name; } -/** Records a prediction so it can be scored when the outcome arrives. */ -export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) { +/** + * Records a prediction so it can be scored when the outcome arrives. + * + * `turns` is the session's turn count AT PREDICTION TIME, and it is what makes the outcome + * computable at all: predictedTurns means "turns from here until compaction", so the ground truth + * is the turns that ELAPSED, not the turn number compaction happened on. Without it the caller at + * compaction has nothing to subtract from and would have to score an interval against an absolute. + */ +export function logForecast(dir, { sessionId, predictedTurns, used, capacity, turns }) { if (!Number.isFinite(predictedTurns)) return; record(dir, { kind: 'forecast', @@ -61,6 +81,7 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) horizon: horizonOf(predictedTurns), used, capacity, + turns: Number.isFinite(turns) ? turns : null, }); } @@ -71,21 +92,74 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) * Called when compaction fires: the number of turns that elapsed since the * prediction is the ground truth it was predicting. */ -export function observeOutcome(dir, { sessionId, actualTurns }) { - if (!Number.isFinite(actualTurns)) return; - const open = readMetrics(dir) - .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && !e.scored) +export function observeOutcome(dir, { sessionId, actualTurns, atTurn }) { + // EITHER THE ELAPSED COUNT OR THE TURN IT HAPPENED ON. + // + // The only real caller -- the PreCompact hook -- knows what turn compaction fired on, not how + // many turns have passed since a prediction it did not make. Deriving the interval here keeps + // that subtraction next to the record that defines it, rather than making every caller + // re-discover which forecast is open in order to compute its own ground truth. + const elapsed = Number.isFinite(actualTurns) + ? actualTurns + : null; + if (elapsed === null && !Number.isFinite(atTurn)) return; + + // CLOSURE IS DERIVED FROM THE OUTCOME ROWS, not from a flag on the forecast. + // + // The old filter was `!e.scored`, but nothing ever writes `scored`: logForecast records + // kind/sessionId/predictedTurns/horizon/used/capacity, and record() is an append-only JSONL + // writer with no update path, so a forecast can never gain the field. The predicate was always + // true, and the last forecast for a session was therefore re-closable without limit. + // + // A session where compaction fires twice with no intervening logForecast popped the SAME + // forecast twice, producing two outcome rows for one prediction. reliability counts ROWS, not + // predictions, so the hit rate and the bias mean both inflate and MIN_SCORED can be satisfied + // by a single prediction re-closed eight times -- precisely the "threshold from a single + // sample" this module exists to prevent. The existing test used a fresh sessionId per + // iteration, so it never exposed it. + const events = readBalance(dir); + const closed = new Set( + events.filter((e) => e.kind === 'forecast-outcome' && e.forecastId).map((e) => e.forecastId), + ); + // ID-BEARING ROWS ONLY, AND NEWEST BY TIME RATHER THAN BY POSITION. + // + // Two separate hazards, both from readBalance's shape. It returns balance-log rows followed by + // the legacy firehose rows it migrates, unsorted -- so `.pop()` returns the last row in the + // array, which is not the newest forecast. And legacy rows predate `id`, so `closed.has(e.id)` + // is `closed.has(undefined)`, always false: such a forecast can be closed again and again, and + // each outcome stores `forecastId: undefined`, which JSON.stringify drops entirely. That is the + // repeat-scoring defect this function was written to fix, reappearing through the back door. + const open = events + .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && e.id && !closed.has(e.id)) + .sort((a, b) => (a.at ?? 0) - (b.at ?? 0)) .pop(); if (!open) return; + // Derived only once the open forecast is known, since the interval is measured from ITS turn. + // A forecast recorded before `turns` existed has none, and there is nothing to subtract from -- + // scoring it against an absolute turn number would manufacture an error rather than measure one, + // so it is left open instead. + const actual = elapsed !== null ? elapsed + : (Number.isFinite(open.turns) ? atTurn - open.turns : null); + // AT LEAST ONE TURN MUST HAVE PASSED. `atTurn` is the transcript's current turn count and + // `open.turns` the count at prediction time, so a truncated, rotated or replaced transcript + // makes the difference zero or negative. Recording that as ground truth feeds a large negative + // `error` into `reliability`, which shifts `bucket.bias`, which is what `calibrate` publishes a + // corrected forecast from -- so one bad transcript would skew every later prediction. Leaving + // the forecast open loses nothing: the next compaction closes it against a sane count. + if (actual === null || !Number.isFinite(actual) || actual < 1) return; + record(dir, { kind: 'forecast-outcome', sessionId, + // Stamped so the next call can see this forecast is spoken for. Without it the set above is + // empty and the same defect returns. + forecastId: open.id, horizon: open.horizon, predictedTurns: open.predictedTurns, - actualTurns, - error: actualTurns - open.predictedTurns, - hit: Math.abs(actualTurns - open.predictedTurns) <= TOLERANCE, + actualTurns: actual, + error: actual - open.predictedTurns, + hit: Math.abs(actual - open.predictedTurns) <= TOLERANCE, }); } @@ -96,7 +170,7 @@ export function observeOutcome(dir, { sessionId, actualTurns }) { * actionable, where "62% accurate" is not. */ export function reliability(dir) { - const outcomes = readMetrics(dir).filter((e) => e.kind === 'forecast-outcome'); + const outcomes = readBalance(dir).filter((e) => e.kind === 'forecast-outcome'); const buckets = {}; for (const horizon of HORIZONS) { @@ -162,7 +236,28 @@ export function calibrate(dir, predictedTurns) { // Refit: shift by the observed bias for this horizon. A bucket that runs long // gets pulled in, and vice versa. - const adjusted = Math.max(0, Math.round(predictedTurns + bucket.bias)); + // + // NOT CLAMPED WITH Math.max, which converted nonsense into a plausible-looking emergency. bias + // is a plain arithmetic mean of signed error, and a mean is not robust to the shape the + // 50%-hit-rate floor admits: a near bucket with errors [0,0,0,0,-30,-30,-30,-30] has four hits, + // so it clears the floor and is called calibrated, and its bias is -15. A raw forecast of 3 + // turns then became Math.max(0, Math.round(3 - 15)) = 0 and was published as fact, with the + // note "within 3 on 50% of 8 past forecasts" -- a zero-turn deadline the forecaster never + // produced, from a bucket that is wrong half the time. + // + // A correction at least as large as the thing it corrects is not a correction. + const adjusted = Math.round(predictedTurns + bucket.bias); + if (Math.abs(bucket.bias) >= predictedTurns || adjusted < 1) { + return { + publishable: false, + predictedTurns, + horizon, + bias: bucket.bias, + reason: `the observed bias at this horizon (${Math.round(bucket.bias)} turns) is as large as ` + + `the forecast itself (${predictedTurns}), so the correction would replace it rather than ` + + 'adjust it', + }; + } return { publishable: true, diff --git a/hooks-core/forecast.mjs b/hooks-core/forecast.mjs index 9cbea77d7..60461f4af 100644 --- a/hooks-core/forecast.mjs +++ b/hooks-core/forecast.mjs @@ -29,13 +29,30 @@ * other rather than one standing in for the other. */ -import { readMetrics } from './metrics.mjs'; +import { readMetrics, readBalance, isFixtureAnchor, BALANCE_KINDS } from './metrics.mjs'; import { aggregateConsolidation } from './consolidate.mjs'; import { previewQuality } from './expand.mjs'; +import { calibrate } from './calibration.mjs'; /** Turns of headroom below which the runway is worth interrupting for. */ export const ACTIONABLE_RUNWAY = 8; +/** + * Held-out touches required before the runway counterfactual is published. + * + * The header of this file promises the counterfactual appears "only once the holdout carries + * volume", and the fallback string says "the control arm needs more volume" -- but the only check + * was `if (!rows.length) return null`, so "volume" meant "at least one". With a single withheld + * touch, withheldCost IS that one touch's downstream read total: one held-out read of a + * 3,000-line module set savedPerTouch to several thousand tokens and published a counterfactual + * an order of magnitude off, as a bare fact with no caveat. + * + * Ten is a floor rather than a statistically derived threshold, and is deliberately stated as + * such where it surfaces: the panel prints the held-out count against it, so a reader can see how + * close the arm is instead of being told only that it is short. + */ +export const MIN_CONTROL_TOUCHES = 10; + /** * Tokens per turn, and how many the graph is removing. * @@ -55,13 +72,42 @@ export function burnRate(events) { reads.get(key).push(event); } + // EACH READ IS CHARGED TO EXACTLY ONE INJECTION. + // + // The windows used to run from each injection to the end of the log, so they overlapped + // completely: for anchor A with injections i1..i5 interleaved with five reads of 100 tokens, + // i1 saw 500, i2 400, i3 300, i4 200, i5 100 -- 1500 / 5 = 300 against a true per-touch cost of + // 100. A 3x inflation growing with the repeat count. + // + // It does not cancel between the arms. metrics.mjs makes the holdout deterministic in + // (anchor, epoch), so every repeat touch of a file lands in the SAME arm -- whichever arm drew + // the heavily re-touched anchors is inflated and the other is not. savedPerTouch's sign and + // magnitude were therefore driven by which files happened to be held out, and that value is + // what the published counterfactual divides by. + // + // Bounding each window at the next injection of the same (session, anchor) -- across BOTH arms, + // since an injection ends the previous window regardless of which arm it is in -- makes the + // charge exact. + const nextInjectAfter = new Map(); + for (const row of injects) { + const key = `${row.sessionId || ''}|${row.anchor}`; + if (!nextInjectAfter.has(key)) nextInjectAfter.set(key, []); + nextInjectAfter.get(key).push(row.at ?? 0); + } + for (const times of nextInjectAfter.values()) times.sort((a, b) => a - b); + const downstream = (rows) => { if (!rows.length) return null; let total = 0; for (const row of rows) { - const bucket = reads.get(`${row.sessionId || ''}|${row.anchor}`) || []; + const key = `${row.sessionId || ''}|${row.anchor}`; + const bucket = reads.get(key) || []; const after = row.at ?? 0; - total += bucket.reduce((sum, r) => sum + ((r.at ?? 0) >= after ? (r.tokens || 0) : 0), 0); + const until = (nextInjectAfter.get(key) || []).find((t) => t > after) ?? Infinity; + total += bucket.reduce( + (sum, r) => sum + ((r.at ?? 0) >= after && (r.at ?? 0) < until ? (r.tokens || 0) : 0), + 0, + ); } return total / rows.length; }; @@ -96,12 +142,27 @@ export function burnRate(events) { export function shadowAvoided(events) { let avoided = 0; let spent = 0; + let seen = 0; for (const event of events) { if (event.kind !== 'substitute') continue; + // FIXTURES EXCLUDED, as balanceSheet already does. metrics.mjs records the measurement: + // "366 of 370 substitutions pointed at the enforcement suite's own big.ts fixture under a + // temp dir, and counting them made the product look like it had avoided 40 MB of reads. It + // had avoided 154 KB." Running the test suite against a real graph inflated `avoided` by two + // orders of magnitude, which blew up shadow.net, pushed `spread` past 0.5 and fired the + // divergence note with a fabricated number -- so the panel's own credibility check was the + // thing the fixture noise corrupted. + if (isFixtureAnchor(event.anchor)) continue; + seen += 1; avoided += Math.ceil((event.bytesAvoided || 0) / 4); spent += event.tokens || 0; } - return avoided ? { avoided, spent, net: avoided - spent } : null; + // GUARDED ON WHETHER ANYTHING WAS SEEN, not on whether it was favourable. Guarding on `avoided` + // meant twenty substitutions that each spent 20 tokens and avoided nothing -- a measured net + // loss of 400 -- returned null, so the divergence block never ran and the panel said nothing + // about the shadow arm. The one outcome the function could not express was the one unfavourable + // to the product, collapsed into the same return value as "no data". + return seen ? { avoided, spent, net: avoided - spent, substitutions: seen } : null; } /** @@ -110,8 +171,10 @@ export function shadowAvoided(events) { * @param used Tokens of context consumed so far. * @param capacity Context window size. * @param turns Turns taken to consume it. + * @param touches Intercepted touches in this session. Required for the counterfactual: it is + * what converts a per-TOUCH saving into a per-TURN one. */ -export function runway({ used, capacity, turns }, rate) { +export function runway({ used, capacity, turns, touches }, rate) { if (!capacity || !turns || used == null) return null; const perTurn = used / turns; @@ -119,15 +182,53 @@ export function runway({ used, capacity, turns }, rate) { const remaining = Math.max(0, capacity - used); const withGraph = Math.floor(remaining / perTurn); + const base = { withGraph, perTurn: Math.round(perTurn) }; - // Without the graph each turn costs what it costs now PLUS what the graph is - // removing. Only computable when the control arm has spoken. const saved = rate?.savedPerTouch; - const withoutGraph = saved != null && saved > 0 - ? Math.floor(remaining / (perTurn + saved)) - : null; + if (saved == null) return { ...base, withoutGraph: null, counterfactual: 'unmeasured' }; + + // VOLUME, MEANING VOLUME. See MIN_CONTROL_TOUCHES. + const withheld = rate.withheldTouches ?? 0; + if (withheld < MIN_CONTROL_TOUCHES) { + return { ...base, withoutGraph: null, counterfactual: 'thin-control', withheld }; + } + + // UNITS. `perTurn` is tokens per TURN for this session; `savedPerTouch` is an average over + // injection EVENTS across the whole metrics log. Adding them directly was only correct with + // exactly one intercepted touch per turn, which is not the usual case in either direction. + // + // Session {used 60000, capacity 200000, turns 30} gives perTurn 2000 and remaining 140000. With + // 5 injections over those 30 turns and savedPerTouch 1500, the true per-turn saving is 250, so + // the honest counterfactual is floor(140000/2250) = 62. Adding the per-touch figure gave + // floor(140000/3500) = 40, and the panel published "~70 turns to compaction; without the graph, + // ~40" -- claiming the graph nearly doubled the runway when it added about eight turns. + // + // Without a touch count there is no density to convert with, so the counterfactual is withheld + // rather than computed on the one-touch-per-turn assumption. + if (!touches) return { ...base, withoutGraph: null, counterfactual: 'no-touch-density' }; + + const savedPerTurn = saved * (touches / turns); + + // A MEASURED NON-POSITIVE SAVING IS A RESULT, NOT A MISSING ONE. The old `saved > 0` guard + // discarded a measurement: with withheldCost 1000, treatedCost 900 and injectionCost 200, + // savedPerTouch is -100 -- the control arm has spoken and said the graph is a net cost -- and + // the panel printed "no counterfactual yet, the control arm needs more volume". The one result + // unfavourable to the product was the one result the panel could not show, disguised as + // insufficient data. The runway is longer without the graph in that case, and says so. + const denominator = perTurn + savedPerTurn; + if (denominator <= 0) { + // The graph is measured to be removing more per turn than the session is spending, which is + // arithmetically possible and not interpretable as a runway. Reported as such. + return { ...base, withoutGraph: null, counterfactual: 'implausible', savedPerTurn }; + } - return { withGraph, withoutGraph, perTurn: Math.round(perTurn) }; + return { + ...base, + withoutGraph: Math.floor(remaining / denominator), + counterfactual: savedPerTurn > 0 ? 'saving' : 'costing', + savedPerTurn, + withheld, + }; } /** @@ -139,21 +240,99 @@ export function runway({ used, capacity, turns }, rate) { * confident figure derived from four samples -- is precisely what this project * criticises elsewhere. */ +/** + * The events the forecast reasons about, each kind read from the log that keeps it. + * + * Reads come from readMetrics, which applies MAX_EVENTS and a tail byte cap -- correct, because + * reads are the high-volume kind and only the recent ones describe the current burn. + * + * Inject, harvest and substitute come from readBalance, which is exempt from that window and + * exists for precisely this reason. metrics.mjs records what the window does to them, measured in + * this repository: "44 inject records in the file, 9 of them holdout, all at lines 60-76 of + * 9,058 -- every single one outside the window. report() therefore said 0 holdout." + * + * The 10% holdout arm is the rarer kind, so it ages out FIRST. On any graph with more than 5,000 + * events since the last injection burst, downstream(withheld) returned null, savedPerTouch was + * null, and the panel printed "no counterfactual yet -- the control arm needs more volume" while + * balance.jsonl held the control arm in full. The panel blamed insufficient data for a read-path + * bug; in the partial case it computed the published counterfactual from whichever slice of the + * control arm happened to survive the window. + */ +export function balanceAwareEvents(dir) { + const balance = readBalance(dir); + // No id-matching needed, and deliberately so: readBalance already reads balance.jsonl in full, + // migrates the pre-split records out of metrics.jsonl WITHOUT the window, and dedupes the two + // on record id (falling back to a composite for records written before ids existed). It is + // therefore a strict superset of the balance kinds in the windowed read, so the windowed copies + // are dropped outright. Trying to merge them back by id would double-count exactly the legacy + // records that have no id. + const out = readMetrics(dir).filter((e) => !BALANCE_KINDS.has(e?.kind)); + out.push(...balance); + return out; +} + +/** How the runway reads, given what the control arm was actually able to say. */ +function runwayLine(air) { + const head = `~${air.withGraph} turns to compaction`; + switch (air.counterfactual) { + case 'saving': + return `${head}; without the graph, ~${air.withoutGraph}.`; + case 'costing': + // Said plainly rather than suppressed. A tool that can only report results in its own + // favour is not measuring anything. + return `${head}; without the graph, ~${air.withoutGraph} -- the graph is measurably NOT ` + + 'extending the runway here.'; + case 'thin-control': + return `${head} (no counterfactual yet -- the control arm holds ${air.withheld} of the ` + + `${MIN_CONTROL_TOUCHES} held-out touches needed).`; + case 'no-touch-density': + return `${head} (no counterfactual -- this session's intercepted-touch count is unknown, ` + + 'and a per-touch saving cannot be converted to a per-turn one without it).'; + case 'implausible': + return `${head} (no counterfactual -- the measured saving exceeds this session's whole ` + + 'per-turn cost, so the arithmetic does not describe a runway).'; + default: + return `${head} (no counterfactual yet -- the control arm has not spoken).`; + } +} + export function forecastPanel(dir, session = {}, findings = []) { - const events = readMetrics(dir); + const events = balanceAwareEvents(dir); const rate = burnRate(events); const shadow = shadowAvoided(events); const consolidation = aggregateConsolidation(findings); - const air = runway(session, rate); + // The touch count converts the per-touch saving into a per-turn one. Taken from the session + // when the caller knows it, and otherwise from the treated touches this rate was built from. + const touches = session.touches ?? rate?.treatedTouches ?? 0; + const air = runway({ ...session, touches }, rate); const lines = []; const parts = {}; if (air) { parts.runway = air; - lines.push(air.withoutGraph != null - ? `~${air.withGraph} turns to compaction; without the graph, ~${air.withoutGraph}.` - : `~${air.withGraph} turns to compaction (no counterfactual yet -- the control arm needs more volume).`); + + // THE FORECAST KEEPS ITS OWN SCORE -- from here, because this is the only place a forecast is + // actually made. calibration.mjs was written for exactly this and had zero shipped importers: + // no forecast was ever logged, no outcome ever observed, reliability always saw an empty set, + // and calibrate was unreachable. So the shipped panel printed precisely the uncalibrated + // number that module's docstring calls "a vibe with a typeface". + // + // NOT LOGGED HERE. Building a panel is not making a forecast. maybeSurface builds one per + // throttle window and worthSurfacing rejects most of them, so logging on every build appended + // an open forecast record nobody ever saw. observeOutcome closes only the LAST open forecast + // for a session, so every earlier one stayed open forever -- and they accumulate in + // balance.jsonl, whose tail-bytes read is precisely what would then displace the + // inject/harvest/substitute rows BALANCE_KINDS exists to protect. That would have undone this + // change's own fix. The surfacing path logs instead, at the one moment a prediction is + // actually made to somebody. + const score = calibrate(dir, air.withGraph); + parts.calibration = score; + // Published only when the horizon has earned it. An unpublishable score does not replace the + // number -- it is simply absent, exactly as the uncalibrated case reads today. + lines.push(score.publishable + ? `${runwayLine({ ...air, withGraph: score.predictedTurns })}\n (corrected: ${score.note})` + : runwayLine(air)); } if (rate) { diff --git a/hooks-core/metrics.mjs b/hooks-core/metrics.mjs index 52f0d4b23..ae62a1166 100644 --- a/hooks-core/metrics.mjs +++ b/hooks-core/metrics.mjs @@ -61,7 +61,22 @@ const metricsPath = (dir) => join(dir, 'metrics.jsonl'); const balancePath = (dir) => join(dir, 'balance.jsonl'); /** Kinds the net balance is computed from, and which therefore must survive. */ -const BALANCE_KINDS = new Set(['inject', 'harvest', 'substitute']); +/** + * Kinds written to balance.jsonl as well as the firehose, so the event window can never + * starve the measurement. Exported because any reader that needs the unwindowed truth needs to + * know which kinds it applies to, and a second copy of this set elsewhere would drift. + */ +export const BALANCE_KINDS = new Set([ + 'inject', + 'harvest', + 'substitute', + // The forecast's own score. Both kinds are rare and accumulate slowly -- MIN_SCORED is 8 -- + // while the firehose they shared is dominated by per-tool-call records, so they were evicted + // faster than they could accumulate: calibrate() returned 'not yet calibrated (n/8)' forever + // and an outcome whose forecast had scrolled away was dropped with no record at all. + 'forecast', + 'forecast-outcome', +]); /** * Is this touch in the holdout arm? diff --git a/hooks-core/policy.mjs b/hooks-core/policy.mjs index bbe5644fd..523961608 100644 --- a/hooks-core/policy.mjs +++ b/hooks-core/policy.mjs @@ -270,7 +270,7 @@ function statePath(sessionId, agent) { /** A usable state object, whatever was on disk. */ function emptyState() { - return { seen: {}, denied: {}, injected: [], actCounts: {} }; + return { seen: {}, denied: {}, injected: [], actCounts: {}, forecast: null }; } /** @@ -310,6 +310,22 @@ export function loadState(sessionId, agent) { parsed.actCounts && typeof parsed.actCounts === 'object' && !Array.isArray(parsed.actCounts) ? parsed.actCounts : {}, + // THE SAME REASON A THIRD TIME. The forecast throttle records when the panel was last + // computed and what runway it last SHOWED, and both are meaningless within a single hook + // process -- every tool call is a new one. Dropped here, `checkedAt` would reset on every + // call and the panel would rebuild itself, transcript parse and all, on each tool use; and + // `shown` would reset, so worthSurfacing would compare against nothing and re-interrupt with + // the same runway forever. + // AND ITS TIMESTAMP MUST BE A NUMBER. The shape check alone accepted `checkedAt: "1700"`, + // and both the throttle comparison and the merge below order by it -- so a persisted numeric + // STRING compares by lexicographic coercion and can beat a later real timestamp, freezing + // the throttle open or shut. Same class as every other field validated here: a value that + // parsed is not a value that means anything. + forecast: + parsed.forecast && typeof parsed.forecast === 'object' && !Array.isArray(parsed.forecast) + && Number.isFinite(parsed.forecast.checkedAt) + ? parsed.forecast + : null, }; } catch { return emptyState(); @@ -381,6 +397,19 @@ export function saveState(sessionId, state, agent) { } return out; })(), + // LATEST CHECK WINS, which is the opposite direction from the fields above and correct for + // this one. `seen`, `denied`, `injected` and `actCounts` are all append-only, so a union or + // a max is the safe merge. The forecast throttle is a POINT IN TIME: taking the older of two + // concurrent checks would re-open the window and let the panel be rebuilt immediately, which + // is the cost the throttle exists to bound. + forecast: (() => { + const mine = state.forecast || null; + const theirs = current.forecast || null; + const stamp = (f) => (Number.isFinite(f?.checkedAt) ? f.checkedAt : null); + if (stamp(mine) === null) return theirs; + if (stamp(theirs) === null) return mine; + return stamp(mine) >= stamp(theirs) ? mine : theirs; + })(), }; // Write-then-rename so a reader never observes a half-written file. diff --git a/hooks-core/surface.mjs b/hooks-core/surface.mjs new file mode 100644 index 000000000..21ed49795 --- /dev/null +++ b/hooks-core/surface.mjs @@ -0,0 +1,183 @@ +/** + * The entry point the forecast never had. + * + * forecast.mjs computes a runway, calibration.mjs scores it, and worthSurfacing decides whether a + * change is worth interrupting for -- and none of it ran. The reachability guard listed + * forecastPanel, worthSurfacing, logForecast and observeOutcome as unwired, which is a polite way + * of saying the feature did not exist for any user. This module is the missing half: it derives + * the session numbers the panel needs from the transcript, decides when the panel has earned an + * interruption, and closes the calibration loop when compaction fires. + * + * THREE COSTS ARE BOUNDED HERE, because this runs on the PreToolUse path and every tool call is a + * separate process: + * + * THE READ readCacheUsage parses up to 4 MB of transcript. Doing that on every tool call + * would tax the hook that exists to save the user money. + * THE COMPUTE the panel walks the metrics log, the balance log and the graph. + * THE CONTEXT the panel is only worth its own tokens when it says something actionable. + * + * So the throttle is checked FIRST, from persisted state, before any file is opened. Everything + * expensive happens after a clock comparison. + * + * NOTHING HERE IS ALLOWED TO BREAK A TOOL CALL. Every entry point returns null on failure rather + * than throwing: a forecast is a courtesy, and the hook's promise is that a defect in it costs the + * user nothing. + */ + +import { forecastPanel, worthSurfacing } from './forecast.mjs'; +import { observeOutcome, logForecast } from './calibration.mjs'; +import { readCacheUsage } from './cache.mjs'; +import { readBalance } from './metrics.mjs'; + +/** + * How long between two attempts to build the panel. + * + * Not a tuning knob so much as an admission: the panel costs a transcript parse, and a user makes + * many tool calls per minute. Two minutes is short enough that a runway falling into single digits + * is still caught with turns to spare, and long enough that the check is free in aggregate. + */ +export const SURFACE_INTERVAL_MS = + Number(process.env.TOKEN_OPTIMIZER_SURFACE_INTERVAL_MS) || 120_000; + +/** + * The context window to measure the runway against. + * + * NOT DISCOVERABLE from the transcript: the usage rows carry what was spent, never the ceiling it + * was spent against. Defaulting is therefore unavoidable, so the default is stated rather than + * buried, and overridable for anyone running a different window. + */ +export const DEFAULT_CAPACITY = + Number(process.env.TOKEN_OPTIMIZER_CONTEXT_CAPACITY) || 200_000; + +/** + * What the session has spent, read from its own transcript. + * + * `used` is the FULL input size of the most recent request -- cache reads plus cache writes plus + * fresh input -- which is what the context window actually holds, rather than the marginal cost of + * that one turn. Summing the per-turn inputs instead would count the same carried prefix once per + * turn and report a number many times the window size. + * + * Returns null rather than a zeroed object when there is nothing to measure, so a caller cannot + * mistake "no data" for "an empty context". + */ +export function sessionUsage(transcriptPath, { capacity = DEFAULT_CAPACITY } = {}) { + let turns; + try { + turns = readCacheUsage(transcriptPath); + } catch { + return null; + } + if (!turns?.length) return null; + + const last = turns[turns.length - 1]; + const used = (last.read || 0) + (last.written || 0) + (last.input || 0); + if (!used) return null; + + return { used, capacity, turns: turns.length }; +} + +/** Intercepted touches recorded for this session, which converts a per-touch saving to per-turn. */ +function touchesFor(dir, sessionId) { + try { + return readBalance(dir).filter((e) => e.kind === 'inject' && e.sessionId === sessionId).length; + } catch { + return 0; + } +} + +/** + * Should the forecast interrupt right now, and with what? + * + * The state object is the caller's persisted per-session state; this reads `forecast` from it and + * returns the value to store back, so the throttle and the last-shown runway survive across hook + * processes. Returning the next state rather than mutating it keeps this testable without a + * filesystem. + * + * @returns { text, state } -- text is null when nothing should be shown. + */ +export function maybeSurface(dir, { + transcriptPath, sessionId, state = {}, now = Date.now(), findings = [], +} = {}) { + const previous = state.forecast || null; + + // THE THROTTLE IS FIRST, and deliberately before any I/O. Everything below opens files. + if (previous?.checkedAt && now - previous.checkedAt < SURFACE_INTERVAL_MS) { + return { text: null, state: previous }; + } + + let usage; + try { + usage = sessionUsage(transcriptPath); + } catch { + return { text: null, state: previous }; + } + // The clock is stamped even when there is nothing to measure, so a session with no usage rows + // does not re-attempt the transcript parse on every single tool call. + if (!usage) return { text: null, state: { ...previous, checkedAt: now } }; + + let panel; + try { + panel = forecastPanel(dir, { + ...usage, + sessionId, + touches: touchesFor(dir, sessionId), + }, findings); + } catch { + return { text: null, state: { ...previous, checkedAt: now } }; + } + if (!panel) return { text: null, state: { ...previous, checkedAt: now } }; + + // worthSurfacing compares against the last panel that was actually SHOWN, not the last one + // computed. Comparing against the last computed panel would let the runway drift down past the + // threshold one throttle window at a time and never trip the "crossed it" test. + const shown = previous?.shown ? { parts: { runway: { withGraph: previous.shown } } } : null; + if (!worthSurfacing(panel, shown)) { + return { text: null, state: { ...previous, checkedAt: now } }; + } + + // LOGGED HERE, AND ONLY HERE -- past the throttle and past worthSurfacing, so exactly one record + // is written per prediction a person is actually shown. forecastPanel used to log on every build; + // since most builds are rejected above, that appended open forecast records nobody saw, and + // observeOutcome closes only the newest one per session. The rest stayed open forever and piled + // up in balance.jsonl, whose tail-bytes read would then evict the inject/harvest/substitute rows + // BALANCE_KINDS protects -- undoing the very repair this work exists to make. + // + // The RAW runway is logged, not the calibrated one: scoring a corrected number would fold each + // correction into the next and the bias would chase its own tail. + try { + logForecast(dir, { + sessionId, + predictedTurns: panel.parts.runway.withGraph, + used: usage.used, + capacity: usage.capacity, + turns: usage.turns, + }); + } catch { /* a forecast is a courtesy; failing to score it must not cost a tool call */ } + + return { + text: panel.text, + state: { checkedAt: now, shown: panel.parts.runway.withGraph }, + }; +} + +/** + * Closes the calibration loop, at the only moment that can close it. + * + * Compaction firing IS the ground truth a runway forecast was predicting, so this belongs on the + * PreCompact path and nowhere else. `actualTurns` is turns ELAPSED between the prediction and + * compaction, which is what predictedTurns meant -- so the forecast has to have recorded the turn + * it was made on, and observeOutcome subtracts. + * + * Silent by design: nothing is shown to anybody. It writes one record so that the NEXT session's + * forecast can be published with a track record instead of as a bare assertion. + */ +export function closeForecast(dir, { transcriptPath, sessionId } = {}) { + try { + const usage = sessionUsage(transcriptPath); + if (!usage) return false; + observeOutcome(dir, { sessionId, atTurn: usage.turns }); + return true; + } catch { + return false; + } +} diff --git a/integrations/codex/hooks/lib/calibration.mjs b/integrations/codex/hooks/lib/calibration.mjs index 718db89cd..a126fcaef 100644 --- a/integrations/codex/hooks/lib/calibration.mjs +++ b/integrations/codex/hooks/lib/calibration.mjs @@ -31,7 +31,20 @@ * that cannot support one. */ -import { record, readMetrics } from './metrics.mjs'; +// readBalance, not readMetrics. Both readers used the windowed firehose, which returns at most +// 2,000,000 bytes and the last 5,000 lines -- and forecast events were not among the protected +// kinds, so they lived in a log dominated by per-tool-call records (metrics.mjs's own comment +// cites 4,735 capture events inside one window). +// +// Two failures followed. In observeOutcome, on a busy project the forecast had scrolled past the +// tail by the time compaction fired, so the outcome was discarded with no record, no error and no +// counter. In reliability, outcomes vanished faster than they accumulated, so rows.length could +// never reach MIN_SCORED and calibrate returned 'not yet calibrated (n/8)' permanently -- leaving +// a user unable to distinguish "never had data" from "the data is being thrown away". +// +// 'forecast' and 'forecast-outcome' are now balance kinds, so they are written to balance.jsonl +// as well and read back unwindowed. +import { record, readBalance } from './metrics.mjs'; /** Horizon buckets, in turns. Error behaves differently across these ranges. */ const HORIZONS = [ @@ -53,8 +66,15 @@ function horizonOf(turns) { return HORIZONS.find((h) => turns <= h.max).name; } -/** Records a prediction so it can be scored when the outcome arrives. */ -export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) { +/** + * Records a prediction so it can be scored when the outcome arrives. + * + * `turns` is the session's turn count AT PREDICTION TIME, and it is what makes the outcome + * computable at all: predictedTurns means "turns from here until compaction", so the ground truth + * is the turns that ELAPSED, not the turn number compaction happened on. Without it the caller at + * compaction has nothing to subtract from and would have to score an interval against an absolute. + */ +export function logForecast(dir, { sessionId, predictedTurns, used, capacity, turns }) { if (!Number.isFinite(predictedTurns)) return; record(dir, { kind: 'forecast', @@ -63,6 +83,7 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) horizon: horizonOf(predictedTurns), used, capacity, + turns: Number.isFinite(turns) ? turns : null, }); } @@ -73,21 +94,74 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) * Called when compaction fires: the number of turns that elapsed since the * prediction is the ground truth it was predicting. */ -export function observeOutcome(dir, { sessionId, actualTurns }) { - if (!Number.isFinite(actualTurns)) return; - const open = readMetrics(dir) - .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && !e.scored) +export function observeOutcome(dir, { sessionId, actualTurns, atTurn }) { + // EITHER THE ELAPSED COUNT OR THE TURN IT HAPPENED ON. + // + // The only real caller -- the PreCompact hook -- knows what turn compaction fired on, not how + // many turns have passed since a prediction it did not make. Deriving the interval here keeps + // that subtraction next to the record that defines it, rather than making every caller + // re-discover which forecast is open in order to compute its own ground truth. + const elapsed = Number.isFinite(actualTurns) + ? actualTurns + : null; + if (elapsed === null && !Number.isFinite(atTurn)) return; + + // CLOSURE IS DERIVED FROM THE OUTCOME ROWS, not from a flag on the forecast. + // + // The old filter was `!e.scored`, but nothing ever writes `scored`: logForecast records + // kind/sessionId/predictedTurns/horizon/used/capacity, and record() is an append-only JSONL + // writer with no update path, so a forecast can never gain the field. The predicate was always + // true, and the last forecast for a session was therefore re-closable without limit. + // + // A session where compaction fires twice with no intervening logForecast popped the SAME + // forecast twice, producing two outcome rows for one prediction. reliability counts ROWS, not + // predictions, so the hit rate and the bias mean both inflate and MIN_SCORED can be satisfied + // by a single prediction re-closed eight times -- precisely the "threshold from a single + // sample" this module exists to prevent. The existing test used a fresh sessionId per + // iteration, so it never exposed it. + const events = readBalance(dir); + const closed = new Set( + events.filter((e) => e.kind === 'forecast-outcome' && e.forecastId).map((e) => e.forecastId), + ); + // ID-BEARING ROWS ONLY, AND NEWEST BY TIME RATHER THAN BY POSITION. + // + // Two separate hazards, both from readBalance's shape. It returns balance-log rows followed by + // the legacy firehose rows it migrates, unsorted -- so `.pop()` returns the last row in the + // array, which is not the newest forecast. And legacy rows predate `id`, so `closed.has(e.id)` + // is `closed.has(undefined)`, always false: such a forecast can be closed again and again, and + // each outcome stores `forecastId: undefined`, which JSON.stringify drops entirely. That is the + // repeat-scoring defect this function was written to fix, reappearing through the back door. + const open = events + .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && e.id && !closed.has(e.id)) + .sort((a, b) => (a.at ?? 0) - (b.at ?? 0)) .pop(); if (!open) return; + // Derived only once the open forecast is known, since the interval is measured from ITS turn. + // A forecast recorded before `turns` existed has none, and there is nothing to subtract from -- + // scoring it against an absolute turn number would manufacture an error rather than measure one, + // so it is left open instead. + const actual = elapsed !== null ? elapsed + : (Number.isFinite(open.turns) ? atTurn - open.turns : null); + // AT LEAST ONE TURN MUST HAVE PASSED. `atTurn` is the transcript's current turn count and + // `open.turns` the count at prediction time, so a truncated, rotated or replaced transcript + // makes the difference zero or negative. Recording that as ground truth feeds a large negative + // `error` into `reliability`, which shifts `bucket.bias`, which is what `calibrate` publishes a + // corrected forecast from -- so one bad transcript would skew every later prediction. Leaving + // the forecast open loses nothing: the next compaction closes it against a sane count. + if (actual === null || !Number.isFinite(actual) || actual < 1) return; + record(dir, { kind: 'forecast-outcome', sessionId, + // Stamped so the next call can see this forecast is spoken for. Without it the set above is + // empty and the same defect returns. + forecastId: open.id, horizon: open.horizon, predictedTurns: open.predictedTurns, - actualTurns, - error: actualTurns - open.predictedTurns, - hit: Math.abs(actualTurns - open.predictedTurns) <= TOLERANCE, + actualTurns: actual, + error: actual - open.predictedTurns, + hit: Math.abs(actual - open.predictedTurns) <= TOLERANCE, }); } @@ -98,7 +172,7 @@ export function observeOutcome(dir, { sessionId, actualTurns }) { * actionable, where "62% accurate" is not. */ export function reliability(dir) { - const outcomes = readMetrics(dir).filter((e) => e.kind === 'forecast-outcome'); + const outcomes = readBalance(dir).filter((e) => e.kind === 'forecast-outcome'); const buckets = {}; for (const horizon of HORIZONS) { @@ -164,7 +238,28 @@ export function calibrate(dir, predictedTurns) { // Refit: shift by the observed bias for this horizon. A bucket that runs long // gets pulled in, and vice versa. - const adjusted = Math.max(0, Math.round(predictedTurns + bucket.bias)); + // + // NOT CLAMPED WITH Math.max, which converted nonsense into a plausible-looking emergency. bias + // is a plain arithmetic mean of signed error, and a mean is not robust to the shape the + // 50%-hit-rate floor admits: a near bucket with errors [0,0,0,0,-30,-30,-30,-30] has four hits, + // so it clears the floor and is called calibrated, and its bias is -15. A raw forecast of 3 + // turns then became Math.max(0, Math.round(3 - 15)) = 0 and was published as fact, with the + // note "within 3 on 50% of 8 past forecasts" -- a zero-turn deadline the forecaster never + // produced, from a bucket that is wrong half the time. + // + // A correction at least as large as the thing it corrects is not a correction. + const adjusted = Math.round(predictedTurns + bucket.bias); + if (Math.abs(bucket.bias) >= predictedTurns || adjusted < 1) { + return { + publishable: false, + predictedTurns, + horizon, + bias: bucket.bias, + reason: `the observed bias at this horizon (${Math.round(bucket.bias)} turns) is as large as ` + + `the forecast itself (${predictedTurns}), so the correction would replace it rather than ` + + 'adjust it', + }; + } return { publishable: true, diff --git a/integrations/codex/hooks/lib/forecast.mjs b/integrations/codex/hooks/lib/forecast.mjs index aed24df29..474ed84f9 100644 --- a/integrations/codex/hooks/lib/forecast.mjs +++ b/integrations/codex/hooks/lib/forecast.mjs @@ -31,13 +31,30 @@ * other rather than one standing in for the other. */ -import { readMetrics } from './metrics.mjs'; +import { readMetrics, readBalance, isFixtureAnchor, BALANCE_KINDS } from './metrics.mjs'; import { aggregateConsolidation } from './consolidate.mjs'; import { previewQuality } from './expand.mjs'; +import { calibrate } from './calibration.mjs'; /** Turns of headroom below which the runway is worth interrupting for. */ export const ACTIONABLE_RUNWAY = 8; +/** + * Held-out touches required before the runway counterfactual is published. + * + * The header of this file promises the counterfactual appears "only once the holdout carries + * volume", and the fallback string says "the control arm needs more volume" -- but the only check + * was `if (!rows.length) return null`, so "volume" meant "at least one". With a single withheld + * touch, withheldCost IS that one touch's downstream read total: one held-out read of a + * 3,000-line module set savedPerTouch to several thousand tokens and published a counterfactual + * an order of magnitude off, as a bare fact with no caveat. + * + * Ten is a floor rather than a statistically derived threshold, and is deliberately stated as + * such where it surfaces: the panel prints the held-out count against it, so a reader can see how + * close the arm is instead of being told only that it is short. + */ +export const MIN_CONTROL_TOUCHES = 10; + /** * Tokens per turn, and how many the graph is removing. * @@ -57,13 +74,42 @@ export function burnRate(events) { reads.get(key).push(event); } + // EACH READ IS CHARGED TO EXACTLY ONE INJECTION. + // + // The windows used to run from each injection to the end of the log, so they overlapped + // completely: for anchor A with injections i1..i5 interleaved with five reads of 100 tokens, + // i1 saw 500, i2 400, i3 300, i4 200, i5 100 -- 1500 / 5 = 300 against a true per-touch cost of + // 100. A 3x inflation growing with the repeat count. + // + // It does not cancel between the arms. metrics.mjs makes the holdout deterministic in + // (anchor, epoch), so every repeat touch of a file lands in the SAME arm -- whichever arm drew + // the heavily re-touched anchors is inflated and the other is not. savedPerTouch's sign and + // magnitude were therefore driven by which files happened to be held out, and that value is + // what the published counterfactual divides by. + // + // Bounding each window at the next injection of the same (session, anchor) -- across BOTH arms, + // since an injection ends the previous window regardless of which arm it is in -- makes the + // charge exact. + const nextInjectAfter = new Map(); + for (const row of injects) { + const key = `${row.sessionId || ''}|${row.anchor}`; + if (!nextInjectAfter.has(key)) nextInjectAfter.set(key, []); + nextInjectAfter.get(key).push(row.at ?? 0); + } + for (const times of nextInjectAfter.values()) times.sort((a, b) => a - b); + const downstream = (rows) => { if (!rows.length) return null; let total = 0; for (const row of rows) { - const bucket = reads.get(`${row.sessionId || ''}|${row.anchor}`) || []; + const key = `${row.sessionId || ''}|${row.anchor}`; + const bucket = reads.get(key) || []; const after = row.at ?? 0; - total += bucket.reduce((sum, r) => sum + ((r.at ?? 0) >= after ? (r.tokens || 0) : 0), 0); + const until = (nextInjectAfter.get(key) || []).find((t) => t > after) ?? Infinity; + total += bucket.reduce( + (sum, r) => sum + ((r.at ?? 0) >= after && (r.at ?? 0) < until ? (r.tokens || 0) : 0), + 0, + ); } return total / rows.length; }; @@ -98,12 +144,27 @@ export function burnRate(events) { export function shadowAvoided(events) { let avoided = 0; let spent = 0; + let seen = 0; for (const event of events) { if (event.kind !== 'substitute') continue; + // FIXTURES EXCLUDED, as balanceSheet already does. metrics.mjs records the measurement: + // "366 of 370 substitutions pointed at the enforcement suite's own big.ts fixture under a + // temp dir, and counting them made the product look like it had avoided 40 MB of reads. It + // had avoided 154 KB." Running the test suite against a real graph inflated `avoided` by two + // orders of magnitude, which blew up shadow.net, pushed `spread` past 0.5 and fired the + // divergence note with a fabricated number -- so the panel's own credibility check was the + // thing the fixture noise corrupted. + if (isFixtureAnchor(event.anchor)) continue; + seen += 1; avoided += Math.ceil((event.bytesAvoided || 0) / 4); spent += event.tokens || 0; } - return avoided ? { avoided, spent, net: avoided - spent } : null; + // GUARDED ON WHETHER ANYTHING WAS SEEN, not on whether it was favourable. Guarding on `avoided` + // meant twenty substitutions that each spent 20 tokens and avoided nothing -- a measured net + // loss of 400 -- returned null, so the divergence block never ran and the panel said nothing + // about the shadow arm. The one outcome the function could not express was the one unfavourable + // to the product, collapsed into the same return value as "no data". + return seen ? { avoided, spent, net: avoided - spent, substitutions: seen } : null; } /** @@ -112,8 +173,10 @@ export function shadowAvoided(events) { * @param used Tokens of context consumed so far. * @param capacity Context window size. * @param turns Turns taken to consume it. + * @param touches Intercepted touches in this session. Required for the counterfactual: it is + * what converts a per-TOUCH saving into a per-TURN one. */ -export function runway({ used, capacity, turns }, rate) { +export function runway({ used, capacity, turns, touches }, rate) { if (!capacity || !turns || used == null) return null; const perTurn = used / turns; @@ -121,15 +184,53 @@ export function runway({ used, capacity, turns }, rate) { const remaining = Math.max(0, capacity - used); const withGraph = Math.floor(remaining / perTurn); + const base = { withGraph, perTurn: Math.round(perTurn) }; - // Without the graph each turn costs what it costs now PLUS what the graph is - // removing. Only computable when the control arm has spoken. const saved = rate?.savedPerTouch; - const withoutGraph = saved != null && saved > 0 - ? Math.floor(remaining / (perTurn + saved)) - : null; + if (saved == null) return { ...base, withoutGraph: null, counterfactual: 'unmeasured' }; + + // VOLUME, MEANING VOLUME. See MIN_CONTROL_TOUCHES. + const withheld = rate.withheldTouches ?? 0; + if (withheld < MIN_CONTROL_TOUCHES) { + return { ...base, withoutGraph: null, counterfactual: 'thin-control', withheld }; + } + + // UNITS. `perTurn` is tokens per TURN for this session; `savedPerTouch` is an average over + // injection EVENTS across the whole metrics log. Adding them directly was only correct with + // exactly one intercepted touch per turn, which is not the usual case in either direction. + // + // Session {used 60000, capacity 200000, turns 30} gives perTurn 2000 and remaining 140000. With + // 5 injections over those 30 turns and savedPerTouch 1500, the true per-turn saving is 250, so + // the honest counterfactual is floor(140000/2250) = 62. Adding the per-touch figure gave + // floor(140000/3500) = 40, and the panel published "~70 turns to compaction; without the graph, + // ~40" -- claiming the graph nearly doubled the runway when it added about eight turns. + // + // Without a touch count there is no density to convert with, so the counterfactual is withheld + // rather than computed on the one-touch-per-turn assumption. + if (!touches) return { ...base, withoutGraph: null, counterfactual: 'no-touch-density' }; + + const savedPerTurn = saved * (touches / turns); + + // A MEASURED NON-POSITIVE SAVING IS A RESULT, NOT A MISSING ONE. The old `saved > 0` guard + // discarded a measurement: with withheldCost 1000, treatedCost 900 and injectionCost 200, + // savedPerTouch is -100 -- the control arm has spoken and said the graph is a net cost -- and + // the panel printed "no counterfactual yet, the control arm needs more volume". The one result + // unfavourable to the product was the one result the panel could not show, disguised as + // insufficient data. The runway is longer without the graph in that case, and says so. + const denominator = perTurn + savedPerTurn; + if (denominator <= 0) { + // The graph is measured to be removing more per turn than the session is spending, which is + // arithmetically possible and not interpretable as a runway. Reported as such. + return { ...base, withoutGraph: null, counterfactual: 'implausible', savedPerTurn }; + } - return { withGraph, withoutGraph, perTurn: Math.round(perTurn) }; + return { + ...base, + withoutGraph: Math.floor(remaining / denominator), + counterfactual: savedPerTurn > 0 ? 'saving' : 'costing', + savedPerTurn, + withheld, + }; } /** @@ -141,21 +242,99 @@ export function runway({ used, capacity, turns }, rate) { * confident figure derived from four samples -- is precisely what this project * criticises elsewhere. */ +/** + * The events the forecast reasons about, each kind read from the log that keeps it. + * + * Reads come from readMetrics, which applies MAX_EVENTS and a tail byte cap -- correct, because + * reads are the high-volume kind and only the recent ones describe the current burn. + * + * Inject, harvest and substitute come from readBalance, which is exempt from that window and + * exists for precisely this reason. metrics.mjs records what the window does to them, measured in + * this repository: "44 inject records in the file, 9 of them holdout, all at lines 60-76 of + * 9,058 -- every single one outside the window. report() therefore said 0 holdout." + * + * The 10% holdout arm is the rarer kind, so it ages out FIRST. On any graph with more than 5,000 + * events since the last injection burst, downstream(withheld) returned null, savedPerTouch was + * null, and the panel printed "no counterfactual yet -- the control arm needs more volume" while + * balance.jsonl held the control arm in full. The panel blamed insufficient data for a read-path + * bug; in the partial case it computed the published counterfactual from whichever slice of the + * control arm happened to survive the window. + */ +export function balanceAwareEvents(dir) { + const balance = readBalance(dir); + // No id-matching needed, and deliberately so: readBalance already reads balance.jsonl in full, + // migrates the pre-split records out of metrics.jsonl WITHOUT the window, and dedupes the two + // on record id (falling back to a composite for records written before ids existed). It is + // therefore a strict superset of the balance kinds in the windowed read, so the windowed copies + // are dropped outright. Trying to merge them back by id would double-count exactly the legacy + // records that have no id. + const out = readMetrics(dir).filter((e) => !BALANCE_KINDS.has(e?.kind)); + out.push(...balance); + return out; +} + +/** How the runway reads, given what the control arm was actually able to say. */ +function runwayLine(air) { + const head = `~${air.withGraph} turns to compaction`; + switch (air.counterfactual) { + case 'saving': + return `${head}; without the graph, ~${air.withoutGraph}.`; + case 'costing': + // Said plainly rather than suppressed. A tool that can only report results in its own + // favour is not measuring anything. + return `${head}; without the graph, ~${air.withoutGraph} -- the graph is measurably NOT ` + + 'extending the runway here.'; + case 'thin-control': + return `${head} (no counterfactual yet -- the control arm holds ${air.withheld} of the ` + + `${MIN_CONTROL_TOUCHES} held-out touches needed).`; + case 'no-touch-density': + return `${head} (no counterfactual -- this session's intercepted-touch count is unknown, ` + + 'and a per-touch saving cannot be converted to a per-turn one without it).'; + case 'implausible': + return `${head} (no counterfactual -- the measured saving exceeds this session's whole ` + + 'per-turn cost, so the arithmetic does not describe a runway).'; + default: + return `${head} (no counterfactual yet -- the control arm has not spoken).`; + } +} + export function forecastPanel(dir, session = {}, findings = []) { - const events = readMetrics(dir); + const events = balanceAwareEvents(dir); const rate = burnRate(events); const shadow = shadowAvoided(events); const consolidation = aggregateConsolidation(findings); - const air = runway(session, rate); + // The touch count converts the per-touch saving into a per-turn one. Taken from the session + // when the caller knows it, and otherwise from the treated touches this rate was built from. + const touches = session.touches ?? rate?.treatedTouches ?? 0; + const air = runway({ ...session, touches }, rate); const lines = []; const parts = {}; if (air) { parts.runway = air; - lines.push(air.withoutGraph != null - ? `~${air.withGraph} turns to compaction; without the graph, ~${air.withoutGraph}.` - : `~${air.withGraph} turns to compaction (no counterfactual yet -- the control arm needs more volume).`); + + // THE FORECAST KEEPS ITS OWN SCORE -- from here, because this is the only place a forecast is + // actually made. calibration.mjs was written for exactly this and had zero shipped importers: + // no forecast was ever logged, no outcome ever observed, reliability always saw an empty set, + // and calibrate was unreachable. So the shipped panel printed precisely the uncalibrated + // number that module's docstring calls "a vibe with a typeface". + // + // NOT LOGGED HERE. Building a panel is not making a forecast. maybeSurface builds one per + // throttle window and worthSurfacing rejects most of them, so logging on every build appended + // an open forecast record nobody ever saw. observeOutcome closes only the LAST open forecast + // for a session, so every earlier one stayed open forever -- and they accumulate in + // balance.jsonl, whose tail-bytes read is precisely what would then displace the + // inject/harvest/substitute rows BALANCE_KINDS exists to protect. That would have undone this + // change's own fix. The surfacing path logs instead, at the one moment a prediction is + // actually made to somebody. + const score = calibrate(dir, air.withGraph); + parts.calibration = score; + // Published only when the horizon has earned it. An unpublishable score does not replace the + // number -- it is simply absent, exactly as the uncalibrated case reads today. + lines.push(score.publishable + ? `${runwayLine({ ...air, withGraph: score.predictedTurns })}\n (corrected: ${score.note})` + : runwayLine(air)); } if (rate) { diff --git a/integrations/codex/hooks/lib/metrics.mjs b/integrations/codex/hooks/lib/metrics.mjs index 3fc52e22b..4b0eb2712 100644 --- a/integrations/codex/hooks/lib/metrics.mjs +++ b/integrations/codex/hooks/lib/metrics.mjs @@ -63,7 +63,22 @@ const metricsPath = (dir) => join(dir, 'metrics.jsonl'); const balancePath = (dir) => join(dir, 'balance.jsonl'); /** Kinds the net balance is computed from, and which therefore must survive. */ -const BALANCE_KINDS = new Set(['inject', 'harvest', 'substitute']); +/** + * Kinds written to balance.jsonl as well as the firehose, so the event window can never + * starve the measurement. Exported because any reader that needs the unwindowed truth needs to + * know which kinds it applies to, and a second copy of this set elsewhere would drift. + */ +export const BALANCE_KINDS = new Set([ + 'inject', + 'harvest', + 'substitute', + // The forecast's own score. Both kinds are rare and accumulate slowly -- MIN_SCORED is 8 -- + // while the firehose they shared is dominated by per-tool-call records, so they were evicted + // faster than they could accumulate: calibrate() returned 'not yet calibrated (n/8)' forever + // and an outcome whose forecast had scrolled away was dropped with no record at all. + 'forecast', + 'forecast-outcome', +]); /** * Is this touch in the holdout arm? diff --git a/integrations/codex/hooks/lib/policy.mjs b/integrations/codex/hooks/lib/policy.mjs index b3871c27d..f89a48210 100644 --- a/integrations/codex/hooks/lib/policy.mjs +++ b/integrations/codex/hooks/lib/policy.mjs @@ -272,7 +272,7 @@ function statePath(sessionId, agent) { /** A usable state object, whatever was on disk. */ function emptyState() { - return { seen: {}, denied: {}, injected: [], actCounts: {} }; + return { seen: {}, denied: {}, injected: [], actCounts: {}, forecast: null }; } /** @@ -312,6 +312,22 @@ export function loadState(sessionId, agent) { parsed.actCounts && typeof parsed.actCounts === 'object' && !Array.isArray(parsed.actCounts) ? parsed.actCounts : {}, + // THE SAME REASON A THIRD TIME. The forecast throttle records when the panel was last + // computed and what runway it last SHOWED, and both are meaningless within a single hook + // process -- every tool call is a new one. Dropped here, `checkedAt` would reset on every + // call and the panel would rebuild itself, transcript parse and all, on each tool use; and + // `shown` would reset, so worthSurfacing would compare against nothing and re-interrupt with + // the same runway forever. + // AND ITS TIMESTAMP MUST BE A NUMBER. The shape check alone accepted `checkedAt: "1700"`, + // and both the throttle comparison and the merge below order by it -- so a persisted numeric + // STRING compares by lexicographic coercion and can beat a later real timestamp, freezing + // the throttle open or shut. Same class as every other field validated here: a value that + // parsed is not a value that means anything. + forecast: + parsed.forecast && typeof parsed.forecast === 'object' && !Array.isArray(parsed.forecast) + && Number.isFinite(parsed.forecast.checkedAt) + ? parsed.forecast + : null, }; } catch { return emptyState(); @@ -383,6 +399,19 @@ export function saveState(sessionId, state, agent) { } return out; })(), + // LATEST CHECK WINS, which is the opposite direction from the fields above and correct for + // this one. `seen`, `denied`, `injected` and `actCounts` are all append-only, so a union or + // a max is the safe merge. The forecast throttle is a POINT IN TIME: taking the older of two + // concurrent checks would re-open the window and let the panel be rebuilt immediately, which + // is the cost the throttle exists to bound. + forecast: (() => { + const mine = state.forecast || null; + const theirs = current.forecast || null; + const stamp = (f) => (Number.isFinite(f?.checkedAt) ? f.checkedAt : null); + if (stamp(mine) === null) return theirs; + if (stamp(theirs) === null) return mine; + return stamp(mine) >= stamp(theirs) ? mine : theirs; + })(), }; // Write-then-rename so a reader never observes a half-written file. diff --git a/integrations/codex/hooks/lib/surface.mjs b/integrations/codex/hooks/lib/surface.mjs new file mode 100644 index 000000000..4054c8cf9 --- /dev/null +++ b/integrations/codex/hooks/lib/surface.mjs @@ -0,0 +1,185 @@ +// GENERATED FILE -- do not edit. +// Source of truth: hooks-core/surface.mjs. Regenerate with `npm run sync:hooks`. +/** + * The entry point the forecast never had. + * + * forecast.mjs computes a runway, calibration.mjs scores it, and worthSurfacing decides whether a + * change is worth interrupting for -- and none of it ran. The reachability guard listed + * forecastPanel, worthSurfacing, logForecast and observeOutcome as unwired, which is a polite way + * of saying the feature did not exist for any user. This module is the missing half: it derives + * the session numbers the panel needs from the transcript, decides when the panel has earned an + * interruption, and closes the calibration loop when compaction fires. + * + * THREE COSTS ARE BOUNDED HERE, because this runs on the PreToolUse path and every tool call is a + * separate process: + * + * THE READ readCacheUsage parses up to 4 MB of transcript. Doing that on every tool call + * would tax the hook that exists to save the user money. + * THE COMPUTE the panel walks the metrics log, the balance log and the graph. + * THE CONTEXT the panel is only worth its own tokens when it says something actionable. + * + * So the throttle is checked FIRST, from persisted state, before any file is opened. Everything + * expensive happens after a clock comparison. + * + * NOTHING HERE IS ALLOWED TO BREAK A TOOL CALL. Every entry point returns null on failure rather + * than throwing: a forecast is a courtesy, and the hook's promise is that a defect in it costs the + * user nothing. + */ + +import { forecastPanel, worthSurfacing } from './forecast.mjs'; +import { observeOutcome, logForecast } from './calibration.mjs'; +import { readCacheUsage } from './cache.mjs'; +import { readBalance } from './metrics.mjs'; + +/** + * How long between two attempts to build the panel. + * + * Not a tuning knob so much as an admission: the panel costs a transcript parse, and a user makes + * many tool calls per minute. Two minutes is short enough that a runway falling into single digits + * is still caught with turns to spare, and long enough that the check is free in aggregate. + */ +export const SURFACE_INTERVAL_MS = + Number(process.env.TOKEN_OPTIMIZER_SURFACE_INTERVAL_MS) || 120_000; + +/** + * The context window to measure the runway against. + * + * NOT DISCOVERABLE from the transcript: the usage rows carry what was spent, never the ceiling it + * was spent against. Defaulting is therefore unavoidable, so the default is stated rather than + * buried, and overridable for anyone running a different window. + */ +export const DEFAULT_CAPACITY = + Number(process.env.TOKEN_OPTIMIZER_CONTEXT_CAPACITY) || 200_000; + +/** + * What the session has spent, read from its own transcript. + * + * `used` is the FULL input size of the most recent request -- cache reads plus cache writes plus + * fresh input -- which is what the context window actually holds, rather than the marginal cost of + * that one turn. Summing the per-turn inputs instead would count the same carried prefix once per + * turn and report a number many times the window size. + * + * Returns null rather than a zeroed object when there is nothing to measure, so a caller cannot + * mistake "no data" for "an empty context". + */ +export function sessionUsage(transcriptPath, { capacity = DEFAULT_CAPACITY } = {}) { + let turns; + try { + turns = readCacheUsage(transcriptPath); + } catch { + return null; + } + if (!turns?.length) return null; + + const last = turns[turns.length - 1]; + const used = (last.read || 0) + (last.written || 0) + (last.input || 0); + if (!used) return null; + + return { used, capacity, turns: turns.length }; +} + +/** Intercepted touches recorded for this session, which converts a per-touch saving to per-turn. */ +function touchesFor(dir, sessionId) { + try { + return readBalance(dir).filter((e) => e.kind === 'inject' && e.sessionId === sessionId).length; + } catch { + return 0; + } +} + +/** + * Should the forecast interrupt right now, and with what? + * + * The state object is the caller's persisted per-session state; this reads `forecast` from it and + * returns the value to store back, so the throttle and the last-shown runway survive across hook + * processes. Returning the next state rather than mutating it keeps this testable without a + * filesystem. + * + * @returns { text, state } -- text is null when nothing should be shown. + */ +export function maybeSurface(dir, { + transcriptPath, sessionId, state = {}, now = Date.now(), findings = [], +} = {}) { + const previous = state.forecast || null; + + // THE THROTTLE IS FIRST, and deliberately before any I/O. Everything below opens files. + if (previous?.checkedAt && now - previous.checkedAt < SURFACE_INTERVAL_MS) { + return { text: null, state: previous }; + } + + let usage; + try { + usage = sessionUsage(transcriptPath); + } catch { + return { text: null, state: previous }; + } + // The clock is stamped even when there is nothing to measure, so a session with no usage rows + // does not re-attempt the transcript parse on every single tool call. + if (!usage) return { text: null, state: { ...previous, checkedAt: now } }; + + let panel; + try { + panel = forecastPanel(dir, { + ...usage, + sessionId, + touches: touchesFor(dir, sessionId), + }, findings); + } catch { + return { text: null, state: { ...previous, checkedAt: now } }; + } + if (!panel) return { text: null, state: { ...previous, checkedAt: now } }; + + // worthSurfacing compares against the last panel that was actually SHOWN, not the last one + // computed. Comparing against the last computed panel would let the runway drift down past the + // threshold one throttle window at a time and never trip the "crossed it" test. + const shown = previous?.shown ? { parts: { runway: { withGraph: previous.shown } } } : null; + if (!worthSurfacing(panel, shown)) { + return { text: null, state: { ...previous, checkedAt: now } }; + } + + // LOGGED HERE, AND ONLY HERE -- past the throttle and past worthSurfacing, so exactly one record + // is written per prediction a person is actually shown. forecastPanel used to log on every build; + // since most builds are rejected above, that appended open forecast records nobody saw, and + // observeOutcome closes only the newest one per session. The rest stayed open forever and piled + // up in balance.jsonl, whose tail-bytes read would then evict the inject/harvest/substitute rows + // BALANCE_KINDS protects -- undoing the very repair this work exists to make. + // + // The RAW runway is logged, not the calibrated one: scoring a corrected number would fold each + // correction into the next and the bias would chase its own tail. + try { + logForecast(dir, { + sessionId, + predictedTurns: panel.parts.runway.withGraph, + used: usage.used, + capacity: usage.capacity, + turns: usage.turns, + }); + } catch { /* a forecast is a courtesy; failing to score it must not cost a tool call */ } + + return { + text: panel.text, + state: { checkedAt: now, shown: panel.parts.runway.withGraph }, + }; +} + +/** + * Closes the calibration loop, at the only moment that can close it. + * + * Compaction firing IS the ground truth a runway forecast was predicting, so this belongs on the + * PreCompact path and nowhere else. `actualTurns` is turns ELAPSED between the prediction and + * compaction, which is what predictedTurns meant -- so the forecast has to have recorded the turn + * it was made on, and observeOutcome subtracts. + * + * Silent by design: nothing is shown to anybody. It writes one record so that the NEXT session's + * forecast can be published with a track record instead of as a bare assertion. + */ +export function closeForecast(dir, { transcriptPath, sessionId } = {}) { + try { + const usage = sessionUsage(transcriptPath); + if (!usage) return false; + observeOutcome(dir, { sessionId, atTurn: usage.turns }); + return true; + } catch { + return false; + } +} diff --git a/integrations/codex/plugin/hooks/lib/calibration.mjs b/integrations/codex/plugin/hooks/lib/calibration.mjs index 718db89cd..a126fcaef 100644 --- a/integrations/codex/plugin/hooks/lib/calibration.mjs +++ b/integrations/codex/plugin/hooks/lib/calibration.mjs @@ -31,7 +31,20 @@ * that cannot support one. */ -import { record, readMetrics } from './metrics.mjs'; +// readBalance, not readMetrics. Both readers used the windowed firehose, which returns at most +// 2,000,000 bytes and the last 5,000 lines -- and forecast events were not among the protected +// kinds, so they lived in a log dominated by per-tool-call records (metrics.mjs's own comment +// cites 4,735 capture events inside one window). +// +// Two failures followed. In observeOutcome, on a busy project the forecast had scrolled past the +// tail by the time compaction fired, so the outcome was discarded with no record, no error and no +// counter. In reliability, outcomes vanished faster than they accumulated, so rows.length could +// never reach MIN_SCORED and calibrate returned 'not yet calibrated (n/8)' permanently -- leaving +// a user unable to distinguish "never had data" from "the data is being thrown away". +// +// 'forecast' and 'forecast-outcome' are now balance kinds, so they are written to balance.jsonl +// as well and read back unwindowed. +import { record, readBalance } from './metrics.mjs'; /** Horizon buckets, in turns. Error behaves differently across these ranges. */ const HORIZONS = [ @@ -53,8 +66,15 @@ function horizonOf(turns) { return HORIZONS.find((h) => turns <= h.max).name; } -/** Records a prediction so it can be scored when the outcome arrives. */ -export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) { +/** + * Records a prediction so it can be scored when the outcome arrives. + * + * `turns` is the session's turn count AT PREDICTION TIME, and it is what makes the outcome + * computable at all: predictedTurns means "turns from here until compaction", so the ground truth + * is the turns that ELAPSED, not the turn number compaction happened on. Without it the caller at + * compaction has nothing to subtract from and would have to score an interval against an absolute. + */ +export function logForecast(dir, { sessionId, predictedTurns, used, capacity, turns }) { if (!Number.isFinite(predictedTurns)) return; record(dir, { kind: 'forecast', @@ -63,6 +83,7 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) horizon: horizonOf(predictedTurns), used, capacity, + turns: Number.isFinite(turns) ? turns : null, }); } @@ -73,21 +94,74 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) * Called when compaction fires: the number of turns that elapsed since the * prediction is the ground truth it was predicting. */ -export function observeOutcome(dir, { sessionId, actualTurns }) { - if (!Number.isFinite(actualTurns)) return; - const open = readMetrics(dir) - .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && !e.scored) +export function observeOutcome(dir, { sessionId, actualTurns, atTurn }) { + // EITHER THE ELAPSED COUNT OR THE TURN IT HAPPENED ON. + // + // The only real caller -- the PreCompact hook -- knows what turn compaction fired on, not how + // many turns have passed since a prediction it did not make. Deriving the interval here keeps + // that subtraction next to the record that defines it, rather than making every caller + // re-discover which forecast is open in order to compute its own ground truth. + const elapsed = Number.isFinite(actualTurns) + ? actualTurns + : null; + if (elapsed === null && !Number.isFinite(atTurn)) return; + + // CLOSURE IS DERIVED FROM THE OUTCOME ROWS, not from a flag on the forecast. + // + // The old filter was `!e.scored`, but nothing ever writes `scored`: logForecast records + // kind/sessionId/predictedTurns/horizon/used/capacity, and record() is an append-only JSONL + // writer with no update path, so a forecast can never gain the field. The predicate was always + // true, and the last forecast for a session was therefore re-closable without limit. + // + // A session where compaction fires twice with no intervening logForecast popped the SAME + // forecast twice, producing two outcome rows for one prediction. reliability counts ROWS, not + // predictions, so the hit rate and the bias mean both inflate and MIN_SCORED can be satisfied + // by a single prediction re-closed eight times -- precisely the "threshold from a single + // sample" this module exists to prevent. The existing test used a fresh sessionId per + // iteration, so it never exposed it. + const events = readBalance(dir); + const closed = new Set( + events.filter((e) => e.kind === 'forecast-outcome' && e.forecastId).map((e) => e.forecastId), + ); + // ID-BEARING ROWS ONLY, AND NEWEST BY TIME RATHER THAN BY POSITION. + // + // Two separate hazards, both from readBalance's shape. It returns balance-log rows followed by + // the legacy firehose rows it migrates, unsorted -- so `.pop()` returns the last row in the + // array, which is not the newest forecast. And legacy rows predate `id`, so `closed.has(e.id)` + // is `closed.has(undefined)`, always false: such a forecast can be closed again and again, and + // each outcome stores `forecastId: undefined`, which JSON.stringify drops entirely. That is the + // repeat-scoring defect this function was written to fix, reappearing through the back door. + const open = events + .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && e.id && !closed.has(e.id)) + .sort((a, b) => (a.at ?? 0) - (b.at ?? 0)) .pop(); if (!open) return; + // Derived only once the open forecast is known, since the interval is measured from ITS turn. + // A forecast recorded before `turns` existed has none, and there is nothing to subtract from -- + // scoring it against an absolute turn number would manufacture an error rather than measure one, + // so it is left open instead. + const actual = elapsed !== null ? elapsed + : (Number.isFinite(open.turns) ? atTurn - open.turns : null); + // AT LEAST ONE TURN MUST HAVE PASSED. `atTurn` is the transcript's current turn count and + // `open.turns` the count at prediction time, so a truncated, rotated or replaced transcript + // makes the difference zero or negative. Recording that as ground truth feeds a large negative + // `error` into `reliability`, which shifts `bucket.bias`, which is what `calibrate` publishes a + // corrected forecast from -- so one bad transcript would skew every later prediction. Leaving + // the forecast open loses nothing: the next compaction closes it against a sane count. + if (actual === null || !Number.isFinite(actual) || actual < 1) return; + record(dir, { kind: 'forecast-outcome', sessionId, + // Stamped so the next call can see this forecast is spoken for. Without it the set above is + // empty and the same defect returns. + forecastId: open.id, horizon: open.horizon, predictedTurns: open.predictedTurns, - actualTurns, - error: actualTurns - open.predictedTurns, - hit: Math.abs(actualTurns - open.predictedTurns) <= TOLERANCE, + actualTurns: actual, + error: actual - open.predictedTurns, + hit: Math.abs(actual - open.predictedTurns) <= TOLERANCE, }); } @@ -98,7 +172,7 @@ export function observeOutcome(dir, { sessionId, actualTurns }) { * actionable, where "62% accurate" is not. */ export function reliability(dir) { - const outcomes = readMetrics(dir).filter((e) => e.kind === 'forecast-outcome'); + const outcomes = readBalance(dir).filter((e) => e.kind === 'forecast-outcome'); const buckets = {}; for (const horizon of HORIZONS) { @@ -164,7 +238,28 @@ export function calibrate(dir, predictedTurns) { // Refit: shift by the observed bias for this horizon. A bucket that runs long // gets pulled in, and vice versa. - const adjusted = Math.max(0, Math.round(predictedTurns + bucket.bias)); + // + // NOT CLAMPED WITH Math.max, which converted nonsense into a plausible-looking emergency. bias + // is a plain arithmetic mean of signed error, and a mean is not robust to the shape the + // 50%-hit-rate floor admits: a near bucket with errors [0,0,0,0,-30,-30,-30,-30] has four hits, + // so it clears the floor and is called calibrated, and its bias is -15. A raw forecast of 3 + // turns then became Math.max(0, Math.round(3 - 15)) = 0 and was published as fact, with the + // note "within 3 on 50% of 8 past forecasts" -- a zero-turn deadline the forecaster never + // produced, from a bucket that is wrong half the time. + // + // A correction at least as large as the thing it corrects is not a correction. + const adjusted = Math.round(predictedTurns + bucket.bias); + if (Math.abs(bucket.bias) >= predictedTurns || adjusted < 1) { + return { + publishable: false, + predictedTurns, + horizon, + bias: bucket.bias, + reason: `the observed bias at this horizon (${Math.round(bucket.bias)} turns) is as large as ` + + `the forecast itself (${predictedTurns}), so the correction would replace it rather than ` + + 'adjust it', + }; + } return { publishable: true, diff --git a/integrations/codex/plugin/hooks/lib/forecast.mjs b/integrations/codex/plugin/hooks/lib/forecast.mjs index aed24df29..474ed84f9 100644 --- a/integrations/codex/plugin/hooks/lib/forecast.mjs +++ b/integrations/codex/plugin/hooks/lib/forecast.mjs @@ -31,13 +31,30 @@ * other rather than one standing in for the other. */ -import { readMetrics } from './metrics.mjs'; +import { readMetrics, readBalance, isFixtureAnchor, BALANCE_KINDS } from './metrics.mjs'; import { aggregateConsolidation } from './consolidate.mjs'; import { previewQuality } from './expand.mjs'; +import { calibrate } from './calibration.mjs'; /** Turns of headroom below which the runway is worth interrupting for. */ export const ACTIONABLE_RUNWAY = 8; +/** + * Held-out touches required before the runway counterfactual is published. + * + * The header of this file promises the counterfactual appears "only once the holdout carries + * volume", and the fallback string says "the control arm needs more volume" -- but the only check + * was `if (!rows.length) return null`, so "volume" meant "at least one". With a single withheld + * touch, withheldCost IS that one touch's downstream read total: one held-out read of a + * 3,000-line module set savedPerTouch to several thousand tokens and published a counterfactual + * an order of magnitude off, as a bare fact with no caveat. + * + * Ten is a floor rather than a statistically derived threshold, and is deliberately stated as + * such where it surfaces: the panel prints the held-out count against it, so a reader can see how + * close the arm is instead of being told only that it is short. + */ +export const MIN_CONTROL_TOUCHES = 10; + /** * Tokens per turn, and how many the graph is removing. * @@ -57,13 +74,42 @@ export function burnRate(events) { reads.get(key).push(event); } + // EACH READ IS CHARGED TO EXACTLY ONE INJECTION. + // + // The windows used to run from each injection to the end of the log, so they overlapped + // completely: for anchor A with injections i1..i5 interleaved with five reads of 100 tokens, + // i1 saw 500, i2 400, i3 300, i4 200, i5 100 -- 1500 / 5 = 300 against a true per-touch cost of + // 100. A 3x inflation growing with the repeat count. + // + // It does not cancel between the arms. metrics.mjs makes the holdout deterministic in + // (anchor, epoch), so every repeat touch of a file lands in the SAME arm -- whichever arm drew + // the heavily re-touched anchors is inflated and the other is not. savedPerTouch's sign and + // magnitude were therefore driven by which files happened to be held out, and that value is + // what the published counterfactual divides by. + // + // Bounding each window at the next injection of the same (session, anchor) -- across BOTH arms, + // since an injection ends the previous window regardless of which arm it is in -- makes the + // charge exact. + const nextInjectAfter = new Map(); + for (const row of injects) { + const key = `${row.sessionId || ''}|${row.anchor}`; + if (!nextInjectAfter.has(key)) nextInjectAfter.set(key, []); + nextInjectAfter.get(key).push(row.at ?? 0); + } + for (const times of nextInjectAfter.values()) times.sort((a, b) => a - b); + const downstream = (rows) => { if (!rows.length) return null; let total = 0; for (const row of rows) { - const bucket = reads.get(`${row.sessionId || ''}|${row.anchor}`) || []; + const key = `${row.sessionId || ''}|${row.anchor}`; + const bucket = reads.get(key) || []; const after = row.at ?? 0; - total += bucket.reduce((sum, r) => sum + ((r.at ?? 0) >= after ? (r.tokens || 0) : 0), 0); + const until = (nextInjectAfter.get(key) || []).find((t) => t > after) ?? Infinity; + total += bucket.reduce( + (sum, r) => sum + ((r.at ?? 0) >= after && (r.at ?? 0) < until ? (r.tokens || 0) : 0), + 0, + ); } return total / rows.length; }; @@ -98,12 +144,27 @@ export function burnRate(events) { export function shadowAvoided(events) { let avoided = 0; let spent = 0; + let seen = 0; for (const event of events) { if (event.kind !== 'substitute') continue; + // FIXTURES EXCLUDED, as balanceSheet already does. metrics.mjs records the measurement: + // "366 of 370 substitutions pointed at the enforcement suite's own big.ts fixture under a + // temp dir, and counting them made the product look like it had avoided 40 MB of reads. It + // had avoided 154 KB." Running the test suite against a real graph inflated `avoided` by two + // orders of magnitude, which blew up shadow.net, pushed `spread` past 0.5 and fired the + // divergence note with a fabricated number -- so the panel's own credibility check was the + // thing the fixture noise corrupted. + if (isFixtureAnchor(event.anchor)) continue; + seen += 1; avoided += Math.ceil((event.bytesAvoided || 0) / 4); spent += event.tokens || 0; } - return avoided ? { avoided, spent, net: avoided - spent } : null; + // GUARDED ON WHETHER ANYTHING WAS SEEN, not on whether it was favourable. Guarding on `avoided` + // meant twenty substitutions that each spent 20 tokens and avoided nothing -- a measured net + // loss of 400 -- returned null, so the divergence block never ran and the panel said nothing + // about the shadow arm. The one outcome the function could not express was the one unfavourable + // to the product, collapsed into the same return value as "no data". + return seen ? { avoided, spent, net: avoided - spent, substitutions: seen } : null; } /** @@ -112,8 +173,10 @@ export function shadowAvoided(events) { * @param used Tokens of context consumed so far. * @param capacity Context window size. * @param turns Turns taken to consume it. + * @param touches Intercepted touches in this session. Required for the counterfactual: it is + * what converts a per-TOUCH saving into a per-TURN one. */ -export function runway({ used, capacity, turns }, rate) { +export function runway({ used, capacity, turns, touches }, rate) { if (!capacity || !turns || used == null) return null; const perTurn = used / turns; @@ -121,15 +184,53 @@ export function runway({ used, capacity, turns }, rate) { const remaining = Math.max(0, capacity - used); const withGraph = Math.floor(remaining / perTurn); + const base = { withGraph, perTurn: Math.round(perTurn) }; - // Without the graph each turn costs what it costs now PLUS what the graph is - // removing. Only computable when the control arm has spoken. const saved = rate?.savedPerTouch; - const withoutGraph = saved != null && saved > 0 - ? Math.floor(remaining / (perTurn + saved)) - : null; + if (saved == null) return { ...base, withoutGraph: null, counterfactual: 'unmeasured' }; + + // VOLUME, MEANING VOLUME. See MIN_CONTROL_TOUCHES. + const withheld = rate.withheldTouches ?? 0; + if (withheld < MIN_CONTROL_TOUCHES) { + return { ...base, withoutGraph: null, counterfactual: 'thin-control', withheld }; + } + + // UNITS. `perTurn` is tokens per TURN for this session; `savedPerTouch` is an average over + // injection EVENTS across the whole metrics log. Adding them directly was only correct with + // exactly one intercepted touch per turn, which is not the usual case in either direction. + // + // Session {used 60000, capacity 200000, turns 30} gives perTurn 2000 and remaining 140000. With + // 5 injections over those 30 turns and savedPerTouch 1500, the true per-turn saving is 250, so + // the honest counterfactual is floor(140000/2250) = 62. Adding the per-touch figure gave + // floor(140000/3500) = 40, and the panel published "~70 turns to compaction; without the graph, + // ~40" -- claiming the graph nearly doubled the runway when it added about eight turns. + // + // Without a touch count there is no density to convert with, so the counterfactual is withheld + // rather than computed on the one-touch-per-turn assumption. + if (!touches) return { ...base, withoutGraph: null, counterfactual: 'no-touch-density' }; + + const savedPerTurn = saved * (touches / turns); + + // A MEASURED NON-POSITIVE SAVING IS A RESULT, NOT A MISSING ONE. The old `saved > 0` guard + // discarded a measurement: with withheldCost 1000, treatedCost 900 and injectionCost 200, + // savedPerTouch is -100 -- the control arm has spoken and said the graph is a net cost -- and + // the panel printed "no counterfactual yet, the control arm needs more volume". The one result + // unfavourable to the product was the one result the panel could not show, disguised as + // insufficient data. The runway is longer without the graph in that case, and says so. + const denominator = perTurn + savedPerTurn; + if (denominator <= 0) { + // The graph is measured to be removing more per turn than the session is spending, which is + // arithmetically possible and not interpretable as a runway. Reported as such. + return { ...base, withoutGraph: null, counterfactual: 'implausible', savedPerTurn }; + } - return { withGraph, withoutGraph, perTurn: Math.round(perTurn) }; + return { + ...base, + withoutGraph: Math.floor(remaining / denominator), + counterfactual: savedPerTurn > 0 ? 'saving' : 'costing', + savedPerTurn, + withheld, + }; } /** @@ -141,21 +242,99 @@ export function runway({ used, capacity, turns }, rate) { * confident figure derived from four samples -- is precisely what this project * criticises elsewhere. */ +/** + * The events the forecast reasons about, each kind read from the log that keeps it. + * + * Reads come from readMetrics, which applies MAX_EVENTS and a tail byte cap -- correct, because + * reads are the high-volume kind and only the recent ones describe the current burn. + * + * Inject, harvest and substitute come from readBalance, which is exempt from that window and + * exists for precisely this reason. metrics.mjs records what the window does to them, measured in + * this repository: "44 inject records in the file, 9 of them holdout, all at lines 60-76 of + * 9,058 -- every single one outside the window. report() therefore said 0 holdout." + * + * The 10% holdout arm is the rarer kind, so it ages out FIRST. On any graph with more than 5,000 + * events since the last injection burst, downstream(withheld) returned null, savedPerTouch was + * null, and the panel printed "no counterfactual yet -- the control arm needs more volume" while + * balance.jsonl held the control arm in full. The panel blamed insufficient data for a read-path + * bug; in the partial case it computed the published counterfactual from whichever slice of the + * control arm happened to survive the window. + */ +export function balanceAwareEvents(dir) { + const balance = readBalance(dir); + // No id-matching needed, and deliberately so: readBalance already reads balance.jsonl in full, + // migrates the pre-split records out of metrics.jsonl WITHOUT the window, and dedupes the two + // on record id (falling back to a composite for records written before ids existed). It is + // therefore a strict superset of the balance kinds in the windowed read, so the windowed copies + // are dropped outright. Trying to merge them back by id would double-count exactly the legacy + // records that have no id. + const out = readMetrics(dir).filter((e) => !BALANCE_KINDS.has(e?.kind)); + out.push(...balance); + return out; +} + +/** How the runway reads, given what the control arm was actually able to say. */ +function runwayLine(air) { + const head = `~${air.withGraph} turns to compaction`; + switch (air.counterfactual) { + case 'saving': + return `${head}; without the graph, ~${air.withoutGraph}.`; + case 'costing': + // Said plainly rather than suppressed. A tool that can only report results in its own + // favour is not measuring anything. + return `${head}; without the graph, ~${air.withoutGraph} -- the graph is measurably NOT ` + + 'extending the runway here.'; + case 'thin-control': + return `${head} (no counterfactual yet -- the control arm holds ${air.withheld} of the ` + + `${MIN_CONTROL_TOUCHES} held-out touches needed).`; + case 'no-touch-density': + return `${head} (no counterfactual -- this session's intercepted-touch count is unknown, ` + + 'and a per-touch saving cannot be converted to a per-turn one without it).'; + case 'implausible': + return `${head} (no counterfactual -- the measured saving exceeds this session's whole ` + + 'per-turn cost, so the arithmetic does not describe a runway).'; + default: + return `${head} (no counterfactual yet -- the control arm has not spoken).`; + } +} + export function forecastPanel(dir, session = {}, findings = []) { - const events = readMetrics(dir); + const events = balanceAwareEvents(dir); const rate = burnRate(events); const shadow = shadowAvoided(events); const consolidation = aggregateConsolidation(findings); - const air = runway(session, rate); + // The touch count converts the per-touch saving into a per-turn one. Taken from the session + // when the caller knows it, and otherwise from the treated touches this rate was built from. + const touches = session.touches ?? rate?.treatedTouches ?? 0; + const air = runway({ ...session, touches }, rate); const lines = []; const parts = {}; if (air) { parts.runway = air; - lines.push(air.withoutGraph != null - ? `~${air.withGraph} turns to compaction; without the graph, ~${air.withoutGraph}.` - : `~${air.withGraph} turns to compaction (no counterfactual yet -- the control arm needs more volume).`); + + // THE FORECAST KEEPS ITS OWN SCORE -- from here, because this is the only place a forecast is + // actually made. calibration.mjs was written for exactly this and had zero shipped importers: + // no forecast was ever logged, no outcome ever observed, reliability always saw an empty set, + // and calibrate was unreachable. So the shipped panel printed precisely the uncalibrated + // number that module's docstring calls "a vibe with a typeface". + // + // NOT LOGGED HERE. Building a panel is not making a forecast. maybeSurface builds one per + // throttle window and worthSurfacing rejects most of them, so logging on every build appended + // an open forecast record nobody ever saw. observeOutcome closes only the LAST open forecast + // for a session, so every earlier one stayed open forever -- and they accumulate in + // balance.jsonl, whose tail-bytes read is precisely what would then displace the + // inject/harvest/substitute rows BALANCE_KINDS exists to protect. That would have undone this + // change's own fix. The surfacing path logs instead, at the one moment a prediction is + // actually made to somebody. + const score = calibrate(dir, air.withGraph); + parts.calibration = score; + // Published only when the horizon has earned it. An unpublishable score does not replace the + // number -- it is simply absent, exactly as the uncalibrated case reads today. + lines.push(score.publishable + ? `${runwayLine({ ...air, withGraph: score.predictedTurns })}\n (corrected: ${score.note})` + : runwayLine(air)); } if (rate) { diff --git a/integrations/codex/plugin/hooks/lib/metrics.mjs b/integrations/codex/plugin/hooks/lib/metrics.mjs index 3fc52e22b..4b0eb2712 100644 --- a/integrations/codex/plugin/hooks/lib/metrics.mjs +++ b/integrations/codex/plugin/hooks/lib/metrics.mjs @@ -63,7 +63,22 @@ const metricsPath = (dir) => join(dir, 'metrics.jsonl'); const balancePath = (dir) => join(dir, 'balance.jsonl'); /** Kinds the net balance is computed from, and which therefore must survive. */ -const BALANCE_KINDS = new Set(['inject', 'harvest', 'substitute']); +/** + * Kinds written to balance.jsonl as well as the firehose, so the event window can never + * starve the measurement. Exported because any reader that needs the unwindowed truth needs to + * know which kinds it applies to, and a second copy of this set elsewhere would drift. + */ +export const BALANCE_KINDS = new Set([ + 'inject', + 'harvest', + 'substitute', + // The forecast's own score. Both kinds are rare and accumulate slowly -- MIN_SCORED is 8 -- + // while the firehose they shared is dominated by per-tool-call records, so they were evicted + // faster than they could accumulate: calibrate() returned 'not yet calibrated (n/8)' forever + // and an outcome whose forecast had scrolled away was dropped with no record at all. + 'forecast', + 'forecast-outcome', +]); /** * Is this touch in the holdout arm? diff --git a/integrations/codex/plugin/hooks/lib/policy.mjs b/integrations/codex/plugin/hooks/lib/policy.mjs index b3871c27d..f89a48210 100644 --- a/integrations/codex/plugin/hooks/lib/policy.mjs +++ b/integrations/codex/plugin/hooks/lib/policy.mjs @@ -272,7 +272,7 @@ function statePath(sessionId, agent) { /** A usable state object, whatever was on disk. */ function emptyState() { - return { seen: {}, denied: {}, injected: [], actCounts: {} }; + return { seen: {}, denied: {}, injected: [], actCounts: {}, forecast: null }; } /** @@ -312,6 +312,22 @@ export function loadState(sessionId, agent) { parsed.actCounts && typeof parsed.actCounts === 'object' && !Array.isArray(parsed.actCounts) ? parsed.actCounts : {}, + // THE SAME REASON A THIRD TIME. The forecast throttle records when the panel was last + // computed and what runway it last SHOWED, and both are meaningless within a single hook + // process -- every tool call is a new one. Dropped here, `checkedAt` would reset on every + // call and the panel would rebuild itself, transcript parse and all, on each tool use; and + // `shown` would reset, so worthSurfacing would compare against nothing and re-interrupt with + // the same runway forever. + // AND ITS TIMESTAMP MUST BE A NUMBER. The shape check alone accepted `checkedAt: "1700"`, + // and both the throttle comparison and the merge below order by it -- so a persisted numeric + // STRING compares by lexicographic coercion and can beat a later real timestamp, freezing + // the throttle open or shut. Same class as every other field validated here: a value that + // parsed is not a value that means anything. + forecast: + parsed.forecast && typeof parsed.forecast === 'object' && !Array.isArray(parsed.forecast) + && Number.isFinite(parsed.forecast.checkedAt) + ? parsed.forecast + : null, }; } catch { return emptyState(); @@ -383,6 +399,19 @@ export function saveState(sessionId, state, agent) { } return out; })(), + // LATEST CHECK WINS, which is the opposite direction from the fields above and correct for + // this one. `seen`, `denied`, `injected` and `actCounts` are all append-only, so a union or + // a max is the safe merge. The forecast throttle is a POINT IN TIME: taking the older of two + // concurrent checks would re-open the window and let the panel be rebuilt immediately, which + // is the cost the throttle exists to bound. + forecast: (() => { + const mine = state.forecast || null; + const theirs = current.forecast || null; + const stamp = (f) => (Number.isFinite(f?.checkedAt) ? f.checkedAt : null); + if (stamp(mine) === null) return theirs; + if (stamp(theirs) === null) return mine; + return stamp(mine) >= stamp(theirs) ? mine : theirs; + })(), }; // Write-then-rename so a reader never observes a half-written file. diff --git a/integrations/codex/plugin/hooks/lib/surface.mjs b/integrations/codex/plugin/hooks/lib/surface.mjs new file mode 100644 index 000000000..4054c8cf9 --- /dev/null +++ b/integrations/codex/plugin/hooks/lib/surface.mjs @@ -0,0 +1,185 @@ +// GENERATED FILE -- do not edit. +// Source of truth: hooks-core/surface.mjs. Regenerate with `npm run sync:hooks`. +/** + * The entry point the forecast never had. + * + * forecast.mjs computes a runway, calibration.mjs scores it, and worthSurfacing decides whether a + * change is worth interrupting for -- and none of it ran. The reachability guard listed + * forecastPanel, worthSurfacing, logForecast and observeOutcome as unwired, which is a polite way + * of saying the feature did not exist for any user. This module is the missing half: it derives + * the session numbers the panel needs from the transcript, decides when the panel has earned an + * interruption, and closes the calibration loop when compaction fires. + * + * THREE COSTS ARE BOUNDED HERE, because this runs on the PreToolUse path and every tool call is a + * separate process: + * + * THE READ readCacheUsage parses up to 4 MB of transcript. Doing that on every tool call + * would tax the hook that exists to save the user money. + * THE COMPUTE the panel walks the metrics log, the balance log and the graph. + * THE CONTEXT the panel is only worth its own tokens when it says something actionable. + * + * So the throttle is checked FIRST, from persisted state, before any file is opened. Everything + * expensive happens after a clock comparison. + * + * NOTHING HERE IS ALLOWED TO BREAK A TOOL CALL. Every entry point returns null on failure rather + * than throwing: a forecast is a courtesy, and the hook's promise is that a defect in it costs the + * user nothing. + */ + +import { forecastPanel, worthSurfacing } from './forecast.mjs'; +import { observeOutcome, logForecast } from './calibration.mjs'; +import { readCacheUsage } from './cache.mjs'; +import { readBalance } from './metrics.mjs'; + +/** + * How long between two attempts to build the panel. + * + * Not a tuning knob so much as an admission: the panel costs a transcript parse, and a user makes + * many tool calls per minute. Two minutes is short enough that a runway falling into single digits + * is still caught with turns to spare, and long enough that the check is free in aggregate. + */ +export const SURFACE_INTERVAL_MS = + Number(process.env.TOKEN_OPTIMIZER_SURFACE_INTERVAL_MS) || 120_000; + +/** + * The context window to measure the runway against. + * + * NOT DISCOVERABLE from the transcript: the usage rows carry what was spent, never the ceiling it + * was spent against. Defaulting is therefore unavoidable, so the default is stated rather than + * buried, and overridable for anyone running a different window. + */ +export const DEFAULT_CAPACITY = + Number(process.env.TOKEN_OPTIMIZER_CONTEXT_CAPACITY) || 200_000; + +/** + * What the session has spent, read from its own transcript. + * + * `used` is the FULL input size of the most recent request -- cache reads plus cache writes plus + * fresh input -- which is what the context window actually holds, rather than the marginal cost of + * that one turn. Summing the per-turn inputs instead would count the same carried prefix once per + * turn and report a number many times the window size. + * + * Returns null rather than a zeroed object when there is nothing to measure, so a caller cannot + * mistake "no data" for "an empty context". + */ +export function sessionUsage(transcriptPath, { capacity = DEFAULT_CAPACITY } = {}) { + let turns; + try { + turns = readCacheUsage(transcriptPath); + } catch { + return null; + } + if (!turns?.length) return null; + + const last = turns[turns.length - 1]; + const used = (last.read || 0) + (last.written || 0) + (last.input || 0); + if (!used) return null; + + return { used, capacity, turns: turns.length }; +} + +/** Intercepted touches recorded for this session, which converts a per-touch saving to per-turn. */ +function touchesFor(dir, sessionId) { + try { + return readBalance(dir).filter((e) => e.kind === 'inject' && e.sessionId === sessionId).length; + } catch { + return 0; + } +} + +/** + * Should the forecast interrupt right now, and with what? + * + * The state object is the caller's persisted per-session state; this reads `forecast` from it and + * returns the value to store back, so the throttle and the last-shown runway survive across hook + * processes. Returning the next state rather than mutating it keeps this testable without a + * filesystem. + * + * @returns { text, state } -- text is null when nothing should be shown. + */ +export function maybeSurface(dir, { + transcriptPath, sessionId, state = {}, now = Date.now(), findings = [], +} = {}) { + const previous = state.forecast || null; + + // THE THROTTLE IS FIRST, and deliberately before any I/O. Everything below opens files. + if (previous?.checkedAt && now - previous.checkedAt < SURFACE_INTERVAL_MS) { + return { text: null, state: previous }; + } + + let usage; + try { + usage = sessionUsage(transcriptPath); + } catch { + return { text: null, state: previous }; + } + // The clock is stamped even when there is nothing to measure, so a session with no usage rows + // does not re-attempt the transcript parse on every single tool call. + if (!usage) return { text: null, state: { ...previous, checkedAt: now } }; + + let panel; + try { + panel = forecastPanel(dir, { + ...usage, + sessionId, + touches: touchesFor(dir, sessionId), + }, findings); + } catch { + return { text: null, state: { ...previous, checkedAt: now } }; + } + if (!panel) return { text: null, state: { ...previous, checkedAt: now } }; + + // worthSurfacing compares against the last panel that was actually SHOWN, not the last one + // computed. Comparing against the last computed panel would let the runway drift down past the + // threshold one throttle window at a time and never trip the "crossed it" test. + const shown = previous?.shown ? { parts: { runway: { withGraph: previous.shown } } } : null; + if (!worthSurfacing(panel, shown)) { + return { text: null, state: { ...previous, checkedAt: now } }; + } + + // LOGGED HERE, AND ONLY HERE -- past the throttle and past worthSurfacing, so exactly one record + // is written per prediction a person is actually shown. forecastPanel used to log on every build; + // since most builds are rejected above, that appended open forecast records nobody saw, and + // observeOutcome closes only the newest one per session. The rest stayed open forever and piled + // up in balance.jsonl, whose tail-bytes read would then evict the inject/harvest/substitute rows + // BALANCE_KINDS protects -- undoing the very repair this work exists to make. + // + // The RAW runway is logged, not the calibrated one: scoring a corrected number would fold each + // correction into the next and the bias would chase its own tail. + try { + logForecast(dir, { + sessionId, + predictedTurns: panel.parts.runway.withGraph, + used: usage.used, + capacity: usage.capacity, + turns: usage.turns, + }); + } catch { /* a forecast is a courtesy; failing to score it must not cost a tool call */ } + + return { + text: panel.text, + state: { checkedAt: now, shown: panel.parts.runway.withGraph }, + }; +} + +/** + * Closes the calibration loop, at the only moment that can close it. + * + * Compaction firing IS the ground truth a runway forecast was predicting, so this belongs on the + * PreCompact path and nowhere else. `actualTurns` is turns ELAPSED between the prediction and + * compaction, which is what predictedTurns meant -- so the forecast has to have recorded the turn + * it was made on, and observeOutcome subtracts. + * + * Silent by design: nothing is shown to anybody. It writes one record so that the NEXT session's + * forecast can be published with a track record instead of as a bare assertion. + */ +export function closeForecast(dir, { transcriptPath, sessionId } = {}) { + try { + const usage = sessionUsage(transcriptPath); + if (!usage) return false; + observeOutcome(dir, { sessionId, atTurn: usage.turns }); + return true; + } catch { + return false; + } +} diff --git a/integrations/gemini/hooks/lib/calibration.mjs b/integrations/gemini/hooks/lib/calibration.mjs index 718db89cd..a126fcaef 100644 --- a/integrations/gemini/hooks/lib/calibration.mjs +++ b/integrations/gemini/hooks/lib/calibration.mjs @@ -31,7 +31,20 @@ * that cannot support one. */ -import { record, readMetrics } from './metrics.mjs'; +// readBalance, not readMetrics. Both readers used the windowed firehose, which returns at most +// 2,000,000 bytes and the last 5,000 lines -- and forecast events were not among the protected +// kinds, so they lived in a log dominated by per-tool-call records (metrics.mjs's own comment +// cites 4,735 capture events inside one window). +// +// Two failures followed. In observeOutcome, on a busy project the forecast had scrolled past the +// tail by the time compaction fired, so the outcome was discarded with no record, no error and no +// counter. In reliability, outcomes vanished faster than they accumulated, so rows.length could +// never reach MIN_SCORED and calibrate returned 'not yet calibrated (n/8)' permanently -- leaving +// a user unable to distinguish "never had data" from "the data is being thrown away". +// +// 'forecast' and 'forecast-outcome' are now balance kinds, so they are written to balance.jsonl +// as well and read back unwindowed. +import { record, readBalance } from './metrics.mjs'; /** Horizon buckets, in turns. Error behaves differently across these ranges. */ const HORIZONS = [ @@ -53,8 +66,15 @@ function horizonOf(turns) { return HORIZONS.find((h) => turns <= h.max).name; } -/** Records a prediction so it can be scored when the outcome arrives. */ -export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) { +/** + * Records a prediction so it can be scored when the outcome arrives. + * + * `turns` is the session's turn count AT PREDICTION TIME, and it is what makes the outcome + * computable at all: predictedTurns means "turns from here until compaction", so the ground truth + * is the turns that ELAPSED, not the turn number compaction happened on. Without it the caller at + * compaction has nothing to subtract from and would have to score an interval against an absolute. + */ +export function logForecast(dir, { sessionId, predictedTurns, used, capacity, turns }) { if (!Number.isFinite(predictedTurns)) return; record(dir, { kind: 'forecast', @@ -63,6 +83,7 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) horizon: horizonOf(predictedTurns), used, capacity, + turns: Number.isFinite(turns) ? turns : null, }); } @@ -73,21 +94,74 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) * Called when compaction fires: the number of turns that elapsed since the * prediction is the ground truth it was predicting. */ -export function observeOutcome(dir, { sessionId, actualTurns }) { - if (!Number.isFinite(actualTurns)) return; - const open = readMetrics(dir) - .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && !e.scored) +export function observeOutcome(dir, { sessionId, actualTurns, atTurn }) { + // EITHER THE ELAPSED COUNT OR THE TURN IT HAPPENED ON. + // + // The only real caller -- the PreCompact hook -- knows what turn compaction fired on, not how + // many turns have passed since a prediction it did not make. Deriving the interval here keeps + // that subtraction next to the record that defines it, rather than making every caller + // re-discover which forecast is open in order to compute its own ground truth. + const elapsed = Number.isFinite(actualTurns) + ? actualTurns + : null; + if (elapsed === null && !Number.isFinite(atTurn)) return; + + // CLOSURE IS DERIVED FROM THE OUTCOME ROWS, not from a flag on the forecast. + // + // The old filter was `!e.scored`, but nothing ever writes `scored`: logForecast records + // kind/sessionId/predictedTurns/horizon/used/capacity, and record() is an append-only JSONL + // writer with no update path, so a forecast can never gain the field. The predicate was always + // true, and the last forecast for a session was therefore re-closable without limit. + // + // A session where compaction fires twice with no intervening logForecast popped the SAME + // forecast twice, producing two outcome rows for one prediction. reliability counts ROWS, not + // predictions, so the hit rate and the bias mean both inflate and MIN_SCORED can be satisfied + // by a single prediction re-closed eight times -- precisely the "threshold from a single + // sample" this module exists to prevent. The existing test used a fresh sessionId per + // iteration, so it never exposed it. + const events = readBalance(dir); + const closed = new Set( + events.filter((e) => e.kind === 'forecast-outcome' && e.forecastId).map((e) => e.forecastId), + ); + // ID-BEARING ROWS ONLY, AND NEWEST BY TIME RATHER THAN BY POSITION. + // + // Two separate hazards, both from readBalance's shape. It returns balance-log rows followed by + // the legacy firehose rows it migrates, unsorted -- so `.pop()` returns the last row in the + // array, which is not the newest forecast. And legacy rows predate `id`, so `closed.has(e.id)` + // is `closed.has(undefined)`, always false: such a forecast can be closed again and again, and + // each outcome stores `forecastId: undefined`, which JSON.stringify drops entirely. That is the + // repeat-scoring defect this function was written to fix, reappearing through the back door. + const open = events + .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && e.id && !closed.has(e.id)) + .sort((a, b) => (a.at ?? 0) - (b.at ?? 0)) .pop(); if (!open) return; + // Derived only once the open forecast is known, since the interval is measured from ITS turn. + // A forecast recorded before `turns` existed has none, and there is nothing to subtract from -- + // scoring it against an absolute turn number would manufacture an error rather than measure one, + // so it is left open instead. + const actual = elapsed !== null ? elapsed + : (Number.isFinite(open.turns) ? atTurn - open.turns : null); + // AT LEAST ONE TURN MUST HAVE PASSED. `atTurn` is the transcript's current turn count and + // `open.turns` the count at prediction time, so a truncated, rotated or replaced transcript + // makes the difference zero or negative. Recording that as ground truth feeds a large negative + // `error` into `reliability`, which shifts `bucket.bias`, which is what `calibrate` publishes a + // corrected forecast from -- so one bad transcript would skew every later prediction. Leaving + // the forecast open loses nothing: the next compaction closes it against a sane count. + if (actual === null || !Number.isFinite(actual) || actual < 1) return; + record(dir, { kind: 'forecast-outcome', sessionId, + // Stamped so the next call can see this forecast is spoken for. Without it the set above is + // empty and the same defect returns. + forecastId: open.id, horizon: open.horizon, predictedTurns: open.predictedTurns, - actualTurns, - error: actualTurns - open.predictedTurns, - hit: Math.abs(actualTurns - open.predictedTurns) <= TOLERANCE, + actualTurns: actual, + error: actual - open.predictedTurns, + hit: Math.abs(actual - open.predictedTurns) <= TOLERANCE, }); } @@ -98,7 +172,7 @@ export function observeOutcome(dir, { sessionId, actualTurns }) { * actionable, where "62% accurate" is not. */ export function reliability(dir) { - const outcomes = readMetrics(dir).filter((e) => e.kind === 'forecast-outcome'); + const outcomes = readBalance(dir).filter((e) => e.kind === 'forecast-outcome'); const buckets = {}; for (const horizon of HORIZONS) { @@ -164,7 +238,28 @@ export function calibrate(dir, predictedTurns) { // Refit: shift by the observed bias for this horizon. A bucket that runs long // gets pulled in, and vice versa. - const adjusted = Math.max(0, Math.round(predictedTurns + bucket.bias)); + // + // NOT CLAMPED WITH Math.max, which converted nonsense into a plausible-looking emergency. bias + // is a plain arithmetic mean of signed error, and a mean is not robust to the shape the + // 50%-hit-rate floor admits: a near bucket with errors [0,0,0,0,-30,-30,-30,-30] has four hits, + // so it clears the floor and is called calibrated, and its bias is -15. A raw forecast of 3 + // turns then became Math.max(0, Math.round(3 - 15)) = 0 and was published as fact, with the + // note "within 3 on 50% of 8 past forecasts" -- a zero-turn deadline the forecaster never + // produced, from a bucket that is wrong half the time. + // + // A correction at least as large as the thing it corrects is not a correction. + const adjusted = Math.round(predictedTurns + bucket.bias); + if (Math.abs(bucket.bias) >= predictedTurns || adjusted < 1) { + return { + publishable: false, + predictedTurns, + horizon, + bias: bucket.bias, + reason: `the observed bias at this horizon (${Math.round(bucket.bias)} turns) is as large as ` + + `the forecast itself (${predictedTurns}), so the correction would replace it rather than ` + + 'adjust it', + }; + } return { publishable: true, diff --git a/integrations/gemini/hooks/lib/forecast.mjs b/integrations/gemini/hooks/lib/forecast.mjs index aed24df29..474ed84f9 100644 --- a/integrations/gemini/hooks/lib/forecast.mjs +++ b/integrations/gemini/hooks/lib/forecast.mjs @@ -31,13 +31,30 @@ * other rather than one standing in for the other. */ -import { readMetrics } from './metrics.mjs'; +import { readMetrics, readBalance, isFixtureAnchor, BALANCE_KINDS } from './metrics.mjs'; import { aggregateConsolidation } from './consolidate.mjs'; import { previewQuality } from './expand.mjs'; +import { calibrate } from './calibration.mjs'; /** Turns of headroom below which the runway is worth interrupting for. */ export const ACTIONABLE_RUNWAY = 8; +/** + * Held-out touches required before the runway counterfactual is published. + * + * The header of this file promises the counterfactual appears "only once the holdout carries + * volume", and the fallback string says "the control arm needs more volume" -- but the only check + * was `if (!rows.length) return null`, so "volume" meant "at least one". With a single withheld + * touch, withheldCost IS that one touch's downstream read total: one held-out read of a + * 3,000-line module set savedPerTouch to several thousand tokens and published a counterfactual + * an order of magnitude off, as a bare fact with no caveat. + * + * Ten is a floor rather than a statistically derived threshold, and is deliberately stated as + * such where it surfaces: the panel prints the held-out count against it, so a reader can see how + * close the arm is instead of being told only that it is short. + */ +export const MIN_CONTROL_TOUCHES = 10; + /** * Tokens per turn, and how many the graph is removing. * @@ -57,13 +74,42 @@ export function burnRate(events) { reads.get(key).push(event); } + // EACH READ IS CHARGED TO EXACTLY ONE INJECTION. + // + // The windows used to run from each injection to the end of the log, so they overlapped + // completely: for anchor A with injections i1..i5 interleaved with five reads of 100 tokens, + // i1 saw 500, i2 400, i3 300, i4 200, i5 100 -- 1500 / 5 = 300 against a true per-touch cost of + // 100. A 3x inflation growing with the repeat count. + // + // It does not cancel between the arms. metrics.mjs makes the holdout deterministic in + // (anchor, epoch), so every repeat touch of a file lands in the SAME arm -- whichever arm drew + // the heavily re-touched anchors is inflated and the other is not. savedPerTouch's sign and + // magnitude were therefore driven by which files happened to be held out, and that value is + // what the published counterfactual divides by. + // + // Bounding each window at the next injection of the same (session, anchor) -- across BOTH arms, + // since an injection ends the previous window regardless of which arm it is in -- makes the + // charge exact. + const nextInjectAfter = new Map(); + for (const row of injects) { + const key = `${row.sessionId || ''}|${row.anchor}`; + if (!nextInjectAfter.has(key)) nextInjectAfter.set(key, []); + nextInjectAfter.get(key).push(row.at ?? 0); + } + for (const times of nextInjectAfter.values()) times.sort((a, b) => a - b); + const downstream = (rows) => { if (!rows.length) return null; let total = 0; for (const row of rows) { - const bucket = reads.get(`${row.sessionId || ''}|${row.anchor}`) || []; + const key = `${row.sessionId || ''}|${row.anchor}`; + const bucket = reads.get(key) || []; const after = row.at ?? 0; - total += bucket.reduce((sum, r) => sum + ((r.at ?? 0) >= after ? (r.tokens || 0) : 0), 0); + const until = (nextInjectAfter.get(key) || []).find((t) => t > after) ?? Infinity; + total += bucket.reduce( + (sum, r) => sum + ((r.at ?? 0) >= after && (r.at ?? 0) < until ? (r.tokens || 0) : 0), + 0, + ); } return total / rows.length; }; @@ -98,12 +144,27 @@ export function burnRate(events) { export function shadowAvoided(events) { let avoided = 0; let spent = 0; + let seen = 0; for (const event of events) { if (event.kind !== 'substitute') continue; + // FIXTURES EXCLUDED, as balanceSheet already does. metrics.mjs records the measurement: + // "366 of 370 substitutions pointed at the enforcement suite's own big.ts fixture under a + // temp dir, and counting them made the product look like it had avoided 40 MB of reads. It + // had avoided 154 KB." Running the test suite against a real graph inflated `avoided` by two + // orders of magnitude, which blew up shadow.net, pushed `spread` past 0.5 and fired the + // divergence note with a fabricated number -- so the panel's own credibility check was the + // thing the fixture noise corrupted. + if (isFixtureAnchor(event.anchor)) continue; + seen += 1; avoided += Math.ceil((event.bytesAvoided || 0) / 4); spent += event.tokens || 0; } - return avoided ? { avoided, spent, net: avoided - spent } : null; + // GUARDED ON WHETHER ANYTHING WAS SEEN, not on whether it was favourable. Guarding on `avoided` + // meant twenty substitutions that each spent 20 tokens and avoided nothing -- a measured net + // loss of 400 -- returned null, so the divergence block never ran and the panel said nothing + // about the shadow arm. The one outcome the function could not express was the one unfavourable + // to the product, collapsed into the same return value as "no data". + return seen ? { avoided, spent, net: avoided - spent, substitutions: seen } : null; } /** @@ -112,8 +173,10 @@ export function shadowAvoided(events) { * @param used Tokens of context consumed so far. * @param capacity Context window size. * @param turns Turns taken to consume it. + * @param touches Intercepted touches in this session. Required for the counterfactual: it is + * what converts a per-TOUCH saving into a per-TURN one. */ -export function runway({ used, capacity, turns }, rate) { +export function runway({ used, capacity, turns, touches }, rate) { if (!capacity || !turns || used == null) return null; const perTurn = used / turns; @@ -121,15 +184,53 @@ export function runway({ used, capacity, turns }, rate) { const remaining = Math.max(0, capacity - used); const withGraph = Math.floor(remaining / perTurn); + const base = { withGraph, perTurn: Math.round(perTurn) }; - // Without the graph each turn costs what it costs now PLUS what the graph is - // removing. Only computable when the control arm has spoken. const saved = rate?.savedPerTouch; - const withoutGraph = saved != null && saved > 0 - ? Math.floor(remaining / (perTurn + saved)) - : null; + if (saved == null) return { ...base, withoutGraph: null, counterfactual: 'unmeasured' }; + + // VOLUME, MEANING VOLUME. See MIN_CONTROL_TOUCHES. + const withheld = rate.withheldTouches ?? 0; + if (withheld < MIN_CONTROL_TOUCHES) { + return { ...base, withoutGraph: null, counterfactual: 'thin-control', withheld }; + } + + // UNITS. `perTurn` is tokens per TURN for this session; `savedPerTouch` is an average over + // injection EVENTS across the whole metrics log. Adding them directly was only correct with + // exactly one intercepted touch per turn, which is not the usual case in either direction. + // + // Session {used 60000, capacity 200000, turns 30} gives perTurn 2000 and remaining 140000. With + // 5 injections over those 30 turns and savedPerTouch 1500, the true per-turn saving is 250, so + // the honest counterfactual is floor(140000/2250) = 62. Adding the per-touch figure gave + // floor(140000/3500) = 40, and the panel published "~70 turns to compaction; without the graph, + // ~40" -- claiming the graph nearly doubled the runway when it added about eight turns. + // + // Without a touch count there is no density to convert with, so the counterfactual is withheld + // rather than computed on the one-touch-per-turn assumption. + if (!touches) return { ...base, withoutGraph: null, counterfactual: 'no-touch-density' }; + + const savedPerTurn = saved * (touches / turns); + + // A MEASURED NON-POSITIVE SAVING IS A RESULT, NOT A MISSING ONE. The old `saved > 0` guard + // discarded a measurement: with withheldCost 1000, treatedCost 900 and injectionCost 200, + // savedPerTouch is -100 -- the control arm has spoken and said the graph is a net cost -- and + // the panel printed "no counterfactual yet, the control arm needs more volume". The one result + // unfavourable to the product was the one result the panel could not show, disguised as + // insufficient data. The runway is longer without the graph in that case, and says so. + const denominator = perTurn + savedPerTurn; + if (denominator <= 0) { + // The graph is measured to be removing more per turn than the session is spending, which is + // arithmetically possible and not interpretable as a runway. Reported as such. + return { ...base, withoutGraph: null, counterfactual: 'implausible', savedPerTurn }; + } - return { withGraph, withoutGraph, perTurn: Math.round(perTurn) }; + return { + ...base, + withoutGraph: Math.floor(remaining / denominator), + counterfactual: savedPerTurn > 0 ? 'saving' : 'costing', + savedPerTurn, + withheld, + }; } /** @@ -141,21 +242,99 @@ export function runway({ used, capacity, turns }, rate) { * confident figure derived from four samples -- is precisely what this project * criticises elsewhere. */ +/** + * The events the forecast reasons about, each kind read from the log that keeps it. + * + * Reads come from readMetrics, which applies MAX_EVENTS and a tail byte cap -- correct, because + * reads are the high-volume kind and only the recent ones describe the current burn. + * + * Inject, harvest and substitute come from readBalance, which is exempt from that window and + * exists for precisely this reason. metrics.mjs records what the window does to them, measured in + * this repository: "44 inject records in the file, 9 of them holdout, all at lines 60-76 of + * 9,058 -- every single one outside the window. report() therefore said 0 holdout." + * + * The 10% holdout arm is the rarer kind, so it ages out FIRST. On any graph with more than 5,000 + * events since the last injection burst, downstream(withheld) returned null, savedPerTouch was + * null, and the panel printed "no counterfactual yet -- the control arm needs more volume" while + * balance.jsonl held the control arm in full. The panel blamed insufficient data for a read-path + * bug; in the partial case it computed the published counterfactual from whichever slice of the + * control arm happened to survive the window. + */ +export function balanceAwareEvents(dir) { + const balance = readBalance(dir); + // No id-matching needed, and deliberately so: readBalance already reads balance.jsonl in full, + // migrates the pre-split records out of metrics.jsonl WITHOUT the window, and dedupes the two + // on record id (falling back to a composite for records written before ids existed). It is + // therefore a strict superset of the balance kinds in the windowed read, so the windowed copies + // are dropped outright. Trying to merge them back by id would double-count exactly the legacy + // records that have no id. + const out = readMetrics(dir).filter((e) => !BALANCE_KINDS.has(e?.kind)); + out.push(...balance); + return out; +} + +/** How the runway reads, given what the control arm was actually able to say. */ +function runwayLine(air) { + const head = `~${air.withGraph} turns to compaction`; + switch (air.counterfactual) { + case 'saving': + return `${head}; without the graph, ~${air.withoutGraph}.`; + case 'costing': + // Said plainly rather than suppressed. A tool that can only report results in its own + // favour is not measuring anything. + return `${head}; without the graph, ~${air.withoutGraph} -- the graph is measurably NOT ` + + 'extending the runway here.'; + case 'thin-control': + return `${head} (no counterfactual yet -- the control arm holds ${air.withheld} of the ` + + `${MIN_CONTROL_TOUCHES} held-out touches needed).`; + case 'no-touch-density': + return `${head} (no counterfactual -- this session's intercepted-touch count is unknown, ` + + 'and a per-touch saving cannot be converted to a per-turn one without it).'; + case 'implausible': + return `${head} (no counterfactual -- the measured saving exceeds this session's whole ` + + 'per-turn cost, so the arithmetic does not describe a runway).'; + default: + return `${head} (no counterfactual yet -- the control arm has not spoken).`; + } +} + export function forecastPanel(dir, session = {}, findings = []) { - const events = readMetrics(dir); + const events = balanceAwareEvents(dir); const rate = burnRate(events); const shadow = shadowAvoided(events); const consolidation = aggregateConsolidation(findings); - const air = runway(session, rate); + // The touch count converts the per-touch saving into a per-turn one. Taken from the session + // when the caller knows it, and otherwise from the treated touches this rate was built from. + const touches = session.touches ?? rate?.treatedTouches ?? 0; + const air = runway({ ...session, touches }, rate); const lines = []; const parts = {}; if (air) { parts.runway = air; - lines.push(air.withoutGraph != null - ? `~${air.withGraph} turns to compaction; without the graph, ~${air.withoutGraph}.` - : `~${air.withGraph} turns to compaction (no counterfactual yet -- the control arm needs more volume).`); + + // THE FORECAST KEEPS ITS OWN SCORE -- from here, because this is the only place a forecast is + // actually made. calibration.mjs was written for exactly this and had zero shipped importers: + // no forecast was ever logged, no outcome ever observed, reliability always saw an empty set, + // and calibrate was unreachable. So the shipped panel printed precisely the uncalibrated + // number that module's docstring calls "a vibe with a typeface". + // + // NOT LOGGED HERE. Building a panel is not making a forecast. maybeSurface builds one per + // throttle window and worthSurfacing rejects most of them, so logging on every build appended + // an open forecast record nobody ever saw. observeOutcome closes only the LAST open forecast + // for a session, so every earlier one stayed open forever -- and they accumulate in + // balance.jsonl, whose tail-bytes read is precisely what would then displace the + // inject/harvest/substitute rows BALANCE_KINDS exists to protect. That would have undone this + // change's own fix. The surfacing path logs instead, at the one moment a prediction is + // actually made to somebody. + const score = calibrate(dir, air.withGraph); + parts.calibration = score; + // Published only when the horizon has earned it. An unpublishable score does not replace the + // number -- it is simply absent, exactly as the uncalibrated case reads today. + lines.push(score.publishable + ? `${runwayLine({ ...air, withGraph: score.predictedTurns })}\n (corrected: ${score.note})` + : runwayLine(air)); } if (rate) { diff --git a/integrations/gemini/hooks/lib/metrics.mjs b/integrations/gemini/hooks/lib/metrics.mjs index 3fc52e22b..4b0eb2712 100644 --- a/integrations/gemini/hooks/lib/metrics.mjs +++ b/integrations/gemini/hooks/lib/metrics.mjs @@ -63,7 +63,22 @@ const metricsPath = (dir) => join(dir, 'metrics.jsonl'); const balancePath = (dir) => join(dir, 'balance.jsonl'); /** Kinds the net balance is computed from, and which therefore must survive. */ -const BALANCE_KINDS = new Set(['inject', 'harvest', 'substitute']); +/** + * Kinds written to balance.jsonl as well as the firehose, so the event window can never + * starve the measurement. Exported because any reader that needs the unwindowed truth needs to + * know which kinds it applies to, and a second copy of this set elsewhere would drift. + */ +export const BALANCE_KINDS = new Set([ + 'inject', + 'harvest', + 'substitute', + // The forecast's own score. Both kinds are rare and accumulate slowly -- MIN_SCORED is 8 -- + // while the firehose they shared is dominated by per-tool-call records, so they were evicted + // faster than they could accumulate: calibrate() returned 'not yet calibrated (n/8)' forever + // and an outcome whose forecast had scrolled away was dropped with no record at all. + 'forecast', + 'forecast-outcome', +]); /** * Is this touch in the holdout arm? diff --git a/integrations/gemini/hooks/lib/policy.mjs b/integrations/gemini/hooks/lib/policy.mjs index b3871c27d..f89a48210 100644 --- a/integrations/gemini/hooks/lib/policy.mjs +++ b/integrations/gemini/hooks/lib/policy.mjs @@ -272,7 +272,7 @@ function statePath(sessionId, agent) { /** A usable state object, whatever was on disk. */ function emptyState() { - return { seen: {}, denied: {}, injected: [], actCounts: {} }; + return { seen: {}, denied: {}, injected: [], actCounts: {}, forecast: null }; } /** @@ -312,6 +312,22 @@ export function loadState(sessionId, agent) { parsed.actCounts && typeof parsed.actCounts === 'object' && !Array.isArray(parsed.actCounts) ? parsed.actCounts : {}, + // THE SAME REASON A THIRD TIME. The forecast throttle records when the panel was last + // computed and what runway it last SHOWED, and both are meaningless within a single hook + // process -- every tool call is a new one. Dropped here, `checkedAt` would reset on every + // call and the panel would rebuild itself, transcript parse and all, on each tool use; and + // `shown` would reset, so worthSurfacing would compare against nothing and re-interrupt with + // the same runway forever. + // AND ITS TIMESTAMP MUST BE A NUMBER. The shape check alone accepted `checkedAt: "1700"`, + // and both the throttle comparison and the merge below order by it -- so a persisted numeric + // STRING compares by lexicographic coercion and can beat a later real timestamp, freezing + // the throttle open or shut. Same class as every other field validated here: a value that + // parsed is not a value that means anything. + forecast: + parsed.forecast && typeof parsed.forecast === 'object' && !Array.isArray(parsed.forecast) + && Number.isFinite(parsed.forecast.checkedAt) + ? parsed.forecast + : null, }; } catch { return emptyState(); @@ -383,6 +399,19 @@ export function saveState(sessionId, state, agent) { } return out; })(), + // LATEST CHECK WINS, which is the opposite direction from the fields above and correct for + // this one. `seen`, `denied`, `injected` and `actCounts` are all append-only, so a union or + // a max is the safe merge. The forecast throttle is a POINT IN TIME: taking the older of two + // concurrent checks would re-open the window and let the panel be rebuilt immediately, which + // is the cost the throttle exists to bound. + forecast: (() => { + const mine = state.forecast || null; + const theirs = current.forecast || null; + const stamp = (f) => (Number.isFinite(f?.checkedAt) ? f.checkedAt : null); + if (stamp(mine) === null) return theirs; + if (stamp(theirs) === null) return mine; + return stamp(mine) >= stamp(theirs) ? mine : theirs; + })(), }; // Write-then-rename so a reader never observes a half-written file. diff --git a/integrations/gemini/hooks/lib/surface.mjs b/integrations/gemini/hooks/lib/surface.mjs new file mode 100644 index 000000000..4054c8cf9 --- /dev/null +++ b/integrations/gemini/hooks/lib/surface.mjs @@ -0,0 +1,185 @@ +// GENERATED FILE -- do not edit. +// Source of truth: hooks-core/surface.mjs. Regenerate with `npm run sync:hooks`. +/** + * The entry point the forecast never had. + * + * forecast.mjs computes a runway, calibration.mjs scores it, and worthSurfacing decides whether a + * change is worth interrupting for -- and none of it ran. The reachability guard listed + * forecastPanel, worthSurfacing, logForecast and observeOutcome as unwired, which is a polite way + * of saying the feature did not exist for any user. This module is the missing half: it derives + * the session numbers the panel needs from the transcript, decides when the panel has earned an + * interruption, and closes the calibration loop when compaction fires. + * + * THREE COSTS ARE BOUNDED HERE, because this runs on the PreToolUse path and every tool call is a + * separate process: + * + * THE READ readCacheUsage parses up to 4 MB of transcript. Doing that on every tool call + * would tax the hook that exists to save the user money. + * THE COMPUTE the panel walks the metrics log, the balance log and the graph. + * THE CONTEXT the panel is only worth its own tokens when it says something actionable. + * + * So the throttle is checked FIRST, from persisted state, before any file is opened. Everything + * expensive happens after a clock comparison. + * + * NOTHING HERE IS ALLOWED TO BREAK A TOOL CALL. Every entry point returns null on failure rather + * than throwing: a forecast is a courtesy, and the hook's promise is that a defect in it costs the + * user nothing. + */ + +import { forecastPanel, worthSurfacing } from './forecast.mjs'; +import { observeOutcome, logForecast } from './calibration.mjs'; +import { readCacheUsage } from './cache.mjs'; +import { readBalance } from './metrics.mjs'; + +/** + * How long between two attempts to build the panel. + * + * Not a tuning knob so much as an admission: the panel costs a transcript parse, and a user makes + * many tool calls per minute. Two minutes is short enough that a runway falling into single digits + * is still caught with turns to spare, and long enough that the check is free in aggregate. + */ +export const SURFACE_INTERVAL_MS = + Number(process.env.TOKEN_OPTIMIZER_SURFACE_INTERVAL_MS) || 120_000; + +/** + * The context window to measure the runway against. + * + * NOT DISCOVERABLE from the transcript: the usage rows carry what was spent, never the ceiling it + * was spent against. Defaulting is therefore unavoidable, so the default is stated rather than + * buried, and overridable for anyone running a different window. + */ +export const DEFAULT_CAPACITY = + Number(process.env.TOKEN_OPTIMIZER_CONTEXT_CAPACITY) || 200_000; + +/** + * What the session has spent, read from its own transcript. + * + * `used` is the FULL input size of the most recent request -- cache reads plus cache writes plus + * fresh input -- which is what the context window actually holds, rather than the marginal cost of + * that one turn. Summing the per-turn inputs instead would count the same carried prefix once per + * turn and report a number many times the window size. + * + * Returns null rather than a zeroed object when there is nothing to measure, so a caller cannot + * mistake "no data" for "an empty context". + */ +export function sessionUsage(transcriptPath, { capacity = DEFAULT_CAPACITY } = {}) { + let turns; + try { + turns = readCacheUsage(transcriptPath); + } catch { + return null; + } + if (!turns?.length) return null; + + const last = turns[turns.length - 1]; + const used = (last.read || 0) + (last.written || 0) + (last.input || 0); + if (!used) return null; + + return { used, capacity, turns: turns.length }; +} + +/** Intercepted touches recorded for this session, which converts a per-touch saving to per-turn. */ +function touchesFor(dir, sessionId) { + try { + return readBalance(dir).filter((e) => e.kind === 'inject' && e.sessionId === sessionId).length; + } catch { + return 0; + } +} + +/** + * Should the forecast interrupt right now, and with what? + * + * The state object is the caller's persisted per-session state; this reads `forecast` from it and + * returns the value to store back, so the throttle and the last-shown runway survive across hook + * processes. Returning the next state rather than mutating it keeps this testable without a + * filesystem. + * + * @returns { text, state } -- text is null when nothing should be shown. + */ +export function maybeSurface(dir, { + transcriptPath, sessionId, state = {}, now = Date.now(), findings = [], +} = {}) { + const previous = state.forecast || null; + + // THE THROTTLE IS FIRST, and deliberately before any I/O. Everything below opens files. + if (previous?.checkedAt && now - previous.checkedAt < SURFACE_INTERVAL_MS) { + return { text: null, state: previous }; + } + + let usage; + try { + usage = sessionUsage(transcriptPath); + } catch { + return { text: null, state: previous }; + } + // The clock is stamped even when there is nothing to measure, so a session with no usage rows + // does not re-attempt the transcript parse on every single tool call. + if (!usage) return { text: null, state: { ...previous, checkedAt: now } }; + + let panel; + try { + panel = forecastPanel(dir, { + ...usage, + sessionId, + touches: touchesFor(dir, sessionId), + }, findings); + } catch { + return { text: null, state: { ...previous, checkedAt: now } }; + } + if (!panel) return { text: null, state: { ...previous, checkedAt: now } }; + + // worthSurfacing compares against the last panel that was actually SHOWN, not the last one + // computed. Comparing against the last computed panel would let the runway drift down past the + // threshold one throttle window at a time and never trip the "crossed it" test. + const shown = previous?.shown ? { parts: { runway: { withGraph: previous.shown } } } : null; + if (!worthSurfacing(panel, shown)) { + return { text: null, state: { ...previous, checkedAt: now } }; + } + + // LOGGED HERE, AND ONLY HERE -- past the throttle and past worthSurfacing, so exactly one record + // is written per prediction a person is actually shown. forecastPanel used to log on every build; + // since most builds are rejected above, that appended open forecast records nobody saw, and + // observeOutcome closes only the newest one per session. The rest stayed open forever and piled + // up in balance.jsonl, whose tail-bytes read would then evict the inject/harvest/substitute rows + // BALANCE_KINDS protects -- undoing the very repair this work exists to make. + // + // The RAW runway is logged, not the calibrated one: scoring a corrected number would fold each + // correction into the next and the bias would chase its own tail. + try { + logForecast(dir, { + sessionId, + predictedTurns: panel.parts.runway.withGraph, + used: usage.used, + capacity: usage.capacity, + turns: usage.turns, + }); + } catch { /* a forecast is a courtesy; failing to score it must not cost a tool call */ } + + return { + text: panel.text, + state: { checkedAt: now, shown: panel.parts.runway.withGraph }, + }; +} + +/** + * Closes the calibration loop, at the only moment that can close it. + * + * Compaction firing IS the ground truth a runway forecast was predicting, so this belongs on the + * PreCompact path and nowhere else. `actualTurns` is turns ELAPSED between the prediction and + * compaction, which is what predictedTurns meant -- so the forecast has to have recorded the turn + * it was made on, and observeOutcome subtracts. + * + * Silent by design: nothing is shown to anybody. It writes one record so that the NEXT session's + * forecast can be published with a track record instead of as a bare assertion. + */ +export function closeForecast(dir, { transcriptPath, sessionId } = {}) { + try { + const usage = sessionUsage(transcriptPath); + if (!usage) return false; + observeOutcome(dir, { sessionId, atTurn: usage.turns }); + return true; + } catch { + return false; + } +} diff --git a/integrations/opencode/hooks/lib/calibration.mjs b/integrations/opencode/hooks/lib/calibration.mjs index 718db89cd..a126fcaef 100644 --- a/integrations/opencode/hooks/lib/calibration.mjs +++ b/integrations/opencode/hooks/lib/calibration.mjs @@ -31,7 +31,20 @@ * that cannot support one. */ -import { record, readMetrics } from './metrics.mjs'; +// readBalance, not readMetrics. Both readers used the windowed firehose, which returns at most +// 2,000,000 bytes and the last 5,000 lines -- and forecast events were not among the protected +// kinds, so they lived in a log dominated by per-tool-call records (metrics.mjs's own comment +// cites 4,735 capture events inside one window). +// +// Two failures followed. In observeOutcome, on a busy project the forecast had scrolled past the +// tail by the time compaction fired, so the outcome was discarded with no record, no error and no +// counter. In reliability, outcomes vanished faster than they accumulated, so rows.length could +// never reach MIN_SCORED and calibrate returned 'not yet calibrated (n/8)' permanently -- leaving +// a user unable to distinguish "never had data" from "the data is being thrown away". +// +// 'forecast' and 'forecast-outcome' are now balance kinds, so they are written to balance.jsonl +// as well and read back unwindowed. +import { record, readBalance } from './metrics.mjs'; /** Horizon buckets, in turns. Error behaves differently across these ranges. */ const HORIZONS = [ @@ -53,8 +66,15 @@ function horizonOf(turns) { return HORIZONS.find((h) => turns <= h.max).name; } -/** Records a prediction so it can be scored when the outcome arrives. */ -export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) { +/** + * Records a prediction so it can be scored when the outcome arrives. + * + * `turns` is the session's turn count AT PREDICTION TIME, and it is what makes the outcome + * computable at all: predictedTurns means "turns from here until compaction", so the ground truth + * is the turns that ELAPSED, not the turn number compaction happened on. Without it the caller at + * compaction has nothing to subtract from and would have to score an interval against an absolute. + */ +export function logForecast(dir, { sessionId, predictedTurns, used, capacity, turns }) { if (!Number.isFinite(predictedTurns)) return; record(dir, { kind: 'forecast', @@ -63,6 +83,7 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) horizon: horizonOf(predictedTurns), used, capacity, + turns: Number.isFinite(turns) ? turns : null, }); } @@ -73,21 +94,74 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) * Called when compaction fires: the number of turns that elapsed since the * prediction is the ground truth it was predicting. */ -export function observeOutcome(dir, { sessionId, actualTurns }) { - if (!Number.isFinite(actualTurns)) return; - const open = readMetrics(dir) - .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && !e.scored) +export function observeOutcome(dir, { sessionId, actualTurns, atTurn }) { + // EITHER THE ELAPSED COUNT OR THE TURN IT HAPPENED ON. + // + // The only real caller -- the PreCompact hook -- knows what turn compaction fired on, not how + // many turns have passed since a prediction it did not make. Deriving the interval here keeps + // that subtraction next to the record that defines it, rather than making every caller + // re-discover which forecast is open in order to compute its own ground truth. + const elapsed = Number.isFinite(actualTurns) + ? actualTurns + : null; + if (elapsed === null && !Number.isFinite(atTurn)) return; + + // CLOSURE IS DERIVED FROM THE OUTCOME ROWS, not from a flag on the forecast. + // + // The old filter was `!e.scored`, but nothing ever writes `scored`: logForecast records + // kind/sessionId/predictedTurns/horizon/used/capacity, and record() is an append-only JSONL + // writer with no update path, so a forecast can never gain the field. The predicate was always + // true, and the last forecast for a session was therefore re-closable without limit. + // + // A session where compaction fires twice with no intervening logForecast popped the SAME + // forecast twice, producing two outcome rows for one prediction. reliability counts ROWS, not + // predictions, so the hit rate and the bias mean both inflate and MIN_SCORED can be satisfied + // by a single prediction re-closed eight times -- precisely the "threshold from a single + // sample" this module exists to prevent. The existing test used a fresh sessionId per + // iteration, so it never exposed it. + const events = readBalance(dir); + const closed = new Set( + events.filter((e) => e.kind === 'forecast-outcome' && e.forecastId).map((e) => e.forecastId), + ); + // ID-BEARING ROWS ONLY, AND NEWEST BY TIME RATHER THAN BY POSITION. + // + // Two separate hazards, both from readBalance's shape. It returns balance-log rows followed by + // the legacy firehose rows it migrates, unsorted -- so `.pop()` returns the last row in the + // array, which is not the newest forecast. And legacy rows predate `id`, so `closed.has(e.id)` + // is `closed.has(undefined)`, always false: such a forecast can be closed again and again, and + // each outcome stores `forecastId: undefined`, which JSON.stringify drops entirely. That is the + // repeat-scoring defect this function was written to fix, reappearing through the back door. + const open = events + .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && e.id && !closed.has(e.id)) + .sort((a, b) => (a.at ?? 0) - (b.at ?? 0)) .pop(); if (!open) return; + // Derived only once the open forecast is known, since the interval is measured from ITS turn. + // A forecast recorded before `turns` existed has none, and there is nothing to subtract from -- + // scoring it against an absolute turn number would manufacture an error rather than measure one, + // so it is left open instead. + const actual = elapsed !== null ? elapsed + : (Number.isFinite(open.turns) ? atTurn - open.turns : null); + // AT LEAST ONE TURN MUST HAVE PASSED. `atTurn` is the transcript's current turn count and + // `open.turns` the count at prediction time, so a truncated, rotated or replaced transcript + // makes the difference zero or negative. Recording that as ground truth feeds a large negative + // `error` into `reliability`, which shifts `bucket.bias`, which is what `calibrate` publishes a + // corrected forecast from -- so one bad transcript would skew every later prediction. Leaving + // the forecast open loses nothing: the next compaction closes it against a sane count. + if (actual === null || !Number.isFinite(actual) || actual < 1) return; + record(dir, { kind: 'forecast-outcome', sessionId, + // Stamped so the next call can see this forecast is spoken for. Without it the set above is + // empty and the same defect returns. + forecastId: open.id, horizon: open.horizon, predictedTurns: open.predictedTurns, - actualTurns, - error: actualTurns - open.predictedTurns, - hit: Math.abs(actualTurns - open.predictedTurns) <= TOLERANCE, + actualTurns: actual, + error: actual - open.predictedTurns, + hit: Math.abs(actual - open.predictedTurns) <= TOLERANCE, }); } @@ -98,7 +172,7 @@ export function observeOutcome(dir, { sessionId, actualTurns }) { * actionable, where "62% accurate" is not. */ export function reliability(dir) { - const outcomes = readMetrics(dir).filter((e) => e.kind === 'forecast-outcome'); + const outcomes = readBalance(dir).filter((e) => e.kind === 'forecast-outcome'); const buckets = {}; for (const horizon of HORIZONS) { @@ -164,7 +238,28 @@ export function calibrate(dir, predictedTurns) { // Refit: shift by the observed bias for this horizon. A bucket that runs long // gets pulled in, and vice versa. - const adjusted = Math.max(0, Math.round(predictedTurns + bucket.bias)); + // + // NOT CLAMPED WITH Math.max, which converted nonsense into a plausible-looking emergency. bias + // is a plain arithmetic mean of signed error, and a mean is not robust to the shape the + // 50%-hit-rate floor admits: a near bucket with errors [0,0,0,0,-30,-30,-30,-30] has four hits, + // so it clears the floor and is called calibrated, and its bias is -15. A raw forecast of 3 + // turns then became Math.max(0, Math.round(3 - 15)) = 0 and was published as fact, with the + // note "within 3 on 50% of 8 past forecasts" -- a zero-turn deadline the forecaster never + // produced, from a bucket that is wrong half the time. + // + // A correction at least as large as the thing it corrects is not a correction. + const adjusted = Math.round(predictedTurns + bucket.bias); + if (Math.abs(bucket.bias) >= predictedTurns || adjusted < 1) { + return { + publishable: false, + predictedTurns, + horizon, + bias: bucket.bias, + reason: `the observed bias at this horizon (${Math.round(bucket.bias)} turns) is as large as ` + + `the forecast itself (${predictedTurns}), so the correction would replace it rather than ` + + 'adjust it', + }; + } return { publishable: true, diff --git a/integrations/opencode/hooks/lib/forecast.mjs b/integrations/opencode/hooks/lib/forecast.mjs index aed24df29..474ed84f9 100644 --- a/integrations/opencode/hooks/lib/forecast.mjs +++ b/integrations/opencode/hooks/lib/forecast.mjs @@ -31,13 +31,30 @@ * other rather than one standing in for the other. */ -import { readMetrics } from './metrics.mjs'; +import { readMetrics, readBalance, isFixtureAnchor, BALANCE_KINDS } from './metrics.mjs'; import { aggregateConsolidation } from './consolidate.mjs'; import { previewQuality } from './expand.mjs'; +import { calibrate } from './calibration.mjs'; /** Turns of headroom below which the runway is worth interrupting for. */ export const ACTIONABLE_RUNWAY = 8; +/** + * Held-out touches required before the runway counterfactual is published. + * + * The header of this file promises the counterfactual appears "only once the holdout carries + * volume", and the fallback string says "the control arm needs more volume" -- but the only check + * was `if (!rows.length) return null`, so "volume" meant "at least one". With a single withheld + * touch, withheldCost IS that one touch's downstream read total: one held-out read of a + * 3,000-line module set savedPerTouch to several thousand tokens and published a counterfactual + * an order of magnitude off, as a bare fact with no caveat. + * + * Ten is a floor rather than a statistically derived threshold, and is deliberately stated as + * such where it surfaces: the panel prints the held-out count against it, so a reader can see how + * close the arm is instead of being told only that it is short. + */ +export const MIN_CONTROL_TOUCHES = 10; + /** * Tokens per turn, and how many the graph is removing. * @@ -57,13 +74,42 @@ export function burnRate(events) { reads.get(key).push(event); } + // EACH READ IS CHARGED TO EXACTLY ONE INJECTION. + // + // The windows used to run from each injection to the end of the log, so they overlapped + // completely: for anchor A with injections i1..i5 interleaved with five reads of 100 tokens, + // i1 saw 500, i2 400, i3 300, i4 200, i5 100 -- 1500 / 5 = 300 against a true per-touch cost of + // 100. A 3x inflation growing with the repeat count. + // + // It does not cancel between the arms. metrics.mjs makes the holdout deterministic in + // (anchor, epoch), so every repeat touch of a file lands in the SAME arm -- whichever arm drew + // the heavily re-touched anchors is inflated and the other is not. savedPerTouch's sign and + // magnitude were therefore driven by which files happened to be held out, and that value is + // what the published counterfactual divides by. + // + // Bounding each window at the next injection of the same (session, anchor) -- across BOTH arms, + // since an injection ends the previous window regardless of which arm it is in -- makes the + // charge exact. + const nextInjectAfter = new Map(); + for (const row of injects) { + const key = `${row.sessionId || ''}|${row.anchor}`; + if (!nextInjectAfter.has(key)) nextInjectAfter.set(key, []); + nextInjectAfter.get(key).push(row.at ?? 0); + } + for (const times of nextInjectAfter.values()) times.sort((a, b) => a - b); + const downstream = (rows) => { if (!rows.length) return null; let total = 0; for (const row of rows) { - const bucket = reads.get(`${row.sessionId || ''}|${row.anchor}`) || []; + const key = `${row.sessionId || ''}|${row.anchor}`; + const bucket = reads.get(key) || []; const after = row.at ?? 0; - total += bucket.reduce((sum, r) => sum + ((r.at ?? 0) >= after ? (r.tokens || 0) : 0), 0); + const until = (nextInjectAfter.get(key) || []).find((t) => t > after) ?? Infinity; + total += bucket.reduce( + (sum, r) => sum + ((r.at ?? 0) >= after && (r.at ?? 0) < until ? (r.tokens || 0) : 0), + 0, + ); } return total / rows.length; }; @@ -98,12 +144,27 @@ export function burnRate(events) { export function shadowAvoided(events) { let avoided = 0; let spent = 0; + let seen = 0; for (const event of events) { if (event.kind !== 'substitute') continue; + // FIXTURES EXCLUDED, as balanceSheet already does. metrics.mjs records the measurement: + // "366 of 370 substitutions pointed at the enforcement suite's own big.ts fixture under a + // temp dir, and counting them made the product look like it had avoided 40 MB of reads. It + // had avoided 154 KB." Running the test suite against a real graph inflated `avoided` by two + // orders of magnitude, which blew up shadow.net, pushed `spread` past 0.5 and fired the + // divergence note with a fabricated number -- so the panel's own credibility check was the + // thing the fixture noise corrupted. + if (isFixtureAnchor(event.anchor)) continue; + seen += 1; avoided += Math.ceil((event.bytesAvoided || 0) / 4); spent += event.tokens || 0; } - return avoided ? { avoided, spent, net: avoided - spent } : null; + // GUARDED ON WHETHER ANYTHING WAS SEEN, not on whether it was favourable. Guarding on `avoided` + // meant twenty substitutions that each spent 20 tokens and avoided nothing -- a measured net + // loss of 400 -- returned null, so the divergence block never ran and the panel said nothing + // about the shadow arm. The one outcome the function could not express was the one unfavourable + // to the product, collapsed into the same return value as "no data". + return seen ? { avoided, spent, net: avoided - spent, substitutions: seen } : null; } /** @@ -112,8 +173,10 @@ export function shadowAvoided(events) { * @param used Tokens of context consumed so far. * @param capacity Context window size. * @param turns Turns taken to consume it. + * @param touches Intercepted touches in this session. Required for the counterfactual: it is + * what converts a per-TOUCH saving into a per-TURN one. */ -export function runway({ used, capacity, turns }, rate) { +export function runway({ used, capacity, turns, touches }, rate) { if (!capacity || !turns || used == null) return null; const perTurn = used / turns; @@ -121,15 +184,53 @@ export function runway({ used, capacity, turns }, rate) { const remaining = Math.max(0, capacity - used); const withGraph = Math.floor(remaining / perTurn); + const base = { withGraph, perTurn: Math.round(perTurn) }; - // Without the graph each turn costs what it costs now PLUS what the graph is - // removing. Only computable when the control arm has spoken. const saved = rate?.savedPerTouch; - const withoutGraph = saved != null && saved > 0 - ? Math.floor(remaining / (perTurn + saved)) - : null; + if (saved == null) return { ...base, withoutGraph: null, counterfactual: 'unmeasured' }; + + // VOLUME, MEANING VOLUME. See MIN_CONTROL_TOUCHES. + const withheld = rate.withheldTouches ?? 0; + if (withheld < MIN_CONTROL_TOUCHES) { + return { ...base, withoutGraph: null, counterfactual: 'thin-control', withheld }; + } + + // UNITS. `perTurn` is tokens per TURN for this session; `savedPerTouch` is an average over + // injection EVENTS across the whole metrics log. Adding them directly was only correct with + // exactly one intercepted touch per turn, which is not the usual case in either direction. + // + // Session {used 60000, capacity 200000, turns 30} gives perTurn 2000 and remaining 140000. With + // 5 injections over those 30 turns and savedPerTouch 1500, the true per-turn saving is 250, so + // the honest counterfactual is floor(140000/2250) = 62. Adding the per-touch figure gave + // floor(140000/3500) = 40, and the panel published "~70 turns to compaction; without the graph, + // ~40" -- claiming the graph nearly doubled the runway when it added about eight turns. + // + // Without a touch count there is no density to convert with, so the counterfactual is withheld + // rather than computed on the one-touch-per-turn assumption. + if (!touches) return { ...base, withoutGraph: null, counterfactual: 'no-touch-density' }; + + const savedPerTurn = saved * (touches / turns); + + // A MEASURED NON-POSITIVE SAVING IS A RESULT, NOT A MISSING ONE. The old `saved > 0` guard + // discarded a measurement: with withheldCost 1000, treatedCost 900 and injectionCost 200, + // savedPerTouch is -100 -- the control arm has spoken and said the graph is a net cost -- and + // the panel printed "no counterfactual yet, the control arm needs more volume". The one result + // unfavourable to the product was the one result the panel could not show, disguised as + // insufficient data. The runway is longer without the graph in that case, and says so. + const denominator = perTurn + savedPerTurn; + if (denominator <= 0) { + // The graph is measured to be removing more per turn than the session is spending, which is + // arithmetically possible and not interpretable as a runway. Reported as such. + return { ...base, withoutGraph: null, counterfactual: 'implausible', savedPerTurn }; + } - return { withGraph, withoutGraph, perTurn: Math.round(perTurn) }; + return { + ...base, + withoutGraph: Math.floor(remaining / denominator), + counterfactual: savedPerTurn > 0 ? 'saving' : 'costing', + savedPerTurn, + withheld, + }; } /** @@ -141,21 +242,99 @@ export function runway({ used, capacity, turns }, rate) { * confident figure derived from four samples -- is precisely what this project * criticises elsewhere. */ +/** + * The events the forecast reasons about, each kind read from the log that keeps it. + * + * Reads come from readMetrics, which applies MAX_EVENTS and a tail byte cap -- correct, because + * reads are the high-volume kind and only the recent ones describe the current burn. + * + * Inject, harvest and substitute come from readBalance, which is exempt from that window and + * exists for precisely this reason. metrics.mjs records what the window does to them, measured in + * this repository: "44 inject records in the file, 9 of them holdout, all at lines 60-76 of + * 9,058 -- every single one outside the window. report() therefore said 0 holdout." + * + * The 10% holdout arm is the rarer kind, so it ages out FIRST. On any graph with more than 5,000 + * events since the last injection burst, downstream(withheld) returned null, savedPerTouch was + * null, and the panel printed "no counterfactual yet -- the control arm needs more volume" while + * balance.jsonl held the control arm in full. The panel blamed insufficient data for a read-path + * bug; in the partial case it computed the published counterfactual from whichever slice of the + * control arm happened to survive the window. + */ +export function balanceAwareEvents(dir) { + const balance = readBalance(dir); + // No id-matching needed, and deliberately so: readBalance already reads balance.jsonl in full, + // migrates the pre-split records out of metrics.jsonl WITHOUT the window, and dedupes the two + // on record id (falling back to a composite for records written before ids existed). It is + // therefore a strict superset of the balance kinds in the windowed read, so the windowed copies + // are dropped outright. Trying to merge them back by id would double-count exactly the legacy + // records that have no id. + const out = readMetrics(dir).filter((e) => !BALANCE_KINDS.has(e?.kind)); + out.push(...balance); + return out; +} + +/** How the runway reads, given what the control arm was actually able to say. */ +function runwayLine(air) { + const head = `~${air.withGraph} turns to compaction`; + switch (air.counterfactual) { + case 'saving': + return `${head}; without the graph, ~${air.withoutGraph}.`; + case 'costing': + // Said plainly rather than suppressed. A tool that can only report results in its own + // favour is not measuring anything. + return `${head}; without the graph, ~${air.withoutGraph} -- the graph is measurably NOT ` + + 'extending the runway here.'; + case 'thin-control': + return `${head} (no counterfactual yet -- the control arm holds ${air.withheld} of the ` + + `${MIN_CONTROL_TOUCHES} held-out touches needed).`; + case 'no-touch-density': + return `${head} (no counterfactual -- this session's intercepted-touch count is unknown, ` + + 'and a per-touch saving cannot be converted to a per-turn one without it).'; + case 'implausible': + return `${head} (no counterfactual -- the measured saving exceeds this session's whole ` + + 'per-turn cost, so the arithmetic does not describe a runway).'; + default: + return `${head} (no counterfactual yet -- the control arm has not spoken).`; + } +} + export function forecastPanel(dir, session = {}, findings = []) { - const events = readMetrics(dir); + const events = balanceAwareEvents(dir); const rate = burnRate(events); const shadow = shadowAvoided(events); const consolidation = aggregateConsolidation(findings); - const air = runway(session, rate); + // The touch count converts the per-touch saving into a per-turn one. Taken from the session + // when the caller knows it, and otherwise from the treated touches this rate was built from. + const touches = session.touches ?? rate?.treatedTouches ?? 0; + const air = runway({ ...session, touches }, rate); const lines = []; const parts = {}; if (air) { parts.runway = air; - lines.push(air.withoutGraph != null - ? `~${air.withGraph} turns to compaction; without the graph, ~${air.withoutGraph}.` - : `~${air.withGraph} turns to compaction (no counterfactual yet -- the control arm needs more volume).`); + + // THE FORECAST KEEPS ITS OWN SCORE -- from here, because this is the only place a forecast is + // actually made. calibration.mjs was written for exactly this and had zero shipped importers: + // no forecast was ever logged, no outcome ever observed, reliability always saw an empty set, + // and calibrate was unreachable. So the shipped panel printed precisely the uncalibrated + // number that module's docstring calls "a vibe with a typeface". + // + // NOT LOGGED HERE. Building a panel is not making a forecast. maybeSurface builds one per + // throttle window and worthSurfacing rejects most of them, so logging on every build appended + // an open forecast record nobody ever saw. observeOutcome closes only the LAST open forecast + // for a session, so every earlier one stayed open forever -- and they accumulate in + // balance.jsonl, whose tail-bytes read is precisely what would then displace the + // inject/harvest/substitute rows BALANCE_KINDS exists to protect. That would have undone this + // change's own fix. The surfacing path logs instead, at the one moment a prediction is + // actually made to somebody. + const score = calibrate(dir, air.withGraph); + parts.calibration = score; + // Published only when the horizon has earned it. An unpublishable score does not replace the + // number -- it is simply absent, exactly as the uncalibrated case reads today. + lines.push(score.publishable + ? `${runwayLine({ ...air, withGraph: score.predictedTurns })}\n (corrected: ${score.note})` + : runwayLine(air)); } if (rate) { diff --git a/integrations/opencode/hooks/lib/metrics.mjs b/integrations/opencode/hooks/lib/metrics.mjs index 3fc52e22b..4b0eb2712 100644 --- a/integrations/opencode/hooks/lib/metrics.mjs +++ b/integrations/opencode/hooks/lib/metrics.mjs @@ -63,7 +63,22 @@ const metricsPath = (dir) => join(dir, 'metrics.jsonl'); const balancePath = (dir) => join(dir, 'balance.jsonl'); /** Kinds the net balance is computed from, and which therefore must survive. */ -const BALANCE_KINDS = new Set(['inject', 'harvest', 'substitute']); +/** + * Kinds written to balance.jsonl as well as the firehose, so the event window can never + * starve the measurement. Exported because any reader that needs the unwindowed truth needs to + * know which kinds it applies to, and a second copy of this set elsewhere would drift. + */ +export const BALANCE_KINDS = new Set([ + 'inject', + 'harvest', + 'substitute', + // The forecast's own score. Both kinds are rare and accumulate slowly -- MIN_SCORED is 8 -- + // while the firehose they shared is dominated by per-tool-call records, so they were evicted + // faster than they could accumulate: calibrate() returned 'not yet calibrated (n/8)' forever + // and an outcome whose forecast had scrolled away was dropped with no record at all. + 'forecast', + 'forecast-outcome', +]); /** * Is this touch in the holdout arm? diff --git a/integrations/opencode/hooks/lib/policy.mjs b/integrations/opencode/hooks/lib/policy.mjs index b3871c27d..f89a48210 100644 --- a/integrations/opencode/hooks/lib/policy.mjs +++ b/integrations/opencode/hooks/lib/policy.mjs @@ -272,7 +272,7 @@ function statePath(sessionId, agent) { /** A usable state object, whatever was on disk. */ function emptyState() { - return { seen: {}, denied: {}, injected: [], actCounts: {} }; + return { seen: {}, denied: {}, injected: [], actCounts: {}, forecast: null }; } /** @@ -312,6 +312,22 @@ export function loadState(sessionId, agent) { parsed.actCounts && typeof parsed.actCounts === 'object' && !Array.isArray(parsed.actCounts) ? parsed.actCounts : {}, + // THE SAME REASON A THIRD TIME. The forecast throttle records when the panel was last + // computed and what runway it last SHOWED, and both are meaningless within a single hook + // process -- every tool call is a new one. Dropped here, `checkedAt` would reset on every + // call and the panel would rebuild itself, transcript parse and all, on each tool use; and + // `shown` would reset, so worthSurfacing would compare against nothing and re-interrupt with + // the same runway forever. + // AND ITS TIMESTAMP MUST BE A NUMBER. The shape check alone accepted `checkedAt: "1700"`, + // and both the throttle comparison and the merge below order by it -- so a persisted numeric + // STRING compares by lexicographic coercion and can beat a later real timestamp, freezing + // the throttle open or shut. Same class as every other field validated here: a value that + // parsed is not a value that means anything. + forecast: + parsed.forecast && typeof parsed.forecast === 'object' && !Array.isArray(parsed.forecast) + && Number.isFinite(parsed.forecast.checkedAt) + ? parsed.forecast + : null, }; } catch { return emptyState(); @@ -383,6 +399,19 @@ export function saveState(sessionId, state, agent) { } return out; })(), + // LATEST CHECK WINS, which is the opposite direction from the fields above and correct for + // this one. `seen`, `denied`, `injected` and `actCounts` are all append-only, so a union or + // a max is the safe merge. The forecast throttle is a POINT IN TIME: taking the older of two + // concurrent checks would re-open the window and let the panel be rebuilt immediately, which + // is the cost the throttle exists to bound. + forecast: (() => { + const mine = state.forecast || null; + const theirs = current.forecast || null; + const stamp = (f) => (Number.isFinite(f?.checkedAt) ? f.checkedAt : null); + if (stamp(mine) === null) return theirs; + if (stamp(theirs) === null) return mine; + return stamp(mine) >= stamp(theirs) ? mine : theirs; + })(), }; // Write-then-rename so a reader never observes a half-written file. diff --git a/integrations/opencode/hooks/lib/surface.mjs b/integrations/opencode/hooks/lib/surface.mjs new file mode 100644 index 000000000..4054c8cf9 --- /dev/null +++ b/integrations/opencode/hooks/lib/surface.mjs @@ -0,0 +1,185 @@ +// GENERATED FILE -- do not edit. +// Source of truth: hooks-core/surface.mjs. Regenerate with `npm run sync:hooks`. +/** + * The entry point the forecast never had. + * + * forecast.mjs computes a runway, calibration.mjs scores it, and worthSurfacing decides whether a + * change is worth interrupting for -- and none of it ran. The reachability guard listed + * forecastPanel, worthSurfacing, logForecast and observeOutcome as unwired, which is a polite way + * of saying the feature did not exist for any user. This module is the missing half: it derives + * the session numbers the panel needs from the transcript, decides when the panel has earned an + * interruption, and closes the calibration loop when compaction fires. + * + * THREE COSTS ARE BOUNDED HERE, because this runs on the PreToolUse path and every tool call is a + * separate process: + * + * THE READ readCacheUsage parses up to 4 MB of transcript. Doing that on every tool call + * would tax the hook that exists to save the user money. + * THE COMPUTE the panel walks the metrics log, the balance log and the graph. + * THE CONTEXT the panel is only worth its own tokens when it says something actionable. + * + * So the throttle is checked FIRST, from persisted state, before any file is opened. Everything + * expensive happens after a clock comparison. + * + * NOTHING HERE IS ALLOWED TO BREAK A TOOL CALL. Every entry point returns null on failure rather + * than throwing: a forecast is a courtesy, and the hook's promise is that a defect in it costs the + * user nothing. + */ + +import { forecastPanel, worthSurfacing } from './forecast.mjs'; +import { observeOutcome, logForecast } from './calibration.mjs'; +import { readCacheUsage } from './cache.mjs'; +import { readBalance } from './metrics.mjs'; + +/** + * How long between two attempts to build the panel. + * + * Not a tuning knob so much as an admission: the panel costs a transcript parse, and a user makes + * many tool calls per minute. Two minutes is short enough that a runway falling into single digits + * is still caught with turns to spare, and long enough that the check is free in aggregate. + */ +export const SURFACE_INTERVAL_MS = + Number(process.env.TOKEN_OPTIMIZER_SURFACE_INTERVAL_MS) || 120_000; + +/** + * The context window to measure the runway against. + * + * NOT DISCOVERABLE from the transcript: the usage rows carry what was spent, never the ceiling it + * was spent against. Defaulting is therefore unavoidable, so the default is stated rather than + * buried, and overridable for anyone running a different window. + */ +export const DEFAULT_CAPACITY = + Number(process.env.TOKEN_OPTIMIZER_CONTEXT_CAPACITY) || 200_000; + +/** + * What the session has spent, read from its own transcript. + * + * `used` is the FULL input size of the most recent request -- cache reads plus cache writes plus + * fresh input -- which is what the context window actually holds, rather than the marginal cost of + * that one turn. Summing the per-turn inputs instead would count the same carried prefix once per + * turn and report a number many times the window size. + * + * Returns null rather than a zeroed object when there is nothing to measure, so a caller cannot + * mistake "no data" for "an empty context". + */ +export function sessionUsage(transcriptPath, { capacity = DEFAULT_CAPACITY } = {}) { + let turns; + try { + turns = readCacheUsage(transcriptPath); + } catch { + return null; + } + if (!turns?.length) return null; + + const last = turns[turns.length - 1]; + const used = (last.read || 0) + (last.written || 0) + (last.input || 0); + if (!used) return null; + + return { used, capacity, turns: turns.length }; +} + +/** Intercepted touches recorded for this session, which converts a per-touch saving to per-turn. */ +function touchesFor(dir, sessionId) { + try { + return readBalance(dir).filter((e) => e.kind === 'inject' && e.sessionId === sessionId).length; + } catch { + return 0; + } +} + +/** + * Should the forecast interrupt right now, and with what? + * + * The state object is the caller's persisted per-session state; this reads `forecast` from it and + * returns the value to store back, so the throttle and the last-shown runway survive across hook + * processes. Returning the next state rather than mutating it keeps this testable without a + * filesystem. + * + * @returns { text, state } -- text is null when nothing should be shown. + */ +export function maybeSurface(dir, { + transcriptPath, sessionId, state = {}, now = Date.now(), findings = [], +} = {}) { + const previous = state.forecast || null; + + // THE THROTTLE IS FIRST, and deliberately before any I/O. Everything below opens files. + if (previous?.checkedAt && now - previous.checkedAt < SURFACE_INTERVAL_MS) { + return { text: null, state: previous }; + } + + let usage; + try { + usage = sessionUsage(transcriptPath); + } catch { + return { text: null, state: previous }; + } + // The clock is stamped even when there is nothing to measure, so a session with no usage rows + // does not re-attempt the transcript parse on every single tool call. + if (!usage) return { text: null, state: { ...previous, checkedAt: now } }; + + let panel; + try { + panel = forecastPanel(dir, { + ...usage, + sessionId, + touches: touchesFor(dir, sessionId), + }, findings); + } catch { + return { text: null, state: { ...previous, checkedAt: now } }; + } + if (!panel) return { text: null, state: { ...previous, checkedAt: now } }; + + // worthSurfacing compares against the last panel that was actually SHOWN, not the last one + // computed. Comparing against the last computed panel would let the runway drift down past the + // threshold one throttle window at a time and never trip the "crossed it" test. + const shown = previous?.shown ? { parts: { runway: { withGraph: previous.shown } } } : null; + if (!worthSurfacing(panel, shown)) { + return { text: null, state: { ...previous, checkedAt: now } }; + } + + // LOGGED HERE, AND ONLY HERE -- past the throttle and past worthSurfacing, so exactly one record + // is written per prediction a person is actually shown. forecastPanel used to log on every build; + // since most builds are rejected above, that appended open forecast records nobody saw, and + // observeOutcome closes only the newest one per session. The rest stayed open forever and piled + // up in balance.jsonl, whose tail-bytes read would then evict the inject/harvest/substitute rows + // BALANCE_KINDS protects -- undoing the very repair this work exists to make. + // + // The RAW runway is logged, not the calibrated one: scoring a corrected number would fold each + // correction into the next and the bias would chase its own tail. + try { + logForecast(dir, { + sessionId, + predictedTurns: panel.parts.runway.withGraph, + used: usage.used, + capacity: usage.capacity, + turns: usage.turns, + }); + } catch { /* a forecast is a courtesy; failing to score it must not cost a tool call */ } + + return { + text: panel.text, + state: { checkedAt: now, shown: panel.parts.runway.withGraph }, + }; +} + +/** + * Closes the calibration loop, at the only moment that can close it. + * + * Compaction firing IS the ground truth a runway forecast was predicting, so this belongs on the + * PreCompact path and nowhere else. `actualTurns` is turns ELAPSED between the prediction and + * compaction, which is what predictedTurns meant -- so the forecast has to have recorded the turn + * it was made on, and observeOutcome subtracts. + * + * Silent by design: nothing is shown to anybody. It writes one record so that the NEXT session's + * forecast can be published with a track record instead of as a bare assertion. + */ +export function closeForecast(dir, { transcriptPath, sessionId } = {}) { + try { + const usage = sessionUsage(transcriptPath); + if (!usage) return false; + observeOutcome(dir, { sessionId, atTurn: usage.turns }); + return true; + } catch { + return false; + } +} diff --git a/integrations/qwen/hooks/lib/calibration.mjs b/integrations/qwen/hooks/lib/calibration.mjs index 718db89cd..a126fcaef 100644 --- a/integrations/qwen/hooks/lib/calibration.mjs +++ b/integrations/qwen/hooks/lib/calibration.mjs @@ -31,7 +31,20 @@ * that cannot support one. */ -import { record, readMetrics } from './metrics.mjs'; +// readBalance, not readMetrics. Both readers used the windowed firehose, which returns at most +// 2,000,000 bytes and the last 5,000 lines -- and forecast events were not among the protected +// kinds, so they lived in a log dominated by per-tool-call records (metrics.mjs's own comment +// cites 4,735 capture events inside one window). +// +// Two failures followed. In observeOutcome, on a busy project the forecast had scrolled past the +// tail by the time compaction fired, so the outcome was discarded with no record, no error and no +// counter. In reliability, outcomes vanished faster than they accumulated, so rows.length could +// never reach MIN_SCORED and calibrate returned 'not yet calibrated (n/8)' permanently -- leaving +// a user unable to distinguish "never had data" from "the data is being thrown away". +// +// 'forecast' and 'forecast-outcome' are now balance kinds, so they are written to balance.jsonl +// as well and read back unwindowed. +import { record, readBalance } from './metrics.mjs'; /** Horizon buckets, in turns. Error behaves differently across these ranges. */ const HORIZONS = [ @@ -53,8 +66,15 @@ function horizonOf(turns) { return HORIZONS.find((h) => turns <= h.max).name; } -/** Records a prediction so it can be scored when the outcome arrives. */ -export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) { +/** + * Records a prediction so it can be scored when the outcome arrives. + * + * `turns` is the session's turn count AT PREDICTION TIME, and it is what makes the outcome + * computable at all: predictedTurns means "turns from here until compaction", so the ground truth + * is the turns that ELAPSED, not the turn number compaction happened on. Without it the caller at + * compaction has nothing to subtract from and would have to score an interval against an absolute. + */ +export function logForecast(dir, { sessionId, predictedTurns, used, capacity, turns }) { if (!Number.isFinite(predictedTurns)) return; record(dir, { kind: 'forecast', @@ -63,6 +83,7 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) horizon: horizonOf(predictedTurns), used, capacity, + turns: Number.isFinite(turns) ? turns : null, }); } @@ -73,21 +94,74 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) * Called when compaction fires: the number of turns that elapsed since the * prediction is the ground truth it was predicting. */ -export function observeOutcome(dir, { sessionId, actualTurns }) { - if (!Number.isFinite(actualTurns)) return; - const open = readMetrics(dir) - .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && !e.scored) +export function observeOutcome(dir, { sessionId, actualTurns, atTurn }) { + // EITHER THE ELAPSED COUNT OR THE TURN IT HAPPENED ON. + // + // The only real caller -- the PreCompact hook -- knows what turn compaction fired on, not how + // many turns have passed since a prediction it did not make. Deriving the interval here keeps + // that subtraction next to the record that defines it, rather than making every caller + // re-discover which forecast is open in order to compute its own ground truth. + const elapsed = Number.isFinite(actualTurns) + ? actualTurns + : null; + if (elapsed === null && !Number.isFinite(atTurn)) return; + + // CLOSURE IS DERIVED FROM THE OUTCOME ROWS, not from a flag on the forecast. + // + // The old filter was `!e.scored`, but nothing ever writes `scored`: logForecast records + // kind/sessionId/predictedTurns/horizon/used/capacity, and record() is an append-only JSONL + // writer with no update path, so a forecast can never gain the field. The predicate was always + // true, and the last forecast for a session was therefore re-closable without limit. + // + // A session where compaction fires twice with no intervening logForecast popped the SAME + // forecast twice, producing two outcome rows for one prediction. reliability counts ROWS, not + // predictions, so the hit rate and the bias mean both inflate and MIN_SCORED can be satisfied + // by a single prediction re-closed eight times -- precisely the "threshold from a single + // sample" this module exists to prevent. The existing test used a fresh sessionId per + // iteration, so it never exposed it. + const events = readBalance(dir); + const closed = new Set( + events.filter((e) => e.kind === 'forecast-outcome' && e.forecastId).map((e) => e.forecastId), + ); + // ID-BEARING ROWS ONLY, AND NEWEST BY TIME RATHER THAN BY POSITION. + // + // Two separate hazards, both from readBalance's shape. It returns balance-log rows followed by + // the legacy firehose rows it migrates, unsorted -- so `.pop()` returns the last row in the + // array, which is not the newest forecast. And legacy rows predate `id`, so `closed.has(e.id)` + // is `closed.has(undefined)`, always false: such a forecast can be closed again and again, and + // each outcome stores `forecastId: undefined`, which JSON.stringify drops entirely. That is the + // repeat-scoring defect this function was written to fix, reappearing through the back door. + const open = events + .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && e.id && !closed.has(e.id)) + .sort((a, b) => (a.at ?? 0) - (b.at ?? 0)) .pop(); if (!open) return; + // Derived only once the open forecast is known, since the interval is measured from ITS turn. + // A forecast recorded before `turns` existed has none, and there is nothing to subtract from -- + // scoring it against an absolute turn number would manufacture an error rather than measure one, + // so it is left open instead. + const actual = elapsed !== null ? elapsed + : (Number.isFinite(open.turns) ? atTurn - open.turns : null); + // AT LEAST ONE TURN MUST HAVE PASSED. `atTurn` is the transcript's current turn count and + // `open.turns` the count at prediction time, so a truncated, rotated or replaced transcript + // makes the difference zero or negative. Recording that as ground truth feeds a large negative + // `error` into `reliability`, which shifts `bucket.bias`, which is what `calibrate` publishes a + // corrected forecast from -- so one bad transcript would skew every later prediction. Leaving + // the forecast open loses nothing: the next compaction closes it against a sane count. + if (actual === null || !Number.isFinite(actual) || actual < 1) return; + record(dir, { kind: 'forecast-outcome', sessionId, + // Stamped so the next call can see this forecast is spoken for. Without it the set above is + // empty and the same defect returns. + forecastId: open.id, horizon: open.horizon, predictedTurns: open.predictedTurns, - actualTurns, - error: actualTurns - open.predictedTurns, - hit: Math.abs(actualTurns - open.predictedTurns) <= TOLERANCE, + actualTurns: actual, + error: actual - open.predictedTurns, + hit: Math.abs(actual - open.predictedTurns) <= TOLERANCE, }); } @@ -98,7 +172,7 @@ export function observeOutcome(dir, { sessionId, actualTurns }) { * actionable, where "62% accurate" is not. */ export function reliability(dir) { - const outcomes = readMetrics(dir).filter((e) => e.kind === 'forecast-outcome'); + const outcomes = readBalance(dir).filter((e) => e.kind === 'forecast-outcome'); const buckets = {}; for (const horizon of HORIZONS) { @@ -164,7 +238,28 @@ export function calibrate(dir, predictedTurns) { // Refit: shift by the observed bias for this horizon. A bucket that runs long // gets pulled in, and vice versa. - const adjusted = Math.max(0, Math.round(predictedTurns + bucket.bias)); + // + // NOT CLAMPED WITH Math.max, which converted nonsense into a plausible-looking emergency. bias + // is a plain arithmetic mean of signed error, and a mean is not robust to the shape the + // 50%-hit-rate floor admits: a near bucket with errors [0,0,0,0,-30,-30,-30,-30] has four hits, + // so it clears the floor and is called calibrated, and its bias is -15. A raw forecast of 3 + // turns then became Math.max(0, Math.round(3 - 15)) = 0 and was published as fact, with the + // note "within 3 on 50% of 8 past forecasts" -- a zero-turn deadline the forecaster never + // produced, from a bucket that is wrong half the time. + // + // A correction at least as large as the thing it corrects is not a correction. + const adjusted = Math.round(predictedTurns + bucket.bias); + if (Math.abs(bucket.bias) >= predictedTurns || adjusted < 1) { + return { + publishable: false, + predictedTurns, + horizon, + bias: bucket.bias, + reason: `the observed bias at this horizon (${Math.round(bucket.bias)} turns) is as large as ` + + `the forecast itself (${predictedTurns}), so the correction would replace it rather than ` + + 'adjust it', + }; + } return { publishable: true, diff --git a/integrations/qwen/hooks/lib/forecast.mjs b/integrations/qwen/hooks/lib/forecast.mjs index aed24df29..474ed84f9 100644 --- a/integrations/qwen/hooks/lib/forecast.mjs +++ b/integrations/qwen/hooks/lib/forecast.mjs @@ -31,13 +31,30 @@ * other rather than one standing in for the other. */ -import { readMetrics } from './metrics.mjs'; +import { readMetrics, readBalance, isFixtureAnchor, BALANCE_KINDS } from './metrics.mjs'; import { aggregateConsolidation } from './consolidate.mjs'; import { previewQuality } from './expand.mjs'; +import { calibrate } from './calibration.mjs'; /** Turns of headroom below which the runway is worth interrupting for. */ export const ACTIONABLE_RUNWAY = 8; +/** + * Held-out touches required before the runway counterfactual is published. + * + * The header of this file promises the counterfactual appears "only once the holdout carries + * volume", and the fallback string says "the control arm needs more volume" -- but the only check + * was `if (!rows.length) return null`, so "volume" meant "at least one". With a single withheld + * touch, withheldCost IS that one touch's downstream read total: one held-out read of a + * 3,000-line module set savedPerTouch to several thousand tokens and published a counterfactual + * an order of magnitude off, as a bare fact with no caveat. + * + * Ten is a floor rather than a statistically derived threshold, and is deliberately stated as + * such where it surfaces: the panel prints the held-out count against it, so a reader can see how + * close the arm is instead of being told only that it is short. + */ +export const MIN_CONTROL_TOUCHES = 10; + /** * Tokens per turn, and how many the graph is removing. * @@ -57,13 +74,42 @@ export function burnRate(events) { reads.get(key).push(event); } + // EACH READ IS CHARGED TO EXACTLY ONE INJECTION. + // + // The windows used to run from each injection to the end of the log, so they overlapped + // completely: for anchor A with injections i1..i5 interleaved with five reads of 100 tokens, + // i1 saw 500, i2 400, i3 300, i4 200, i5 100 -- 1500 / 5 = 300 against a true per-touch cost of + // 100. A 3x inflation growing with the repeat count. + // + // It does not cancel between the arms. metrics.mjs makes the holdout deterministic in + // (anchor, epoch), so every repeat touch of a file lands in the SAME arm -- whichever arm drew + // the heavily re-touched anchors is inflated and the other is not. savedPerTouch's sign and + // magnitude were therefore driven by which files happened to be held out, and that value is + // what the published counterfactual divides by. + // + // Bounding each window at the next injection of the same (session, anchor) -- across BOTH arms, + // since an injection ends the previous window regardless of which arm it is in -- makes the + // charge exact. + const nextInjectAfter = new Map(); + for (const row of injects) { + const key = `${row.sessionId || ''}|${row.anchor}`; + if (!nextInjectAfter.has(key)) nextInjectAfter.set(key, []); + nextInjectAfter.get(key).push(row.at ?? 0); + } + for (const times of nextInjectAfter.values()) times.sort((a, b) => a - b); + const downstream = (rows) => { if (!rows.length) return null; let total = 0; for (const row of rows) { - const bucket = reads.get(`${row.sessionId || ''}|${row.anchor}`) || []; + const key = `${row.sessionId || ''}|${row.anchor}`; + const bucket = reads.get(key) || []; const after = row.at ?? 0; - total += bucket.reduce((sum, r) => sum + ((r.at ?? 0) >= after ? (r.tokens || 0) : 0), 0); + const until = (nextInjectAfter.get(key) || []).find((t) => t > after) ?? Infinity; + total += bucket.reduce( + (sum, r) => sum + ((r.at ?? 0) >= after && (r.at ?? 0) < until ? (r.tokens || 0) : 0), + 0, + ); } return total / rows.length; }; @@ -98,12 +144,27 @@ export function burnRate(events) { export function shadowAvoided(events) { let avoided = 0; let spent = 0; + let seen = 0; for (const event of events) { if (event.kind !== 'substitute') continue; + // FIXTURES EXCLUDED, as balanceSheet already does. metrics.mjs records the measurement: + // "366 of 370 substitutions pointed at the enforcement suite's own big.ts fixture under a + // temp dir, and counting them made the product look like it had avoided 40 MB of reads. It + // had avoided 154 KB." Running the test suite against a real graph inflated `avoided` by two + // orders of magnitude, which blew up shadow.net, pushed `spread` past 0.5 and fired the + // divergence note with a fabricated number -- so the panel's own credibility check was the + // thing the fixture noise corrupted. + if (isFixtureAnchor(event.anchor)) continue; + seen += 1; avoided += Math.ceil((event.bytesAvoided || 0) / 4); spent += event.tokens || 0; } - return avoided ? { avoided, spent, net: avoided - spent } : null; + // GUARDED ON WHETHER ANYTHING WAS SEEN, not on whether it was favourable. Guarding on `avoided` + // meant twenty substitutions that each spent 20 tokens and avoided nothing -- a measured net + // loss of 400 -- returned null, so the divergence block never ran and the panel said nothing + // about the shadow arm. The one outcome the function could not express was the one unfavourable + // to the product, collapsed into the same return value as "no data". + return seen ? { avoided, spent, net: avoided - spent, substitutions: seen } : null; } /** @@ -112,8 +173,10 @@ export function shadowAvoided(events) { * @param used Tokens of context consumed so far. * @param capacity Context window size. * @param turns Turns taken to consume it. + * @param touches Intercepted touches in this session. Required for the counterfactual: it is + * what converts a per-TOUCH saving into a per-TURN one. */ -export function runway({ used, capacity, turns }, rate) { +export function runway({ used, capacity, turns, touches }, rate) { if (!capacity || !turns || used == null) return null; const perTurn = used / turns; @@ -121,15 +184,53 @@ export function runway({ used, capacity, turns }, rate) { const remaining = Math.max(0, capacity - used); const withGraph = Math.floor(remaining / perTurn); + const base = { withGraph, perTurn: Math.round(perTurn) }; - // Without the graph each turn costs what it costs now PLUS what the graph is - // removing. Only computable when the control arm has spoken. const saved = rate?.savedPerTouch; - const withoutGraph = saved != null && saved > 0 - ? Math.floor(remaining / (perTurn + saved)) - : null; + if (saved == null) return { ...base, withoutGraph: null, counterfactual: 'unmeasured' }; + + // VOLUME, MEANING VOLUME. See MIN_CONTROL_TOUCHES. + const withheld = rate.withheldTouches ?? 0; + if (withheld < MIN_CONTROL_TOUCHES) { + return { ...base, withoutGraph: null, counterfactual: 'thin-control', withheld }; + } + + // UNITS. `perTurn` is tokens per TURN for this session; `savedPerTouch` is an average over + // injection EVENTS across the whole metrics log. Adding them directly was only correct with + // exactly one intercepted touch per turn, which is not the usual case in either direction. + // + // Session {used 60000, capacity 200000, turns 30} gives perTurn 2000 and remaining 140000. With + // 5 injections over those 30 turns and savedPerTouch 1500, the true per-turn saving is 250, so + // the honest counterfactual is floor(140000/2250) = 62. Adding the per-touch figure gave + // floor(140000/3500) = 40, and the panel published "~70 turns to compaction; without the graph, + // ~40" -- claiming the graph nearly doubled the runway when it added about eight turns. + // + // Without a touch count there is no density to convert with, so the counterfactual is withheld + // rather than computed on the one-touch-per-turn assumption. + if (!touches) return { ...base, withoutGraph: null, counterfactual: 'no-touch-density' }; + + const savedPerTurn = saved * (touches / turns); + + // A MEASURED NON-POSITIVE SAVING IS A RESULT, NOT A MISSING ONE. The old `saved > 0` guard + // discarded a measurement: with withheldCost 1000, treatedCost 900 and injectionCost 200, + // savedPerTouch is -100 -- the control arm has spoken and said the graph is a net cost -- and + // the panel printed "no counterfactual yet, the control arm needs more volume". The one result + // unfavourable to the product was the one result the panel could not show, disguised as + // insufficient data. The runway is longer without the graph in that case, and says so. + const denominator = perTurn + savedPerTurn; + if (denominator <= 0) { + // The graph is measured to be removing more per turn than the session is spending, which is + // arithmetically possible and not interpretable as a runway. Reported as such. + return { ...base, withoutGraph: null, counterfactual: 'implausible', savedPerTurn }; + } - return { withGraph, withoutGraph, perTurn: Math.round(perTurn) }; + return { + ...base, + withoutGraph: Math.floor(remaining / denominator), + counterfactual: savedPerTurn > 0 ? 'saving' : 'costing', + savedPerTurn, + withheld, + }; } /** @@ -141,21 +242,99 @@ export function runway({ used, capacity, turns }, rate) { * confident figure derived from four samples -- is precisely what this project * criticises elsewhere. */ +/** + * The events the forecast reasons about, each kind read from the log that keeps it. + * + * Reads come from readMetrics, which applies MAX_EVENTS and a tail byte cap -- correct, because + * reads are the high-volume kind and only the recent ones describe the current burn. + * + * Inject, harvest and substitute come from readBalance, which is exempt from that window and + * exists for precisely this reason. metrics.mjs records what the window does to them, measured in + * this repository: "44 inject records in the file, 9 of them holdout, all at lines 60-76 of + * 9,058 -- every single one outside the window. report() therefore said 0 holdout." + * + * The 10% holdout arm is the rarer kind, so it ages out FIRST. On any graph with more than 5,000 + * events since the last injection burst, downstream(withheld) returned null, savedPerTouch was + * null, and the panel printed "no counterfactual yet -- the control arm needs more volume" while + * balance.jsonl held the control arm in full. The panel blamed insufficient data for a read-path + * bug; in the partial case it computed the published counterfactual from whichever slice of the + * control arm happened to survive the window. + */ +export function balanceAwareEvents(dir) { + const balance = readBalance(dir); + // No id-matching needed, and deliberately so: readBalance already reads balance.jsonl in full, + // migrates the pre-split records out of metrics.jsonl WITHOUT the window, and dedupes the two + // on record id (falling back to a composite for records written before ids existed). It is + // therefore a strict superset of the balance kinds in the windowed read, so the windowed copies + // are dropped outright. Trying to merge them back by id would double-count exactly the legacy + // records that have no id. + const out = readMetrics(dir).filter((e) => !BALANCE_KINDS.has(e?.kind)); + out.push(...balance); + return out; +} + +/** How the runway reads, given what the control arm was actually able to say. */ +function runwayLine(air) { + const head = `~${air.withGraph} turns to compaction`; + switch (air.counterfactual) { + case 'saving': + return `${head}; without the graph, ~${air.withoutGraph}.`; + case 'costing': + // Said plainly rather than suppressed. A tool that can only report results in its own + // favour is not measuring anything. + return `${head}; without the graph, ~${air.withoutGraph} -- the graph is measurably NOT ` + + 'extending the runway here.'; + case 'thin-control': + return `${head} (no counterfactual yet -- the control arm holds ${air.withheld} of the ` + + `${MIN_CONTROL_TOUCHES} held-out touches needed).`; + case 'no-touch-density': + return `${head} (no counterfactual -- this session's intercepted-touch count is unknown, ` + + 'and a per-touch saving cannot be converted to a per-turn one without it).'; + case 'implausible': + return `${head} (no counterfactual -- the measured saving exceeds this session's whole ` + + 'per-turn cost, so the arithmetic does not describe a runway).'; + default: + return `${head} (no counterfactual yet -- the control arm has not spoken).`; + } +} + export function forecastPanel(dir, session = {}, findings = []) { - const events = readMetrics(dir); + const events = balanceAwareEvents(dir); const rate = burnRate(events); const shadow = shadowAvoided(events); const consolidation = aggregateConsolidation(findings); - const air = runway(session, rate); + // The touch count converts the per-touch saving into a per-turn one. Taken from the session + // when the caller knows it, and otherwise from the treated touches this rate was built from. + const touches = session.touches ?? rate?.treatedTouches ?? 0; + const air = runway({ ...session, touches }, rate); const lines = []; const parts = {}; if (air) { parts.runway = air; - lines.push(air.withoutGraph != null - ? `~${air.withGraph} turns to compaction; without the graph, ~${air.withoutGraph}.` - : `~${air.withGraph} turns to compaction (no counterfactual yet -- the control arm needs more volume).`); + + // THE FORECAST KEEPS ITS OWN SCORE -- from here, because this is the only place a forecast is + // actually made. calibration.mjs was written for exactly this and had zero shipped importers: + // no forecast was ever logged, no outcome ever observed, reliability always saw an empty set, + // and calibrate was unreachable. So the shipped panel printed precisely the uncalibrated + // number that module's docstring calls "a vibe with a typeface". + // + // NOT LOGGED HERE. Building a panel is not making a forecast. maybeSurface builds one per + // throttle window and worthSurfacing rejects most of them, so logging on every build appended + // an open forecast record nobody ever saw. observeOutcome closes only the LAST open forecast + // for a session, so every earlier one stayed open forever -- and they accumulate in + // balance.jsonl, whose tail-bytes read is precisely what would then displace the + // inject/harvest/substitute rows BALANCE_KINDS exists to protect. That would have undone this + // change's own fix. The surfacing path logs instead, at the one moment a prediction is + // actually made to somebody. + const score = calibrate(dir, air.withGraph); + parts.calibration = score; + // Published only when the horizon has earned it. An unpublishable score does not replace the + // number -- it is simply absent, exactly as the uncalibrated case reads today. + lines.push(score.publishable + ? `${runwayLine({ ...air, withGraph: score.predictedTurns })}\n (corrected: ${score.note})` + : runwayLine(air)); } if (rate) { diff --git a/integrations/qwen/hooks/lib/metrics.mjs b/integrations/qwen/hooks/lib/metrics.mjs index 3fc52e22b..4b0eb2712 100644 --- a/integrations/qwen/hooks/lib/metrics.mjs +++ b/integrations/qwen/hooks/lib/metrics.mjs @@ -63,7 +63,22 @@ const metricsPath = (dir) => join(dir, 'metrics.jsonl'); const balancePath = (dir) => join(dir, 'balance.jsonl'); /** Kinds the net balance is computed from, and which therefore must survive. */ -const BALANCE_KINDS = new Set(['inject', 'harvest', 'substitute']); +/** + * Kinds written to balance.jsonl as well as the firehose, so the event window can never + * starve the measurement. Exported because any reader that needs the unwindowed truth needs to + * know which kinds it applies to, and a second copy of this set elsewhere would drift. + */ +export const BALANCE_KINDS = new Set([ + 'inject', + 'harvest', + 'substitute', + // The forecast's own score. Both kinds are rare and accumulate slowly -- MIN_SCORED is 8 -- + // while the firehose they shared is dominated by per-tool-call records, so they were evicted + // faster than they could accumulate: calibrate() returned 'not yet calibrated (n/8)' forever + // and an outcome whose forecast had scrolled away was dropped with no record at all. + 'forecast', + 'forecast-outcome', +]); /** * Is this touch in the holdout arm? diff --git a/integrations/qwen/hooks/lib/policy.mjs b/integrations/qwen/hooks/lib/policy.mjs index b3871c27d..f89a48210 100644 --- a/integrations/qwen/hooks/lib/policy.mjs +++ b/integrations/qwen/hooks/lib/policy.mjs @@ -272,7 +272,7 @@ function statePath(sessionId, agent) { /** A usable state object, whatever was on disk. */ function emptyState() { - return { seen: {}, denied: {}, injected: [], actCounts: {} }; + return { seen: {}, denied: {}, injected: [], actCounts: {}, forecast: null }; } /** @@ -312,6 +312,22 @@ export function loadState(sessionId, agent) { parsed.actCounts && typeof parsed.actCounts === 'object' && !Array.isArray(parsed.actCounts) ? parsed.actCounts : {}, + // THE SAME REASON A THIRD TIME. The forecast throttle records when the panel was last + // computed and what runway it last SHOWED, and both are meaningless within a single hook + // process -- every tool call is a new one. Dropped here, `checkedAt` would reset on every + // call and the panel would rebuild itself, transcript parse and all, on each tool use; and + // `shown` would reset, so worthSurfacing would compare against nothing and re-interrupt with + // the same runway forever. + // AND ITS TIMESTAMP MUST BE A NUMBER. The shape check alone accepted `checkedAt: "1700"`, + // and both the throttle comparison and the merge below order by it -- so a persisted numeric + // STRING compares by lexicographic coercion and can beat a later real timestamp, freezing + // the throttle open or shut. Same class as every other field validated here: a value that + // parsed is not a value that means anything. + forecast: + parsed.forecast && typeof parsed.forecast === 'object' && !Array.isArray(parsed.forecast) + && Number.isFinite(parsed.forecast.checkedAt) + ? parsed.forecast + : null, }; } catch { return emptyState(); @@ -383,6 +399,19 @@ export function saveState(sessionId, state, agent) { } return out; })(), + // LATEST CHECK WINS, which is the opposite direction from the fields above and correct for + // this one. `seen`, `denied`, `injected` and `actCounts` are all append-only, so a union or + // a max is the safe merge. The forecast throttle is a POINT IN TIME: taking the older of two + // concurrent checks would re-open the window and let the panel be rebuilt immediately, which + // is the cost the throttle exists to bound. + forecast: (() => { + const mine = state.forecast || null; + const theirs = current.forecast || null; + const stamp = (f) => (Number.isFinite(f?.checkedAt) ? f.checkedAt : null); + if (stamp(mine) === null) return theirs; + if (stamp(theirs) === null) return mine; + return stamp(mine) >= stamp(theirs) ? mine : theirs; + })(), }; // Write-then-rename so a reader never observes a half-written file. diff --git a/integrations/qwen/hooks/lib/surface.mjs b/integrations/qwen/hooks/lib/surface.mjs new file mode 100644 index 000000000..4054c8cf9 --- /dev/null +++ b/integrations/qwen/hooks/lib/surface.mjs @@ -0,0 +1,185 @@ +// GENERATED FILE -- do not edit. +// Source of truth: hooks-core/surface.mjs. Regenerate with `npm run sync:hooks`. +/** + * The entry point the forecast never had. + * + * forecast.mjs computes a runway, calibration.mjs scores it, and worthSurfacing decides whether a + * change is worth interrupting for -- and none of it ran. The reachability guard listed + * forecastPanel, worthSurfacing, logForecast and observeOutcome as unwired, which is a polite way + * of saying the feature did not exist for any user. This module is the missing half: it derives + * the session numbers the panel needs from the transcript, decides when the panel has earned an + * interruption, and closes the calibration loop when compaction fires. + * + * THREE COSTS ARE BOUNDED HERE, because this runs on the PreToolUse path and every tool call is a + * separate process: + * + * THE READ readCacheUsage parses up to 4 MB of transcript. Doing that on every tool call + * would tax the hook that exists to save the user money. + * THE COMPUTE the panel walks the metrics log, the balance log and the graph. + * THE CONTEXT the panel is only worth its own tokens when it says something actionable. + * + * So the throttle is checked FIRST, from persisted state, before any file is opened. Everything + * expensive happens after a clock comparison. + * + * NOTHING HERE IS ALLOWED TO BREAK A TOOL CALL. Every entry point returns null on failure rather + * than throwing: a forecast is a courtesy, and the hook's promise is that a defect in it costs the + * user nothing. + */ + +import { forecastPanel, worthSurfacing } from './forecast.mjs'; +import { observeOutcome, logForecast } from './calibration.mjs'; +import { readCacheUsage } from './cache.mjs'; +import { readBalance } from './metrics.mjs'; + +/** + * How long between two attempts to build the panel. + * + * Not a tuning knob so much as an admission: the panel costs a transcript parse, and a user makes + * many tool calls per minute. Two minutes is short enough that a runway falling into single digits + * is still caught with turns to spare, and long enough that the check is free in aggregate. + */ +export const SURFACE_INTERVAL_MS = + Number(process.env.TOKEN_OPTIMIZER_SURFACE_INTERVAL_MS) || 120_000; + +/** + * The context window to measure the runway against. + * + * NOT DISCOVERABLE from the transcript: the usage rows carry what was spent, never the ceiling it + * was spent against. Defaulting is therefore unavoidable, so the default is stated rather than + * buried, and overridable for anyone running a different window. + */ +export const DEFAULT_CAPACITY = + Number(process.env.TOKEN_OPTIMIZER_CONTEXT_CAPACITY) || 200_000; + +/** + * What the session has spent, read from its own transcript. + * + * `used` is the FULL input size of the most recent request -- cache reads plus cache writes plus + * fresh input -- which is what the context window actually holds, rather than the marginal cost of + * that one turn. Summing the per-turn inputs instead would count the same carried prefix once per + * turn and report a number many times the window size. + * + * Returns null rather than a zeroed object when there is nothing to measure, so a caller cannot + * mistake "no data" for "an empty context". + */ +export function sessionUsage(transcriptPath, { capacity = DEFAULT_CAPACITY } = {}) { + let turns; + try { + turns = readCacheUsage(transcriptPath); + } catch { + return null; + } + if (!turns?.length) return null; + + const last = turns[turns.length - 1]; + const used = (last.read || 0) + (last.written || 0) + (last.input || 0); + if (!used) return null; + + return { used, capacity, turns: turns.length }; +} + +/** Intercepted touches recorded for this session, which converts a per-touch saving to per-turn. */ +function touchesFor(dir, sessionId) { + try { + return readBalance(dir).filter((e) => e.kind === 'inject' && e.sessionId === sessionId).length; + } catch { + return 0; + } +} + +/** + * Should the forecast interrupt right now, and with what? + * + * The state object is the caller's persisted per-session state; this reads `forecast` from it and + * returns the value to store back, so the throttle and the last-shown runway survive across hook + * processes. Returning the next state rather than mutating it keeps this testable without a + * filesystem. + * + * @returns { text, state } -- text is null when nothing should be shown. + */ +export function maybeSurface(dir, { + transcriptPath, sessionId, state = {}, now = Date.now(), findings = [], +} = {}) { + const previous = state.forecast || null; + + // THE THROTTLE IS FIRST, and deliberately before any I/O. Everything below opens files. + if (previous?.checkedAt && now - previous.checkedAt < SURFACE_INTERVAL_MS) { + return { text: null, state: previous }; + } + + let usage; + try { + usage = sessionUsage(transcriptPath); + } catch { + return { text: null, state: previous }; + } + // The clock is stamped even when there is nothing to measure, so a session with no usage rows + // does not re-attempt the transcript parse on every single tool call. + if (!usage) return { text: null, state: { ...previous, checkedAt: now } }; + + let panel; + try { + panel = forecastPanel(dir, { + ...usage, + sessionId, + touches: touchesFor(dir, sessionId), + }, findings); + } catch { + return { text: null, state: { ...previous, checkedAt: now } }; + } + if (!panel) return { text: null, state: { ...previous, checkedAt: now } }; + + // worthSurfacing compares against the last panel that was actually SHOWN, not the last one + // computed. Comparing against the last computed panel would let the runway drift down past the + // threshold one throttle window at a time and never trip the "crossed it" test. + const shown = previous?.shown ? { parts: { runway: { withGraph: previous.shown } } } : null; + if (!worthSurfacing(panel, shown)) { + return { text: null, state: { ...previous, checkedAt: now } }; + } + + // LOGGED HERE, AND ONLY HERE -- past the throttle and past worthSurfacing, so exactly one record + // is written per prediction a person is actually shown. forecastPanel used to log on every build; + // since most builds are rejected above, that appended open forecast records nobody saw, and + // observeOutcome closes only the newest one per session. The rest stayed open forever and piled + // up in balance.jsonl, whose tail-bytes read would then evict the inject/harvest/substitute rows + // BALANCE_KINDS protects -- undoing the very repair this work exists to make. + // + // The RAW runway is logged, not the calibrated one: scoring a corrected number would fold each + // correction into the next and the bias would chase its own tail. + try { + logForecast(dir, { + sessionId, + predictedTurns: panel.parts.runway.withGraph, + used: usage.used, + capacity: usage.capacity, + turns: usage.turns, + }); + } catch { /* a forecast is a courtesy; failing to score it must not cost a tool call */ } + + return { + text: panel.text, + state: { checkedAt: now, shown: panel.parts.runway.withGraph }, + }; +} + +/** + * Closes the calibration loop, at the only moment that can close it. + * + * Compaction firing IS the ground truth a runway forecast was predicting, so this belongs on the + * PreCompact path and nowhere else. `actualTurns` is turns ELAPSED between the prediction and + * compaction, which is what predictedTurns meant -- so the forecast has to have recorded the turn + * it was made on, and observeOutcome subtracts. + * + * Silent by design: nothing is shown to anybody. It writes one record so that the NEXT session's + * forecast can be published with a track record instead of as a bare assertion. + */ +export function closeForecast(dir, { transcriptPath, sessionId } = {}) { + try { + const usage = sessionUsage(transcriptPath); + if (!usage) return false; + observeOutcome(dir, { sessionId, atTurn: usage.turns }); + return true; + } catch { + return false; + } +} diff --git a/plugin/hooks/lib/calibration.mjs b/plugin/hooks/lib/calibration.mjs index 718db89cd..a126fcaef 100644 --- a/plugin/hooks/lib/calibration.mjs +++ b/plugin/hooks/lib/calibration.mjs @@ -31,7 +31,20 @@ * that cannot support one. */ -import { record, readMetrics } from './metrics.mjs'; +// readBalance, not readMetrics. Both readers used the windowed firehose, which returns at most +// 2,000,000 bytes and the last 5,000 lines -- and forecast events were not among the protected +// kinds, so they lived in a log dominated by per-tool-call records (metrics.mjs's own comment +// cites 4,735 capture events inside one window). +// +// Two failures followed. In observeOutcome, on a busy project the forecast had scrolled past the +// tail by the time compaction fired, so the outcome was discarded with no record, no error and no +// counter. In reliability, outcomes vanished faster than they accumulated, so rows.length could +// never reach MIN_SCORED and calibrate returned 'not yet calibrated (n/8)' permanently -- leaving +// a user unable to distinguish "never had data" from "the data is being thrown away". +// +// 'forecast' and 'forecast-outcome' are now balance kinds, so they are written to balance.jsonl +// as well and read back unwindowed. +import { record, readBalance } from './metrics.mjs'; /** Horizon buckets, in turns. Error behaves differently across these ranges. */ const HORIZONS = [ @@ -53,8 +66,15 @@ function horizonOf(turns) { return HORIZONS.find((h) => turns <= h.max).name; } -/** Records a prediction so it can be scored when the outcome arrives. */ -export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) { +/** + * Records a prediction so it can be scored when the outcome arrives. + * + * `turns` is the session's turn count AT PREDICTION TIME, and it is what makes the outcome + * computable at all: predictedTurns means "turns from here until compaction", so the ground truth + * is the turns that ELAPSED, not the turn number compaction happened on. Without it the caller at + * compaction has nothing to subtract from and would have to score an interval against an absolute. + */ +export function logForecast(dir, { sessionId, predictedTurns, used, capacity, turns }) { if (!Number.isFinite(predictedTurns)) return; record(dir, { kind: 'forecast', @@ -63,6 +83,7 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) horizon: horizonOf(predictedTurns), used, capacity, + turns: Number.isFinite(turns) ? turns : null, }); } @@ -73,21 +94,74 @@ export function logForecast(dir, { sessionId, predictedTurns, used, capacity }) * Called when compaction fires: the number of turns that elapsed since the * prediction is the ground truth it was predicting. */ -export function observeOutcome(dir, { sessionId, actualTurns }) { - if (!Number.isFinite(actualTurns)) return; - const open = readMetrics(dir) - .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && !e.scored) +export function observeOutcome(dir, { sessionId, actualTurns, atTurn }) { + // EITHER THE ELAPSED COUNT OR THE TURN IT HAPPENED ON. + // + // The only real caller -- the PreCompact hook -- knows what turn compaction fired on, not how + // many turns have passed since a prediction it did not make. Deriving the interval here keeps + // that subtraction next to the record that defines it, rather than making every caller + // re-discover which forecast is open in order to compute its own ground truth. + const elapsed = Number.isFinite(actualTurns) + ? actualTurns + : null; + if (elapsed === null && !Number.isFinite(atTurn)) return; + + // CLOSURE IS DERIVED FROM THE OUTCOME ROWS, not from a flag on the forecast. + // + // The old filter was `!e.scored`, but nothing ever writes `scored`: logForecast records + // kind/sessionId/predictedTurns/horizon/used/capacity, and record() is an append-only JSONL + // writer with no update path, so a forecast can never gain the field. The predicate was always + // true, and the last forecast for a session was therefore re-closable without limit. + // + // A session where compaction fires twice with no intervening logForecast popped the SAME + // forecast twice, producing two outcome rows for one prediction. reliability counts ROWS, not + // predictions, so the hit rate and the bias mean both inflate and MIN_SCORED can be satisfied + // by a single prediction re-closed eight times -- precisely the "threshold from a single + // sample" this module exists to prevent. The existing test used a fresh sessionId per + // iteration, so it never exposed it. + const events = readBalance(dir); + const closed = new Set( + events.filter((e) => e.kind === 'forecast-outcome' && e.forecastId).map((e) => e.forecastId), + ); + // ID-BEARING ROWS ONLY, AND NEWEST BY TIME RATHER THAN BY POSITION. + // + // Two separate hazards, both from readBalance's shape. It returns balance-log rows followed by + // the legacy firehose rows it migrates, unsorted -- so `.pop()` returns the last row in the + // array, which is not the newest forecast. And legacy rows predate `id`, so `closed.has(e.id)` + // is `closed.has(undefined)`, always false: such a forecast can be closed again and again, and + // each outcome stores `forecastId: undefined`, which JSON.stringify drops entirely. That is the + // repeat-scoring defect this function was written to fix, reappearing through the back door. + const open = events + .filter((e) => e.kind === 'forecast' && e.sessionId === sessionId && e.id && !closed.has(e.id)) + .sort((a, b) => (a.at ?? 0) - (b.at ?? 0)) .pop(); if (!open) return; + // Derived only once the open forecast is known, since the interval is measured from ITS turn. + // A forecast recorded before `turns` existed has none, and there is nothing to subtract from -- + // scoring it against an absolute turn number would manufacture an error rather than measure one, + // so it is left open instead. + const actual = elapsed !== null ? elapsed + : (Number.isFinite(open.turns) ? atTurn - open.turns : null); + // AT LEAST ONE TURN MUST HAVE PASSED. `atTurn` is the transcript's current turn count and + // `open.turns` the count at prediction time, so a truncated, rotated or replaced transcript + // makes the difference zero or negative. Recording that as ground truth feeds a large negative + // `error` into `reliability`, which shifts `bucket.bias`, which is what `calibrate` publishes a + // corrected forecast from -- so one bad transcript would skew every later prediction. Leaving + // the forecast open loses nothing: the next compaction closes it against a sane count. + if (actual === null || !Number.isFinite(actual) || actual < 1) return; + record(dir, { kind: 'forecast-outcome', sessionId, + // Stamped so the next call can see this forecast is spoken for. Without it the set above is + // empty and the same defect returns. + forecastId: open.id, horizon: open.horizon, predictedTurns: open.predictedTurns, - actualTurns, - error: actualTurns - open.predictedTurns, - hit: Math.abs(actualTurns - open.predictedTurns) <= TOLERANCE, + actualTurns: actual, + error: actual - open.predictedTurns, + hit: Math.abs(actual - open.predictedTurns) <= TOLERANCE, }); } @@ -98,7 +172,7 @@ export function observeOutcome(dir, { sessionId, actualTurns }) { * actionable, where "62% accurate" is not. */ export function reliability(dir) { - const outcomes = readMetrics(dir).filter((e) => e.kind === 'forecast-outcome'); + const outcomes = readBalance(dir).filter((e) => e.kind === 'forecast-outcome'); const buckets = {}; for (const horizon of HORIZONS) { @@ -164,7 +238,28 @@ export function calibrate(dir, predictedTurns) { // Refit: shift by the observed bias for this horizon. A bucket that runs long // gets pulled in, and vice versa. - const adjusted = Math.max(0, Math.round(predictedTurns + bucket.bias)); + // + // NOT CLAMPED WITH Math.max, which converted nonsense into a plausible-looking emergency. bias + // is a plain arithmetic mean of signed error, and a mean is not robust to the shape the + // 50%-hit-rate floor admits: a near bucket with errors [0,0,0,0,-30,-30,-30,-30] has four hits, + // so it clears the floor and is called calibrated, and its bias is -15. A raw forecast of 3 + // turns then became Math.max(0, Math.round(3 - 15)) = 0 and was published as fact, with the + // note "within 3 on 50% of 8 past forecasts" -- a zero-turn deadline the forecaster never + // produced, from a bucket that is wrong half the time. + // + // A correction at least as large as the thing it corrects is not a correction. + const adjusted = Math.round(predictedTurns + bucket.bias); + if (Math.abs(bucket.bias) >= predictedTurns || adjusted < 1) { + return { + publishable: false, + predictedTurns, + horizon, + bias: bucket.bias, + reason: `the observed bias at this horizon (${Math.round(bucket.bias)} turns) is as large as ` + + `the forecast itself (${predictedTurns}), so the correction would replace it rather than ` + + 'adjust it', + }; + } return { publishable: true, diff --git a/plugin/hooks/lib/forecast.mjs b/plugin/hooks/lib/forecast.mjs index aed24df29..474ed84f9 100644 --- a/plugin/hooks/lib/forecast.mjs +++ b/plugin/hooks/lib/forecast.mjs @@ -31,13 +31,30 @@ * other rather than one standing in for the other. */ -import { readMetrics } from './metrics.mjs'; +import { readMetrics, readBalance, isFixtureAnchor, BALANCE_KINDS } from './metrics.mjs'; import { aggregateConsolidation } from './consolidate.mjs'; import { previewQuality } from './expand.mjs'; +import { calibrate } from './calibration.mjs'; /** Turns of headroom below which the runway is worth interrupting for. */ export const ACTIONABLE_RUNWAY = 8; +/** + * Held-out touches required before the runway counterfactual is published. + * + * The header of this file promises the counterfactual appears "only once the holdout carries + * volume", and the fallback string says "the control arm needs more volume" -- but the only check + * was `if (!rows.length) return null`, so "volume" meant "at least one". With a single withheld + * touch, withheldCost IS that one touch's downstream read total: one held-out read of a + * 3,000-line module set savedPerTouch to several thousand tokens and published a counterfactual + * an order of magnitude off, as a bare fact with no caveat. + * + * Ten is a floor rather than a statistically derived threshold, and is deliberately stated as + * such where it surfaces: the panel prints the held-out count against it, so a reader can see how + * close the arm is instead of being told only that it is short. + */ +export const MIN_CONTROL_TOUCHES = 10; + /** * Tokens per turn, and how many the graph is removing. * @@ -57,13 +74,42 @@ export function burnRate(events) { reads.get(key).push(event); } + // EACH READ IS CHARGED TO EXACTLY ONE INJECTION. + // + // The windows used to run from each injection to the end of the log, so they overlapped + // completely: for anchor A with injections i1..i5 interleaved with five reads of 100 tokens, + // i1 saw 500, i2 400, i3 300, i4 200, i5 100 -- 1500 / 5 = 300 against a true per-touch cost of + // 100. A 3x inflation growing with the repeat count. + // + // It does not cancel between the arms. metrics.mjs makes the holdout deterministic in + // (anchor, epoch), so every repeat touch of a file lands in the SAME arm -- whichever arm drew + // the heavily re-touched anchors is inflated and the other is not. savedPerTouch's sign and + // magnitude were therefore driven by which files happened to be held out, and that value is + // what the published counterfactual divides by. + // + // Bounding each window at the next injection of the same (session, anchor) -- across BOTH arms, + // since an injection ends the previous window regardless of which arm it is in -- makes the + // charge exact. + const nextInjectAfter = new Map(); + for (const row of injects) { + const key = `${row.sessionId || ''}|${row.anchor}`; + if (!nextInjectAfter.has(key)) nextInjectAfter.set(key, []); + nextInjectAfter.get(key).push(row.at ?? 0); + } + for (const times of nextInjectAfter.values()) times.sort((a, b) => a - b); + const downstream = (rows) => { if (!rows.length) return null; let total = 0; for (const row of rows) { - const bucket = reads.get(`${row.sessionId || ''}|${row.anchor}`) || []; + const key = `${row.sessionId || ''}|${row.anchor}`; + const bucket = reads.get(key) || []; const after = row.at ?? 0; - total += bucket.reduce((sum, r) => sum + ((r.at ?? 0) >= after ? (r.tokens || 0) : 0), 0); + const until = (nextInjectAfter.get(key) || []).find((t) => t > after) ?? Infinity; + total += bucket.reduce( + (sum, r) => sum + ((r.at ?? 0) >= after && (r.at ?? 0) < until ? (r.tokens || 0) : 0), + 0, + ); } return total / rows.length; }; @@ -98,12 +144,27 @@ export function burnRate(events) { export function shadowAvoided(events) { let avoided = 0; let spent = 0; + let seen = 0; for (const event of events) { if (event.kind !== 'substitute') continue; + // FIXTURES EXCLUDED, as balanceSheet already does. metrics.mjs records the measurement: + // "366 of 370 substitutions pointed at the enforcement suite's own big.ts fixture under a + // temp dir, and counting them made the product look like it had avoided 40 MB of reads. It + // had avoided 154 KB." Running the test suite against a real graph inflated `avoided` by two + // orders of magnitude, which blew up shadow.net, pushed `spread` past 0.5 and fired the + // divergence note with a fabricated number -- so the panel's own credibility check was the + // thing the fixture noise corrupted. + if (isFixtureAnchor(event.anchor)) continue; + seen += 1; avoided += Math.ceil((event.bytesAvoided || 0) / 4); spent += event.tokens || 0; } - return avoided ? { avoided, spent, net: avoided - spent } : null; + // GUARDED ON WHETHER ANYTHING WAS SEEN, not on whether it was favourable. Guarding on `avoided` + // meant twenty substitutions that each spent 20 tokens and avoided nothing -- a measured net + // loss of 400 -- returned null, so the divergence block never ran and the panel said nothing + // about the shadow arm. The one outcome the function could not express was the one unfavourable + // to the product, collapsed into the same return value as "no data". + return seen ? { avoided, spent, net: avoided - spent, substitutions: seen } : null; } /** @@ -112,8 +173,10 @@ export function shadowAvoided(events) { * @param used Tokens of context consumed so far. * @param capacity Context window size. * @param turns Turns taken to consume it. + * @param touches Intercepted touches in this session. Required for the counterfactual: it is + * what converts a per-TOUCH saving into a per-TURN one. */ -export function runway({ used, capacity, turns }, rate) { +export function runway({ used, capacity, turns, touches }, rate) { if (!capacity || !turns || used == null) return null; const perTurn = used / turns; @@ -121,15 +184,53 @@ export function runway({ used, capacity, turns }, rate) { const remaining = Math.max(0, capacity - used); const withGraph = Math.floor(remaining / perTurn); + const base = { withGraph, perTurn: Math.round(perTurn) }; - // Without the graph each turn costs what it costs now PLUS what the graph is - // removing. Only computable when the control arm has spoken. const saved = rate?.savedPerTouch; - const withoutGraph = saved != null && saved > 0 - ? Math.floor(remaining / (perTurn + saved)) - : null; + if (saved == null) return { ...base, withoutGraph: null, counterfactual: 'unmeasured' }; + + // VOLUME, MEANING VOLUME. See MIN_CONTROL_TOUCHES. + const withheld = rate.withheldTouches ?? 0; + if (withheld < MIN_CONTROL_TOUCHES) { + return { ...base, withoutGraph: null, counterfactual: 'thin-control', withheld }; + } + + // UNITS. `perTurn` is tokens per TURN for this session; `savedPerTouch` is an average over + // injection EVENTS across the whole metrics log. Adding them directly was only correct with + // exactly one intercepted touch per turn, which is not the usual case in either direction. + // + // Session {used 60000, capacity 200000, turns 30} gives perTurn 2000 and remaining 140000. With + // 5 injections over those 30 turns and savedPerTouch 1500, the true per-turn saving is 250, so + // the honest counterfactual is floor(140000/2250) = 62. Adding the per-touch figure gave + // floor(140000/3500) = 40, and the panel published "~70 turns to compaction; without the graph, + // ~40" -- claiming the graph nearly doubled the runway when it added about eight turns. + // + // Without a touch count there is no density to convert with, so the counterfactual is withheld + // rather than computed on the one-touch-per-turn assumption. + if (!touches) return { ...base, withoutGraph: null, counterfactual: 'no-touch-density' }; + + const savedPerTurn = saved * (touches / turns); + + // A MEASURED NON-POSITIVE SAVING IS A RESULT, NOT A MISSING ONE. The old `saved > 0` guard + // discarded a measurement: with withheldCost 1000, treatedCost 900 and injectionCost 200, + // savedPerTouch is -100 -- the control arm has spoken and said the graph is a net cost -- and + // the panel printed "no counterfactual yet, the control arm needs more volume". The one result + // unfavourable to the product was the one result the panel could not show, disguised as + // insufficient data. The runway is longer without the graph in that case, and says so. + const denominator = perTurn + savedPerTurn; + if (denominator <= 0) { + // The graph is measured to be removing more per turn than the session is spending, which is + // arithmetically possible and not interpretable as a runway. Reported as such. + return { ...base, withoutGraph: null, counterfactual: 'implausible', savedPerTurn }; + } - return { withGraph, withoutGraph, perTurn: Math.round(perTurn) }; + return { + ...base, + withoutGraph: Math.floor(remaining / denominator), + counterfactual: savedPerTurn > 0 ? 'saving' : 'costing', + savedPerTurn, + withheld, + }; } /** @@ -141,21 +242,99 @@ export function runway({ used, capacity, turns }, rate) { * confident figure derived from four samples -- is precisely what this project * criticises elsewhere. */ +/** + * The events the forecast reasons about, each kind read from the log that keeps it. + * + * Reads come from readMetrics, which applies MAX_EVENTS and a tail byte cap -- correct, because + * reads are the high-volume kind and only the recent ones describe the current burn. + * + * Inject, harvest and substitute come from readBalance, which is exempt from that window and + * exists for precisely this reason. metrics.mjs records what the window does to them, measured in + * this repository: "44 inject records in the file, 9 of them holdout, all at lines 60-76 of + * 9,058 -- every single one outside the window. report() therefore said 0 holdout." + * + * The 10% holdout arm is the rarer kind, so it ages out FIRST. On any graph with more than 5,000 + * events since the last injection burst, downstream(withheld) returned null, savedPerTouch was + * null, and the panel printed "no counterfactual yet -- the control arm needs more volume" while + * balance.jsonl held the control arm in full. The panel blamed insufficient data for a read-path + * bug; in the partial case it computed the published counterfactual from whichever slice of the + * control arm happened to survive the window. + */ +export function balanceAwareEvents(dir) { + const balance = readBalance(dir); + // No id-matching needed, and deliberately so: readBalance already reads balance.jsonl in full, + // migrates the pre-split records out of metrics.jsonl WITHOUT the window, and dedupes the two + // on record id (falling back to a composite for records written before ids existed). It is + // therefore a strict superset of the balance kinds in the windowed read, so the windowed copies + // are dropped outright. Trying to merge them back by id would double-count exactly the legacy + // records that have no id. + const out = readMetrics(dir).filter((e) => !BALANCE_KINDS.has(e?.kind)); + out.push(...balance); + return out; +} + +/** How the runway reads, given what the control arm was actually able to say. */ +function runwayLine(air) { + const head = `~${air.withGraph} turns to compaction`; + switch (air.counterfactual) { + case 'saving': + return `${head}; without the graph, ~${air.withoutGraph}.`; + case 'costing': + // Said plainly rather than suppressed. A tool that can only report results in its own + // favour is not measuring anything. + return `${head}; without the graph, ~${air.withoutGraph} -- the graph is measurably NOT ` + + 'extending the runway here.'; + case 'thin-control': + return `${head} (no counterfactual yet -- the control arm holds ${air.withheld} of the ` + + `${MIN_CONTROL_TOUCHES} held-out touches needed).`; + case 'no-touch-density': + return `${head} (no counterfactual -- this session's intercepted-touch count is unknown, ` + + 'and a per-touch saving cannot be converted to a per-turn one without it).'; + case 'implausible': + return `${head} (no counterfactual -- the measured saving exceeds this session's whole ` + + 'per-turn cost, so the arithmetic does not describe a runway).'; + default: + return `${head} (no counterfactual yet -- the control arm has not spoken).`; + } +} + export function forecastPanel(dir, session = {}, findings = []) { - const events = readMetrics(dir); + const events = balanceAwareEvents(dir); const rate = burnRate(events); const shadow = shadowAvoided(events); const consolidation = aggregateConsolidation(findings); - const air = runway(session, rate); + // The touch count converts the per-touch saving into a per-turn one. Taken from the session + // when the caller knows it, and otherwise from the treated touches this rate was built from. + const touches = session.touches ?? rate?.treatedTouches ?? 0; + const air = runway({ ...session, touches }, rate); const lines = []; const parts = {}; if (air) { parts.runway = air; - lines.push(air.withoutGraph != null - ? `~${air.withGraph} turns to compaction; without the graph, ~${air.withoutGraph}.` - : `~${air.withGraph} turns to compaction (no counterfactual yet -- the control arm needs more volume).`); + + // THE FORECAST KEEPS ITS OWN SCORE -- from here, because this is the only place a forecast is + // actually made. calibration.mjs was written for exactly this and had zero shipped importers: + // no forecast was ever logged, no outcome ever observed, reliability always saw an empty set, + // and calibrate was unreachable. So the shipped panel printed precisely the uncalibrated + // number that module's docstring calls "a vibe with a typeface". + // + // NOT LOGGED HERE. Building a panel is not making a forecast. maybeSurface builds one per + // throttle window and worthSurfacing rejects most of them, so logging on every build appended + // an open forecast record nobody ever saw. observeOutcome closes only the LAST open forecast + // for a session, so every earlier one stayed open forever -- and they accumulate in + // balance.jsonl, whose tail-bytes read is precisely what would then displace the + // inject/harvest/substitute rows BALANCE_KINDS exists to protect. That would have undone this + // change's own fix. The surfacing path logs instead, at the one moment a prediction is + // actually made to somebody. + const score = calibrate(dir, air.withGraph); + parts.calibration = score; + // Published only when the horizon has earned it. An unpublishable score does not replace the + // number -- it is simply absent, exactly as the uncalibrated case reads today. + lines.push(score.publishable + ? `${runwayLine({ ...air, withGraph: score.predictedTurns })}\n (corrected: ${score.note})` + : runwayLine(air)); } if (rate) { diff --git a/plugin/hooks/lib/metrics.mjs b/plugin/hooks/lib/metrics.mjs index 3fc52e22b..4b0eb2712 100644 --- a/plugin/hooks/lib/metrics.mjs +++ b/plugin/hooks/lib/metrics.mjs @@ -63,7 +63,22 @@ const metricsPath = (dir) => join(dir, 'metrics.jsonl'); const balancePath = (dir) => join(dir, 'balance.jsonl'); /** Kinds the net balance is computed from, and which therefore must survive. */ -const BALANCE_KINDS = new Set(['inject', 'harvest', 'substitute']); +/** + * Kinds written to balance.jsonl as well as the firehose, so the event window can never + * starve the measurement. Exported because any reader that needs the unwindowed truth needs to + * know which kinds it applies to, and a second copy of this set elsewhere would drift. + */ +export const BALANCE_KINDS = new Set([ + 'inject', + 'harvest', + 'substitute', + // The forecast's own score. Both kinds are rare and accumulate slowly -- MIN_SCORED is 8 -- + // while the firehose they shared is dominated by per-tool-call records, so they were evicted + // faster than they could accumulate: calibrate() returned 'not yet calibrated (n/8)' forever + // and an outcome whose forecast had scrolled away was dropped with no record at all. + 'forecast', + 'forecast-outcome', +]); /** * Is this touch in the holdout arm? diff --git a/plugin/hooks/lib/policy.mjs b/plugin/hooks/lib/policy.mjs index b3871c27d..f89a48210 100644 --- a/plugin/hooks/lib/policy.mjs +++ b/plugin/hooks/lib/policy.mjs @@ -272,7 +272,7 @@ function statePath(sessionId, agent) { /** A usable state object, whatever was on disk. */ function emptyState() { - return { seen: {}, denied: {}, injected: [], actCounts: {} }; + return { seen: {}, denied: {}, injected: [], actCounts: {}, forecast: null }; } /** @@ -312,6 +312,22 @@ export function loadState(sessionId, agent) { parsed.actCounts && typeof parsed.actCounts === 'object' && !Array.isArray(parsed.actCounts) ? parsed.actCounts : {}, + // THE SAME REASON A THIRD TIME. The forecast throttle records when the panel was last + // computed and what runway it last SHOWED, and both are meaningless within a single hook + // process -- every tool call is a new one. Dropped here, `checkedAt` would reset on every + // call and the panel would rebuild itself, transcript parse and all, on each tool use; and + // `shown` would reset, so worthSurfacing would compare against nothing and re-interrupt with + // the same runway forever. + // AND ITS TIMESTAMP MUST BE A NUMBER. The shape check alone accepted `checkedAt: "1700"`, + // and both the throttle comparison and the merge below order by it -- so a persisted numeric + // STRING compares by lexicographic coercion and can beat a later real timestamp, freezing + // the throttle open or shut. Same class as every other field validated here: a value that + // parsed is not a value that means anything. + forecast: + parsed.forecast && typeof parsed.forecast === 'object' && !Array.isArray(parsed.forecast) + && Number.isFinite(parsed.forecast.checkedAt) + ? parsed.forecast + : null, }; } catch { return emptyState(); @@ -383,6 +399,19 @@ export function saveState(sessionId, state, agent) { } return out; })(), + // LATEST CHECK WINS, which is the opposite direction from the fields above and correct for + // this one. `seen`, `denied`, `injected` and `actCounts` are all append-only, so a union or + // a max is the safe merge. The forecast throttle is a POINT IN TIME: taking the older of two + // concurrent checks would re-open the window and let the panel be rebuilt immediately, which + // is the cost the throttle exists to bound. + forecast: (() => { + const mine = state.forecast || null; + const theirs = current.forecast || null; + const stamp = (f) => (Number.isFinite(f?.checkedAt) ? f.checkedAt : null); + if (stamp(mine) === null) return theirs; + if (stamp(theirs) === null) return mine; + return stamp(mine) >= stamp(theirs) ? mine : theirs; + })(), }; // Write-then-rename so a reader never observes a half-written file. diff --git a/plugin/hooks/lib/surface.mjs b/plugin/hooks/lib/surface.mjs new file mode 100644 index 000000000..4054c8cf9 --- /dev/null +++ b/plugin/hooks/lib/surface.mjs @@ -0,0 +1,185 @@ +// GENERATED FILE -- do not edit. +// Source of truth: hooks-core/surface.mjs. Regenerate with `npm run sync:hooks`. +/** + * The entry point the forecast never had. + * + * forecast.mjs computes a runway, calibration.mjs scores it, and worthSurfacing decides whether a + * change is worth interrupting for -- and none of it ran. The reachability guard listed + * forecastPanel, worthSurfacing, logForecast and observeOutcome as unwired, which is a polite way + * of saying the feature did not exist for any user. This module is the missing half: it derives + * the session numbers the panel needs from the transcript, decides when the panel has earned an + * interruption, and closes the calibration loop when compaction fires. + * + * THREE COSTS ARE BOUNDED HERE, because this runs on the PreToolUse path and every tool call is a + * separate process: + * + * THE READ readCacheUsage parses up to 4 MB of transcript. Doing that on every tool call + * would tax the hook that exists to save the user money. + * THE COMPUTE the panel walks the metrics log, the balance log and the graph. + * THE CONTEXT the panel is only worth its own tokens when it says something actionable. + * + * So the throttle is checked FIRST, from persisted state, before any file is opened. Everything + * expensive happens after a clock comparison. + * + * NOTHING HERE IS ALLOWED TO BREAK A TOOL CALL. Every entry point returns null on failure rather + * than throwing: a forecast is a courtesy, and the hook's promise is that a defect in it costs the + * user nothing. + */ + +import { forecastPanel, worthSurfacing } from './forecast.mjs'; +import { observeOutcome, logForecast } from './calibration.mjs'; +import { readCacheUsage } from './cache.mjs'; +import { readBalance } from './metrics.mjs'; + +/** + * How long between two attempts to build the panel. + * + * Not a tuning knob so much as an admission: the panel costs a transcript parse, and a user makes + * many tool calls per minute. Two minutes is short enough that a runway falling into single digits + * is still caught with turns to spare, and long enough that the check is free in aggregate. + */ +export const SURFACE_INTERVAL_MS = + Number(process.env.TOKEN_OPTIMIZER_SURFACE_INTERVAL_MS) || 120_000; + +/** + * The context window to measure the runway against. + * + * NOT DISCOVERABLE from the transcript: the usage rows carry what was spent, never the ceiling it + * was spent against. Defaulting is therefore unavoidable, so the default is stated rather than + * buried, and overridable for anyone running a different window. + */ +export const DEFAULT_CAPACITY = + Number(process.env.TOKEN_OPTIMIZER_CONTEXT_CAPACITY) || 200_000; + +/** + * What the session has spent, read from its own transcript. + * + * `used` is the FULL input size of the most recent request -- cache reads plus cache writes plus + * fresh input -- which is what the context window actually holds, rather than the marginal cost of + * that one turn. Summing the per-turn inputs instead would count the same carried prefix once per + * turn and report a number many times the window size. + * + * Returns null rather than a zeroed object when there is nothing to measure, so a caller cannot + * mistake "no data" for "an empty context". + */ +export function sessionUsage(transcriptPath, { capacity = DEFAULT_CAPACITY } = {}) { + let turns; + try { + turns = readCacheUsage(transcriptPath); + } catch { + return null; + } + if (!turns?.length) return null; + + const last = turns[turns.length - 1]; + const used = (last.read || 0) + (last.written || 0) + (last.input || 0); + if (!used) return null; + + return { used, capacity, turns: turns.length }; +} + +/** Intercepted touches recorded for this session, which converts a per-touch saving to per-turn. */ +function touchesFor(dir, sessionId) { + try { + return readBalance(dir).filter((e) => e.kind === 'inject' && e.sessionId === sessionId).length; + } catch { + return 0; + } +} + +/** + * Should the forecast interrupt right now, and with what? + * + * The state object is the caller's persisted per-session state; this reads `forecast` from it and + * returns the value to store back, so the throttle and the last-shown runway survive across hook + * processes. Returning the next state rather than mutating it keeps this testable without a + * filesystem. + * + * @returns { text, state } -- text is null when nothing should be shown. + */ +export function maybeSurface(dir, { + transcriptPath, sessionId, state = {}, now = Date.now(), findings = [], +} = {}) { + const previous = state.forecast || null; + + // THE THROTTLE IS FIRST, and deliberately before any I/O. Everything below opens files. + if (previous?.checkedAt && now - previous.checkedAt < SURFACE_INTERVAL_MS) { + return { text: null, state: previous }; + } + + let usage; + try { + usage = sessionUsage(transcriptPath); + } catch { + return { text: null, state: previous }; + } + // The clock is stamped even when there is nothing to measure, so a session with no usage rows + // does not re-attempt the transcript parse on every single tool call. + if (!usage) return { text: null, state: { ...previous, checkedAt: now } }; + + let panel; + try { + panel = forecastPanel(dir, { + ...usage, + sessionId, + touches: touchesFor(dir, sessionId), + }, findings); + } catch { + return { text: null, state: { ...previous, checkedAt: now } }; + } + if (!panel) return { text: null, state: { ...previous, checkedAt: now } }; + + // worthSurfacing compares against the last panel that was actually SHOWN, not the last one + // computed. Comparing against the last computed panel would let the runway drift down past the + // threshold one throttle window at a time and never trip the "crossed it" test. + const shown = previous?.shown ? { parts: { runway: { withGraph: previous.shown } } } : null; + if (!worthSurfacing(panel, shown)) { + return { text: null, state: { ...previous, checkedAt: now } }; + } + + // LOGGED HERE, AND ONLY HERE -- past the throttle and past worthSurfacing, so exactly one record + // is written per prediction a person is actually shown. forecastPanel used to log on every build; + // since most builds are rejected above, that appended open forecast records nobody saw, and + // observeOutcome closes only the newest one per session. The rest stayed open forever and piled + // up in balance.jsonl, whose tail-bytes read would then evict the inject/harvest/substitute rows + // BALANCE_KINDS protects -- undoing the very repair this work exists to make. + // + // The RAW runway is logged, not the calibrated one: scoring a corrected number would fold each + // correction into the next and the bias would chase its own tail. + try { + logForecast(dir, { + sessionId, + predictedTurns: panel.parts.runway.withGraph, + used: usage.used, + capacity: usage.capacity, + turns: usage.turns, + }); + } catch { /* a forecast is a courtesy; failing to score it must not cost a tool call */ } + + return { + text: panel.text, + state: { checkedAt: now, shown: panel.parts.runway.withGraph }, + }; +} + +/** + * Closes the calibration loop, at the only moment that can close it. + * + * Compaction firing IS the ground truth a runway forecast was predicting, so this belongs on the + * PreCompact path and nowhere else. `actualTurns` is turns ELAPSED between the prediction and + * compaction, which is what predictedTurns meant -- so the forecast has to have recorded the turn + * it was made on, and observeOutcome subtracts. + * + * Silent by design: nothing is shown to anybody. It writes one record so that the NEXT session's + * forecast can be published with a track record instead of as a bare assertion. + */ +export function closeForecast(dir, { transcriptPath, sessionId } = {}) { + try { + const usage = sessionUsage(transcriptPath); + if (!usage) return false; + observeOutcome(dir, { sessionId, atTurn: usage.turns }); + return true; + } catch { + return false; + } +} diff --git a/plugin/hooks/precompact-optimize.mjs b/plugin/hooks/precompact-optimize.mjs index d546aa9dc..f3f436190 100644 --- a/plugin/hooks/precompact-optimize.mjs +++ b/plugin/hooks/precompact-optimize.mjs @@ -24,6 +24,7 @@ import { join } from 'node:path'; import { mode, MODE_OFF, loadState, clearSeen } from './lib/policy.mjs'; import { linkCoOccurrence } from './lib/inject.mjs'; import { wikiDir, projectRootFor } from './lib/wiki.mjs'; +import { closeForecast } from './lib/surface.mjs'; /** Longest compaction may be delayed. Past this the work is abandoned. */ const TIMEOUT_MS = @@ -125,6 +126,31 @@ async function main() { // silent no-op until a test caught it. clearSeen(payload.session_id, payload.transcript_path || null); + // THE GROUND TRUTH, AT THE ONLY MOMENT IT EXISTS. + // + // Compaction firing is exactly the event every runway forecast was predicting, so this is the + // one place the calibration loop can be closed. Without a caller here the loop collected + // predictions and never scored one: reliability saw an empty set forever and calibrate returned + // "not yet calibrated" for the life of the project, which is indistinguishable from the feature + // not existing. + // + // Silent, and before the wrapper check: nothing is shown to anybody, and a plugin-only install + // returns early below, so putting it after would have excluded every such user from the + // measurement -- the same shape as the co-occurrence write above. + try { + // THE SAME RESOLVER THE PreToolUse ROUTER USES. projectRootFor starts its marker walk from + // `dirname(canonicalPath(filePath))`, so handing it a DIRECTORY starts the walk one level + // above that directory -- and closeForecast would read a different graph than maybeSurface + // wrote to, silently scoring nothing. The router passes a file path and resolves per file; + // here the equivalent is to anchor inside the cwd rather than at it. + closeForecast(wikiDir(projectRootFor(join(payload.cwd || process.cwd(), 'x'), payload.cwd)), { + transcriptPath: payload.transcript_path, + sessionId: payload.session_id, + }); + } catch { + // Scoring a forecast must never delay compaction. + } + // NOW the wrapper, which only the optimize_session spawn needs. Plugin-only // installs stop here having still recorded their co-occurrence above. const wrapper = findWrapper(); diff --git a/plugin/hooks/pretooluse-router.mjs b/plugin/hooks/pretooluse-router.mjs index 6edf23136..abcdcfea8 100644 --- a/plugin/hooks/pretooluse-router.mjs +++ b/plugin/hooks/pretooluse-router.mjs @@ -32,6 +32,7 @@ import { commandProjectRoot, } from './lib/decide.mjs'; import { recordRead, fingerprint } from './lib/metrics.mjs'; +import { maybeSurface } from './lib/surface.mjs'; import { wikiDir, load, @@ -252,6 +253,28 @@ try { // their tool call, so a failure falls through to a plain allow. } + // THE FORECAST, AND THE ONLY PLACE IT CAN REACH A USER. + // + // This is the mid-session hook -- SessionStart is too early for a runway to mean anything and + // PreCompact is too late to act on one -- so it is where a deadline can still change what + // somebody does next. Everything about the cost is bounded inside maybeSurface: the throttle + // is checked from persisted state BEFORE any file is opened, and worthSurfacing withholds the + // panel unless the runway is actionable and has moved since the last one shown. + try { + const surfaced = maybeSurface(dirFor(payload.cwd || process.cwd()), { + transcriptPath: payload.transcript_path, + sessionId: payload.session_id, + state, + }); + if (surfaced.state !== state.forecast) { + state.forecast = surfaced.state; + saveState(payload.session_id, state, agentScope); + } + if (surfaced.text) context = context ? `${context}\n\n${surfaced.text}` : surfaced.text; + } catch { + // Same rule as above: a forecast is a courtesy and must never cost a tool call. + } + // THE MEMORY HALF. `harvest` records that this file was touched, at this // content hash, by this task -- and nothing had been calling it, anywhere. // The graph therefore accumulated no nodes and no task edges from ordinary diff --git a/tests/hooks/forecast.test.mjs b/tests/hooks/forecast.test.mjs index 93acfbbf3..6427e355c 100644 --- a/tests/hooks/forecast.test.mjs +++ b/tests/hooks/forecast.test.mjs @@ -14,11 +14,12 @@ import { tmpdir } from 'node:os'; import { record, recordRead } from '../../hooks-core/metrics.mjs'; import { burnRate, runway, shadowAvoided, forecastPanel, worthSurfacing, ACTIONABLE_RUNWAY, + MIN_CONTROL_TOUCHES, balanceAwareEvents, } from '../../hooks-core/forecast.mjs'; import { logForecast, observeOutcome, reliability, calibrate, } from '../../hooks-core/calibration.mjs'; -import { readMetrics } from '../../hooks-core/metrics.mjs'; +import { readMetrics, readBalance } from '../../hooks-core/metrics.mjs'; let dir; @@ -58,8 +59,13 @@ describe('the causal delta comes from the arms, not a model', () => { describe('the runway is a deadline, with a counterfactual half', () => { test('both halves appear once the control arm has spoken', () => { - seedArms(); - const air = runway({ used: 60_000, capacity: 200_000, turns: 30 }, burnRate(readMetrics(dir))); + // The fixture now supplies what this test's own name asserts: a control arm carrying enough + // held-out touches to have spoken, and the session's intercepted-touch count. Both are new + // preconditions, and both are asserted on their own below. + seedArms({ treated: 30, withheld: MIN_CONTROL_TOUCHES + 2 }); + const air = runway( + { used: 60_000, capacity: 200_000, turns: 30, touches: 30 }, burnRate(readMetrics(dir)), + ); expect(air.withGraph).toBeGreaterThan(0); // The half no tool without a holdout can produce. expect(air.withoutGraph).toBeLessThan(air.withGraph); @@ -185,3 +191,335 @@ describe('the forecast keeps its own score', () => { expect(rel.buckets.far.hitRate).toBe(0); }); }); + +// --- the panel must be able to publish a result that is bad for the product -------- + +describe('each read is charged to exactly one injection', () => { + test('five injections of one anchor cost one read, not five', () => { + // THE DEFECT: every injection's window ran to the end of the log, so the windows overlapped + // completely. i1 saw all five reads, i2 four, and so on -- 1500/5 = 300 against a true + // per-touch cost of 100. A 3x inflation growing with the repeat count, and it does NOT cancel + // between the arms: the holdout is deterministic in (anchor, epoch), so all repeat touches of + // a file land in the same arm. Whichever arm drew the re-touched anchors was inflated. + // EXPLICIT, INTERLEAVED TIMESTAMPS. This depended on Date.now() advancing between each pair + // of writes; if all ten landed in one millisecond every injection would see `until = Infinity`, + // charge all five reads, and perTouch would be 500 rather than 100 -- the test would pass or + // fail on machine speed. The very next test already documents this hazard and works around it. + // record() directly for the read, because recordRead() does not forward `at`. + for (let i = 0; i < 5; i++) { + record(dir, { + kind: 'inject', anchor: '/repeat.ts', sessionId: 's', holdout: false, tokens: 0, + at: 1_000 + i * 100, + }); + record(dir, { + kind: 'read', anchor: '/repeat.ts', sessionId: 's', tokens: 100, + at: 1_000 + i * 100 + 50, + }); + } + const rate = burnRate(readMetrics(dir)); + expect(rate.perTouch).toBe(100); + }); + + test('a read before any injection is charged to none of them', () => { + // Explicit timestamps: record() and recordRead() called back to back land in the same + // millisecond, and a window bounded on `at` cannot order two events that share one. + // record() directly, because recordRead() does not forward `at`. + record(dir, { kind: 'read', anchor: '/early.ts', sessionId: 's', tokens: 1_000, at: 1_000 }); + record(dir, { kind: 'inject', anchor: '/early.ts', sessionId: 's', holdout: false, tokens: 0, at: 2_000 }); + expect(burnRate(readMetrics(dir)).perTouch).toBe(0); + }); +}); + +describe('the counterfactual is not published on a thin control arm', () => { + test('one held-out touch is not volume', () => { + // THE DEFECT: the only volume check was `if (!rows.length) return null`, so a single withheld + // touch published '~70 turns to compaction; without the graph, ~40' as a bare fact -- while + // the header promises the counterfactual appears 'only once the holdout carries volume' and + // the fallback string says 'the control arm needs more volume'. + seedArms({ treated: 30, withheld: 1 }); + const air = runway( + { used: 60_000, capacity: 200_000, turns: 30, touches: 30 }, burnRate(readMetrics(dir)), + ); + expect(air.withoutGraph).toBeNull(); + expect(air.counterfactual).toBe('thin-control'); + expect(air.withheld).toBe(1); + }); + + test('the panel says how short the arm is, rather than only that it is short', () => { + seedArms({ treated: 30, withheld: 2 }); + const panel = forecastPanel(dir, { used: 60_000, capacity: 200_000, turns: 30, touches: 30 }); + expect(panel.text).toContain(`2 of the ${MIN_CONTROL_TOUCHES} held-out touches needed`); + }); + + test('the threshold is a real bound', () => { + expect(MIN_CONTROL_TOUCHES).toBeGreaterThan(1); + }); +}); + +describe('a per-touch saving is converted before it is added to a per-turn cost', () => { + test('the counterfactual shrinks as touch density falls', () => { + // THE DEFECT: perTurn is tokens per TURN for this session; savedPerTouch is an average over + // injection EVENTS across the whole log. Adding them was only correct at exactly one + // intercepted touch per turn. At 5 touches over 30 turns the honest counterfactual is 62 and + // the old arithmetic gave 40 -- the panel claiming the graph nearly doubled the runway when + // it added about eight turns. + const rate = { savedPerTouch: 1500, treatedTouches: 5, withheldTouches: MIN_CONTROL_TOUCHES }; + const session = { used: 60_000, capacity: 200_000, turns: 30 }; + + const sparse = runway({ ...session, touches: 5 }, rate); + const dense = runway({ ...session, touches: 30 }, rate); + + expect(sparse.withoutGraph).toBe(62); + expect(dense.withoutGraph).toBe(40); + expect(sparse.withoutGraph).toBeGreaterThan(dense.withoutGraph); + }); + + test('with no touch count the counterfactual is withheld, not assumed', () => { + const rate = { savedPerTouch: 1500, treatedTouches: 5, withheldTouches: MIN_CONTROL_TOUCHES }; + const air = runway({ used: 60_000, capacity: 200_000, turns: 30 }, rate); + expect(air.withoutGraph).toBeNull(); + expect(air.counterfactual).toBe('no-touch-density'); + }); +}); + +describe('a measured loss is reported as a loss', () => { + test('a negative saving yields a counterfactual, not an absent one', () => { + // THE DEFECT: `saved > 0` discarded a MEASUREMENT rather than a missing one. The control arm + // saying the graph is a net cost rendered as 'no counterfactual yet -- the control arm needs + // more volume'. The one result unfavourable to the product was the one result the panel was + // structurally incapable of showing, disguised as insufficient data. + const rate = { savedPerTouch: -100, treatedTouches: 30, withheldTouches: MIN_CONTROL_TOUCHES }; + const air = runway({ used: 60_000, capacity: 200_000, turns: 30, touches: 30 }, rate); + + expect(air.counterfactual).toBe('costing'); + expect(air.withoutGraph).not.toBeNull(); + // Without the graph each turn is CHEAPER, so the runway is longer. + expect(air.withoutGraph).toBeGreaterThan(air.withGraph); + }); + + test('the panel says so in words', () => { + const rate = { savedPerTouch: -100, treatedTouches: 30, withheldTouches: MIN_CONTROL_TOUCHES }; + const air = runway({ used: 60_000, capacity: 200_000, turns: 30, touches: 30 }, rate); + expect(air.counterfactual).toBe('costing'); + // and the same through the panel, which is where a user actually reads it + // A control arm that reads LESS than the treated arm: the graph is measurably a net cost. + // Mild enough that the per-turn saving does not exceed the session's whole per-turn spend, + // which is a different (and separately reported) situation. + seedArms({ treated: 12, withheld: MIN_CONTROL_TOUCHES + 1, treatedRead: 800, withheldRead: 500 }); + const panel = forecastPanel(dir, { used: 60_000, capacity: 200_000, turns: 30, touches: 12 }); + expect(panel.text).toMatch(/measurably NOT extending the runway/); + }); + + test('an unmeasured saving is still distinguishable from a measured zero', () => { + const air = runway({ used: 60_000, capacity: 200_000, turns: 30, touches: 5 }, + { savedPerTouch: null, treatedTouches: 5, withheldTouches: 0 }); + expect(air.counterfactual).toBe('unmeasured'); + expect(air.withoutGraph).toBeNull(); + }); +}); + +describe('the shadow arm reports a net loss instead of nothing', () => { + test('substitutions that avoided nothing report a negative net', () => { + // THE DEFECT: `return avoided ? ... : null` guarded on the FAVOURABLE quantity. Twenty + // substitutions each spending 20 tokens and avoiding nothing -- a measured net loss of 400 -- + // returned null, so the divergence block never ran and the panel said nothing at all. + const events = Array.from({ length: 20 }, () => ({ + kind: 'substitute', anchor: '/src/real.ts', bytesAvoided: 0, tokens: 20, + })); + const shadow = shadowAvoided(events); + expect(shadow).not.toBeNull(); + expect(shadow.net).toBe(-400); + expect(shadow.substitutions).toBe(20); + }); + + test('no substitutions at all is still nothing to say', () => { + expect(shadowAvoided([{ kind: 'read', anchor: '/a.ts', tokens: 10 }])).toBeNull(); + }); + + test('fixture anchors are excluded, as the balance sheet already excludes them', () => { + // Measured in metrics.mjs: 366 of 370 substitutions pointed at the enforcement suite's own + // fixture under a temp dir, making the product look like it had avoided 40 MB. It had avoided + // 154 KB. Counting them here blew up shadow.net, pushed the divergence spread past 0.5 and + // fired the credibility check with a fabricated number. + const fixture = join(tmpdir(), 'to-hooks-abc123', 'big.ts'); + const events = [ + { kind: 'substitute', anchor: fixture, bytesAvoided: 40_000_000, tokens: 10 }, + { kind: 'substitute', anchor: '/src/real.ts', bytesAvoided: 4_000, tokens: 10 }, + ]; + const shadow = shadowAvoided(events); + expect(shadow.substitutions).toBe(1); + expect(shadow.avoided).toBe(1_000); + }); +}); + +describe('the balance kinds are read from the log that is not windowed', () => { + test('the control arm survives a firehose long enough to have evicted it', () => { + // THE DEFECT: the panel was built from readMetrics, which applies MAX_EVENTS and a tail byte + // cap. metrics.mjs records what that does, measured in this repository: '44 inject records in + // the file, 9 of them holdout, all at lines 60-76 of 9,058 -- every single one outside the + // window.' The 10% holdout arm is the rarer kind, so it ages out FIRST: downstream(withheld) + // returned null and the panel blamed 'the control arm needs more volume' for a read-path bug, + // while balance.jsonl held the arm in full. + seedArms({ treated: 12, withheld: MIN_CONTROL_TOUCHES + 2 }); + + // Bury the injections under more read events than the window will hold. + for (let i = 0; i < 5_200; i++) recordRead(dir, { anchor: `/noise${i}.ts`, sessionId: 's', bytes: 40 }); + + const windowed = burnRate(readMetrics(dir)); + const complete = burnRate(balanceAwareEvents(dir)); + + // The windowed read has lost the arms entirely; the balance-aware read has not. + expect(windowed).toBeNull(); + expect(complete).not.toBeNull(); + expect(complete.withheldTouches).toBe(MIN_CONTROL_TOUCHES + 2); + expect(complete.savedPerTouch).not.toBeNull(); + }); + + test('balance kinds are not counted twice when both logs hold them', () => { + seedArms({ treated: 4, withheld: 3 }); + const merged = balanceAwareEvents(dir); + expect(merged.filter((e) => e.kind === 'inject')).toHaveLength(7); + }); +}); + +// --- the calibration loop is wired, and cannot score one prediction twice ----------- + +describe('a forecast is closed once', () => { + test('two compactions in one session do not score the same prediction twice', () => { + // THE DEFECT: the open-forecast filter was `!e.scored`, but nothing ever writes `scored` -- + // record() is an append-only JSONL writer with no update path, so a forecast could never gain + // the field and the predicate was always true. A session where compaction fires twice with no + // intervening logForecast popped the SAME forecast twice. reliability counts ROWS, not + // predictions, so MIN_SCORED could be satisfied by one prediction re-closed eight times -- + // exactly the 'threshold from a single sample' this module exists to prevent. The existing + // test used a fresh sessionId per iteration, so it never exposed it. + logForecast(dir, { sessionId: 'one', predictedTurns: 10, used: 1, capacity: 2 }); + observeOutcome(dir, { sessionId: 'one', actualTurns: 11 }); + observeOutcome(dir, { sessionId: 'one', actualTurns: 40 }); + + expect(reliability(dir).scored).toBe(1); + }); + + test('a second prediction in the same session is still scorable', () => { + logForecast(dir, { sessionId: 'one', predictedTurns: 10, used: 1, capacity: 2 }); + observeOutcome(dir, { sessionId: 'one', actualTurns: 11 }); + logForecast(dir, { sessionId: 'one', predictedTurns: 12, used: 1, capacity: 2 }); + observeOutcome(dir, { sessionId: 'one', actualTurns: 13 }); + + expect(reliability(dir).scored).toBe(2); + }); +}); + +describe('forecast events outlive the event window', () => { + test('an outcome still finds its forecast under a firehose', () => { + // THE DEFECT: both readers used readMetrics, which returns at most 2 MB and the last 5,000 + // lines, and forecast events were not protected kinds. On a busy project the forecast had + // scrolled past the tail by the time compaction fired, so the outcome was discarded with no + // record, no error and no counter -- and reliability could never reach MIN_SCORED, leaving + // 'not yet calibrated (n/8)' forever with no way to tell it from having no data at all. + logForecast(dir, { sessionId: 'buried', predictedTurns: 10, used: 1, capacity: 2 }); + for (let i = 0; i < 5_200; i++) { + recordRead(dir, { anchor: `/noise${i}.ts`, sessionId: 'buried', bytes: 40 }); + } + observeOutcome(dir, { sessionId: 'buried', actualTurns: 11 }); + + expect(reliability(dir).scored).toBe(1); + expect(readMetrics(dir).filter((e) => e.kind === 'forecast')).toHaveLength(0); + }); +}); + +describe('a correction never replaces the thing it corrects', () => { + test('a bimodal bucket at the reliability floor refuses to publish', () => { + // THE DEFECT: bias is a plain arithmetic mean of signed error, applied unguarded to any + // bucket clearing hitRate >= 0.5 -- and a mean is not robust to the shape that rule admits. + // Errors [0,0,0,0,-30,-30,-30,-30] give four hits, so the bucket passes the floor and is + // called calibrated, with bias -15. A raw forecast of 3 turns became + // Math.max(0, Math.round(3 - 15)) = 0, published as fact with the note 'within 3 on 50% of 8 + // past forecasts'. The clamp converted nonsense into a plausible-looking emergency. + // POSITIVE errors, so every actualTurns is a sane turn count. The original fixture used -30 + // against a forecast of 3, producing actualTurns of -27 -- which observeOutcome now rejects, + // correctly: a session cannot have run a negative number of turns. +30 gives the identical + // |bias| of 15 and the identical 4-of-8 hit rate, so the property under test is unchanged. + const errors = [0, 0, 0, 0, 30, 30, 30, 30]; + errors.forEach((error, i) => { + logForecast(dir, { sessionId: `b${i}`, predictedTurns: 3, used: 1, capacity: 2, turns: 1 }); + observeOutcome(dir, { sessionId: `b${i}`, actualTurns: 3 + error }); + }); + + const bucket = reliability(dir).buckets.near; + expect(bucket.scored).toBe(8); + expect(bucket.calibrated).toBe(true); // it really does clear the floor + expect(Math.abs(bucket.bias)).toBe(15); + + const out = calibrate(dir, 3); + expect(out.publishable).toBe(false); + expect(out.predictedTurns).toBe(3); // the raw forecast, not a zero + expect(out.reason).toMatch(/as large as the forecast itself/); + }); + + test('a proportionate correction still publishes', () => { + for (let i = 0; i < 10; i++) { + logForecast(dir, { sessionId: `p${i}`, predictedTurns: 12, used: 1, capacity: 2 }); + observeOutcome(dir, { sessionId: `p${i}`, actualTurns: 14 }); + } + const out = calibrate(dir, 12); + expect(out.publishable).toBe(true); + expect(out.predictedTurns).toBe(14); + }); +}); + +describe('the panel actually runs the calibration loop', () => { + const session = { sessionId: 'live', used: 60_000, capacity: 200_000, turns: 30, touches: 30 }; + + test('building a panel does NOT log a forecast', () => { + // Inverted deliberately. calibration.mjs had zero shipped importers, and the first fix logged + // from forecastPanel -- but a panel is built once per throttle window and most builds are + // never shown, so that appended an open forecast record nobody saw. observeOutcome closes only + // the newest per session, so the rest stayed open forever and accumulated in balance.jsonl, + // whose tail-bytes read would then evict the very inject/harvest/substitute rows BALANCE_KINDS + // protects. Logging belongs on the surfacing path; see surface.test.mjs. + forecastPanel(dir, session, []); + expect(readBalance(dir).filter((e) => e.kind === 'forecast')).toHaveLength(0); + }); + + test('the panel still renders a calibration verdict without logging one', () => { + // The read half stays here: calibrate() is what turns the raw runway into a published number. + const panel = forecastPanel(dir, session, []); + expect(panel.parts.calibration).toBeTruthy(); + expect(typeof panel.parts.calibration.publishable).toBe('boolean'); + }); + + test('a calibrated panel publishes the corrected number while the raw one is what gets scored', () => { + // Scoring the corrected number would fold each correction into the next and the bias would + // chase its own tail. The logging half of this now lives in surface.test.mjs, which is where + // the write happens; what stays checkable here is that the two numbers differ as expected. + for (let i = 0; i < 12; i++) { + logForecast(dir, { sessionId: `w${i}`, predictedTurns: 70, used: 1, capacity: 2, turns: 1 }); + observeOutcome(dir, { sessionId: `w${i}`, actualTurns: 72 }); + } + const panel = forecastPanel(dir, session, []); + expect(panel.parts.calibration.publishable).toBe(true); + expect(panel.parts.calibration.raw).toBe(panel.parts.runway.withGraph); + expect(panel.parts.calibration.predictedTurns).not.toBe(panel.parts.calibration.raw); + }); + + test('an uncalibrated horizon prints the number without a track record', () => { + const panel = forecastPanel(dir, session, []); + expect(panel.parts.calibration.publishable).toBe(false); + expect(panel.text).not.toMatch(/corrected:/); + }); + + test('a calibrated horizon prints the corrected number with its track record', () => { + const panel = forecastPanel(dir, session, []); + const raw = panel.parts.runway.withGraph; + // Score enough forecasts at that horizon for the bucket to earn publication. + for (let i = 0; i < 12; i++) { + logForecast(dir, { sessionId: `c${i}`, predictedTurns: raw, used: 1, capacity: 2 }); + observeOutcome(dir, { sessionId: `c${i}`, actualTurns: raw + 2 }); + } + const after = forecastPanel(dir, session, []); + expect(after.parts.calibration.publishable).toBe(true); + expect(after.text).toMatch(/corrected: within 3 on \d+% of \d+ past forecasts/); + expect(after.text).toContain(`~${raw + 2} turns to compaction`); + }); +}); diff --git a/tests/hooks/reachability.test.mjs b/tests/hooks/reachability.test.mjs index 1cdfbf845..f5d9cd547 100644 --- a/tests/hooks/reachability.test.mjs +++ b/tests/hooks/reachability.test.mjs @@ -91,17 +91,18 @@ const ALLOWED = new Map([ // WIRED ELSEWHERE -- leaves this list when that PR merges. ['forTouch', 'WIRED by the injection PR: just-in-time delivery, imported by nothing before it.'], - // A COMPLETE LOOP WITH NO ENTRY POINT. calibration.mjs has no importer at - // all: logForecast and observeOutcome collect the data, reliability scores it - // and calibrate applies the score. Nothing starts it, so the module cannot - // score anything -- and its own docstring says an uncalibrated forecast is - // "a vibe with a typeface". - ['logForecast', 'UNWIRED LOOP: calibration.mjs has zero importers; this records the prediction half.'], - ['observeOutcome', 'UNWIRED LOOP: records the outcome half of a calibration loop nothing starts.'], - - // FORECAST DISPLAY, unreachable while the calibration behind it is unreachable. - ['forecastPanel', 'UNWIRED: renders a forecast whose calibration loop never runs.'], - ['worthSurfacing', 'UNWIRED: decides whether a forecast change is worth showing; same dormant loop.'], + // THE FORECAST IS WIRED, so nothing from it is listed here any more. + // logForecast and calibrate are called by forecastPanel; forecastPanel and + // worthSurfacing by surface.mjs; observeOutcome by surface.closeForecast; and + // surface itself by the PreToolUse router and the PreCompact hook. Five + // entries left this list at once, which is what wiring a whole feature looks + // like -- and is why it was worth doing rather than annotating. + + // SURFACED BY TIGHTENING THE SCAN. Previously counted as reachable only + // because the bare word match read comment prose as a call site. Verified + // orphaned: in shipped code it appears as its own declaration plus two + // mentions in comments, and its only importer is a test. + ['sessionIndex', 'UNWIRED: lists graph keys and truncated claims for a session. Found by the comment-stripping fix; only a test imports it.'], // CONSOLIDATION. forecast.mjs imports aggregateConsolidation from this module, // so it is partly live -- but the selection, the ratio and the content anchor @@ -174,20 +175,71 @@ function exportedFunctions() { return found; } +/** + * Strips comments, so prose cannot be mistaken for a call. + * + * THIS IS WHAT LET A DEAD PUBLIC ENTRY POINT THROUGH. `calibrate` counted as + * reachable because curate.mjs:13 contains the English phrase "the reader's + * ability to calibrate trust", and `reliability` counted because the word + * appears in its own file's prose. calibration.mjs had ZERO importers -- no + * forecast was ever logged, no outcome observed, and the shipped panel printed + * precisely the uncalibrated number that module's docstring calls "a vibe with + * a typeface" -- while the check that exists to catch exactly this reported it + * as wired, on a coincidence of comment wording. + * + * A guard that reads documentation as code is worse than none: it is a clean + * bill of health nobody re-examines. This module's files are heavily commented + * by design, which makes the false-positive rate high rather than incidental. + * + * COMMENTS ONLY, NOT STRING LITERALS -- and that boundary was found the hard + * way. Stripping quotes as well made `routingReport` and `modelSwitchCost` look + * orphaned when routing-tool.ts calls both: three independent global passes + * cannot nest, so an apostrophe inside a DOUBLE-quoted sentence opened a + * "single-quoted string" that ran to the next apostrophe further down the file + * and swallowed the real call sites in between. + * + * Getting that right needs a scanner, and this guard's own rule says why not to + * build one: a check wrong in the permissive direction is merely useless, while + * one wrong in the strict direction fails CI on working code and gets deleted. + * The observed defect was comment prose, comments nest predictably, and that is + * where the line belongs. + */ +function stripComments(text) { + return text + .replace(/\/\*[\s\S]*?\*\//g, ' ') // block comments, including docblocks + .replace(/(^|[^:])\/\/[^\n]*/g, '$1 '); // line comments, sparing the // in a URL +} + /** * Referenced anywhere that ships, other than its own declaration? * - * A bare word match, deliberately. Anything cleverer would need to resolve the - * dynamic `mods..` handles that src/server uses, and a check that - * is wrong in the permissive direction is merely useless -- one that is wrong in - * the strict direction fails CI on working code and gets deleted. + * A bare word match over CODE, deliberately. Anything cleverer would need to + * resolve the dynamic `mods..` handles that src/server uses, and a + * check that is wrong in the permissive direction is merely useless -- one that + * is wrong in the strict direction fails CI on working code and gets deleted. + * COMMENTS are removed first, because prose is not a caller. Strings are NOT -- + * see stripComments above for the measurement that settled that boundary. */ +const codeOnly = new Map(); +function shippedCode(file, text) { + if (!codeOnly.has(file)) codeOnly.set(file, stripComments(text)); + return codeOnly.get(file); +} + +// AN IN-FILE CALLER STILL COUNTS, and the attempt to change that is worth recording. Excluding +// the declaring file outright -- on the reasoning that `reliability` passed only because +// `calibrate` calls it from the same module -- reported 40+ false orphans in one run, among them +// burnRate, runway and shadowAvoided, which forecastPanel calls from inside forecast.mjs. Those +// are genuinely reached; reachability propagates THROUGH an in-file caller. The real defect in the +// reliability case was that its whole module had no importer, which is a module-level question +// this function cannot answer and should not try to. function usedInShippedCode(name, declaringFile) { const word = new RegExp(`\\b${name}\\b`, 'g'); const decl = new RegExp(`export\\s+(?:async\\s+)?function\\s+${name}\\b`, 'g'); for (const { file, text } of usages) { - const uses = (text.match(word) || []).length; - const declared = file === declaringFile ? (text.match(decl) || []).length : 0; + const code = shippedCode(file, text); + const uses = (code.match(word) || []).length; + const declared = file === declaringFile ? (code.match(decl) || []).length : 0; if (uses - declared > 0) return true; } return false; diff --git a/tests/hooks/surface.test.mjs b/tests/hooks/surface.test.mjs new file mode 100644 index 000000000..103f604b2 --- /dev/null +++ b/tests/hooks/surface.test.mjs @@ -0,0 +1,267 @@ +/** + * The entry point the forecast never had. + * + * The properties under test are the ones that decide whether this is a feature or a liability: + * the cost is bounded before any file is opened, the panel only interrupts when it is actionable + * AND has moved since the last one shown, the throttle survives the process boundary that every + * hook invocation crosses, and the calibration loop is closed by the one event that can close it. + * + * The last test in this file is the one that matters most: the reachability guard's own verdict + * that these functions now have shipped callers. Everything else here would pass just as well on + * a module nothing imports -- which is exactly the defect this wiring exists to end. + */ + +import { mkdtempSync, rmSync, writeFileSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { + maybeSurface, closeForecast, sessionUsage, SURFACE_INTERVAL_MS, DEFAULT_CAPACITY, +} from '../../hooks-core/surface.mjs'; +import { logForecast, observeOutcome, reliability } from '../../hooks-core/calibration.mjs'; +import { record, recordRead, readBalance } from '../../hooks-core/metrics.mjs'; + +let workspace; +let dir; + +beforeEach(() => { + workspace = mkdtempSync(join(tmpdir(), 'surface-')); + dir = join(workspace, 'wiki'); +}); +afterEach(() => rmSync(workspace, { recursive: true, force: true })); + +/** + * A transcript with `turns` assistant rows, the last one reporting `used` tokens of context. + * + * The usage shape mirrors a real transcript: the context size at a turn is cache reads plus cache + * writes plus fresh input, not the marginal cost of that turn alone. + */ +function transcript(turns, used = 60_000) { + const path = join(workspace, `t-${turns}-${used}.jsonl`); + const rows = []; + for (let i = 0; i < turns; i++) { + const last = i === turns - 1; + rows.push(JSON.stringify({ + type: 'assistant', + timestamp: new Date(1_700_000_000_000 + i * 1000).toISOString(), + message: { + role: 'assistant', + model: 'claude-opus-4', + usage: { + cache_read_input_tokens: last ? used - 1_000 : 100, + cache_creation_input_tokens: last ? 900 : 10, + input_tokens: last ? 100 : 5, + }, + }, + })); + } + writeFileSync(path, `${rows.join('\n')}\n`); + return path; +} + +/** Arms with a control arm big enough to publish a counterfactual, plus a short runway. */ +function seedArms({ treated = 12, withheld = 12, treatedRead = 500, withheldRead = 9000 } = {}) { + for (let i = 0; i < treated; i++) { + record(dir, { kind: 'inject', anchor: `/t${i}.ts`, sessionId: 'live', holdout: false, tokens: 100 }); + recordRead(dir, { anchor: `/t${i}.ts`, sessionId: 'live', bytes: treatedRead * 4 }); + } + for (let i = 0; i < withheld; i++) { + record(dir, { kind: 'inject', anchor: `/c${i}.ts`, sessionId: 'live', holdout: true, tokens: 0 }); + recordRead(dir, { anchor: `/c${i}.ts`, sessionId: 'live', bytes: withheldRead * 4 }); + } +} + +describe('the session numbers come from the transcript', () => { + test('used is the size of the context, not the sum of the turns', () => { + // Summing per-turn inputs counts the carried prefix once per turn and reports a number many + // times the window size -- which would make every runway look like an emergency. + const usage = sessionUsage(transcript(20, 60_000)); + expect(usage.used).toBe(60_000); + expect(usage.turns).toBe(20); + expect(usage.capacity).toBe(DEFAULT_CAPACITY); + }); + + test('no usage rows is null, not a zeroed session', () => { + const path = join(workspace, 'empty.jsonl'); + writeFileSync(path, '{"type":"user","message":{"role":"user","content":"hi"}}\n'); + expect(sessionUsage(path)).toBeNull(); + }); + + test('a missing transcript is null rather than a throw', () => { + expect(sessionUsage(join(workspace, 'nope.jsonl'))).toBeNull(); + }); +}); + +describe('the cost is bounded before anything is opened', () => { + test('inside the throttle window nothing is read at all', () => { + // The transcript path is deliberately nonexistent: if the throttle were checked after the + // read, this would still return null and the test would pass for the wrong reason. It cannot + // even be attempted, so the state must come back byte-identical. + const previous = { checkedAt: 1_000, shown: 4 }; + const out = maybeSurface(dir, { + transcriptPath: join(workspace, 'never-opened.jsonl'), + sessionId: 'live', + state: { forecast: previous }, + now: 1_000 + SURFACE_INTERVAL_MS - 1, + }); + expect(out.text).toBeNull(); + expect(out.state).toBe(previous); // the same object, untouched + }); + + test('past the window the clock is stamped even when there is nothing to say', () => { + // Otherwise a session with no usage rows re-parses the transcript on every single tool call. + const out = maybeSurface(dir, { + transcriptPath: join(workspace, 'absent.jsonl'), + sessionId: 'live', + state: { forecast: { checkedAt: 1_000, shown: null } }, + now: 1_000 + SURFACE_INTERVAL_MS + 1, + }); + expect(out.text).toBeNull(); + expect(out.state.checkedAt).toBe(1_000 + SURFACE_INTERVAL_MS + 1); + }); + + test('the interval is a real bound', () => { + expect(SURFACE_INTERVAL_MS).toBeGreaterThan(0); + expect(Number.isFinite(SURFACE_INTERVAL_MS)).toBe(true); + }); +}); + +describe('it interrupts only when the forecast has earned it', () => { + test('a comfortable runway says nothing', () => { + seedArms(); + // 10% used over 20 turns leaves a very long runway. + const out = maybeSurface(dir, { + transcriptPath: transcript(20, 20_000), sessionId: 'live', state: {}, now: 5_000, + }); + expect(out.text).toBeNull(); + expect(out.state.checkedAt).toBe(5_000); + }); + + test('a short runway surfaces, once', () => { + seedArms(); + // 190k of 200k used over 20 turns: about 1 turn left. + const path = transcript(20, 190_000); + const first = maybeSurface(dir, { transcriptPath: path, sessionId: 'live', state: {}, now: 5_000 }); + expect(first.text).toMatch(/turns to compaction/); + expect(first.state.shown).toBeGreaterThanOrEqual(0); + + // Same runway a window later: different, not actionable, so it stays quiet. + const second = maybeSurface(dir, { + transcriptPath: path, + sessionId: 'live', + state: { forecast: first.state }, + now: 5_000 + SURFACE_INTERVAL_MS + 1, + }); + expect(second.text).toBeNull(); + }); + + test('the comparison is against the last panel SHOWN, not the last computed', () => { + // Otherwise the runway drifts down one throttle window at a time, each step too small to trip + // the threshold test, and the interruption never fires at all. + seedArms(); + const shownAt = maybeSurface(dir, { + transcriptPath: transcript(20, 190_000), sessionId: 'live', state: {}, now: 5_000, + }); + expect(shownAt.text).not.toBeNull(); + expect(shownAt.state.shown).not.toBeUndefined(); + }); +}); + +describe('the calibration loop is closed by compaction', () => { + test('a forecast made earlier in the session is scored against the elapsed turns', () => { + // predictedTurns is an INTERVAL, so the ground truth is turns elapsed between the prediction + // and compaction -- not the turn number compaction happened on. + logForecast(dir, { sessionId: 'live', predictedTurns: 10, used: 60_000, capacity: 200_000, turns: 20 }); + closeForecast(dir, { transcriptPath: transcript(31, 190_000), sessionId: 'live' }); + + const scored = reliability(dir); + expect(scored.scored).toBe(1); + // 31 turns at compaction minus 20 at prediction = 11 elapsed, against a prediction of 10. + expect(scored.hitRate).toBe(1); + }); + + test('a forecast with no recorded origin is left open rather than scored against an absolute', () => { + // Scoring an interval against a turn NUMBER manufactures an error instead of measuring one. + record(dir, { + kind: 'forecast', sessionId: 'live', predictedTurns: 10, horizon: 'mid', turns: null, + }); + closeForecast(dir, { transcriptPath: transcript(31, 190_000), sessionId: 'live' }); + expect(reliability(dir).scored).toBe(0); + }); + + test('an explicit actualTurns still works, so the interval can be supplied directly', () => { + logForecast(dir, { sessionId: 'live', predictedTurns: 10, used: 1, capacity: 2, turns: 5 }); + observeOutcome(dir, { sessionId: 'live', actualTurns: 12 }); + expect(reliability(dir).scored).toBe(1); + }); + + test('no transcript means no score, and no crash', () => { + logForecast(dir, { sessionId: 'live', predictedTurns: 10, used: 1, capacity: 2, turns: 5 }); + expect(closeForecast(dir, { transcriptPath: join(workspace, 'gone.jsonl'), sessionId: 'live' })).toBe(false); + expect(reliability(dir).scored).toBe(0); + }); +}); + +describe('the wiring is real, not merely present', () => { + // This is the test that would have failed for the whole life of calibration.mjs. Every other + // test in this file passes just as well on a module nothing imports. + const hook = (name) => readFileSync(join(process.cwd(), 'plugin', 'hooks', name), 'utf8'); + + test('the PreToolUse router calls maybeSurface', () => { + expect(hook('pretooluse-router.mjs')).toMatch(/maybeSurface\(/); + }); + + test('the PreCompact hook calls closeForecast', () => { + expect(hook('precompact-optimize.mjs')).toMatch(/closeForecast\(/); + }); + + test('the vendored copy of surface.mjs is in place for the plugin to import', () => { + // The hooks import from ./lib/, which is synced rather than symlinked -- an unsynced module + // means the hook throws on load and fails open, silently doing nothing. + expect(() => readFileSync(join(process.cwd(), 'plugin', 'hooks', 'lib', 'surface.mjs'), 'utf8')) + .not.toThrow(); + }); +}); + +// --- a forecast is logged when it is SHOWN, and only then --------------------------- + +describe('only a surfaced forecast is scored', () => { + test('a panel the throttle or worthSurfacing rejects logs nothing', () => { + // THE DEFECT: forecastPanel logged on every build. maybeSurface builds one per throttle window + // and worthSurfacing rejects most of them, so open forecast records nobody ever saw piled up + // in balance.jsonl -- and that log is read by its TAIL BYTES, so those rows would eventually + // evict the inject/harvest/substitute rows BALANCE_KINDS exists to protect. The fix this work + // is built on would have been undone by its own display path. + seedArms(); + const out = maybeSurface(dir, { + transcriptPath: transcript(20, 20_000), sessionId: 'live', state: {}, now: 5_000, + }); + expect(out.text).toBeNull(); // comfortable runway: nothing worth showing + expect(readBalance(dir).filter((e) => e.kind === 'forecast')).toHaveLength(0); + }); + + test('a surfaced panel logs exactly one forecast, carrying its origin turn', () => { + seedArms(); + const out = maybeSurface(dir, { + transcriptPath: transcript(20, 190_000), sessionId: 'live', state: {}, now: 5_000, + }); + expect(out.text).not.toBeNull(); + + const logged = readBalance(dir).filter((e) => e.kind === 'forecast'); + expect(logged).toHaveLength(1); + expect(logged[0].sessionId).toBe('live'); + expect(logged[0].predictedTurns).toBe(out.state.shown); + // The origin turn is what makes the outcome an interval rather than an absolute. + expect(logged[0].turns).toBe(20); + }); + + test('a throttled second call adds no second forecast', () => { + seedArms(); + const path = transcript(20, 190_000); + const first = maybeSurface(dir, { transcriptPath: path, sessionId: 'live', state: {}, now: 5_000 }); + expect(first.text).not.toBeNull(); + maybeSurface(dir, { + transcriptPath: path, sessionId: 'live', state: { forecast: first.state }, now: 5_100, + }); + expect(readBalance(dir).filter((e) => e.kind === 'forecast')).toHaveLength(1); + }); +});