fix(agent-sessions): decode tool-error payloads the way the session page does - #1148
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 25 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Maple reviewConfidence 4/5 · likely safe to merge The tool-error payload read stops picking the arguments/result out of the span's attributes in SQL and instead returns the same filtered attribute map the session span read uses, decoded through the vendor integration in TypeScript. The modal, the MCP tool and the session page now show one tool span the same way; the change is contained and tested.
What was checked
Observability coverage: 1 of 1 changes observable
|
620d24b to
9c28a72
Compare
|
Note A newer push replaced |
Maple reviewConfidence 4/5 · likely safe to merge The tool-error payload read now returns the span's filtered attribute map and decodes
What was checked
|
9c28a72 to
0e3e0ce
Compare
Maple reviewConfidence 3/5 · needs attention Warning This review ended early; what follows is what it established. The tool-errors read now projects the span's attribute map through
FindingsWarning · F1 ·
|
![maple-review-bot[bot]](https://avatars.githubusercontent.com/in/4108169?s=60&v=4){kind=small}
| traceId: $.TraceId, | ||
| spanId: $.SpanId, | ||
| statusCode: $.StatusCode, | ||
| spanAttributes: aiSpanAttributes($.SpanAttributes), |
There was a problem hiding this comment.
aiToolErrorPayloadsQuery ships each sampled span's full uncapped payload map
F1 · Warning · performance
The SQL no longer computes leftUTF8(args, AI_TOOL_ERROR_PAYLOAD_MAX)/length; the projection returns the whole filtered attribute map, and aiSpanAttributes deliberately keeps input.value/output.value, which hold the untruncated tool arguments and result. For up to AI_TOOL_ERROR_OCCURRENCES_LIMIT (and 100 via MCP samples_limit) rows, ClickHouse now egresses payloads the session read's own comment calls megabyte-scale, and aiToolErrorPayload keeps the full string in memory only to cut it to 4000 characters (path:1015). A tool that returned a file or query result makes the tool-errors view pull megabytes per sample where it used to transfer at most AI_TOOL_ERROR_PAYLOAD_MAX characters per field.
Bound the read in SQL as before — compute the byte totals with `length(...)` on the source attribute and return a byte-capped copy of `input.value`/`output.value` alongside the map — so `aiToolErrorPayload` decodes from a bounded string instead of materialising the whole payload.
Prompt for an AI agent
In `packages/query-engine-integrations/src/ai/ai-tools.ts:973`: `aiToolErrorPayloadsQuery` ships each sampled span's full uncapped payload map.
The SQL no longer computes `leftUTF8(args, AI_TOOL_ERROR_PAYLOAD_MAX)`/`length`; the projection returns the whole filtered attribute map, and `aiSpanAttributes` deliberately keeps `input.value`/`output.value`, which hold the untruncated tool arguments and result. For up to `AI_TOOL_ERROR_OCCURRENCES_LIMIT` (and 100 via MCP `samples_limit`) rows, ClickHouse now egresses payloads the session read's own comment calls megabyte-scale, and `aiToolErrorPayload` keeps the full string in memory only to cut it to 4000 characters (`path:1015`). A tool that returned a file or query result makes the tool-errors view pull megabytes per sample where it used to transfer at most `AI_TOOL_ERROR_PAYLOAD_MAX` characters per field.
Suggested fix: Bound the read in SQL as before — compute the byte totals with `length(...)` on the source attribute and return a byte-capped copy of `input.value`/`output.value` alongside the map — so `aiToolErrorPayload` decodes from a bounded string instead of materialising the whole payload.
Verify the problem exists at that location before changing it, and keep the fix to those lines.
There was a problem hiding this comment.
Fixed in 7525624: the read cuts every returned attribute value to AI_TOOL_ERROR_ATTRIBUTE_MAX (16384 chars, 4x the display cut so a payload the modal can show whole still parses) with mapApply(... leftUTF8 ...), and returns cutAttributeBytes, the length() of each value it cut, so the reported size stays exact. A cut JSON value decodes as its raw text and is cut again for display; the schema swap still matches because both copies are cut to the same prefix. Tests cover both.
…age does The tool-errors samples (web modal, get_agent_tool_error) read gen_ai.tool.call.arguments raw in SQL, so an OpenInference tool whose GenAI dual-write copied its parameter schema there showed the schema, and a tool without the dual-write showed nothing. The payload read now returns the span's projected attributes and runs them through the same integration decode as the session page (schema swapped for input.value, LangChain ToolMessage unwrapped), then truncates and sizes the result in TypeScript.
0e3e0ce to
52ee022
Compare
Maple review🟡 Confidence 3/5 · needs attention The tool-error payload read now returns the span's filtered attribute map and decodes it through the vendor integration (
Still open from earlier reviews
What was checked
|
…-arguments-parity # Conflicts: # packages/query-engine-integrations/src/ai/ai-tools.ts
Maple review🟡 Confidence 3/5 · needs attention Moves the tool-error payload decode out of SQL into the same TypeScript path the session page uses, so the modal shows a tool's real arguments; truncation and byte sizes move into
Still open from earlier reviews
What was checked
|
Maple review🟡 Confidence 3/5 · needs attention Warning This review ended early; what follows is what it established. The tool-error payload path now hands back the span's attribute map (cut, with the true sizes of the values cut) and decodes arguments/result in TypeScript through
Still open from earlier reviews
What was checked
Observability coverage: 1 of 1 changes observable
|
1 participant
Based on main, no migration
Based on
main(#1143 landed as 3688ee9); rebased so only this PR's commits remain. It adds no migration and no SQL rule. It changes only the TS read path.Rebase notes: the new
decodeGenAikeeps #1143's stamped-agent override (maple_ai.agent.name), somapAiSpanbehaves as it does onmain. The payload read's key list now includes #1143'smaple_ai.agent.namekey, and the SQL baseline was regenerated for it.Bug #44
The tool-errors views (the web tool-errors modal and MCP
get_agent_tool_error) readgen_ai.tool.call.arguments/gen_ai.tool.call.resultraw in SQL. The session page decodes the same span through the vendor integration, so the two disagreed:update_seat, sessionverify-oa-py-nofix1-1; LlamaIndexverify-li-sem1-1). The session page showsinput.value.verify-li-sem0-1).ToolMessageresults were not unwrapped.Change
aiToolErrorPayloadsQueryreturns the span's attribute map, projected with the samemapFilterthe session span read uses (aiSpanAttributes, extracted fromai-sessions.ts).mapAiSpan's decode is split out intodecodeGenAi. The newaiToolCallPayload(attributes)returns the decodedtoolCallArguments/toolCallResult.aiToolErrorPayload(row)re-serialises the payload, truncates it toAI_TOOL_ERROR_PAYLOAD_MAXcodepoints and reports its size in UTF-8 bytes. SQL used to do this withleftUTF8/length.readAiToolErrorSamplesmaps rows through it. The response shape is unchanged.AI_TOOL_ERROR_ATTRIBUTE_MAX(16384 chars, 4x the display cut) in SQL and returnscutAttributeBytes, the byte length of each value it cut, so a sample costs kilobytes and the reported size stays exact. A cut JSON value decodes as its raw text.Tests
main:query-engine-integrationsai-tools,ai-integrations,ai-sessions,ai-span-columns, and the catalog SQL baseline (regenerated); MCPagent-tools.test.ts; APIai-sessions.http.test.ts.tsc --noEmitinpackages/query-engine-integrations.ai-tools.test.ts: fixtures from the EU spans (openai_agents_sdk schema vs real args, LlamaIndex with and without the dual-write), the ToolMessage unwrap, codepoint truncation and byte size, a value cut by the read (raw text, true size), and the schema swap when both copies are cut.ai-tools.clickhouse.e2e.test.ts:tools-flaky-2is seeded as an OpenInference dual-write span and still reads{"retries":1}. This passed before the rebase. It was not rerun onmain(no Docker on this pass).Summary by CodeRabbit
New Features
Bug Fixes