Skip to content

Trace anomalies for stall diagnosis: design doc + --json on inspect trace anomalies/http#83

Closed
i-am-marvin wants to merge 6 commits into
mainfrom
claude/issue-81-20260714-2157
Closed

Trace anomalies for stall diagnosis: design doc + --json on inspect trace anomalies/http#83
i-am-marvin wants to merge 6 commits into
mainfrom
claude/issue-81-20260714-2157

Conversation

@i-am-marvin

@i-am-marvin i-am-marvin commented Jul 14, 2026

Copy link
Copy Markdown

This PR contains:

  • New features
  • Docs
  • Bug fixes

What is the current behavior? (You can also link to an open issue here)

Refs #81 (the issue stays open for the remaining slices).

design/control-channel.md has no coverage of the trace log / inspect trace anomalies content, even though field use (per JJ) showed that telling the LLM agent about trace anomalies — what, if anything, is still running or hung — is a valuable part of the live-eval diagnosis loop. Today inspect trace anomalies and inspect trace http emit only styled Rich tables (the worst case for an agent parser), nothing in inspect ctl joins to the trace subsystem, and anomalies --filter crashes with a RuntimeError when the filter matches an action's completion record but not its start record.

What is the new behavior?

Design doc. Adds a "Trace-log anomalies for stall diagnosis" section to design/control-channel.md (slotted into phase 3, since it needs no server changes), designing:

  • --json on inspect trace anomalies and http (implemented in this PR — see below).
  • inspect ctl process anomalies [PID] [--filter TEXT] [--all] [--json] — a client-side trace-file read (deliberately no HTTP endpoint, so it works against exactly the busy/hung process it exists to diagnose), placed in the process noun group since trace records carry no task/sample identity, with a dead-pid post-mortem fallback to trace-<pid>.log[.gz].
  • Stall-site pointers (long-idle samples, busy-skip notes) that teach the escalation path through the surface itself.
  • Open question: inlining running actions into sample show.

Implementation (first slice). --json on both commands:

  • inspect trace anomalies --json emits a {trace_file, as_of, running, cancelled, errors, timeouts} envelope with per-action rows {action, detail, start_time, duration, error?}; running durations are computed against the envelope's as_of. All four buckets are always populated — --all gates only the human table — so an empty list always means "none occurred", never "not collected".
  • inspect trace http --json emits a {trace_file, as_of, requests} envelope.
  • The anomalies flags (--filter / --all / --json) are defined once as a shared click decorator, so the planned ctl process anomalies mirrors them structurally rather than by convention.
  • The anomaly reconstruction is refactored out of the command into a pure function returning a TraceAnomalies NamedTuple (one reconstruction, two renderings), fixing the misspelled anomolies_* names along the way.
  • Bug fix: anomalies --filter no longer raises when a completion record matches the filter but its start record doesn't (orphan completions are now skipped).

Covered by new CLI tests (tests/cli/test_cli_trace.py) and two CHANGELOG entries.

Still to come (tracked in #81): inspect ctl process anomalies and the stall-site pointers.

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

No.

Other information:

None.

🤖 Generated with Claude Code

Designs the affordances requested in issue #81: --json on
`inspect trace anomalies`, an `inspect ctl process anomalies` verb
(client-side trace-file read, deliberately no HTTP endpoint so it
works against a wedged process), and stall-site pointers. Design
only — no implementation.

Co-authored-by: Ransom Richardson <1209015+ransomr@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@i-am-marvin

Copy link
Copy Markdown
Author

@review

Comment thread design/control-channel.md Outdated
Comment thread design/control-channel.md Outdated
@claude

claude Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review summary

Docs-only PR adding a "Trace-log anomalies for stall diagnosis (planned)" section to design/control-channel.md plus a phasing-list pointer. I verified the section's factual claims against the code and its cross-references against the rest of the doc; it holds up well. Two minor, non-blocking nits posted inline.

Verified accurate against the code (src/inspect_ai/_cli/trace.py, src/inspect_ai/_util/trace.py):

  • anomalies default buckets are running + cancelled, with --all gating errors/timeouts; the human output is a rich table exported with ANSI styles (export_text(styles=True)); running-row durations are computed as now − start_time at read time — all exactly as the section states.
  • Trace files are pid-keyed at <inspect_data_dir>/traces/trace-<pid>.log; ActionTraceRecord carries no task or sample identity; neither anomalies nor http has --json today. The proposed row shape {action, detail, start_time, duration, error?} matches the fields the records actually carry.
  • Internal cross-references (constraint 1, the Phase 1 GET /evals/<id>/samples blind-spot caveat, "Busy is not absent", the unknown-command handler, the as_of envelope convention, unconditional task_id) all resolve to real passages in the doc.

Nits (both non-blocking, inline):

  1. The dead-pid post-mortem open question should account for the file usually being trace-<pid>.log.gz — gzip happens via atexit on clean exit, so only a hard-killed process leaves the bare .log.
  2. The ctl process anomalies spec doesn't say whether --filter (which inspect trace anomalies has) is mirrored or deliberately omitted.

The design decisions themselves (client-side file read with no HTTP endpoint, one shared reconstruction, process-noun placement, stall-site pointers) are well argued and consistent with the doc's existing conventions. No CHANGELOG entry is correct for a design-doc-only change.

If you'd like these nits addressed by the dev agent, reply with a top-level @claude <instruction> comment (inline replies don't reach it).

@claude

claude Bot commented Jul 14, 2026

Copy link
Copy Markdown

🔎 Review complete.

…otting

- ctl process anomalies mirrors every inspect trace anomalies flag
  (--filter included) via a shared click decorator so they can't drift
- Resolve the dead-pid open question: fall back to trace-<pid>.log or
  .log.gz for post-mortem reads; fix the rotation-vs-gzip conflation
- Slot the work into phase 3 (current phase), --json first; remove the
  post-phase-4 deferral and add the verb to the CLI listings

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ransomr

ransomr commented Jul 16, 2026

Copy link
Copy Markdown
Member

@auto implement the first part of this design - adding --json to trace anomalies

@i-am-marvin i-am-marvin added the auto @auto: drive this PR autonomously (remove to stop) label Jul 16, 2026
@i-am-marvin

i-am-marvin commented Jul 16, 2026

Copy link
Copy Markdown
Author

Claude finished @ransomr's task in 11m 12s —— View job


Implemented: --json on inspect trace anomalies (and http)

  • Merge origin/main into the PR branch
  • Read the design section and current trace CLI code
  • Implement --json on inspect trace anomalies (enveloped buckets) and inspect trace http, with shared click options
  • Add tests (tests/cli/test_cli_trace.py, 5 tests)
  • Install project, run pytest / ruff / mypy on changed files — all green
  • Add CHANGELOG entry (verified under ## Unreleased)
  • Commit and push (0ef100dba, plus merge of origin/main in dffd50761)

What shipped

First slice of the design section this PR added, exactly per its spec (src/inspect_ai/_cli/trace.py):

  • inspect trace anomalies --json emits a {"trace_file", "as_of", "running", "cancelled", "errors", "timeouts"} envelope (as_of a time.time() float, matching the ctl convention). All four bucket keys are always present as lists — errors/timeouts stay empty unless --all, mirroring the human default. Rows are {action, detail, start_time, duration, error?} with error present only on error rows; running rows compute duration as as_of − start_time at read time. With no anomalies, --json still emits the envelope (empty buckets) rather than the human prose.
  • inspect trace http --json emits {"trace_file", "as_of", "requests"} with {timestamp, message} rows; --failed filters the JSON output the same way it filters the table.
  • Shared flags, shared reconstruction: the anomalies options (--filter, --all, --json) now live in one anomalies_options click decorator and the bucket reconstruction in a shared _trace_anomalies() function returning a TraceAnomalies NamedTuple — the structural single-home the design specs so the planned inspect ctl process anomalies can apply/delegate to the same code. Human rendering is unchanged (verified end-to-end against a real trace file alongside the JSON output).
  • Design doc updated to mark this slice shipped (the section previously said "nothing here is implemented yet"), and a CHANGELOG entry added under ## Unreleased.

Verification

pytest tests/cli tests/util/test_trace.py → 22 passed, 1 skipped (unrelated ts-mono submodule); ruff format/ruff check clean; mypy clean on both changed Python files. Also ran the real CLI end-to-end: wrote a trace file with a hung action and an errored action, and confirmed --json (both commands) and the human table render as designed.
· Branch

github-actions Bot and others added 2 commits July 16, 2026 14:04
First slice of the trace-log anomalies design (issue #81): both commands
emit `{trace_file, as_of, ...}` envelopes per the agent output contract.
Anomalies buckets (running/cancelled/errors/timeouts) are always present,
rows are `{action, detail, start_time, duration, error?}` with running
durations computed against `as_of`. Flags live in a shared
`anomalies_options` decorator and the reconstruction in a shared
`_trace_anomalies` function, ready for the planned
`inspect ctl process anomalies` to reuse.

Co-authored-by: Ransom Richardson <1209015+ransomr@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@i-am-marvin

Copy link
Copy Markdown
Author

@review

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the --json implementation slice. Verification details in the top-level summary comment; two non-blocking inline notes below.

Comment thread src/inspect_ai/_cli/trace.py
Comment thread src/inspect_ai/_cli/trace.py Outdated
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review summary

This round reviews the newly implemented slice (commit 0ef100d): --json on inspect trace anomalies and inspect trace http, the shared anomalies_options decorator, the _trace_anomalies() extraction, tests, CHANGELOG, and the design-doc updates marking the slice shipped.

Verified:

  • Ran the new test suite (5 passed), ruff check/ruff format --check (clean), and mypy on both changed Python files (clean).
  • Exercised the real CLI end-to-end against a synthetic trace file: both JSON envelopes match the design spec exactly (trace_file/as_of envelope, all four bucket keys always present, running-row duration = as_of - start_time, error only on error rows, --failed filtering the http JSON), and the human rendering is byte-for-byte the same shape as before (ANSI table, per-bucket ordering, empty-case prose).
  • The refactor is behavior-preserving: _trace_anomalies is a straight extraction of the old reconstruction loop, _sorted_actions/_action_duration consolidate previously-duplicated logic, and the http --failed filter is unchanged semantics.
  • CHANGELOG entry sits under ## Unreleased; ts-mono submodule untouched; design-doc status/phase-3 updates are consistent with what shipped.

Findings (both non-blocking, posted inline):

  1. --json without --all emits "errors": [] / "timeouts": [] even when the file contains errors/timeouts, so an agent cannot tell "none occurred" from "not collected" -- worth either always populating buckets under --json or stamping the mode into the envelope (fine to defer to the ctl process anomalies slice).
  2. "anomolies" typo in the now-shared --all help text (pre-existing, but it will surface in ctl process anomalies --help too).

Pre-existing issue, not introduced here (no action required): --filter applies to raw messages before reconstruction, so a filter matching an exit-side record but not its enter (e.g. --filter error --all) crashes with RuntimeError: Expected ... in action dictionary -- confirmed on main as well. Worth keeping in mind for the ctl process anomalies slice, which mirrors --filter onto an agent-facing path where a traceback on stdout is worse.

If you would like the nits addressed by the dev agent, reply with a top-level @claude <instruction> comment (inline replies do not reach it).

@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

🔎 Review complete.

@i-am-marvin

Copy link
Copy Markdown
Author

🤖 auto review rounds: 1 (cap 10).

…crash

- `inspect trace anomalies --json` now always populates the errors/timeouts
  buckets so an empty list always means "none occurred"; --all gates only
  the human rendering (design doc updated to match)
- fix "anomolies" typo in the shared --all help text and rename the internal
  anomolies_command* functions
- tolerate exit-side action records with no matching enter record (e.g.
  --filter matching only the exit side, or a truncated log) instead of
  crashing with RuntimeError

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@i-am-marvin

Copy link
Copy Markdown
Author

@review

@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review summary

This round reviews commit 63b67f3 (the response to the previous round): always-populated JSON buckets, the --filter orphan-exit-record crash fix, the help-text typo fix, and the anomoliesanomalies internal renames.

Verified:

  • All three items from the previous round are correctly addressed: _trace_anomalies() now collects all four buckets unconditionally (the all flag gates only the human rendering), so an empty errors/timeouts list in the envelope always means "none occurred"; exit-side records with no matching enter record are bucketed with start_time: null instead of raising RuntimeError (this also fixes the pre-existing crash where a plain exit record's enter was filtered out); the help-text typo and internal names are fixed, with no anomolies spellings remaining anywhere.
  • Ran the test suite (tests/cli/ — 23 passed, 1 unrelated skip), ruff check / ruff format --check (clean), and mypy on both changed files (clean).
  • Smoke-tested the CLI against a synthetic trace file: inspect trace anomalies --json emits a clean envelope on stdout (running rows compute live durations against the single as_of stamp; orphaned error records carry start_time: null and their recorded duration), the human rendering and --help text are correct, and inspect trace http --json works including the --failed filter.
  • Both CHANGELOG entries sit under ## Unreleased; no submodule pointer changes; the design doc's envelope/row schema and "always populated" contract match the implementation exactly.

No outstanding suggestions — the shared anomalies_options decorator and the extracted _trace_anomalies() reconstruction are well-positioned for the planned ctl process anomalies verb.

@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

🔎 Review complete — no outstanding suggestions.

@i-am-marvin

Copy link
Copy Markdown
Author

🤖 auto: the reviewer has no outstanding feedback — this PR has converged. auto never takes the final step (merge, or upstream promotion on the fork), so it's yours now; see the checks above for CI status. The auto label is left on, so auto will still react to new review or CI activity; remove it to disengage.

@ransomr ransomr changed the title Design: trace-log anomalies integration for the control channel Trace anomalies for stall diagnosis: design doc + --json on inspect trace anomalies/http Jul 16, 2026
@ransomr

ransomr commented Jul 16, 2026

Copy link
Copy Markdown
Member

Superseded by the upstream PR: UKGovernmentBEIS#4524 (now the system of record).

@ransomr ransomr closed this Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto @auto: drive this PR autonomously (remove to stop)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants