feat(agents): playground perf, chat UX, and connection-flow fixes - #5145
Conversation
|
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:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds deferred request loading and persistence, agent-aware playground initialization, a session-based chat UI, trace summary queries, OAuth completion identity propagation, and filtering for deferred client-tool results. ChangesPlatform data loading and persistence
Agent playground and chat
Trace and integration flows
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
plansQueryAtom and rolesQueryAtom were enabled as soon as the session existed, so they fired before the profile resolved and the axios interceptor had the user + project it needs — the request went out and was immediately aborted. Gate `enabled` on user id + projectId as well so each query fires once, only when it can actually succeed.
…gates Stop firing project-wide list/catalog queries on plain page loads; defer them behind gates that open only when a feature needs the data. - Evaluator revision fan-out (POST /workflows/revisions/query): one-way evaluatorEnrichmentActivatedAtom gate; enrichment atoms return stable empties until a picker/switcher activates. - Evaluator catalog (GET /evaluators/catalog/templates): dropdown body mounts only when popover opens; playground + revision drawer gate on the is_evaluator flag (builtin apps share the agenta:builtin: URI prefix). - Current workflow/app by id: workflowDetailQueryAtomFamily + useCurrentAppLite resolve one artifact by id instead of listing the whole apps/evaluator catalog; dedupe duplicate /workflows/query calls. - Sidebar: switcher lists read EMPTY_EVALUATORS_ATOM until opened; suppress recent-evaluator type-tag flash during workflow resolution. - Browse-mode pickers read a stable empty list atom in scoped (app) mode. - Annotation drawer reads empty evaluator refs while closed. - Projects/webhooks: drop workspaceId/projectId params (backend scopes from session).
22b298c to
5862651
Compare
The playground drill-in provider (OSSdrillInUIProvider) is always mounted in OSSPlaygroundShell and eagerly subscribed to the project-wide workflow list (apps + evaluators) and the evaluator catalog via useWorkflowReferenceBridge — a big-agents-only feature (workflow-as-tool references) that main never had. So on big-agents those queries fired on every playground load regardless of the list/catalog lazy-gates, since one eager subscriber mounts the shared atoms and makes the other gates no-ops. Gate the bridge behind a one-way activation latch: `workflows`/`workflowsLoading` and the evaluator-catalog name map stay dormant until `activate()` is called — on reference-picker open (AgentTemplateControl) or when an existing reference is displayed (ReferenceToolFormView). A plain playground load with no workflow references now pulls neither the workflow list nor the catalog.
Move the Model & harness draft useModelHarness into a ModelHarnessSectionDrawerBody that only mounts while the section drawer is open (SectionDrawer destroyOnClose), and extract the app-trigger provider groups into AppTriggerProviderGroups that fetches the connections + ~90-app catalog only when there are app subscriptions to decorate. Both previously fired on every agent-config render with the drawer/section collapsed.
Disk-backed SWR (localStorage seed + one background revalidate) for the static
agent-template schema and harness catalog, per-revision inspect (bounded LRU, the reload
long pole), and a {workflowId: type} map so playgroundEarlyAgentStateAtom knows agent-ness
synchronously on reload. Revalidations that serve from disk are demoted to low network
priority (fetch-adapter Fetch Priority hint) so they don't compete with the critical-path
queries. Versioned cache keys allow hard invalidation on response-shape changes.
Add playgroundEarlyAgentStateAtom (app-id keyed, backed by the persisted type map) so the playground commits to the agent layout up front instead of defaulting to non-agent and flipping once the revision's is_agent flag loads. Gate the page-header eval chrome, the config-panel header (variant selector / view-type), the loading shell, and the MainLayout split on this signal, staying neutral until agent-ness is confirmed (settled) so neither the eval stack nor the prompt config header flashes on an agent reload.
…aterfall Mount AgentCatalogPrefetcher on the playground when it's an agent (onboarding, or the early signal), warming the agent-template schema + harness catalog alongside the revision/inspect waterfall. On a cold first load the schema no longer fetches last (after inspect resolves its ref); on a warm reload it just kicks the background revalidate a little earlier.
Hydrate per-run trace summaries in the background without competing with render-critical traffic. Adds a low-priority traces client (withLowPriorityFetch / getLowPriorityTracesClient sending the priority: "low" fetch hint) and a lighter traceSummaryQueryAtomFamily that the loadable controller reads to derive a run's error/root-span from the summary instead of the full trace. A just-finished run's trace may not be ingested yet, so markTraceAsFresh/isTraceFresh give fresh traces an aggressive not-found retry grace (5x vs once), marked at run-finish in the agent chat panel and execution runners. createBatchFetcher gains an idle flush mode (requestIdleCallback) so background-hydration batches coalesce and yield to render.
…riority The tool connections/catalog, trigger subscriptions/schedules, deploy environments, and build-kit overlay all fire on agent-playground load but none are on the critical path to first interactivity, so they shouldn't compete at High priority with the config/chat queries. Add low-priority variants of the Fern tools/workflows clients (getLowPriority*Client, reusing withLowPriorityFetch) and thread a lowPriority option through the axios fetchers via lowPriorityWhenCached, then send all six with the priority: "low" fetch hint. Low priority only bites under contention, so pages where this data is primary are unaffected.
…teway errors
The /billing/subscription, /billing/{catalog,pricing}, /access/{plans,roles} queries are
entitlement bootstrap that gate no render-critical UI, yet fired at High priority on every
load — and a degraded billing service 502-ing retried 3x at High (0.5-4s each), competing
with the config/chat critical path. Send them with the priority: "low" hint and share a
retry policy that fails fast on gateway errors (502/503/504) instead of hammering — the
entitlements fall back to hobby/free anyway.
…ound critical path On an agent-playground load, only the by-id workflow detail, the current-app latest revision, and the current-app revision detail are needed for first paint. The sidebar apps list (queryWorkflows), the sidebar prompt/agent-split batch that fetches every app's latest revision for the is_agent badge (fetchWorkflowsBatch), and the variant-picker label query (queryWorkflowVariants) were all firing High and competing with that critical path. Thread a lowPriority option through the shared functions and pass it only at those non-critical call sites. The agent-flags batch still primes the per-app caches, so the critical current-app fetch can share it. (Left the per-revision detail batch's comparison wave for a dedup follow-up.)
…isions list On a cold playground first paint the displayed revision was fetched twice: the revisions-by-workflow list fetches it (and primes the per-revision detail cache under the same key), while workflowQueryAtomFamily races ahead and fires its own by-id round-trip. Before that standalone fetch, await any in-flight revisions-by-workflow query and re-check the primed cache, so the revision resolves from the list instead. Best-effort and wrapped in try/catch — any miss or error falls through to the direct fetch, so the molecule's data path is never at risk.
…r idle
The access/plans, access/roles, billing/{subscription,catalog,pricing} queries are entitlement
chrome — never needed for first paint, and their permission/entitlement consumers already default
to not-ready until loaded. Add an idleReadyAtom (flips true on the first requestIdleCallback, ~2s
timeout fallback, then sticky) and gate these queries' enabled on it, so they fire once the browser
has a spare moment instead of joining the load burst. Complements the earlier priority demotion:
demote reorders, this keeps them out of the concurrent flood that saturates a capacity-limited backend.
On a cold load with no cached workspace->org mapping, selectedOrgQueryAtom first keys by the
workspace id, resolves the org via the project, and fetches the org; caching the workspace->org
pair then lets selectedOrgIdAtom resolve the org id, so the atom re-keys to ["selectedOrg", orgId]
and refetches the same org — two identical /organizations/{id} round trips on load. Seed the
org-id-keyed cache from the first fetch so the re-keyed query (refetchOnMount:false) and
resolveWorkspaceIdForOrg's getQueryData fast-path serve it from cache.
Hold the agent panes' real shape while they load instead of a blank canvas. AgentChatSkeleton mirrors the chat layout (session tabs, transcript turns, composer) and fills the two gaps — the revision still resolving the agent flag, and the lazy AgentChatPanel (AI SDK) chunk loading — as the dynamic-import fallback and the pending agent-generation host. AgentConfigSkeleton mirrors the config section-row list (Model & harness, Instructions, Tools, MCP, Skills, Triggers, Advanced), surfaced via a new loadingFallback prop on PlaygroundConfigSection so the panel shows a layout-matched skeleton instead of the generic pulse boxes.
A restored turn's first-seen timestamp is the reload moment, not the turn's send time — stamping it made old turns read "just now" until (or forever if) their trace loaded. Track restored ids and skip stamping them, so their timestamp slot holds a placeholder until the real trace time arrives (settled-with-no-trace shows nothing, never a wrong time). Likewise the per-message metrics: usage renders immediately while only the latency slot waits on the trace, held by a fixed-size Skeleton placeholder so the row neither shifts nor blanks known data.
…t-optimizations-onbig # Conflicts: # web/packages/agenta-shared/src/api/index.ts
ardaerzin
changed the title
|
@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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/oss/src/components/AgentChatSlice/components/AgentChatSkeleton.tsx (1)
1-63: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPrettier format check is failing in CI.
The pipeline logs show
pnpm run format(prettier --check "**/*.{js,jsx,cjs,mjs,ts,tsx,cts,mts,mdx}") failing. Runpnpm lint-fixin thewebfolder and commit the formatted output before merging.As per coding guidelines: "Frontend changes require running
pnpm lint-fixwithin thewebfolder before committing."Sources: Coding guidelines, Pipeline failures
🧹 Nitpick comments (7)
web/packages/agenta-entities/src/workflow/state/store.ts (1)
1124-1147: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid relying on
query.promisehere — TanStack Query v5.90.21 exposes it, but only as an experimental/internal API, so this dedup path is brittle across upgrades. A supported await path would keep the optimization stable.web/ee/src/components/Scripts/assets/CloudScripts.tsx (1)
13-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove commented-out Crisp code and orphaned explanatory comments.
Commenting out code rather than removing it is a code smell. The explanatory comments at lines 23–32 now describe behavior that no longer exists in active code, which is confusing. Additionally,
useAppTheme()at line 11 still runs butappThemeis only referenced in the commented-out blocks, causing unnecessary re-renders on theme changes.If Crisp removal is permanent, remove the commented-out code, the orphaned comments, and the
useAppTheme()call. If temporary, add a TODO explaining why and when it should be re-enabled.♻️ Proposed cleanup
const CloudScripts = () => { - const {appTheme} = useAppTheme() - - // useEffect(() => { - // const isCrispEnabled = !!getEnv("NEXT_PUBLIC_CRISP_WEBSITE_ID") - - // if (!isCrispEnabled) { - // return - // } - - // Crisp.configure(getEnv("NEXT_PUBLIC_CRISP_WEBSITE_ID")) - // }, []) - - // The Crisp chatbox renders in its own cross-origin iframe, so we can't style - // its light/dark useCrispChat from our CSS, and crisp-sdk-web exposes no runtime - // light/dark toggle (only the accent color via setColorTheme). Darken the - // accent in dark mode so the launcher/accent reads less out-of-place; light - // restores the dashboard's "default" accent. - // - // TODO(dark-mode): for the chat WINDOW itself to render dark, enable dark mode - // in the Crisp dashboard (Settings → Chatbox → Appearance). That follows the - // visitor's *system* color scheme — the SDK has no API to bind it to our - // in-app theme toggle, so this accent tweak is the only code-side lever. - // useEffect(() => { - // const isCrispEnabled = !!getEnv("NEXT_PUBLIC_CRISP_WEBSITE_ID") - - // if (!isCrispEnabled) { - // return - // } - - // Crisp.setColorTheme( - // appTheme === ThemeMode.Dark ? ChatboxColors.Black : ChatboxColors.Default, - // ) - // }, [appTheme]) return (web/packages/agenta-entities/src/loadable/controller.ts (2)
224-245: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate trace-derived output logic across two atom families.
getDerivedOutputValuesinsideconnectedRowsAtomFamily(lines 224-245) and the body ofderivedOutputValuesAtomFamily(lines 1920-1945) contain the identical rootSpan-extraction +extractAgData+ fallback block. This PR touches both copies in lockstep to swap totraceSummaryQueryAtomFamily, which is exactly the kind of duplication that's easy to let drift on the next change.Consider extracting a shared helper, e.g.
getDataSourceForOutputMapping(get, executionResult), and calling it from both atom families.♻️ Proposed extraction
+const getOutputMappingDataSource = ( + get: Getter, + executionResult: {traceId?: string; output?: unknown}, +): Record<string, unknown> | null => { + let dataSource: Record<string, unknown> | null = null + if (executionResult.traceId) { + const rootSpan = get(traceSummaryQueryAtomFamily(executionResult.traceId)).data?.rootSpan + if (rootSpan) { + const agData = extractAgData(rootSpan) + if (agData && Object.keys(agData).length > 0) { + dataSource = {data: agData} + } + } + } + if (!dataSource && executionResult.output && typeof executionResult.output === "object") { + dataSource = executionResult.output as Record<string, unknown> + } + return dataSource +}Then both call sites reduce to
const dataSource = getOutputMappingDataSource(get, executionResult).Also applies to: 1920-1945
1626-1660: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
getTraceErrorFromSpansduplicates the traversal intent ofgetTraceErrorFromResponse.Both walk spans looking for the first non-tool run-failure span and extract its message; one recurses a tree, the other iterates a flat list. Since they already share
isRunFailureSpanType/spanErrorMessage, this is a minor, low-risk duplication (different input shapes justify separate functions), but worth a one-line comment noting the relationship if kept as-is.web/oss/src/components/Playground/Components/PlaygroundVariantConfig/assets/AgentConfigSkeleton.tsx (1)
27-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
clsxfor conditional className instead of string concatenation.Sibling files in this cohort (
PlaygroundVariantConfig/index.tsx,PlaygroundConfigSection.tsx) useclsxfor conditional classNames; this file mixes plain string concatenation.♻️ Proposed refactor
+import clsx from "clsx" + const AgentConfigSkeleton = () => ( <div className="flex flex-col px-4" aria-busy aria-label="Loading agent configuration"> {ROWS.map((row, i) => ( <div key={i} - className={ - "flex items-center gap-3 py-6" + - (i < ROWS.length - 1 - ? " border-0 border-b border-solid border-[var(--ag-rgba-051729-06)]" - : "") - } + className={clsx( + "flex items-center gap-3 py-6", + i < ROWS.length - 1 && + "border-0 border-b border-solid border-[var(--ag-rgba-051729-06)]", + )} >web/oss/src/components/Playground/Components/PlaygroundVariantConfig/assets/PlaygroundVariantConfigHeader.tsx (1)
101-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate agent-header-mode formula.
isAgentEffective/showAgentHeaderhere is computed with the exact same three inputs (isAgentModeAtomFamily,playgroundEarlyAgentStateAtom, workflow queryisPending) asisAgentHeaderModeinPlaygroundVariantConfig/index.tsx(lines 71-78). See the consolidated refactor suggestion there.web/oss/src/components/Playground/Components/PlaygroundVariantConfig/index.tsx (1)
71-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared
useAgentHeaderMode/atom to avoid duplicated agent-loading logic.
isAgentHeaderModehere andshowAgentHeaderinPlaygroundVariantConfigHeader.tsx(lines 101-110) are computed with the exact same formula and inputs (isAgentModeAtomFamily(variantId),playgroundEarlyAgentStateAtom, and the workflow query'sisPending). Sinceindex.tsxuses this flag to decide what to pass asextraActionsto the header, and the header independently recomputes the same flag to decide its own rendering, the two must stay manually in sync — a future edit to one without the other could desync the header's "Configuration" title from theextraActionsit's given.Consider extracting this into a single derived atom or hook (e.g. alongside
playgroundEarlyAgentStateAtomin@/oss/state/workflow) and consuming it from both files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a0d17c1a-7944-41dc-8e61-c45319c7bf61
📒 Files selected for processing (74)
.gitignoreweb/ee/src/components/Scripts/assets/CloudScripts.tsxweb/oss/src/components/AgentChatSlice/AgentChatPanel.tsxweb/oss/src/components/AgentChatSlice/components/AgentChatSkeleton.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/oss/src/components/DrillInView/OSSdrillInUIProvider.tsxweb/oss/src/components/Evaluators/components/EvaluatorTemplateDropdown.tsxweb/oss/src/components/Filters/Filters.tsxweb/oss/src/components/Playground/Components/AgentCatalogPrefetcher.tsxweb/oss/src/components/Playground/Components/MainLayout/index.tsxweb/oss/src/components/Playground/Components/Menus/SelectVariant/index.tsxweb/oss/src/components/Playground/Components/PlaygroundHeader/index.tsxweb/oss/src/components/Playground/Components/PlaygroundVariantConfig/assets/AgentConfigSkeleton.tsxweb/oss/src/components/Playground/Components/PlaygroundVariantConfig/assets/PlaygroundVariantConfigHeader.tsxweb/oss/src/components/Playground/Components/PlaygroundVariantConfig/index.tsxweb/oss/src/components/Playground/Playground.tsxweb/oss/src/components/PlaygroundRouter/index.tsxweb/oss/src/components/SharedDrawers/AnnotateDrawer/index.tsxweb/oss/src/components/Sidebar/hooks/useSidebarConfig/index.tsxweb/oss/src/components/Sidebar/hooks/useWorkflowSwitcher.tsxweb/oss/src/components/WorkflowRevisionDrawerWrapper/index.tsxweb/oss/src/components/pages/app-management/modals/CustomWorkflowModal/hooks/useCustomWorkflowConfig.tsxweb/oss/src/hooks/usePlaygroundNavigation.tsweb/oss/src/lib/atoms/breadcrumb/index.tsweb/oss/src/lib/helpers/auth/AuthProvider.tsxweb/oss/src/pages/w/[workspace_id]/p/[project_id]/apps/[app_id]/index.tsxweb/oss/src/state/access/atoms.tsweb/oss/src/state/app/atoms/fetcher.tsweb/oss/src/state/app/hooks.tsweb/oss/src/state/app/selectors/app.tsweb/oss/src/state/boot/idleReady.tsweb/oss/src/state/org/selectors/org.tsweb/oss/src/state/workflow/index.tsweb/oss/src/state/workflow/selectors/workflow.tsweb/packages/agenta-annotation-ui/src/components/CreateQueueDrawer/index.tsxweb/packages/agenta-entities/src/environment/api/api.tsweb/packages/agenta-entities/src/environment/state/store.tsweb/packages/agenta-entities/src/gatewayTool/api/api.tsweb/packages/agenta-entities/src/gatewayTool/api/client.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useToolCatalogIntegrations.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useToolConnectionsQuery.tsweb/packages/agenta-entities/src/gatewayTrigger/api/api.tsweb/packages/agenta-entities/src/gatewayTrigger/hooks/useTriggerSchedules.tsweb/packages/agenta-entities/src/gatewayTrigger/hooks/useTriggerSubscriptions.tsweb/packages/agenta-entities/src/loadable/controller.tsweb/packages/agenta-entities/src/trace/api/api.tsweb/packages/agenta-entities/src/trace/api/client.tsweb/packages/agenta-entities/src/trace/index.tsweb/packages/agenta-entities/src/trace/state/index.tsweb/packages/agenta-entities/src/trace/state/store.tsweb/packages/agenta-entities/src/workflow/api/api.tsweb/packages/agenta-entities/src/workflow/index.tsweb/packages/agenta-entities/src/workflow/state/evaluatorUtils.tsweb/packages/agenta-entities/src/workflow/state/index.tsweb/packages/agenta-entities/src/workflow/state/inspectMeta.tsweb/packages/agenta-entities/src/workflow/state/persistedAgentType.tsweb/packages/agenta-entities/src/workflow/state/persistedCatalog.tsweb/packages/agenta-entities/src/workflow/state/persistedInspect.tsweb/packages/agenta-entities/src/workflow/state/store.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/ReferenceToolFormView.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/TriggerManagementSection.tsxweb/packages/agenta-entity-ui/src/DrillInView/components/PlaygroundConfigSection.tsxweb/packages/agenta-entity-ui/src/selection/adapters/index.tsweb/packages/agenta-entity-ui/src/selection/adapters/useEnrichedEvaluatorAdapter.tsweb/packages/agenta-entity-ui/src/selection/index.tsweb/packages/agenta-playground/src/state/execution/executionItems.tsweb/packages/agenta-playground/src/state/execution/executionRunner.tsweb/packages/agenta-sdk/src/config.tsweb/packages/agenta-sdk/src/resources.tsweb/packages/agenta-shared/src/api/axios.tsweb/packages/agenta-shared/src/api/index.tsweb/packages/agenta-shared/src/utils/createBatchFetcher.tsweb/packages/agenta-ui/src/drill-in/context/DrillInUIContext.tsx
| // Agent identified early (persisted agent-type map) but the | ||
| // revision hasn't resolved the flag yet — hold the chat pane's | ||
| // shape instead of a blank canvas until the host mounts. | ||
| if (isAgentConfig && singleEntityQuery.isPending) { | ||
| return <AgentChatSkeleton key="agent-generation-skeleton" /> | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
ast-grep outline web/oss/src/components/Playground/Components/MainLayout/index.tsx --view expandedRepository: Agenta-AI/agenta
Length of output: 597
🏁 Script executed:
sed -n '100,240p' web/oss/src/components/Playground/Components/MainLayout/index.tsx
printf '\n----\n'
sed -n '430,475p' web/oss/src/components/Playground/Components/MainLayout/index.tsx
printf '\n----\n'
rg -n "isAgentModeAtomFamily|playgroundEarlyAgentStateAtom|layoutEntityIds|singleEntityQuery|isComparisonView|configEntityIds" web/oss/src/components/Playground -nRepository: Agenta-AI/agenta
Length of output: 19733
🏁 Script executed:
rg -n "selectors\.query\(|workflowMolecule\.selectors\.query|query\(singleEntityId\)|isPending" web/oss/src -g '*.ts' -g '*.tsx'Repository: Agenta-AI/agenta
Length of output: 37772
🏁 Script executed:
ast-grep outline web/oss/src/state/workflow/selectors/workflow.ts --view expanded
printf '\n----\n'
sed -n '1,220p' web/oss/src/state/workflow/selectors/workflow.tsRepository: Agenta-AI/agenta
Length of output: 7332
🏁 Script executed:
rg -n "workflowMolecule|selectors:.*query|queryAtomFamily|workflowDetailQueryAtomFamily|createEntityController|empty string|if \(!id\)" web/oss/src/state web/oss/src/components/Playground web/oss/src/components/AgentChatSlice -g '*.ts' -g '*.tsx'Repository: Agenta-AI/agenta
Length of output: 17763
🏁 Script executed:
ast-grep outline web/oss/src/state/entities/workflow --view expanded
printf '\n----\n'
rg -n "createEntityController|queryAtomFamily|selectors:.*query|workflowMolecule" web/oss/src/state/entities/workflow -g '*.ts' -g '*.tsx'Repository: Agenta-AI/agenta
Length of output: 333
🏁 Script executed:
fd -a "workflow*.ts" web/oss/src
printf '\n----\n'
fd -a "*workflow*" web/oss/src/state web/oss/src/components | head -n 200Repository: Agenta-AI/agenta
Length of output: 518
🏁 Script executed:
rg -n --fixed-strings "workflowMolecule" web/oss/src/state web/oss/src/components web/oss/src/entities -g '*.ts' -g '*.tsx'
printf '\n----\n'
rg -n --fixed-strings "queryAtomFamily" web/oss/src/state web/oss/src/components web/oss/src/entities -g '*.ts' -g '*.tsx'Repository: Agenta-AI/agenta
Length of output: 32317
🏁 Script executed:
fd -a "workflow*.ts" . | head -n 200
printf '\n----\n'
fd -a "createEntityController.ts" . | head -n 50Repository: Agenta-AI/agenta
Length of output: 579
🏁 Script executed:
ast-grep outline web/packages/agenta-entities/src --view expanded
printf '\n----\n'
rg -n --fixed-strings "workflowMolecule" web/packages/agenta-entities/src -g '*.ts' -g '*.tsx'Repository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
ast-grep outline web/packages/agenta-entities/src/workflow/state/molecule.ts --view expanded
printf '\n----\n'
sed -n '1,220p' web/packages/agenta-entities/src/workflow/state/molecule.tsRepository: Agenta-AI/agenta
Length of output: 11166
🏁 Script executed:
rg -n "workflowQueryAtomFamily|atomFamily\\(\\(workflowId: string\\)|queryAtomFamily = atomFamily|isPlaceholderId|isLocalDraftId" web/packages/agenta-entities/src/workflow/state/store.ts
printf '\n----\n'
sed -n '1,220p' web/packages/agenta-entities/src/workflow/state/store.tsRepository: Agenta-AI/agenta
Length of output: 10022
🏁 Script executed:
sed -n '1090,1185p' web/packages/agenta-entities/src/workflow/state/store.ts
printf '\n----\n'
sed -n '1185,1235p' web/packages/agenta-entities/src/workflow/state/store.tsRepository: Agenta-AI/agenta
Length of output: 8016
Make the agent skeleton key unique per rendered entity.
Because this branch sits inside layoutEntityIds.map(...), a multi-entity render gives every match the same <AgentChatSkeleton key="agent-generation-skeleton" />, which can collapse entries in React. Key it by variantId instead.
🛡️ Proposed defensive fix
if (isAgentConfig && singleEntityQuery.isPending) {
- return <AgentChatSkeleton key="agent-generation-skeleton" />
+ return (
+ <AgentChatSkeleton
+ key={`agent-generation-skeleton-${variantId}`}
+ />
+ )
}📝 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.
| // Agent identified early (persisted agent-type map) but the | |
| // revision hasn't resolved the flag yet — hold the chat pane's | |
| // shape instead of a blank canvas until the host mounts. | |
| if (isAgentConfig && singleEntityQuery.isPending) { | |
| return <AgentChatSkeleton key="agent-generation-skeleton" /> | |
| } | |
| if (isAgentConfig && singleEntityQuery.isPending) { | |
| return ( | |
| <AgentChatSkeleton | |
| key={`agent-generation-skeleton-${variantId}`} | |
| /> | |
| ) | |
| } |
| import {atom} from "jotai" | ||
|
|
||
| /** | ||
| * One-shot "the browser has had a spare (idle) moment since load" flag. | ||
| * | ||
| * Gate NON-critical bootstrap queries (entitlements, billing, permission catalogs) on this so they | ||
| * don't fire in the same burst as the first-paint-critical requests — on a capacity-limited backend | ||
| * a flood of concurrent requests on load saturates the workers and slows the critical ones. Flips | ||
| * true on the first `requestIdleCallback` (or within ~2s via its timeout, whichever comes first), | ||
| * then stays true for the session, so it defers the FIRST load without re-deferring on every read. | ||
| * | ||
| * SSR-safe: resolves immediately when there is no `window` (deferral is a client-only concern). | ||
| */ | ||
| const idleReadyStateAtom = atom(false) | ||
|
|
||
| idleReadyStateAtom.onMount = (set) => { | ||
| if (typeof window === "undefined") { | ||
| set(true) | ||
| return | ||
| } | ||
| const w = window as Window & { | ||
| requestIdleCallback?: (cb: () => void, opts?: {timeout: number}) => number | ||
| cancelIdleCallback?: (handle: number) => void | ||
| } | ||
| if (typeof w.requestIdleCallback === "function") { | ||
| const handle = w.requestIdleCallback(() => set(true), {timeout: 2000}) | ||
| return () => w.cancelIdleCallback?.(handle) | ||
| } | ||
| // Safari (no rIC): fall back to a short timeout. | ||
| const timer = window.setTimeout(() => set(true), 1500) | ||
| return () => window.clearTimeout(timer) | ||
| } | ||
|
|
||
| export const idleReadyAtom = atom((get) => get(idleReadyStateAtom)) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Logic is sound (SSR-safe, one-shot, rIC + Safari fallback with cleanup). No functional concerns with the atom itself.
However, the check code styling / TypeScript format job is failing (prettier --check). Run pnpm lint-fix in the web folder and commit the reformatted output before merging.
As per coding guidelines: "Frontend changes require running pnpm lint-fix within the web folder before committing".
🧰 Tools
🪛 GitHub Actions: 11 - check code styling / 1_TypeScript format.txt
[error] 1-1: Command failed with exit code 1: pnpm run format (prettier --check "**/*.{js,jsx,cjs,mjs,ts,tsx,cts,mts,mdx}").
🪛 GitHub Actions: 11 - check code styling / TypeScript format
[error] 1-1: Command failed with exit code 1: pnpm run format (prettier --check "**/*.{js,jsx,cjs,mjs,ts,tsx,cts,mts,mdx}").
Sources: Coding guidelines, Pipeline failures
| playgroundEarlyAgentStateAtom, | ||
| type CurrentWorkflowContext, | ||
| type PlaygroundAgentState, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Run pnpm lint-fix to resolve prettier formatting failures.
Pipeline failure: pnpm run format (prettier check) is failing. Per coding guidelines, frontend changes require running pnpm lint-fix within the web folder before committing.
Sources: Coding guidelines, Pipeline failures
| // Persisted agent-type map (cold-reload fallback for playgroundEarlyAgentStateAtom) | ||
| export {readPersistedAgentType} from "./persistedAgentType" | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
CI prettier --check is failing — run the formatter before merge.
The check code styling / TypeScript format job failed (pnpm run format). Run pnpm lint-fix (or pnpm run format) in web and commit the result so CI passes.
As per coding guidelines: "Frontend changes require running pnpm lint-fix within the web folder before committing".
Sources: Coding guidelines, Pipeline failures
| export { | ||
| axios, | ||
| createAxiosInstance, | ||
| configureAxios, | ||
| resetAxiosConfig, | ||
| lowPriorityWhenCached, | ||
| } from "./axios" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Pipeline format check is failing.
The pnpm run format (prettier --check) pipeline step fails. Run pnpm lint-fix in the web folder before committing, as required by the coding guidelines for frontend changes.
As per coding guidelines: **/*.{js,jsx,ts,tsx}: Frontend changes require running pnpm lint-fix within the web folder before committing.
Sources: Coding guidelines, Pipeline failures
An earlier change commented out the two Crisp effects (configure + accent-color-on-theme), which also left useEffect/Crisp/ChatboxColors/ThemeMode/getEnv/appTheme unused and failed the lint check. Restore the effects (both still gated on NEXT_PUBLIC_CRISP_WEBSITE_ID).
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
web/oss/src/state/url/playground.ts (1)
156-179: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRead-modify-write on a shared localStorage key races across tabs.
readPersistedSelection/writePersistedSelectionshare a single JSON blob keyed byappId. Two tabs updating different apps' selections concurrently can lose one tab's write (read stale map → overwrite with only its own update). Impact is low since it's self-healing UX state, but a per-app key avoids the read-modify-write entirely.♻️ Proposed fix: scope storage key per app
-const PERSISTED_SELECTION_KEY = "agenta:playground:last-selection" - -const readPersistedSelection = (appId: string): string[] => { - if (!isBrowser) return [] - try { - const raw = window.localStorage.getItem(PERSISTED_SELECTION_KEY) - const map = raw ? (JSON.parse(raw) as Record<string, string[]>) : {} - return sanitizeRevisionList(Array.isArray(map[appId]) ? map[appId] : []) - } catch { - return [] - } -} - -const writePersistedSelection = (appId: string, ids: string[]) => { - if (!isBrowser) return - try { - const raw = window.localStorage.getItem(PERSISTED_SELECTION_KEY) - const map = raw ? (JSON.parse(raw) as Record<string, string[]>) : {} - map[appId] = ids - window.localStorage.setItem(PERSISTED_SELECTION_KEY, JSON.stringify(map)) - } catch { - // Quota/serialization failures are non-fatal — the next entry just waits for defaults. - } -} +const PERSISTED_SELECTION_PREFIX = "agenta:playground:last-selection:" + +const readPersistedSelection = (appId: string): string[] => { + if (!isBrowser) return [] + try { + const raw = window.localStorage.getItem(`${PERSISTED_SELECTION_PREFIX}${appId}`) + return sanitizeRevisionList(raw ? (JSON.parse(raw) as string[]) : []) + } catch { + return [] + } +} + +const writePersistedSelection = (appId: string, ids: string[]) => { + if (!isBrowser) return + try { + window.localStorage.setItem(`${PERSISTED_SELECTION_PREFIX}${appId}`, JSON.stringify(ids)) + } catch { + // Quota/serialization failures are non-fatal — the next entry just waits for defaults. + } +}
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b1b5541e-8ab7-44f8-a73c-7110a3844c51
📒 Files selected for processing (56)
.gitignoreapi/oss/src/apis/fastapi/tools/router.pyapi/oss/src/core/gateway/connections/service.pyapi/oss/src/core/gateway/connections/utils.pyapi/oss/tests/pytest/unit/tools/test_oauth_state_identity.pyservices/runner/src/responder.tsservices/runner/src/tracing/otel.tsservices/runner/tests/unit/responder.test.tsweb/oss/src/components/AgentChatSlice/AgentChatPanel.tsxweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/assets/toolDisplay.tsweb/oss/src/components/AgentChatSlice/components/AgentChatSkeleton.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/oss/src/components/AgentChatSlice/components/ApprovalDock.tsxweb/oss/src/components/AgentChatSlice/components/MountFade.tsxweb/oss/src/components/AgentChatSlice/components/SessionRail.tsxweb/oss/src/components/AgentChatSlice/components/SessionTabLabel.tsxweb/oss/src/components/AgentChatSlice/components/SessionTagBar.tsxweb/oss/src/components/AgentChatSlice/components/ToolActivity.tsxweb/oss/src/components/AgentChatSlice/components/clientTools/ConnectToolWidget.tsxweb/oss/src/components/AgentChatSlice/hooks/useAgentChatQueue.tsweb/oss/src/components/AgentChatSlice/state/sessions.tsweb/oss/src/components/Layout/Layout.tsxweb/oss/src/components/Playground/Components/AgentCatalogPrefetcher.tsxweb/oss/src/components/Playground/Components/AgentCommitNotice.tsxweb/oss/src/components/Playground/Components/AgentRevisionSelector/index.tsxweb/oss/src/components/Playground/Components/MainLayout/index.tsxweb/oss/src/components/Playground/Components/PlaygroundVariantConfig/assets/PlaygroundVariantConfigHeader.tsxweb/oss/src/components/Playground/Components/PlaygroundVariantConfig/index.tsxweb/oss/src/components/Playground/Playground.tsxweb/oss/src/components/pages/agent-home/PlaygroundOnboarding/Reveal.tsxweb/oss/src/state/url/playground.tsweb/oss/src/styles/globals.cssweb/oss/tailwind.config.tsweb/packages/agenta-entities/src/workflow/commitDiff/gatewayName.tsweb/packages/agenta-entities/src/workflow/core/schema.tsweb/packages/agenta-entities/src/workflow/index.tsweb/packages/agenta-entities/src/workflow/state/index.tsweb/packages/agenta-entities/src/workflow/state/persistedInspect.tsweb/packages/agenta-entities/src/workflow/state/store.tsweb/packages/agenta-entities/tests/unit/agent-commit-diff.test.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/SchemaPropertyRenderer.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/AgentConfigSkeleton.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/sectionGroups.tsxweb/packages/agenta-entity-ui/src/DrillInView/components/PlaygroundConfigSection.tsxweb/packages/agenta-entity-ui/src/DrillInView/index.tsweb/packages/agenta-entity-ui/src/index.tsweb/packages/agenta-playground-ui/src/components/ExecutionItems/index.tsxweb/packages/agenta-playground-ui/src/components/index.tsweb/packages/agenta-playground/src/state/execution/agentMessageQueue.tsweb/packages/agenta-playground/tests/unit/agentMessageQueue.test.tsweb/packages/agenta-shared/src/state/agentCommitSignal.tsweb/packages/agenta-shared/src/state/index.tsweb/packages/agenta-ui/src/RichChatInput/RichChatInput.tsxweb/packages/agenta-ui/src/components/presentational/section/ConfigAccordionSection.tsx
✅ Files skipped from review due to trivial changes (7)
- api/oss/tests/pytest/unit/tools/test_oauth_state_identity.py
- web/packages/agenta-shared/src/state/index.ts
- web/packages/agenta-entities/tests/unit/agent-commit-diff.test.ts
- web/oss/tailwind.config.ts
- web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/sectionGroups.tsx
- services/runner/src/tracing/otel.ts
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (10)
- web/packages/agenta-entities/src/workflow/state/index.ts
- web/oss/src/components/AgentChatSlice/components/AgentChatSkeleton.tsx
- web/packages/agenta-entities/src/workflow/index.ts
- web/oss/src/components/Playground/Components/AgentCatalogPrefetcher.tsx
- web/oss/src/components/AgentChatSlice/components/AgentMessage.tsx
- web/oss/src/components/Playground/Components/PlaygroundVariantConfig/assets/PlaygroundVariantConfigHeader.tsx
- web/packages/agenta-entities/src/workflow/state/persistedInspect.ts
- web/oss/src/components/Playground/Components/PlaygroundVariantConfig/index.tsx
- web/packages/agenta-entities/src/workflow/state/store.ts
- web/oss/src/components/AgentChatSlice/AgentChatPanel.tsx
| key={ | ||
| renderAgentGenerationHost | ||
| ? "agent-config-host" | ||
| : `variant-config-${variantId}` | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether any caller passes configEntityIdsOverride with more than one id
rg -n "configEntityIdsOverride" web/oss/src -g '*.tsx' -A5 -B5Repository: Agenta-AI/agenta
Length of output: 8573
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the MainLayout component structure and inspect the keying logic.
ast-grep outline web/oss/src/components/Playground/Components/MainLayout/index.tsx --view expanded
printf '\n--- MainLayout excerpt ---\n'
sed -n '160,420p' web/oss/src/components/Playground/Components/MainLayout/index.tsx
printf '\n--- Callers / configEntityIds construction ---\n'
sed -n '1,240p' web/oss/src/components/Evaluators/components/ConfigureEvaluator/index.tsx
printf '\n---\n'
sed -n '1,260p' web/oss/src/components/WorkflowRevisionDrawerWrapper/index.tsx
printf '\n---\n'
sed -n '1,240p' web/oss/src/components/pages/evaluations/NewEvaluation/Components/CreateEvaluatorDrawer/index.tsxRepository: Agenta-AI/agenta
Length of output: 39912
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the evaluator config IDs atom and the code that builds it.
rg -n "evaluatorConfigEntityIdsAtom|configEntityIds" web/oss/src/components/Evaluators -g '*.ts' -g '*.tsx' -A4 -B4
printf '\n--- atom file(s) ---\n'
fd -a "atoms.ts" web/oss/src/components/Evaluators
fd -a "atoms.tsx" web/oss/src/components/Evaluators
printf '\n--- around evaluatorConfigEntityIdsAtom definition ---\n'
for f in $(rg -l "evaluatorConfigEntityIdsAtom" web/oss/src/components/Evaluators -g '*.ts' -g '*.tsx'); do
echo "FILE: $f"
sed -n '1,260p' "$f"
echo
doneRepository: Agenta-AI/agenta
Length of output: 19915
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect graph mutation logic and node-graph shape constraints.
rg -n "connectDownstreamNode|playgroundNodesAtom|depth > 0|downstreamNodes|changePrimaryNode|add.*downstream|remove.*downstream" web/oss/src web/src -g '*.ts' -g '*.tsx' -A4 -B4
printf '\n--- playground state / graph atoms ---\n'
for f in $(rg -l "playgroundNodesAtom|connectDownstreamNode|changePrimaryNode" web/oss/src web/src -g '*.ts' -g '*.tsx'); do
echo "FILE: $f"
sed -n '1,260p' "$f"
echo
doneRepository: Agenta-AI/agenta
Length of output: 36579
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how layoutEntityIds is derived in MainLayout and whether it can differ from configEntityIds.
rg -n "layoutEntityIds|renderAgentGenerationHost|isSingleAgentEntity|singleEntityId|configEntityIdsOverride" web/oss/src/components/Playground/Components/MainLayout/index.tsx -A8 -B8
printf '\n--- surrounding MainLayout state derivation ---\n'
sed -n '120,220p' web/oss/src/components/Playground/Components/MainLayout/index.tsxRepository: Agenta-AI/agenta
Length of output: 16355
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the implementation and contract of connectDownstreamNode / downstream limits.
rg -n "connectDownstreamNode|disconnectSingleDownstreamNode|disconnectDownstreamNode" . -g '*.ts' -g '*.tsx' -A6 -B6
printf '\n--- candidate implementations ---\n'
for f in $(rg -l "disconnectSingleDownstreamNode|connectDownstreamNode" . -g '*.ts' -g '*.tsx'); do
echo "FILE: $f"
sed -n '1,260p' "$f"
echo
doneRepository: Agenta-AI/agenta
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the exact downstream-connection implementation and the entity-id selectors.
sed -n '320,400p' web/packages/agenta-playground/src/state/controllers/playgroundController.ts
printf '\n--- entity id selectors ---\n'
rg -n "selectedEntityIdsAtom|displayedEntityIdsAtom|entityIdsAtom" web/packages/agenta-playground/src/state -g '*.ts' -A6 -B6Repository: Agenta-AI/agenta
Length of output: 50372
Avoid a shared agent key here
configEntityIdsOverride can hold multiple downstream IDs, while renderAgentGenerationHost only follows layoutEntityIds. If this list ever has more than one item, "agent-config-host" collides across siblings; include variantId in the agent branch too.
| const unsubPersistSelection = store.sub(playgroundController.selectors.entityIds(), () => { | ||
| const appId = store.get(routerAppIdAtom) as string | null | ||
| if (!appId) return | ||
| const ids = sanitizeRevisionList(store.get(playgroundController.selectors.entityIds())) | ||
| if (ids.length > 0) writePersistedSelection(appId, ids) | ||
| }) | ||
| unsubs.push(unsubPersistSelection) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find flows that clear entityIds to [] while remaining on the same app (not the app-switch reset)
rg -n "setEntityIds,\s*\[\]" web/oss/src -g '*.ts' -g '*.tsx' -A5 -B5Repository: Agenta-AI/agenta
Length of output: 3320
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant implementation around persisted playground selection.
sed -n '1,260p' web/oss/src/state/url/playground.ts | cat -n
printf '\n--- SNIP ---\n'
sed -n '900,1145p' web/oss/src/state/url/playground.ts | cat -n
printf '\n--- SEARCH ---\n'
rg -n "writePersistedSelection|ensurePlaygroundDefaults|entityIds\\(|persistedSelection|persistSelection|removeItem|delete.*selection|clear.*selection" web/oss/src/state/url/playground.ts web/oss/src/state -g '*.ts' -g '*.tsx' -A4 -B4Repository: Agenta-AI/agenta
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '599,700p' web/oss/src/state/url/playground.ts | cat -n
printf '\n--- SNIP ---\n'
sed -n '1310,1375p' web/oss/src/state/url/playground.ts | cat -n
printf '\n--- WRITE SEARCH ---\n'
rg -n "PERSISTED_SELECTION_KEY|readPersistedSelection\\(|writePersistedSelection\\(|removeItem\\(PERSISTED_SELECTION_KEY\\)|delete map\\[appId\\]|map\\[appId\\] = ids" web/oss/src/state/url/playground.ts -A3 -B3Repository: Agenta-AI/agenta
Length of output: 10331
Clear persisted selection when the app selection is emptied
web/oss/src/state/url/playground.ts:1095-1099 writePersistedSelection only runs for non-empty ids, so clearing the selection on the same app leaves the previous value in localStorage and it can reappear on reload. Remove that app entry (or persist []) when ids.length === 0.
…script json.dumps leaves '<' intact, so an integration_key containing '</script>' could terminate the completion page's script block (slug is regex-validated URL-safe; integration_key is not). Escape '<' in every JSON value embedded in the inline script.
…l both commit-notice rAFs The per-session Maps (virtuoso snapshots, composer drafts, attachments with live File blobs) outlived permanently deleted sessions. Move them to state/sessionEphemera.ts and clear them from deleteSession/resetScope, alongside the existing sessionMessagesAtom cleanup. AgentCommitNotice's cleanup cancelled only the outer rAF; a fast active on->off let the orphaned inner frame flash the notice back in.
…t variants The fetch-priority refactor routes gatewayTrigger/workflow/trace api calls through lowPriorityWhenCached and the @agenta/sdk/resources getters; the unit mocks predated it, so the fakes never intercepted (real 404s) or the missing export threw.
…t-optimizations-onbig # Conflicts: # web/packages/agenta-entities/tests/unit/retrieveWorkflowRevision.test.ts # web/packages/agenta-entities/tests/unit/trace-migration-api.test.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
A freshly committed revision reads non-agent while its flags load, and the early app-id signal can't bridge the gap when the router has no app id (the onboarding flow rewrites the URL via history.replaceState, so routerAppIdAtom never resolves). Unlatched, isAgentConfig flipped false and back — flipping splitterKey agent->std->agent and remounting the whole Splitter twice on an agent self-commit. Latch it exactly like the agent generation host: hold until the entity loads as a definitively non-agent workflow.
…scope The founding conversation lived under the fixed 'onboarding' scope forever: after a reload the app playground (scoped to the app id) couldn't see it, and the next onboarding entry's scope reset destroyed it permanently. On commit, move the scope's sessions/open-tabs/active-id into the new app's bucket (adoptScopeSessionsAtom) and flip the scope provider to the app scope in the same batched update — the mounted panel re-reads identical sessions under the new key, so nothing remounts, post-commit chat state lands where the app playground reads it, and the reset can no longer reach it.
The onboarding composer disabled submit-on-Enter (Enter inserted a newline for long descriptions) while the composer's shortcut hints still promised '<Enter> Send' — so Enter appeared to do nothing. Enable it: Enter commits the agent (same path as the Create-agent button, empty text is a no-op), and Cmd/Shift+ Enter keeps inserting newlines for multi-line descriptions.
The one-pause-per-turn latch gated the client-tool interaction EMIT, not just the pause, so when an agent requested several connections in one step only the first got a widget; the rest were force-settled as DEFERRED_NOT_EXECUTED siblings. The model then didn't reliably re-request them (it narrated 'pending' instead), so the second connection never appeared and the run continued without it. Client tools are independent surfaces, unlike a single approval dialog. Emit a widget for each pending client tool and mark each parked so it isn't force-settled; the turn still ends once because pause() is idempotent. All requested connections now render and the user resolves each; no deferral and no dependency on the model re-asking.
|
@coderabbitai review |
✅ Action performedReview finished.
|
…t-optimizations-onbig
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
api/oss/src/apis/fastapi/tools/router.py (2)
918-923: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFailure cards never notify the initiating opener.
Every failure call omits
agenta_url, makingAGENTA_POST_MESSAGE_ORIGINnull and skipping bothpostMessagecalls. The requesting connection widget can therefore remain pending. Pass the web URL, but only post when both identity fields exist to avoid untagged multi-flow completions.Proposed fix
content=_oauth_card( success=False, error=..., + agenta_url=env.agenta.web_url, slug=state_slug, integration_key=state_integration, ),Apply this to each failure card, then scope notification:
- agenta_post_message_origin_js = _json_for_inline_script(agenta_origin) + completion_is_scoped = bool(agenta_origin and slug and integration_key) + agenta_post_message_origin_js = _json_for_inline_script( + agenta_origin if completion_is_scoped else None + )Also applies to: 929-934, 949-954, 970-975, 981-986, 1477-1482, 1683-1702
939-944: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard
project_idbefore callingUUID.decode_oauth_state()returns arbitrary JSON values, sostate_payload["project_id"]can benull, a number, orbool;UUID(...)then raisesTypeError/AttributeError, which isn’t caught here and turns this callback into a 500.
🧹 Nitpick comments (3)
web/oss/src/components/pages/agent-home/PlaygroundOnboarding/useAgentOnboarding.ts (1)
51-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCondense the new explanatory comments.
Keep only the non-obvious invariant in a single line.
Proposed cleanup
- /** - * Chat scope for the onboarding surface: the fixed onboarding key pre-commit, the created - * app's own scope once real (the commit adopts the session into it in the same update). - */ + /** Chat scope before commit, then the created app scope after session adoption. */ chatScopeKey: string - // Adopt the founding conversation into the new app's chat scope, in the SAME - // batched update as the `chatScopeKey` flip below (`setRealAppId`): the mounted - // panel re-reads identical sessions under the new key, so nothing remounts — - // and after a reload the app playground finds the session (previously it was - // stranded under the fixed onboarding scope, which the next onboarding entry - // wipes — permanently destroying the conversation). + // Move the founding session before switching scopes so reloads preserve it.As per coding guidelines, “Keep in-code comments to one short line maximum unless a genuinely surprising constraint requires a brief exception.”
Also applies to: 171-176
Source: Coding guidelines
web/oss/src/components/Playground/Playground.tsx (1)
93-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShorten the new multi-line comments.
These explanations can each be reduced to one line while preserving the relevant invariant.
As per coding guidelines, “Keep in-code comments to one short line maximum unless a genuinely surprising constraint requires a brief exception.”
Also applies to: 118-120, 145-146
Source: Coding guidelines
api/oss/src/apis/fastapi/tools/router.py (1)
897-901: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCondense the new block comments to single lines.
These comments repeat implementation details and can be reduced to concise summaries.
- # Decode the HMAC-signed state up front to recover BOTH the project scope and the - # connection identity. The identity tags every card (success or failure) so the - # opener can tell WHICH connect flow finished — the playground can have several - # live at once (see ConnectToolWidget), and an untagged completion would settle - # all of them. + # Recover project scope and connection identity from the signed OAuth state.As per coding guidelines, “Keep in-code comments to one short line maximum unless a genuinely surprising constraint requires a brief exception.”
Also applies to: 937-938, 1484-1488
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c05028c1-4871-454a-9918-b6ca26416c69
📒 Files selected for processing (14)
api/oss/src/apis/fastapi/tools/router.pyservices/runner/src/engines/sandbox_agent/client-tools.tsservices/runner/tests/unit/client-tools.test.tsweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/state/sessionEphemera.tsweb/oss/src/components/AgentChatSlice/state/sessions.tsweb/oss/src/components/Playground/Components/AgentCommitNotice.tsxweb/oss/src/components/Playground/Components/MainLayout/index.tsxweb/oss/src/components/Playground/Playground.tsxweb/oss/src/components/pages/agent-home/PlaygroundOnboarding/useAgentOnboarding.tsweb/packages/agenta-entities/tests/unit/agent-build-kit-overlay-api.test.tsweb/packages/agenta-entities/tests/unit/gatewayTriggerApi.test.tsweb/packages/agenta-entities/tests/unit/retrieveWorkflowRevision.test.tsweb/packages/agenta-entities/tests/unit/trace-migration-api.test.ts
✅ Files skipped from review due to trivial changes (1)
- web/packages/agenta-entities/tests/unit/gatewayTriggerApi.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- web/packages/agenta-entities/tests/unit/retrieveWorkflowRevision.test.ts
- web/oss/src/components/AgentChatSlice/state/sessionEphemera.ts
- web/packages/agenta-entities/tests/unit/trace-migration-api.test.ts
- web/packages/agenta-entities/tests/unit/agent-build-kit-overlay-api.test.ts
- web/oss/src/components/Playground/Components/AgentCommitNotice.tsx
- web/oss/src/components/AgentChatSlice/state/sessions.ts
- web/oss/src/components/Playground/Components/MainLayout/index.tsx
- web/oss/src/components/AgentChatSlice/AgentConversation.tsx
|
@ardaerzin tested it, two connections after each other don't work for me. The second one succeeds but the frontend thinks it failed |
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
other than the comment above it looks great and much snappier. Commit behvaior is awesome
…t-optimizations-onbig # Conflicts: # web/oss/src/components/Sidebar/hooks/useWorkflowSwitcher.tsx
Every connect widget opened its OAuth popup with the same window name 'tools_oauth'.
With two connections in a row, the second window.open collided with the first widget's
popup instead of getting its own, so the second flow's tools:oauth:complete message
never reached the second widget — its popup-closed poll then settled it as cancelled
('Connection not completed') even though the OAuth had succeeded. Name the window per
tool-call id so each widget owns its popup.
@mmabrouk I pushed another commit, can you please check if this issue is still happening |
2 participants
Context
This branch started as agent-playground load and reload performance and grew to cover the agent chat experience and the multi-connection flow alongside it. Three areas: the playground was slow and flashed the wrong chrome on load; the agent chat panel and session UI had rough edges; and asking for more than one connection in a single step failed in confusing ways.
Base is
big-agents, and the latest base is merged in.Changes
Playground load and reload
No chrome flash on load. An early, app-id-keyed agent signal (
playgroundEarlyAgentStateAtom), backed by a persisted{workflowId: type}map, lets the header, config-panel header, loading shell, and split layout commit to the agent layout up front instead of defaulting to prompt and flipping. The prompt playground is unchanged.Instant reloads via disk-backed SWR. The static agent-template schema, harness catalog, and per-revision inspect persist to
localStorage, paint from disk on reload, and revalidate once in the background at low priority. A catalog prefetcher warms the static catalogs in parallel with the revision and inspect waterfall. The inspect cache is bounded by a byte budget (not an entry count) with quota self-healing, so large payloads can't monopolize the sharedlocalStorageorigin and starve auth state.Correct request priorities and fewer requests. Secondary playground requests (tool connections and catalog, triggers, deploy environments, build-kit overlay, sidebar and variant-picker queries) and the entitlement/billing bootstrap send the
priority: "low"fetch hint; Fern clients gained low-priority variants. A degraded billing service fails fast instead of retrying a 502 three times at high priority. Two round-trips were deduped (current-app revision detail from the revisions list, selected-org detail seeded from the workspace fetch), and the entitlement/billing bootstrap is deferred to browser idle.Lighter chunks and skeletons. The chat panel regions, prompt-generation UIs, comparison view, and drawers are code-split out of the playground chunk, and the agent-template control idle-warms. Layout-matched loading skeletons and low-priority background trace-summary hydration smooth the first paint. Drawer data (the Model and harness draft, the trigger provider groups) fetches only when its drawer or section is open.
Agent chat UX
The chat panel is frame-split: a thin synchronous shell paints the structure in the first frame, and the conversation body, session bar, and rail lazy-load with their own region skeletons. Chat session state persists across pane remounts, and the session rail is resizable. Session status dots hold the streaming colour on the active session and surface an awaiting-input state.
Agent self-commit reads clearly now: a bottom-pinned banner announces it, changed sections get indicators, the config header shows an agent-update notice, and the transition animates instead of snapping. Config sections stay mounted across revision switches so they don't tear down and refetch. Tool-step names in chat are humanized, and trace-pending turns show a graceful state instead of a broken metric row.
Connection flow (multiple connections in one step)
Asking for two connections in a single step was broken end to end. This bundles the full fix across frontend, api, and
services/runner.Before: the agent asks for GitHub and Slack in one step. Slack immediately shows "Connection not completed" with no user action, completing GitHub resumes the run early, and the agent's re-ask of Slack never produces a widget. The user is stuck.
After: GitHub parks and shows its widget, Slack shows "Connecting next" (not a failure), completing GitHub resumes and the agent re-asks Slack, and a fresh Slack widget parks for the user to connect.
The pieces:
slug/integration, and the widget settles only on its own completion, so several live connect widgets no longer settle each other (api + frontend).DEFERRED_NOT_EXECUTEDresult is kept out of the client-output store, so the model's re-request re-parks and emits a fresh widget (services/runner).Reliability
workflowSchema(degrade to null instead of failing the whole revision parse).Tests / notes
services/runner: full unit suite green (741 tests),tscclean.@agenta/entitiesand@agenta/playground: package unit tests green (workflow version schema, agent message queue),tscclean.eslintclean on touched files.origin/big-agentsmerged in. Conflicts resolved deliberately: kept the frame-split chat panel and ported big-agents' build-kit overlay gating intoAgentConversation; unioned bothversionschema fixes (strip thevNstatic-catalog prefix and tolerate a non-numeric value).What to QA
agent-template,harnesses/, andinspectat Low priority and the config paints from cache.