[Proposal] compose: ToolsNode interrupt address segment should not depend solely on the model-generated tool call ID
- Dominant language
- Go
- Stars
- 13k
- Forks
- 1.1k
- Avg merge
- 4h 6m
- Merged PRs (30d)
- 41
Description
## Problem
`ToolsNode` builds the address of a tool invocation in two different shapes, both keyed by the model-generated call ID (v0.9.19):
- At execution time the tool's context gets `{Type: tool, ID: , SubID: }` — `compose/tool_node.go:903` / `:942` → `appendToolAddressSegment(ctx, task.name, task.callID)` (`compose/resume.go:154-156`).
- When wrapping a legacy `InterruptAndRerun` / address-less `InterruptSignal` after the batch runs, it uses `{Type: tool, ID: }` with no `SubID` — `compose/tool_node.go:1112` and `:1212` (`WrapInterruptAndRerunIfNeeded`, `compose/interrupt.go:78-100`).
Meanwhile `internal/core/address.go:67-77` documents `AddressSegment.ID` as *"the node's key or the tool's name"* and `SubID` as the disambiguator *"e.g. parallel tool calls with the same name but different tool call IDs"*. The `:1112` / `:1212` form contradicts that contract (ID is not a tool name, SubID is empty), and the two forms produce different `Address.String()` values (`address.go:35-51`) for the same invocation.
More fundamentally, in both forms uniqueness of the tool segment rests entirely on the call ID, which the framework does not control: it is minted by the model or gateway, may be empty, and may repeat within one batch. `Address.Equals` (`:55-65`) and the `ExecutedTools` / `RerunTools` maps (`:1105-1122`) then cannot tell two such calls apart.
## Why it matters
Targeted resume (`ResumeWithData` / `BatchResumeWithData`, `compose/resume.go:102-120`) addresses interrupt points by their address. A host SDK that persists interrupt IDs and later resumes a specific tool call therefore inherits the model's ID hygiene: duplicate or empty IDs make resume data land on the wrong call or on none. Today the only safe host-side option is to reject a whole batch when IDs collide, which discards otherwise valid model output.
## Proposed API (additive / internal)
Derive the tool segment from a value the node owns — the call's position in the batch — and keep the call ID as a secondary, informative field:
```go
// execution-time and interrupt-time must agree; e.g.
AddressSegment{Type: AddressSegmentTool, ID: task.name, SubID: strconv.Itoa(task.index)}
```
or, if the call ID must stay in the address for compatibility, at least (1) use the same `{ID: name, SubID: callID}` shape at `:1112` / `:1212` as at `:903` / `:942`, and (2) fall back to the batch index when `callID == ""` or when it repeats within the batch. Interrupt IDs surfaced to users are opaque, so switching the underlying segment key is not an API break; it would need a note for persisted checkpoints.
## Workaround today & its cost
A host-side middleware validates every assistant turn before dispatch and aborts the run when two tool calls share an ID or any ID is empty, because otherwise interrupt addresses, executed-tool maps and side-effect bookkeeping all silently alias. This turns a recoverable model quirk into a hard failure and duplicates a uniqueness guarantee the node is better placed to provide.
## Minimal reproduction (v0.9.19)
```go
msg := &schema.Message{Role: schema.Assistant, ToolCalls: []schema.ToolCall{
{ID: "call_1", Function: schema.FunctionCall{Name: "interrupting_tool", Arguments: "{}"}},
{ID: "call_1", Function: schema.FunctionCall{Name: "interrupting_tool", Arguments: "{}"}},
}}
// Both tools call compose.Interrupt(...) -> two InterruptSignals whose tool segments
// compare equal; ResumeWithData(ctx, id, data) cannot target the second one.
```
Contributor guide
Research direction
Start with the minimal reproduction in the issue, then trace address construction in compose/tool_node.go, compose/resume.go, compose/interrupt.go, and internal/core/address.go. Compare execution-time and interrupt-time segments and verify that targeted resume, ExecutedTools, and RerunTools distinguish empty and duplicate tool-call IDs; the final approach should also document any persisted-checkpoint impact.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100