feat(studio): bulk delete agents from the table#320
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAgentsDataView adds multi-row selection and a bulk-action toolbar. Deletion now removes each agent’s deployments (parallel, ignoring 404s) then deletes agents, aggregates outcomes with Promise.allSettled, refreshes queries, conditionally clears selection, and shows pluralized modal/toast. Tests cover success and partial-failure bulk flows. ChangesBulk Agent Deletion
Possibly related PRs
Suggested reviewers
🚥 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/packages/studio/src/components/dataViews/AgentsDataView/index.spec.tsx (1)
621-622: 📐 Maintainability & Code Quality | 💤 Low valueConsider adding partial failure test coverage.
The bulk delete logic handles partial failures ("Failed to delete X of Y agents."), but this path isn't tested. Adding a test where one agent delete succeeds and another fails would validate the aggregation logic and error toast.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/components/dataViews/AgentsDataView/index.spec.tsx` around lines 621 - 622, Add a unit test in AgentsDataView's index.spec.tsx that simulates a partial failure for the bulk delete path: mock the API used by the component (the same mock used elsewhere in the file) so one agent delete resolves and the other rejects, render the AgentsDataView, select both agents, trigger the bulk delete action (the same UI interaction used in existing bulk-delete tests), then assert that the error toast contains the partial-failure message (e.g., "Failed to delete 1 of 2 agents.") and that the success/refresh flow for the succeeded delete still ran; reference the AgentsDataView component and the bulk delete handler (e.g., bulkDeleteAgents/handleBulkDelete) to locate the code to mock and the toast assertion to add.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/packages/studio/src/components/dataViews/AgentsDataView/index.tsx`:
- Around line 314-316: Remove the unsupported variant prop from the lucide-react
X icon usage in AgentsDataView: locate the JSX element <X variant="line" /> in
the component (index.tsx) and delete the variant="line" attribute; if you
intended a thinner stroke, replace it with a supported prop such as
strokeWidth={1} or absoluteStrokeWidth as needed.
---
Nitpick comments:
In `@web/packages/studio/src/components/dataViews/AgentsDataView/index.spec.tsx`:
- Around line 621-622: Add a unit test in AgentsDataView's index.spec.tsx that
simulates a partial failure for the bulk delete path: mock the API used by the
component (the same mock used elsewhere in the file) so one agent delete
resolves and the other rejects, render the AgentsDataView, select both agents,
trigger the bulk delete action (the same UI interaction used in existing
bulk-delete tests), then assert that the error toast contains the
partial-failure message (e.g., "Failed to delete 1 of 2 agents.") and that the
success/refresh flow for the succeeded delete still ran; reference the
AgentsDataView component and the bulk delete handler (e.g.,
bulkDeleteAgents/handleBulkDelete) to locate the code to mock and the toast
assertion to add.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 51177801-a746-4fc5-9cca-58054965b2c8
📒 Files selected for processing (2)
web/packages/studio/src/components/dataViews/AgentsDataView/index.spec.tsxweb/packages/studio/src/components/dataViews/AgentsDataView/index.tsx
|
5a83c75 to
bcdfe53
Compare
Add row selection (checkboxes + select-all) to the agents table and a selection action bar (N selected / Delete / Clear) shown when rows are selected. Delete opens the existing confirmation modal and tears down each selected agent (deployments first, then the agent) via Promise.allSettled, with a single summary toast. The DataView's built-in bulk-action toolbar only renders when a search field or filterable column exists, and the agents list API supports neither, so the selection bar is rendered directly in AgentsDataView. ASTD-246 Signed-off-by: mschwab <mschwab@nvidia.com>
bcdfe53 to
8458707
Compare
|
@coderabbitai Claude says: "Added a partial-failure test in 8458707 — one of two selected agents fails to delete; it asserts the |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
gh api repos/NVIDIA-NeMo/nemo-platform/commits/8458707d6e --jq '.commit.message' 2>&1 || echo "Commit not found or inaccessible"
gh api "repos/NVIDIA-NeMo/nemo-platform/pulls/320/commits" --jq '.[].sha' 2>&1 | head -20Length of output: 950 🏁 Script executed: #!/bin/bash
gh api "repos/NVIDIA-NeMo/nemo-platform/pulls/320/commits" --jq '.[] | {sha: .sha, message: .commit.message}' 2>&1Length of output: 892 🏁 Script executed: #!/bin/bash
gh api "repos/NVIDIA-NeMo/nemo-platform/contents/web/packages/studio/src/components/dataViews/AgentsDataView/index.spec.tsx?ref=8458707d6e260ccc391735162a177ccdacdf035d" --jq '.content' | base64 -d | grep -n "partial\|Failed to delete\|fail\|1 of 2\|allSettled\|reject\|error" 2>&1 | head -40Length of output: 1131
|
Summary
Adds bulk delete of agents from the agents table.
BulkActionsUX).DeleteConfirmationModal("Delete N Agents"), then tears down each selected agent the same way single-delete does — delete the agent's deployments first, then the agent — running all selected agents viaPromise.allSettled. One summary toast ("N agents deleted."or"Failed to delete X of N agents."), selection cleared, list refetched once. Single-row delete uses the same unified path.Why a custom selection bar (not the DataView toolbar slot)
The DataView's built-in
renderBulkActionsonly renders insideStudioDataViewToolbar, which returnsnullunless the table has a search field or a filterable column.AgentsListAgentsParamssupports neither (no filter/search on the agents list API), so that toolbar can't appear for agents. The selection bar is therefore rendered directly inAgentsDataView(the checkboxes are columns, so they're unaffected), styled to match the canonicalBulkActionscomponent.Testing
AgentsDataViewspec: existing 17 + new bulk-delete test (select all → Delete → both agents' deployments + agents deleted, summary toast) → 18 pass.Resolves ASTD-246.
Summary by CodeRabbit