Skip to content

Conversation

@ArcticFaded
Copy link
Contributor

@ArcticFaded ArcticFaded commented Nov 13, 2025

Note

Replaces add/remove split mutations with a single idempotent set-example-splits operation and updates the UI to use it with improved menu behavior.

  • GraphQL API:
    • Remove addDatasetExamplesToDatasetSplits and removeDatasetExamplesFromDatasetSplits inputs/mutations/payloads.
    • Add setDatasetExampleSplits with SetDatasetExampleSplitsInput and SetDatasetExampleSplitsMutationPayload returning a single example.
  • Backend (src/phoenix/server/api/mutations/dataset_split_mutations.py):
    • Implement set_dataset_example_splits to atomically set splits for an example (validates IDs, deletes missing, inserts new; uses joinedload and tuple_).
    • Remove legacy add/remove example-split mutations and related types.
  • Frontend (app/src/pages/examples/ExamplesSplitMenu.tsx):
    • Switch to ExamplesSplitMenuSetDatasetExampleSplitsMutation and update apply logic to send full splitIds per exampleId.
    • Change apply menu to single-select with manual open-state control to keep menu open on actions; add onAction handler and checkbox state sync.
    • Remove generated file for old remove mutation; add/update generated file for new set mutation.

Written by Cursor Bugbot for commit 18bb22d. This will update automatically on new commits. Configure here.

@ArcticFaded ArcticFaded marked this pull request as ready for review November 14, 2025 16:58
@ArcticFaded ArcticFaded requested review from a team as code owners November 14, 2025 16:58
@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Nov 14, 2025
}, {} as SplitState);
}, [splits, selectedPartialExamples]);
const handleSplitToggle = useCallback(
(selectedId: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(selectedId: string) => {
(selectedSplitId: string) => {

Comment on lines +112 to +118
if (!open && shouldKeepMenuOpenRef.current) {
shouldKeepMenuOpenRef.current = false;
setIsMenuOpen(true);
return;
}
shouldKeepMenuOpenRef.current = false;
setIsMenuOpen(open);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a comment here explaining the need for this?

Comment on lines 85 to 86
const [isMenuOpen, setIsMenuOpen] = useState(false);
const shouldKeepMenuOpenRef = useRef(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably worth a comment.

}, [splits, selectedPartialExamples]);
const handleSplitToggle = useCallback(
(selectedId: string) => {
onRequestKeepMenuOpen();
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth a comment.

@github-project-automation github-project-automation bot moved this from 📘 Todo to 👍 Approved in phoenix Nov 14, 2025
@ArcticFaded ArcticFaded merged commit 2d47be0 into main Nov 19, 2025
46 of 50 checks passed
@ArcticFaded ArcticFaded deleted the splits-cleanup branch November 19, 2025 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants