feat(experiments): Add inline add/remove from experiment actions - #923
Conversation
Signed-off-by: shanaiabuggy <59746633+shanaiabuggy@users.noreply.github.com>
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a right-pinned row actions menu to the experiment group table and implements ChangesExperiment group actions
Sequence Diagram(s)sequenceDiagram
participant User
participant QuickActionsMenuRoot
participant useExperimentGroupEvaluations
participant usePatchEvaluation
User->>QuickActionsMenuRoot: Select Remove from group
QuickActionsMenuRoot->>useExperimentGroupEvaluations: removeFromGroup(row)
useExperimentGroupEvaluations->>usePatchEvaluation: Patch remaining experiment_ids
usePatchEvaluation-->>useExperimentGroupEvaluations: Return mutation result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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/ExperimentGroupDataView/index.tsx (1)
39-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePlace
lucide-reactwith external imports.Line 44 is an external import but follows internal
@studioimports.As per coding guidelines, group imports as external libraries, internal modules, then relative imports.
🤖 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/ExperimentGroupDataView/index.tsx` around lines 39 - 44, Move the lucide-react import containing Columns3, FolderMinus, FolderPlus, and Pin above the internal `@studio` imports, preserving the grouping order of external libraries, internal modules, and relative imports.Source: Coding guidelines
🤖 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/ExperimentGroupDataView/useExperimentGroupEvaluations.ts`:
- Around line 195-201: Update the onSuccess handler in the evaluation removal
mutation to return the invalidateList() promise after showing the success toast.
Keep the existing onSettled cleanup unchanged so pendingRef remains locked until
invalidation completes.
---
Nitpick comments:
In
`@web/packages/studio/src/components/dataViews/ExperimentGroupDataView/index.tsx`:
- Around line 39-44: Move the lucide-react import containing Columns3,
FolderMinus, FolderPlus, and Pin above the internal `@studio` imports, preserving
the grouping order of external libraries, internal modules, and relative
imports.
🪄 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: fa9931ab-fc94-4ab3-88cf-f497b0b8d1ad
📒 Files selected for processing (3)
web/packages/studio/src/components/dataViews/ExperimentGroupDataView/index.tsxweb/packages/studio/src/components/dataViews/ExperimentGroupDataView/useExperimentGroupEvaluations.test.tsweb/packages/studio/src/components/dataViews/ExperimentGroupDataView/useExperimentGroupEvaluations.ts
|
Address CodeRabbit on PR #923: removeFromGroup's onSuccess called invalidateList() fire-and-forget, so onSettled could clear the pending-row lock before the refetch finished. Return the invalidation promise (like the pin/unpin mutations) so the row stays locked until the list updates. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: shanaiabuggy <59746633+shanaiabuggy@users.noreply.github.com>
Address CodeRabbit on PR #923: removeFromGroup's onSuccess called invalidateList() fire-and-forget, so onSettled could clear the pending-row lock before the refetch finished. Return the invalidation promise (like the pin/unpin mutations) so the row stays locked until the list updates. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: shanaiabuggy <59746633+shanaiabuggy@users.noreply.github.com>
5afa1fb to
3117dee
Compare
Screen.Recording.2026-07-27.at.2.15.07.PM.mov
Summary by CodeRabbit
Summary
New Features
Bug Fixes
Tests