feat(trajectories): canonical trace IR with ACP and ATIF converters - #984
feat(trajectories): canonical trace IR with ACP and ATIF converters#984Galius5136 wants to merge 13 commits into
Conversation
`trajectory/acp_trajectory.jsonl` is produced by the ACP-session capture path and read by the viewer, the judges, the skill evaluator, task verifiers and the review skill — but it had no schema, no version and no written specification. Every consumer hand-rolls its own parser. This writes the format down and makes it machine-checkable, with zero runtime behaviour change: - `src/benchflow/trajectories/schemas/acp-capture-event-v1.schema.json` — Draft 2020-12 schema for one line of the artifact, scoped deliberately to the ACP-session capture events (`_events_to_trajectory` plus the `ACPSession` legacy fallback). `kind` stays an open string because `_canonical_tool_kind` passes agent-supplied values through unchanged; `status` is a closed enum because the serialized value is always `ToolCallStatus(...).value`. `additionalProperties: false` on every event variant, so a field the emitter does not produce today fails the suite. - `docs/trace-interop.md` — the format as emitted, per-event required and optional fields, producers and consumers, the current state of ATIF and OpenTelemetry, and a table of the information losses observable in the existing ACP -> ATIF / ADP conversions. Claims are marked FACT or PROPOSAL; nothing marked PROPOSAL is agreed or implemented. - `tests/trajectories/test_acp_capture_event_schema.py` — conformance suite over two corpora: records generated by driving real `ACPSession` objects through the production capture path and writer, and the ACP event lists already used as exporter inputs by the ATIF and ADP tests. Ten cases pin what the schema must reject. The event-type vocabulary is read out of `_events_to_trajectory` by AST rather than hardcoded, so adding a branch to that function fails the suite until the schema documents it. - `jsonschema` declared as a dev dependency. It was already resolved as a litellm transitive; declaring it keeps the test suite off another package's dependency tree. `uv.lock` gains the two corresponding lines. Scope is narrower than the artifact and the schema says so: oracle-mode rollouts replace the trajectory with an oracle-only list, and session-factory Sessions bypass the ACP-session emitter entirely via `_snapshot_session_trajectory`. Neither is modelled here. No artifact-level contract is defined or implied. No runtime module imports the schema. Nothing under `src/benchflow` changes behaviour. Verified: 25 new tests; tests/trajectories 239 passed; ACP regression (capture, streaming, acp, native usage) 147 passed; ruff check/format clean; `uv lock --check` and `uv sync --locked --extra dev` both pass.
Slice A completeFormalizes the ACP-session capture-event subset written to What landed
Design choices worth flagging for review
Human verification (run by @Galius5136, not by CI)
What this surfaced Preparing H2 exposed a real boundary that the first draft of the documentation So the same artifact path can hold different record families depending on how Not included, deliberately No canonical IR, no OTel work, no ATIF reader, and no capture-layer enrichment. |
Picks up d30527b (fix(acp): retain live subprocess stderr, benchflow-ai#980). No conflicts: that commit touches acp/container_transport.py, sandbox/process/* and their tests, none of which Slice A or the trajectories export path touches.
test_export_atif.py pins the shape ATIF export produces. This pins which
information survives the conversion and which does not, so a future change to
the converter either keeps the property or fails a test that says in one line
what changed.
Adds tests/trajectories/test_atif_preservation.py: 10 preservation invariants,
12 loss characterizations, 2 producer-boundary tests. No runtime module and no
public format is touched, and no dependency is added.
Two choices make the suite falsifiable rather than decorative. Losses are
asserted with sentinel values absent from the whole serialized document, not
with structural checks on the one field a test happened to look at. And both
sides of the oracle-source divergence are read from source via AST -- the
converter's event-type branches and the validator's accepted `source` set -- so
changing either fails a test instead of silently invalidating the premise.
The load-bearing producer-boundary test drives a real ACPSession with a
tool_call update carrying rawInput, rawOutput, locations and _meta and shows
that those fields are absent before the ACP -> ATIF converter runs. Real
rollout evidence independently shows non-empty tool arguments in provider
capture for calls whose ATIF arguments are {}. Together, these observations
place the loss upstream of the ATIF converter rather than inside it.
docs/trace-interop.md gains a per-field table (5.1), one previously unrecorded
loss -- the agent_thought join is irreversible, so one thought containing a
blank line is indistinguishable from two events -- and a section recording what
two real gemini rollouts showed (5.2), including that the proxy capture holds
the tool arguments the ATIF document records as {}.
Slice A2 — ACP → ATIF preservation/loss characterizationTurns the ATIF rows of the §5 loss table into executable assertions. 24 tests, one new file, no runtime change, no public format change, no new dependency — same profile as Slice A. Deliberately not Slice B. Slice B in the roadmap above is the canonical-IR prototype, and it is gated on open question 1. This work is a sibling of A: it holds whichever way that question is answered, and it gives a baseline of what the existing converter preserves before anything is redesigned.
Both sides of the One previously unrecorded lossDocumented as #10: What real rollouts showed§5.2 records two
The observation worth calling out: in those rollouts the proxy capture ( This is not evidence that the ACP Also observed and now documented: an ATIF document opens with two identical Not exercised by a real rolloutStill resting on code reading plus the synthetic tests, and labelled as such in the document: the non-text content-block loss (#5), because neither agent emitted a file-edit or terminal block; and the Still a draft — Slices B–F remain a proposal pending the open questions above. |
Four trace-shaped representations already exist here — the ACP-session capture
events, ATIF, ADP and the Verifiers/ORS record — and all three exporters walk
the same ACP event list from the same call site. The cost of that is not the
edge count; it is that each edge answers the same questions privately, and the
answers already diverge: for one `tool_call` event ATIF emits `"arguments": {}`
while ADP emits `"kwargs": {}`, ATIF keeps the tool status in a non-standard
`extra` while ADP drops it, both join thought boundaries irreversibly, and
neither represents `agent_timeout` at all.
This adds a canonical hub so each format becomes one edge against a written
contract, and so the information loss is a typed value rather than a comment in
a module docstring.
The direction is not approved. The four open questions in docs/trace-interop.md
§6 have had no maintainer answer, so this takes a provisional position on the
first one and implements it in isolation, where it can be reviewed as code and
reverted by deleting two files.
- `src/benchflow/trajectories/ir.py` — the IR types, the loss model
(`LossReport` / `LossRecord` over an unsupported/dropped/normalized/
synthesized taxonomy, the same one §5.1 already uses), and `validate_trace`,
which returns one string per invariant violation rather than raising.
The rule the module is built on is that the IR is a pragmatic superset of
what BenchFlow can observe, not a model of what an agent trace could
contain. Three consequences are load-bearing: optional values are tri-state
(a value / `None` "this source never carried it" / an empty value "carried
and empty"); a `None` that is not covered by a loss record makes the trace
invalid, so absence is declared and never silent; and normalization is
non-destructive — `source_type` keeps the source's own type string next to
the normalized `kind`, and `name_semantics` records that an ACP `kind` is a
category rather than a function name.
The IR deliberately cannot fabricate an agent version, a synthetic tool-call
id, a timestamp, or an OTel span id. Those are target-side obligations and
belong in converters, which record them as SYNTHESIZED.
- `tests/trajectories/test_trace_ir.py` — 25 tests. Each invariant is exercised
with a violating trace and a clean one, so a rule that stopped firing fails
here instead of passing silently. The IR's tool-status vocabulary is checked
against the ACP `ToolCallStatus` enum and its event kinds against the
vocabulary `_events_to_trajectory` actually emits, read from source by AST —
the mechanism the Slice A conformance suite already uses, so adding a branch
to the capture path fails this suite until the IR accounts for it.
Two tests pin the isolation claim rather than the design: no module under
`src/benchflow` imports the IR, and the IR imports no benchflow module. The
first is the executable form of "zero runtime behaviour change"; wiring the
IR into a run path has to update it deliberately.
- `docs/trace-interop.md` §8 — replaces the "ideas not yet agreed" placeholder
with the design: why a hub, the four alternatives considered and why each was
not taken, the field classes (supported today / optional / needs enrichment /
must not be invented), the planned ACP→IR and IR→ATIF mappings, an OTel
sketch marked unverified because no version of the GenAI conventions is
vendored here, the invariants, and an explicit list of what a review can
still reject. §6 gains a note that question 1 now has a provisional, unagreed
position and that questions 2-5 are untouched.
The worked example in §8.4 is generated from the models and compared against
the document by a test, so it cannot drift.
Not included, deliberately: `ACP → IR`, `ATIF ↔ IR`, any OTel work, any wiring
into a run path, any on-disk artifact. The representation is meant to be
reviewed before anything depends on it.
No existing format, exporter, artifact or code path changes. No new dependency
— pydantic is already a runtime dependency and `uv.lock` is untouched.
Verified: 25 new tests; tests/trajectories 288 passed; the trajectory, capture,
streaming and ACP regression lane 453 passed; `ruff check .` and
`ruff format --check` clean; `ty check src/` clean.
Slice B — provisional canonical Trace IRThe four open questions in the PR description are still unanswered. Rather than Feedback on the architecture is still very welcome, and it is still cheap to Why a hub rather than direct convertersFour trace-shaped representations already exist here — the ACP-session capture
Three independent decisions about the same event, taken three times, recorded The alternatives I considered and did not take — promoting ATIF or the ACP What is in the slice
The rule the module is built on: the IR is a pragmatic superset of what
The IR deliberately cannot fabricate an agent version, a synthetic Two tests pin the isolation claim rather than the design: nothing under The event-kind and tool-status vocabularies are checked against the real Deliberately not included
What a review can still changeEverything, and cheaply. §8.6 lists it explicitly — the hub itself, the What a review cannot change by rejecting the IR: the losses in §5 are properties Verification25 new tests; |
…nwired Slice B proposed a hub and a contract: every `None` in the IR is covered by a `LossRecord`, and a conversion's cost is a typed value rather than a comment in a docstring. This is the first real edge, and its job is as much to stress that contract as to convert. `acp_events_to_ir` reads the event list of `trajectory/acp_trajectory.jsonl` — the ACP-session capture vocabulary Slice A pinned, plus the oracle and unknown record families that share the file — and returns one `CanonicalTrace` carrying its own report. It reads no other artifact: `result.json`, `timing.json` and the proxy capture are separate, so a value that lives only there is a declared loss rather than a silent enrichment. What the conversion keeps that the existing exporters lose: - `agent_timeout` becomes an event with its fields in `extensions`, and sets the trace outcome. Every exporter drops it today (§5 loss benchflow-ai#4). - Non-text content blocks are carried as `opaque` with the block verbatim, instead of being skipped by `content_blocks_to_text` (loss benchflow-ai#5). - Thought boundaries survive: one capture record is one reasoning segment, and nothing is joined, so the ambiguity `ThoughtBuffer` creates never arises (loss benchflow-ai#10). A thought whose own text contains a blank line stays one segment, because splitting it would invent a boundary the source does not have. - `""` and absent stay distinguishable everywhere; text-empty events are kept rather than dropped. - The `oracle` record keeps its own kind and role instead of becoming an agent step prefixed `[oracle: …]` that a consumer can only undo by string matching. - An unrecognized `type` becomes `unknown` with the record carried verbatim, instead of being skipped silently. What it refuses to invent: arguments, timestamps, tool-call ids, agent version. It also does not prepend the `prompts` argument as leading user events, which both existing exporters do — §5.2 showed the cost, an ATIF document that opens with two identical `user` steps so user turns over-count by one. Those steps are not ACP events; a target that wants them adds them at its own edge as SYNTHESIZED. Loss addressing distinguishes three shapes. `events[i].…` is a field of one IR event, and is what `validate_trace` matches, so the per-call `arguments` records use it. `source[i]` is an input entry that produced no IR event, which cannot be addressed as `events[i]` because that index belongs to a different event once an entry is skipped. Systemic losses — timestamps, per-event usage, agent version, stop reason — are declared once each under an unindexed `events[].…` path. That last choice is what makes the contract affordable, and it is measured rather than asserted: the report is `n_tool_calls + 5` records and does not grow with trace length. On the two real gemini rollouts of §5.2 it is 7 records (H1, 2 tool calls) and 6 (H2, 1 tool call and a real wall-clock timeout). - `tests/trajectories/test_ir_from_acp.py` — 35 tests. Preservation is checked against events produced by driving a real `ACPSession` through the production capture path, reusing the Slice A2 fixture rather than hand-written dicts that would only prove the converter agrees with itself. The report is asserted as a complete set, so an undeclared loss and a spurious one both fail. One test removes a declared loss from a converted trace and shows the trace becomes invalid — the Slice B contract, demonstrated end to end. Two tests pin the volume property, including that doubling the non-tool events does not change the report at all. The block classifier is pinned against `content_blocks_to_text` so the IR and every existing consumer cannot disagree about what counts as text output. - `tests/trajectories/test_trace_ir.py` — the isolation test is restated at the boundary that now matters. Slice B asserted that nothing under `src/benchflow` imports the IR; the converter necessarily does, so `ir.py` and `ir_from_acp.py` are declared a closed family and the test asserts nothing outside it imports either. The guarantee is unchanged in substance and the test is strictly stronger: a new converter has to join the family explicitly, and a stale name in that list now fails a test of its own. - `docs/trace-interop.md` — §8.3 gains the implemented mapping table with a class per row and the measured loss counts; §8.7 records what is now implemented and restates the isolation property; §8.8 records what writing the first converter showed about the declared-absence rule, including the part of it most likely to be revised in review. No runtime module imports either module, no capture path or exporter changes, no on-disk format changes, and no new dependency. Verified: 35 new tests; tests/trajectories 324 passed; the trajectory, capture, streaming and ACP regression lane 489 passed; `ruff check .`, `ruff format --check` and `ty check src/` clean. Two real rollouts converted and cross-checked against their source files by hand.
Found by reading a converted real rollout by hand, not by the test suite, which
was green: §8.4 published its worked example with `exclude_none=True`, so
`arguments` was absent from the document while the loss report kept a record
addressing `events[1].tool_call.arguments`. The declaration that legalizes the
absence pointed at a key no reader of that document could find.
The suite could not see it because both sides of the comparison used the same
non-canonical encoding, so it was self-consistent.
`None` in this IR is a positive statement — the source did not carry this field
— and every one of them is paired with a `LossRecord` that addresses the field
by path. Dropping the key makes the address dangle and collapses "we looked and
it was not there" into "this version has no such field". Both encodings
re-validate to an equal pydantic model, so the model layer cannot enforce this;
the rule is stated in the `ir.py` docstring and enforced by tests.
- `ir.py` — declares the canonical encoding: nulls retained,
`exclude_none=True` is not a valid encoding of a Trace IR document. No
serializer is added: there is no on-disk artifact yet, and a writer would
anticipate an interface this proposal has not earned. Adds the corollary that
a record names the outermost absent node, so a conversion with no usage
declares `usage`, not `usage.input_tokens`.
- `CanonicalTrace.outcome` is no longer optional. Applying the new guard
immediately found a second instance of the same class: `outcome.stop_reason`
is a loss every ACP conversion declares, and it could not resolve in any trace
that did not time out, because the section itself was null. It is now always
present with `None` fields, like `agent` — which is exactly why
`agent.agent_version` resolved and `outcome.stop_reason` did not.
- `docs/trace-interop.md` — §8.2 gains the encoding rule as a fourth design
choice, §8.4 is regenerated in the canonical encoding so `arguments: null` and
its loss record are visible in the same document, §8.5 records the new
test-pinned property, and §8.8 records what the human end-to-end pass showed,
including the limit below.
- Tests — `test_every_concrete_loss_path_resolves_in_the_canonical_encoding`
(IR) and `test_the_canonical_document_shows_null_arguments_beside_their_loss_record`
plus `test_every_concrete_loss_path_of_a_converted_trace_resolves` (converter).
Each asserts in the same test that the discarded encoding *fails* to resolve
those paths, so none of them can pass for both encodings at once.
Known limit, recorded rather than fixed: the invariant forces a converter to
declare an absence, it cannot stop one from writing `arguments: {}` instead of
`null`. Such a trace is valid. Closing that would mean the IR taking a position
on what an empty map means for each source, which the tri-state rule
deliberately leaves to the converter.
Verified: tests/trajectories 328 passed; the trajectory, capture, streaming and
ACP regression lane 493 passed; `ruff check .`, `ruff format --check` and
`ty check src/` clean. Human end-to-end H1-H4 run against real captured
rollouts, all PASS.
Slice C —
|
Preparation for the first outbound converter, and a gap the inbound one had already papered over. `LossRecord.field` was documented as a path in IR terms, but not every record is about an IR node. Slice C already needed an escape and invented a `source[i]` string convention in a module docstring, recognized by `startswith` in two guards. The ATIF edge makes the same gap unavoidable in the other direction: a prompt-derived step, a `message: ""` the format requires, a `total_steps` the document computes about itself — none of these has an IR antecedent, and giving them an invented IR path would produce an address that does not resolve. Adds `PathSpace` — `hub` · `source` · `target` — and `LossRecord.space`, defaulting to `hub` so every record written before this field existed keeps its meaning. `field` is the path *inside* its space and carries no prefix repeating it: the space is a property of the record, never inferred from the string. Two records may legitimately hold the identical path in different spaces, which is exactly the `acp -> ir` case where a skipped input entry and the IR event that inherited its index are both `events[3]`. Three spaces cover every direction and a new format adds none, because every edge has the IR on exactly one side and therefore exactly one non-hub space: an inbound edge can talk about its source, an outbound one about its target. Only `hub` records compose across edges — the IR is the output of an inbound conversion and the input of an outbound one, so `events[1].tool_call.arguments` denotes the same field in both reports and the records join on it. `source` and `target` records are terminal by construction. No unified report is introduced; composition happens at read time over `(direction, field, space)`. The same asymmetry settles which side owns a report, now stated explicitly in the module docstring: a trace is built exactly once so an inbound conversion may attach its report to it, while a trace may be converted to many targets so an outbound conversion returns its report and leaves `trace.losses` untouched. - `validate_trace` invariant 7 now requires a *hub* record. A `target` record holding the same path addresses another document and declares nothing about this trace; previously the string alone would have satisfied it. - Both canonical-path guards filter on `space is HUB` instead of testing string prefixes. `_is_per_event` classifies by space and takes a record rather than a string. - `LossReport` gains `by_space`, and `for_field` takes the space — the space is part of the address, so it is not defaulted away silently. - `ir_from_acp` migrates its one `source[i]` record to `space=SOURCE, field=events[i]`: the path is now the one the input actually uses, since the space no longer has to be spelled into the string. Five new tests pin the contract, including that no space is inferred from the string in either direction: a `source`/`target` record with a hub-shaped path does not satisfy the declared-absence invariant, and a hub record with a source-shaped path does. `docs/trace-interop.md` §8.2 gains the spaces and the report-ownership rule as a fifth design choice; §8.4 is regenerated so the example carries the new field. Additive: no existing path changes meaning, no runtime module imports either IR module, and no format, exporter or artifact changes. Verified: tests/trajectories 334 passed; the wider trajectory/capture/ACP lane 466 passed; `ruff check .`, `ruff format --check` and `ty check src/` clean.
The first outbound edge, and the first converter that has to fabricate. Where
`ACP -> IR` stress-tested whether every absence could be declared, this one
stress-tests the other half of the taxonomy: ATIF requires values the IR does
not carry, so `SYNTHESIZED` stops being a decorative enum member.
`export_atif.py` is untouched and remains the only writer of
`trainer/atif.json`. Nothing imports the new module.
The claim it is built to support, and the reason the slice is worth anything:
ir_to_atif(acp_events_to_ir(events), prompts=P)
==
trajectory_to_atif_record(events=events, prompts=P)
Parity with the existing direct exporter, on the same inputs, for the document.
A hub that lost anything the direct path preserved would fail that equality.
Asserted over events driven through the production capture path plus nine
further shapes, and confirmed against the two real gemini rollouts of §5.2: for
both, the document produced through the hub is identical to the
`trainer/atif.json` those rollouts actually wrote, back when they ran.
Parity is about the document, not the report — the direct exporter produces no
report, which is the difference this whole proposal is about.
**One deliberate deviation, enumerated by a test rather than left in a diff.**
`acp_events_to_atif_steps` renders an oracle record as a `source: "agent"` step
prefixed `[oracle: …]`, recoverable only by string matching; §5.1 records this
as a live divergence, since the in-repo validator already accepts
`source: "oracle"` while no emitter produces one. The IR carries the role, so
this edge emits `source: "oracle"` with the command as the message. A test runs
both paths over a trajectory holding every capture event type plus an oracle
record and asserts that step is the only one that differs.
Seven fabrications, each recorded where it is invented: `agent.version` and
`agent.name`, a tool-call id, a `function_name`, `arguments`, `steps[].message`,
and `final_metrics.total_steps` — plus the prompt-derived user steps. The first
five are hub-space records at the IR field whose absence forced them; the last
three have no IR antecedent and are target-space.
`arguments` keeps its `{}`. It is what ATIF requires and what the direct
exporter writes, and departing from it would trade a real compatibility property
for a cosmetic one. What changes is that it is no longer silent: the fabrication
is declared at the same hub path the ACP edge declared `UNSUPPORTED`, so reading
the two reports together says the source never had arguments and the target
demanded them anyway. A tool call carrying real arguments — including a
genuinely captured `{}` — passes through and is declared nothing.
Losses are declared only for what this edge actually loses given the trace in
hand. An ACP-derived trace has no per-event timestamps or usage, and the inbound
report already declared those as `UNSUPPORTED`; repeating them here as `DROPPED`
would double-count one fact and misdescribe an edge that loses nothing it was
given.
- `src/benchflow/trajectories/ir_to_atif.py` — returns `(document, report)`
rather than attaching, because a trace may be converted to many targets and
none of those conversions describes how the trace was built. It imports one
benchflow module, the IR: `ATIF_SCHEMA_VERSION` is redefined and pinned equal
to `export_atif`'s by a test, and the `ThoughtBuffer` join is reimplemented and
pinned the same way, so the hub does not depend on export plumbing.
- `tests/trajectories/test_ir_to_atif.py` — 35 tests, organized around parity.
Also pins that an outbound conversion leaves the input trace byte-identical,
that two conversions of one trace are independent, that every hub record of
the outbound report resolves in the canonical encoding while target records do
not, and that a hand-built trace converts without going through ACP.
- `test_only_the_ir_family_imports_the_ir` gains `ir_to_atif`. That test failing
first was the mechanism working: a new converter joins the family explicitly.
- `docs/trace-interop.md` — §8.3 gains the implemented mapping with a class per
row and the measured counts, §8.7 the status, §8.9 what the first outbound
converter settled.
Measured on the real rollouts: H1 produces 12 outbound records (6 synthesized,
4 dropped, 2 normalized; 9 hub, 3 target), H2 produces 14, the extra drops being
the timeout event, its extensions and the trace outcome.
Known gap, recorded not fixed: `TraceUsage` has no cost field, so
`final_metrics.total_cost_usd` cannot be produced through the hub. Neither real
rollout carries one, so parity is unaffected there; closing it means adding a
field to the IR.
Verified: 35 new tests; tests/trajectories 369 passed; the trajectory, capture,
streaming and ACP regression lane 534 passed; `ruff check .`,
`ruff format --check` and `ty check src/` clean.
…ound loss Two holes in Slice D's central claim, found by auditing it rather than by a test. Both are about the same thing: the outbound edge was describing a trace shaped like the one ACP happens to produce, not the trace it actually received. ## Cost `trajectory_to_atif_record` accepts `total_cost_usd` and writes it to `final_metrics`. The IR had no cost field, so a document produced through the hub could not carry it — the parity claim held only for traces without one. `TraceUsage` gains `cost_usd` and `price_source`, mapped to `final_metrics.total_cost_usd` (preserved) and to a declared `DROPPED` (ATIF has no slot). Cost sits with the token counters because that is where every BenchFlow object already puts it — `agent_result`, `TaskTelemetry` and ATIF's own `final_metrics` — and because it is derived from them. `price_source` is to `cost_usd` what `source` is to the counters: BenchFlow computes no prices of its own, it imports a number from the model gateway's log, so a cost without the table that produced it is not comparable. The writing path is real: `providers/litellm_logging.py:618-623` sums the callback log's per-entry `cost` into `Trajectory.metadata["cost_usd"]`, it surfaces as `AgentResult.cost_usd`, and `rollout/_results.py:448` hands it to the ATIF writer. Stated precisely, because the audit turned up a nuance worth recording: **no rollout artifact available here has ever carried a non-null cost** — including four whose `usage_source` is `provider_response`, which went through the proxy but whose gateway log carried no per-entry cost. So the field's production is established by reading the code, not by observation, and parity with a cost is asserted on synthetic input given to both paths. Per-call cost stays out of scope: it exists in the proxy capture, and modelling it is a larger question this proposal does not open. ## Conditional outbound losses The rule was right and the coverage was not. Probing with a trace that really carried the values showed six kinds of information the IR can hold, ATIF cannot represent, and the report did not mention: trace `trace_id` / `started_at` / `finished_at` / `provenance` / `extensions`, `agent.provider`, per-event `outcome`, tool-call timestamps, the `raw` block behind a rendered text block, and a `role` that disagrees with the source ATIF derives from the event kind. Two were also addressing bugs rather than gaps: a tool-call timestamp was covered by a condition that emitted a record at `events[].started_at`, blaming the event for a value belonging to its tool call, and `finished_at` was never named at all. Timestamps are now four separate claims, each addressed where the value lives. `role` is declared per event and only when it disagrees with the implied source — for anything ACP produces the two always agree, so an ACP trace declares nothing for it, which is the rule working. ## Pinning the rule Two tests convert the same shape twice: once through the ACP edge, where the fields are absent and nothing is declared, and once hand-built with every field present, where the complete `DROPPED` set is asserted as an equality. A third test derives the field list from the IR models themselves and requires each one to have a disposition here — mapped, normalized, declared, container, or representation. Adding a field to the IR that ATIF cannot represent now fails the suite until its fate is decided, which is what makes the first two tests stay honest as the IR grows. ## Verification H1 and H2 parity is unchanged: for both, the document produced through the hub is still identical to the `trainer/atif.json` the rollout really wrote. Their outbound reports grow from 12 to 14 and from 14 to 16 records, the two additions being the trace-level `provenance` and the `raw` behind each text block — both values those traces really carry and ATIF really drops. 45 tests in the ATIF suite; tests/trajectories 379 passed; the wider lane 544 passed; `ruff check .`, `ruff format --check` and `ty check src/` clean. One intermittent failure was seen once in `tests/test_acp.py::TestIdleTimeoutDiagnostics` — a wall-clock assertion that passes alone and on re-run, in a module that imports nothing from the IR family.
Slice D —
|
| edge | space | field | class |
|---|---|---|---|
acp -> ir |
hub | events[2].tool_call.arguments |
unsupported |
ir -> atif |
hub | events[2].tool_call.arguments |
synthesized |
One field, two edges, one path: the source never carried arguments and the
target demanded them anyway. source and target records are terminal by
construction, and there is no unified report — composition happens at read time.
The same asymmetry settles ownership: a trace is built once, so an inbound
conversion may attach its report to it; a trace may be converted to many
targets, so ir_to_atif returns (document, LossReport) and leaves the
input trace byte-identical.
One deliberate deviation
acp_events_to_atif_steps renders an oracle record as a source: "agent" step
prefixed [oracle: …], recoverable only by string matching — §5.1 records this
as a live divergence, since the in-repo validator already accepts
source: "oracle" while no emitter produces one. The IR carries the role, so
this edge emits source: "oracle" with the command as the message. This is
the only intended difference, and a test runs both paths over a trajectory
holding every capture event type plus an oracle record and asserts that step is
the only one that differs.
The loss report describes the trace it receives
Not the trace ACP happens to produce. An ACP-derived trace has no per-event
timestamps or usage, and the inbound report already declared those absences —
re-declaring them here would count one fact twice. The same conversion over a
trace that does carry them declares every one, addressed where the value lives
(a tool-call timestamp under the tool call, not under its event).
Both halves are asserted, the second as a complete set. A companion test derives
the field list from the IR models themselves, so a field added to the IR that
ATIF cannot represent fails the suite until its fate is decided.
Cost
TraceUsage gained cost_usd and price_source before this was published,
because the direct exporter accepts total_cost_usd and the IR could not carry
it — the parity claim would have been true only for traces without one. Cost
sits with the token counters because that is where agent_result,
TaskTelemetry and ATIF's own final_metrics already put it. price_source has
no ATIF slot and is declared dropped: BenchFlow computes no prices of its own,
so a cost without the table that produced it is not comparable. Per-call cost is
out of scope.
Verification
Two kinds, deliberately not merged.
Machine checks
45 tests in the ATIF suite; tests/trajectories 379 passed; the wider
trajectory / capture / streaming / ACP lane 544 passed; ruff check .,
ruff format --check and ty check src/ clean.
One intermittent failure was seen once, in
tests/test_acp.py::TestIdleTimeoutDiagnostics — a wall-clock assertion that
passed alone and on re-run, in a module that imports nothing from these modules.
Recorded rather than attributed.
Human end-to-end — run by @Galius5136, not by CI
D1, D2, D3 use real rollouts already captured (the gemini runs of §5.2,
docker sandbox, ACP transport, standard artifact writers).
- D1 — real tool use. PASS. Three-way comparison identical; 14 outbound
records; theunsupported → synthesizedcomposition above verified on both
tool calls. - D2 — real wall-clock timeout. PASS. Three-way comparison identical; the
timeout is preserved as far as the IR and then declareddroppedat the ATIF
edge, which is the honest result — 16 outbound records. - D3 — hand inspection. PASS. An independent
diffof the produced document
against the rollout's ownatif.jsonreports IDENTICAL.arguments: {}
reproduced,agent.version: "unknown"reproduced, and the two leadinguser
steps reproduced — including the duplicate §5.2 documents. Parity means
reproducing the defect too, not silently fixing it.
D4 and D5 are constructed controls, and are labelled as such:
- D4 — the oracle deviation. No real rollout ran in oracle mode; the input is
built by hand. - D5 — negative control. Removing a tool title from the IR flips the parity
check fromTruetoFalse, so the comparison demonstrably discriminates.
Not established
- Real cost end-to-end is not verified. Every rollout artifact available
carriescost_usd: null— including four whoseusage_sourceis
provider_response, whose gateway log simply carried no per-entry cost. The
writing path is established by reading the code; the mapping is covered by
tests. D6 is a machine/integration proof and is not a human E2E. - Non-text content blocks remain covered by tests only — neither rollout
emitted a file-edit or terminal block. - Oracle mode likewise: constructed input, never a real oracle rollout.
- The richly-populated trace used to pin the complete loss set is
hand-built; no source in the repository produces one yet. - No new rollout was run for this slice.
Feedback on the architecture is still welcome and still cheap to act on: nothing
imports these modules, so redirecting this costs a revert rather than a
migration. The open questions in the PR description remain unanswered, and §8.6
still lists what a review can reject.
Galius5136
changed the title
…nwired
Closes the ATIF pair: `ir_to_atif` writes the document, `atif_to_ir` reads one
back. Nothing imports either, `export_atif.py` is untouched, and it is still the
only writer of `trainer/atif.json`.
The module is built on one rule — read what the document says, never what it
probably meant. An ATIF document is the output of a lossy conversion and several
of its values were fabricated by the converter that wrote it: `agent.version` is
the literal "unknown" whenever BenchFlow had none, `arguments` is `{}` for every
ACP-derived call, `message` is `""` on a step carrying only a tool call. Reading
those back as absent would be guessing which values its counterpart invented,
and would make a round trip look better than it is. So every value is taken
verbatim.
The consequence is the point rather than a wart: a fabricated value returns
indistinguishable from an observed one. `arguments: {}` reads back as an
observed empty argument map, which the IR's tri-state contract cannot separate
from a real one. The information was not lost, it was replaced by a false
statement of the same shape — and only the two loss reports carry the truth.
What no ATIF document holds — trace id, timestamps at any level, run outcome,
provider, per-event provenance — is declared UNSUPPORTED, not DROPPED: the value
is absent from the source, not discarded here, and that is the distinction which
decides where a fix would land. Every DROPPED record in this direction addresses
the source document instead, in the source path space.
Two shapes are deliberately not undone. A step with both `message` and
`reasoning_content` stays one event, because the blank-line join that produced
it is not injective (§5 loss benchflow-ai#10) and splitting it would invent a boundary. A
step with several tool calls becomes one event per call, since the IR models one
per event; no writer here emits that, another producer can.
54 tests, over documents from both writers — the direct exporter and the hub —
plus malformed input a document read off disk can actually have.
…wired Slice D proved the hub reproduces the direct exporter's document. With both ATIF edges in place the loop closes, and this answers the harder question: how much of a trace is still there after a trip through the interchange format. It is a measurement, not an assertion. `compare_traces` reads the two traces and never the loss reports, so a converter that lost something without declaring it is caught rather than confirmed. No percentage, because one number would merge two unrelated things. The report crosses an observed axis — preserved / transformed / lost / fabricated, from the comparison alone — with a declared one: whether ATIF has a slot at all. A loss with a slot is a gap in our own edge and fixable; a loss without one is a cost of the format. The declared half is a table with one entry per IR field, and a test derives the field list from the models so a new field cannot reach the round trip undecided. Comparison is by canonical path rather than by event position: after a conversion that fuses and drops events there is no recoverable correspondence between event i and event j, and inventing an alignment would be guessing. Measured on the two real rollouts of docs §5.2, machine measurement only: - Nothing representable is lost, on either. Every value the loop drops is dropped because ATIF has nowhere to put it, so there is no gap in our edges to close and the remaining loss belongs to the format. A test pins this, and a failure there names a converter bug. - The trace comes back with MORE values than it left with — 46 in, 56 out for H1 — while having lost information. Four fields are fabricated every run, and two of them matter: `agent.agent_version` and `tool_call.arguments` leave declared SYNTHESIZED and return unmarked, so the reconstructed trace asserts they were observed. The information was not lost so much as overwritten with a plausible value of the same shape, and only the pair of reports still carries the truth. - A timeout costs six fields. H2 differs from H1 only by ending in one, and that single fact takes the event, its reason, the run status and three extension fields with it — §5 loss benchflow-ai#4, measured rather than asserted. Docs: §8.3 gains the ATIF -> IR mapping table, §8.10 is the measurement, §8.7 is updated. The doc for the converter that landed in the previous commit is here rather than there, so the two tables could be written against each other. 31 tests. Still nothing imported by a run path; `export_atif.py` untouched and still the only writer of trainer/atif.json.
… input Four defects found by adversarially probing the edge with documents it does not write itself. None of them touches a conformant document — the H1/H2 numbers are unchanged — but each one broke the rule the module is built on, which is exactly the rule that makes the round-trip measurement worth anything. - A JSON `null` in a string slot became the literal `"None"`. `message: null` produced a four-character string the document never contained, and `reasoning_content: null` produced a segment list holding it. Invariant 5 was satisfied throughout, which is why the suite could not see it. Null and absent now both read as no value. - A step with no usable `source` was attributed to the agent. Missing, non-string and out-of-vocabulary sources all became `agent_message` — not a harmless default, since that is the kind a consumer counts as a model turn. They become UNKNOWN, as `ir_from_acp` already does for an unrecognized record. - A non-string `source` was discarded. It cannot be the IR's `source_type`, but it was the only thing the document said about the step's origin, so it is kept in extensions with a record. - A `tool_calls` or `observation` this converter cannot read vanished with no record in any path space. Unreadable here is not the same as not present; both are now kept verbatim and declared in the source space. Also closes a hole in the measurement itself. `lost` counts fields the table calls representable that did not survive, and the table is written by hand — so a field wrongly marked unrepresentable would move a real, fixable loss into the format's column and leave `lost` at zero for free. Two tests make the table falsifiable: on a trace populating every IR field `lost` is 2 (`ir_to_atif` writes no per-step metrics, so per-event usage is lost through a slot ATIF has), and no field the table calls unrepresentable comes back with any value intact. `lost = 0` on the captured rollouts is now a result rather than a definition. `arguments` joins `content[].raw` as an opaque path: both hold a mapping whose keys come from a tool rather than from the IR, and without it the field's path depended on its own contents. Docs: the timeout costs five fields and the event carrying them, not six — the sixth entry in H2's column is the source content block, which H1 loses too. 20 tests added, all of them failing on the code before this commit.
§8.10 described its own numbers as a machine measurement and not a human E2E. That was accurate when written and is no longer: the loop's output has since been checked against the raw artifacts by hand on both rollouts — parity, the conversion read in both directions, every fabricated value confirmed present in the ATIF document and absent from the capture, every unrepresentable one confirmed the other way round, and the negative control. The declared limits are unchanged: oracle rollouts and non-text content blocks remain test-only, no artifact here carries a cost, the representability table is checked against our converters rather than against a vendored ATIF schema, and two rollouts from one agent stay a demonstration rather than a survey.
Slice E —
|
1 participant
Trace interoperability: OTel ↔ ATIF ↔ ACP
Draft. This PR tracks the whole trace-interoperability task and lands it in
reviewable slices. Six are included; OpenTelemetry is not, and is the one part
that needs a maintainer decision before it can start.
Nothing is wired into a run path. No module under
src/benchflowimportsany of the code below, no artifact changes, and
export_atif.pyis untouchedand still the only writer of
trainer/atif.json. A test asserts that isolationrather than leaving it as a claim, and deleting the new files restores the tree
to its previous behaviour.
Goal
OTel ↔ ATIF ↔ ACP trace interoperability.
What is included
ACP → IRIR → ATIFATIF → IR+ round-trip measurementEach slice has its own comment on this PR with the details and the verification.
Why an IR at all
From reconnaissance of the current tree:
trajectory/acp_trajectory.jsonlis read by the viewer, judges, the skill evaluator, task verifiers and the
review skill, with no version field and no written specification. Each consumer
parses it independently.
src/readstrainer/atif.jsonback; its shape is pinned by prose in a module docstring.
gen_ai.*handling. The one module that existed was removed as unwired.each with its own private answer to the same questions — what happens to a tool
call with no arguments, whether a thought boundary survives, whether a timeout
is representable. Those answers already diverge (
docs/trace-interop.md§5).A hub makes each format one edge to a written contract, and makes the
information loss a value rather than a comment.
What the round trip measures
Slice D shows the hub reproduces the direct exporter byte for byte, which says
the IR is sufficient for ATIF. Slice E asks the more useful question — how much
of a trace is still there after a trip through the format — and answers it as a
measurement over the two real rollouts in
docs/trace-interop.md§5.2:IR ↔ ATIFfield mapping, nothing representable is loston the two measured rollouts. Everything the loop drops is dropped because
ATIF has nowhere to put it, so the remaining loss belongs to the format rather
than to these converters. That is a result and not a definition: on a trace
populating every IR field the same measurement reports two representable
losses, which is a gap in our own edge rather than a cost of the format.
one rollout — while having lost information.
argumentsleaves declaredSYNTHESIZEDand returns unmarked, so the reconstructed trace asserts the toolwas observed to be called with none. The information was not so much lost as
overwritten with a plausible value of the same shape, and only the pair of loss
reports still carries the difference.
reason, the run's own outcome status, and the three fields the marker carried.
§5 loss Readme suggestion #4, measured instead of asserted.
That last point is the argument for the hub in the one form an assertion cannot
take: it quantifies what a format costs instead of claiming it.
Full tables are in
docs/trace-interop.md§8.10. These are machine measurementsover real captured rollouts; verification status for each slice is documented
separately in its PR comment.
Roadmap
ACP → ATIFpreservation and loss invariantsACP-session capture events → IRIR → ATIF, with parity against the direct exporterATIF → IR, and theACP → IR → ATIF → IR′measurementa maintainer call, and the semantic conventions are not vendored here
Scope / open questions
The canonical IR and the OpenTelemetry direction are still not approved
decisions. Everything above is built so it can be reviewed as code and deleted
in one commit if the answer is no. The questions, in the order they block work:
N ↔ 1intermediaterepresentation the shape you want, or would you rather see direct converters?
Three exporters walking the same event list through shared helpers is what
suggested it, but that is an inference, not a mandate.
only, and it was removed deliberately. Reviving a receiver and emitting spans
are different projects with different risk. This is the one blocking the
next slice.
rawInput,rawOutputandlocationson tool calls;handle_updatereadsfive fields and drops the rest, which is why every exported format emits empty
tool arguments. Per-event timestamps go the same way. Closing those gaps
changes an on-disk format several consumers already parse, so it is a
compatibility decision rather than a converter one.
acp_trajectory.jsonlcontract. Should the file carry aschema_version? And are theoraclerecord and the session-factorypassthrough part of the trajectory contract, or separate concerns that happen
to share a filename?
Happy to split this into separate PRs per slice if that reviews better.