Skip to content

fix(report): expose per-case execution metrics - #191

Merged
zpzjzj merged 2 commits into
alibaba:mainfrom
JHWang-1997:codex/fix-report-case-metrics
Aug 17, 2026
Merged

fix(report): expose per-case execution metrics#191
zpzjzj merged 2 commits into
alibaba:mainfrom
JHWang-1997:codex/fix-report-case-metrics

Conversation

@JHWang-1997

Copy link
Copy Markdown
Collaborator

Summary

  • distinguish evaluation wall time from per-case tested-agent execution time
  • expose per-case input, output, and total tested-agent token usage in JSON, HTML, and Markdown reports
  • capture agent-judge duration and token usage separately and add tested-agent, judge, and overall token totals
  • keep benchmark metrics compact beside each case heading, including with-Skill, without-Skill, and delta annotations
  • preserve the existing total_tokens JSON field for compatibility

Root cause

The top-level report duration comes from EndTime - StartTime, while each case duration comes from the tested-agent session. The HTML report labeled both scopes ambiguously and omitted the per-case token fields that were already present in CaseResult.

Agent-judge sessions also exposed duration and token usage internally, but those metrics were not copied into the report model before JudgeSession was excluded from JSON serialization.

This change is engine-independent. Agent-specific token acquisition, including QoderCLI session parsing, is intentionally out of scope.

User impact

Reports now make Skill execution cost visible without allowing metrics to dominate the response and grading content:

  • the header labels total duration as Evaluation wall time
  • compact case-heading annotations show with-Skill, without-Skill, and delta metrics
  • input/output token details remain available on hover
  • agent-judge cost is reported separately
  • Markdown and JSON reports use the same metric semantics

Validation

  • make fmt
  • make verify
  • go test -race ./...
  • full local benchmark evaluation of expression-calculator using QoderCLI:
    • 2 cases × with/without Skill
    • 3 PASS, 1 baseline FAIL, 0 ERROR
    • evaluation wall time matched Agent + Judge execution time with approximately 0.2s framework overhead
  • generated HTML embedded JavaScript syntax and compact-layout assertions

Closes #190

@JHWang-1997
JHWang-1997 marked this pull request as ready for review August 14, 2026 07:08
@JHWang-1997
JHWang-1997 requested a review from hittyt as a code owner August 14, 2026 07:08
@zpzjzj
zpzjzj self-requested a review August 14, 2026 07:40
Comment thread internal/runner/runner.go Outdated

@zpzjzj zpzjzj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Code review summary

Review scope: codex/fix-report-case-metricsmain at 9a18eb6f

Findings

  • Blocker: 0
  • Major: 1
  • Info: 1

Compatibility assessment

The machine-readable result.json contract remains backward-compatible for normal JSON consumers:

  • Existing fields such as total_tokens, duration_ms, input_tokens, and output_tokens are retained.
  • total_tokens preserves its previous meaning: tested-agent tokens only.
  • Judge and overall metrics are additive fields.
  • Regenerating reports from the existing legacy fixture succeeds and preserves total_tokens=12500, while deriving overall_tokens=12500.

There is still a compatibility consideration for consumers that parse derived presentation formats. The HTML embedded DATA object renames duration / total_tokens and per-case duration_ms / duration, while the Markdown table schema also changes. No repository or publicly searchable downstream consumer was found, so this is informational rather than blocking. If these representations are consumed by internal automation, consider retaining the old HTML aliases for one release or explicitly documenting that only result.json is a stable machine-readable contract. Strict JSON schemas using additionalProperties: false should also be checked before rollout because the new additive fields would require a schema update.

Overall assessment

⚠️ Conditional merge. The existing JSON fields are preserved, but the failed agent-judge path should retain its available session metrics before the new judge and overall totals are treated as reliable business cost data. The inline Major finding contains the concrete data-flow evidence and suggested regression coverage.

Validation reviewed: all GitHub checks are green; make test and targeted race-enabled report, runner, and CLI tests pass.

@zpzjzj zpzjzj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@zpzjzj
zpzjzj enabled auto-merge August 17, 2026 04:03
@zpzjzj
zpzjzj added this pull request to the merge queue Aug 17, 2026
Merged via the queue into alibaba:main with commit 38ccc9e Aug 17, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(report): distinguish evaluation wall time and expose per-case token usage

2 participants