[refactor] Single source of truth for trace span types#5465
Open
ardaerzin wants to merge 1 commit into
Open
Conversation
The OSS tracing module declared its own TraceSpan/TraceSpanNode plus four TypeScript enums that mirrored the zod schemas in @agenta/entities. The two were structurally equivalent but nominally incompatible, so every crossing needed a cast (7 of them, added in #5464 as documented scar tissue). - oss/src/services/tracing/types now owns only TraceSpanNode (entities node plus the annotations the drawer stores attach) and GenerationDashboardData; consumers import span types from @agenta/entities/trace directly, per the app-layer no-re-export rule - 25 enum value usages become string literals; StatusCode values were already identical, SpanCategory differed only in the catch-all - all 7 boundary casts removed Fixes a live crash: AGE-3788 canonicalised the backend catch-all from "undefined" to "unknown", but spanTypeStyles was still keyed "undefined" and both consumers destructure the lookup, so any span typed "unknown" threw a TypeError in the observability table and trace tree. The record is now keyed by SpanCategory (exhaustive, so a new backend category fails the build) with a fallback at both call sites. Nullability now matches the payload: entities declares | null where the backend sends null, so NodeNameCell/StatusRenderer/TimestampCell props and ScannedExportRow were widened rather than cast.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughTracing types are sourced from shared entities, OSS extensions are retained for annotations, and redundant casts are removed across trace conversion and drawer flows. Observability components now handle literal status/category values and nullable fields. ETL scanned-row types accept optional or null values. ChangesTracing type alignment
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
This was referenced Jul 23, 2026
ardaerzin
marked this pull request as ready for review
July 23, 2026 19:34
dosubot
Bot
added
size:L
Contributor
Author
|
@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/oss/src/services/tracing/types/index.ts (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShorten the source-of-truth comment.
This three-line explanation exceeds the repository rule for in-code comments; keep it to one short line or move the rationale to documentation.
As per coding guidelines, in-code comments must be at most one short line unless they document a genuinely surprising constraint.
Suggested wording
-// Span types have a single source of truth: the zod schemas in `@agenta/entities`, -// which validate the backend payload at the boundary. Import them from -// `@agenta/entities/trace` directly; this module owns only the OSS-only additions. +// Span types come from `@agenta/entities`; this module adds OSS annotations.Source: Coding guidelines
web/oss/src/components/pages/observability/assets/constants.ts (1)
1-3: 📐 Maintainability & Code Quality | 🔵 TrivialVerify the required frontend lint-fix command.
Before committing this frontend cohort, run
pnpm lint-fixfromweb; the supplied context does not confirm that exact command. As per coding guidelines, frontend changes must runpnpm lint-fixfromweb.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d6f21bdb-d182-4f40-b36c-46c0492e89b2
📒 Files selected for processing (19)
web/oss/src/components/EvalRunDetails/atoms/traces.tsweb/oss/src/components/SharedDrawers/SessionDrawer/store/sessionDrawerStore.tsweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceContent/components/OverviewTabItem/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceContent/components/TraceTypeHeader/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceHeader/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceTree/assets/spanVisibility.tsweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceTree/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/store/openInPlayground.tsweb/oss/src/components/SharedDrawers/TraceDrawer/store/traceDrawerStore.tsweb/oss/src/components/pages/observability/assets/constants.tsweb/oss/src/components/pages/observability/assets/exportUtils.tsweb/oss/src/components/pages/observability/components/AvatarTreeContent.tsxweb/oss/src/components/pages/observability/components/NodeNameCell.tsxweb/oss/src/components/pages/observability/components/StatusRenderer.tsxweb/oss/src/components/pages/observability/components/TimestampCell.tsxweb/oss/src/services/tracing/types/index.tsweb/oss/src/state/newObservability/atoms/queries.tsweb/oss/src/state/newObservability/atoms/queryHelpers.tsweb/packages/agenta-entities/src/trace/etl/exportMatchingTraces.ts
Comment on lines
+36
to
+40
| trace_id?: string | null | ||
| span_id?: string | null | ||
| parent_id?: string | null | ||
| start_time?: string | number | null | ||
| children?: readonly ScannedExportRow[] | null |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline web/packages/agenta-entities/src/trace/etl/exportMatchingTraces.ts --items all --type function --match 'defaultSelectKey|exportMatchingTraces'
rg -n -C4 '\bdefaultSelectKey\b|new Map|new Set|selectKey' \
web/packages/agenta-entities/src/trace/etl/exportMatchingTraces.ts \
web/packages/agenta-entities/tests/unit/export-matching-traces.test.tsRepository: Agenta-AI/agenta
Length of output: 8255
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file size =="
wc -l web/packages/agenta-entities/src/trace/etl/exportMatchingTraces.ts
echo
echo "== relevant source =="
sed -n '1,120p' web/packages/agenta-entities/src/trace/etl/exportMatchingTraces.ts
echo
echo "== test relevant sections =="
sed -n '300,380p' web/packages/agenta-entities/tests/unit/export-matching-traces.test.ts
echo
echo "== type definitions and call sites =="
rg -n -C3 'type ScannedExportRow|interface ScannedExportRow|exportMatchingTraces\(' web/packages/agenta-entities/src web/packages/agenta-entities/tests/unit
echo
echo "== deterministic fallback key probe =="
node - <<'JS'
const rows = [
{},
{trace_id: null},
{trace_id: null, parent_id: null},
{trace_id: null, parent_id: null, start_time: null},
{parent_id: "p", start_time: 1},
];
for (const trace of rows) {
const fallback = `${trace.trace_id ?? ""}:${trace.parent_id ?? ""}:${trace.start_time ?? ""}`;
console.log(JSON.stringify(trace), '=>', fallback);
}
JSRepository: Agenta-AI/agenta
Length of output: 9329
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ScannedExportRow definitions across repo =="
rg -n -C2 'export interface ScannedExportRow|type ScannedExportRow|interface ScannedExportRow|ScannedExportRow' .
echo
echo "== data shaping and mapping around scan/export matching paths =="
rg -n -C3 'scannedExport|scanned.*row|trace_id|span_id|parent_id|start_time' web/packages web/src -g '*.ts' -g '*.tsx' | head -n 300Repository: Agenta-AI/agenta
Length of output: 36601
Avoid collapsing export rows when identifiers are missing.
The pipeline dedups with defaultSelectKey, but multiple rows with no selected trace_id, parent_id, or start_time all produce the same fallback key (::), so only the first such row is emitted. Use a stable per-row fallback key / index when the primary composite identity is incomplete.
ashrafchowdury
approved these changes
Jul 24, 2026
Contributor
|
Web build is failing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
A trace span was described by two type definitions: hand-written interfaces and TypeScript enums in
oss/src/services/tracing/types, and the zod schemas in@agenta/entities/tracethat actually validate the backend payload. They were structurally equivalent but nominally incompatible, so every place data crossed between them needed a cast. PR #5464 added seven such casts with// same backend span shapecomments. They were safe, but they meant a real backend shape change could pass through untyped.They had also already drifted. AGE-3788 canonicalised the backend catch-all category from
"undefined"to"unknown", and the entities schema followed. The OSS enum did not.Changes
@agenta/entities/traceis now the single source. The OSS module keeps only what the package cannot own:TraceSpanNode(the entities node plus theannotationsthe drawer stores attach at runtime, whoseAnnotationDtolives in the app layer) andGenerationDashboardData. Consumers import span types from the package directly rather than through a forwarding module, which is what the app-layer no-re-export lint rule asks for.The 25 enum value usages became string literals.
StatusCode's values were already byte-identical between the two definitions, andSpanCategorydiffered only in that catch-all. All seven boundary casts are gone.The drift was a live crash
spanTypeStyleswas keyed"undefined"and both consumers destructure the lookup:A span with
span_type: "unknown"misses the record, and destructuringundefinedthrows. That path reaches the observability table (NodeNameCell) and the trace tree (AvatarTreeContent). The record is now keyed bySpanCategory, so it is exhaustive and a new backend category fails the build, and both call sites fall back instead of throwing.Nullability
The entities schemas declare
| nullwhere the backend sends null. Rather than cast that away, the consumers that were under-declaring it were widened:NodeNameCell,StatusRenderer,TimestampCelland its atom, andScannedExportRowin the ETL pipeline (which only reads those fields).Tests / notes
pnpm --filter @agenta/oss exec tsc --noEmitandpnpm --filter @agenta/ee exec tsc --noEmit: 0 errors each.pnpm turbo run build --filter=@agenta/entitiesandpnpm turbo run lint --filter=@agenta/oss: clean.ts-chore/fix-tsc-issues.What to QA
ag.typeset (so the backend reports theunknowncategory), the observability table renders it with the generic icon instead of crashing. That is the bug this fixes.