feat(evalbench): add BigQuery run reader and trace mapping

[sunpan9209]

Summary

• add the optional bigquery_agent_analytics.evalbench module with EvalBenchRun
• read one EvalBench job from configs, results, and scores
• filter every source query in BigQuery with the parameterized job_id
• normalize both current agentic and NL2SQL result shapes
• map scenarios into deterministic, BQAA-compatible synthetic event rows
• document the mapping contract, schema caveats, and remaining implementation phases

Issue status

Tracking issue: #97.

This is the first implementation phase only. Issue #97 should remain open after
this PR merges.

Completed here:

• EvalBenchRun.from_bigquery(...)
• SQL-level job_id filtering
• EvalBench source-schema normalization
• synthetic BQAA event-row mapping
• focused unit tests
• mapping documentation

Still required under #97:

• BQAA-owned mirror-table DDL and writer
• cross-project materialization
• idempotent replacement under WRITE_APPEND
• evalbench_scores_imported materialization
• evalbench-import and evalbench-score CLI commands
• live Client.get_session_trace(...) and LLMAsJudge integration tests
• end-to-end EvalBench example

Why

EvalBench owns benchmark execution and its existing BigQuery output, while BQAA
owns trace reconstruction and semantic evaluation. This reader establishes the
pull-based normalization boundary without coupling EvalBench to the ADK
plugin's production agent_events table.

Keeping the reader and pure row mapping separate from materialization also
allows the mirror-table ownership, idempotency, and write-disposition behavior
to be reviewed independently.

Source compatibility

The mapper supports both major EvalBench result shapes:

Meaning	NL2SQL	Agentic
Scenario ID	id	eval_id
Prompt	nl_prompt	prompt or scenario.starting_prompt
Final output	generated_sql	stdout.response
Tool calls	optional	stdout.tool_calls or accumulated_tools

Nested values may arrive as JSON or Python-literal strings because of
EvalBench's DataFrame reporting path. Both structured encodings are normalized
without interpreting ordinary text as structured data.

Event mapping

Each scenario maps to:

USER_MESSAGE_RECEIVED
  |-- TOOL_STARTING / TOOL_COMPLETED, when tool data exists
  `-- AGENT_COMPLETED, when a usable final response exists

All emitted rows:

• use session_id = trace_id = evalbench:{job_id}:{scenario_id}
• set attributes.experiment_id = job_id
• set attributes.evalbench_scenario_id = scenario_id
• populate content.text_summary for the LLM judge trace-text query
• use deterministic invocation and span IDs
• preserve source error fields
• normalize available token and latency telemetry

Missing tool data emits no tool rows. Missing final output omits
AGENT_COMPLETED. Missing nl_prompt/prompt is a hard error.

Scope boundaries

This PR performs no BigQuery writes and never targets the ADK plugin's
production agent_events table.

run.scores retains source score rows in memory, but writing
evalbench_scores_imported belongs to the materialization phase.

When historical data has no run timestamp, the mapper uses the Unix epoch and
marks attributes.evalbench_run_time_missing = true instead of inventing the
current time.

Validation

• pytest -q tests/test_evalbench_importer.py
  • 12 passed
• complete repository suite on Python 3.13
  • 4082 passed, 73 skipped
• pyink --check
• isort --check-only
• git diff --check

The tests use a stubbed BigQuery client. Live BigQuery materialization and
evaluation validation remain part of the later #97 phases.

[caohy1988]

Code review

Verdict: approve after two fixes. This is a well-built reader: the parameterized job_id filtering with identifier validation is the right injection posture, and I verified the mapping contract against the actual consumers rather than the doc's claims — the _event_row keys match _GET_TRACE_QUERY's 16 columns exactly (client.py:138–158), latency_ms.total_ms / attributes.usage_metadata.total_token_count / attributes.input_tokens+output_tokens line up with SESSION_SUMMARY_QUERY, and populating content.text_summary on every row is exactly what keeps traces visible to LLMAsJudge's ten-character floor. The tests are focused and the doc's "Why These Fields Matter" section is accurate. Findings:

1. returncode: 0 pollutes evalbench_error_fields on every successful run (medium — fix before merge). _source_error_fields includes returncode behind the same _usable_text check as the error strings, and _usable_text(0) → "0" → usable. So every scenario with a successful exit (returncode: 0) gets attributes.evalbench_error_fields = {"returncode": 0} — an "error fields" annotation on every healthy agentic row (repro'd; the current-agentic test passes "returncode": 0 but never asserts the attribute's absence). _source_error_message handles this correctly (it checks int(returncode) != 0), so status stays OK — only the attribute is misleading. Fix: include returncode in error_fields only when it's non-zero or non-numeric, and add the missing-attribute assertion to the success-path test.

2. Duplicate scenario_id rows silently corrupt the trace (medium). EvalBench runs can contain multiple result rows for the same scenario (retries). Two rows with the same id produce identical session_id/trace_id, an identical root span_id, and colliding tool span ids — two USER_MESSAGE_RECEIVED events with the same span id in one trace, which mangles downstream reconstruction. The test is named ..._sorts_unique_scenarios, so uniqueness is assumed but never enforced. Either raise on duplicates (consistent with the hard-error stance on missing prompt/id) or disambiguate the identity with the source index / attempt number.

3. _structured can be killed by one pathological row (low). The ast.literal_eval fallback catches SyntaxError/ValueError/TypeError but not RecursionError (deeply nested literals) or MemoryError — one adversarial or corrupted string cell aborts the entire run mid-mapping. Catching Exception there (returning the raw string) matches the function's degrade-gracefully intent.

4. Non-terminal tool statuses become errors (low). _normalize_tool_calls treats any status outside {completed, ok, success} as an error message — so a "running"/"pending" status (a scenario killed mid-tool-call) marks the tool row ERROR with error_message: "running". Arguably acceptable for benchmark post-mortems, but worth a comment or a narrower rule so an in-progress marker isn't reported as a tool failure.

5. Config determinism caveat (low). The mapping's determinism holds for one loaded run, but _config_values is last-wins and _first_run_time first-wins over BigQuery result order, and _READ_SOURCE_TABLE_QUERY has no ORDER BY — if a job ever carries duplicate config keys, agent identity could differ between two imports of the same job. Cheap insurance: ORDER BY in the configs query or first-wins in _config_values.

Also noted approvingly: the module wires in via direct submodule import instead of touching __init__.py (avoiding the export-block merge-conflict zone that #431/#432 are both in), the epoch-sentinel + evalbench_run_time_missing flag is the right call versus inventing timestamps, and attaching source errors to the lone USER_MESSAGE_RECEIVED row when no terminal row exists is a reasonable, documented trade-off. CI is green across Python 3.10–3.14.

[caohy1988]

Full review - Ready with fixes

I reviewed the complete 1,193-line diff, linked issue #97, all importer tests, BQAA trace/evaluator consumers, and the current EvalBench producer and BigQuery reporter shapes at head ce2cd48. The new test module passes locally (12 passed), and current CI is green. I found three mapping defects to fix:

1. P2 - Failed tools are invisible to error_rate. evalbench.py:215-259 sets status="ERROR" for a failed tool but always emits event_type="TOOL_COMPLETED". SESSION_SUMMARY_QUERY counts tool failures only as TOOL_ERROR, so a session whose imported tool failed reports zero tool errors. Emit TOOL_ERROR when tool_error is present and add a regression tied to the session-summary semantics.

2. P2 - Duplicate scenario IDs collide. evalbench.py:150-170 keeps every result but derives session_id, invocation_id, and root span ID only from job_id plus scenario_id. EvalBench append storage does not enforce scenario-ID uniqueness, so duplicate rows collapse into one trace with identical IDs and timestamps. Reject duplicates with a clear ValueError, or add and document a deterministic per-result discriminator; test the chosen contract.

3. P2 - Multi-model latency is multiplied. evalbench.py:642-658 sums every model's totalLatencyMs. Current EvalBench Claude normalization copies the same run-level wall-clock duration_ms into each modelUsage entry, so a two-model run is reported at twice its actual latency. Use the maximum/select the run value once while continuing to sum token counters, and add a two-model fixture.

Direct reproductions confirmed the failed-tool event mismatch and duplicate trace IDs. I also compared latency handling to the current upstream EvalBench normalizers, not only the PR fixtures.

[caohy1988]

Review: Approve with nits

Verified locally: checked out the PR head, ran the new tests (12/12 pass), pyink/isort/git diff --check clean; the 2 full-suite failures also fail on main (pre-existing). I also verified the assumed EvalBench source schema against the actual evalbench repo and the emitted 16-column row contract against _GET_TRACE_QUERY (client.py:138-159), TraceFilter.to_sql_conditions (trace.py:758-768), SESSION_SUMMARY_QUERY (evaluators.py:923), and the judge-text query (evaluators.py:1079-1102) — all match. Well-scoped, defensive, deterministic, injection-safe. Findings are all minor polish for a read-only first phase.

Findings

MINOR — missing source table aborts the whole load with a raw NotFound (src/bigquery_agent_analytics/evalbench.py:116-133)
from_bigquery unconditionally queries configs, results, and scores. EvalBench's bqstore.py only writes all three when reporters are configured and results exist; a dataset with results but no scores (scoring disabled) or a legacy dataset without configs raises google.api_core.exceptions.NotFound with no context. Suggest catching NotFound for scores/configs and defaulting to () (results rightly mandatory), or wrapping errors with table/job context.

MINOR — failed tools emit TOOL_COMPLETED with status="ERROR" instead of TOOL_ERROR (evalbench.py:221-261)
TOOL_ERROR is a first-class event type in this SDK (trace.py:97), is what SESSION_SUMMARY_QUERY counts as tool_errors (evaluators.py:928), and what trace_evaluator.py:211 matches. Imported runs will always report tool_errors = 0 even when tools failed (has_error still trips via status, so not invisible — just undercounted in the dedicated metric). Consider emitting TOOL_ERROR when tool_error is set.

MINOR — total_tokens under-reports for imported runs (evalbench.py:685-698 + evaluators.py:958-969)
The mapper puts usage_metadata.prompt_token_count on the terminal row; SESSION_SUMMARY_QUERY's total_tokens is COALESCE(prompt_token_count, content.usage.total, input+output) — so a run with input=120/output=30 reports total_tokens = 120, not the 150 the mapper computed. Partly a pre-existing query quirk, but the mapper could avoid shadowing or document it in docs/evalbench.md.

MINOR — duplicate scenario IDs silently merge (evalbench.py:150-158)
Two result rows with the same id/eval_id produce identical session_id/invocation_id/span_ids; after materialization they'd merge into one corrupted trace. Dedupe with a warning, suffix, or raise. Untested.

MINOR — one malformed scenario aborts the entire run (evalbench.py:159-163)
A single row missing nl_prompt/prompt raises ValueError, killing conversion of all other valid scenarios. Documented as intentional, but per-scenario error collection (e.g. run.mapping_errors) would be more useful for a batch importer. Worth revisiting before the CLI phase.

MINOR — no signal for a nonexistent job_id
A typo'd job id returns an empty run and to_agent_event_rows() returns [] silently — looks like a successful empty import. Consider a warning or strict flag. Untested.

NIT — evalbench_error_fields populated on successful runs (evalbench.py:577-591)
returncode: 0 passes _usable_text, so every successful agentic row carries evalbench_error_fields = {"returncode": 0} — benign data in an "error" field. Filter zero returncodes as _source_error_message already does (:609-619).

NIT — _first_int drops float-string telemetry (evalbench.py:706-717)
int("850.5") raises, so token/latency values serialized as float strings (which EvalBench's float-to-string workaround can produce) are silently lost. int(float(value)) would be more forgiving.

NIT — _MISSING_TEXT rejects legitimate content (evalbench.py:43)
A prompt literally equal to "null"/"none" is treated as missing. Extremely unlikely; noting for completeness.

Test coverage gaps — the 12 tests are good, but for an 840-line module: empty results, duplicate scenario IDs, malformed/non-JSON stdout, per-row run_time precedence, and the missing-table error path are all untested.

Positives

• SQL injection properly handled: project_id/evalbench_dataset validated against ^[A-Za-z0-9_-]+$ before interpolation, job_id always a query parameter — both explicitly tested, including a DROP TABLE payload.
• Cost behavior as advertised: all three queries filter WHERE job_id = @job_id in SQL (asserted in tests).
• Epoch-fallback for missing run_time with an explicit flag instead of inventing now() — the right call.
• docs/evalbench.md is unusually accurate; every cited line range spot-checked correct against both this repo and the evalbench source.
• Deterministic sha256-based invocation/span IDs and stable scenario sorting make re-imports reproducible — good groundwork for the idempotency phase.

Unverified claims

• "4082 passed, 73 skipped" full suite — I got 4172 passed / 54 skipped / 2 pre-existing failures (environment drift); consistent with a clean run but exact numbers not reproduced.
• "normalizes both current agentic and NL2SQL result shapes" — verified via tests/code paths, but no live test against real EvalBench BigQuery output (acknowledged in the PR as deferred to a later #97 phase).

Review by @caohy1988's assistant (Kimi Code CLI).

[sunpan9209]

Addressed the confirmed mapping defects in b4b0a06:

• successful returncode: 0 values no longer populate evalbench_error_fields
• duplicate scenario IDs now raise a clear ValueError before colliding trace and span IDs can be emitted
• failed tools emit TOOL_ERROR, so SESSION_SUMMARY_QUERY.tool_errors counts them
• token counts remain additive across model entries, while elapsed latency uses the maximum reported model duration instead of multiplying a run-level duration repeated for each model
• the mapping guide now documents the duplicate-ID, failed-tool, and multi-model latency behavior

Regression coverage now includes the successful-return-code path, failed-tool event semantics, duplicate scenario IDs, and a two-model latency/token fixture.

I kept the documented hard failure for a missing prompt, as required by #97. I also did not broaden _structured to catch every Exception: swallowing MemoryError would hide process-level resource exhaustion rather than degrade safely. Missing optional-table handling, empty-job signaling, conflicting duplicate config rows, and partial per-scenario error collection remain better decisions for the later materialization/CLI phase, where their user-facing behavior can be defined consistently.

The reported total_tokens undercount does not apply to these imported rows: whenever input/output telemetry is available, the mapper also writes attributes.usage_metadata.total_token_count, which is the first path read by SESSION_SUMMARY_QUERY.

Validation:

• pytest -q tests/test_evalbench_importer.py: 14 passed
• full Python 3.13 suite: 4084 passed, 73 skipped
• pyink --check
• isort --check-only
• git diff --check

[sunpan9209]

CI note: the current Format check failure is an upstream formatter-version regression, not a change in this PR. It reproduces on current upstream/main (71b0bbc) when the unbounded workflow installs pyink 26.x. The Python 3.10-3.14 matrix, browser smoke, security scan, and CLA checks pass.

Upstream PR #447 pins pyink <26 specifically to restore this check, and its format job is green. I have not added unrelated repository-wide formatter churn to this branch; the format check should be refreshed after #447 lands.

[caohy1988]

LGTM - approved at exact head b4b0a06.

I rechecked the full importer diff and the latest fixes. Successful return codes no longer emit an error attribute, duplicate scenario IDs are rejected, tool failures emit TOOL_ERROR, scenario latency uses the maximum relevant duration, and token totals are summed. The focused exact-head importer suite passes (14 tests), with regression coverage for the corrected behavior.

The remaining red Format check is the repository-wide pyink 26 workflow regression fixed by merged PR #447. Please refresh from main and rerun required CI before merge; this approval is not a CI waiver.