feat(runner): decide the repair context budget before #27 spends tokens on it

[OM152002]

Closes #125. ADR-0012.

The repair model was given a URL, a page title, and ["main", "navigation"] — then asked to produce a corrected locator, which is a role plus an accessible name. The context did not contain the shape of the answer.

That is not a hard task, it is an underdetermined one, and #125's argument for why it had to be settled before #27 is correct: on a cache miss the system pays replay plus repair plus the fresh run it still owes, and PRD §9's kill line is a ratio. A repair that burns tokens at a structurally near-zero rate pushes the project toward FAIL for a reason that is a design parameter rather than a property of the thesis. An unforced FAIL is as misleading as an unearned PASS.

Measured, not argued

Live Grafana 9.5.21, capturing at each level and checking whether the identifying string of the live bundle's step targets is present:

Page	Level	Elements	Payload	Bundle targets present
/	landmarks	0	2 B	none
	interactive	30	2034 B	none
	tree	43	3317 B	none
/dashboard/new	landmarks	0	2 B	none
	interactive	20	831 B	3 — Add new panel, Save dashboard, Dashboards
	tree	23	989 B	the same 3

Two things follow, and the ADR claims only these two: at landmarks the answer is absent, so repair cannot succeed there for reasons unrelated to the model; and tree found nothing interactive did not, at 19–63% more payload.

What is not measured — and the ADR says so in the decision, not a footnote — is whether a model actually succeeds at either level. That needs #27. #125's protocol (break a locator, n≥3 per level, report the spread) is the right shape and is recorded as the open question.

I also corrected two claims I'd written into the code docstring before measuring: an unsupported "0 of 11" and an invented "~4x payload". The landmarks result is now stated as true by construction — elements is empty — which is a property of the design, not a measurement, and dressing it up as data would have been the exact failure the audit's category B exists for.

Levels, and the one that was rejected

landmarks (the old floor, kept for baselining) · interactive (chosen) · tree (available). A fourth — a DOM excerpt with allowlisted attributes — is rejected in the ADR rather than left unimplemented-but-blessed: it is the first level whose output cannot be reviewed by reading a fixed field list. The others emit a closed set of fields; a DOM excerpt emits whatever the page contains, filtered, and correctness then rests on the filter being exhaustive against markup nobody has seen.

No level reads an input's value

Pinned two ways, because one would not be enough:

• Behaviourally — a filled password field, a filled text input, and a placeholder, all set to one canary, asserted absent at every level.
• Structurally — the source string handed to the browser contains no route to a value at all.

Guard-proven: making the capture emit el.value fails 3 cases.

The egress boundary

serializeRepairContext() is the only authorized shape, built by naming fields rather than deleting them from a copy — so a field added to RepairContext next year is invisible to the model until someone adds a line, and that line is a diff a reviewer sees.

Excluded: params (the bag secrets live in), assertion.expected, param_refs, and error_message — that last is the one worth naming, because it reads like diagnostics and is what a well-meaning change adds first, but a Playwright locator error can quote the resolved selector and surrounding text.

tests/canary/repair-egress.test.ts is merge-blocking and covers both directions: nothing unauthorized in the payload, and no client in the tree serializing the raw context around the boundary. Guard-proven — leaking params fails 3, passing the raw text locator fails 2.

repair_context_level goes on the run row, written only when a repair actually ran. A self-heal rate is not reproducible without knowing what the model could see.

#74's guard caught me mid-change

My first cut copied the visibility predicate into the new capture site — exactly the bug tests/unit/landmarks.test.ts exists to prevent, and it failed immediately. The predicate is now extracted as VISIBILITY_PREDICATE_JS and interpolated into both sites, so there is still one carrier under src/.

It then caught the fix's own docstring for spelling the DOM API name in prose. I reworded the comment rather than loosening the scan — narrow, but the narrowness is what makes it hold.

Third time this session a repo guard has caught something a passing suite hid, and the third time secret-scan's env-assignment pattern has been tripped by a test constant named the obvious thing. That one is noted in the test file for the next person.

Scope

The repair proposal is unchanged — corrected_action only, assertAssertionUnchanged untouched. Richer input is not a licence to touch the assertion.

boundary-spec.md gains a section distinguishing the two exits: it governs what enters the pool, ADR-0012 governs what is sent to a model. Neither supersedes the other. Open and shared with #126: an accessible name is page-authored — vendor vocabulary on pinned open-source software, possibly tenant strings on a closed portal.

npm run ci            # green — 356 unit, 16 integration, secret-scan clean, lint-docs clean (55 docs)
npm run test:canary   # 42 pass

🤖 Generated with Claude Code

[myselfsiddharth]

Review — approving and merging

Full CI green, targets main, and the ADR does the thing the repo keeps asking for: it separates what was measured (payload sizes and target presence at each level, on a pinned Grafana) from what was argued (landmarks is empty by construction, stated as a design property rather than dressed up as data). The two counter-check tests — "the scan is capable of firing" and "the canaries are actually on the page — these are not vacuous" — are the reason I trust the rest of the suite rather than just reading it.

Merging. Four findings below, none blocking; the second is the one I'd actually fix.

1. CONTEXT_ATTRIBUTE_ALLOWLIST is exported and referenced nowhere

git grep CONTEXT_ATTRIBUTE_ALLOWLIST returns exactly one hit — its own declaration. contextExpression() reads attributes by literal string (getAttribute("aria-label"), getAttribute("aria-checked"), …), so the allowlist currently documents a boundary it does not enforce. The set happens to match what the capture reads today; nothing keeps that true. A change adding getAttribute("title") fails no test and contradicts no constant.

Either interpolate it into the generated JS so reads go through it, or delete it. The docstring above it argues "allowlist rather than denylist" — and a reader who finds that argument will reasonably assume it is load-bearing.

2. The run row and the payload can disagree about what the model saw

In the repair path:

const pageState = this.page
  ? await capturePageState(this.page, this.repairContextLevel)
  : emptyPageState();

emptyPageState() carries no context_level and no elements, so serializeRepairContext() falls back to context_level: "landmarks". But the run row is written from the configured level:

...(result.repair_count > 0 ? { repair_context_level: this.repairContextLevel } : {})

With no page, the payload says landmarks while the run row says interactive — two records of the same run disagreeing about what the model could see, which is the precise non-reproducibility the field was added to prevent. The comment right above it ("recording a context level for a run that never asked a model anything would imply a measurement that did not happen") makes the same argument that applies here.

Sourcing the run row from the snapshot actually used (pageState.context_level) closes it. A real gate run always has a page, so impact today is limited to page-less runners — but this field's whole job is to be trustworthy after the fact.

3. The new page.evaluate is uncaught, one line above the guard that exists for that

capturePageState wraps the idle probe in try/catch ("leave it false rather than inventing success") but the new elements capture can throw — page navigating mid-capture, page closed, eval blocked. It is called at the top of the repair loop, outside the try/catch around propose(), whose comment reads:

Before this, an uncaught throw here propagated out of run() entirely — one flaky repair call would abort the whole matrix run instead of recording one step's repair as failed.

The landmarks evaluate has the same exposure and predates this PR, so this is not a regression you introduced — but it adds a second one in the spot that comment identifies. Catching into elements: [] (level still recorded) keeps a flaky capture from taking down a matrix run.

4. "No text content" is not quite what the code does

page-context.ts says every level excludes, unconditionally: "No text content, no input values, …". The name computation falls back to el.textContent for button/link/heading/tab/menuitem/option, and to el.closest("label").textContent. That is accessible-name computation, it is what boundary-spec.md and the ADR describe ("role + accessible name"), and the value canary tests pin the part that matters. So the code is right and the one-line summary is what's off — worth a word ("no free text content beyond the accessible name") given how carefully the rest of this PR polices that distinction.

Nit

contracts/metrics.schema.json got reformatted wholesale (every inline object re-expanded), which buries the one substantive change — the repair_context_level enum — in ~90 lines of whitespace churn. Not worth a revision here; worth knowing if the formatter is going to do that on every contract touch.

🤖 Reviewed with Claude Code

[myselfsiddharth]

Approving — full CI green, ADR separates measured from argued, and the counter-check tests make the canaries non-vacuous. Four non-blocking findings in the review comment above; the run-row/payload context_level disagreement is the one worth a follow-up.