feat(studio): Transform File Modal with preview schema output - #1032
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
|
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:
📝 WalkthroughWalkthroughThe PR extracts reusable mapping rows, adds dataset-aware file actions, and replaces transform schema output with generated source-to-transformed previews. ChangesStudio mapping and transform workflow
Sequence Diagram(s)sequenceDiagram
participant User
participant TransformFileModal
participant useTransformPreview
participant PreviewOutputPanel
User->>TransformFileModal: open transform modal
TransformFileModal->>useTransformPreview: pass file content and mappings
useTransformPreview->>PreviewOutputPanel: provide source and transformed row
TransformFileModal->>PreviewOutputPanel: render preview values
User->>PreviewOutputPanel: navigate rows or copy output
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
web/packages/common/src/components/form/MappingFields/types.ts (1)
13-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType-only imports use value import syntax in both new files. Both files import symbols that appear only in type positions.
web/packages/common/src/components/form/MappingFields/types.ts#L13-L14: convert theControlledComboboxandControlledTextInputimports toimport type.web/packages/common/src/components/form/MappingFields/MappingRow.tsx#L23-L23: convert theControlandFieldValuesimport toimport type.As per coding guidelines: "Use
import typefor type-only imports in TypeScript".🤖 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/common/src/components/form/MappingFields/types.ts` around lines 13 - 14, Update web/packages/common/src/components/form/MappingFields/types.ts lines 13-14 to use type-only imports for ControlledCombobox and ControlledTextInput, and update web/packages/common/src/components/form/MappingFields/MappingRow.tsx line 23 to use a type-only import for Control and FieldValues; no other changes are needed.Source: Coding guidelines
web/packages/common/src/components/form/MappingFields/index.tsx (1)
194-201: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the four assertions.
{}is already assignable toPartial<...>, so??infers the correct type. TypeNO_OVERRIDESonce instead.♻️ Proposed change
- const keyComboboxProps = (attributes?.keyCombobox ?? - NO_OVERRIDES) as Partial<KeyValueComboboxPassthrough>; - const valueComboboxProps = (attributes?.valueCombobox ?? - NO_OVERRIDES) as Partial<KeyValueComboboxPassthrough>; - const keyTextInputProps = (attributes?.keyTextInput ?? - NO_OVERRIDES) as Partial<KeyValueTextInputPassthrough>; - const valueTextInputProps = (attributes?.valueTextInput ?? - NO_OVERRIDES) as Partial<KeyValueTextInputPassthrough>; + const keyComboboxProps = attributes?.keyCombobox ?? NO_OVERRIDES; + const valueComboboxProps = attributes?.valueCombobox ?? NO_OVERRIDES; + const keyTextInputProps = attributes?.keyTextInput ?? NO_OVERRIDES; + const valueTextInputProps = attributes?.valueTextInput ?? NO_OVERRIDES;With line 67:
const NO_OVERRIDES: Readonly<Record<string, never>> = {};As per coding guidelines: "Use type assertions sparingly. Prefer type guards and narrowing in TypeScript".
🤖 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/common/src/components/form/MappingFields/index.tsx` around lines 194 - 201, Remove the four type assertions from the keyComboboxProps, valueComboboxProps, keyTextInputProps, and valueTextInputProps initializations. Type NO_OVERRIDES once as Readonly<Record<string, never>> near its declaration, allowing each nullish-coalescing expression to infer the appropriate Partial passthrough type.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/FilesTable/TransformFileModal/index.tsx`:
- Around line 64-65: Update the fileType derivation near resolvedFilepath so
filenames without a dot produce an empty file type instead of the full filename;
preserve extracting the final suffix for paths with extensions, ensuring
parseFileContent does not misclassify extensionless names based on substring
matches.
In
`@web/packages/studio/src/components/FilesTable/TransformFileModal/TransformPreview.tsx`:
- Around line 4-8: Use type-only imports for the specified symbols: in
web/packages/studio/src/components/FilesTable/TransformFileModal/TransformPreview.tsx
lines 4-8, import TransformFileFormFields, FC, and Control with import type; in
web/packages/studio/src/components/FilesTable/TransformFileModal/index.tsx lines
15-18, mark TransformFileFormFields as type while keeping transformFileSchema a
value import; and on line 25, mark ComponentProps and FC as type while keeping
useMemo as a value import.
In
`@web/packages/studio/src/components/FilesTable/TransformFileModal/useTransformPreview.ts`:
- Around line 21-46: Update the nested-path construction in the mapping loop so
each intermediate segment is verified to be a non-null object before assigning
into it; replace any existing primitive or null value with a fresh object before
reassigning current. Preserve existing object branches and ensure conflicting
mappings such as “user” followed by “user.name” complete without throwing.
In `@web/packages/studio/src/components/PreviewOutputPanel/index.tsx`:
- Around line 51-60: Add an aria-expanded attribute to the collapse toggle
Button, deriving its boolean value from the existing collapsed state so expanded
and collapsed states are accurately exposed to screen readers. Keep the current
onClick handler, icons, and aria-label unchanged.
---
Nitpick comments:
In `@web/packages/common/src/components/form/MappingFields/index.tsx`:
- Around line 194-201: Remove the four type assertions from the
keyComboboxProps, valueComboboxProps, keyTextInputProps, and valueTextInputProps
initializations. Type NO_OVERRIDES once as Readonly<Record<string, never>> near
its declaration, allowing each nullish-coalescing expression to infer the
appropriate Partial passthrough type.
In `@web/packages/common/src/components/form/MappingFields/types.ts`:
- Around line 13-14: Update
web/packages/common/src/components/form/MappingFields/types.ts lines 13-14 to
use type-only imports for ControlledCombobox and ControlledTextInput, and update
web/packages/common/src/components/form/MappingFields/MappingRow.tsx line 23 to
use a type-only import for Control and FieldValues; no other changes are needed.
🪄 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: 9791b903-8fcb-4256-bc16-d8344b32b4a4
📒 Files selected for processing (9)
web/packages/common/src/components/form/MappingFields/MappingRow.tsxweb/packages/common/src/components/form/MappingFields/index.tsxweb/packages/common/src/components/form/MappingFields/types.tsweb/packages/studio/src/components/FilesTable/FileQuickActions/index.tsxweb/packages/studio/src/components/FilesTable/RenameFileModal/index.tsxweb/packages/studio/src/components/FilesTable/TransformFileModal/TransformPreview.tsxweb/packages/studio/src/components/FilesTable/TransformFileModal/index.tsxweb/packages/studio/src/components/FilesTable/TransformFileModal/useTransformPreview.tsweb/packages/studio/src/components/PreviewOutputPanel/index.tsx
|
2cec860 to
723c181
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/FilesTable/TransformFileModal/useTransformPreview.test.ts`:
- Around line 4-5: Reorder the imports in the useTransformPreview test so the
external `@testing-library/react` import appears before the internal
`@studio/components/`... alias import, preserving both imports unchanged.
🪄 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: b2ac7d8e-65c5-45c5-8b25-dd6efaf44b72
📒 Files selected for processing (5)
web/packages/studio/src/components/FilesTable/TransformFileModal/TransformPreview.tsxweb/packages/studio/src/components/FilesTable/TransformFileModal/index.tsxweb/packages/studio/src/components/FilesTable/TransformFileModal/useTransformPreview.test.tsweb/packages/studio/src/components/FilesTable/TransformFileModal/useTransformPreview.tsweb/packages/studio/src/components/PreviewOutputPanel/index.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- web/packages/studio/src/components/PreviewOutputPanel/index.tsx
- web/packages/studio/src/components/FilesTable/TransformFileModal/TransformPreview.tsx
- web/packages/studio/src/components/FilesTable/TransformFileModal/useTransformPreview.ts
- web/packages/studio/src/components/FilesTable/TransformFileModal/index.tsx
74dc94d to
900aa57
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Signed-off-by: Sean Teramae <steramae@nvidia.com>
Signed-off-by: Sean Teramae <steramae@nvidia.com>
900aa57 to
d68ab14
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web/packages/studio/src/components/FilesTable/TransformFileModal/useTransformPreview.ts (1)
60-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit return type for
useTransformPreview.This exported hook is a complex public API. Define its return interface and annotate the function return type.
As per coding guidelines, “specify return types for public APIs and complex functions.”
🤖 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/useTransformPreview.ts` around lines 60 - 93, Define an explicit return interface for the exported useTransformPreview hook, including the types of currentRow, totalRows, sourceRow, afterRow, and onRowChange, then annotate the hook with that return type while preserving its existing behavior.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/FilesTable/TransformFileModal/useTransformPreview.ts`:
- Around line 21-34: Update the mapping traversal in the transform preview logic
to trim the mapping key before splitting it, then reject mappings with empty
path segments or segments named __proto__, constructor, or prototype. Apply
validation before accessing or assigning through current in the keyParts loop,
while preserving normal nested-object construction for valid paths.
- Around line 74-84: Clamp the displayed row in the transform preview when
rows.length changes so currentRow cannot exceed the available rows. Update the
row-selection logic around rowIndex, sourceRow, and totalRows, or reset
currentRow in response to row-count changes, ensuring PreviewOutputPanel
displays a valid “Row N of totalRows” value.
- Around line 24-38: Update the mapping-processing logic in useTransformPreview
so both Handlebars.compile and the compiled template invocation are inside
per-mapping error handling. When either fails, retain a recoverable value for
that mapping and continue processing remaining mappings instead of aborting
useMemo or TransformPreview rendering. Add coverage for invalid/incomplete
Handlebars templates.
---
Nitpick comments:
In
`@web/packages/studio/src/components/FilesTable/TransformFileModal/useTransformPreview.ts`:
- Around line 60-93: Define an explicit return interface for the exported
useTransformPreview hook, including the types of currentRow, totalRows,
sourceRow, afterRow, and onRowChange, then annotate the hook with that return
type while preserving its existing behavior.
🪄 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: 287dd698-4e9f-4d6a-9027-ef5e9fbc655c
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
web/packages/common/src/components/form/MappingFields/MappingRow.tsxweb/packages/common/src/components/form/MappingFields/index.tsxweb/packages/common/src/components/form/MappingFields/types.tsweb/packages/studio/src/components/FilesTable/FileQuickActions/index.tsxweb/packages/studio/src/components/FilesTable/RenameFileModal/index.tsxweb/packages/studio/src/components/FilesTable/TransformFileModal/TransformPreview.tsxweb/packages/studio/src/components/FilesTable/TransformFileModal/index.tsxweb/packages/studio/src/components/FilesTable/TransformFileModal/useTransformPreview.test.tsweb/packages/studio/src/components/FilesTable/TransformFileModal/useTransformPreview.tsweb/packages/studio/src/components/PreviewOutputPanel/index.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- web/packages/studio/src/components/FilesTable/FileQuickActions/index.tsx
- web/packages/common/src/components/form/MappingFields/types.ts
- web/packages/studio/src/components/PreviewOutputPanel/index.tsx
- web/packages/common/src/components/form/MappingFields/index.tsx
- web/packages/studio/src/components/FilesTable/TransformFileModal/index.tsx
- web/packages/studio/src/components/FilesTable/TransformFileModal/useTransformPreview.test.ts
- web/packages/studio/src/components/FilesTable/TransformFileModal/TransformPreview.tsx
- web/packages/common/src/components/form/MappingFields/MappingRow.tsx
- web/packages/studio/src/components/FilesTable/RenameFileModal/index.tsx
Signed-off-by: Sean Teramae <steramae@nvidia.com>
Signed-off-by: Sean Teramae steramae@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes