From d5eef7b393d8a75f8fe6127b86fb1910824de5ca Mon Sep 17 00:00:00 2001 From: Christian-Sidak <61099993+Christian-Sidak@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:53:18 -0700 Subject: [PATCH] fix(playground): chat turns show identical token counts across turns Three bugs caused all chat turns to display the same token-usage numbers in a session: 1. extractLogicalRowId() in webWorkerIntegration used a regex that only matched "lt-" prefixed logical row IDs. Current message IDs start with "msg-", so the regex always fell through and returned the full compound "turn--msg-" string unchanged. The wrong ID was used as the execution step key, making every turn collide on a single shared key. 2. handleExecutionResultAtom in executionItems searched for "-lt-" to find the boundary between the entity segment and the logical row ID in a comparison-mode compound rowId. Because "-lt-" was never found for "msg-" IDs, logicalRowId equalled the full compound string. flatById[compoundId] is always undefined, so the fallback fired and resolved to the LAST shared user message for every turn -- causing all turns to store their result at the same key and therefore show the same traceId and token counts. 3. runStatusByRowEntityAtom in selectors parsed result keys of the form "stepId:sess:entityId" using key.slice(sepIdx + 5) instead of key.slice(sepIdx + 6). The separator ":sess:" is 6 characters, so +5 left a leading ":" on the entityId, producing malformed lookup keys that never matched the UI-side "rowId:entityId" key. Fixes: #5789 Signed-off-by: Christian-Sidak <61099993+Christian-Sidak@users.noreply.github.com> --- .../src/state/execution/executionItems.ts | 9 +- .../src/state/execution/selectors.ts | 3 +- .../state/execution/webWorkerIntegration.ts | 22 ++- .../tests/unit/chatTurnTokenKeys.test.ts | 147 ++++++++++++++++++ 4 files changed, 170 insertions(+), 11 deletions(-) create mode 100644 web/packages/agenta-playground/tests/unit/chatTurnTokenKeys.test.ts diff --git a/web/packages/agenta-playground/src/state/execution/executionItems.ts b/web/packages/agenta-playground/src/state/execution/executionItems.ts index c7871a6ca2..cffef250ce 100644 --- a/web/packages/agenta-playground/src/state/execution/executionItems.ts +++ b/web/packages/agenta-playground/src/state/execution/executionItems.ts @@ -1466,9 +1466,14 @@ export const handleExecutionResultAtom = atom( // shared user message" is racy in comparison mode because the // first-to-finish variant auto-appends a blank user message // before the second variant's result arrives. - // rowId format in comparison mode: "turn--lt-" + // rowId format in comparison mode: "turn--" + // logicalId is "msg-" (current) or "lt-" (legacy). + // Neither prefix can appear in a hex UUID, so searching for + // "-msg-" / "-lt-" unambiguously locates the logical boundary. + const msgIndex = rowId.indexOf("-msg-") const ltIndex = rowId.indexOf("-lt-") - const logicalRowId = ltIndex >= 0 ? rowId.slice(ltIndex + 1) : rowId + const sepIndex = msgIndex >= 0 ? msgIndex : ltIndex + const logicalRowId = sepIndex >= 0 ? rowId.slice(sepIndex + 1) : rowId const flatIds = get(messageIdsAtomFamily(loadableId)) const flatById = get(messagesByIdAtomFamily(loadableId)) diff --git a/web/packages/agenta-playground/src/state/execution/selectors.ts b/web/packages/agenta-playground/src/state/execution/selectors.ts index bd43188746..4663837e5a 100644 --- a/web/packages/agenta-playground/src/state/execution/selectors.ts +++ b/web/packages/agenta-playground/src/state/execution/selectors.ts @@ -922,7 +922,8 @@ export const runStatusByRowEntityAtom = selectAtom( const sepIdx = key.indexOf(":sess:") if (sepIdx === -1) continue const stepId = key.slice(0, sepIdx) - const entityId = key.slice(sepIdx + 5) + // ":sess:" is 6 characters; +5 was off-by-one (left a leading ":"). + const entityId = key.slice(sepIdx + 6) const status = result?.status const isRunning = status === "running" || status === "pending" mapped[`${stepId}:${entityId}`] = { diff --git a/web/packages/agenta-playground/src/state/execution/webWorkerIntegration.ts b/web/packages/agenta-playground/src/state/execution/webWorkerIntegration.ts index a2a0048ae4..701ece6134 100644 --- a/web/packages/agenta-playground/src/state/execution/webWorkerIntegration.ts +++ b/web/packages/agenta-playground/src/state/execution/webWorkerIntegration.ts @@ -179,10 +179,10 @@ export const triggerExecutionAtom = atom( // Multi-entity fan-out: when no specific revision is requested and // multiple entities are shown side-by-side, trigger each one. if (!requestedRevisionId && Array.isArray(entityIds) && entityIds.length > 1) { - const sessionMatch = /^turn-([^-]+)-(lt-.+)$/.exec(String(rowId)) - const logicalIdFromRow = - sessionMatch?.[2] || (String(rowId).startsWith("lt-") ? String(rowId) : "") - const lid = logicalIdFromRow || String(rowId) + // Extract the logical row ID from a possibly-compound turn-ID. + // Use the same sentinel search as extractLogicalRowId so that + // both "msg-" (current) and "lt-" (legacy) are handled. + const lid = extractLogicalRowId(String(rowId)) for (const revId of entityIds) { if (!revId) continue const rid = `turn-${revId}-${lid}` @@ -810,9 +810,15 @@ export const handleExecutionResultFromWorkerAtom = atom( /** * Extract the logical row ID from a turn-style row ID. * Turn IDs have format: `turn--`. - * If it's already a logical ID (starts with "lt-"), return as-is. + * logicalId is "msg-" (current) or "lt-" (legacy). + * Neither prefix can appear inside a hex UUID, so "-msg-" / "-lt-" + * unambiguously marks the boundary between the entity segment and the + * logical row ID regardless of whether the entity UUID itself contains hyphens. */ -function extractLogicalRowId(rowId: string): string { - const match = /^turn-([^-]+)-(lt-.+)$/.exec(rowId) - return match?.[2] || rowId +export function extractLogicalRowId(rowId: string): string { + const msgIdx = rowId.indexOf("-msg-") + if (msgIdx >= 0) return rowId.slice(msgIdx + 1) + const ltIdx = rowId.indexOf("-lt-") + if (ltIdx >= 0) return rowId.slice(ltIdx + 1) + return rowId } diff --git a/web/packages/agenta-playground/tests/unit/chatTurnTokenKeys.test.ts b/web/packages/agenta-playground/tests/unit/chatTurnTokenKeys.test.ts new file mode 100644 index 0000000000..d190544231 --- /dev/null +++ b/web/packages/agenta-playground/tests/unit/chatTurnTokenKeys.test.ts @@ -0,0 +1,147 @@ +/** + * Regression tests for issue #5789 -- token-usage numbers identical across + * every chat turn in a session. + * + * Three bugs caused the symptom: + * + * 1. `extractLogicalRowId` in webWorkerIntegration used a regex that only + * matched "lt-" prefixed logical IDs; current message IDs start with "msg-", + * so the function returned the full compound turn ID unchanged. The wrong + * ID was then used as the execution step key, making all turns collide on + * the last shared user message. + * + * 2. `handleExecutionResultAtom` in executionItems only searched for "-lt-" to + * locate the logical boundary in a compound "turn--" + * rowId. Same root cause; fallback always resolved to the last shared user + * message, so every turn wrote its result at the same key and therefore + * showed the same trace / same token counts. + * + * 3. `runStatusByRowEntityAtom` in selectors sliced result keys with + * `sepIdx + 5` instead of `sepIdx + 6`, leaving a leading ":" on the + * entityId segment and producing a malformed lookup key that could never + * match the UI-side `"${rowId}:${entityId}"` key. + */ + +import {describe, expect, it} from "vitest" + +import {extractLogicalRowId} from "../../src/state/execution/webWorkerIntegration" + +// ── extractLogicalRowId (Bug 1 fix) ───────────────────────────────────────── + +describe("extractLogicalRowId", () => { + it("returns a plain msg- rowId unchanged (single-entity mode)", () => { + expect(extractLogicalRowId("msg-abc123")).toBe("msg-abc123") + }) + + it("returns a plain lt- rowId unchanged (legacy single-entity)", () => { + expect(extractLogicalRowId("lt-old-id")).toBe("lt-old-id") + }) + + it("strips the turn-- prefix from a current msg- logical ID", () => { + expect(extractLogicalRowId("turn-rev1-msg-abc123")).toBe("msg-abc123") + }) + + it("handles a UUID entity ID with hyphens in the compound rowId", () => { + // Entity IDs are hex UUIDs; "msg-" cannot appear in a UUID segment, + // so the sentinel search is unambiguous even when revId contains hyphens. + expect(extractLogicalRowId("turn-a1b2c3d4-e5f6-msg-00000000-dead-beef")).toBe( + "msg-00000000-dead-beef", + ) + }) + + it("strips the turn-- prefix from a legacy lt- logical ID", () => { + expect(extractLogicalRowId("turn-rev1-lt-old-id")).toBe("lt-old-id") + }) + + it("prefers msg- over lt- when both appear (current IDs take precedence)", () => { + // Contrived but guards against a mis-ordered search. + expect(extractLogicalRowId("turn-rev1-msg-foo-lt-bar")).toBe("msg-foo-lt-bar") + }) + + it("returns a non-turn rowId unchanged (no sentinel found)", () => { + expect(extractLogicalRowId("step-abc")).toBe("step-abc") + expect(extractLogicalRowId("")).toBe("") + }) +}) + +// ── resultsByKey entity extraction (Bug 3 fix) ────────────────────────────── +// +// `runStatusByRowEntityAtom` parses keys of the form "stepId:sess:entityId". +// The separator ":sess:" is 6 characters; the old code used `sepIdx + 5`, +// leaving a leading ":" on the entityId and producing a malformed map key. +// We test the parsing logic directly here as a pure string operation. + +function parseResultKey(key: string): {stepId: string; entityId: string} | null { + const sepIdx = key.indexOf(":sess:") + if (sepIdx === -1) return null + return { + stepId: key.slice(0, sepIdx), + // fixed: was sepIdx + 5 (off-by-one) + entityId: key.slice(sepIdx + 6), + } +} + +describe("resultsByKey entity extraction (sepIdx + 6)", () => { + it("extracts stepId and entityId from a well-formed result key", () => { + expect(parseResultKey("msg-abc:sess:entity123")).toEqual({ + stepId: "msg-abc", + entityId: "entity123", + }) + }) + + it("handles a UUID entityId with hyphens", () => { + expect(parseResultKey("msg-abc:sess:a1b2-c3d4-e5f6")).toEqual({ + stepId: "msg-abc", + entityId: "a1b2-c3d4-e5f6", + }) + }) + + it("returns null when the key has no :sess: segment", () => { + expect(parseResultKey("msg-abc:entity123")).toBeNull() + }) + + it("produces a map key that matches the UI-side lookup", () => { + const key = "msg-abc:sess:entity123" + const parsed = parseResultKey(key)! + const mapKey = `${parsed.stepId}:${parsed.entityId}` + // The UI (TurnMessageAdapter) looks up by `"${rowId}:${entityId}"`. + expect(mapKey).toBe("msg-abc:entity123") + }) +}) + +// ── logical rowId extraction in handleExecutionResultAtom (Bug 2 fix) ──────── +// +// In comparison mode rowId = "turn--". The old code +// only searched for "-lt-"; with current message IDs ("msg-") the +// search always failed, leaving logicalRowId equal to the full compound string. +// flatById[compoundId] is always undefined, so the fallback fired and returned +// the LAST shared user message -- causing every turn to collide on that key. + +function extractLogicalFromRowId(rowId: string): string { + const msgIndex = rowId.indexOf("-msg-") + const ltIndex = rowId.indexOf("-lt-") + const sepIndex = msgIndex >= 0 ? msgIndex : ltIndex + return sepIndex >= 0 ? rowId.slice(sepIndex + 1) : rowId +} + +describe("handleExecutionResultAtom logical rowId extraction", () => { + it("returns a plain msg- rowId unchanged", () => { + expect(extractLogicalFromRowId("msg-abc123")).toBe("msg-abc123") + }) + + it("extracts the msg- logical ID from a comparison-mode compound rowId", () => { + expect(extractLogicalFromRowId("turn-rev1-msg-abc123")).toBe("msg-abc123") + }) + + it("extracts the lt- logical ID from a legacy comparison-mode compound rowId", () => { + expect(extractLogicalFromRowId("turn-rev1-lt-old-id")).toBe("lt-old-id") + }) + + it("handles a UUID entity ID with hyphens", () => { + expect(extractLogicalFromRowId("turn-a1b2-c3d4-msg-000-beef")).toBe("msg-000-beef") + }) + + it("returns the rowId unchanged when no sentinel is present", () => { + expect(extractLogicalFromRowId("step-abc")).toBe("step-abc") + }) +})