fix: circuit-breaker decay leak; cap block backoff; chronic-rate watchlist - #482
Conversation
…tion consecutiveBlocks decayed on any run with newCompletions > 0 — including runs whose only completion was a (sub-threshold) fade. A chronic low-volume fader whose fades land in separate cron runs therefore nets zero escalation per fade->block cycle: the cycle's first fade decays the level the block re-added, pinning them on 15-30 minute blocks forever. Confirmed in prod: a filler fading ~94% of a week's orders sat at consecutiveBlocks=2. Decay now requires a streak of CLEAN_RUNS_PER_DECAY (6) consecutive clean runs — runs with >=1 new completion and 0 new fades — per level, tracked in a new consecutiveCleanRuns field on the CB state row. Any new fade resets the streak (including sub-threshold and in-flight-while-blocked fades); idle runs freeze it; blocks and extensions restart it. At the 10-minute cron cadence recovery costs >=1 hour of demonstrated clean activity per level, so decay is structurally slower than escalation. Existing FillerCBTimestampsV2 rows lack the attribute; it reads as 0 (streak starts fresh), so no migration is needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… metric Both changes are backtest-driven (2 weeks of real order history replayed through the cron logic; see PR #482 for full numbers): - Cap the block-backoff exponent at 2^7 (32h per increment). Uncapped, the replay produced a 152h (6.3-day) block; the cap bounds the worst-case sentence (~64h with stacked extensions) and guarantees recovery from pathological stored state, costing ~12% of worst-offender fade containment. - Emit CIRCUIT_BREAKER_V2_CHRONIC_RATE: each filler's raw fade rate over the entire query window with no clean-slate amnesty and no smoothing, gated on a 10-order minimum sample, plus a dashboard watchlist widget. The backtest showed a low-volume ~20% fader living permanently inside the block threshold's envelope (~0.12n + 1.4 fades/day) whom no tested trigger calibration catches without unacceptable collateral -- this keeps such fillers visible for human follow-up instead of a threshold change. Also documents the backtest extract SQL + replay conventions in CLAUDE.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| // Chronic (no-amnesty) view: every row in the query window, regardless of cohort. | ||
| tallies[fillerHash].chronicTotal += 1; | ||
| tallies[fillerHash].chronicFades += row.faded; | ||
| if (row.deadline > lastExaminedTimestamp) { |
There was a problem hiding this comment.
This classifies a row as new activity by comparing its deadline to a watermark that jumps to wall clock on every run, which makes the streak sensitive to Redshift load lag in both directions.
postedorders and archivedorders are batch loaded hourly while the cron runs every 10 minutes, so a row can first become visible with a deadline that is already behind the watermark. A fade that arrives late never counts as a newFade and never resets the streak, so a chronic fader can decay an escalation level while fading inside the window. The fade still moves fadeRate, only the streak reset misses it.
The same lag also cuts the other way. An order whose fill row has not loaded yet is faded per the view's LEFT JOIN (fillTimestamp IS NULL) on the first run after its deadline, which is exactly the run where it counts as a newFade and resets the streak. When the fill row lands, the order flips back to faded=0, but the watermark has moved past it and the correction never re-enters the tally. The smoothed rate tolerates these transients, the streak does not, one transient resets it to 0, so an honest high-volume filler may never decay.
I think the streak needs to tolerate load lag before this ships, either by classifying "new" against a watermark that trails wall clock by the worst-case load lag, or by deriving the streak from the window contents instead of incrementally.
There was a problem hiding this comment.
Confirmed and fixed in cc7c6d6 — this was the sharpest catch of the review; both directions were real. Streak inputs are now classified against a horizon trailing wall clock by STREAK_FINALITY_LAG_SECS (2h): each run classifies deadlines in (lastExamined − LAG, now − LAG]. Since lastExaminedTimestamp advances to now every run, consecutive slices partition time with no extra stored watermark, each row is classified exactly once, and by the time a row is classified its load state is final — a late-loading fade lands in a future slice and still resets the streak, and a transiently-unfilled order has its fill row by classification time. Went with the trailing-watermark variant over window-derived because it preserves the run-cadence semantics the backtest validated (recovery ≥1h of wall-clock clean activity per level regardless of volume). Tests pin deferred classification, exactly-once classification after the horizon, and the trip path staying on the un-lagged rate window.
| consecutiveCleanRuns = 0; // nothing to work off; don't accumulate a stale streak | ||
| } else if (newFades > 0) { | ||
| consecutiveCleanRuns = 0; | ||
| } else if (newCompletions > 0) { |
There was a problem hiding this comment.
When a block expires between two cron runs, the first unblocked run lands here with newCompletions that can consist entirely of during-bench fills (deadline after the watermark but on or before fadeWindowStart). If none of them faded, the streak increments. That contradicts the rule in the blocked branch above, that serving a bench is not recovery. Had the cron run a minute before expiry, the same fills would have earned nothing. Restricting the increment to rows with deadline past fadeWindowStart would close this.
There was a problem hiding this comment.
Fixed in cc7c6d6 exactly as suggested: newCompletions now only counts rows with deadline past fadeWindowStart, so during-bench fills earn nothing toward the streak whether the cron runs before or after expiry. During-bench fades still count in newFades (a fade is a fade — it should reset the streak). The in-flight-cohort test now pins both: 3 during-block completions → 0 streak credit, 2 during-block fades → newFades 2.
| ao.tokenOut AS tokenOut, | ||
| CASE | ||
| WHEN ao.fillTimestamp IS NULL THEN 1 | ||
| WHEN po.ordertype = 'Dutch_V3' AND ao.fillTimeBlocks >= 0 THEN 1 |
There was a problem hiding this comment.
The doc says this extract matches the fade semantics of V2_FADE_RATE_SQL, but the live SQL uses fillTimeBlocks > 0 since #461. With >= 0 the replay counts fills at the decay start block as fades and calibrates the knobs against inflated V3 fade rates.
There was a problem hiding this comment.
Good catch — fixed the doc to fillTimeBlocks > 0 plus an explicit keep-in-sync warning (cc7c6d6). Since the extract keeps the raw columns, I re-ran the whole backtest with fades recomputed locally under the correct semantics: 387 rows (1.6%) were misclassified, concentrated in a few fillers (one "26% chronic" was actually a legitimate 8.5% filler). All qualitative conclusions hold — chronic ≥30% faders: 17% benched pre-fix → 59% shipped, allowed fades 605 → 180; collateral on ≤11% fillers ≤0.8% bench — and the corrected numbers replaced the stale ones in the code comments (uncapped worst block 128h not 152h; cap cost ~20% not ~12%). The pre-fix decay-leak stat holds at 57% of decay events granted by fade-containing runs (100% for the extreme faders).
| */ | ||
| export function calculateBlockUntilTimestamp(fromTimestamp: number, consecutiveBlocks: number | undefined): number { | ||
| const blocks = consecutiveBlocks || 0; | ||
| const blocks = Math.min(consecutiveBlocks || 0, MAX_BLOCK_BACKOFF_EXPONENT); |
There was a problem hiding this comment.
This caps the block duration, but the stored consecutiveBlocks itself still grows without bound. Full recovery costs consecutiveBlocks * 6 clean runs, so a filler with a long block history stays one sub-threshold fade away from a 32h block long after reforming. Capping the counter in newConsecutiveBlocks would bound both the duration and the recovery time.
There was a problem hiding this comment.
Fixed in cc7c6d6: newConsecutiveBlocks caps the stored counter at MAX_BLOCK_BACKOFF_EXPONENT, so full recovery is bounded at 7 × 6 clean runs (~7h of clean activity) and a reformed filler isn't one sub-threshold fade from a 32h block indefinitely. Kept the min() in calculateBlockUntilTimestamp as defense for any legacy stored values above the cap. Test pins that blocking at the cap holds both the duration exponent and the counter.
| > & { | ||
| blockUntilTimestamp?: number; | ||
| fadeWindowStart?: number; | ||
| consecutiveCleanRuns?: number; |
There was a problem hiding this comment.
With this optional and updateTimestampsBatch doing a full-item put, a future caller that omits the field compiles fine and silently wipes a filler's streak (the read path defaults it to 0). The current writer sets it in all branches, so this is latent, but making it required like consecutiveBlocks closes it off.
There was a problem hiding this comment.
Agreed — consecutiveCleanRuns is now required in ToUpdateTimestampRow (cc7c6d6), with a comment explaining why it must stay that way given full-item puts. The repo test keeps the pre-migration missing-attribute coverage via an explicit undefined as unknown as number cast (put omits the attribute → read defaults to 0), so the read-path default stays exercised without weakening the type for real callers.
| const timestamps: FillerTimestamps = new Map(); | ||
| const stats: FillerFadeStatsMap = { newBad: { fadeRate: 0.2, duringBlockRate: 0.05, newCompletions: 1 } }; | ||
| const stats: FillerFadeStatsMap = { | ||
| newBad: { |
There was a problem hiding this comment.
Nit: there are about 20 of these literals now, each spelling out all six fields. A small factory with defaults, like fadeStats({ fadeRate: 0.2, newFades: 1 }), would keep the fields each test cares about visible and absorb the next field addition.
There was a problem hiding this comment.
Done in cc7c6d6: added fadeStats() and cbState() factories with benign defaults; all ~20 literals now spell out only the fields under test (the file shrank by ~170 lines) and the next field addition touches one place.
…g, capped counter Review findings from #482, all verified against the repo and by re-running the backtest with corrected fade semantics: - Streak classification is now lag-tolerant: newCompletions/newFades are classified against a horizon trailing wall clock by STREAK_FINALITY_LAG_SECS (2h), i.e. deadlines in (lastExamined - LAG, now - LAG]. Redshift is batch-loaded hourly, so without the lag a late-loading fade never reset the streak (deadline already behind the watermark) and a not-yet-loaded fill read as a transient fade exactly when classified as new — resetting an honest filler's streak with no correction. Rows older than the lag are final. - Bench fills earn nothing: newCompletions now requires deadline past the clean-slate floor, closing the expiry-gap case where a first unblocked run could build the streak from fills served entirely while benched. During-bench fades still count against the streak. - consecutiveBlocks counter capped at MAX_BLOCK_BACKOFF_EXPONENT, bounding full recovery time along with block duration (an uncapped counter left a long-reformed filler one sub-threshold fade from a 32h block forever). - consecutiveCleanRuns is required in ToUpdateTimestampRow: with full-item puts, an optional field a future caller omitted would silently wipe the stored streak. - CLAUDE.md extract SQL corrected to fillTimeBlocks > 0 (live semantics since #461; the >= 0 extract inflated V3 fade rates) with a sync warning. Backtest re-run with corrected fades: all qualitative conclusions hold; code-comment numbers updated (128h uncapped max block, ~20% cap cost). - Test factories (fadeStats/cbState) so tests spell out only the fields they exercise; new tests pin deferred classification, exactly-once classification after the horizon, bench-fill exclusion, and the counter cap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ation metric Remaining review findings plus eviction-boundary hardening, all validated by rerunning the 2-week backtest with the complete final behavior modeled: - Stored consecutiveBlocks is clamped into [0, MAX_BLOCK_BACKOFF_EXPONENT] at the single read site, so legacy over-cap rows normalize on any path (keep/decay/extend/re-block, and the metric) and corrupted negative values self-heal instead of decaying further and producing 2^negative sub-base blocks. - Chronic watchlist tallies score only rows past the finality horizon, so not-yet-loaded fills can't sawtooth the metric with transient fades; and emission now also requires CHRONIC_RATE_EMISSION_FLOOR (6%), so per-filler series exist only for watch-worthy fillers instead of every healthy one. - New CIRCUIT_BREAKER_V2_SATURATED_ADDRESSES metric + activity-widget line: fires when an address's latest-N window is full AND its oldest row is fresher than the finality horizon — i.e. rows are being evicted before they can be streak-classified. Merely sitting at the latest-N cap is the designed adaptive window and is deliberately not flagged (backtest: cap occupancy is near-constant for big fillers, 98.8% of runs, while true degradation occurred on 1.0% of runs, confined to the two busiest, never-escalated addresses). Boundary documented on STREAK_FINALITY_LAG_SECS (~50 orders/hour sustained per address). - Chronic widget retitled (adaptive window, not "24h"). Backtest with all changes modeled (eviction included): chronic >30% faders 50% benched / 218 allowed fades (vs 17% / 605 pre-#482), worst sentence 64h (vs 128h uncapped), collateral on <=11% fillers unchanged at <=0.9% bench. The review fixes cost no containment beyond the already-priced cap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ation metric Remaining review findings plus eviction-boundary hardening, all validated by rerunning the 2-week backtest with the complete final behavior modeled: - Stored consecutiveBlocks is clamped into [0, MAX_BLOCK_BACKOFF_EXPONENT] at the single read site, so legacy over-cap rows normalize on any path (keep/decay/extend/re-block, and the metric) and corrupted negative values self-heal instead of decaying further and producing 2^negative sub-base blocks. - Chronic watchlist tallies score only rows past the finality horizon, so not-yet-loaded fills can't sawtooth the metric with transient fades; and emission now also requires CHRONIC_RATE_EMISSION_FLOOR (6%), so per-filler series exist only for watch-worthy fillers instead of every healthy one. - New CIRCUIT_BREAKER_V2_SATURATED_ADDRESSES metric + activity-widget line: fires when an address's latest-N window is full AND its oldest row is fresher than the finality horizon — i.e. rows are being evicted before they can be streak-classified. Merely sitting at the latest-N cap is the designed adaptive window and is deliberately not flagged (backtest: cap occupancy is near-constant for big fillers, 98.8% of runs, while true degradation occurred on 1.0% of runs, confined to the two busiest, never-escalated addresses). Boundary documented on STREAK_FINALITY_LAG_SECS (~50 orders/hour sustained per address). - Chronic widget retitled (adaptive window, not "24h"). Backtest with all changes modeled (eviction included): chronic >30% faders 50% benched / 218 allowed fades (vs 17% / 605 pre-#482), worst sentence 64h (vs 128h uncapped), collateral on <=11% fillers unchanged at <=0.9% bench. The review fixes cost no containment beyond the already-priced cap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fe1d637 to
5e28393
Compare
Why
The V2 breaker's exponential backoff never engages against chronic low-volume faders. Decay required only
newCompletions > 0, andnewCompletionscounts faded completions — so a fader whose fades land in separate cron runs gets this cycle:Net escalation per cycle: zero.
consecutiveBlockspins at ~0–2, every block stays 15–30 minutes, and a filler with a ~2h natural order gap loses almost no volume to the bench.Confirmed in production (a filler fading ~94% of a week's orders sat at
consecutiveBlocks = 2with an active block) and quantified by replaying 2 weeks of real order history (24,315 orders) through the cron logic: under the old code, 57% of all decay events granted to chronic fillers came from runs whose completions included a fade — for the extreme faders, 100% did. Moderate chronic faders (24–39% fade mix) sat in near-perfect trip/decay equilibrium (e.g. 64 trips vs 63 decays),consecutiveBlocksnever exceeding 2 for the entire fortnight.Fix 1 — decay requires sustained clean activity
Decay one escalation level only after
CLEAN_RUNS_PER_DECAY(6) consecutive clean runs, tracked in a newconsecutiveCleanRunsfield on the CB state row:newFadesstat)Load-lag tolerance: streak inputs are classified against a horizon trailing wall clock by
STREAK_FINALITY_LAG_SECS(2h) — each run classifies deadlines in(lastExamined − LAG, now − LAG], which partitions time across runs with no extra state. Redshift is batch-loaded hourly, so without this a late-loading fade never reset the streak and a not-yet-loaded fill read as a transient fade that reset it spuriously. Rows are classified exactly once, after their load state is final. (Blocking itself stays on the un-lagged rate window — only decay is deferred.)At the 10-minute cron cadence, recovery costs ≥1 hour of demonstrated clean activity per level, making decay structurally slower than escalation (+1 per block event).
Backtested impact (replay, same fortnight, fade semantics matching live SQL): chronic >30% faders go from 17% time benched to 59%, with fades reaching swappers dropping ~70% (605 → 180); the 16–30% band goes from 1.7% to 27% benched (246 → 128 allowed fades); collateral on legitimate ≤11% fillers stays ≤0.8% bench time with ≤2h worst sentences.
Fix 2 — cap block backoff at 2^7 (32h per increment), counter included
Uncapped, the replay produced a 128h (5.3-day) block.
calculateBlockUntilTimestampcaps the duration exponent atMAX_BLOCK_BACKOFF_EXPONENT = 7, andnewConsecutiveBlockscaps the stored counter at the same bound — so full recovery is bounded at 7 × 6 clean runs (~7h of clean activity) instead of growing with block history. Projected cost: +38 allowed fades per fortnight across the worst offenders (+21%); benefit: bounded worst-case sentence (~64h with stacked extensions) and automatic recovery from pathological stored state.Fix 3 — chronic-rate watchlist metric + dashboard widget
New
CIRCUIT_BREAKER_V2_CHRONIC_RATE: each filler's raw fade rate over the entire query window — no clean-slate amnesty, no smoothing — emitted at a ≥10-order sample, with a dashboard widget annotated at the block threshold.Rationale: the backtest exposed a low-volume ~20% fader living permanently inside the threshold's envelope (
maxFadesPerDay ≈ 0.12n + 1.4, i.e. ~21% raw at 15 orders/day). Every candidate mechanism was tested against them — threshold 10%, weaker prior (α=0.5), raw-rate dual rules, a chronic decay gate, and an EWMA-scored breaker — and each either fails to contain them or benches legitimate fillers heavily. The correct move is visibility, not a trigger change: this metric makes such fillers a deliberate human decision.Not included (deliberately)
Tests / docs
getFillersFadeStats → calculateNewTimestampspipeline with the production profile and failed on the old code (decay 2→1; next block 2^1 instead of 2^2).fadeStats()/cbState()factories keep test literals to the fields under test.V2_FADE_RATE_SQL— thanks review for catching the fix: don't count Dutch_V3 fills at the decay start block as fades #461 drift) and replay conventions.Migration
None. Existing
FillerCBTimestampsV2rows lack the new attribute; reads default it to 0 and the next cron write populates it.Review updates (post-review commit cc7c6d6)
All six review findings addressed: lag-tolerant streak classification, bench fills excluded from streak credit,
consecutiveBlockscounter capped,consecutiveCleanRunsrequired in the write type, extract SQL corrected tofillTimeBlocks > 0, and test factories. The backtest was re-run with corrected fade semantics; all figures above reflect the corrected data.🤖 Generated with Claude Code
Hardening (post-review commit 5e28393)
Follow-ups from the
/code-reviewpass and the eviction-boundary analysis, with the backtest rerun modeling the complete final behavior (lagged streak classification fed only from window-surviving rows, bench-fill exclusion, both caps, read clamp):consecutiveBlocksis clamped into[0, MAX_BLOCK_BACKOFF_EXPONENT]where state is read, so legacy over-cap rows normalize on every path (keep/decay/extend/re-block and the metric) and corrupted negative values self-heal instead of yielding2^negativesub-base blocks.CHRONIC_RATE_EMISSION_FLOOR) so per-filler series exist only for watch-worthy fillers.CIRCUIT_BREAKER_V2_SATURATED_ADDRESSES: fires when an address's latest-N window is full AND its oldest row is fresher than the finality horizon — the true "rows evicted before streak classification" condition. Merely sitting at the latest-N cap is the designed adaptive window and is not flagged (backtest: cap occupancy is near-constant for big fillers — 98.8% of runs — while true degradation occurred on 1.0% of runs, confined to the two busiest, never-escalated addresses at 104–123% of the cap during peak bursts). The boundary (~50 orders/hour sustained per address) is documented onSTREAK_FINALITY_LAG_SECS.Final backtest posture (2 weeks, 24,315 orders, corrected fade semantics, eviction modeled): chronic >30% faders 50% benched, 218 allowed fades (vs 17% / 605 pre-#482); 16–30% band 26% benched, 135 allowed (vs 1.7% / 246); worst continuous sentence 64h (vs 128h uncapped); collateral on ≤11% fillers unchanged at ≤0.9% bench / ≤2h. The review fixes cost no containment beyond the already-priced cap.