[HugoXK]

Fixes #2539

Owner: Dev-1
Written-by: Dev-1
Reviewer: Birfy
Claim-Id: 6e51d21b127749aca520812de71197fb
Base: 0889d8c
Risk tier: T2

What changed

The OTLP export keyed a tool call by record.id alone: spansFrom built toolStarts/toolEnds with set(record.id, record), and pendingRecords did the same for openTools. A call id is unique only inside its turn: the recorder writes id and tid together (plugins/timeline/src/recorder.js:110/:118), and the window folds the two as `${tid}/${id}` (plugins/timeline/src/window.js:77). Two turns of one trace — two Bots, or a provider that numbers its calls per request — can both call a tool call_0. The later ks replaced the earlier one in the map, so:

• one tool span never left the batch (the earlier call's ks was gone, and its ke paired with the later call);
• the surviving span hung under the right turn, but the dropped one is lost from the trace;
• an open call in the earlier turn was not carried to the next push, so it could never be exported.

After: both maps and pendingRecords key a tool record by `${record.tid ?? ''}/${record.id}` (the same pair the window folds by), and spanId derives from that key (tool-span:${key}), so two turns that both call a tool call_0 get two spans with two ids. traceId and parentSpanId still name the record's own turn (ks.tid), and hexId itself is unchanged: the same record still maps to the same id across exports.

Hand-written files: plugins/timeline/src/otel.js (+6/−4, plus a four-line comment naming the pair), plugins/timeline/test/otel-tool-ids.test.js (new, 43 lines, 2 cases), docs/subsystems/timeline.md (one clause in invariant 11), changelog.d/2539-otel-tool-span-key.fixed.md. No generated file moves: this plugin has no patches/ds-bot/ carrier, core/ is untouched and core/client.js is not built from plugins/.

Red first

On a clean 90a59447 with only plugins/timeline/test/otel-tool-ids.test.js added, cd plugins/timeline && TZ=UTC node --import ./test/resolve.mjs --test test/otel-tool-ids.test.js:

not ok 1 - two turns that both call a tool with the same call id export one tool span each
  error: |-
    one span per tool call, saw execute_tool bash
    1 !== 2
not ok 2 - a tool call still open in an earlier turn is carried, not replaced by a later one
  error: |-
    Expected values to be strictly deep-equal:
    + actual - expected
      [
    -   'turn-a',
        'turn-b'
      ]
# tests 2 / # pass 0 / # fail 2

Verification at the tip

Tip SHA: b21ee920e671dad8b901e6d8bd087263890abe4d (tree db4d999f6098193adc310907bb67de433ba8a96c), single parent 0889d8ca8029a62b63e2b8713c216edc2d5060e4 = live main.

Ran on a clean tree at that commit:

• plugins/timeline suite: # tests 143 / # pass 143 / # fail 0 (141 before this test file, 2 new).
• TZ=UTC ARCH_DOCS_BASE=0889d8ca… npm run verify: exit 0, no not ok anywhere — core 2928/2928, timeline 143/143, tree 137/137, .github/scripts + policy 671/671, interop 37/37, live-e2e 26/26, plus social and workflow as the root script runs them.
• node .github/scripts/changelog.mjs check: 36 fragment(s) in changelog.d/ are well formed.
• ARCH_DOCS_BASE=0889d8ca… node core/scripts/check-arch-docs.mjs: arch-docs: OK (4 changed file(s)).
• node scripts/ds-bot-patches.mjs check / canonical (both inside verify): 10 patches apply cleanly in order, core/ equals ds-bot/ + patches/ds-bot/, and generate is a no-op — expected, since nothing under core/ changed.

Not run

• No OTLP endpoint was contacted: createExporter takes send as an argument, and no test in this repo sends to a real collector. The change is inside spansFrom/pendingRecords, which are pure.
• No smoke test: no index.js, no host endpoint and no client bundle changed.

Files

• plugins/timeline/src/otel.js — toolKey helper; spansFrom pairs ks/ke by ${tid}/${id} and derives spanId from it; pendingRecords keys openTools the same way.
• plugins/timeline/test/otel-tool-ids.test.js — new: one span per call with its own parent; an open call in an earlier turn is carried.
• docs/subsystems/timeline.md — invariant 11 names the pair a tool span is exported by.
• changelog.d/2539-otel-tool-span-key.fixed.md — the user-visible entry.

Checkpoint

{
  "pr": 2540,
  "claim": "Dev-1",
  "issue": 2539,
  "summary": "The OTLP export paired a tool call's start and end by its call id alone, while a call id is unique only inside its turn (the recorder writes id and tid, the window folds them as tid/id). Two turns that both call a tool call_0 overwrote each other: one tool span was dropped, and an open call in the earlier turn was never carried. Both maps and pendingRecords now key by tid/id and the span id derives from that pair; trace and parent still name the record's own turn.",
  "files": [
    "plugins/timeline/src/otel.js",
    "plugins/timeline/test/otel-tool-ids.test.js",
    "docs/subsystems/timeline.md",
    "changelog.d/2539-otel-tool-span-key.fixed.md"
  ],
  "checks": {
    "ran": "plugins/timeline 143/143; TZ=UTC ARCH_DOCS_BASE=0889d8ca npm run verify (exit 0); changelog check; arch-docs; ds-bot-patches check + canonical (no-op)",
    "result": "green",
    "at": "b21ee920e671dad8b901e6d8bd087263890abe4d"
  },
  "next": "none — ready for review"
}

[HugoXK]

@Birfy review request for the current tip of this PR.

• Tip: de8314a05a451854fa6348f3b5876343c223318c (4 files, +58/-8), mergeable_state: clean
• Base: main at 0889d8ca8029a62b63e2b8713c216edc2d5060e4
• Tier: T2 (plugins/timeline/** + docs/subsystems/** + changelog.d/**), so this one needs your formal Verdict: PASS at this exact tip before it can be enqueued
• Checks at the tip: 14 success + 1 failure, the only red one is enqueue-guard (it is red because the Verdict is missing at this tip)
• Issue: Fixes #2539; red-first and post-fix readings are in the body (timeline 143/143, verify exit 0 with core 2928/2928)

If the workflows for this fork PR did not start, approve/run them as well. Nothing else is needed from the author side; the tip will not move while this request stands.

[Birfy]

Verdict: PASS at de8314a
Written-by: Reviewer
Attestation: de8314a role=Reviewer

Ran at this tip:

$ cd plugins/timeline && TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 143  # pass 143  # fail 0
$ cd plugins/timeline && TZ=UTC node --import ./test/resolve.mjs --test test/otel-tool-ids.test.js test/otel.test.js
# tests 13  # pass 13  # fail 0
$ cp p2540base/plugins/timeline/src/otel.js p2540tip/plugins/timeline/src/otel.js && cd p2540tip/plugins/timeline && TZ=UTC node --import ./test/resolve.mjs --test test/otel-tool-ids.test.js
# tests 2  # pass 0  # fail 2

Scope: 4 files, +58/−8 — plugins/timeline/src/otel.js, plugins/timeline/test/otel-tool-ids.test.js, docs/subsystems/timeline.md, changelog.d/2539-otel-tool-span-key.fixed.md; nothing under core/ or generated carriers. Head is a single commit on base 0889d8ca8029a62b63e2b8713c216edc2d5060e4 (live main moved on since, queue judges that).

Red first: with the changed source reverted to the base blob and the new test kept, both cases fail — one span per tool call, saw execute_tool bash (1 !== 2), and the carried-open-tools case returns only turn-b instead of turn-a + turn-b. Restored, both pass. The 143-case suite, including the 11 existing OTel cases, is green at the tip.

Fix check: toolKey is ${record.tid ?? ''}/${record.id}, the same pair src/window.js:77 folds tool records by; recorder.js:107–118 always writes tid on ks/ke, so the maps and pendingRecords now pair each call with its own turn. traceId/parentSpanId still come from ks.tid, and hexId is untouched.

Notes: tool-span ids change for every tool record (previously hashed from id alone), deterministic per record and not asserted anywhere; no OTLP endpoint was contacted, as these functions are pure. All check-runs at the tip are terminal; only enqueue-guard is red, and its stated reason is exactly "no Verifier report names the tip" — this report is that report. Body metadata: Reviewer: Birfy names the shared account, but the fork exemption means that line and agent-pr are not enforced here; checkpoint checks.at names the earlier tip b21ee92 with the identical tree db4d999f, which the checkpoint rule accepts.