Repository navigation
feat(digest): trace presets name only real failures, and the pointer is measured - #36
Merged
Merged
Conversation
…ot its log
Measured defect, three sessions of the benchmark's first cell, all three alike: after a bgrun
handoff every session polled the job it had just started — 27 of 30 tool calls were made while
the job was still running — and every session asserted the completion wake in writing, the first
claim 3-4 seconds into a ~23s job ("**Wake received.**"). The wake itself is event-driven and
correct; the loop came from three gaps: an agentic loop has no idle primitive, so "I'll wait for
the job to finish" is implemented as a check; the check paid, because bgtail served a running
job's log in deltas; and no tool result carried a clock, so "has the wake arrived?" was
unfalsifiable from inside the loop.
This makes the tools match the contract they already documented (they said "do not poll" and
then served the poll):
- bgrun's result and guidance state that the task is complete at handoff: "Your part is done —
end your turn now... the wake will arrive as a new turn" with the exit code, stats and last
line.
- bgtail/bggrep on a still-running job return its state and nothing else — elapsed time, no exit
code, and where the wake will come from. The gate fires before any tail bookmark moves, so a
later read is still a first read, not a delta.
- peek: true is the explicit opt-in for a live log, so watching a long job stays possible;
/bgtail passes it, since a human asking for it is the opt-in. bgstatus keeps its output and
gains the same state line; its list rows are left alone — a listing is not a poll of one job.
- A finished job's output is byte-identical to before, which is why no finished-job test changed.
- Descriptions, README and the run-bg skill no longer advertise polling as free. The one existing
test that needed adjusting was itself running-shaped (a log id ending in process.pid with no
exit marker), so it now writes an exit marker and still exercises the match budget.
Verified by the implementing pass: bun test extension/index.test.ts — 231 pass, 0 fail;
bun run lint (tsc --noEmit) exit 0.
Measured in the benchmark's trace-root cell: after a wake that pointed at nothing, all three bgrun sessions searched the log for failure words — a pattern that cannot match a frame line — so the file holding the cause appeared in 0 of 3 of their contexts, against 3 of 3 on the full-read arm. The wake's stats and "last output" (the log's final line) are not pointers. The wake now carries one: Failure: AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal: — at loadStep (.../harness.ts:28:9) - One bounded scan, folded into the line-count read the exit path already performs: no second read of the log, no new dependency. Whole-log rather than tail-only, because long-buried plants its failure around 61% of the way through a ~1000-line log. - FRAME_LOOKAHEAD is 20, not the 5 first sketched, because the real log measures the gap: the assertion header sits 10 lines above its frame with a printed diff payload between. A 5-line window would emit a frame-less failure line — the exact miss this change fixes. The constant's comment carries the measurement. - It runs only when the exit code is non-zero, so a passing job's wake is byte-identical: a green log containing "Error:" or a frame-shaped line cannot produce a Failure: line. - One failure is named and no count is claimed: a total would mean matching every runner's failure spelling, and a wrong total is worse than none. Bun runs files concurrently, so the first failure in the log is a real failure, not necessarily the most important one — the code says as much. - Spaced function names match (`at load part 02 (...)` is what the fixture prints); a name-scoped pattern silently dropped such frames, the same class of bug the profiler hit with line-numbered views. 7 new tests: the failure line with its frame in one wake; a block failure's frame across a printed payload (the trace-root shape); a spaced function name; a failure line with no frame; a failure buried mid-log; a passing log containing "Error:"; and two failure blocks naming the first once. bun test extension/index.test.ts — 238 pass, 0 fail; bun run lint clean. Verified end-to-end against the benchmark's real trace-root log, where the wake renders the harness.ts frame.
…the built-in locator The built-in locator was a default tuned to the product's own runner: measured over a nine-entry multi-runner corpus it worked on 2, both JS/TS. It is deleted. In its place: - `on: "failure"` on a digest entry, enforced by the framework (it alone knows the exit code; a digest command receives only the log path). A trace preset's block is appended for a failing job and cannot appear for a passing one, even when the log contains "Error:". - Three shipped presets, split by stack ORIENTATION because that, not language, is the correctness axis: `js-trace` (innermost first — JS/TS, Go, Rust, JVM), `py-trace` (outermost first: the LAST `File "…", line N` frame is the deepest), `rb-trace` (minitest's raise site). Each is one POSIX awk pass over $1; runtime paths are excluded so the user frame wins. - `junit-xml` now appends a location when the emitter carries one (`name (file:line)`, else `name (file)`, else the name) — the structured path, real-captured from bun and node. Corpus re-run through the presets, per entry: bun equal, node equal, Go and Rust GO from no frame to the user frame, Java detected with a frame where the built-in found none, minitest detected without one (its frame shape is a third pattern), python corrected from the caller's line to the throw site. Nothing regressed; that is what licensed deleting the built-in. Also: the wake's closing line is now "a summary, not the diagnosis — the log holds the detail, including the context around any failure", on both the passing and failing branches, so a session is not invited to conclude a cause from a summary alone. The old FRAME_LOOKAHEAD/FRAME_LOOKBEHIND semantics live on as the js-trace awk cap and backward walk, noted in its comment. bun test extension/index.test.ts — 242 pass, 0 fail; bun run lint clean.
The nudge fired only at session_start, and only for a project with no digest at all - so a project whose digest reports counts only, this repo's own .pi/pi-bgrun.json among them, never hears that trace presets exist. The value of a digest is legible at the moment a job fails, which is where this now fires as well. The nudge body is shared: session_start keeps its wording, and the child-exit handler calls it on a job's first non-zero exit with a moment-aware string (DIGEST_FAILURE_NUDGE_TEXT). Both share the same per-project marker, so a project is nudged at most once either way, and both stay toast-only via ctx.ui.notify - never sendUserMessage, so it costs zero LLM context. The tool still never writes config files. Deliberately NOT added: detection of a digest that "produced no failure signal". Judging whether output contains a failure needs per-ecosystem knowledge - the fitted regex this branch deleted from the core, wearing a smaller hat. The docs state the counts-only limitation instead: a scorecard that reports counts only says a job went red, not where or why, and the trace presets (js-trace, py-trace, rb-trace) exist for that. Tests: 246 pass / 0 fail (four new - fires once on a first failing job in a trusted digest-less project, not on a passing job, not twice, not when a digest is configured). tsc --noEmit clean.
…naming anything Found by the benchmark's first cell with a digest configured, from a kept real log (bun test, 1,281 lines, exit 1): the preset named a PASSING test's line - "Failure: (pass) preset js-trace corpus (captured): bun test - stack above the `(fail)` summary" - and the sessions paid for it. Root cause: the signature regex matched `(fail)` (and FAILED / AssertionError) as an unanchored substring on any line, so a passing test whose *name* mentions `(fail)` won. The trigger is self-referential: the suite under test prints its own preset corpus, so the log contains failure-shaped strings from passing tests. Two rules, both about evidence rather than incident text: - the runner's markers are anchored: `^[[:space:]]*\(fail\)` and `^[[:space:]]*--- FAIL`. A mid-line mention inside a printed name is not a failure record. - a candidate line that is itself a pass record is skipped, so a passing test's name can never win however many failure words it carries. With no anchored evidence the preset now prints nothing. That is the measured rule, not a stylistic choice: a wake with no pointer cost 3-4 calls and 18,760 chars, the same cell with this wrong pointer cost 5-6 calls and 28,633 chars - a claim that may be wrong must be verified, so silence beats a guess. Frame selection is otherwise unchanged. Tests: 248 pass / 0 fail, including two new ones from a 4-line slice of the real log - a passing test named after `(fail)` never wins, and failure words that appear only inside pass records yield no pointer. tsc --noEmit clean.
Contributor
CI report
Ref: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Two things, both about what the completion wake can honestly claim.
The presets name real failures, or nothing.
js-tracenow anchors on the runner's own failure markers ((fail),--- FAIL) and refuses lines that are themselves pass or captured-output records.README.mdandskills/digest-config/SKILL.mdboth promise the presets print nothing when their shape is not recognised; before this they could name a passing test, a docstring, or a build-log warning. A wrong pointer was measurably worse than no pointer: 5/5/6 calls and 28,633 chars against a baseline's 3/4/4 and 18,760.The pointer is honest about being a lead. The wake's hedge - "any failure named here is one failure, not the whole story" - is what makes it safe, and measurement says it also makes it cost: with a correct pointer, sessions ran 5/5/8 calls and 33,799 chars, more searching than a baseline with nothing to verify. That is the finding in #33, not a defect in this diff.
Review
The wake text is seen by every session, so the wording change deserves a read rather than a skim:
extension/digestPresets.ts, and the state line inextension/index.ts.Known limits, stated rather than implied
py-trace/rb-traceare untouched. Trace presets still name non-failure lines: the evidence rule covers one alternative in one preset #34 has the probes and the test gap.anthropic/claude-sonnet-4-6), one synthetic fixture family, one machine. No significance claimed.Checks
tsc --noEmitclean; 248 pass, 0 fail;npm pack --dry-runships 7 files with no bench artefacts.Related: #33, #34, #35.