Skip to content

fix(frontend): settle v0.128.9 review threads on the /m drawers, deploy button and tests - #8200

Merged
mmabrouk merged 1 commit into
release/v0.128.9from
fix/release-01289-review-threads
Nov 6, 2026
Merged

mmabrouk merged 1 commit into
release/v0.128.9from
fix/release-01289-review-threads

Conversation

@mmabrouk

@mmabrouk mmabrouk commented Nov 6, 2026

Copy link
Copy Markdown
Member

This PR fixes the open review threads from the v0.128.9 merge train that were real problems, and updates one stale test.

Changes

Tests

  • pnpm lint-fix in web/.
  • pnpm types:check in web/mobile and web/packages/agenta-playground-ui.
  • Vitest in web/mobile: runDetailsShell, editableRevisionDrawer, revisionDrawerGate, deploymentsTab, appOverviewSections, appOverviewBeta, appOverviewScreen, variantsScreen, promptsScreen. A new revisionDrawerGate test checks that Deploy is off while the revision loads, and it fails without the fix.

…oy button and tests

- Clear the drawer's committed revision when the host asks for another revision.
- Make the drawer config read-only while the next revision loads.
- Keep Deploy off until the revision loads, and read the variant label live.
- Use the destructive tokens for the solid red confirm button.
- Give the editable drawer tests a timeout longer than their polling.
- Expect Stop, not Pause, in the run details tests.
- Reword two test-design rules on mocks.
@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Summary

Summary by CodeRabbit

  • Bug Fixes
    • The Deploy button remains disabled until revision details are available, and deployment details now reflect the currently loaded revision and variant.
    • The revision editor is read-only while a new remote revision is loading, preventing changes to outdated details.
    • Switching revisions clears the previous committed state.
  • Updates
    • Active runs now use Stop wording instead of Pause.
    • Destructive confirmation buttons use the updated destructive color styling.

The changes update revision deployment and drawer states, align run details references with the Stop action, and revise test guidance, test timing, and destructive button styling.

Changes: Revision loading and editing

Layer / File(s) Summary
Gate deployment on revision data
web/mobile/src/features/revisions/DeployRevisionButton.tsx, web/mobile/tests/unit/revisionDrawerGate.render.test.tsx
The button reads revision data and variant label through Jotai hooks, disables itself until revision data is available, and resets its open state when the dialog closes. Tests cover the disabled state.
Handle pending revision state
web/packages/agenta-playground-ui/src/panels/drawer/EditableRevisionDrawer.tsx, web/packages/agenta-playground-ui/src/panels/drawer/EditableRevisionDrawerBody.tsx
A new nonempty revision ID clears the committed ID. The configuration panel is read-only while a non-local revision is pending.

Run details Stop action

Layer / File(s) Summary
Update Stop action references
web/mobile/src/features/evaluation-run/RunDetailsScreen.tsx, web/mobile/tests/unit/runDetailsShell.render.test.tsx
The screen comment and tests refer to Stop instead of Pause. Tests retain the stop-request and success-toast assertions and check that readers without edit rights do not see the Stop button.

Destructive confirm button styling

Layer / File(s) Summary
Update destructive button classes
web/mobile/src/features/settings/ConfirmModal.tsx
The solid destructive button uses bg-destructive and text-destructive-foreground; its hover class is unchanged.

Test-design guidance

Layer / File(s) Summary
Revise mock guidance
.agents/skills/test-design/SKILL.md
The guidance includes browser APIs that jsdom lacks and says to keep assertions on displayed output, including when HTTP is mocked.

Revision drawer test timeout

Layer / File(s) Summary
Allow time for drawer polling
web/mobile/tests/unit/editableRevisionDrawer.render.test.tsx
The test suite uses a 60-second timeout. Comments describe the polling limit and its scope.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

  • Agenta-AI/agenta#8012: Introduces ConfigPanel and its hydration loading state, which the revision drawer uses.
  • Agenta-AI/agenta#8041: Adds the ConfigPanel readOnly prop used by the revision drawer while a replacement revision loads.
Merge Risk: Low · up to 9235f

The deploy dialog can switch its target revision if the URL revision changes while it is open, and it can show a blank variant name until the label loads. Test guidance also conflicts with the save-test rule. These issues are small and can be fixed in follow-up work.

Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title identifies the drawer, deploy button, and test changes.
Description check Passed The description covers the changes and reported tests.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 8 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Website preview

Preview URL: https://pr-8200-agenta-website-preview.mahmoud-637.workers.dev

Built from 9235f0846932216b27d2ac2b52491c32b9cb4d50. This comment updates in place on every push.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between e1362d6 and 9235f08.

Files selected for processing: 9

  • .agents/skills/test-design/SKILL.md
  • web/mobile/src/features/evaluation-run/RunDetailsScreen.tsx
  • web/mobile/src/features/revisions/DeployRevisionButton.tsx
  • web/mobile/src/features/settings/ConfirmModal.tsx
  • web/mobile/tests/unit/editableRevisionDrawer.render.test.tsx
  • web/mobile/tests/unit/revisionDrawerGate.render.test.tsx
  • web/mobile/tests/unit/runDetailsShell.render.test.tsx
  • web/packages/agenta-playground-ui/src/panels/drawer/EditableRevisionDrawer.tsx
  • web/packages/agenta-playground-ui/src/panels/drawer/EditableRevisionDrawerBody.tsx

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review.

- Mock only what lives outside the code: HTTP, time, randomness, the router, and browser APIs that jsdom lacks, such as the clipboard.
- If a test needs more than about 5 `vi.mock` calls, the level is wrong. Move down to the package state, or up to a screen with real children.
- Never assert on a mock. If the assertion still passes after you remove the mock, or fails because of it, delete the assertion.
- Never assert on a mock. An assertion that only checks what a mock returns or records tests the mock, so delete it. Keep an assertion on what the screen shows, even in a test that also mocks HTTP.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor · Data Integrity & Integration · Quick win

Supported by static analysis

Script executed:

git diff e1362d6391b2e59f48c70a7ef9aa4b2ea6b53f36 9235f0846932216b27d2ac2b52491c32b9cb4d50 -- .agents/skills/test-design/SKILL.md
printf '\n--- complete skill ---\n'
nl -ba .agents/skills/test-design/SKILL.md
printf '\n--- request assertions in mobile tests ---\n'
rg -n --glob '*test*' --glob '*spec*' -e 'toHaveBeenCalledWith|mock.calls|requestBody|request\.body' web/mobile | sed -n '1,100p'

Repository: Agenta-AI/agenta

Length of output: 18533


Script executed:

printf '%s\n' '--- HTTP mock setup and save test ---'
nl -ba web/mobile/tests/unit/appActionsMenu.render.test.tsx | sed -n '1,140p;200,278p'

Repository: Agenta-AI/agenta

Length of output: 9991


Exempt request-body assertions from the mock rule.

Save tests can inspect requests recorded by an HTTP mock. The rule tells authors to delete those assertions, but the save rule requires request-body assertions. Clarify that assertions on requests sent by the code under test are allowed.

// edited. Deploy waits for it: before it loads there is no version or variant to send.
const data = useAtomValue(workflowMolecule.atoms.query(revisionId)).data
// Read live, because the variants list can land after the revision.
const variantName = useAtomValue(workflowMolecule.selectors.variantLabel(revisionId)) ?? ""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor · Functional Correctness · Quick win

Supported by static analysis

Script executed:

git diff e1362d6391b2e59f48c70a7ef9aa4b2ea6b53f36 9235f0846932216b27d2ac2b52491c32b9cb4d50 -- web/mobile/src/features/revisions/DeployRevisionButton.tsx
printf '\n--- button ---\n'
nl -ba web/mobile/src/features/revisions/DeployRevisionButton.tsx
printf '\n--- selector locations ---\n'
rg -n -F --glob '*.{ts,tsx}' -e 'variantLabel' -e 'DeployRevisionDialog' -- web/packages/agenta-entities web/mobile/src/features web/packages/agenta-entity-ui || test "$?" -eq 1

Repository: Agenta-AI/agenta

Length of output: 9195


Script executed:

printf '%s\n' '--- selector ---'
nl -ba web/packages/agenta-entities/src/workflow/state/molecule.ts | sed -n '430,525p'
printf '%s\n' '--- dialog ---'
nl -ba web/mobile/src/features/variants/DeployRevisionDialog.tsx
printf '%s\n' '--- variant list selector bindings ---'
rg -n -F -e 'variantLabelAtomFamily' -e 'variantsQuery' -e 'variantsAtom' -e 'workflow_variant_id' -- web/packages/agenta-entities/src/workflow/state/molecule.ts

Repository: Agenta-AI/agenta

Length of output: 11541


Script executed:

printf '%s\n' '--- query and base bindings ---'
rg -n -F -e 'workflowVariantsQueryAtomFamily' -e 'workflowBaseEntityAtomFamily' -e 'workflowQueryAtomFamily' -- web/packages/agenta-entities/src/workflow
printf '%s\n' '--- deployment writes ---'
nl -ba web/mobile/src/features/variants/useDeploymentWrites.ts

Repository: Agenta-AI/agenta

Length of output: 11875


Script executed:

printf '%s\n' '--- variants query ---'
nl -ba web/packages/agenta-entities/src/workflow/state/store.ts | sed -n '600,645p'
printf '%s\n' '--- revision query ---'
nl -ba web/packages/agenta-entities/src/workflow/state/store.ts | sed -n '1280,1345p'
printf '%s\n' '--- base entity query ---'
nl -ba web/packages/agenta-entities/src/workflow/state/store.ts | sed -n '1825,1955p'

Repository: Agenta-AI/agenta

Length of output: 13597


Handle an unavailable variant label in the deploy dialog.

The revision can load before the variants list. The dialog can then show a blank variant name, and a deployment submitted before the label arrives can show a blank name in its success message. Show a loading or unavailable label while preserving deployment if the list cannot load.

Comment on lines +33 to +36
const row: DialogRevision | null =
open && data
? {
revisionId,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor · Data Integrity & Integration · Quick win

Supported by static analysis

Script executed:

git diff --stat e1362d6391b2e59f48c70a7ef9aa4b2ea6b53f36 9235f0846932216b27d2ac2b52491c32b9cb4d50 -- web/mobile/src/features/revisions
git diff e1362d6391b2e59f48c70a7ef9aa4b2ea6b53f36 9235f0846932216b27d2ac2b52491c32b9cb4d50 -- web/mobile/src/features/revisions/DeployRevisionButton.tsx
nl -ba web/mobile/src/features/revisions/DeployRevisionButton.tsx
rg -n -F -e 'DeployRevisionButton' -e 'useRevisionParam' -e 'DeployRevisionDialog' -- web/mobile/src web/packages

Repository: Agenta-AI/agenta

Length of output: 11636


Script executed:

for f in \
 web/mobile/src/features/revisions/useRevisionParam.ts \
 web/mobile/src/features/revisions/EditableRevisionDrawerHost.tsx \
 web/mobile/src/features/variants/DeploymentsTab.tsx \
 web/mobile/src/features/app-overview/AppRecentPromptsSection.tsx \
 web/mobile/src/features/variants/DeployRevisionDialog.tsx; do
  echo "===== $f ====="
  nl -ba "$f"
done

Repository: Agenta-AI/agenta

Length of output: 36013


Script executed:

rg -n -F -e 'export const EditableRevisionDrawer' -e 'function EditableRevisionDrawer' -e 'actions={actions}' -- web/packages/agenta-playground-ui/src/panels
rg -n -F -e 'navigationIds' -e 'onNavigate' -e '{actions}' -- web/packages/agenta-playground-ui/src/panels/drawer

Repository: Agenta-AI/agenta

Length of output: 2069


Script executed:

nl -ba web/packages/agenta-playground-ui/src/panels/drawer/EditableRevisionDrawer.tsx

Repository: Agenta-AI/agenta

Length of output: 11528


Script executed:

rg -n -F -e 'revisionId=' -e 'revisionId:' -e 'revisionId}' -- web/mobile/src/features web/mobile/src/pages | grep -E 'href|push|replace|revisionId='
rg -n -F -e 'onCommitted' -e 'setPending' -e 'isPending' -- web/packages/agenta-playground-ui/src/panels/drawer/EditableRevisionDrawerBody.tsx web/packages/agenta-playground-ui/src/panels/drawer
nl -ba web/packages/agenta-playground-ui/src/panels/drawer/EditableRevisionDrawerBody.tsx

Repository: Agenta-AI/agenta

Length of output: 14883


Script executed:

nl -ba web/packages/agenta-playground-ui/src/panels/drawer/DrawerCommitButton.tsx
rg -n -F -e 'CommitRevisionDialog' -e 'onCommit=' -- web/packages/agenta-playground-ui/src/panels/drawer/DrawerCommitButton.tsx

Repository: Agenta-AI/agenta

Length of output: 3826


Script executed:

rg -n -F -e 'CommitVariantChangesModal' -- web/packages/agenta-playground-ui/src

Repository: Agenta-AI/agenta

Length of output: 1826


Script executed:

nl -ba web/packages/agenta-playground-ui/src/components/CommitVariantChanges/CommitVariantChangesModal.tsx | sed -n '1,230p'

Repository: Agenta-AI/agenta

Length of output: 10946


Script executed:

nl -ba web/mobile/src/features/developer/useUrlParams.ts
nl -ba web/mobile/src/features/observability/TraceReferenceLabel.tsx
rg -n -F -e 'router.push(' -e 'router.replace(' -- web/mobile/src/features/app-overview web/mobile/src/features/variants web/mobile/src/features/developer

Repository: Agenta-AI/agenta

Length of output: 3411


Script executed:

rg -n -F -e 'router.push(' -e 'router.replace(' -e 'history.pushState(' -e 'history.replaceState(' -- web/mobile/src/features web/mobile/src/pages | grep -E 'revision|Revision|query|router\.push' 
nl -ba web/mobile/src/features/variants/VariantsScreen.tsx | sed -n '1,200p'

Repository: Agenta-AI/agenta

Length of output: 15913


Script executed:

rg -n -F -e 'export {' -e 'const Dialog' -e 'function Dialog' -e 'modal' -- web/packages/agenta-ui/src/ui web/mobile/src | head -120
rg -n -F -e 'revisionDrawerGate' -e 'Previous revision' -e 'Next revision' -e 'DeployRevisionButton' -- web/mobile web/packages | head -160

Repository: Agenta-AI/agenta

Length of output: 12120


Keep the deploy dialog bound to the revision selected when it opens.

The drawer can update the URL revision through onNavigate without remounting DeployRevisionButton. Because row is derived from the current revisionId, an open dialog can submit the newly selected revision. The dialog key only resets its internal state; it does not preserve the original deployment target.

Suggested fix

-    const [open, setOpen] = useState(false)
+    const [openRevisionId, setOpenRevisionId] = useState<string | null>(null)
...
-        open && data
+        openRevisionId === revisionId && data
...
-            <Button size="sm" disabled={!data} onClick={() => setOpen(true)}>
+            <Button size="sm" disabled={!data} onClick={() => setOpenRevisionId(revisionId)}>
...
-                    onClose={() => setOpen(false)}
+                    onClose={() => setOpenRevisionId(null)}

@mmabrouk
mmabrouk merged commit 87cd268 into release/v0.128.9 Nov 6, 2026
58 checks passed
@mmabrouk mmabrouk mentioned this pull request Nov 6, 2026
5 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant