Trace interoperability: OTel ↔ ATIF ↔ ACP - #984
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 —
|
1 participant
Trace interoperability: OTel ↔ ATIF ↔ ACP
Draft. This PR tracks the whole trace-interoperability task. It lands in
slices; only Slice A is included so far. This PR is intentionally kept in Draft
while the task is developed in reviewable slices.
Goal
OTel ↔ ATIF ↔ ACP trace interoperability.
Current findings
From reconnaissance of the current tree:
trajectory/acp_trajectory.jsonlis produced by the capture path and read bythe viewer, judges, skill evaluator, task verifiers and the review skill, yet
it carried no schema, no version field and no written specification. Each
consumer parses it independently.
export_atif.pywritestrainer/atif.json;nothing under
src/reads it back. Its shape is pinned by prose in a moduledocstring against upstream artifacts that are not vendored or tested against.
OTel dependency and no OTLP or
gen_ai.*handling undersrc/. The onlymodule that existed was removed as unwired; it converted inbound only.
acp_trajectory.jsonlhas no single artifact-level contract today. Nodocument or schema describes the file as a whole.
vocabulary.
_snapshot_session_trajectoryreturnssession.stepsunchangedfor session-factory Sessions, and oracle-mode rollouts replace the trajectory
with an oracle-only list. The two rollout modes are mutually exclusive; no
production path emits a mixed ACP + oracle artifact.
Details, with references, in
docs/trace-interop.md.Roadmap
Slices B–F are a proposed sequence, not an agreed plan. They will be reshaped —
or replaced — by the answers to the open questions below.
Scope / open questions
The canonical IR and the OpenTelemetry direction are not approved decisions.
Slices B–F are a proposed shape, not a plan that has been signed off. They are
listed so the direction is visible and can be redirected early; each depends on
answers that only a maintainer can give. If the answers point elsewhere, the
roadmap changes.
Open questions, in the order they block work:
N ↔ 1intermediaterepresentation the shape you want, or would you rather see direct converters?
Three exporters already walk the same ACP event list through shared helpers,
which 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.
rawInput,rawOutputandlocationson tool calls;handle_updatereadsfive fields and drops the rest, which is why every exported format emits
empty tool arguments. Per-event timestamps are dropped the same way. Closing
those gaps changes the on-disk format that 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?
Slice A status
Included. Adds a Draft 2020-12 schema for the ACP-session capture events, the
documentation for the format, and a conformance suite over records generated
through the production capture path.
jsonschemais declared as a devdependency.
Zero runtime behaviour change. No runtime module imports the schema and
nothing under
src/benchflowchanges behaviour. The schema is deliberatelyscoped to the ACP-session capture events and does not claim to describe the
whole artifact.
A separate comment covers the Slice A details and the human verification.