fix: Add auto-fix suggestions for definition issues in the warning dialog, starting with relation field type mismatch fixes#818
Conversation
…alog, starting with relation field type mismatch fixes + Also introduce sandbox for Claude Code
🦋 Changeset detectedLatest commit: 08003c2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThis PR introduces an auto-fix system for definition issues. It adds infrastructure to create and apply fixes (createIssueFixSetter), implements a concrete fix for relation type mismatches (createTypeMismatchFix), extends the DefinitionIssueFix type to support full definition mutations, and integrates fix-application UI into the warning dialog with tests and configuration updates. Changes
Sequence DiagramsequenceDiagram
participant User
participant DefinitionWarningDialog as Warning Dialog
participant FixSetter as createIssueFixSetter
participant SaveDef as saveDefinition
participant Definition as ProjectDefinition
User->>DefinitionWarningDialog: Click "Apply fix" button
DefinitionWarningDialog->>FixSetter: createIssueFixSetter(issue, container)
FixSetter-->>DefinitionWarningDialog: returns setter function
DefinitionWarningDialog->>SaveDef: Apply setter via saveDefinition
SaveDef->>Definition: Mutate draft (e.g., align field type)
Definition-->>SaveDef: Updated definition
SaveDef->>DefinitionWarningDialog: Fix applied, definition saved
DefinitionWarningDialog->>DefinitionWarningDialog: Remove issue from warnings or close dialog
DefinitionWarningDialog->>User: Show success toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/project-builder-lib/src/parser/definition-issue-checkers/relation-type-mismatch-checker.ts (2)
30-36: Redundant assignments in enum branch.Lines 31-32 assign
typeandoptionsdirectly todraftField, but lines 33-36 immediately overwrite them withObject.assign. The initial assignments are unnecessary.♻️ Proposed fix to remove redundant assignments
if (foreignField.type === 'enum') { - draftField.type = 'enum'; - draftField.options = { ...foreignField.options }; Object.assign(draftField, { type: 'enum', options: { ...foreignField.options }, }); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/project-builder-lib/src/parser/definition-issue-checkers/relation-type-mismatch-checker.ts` around lines 30 - 36, The enum branch in relation-type-mismatch-checker is doing duplicate assignments to draftField: first setting draftField.type and draftField.options, then immediately calling Object.assign(draftField, { type: 'enum', options: { ...foreignField.options } }), which is redundant; remove the initial direct assignments (draftField.type = 'enum' and draftField.options = { ...foreignField.options }) and keep the single Object.assign call (or vice versa) so draftField is only updated once with the enum type and copied options from foreignField.
37-42: Options reset to{}discards field-specific configurations from the foreign field.For enum fields, options are correctly copied from the foreign field (preserving
enumRef). For other types (uuid, dateTime, date), resetting options to{}discards any user-configured values likegenUuid: trueordefaultToNow: truethat may exist in the foreign field. While schema defaults apply during the initialdefSchema.parse()call before fixes run, resetting options afterwards loses field-specific configurations that users may have explicitly set.Consider copying applicable options from the foreign field consistently (as done for enum), or document this intentional behavior to clarify that non-default options are not preserved during type conversion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/project-builder-lib/src/parser/definition-issue-checkers/relation-type-mismatch-checker.ts` around lines 37 - 42, The code in relation-type-mismatch-checker.ts currently replaces non-enum field options with {} (Object.assign(draftField, { type: foreignField.type, options: {}, })), which discards user-configured options (e.g., genUuid, defaultToNow); modify the fix so it preserves applicable options from the foreign field instead of clearing them—for example, set options to a shallow copy of foreignField.options (or explicitly copy known applicable keys like genUuid, defaultToNow) when assigning draftField.type, while still preserving the existing enum branch that copies enumRef.packages/project-builder-web/src/components/definition-warning-dialog/definition-warning-dialog.tsx (1)
50-72: Consider showing success feedback even when new warnings remain.When a fix is successfully applied but
newWarnings.length > 0, the dialog updates with the new warnings but no success feedback is given. This might confuse users about whether their fix was actually applied.♻️ Proposed enhancement to show success toast when fix is applied
saveDefinition(setter) .then(({ warnings: newWarnings }) => { + toast.success(`Applied fix: ${fixLabel}`); if (newWarnings.length > 0) { setDialogOptions({ warnings: newWarnings }); } else { setDialogOptions(undefined); - toast.success(`Applied fix: ${fixLabel}`); } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/project-builder-web/src/components/definition-warning-dialog/definition-warning-dialog.tsx` around lines 50 - 72, In handleApplyFix, when saveDefinition resolves and returns newWarnings, always show a success toast for the applied fix even if newWarnings.length > 0; update the .then callback for saveDefinition (inside handleApplyFix) to call toast.success(`Applied fix: ${fixLabel}`) before or alongside setDialogOptions({ warnings: newWarnings }) so users get success feedback while the dialog refreshes with remaining warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/project-builder-lib/src/parser/definition-issue-checkers/relation-type-mismatch-checker.ts`:
- Around line 30-36: The enum branch in relation-type-mismatch-checker is doing
duplicate assignments to draftField: first setting draftField.type and
draftField.options, then immediately calling Object.assign(draftField, { type:
'enum', options: { ...foreignField.options } }), which is redundant; remove the
initial direct assignments (draftField.type = 'enum' and draftField.options = {
...foreignField.options }) and keep the single Object.assign call (or vice
versa) so draftField is only updated once with the enum type and copied options
from foreignField.
- Around line 37-42: The code in relation-type-mismatch-checker.ts currently
replaces non-enum field options with {} (Object.assign(draftField, { type:
foreignField.type, options: {}, })), which discards user-configured options
(e.g., genUuid, defaultToNow); modify the fix so it preserves applicable options
from the foreign field instead of clearing them—for example, set options to a
shallow copy of foreignField.options (or explicitly copy known applicable keys
like genUuid, defaultToNow) when assigning draftField.type, while still
preserving the existing enum branch that copies enumRef.
In
`@packages/project-builder-web/src/components/definition-warning-dialog/definition-warning-dialog.tsx`:
- Around line 50-72: In handleApplyFix, when saveDefinition resolves and returns
newWarnings, always show a success toast for the applied fix even if
newWarnings.length > 0; update the .then callback for saveDefinition (inside
handleApplyFix) to call toast.success(`Applied fix: ${fixLabel}`) before or
alongside setDialogOptions({ warnings: newWarnings }) so users get success
feedback while the dialog refreshes with remaining warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 39d8b5ef-f781-49ea-adc7-68e1bfdb711d
📒 Files selected for processing (12)
.changeset/suggest-auto-fixes-for-issues.md.claude/settings.json.vscode/tasks.jsonexamples/blog-with-auth/baseplate/project-definition.jsonpackages/project-builder-lib/src/parser/collect-definition-issues.unit.test.tspackages/project-builder-lib/src/parser/definition-issue-checkers/relation-type-mismatch-checker.tspackages/project-builder-lib/src/parser/definition-issue-checkers/relation-type-mismatch-checker.unit.test.tspackages/project-builder-lib/src/parser/definition-issue-utils.tspackages/project-builder-lib/src/parser/definition-issue-utils.unit.test.tspackages/project-builder-lib/src/parser/index.tspackages/project-builder-lib/src/schema/creator/definition-issue-types.tspackages/project-builder-web/src/components/definition-warning-dialog/definition-warning-dialog.tsx
Summary by CodeRabbit
New Features
Tests