feat(viewer): reviewer-grade trajectory pages, multi-run browsing, and hf:// dataset sources by ljr145733 · Pull Request #1034 · benchflow-ai/benchflow

ljr145733 · 2026-08-17T05:57:13Z

Summary

Reviewers auditing trajectories (health checks, failure attribution, reward
hacking, no-skill leakage — the weekly-sync "[8pt - rollout] viewer" ticket)
need more than the current ACP page gives them: today it truncates every
message at 500 chars, shows tool calls without their output, and has no
stats, filters, or verifier view. This PR upgrades bench eval view's ACP
path into an interactive review page, adds a multi-run browser, and makes
HuggingFace trajectory datasets directly viewable — while keeping every
existing behavior (--confirm contract, session JSONL files, legacy
turn*.txt renderer) byte-for-byte intact.

Built on and styled after the 0.7.x viewer stack (#1019 site restyle, #1020
accents, #1021 --confirm, #1022 redaction summary, #1023 badge rules):
same light monochrome language, same _VIEWER_CSS token names (the confirm
bar injects with zero adaptation), same pastel accent palette.

Usage

1. Local folders. A single rollout renders the interactive page; a
directory of rollouts (a job dir, a whole jobs/ tree) serves a browser
with a run sidebar — task, pass/fail, harness, model, skill mode — traces
loaded dynamically without restarting, and ?run=<id> deep links:

bench eval view jobs/run/task__abc123            # one rollout
bench eval view jobs/ --port 9000                # browse every run under jobs/

2. HuggingFace datasets. The community ground-truth uploads become
reviewable with one command. Only the viewer-relevant files are fetched
(trajectories + result/timing/prompts/verifier sidecars — never the large
llm_trajectory/trainer exports) into the shared huggingface_hub
cache, so repeat views are incremental:

bench eval view hf://benchflow/skillsbench-trajectories-apr2026/jobs/opus47-with-skills-t1

Validated live: 225 runs from that dataset (and 25 local FrontierPhysics
rollouts, including old-schema/partial/timeout cases) browse and render end
to end.

What the page gives reviewers

Full fidelity: no truncation anywhere — long prompts/thoughts/outputs
collapse with an expand control; ACP diff content blocks render as
readable pseudo-diffs; agent_timeout events render as cards (previously
silently dropped); unknown/future event types degrade to generic cards.
Failure attribution at a glance: header surfaces error_category,
verifier_error, and the seven *_info diagnostic blocks from
result.json as banners, plus harness/model/skills identity tiles and
token/cost/duration stats.
Reading tools: Focus/Full modes, per-kind filters (one pastel hue per
tool kind, dark terminal treatment reserved for execute output),
failed-only, text search with jump, per-event #e42 anchors for citing
steps in review threads; chunked rendering stays responsive on large
traces.
Verifier + Metrics tabs: reward, test stdout/stderr, CTRF table,
phase-timing bars, token breakdown.
Timeline, forward-compatibly: steps show +m:ss offsets and tool
durations whenever events carry ts/started_at/finished_at (Subscription-auth runs (agent_native_acp) have no event-level timing — serialize timestamps in acp_trajectory.jsonl #1033);
a scan of 140 non-empty uploaded trajectories (5,698 events) found zero
timestamp fields today, so nothing changes visually until capture writes
them.

Compatibility

serve() becomes a thin dispatcher; single trajectories go through
_serve_single, which is byte-identical to the previous serve() body —
confirm/DECISION: contract, sidecar write-before-inject ordering, exit
semantics, JSONL session files all preserved. --confirm on a multi-run
directory errors out (a confirmation needs exactly one trajectory).
Raw session files keep the existing _render_acp_events renderer whose
output the jsonl-session tests pin; the legacy turn*.txt renderer is
untouched apart from errors="replace" unicode hardening on reads.
All pre-existing viewer tests pass unmodified.

Security

Trajectory content is untrusted input end to end: it travels as JSON data
(</ escaped against script-tag breakout) and the template renders
exclusively via textContent; browse-mode ids resolve only by exact
membership in a fresh directory scan, so crafted ids (../ traversal)
cannot reach the filesystem. An adversarial review pass produced five
verified findings (lone-surrogate encode crash, renderer-halting type
confusion, non-UTF-8 sidecar crash, null-timing crash, diff-block
rendering) — all fixed with regression tests.

Testing

tests/trajectories/test_viewer_acp_renderer.py (new, 19 tests): payload
contract, escaping/hostile-input cases, degradation, browse discovery +
anti-traversal, hf:// spec parsing with a mocked download (asserting the
large files are never requested), timestamp passthrough.
Full suite green locally (pytest tests/: 5685 passed), ruff check,
ruff format --check, ty check clean.
Live smoke: confirm flow E2E on a template page (bar + redaction note,
POST /decision, single DECISION: line, clean sidecar), browse API +
traversal guard, HF dataset end to end.

Screenshots

(attached below: single-run page, browse mode over a real experiment batch,
HF dataset browsing, --confirm bar on the interactive page)

Related: #1033 (capture-side timestamps — the timeline above renders the
moment it lands).

Galius5136 · 2026-08-17T07:08:46Z

Really enjoyed this one! I tested the main paths pretty hard and a lot of it held up well: traversal attempts all 404'd, the single-run path stayed compatible with main, the confirm flow worked end to end, and the hf:// path worked on the real 225-run dataset without pulling the large trajectory/trainer exports.

A few things are worth fixing before merge. bench eval view --help currently exits 1 because Rich interprets [/subpath] in the help text as a closing markup tag; it's also the only head-only failure I found in the full test lane. ruff format --check src tests tools also currently fails on the new viewer test file. The other behavioral issue I reproduced is the 500-run cap: with 620 local rollouts, opening ?run=...0619 leaves that id in the address bar but silently renders run 1 instead, and the requested run is 404 through the API. The advertised HF dataset is below the cap, so this is specifically a larger local jobs-tree case.

One test-coverage thing I’d tighten too: both security mitigations work today, but their regression tests don’t actually pin the enforcement. Removing the </ escaping still leaves all 19 tests green while trajectory text becomes live script markup, and removing the browse membership check also stays green while an outside rollout becomes reachable. Small integration note as well: _DIAGNOSTIC_KEYS currently mirrors DIAGNOSTIC_REGISTRY, but #1025 adds another diagnostic, so deriving that list from the registry would avoid future drift (with a neutral style for category-less behavior flags).

Happy to re-test after any updates 🙂

All five findings from the review pass: - `bench eval view --help` no longer exits 1: the help text carried "[/subpath]", which Rich parsed as a closing markup tag; rephrased without brackets. - `ruff format --check src tests tools` passes (the new viewer test file was unformatted). - The browse-mode run cap is no longer silent: the sidebar heading says "first N runs (capped)" when truncation happened (detected by scanning cap+1, not by len==cap), a `?run=` pointing at an undiscovered id shows an explicit load error instead of silently rendering the first run, and BENCHFLOW_VIEWER_MAX_RUNS overrides the 500 default. - The two security regression tests now pin enforcement (mutation-killing): the script-breakout test asserts the raw `</script><script>` sequence appears nowhere in the emitted page (removing the escape fails it), and a new whitelist test resolves ids through the extracted `_resolve_browse_rollout` helper against a real rollout placed outside the served base (removing the membership check fails it). - Diagnostic banner keys derive from `DIAGNOSTIC_REGISTRY` instead of a hand-copied list (static fallback kept for lenient imports), so new diagnostics like benchflow-ai#1025's chat-only flag appear without drift — and diagnostics on an otherwise-clean rollout (no error/verifier_error) render as neutral info banners rather than red error banners. Co-Authored-By: Claude Code <noreply@anthropic.com>

ljr145733 · 2026-08-17T19:36:31Z

Thanks for the thorough pass — reproduced every finding, all five addressed in d26931b:

--help: Rich was parsing [/subpath] in the argument help as a closing markup tag; rephrased without brackets. Exits 0 now.
ruff format --check src tests tools passes (the new test file was the offender).
Run cap: the sidebar heading now says "first N runs (capped)" when truncation actually happened (detected by scanning cap+1 rather than len == cap), a ?run= pointing at an undiscovered id shows an explicit load error instead of silently rendering run 1, and BENCHFLOW_VIEWER_MAX_RUNS overrides the 500 default.
Security tests now pin enforcement: the breakout test asserts the raw </script><script> sequence appears nowhere in the emitted page (fails with the escape removed), and id resolution moved into a _resolve_browse_rollout helper with a test that places a real rollout outside the served base and asserts it never resolves (fails with the membership check removed). Good catch — the old assertions were theater.
_DIAGNOSTIC_KEYS derives from DIAGNOSTIC_REGISTRY (static fallback kept for lenient imports), and diagnostics on an otherwise-clean rollout now render as neutral info banners instead of red — so Flag chat-only completions (zero tool calls with model output) as a diagnostic #1025's chat-only flag will slot in without reading as a failure.

Would appreciate a re-test whenever you have time 🙏

bingran-you · 2026-08-17T19:51:54Z

@ljr145733 — thanks for the substantial first contribution. I reviewed commit d26931b720bcedae698a8795aafe6d31373d500d.

Review — changes requested

The viewer direction is valuable, but I don’t think this implementation is maintainable or yet matches the intended product design. Please address the following before merge.

1. Decompose the viewer architecture

src/benchflow/trajectories/viewer.py grows from 872 to 1,492 lines, while the new template adds another 917 lines—roughly 278 lines of CSS and 607 lines of global-state JavaScript.

One module now owns:

Legacy rendering
ACP normalization
Diagnostic inference
Verifier/metrics loading
Run discovery and catalog creation
Local and Hugging Face source resolution
Two HTTP-serving modes

Please separate responsibilities, for example:

trajectories/viewer/
  models.py       # Typed payload/view-model contract
  payload.py      # ACP normalization and diagnostics
  sources.py      # LocalPathSource / HfDatasetSource
  catalog.py      # Run discovery and summaries
  server.py       # HTTP routes
  legacy.py       # Existing raw-session renderer
  assets/
    theme.css
    viewer.css
    viewer.js
    template.html

Preserve benchflow.trajectories.viewer through __init__.py.

Also replace the loosely coupled dict[str, Any] Python → JavaScript boundary with an explicit typed payload contract. Tool-kind classification and visual tokens are currently duplicated across Python and JavaScript and can drift.

2. Match SkillsBench’s actual visual system

The current CSS approximates SkillsBench but does not implement it. Declaring font-family: Satoshi does not load Satoshi; the generated page has no font import or @font-face, so most systems will silently use a fallback.

Please use the live SkillsBench site as the visual source of truth:

UI font: Satoshi
Monospace font: Google Sans Code Variable
Background: lab(97.6714% .0993311 -.364053)
Foreground: lab(2.75381% 0 0)
Primary: lab(7.78201% -.0000149012 0)
Border: lab(90.952% 0 -.0000119209)
Radius: .625rem
Canonical semantic tokens: success, warning, info, destructive and chart colors

Extract these into one shared theme rather than maintaining separate approximations in viewer.py and viewer_template.html. Keep the exported viewer self-contained by packaging/inlining the required fonts, subject to their licenses.

3. Adopt PostTrainBench’s information architecture, not its visual styling

With the PR’s example dataset, the viewer renders 225 runs as one flat 280px sidebar. There is no run-level filtering, grouping, sorting or pagination. This will not scale.

Use the PostTrainBench trace browser and its index interaction logic as the layout/behavior reference:

Browse screen

Overview/performance matrix
Corpus statistics
Search and filters for benchmark, model and agent
Grouping by task, task × model, experiment or none
Sorting by reward/accuracy, duration, turns and cost
Collapsible groups
Incremental “show more” pagination
URL-preserved filter and navigation state

Run-detail screen

Sticky summary rail
Trace / Verifier / Metrics tabs
Focus mode by default, with Full as an option
Jump-to-turn and event anchors
Back navigation that preserves the browse state

The PostTrainBench run-detail logic is a useful behavioral reference. Please copy the information architecture, not its cream/monospace visual identity—the BenchFlow viewer should use SkillsBench styling.

4. Tighten the `hf://` download allowlist

_HF_VIEWER_FILES contains verifier/*, but _load_verifier() consumes only:

verifier/reward.txt
verifier/ctrf.json
verifier/test-stdout.txt
verifier/test-stderr.txt

A live resolution of the PR’s example dataset downloaded 1,516 files. Of those, 418 verifier files—approximately 31.3 MB—were not consumed by the viewer, including large OBJ, MP4, PDF, NPZ and log artifacts.

This contradicts the claim that only viewer-relevant files are fetched. Replace the wildcard with the four precise paths and add a regression test that rejects any broader verifier/ pattern.

Please also stop passing hf:// through a Path, which mutates it into hf:/ and forces the parser to accept the corrupted spelling. Parse the CLI value as a string into an explicit LocalPathSource | HfDatasetSource type and validate dataset subpaths.

5. Add browser-level regression coverage

The Python payload tests are useful, but none of the approximately 607 lines of JavaScript interaction logic is exercised in a browser.

Please add browser-level tests covering at least:

Filtering, grouping, sorting and URL-state restoration
Selecting a run and preserving back navigation
Trace/Verifier/Metrics tabs
Focus/Full switching
Search, jump-to-turn and event anchors
Run-loading and malformed-payload error states
Untrusted trajectory content remaining text rather than HTML
Representative desktop and narrow viewport screenshots

6. Preserve the promised “full event stream”

Unknown event payloads and diagnostics are silently truncated at 2,000 characters. That conflicts with the advertised full event stream. Render complete expandable payloads, or explicitly label truncated previews and provide access to the complete content.

Suggested PR split

Since you offered to split the work, I recommend:

Typed viewer payload, ACP single-run rendering and shared SkillsBench theme
Scalable run catalog/browser using the PostTrainBench interaction model
hf:// source adapter with a precise download allowlist

Verification performed

Focused tests: 97 passed
Ruff, formatting and type checks passed
git diff --check passed
The live viewer was exercised with 225 discovered runs
The SkillsBench and PostTrainBench reference sites were inspected interactively

These checks show that the current behavior largely works; the requested changes are about scalability, source boundaries, visual fidelity and long-term maintainability.

ljr145733 · 2026-08-18T07:51:50Z

All six review points should be addressed now:

viewer.py split into a package with a typed payload contract (5a7bbfc, e34c913)
theme rebuilt on the actual skillsbench.ai tokens, Google Sans Code embedded under OFL — Satoshi isn't redistributable, so it falls back (a490d48)
browse mode got a real catalog: group by task / model+harness, sort, filter, URL-state restore (5d1c56f). As discussed, the overview matrix is deferred for now
hf:// downloads use an exact file allowlist and typed source parsing (a4c2f76)
browser-level tests driving the template in real Chromium (16e1082) — CI now installs chromium so they actually run instead of self-skipping
server-side truncation removed; long text collapses client-side instead (e34c913)

After that I did another adversarial pass over the catalog code and fixed what it turned up in 7969512. The two worst: a failed /api/rollout load could leave the previous run's data on screen with the error invisible, and the inherited do_HEAD served files from the process cwd, bypassing the rollout whitelist. Each fix has a browser regression test.

Take another look when you have time — happy to adjust anything.

ir_to_view.py converts a trace into the step list a viewer page renders. It produces steps and deliberately not a whole payload: of a payload's five fields, only the steps are a function of the trace. rollout_name is a directory name, verifier is four sidecar files, and meta comes from result.json and timing.json - the IR has no slot for task_name, skill_mode, reward, partial_trajectory or trajectory_source at all. Returning a payload here would mean synthesizing run metadata to fill a shape rather than converting a trace, so the trace-level fields the IR does carry are declared UNSUPPORTED: outside this edge's codomain, not lost by it. Assembly belongs to the wiring slice, which has the directory. The wire shape is read from the viewer package proposed in benchflow-ai#1034, at the commit recorded in VIEW_SCHEMA_ORIGIN. Nothing is imported, vendored or fetched from that branch - it is unmerged, and the family rule is the one ir_to_atif already follows for ATIF: read the target format as data, pin what matters by test, never reach into the module that handles it. That pin protects this edge from drifting; it cannot notice benchflow-ai#1034 changing. tool.name_semantics is a seventh key on an object benchflow-ai#1034 defines with six, and it is here because the alternative is inference. benchflow-ai#1034 derives a tool's display category with tool_hue(kind, title), which scans both strings for needles: an ATIF function_name of 'execute' becomes the execute category, and an OTel gen_ai.tool.name of 'read_file' becomes 'read' because the word appears in it. This edge emits a hue only when the semantics say acp_kind and the value is already a member of the display vocabulary - membership, never inference - and otherwise the neutral 'other', which the renderer maps to the secondary tokens and which therefore asserts nothing. On the corpora that means 'execute' is spelled identically in the ACP and ATIF rows and gets a category in one only. Unlike the ACP edge this one never raises. A viewer is a display: an event it cannot type must still reach the page. Every event becomes exactly one step, UNKNOWN and ORACLE included, and what the shape cannot hold goes to the loss report. ORACLE has no member in the step vocabulary, so it lands on 'unknown' with its identity in the type slot, and both untyped kinds carry a serialization of the canonical IR event - not a raw source record, since the IR does not hold one, and the loss record says so. Sentinels are declared per slot and only where the target has no null to write. An absent title and an observed empty one both render as "", so the report is the only place that difference survives; a block the IR holds with no text contributes no string rather than having its raw form rendered as JSON. Still unwired. ir_to_view joins the IR family in the isolation test, nothing on a run path imports it, no page is rendered and no artifact changes. ir.py, ir_round_trip.py and the existing viewer.py are untouched. tests/trajectories: 752 passed (699 + 53). Mutation harness: 13/13 applied and caught. Human verification V1-V10 recorded in docs/trace-interop.md section 8.14, along with what it does not cover.

`ir_to_view_html` closes the chain `ir_to_view` opened: a viewer step list becomes one HTML page, built from the card builders `viewer.py` already renders its ACP page with. The dependency runs one way — this module imports the viewer, the viewer imports no part of the IR family — so what crosses the boundary is a plain step document and the renderer stays a renderer. It is written against the viewer on main. benchflow-ai#1034 stays a design reference for the step vocabulary and nothing else: no code from that branch is imported, vendored or fetched, and this edge works whether or not it lands. It does not rebuild steps into ACP capture events. The IR holds records ACP has no type for, a status it cannot spell, and tool names whose semantics are the point; forging capture events would launder all three back into the assumption the hub exists to remove. Measured against `_render_acp_events` on the two captured rollouts: H2's four events render three cards there and the word "timeout" appears nowhere, an unrecognized record reaches no card, and a tool card carries kind/title/status but not the tool's output. Those are that renderer's four branches, not bugs; this edge has six step kinds to place, places them, and declares the difference. `test_viewer_primitives.py` pins the legacy behaviour as a fact so the day a branch is added, this edge is revisited with it. Classification is a table over the eight display hues, never a substring: `viewer._tool_accent_class` is not imported and an AST test asserts the name never appears here. The same `execute` gets `acc-bash` as an ACP kind and stays neutral as an ATIF `function_name` in the same run; `read_file` from OTel stays neutral too. `think` resolves to the neutral accent because the stylesheet has none for it, and that one is declared DROPPED rather than hidden. `name_semantics` rides in the metrics line and in `data-name-semantics`; a step with no typed slot is labelled `Canonical IR representation` so the page never passes a reconstructed event off as a source record; cuts carry an explicit marker and a NORMALIZED record. `render_trace` returns both reports without merging them, and this edge never addresses the hub — it has the IR on neither side. Unwired: nothing under src/benchflow calls it. `python -m benchflow.trajectories.ir_to_view_html <path> [out.html]` is how a person looks at a page. Docs in §8.15, family list and status in §8.7.

V1-V12 and V4b, all PASS, 2026-08-19, in a browser against the four corpora. The table says what a person confirmed, and the paragraph after it says what the sign-off does not extend to: four corpora on one machine in one browser, the OTLP payload's content still a construction, oracle events and non-text content blocks still test-only. The verification found the reasoning loss fixed in the previous commit, which is the third time in this work that a green suite was not evidence a document was right. Recorded as such. Also states, in one place, what this slice depends on and what it does not: benchflow-ai#984 does not depend on benchflow-ai#1034 (design reference for the step vocabulary, no code imported, every commit targets viewer.py as it stands on upstream/main); the wiring is opt-in and the switch off is byte-identical; IR -> OTel stays deferred on the terms of section 8.12; and the ATIF document's two identical user steps are the production exporter's, documented in section 5.2, shown faithfully rather than repaired here. A BrokenPipeError seen from the stdlib server during V10 is recorded as an incidental observation about serve(), not as a finding about this edge.

….7.4 Port of the viewer-v2 prototype (interactive payload/template renderer, multi-run browse mode, semantic color system) onto the 0.7.4 viewer stack (benchflow-ai#1019 restyle, benchflow-ai#1020 accents, benchflow-ai#1021 --confirm, benchflow-ai#1022 redaction summary, benchflow-ai#1023 badge rules, JSONL session files). Merge shape: - serve() becomes a dispatcher: single trajectories (rollout dir or session JSONL file) go through _serve_single — byte-identical to upstream serve()'s body, preserving the confirm/DECISION contract, sidecar ordering, and exit semantics — while a directory of rollouts serves browse mode (run sidebar, /api/rollouts, /api/rollout?id=… with exact-membership whitelist ids, ?run= deep links). --confirm on a multi-run directory errors out: a confirmation needs exactly one trajectory. - ACP rollout directories render through viewer_template.html (full-fidelity collapsible stream, harness/model/skills identity row, error-diagnostic banners, Verifier/Metrics tabs, focus/filters/search/anchors, chunked rendering, per-kind hues); raw session files keep _render_acp_events, whose server-rendered output the jsonl-session tests pin. - Upstream renderers and helpers are byte-equivalent except unicode hardening (errors="replace" on trajectory/sidecar reads, extended to render_jsonl_file and the stream-json paths) and _load_result_json refactored onto the shared lenient loader. - Template gains confirm-bar compatibility: CSS variable aliases for the injected bar plus a theme-aware override for its hardcoded light-palette hover; kind-hue inference falls back to kind+title needles, mirroring _tool_accent_class. Verified: 5685-test suite green, ruff format/check + ty clean, confirm flow E2E on a template page (bar + redaction note injected, POST /decision, single DECISION line, sidecar stays clean), browse API + traversal guard live-tested. Two-agent parity review confirmed no upstream behavior and no v2 feature lost. Supersedes the pre-rebase commits cf3e5a4c + 325e4ec5 (kept on branch viewer-v2-prototype for reference). Co-Authored-By: Claude Code <noreply@anthropic.com>

Adopt the benchflow.ai visual system from the shared _VIEWER_CSS (benchflow-ai#1019/benchflow-ai#1020) in viewer_template.html: light monochrome base (near-white page, white cards, hairline borders, subtle shadows), Satoshi/Google Sans Code stacks, the BenchFlow wordmark header, pill labels (black PROMPT pill, mono uppercase labels), and the exact muted pastel accent palette for tool kinds (execute=amber, edit=blue, read=teal, think/skill=purple, fetch/search=cyan). The dark #141414 code treatment is now reserved for terminal (execute) output, matching the site renderer's .term rule. Pass/fail/warn semantics extend the same pastel grammar (pale bg + darker ink + soft strip) since the site palette defines no failure colors. Template tokens now use _VIEWER_CSS variable names directly, so the injected --confirm bar styles itself with zero adaptation — the alias bridge and the dark-theme hover override are gone along with the dark theme (single light theme, like every other viewer page). Co-Authored-By: Claude Code <noreply@anthropic.com>

…anchors Two refinements from design review of the aligned template: - Tool cards get a full pale frame in their kind hue (border-color: --k-line) on top of the saturated left strip — the same two-layer grammar the failed/timeout cards already used, removing the one card type that still wore a neutral frame. - The all-pastel palette read washed-out at reviewer information density, so verdict moments now carry ink-strength color while everything else stays muted: sidebar status edges 2px→4px in ok/bad ink, PASS/FAIL header badges become solid pills (white on ink — the black PROMPT pill's green/red siblings), failed/cancelled status chips go solid, and error-banner strips deepen to bad ink. Three deliberate intensity tiers: verdicts (solid, rare) → kind annotations (pastel, frequent) → monochrome base. Co-Authored-By: Claude Code <noreply@anthropic.com>

@revision

`bench eval view hf://<org>/<dataset>[@revision][/subpath]` fetches the viewer-relevant slice of a HF trajectory dataset — trajectories plus result/timing/prompts/verifier sidecars, skipping the large llm_trajectory/trainer files — into the shared huggingface_hub cache (repeat views are incremental) and serves it through the existing browse mode. This is the local rehearsal of the hosted-viewer story: the ground truth uploads (e.g. benchflow/skillsbench-trajectories-apr2026) become browsable with one command and no manual downloads. - spec parsing tolerates the hf:/ collapse from typer's Path conversion and supports @revision pinning - _discover_rollouts default depth 3→4: dataset roots nest one level deeper than local job dirs (jobs/<run>/<timestamp>/<rollout>) - lazy huggingface_hub import with a clear error when the extra is absent Validated live against benchflow/skillsbench-trajectories-apr2026 (jobs/opus47-with-skills-t1): 225 runs discovered and served, payloads render end to end. Co-Authored-By: Claude Code <noreply@anthropic.com>

Render event timestamps whenever the capture provides them, invisibly degrading when it does not — which is every capture today: a scan of 140 non-empty trajectories (5,698 events) across the HF ground-truth uploads and recent local experiment batches found zero timestamp fields. - _normalize_steps passes through the proposed capture fields (benchflow#1033): tool_call started_at/finished_at → step t + dur, ts on other events → step t; lenient parsing (ISO-8601, the space-separated str(datetime) form, numeric epochs), unparseable values ignored; steps carry no t/dur keys when the capture has none. - Template shows a "+m:ss" offset chip per timestamped step (origin = first timestamped step, so mixed clock sources cannot skew the base) and a duration chip on tool calls. The day the capture change lands, existing viewers gain the timeline with no further work — and reviewers can finally answer "where did 85 of the 90 minutes go" per event. Co-Authored-By: Claude Code <noreply@anthropic.com>

CHANGELOG entries plus the bench eval view reference section for the new ACP review page, multi-run browsing, hf:// dataset sources, and the forward-compatible timeline. Co-Authored-By: Claude Code <noreply@anthropic.com>

All five findings from the review pass: - `bench eval view --help` no longer exits 1: the help text carried "[/subpath]", which Rich parsed as a closing markup tag; rephrased without brackets. - `ruff format --check src tests tools` passes (the new viewer test file was unformatted). - The browse-mode run cap is no longer silent: the sidebar heading says "first N runs (capped)" when truncation happened (detected by scanning cap+1, not by len==cap), a `?run=` pointing at an undiscovered id shows an explicit load error instead of silently rendering the first run, and BENCHFLOW_VIEWER_MAX_RUNS overrides the 500 default. - The two security regression tests now pin enforcement (mutation-killing): the script-breakout test asserts the raw `</script><script>` sequence appears nowhere in the emitted page (removing the escape fails it), and a new whitelist test resolves ids through the extracted `_resolve_browse_rollout` helper against a real rollout placed outside the served base (removing the membership check fails it). - Diagnostic banner keys derive from `DIAGNOSTIC_REGISTRY` instead of a hand-copied list (static fallback kept for lenient imports), so new diagnostics like benchflow-ai#1025's chat-only flag appear without drift — and diagnostics on an otherwise-clean rollout (no error/verifier_error) render as neutral info banners rather than red error banners. Co-Authored-By: Claude Code <noreply@anthropic.com>

Independent re-review of the review-feedback commit surfaced four leftover gaps around the run cap plus one weak assertion; all closed: - the browse startup line no longer under-reports silently — it prints "first N runs (capped — raise BENCHFLOW_VIEWER_MAX_RUNS)" when truncated (same cap+1 detection as the sidebar), and the --confirm-on-directory error says "N+ runs" - /api/rollouts signals truncation with an X-BenchFlow-Capped: 1 response header (body stays a bare list for backward compatibility) - BENCHFLOW_VIEWER_MAX_RUNS and the truncation signals are documented in the bench eval view reference - has_error includes export_error, so diagnostics riding along the third error channel render as error banners, not neutral flags - the cap test pins the exact discovered-id prefix (the ids[:cap] slice and the whitelist membership set both rely on prefix determinism), not just counts; _discover_rollouts documents that directory symlinks under the served base are followed Co-Authored-By: Claude Code <noreply@anthropic.com>

…ange) First step of the in-PR construction plan from review: split the 1,500-line viewer.py + 900-line inline template into a package with one responsibility per module, exactly as suggested: trajectories/viewer/ payload.py ACP normalization, diagnostics, sidecar loading render.py payload + packaged assets -> one self-contained page legacy.py stream-json / Codex / raw-ACP inline renderers (verbatim) catalog.py run discovery, cap, summaries sources.py hf:// spec parsing and resolution server.py single-run serving (confirm contract) + browse mode assets/ theme.css / viewer.css / viewer.js / template.html __main__.py python -m entrypoint The split is mechanical (functions moved verbatim by line range); the only rewritten body is _load_template(), which now assembles the page from the split assets. benchflow.trajectories.viewer keeps its full import surface via __init__ re-exports — all 394 viewer-battery tests pass unmodified, and the wheel ships the assets subtree. Next steps per the plan: typed payload contract (models.py), the real SkillsBench theme, then the catalog information architecture. Co-Authored-By: Claude Code <noreply@anthropic.com>

Second construction step: the Python → JavaScript boundary is now an explicit typed contract (viewer/models.py) instead of dict[str, Any]: - Step / ToolCall / TimeoutInfo / ErrorBanner / Meta / VerifierArtifacts / ViewerPayload / RunSummary dataclasses; to_payload() is the single serialization site and preserves the exact wire shape, including which optional keys are omitted (not nulled) when absent - tool-kind display classification moves fully server-side: models.tool_hue is the one classification site, the payload ships each tool's hue, and the template's duplicated KINDS/INFER tables are gone (the JS keeps only a class-attribute whitelist against crafted payloads) - no more silent truncation (review point 6): unknown-event payloads and diagnostic banners ship complete; the template collapses long content (banners now use the same expandable body as everything else) Co-Authored-By: Claude Code <noreply@anthropic.com>

@font-face

…theme Third construction step — the theme now IS the SkillsBench system rather than an approximation of it: - assets/theme.css carries the canonical tokens from www.skillsbench.ai verbatim: the hex fallback :root plus the lab() override :root (the site's own progressive enhancement), the semantic palette (success/warning/info/destructive + foregrounds), the five chart colors, --radius: .625rem, and the site's exact font stacks - Google Sans Code (OFL 1.1, license file alongside) is inlined as @font-face data URIs (400/600 latin, ~24 KB) so exported pages stay self-contained and offline; Satoshi is declared in the stack but not redistributed — Fontshare's EULA does not permit shipping its files in this repo, so it falls back to system sans unless the host has it (flagged for maintainers in the PR) - one theme, no drifting copies: the inline legacy renderers' _VIEWER_CSS now prepends the same theme.css instead of defining its own :root, and both renderers color tool kinds from the same --kind-* component tokens; the verdict palette derives from the canonical semantic colors via color-mix, and metrics phase bars use the canonical chart palette - the interactive template migrates to canonical vocabulary (--foreground/--muted-foreground/...), with the small set of viewer-specific aliases documented in the theme Co-Authored-By: Claude Code <noreply@anthropic.com>

Fourth construction step (review point 4): - The hf:// download allowlist is now exact paths only. verifier/* pulled 418 unconsumed files (~31 MB of OBJ/MP4/PDF/NPZ run artifacts) on the example dataset; the verifier portion now derives from payload.VERIFIER_SIDECARS — the same four files _load_verifier reads — so the fetch surface cannot drift wider than what the viewer consumes, and a regression test rejects any widening (no wildcard entries, and the verifier set must equal the sidecar list). - Sources are typed: parse_source() turns the CLI argument into LocalPathSource | HfDatasetSource, validating repo components and dataset subpaths (no "..", no backslashes, no empty revision). The CLI argument is a str end to end — never pathlib.Path, whose normalization mangles hf:// into hf:/ — and the mangled spelling now gets a pointed error instead of silent acceptance. - Verified against a fresh HF cache: resolving a dataset rollout fetches only allowlisted files, zero junk. Co-Authored-By: Claude Code <noreply@anthropic.com>

…hitecture Fifth construction step (review point 3, scoped with the maintainer: the PostTrainBench index interaction model, minus the overview matrix for now). The browse-mode sidebar becomes a full-width run catalog: - corpus line (total + pass/fail/unscored, cap note when truncated) - grouping by task or by model + harness (or none), with per-group aggregates — run count, pass/fail/unscored, pass rate — and collapsible group cards; small group sets open by default, open-state lives in the URL - sorting (name/reward/duration/cost), text filtering, and incremental "show more" pagination inside large groups (100 rows first, +200 per click) - run rows carry the row-level stats reviewers scan for: verdict, duration, total tokens, cost (RunSummary gains duration_sec/cost_usd/ total_tokens/n_tool_calls) - selecting a run opens the detail page (scrolled to top, "← runs" back control); back — button or browser — restores the exact catalog view: group mode, sort, filter text, open groups, and scroll position all round-trip through the URL - ?run= deep links still work and unknown ids still get the explicit error instead of a silent fallback The overview/performance matrix and corpus statistics table from the reference design are intentionally deferred. Co-Authored-By: Claude Code <noreply@anthropic.com>

Final construction step (review point 5): the ~850 lines of template JavaScript now have behavior tests that run them in real headless Chromium via playwright (dev extra; the suite self-skips when playwright or its chromium binary is absent, so plain `pytest tests/` runs anywhere, and carries a `browser` marker for explicit deselection). Coverage, per the review's list: catalog grouping / filtering / sorting with URL-state restoration across reloads; group collapsing (regression: with ≤2 groups the default-open state used to short-circuit the toggle — caught by a user click, now pinned); run selection and back navigation preserving the exact catalog view, including browser back re-entering the detail; Trace/Verifier/Metrics tabs; Focus/Full switching; in-trace search and #eN event anchors plus timeline chips; the unknown-run error state; hostile trajectory content (<img onerror>, </script><script>) staying literal text — no elements created, no dialogs fired; and representative desktop + narrow-viewport screenshots. The playwright lifecycle is module-scoped on purpose: its sync API keeps an asyncio loop running on the calling thread, and a session-scoped instance leaks that loop into other modules' pytest-asyncio tests. Co-Authored-By: Claude Code <noreply@anthropic.com>

Findings from an adversarial review of the catalog + browser-test commits, each pinned by a browser-level regression test: - selectRun failure now clears the previous run's header/tabs/panes, resets pane visibility, and syncs ?run= into the URL before the fetch — a failed load used to silently show the prior run's data with the error hidden inside a collapsed trace pane. - Group keys are component-encoded in the toggled URL param: task names containing commas no longer split into phantom keys on reload. - defaultOpen derives from the whole corpus, not the filtered subset, so narrowing the filter to ≤2 groups no longer inverts explicit toggles; active filters force matches visible instead. - ?group=/?sort= are validated with own-property checks — prototype keys like "constructor" fall back to defaults instead of crashing. - Filter keystrokes re-render only the stats and run list; the input is never rebuilt, preserving caret position and IME composition. - popstate skips refetching when only the hash changed (in-trace #eN anchor back-navigation no longer re-renders the whole run). - fmtDuration rounds once before splitting — no more "1m 60s". - Both viewer HTTP handlers override do_HEAD: the inherited handler served filesystem paths from the process cwd, bypassing the rollout-id whitelist. - CI installs Playwright Chromium so the browser suite actually runs instead of silently self-skipping in every lane. Test corpus grows a third task group with a comma in its name and a 119.6s duration, covering the default-collapsed branch, the flat-list mode, and pagination past PAGE_SIZE via a 105-run corpus. Co-Authored-By: Claude Code <noreply@anthropic.com>

bingran-you · 2026-08-27T20:09:34Z

Maintainer merge pass complete on b88d6f4: rebased onto current main; hardened the payload, source, server, confirmation, and browser boundaries; split the frontend state machine; and added mutation, security, accessibility, and real-Chromium regressions. Local focused suite: 126 passed; browser suite: 30 passed. Native GitHub test, pip-audit, integration-scope, and integration-light all pass. manifest-parity is the pre-existing external benchflow-ai/agents drift (codex-acp 0.0.45 there versus 1.6.0 on main) and has also failed the last three main runs. Version remains 0.7.6.dev0; no release requested.

ljr145733 force-pushed the viewer-v2-rebased branch from 943aca2 to a490d48 Compare August 18, 2026 02:23

yangziao56 mentioned this pull request Aug 19, 2026

Preserve structured failure diagnostics in canonical results.jsonl #1037

Open

Galius5136 mentioned this pull request Aug 19, 2026

feat(trajectories): canonical trace IR with ACP and ATIF converters #984

Open

13 tasks

ljr145733 and others added 16 commits August 27, 2026 11:55

fix(viewer): harden review UI for merge

a101265

bingran-you force-pushed the viewer-v2-rebased branch from 7969512 to a101265 Compare August 27, 2026 19:56

fix(viewer): restore catalog focus synchronously

b88d6f4

bingran-you merged commit ff10b7d into benchflow-ai:main Aug 27, 2026
10 of 11 checks passed

Navigation Menu

Uh oh!

feat(viewer): reviewer-grade trajectory pages, multi-run browsing, and hf:// dataset sources - #1034

feat(viewer): reviewer-grade trajectory pages, multi-run browsing, and hf:// dataset sources#1034
bingran-you merged 17 commits into
benchflow-ai:mainfrom
ljr145733:viewer-v2-rebased

ljr145733 commented Aug 17, 2026 •

edited

Loading

Uh oh!

Galius5136 commented Aug 17, 2026

Uh oh!

ljr145733 commented Aug 17, 2026

Uh oh!

bingran-you commented Aug 17, 2026

Uh oh!

ljr145733 commented Aug 18, 2026

Uh oh!

bingran-you commented Aug 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ljr145733 commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Usage

What the page gives reviewers

Compatibility

Security

Testing

Screenshots

Uh oh!

Galius5136 commented Aug 17, 2026

Uh oh!

ljr145733 commented Aug 17, 2026

Uh oh!

bingran-you commented Aug 17, 2026

Review — changes requested

1. Decompose the viewer architecture

2. Match SkillsBench’s actual visual system

3. Adopt PostTrainBench’s information architecture, not its visual styling

4. Tighten the hf:// download allowlist

5. Add browser-level regression coverage

6. Preserve the promised “full event stream”

Suggested PR split

Verification performed

Uh oh!

ljr145733 commented Aug 18, 2026

Uh oh!

bingran-you commented Aug 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ljr145733 commented Aug 17, 2026 •

edited

Loading

4. Tighten the `hf://` download allowlist