[feat] Extract the compact trace row and the observability list shell - #5962
[feat] Extract the compact trace row and the observability list shell#5962ardaerzin wants to merge 1 commit into
Conversation
…ist shell WP5. Mobile needs to render a trace without a table, and the app already ships exactly one non-table trace presentation: the row inside the trace drawer's tree. This extracts it rather than designing a card that does not exist yet. TraceRow moves to @agenta/observability-ui, antd-free, with the span type glyph, the name with its error styling, and inline latency, cost and tokens read from the packaged span selectors. Both trace trees on desktop adopt it in this same commit, so there is one implementation rather than a copy per surface. The tree chrome around it (CustomTreeComponent, the settings popover) stays in the app, because it is desktop-only. The span-tree shaping goes to @agenta/observability/trace: filterTree, which narrows by name while keeping the ancestors that lead to a match, and the whole key-span filter with its rules. Both were app-local and both are pure, and mobile needs them to offer the same Key spans view. Six unit tests cover them, including the promote-past-wrappers behaviour, which is the part most likely to regress silently. ObservabilityList is the one genuinely new piece: the container behind every list-shaped observability surface. It owns paging, skeletons and the error and empty slots, and knows nothing about how a row looks, so traces and sessions can share paging without sharing a row. Paging is an IntersectionObserver on a sentinel rather than a scroll handler, so it costs nothing per frame and still works nested inside another scroller. oss/src/components/pages/observability/assets/utils.ts held only filterTree and is deleted rather than left as a shim, which OSS lint blocks anyway.
dosubot
Bot
added
the
size:L
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
the
refactoring
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe change moves trace filtering and span visibility utilities into Trace tree migration
Observability list
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🔵 Low · up to The shared observability list can stop loading additional pages after a query or filter changes without changing the item count, leaving users with incomplete results; the observer should be reset for each query state. A minor bounded memory-retention concern also remains for filtered trace rows, so merge is reasonable with explicit owner follow-up. Sequence Diagram(s)sequenceDiagram
participant TraceTree
participant filterTree
participant TraceRow
TraceTree->>filterTree: filter span tree by search text
filterTree-->>TraceTree: return filtered tree or null
TraceTree->>TraceRow: render span with traceTreeSettings metrics
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
Railway Preview Environment
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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
🧹 Nitpick comments (2)
web/packages/agenta-observability-ui/src/trace/TraceRow.tsx (1)
39-41: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid object-identity atom-family keys for filtered spans.
filterTreecreates new span objects for each search. These threeatomFamilyinstances retain one cached atom per object identity, so repeated searches can retain three atoms per cloned span. Use a stable span key or configure cache eviction.web/packages/agenta-observability/src/trace/spanTree.ts (1)
3-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShorten routine comments. Keep comments in the affected trace and list modules to one short line, or remove them when the code is self-explanatory; reserve longer comments for surprising constraints such as bugs, races, or ordering requirements.
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 46ea36eb-6b36-41a4-abe9-29f440f11967
📒 Files selected for processing (14)
web/oss/src/components/SharedDrawers/SessionDrawer/components/SessionTree/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceTree/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceTreeSettings/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceTreeSettings/types.tsweb/oss/src/components/pages/observability/assets/utils.tsweb/packages/agenta-observability-ui/src/index.tsweb/packages/agenta-observability-ui/src/list/ObservabilityList.tsxweb/packages/agenta-observability-ui/src/trace/TraceRow.tsxweb/packages/agenta-observability/package.jsonweb/packages/agenta-observability/src/index.tsweb/packages/agenta-observability/src/trace/index.tsweb/packages/agenta-observability/src/trace/spanTree.tsweb/packages/agenta-observability/src/trace/spanVisibility.tsweb/packages/agenta-observability/tests/unit/spanTree.test.ts
💤 Files with no reviewable changes (1)
- web/oss/src/components/pages/observability/assets/utils.ts
| ) | ||
| observer.observe(node) | ||
| return () => observer.disconnect() | ||
| }, [hasMore, isLoadingMore, rootMargin, items.length]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="web/packages/agenta-observability-ui/src/list/ObservabilityList.tsx"
printf '%s\n' '--- outline ---'
ast-grep outline "$file" || true
printf '%s\n' '--- file ---'
cat -n "$file"
printf '%s\n' '--- usages ---'
rg -n --glob '*.{ts,tsx}' 'ObservabilityList|loadMore|hasMore|isLoadingMore' web/packages/agenta-observability-ui web 2>/dev/null | head -300Repository: Agenta-AI/agenta
Length of output: 39315
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- direct imports and package references ---'
rg -n --glob '*.{ts,tsx,js,jsx,md,mdx}' \
'ObservabilityList|`@agenta/observability-ui`|observability-ui' . \
--glob '!**/node_modules/**' --glob '!**/dist/**' | head -300
printf '%s\n' '--- nearby pagination components ---'
cat -n web/packages/agenta-ui/src/components/ScrollSentinel.tsx
cat -n web/packages/agenta-observability-ui/package.json 2>/dev/null || true
printf '%s\n' '--- available tests and list files ---'
git ls-files 'web/packages/agenta-observability-ui' | sed -n '1,200p'Repository: Agenta-AI/agenta
Length of output: 20534
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- design contract for ObservabilityList ---'
sed -n '385,425p' docs/design/observability-packages/plan.md
printf '%s\n' '--- observability list-like consumers and filter state ---'
rg -n --glob '*.{ts,tsx}' \
'useObservability\(|useSessions\(|fetchMoreTraces|fetchMoreSessions|resetTracePages|reset.*Session|filter|search' \
web/oss/src/components/pages/observability web/oss/src/components/SharedDrawers \
| head -300
printf '%s\n' '--- standalone dependency and observer-behavior probe ---'
node - <<'JS'
const sameDeps = (a, b) =>
a.length === b.length && a.every((value, index) => Object.is(value, b[index]))
const previous = [true, false, "300px", 20]
const sameLengthReplacement = [true, false, "300px", 20]
const lengthChanged = [true, false, "300px", 21]
console.log(JSON.stringify({
sameLengthReplacementRerunsEffect: !sameDeps(previous, sameLengthReplacement),
lengthChangedRerunsEffect: !sameDeps(previous, lengthChanged),
sameTargetAlreadyIntersectingNeedsReobserve: true,
}))
JSRepository: Agenta-AI/agenta
Length of output: 35346
🌐 Web query:
IntersectionObserver observe initial notification threshold crossing callback remains intersecting target layout change same intersection ratio specification
💡 Result:
The IntersectionObserver API generates callback notifications based on two primary state changes for an observed target: the crossing of a threshold and the change in its intersection status (isIntersecting). According to the W3C specification [1][2][3], a callback is triggered if: 1. The target's isIntersecting boolean property changes (e.g., from false to true or vice versa) [3]. 2. The target's thresholdIndex changes [3]. The thresholdIndex is determined by comparing the current intersectionRatio against the thresholds array defined in the observer's configuration [1][2]. Regarding your specific scenario: Initial Notification: When you first call observe(), an initial notification is guaranteed to occur [4][5]. This is achieved because the internal state for an observed target is initialized with a previousThresholdIndex of -1 and a previousIsIntersecting of false, ensuring that the first processing step in the event loop will trigger a callback regardless of the target's actual initial intersection state [5]. Layout Changes with Constant Ratio: If an element's layout changes—such as a shift in its bounding box or the root's bounds—but the intersectionRatio remains constant, the callback will generally not be triggered, provided that neither the thresholdIndex nor the isIntersecting status has changed [3]. The thresholdIndex is a direct function of the intersectionRatio; therefore, if the ratio does not cross a new threshold value, the thresholdIndex remains the same [1][2]. In summary, the API is designed to notify you only when meaningful intersection changes occur (crossing a threshold or entering/exiting the root) [6][7]. It does not fire for layout changes that do not result in a change to the intersection state or a threshold boundary crossing [3]. If you require updates for every layout change, you would need to use a different mechanism, such as ResizeObserver or requestAnimationFrame, to monitor those changes manually.
Citations:
- 1: https://www.w3.org/TR/intersection-observer/
- 2: https://w3c.github.io/IntersectionObserver/
- 3: https://www.w3.org/TR/2019/WD-intersection-observer-20190531/
- 4: Incompatibilities between spec and implementations for observer events w3c/IntersectionObserver#222
- 5: Clarify IntersectionObserver behavior when an element is observed the first time w3c/IntersectionObserver#426
- 6: https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API
- 7: https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserver
Reset the paging observer when the query changes.
If a new query or filter returns the same number of items, the observer effect does not rerun while the sentinel remains intersecting. loadMore can then stall. Add a stable paginationKey prop and include it in the effect dependencies.
1 participant
Context
Mobile needs to render a trace, and a resizable ten-column grid is not the answer on a phone. But there is no card presentation of a trace anywhere in the repo: traces exist only as antd table rows.
There is one exception. The row inside the trace drawer's tree is a designed, shipping, non-table trace presentation. WP5 extracts that rather than inventing a card, which is also why this PR adds no new visual design.
Changes
TraceRowmoves to@agenta/observability-ui, antd-free: the span-type glyph, the span name with its error styling, and inline latency, cost and tokens read from the packaged span selectors. Both desktop trace trees adopt it in this same commit, so there is one implementation rather than a copy per surface. The tree chrome around it (CustomTreeComponent, the settings popover) stays in the app, because it is desktop-only.The span-tree shaping moves to
@agenta/observability/trace.filterTreenarrows a tree by span name while keeping every ancestor that leads to a match. The key-span filter comes across whole, with its rules intact. Both were app-local, both are pure, and mobile needs them to offer the same "Key spans" view desktop has.ObservabilityListis the one genuinely new piece. It is the container behind every list-shaped observability surface: it owns paging, skeletons, and the error and empty slots, and knows nothing about how a row looks. Traces and sessions can therefore share paging behaviour without sharing a row.Paging is an
IntersectionObserveron a sentinel after the last row rather than a scroll handler, so it costs nothing per frame and keeps working when the list is nested inside another scroller.oss/src/components/pages/observability/assets/utils.tsheld onlyfilterTree, so it is deleted rather than left as a shim, which OSS lint blocks anyway.Tests / notes
filterKeySpans, which is the part most likely to regress silently.@agenta/observabilityis 63 tests green.What to QA
The trace drawer's tree is the surface that changed. It should look and behave exactly as before.
main.