Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions src/benchflow/diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,36 @@ def format_issue(self, task_name: str) -> str:
)


@dataclass
class NoToolCallCompletionDiagnostic(Diagnostic):
"""Chat-only completion: the agent produced model output (tokens and at
least one agent message) but ended its turn without issuing a single tool
call (#988). Unlike the api-error verdicts this is agent/model BEHAVIOR,
not an infrastructure failure — the reward and error channels are left
untouched (``category`` stays None), so the slot remains a scored fail.
The diagnostic only makes the no-op visible in result.json, the CLI line,
and the job summary so baseline sweeps don't read these rollouts as
genuine attempts."""

total_tokens: int = 0
n_output_tokens: int = 0
n_agent_messages: int = 0
n_message_chars: int = 0

field: ClassVar[str] = "no_tool_call_completion_info"
category: ClassVar[str | None] = None
summary_description: ClassVar[str] = (
"completed without a single tool call (chat-only)"
)

def format_issue(self, task_name: str) -> str:
return (
f"{task_name}: chat-only completion — agent produced "
f"{self.n_output_tokens or self.total_tokens} tokens across "
f"{self.n_agent_messages} message(s) but made 0 tool calls"
)


# Public registry — every diagnostic kind goes here exactly once.
DIAGNOSTIC_REGISTRY: tuple[type[Diagnostic], ...] = (
IdleTimeoutDiagnostic,
Expand All @@ -343,6 +373,7 @@ def format_issue(self, task_name: str) -> str:
VerifierTimeoutDiagnostic,
ProviderApiErrorDiagnostic,
SuspectedApiErrorDiagnostic,
NoToolCallCompletionDiagnostic,
)

# field_name → Diagnostic class, for check_results lookup.
Expand Down
66 changes: 41 additions & 25 deletions src/benchflow/evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -1164,20 +1164,21 @@ def _prune_docker(self):
finally:
_PRUNE_LOCK.release()

def _enrich_payload_with_persisted_timing(
def _enrich_payload_with_persisted_fields(
self, payload: dict, result: RolloutResult
) -> None:
"""Copy ``timing`` from the rollout's on-disk result.json into payload.

``RolloutResult`` does not carry phase timing, but the rollout writer
(``rollout.py``) persists it under ``rollout_dir/result.json``. Reading
it back lets ``phase_timing_summary`` aggregate phase totals for fresh
runs (issue #501). Best-effort: legacy SDK paths that mock the writer
— or any case where no rollout_name is set — silently leave timing
absent rather than crash summary generation.
"""Copy persisted-only fields from the rollout's on-disk result.json.

``RolloutResult`` does not carry phase timing or the structured
diagnostic payloads, but the rollout writer (``rollout.py``) persists
both under ``rollout_dir/result.json``. Reading them back lets
``phase_timing_summary`` (issue #501) and the diagnostic counters
(#988) aggregate fresh runs the same way they cover resumed tasks,
which are read straight from result.json. Best-effort: legacy SDK
paths that mock the writer — or any case where no rollout_name is
set — silently leave the fields absent rather than crash summary
generation.
"""
if "timing" in payload:
return
rollout_name = getattr(result, "rollout_name", "") or ""
if not rollout_name:
return
Expand All @@ -1187,11 +1188,15 @@ def _enrich_payload_with_persisted_timing(
try:
persisted = json.loads(rfile.read_text())
except (json.JSONDecodeError, OSError) as e:
logger.debug("Could not read persisted timing from %s: %s", rfile, e)
logger.debug("Could not read persisted fields from %s: %s", rfile, e)
return
timing = persisted.get("timing")
if isinstance(timing, dict):
if "timing" not in payload and isinstance(timing, dict):
payload["timing"] = timing
for diag_cls in DIAGNOSTIC_REGISTRY:
info = persisted.get(diag_cls.field)
if diag_cls.field not in payload and isinstance(info, dict):
payload[diag_cls.field] = info

async def _run_single_task(
self, task_dir: Path, cfg: EvaluationConfig
Expand Down Expand Up @@ -1403,8 +1408,11 @@ def _log_and_report(self, td: Path, result: RunResult) -> None:
if isinstance(reward, (int, float)) and not isinstance(reward, bool)
else ""
)
# Chat-only completions (#988): the agent answered but never called a
# tool — mark the line so a sweep log shows the no-op at a glance.
no_op = ", no-op" if getattr(result, "no_tool_completion", False) else ""
logger.info(
f"[{status}] {td.name} ({reward_part}tools={result.n_tool_calls}){err}"
f"[{status}] {td.name} ({reward_part}tools={result.n_tool_calls}{no_op}){err}"
)
self._fire_progress(self._on_result, td.name, result)

Expand Down Expand Up @@ -1799,10 +1807,11 @@ async def run(self) -> EvaluationResult:
task_name=name,
)
# ``rollout_result_payload`` is RolloutResult-driven and so cannot
# see ``timing`` (it lives only in the persisted result.json).
# Pull it from disk so phase-timing aggregates cover fresh pairs
# the same way they cover resumed tasks (issue #501).
self._enrich_payload_with_persisted_timing(payload, result)
# see ``timing`` or the diagnostic payloads (they live only in
# the persisted result.json). Pull them from disk so phase-timing
# (#501) and diagnostic aggregates (#988) cover fresh pairs the
# same way they cover resumed tasks.
self._enrich_payload_with_persisted_fields(payload, result)
all_results[name] = payload

# EvaluationResult is the score/invariant view. summary.json is the
Expand Down Expand Up @@ -1890,6 +1899,9 @@ async def run(self) -> EvaluationResult:
"error": audit_counts["errored"],
"verifier_errored": audit_counts["verifier_errored"],
"idle_timeout": error_category_counts.get(IDLE_TIMEOUT, 0),
"no_tool_call_completions": sum(
1 for r in all_results.values() if r.get("no_tool_call_completion_info")
),
"error_categories": error_category_counts or None,
"verifier_error_categories": verifier_error_category_counts or None,
"score": f"{pass_rate(passed=audit_counts['passed'], total=job_result.total):.1%}",
Expand Down Expand Up @@ -1968,13 +1980,17 @@ async def run(self) -> EvaluationResult:
# new diagnostic class adds its warning automatically (issue #503).
for diag_cls in DIAGNOSTIC_REGISTRY:
if diag_cls.category is None:
continue
counts = (
error_category_counts
if diag_cls.channel == "error"
else verifier_error_category_counts
)
count = counts.get(diag_cls.category, 0)
# Category-less diagnostics are behavior flags (e.g. chat-only
# completions, #988) that surface no error channel — count
# them by field presence in result.json instead.
count = sum(1 for r in all_results.values() if r.get(diag_cls.field))
else:
counts = (
error_category_counts
if diag_cls.channel == "error"
else verifier_error_category_counts
)
count = counts.get(diag_cls.category, 0)
if count > 0:
logger.warning(summary_warning(diag_cls, count, job_result.total))

Expand Down
6 changes: 6 additions & 0 deletions src/benchflow/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ class RolloutResult:
and ``verifier_error``. See #389 follow-up.
partial_trajectory: True when the trajectory was salvaged from a timed-out
or crashed session and may be incomplete.
no_tool_completion: True when the agent produced model output but ended
without a single tool call (chat-only completion, #988).
Behavior visibility only — rewards and error stay as the
verifier/agent left them.
trajectory_source: Provenance label for ``trajectory`` — one of
``"acp"`` (trusted), ``"scraped"`` (UNTRUSTED, agent-writable,
forgeable), ``"partial_acp"`` (partial ACP capture). Verifier
Expand Down Expand Up @@ -149,6 +153,7 @@ def __init__(
verifier_error_category: str | None = None,
export_error: str | None = None,
partial_trajectory: bool = False,
no_tool_completion: bool = False,
trajectory_source: TrajectorySource | None = None,
reward_events: list[RewardEvent] | None = None,
evolved_skills: dict[str, str] | None = None,
Expand Down Expand Up @@ -181,6 +186,7 @@ def __init__(
self.verifier_error_category = verifier_error_category
self.export_error = export_error
self.partial_trajectory = partial_trajectory
self.no_tool_completion = no_tool_completion
self.trajectory_source = trajectory_source
self.reward_events = reward_events
self.evolved_skills = evolved_skills
Expand Down
73 changes: 73 additions & 0 deletions src/benchflow/rollout/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
from benchflow.contracts import RoundResult as RoundResult
from benchflow.diagnostics import (
AgentPromptTimeoutError,
NoToolCallCompletionDiagnostic,
ProviderApiErrorDiagnostic,
RolloutDiagnostics,
SuspectedApiErrorDiagnostic,
Expand Down Expand Up @@ -2625,6 +2626,77 @@ def _maybe_classify_api_error(self) -> None:
# excluded from score denominators (rerun-able, never counted).
self._rewards = None

def _maybe_flag_no_tool_completion(self) -> None:
"""Flag a chat-only completion: model output but zero tool calls (#988).

The complement of the zero-signal heuristic above: the agent DID
respond (tokens, agent messages) yet ended its turn without a single
tool call — typically narrating intent ("Proceeding to add ...") and
stopping. That is agent/model behavior, not an infrastructure
failure, so unlike the api-error verdicts this leaves reward and
error untouched (the slot stays a scored fail); it only records a
diagnostic so result.json, the CLI line, and the job summary make
the no-op visible instead of letting baseline sweeps read it as a
genuine attempt.
"""
if self._error is not None:
return
# Same rationale as _maybe_classify_api_error: only judge rollouts
# where the agent actually ran (#389).
if not getattr(self, "_executed_prompts", None):
return
if getattr(self, "_n_tool_calls", 0):
return
config = getattr(self, "_config", None)
# Oracle rollouts run the reference solution directly — zero tool
# calls is their normal shape (they also record no executed prompts,
# so this is belt-and-braces).
if config is not None and config.primary_agent == "oracle":
return
# Same guard as _maybe_classify_api_error: agents whose trajectories
# carry no tool telemetry (e.g. omnigent's flat session events) look
# tool-free on every healthy run, so the zero-tool signal is
# meaningless there. TODO(#988): swap for a per-adapter
# tool-telemetry capability flag if maintainers prefer that gate.
from benchflow.agents.env import uses_native_subscription_auth

if config is not None and uses_native_subscription_auth(
config.agent,
config.model,
getattr(self, "_agent_env", None) or {},
):
return
trajectory = getattr(self, "_trajectory", None) or []
# The tool-call counter and the trajectory can disagree on salvage
# paths: the scraped-trajectory fallback (e.g. gemini's native
# trajectory file) rebuilds tool_call events the ACP session never
# counted, so _n_tool_calls stays 0 while the trajectory shows real
# tool activity. Any tool_call event means the agent DID act.
if any(
isinstance(e, dict) and e.get("type") == "tool_call" for e in trajectory
):
return
agent_messages = [
e
for e in trajectory
if isinstance(e, dict) and e.get("type") == "agent_message"
]
# No agent messages either: that is a zero-signal/capture question
# owned by the verdicts above (#982), not a chat-only completion.
if not agent_messages:
return
usage_metrics = getattr(self, "_usage_metrics", None) or {}
self._diagnostics.set(
NoToolCallCompletionDiagnostic(
total_tokens=_as_nonnegative_int(usage_metrics.get("total_tokens")),
n_output_tokens=_as_nonnegative_int(
usage_metrics.get("n_output_tokens")
),
n_agent_messages=len(agent_messages),
n_message_chars=sum(len(e.get("text") or "") for e in agent_messages),
)
)

def _loop_strategy_metadata(self) -> dict[str, Any] | None:
"""Loop-strategy run summary for the result.json ``loop`` block.

Expand All @@ -2649,6 +2721,7 @@ def _loop_strategy_metadata(self) -> dict[str, Any] | None:
def _build_result(self) -> RolloutResult:
rollout_dir = self._require_rollout_dir()
self._maybe_classify_api_error()
self._maybe_flag_no_tool_completion()
# For Scene/multi-turn rollouts, each execute() call records the
# prompt(s) it sent into self._executed_prompts. Use that as the
# authoritative prompt list so n_prompts and prompts.json reflect
Expand Down
3 changes: 3 additions & 0 deletions src/benchflow/rollout/_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,9 @@ def _build_rollout_result(
verifier_error_category=verifier_error_category,
export_error=export_error,
partial_trajectory=partial_trajectory,
no_tool_completion=(
diagnostics.get("no_tool_call_completion_info") is not None
),
trajectory_source=trajectory_source,
evolved_skills=evolved_skills,
source_provenance=source_provenance,
Expand Down
Loading