Flag chat-only completions (zero tool calls with model output) as a diagnostic

[ljr145733]

Fixes #988 — implements the design proposed in my comment there. Opening the PR now so the semantics discussion can happen on concrete code rather than in the abstract; happy to rework any of it if maintainers prefer a different call.

Problem

A rollout where the agent produced real model output but ended its turn without a single tool call was recorded as a clean scored fail — error: null, reward 0.0 from the verifier — indistinguishable in aggregates from a genuine attempt. The existing zero-signal net (suspected_api_error) only covers the zero-token half of the no-op space; the reported rollout had 2,727 output tokens ("Proceeding to add PLAN.md…") and fell through.

Semantics (the key design decision)

A chat-only completion is agent/model behavior, not infrastructure failure — so unlike suspected_api_error this diagnostic leaves reward and error untouched: the slot stays a scored fail and stays in score denominators. It only becomes visible. This matches the intent already documented in test_zero_tools_with_tokens_not_flagged ("Prompt-only answer (no tools) with real usage is a legitimate rollout") — legitimate for the error classifier, but worth surfacing for sweep forensics.

What this adds

• NoToolCallCompletionDiagnostic (diagnostics.py), field no_tool_call_completion_info, category = None — registered, so it lands in result.json via the existing to_result_fields() spread.
• Detection in _maybe_flag_no_tool_completion() (rollout/__init__.py), called right after _maybe_classify_api_error(). Guards reuse the established ones: error channel already claimed → skip (zero-token cases still route to suspected_api_error); no executed prompts → skip (Self-gen skill export failures are swallowed, allowing empty skill updates to look successful #389); oracle → skip; native-subscription telemetry exemption from Registry-driven Claude subscription gate + zero-signal heuristic exemption (Anthropic OAuth unlock) #886 → skip; and the trajectory must contain at least one agent_message event, so trajectory-capture loss (Claude ACP trajectories capture no thinking text on models that default thinking.display to "omitted" #982) is never misflagged as a chat-only completion.
• Surfacing: per-rollout CLI line gains a , no-op marker ([FAIL] task-x (reward=0.00, tools=0, no-op)); summary.json gains a no_tool_call_completions count; the registry-driven job-summary warning loop now counts category-less diagnostics by field presence in result.json (no behavior change for the seven existing diagnostics, which all carry categories).
• RolloutResult gains a trailing optional no_tool_completion: bool = False so the CLI reads the authoritative flag instead of re-deriving it.

Open question from the issue thread

The tool-telemetry guard currently reuses uses_native_subscription_auth() (option (a) from my comment) — symmetric with _maybe_classify_api_error, but over-broad for ACP adapters that do report tool_call events under subscription auth. If you'd rather gate on a per-adapter tool-telemetry capability flag (option (b)), I'm happy to do that here or as a follow-up — the detection site has a TODO marking the swap point.

Testing

• New tests/test_no_tool_completion.py (13 tests): core semantics (flag set, reward/error untouched), one negative test per guard, precedence (zero-token case still routes to suspected_api_error), registry integrity, format_issue rendering.
• Adjacent suites pass unchanged: test_api_error_capture.py (30), test_diagnostics.py + test_eval_single_task_summary.py (17), test_rollout_architecture.py + test_integration_check_results.py + test_rollout_import_no_side_effects.py (69). ruff check clean on all touched files.

---

Disclosure: Claude assisted with the implementation and tests under my direction; I reviewed and take responsibility for the design, the semantics, and all code.

[Galius5136]

Thanks for putting this together! I tested the PR end to end and the per-rollout diagnostic looks good on the #988 shape: the flag is set, while reward/error stay unchanged.

I did hit one issue at the aggregate level though. On a fresh bench eval run, the chat-only rollout had no_tool_call_completion_info populated, but summary.json still reported no_tool_call_completions: 0 and no job-summary warning was shown. Re-running the same job from disk, with the same artifacts and 0 tasks re-executed, changed the count to 1 and emitted the warning. It looks like fresh results go through rollout_result_payload(), which currently doesn't carry no_tool_call_completion_info, while resumed results are read back from result.json.

I also noticed a narrower edge case in the Gemini scraped-trajectory fallback: the saved trajectory can contain tool calls while _n_tool_calls remains 0, so the new diagnostic can say “made 0 tool calls” next to trajectory_summary.tool_call_steps: 3. I only reproduced this by forcing the salvage precondition in a probe, so I can't say how common it is in real runs.

Small cleanup: ruff format --check src tests tools currently fails on the PR head (base passes) for the new formatting in evaluation.py and rollout/__init__.py.

Hope this helps, happy to re-test if you push a fix.

[ljr145733]

Thanks for the thorough end-to-end test — all three findings were real. I really appreciate it. They should be fixed in 01bb6bc:

1. Fresh-run aggregation: rollout_result_payload() is RolloutResult-driven and never carried the diagnostic payloads, so summary.json only counted them after a resume re-read result.json from disk. Rather than special-casing the new field, I generalized the existing summary.json omits aggregate tool-call and phase-timing metrics #501 persisted-timing enrichment into _enrich_payload_with_persisted_fields(), which now copies every DIAGNOSTIC_REGISTRY field from the persisted result.json into fresh-run payloads — fresh and resumed jobs now aggregate identically, for this diagnostic and any future one. Covered by three new tests (fields copied, nulls skipped, in-memory values never overwritten).

2. Scraped-trajectory fallback: confirmed — the salvage path rebuilds tool_call events the ACP session never counted, so _n_tool_calls can be 0 next to trajectory_summary.tool_call_steps > 0. The detection now treats the trajectory as authoritative: any tool_call event in it disqualifies the chat-only flag. Negative test added with exactly that shape.

3. Formatting: reproduced and fixed; ruff format --check src tests tools is clean on the new head.

Would appreciate a re-test if you have the time — the fresh-run repro you described should now show no_tool_call_completions: 1 and the job-summary warning on the first pass.

[Galius5136]

Re-tested on 01bb6bc and all three look fixed now. Thanks for the quick turnaround!

Fresh-run aggregation now matches resume on the same artifacts, the scraped-trajectory case no longer gets the chat-only diagnostic, and ruff format --check src tests tools is clean.

One small test-coverage note: I tried removing the new enrichment call site and the current test suite still stays green, so it may be worth adding an Evaluation-level regression test for that wiring. Nothing blocking from my side though, happy with the fixes :)

[ljr145733]

Good catch on the coverage gap — you're right that the enrichment tests exercised the helper in isolation, so the call-site wiring itself was unguarded. Added in c378761: an Evaluation.run()-level regression test where a mocked rollout persists no_tool_call_completion_info only to its on-disk result.json (the exact fresh-run shape) and the test asserts summary.json counts it on the first pass. Verified it goes red with the call site removed and green with it present. Thanks again for the careful testing!

[Galius5136]

Confirmed the new test goes red without the call site. Thanks, looks good to me!