[refactor] Drop duplicate EvaluationRun type, align step-reference shape - #5466
Merged
Conversation
The playwright suites are owned and type-checked by the tests workspace, which holds the @playwright/test dependency; tests/manual scripts import state modules that no longer exist.
- Restore Parameter, CorrectAnswer and _EvaluationScenario to lib/Types.ts (removed by cleanup while consumers remained) - Re-export MetricColumnDefinition from the EvalRunDetails table barrel - TooltipButtonProps was renamed EnhancedButtonProps; statusMap moved to @agenta/entity-ui/variant - Synthesize full Parameter objects in UseApiContent
- useRunMetricData: type selection as the unwrapped value, not the atom - Webhook builders: narrow WebhookFormValues union before field access - filtersAtom: accept updater functions in the write signature - Organization settings: drop stale react-query v4 useErrorBoundary and migrate setQueriesData to the v5 filters shape - evaluations/utils: surface variantId from invocation metadata - PreviewTableRow: add index signature required by InfiniteTableRowBase
Mirrors the vetted WP-4e-2a in-place fixes from fe-chore/move-evals-to-packages (adapted to OSS import paths) plus new fixes for the component layer: - restore PreviewTestCase to lib/Types; re-export MetricProcessor/MetricScope - add "input" to EvaluationColumnKind (backend mapping emits it) and type the visibility-label column extension in buildPreviewColumns - recharts v3 API drift in EvaluatorMetricsChart (TooltipContentProps, tuple radius, MetricStripEntry contextual typing) - widen evaluationType to EvaluationRunKind; type query atoms as nullable - latent runtime bugs typed as-is per WP-4e-2a convention (applyAggregatesToRaw, metricProcessor ReferenceErrors, dead metric-group branches) - flagged, not fixed
…ant id - createPaginatedEntityStore: refreshAtom/actions.refresh accept an optional value or updater (bare set() still bumps the counter) - clears the 'Expected 0 arguments' cluster across testset modals and other consumers - EnvironmentStatus: variant.id optional; the component already guards it
…ing layers; oss tsc 347->105 Parallel per-area pass, behavior-preserving throughout (latent runtime bugs are typed as-is with NOTE comments per the WP-4e-2a convention, not silently fixed): - TraceSpanNode OSS<->entities dual-type: aligned at every crossing with documented boundary casts (16 errors -> 0); annotations field added to the OSS node (drawer stores attach it at runtime) - SharedDrawers: broken SessionDrawerButton imports repaired, JSON-schema access typed, null-safety in SelectEvaluators/AnnotateDrawer, drawer payload/ref/label types aligned with runtime - observability: extended-column types for custom antd props, TraceRow/ SessionRow InfiniteTableRowBase conformance, traceTabsAtom updater support, ag-attributes selector typing at the producer - playground/url-state: removed drifted local duplicates of package types, eagerAtom->atom where deps are sync, modal/store prop alignment - onboarding/testsets/org: updater-widened widget UI atom, tour placement vocabulary fix, OnboardingLoader next/dynamic compat, org provider API types matched to the backend wire shape (settings/flags dicts) - EvalRunDetails remainder: WP-4e-2a-vetted atom fixes adapted, recharts v3 formatter/content signatures, stale import paths repointed Flagged for triage (typed as-is): AddToTestsetDrawer trace draft discard/ update call missing molecule actions; orphaned SessionDrawerButton
…atterns; oss tsc 105->82 - InfiniteVirtualTable: canonical ExtendedColumnType exported from the barrel (three local extended-column copies repointed as aliases); pure read fn for columnHiddenKeys (write path reconciles versions); memo generic preservation; onRow/getRowProps/rowSelection.fixed aligned with antd 6 - antd v6 PopoverStylesType 'body' drops typed as-is (2 sites, existing pattern) - React 19: JSX.Element -> ReactElement in RequireWorkflowKind - AgentaNodeDTO: trace_id/span_id added (backend SpanDTO sends them) - Org.default_workspace: list endpoint strips it - both lookups are latent dead code, typed as-is with comments - FiltersPreview: operatorLabel is a display string, not the operator union
…windowing export - DrillInUIContext: EditorProvider/SharedEditor slots typed with the real component prop types instead of a hand-written subset (root cause of the OSS provider assignment errors) - InfiniteVirtualTable: rowSelection.fixed accepts antd FixedType; locale accepted (not yet forwarded - flagged); Editor barrel exports CodeLanguage - workflow barrel exports WorkflowRevisionWindowing
Wave-2 parallel pass over the final ~105 OSS + 11 EE-only errors, behavior- preserving throughout (latent bugs typed as-is with NOTE comments per the WP-4e-2a convention): - Evaluators/Evaluations: generic evaluator filtering, chart datum typing - pages/evaluations: NewEvaluation modal props retyped to entities-package shapes (stale legacy Types imports dropped); antd 6 tabPlacement vocabulary - Testcases/Testsets/Deployments: canonical ExtendedColumnType adoption, dataset-store variance via Pick<...,'hooks'> - DrillInView/EditorViews: TMode narrowing via consts, Format|CodeLanguage state union, html format menu typing - app-shell misc: services/state/pages sweep with backend-verified API types - EE Billing + misc EE-only files Latent bugs surfaced and typed as-is (chips filed where actionable): workspace rename sends invalid org PATCH body; SessionInspector reads status.code the backend never sends; axios.isCancel dead on created instance; invite accept can interpolate undefined ids
Both apps are at zero tsc errors; flip ignoreBuildErrors to false so next build guards the baseline. Verified: full oss and ee builds pass with the gate on.
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.
…e shape usePreviewEvaluations declared its own EvaluationRun while @agenta/entities/evaluationRun already had the canonical zod-derived one. The local copy over-declared requiredness (name/description/status/meta/flags as non-null) where the schema says the backend can send null, so #5464 had to hand-add updated_at to keep it current. Deleted it; the two importers and the hook now take the type from the package. Swapping surfaced drift inside @agenta/entities itself: the step-reference shape was redeclared three times with different types. The core zod schema has {id, slug?: string|null, version?: number|null}, while the ETL RunStep said slug?: string and version?: STRING, and ColumnGroup.refs said id?: string. Both now reference the exported EvaluationRunStepReference from the validated schema. CandidateRun.status widened to match. Scoped out, with reasons: - PreviewTestset is NOT the entities Testset (different endpoint shape: it carries slug plus embedded data.testcases; entities Testset is lean metadata). PreviewTestCase does match Fern TestcaseOutput, but entities has no canonical testcase entity type to route it through yet. - OrganizationProvider has no entities domain at all, and the response settings is a free-form dict while Fern's SsoProviderSettingsDto models create-input. Creating a domain for one interface is disproportionate.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesThe PR centralizes Evaluation run type consolidation
Estimated code review effort: 2 (Simple) | ~10 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 |
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Conflicts were all in the same direction: release/v0.106.1 carries the pre-refactor span typing (#5464's boundary casts and narrow prop types) that this branch removes. Resolved to this branch's side in all six files. - services/tracing/types: keep the entities-backed TraceSpanNode; the release-side annotations field is already on it - OverviewTabItem: drop the EntityTraceSpan cast and its now-unused import - traceDrawerStore: SpanLink/TracesResponse come from @agenta/entities/trace - NodeNameCell/TimestampCell/formattedTimestampAtomFamily: keep the widened nullable types oss and ee tsc both at 0 errors after the merge.
ardaerzin
marked this pull request as ready for review
July 29, 2026 10:01
dosubot
Bot
added
size:M
Review follow-up on the export ETL row contract. ScannedExportRow hand-redeclared five span fields, and this branch widened all five to `| null` to make TraceSpanNode assignable. Only three needed it: the schema declares trace_id and span_id required and non-null, so widening those two moved the contract away from the source of truth in the one file this PR was meant to unify. Pick the four span fields from TraceSpan instead. Same assignability for the caller, but the contract can no longer drift. That widening was what made the dedup collapse CodeRabbit flagged expressible: rows missing both ids all hashed to the fallback `trace_id:parent_id:start_time`, so a page of them deduped down to one row. Not reachable from the tracing API (zod validates both ids at the boundary), but the fallback was wrong on its own terms — it also collapsed sibling spans sharing a parent and start_time. Dedup may only drop what it can prove is a repeat, so a row with no identity is now passed through undeduped: for an export, a duplicate row beats a dropped one. Also shortens the span-types comment to the one-line repo rule. Test added for the no-identity path; it fails on the old key (1 row, not 3).
Contributor
Railway Preview Environment
|
ardaerzin
changed the base branch from
ts-chore/unify-span-types
to
release/v0.106.1
July 29, 2026 10:53
dosubot
Bot
added
size:L
dosubot
Bot
added
size:M
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
1 participant
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
usePreviewEvaluationsdeclared its ownEvaluationRuninterface while@agenta/entities/evaluationRunalready had the canonical zod-derived one for the same entity. The local copy declaredname,description,status,metaandflagsas always-present, but the schema (which validates the actual payload) says the backend can send null for all of them. That gap is why #5464 had to hand-addupdated_atto the local copy to keep it current, while the entities type already had it.Changes
The local interface is deleted. Its two importers and the hook take
EvaluationRunfrom@agenta/entities/evaluationRun.CandidateRun.statusinCompareRunsMenuwas widened tostring | nullto match.Swapping surfaced drift inside
@agenta/entitiesitself. The step-reference shape is declared in three places with three different types:The core schema is the boundary-validated truth, so it now exports
EvaluationRunStepReferenceand both ETL declarations reference it instead of redeclaring it.Scoped out, and why
The other two candidates in the original list did not survive verification. Neither is a swap I would make without changing behavior or inventing structure.
PreviewTestsetis not the entitiesTestset. They describe different endpoints.PreviewTestsetcarriesslugplus an embeddeddata.testcases; the entitiesTestsetis lean metadata (id,name,description?,project_id?). Pointing one at the other would silently drop fields the UI reads.PreviewTestCasedoes match Fern'sTestcaseOutput(Fern's is a superset, all-optional). Butweb/osscannot import the Fern client directly, and the entitiestestcasedomain has no canonical testcase entity type to route it through. Adding one is reasonable follow-up work, not a drive-by.OrganizationProviderhas no entities domain at all. The EE response'ssettingsis a free-form dict, while Fern'sSsoProviderSettingsDtomodels the create-input (requiredclient_id/issuer_url). Creating a whole entities domain for one settings interface is disproportionate to the drift risk.Tests / notes
pnpm --filter @agenta/oss exec tsc --noEmitandpnpm --filter @agenta/ee exec tsc --noEmit: 0 errors each.pnpm turbo run build --filter=@agenta/entitiesandlint --filter=@agenta/oss: clean.What to QA
Nothing user-visible. If you want a smoke check: the evaluation runs table and the compare-runs menu still list runs with their status and created date, and the eval run details table still renders columns (the step-reference type feeds column grouping).