feat(studio): Support Task based transforms - #1132
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
05f464b to
5b8f789
Compare
5b8f789 to
d758309
Compare
Signed-off-by: Sean Teramae <steramae@nvidia.com>
a3b8106 to
39c5523
Compare
📝 WalkthroughWalkthroughChangesThe pull request adds configurable target formats and output paths for dataset transformations. It supports row-aware Handlebars mappings, format-specific validation, previews, and mapping help. It also replaces preview file actions with shared quick actions and adds missing-path safeguards. Dataset transformation workflow
File action handling
Sequence Diagram(s)sequenceDiagram
participant TransformFileModal
participant useTransformPreview
participant useDatasetFileTransform
participant DatasetStorage
TransformFileModal->>useTransformPreview: apply mappings with one-based row number
useTransformPreview->>TransformFileModal: render preview values
TransformFileModal->>useDatasetFileTransform: submit source and output paths
useDatasetFileTransform->>DatasetStorage: upload transformed content to output path
useDatasetFileTransform->>DatasetStorage: invalidate output-file cache
Suggested reviewers: 🚥 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: 3
🧹 Nitpick comments (1)
web/packages/studio/src/components/FilesTable/TransformFileModal/types.ts (1)
4-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse type-only imports.
web/packages/studio/src/components/FilesTable/TransformFileModal/types.ts#L4-L8: importTargetFormatwithtype TargetFormat.web/packages/common/src/components/form/MappingFields/MappingRow.tsx#L19-L22: importReactNodewithtype ReactNode.web/packages/common/src/components/form/MappingFields/index.tsx#L20: importReactNodewithtype ReactNode.web/packages/studio/src/components/FilesTable/TransformFileModal/MappingValueHelp.tsx#L5: importFCwithtype FC.As per coding guidelines: “Use
import typefor type-only 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/FilesTable/TransformFileModal/types.ts` around lines 4 - 8, Convert the type-only imports to use import type: TargetFormat in web/packages/studio/src/components/FilesTable/TransformFileModal/types.ts lines 4-8, ReactNode in web/packages/common/src/components/form/MappingFields/MappingRow.tsx lines 19-22, ReactNode in web/packages/common/src/components/form/MappingFields/index.tsx line 20, and FC in web/packages/studio/src/components/FilesTable/TransformFileModal/MappingValueHelp.tsx line 5. Keep runtime imports such as getRequiredKeys and TARGET_FORMATS unchanged.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/FilesetFilePreviewPanel/components/FilesetFilePreviewHeader/index.tsx`:
- Around line 41-47: Update every production preview host that renders
FilesetFilePreviewHeader to pass the correct isReadWriteDataset value; where a
host cannot support writes, explicitly pass false. Ensure read-only previews do
not expose Move, Duplicate, Create Split, Transform, Rename, or Delete actions.
In `@web/packages/studio/src/components/FilesTable/FileQuickActions/index.tsx`:
- Around line 96-98: Restructure handleDeleteFile so the try/catch around
mutateAsync only handles mutation failures; after a successful response and
completed catch path, invoke onDeleteSuccess?.() outside the try/catch so
callback exceptions do not cause the delete operation to return false.
In
`@web/packages/studio/src/components/FilesTable/TransformFileModal/targetFormats.ts`:
- Around line 75-79: In targetFormats.ts, add capability metadata marking the
agent-eval-task format as incompatible with model inference. In
TransformFileModal/index.tsx, use that metadata to hide or disable ModelSelect,
clear any stale model value when the format changes, and reject incompatible
format/model combinations during form validation; update both consolidated sites
accordingly.
---
Nitpick comments:
In `@web/packages/studio/src/components/FilesTable/TransformFileModal/types.ts`:
- Around line 4-8: Convert the type-only imports to use import type:
TargetFormat in
web/packages/studio/src/components/FilesTable/TransformFileModal/types.ts lines
4-8, ReactNode in
web/packages/common/src/components/form/MappingFields/MappingRow.tsx lines
19-22, ReactNode in
web/packages/common/src/components/form/MappingFields/index.tsx line 20, and FC
in
web/packages/studio/src/components/FilesTable/TransformFileModal/MappingValueHelp.tsx
line 5. Keep runtime imports such as getRequiredKeys and TARGET_FORMATS
unchanged.
🪄 Autofix
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: 2abc285c-5e63-4fc8-ba6b-1c54f35c333a
📒 Files selected for processing (18)
web/packages/common/src/components/form/MappingFields/MappingRow.tsxweb/packages/common/src/components/form/MappingFields/index.tsxweb/packages/studio/src/api/datasets/useDatasetFileTransform.tsweb/packages/studio/src/components/FilesTable/FileQuickActions/index.test.tsxweb/packages/studio/src/components/FilesTable/FileQuickActions/index.tsxweb/packages/studio/src/components/FilesTable/TransformFileModal/MappingValueHelp.tsxweb/packages/studio/src/components/FilesTable/TransformFileModal/index.tsxweb/packages/studio/src/components/FilesTable/TransformFileModal/targetFormats.test.tsweb/packages/studio/src/components/FilesTable/TransformFileModal/targetFormats.tsweb/packages/studio/src/components/FilesTable/TransformFileModal/types.tsweb/packages/studio/src/components/FilesTable/TransformFileModal/useTransformPreview.tsweb/packages/studio/src/components/FilesetFilePreviewPanel/FilesetFilePreviewContent/index.tsxweb/packages/studio/src/components/FilesetFilePreviewPanel/components/FileActions/index.tsxweb/packages/studio/src/components/FilesetFilePreviewPanel/components/FilesetFilePreviewHeader/index.tsxweb/packages/studio/src/components/FilesetFilePreviewPanel/index.test.tsxweb/packages/studio/src/components/FilesetFilePreviewPanel/index.tsxweb/packages/studio/src/util/files.test.tsweb/packages/studio/src/util/files.ts
💤 Files with no reviewable changes (1)
- web/packages/studio/src/components/FilesetFilePreviewPanel/components/FileActions/index.tsx
| export const FilesetFilePreviewHeader: FC<FilesetFilePreviewHeaderProps> = ({ | ||
| workspace, | ||
| filesetName, | ||
| filePath, | ||
| file, | ||
| isReadWriteDataset = true, | ||
| actionsPlacement = 'inline', |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'<FilesetFilePreview(Header|Content|Panel)\b|isReadWriteDataset' \
web/packages/studio \
--glob '*.{ts,tsx}'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- preview panel/content call sites ---'
rg -n --glob '*.{ts,tsx}' \
'<FilesetFilePreviewPanel\b|<FilesetFilePreviewContent\b|<FilesetFilePreviewHeader\b|isReadWriteDataset' \
web/packages/studio/src \
| rg -v 'index\.test\.tsx'
printf '%s\n' '--- component implementations ---'
sed -n '1,180p' web/packages/studio/src/components/FilesetFilePreviewPanel/index.tsx
sed -n '1,190p' web/packages/studio/src/components/FilesetFilePreviewPanel/FilesetFilePreviewContent/index.tsx
sed -n '1,140p' web/packages/studio/src/components/FilesetFilePreviewPanel/components/FilesetFilePreviewHeader/index.tsx
printf '%s\n' '--- FileQuickActions implementation and usages ---'
rg -n -C 6 --glob '*.{ts,tsx}' 'FileQuickActions|isReadWriteDataset' web/packages/studio/srcRepository: NVIDIA-NeMo/nemo-platform
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for file in \
web/packages/studio/src/routes/DataDesignerJobDetailsRoute/JobOutputFilesetSection.tsx \
web/packages/studio/src/routes/JobDetailRoute/components/ArtifactFilesPanel.tsx \
web/packages/studio/src/routes/FilesetListRoute/PanelManagement/index.tsx \
web/packages/studio/src/routes/FilesetDetailRoute/FilesTab/index.tsx
do
echo "--- $file ---"
rg -n -C 18 '<FilesetFilePreview(Panel|Content)\b' "$file"
done
echo '--- quick-action defaults and action branches ---'
sed -n '1,175p' web/packages/studio/src/components/FilesTable/FileQuickActions/index.tsx
echo '--- dataset mutability derivation near preview hosts ---'
rg -n -C 10 'isReadWriteDataset\s*=|storage\.type|FilesetFilePreviewPanel|FilesetFilePreviewContent' \
web/packages/studio/src/routes \
--glob '*.{ts,tsx}' \
| rg -v 'index\.test\.tsx'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 39916
Pass isReadWriteDataset from every preview host, or default it to false.
All production preview hosts omit this prop. The header therefore enables Move, Duplicate, Create Split, Transform, Rename, and Delete for read-only previews.
🤖 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/FilesetFilePreviewPanel/components/FilesetFilePreviewHeader/index.tsx`
around lines 41 - 47, Update every production preview host that renders
FilesetFilePreviewHeader to pass the correct isReadWriteDataset value; where a
host cannot support writes, explicitly pass false. Ensure read-only previews do
not expose Move, Duplicate, Create Split, Transform, Rename, or Delete actions.
| if (response) { | ||
| onDeleteSuccess?.(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep callback failures separate from mutation failures.
onDeleteSuccess?.() runs inside the try block that catches mutateAsync. If the callback throws after the delete succeeds, handleDeleteFile returns false, so the caller receives an incorrect failure result. Limit the try/catch to mutateAsync, then invoke the callback after the catch.
Proposed fix
- try {
- const response = await mutateAsync({ workspace, datasetName: name, path });
- if (response) {
- onDeleteSuccess?.();
- }
- return Boolean(response);
+ let response: Awaited<ReturnType<typeof mutateAsync>>;
+ try {
+ response = await mutateAsync({ workspace, datasetName: name, path });
} catch {
return false;
}
+ if (response) {
+ onDeleteSuccess?.();
+ }
+ return Boolean(response);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (response) { | |
| onDeleteSuccess?.(); | |
| } | |
| let response: Awaited<ReturnType<typeof mutateAsync>>; | |
| try { | |
| response = await mutateAsync({ workspace, datasetName: name, path }); | |
| } catch { | |
| return false; | |
| } | |
| if (response) { | |
| onDeleteSuccess?.(); | |
| } | |
| return Boolean(response); |
🤖 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/FilesTable/FileQuickActions/index.tsx`
around lines 96 - 98, Restructure handleDeleteFile so the try/catch around
mutateAsync only handles mutation failures; after a successful response and
completed catch path, invoke onDeleteSuccess?.() outside the try/catch so
callback exceptions do not cause the delete operation to return false.
| key: 'inputs.instruction', | ||
| description: taskShape.inputs.unwrap().shape.instruction.description ?? '', | ||
| prefill: true, | ||
| defaultValue: firstMatchingColumn(['instruction', 'prompt', 'question']), | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Block model inference for agent-eval tasks.
The default mapping uses inputs.instruction. Model validation only accepts top-level prompt keys, so the default task mapping cannot submit with a model. If a user adds a top-level prompt key, inference wraps each row as { input, response, llm_name }, which is not an agent-eval task.
web/packages/studio/src/components/FilesTable/TransformFileModal/targetFormats.ts#L75-L79: add target-format capability metadata that marksagent-eval-taskas incompatible with model inference.web/packages/studio/src/components/FilesTable/TransformFileModal/index.tsx#L202-L212: hide or disableModelSelectfor incompatible formats, clear stale model values on format changes, and reject them in form validation.
📍 Affects 2 files
web/packages/studio/src/components/FilesTable/TransformFileModal/targetFormats.ts#L75-L79(this comment)web/packages/studio/src/components/FilesTable/TransformFileModal/index.tsx#L202-L212
🤖 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/FilesTable/TransformFileModal/targetFormats.ts`
around lines 75 - 79, In targetFormats.ts, add capability metadata marking the
agent-eval-task format as incompatible with model inference. In
TransformFileModal/index.tsx, use that metadata to hide or disable ModelSelect,
clear any stale model value when the format changes, and reject incompatible
format/model combinations during form validation; update both consolidated sites
accordingly.
|
|
Closing to refocus on what the best path forward should be for transforming a file. |
Signed-off-by: Sean Teramae steramae@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes