feat(frontend): show span parameters in the trace overview#4398
feat(frontend): show span parameters in the trace overview#4398mmabrouk wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedFailed to post review comments 📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds span parameters visibility to the trace overview. It introduces selectors to extract parameters from trace span data, enables initial collapse control through a ChangesSpan Parameters & Collapse State
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
dosubot
Bot
added
size:M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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: abbd81e8-5353-4e91-bd4d-a3835a8358b5
📒 Files selected for processing (4)
web/oss/src/components/DrillInView/TraceSpanDrillInView.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/AccordionTreePanel.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceContent/components/OverviewTabItem/index.tsxweb/oss/src/state/newObservability/selectors/tracing.ts
mmabrouk
force-pushed
the
feat/trace-overview-parameters
branch
from
235dcb0 to
77ac31d
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/oss/src/components/SharedDrawers/TraceDrawer/components/TraceContent/components/OverviewTabItem/index.tsx (1)
69-72: 💤 Low valueConsider excluding arrays in the
hasParameterscheck.Arrays are objects in JavaScript, so
typeof parameters === "object"will pass for arrays. Ifag.data.parameterscould ever be an array (even unexpectedly), the current check would treat it as valid and attempt to render it as an object.🛡️ More defensive check
const hasParameters = parameters != null && typeof parameters === "object" && + !Array.isArray(parameters) && Object.keys(parameters as Record<string, unknown>).length > 0
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d5c8444d-950d-470c-a1a1-8f987b531efe
📒 Files selected for processing (4)
web/oss/src/components/DrillInView/TraceSpanDrillInView.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/AccordionTreePanel.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceContent/components/OverviewTabItem/index.tsxweb/oss/src/state/newObservability/selectors/tracing.ts
Railway Preview Environment
|
dosubot
Bot
added
size:L
Render ag.data.parameters as a collapsed, beautified panel in the trace overview, alongside inputs and outputs, shown only when parameters exist. Adds a defaultCollapsed prop to TraceSpanDrillInView and AccordionTreePanel.
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e3d8f991-1ce8-47fe-acfb-a8f12877a00e
📒 Files selected for processing (2)
docs/designs/tool-call-roundtrip/rfc.mdweb/oss/src/components/DrillInView/BeautifiedJsonView.tsx
| const safeStringify = (value: unknown): string => { | ||
| try { | ||
| return JSON.stringify(value, null, 2) | ||
| } catch { | ||
| return String(value) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In TypeScript/JavaScript, can JSON.stringify return undefined without throwing? Please include examples and the return type from MDN/TS lib definitions.
💡 Result:
Yes. In JavaScript/TypeScript, JSON.stringify can return undefined without throwing. 1) MDN behavior (runtime) - MDN states that JSON.stringify “Return value: A JSON string representing the given value, or undefined.” [1] - MDN also notes that when undefined/function/symbol are encountered, they are omitted from object output (and censored in arrays), rather than causing an error. [1] Examples (no throwing): - JSON.stringify(undefined) returns undefined: const s1 = JSON.stringify(undefined); // undefined (MDN return value explicitly allows undefined.) [1] - Omits object properties whose values are undefined: const s2 = JSON.stringify({ a: undefined }); // "{}" (property omitted) (MDN: if you return undefined, “the property is not included” in object output.) [1] - In arrays, undefined becomes null instead of throwing: const s3 = JSON.stringify([undefined]); // "[null]" (MDN: “You cannot use the replacer function to remove values from an array. If you return undefined … null is used instead.”) [1] 2) TypeScript lib definition (return type) - In TypeScript’s standard library type definitions, JSON.stringify is currently declared to return string (not undefined): interface JSON { stringify(value: any,...): string;... } [2] - This is a typing mismatch with the actual JavaScript behavior described by MDN (and discussed in the TypeScript issue about the incorrect return type). [1][3] So: - Runtime: JSON.stringify may return undefined (no throw). [1] - TypeScript types (lib.d.ts): return type is string. [2] If you need to model the runtime accurately in your code, you can wrap/cast, e.g.: const maybeJson = JSON.stringify(value) as string | undefined; (Or define your own helper typing.)
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify
- 2: https://github.com/Microsoft/TypeScript/blob/fb23e6dba1e79c2c13f116299756062ee36cbf09/lib/lib.d.ts
- 3: Lib type definition for JSON.stringify asserts incorrect return type microsoft/TypeScript#55436
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="web/oss/src/components/DrillInView/BeautifiedJsonView.tsx"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }
echo "---- safeStringify definition ----"
nl -ba "$FILE" | sed -n '120,180p'
echo "---- getMessagePartText usage of safeStringify ----"
# show the function region if present
rg -n "getMessagePartText" "$FILE" || true
# print a wider window around the call sites / related logic
rg -n "safeStringify" "$FILE"
# show nearby lines around each occurrence
for ln in $(rg -n "safeStringify" "$FILE" | cut -d: -f1 | sort -u); do
echo "---- around safeStringify at line $ln ----"
nl -ba "$FILE" | sed -n "$((ln-20)),$((ln+20))p"
doneRepository: Agenta-AI/agenta
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="web/oss/src/components/DrillInView/BeautifiedJsonView.tsx"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }
echo "---- safeStringify definition (approx lines 130-170) ----"
awk 'NR>=120 && NR<=180 {printf "%6d | %s\n", NR, $0}' "$FILE"
echo "---- lines containing safeStringify ----"
rg -n "safeStringify" "$FILE" || true
echo "---- lines containing getMessagePartText ----"
rg -n "getMessagePartText" "$FILE" || true
echo "---- window around safeStringify call sites ----"
for ln in $(rg -n "safeStringify" "$FILE" | cut -d: -f1 | sort -u); do
echo "=== around $ln ==="
awk -v start=$((ln-25)) -v end=$((ln+25)) 'NR>=start && NR<=end {printf "%6d | %s\n", NR, $0}' "$FILE"
doneRepository: Agenta-AI/agenta
Length of output: 7217
Fix safeStringify to always return a string
JSON.stringify can return undefined without throwing (e.g., when an object’s toJSON() returns undefined), so this helper can violate its string return type and propagate an invalid value into getMessagePartText.
Suggested fix
const safeStringify = (value: unknown): string => {
try {
- return JSON.stringify(value, null, 2)
+ return JSON.stringify(value, null, 2) ?? String(value)
} catch {
return String(value)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const safeStringify = (value: unknown): string => { | |
| try { | |
| return JSON.stringify(value, null, 2) | |
| } catch { | |
| return String(value) | |
| } | |
| const safeStringify = (value: unknown): string => { | |
| try { | |
| return JSON.stringify(value, null, 2) ?? String(value) | |
| } catch { | |
| return String(value) | |
| } | |
| } |
mmabrouk
force-pushed
the
feat/trace-overview-parameters
branch
from
23447af to
1f7824c
Compare
dosubot
Bot
added
size:M
1 participant
What was missing
When you open a trace span in the observability overview, it shows inputs and outputs but never shows
ag.data.parameters. For chat spans, that is where the model config and the tool definitions live. So the tools, and the rest of the config, were invisible in the overview.The change
This adds a "parameters" panel to the overview, next to inputs and outputs. It renders
ag.data.parametersin the same beautified view as the other panels. It stays collapsed by default so it does not get in the way, and it only appears when the span actually has parameters.To support starting collapsed,
TraceSpanDrillInViewandAccordionTreePaneleach get an optionaldefaultCollapsedprop. The default isfalse, so every existing usage renders exactly as before.Note
This is the first of two steps. It makes tools and config visible in the overview. A follow-up will make the playground load these tools correctly when you open such a span.