fix(frontend): resolve every identifying trace ref when opening playground#4425
Merged
mmabrouk merged 4 commits intoMay 27, 2026
Merged
Conversation
…round
Opening a trace span in the playground only resolved when app.slug was
set without app.id, sent a hand-picked subset of refs, and overwrote
existing ids with the resolver's result. Spans that carried only
variant.slug, only rev.id, only rev.slug, or app.id without a revision
either silently opened the wrong app, dropped to a useless ephemeral
playground, or navigated to /apps/{id}/playground with no ?revisions=
(the page does not auto-pick a default revision).
This change broadens the resolver to fire whenever the trace gives us
any identifying ref the backend can scope a lookup from (any of
app/variant/revision x id/slug, or version when variant is also present).
The resolver sends every identifier the trace carries and only backfills
the ids that are missing - refs the trace supplied are authoritative and
never overwritten. The appDefault branch is removed; under the new rule
a successful resolve always returns both app.id and revision.id
together, so the URL we navigate to always carries ?revisions=.
Specifics:
- New resolveTraceRefs(refs, projectId) in playgroundController.ts.
Picks the smallest identifying ref per level (id ?? slug ?? version),
caches successful results with a 5-minute TTL, falls back to ephemeral
on null/error.
- openFromTraceAtom skips the round-trip when app.id and revision.id
are both already on the refs.
- retrieveWorkflowRevision accepts an optional workflowRef so callers
can resolve from variant-only or revision-only refs.
- hasAppReference in TraceTypeHeader now requires id or slug on the ref
(matching the resolver's gate) so the button enable predicate and the
resolver predicate agree on bare-version refs.
- Drops OpenFromTraceResult.appDefault from the union and the navigation
branch that depended on it.
Verified against the live /workflows/revisions/retrieve endpoint with a
probe that exercises 19 input combinations (matrix in
docs/design/playground-open-from-trace/plan.md). Every combination with
at least one id or slug resolves to the expected revision; bare-version
requests are correctly rejected by the backend (PR #4418 guard) and
fall through to ephemeral.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds click-time slug→UUID resolution for trace → Playground navigation: new retrieveWorkflowRevision API wrapper, TTL-cached resolver in playgroundController, async openFromTrace flow with backfill, stricter reference validation, and UI spinner/disable while resolving. ChangesTrace Reference Resolution for Playground Navigation
Sequence DiagramsequenceDiagram
participant TraceTypeHeader
participant openFromTraceAtom
participant resolveTraceRefs
participant retrieveWorkflowRevision
TraceTypeHeader->>openFromTraceAtom: setOpenInPlayground(activeSpan)
openFromTraceAtom->>resolveTraceRefs: resolveTraceRefs(refs, projectId) (if IDs missing)
resolveTraceRefs->>retrieveWorkflowRevision: POST /workflows/revisions/retrieve (id/slug/variant/revision)
retrieveWorkflowRevision-->>resolveTraceRefs: workflow_revision + parent slugs
resolveTraceRefs-->>openFromTraceAtom: {appId, revisionId}
openFromTraceAtom->>openFromTraceAtom: Backfill refs.application.id / refs.application_revision.id
openFromTraceAtom-->>TraceTypeHeader: Open result (appId/revisionId/entityId)
TraceTypeHeader->>TraceTypeHeader: navigate to app playground or open revision drawer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
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:XL
Contributor
Railway Preview Environment
Updated at 2026-05-27T08:10:03.100Z |
![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)
docs/design/playground-open-from-trace/plan.md (2)
72-82: ⚡ Quick winConsider using
interfacefor object shapes.The parameter types use inline object shapes. Per coding guidelines, TypeScript code should prefer
interfacefor defining object shapes. When implementing this signature, consider defining named interfaces for better reusability and clarity.Example:
interface WorkflowRef { id?: string slug?: string version?: string } interface RetrieveWorkflowRevisionParams { projectId: string workflowRef: WorkflowRef workflowRevisionRef?: WorkflowRef }As per coding guidelines: "Prefer
interfacefor defining object shapes in TypeScript".
25-47: 💤 Low valueAdd language identifier to fenced code block.
The fenced code block should specify a language for proper syntax highlighting and linting.
📝 Suggested improvement
-``` +```typescript 1. If references.application_revision.id is a UUID
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 45106b73-f9fe-4be3-b343-de09f6d53080
📒 Files selected for processing (10)
docs/design/playground-open-from-trace/plan.mddocs/design/playground-open-from-trace/research.mdweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceContent/components/TraceTypeHeader/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/store/openInPlayground.tsweb/packages/agenta-entities/src/workflow/api/api.tsweb/packages/agenta-entities/src/workflow/api/index.tsweb/packages/agenta-entities/src/workflow/core/schema.tsweb/packages/agenta-entities/src/workflow/index.tsweb/packages/agenta-playground/src/index.tsweb/packages/agenta-playground/src/state/controllers/playgroundController.ts
… naming
- `hasAppReference` now requires `id`/`slug` to be a non-empty string,
matching `asString` semantics in the playground resolver. Spans
carrying `{id: "", slug: ""}` previously enabled the Playground
button only to fall through to ephemeral after the resolver gated
them out.
- Rename `onlyRevVersion` → `hasNoIdentifyingRef` in both the
resolver and `retrieveWorkflowRevision` — the variable was true
whenever no identifying ref existed at any level, not specifically
when only a revision `version` was supplied. Comments updated to
match.
Extracts buildRefForResolver, hasAppReference, resolveTraceRefs, and related types from playgroundController into a dedicated module so the predicate the UI gate uses (in OSS TraceTypeHeader) and the one the resolver uses (inside resolveTraceRefs) live in one file — they structurally cannot drift apart now. Adds vitest infrastructure to @agenta/playground mirroring @agenta/entities (config, scripts, devDep) and unit tests covering: - buildRefForResolver: id > slug > version priority, empty-string rejection, never-mix invariant - hasAppReference: UI gate predicate across ag.references dict format and top-level references array; empty-id-or-slug rejection (matches resolver gate) - resolveTraceRefs: projectId guard, no-identifier guard, request shaping, mismatch verification, cache TTL behaviour, no-cache-on- failure invariant - retrieveWorkflowRevision: request URL/body/params shape, early- return paths, response validation failure handling 35 new tests in @agenta/playground (new vitest setup) and 10 in @agenta/entities (existing vitest setup) — all green.
…e ref resolver Two changes that the original PR description tagged as follow-ups, lifted into this PR now that the resolver is isolated in its own module with tests covering the contract: 1) retrieveWorkflowRevision swaps the hand-rolled axios.post for the Fern-generated `client.workflows.retrieveWorkflowRevision`. The request/response shape is identical (WorkflowRevisionRetrieveRequest maps 1:1) and zod validation stays at the boundary because Fern's compile-time types under-declare backend `extra="allow"` fields (artifact_slug, variant_slug). Aligns this function with the project's "all new API code uses @agentaai/api-client" rule. 2) resolveTraceRefs migrates its in-memory Map + manual TTL check to queryClient.fetchQuery with staleTime = TRACE_REF_RESOLUTION_TTL_MS and gcTime = 2× that. Failures (no-match, slug mismatch, axios error) are removed from the cache in the outer catch so subsequent calls retry rather than serving stale absence. A small TraceRefResolverFailure sentinel preserves the original "no warn on silent fallback, warn on diagnostic paths" behaviour without muddling the queryFn's contract. Tests adapt: entities mocks `@agenta/sdk` (Fern client) instead of axios; playground tests unchanged — sentinel design preserved all semantics so the cache TTL, no-cache-on-failure, and mismatch warn assertions still pass verbatim.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/packages/agenta-playground/tests/unit/traceRefResolution.test.ts (1)
337-345: ⚡ Quick winAdd a retry assertion for ID-omitted responses.
This test checks output mapping, but it should also verify that this path is not cached (second call should hit
retrieveWorkflowRevisionagain).Suggested extension
it("returns null result fields when the response omits ids", async () => { mockedRetrieve.mockResolvedValueOnce({ id: "", workflow_id: null, artifact_slug: "my-app", } as never) const out = await resolveTraceRefs({application: {slug: "my-app"}}, "proj-1") expect(out).toEqual({appId: null, revisionId: null}) + + mockedRetrieve.mockResolvedValueOnce(fakeRevision as never) + const retry = await resolveTraceRefs({application: {slug: "my-app"}}, "proj-1") + expect(retry).toEqual({appId: "workflow-uuid", revisionId: "rev-uuid"}) + expect(mockedRetrieve).toHaveBeenCalledTimes(2) })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 45206159-41df-4699-ab62-f8663454b0f4
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
web/oss/src/components/SharedDrawers/TraceDrawer/components/TraceContent/components/TraceTypeHeader/index.tsxweb/packages/agenta-entities/tests/unit/retrieveWorkflowRevision.test.tsweb/packages/agenta-playground/.gitignoreweb/packages/agenta-playground/package.jsonweb/packages/agenta-playground/src/index.tsweb/packages/agenta-playground/src/state/controllers/playgroundController.tsweb/packages/agenta-playground/src/state/controllers/traceRefResolution.tsweb/packages/agenta-playground/tests/unit/traceRefResolution.test.tsweb/packages/agenta-playground/vitest.config.ts
✅ Files skipped from review due to trivial changes (1)
- web/packages/agenta-playground/.gitignore
ardaerzin
approved these changes
May 26, 2026
ardaerzin
added a commit
that referenced
this pull request
May 26, 2026
Bundles fixes for the remaining four problems from issue #4426 (the trace→playground flow) on top of #4425's resolver. Problem 1 (slug-only side panel) shipped on the first commit of this branch. Problem 3 — evaluator spans open the graded app (playgroundController.ts): PR #4425's resolver backfills `refs.application_revision.id` for any span carrying app refs. Evaluator spans always carry the graded app's refs alongside the evaluator's, so the application_revision branch fired before the evaluator_revision branch and the user landed on the wrong entity. Extracts the branch-selection logic into a pure helper `selectOpenFromTraceBranch(refs, isEvaluatorSpan)` and gates the application branch on `!isEvaluatorSpan`. The helper is unit-tested in isolation so the regression cannot reappear without a test failure. Problem 2d — environment refs dropped (traceRefResolution.ts, resolvedTraceRefsAtom.ts, retrieveWorkflowRevision): Env refs (environment / environment_variant / environment_revision) are now extracted by `extractReferences`, included in `IDENTIFYING_REF_KEYS` so `hasAppReference` enables the Playground button for env-only traces, forwarded to `POST /workflows/revisions/ retrieve` (the Fern client already accepts these fields), and included in `buildResolvedTraceRefsKey` so the side panel atom resolves them too. End result: env-only traces open the deployed revision instead of disabling the button. Problem 2a/2c — ephemeral has no URI, config panel empty (store.ts createEphemeralWorkflow): Without `data.uri` the playground cannot fetch an OpenAPI schema and falls back to the empty "No configuration needed" state, hiding the parameters we already extract. The constructor now defaults the URI to `agenta:builtin:completion:v0` / `agenta:builtin:chat:v0` based on detected chat mode for non-evaluator ephemerals — same builtin URIs the runnable layer already uses. Evaluator ephemerals continue to take their URI from `deriveBuiltinUriFromSpanName` at the call site. Problem 2b — "Create" spawns a new app instead of forking the source (commit.ts createWorkflowFromEphemeralAtom): Reads `meta.sourceRef.type === "application"` and, when an app id is available, forks the source workflow as a new variant (seed v0 + data v1, mirroring `createWorkflowVariantAtom`) instead of calling `createWorkflow`. Falls back to the new-workflow path when no source id is available (third-party traces with slug-only refs that didn't resolve). This branch lacks dedicated unit tests because the function's dependency surface is heavy; manual QA covers the fork vs new-app decision. Tests - traceRefResolution.test.ts: 13 new cases — `hasAppReference` with env refs, `resolveTraceRefs` forwarding env refs (single / combined / variant / revision), `selectOpenFromTraceBranch` (5 branch scenarios including the issue-#4426 problem-3 regression). - resolvedTraceRefsAtom.test.ts: env refs included in the cache key and produce distinct keys from app-only refs. - retrieveWorkflowRevision.test.ts: forwards env refs in the request body and treats an env ref as identifying. - createEphemeralWorkflow.test.ts (new file): URI derivation for completion / chat / explicit / evaluator-without-uri cases, and parameter pass-through.
4 tasks
mmabrouk
merged commit 9f94286
into
feat/trace-observability-batch
31 of 32 checks passed
4 tasks
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.
Summary
Opening a trace span in the playground only resolved when
app.slugwas set withoutapp.id, sent a hand-picked subset of refs, and overwrote existing ids with the resolver's result. Spans that carried onlyvariant.slug, onlyrev.id, onlyrev.slug, orapp.idwithout a revision either silently opened the wrong app, dropped to a useless ephemeral playground, or navigated to/apps/{id}/playgroundwith no?revisions=(the page does not auto-pick a default revision).The resolver now fires whenever the trace gives us any identifying ref the backend can scope a lookup from (any of app/variant/revision × id/slug, or version when variant is also present). It sends every identifier the trace carries and only backfills the ids that are missing — refs the trace supplied are authoritative and never overwritten. The
appDefaultbranch is removed; under the new rule a successful resolve always returns both ids together, so the URL always carries?revisions=.Behavior matrix
The full 19-row matrix lives in
docs/design/playground-open-from-trace/plan.md. Highlights:app.idonly/apps/{id}/playground(no revision selected)?revisions=populated via resolverapp.slug+rev.idvariant.idonly?revisions=populatedrev.idonly/apps//playground?revisions=...URL?revisions=with app id from resolverrev.slugonly?revisions=populatedrev.versiononlySpecifics
resolveTraceRefs(refs, projectId)inplaygroundController.ts. Picks the smallest identifying ref per level (id ?? slug ?? version), caches successful results with a 5-minute TTL, falls back to ephemeral on null/error. Includes a defensive check that the responseartifact_slugmatches what we asked for when the request was slug-based.openFromTraceAtomskips the round-trip whenapp.idandrevision.idare both already on the refs.retrieveWorkflowRevisionaccepts an optionalworkflowRefso callers can resolve from variant-only or revision-only refs.hasAppReferenceinTraceTypeHeadernow requiresidorslugon the ref (matching the resolver's gate) so the button enable predicate and the resolver predicate agree on bare-versionrefs.OpenFromTraceResult.appDefaultfrom the union and the navigation branch that depended on it.Dependencies
Backend PR #4418 (already merged) is required for the bare-
version400 response that this PR's fallback path relies on. PR #4422 (already merged) extends those guardrails. The relaxation of rows 3 and 7 ({app + rev.version}returning the default variant's revision instead of 400) is a separate backend PR in progress — this PR works correctly with or without it (graceful 400 → ephemeral fallback today; exact revision after).Verified
/workflows/revisions/retrieveendpoint. Every combination with at least oneidorslugresolved to the expected n8n revision; bare-versionrequests were rejected as expected.pnpm lint-fixclean;tsc --noEmitclean for@agenta/playgroundand@agenta/entities.Follow-ups (not in this PR)
Mapcache toqueryClient.fetchQueryfor consistency with the rest of the data-fetching layer and to get invalidation for free. Real refactor; isolating it.TraceReferencesside panel can render working links for slug-only traces.Test plan
?revisions={uuid}in the URL.application_revision.id— resolves the app id and opens the revision.application_variant.slug— resolves and opens the variant's latest revision.application.slugno longer exists (delete the n8n workflow, click again within session, then beyond 5min) — falls back to ephemeral after TTL.application_revision.versionwith no id/slug.