feat(frontend): agent commit modal redesign + config-panel change indicators#5035
Conversation
…icators Commit modal (agent workflows): summary-first two-pane layout with a save-mode rail (new version / new variant), a plain-language change summary that drills into per-section detail, an editable auto-generated message, and multi-environment deploy fan-out. Backed by a new semantic commit-diff classifier (@agenta/entities/workflow/commitDiff) that groups a config diff into the agent template's control sections (Model & harness, Instructions, Tools, MCP servers, Skills, Advanced). Non-agent testset/evaluator/create modals are unchanged. Config panel: flag sections and line items that have unsaved edits or invalid/missing fields. Section headers get a soft icon tint + status dot + explanatory tooltip; tool/MCP/skill/instruction rows get a border tint + tag. Draft state reuses the classifier against the committed server config; validity reuses each item kind's draftInvalid guard. Adds the missing colorInfo / *Border theme tokens.
dosubot
Bot
added
the
size:XXL
|
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:
📝 WalkthroughWalkthroughThis PR adds commit-diff normalization and classification utilities, wires semantic change sections into commit context and commit modals, introduces split commit-and-deploy behavior, updates Playground variant deployment to publish per environment, and adds draft/validation indicators to agent template controls. ChangesCommit-diff workflow and UI
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant EntityCommitFooter
participant EntityCommitModal
participant onSubmit
User->>EntityCommitFooter: Click "Commit & deploy"
EntityCommitFooter->>EntityCommitFooter: Select environments/message
EntityCommitFooter->>EntityCommitModal: onConfirm(deployEnvironments, deployMessage)
EntityCommitModal->>onSubmit: submit payload with deployEnvironments/deployMessage
onSubmit-->>EntityCommitModal: success/error
sequenceDiagram
participant CommitVariantChangesModal
participant deployRevision
participant publish
CommitVariantChangesModal->>deployRevision: revisionId, environments, message
loop each environment
deployRevision->>publish: publish(revisionRef, environment)
publish-->>deployRevision: success/failure
end
deployRevision-->>CommitVariantChangesModal: aggregated success/error message
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 |
dosubot
Bot
added
Feature Request
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
web/packages/agenta-entity-ui/src/adapters/variantAdapters.ts (1)
78-97: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueFallback classification against the raw wrapper object may rarely misfire.
When
parameters-based classification returns[]buthasDiffis true (e.g. a non-parametersfield changed), the fallback callsclassifyAgentChanges(localSide, remoteSide)on the whole wrapper object (which itself contains aparameterskey alongside other top-level fields like id/name).readAgentConfigwon't recognize this shape as any of the three supported schemas, so this fallback likely still yields[]in most cases — functionally harmless (falls through to the coarse "Configuration modified" summary) but slightly redundant computation on every commit-context recompute.web/packages/agenta-entity-ui/src/modals/commit/components/EntityCommitContent.tsx (1)
358-366: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInline
stylefor a static height value — use a Tailwind arbitrary-value class instead.
style={isAgentTwoPane ? {height: "min(450px, 72vh)"} : undefined}is a plain layout div, not an Ant Design internals override, and the value doesn't vary per render beyond the boolean toggle. As per coding guidelines, "Prefer Tailwind utility classes over CSS-in-JS, inlinestyle={{...}}, ... use CSS-in-JS only for complex Ant Design overrides, JS-calculated dynamic theme styles, or legacy components."- style={isAgentTwoPane ? {height: "min(450px, 72vh)"} : undefined} + className={cn(..., isAgentTwoPane && "h-[min(450px,72vh)]")}Source: Coding guidelines
web/oss/src/components/Playground/Components/Modals/CommitVariantChangesModal/index.tsx (1)
95-104: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSequential per-environment deploy — parallelize with
Promise.allSettled.Deploying to N environments one at a time adds N× latency versus a concurrent fan-out.
Promise.allSettledwould also remove the manual try/catch bookkeeping.- const succeeded: string[] = [] - const failed: string[] = [] - for (const environmentSlug of deployEnvironments) { - try { - await publish({...refs, environmentSlug}) - succeeded.push(environmentSlug) - } catch { - failed.push(environmentSlug) - } - } + const results = await Promise.allSettled( + deployEnvironments.map((environmentSlug) => + publish({...refs, environmentSlug}).then(() => environmentSlug), + ), + ) + const succeeded = results + .filter((r): r is PromiseFulfilledResult<string> => r.status === "fulfilled") + .map((r) => r.value) + const failed = deployEnvironments.filter((e) => !succeeded.includes(e))web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx (1)
386-392: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
indicatorColorlogic.This tone→color mapping duplicates the identical ternary already defined inside
ConfigAccordionSection(web/packages/agenta-ui/src/components/presentational/section/ConfigAccordionSection.tsx, lines 125-131). Consider exporting one shared helper (e.g. fromConfigAccordionSection.tsxor a small shared util) and importing it in both places to avoid drift if the tone→token mapping changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f420d56c-70b5-40ea-9b4c-d6c4e3d88d7f
📒 Files selected for processing (29)
web/oss/src/components/Playground/Components/Modals/CommitVariantChangesModal/index.tsxweb/oss/src/styles/theme-variables.cssweb/packages/agenta-entities/package.jsonweb/packages/agenta-entities/src/workflow/commitDiff/accessors.tsweb/packages/agenta-entities/src/workflow/commitDiff/classify.tsweb/packages/agenta-entities/src/workflow/commitDiff/gatewayName.tsweb/packages/agenta-entities/src/workflow/commitDiff/index.tsweb/packages/agenta-entities/src/workflow/commitDiff/summaryMessage.tsweb/packages/agenta-entities/src/workflow/commitDiff/types.tsweb/packages/agenta-entities/tests/unit/agent-commit-diff.test.tsweb/packages/agenta-entities/vitest.config.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/ConfigItemList.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/ItemRow.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/ToolManagementList.tsxweb/packages/agenta-entity-ui/src/adapters/variantAdapters.tsweb/packages/agenta-entity-ui/src/drawers/shared/SectionRail.tsxweb/packages/agenta-entity-ui/src/index.tsweb/packages/agenta-entity-ui/src/modals/commit/components/EntityCommitContent.tsxweb/packages/agenta-entity-ui/src/modals/commit/components/EntityCommitFooter.tsxweb/packages/agenta-entity-ui/src/modals/commit/components/EntityCommitModal.tsxweb/packages/agenta-entity-ui/src/modals/commit/components/changes/AgentChangesSummary.tsxweb/packages/agenta-entity-ui/src/modals/index.tsweb/packages/agenta-entity-ui/src/modals/types.tsweb/packages/agenta-ui/package.jsonweb/packages/agenta-ui/src/Editor/index.tsweb/packages/agenta-ui/src/Editor/utils/diffUtils.tsweb/packages/agenta-ui/src/components/presentational/inputs/CommitMessageInput.tsxweb/packages/agenta-ui/src/components/presentational/section/ConfigAccordionSection.tsx
…edback Root cause: per-item draft detection matched live vs committed items by a name-only identity that returned an empty string for builtin/reference tools and 'item' for unnamed mcps/skills, so distinct items collapsed onto one Map key and got wrong new/edited/unchanged indicators (and off-by-one summary counts). Add a canonical, collision-free agentItemIdentity (reference slug / function name / platform op / name / embed slug, with a positional fallback) in @agenta/entities/workflow/commitDiff, shared by both the classifier's mcp/skill diff and the config panel's per-row status. Also: extract a shared sectionIndicatorColor (single source for the tone->token mapping, was duplicated in ConfigAccordionSection and AgentTemplateControl); drop the dead classify fallback on the wrapper object in variantAdapters; move the static two-pane height from an inline style to a Tailwind class; and fan out per-environment deploy with Promise.allSettled instead of a sequential loop.
|
@coderabbitai review |
✅ Action performedReview finished.
|
QA repro: adding then removing a tool/skill/MCP before committing leaves e.g. `skills: []` where the committed baseline had no `skills` key. The raw comparisons (workflow isDirty and the commit modal's hasDiff) saw `[] != absent`, so (1) the unsaved-changes badge never cleared and (2) the semantic classifier normalized both sides to empty, returned no sections, and the modal fell back to the old generic JSON-diff layout instead of the new two-pane one. Root cause: the raw diffs were more sensitive than the semantic classifier (which already treats empty as absent). Add a shared stripEmptyCollectionsDeep that drops present-but-empty arrays/objects, applied symmetrically to both sides of isDirty (normalizeForComparison) and the commit-context buildSide. Symmetric application only removes false-positive dirtiness — a non-empty side is never stripped, so real changes still register. Array elements/order are untouched (only object keys are dropped).
2 participants
What
Two related frontend features for the agent playground, both driven by a new semantic commit-diff classifier.
Commit modal redesign (agent workflows)
The commit modal for agent-type workflows is reworked for non-technical users while keeping full JSON access:
SectionRail) to pick New version vs New variant; the variant name/slug form renders inside the rail's content.Non-agent (testset / evaluator / create) modals are gated out and unchanged.
Config-panel change indicators
The agent template panel now flags what's changed or incomplete:
draft/invalid/incomplete).serverConfiguration); validity reuses each item kind'sdraftInvalidguard.colorInfo/*Bordersemantic theme tokens (the theme previously neutralizedcolorInfoto grey).Key implementation notes
@agenta/entities/workflow/commitDiff— pure, dependency-light; buckets a config diff to mirror the panel's control sections. Note the "Advanced" grouping is artificial: it pulls auth/connection out ofllm(everything exceptllm.model), plusrunner/sandbox/harness(non-kind).ConfigAccordionSection(indicator),ItemRow(status),CommitMessageInput(fill).Testing
agent-commit-diff.test.ts) covering all schema shapes and change categories.tsc --noEmitclean across@agenta/ui,@agenta/entities,@agenta/entity-ui; eslint + prettier clean (pre-commit + pre-push turbo-lint passed).