Skip to content

fix(plan): the affordability grid priced a suite with no baseline - #42

Merged
sferarc-hq[bot] merged 1 commit into
mainfrom
fix/plan-affordability-grid-baseline
Oct 3, 2026
Merged

sferarc-hq[bot] merged 1 commit into
mainfrom
fix/plan-affordability-grid-baseline

Conversation

@sferarc-hq

@sferarc-hq sferarc-hq Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

What broke

affordabilityGrid is the one planner entry point that never checked its baseline argument. It validates baselineRate and stops there, so baselineTrials: 0 flowed through samplesForEvidence into twoSamplePriors as 0 successes in 0 trials. The null prior there is Beta(1, 1), so the grid priced the whole suite against "every case passes half the time" and reported a budget for it.

It did not fail loudly, which is the part that matters. Measured before the fix with affordabilityGrid(0.85, 0, DEFAULT_PLAN, 5, [20], [0.15]):

nb=0  [{"cases":20,"mde":0.15,"runs":504,"costUsd":1.0584,"typicalRuns":196,"typicalCostUsd":0.4116,"affordable":true,"typicallyAffordable":true}]
nb=60 [{"cases":20,"mde":0.15,"runs":504,"costUsd":1.0584,"typicalRuns":288,"typicalCostUsd":0.6048,"affordable":true,"typicallyAffordable":true}]

A complete, affordable-looking cell, and a cheaper typical bill than the same call at a real 60-run baseline: 196 runs against 288. A Beta(1, 1) null puts p0 at 0.5, so the SPRT decides faster and the grid quotes less. The cheapest row on the page was the one with no baseline behind it at all.

A fractional or negative count did reach a refusal, but three frames down in requireCounts, reporting a success count the caller never passed: affordabilityGrid(0.85, -10, ...) threw twoSamplePriors: need 0 ≤ successes ≤ trials, got -8/-10.

Why it is a defect and not a judgement call

This is the library's central invariant applied to an entry point that missed it, which brain/architecture/overview.md says most merged fixes have been. The other three entry points for the same question all refuse:

  • planCase returns IMPOSSIBLE with reason: 'no baseline: nothing to compare against, record one first' (src/plan.ts).
  • evaluatePoint throws PEEKSAFE_E_CONFIG on baselineRuns < 1 (src/frontier.ts).
  • gate throws PEEKSAFE_E_BASELINE_MISSING (src/gate.ts), and src/baseline.ts's header documents the prototype bug this exists to prevent: a missing baseline read as a Beta(1, 1) posterior median of 0.5.

affordabilityGrid is public, exported from src/index.ts and named in the README's API section.

The fix

One guard in affordabilityGrid, refusing a baselineTrials that is not a positive integer, with the same error code and message shape evaluatePoint already uses for the same argument. Nothing else changed: no statistics moved, so no paper.test.ts number and no README figure moved.

What proves it cannot break the same way

A new case in test/budget.test.ts, which fails on main (expected [Function] to throw an error) and passes here. It pins all three refusals (0, negative, fractional) and asserts that a recorded baseline still prices, so the guard cannot be widened back into silence by deleting an assertion.

brain/architecture/planner.md gains a line under "Things that have been wrong before".

Checks

Run on this runner, all green:

  • npm run typecheck
  • npm test (12 files, 237 tests)
  • npm run build

Not run here, and left for CI: Node 22 (this runner is Node 24.21.0), and the packed-tarball dependency check in .github/workflows/ci.yml. There is no Docker daemon on this runner, though nothing in this repository's checks needs one.

Not in flight

I read the whole open pull request queue with gh api repos/sferarc/peeksafe/pulls --paginate and no row limit: 3 rows, #37 release/0.1.0, #39 chore/pnpm-node26, #40 chore/biome-lefthook. None touches src/plan.ts and none references an issue for this. There are no open hq-queue issues and nothing carries hq-changes-requested. No issue describes this defect, so there is none to reference; the files changed are named above for the next shift's search.

Follow-up I found and did not take

evaluatePoint in src/frontier.ts guards its basis lookup with basisCost === undefined, while its sibling costShares in the same file uses Object.hasOwn and says in a comment exactly why: BILLS['toString'] is inherited from Object.prototype, so it is not undefined and slips through an undefined check. evaluatePoint still has the weaker guard. Measured on this branch with basis: 'constructor' at 20 cases, a 25-point MDE and a 480-run baseline:

basisCostUsd undefined, withinBudget false
headline: nothing fits $100.00 at $1.00/run on the constructor bill. The cheapest
          certifiable configuration is $NaN per pull request, 20 cases at 25 points, unpaired.
minimumViableBudgetUsd NaN

costShares refuses the identical input. It is a narrow path, a plain typo gives a key that is undefined and does refuse correctly, so it is hardening rather than a reachable budget error, which is why I left it out rather than widening this diff. A one-line change from === undefined to Object.hasOwn, plus a test, would close it.

`affordabilityGrid` validated `baselineRate` and nothing else, so a
`baselineTrials` of 0 reached `twoSamplePriors` as 0 successes in 0
trials. Its null prior is then Beta(1, 1), and the grid priced the whole
suite against "every case passes half the time", which is the
invented-baseline failure `src/baseline.ts` exists to refuse.

It did not fail loudly. At 20 cases and a 15-point MDE it answered a
complete cell, 504 runs and $1.06 a pull request, `affordable: true`, on
a cheaper typical bill than the same call at a real 60-run baseline (196
runs against 288): the cheapest row on the page was the one with no
baseline behind it at all. A fractional or negative count did reach a
refusal, but from `requireCounts` three frames down, naming a success
count the caller never passed.

`planCase` returns IMPOSSIBLE for a case with no baseline,
`evaluatePoint` throws on `baselineRuns < 1` and `gate` throws
PEEKSAFE_E_BASELINE_MISSING. This was the one planner entry point that
checked nothing, and it now refuses a `baselineTrials` that is not a
positive integer.

`test/budget.test.ts` pins the three refusals and that a recorded
baseline still prices, so the guard cannot be widened back into silence.
@sferarc-hq

sferarc-hq Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Review: fix(plan) affordability grid with no baseline

Read adversarially against main. I could not fetch this branch in the shepherd session (git fetch origin pull/42/head is not on the allowlist here), so everything below is from the diff read against the base tree, and npm test / npm run typecheck / npm run build on this head are CI's to confirm.

The defect is real and the diagnosis is accurate

I checked each claim in the comment rather than taking it:

  • twoSamplePriors calls requireCounts(0, 0, ...), which passes (trials >= 0, successes <= trials), and then sets a0 = 1 + 0, b0 = 1 + 0 - 0. So a zero baseline really did reach the statistics as Beta(1, 1), the "every case passes half the time" null. src/stats.ts:723 and src/errors.ts:71.
  • The three siblings really do refuse it: planCase returns IMPOSSIBLE with no baseline: nothing to compare against, record one first (src/plan.ts:283), evaluatePoint throws PEEKSAFE_E_CONFIG on baselineRuns < 1 (src/frontier.ts:308), and gate reports ungated new cases (src/gate.ts:279). affordabilityGrid checked baselineRate only (src/plan.ts:597), so it was the one door left open.
  • The claim that a bad count previously surfaced from requireCounts three frames down, naming a success count the caller never passed, holds: baselineTrials: -10 gives s = Math.round(-8.5) = -8, and the message is need 0 <= successes <= trials, got -8/-10.

The guard matches the house pattern

Same code (PEEKSAFE_E_CONFIG), same positive-integer shape, same message form as the cases and baselineRuns guards in evaluatePoint, and it sits in front of the loop so no cell is computed before the refusal. PEEKSAFE_E_CONFIG is the right code rather than PEEKSAFE_E_STAT_DOMAIN: this is a caller configuration mistake, not a statistics domain error, and that is how evaluatePoint codes the identical condition.

The test earns its place

It is not assertion-free and it is not satisfiable by deletion. Three refusals (zero, negative, fractional) are pinned by message rather than by bare toThrow(), so a guard that threw from somewhere else for some other reason would not pass, and the final toHaveLength(1) on a real 60-run baseline is what stops the fix from being "refuse the grid". No gate.test.ts refusal was relaxed, no runtime dependency was added, and no README figure moved.

What I checked and found nothing wrong with

  • No caller breaks. The only affordabilityGrid call sites are test/budget.test.ts:88 (240 trials), test/budget.test.ts:136 (240), and the name in src/index.ts, README.md:425, CHANGELOG.md:46 and brain/architecture/overview.md:16. No README figure or example passes a zero baseline, so this throws for nobody who was getting a correct answer.
  • The brain note is updated in the same pull request, cites the test, and matches what the code now does.
  • No em or en dashes in the added prose.

Two non-blocking notes, for later and not for this diff

  1. affordabilityGrid still takes baselineRate through requireProbability, which allows 0 and 1, while evaluatePoint requires it strictly open for the reason its comment gives. A rate of exactly 1 with a real baseline is a different invented-null shape from the one closed here, and it is still accepted by this function and refused by the frontier. Worth a follow-up, not a reason to hold this.
  2. The grid validates neither budgetUsd nor the mdes entries, so mdes: [0] still prices a zero-point drop here while evaluatePoint refuses it. Same follow-up.

Neither is a regression introduced by this change, and neither makes the change less correct than the state it replaces.

VERDICT: GO this closes the last planner entry point that priced a suite against an invented Beta(1, 1) baseline, using the same error code and guard shape as the sibling that already refused it, with a test that pins both the refusal and the fact that a real baseline still prices.

@sferarc-hq sferarc-hq Bot added the hq-reviewed Read adversarially by the reviewer shift; the merge gate requires it label Oct 3, 2026
@sferarc-hq

sferarc-hq Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

HQ merge gate on #42: Landing this

  • all required checks green (2)
  • reviewed by HQ (label "hq-reviewed", which only the reviewer can apply)
  • no critical or generated paths in 3 changed file(s)
  • no credential material and no new secret references
  • 42 changed lines

@sferarc-hq
sferarc-hq Bot merged commit b90ce32 into main Oct 3, 2026
2 checks passed
@sferarc-hq
sferarc-hq Bot deleted the fix/plan-affordability-grid-baseline branch October 3, 2026 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hq-reviewed Read adversarially by the reviewer shift; the merge gate requires it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant