Repository navigation
Fix #717: run.json totals count subgraph agents' tokens twice - #747
Merged
Merged
Conversation
|
Important Review skippedThis PR was authored by a bot without an assigned CodeRabbit review seat. Ask an organization administrator to assign a seat to the bot. See the seat-assignment guide. After the seat is assigned, request a review again.
|
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.
Issue #717 —
run.jsoncounts subgraph agents' tokens twiceRoot cause
Two sibling handlers stamp a node id onto events, and they disagreed:
pipeline.NodeScopedPipelineHandler(pipeline events) prefixes:evt.NodeID = parentNodeID + "/" + evt.NodeID.agent.NodeScopedHandler(agent events) overwrote:evt.NodeID = h.nodeID.Scopes nest by wrapping. A subgraph's registry factory wraps the parent's agent
handler with a handler scoped to the subgraph node (
"Task02"); the child'scodergen handler wraps that again with its own node id (
"Implement"). Becausethe agent stamp overwrote, the agent's call-level lines (
llm_finish,turn_metrics) landed under"Task02"(or"Implement"), while every pipelineevent for the same node — including the
decision_outcomerollup that carriestoken_input/token_output— landed under"Task02/Implement".pipeline/runusage.gokeys per-node usage by node id and takes each node's mostdirect tier, so it counted the call tier under one id and the rollup tier under
the other.
.totalscame out at exactly twicecombinedUsageForBudget'ssnapshot. Reproduced before the fix on a two-agent child subgraph: engine and
the last
cost_updatedboth said 240 tokens;.totalssaid 480 (ratio exactly2.00), with the container node
Task02also carrying its children's 200 inputtokens. The same defect showed up for
manager_loopchildren.Fix
agent/events.go— the agent stamp now composes exactly as the pipeline onedoes (new
scopeNodeIDhelper): adopt the scope when the event names no node,otherwise prefix
"<scope>/<existing>". One node now has one id across the agentand pipeline streams, so
runusage.go's per-node tier selection sees a singlenode and counts its work once. A top-level agent node is unchanged (
"Gen").Supporting changes:
docs/architecture/agent.md(the NodeID invariant nowstates the composition rule) and a CHANGELOG
### Fixedentry.Tests (all written first, all fail before the fix and pass after)
agent/events_test.go—TestNodeScopedHandlerComposesWithAnEnclosingScope(the registry-factory + codergen nesting),
…StampsABareNodeID,…MatchesPipelineScoping(table pinning the rule shared withNodeScopedPipelineHandler, including the degenerate"Review/Review"case),…ComposesANestedSubgraphPath("L1/L2/Leaf").pipeline/handlers/expand_integration_test.go—TestSubgraphAgentEventsShareTheirNodesScopedID: runs a real registry/enginewith a subgraph and asserts the agent's usage-bearing events and the matching
decision_outcomecarry the same"Sg/Child"id.tracker_capture_totals_test.go—TestRunManifestSubgraphTotalsCountEachAgentOnce: end-to-end run with capture,comparing
run.jsonagainst three independent oracles — the engine's ownaccumulated usage, the run's last
cost_updatedevent (the exact discrepancythe issue reports), and per-node attribution (agent spend must land on
Task02/Implement/Task02/Review, not on the containerTask02).Verified by reverting only
agent/events.goand re-running: the end-to-end testthen reports
run.json totals = 480 tokens, but the last cost_updated says 240.Verification actually run
go build -o /dev/null ./...— pass.go test -vet=off -count=1 ./...— pass, no failures.go test -count=1 ./cmd/tracker-conformance -run TestGoldenTraces— pass (thegolden traces include
node_idand usage; no fixture needed regeneration).go vet ./agent/ ./pipeline/handlers/ .— clean.make complexity—complexity gate OK (135 grandfathered, 0 new).prek run --from-ref FETCH_HEAD --to-ref HEADcommand):prek run --config .pre-commit-config.yaml --files CHANGELOG.md agent/events.go agent/events_test.go docs/architecture/agent.md pipeline/handlers/expand_integration_test.go tracker_capture_totals_test.go— all hooks pass (incl. gofmt, goimports,
make test); no files were modifiedby the hooks.
Limitations / notes
emission time; a pre-fix log still contains both node ids, so re-assembling it
with
tracker run-jsondoes not recover the correct figure. Only new runs arefixed.
parallelfan-out shows the same aggregate-vs-childrenshape (the
componentnode'sdecision_outcomerollup restates its branchtargets' spend, which is booked under the branches as well). That is a
separate construct from this issue's subgraph nesting and was left untouched.
agent.Completeremits no trace events, sollm_callsis unavailablein the end-to-end test (a limit already documented by
TestRunManifestTotalsMatchEngineUsage); the test therefore asserts token andcost totals rather than a call count.
Starting base:
c697043bf705b9cfec28326d7b6183cfee2a3832Verified base:
c697043bf705b9cfec28326d7b6183cfee2a3832Verified commit:
4815e57b29b722f6237e70f773c7d3cf7e5631e8Verification commands and exit codes:
[{"Argv":["prek","run","--from-ref","FETCH_HEAD","--to-ref","HEAD","--config",".pre-commit-config.yaml"],"ExitCode":0}]Closes #717
Final review: pass (pi/lunaroute/deepseek-4.1-flash), job 1, round 1.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.