report(bench): single results table with per-cell color + baked-in delta#194
Closed
hfurkanbozkurt wants to merge 23 commits into
Closed
report(bench): single results table with per-cell color + baked-in delta#194hfurkanbozkurt wants to merge 23 commits into
hfurkanbozkurt wants to merge 23 commits into
Conversation
added 2 commits
July 13, 2026 23:19
… Overview/Δ column Collapse the two-table report (Overview glyphs + Detailed numbers) into ONE results table. Each metric cell now shows the current value on line 1 and the signed delta vs the main baseline on line 2 (via <br>), colored by the significance+direction of the change: - green/red beyond a per-metric threshold, yellow within the noise band, ⚪ when the baseline has no value for that field (value still shown, tagged (new)). - thresholds centralized in DELTA_THRESHOLDS: composite/score ±5, judge ±0.3, tests ±1, cost ±10%, tokens ±10%; direction per metric (tests/judge/score up, cost/tokens down). No arrows — color only. Drops the whole-row schema-2 gate (baselineHasMetrics/perMetricBaseline): the baseline is diffed PER FIELD, so a field the baseline lacks degrades to ⚪ (new) for that cell only instead of forcing the entire row to 🆕. Removes renderOverview and the separate Δ-vs-base column. Presentational only — buildAggregate/ diffAgainstBaseline data shape (current/delta) unchanged, so analyze.mjs is unaffected.
Harden renderDetailed so a crashed/errored cell (0 tests, no tokens → null cost/score, no judge) can never drop a metric column and misalign the row: build the 5 metric cells into an array and coerce any empty string to the NONE placeholder before joining, guaranteeing the 8-column invariant structurally even if a future cell helper returns ''. Add a crashed-cell regression test that asserts every rendered row has exactly 8 columns with no empty cell. Presentational only.
…judge_dimensions is aggregate-only Sync the docs to the shipped single-table render: - README: replace the two-table (Overview colors-only + Detailed baseline->pr) description with the one results table (renderDetailed) — each metric cell is current value + colored signed delta (two lines via <br>), ⚪ (new) when the baseline lacks that field, value always shown. Swap MARGIN_PCT/metricColor/ renderOverview for DELTA_THRESHOLDS + deltaColor and list the per-metric bands and directions. Drop the baseline->pr / multi-line per-dimension Judge-cell wording from Re-derivability. - overview.mjs: one-line comment that cellMetrics.judge_dimensions is carried for the persisted S3 aggregate / re-derivability only and is NOT consumed by the rendered table (behavior unchanged). - changeset: refresh the body to describe the single table + baked-in per-cell signed delta + DELTA_THRESHOLDS coloring; still non-releasing. Docs + one comment only — no functional code change.
…C/files cols, inline delta, preword Render (overview.mjs, summary.mjs): - Scalar cells (tests/cost/turns/score) now inline: <ball> <value> (<Δ>), dropping the two-line <br> form. - JUDGE column stacks one <br>-joined line per rubric dimension (functional/selectors/persistence/code/blocks) with per-dim baseline deltas; overall judge folded into the preword. - TOKENS column stacks in / out / cached in / cached out; cache lines render ⚪ '(new)' until a baseline carries them. - New columns: Turns (cycle_count, lower-better), LOC (created/edited), Files (created/edited). - LOC & Files are NEUTRAL — always ⚪, value + signed delta, never green/red (no inherent good/bad direction). Cache tokens are DISPLAYED only, not deducted from cost/SCORE. - New DELTA_THRESHOLDS bands: turns ±3, cache read/write ±20%. humanTokens now compacts ≥1M to 'M'. - renderPreword: bulleted run summary (mean composite + Δ vs main, verdict counts, totals, biggest gains/drops, config). Replaces the old headline. - N-column guard extended to 11 columns; every row (incl. crashed/removed) emits all 11. Aggregate (overview.mjs buildAggregate/cellMetrics): - Persist cycle_count, cache_read_tokens, cache_write_tokens, loc_created/edited, files_created/edited so future baselines carry them. Instrumentation (2-agent-run.ts, partial-envelope.mjs): - Accumulate usage.cacheReadInputTokens/cacheWriteInputTokens in the ModelStreamUpdate hook; persist cache_read/write_tokens on every envelope path (checkpoint, partial, error, success), success taking max(hook, winnerUsage). - workspace-diff.mjs: snapshot the scaffolded workspace before the agent runs via a throwaway external GIT_DIR (never touches the workspace's own .git), diff after for LOC/files churn on the success path. Degrades to null on any failure. Tests: overview.test.mjs updated for the 11-column layout (per-dim judge, 4-line tokens, turns/LOC/files, ⚪ '(new)', 11-column guard, M formatting); workspace-diff.test.mjs added (real git against a real temp workspace + pure parseChurn). tsc clean; 182 pass / 4 skipped (isolation tests need unshare).
…ean) — stop masking backend crashes
added 5 commits
July 15, 2026 04:57
…ale logs, never abort under set -e (fixes false build-fail from 2d53028)
…ace analysis confabulation - scoring: rename AGENT_HARNESS_TEARDOWN_REASON -> AGENT_HARNESS_TIMEOUT_REASON (value 'agent_harness_teardown' -> 'wall_clock_timeout'). Under active isolation an ungraceful step-2 death cannot be an agent pkill-storm (cross-uid EPERM), so it is an infra wall-clock timeout, not a teardown. Exclusion behavior unchanged (still harness_error, still excluded). - analysis: add pure deterministicCellAnalysis + NO_TRACE_ANALYSIS; when a cell has no agent trace (timeout path writes none) emit a deterministic 'undetermined' note instead of letting the model confabulate an owner/cause. - analyze-cell: route through deterministicCellAnalysis. - tests: scoring.test + analysis.test cover both changes.
…tic and add email-digest scope-hygiene note
…ability falsy-echo and oidc unauthenticated probe
…rrors (all 10 specs)
added 3 commits
July 15, 2026 11:36
… compact Judge to one line The single-table agent-bench comparison report was too verbose. Reduce it: - Remove the Tokens, LOC, and Files columns (8 cols now: Task|Template|Tests| Judge|Cost|Turns|Score|Stop reason). buildAggregate still persists the token/cache/LOC/file counts to the S3 baseline for offline analysis — only the rendered columns are dropped. - Compact Judge to ONE line: overall score + per-dimension shorthand (F/S/P/C/B) with per-dim signed deltas, instead of one line per dimension. - Update the glossary (summary.mjs), the README table description, the changeset, and the overview.test.mjs snapshot expectations to match. - Fix a latent $-escaping bug in signedCost surfaced while removing signedTokens.
Fixes 3 harness/analysis bugs that let the bench silently score, exclude, or misattribute cells for the harness's own failures (cognito-profile MaxTokensError mislabeled a timeout; kb-chat ungraceful kill given a fabricated "build failed"). - 2-agent-run.ts: persist trace.json on EVERY exit path via a per-turn MessageAddedEvent accumulator (captures toolResult blocks, not just toolUse), so a non-returning invoke() (MaxTokensError / wall-clock kill) is still analyzable. Factored pure buildTraceArtifact() into partial-envelope.mjs. - 2-agent-run.ts: self-imposed internal deadline (AGENT_DEADLINE_SEC, ~33.5min, < the 35min GH step timeout) proactively flushes a graceful partial envelope + trace; agent-bench.yml execs tsx so signals reach the process. - scoring.mjs: MaxTokensError -> klass_reason='max_tokens' (kept agent_fail / composite 0 / INCLUDED), no longer mislabeled 'agent_timeout'. - analysis.mjs + analyze-cell.mjs: deterministicFailureAnalysis() short-circuits the model for harness_error / no-trace / failed_at==='2-agent' (no confabulation); buildEvidenceObserved() stops reporting seeded pessimistic build defaults from a skipped step-3 as observed build failures. - maxTokens left at the SDK default (64000); tests added for classify + honesty guard + trace-artifact.
🦋 Changeset detectedLatest commit: ab85029 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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
Redesign the bench report into one results table, replacing the previous two-table layout (a colors-only Overview + a numbers-only Detailed table, plus a separate
Δ vs basecolumn).Each metric cell now shows the current value on top and the signed delta vs the
mainbaseline underneath (two lines via<br>), colored by the significance + direction of the change:(0)
(0)
(+$10.11)
(+1968.3K)
(-32.6)
(new)
(new)
(new)
(new)
(new)
Why
The report should always show the current numbers — the old Overview was glyphs-only and hid values, and no-baseline cells rendered as a bare 🆕 with the value suppressed. Now the current value is always shown, tagged
⚪ (new)when there's no baseline value for that field.Details
renderOverviewand the separateΔ vs basecolumn are removed;renderDetailedis the one table.⚪ <value><br>(new)when the baseline has no value for that field (never hidden).baselineHasMetrics/perMetricBaseline): a field the baseline lacks degrades to⚪ (new)for that cell only instead of forcing the entire row to 🆕. Self-heals per-metric as baselines gain fields.DELTA_THRESHOLDS(one place, commented): composite/score ±5 · judge ±0.3 · tests ±1 pass · cost ±10% · tokens ±10% (in+out combined).—placeholder in every column and can never drop a cell / misalign the row (structurally guaranteed + regression-tested).Scope
Presentational only. Changes are confined to the render layer —
steps/lib/overview.mjs,steps/summary.mjs, and their tests (steps/lib/overview.test.mjs). No scoring / aggregate / baseline-data changes:buildAggregateanddiffAgainstBaselinekeep the same data shape (current/delta), soanalyze.mjsis unaffected.Validation
node --test: full agent-bench suite 162 pass / 0 fail (overview suite 32/32).tsc --noEmit: no new errors (only the pre-existing@strands-agentstype mismatches).29283998687vs baseline run29283582822) through GitHub's GFM renderer (gh api /markdown) and confirmed the<br>two-line cells render inside the table cells and every row has exactly 8<td>s.➕ Now also includes: deep root-cause analysis on failing cells (folded in from #195)
This branch was merged with
bench/deep-failure-analysis(PR #195, now closed as superseded) so the two bench improvements ship as one PR. Both features coexist — the merge was conflict-free (disjoint files) and the combined suite is green.What the folded-in change adds — a deeper analysis pass that fires only on failing cells (passing / clean-partial cells stay on the existing cheap per-cell path, no extra model call). Motivated by
auth-notes/demo(0/11) being misdiagnosed as a "scoring/harness mismatch" because the analysis never saw the failing test output.steps/lib/analysis.mjs—isFailureCell(trigger predicate),extractFailingTests(walks the nested Playwright JSON, dedups identical errors → grouped counts, caps 8 groups × 300 chars),FAILURE_SYSTEM(strict-JSON prompt),buildFailureUserText(failing tests + dev/build log tails ≤1500 + judge ≤1500 + trace tail),parseFailureAnalysis(tolerant bare/fenced/prose-wrapped JSON → object, malformed →null, never throws),FAILURE_MAX_TOKENS=600.steps/analyze-cell.mjs— after the cheap analysis,if (isFailureCell(result))reads the staged evidence, calls the same Bedrockconversepath (reusesBENCH_JUDGE_MODEL), writesresult.failure_analysis. Fully inside the never-throw / always-exit-0 isolation.steps/3-build-and-test.sh— copiespw-results.json/dev.log/build.logto stable/tmppaths (per-stepCELL_TMPuses$$; mirrors how/tmp/trace.jsonis staged). Best-effort, never fails the build step..github/workflows/agent-bench.yml— passesPW_RESULTS/DEV_LOG/BUILD_LOGenv into the "4b. Analyze cell" step.steps/analyze.mjs— renders a## 🔴 Failure root-causesection (category badge, owner, root cause, quoted-evidence blockquote, likely fix) and folds category/owner counts into the exec summary. Renders alongside the single Results table + preword above.steps/lib/analysis.test.mjs— unit tests for all four pure helpers (dedup, cap, nested suites, build-log gating, prose/fence parsing, null-safety). No live Bedrock.Combined validation (post-merge):
tsc --noEmitclean ·node --test211 tests / 207 pass / 0 fail / 4 skipped (both suites) ·bash -nOK ·agent-bench.ymlYAML parses · no conflict markers.➕ Now also includes: quality-audit fixes (folded in from #201)
This branch was merged with
bench/quality-audit-fixes(PR #201, now closed as folded-in) so the bench quality-audit fixes ship in this PR. The merge preserved both intents — this branch'sdead_server→fail classification + the audit fixes above, and the #201 fixes below — and the combinedscripts/agent-benchsuite is green.Fixes from the deep-dive audit of the prior bench run (4 cells — C1 sql-kb-catalog, C2 observability-api, C3 oidc-dsql-notes, C4 cognito-profile — plus the auth + kb-chat harness reviews):
Included (8):
tasks/*/test.spec.ts: the sharedwatchErrors()gate now also fails on genuineconsole.error(scoped to error-type; excludes benignFailed to load resource/net::ERR/favicon/WebSocket/ ReactWarning:noise). Every PROMPT promises "no errors in the browser console"; the gate only checkedpageerrorbefore.tasks/async-word-counter/PROMPT.md: fixed the arithmetically-wrong worked example (hello,world foo.bar-baz!is 2 whitespace tokens) →hello,world foo.bar baz(genuinely 3; naive\W+split = 5), preserving the "3 not 5" contrast. The spec's own punctuation test was verified consistent and left unchanged.tasks/oidc-dsql-notes/test.spec.ts: unauthenticated JSON-RPC probe (no session → no note data; robust to unknown-method, so never spuriously fails).tasks/observability-api/test.spec.ts:api.echoround-trips falsy-but-present args (0,false,null,"") — catches truthiness-based "missing arg" footguns.tasks/sql-kb-catalog/test.spec.ts: FAQ tests now assert the returnedkb-resultcontent (/refund/i), not just element presence.tasks/email-digest/PROMPT.md: scope-hygiene instruction to remove unneeded scaffold/demo code.steps/lib/scoring.mjs(+test): renamedagent_harness_teardown→wall_clock_timeout(a checkpoint-survived timeout under active isolation can't be an agent pkill-storm — cross-uidEPERM; exclusion behavior unchanged). Coexists with this branch'sdead_server→fail classification.steps/lib/analysis.mjs+analyze-cell.mjs(+test): when no agent trace exists (timeout path) and no failing tests, emit a deterministicNO_TRACE_ANALYSIS("no trace — undetermined", owner/categorynull) instead of confabulating owner=agent / category=build.Skipped (3, noted): #6 C3-F1 oidc per-user isolation (stub IdP exposes one fixed subject — needs a harness/stub change); #7 C4-F1 cognito
getLastCode(mock-only, not observable outside local/mock); #8 C2-F1 observability instrumentation-fires (no telemetry readback path — not faked). Framework defects (packages/**) were drafted, not filed.Combined validation (post-merge):
npm run test:ci -w scripts/agent-bench(tsc --noEmit+node --test) green — 243 tests / 239 pass / 0 fail / 4 skipped (union of both suites) · all 10 specstsc --noEmitclean · no conflict markers ·dead_server→fail +wall_clock_timeout+NO_TRACE_ANALYSIS+ console-gate all verified to coexist.