fix(reasoning): attach tool calls to their step in get_trace_with_steps - #142
Open
Andy2003 wants to merge 1 commit into
Open
fix(reasoning): attach tool calls to their step in get_trace_with_steps#142Andy2003 wants to merge 1 commit into
Andy2003 wants to merge 1 commit into
Conversation
`record_tool_call` links a ToolCall to its ReasoningStep via the
`USES_TOOL` relationship but never sets a `step_id` property on the node.
`get_trace_with_steps` grouped tool calls by that absent property, so
`step.tool_calls` always came back empty even though the data was stored
correctly.
Derive the step association from the relationship in `GET_TRACE_WITH_STEPS`
(emitting `{step_id, tool_call}` pairs) and update the parser accordingly.
Works for already-stored data — no migration needed — and now also
populates `ToolCall.step_id` on the returned objects.
Adds a regression integration test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@Andy2003 is attempting to deploy a commit to the lyonwj's projects Team on Vercel. A member of the Team first needs to authorize it. |
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
1 participant
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.
Problem
ReasoningMemory.get_trace_with_steps()always returnsstep.tool_calls == [], even when tool calls were recorded. The data is stored correctly — the reader just can't see it.Root cause:
record_tool_call()links aToolCallto itsReasoningStepvia the(:ReasoningStep)-[:USES_TOOL]->(:ToolCall)relationship, but never sets astep_idproperty on the node.GET_TRACE_WITH_STEPSflat-collected the tool-call nodes and the parser grouped them bytc.get("step_id")— which is alwaysNone— so no tool call ever attached to a step.This surfaced while wiring
record_tool_calls=Trueon the StrandsNeo4jSessionManager: theToolCallnodes were present in Neo4j (correctUSES_TOOLedges), but the high-level reader reported none.Fix
Derive the step↔tool-call association from the relationship instead of an absent property:
GET_TRACE_WITH_STEPSnow emits{step_id: rs.id, tool_call: tc}pairs.get_trace_with_steps()parses that shape and also populatesToolCall.step_idon the returned objects.Relationship-sourced, so it works for already-stored data — no migration required. No change to the write path.
Test
Adds
test_get_trace_with_steps_includes_tool_calls(integration). Verified red→green: fails (assert 0 == 1) before the fix, passes after. Fulltests/integration/test_reasoning_memory.pysuite (30 tests) andtests/unit/nams/test_reasoning.py(22 tests) pass.🤖 Generated with Claude Code