Wire the B3 token breakdown into the runner (12.3) - #188
Merged
Conversation
The instrument existed and nothing called it. A measured run would have spent money and emitted no breakdown -- the exact dead-instrument shape this track has now hit five times, and the one most likely to be discovered only after paying for the run. ContextTokenBreakdown is now computed in RecordTelemetry from the recalled context and the question's full transcript, and emitted on every telemetry row. Proving it required the adapter's own end-to-end test, not the instrument's arithmetic tests: a breakdown nothing invokes passes every one of those while every run stays silent. It sits beside EstimatedContextTokens rather than replacing it. That field is a chars/4 estimate and is named one, and every earlier run recorded it -- swapping in a real count under the same name would make this run incomparable with the series it belongs to while looking like a continuation of it. Measured only when both halves are present. A breakdown against an absent transcript yields a null ratio, which is an absent measurement rather than a flattering one, but emitting the row at all would invite it being read as zero. The end-to-end test also confirmed the design decision that the counting method must travel with the number: the fixture's model resolves no tiktoken encoding, so the run reports CharacterHeuristic and its ratio cannot be mistaken for a measurement. One positional RecordTelemetry call site was converted to named arguments -- a positional tail is precisely what adding a parameter breaks, and silently binding the wrong argument is how it breaks. 4,370 unit and 425 LongMemEval tests green. Release 0 warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PgDgctPpbTziBNE2RT8VdE
There was a problem hiding this comment.
Pull request overview
Wires the B3 ContextTokenBreakdown measurement into the LongMemEval runner’s per-question telemetry so benchmark runs reliably emit a real context-vs-transcript token breakdown (instead of leaving the instrument unused).
Changes:
- Exposes
ContextTokenBreakdown/ContextSectionTokenspublicly so they can appear on public telemetry records. - Adds a lazy
LongMemEvalTokenCounterand recordsTokenBreakdownduringRecordTelemetrywhen both recalled context and full transcript are available. - Extends the adapter end-to-end/unit test to assert the breakdown is actually produced during a real run and excludes it from snapshot equivalence.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/AgentMemory.LongMemEval/ContextTokenBreakdown.cs | Makes breakdown DTOs public for telemetry consumption. |
| tools/AgentMemory.LongMemEval/AgentMemoryLongMemEvalAdapter.cs | Lazily constructs a tokenizer and emits TokenBreakdown into question telemetry. |
| tests/AgentMemory.Tests.Unit.LongMemEval/AgentMemoryLongMemEvalAdapterTests.cs | Adds assertions proving the breakdown is emitted by an actual adapter run. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
8
to
+11
| /// <param name="Section">Section name, as it appears on <see cref="MemoryContext"/>.</param> | ||
| /// <param name="Items">How many memories the section contributed.</param> | ||
| /// <param name="Tokens">Their cost, counted with the real tokenizer.</param> | ||
| internal sealed record ContextSectionTokens(string Section, int Items, int Tokens); | ||
| public sealed record ContextSectionTokens(string Section, int Items, int Tokens); |
Comment on lines
+1016
to
+1018
| TokenBreakdown = context is not null && fullHistory is { Count: > 0 } | ||
| ? ContextTokenBreakdown.Measure(context, fullHistory, TokenCounter) | ||
| : null, |
This was referenced Aug 27, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The instrument existed and nothing called it. A measured run would have spent money and emitted no breakdown — the dead-instrument shape this track has hit five times, and the one most likely to be discovered after paying for the run.
ContextTokenBreakdownis now computed inRecordTelemetryfrom the recalled context and the question's full transcript, emitted on every telemetry row. Proving it required the adapter's end-to-end test, not the instrument's arithmetic tests — a breakdown nothing invokes passes all of those while every run stays silent.Placed beside
EstimatedContextTokens, not replacing it: that field is a chars/4 estimate and is named one, and every earlier run recorded it. Swapping a real count under the same name would make this run incomparable with the series it belongs to while looking like a continuation of it.The end-to-end test also vindicated a design choice — the fixture's model resolves no tiktoken encoding, so the run reports
CharacterHeuristicand its ratio cannot be mistaken for a measurement.One positional
RecordTelemetrycall site converted to named arguments; a positional tail is exactly what a new parameter breaks, and binding the wrong argument silently is how it breaks.4,370 unit and 425 LongMemEval green. Release 0 warnings.
🤖 Generated with Claude Code
https://claude.ai/code/session_01PgDgctPpbTziBNE2RT8VdE