[david6666666]

Refs #333 and #345.

Open a profiled campaign to see the performance trend first, then the acceptance test, the request profiles, the round measurements and the adoption decisions. The review works live and as an offline HTML export. There is no change to src/inferrsi/harness/runner.py relative to main.

What the profile shows. A fresh 4096×4096 single-card run started by capturing a complete warm worker request with torch.profiler/Kineto. Of 5.35 s in the worker, six denoising steps occupied 4.49 s. The captured launch correlations attributed about 2.32 s to the two largest projection GEMM families and 1.61 s to dense attention. HTTP latency was timed separately without the profiler. The dashboard initially shows representative stage, CPU and GPU tracks; zoom, search or load the complete event table to inspect individual events. The original trace stays local. A repeated capture with the same comparison key shows whether the bottleneck changed.

How the evidence influenced the trials. The first candidate combined paired Q/K kernel launches. The repeated capture verified that launches fell from 600 to 300, but GPU time only fell from 148.9 to 146.3 ms. Paired HTTP latency was 5.432 → 5.404 s (0.52%, within noise); all six RGB pairs matched. The bottleneck remained in the projections and attention. The second candidate split each projection's 16,416 rows into 16,384 aligned rows and a 32-row tail, retaining the full K dimension. It measured 5.411 → 5.408 s (0.06%) and changed pixels in all six output pairs. Neither candidate was adopted. The 50% target was not reached.

Evidence and discovery. Each round links its actual retrieval, used skills, choices and outcome. Profiles are explicitly registered and hash-checked; only a bounded projection of timing data is served, without stacks or raw event arguments. The trend compares paired measurements and distinguishes incompatible runs. Campaign search now considers portable discovery skills and engine records under the specified hardware and workload; captures distinguish the total matched, the number returned and the resources actually loaded.

Use campaign init --profile-first --backend cuda, campaign attach-profile and dashboard --live; the recording guide includes the review file and command. Existing campaigns and historical event records remain readable. The screenshot is hosted on the PR's existing preview branch; the PR adds no image files to docs/.

Python import migration (breaking). Change inferrsi.harness.<module> to inferrsi.observability.<module> for campaign_records, campaign_trace, dashboard, dashboard_data, dashboard_external, dashboard_live, dashboard_timeline, knowledge_capture, optimization_log, optimization_story, and optimization_trace. For example, import CampaignTrace from inferrsi.observability.campaign_trace. The CLI entry points remain available.

Native run evidence. The runner keeps main’s start-only phase log. The dashboard takes terminal states and command durations from the snapshot and execution artifacts, and identifies the open native step from the running status and latest start. A phase finish timestamp requires a recorded end event. Historical paired events still provide phase durations. Invalid validator progress leaves the command result visible.

Search defaults. campaign capture-context --relevance uses task-lexical-v1 to discover both portable skills and applicable repository records. Standalone collect --relevance retains its existing skill-only default. Pass --relevance-method task-lexical-v1 to use the campaign ranking with standalone collection.

Verification. 233 dashboard, trace, campaign and packaging tests passed locally, and Ruff passed. Verified rendering in desktop and phone viewports, the live connection and loading all 530,926 complete duration events from the baseline capture. The two experimental trials ran with main's runner on one B300.

{
  "schema_version": 1,
  "head_sha": "b9fe71c23fca52f7625e99ba87161f6076989a33",
  "human_only": false,
  "material_ai_authors": [
    {
      "tool": "Codex (retrieval implementation)",
      "model_id": "gpt-6-astra",
      "effort": "xhigh",
      "role": "author"
    },
    {
      "tool": "Codex (retrieval scope and integration)",
      "model_id": "gpt-6-astra",
      "effort": "xhigh",
      "role": "author"
    }
  ]
}

[chatgpt-codex-connector]

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-11T02:59:37.885427Z	e66fee4	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[amy-why-3459]

Review at head e66fee470cfa1cd55af22e40e406662b8c370daa (base main @ 4b3afa55f278ca314e07d5f8acbd0a18f3fa72bf). This is a first full review of all 7 files. Not approving yet: the questions below need an answer first.

What I checked

• runner.py: the new node() context manager (L52-67) writes a running start and an <phase>_end event with state and monotonic elapsed_seconds. It wraps the source snapshot (L181-183), each measurement trial (L187-194) and the validation command (L204-208). For a trial that comes back cancelled, progress["status"] is set before the block exits, so the _end event records cancelled before the outer raise KeyboardInterrupt.
• dashboard_workflow.run_workflow() matches start/end edges per (kind, side, trial). Exact duplicates count once, and conflicting edges give unknown. Group totals come from repetitions * 2 and from whether a validation_command is set. The sort key is safe when several nodes have no start time.
• dashboard_data.py: the event activity mapping, the item_kind / elapsed_seconds details and caching the trial refs. Tool-call detection now uses activity == "tool". The template builds the new run-review rows with node() / textContent, and I didn't see any new innerHTML sinks.
• CI at this head is green: full-check (3.10/3.11), installed-package (3.10/3.11), lint-and-smoke, pr-references and declarations.

Questions / requested changes

1. Nothing tests the runner side of the new contract. tests/test_dashboard_workflow.py replays hand-written events.jsonl records only. No test runs run_experiment() and checks that it actually emits start/_end pairs with state and elapsed_seconds. That leaves three cases unpinned: a cancelled trial, an exception inside execute_checked (failed via L62-64), and a validation that ends failed/timeout. The dashboard depends entirely on these records. Could you add one runner-level test with a fake execute / snapshot that feeds its run directory into load_dashboard()? That would protect the producer–consumer pairing against later renames such as phase + "_end".
2. A malformed progress.json hides a known validation result. dashboard_data.py L734-735 sets the quality group to state="unknown", total=None, completed=0 when validation_progress() rejects the snapshot. That overwrites a state the command node already established. For example, a validator that exits failed but leaves a half-written or out-of-range progress file shows up as unknown instead of failed. Is that intended? If not, it might be better to drop only the counters (keep the command-level count and state) and expose the invalid snapshot through refs["validation_progress"]. A test case next to the 3/2 check in test_validator_counts_and_legacy_trace would pin this down.
3. The Qwen validator's progress path. examples/qwen21_turbo_quality.py L142-146 writes Path(os.environ['INFERRSI_RESULT_PATH']).with_name('progress.json'), but the dashboard reads <run>/validation/progress.json. These only match if the runner's result path for the validation command is always directly inside run_dir / "validation". Please confirm that this is part of the execute_command contract, or derive both paths from one shared helper or name. There is also no test that this example writes the snapshot validation_progress() accepts (unit: manifests, completed <= total).

Minor, non-blocking: in dashboard_data.py L751-752, evaluation.get("metric_key", lock.get("metric_key")) falls back only when the key is absent, not when it is present with null. Use or if that matters.

Static review of the diff plus runner.py and scenarios/external.py at this head. I didn't run the tests or the dashboard.

Reviewed commit range: 4b3afa55f278ca314e07d5f8acbd0a18f3fa72bf … e66fee470cfa1cd55af22e40e406662b8c370daa.

[cursor]

PR review verdict — DO NOT PASS

Reviewed base 4b3afa55f278ca314e07d5f8acbd0a18f3fa72bf and head 3b25c1057a7c1cdca8e92239e4b8a00e745fd6e3 (full diff; no earlier review in seed).

Blockers

1. Generated campaign snapshot is in the tree. _state.json (new, repo root) is a one-line load_dashboard payload for campaign h3-image-b300 (synthetic: false), including run_workflow, optimization traces, and the chart / temporal SVG. Nothing in the diff loads it, and it is outside evidence/<campaign-id>/. Drop it from the revision. A fixture belongs under tests and has to be a minimal synthetic file that a test actually reads.

2. Run review never shows the recorded attempt result. src/inferrsi/observability/template.html attemptResult (around line 648) returns null unless optimization_trace.steps.state === 'present'. The trace section state this loader emits is complete / partial. The snapshot in _state.json shows that shape: batch-eight and dit-fp8 both have steps.state complete and a non-empty attempt result (the FP8 result includes SSIM 0.8764 and mean RGB error 12.18). Those sentences are skipped, and the Outcome line falls through to outcomeSentence. Accept a recorded attempt result when the section state is complete (and when a partial section still carries items). present is the story-object token, not the trace-section token.

3. The fallback Outcome sentence misstates the measurement. Same template, outcomeSentence (around lines 661–663):

   • Every signed improvement_pct is labeled "Latency fell" / "Latency rose". The row already has metric_key, metric_unit, and direction (minimize / maximize). A non-latency metric, or a maximize metric, renders a false sentence. Phrase the change from those fields.
   • The short-of-target branch always inserts an ideographic comma (，) before "short of the … target", including when the UI language is English. The later clauses join with , . Use the active language's separator in both places.
     This is the sentence reviewers see for every current row, because of blocker 2, and it remains the legacy-row sentence after that check is fixed.

4. Validator progress can replace the quality verdict. docs/dashboard.md (around line 77) says progress never sets the run verdict. examples/qwen21_turbo_quality.py (around lines 140–144) writes progress.json inside the same try that builds reports. The except Exception (around line 146) then discards reports and stores verdict: FAIL. Trigger: args.inferrsi_run is set and INFERRSI_RESULT_PATH is missing (KeyError), or lifecycle.atomic_write_json raises. A passing manifest list, or a real quality failure, becomes an I/O error verdict. Compute the verdict from reports first. Write progress in a nested try that leaves result unchanged when the write fails.

Checked

• Runner node() emits a start event (state=running) and a matching _end event with elapsed_seconds and the command status returned by execute_checked (src/inferrsi/harness/runner.py).
• run_workflow pairs start/_end by (kind, side, trial), keeps one identical edge, and marks conflicting edges unknown. tests/test_dashboard_workflow.py covers in-flight replay, cancel/skip, invalid progress counts, and redaction of command/tool payloads.
• In-repo imports, pyproject.toml package data, the dsh template path, and docs/module-layout.md follow the move from inferrsi.harness.dashboard* / inferrsi.data/dashboard/template.html to inferrsi.observability.

Follow-up

validation_progress calls .get on the progress.json snapshot whenever _object_snapshot reports a state other than missing (src/inferrsi/observability/dashboard_data.py, around the new validation_progress call). If a non-missing corrupt snapshot is not a dict, load_dashboard raises and the live poll fails. Ignore a non-dict the same way other corrupt artifacts are skipped.

Not verified

Tests were not executed. CI was not inspected. The harness → observability import graph was reviewed from the diff only.

I will re-read after the author pushes.

[cursor]

PR review verdict — DO NOT PASS

Reviewed base 4b3afa55f278ca314e07d5f8acbd0a18f3fa72bf and head 65dcef749816a996e48bfed5d1e510f2628470db (full diff). pr/prior-reviews.md had no earlier findings and no previous head was seeded.

This review does not approve the change. I will re-read after the author pushes.

Blockers

1. Live "Current" stage sticks on the first start event. src/inferrsi/harness/runner.py node() (about line 52) appends state=running and never rewrites that line; the finish is a second _end event (about line 64). src/inferrsi/observability/template.html renderWorkflow (about line 825) then does events.find(e => e.details?.state === 'running'). On a running run whose log is source_snapshot (running), source_snapshot_end (completed), then quality_validation (running), find returns the source snapshot, so Current stays on that stage for the rest of the run. stageFacts (about line 799) sets open from the same sticky flag and nothing reads it; using open would mark every stage that has already started. Current should follow the still-open workflow node (a start with no later _end), which run_workflow already computes.

2. A crashed trial is shown as an incomplete record. src/inferrsi/observability/dashboard_workflow.py about line 76 treats only failed, error, timeout, and cancelled as terminal. crashed is an allowed event state and is already a run state on the skipped path just below, but a node whose end state is crashed falls through to unknown. Trigger: measurement_end with state=crashed (including execute_checked returning that status through node()). The progress card renders "Record incomplete" and completed does not advance, while the node row still says crashed. Include crashed with the other terminal states. The stage strip has the same hole: stageFacts paints stage-failed only for failed, error, and timeout.

3. Stage span is drawn when some timestamps are missing. template.html stageFacts (about lines 787–796) says the span is withheld when any timestamp is missing, but it only requires one parseable time. Untimed events are dropped, so the shown span can be shorter than the stage. Trigger: two events in one stage, one with time null or unparseable. Omit the span in that case, matching run_workflow wall time, which already requires every start and end.

4. Non-object validation progress aborts the candidate load. src/inferrsi/observability/dashboard_data.py about lines 730–732 calls validation_progress for every snapshot state other than missing. That function calls record.get immediately. Trigger: validation/progress.json is a non-dict (for example []), or the snapshot value is None for a corrupt or unreadable file. AttributeError fails load_dashboard for that run unless _object_snapshot already guarantees a dict for every non-missing state; this diff does not show that guarantee. Call validation_progress only for a dict. Any other present-but-unusable file should take the existing unknown-count path (state=unknown, no exception), the same path illegal counters already use.

Checked in the diff (not executed)

• Observability move: cli.py, campaign_records.py, package-data, the template path, and the tests included here import inferrsi.observability.
• tests/test_dashboard_workflow.py matches the projection for an identical duplicate edge, a conflicting end → unknown, a legacy measurement without _end → unknown performance, cancel → skipped accuracy, and validator counters (cases, null total, completed > total → unknown, private fields not exported).
• node() writes a start plus _end, and measurement / quality validation copy record["status"] onto the end event.
• The Qwen validator writes {"unit":"manifests","completed","total"} next to INFERRSI_RESULT_PATH when args.inferrsi_run is set.

Not verified here

• Tests were not run. CI was not inspected.
• The review workspace only had the seeded diff, so leftover inferrsi.harness.dashboard* imports outside this diff were not searched.
• examples/qwen21_turbo_quality.py calls lifecycle.atomic_write_json, which this diff does not define.

Follow-ups (non-blocking)

• outcomeSentence in template.html (about line 663) always inserts the Chinese comma ， in the shortfall clause, including when the UI language is English.
• No test covers node() pairing, or a non-completed command status becoming the _end state instead of the default completed.

[cursor]

PR review verdict — DO NOT PASS

Reviewed base 4b3afa55f278ca314e07d5f8acbd0a18f3fa72bf and head bd996535f83c4f6d2f990f07109ce5b8dc91d880.
pr/prior-reviews.md is empty and previousHead is unset, so this pass covers the full synchronize diff.

Blockers

1. Outcome sentence states latency and inserts an ideographic comma in every language. src/inferrsi/observability/template.html outcomeSentence (about line 657). The gain is always Latency fell / Latency rose, while the same block prints row.metric_unit and the row has direction. The below-target branch uses \uFF0C with no language check, so English becomes Latency fell 5.0%\uFF0Cshort of the 10.0% target.
   Trigger: improvement_pct below minimum_improvement_pct (comma); direction === "maximize" or a non-latency metric_unit (wording).
   Expected: join with language === 'zh' ? '\uFF0C' : ', ', and phrase the gain from direction plus metric_unit / metric_key.

2. The stage strip labels the filtered stage as in-flight. template.html renderWorkflow (about line 825). here is stageSelect.value || <first running event's stage>, and both the current class and the "Current" label follow here.
   Trigger: selected candidate state === "running" with a running event in one stage, and the stage filter set to another stage. The filtered stage shows "Current"; the stage that still has details.state === "running" does not.
   Expected: keep selected for the filter, and apply current / "Current" only to the running stage. events.find also takes the first running event in list order, so an older unclosed start outranks a later one.

3. A non-object validation/progress.json fails the snapshot. src/inferrsi/observability/dashboard_data.py (about line 731) calls validation_progress(progress) for every progress_state != "missing". validation_progress in dashboard_workflow.py (about line 115) calls record.get immediately.
   Trigger: the file is present and the snapshot value is not a dict (null, [], or another non-object). load_dashboard raises AttributeError and the run cannot be reviewed.
   Expected: treat a non-dict like the existing invalid-count path (quality group state unknown, candidate still returned).
   tests/test_dashboard_workflow.py covers well-formed objects, completed > total, and private-key redaction. It does not cover a non-object file.

4. Native-only fields are read on the external-protocol path. template.html renderCandidate reads row.optimization and info.code_locations (about line 879) before if (row.external && row.external.protocol) (about line 886). That block previously ran only after the external return. renderRunReview (about line 724) reads row.run_workflow.groups for every row with run_id. run_workflow is added on the native candidate return; dashboard_external.py is a pure rename in this diff.
   Trigger: render a selected external protocol row without optimization.code_locations, or a run_id row without run_workflow. render() throws, so renderDetail and localizePage do not run. Default selection still prefers external.metrics.
   Expected: guard missing optimization and run_workflow, and keep the external return ahead of native-only fields unless an external fixture shows those fields are always present. A node test that renders an external protocol row and a native run would lock this.

I will re-read after the author pushes.

Verified

• runner.py node() writes a start event and a phase_end event with elapsed_seconds, maps KeyboardInterrupt to cancelled, and copies the execute_checked status onto the end event when the command returns.
• run_workflow pairs start/end inside one run, collapses identical duplicate edges, marks conflicting edges unknown, withholds wall_seconds when a timestamp is missing or an end is before its start, and leaves the run verdict untouched (test_dashboard_workflow.py expects not_recorded).
• Progress counts accept only int completed / total and units cases, samples, or manifests. Extra keys such as private stay off the candidate row.
• Codex / Anthropic activity labels keep command text, tool output, and error messages out of the exported event.
• Package data, the files("inferrsi") template path, CLI imports, and the CampaignTrace doc path move together to inferrsi.observability.

Follow-ups

• logFacts always sets a knowledge note, including "No capture recorded", and renderWorkflow appends that log line whenever the stage has no events. A not-applicable knowledge stage picks up the line. Attach it when the stage is in use.
• "Review the failed check" is created only when both medians are non-null. A quality failure without medians still has the table "Open event" control.
• examples/qwen21_turbo_quality.py writes progress.json inside the validator try. A missing INFERRSI_RESULT_PATH or a Path.with_name failure becomes verdict FAIL through the existing except. Keep that I/O off the quality verdict.
• This diff adds no node/browser test for the run-review section, the stage strip, or trace labels.

Not verified here

Pytest, node UI tests, and CI were not run. The three new PNGs were not inspected. INFERRSI_RESULT_PATH's directory was not traced to {run}/validation/. Modules outside this diff were not searched for remaining inferrsi.harness.dashboard* imports.

[amy-why-3459]

Incremental review: e66fee470cfa1cd55af22e40e406662b8c370daa … bd996535f83c4f6d2f990f07109ce5b8dc91d880. That covers 2 new commits, 65dcef749816a996e48bfed5d1e510f2628470db (run-review rewrite plus the harness → observability move) and bd996535f83c4f6d2f990f07109ce5b8dc91d880 (screenshots). I read them against my earlier review at e66fee4 and the bot reviews at 65dcef7. Not approving yet: full-check (3.10/3.11) is red at this head, and I reproduced one cause locally.

Blocking

1. The DSH plugin bundle drops the moved template, so full-check fails. tools/plugin_bundle.py L52-58 only treats non-.py files as package data when relative.parts[0] == "data", and the HTML rule still checks ("data", "dashboard"). src/inferrsi/observability/template.html is now skipped. tests/test_dsh_plugin.py::test_manual_bundle_runs_from_an_external_workspace fails at L69-71 with KeyError: "There is no item named 'inferrsi/observability/template.html' in the archive" (run locally at this head). An installed plugin runtime would also lack the template, so render_dashboard could not find it. Please change the bundle rule to match pyproject.toml's new observability/*.html entry.
2. The recorded attempt result is never used. In template.html L646-651, attemptResult requires steps.state === 'present'. But optimization_trace._section only accepts complete, partial or redacted (optimization_trace.py L18, L53-62), and present is the reference state at L234. As a result, attemptResult always returns null and the Outcome line always falls back to outcomeSentence. Please accept complete, and partial when it has items. A Node test with a row that carries an attempt step would pin this down.
3. outcomeSentence assumes latency and minimization, and hard-codes a Chinese comma. L661 labels every signed improvement_pct as "Latency fell/rose". For a throughput (maximize) metric or a non-latency metric_key, that sentence is wrong. Please phrase it from metric_key / direction, or use a neutral "improved/regressed by X%". L663 also always inserts ， before "short of the … target", even in English, while L672 joins the other clauses with , .
4. The Current stage sticks on the first start event. At L826, events.find(e => e.details?.state === 'running') returns the earliest start record. node() never rewrites start lines and writes the finish as a separate _end event, so a running round stays on source_snapshot after it has finished. Please derive Current from the open node in row.run_workflow (a start with no _end), which the loader already computes.

Smaller issues in the new stage strip

• The comment at L784-787 says spans "are withheld when any timestamp is missing". But stageFacts (L788-803) drops events it can't parse and still draws a span when any one time parses. Please either withhold the span or fix the comment.
• stageFacts L799 marks stage-failed only for failed/error/timeout, while crashed is a valid event/run state. Also, fact.open (L800) is set but never read.
• docs/dashboard-run-review.png is about 534 KB and is described as "for the PR description". Do the three PNGs need to stay in the repo, or could they just be attached to the PR body? If they stay, is anything in docs/ linking to them?
• Removing inferrsi.harness.{dashboard,dashboard_data,campaign_trace,optimization_trace,...} without a shim is a public import break. docs/dashboard.md presented CampaignTrace as the agent-integration hook. If that's intended, please mention it in the PR body as a breaking change.

Still open from my previous review at e66fee4 (this range doesn't touch them): (1) there is still no runner-level test of the start/_end producer contract; (2) a malformed progress.json still overrides a known validation state with unknown in dashboard_data.py; (3) examples/qwen21_turbo_quality.py's progress path vs <run>/validation/progress.json. I also share the concern another reviewer raised about (3): the progress write sits inside the same try that builds the verdict.

Method: static review of the incremental diff plus the head files, and I ran tests/test_dsh_plugin.py locally at bd99653. I did not run the dashboard UI.

[cursor]

PR review verdict — DO NOT PASS

Reviewed base 4b3afa55f278ca314e07d5f8acbd0a18f3fa72bf and head ee11d3ba270686d805e65087dc6391d0ac2b2461 (incremental bd996535f83c4f6d2f990f07109ce5b8dc91d880..ee11d3ba270686d805e65087dc6391d0ac2b2461).

This review does not approve the change. I will re-read after the author pushes.

Still open (from the review of bd996535)

The incremental diff does not touch src/inferrsi/observability/template.html, so both blockers remain.

1. Outcome sentence states latency and inserts an ideographic comma in every language. src/inferrsi/observability/template.html outcomeSentence (about line 657). The gain is always Latency fell / Latency rose, while the same block prints row.metric_unit and the row has direction. The below-target branch uses \uFF0C with no language check, so English becomes Latency fell 5.0%\uFF0Cshort of the 10.0% target.
   Trigger: improvement_pct below minimum_improvement_pct (comma); direction === "maximize" or a non-latency metric_unit (wording).
   Expected: join with language === 'zh' ? '\uFF0C' : ', ', and phrase the gain from direction plus metric_unit / metric_key.

2. The stage strip labels the filtered stage as in-flight. template.html renderWorkflow (about line 825). here is stageSelect.value || <first running event's stage>, and both the current class and the "Current" label follow that value. A stage filter is treated as the live stage.
   Trigger: stageSelect.value set while another workflow node is still open (start event with no later _end).
   Expected: keep the filter as a view constraint. Mark Current from the still-open workflow node, which run_workflow already computes — not from the first state === 'running' event and not from the selected stage.

This push

bd996535..ee11d3ba only relocates campaign_records.py, knowledge_capture.py, optimization_log.py, and optimization_story.py from src/inferrsi/harness/ to src/inferrsi/observability/. Checked the seeded hunks:

• campaign_records.py relative imports now use .dashboard_data and .optimization_trace (was ..observability.*).
• dashboard_data.py lazy-imports .optimization_story (was ..harness.optimization_story).
• cli.py and tests/test_campaign_records.py, tests/test_optimization_story.py, tests/test_optimization_trace.py import the new modules.
• The other three moves are content-identical (similarity index 100%), so relative imports among those four modules stay valid.
• docs/architecture.md lists the observability modules.

No new blocker in those hunks.

Not verified

Tests were not run. No full-tree scan beyond this incremental diff for leftover inferrsi.harness.{campaign_records,knowledge_capture,optimization_log,optimization_story} imports. CI was not checked.

[amy-why-3459]

Incremental review: bd996535f83c4f6d2f990f07109ce5b8dc91d880 … ee11d3ba270686d805e65087dc6391d0ac2b2461. This covers 1 new commit, ee11d3ba270686d805e65087dc6391d0ac2b2461 ("Finish the observability move").

This commit: it looks clean. The four moves (campaign_records, knowledge_capture, optimization_log, optimization_story) are pure renames apart from relative-import fixes. At this head, no reference to inferrsi.harness.{campaign_records,knowledge_capture,optimization_log,optimization_story} is left anywhere in src/, tests/, docs/ or tools/. cli.py L301/L309/L325 and dashboard_data.py L973 point to the new locations. The docs/architecture.md directory map now matches the tree. I have no new findings on this range. One note: like the earlier move, this removes four more inferrsi.harness.* import paths without a shim, so the breaking-change note I asked for in the PR body should list these too.

Still not approving. Nothing from my review at bd99653 is addressed at this head:

1. tools/plugin_bundle.py L52-58 still only packages .html under ("data", "dashboard"), so inferrsi/observability/template.html is still left out of the DSH bundle. full-check (3.10/3.11) was still running when I looked. The cause I reproduced (tests/test_dsh_plugin.py::test_manual_bundle_runs_from_an_external_workspace) is unchanged.
2. template.html L648: attemptResult still requires steps.state === 'present', so it never fires for complete/partial traces.
3. L661: outcomeSentence still always says "Latency fell/rose", and the hard-coded ， is still there.
4. The Current stage still comes from the first running event, not from the open run_workflow node.

• The smaller stage-strip items (span comment vs. behaviour, crashed, the unused fact.open, the PNG size) and the three items still open from e66fee4 also stand.

Method: static review of the incremental diff, plus a repo-wide grep for the old module paths at ee11d3b. I did not run tests this round.

Reviewed: bd996535f83c4f6d2f990f07109ce5b8dc91d880 → ee11d3ba270686d805e65087dc6391d0ac2b2461.

[cursor]

PR review verdict — DO NOT PASS

Reviewed base 4b3afa55f278ca314e07d5f8acbd0a18f3fa72bf and head 6d0207f5380c006ab4a65f0aee64d13dfbeb32d2 (incremental ee11d3ba270686d805e65087dc6391d0ac2b2461..6d0207f5380c006ab4a65f0aee64d13dfbeb32d2).

This review does not approve the change. I will re-read after the author pushes.

Still open (from the review of ee11d3ba)

This range edits template.html (round inputs, event dialog, trace panel removed) but does not change outcomeSentence or how here is chosen.

1. Outcome sentence states latency and inserts an ideographic comma in every language. src/inferrsi/observability/template.html outcomeSentence (about line 660; not in this diff). The gain is still Latency fell / Latency rose while the same block prints row.metric_unit and the row has direction. The below-target branch still uses \uFF0C with no language check, so English becomes Latency fell 5.0%\uFF0Cshort of the 10.0% target.
   Trigger: improvement_pct below minimum_improvement_pct (comma); direction === "maximize" or a non-latency metric_unit (wording).
   Expected: join with language === 'zh' ? '\uFF0C' : ', ', and phrase the gain from direction plus metric_unit / metric_key.

2. The stage strip labels the filtered stage as in-flight. src/inferrsi/observability/template.html renderWorkflow line 929 still does if (here === stage.name) and appends Current. This diff only drops jumpNotice on that button. here is still stageSelect.value or the first event with details.state === 'running'.
   Trigger: choose a stage in the strip, or a running log whose first state=running event is an earlier stage that already has an _end.
   Expected: Current follows the still-open workflow node (a start with no later _end), which run_workflow already computes. A stage filter must not paint that stage as in-flight.

New blockers

3. selection_rules is not required to be a list. src/inferrsi/observability/optimization_trace.py line 164 calls _strings(space["selection_rules"]). _strings is [_string(item) for item in value], so a JSON string is walked one character at a time and accepted. applied_skills in the same function already rejects a non-list. tests/test_optimization_trace.py test_selection_rules_are_optional_and_remain_bound_to_the_trace sets selection_rules to "invalid" and expects TraceError.
   Trigger: a trace whose search_space.selection_rules is a string.
   Expected: isinstance(..., list) before _strings, same as applied_skills, and raise TraceError("invalid_format").

4. Default lexical ranking calls early.get when early is None. src/inferrsi/knowledge_operations.py line 646. The filter is authority != "package" or (not task_ranking and early.get("index_type") != "packaged-skill"). The next line is if early is not None and not matches(...), so early is None is a real state (task-lexical reaches it via short-circuit). For weighted-lexical-v1 with authority == "package", early.get runs and raises AttributeError before that guard. The previous check used self.records()[identity] and did not touch early. This aborts the whole collect, not one id. collect --relevance still defaults to weighted-lexical-v1.
   Trigger: collect --relevance <cue> (or capture with --relevance-method weighted-lexical-v1) when the package peek early is None.
   Expected: if early is None, do not call .get. Keep the catalog index_type check for non-task ranking; only task-lexical should admit non-skill package records.

5. Profile-first live view never polls. src/inferrsi/observability/review_template.html line 60 sets const live = data.live === true, and line 268 only calls poll() when that is true. Mode text is READ ONLY · OFFLINE otherwise. This diff never sets live on the snapshot data. The JSON payload is escaped before the template is read (dashboard.py around line 313), and the main template still switches on liveMode, which review_template.html does not read.
   Trigger: render_dashboard(..., live=True) for review_layout: profile_first.
   Expected: the embedded data has live: true (or this template reads the same flag as template.html) so /api/state polling starts.

Follow-ups (not blocking)

• Profile gap copy is latency-only. review_template.html around line 201 always says "Best measured reduction", and _round_trend labels the axis "Speedup index" whenever review_layout == "profile_first", including direction === "maximize". The minimize/maximize ratios themselves match the target math. A target >= 100 on a minimize baseline still renders a negative "Target latency" (ref * (1 - target/100)); _round_trend already drops that reference.
• task_score calls json.dumps (_skill_selection.py). The import hunk only shows posixpath and re. If lines 1–3 do not import json, every task-lexical-v1 score raises NameError.

Not verified

Tests were not executed, and CI was not checked. Binary screenshots docs/dashboard-profile-review.png and docs/dashboard-profile-trend.png were not inspected. render_dashboard references view (dashboard.py around line 318) while its parameter is snapshot; the binding, if any, sits in the lines above this hunk and was not in the diff. If view is unbound, every dashboard render raises NameError.

[cursor]

PR review verdict — DO NOT PASS

Reviewed base 4b3afa55f278ca314e07d5f8acbd0a18f3fa72bf and head b3f748616d704706e380c0c72fada92ffefed74e (incremental 6d0207f5380c006ab4a65f0aee64d13dfbeb32d2..b3f748616d704706e380c0c72fada92ffefed74e).

This review does not approve the change. I will re-read after the author pushes.

Still open (from the review of 6d0207f)

This range updates docs/dashboard-profile-*.png and tools/plugin_bundle.py. src/inferrsi/observability/template.html is unchanged, so both blockers remain.

1. Outcome sentence states latency and inserts an ideographic comma in every language. src/inferrsi/observability/template.html outcomeSentence (about line 660 at 6d0207f; unchanged in this range). The gain is still Latency fell / Latency rose while the same block prints row.metric_unit and the row has direction. The below-target branch still uses \uFF0C with no language check, so English becomes Latency fell 5.0%\uFF0Cshort of the 10.0% target.
   Trigger: improvement_pct below minimum_improvement_pct (comma); direction === "maximize" or a non-latency metric_unit (wording).
   Expected: join with language === 'zh' ? '\uFF0C' : ', ', and phrase the gain from direction plus metric_unit / metric_key.

2. The stage strip labels the filtered stage as in-flight. template.html renderWorkflow (about line 825 at bd996535; 6d0207f and this range leave how here is chosen unchanged). here is stageSelect.value || <first running event's stage>, and both the current class and the "Current" label use here. A selected filter stage is painted in-flight. With no selection, events.find of details.state === 'running' stays on the first start, because runner.py node() records finish as a separate _end event and does not clear the earlier state=running line.
   Trigger: stage filter set to any stage, or a running log whose first stage has already ended (source_snapshot running, source_snapshot_end completed, later stage running).
   Expected: Current follows the still-open workflow node (a start with no later _end). stageSelect only chooses which stage detail is shown.

Checked in this range

• tools/plugin_bundle.py bundle_runtime: data/dashboard *.html is no longer package_data. HTML whose parent parts are exactly ("observability",) is now copied. Diff only; bundler not run. If data/dashboard/*.html is still a runtime input, that package_data clause still needs to stay.
• docs/dashboard-profile-review.png, docs/dashboard-profile-rounds.png, and docs/dashboard-profile-trend.png are binary. Pixel content not inspected. Screenshots do not change the two template paths above.

Not verified here

Tests not executed. CI not checked. template.html behavior is carried from the review of 6d0207f, where outcomeSentence and here were already unchanged.

[amy-why-3459]

Incremental review of the two new commits (6d0207f profile-first review layout + task-lexical discovery, b3f7486 plugin template bundling). I ran the full test suite locally at both heads.

Resolved

• Plugin runtimes now ship observability/*.html (tools/plugin_bundle.py L60-61). At b3f7486 test_isolated_runtime_can_export_dashboard[codex|workswarm] and test_dsh_plugin.py::test_manual_bundle_runs_from_an_external_workspace pass locally (all three failed at 6d0207f).

Still failing / open

1. full-check (3.11) is red and reproduces locally: tests/test_codex_plugin.py::test_bundle_contains_all_declared_package_data[codex|workswarm]. The oracle at L76-86 expands all package-data patterns (now including observability/*.html, pyproject.toml L26) but only collects installed names starting with inferrsi/data/ (L84), so observability/template.html and observability/review_template.html show up as missing even though they are bundled. The test filter (or the expected set) needs to cover the new pattern. Also note this commit drops the data/dashboard/*.html clause; there are no such files today, so that looks fine, but please confirm it was intentional.
2. Unchanged from my previous comment: template.html L651 attemptResult still requires steps.state === 'present'; L664-666 outcomeSentence still assumes a latency metric ("Latency fell/rose") and hard-codes the full-width ，; L910-911 the Current stage still comes from the first running event rather than the open run_workflow node. The smaller stage-strip items and the three e66fee4 items (runner-level start/_end test, malformed progress.json overriding validation state in dashboard_data.py, qwen21_turbo_quality.py progress path) also look unaddressed — please reply if any were intentionally left.
3. Breaking-change note in the PR body is still missing. This push adds another user-visible change: campaign capture --relevance without --relevance-method now defaults to task-lexical-v1 (knowledge_capture.py L73-76, arguments.py L172-174) and widens the scope from packaged skills to package records, while knowledge collect keeps weighted-lexical-v1. Is that asymmetry intended? Worth documenting either way.

New code — questions
4. _skill_selection.py L101-102: skill_score(..., method="task-lexical-v1") calls task_score(..., backend=None), so the backend boost is inferred only from cue words, whereas the collect path passes the real backend (knowledge_operations.py L686). Is any caller expected to reach the first path? If not, it may be simpler to reject it there.
5. _skill_selection.py task_score indexes record["title"], ["summary"], ["repo"], ["scenario"], ["backend"] directly; a record missing one is reported as index_unavailable and silently dropped from ranking. Fine if the catalog guarantees these fields for every package record (not only packaged skills) — can you confirm?
6. dashboard_data.py L927-937: profile_best_improvement_pct is computed from rows matching the single profile anchor even when comparison_state is incompatible/unverified (where best_improvement_pct is suppressed). That seems to be the point of the anchor, but please confirm the review template labels it so it is not read as the campaign-wide best.
7. declarations has one failed and one successful run on b3f7486; please check whether the failed one needs action.

Reviewed: ee11d3ba270686d805e65087dc6391d0ac2b2461 → b3f748616d704706e380c0c72fada92ffefed74e

[cursor]

PR review verdict — DO NOT PASS

Reviewed base 4b3afa55f278ca314e07d5f8acbd0a18f3fa72bf and head 0cea874aa2b7720cd9cd228a86f8a281568e1429 (incremental b3f748616d704706e380c0c72fada92ffefed74e..0cea874aa2b7720cd9cd228a86f8a281568e1429).

This review does not approve the change. I will re-read after the author pushes.

Still open (from the review of b3f7486)

src/inferrsi/observability/template.html is not in this range, so both earlier blockers remain.

1. Outcome sentence states latency and inserts an ideographic comma in every language. template.html outcomeSentence (about line 660 at 6d0207f; unchanged since). The gain is still Latency fell / Latency rose while the same block prints row.metric_unit and the row has direction. The below-target branch still uses \uFF0C with no language check, so English becomes Latency fell 5.0%\uFF0Cshort of the 10.0% target.
   Trigger: improvement_pct below minimum_improvement_pct (comma); direction === "maximize" or a non-latency metric_unit (wording).
   Expected: join with language === 'zh' ? '\uFF0C' : ', ', and phrase the gain from direction plus metric_unit / metric_key.

2. The stage strip labels the filtered stage as in-flight. template.html renderWorkflow (about line 825 at bd996535; how here is chosen was still unchanged at 6d0207f). here is stageSelect.value or the first event with state === 'running', and that value drives both the current class and the "Current" label.
   Trigger: a stage filter is selected, or a later stage is the open one while an earlier event is still state === 'running'.
   Expected: use the filter only to choose which stage's detail is shown. Mark Current from the still-open workflow node (a start with no later _end).

New in this range

3. Native runs no longer record phase completion or duration. src/inferrsi/harness/runner.py removes node() (the finally that emitted phase + "_end" with state and elapsed_seconds) and now only calls event("source_snapshot") (line 161), event("measurement", ...) (line 167), and event("quality_validation") (line 183), with no terminal state. tests/test_dashboard_workflow.py lines 84–85 and 111 lock that in: a start-only quality_validation event leaves the workflow node unknown while execution.json still says failed, so row["quality"] == "failed" and the stage state is unknown. A cancelled or failed execute_checked no longer writes that status onto the phase event.
   Trigger: any native run after this revision, including Ctrl+C and a non-zero validator.
   Expected: keep the start/_end pair (completed, failed, or cancelled, plus elapsed seconds). Fix the strip in template.html (item 2) so Current follows the open node; dropping _end leaves every native stage unknown.

4. A withheld capture disappears from the review. dashboard.py line 302 stores unavailable_profiles from read_profiles, and tests/test_profile_timeline.py expects a tampered source to become ([], ["baseline"]). review_template.html renderCaptures (line 448) only keeps captures with duration_us > 0 and hides #capture-section when that list is empty. Nothing reads unavailable_profiles.
   Trigger: the raw profiles/*.json[.gz] is missing or its SHA-256 no longer matches the summary.
   Expected: show each unavailable id in the capture section with the failure, including when no timeline remains.

5. New timeline copy uses an ideographic stop, and one clause is untranslated. review_template.html line 288 always appends \u3002, so English reads Matched baseline capture span 1.2 s; this capture 0.9 s\u3002. Line 438 appends ' \u00b7 Open trace file\u3002' outside tr(), so Chinese keeps the English fragment and English keeps \u3002.
   Trigger: a candidate whose comparison_key matches the baseline (line 288); the source-file fold in either language (line 438).
   Expected: language === 'zh' ? '\u3002' : '.', and put "Open trace file" in both tr() strings.

Follow-ups

• renderCaptures (line 451) takes the first phase === 'baseline' capture as the only comparison target. A later baseline with the candidate's comparison_key gets no span sentence and no baseline GPU column.
• Each dashboard snapshot and each /api/profile-events/ request re-reads and hashes every raw capture (up to 64 MiB, 16 captures). That will dominate a live refresh once a trace is large.

Verified in this range

• Profile attachment stays under profiles/<name>.json[.gz], with the parent path resolved against the campaign profiles/ directory. The event table is content-addressed; /api/profile-events/<sha256> is served only when that digest is registered and the gzip bytes match. Overview rows keep lane, name, start, and duration; the new test expects argument strings such as /private to be absent.
• The 256-bin difference array matches the node fixture in test_full_profile_reader_checks_original_bytes_and_tracks_overlapping_events (gpu bins 2 and 3).

Not verified here: tests were not executed, CI was not inspected, and template.html was not re-opened from the tree (it is absent from this diff). This range also deletes six docs/dashboard-*.png files; the docs/dashboard.md hunks do not drop image links, and the unchanged doc body was not in the seed.

[amy-why-3459]

Incremental review: b3f748616d704706e380c0c72fada92ffefed74e … 0cea874aa2b7720cd9cd228a86f8a281568e1429. This covers 1 new commit, 0cea874 ("Lead with the trend and inspect complete request profiles between trials"), and 19 files. I read it against my comment at b3f7486 and the bot reviews up to that head. This review is static. I checked the diff and the files at the new head, but I did not run the tests locally this round. Not approving yet.

Resolved

• tests/test_codex_plugin.py L84-87 now also collects inferrsi/observability/*.html, which matches the pyproject.toml package-data pattern. full-check (3.10/3.11) is green at this head. declarations failed once and then passed on a rerun.
• The removed docs/dashboard-*.png files are no longer referenced anywhere in the tree.

Questions about the runner change

1. runner.py no longer emits _end events, but the consumers still depend on them. This commit removes node() and replaces it with bare event("source_snapshot"), event("measurement", …) and event("quality_validation") calls (L161, L167, L183). These events carry no state and no elapsed_seconds. node() was added by this PR, since main doesn't have it. That means:

   • dashboard_workflow.run_workflow L42-47 can only mark a node running when its start event has state == "running". No native run can produce that any more, so every native node is unknown. The updated test_validator_counts_for_native_and_paired_traces pins exactly that.
   • template.html L910-911 picks Current with events.find(e => e.details?.state === 'running'). That now never matches for a native run. The "sticky Current" problem the bots and I raised goes away, but only because Current is never shown. The filter-stage-as-Current problem is still there.
   • run_workflow's seconds and wall time come from _end elapsed_seconds, so for native runs they are always empty.

   Was dropping per-step durations and completion state from the native runner intentional? If it was, please remove or simplify the native-run paths in run_workflow and renderWorkflow/stageFacts so they don't advertise data that can't occur. Please also say so in the PR body, because it reverses the start/_end contract described earlier in this PR. If it wasn't intentional, please restore node(), or emit _end explicitly, and add the runner-level test I asked for at e66fee4.

New code: profile_timeline.py and the live endpoint

2. Every /api/state poll re-reads and re-hashes every raw trace. dashboard.py L299-302 calls read_profiles() on every dashboard_snapshot(). dashboard_live.py L123 builds a snapshot for each /api/state request, and the page polls every 3 s (review_template.html L564). _verified_summary L309 reads the complete raw source and computes its SHA-256 each time. With the current limits (16 captures × 64 MiB, _MAX_FILE L24), one open tab could hash up to about 1 GiB every 3 seconds. profile_event_table L334 runs the whole verification again for each expand. Could you cache the verification by (path, st_size, st_mtime_ns), or verify only the summary/compact digests on poll and the raw digest at attach and expand time?

3. A capture with more than 512 lanes can be attached but not expanded. _compile has no lane limit. compactTimeline (review_template.html L216) and fullTimeline (L207) both reject more than 512 lanes. So Expand full timeline, and loading the saved event table, fail for a capture that attached successfully. Please apply the same limit at attach time, or fold the extra lanes into "other lanes" in the compact table as the overview already does.

4. The size limits in the docs don't match the code. docs/dashboard.md L199-200 tells users to open the source in Perfetto when it exceeds the browser's 384 MiB decompression limit. But attach-profile rejects any stored source over 64 MiB (_MAX_FILE, L24), and the browser rejects local files over 256 MiB (review_template.html L262). Please document the 64 MiB attach limit next to the attach-profile example. Otherwise, a large trace fails with "profile exceeds the stored size limit" and the doc gives no hint.

5. (Minor) Zero-duration X events are dropped. _duration requires 0 < dur. The browser side does the same with e.dur<=0. Kineto can emit X events with dur: 0. The doc says the page "renders every timed X event", so please either keep them, or say that zero-duration events are excluded.

Still open from earlier reviews

This commit doesn't touch src/inferrsi/observability/template.html, so these remain:

• attemptResult still requires steps.state === 'present'.
• outcomeSentence still says "Latency fell/rose" and still hard-codes ，.
• The Current/filter logic is unchanged. See point 1 for how this commit changes it.
• The breaking-change note in the PR body, and the task-lexical-v1 default asymmetry, are still unanswered.

Reviewed: b3f748616d704706e380c0c72fada92ffefed74e → 0cea874aa2b7720cd9cd228a86f8a281568e1429.

[david6666666]

@amy-why-3459 @cursor PTAL at b9fe71c. Thanks for flagging the evidence and refresh issues. The fixes since your reviewed head are in 0cea874…b9fe71c.

• Native and historical runs. The native runner keeps main’s start-only log. A real AB/BA integration test inspects the dashboard during all four measurements and validation. Current follows the open workflow node, independently of the stage filter. Terminal states and command durations come from the matching execution record. A phase finish time requires a recorded end event. Crashed nodes are marked, and an untimed event suppresses the stage span.
• Outcomes and validation. Complete and partial traces display the recorded attempt result. The fallback sentence uses the metric and its direction, with localized punctuation. Invalid progress preserves the known command result. The Qwen validator writes manifest counts beside <run>/validation/result.json, the result path set by the executor. A progress-write failure preserves either quality verdict.
• Profile evidence. Replaying three real captures read 66,649,329 raw bytes on the first overview and zero on each of the next two polls. Expanding R1 reverified only its raw source and its event table. The UI lists unavailable capture IDs even when all captures fail verification, matches a unique baseline by comparison key, and labels results scoped to that baseline. Import rejects a 513th lane. The recording guide now gives the attach and browser limits and the positive-duration rule.
• Compatibility and search. The PR description lists the Python import migration and explains the different defaults for campaign discovery and standalone collection. Task ranking receives the actual backend through collect. The skill-only scorer rejects task ranking. The catalog requires title, summary, repository, scenario, and backend for every package record. Both dashboard templates live under observability/, which is why the old data/dashboard/*.html packaging rule was removed.

On Windows, 158 dashboard, profile, Qwen, and search tests passed, plus 8 subtests. Ruff passed. In Chrome I also verified both the three real timelines and the list of unavailable IDs when every capture is withheld.

[amy-why-3459]

LGTM

PR review verdict — PASS
Reviewed base main and head b9fe71c23fca52f7625e99ba87161f6076989a33. This is an incremental review of 0cea874aa2b7720cd9cd228a86f8a281568e1429 … b9fe71c23fca52f7625e99ba87161f6076989a33 (2 commits, 14 files), checked against my comment at 0cea874 and the latest bot review.

Verified:

• Native runs. runner.py is unchanged from main. The dashboard now gets terminal state and command duration from each trial's or validation's execution.json (command_state, dashboard_data.py L611-631). The open step comes from status.json phase plus the latest start (dashboard_workflow.py L242-246). A recorded _end that disagrees with the command record makes the step unknown. test_dashboard_tracks_real_native_abba_and_validation drives the real run_experiment AB/BA flow and checks, at every step, that exactly the right node is running.
• Stage strip. Current now follows the open workflow nodes (stageFacts / renderWorkflow), separately from the stage filter. Untimed events suppress the stage span, and crashed or failed states are marked. There is a Node regression test for all of this.
• Outcome sentence. The wording comes from metric_key + direction (rose/fell/unchanged), with a neutral "Measured change" fallback. Joiners and punctuation are localized. attemptResult accepts complete and partial traces and uses the last recorded attempt.
• Profiles.
  • Overview polls reuse a digest keyed on dev/inode/mode/size/mtime_ns/ctime_ns (_overview_digest). Opening the event table rehashes only the selected source.
  • Tests cover a same-size edit with the mtime restored, and an atomic replacement.
  • The 512-lane limit is now enforced at attach time.
  • Unavailable capture IDs are listed, and the baseline is matched only by a unique comparison_key.
  • The attach, decode and lane limits and the positive-duration rule are documented in docs/dashboard.md.
  • review_template.html L288/L438 punctuation and the Perfetto instruction are localized.
• Validation progress. Invalid or unparsable progress.json keeps the command-level verdict and count. The Qwen validator writes next to INFERRSI_RESULT_PATH, and a failed progress write doesn't change either verdict.
• Search. skill_score rejects task-lexical-v1. The only caller (knowledge_operations.py L685-688) sends task ranking to task_score. The PR body now documents the breaking import migration and the different campaign/standalone relevance defaults.
• CI. At this head, full-check (3.10/3.11), installed-package (3.10/3.11), lint-and-smoke, declarations and pr-references all pass.

Not verified: I didn't run the tests locally. This review is based on reading the diff and the files at this head.

[cursor]

PR review verdict — DO NOT PASS

Reviewed base 4b3afa55f278ca314e07d5f8acbd0a18f3fa72bf and head b9fe71c23fca52f7625e99ba87161f6076989a33 (incremental 0cea874aa2b7720cd9cd228a86f8a281568e1429..b9fe71c23fca52f7625e99ba87161f6076989a33).

This review does not approve the change. I will re-read after the author pushes.

Resolved from the review of 0cea874

src/inferrsi/observability/template.html is in this range.

1. Outcome sentence. outcomeSentence now takes the verb from direction (rose / fell / was unchanged) and the noun from metric_key (latency* → Latency, throughput* → Throughput, otherwise the key; "Measured change" when direction is missing). The below-target join is language === 'zh' ? '，' : ', '. test_round_outcome_and_stage_strip_follow_recorded_evidence locks the English strings (throughput rose / fell / unchanged, and a non-latency key).

2. Stage strip. renderWorkflow no longer treats stageSelect.value or the first details.state === 'running' event as in-flight. current and the Current label follow stageFacts().open, set only when a workflow node is running on a candidate whose state === 'running'. The same test selects diagnosis and asserts validation is the current stage, and that a crashed row clears open.

Attempt results: attemptResult at this head's parent still required steps.state === 'present'. It now accepts complete / partial and uses the last non-empty attempt. That test covers it.

Blockers

1. A non-dict execution or validation snapshot aborts the dashboard load. command_state in src/inferrsi/observability/dashboard_workflow.py (new helper, record.get immediately) does not check isinstance(record, dict). load_dashboard now feeds it _object_snapshot payloads whenever missing_reason != "file_absent":

   • measurement: native["measurement", side, trial] = command_state(execution) in src/inferrsi/observability/dashboard_data.py (trial execution.json branch)
   • quality: native["quality_validation", None, None] = command_state(validation) in the same file

   This diff changes validation_progress to return None unless record is a dict, and test_validator_counts_for_native_and_paired_traces writes [] and { into validation/progress.json and still calls load_dashboard. Those values reach the snapshot consumer. The same helper's execution payload is not guarded. execution.json containing a JSON array (or any non-dict) raises AttributeError inside command_state. One bad trial file drops the run view. The native runner records command state without _end events (test_dashboard_tracks_real_native_abba_and_validation expects every node end is None), so this path is the one that decides the work-unit state.

   Trigger: a present execution.json or validation record whose JSON value is not an object.
   Expected: do not call .get on a non-dict. Leave the native entry unset, same as file_absent, so the event-derived state (including a still-open running start) stays intact. A malformed progress.json already must not replace command-level state; a malformed command record should not either. Returning ("unknown", None) and taking the not end and command branch would still wipe a live start, because that branch does not treat "unknown" as "no command record".

Follow-ups (not blocking)

• refreshElapsed in template.html labels every live data-started cell "Phase duration". workDuration uses that label only when duration_source === 'phase' or both endpoints exist. An open step has neither a paired _end nor a command elapsed (docs: phase finish time stays unknown without _end).
• skill_score now rejects task-lexical-v1 (src/inferrsi/_skill_selection.py). test_raw_string_selection.py expects the ValueError. Callers outside this diff were not traced.

Not verified

Tests were not executed. CI was not checked. Of the new limits in docs/dashboard.md, this diff shows 64 MiB (_MAX_FILE) and 512 duration lanes (_MAX_LANES, tested). The 16-capture, 2³⁰-character, 2,000,000-event, and 128 MiB compact-table figures are not in the incremental diff.