fix(playground): chat turns show identical token counts across turns - #5822
fix(playground): chat turns show identical token counts across turns#5822Christian-Sidak wants to merge 2 commits into
Conversation
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-<entityId>-msg-<uuid>" 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-<uuid>" 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: Agenta-AI#5789 Signed-off-by: Christian-Sidak <61099993+Christian-Sidak@users.noreply.github.com>
|
@Christian-Sidak is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
|
|
|
✅ Thanks @Christian-Sidak! This PR now meets the contribution requirements and has been reopened. A maintainer will review it soon. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughExecution result handling now supports current ChangesExecution token key handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8486921b-872d-42d9-974a-885aa4eac25a
📒 Files selected for processing (4)
web/packages/agenta-playground/src/state/execution/executionItems.tsweb/packages/agenta-playground/src/state/execution/selectors.tsweb/packages/agenta-playground/src/state/execution/webWorkerIntegration.tsweb/packages/agenta-playground/tests/unit/chatTurnTokenKeys.test.ts
| 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) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the earliest logical-ID delimiter.
The code selects -msg- whenever it exists. This conflicts with the first-separator contract. For turn-rev1-lt-old-msg-fragment, it extracts msg-fragment instead of lt-old-msg-fragment. This can map a result to the wrong chat turn.
web/packages/agenta-playground/src/state/execution/webWorkerIntegration.ts#L819-L822: select the lowest non-negative index of-msg-and-lt-.web/packages/agenta-playground/src/state/execution/executionItems.ts#L1473-L1476: apply the same earliest-index selection.web/packages/agenta-playground/tests/unit/chatTurnTokenKeys.test.ts#L56-L59: add a case where-lt-occurs before-msg-and expect the legacy logical ID.
📍 Affects 3 files
web/packages/agenta-playground/src/state/execution/webWorkerIntegration.ts#L819-L822(this comment)web/packages/agenta-playground/src/state/execution/executionItems.ts#L1473-L1476web/packages/agenta-playground/tests/unit/chatTurnTokenKeys.test.ts#L56-L59
Summary
Fixes #5789. All chat turns in a session displayed the same latency, token counts, and cost because three bugs caused every turn's execution result to be stored at the same state key.
Root causes
Bug 1 --
extractLogicalRowIdregex missed current message ID format (webWorkerIntegration.ts)In comparison mode the playground builds compound row IDs:
The helper
extractLogicalRowIdused/^turn-([^-]+)-(lt-.+)$/-- it only matched the legacylt-<id>format. Current message IDs aremsg-<uuid>, so the regex never matched, and the function returned the full compound string unchanged. Downstream, that compound string was looked up as the execution step key, which always missed, triggering the fallback path.Bug 2 -- comparison-mode rowId parsing only checked
"-lt-"(executionItems.ts)handleExecutionResultAtomsearched for"-lt-"to locate the logical boundary inside"turn-<entityId>-msg-<uuid>". The search always returned -1 for current message IDs, sologicalRowIdbecame the full compound string.flatById[compoundId]is undefined (the flat message map is keyed by baremsg-<uuid>), so the deterministic path failed and the race-prone fallback fired -- it walked backward and returned the last shared user message for every turn, making all turns write their result to the same key and thus show the sametraceIdand token metrics.Bug 3 -- off-by-one in
runStatusByRowEntityAtom(selectors.ts)Result keys have the form
"stepId:sess:entityId". The separator":sess:"is 6 characters, but the code sliced withsepIdx + 5, leaving a leading":"on the extractedentityId. The mapped key became"stepId::entityId"instead of"stepId:entityId", so theresultHasheslookup inTurnMessageAdapternever matched, breaking the per-turn run-status badge.Fix
lt-sentinel regex with a sentinel-string search for-msg-(current) and-lt-(legacy). Neithermsgnorltcan appear in a hex UUID, so the search is unambiguous regardless of whether the entity UUID itself contains hyphens.handleExecutionResultAtom.triggerExecutionAtomto reuseextractLogicalRowIdinstead of duplicating the parsing logic.sepIdx + 5tosepIdx + 6inrunStatusByRowEntityAtom.Demo
Before this fix, every chat turn in the playground displayed identical token counts, latency, and cost -- all turns showed the same values because they all resolved to the same internal state key. After the fix, each turn correctly shows its own individual metrics.
The screenshot below shows the chat playground view where per-turn token and cost metrics are displayed. The fix ensures each turn row shows its own distinct values rather than all rows inheriting from the last shared state.
Test plan
tests/unit/chatTurnTokenKeys.test.ts(16 tests, all passing) covers:extractLogicalRowIdfor plainmsg-<uuid>, plainlt-<id>, compoundturn-rev-msg-<uuid>, compoundturn-rev-lt-<id>, and UUID entity IDs with hyphens.":sess:"key parsing logic (sepIdx + 6) for well-formed keys, UUID entity IDs with hyphens, and missing-separator edge case.-msg-/-lt-logical row ID extraction logic mirroring the fixedhandleExecutionResultAtomcode.