feat(studio): DD row editor in details - #559
Merged
Merged
Conversation
steramae-nvidia
changed the title
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Data tab for dataset editing, with staged-save and file-persistence behavior in the row editor. Profiler and output sections now use shared empty/loading states. The mock dataset request spec is rewritten to a smaller quarterly company schema. ChangesData Designer Job Details UI
Sequence Diagram(s)sequenceDiagram
participant User
participant JobDatasetEditorSection
participant useDatasetFileContent
participant parseDataFile
participant FileRowEditor
User->>JobDatasetEditorSection: select file
JobDatasetEditorSection->>useDatasetFileContent: fetch content
useDatasetFileContent-->>JobDatasetEditorSection: content or error
JobDatasetEditorSection->>parseDataFile: parse content
parseDataFile-->>JobDatasetEditorSection: rows or parse error
JobDatasetEditorSection->>FileRowEditor: render rows
sequenceDiagram
participant User
participant FileHeader
participant FileRowEditor
participant onSaveFile
User->>FileHeader: click Save File
FileHeader->>FileRowEditor: trigger save
FileRowEditor->>onSaveFile: persist edited rows
onSaveFile-->>FileRowEditor: success or error
FileRowEditor->>FileHeader: update dirty and disabled state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
web/packages/studio/src/routes/DataDesignerJobDetailsRoute/JobDatasetEditorSection.tsx (1)
4-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport order:
react(external) placed after internal@studioimports.As per coding guidelines, "Group imports: external libraries, internal modules, relative imports".
reactshould be grouped with@nvidia/foundations-react-coreat the top, not trailing internal 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/routes/DataDesignerJobDetailsRoute/JobDatasetEditorSection.tsx` around lines 4 - 27, The import grouping in JobDatasetEditorSection is out of order because the external react import is placed after internal `@studio` imports. Reorder the imports so all external libraries, including react and `@nvidia/foundations-react-core`, are grouped at the top before internal modules, keeping the existing symbols like useEffect, useMemo, useState, FC, and ReactNode in the same import from react.Source: Coding guidelines
web/packages/studio/src/routes/DataDesignerJobDetailsRoute/JobOutputFilesetSection.tsx (1)
144-187: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated
Cardwrapper markup across three branches.Same
className/attributescombo repeated for loading, error, and empty states. Consider extracting a small local wrapper component.🤖 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/routes/DataDesignerJobDetailsRoute/JobOutputFilesetSection.tsx` around lines 144 - 187, Repeated Card wrapper markup is duplicated across the loading, error, and empty branches in JobOutputFilesetSection; extract a small local helper/component around the shared Card structure and reuse it in the isResultsLoading, isResultsError, and !artifactsResult paths. Keep the branch-specific Spinner and Empty content inside the wrapper, but centralize the common className and attributes props so the section is easier to maintain.
🤖 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/routes/DataDesignerJobDetailsRoute/JobDatasetEditorSection.tsx`:
- Around line 182-184: The parse-error branch in JobDatasetEditorSection is
rendering Empty directly, unlike the other terminal states that use
centered(...) for consistent Card alignment. Update the parsed.error path in
JobDatasetEditorSection to wrap the Empty component with centered(...) so the
“Could not parse file” state matches the layout of the no-files, loading, and
content-error states.
- Around line 69-153: `SelectRoot` in `JobDatasetEditorSection` is switching
from uncontrolled to controlled because `selectedPath` starts as null and later
becomes a file path via the effect. Initialize the selection state from
`defaultPath` (or another stable controlled value) on first render, and keep the
`value` prop consistently defined so `SelectRoot`, `selectedPath`, and the
`useEffect` defaulting logic stay in sync.
In
`@web/packages/studio/src/routes/DataDesignerJobDetailsRoute/JobOutputFilesetSection.tsx`:
- Around line 172-184: The fallback content in JobOutputFilesetSection’s
artifactsResult-empty state is duplicating the same message for both Empty
props. Update the conditional logic around emptyTitle and emptyDescription so
the Empty component gets distinct title and description text in both the
terminal and non-terminal branches, using the existing isTerminal check in
JobOutputFilesetSection.
---
Nitpick comments:
In
`@web/packages/studio/src/routes/DataDesignerJobDetailsRoute/JobDatasetEditorSection.tsx`:
- Around line 4-27: The import grouping in JobDatasetEditorSection is out of
order because the external react import is placed after internal `@studio`
imports. Reorder the imports so all external libraries, including react and
`@nvidia/foundations-react-core`, are grouped at the top before internal modules,
keeping the existing symbols like useEffect, useMemo, useState, FC, and
ReactNode in the same import from react.
In
`@web/packages/studio/src/routes/DataDesignerJobDetailsRoute/JobOutputFilesetSection.tsx`:
- Around line 144-187: Repeated Card wrapper markup is duplicated across the
loading, error, and empty branches in JobOutputFilesetSection; extract a small
local helper/component around the shared Card structure and reuse it in the
isResultsLoading, isResultsError, and !artifactsResult paths. Keep the
branch-specific Spinner and Empty content inside the wrapper, but centralize the
common className and attributes props so the section is easier to maintain.
🪄 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: 3b6ccbca-c0a0-4b42-a0d7-09e33c640b2d
📒 Files selected for processing (5)
web/packages/studio/src/mocks/data-designer/example_request.txtweb/packages/studio/src/routes/DataDesignerJobDetailsRoute/DatasetProfilerSection.tsxweb/packages/studio/src/routes/DataDesignerJobDetailsRoute/JobDatasetEditorSection.tsxweb/packages/studio/src/routes/DataDesignerJobDetailsRoute/JobOutputFilesetSection.tsxweb/packages/studio/src/routes/DataDesignerJobDetailsRoute/index.tsx
Contributor
|
Signed-off-by: Sean Teramae <steramae@nvidia.com>
Signed-off-by: Sean Teramae <steramae@nvidia.com>
Signed-off-by: Sean Teramae <steramae@nvidia.com>
Signed-off-by: Sean Teramae <steramae@nvidia.com>
Contributor
Author
|
This change is part of the following stack: Change managed by git-spice. |
steramae-nvidia
force-pushed
the
steramae/dd-row-editor
branch
from
July 7, 2026 21:27
0906a83 to
81cd00b
Compare
htolentino-nvidia
approved these changes
Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Summary by CodeRabbit