Skip to content

emrg: parallel tool calls carry a batch label - #2403

Merged
how2how2how2-arch merged 1 commit into
masterfrom
feature/tool-call-batch-label
Oct 20, 2026
Merged

how2how2how2-arch merged 1 commit into
masterfrom
feature/tool-call-batch-label

Conversation

@argszero

Copy link
Copy Markdown
Owner

Closes #2402

What was wrong

A round of the tool loop can run several tool calls at once. Every card in the
transcript looked identical, so a batch of three bash calls read as one call
repeated — a reader could not tell which card belonged to which command, or that
the model had asked for several things in one round at all.

What changed

Each tool card now carries the batch label (n/m) in its header, before the tool
name: the n-th call of the m calls in that round.

  • emrg/server/daemon.py — _tool_call_labels() derives the label from the
    call's position in the round; the tool_start and tool_end frames carry it
    as call_label; the history projection computes it at read time.
  • emrg/protocol.py — ToolStart carries call_label.
  • emrg/client/python_tui/widgets/tool_card.py + emrg/client/app.py — the TUI
    renders it.
  • emrg/gui/renderer/src/lib/transcript.ts, historyReplay.ts,
    components/TranscriptView.tsx — the GUI's ToolRow.callLabel, set from the
    frame and passed through the replay, rendered before .tool-name.

The label is derived at read time, never stored. A saved record keeps the
call's own data and nothing derived from the round it happened to sit in, so a
replayed session relabels itself from the records instead of trusting a string
written by an older version of this code.

Verification

  • tests/test_tool_call_batch_label.py — 10 tests: the helper's arithmetic
    (including a single call, which takes no label), the two frames, the history
    projection, and a check that the saved record carries no stored label.
  • Renderer: 2 new tests in TranscriptView.test.tsx, 2 in historyReplay.test.ts
    (npm test: 714 passed / 48 files; tsc --noEmit clean).
  • Full Python suite: 5514 passed, 18 skipped.

CI is pending on this head.

When the model issues several tool calls in one round, every card now
shows which call of the batch it is, as "(n/m)" before the tool name.

The label is computed at read time from the call's position in the
round, never written into the saved record, so replaying an old session
relabels it rather than trusting a stored string. Both clients render
the same field: the TUI tool card and the GUI transcript row.

Closes #2402

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261020-140129

The rule lives in exactly one place and both clients print what they are handed.

daemon._tool_call_labels() is the only answer to "what is a batch": y is one
assistant response's tool_calls, x counts from 1, the returned string carries its
own trailing space so a renderer is f"{icon} {label}{name}" with no branch, and a
single call gets "" rather than (1/1). The live frames and the history projection
call the same helper, so there is no second implementation to drift.

The history projection copies the record instead of mutating it, and computes the
label at read time — which is the part I checked hardest, because the tempting shortcut
(write the label into the record) would make the answer a property of when a session was
recorded, and every session recorded before today would stay unlabelled for ever. It
also means no migration was needed.

Verified on the head, not the page:

  • tests/test_tool_call_batch_label.py → 10 passed, and it asserts the rendered
    text rather than the field: the three frames of a batch carry (1/3) (2/3) (3/3),
    a batch of one carries "", and a record with no call_label on disk still
    replays with one — the proof that the projection computes it.
  • The absence guard is the rant's own acceptance test: with the field missing, both
    renderings are character-identical to the pre-change one and contain no (1/1) and no
    empty brackets — the evidence that neither client counts.
  • check-merge-freshness.py 2403 → FRESH, merge base 1a060c2c is master's tip.
  • CI (test + test-windows, run 39373928960) green on this head.

Scope is what the issue asked for and nothing more: tool_name, intent, arguments,
the record schema and the GUI's merged-group summary are all untouched, and tool_end
deliberately does not carry the field.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261020-141450

Landing tree 099f498f9ab58b10899a0d0de39b208049807476 — this review is about the tree check-merge-plan-suite.py computed and ran, not about the head's own CI alone. Head cf5c0759 is one commit behind master (1a060c2c → base 2c55a36a), so its green run is about a tree that can no longer be merged; I did not ask for a refresh, since a push would void the standing approval and this reading is available without one.

The suite on the landing tree: 5059 passed, 490 skipped in 920.82s (this host skips far more than CI's Linux leg — its own count, not a CI figure).

The rule has one home, and both carriers reach it

daemon._tool_call_labels() returns the whole batch's labels and is called from exactly two places: the live loop (call_labels = _tool_call_labels(openai_tool_calls), computed before the loop so a refused or failed call still consumes its position) and the history projection. I checked the carrier the issue is most specific about: tool_end carries nothing, and test_the_end_frame_does_not_carry_a_second_copy_of_the_label pins that. The projection builds a fresh item dict and {**call, "call_label": …} copies, so the stored record keeps its own shape — and test_the_projection_copies_the_record_instead_of_mutating_it reads it back off disk to prove it.

One thing I went looking for and did not find: the live label counts openai_tool_calls while the persisted record is built from tool_calls, which are two names for two lists. They are one-for-one here (openai_tool_calls is appended inside for tc in tool_calls), so a live (2/3) and its replay agree. Had they diverged, the two clients would have disagreed in exactly the way the issue forbids, and nothing would have caught it — the arms below do not cover that pair.

Mutation arms, run here — three killed, one control

  • A. Labels forced empty (return [""] * total) → red on the frame test and the history test. Killed.
  • B. Denominator forced to 1 ((x/1) ) → red on the same two. Killed.
  • C. The TUI counts the batch itself (f"({_i+1}/{len(tool_calls)}) " in _cards_from_tool_calls) → red on test_a_replayed_batch_labels_its_cards_from_the_projected_record. This is the one that matters: it is the arm that proves the absence guard is testing what it claims, and it discriminates.
  • D. Control, a comment only → 10 passed. Survived, as it must.

All four restored byte-identically (sha verified), working tree clean after.

Renderer: 53 passed across TranscriptView.test.tsx (28) and historyReplay.test.ts (25) — and check-node-test-count.py confirms Agent.md's 714/161 against both runners at this head, so the bumped numbers are measured, not asserted.

Notes, none of them blocking

  1. The body describes a different change from the one in the tree. It says "the tool_start and tool_end frames carry it as call_label". tool_end does not, and the issue's item 2 says it must not — the code and the test are right, the body is wrong. A body-only defect: the natural fix does not move the head, so it is a note and not a veto. Worth editing, because the next reader will believe the sentence.
  2. .tool-call-label has no CSS rule. Every sibling span in .tool-row has one; .tool-name is --font-mono, weight 600, --fs-secondary. The new span therefore inherits the row's body font, so the label renders larger and non-mono beside the name it belongs to. One rule beside .tool-name would settle it.
  3. The issue's arm (b) predicted the wrong guards. It expected the TUI and GUI rendered assertions (items 3 and 4) to redden when y is forced to 1. Measured, only the frame and history tests do — the renderer tests construct a literal call_label and never travel through the daemon. Coverage is complete in aggregate (item C pins the pass-through on the client side), but the expectation in the issue is not what the arms do.
  4. The acceptance evidence the issue asks for is not quoted: a real multi-call turn's three rendered TUI lines and the GUI row set, plus a session recorded before the change opening with (x/y). I verified both paths by test rather than taking the claim, so the mechanism is proven — but the issue asks for the quotation, and a future reader of this PR will not find it.

What I did check, item by item

One helper, two callers ✓ · tool_start only ✓ · projection computed at read time on copies ✓ · ToolStart.call_label defaults to "" through d.get ✓ · both clients pass it through, no arithmetic on either side ({row.callLabel ? … : null}, f"{icon} {label}{name}") ✓ · detail views bare — the GUI detail name row reads row.toolName, the TUI selector's tool: line reads card.name ✓ · never (1/1), never empty brackets ✓.

Landing diff: 11 paths, exactly this PR's own files, no smuggled change — the seven paths the tool prints as reversals are master's own later commit, which is the stale-base reading hazard and not a defect here.

@how2how2how2-arch how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261020-150141

The head is one commit behind master, so I measured the tree this merge would land rather than the head:
check-merge-plan-suite.py 2403 → final tree 099f498f9ab5, 5515 passed, 35 skipped. The landing diff shows the
real change set (protocol.py, daemon.py, TUI card, three GUI files, plus a 322-line test file); the seven paths it
lists as "reads backwards" are the base's own later commits, which this PR does not touch.

The load-bearing claim in the body is that the label is derived at read time and never stored, and that is the
half worth testing, because a stored label would make a replayed session's answer a property of which version wrote it.
Measured on this head:

  • daemon._tool_call_labels([3 calls]) → ['(1/3) ', '(2/3) ', '(3/3) '].
  • Projecting a saved record does not mutate the record: the record's JSON is byte-identical before and after
    projection, while the projected copies carry the labels. That is {**call, "call_label": …} doing what the comment
    says it does.
  • Edge shapes: one call → '' (not (1/1), which would be noise on the common case), zero calls → [].

Both derivation sites are the frame and the read-time projection; there is no write path that mentions call_label.

CI on this head is green on both legs.

@how2how2how2-arch
how2how2how2-arch merged commit e9f4bd4 into master Oct 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

emrg: a tool row shows which call of the batch it is, decided once in the daemon

3 participants