✨(db-structure): Remove column.primary duplication with constraints #2205
Conversation
Remove the duplication between column.primary field and PRIMARY KEY constraints by: - Removing primary field from column schema - Creating isPrimaryKey() utility function to detect primary keys from constraints - Updating all UI components to use isPrimaryKey() instead of column.primary - Updating parsers to only set PRIMARY KEY constraints - Providing migratePrimaryToConstraints() for data migration This improves data consistency, simplifies editing, and better supports composite primary keys by having a single source of truth for primary key information. Closes #2192 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
|
Updates to Preview Branch (remove-primary-field) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
…statements - Modified generateCreateTableStatement to add PRIMARY KEY inline with column definitions - Fixed TypeScript errors for array access safety - Updated test snapshots to reflect correct PRIMARY KEY generation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Delete migratePrimaryToConstraints.ts file as it was never used - Remove export from index.ts 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Modified generateColumnDefinition to accept isPrimaryKey parameter - Prevent adding NOT NULL when column will be PRIMARY KEY - PRIMARY KEY columns now show as 'bigint PRIMARY KEY' not 'bigint PRIMARY KEY NOT NULL' - Updated test snapshots to match correct behavior from origin/main 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Remove the duplication between column.primary field and PRIMARY KEY constraints by: - Removing primary field from column schema - Creating isPrimaryKey() utility function to detect primary keys from constraints - Updating all UI components to use isPrimaryKey() instead of column.primary - Updating parsers to only set PRIMARY KEY constraints - Providing migratePrimaryToConstraints() for data migration This improves data consistency, simplifies editing, and better supports composite primary keys by having a single source of truth for primary key information. Closes #2192 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Fixed merge conflicts in deparser test files to maintain PRIMARY KEY behavior - Removed remaining primary field references from agent test - Fixed import order in index.ts to satisfy biome linter - All lint and type checks now pass 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…oyment - Added dependency on @liam-hq/db-structure for Storybook build in turbo.json - Updated build command in vercel.json to ensure correct path and filtering for Storybook This change ensures that the Storybook build process correctly includes necessary dependencies and operates as expected in the Vercel environment.
PR Reviewer Guide 🔍(Review updated until commit 25c5c52)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to 25c5c52
Previous suggestions✅ Suggestions up to commit 529e683
|
||||||||||||||||||
WalkthroughThis change removes the Changes
Sequence Diagram(s)sequenceDiagram
participant UI_Component
participant isPrimaryKey
participant Table_Constraints
UI_Component->>isPrimaryKey: isPrimaryKey(column.name, table.constraints)
isPrimaryKey->>Table_Constraints: Check for PRIMARY KEY constraint on column
isPrimaryKey-->>UI_Component: true/false (primary key status)
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/packages/db-structure/src/utils/isPrimaryKey.ts (1)
3-11: Well-implemented utility function with proper TypeScript usage.The function correctly implements primary key detection based on constraints. The use of
Object.values()withsome()provides efficient early termination, and the type annotations follow the coding guidelines.Consider future enhancement for composite primary keys if the constraint model supports them (e.g., if
columnNamecould be an array or if there are separate constraint types for composite keys).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/packages/db-structure/src/parser/__snapshots__/index.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (38)
frontend/apps/app/components/ChatInput/components/MentionSuggestor/utils/getAllMentionCandidates.ts(2 hunks)frontend/internal-packages/agent/src/chat/workflow/workflow.test.ts(1 hunks)frontend/internal-packages/agent/src/utils/convertSchemaToText.ts(2 hunks)frontend/packages/db-structure/src/deparser/postgresql/operationDeparser.test.ts(2 hunks)frontend/packages/db-structure/src/deparser/postgresql/schemaDeparser.test.ts(11 hunks)frontend/packages/db-structure/src/deparser/postgresql/utils.ts(2 hunks)frontend/packages/db-structure/src/diff/buildSchemaDiff.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnCheckDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnCommentDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnDefaultDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnNameDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnNotNullDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnPrimaryDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnUniqueDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/buildColumnPrimaryDiffItem.ts(0 hunks)frontend/packages/db-structure/src/diff/types.ts(0 hunks)frontend/packages/db-structure/src/index.ts(1 hunks)frontend/packages/db-structure/src/operation/constants.ts(0 hunks)frontend/packages/db-structure/src/parser/prisma/index.test.ts(0 hunks)frontend/packages/db-structure/src/parser/prisma/parser.ts(0 hunks)frontend/packages/db-structure/src/parser/schemarb/index.test.ts(0 hunks)frontend/packages/db-structure/src/parser/schemarb/parser.ts(0 hunks)frontend/packages/db-structure/src/parser/sql/postgresql/converter.ts(0 hunks)frontend/packages/db-structure/src/parser/sql/postgresql/index.test.ts(0 hunks)frontend/packages/db-structure/src/parser/tbls/index.test.ts(0 hunks)frontend/packages/db-structure/src/parser/tbls/parser.ts(1 hunks)frontend/packages/db-structure/src/schema/factories.ts(0 hunks)frontend/packages/db-structure/src/schema/index.ts(0 hunks)frontend/packages/db-structure/src/schema/mergeSchema.test.ts(5 hunks)frontend/packages/db-structure/src/schema/schema.ts(0 hunks)frontend/packages/db-structure/src/utils/constraintsToRelationships.test.ts(0 hunks)frontend/packages/db-structure/src/utils/isPrimaryKey.ts(1 hunks)frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableColumnList/TableColumn/TableColumn.tsx(3 hunks)frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableColumnList/TableColumnList.tsx(3 hunks)frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/Columns.tsx(3 hunks)frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx(3 hunks)frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/TableDetail.tsx(1 hunks)
💤 Files with no reviewable changes (23)
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnDefaultDiffItem.test.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnDiffItem.test.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnNotNullDiffItem.test.ts
- frontend/packages/db-structure/src/schema/index.ts
- frontend/packages/db-structure/src/schema/factories.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnCheckDiffItem.test.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnCommentDiffItem.test.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnNameDiffItem.test.ts
- frontend/packages/db-structure/src/parser/sql/postgresql/index.test.ts
- frontend/packages/db-structure/src/parser/schemarb/index.test.ts
- frontend/packages/db-structure/src/operation/constants.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnUniqueDiffItem.test.ts
- frontend/packages/db-structure/src/diff/buildSchemaDiff.ts
- frontend/packages/db-structure/src/parser/prisma/parser.ts
- frontend/packages/db-structure/src/parser/schemarb/parser.ts
- frontend/packages/db-structure/src/utils/constraintsToRelationships.test.ts
- frontend/packages/db-structure/src/parser/tbls/index.test.ts
- frontend/packages/db-structure/src/schema/schema.ts
- frontend/packages/db-structure/src/parser/sql/postgresql/converter.ts
- frontend/packages/db-structure/src/parser/prisma/index.test.ts
- frontend/packages/db-structure/src/diff/columns/buildColumnPrimaryDiffItem.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnPrimaryDiffItem.test.ts
- frontend/packages/db-structure/src/diff/types.ts
🧰 Additional context used
📓 Path-based instructions (4)
`**/*.ts`: Use TypeScript for all components and functions. Avoid using type ass...
**/*.ts: Use TypeScript for all components and functions.
Avoid using type assertions (askeyword) in TypeScript code.
Use runtime type validation withvalibotinstead of type assertions for API responses and external data.
Use type predicates orinstanceofchecks for DOM element type narrowing.
Use descriptive variable and function/const names. Event functions should be named with a 'handle' prefix, like 'handleClick' for onClick and 'handleKeyDown' for onKeyDown.
Use consts instead of functions, for example, 'const toggle = () =>'. Also, define a type if possible.
📄 Source: CodeRabbit Inference Engine (.cursorrules)
List of files the instruction was applied to:
frontend/packages/db-structure/src/index.tsfrontend/packages/db-structure/src/schema/mergeSchema.test.tsfrontend/packages/db-structure/src/utils/isPrimaryKey.tsfrontend/apps/app/components/ChatInput/components/MentionSuggestor/utils/getAllMentionCandidates.tsfrontend/internal-packages/agent/src/utils/convertSchemaToText.tsfrontend/internal-packages/agent/src/chat/workflow/workflow.test.tsfrontend/packages/db-structure/src/deparser/postgresql/schemaDeparser.test.tsfrontend/packages/db-structure/src/deparser/postgresql/operationDeparser.test.tsfrontend/packages/db-structure/src/parser/tbls/parser.tsfrontend/packages/db-structure/src/deparser/postgresql/utils.ts
`**/*.tsx`: Do not code within the `page.tsx` file in Next.js App Router. Instead, create a separate `XXXPage` component and write all code there.
**/*.tsx: Do not code within thepage.tsxfile in Next.js App Router. Instead, create a separateXXXPagecomponent and write all code there.
📄 Source: CodeRabbit Inference Engine (.cursorrules)
List of files the instruction was applied to:
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/Columns.tsxfrontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/TableDetail.tsxfrontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsxfrontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableColumnList/TableColumn/TableColumn.tsxfrontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableColumnList/TableColumnList.tsx
`frontend/apps/app/**/*`: Run development commands such as pnpm dev, pnpm dev:cs...
frontend/apps/app/**/*: Run development commands such as pnpm dev, pnpm dev:css, and pnpm lint:tsc specifically within the frontend/apps/app directory for app-specific development.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
frontend/apps/app/components/ChatInput/components/MentionSuggestor/utils/getAllMentionCandidates.ts
`frontend/apps/app/**/*.ts{,x}`: Use database types from '@liam-hq/db/supabase/database.types' for database entities to ensure type safety and consistency with the database schema.
frontend/apps/app/**/*.ts{,x}: Use database types from '@liam-hq/db/supabase/database.types' for database entities to ensure type safety and consistency with the database schema.
📄 Source: CodeRabbit Inference Engine (.clinerules/general.md)
List of files the instruction was applied to:
frontend/apps/app/components/ChatInput/components/MentionSuggestor/utils/getAllMentionCandidates.ts
🧬 Code Graph Analysis (6)
frontend/packages/db-structure/src/utils/isPrimaryKey.ts (3)
frontend/packages/db-structure/src/index.ts (2)
isPrimaryKey(49-49)Constraints(29-29)frontend/packages/db-structure/src/schema/index.ts (1)
Constraints(17-17)frontend/packages/db-structure/src/schema/schema.ts (1)
Constraints(113-113)
frontend/apps/app/components/ChatInput/components/MentionSuggestor/utils/getAllMentionCandidates.ts (2)
frontend/packages/db-structure/src/index.ts (1)
isPrimaryKey(49-49)frontend/packages/db-structure/src/utils/isPrimaryKey.ts (1)
isPrimaryKey(3-11)
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx (2)
frontend/packages/db-structure/src/schema/schema.ts (1)
Constraints(113-113)frontend/packages/db-structure/src/utils/isPrimaryKey.ts (1)
isPrimaryKey(3-11)
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableColumnList/TableColumn/TableColumn.tsx (2)
frontend/packages/db-structure/src/schema/schema.ts (2)
Table(122-122)Column(34-34)frontend/packages/db-structure/src/utils/isPrimaryKey.ts (1)
isPrimaryKey(3-11)
frontend/internal-packages/agent/src/utils/convertSchemaToText.ts (2)
frontend/packages/db-structure/src/index.ts (1)
isPrimaryKey(49-49)frontend/packages/db-structure/src/utils/isPrimaryKey.ts (1)
isPrimaryKey(3-11)
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableColumnList/TableColumnList.tsx (5)
frontend/packages/db-structure/src/index.ts (2)
Table(39-39)isPrimaryKey(49-49)frontend/packages/db-structure/src/schema/index.ts (1)
Table(24-24)frontend/packages/db-structure/src/schema/schema.ts (1)
Table(122-122)frontend/packages/erd-core/src/features/erd/types.ts (1)
TableNodeData(5-15)frontend/packages/db-structure/src/utils/isPrimaryKey.ts (1)
isPrimaryKey(3-11)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Supabase Preview
- GitHub Check: frontend-ci
- GitHub Check: frontend-lint
- GitHub Check: codeql / languages (javascript) / Perform CodeQL for javascript
- GitHub Check: trigger_dev
- GitHub Check: Supabase Preview
🔇 Additional comments (27)
frontend/packages/db-structure/src/index.ts (1)
49-49: LGTM! Clean export statement.The export follows the established pattern and makes the
isPrimaryKeyutility available for use across the codebase, which aligns well with the PR's goal of centralizing primary key detection logic.frontend/packages/db-structure/src/schema/mergeSchema.test.ts (2)
20-30: LGTM! Correctly migrated to constraint-based primary keys.The test data has been properly updated to use constraints instead of the
primaryproperty. The constraint structure withtype: 'PRIMARY KEY',name: 'users_pkey', andcolumnName: 'id'follows the expected format and aligns with the PR's goal of eliminating primary key duplication.
65-78: Consistent constraint usage across test scenarios.The migration to constraint-based primary keys is consistently applied across all test scenarios, ensuring comprehensive test coverage for the new approach.
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/Columns.tsx (3)
1-1: Clean TypeScript import with proper type usage.The import statement correctly includes the
Constraintstype, following the coding guidelines for TypeScript usage.
8-13: Well-structured props interface and component signature.The component correctly accepts the
constraintsprop and follows TypeScript best practices with proper type annotations and destructuring.
26-26: Proper prop propagation to child components.The
constraintsprop is correctly passed down to eachColumnsItem, enabling primary key detection at the child component level.frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/TableDetail.tsx (1)
111-111: Correct prop passing for constraint-based primary key detection.The addition of the
constraintsprop enables theColumnscomponent to access table constraints for primary key detection, properly supporting the migration from column-level primary key flags to constraint-based detection.frontend/internal-packages/agent/src/chat/workflow/workflow.test.ts (1)
82-88: LGTM! Test schema correctly migrated to constraint-based primary keys.The test schema has been properly updated to use table-level constraints instead of column-level
primaryflags. The constraint definition includes all required fields (type,name,columnName) and follows the expected structure for the refactor.frontend/internal-packages/agent/src/utils/convertSchemaToText.ts (2)
1-1: LGTM! Proper import of isPrimaryKey utility.The import statement correctly includes the
isPrimaryKeyfunction needed for constraint-based primary key detection.
25-25: LGTM! Correct implementation of constraint-based primary key detection.The logic properly replaces the column-level
primarycheck with theisPrimaryKeyfunction call. The defensive programming with|| {}fallback handles cases where constraints might be undefined.frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableDetail/Columns/ColumnsItem/ColumnsItem.tsx (3)
1-5: LGTM! Proper imports for constraint-based primary key detection.The import statement correctly includes both the
Constraintstype andisPrimaryKeyfunction needed for the refactor.
21-21: LGTM! Component props properly extended for constraints.The
constraintsprop has been correctly added to the Props type and function signature to support constraint-based primary key detection.Also applies to: 24-24
40-40: LGTM! Correct implementation of constraint-based primary key check.The conditional logic properly replaces the direct
column.primarycheck with theisPrimaryKey(column.name, constraints)function call, maintaining the same UI behavior while using the new schema structure.frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableColumnList/TableColumn/TableColumn.tsx (4)
1-6: LGTM! Proper import of isPrimaryKey utility.The import statement correctly includes the
isPrimaryKeyfunction needed for constraint-based primary key detection.
28-28: LGTM! Component props properly extended for table constraints.The
ColumnIconPropstype andColumnIconfunction signature have been correctly updated to accept thetableprop, enabling access to constraints for primary key detection.Also applies to: 35-35
40-40: LGTM! Correct implementation of constraint-based primary key check.The primary key detection logic properly replaces the direct
column.primarycheck withisPrimaryKey(column.name, table.constraints), maintaining consistent behavior while using the new constraint-based approach.
139-139: LGTM! Table prop properly passed to ColumnIcon.The
tableprop is correctly passed to theColumnIconcomponent to enable constraint-based primary key detection.frontend/apps/app/components/ChatInput/components/MentionSuggestor/utils/getAllMentionCandidates.ts (2)
1-8: LGTM! Proper import of isPrimaryKey utility.The import statement correctly includes the
isPrimaryKeyfunction needed for constraint-based primary key detection in column type classification.
51-51: LGTM! Correct implementation of constraint-based primary key detection.The column type determination logic properly replaces the direct
column.primarycheck withisPrimaryKey(column.name, table.constraints || {}). The defensive programming with|| {}fallback ensures proper handling when constraints might be undefined.frontend/packages/db-structure/src/deparser/postgresql/schemaDeparser.test.ts (1)
34-40: LGTM! Consistent constraint-based primary key definition.The test cases have been correctly updated to remove the
primaryproperty from columns and define primary keys via explicit constraints. The generated SQL snapshots show that the deparser correctly translates these constraints into inline PRIMARY KEY declarations, maintaining the same SQL output format.Also applies to: 76-82, 147-153, 191-197, 237-243, 269-275
frontend/packages/db-structure/src/deparser/postgresql/operationDeparser.test.ts (1)
36-42: LGTM! Operation deparser tests updated consistently.The test operations have been correctly updated to use constraint-based primary key definitions instead of column-level
primaryproperties. The generated SQL maintains the same format with inline PRIMARY KEY declarations, demonstrating that the operation deparser correctly handles the new constraint-based schema format.Also applies to: 100-106
frontend/packages/erd-core/src/features/erd/components/ERDContent/components/TableNode/TableColumnList/TableColumnList.tsx (3)
1-1: LGTM! Proper import of constraint-based primary key utility.The import correctly adds the
isPrimaryKeyutility function andTabletype needed for the constraint-based primary key detection refactor.
12-25: LGTM! Correctly updated to use constraint-based primary key detection.The
shouldDisplayColumnfunction has been properly updated to:
- Accept a
tableparameter to access constraints- Use
isPrimaryKey(column.name, table.constraints)instead of checking the removedcolumn.primaryproperty- Maintain the same filtering logic for the 'KEY_ONLY' mode
This change aligns perfectly with the broader refactor to move primary key identification from column properties to table constraints.
32-37: LGTM! Function call updated to pass required table parameter.The call to
shouldDisplayColumnhas been correctly updated to pass thedata.tableparameter, which is needed for the constraint-based primary key detection.frontend/packages/db-structure/src/parser/tbls/parser.ts (1)
353-353: LGTM! Simplified column processing to use constraint-only primary keys.The removal of primary key column names from the
processColumnsfunction call correctly implements the refactor goal of eliminating theprimaryproperty from columns. Primary key information is now managed exclusively through the table's constraints, which is processed separately in theprocessConstraintsfunction.This change simplifies the parsing logic and eliminates the duplication of primary key information that was previously stored in both column properties and constraints.
frontend/packages/db-structure/src/deparser/postgresql/utils.ts (2)
6-27: LGTM! Improved column definition with constraint-aware logic.The
generateColumnDefinitionfunction has been correctly updated to:
- Accept an optional
isPrimaryKeyparameter for constraint-aware generation- Avoid redundant NOT NULL constraints for primary key columns (since PRIMARY KEY implies NOT NULL in PostgreSQL)
- Maintain the same column definition format while being aware of primary key status
This change follows PostgreSQL best practices and aligns with the constraint-based primary key refactor.
87-108: LGTM! Correctly implemented constraint-based primary key identification.The
generateCreateTableStatementfunction has been properly updated to:
- Identify primary key columns by iterating through table constraints instead of checking column properties
- Collect primary key column names into a Set for efficient lookup
- Pass the
isPrimaryKeyflag togenerateColumnDefinitionfor each column- Add inline PRIMARY KEY declarations for primary key columns
This implementation correctly translates the constraint-based schema format to the expected SQL output while maintaining the same generated SQL format as before the refactor.
- Fix import order in getAllMentionCandidates.ts - Add proper type checking for valibot safeParse result.output in evaluate.ts 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Resolved conflicts by removing both primary and unique fields from column schema 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
frontend/packages/db-structure/src/parser/tbls/parser.ts (2)
62-92: Clarify the purpose of extracting unique column names.The function was renamed from
extractPrimaryKeyColumnNamestoextractUniqueColumnNamesand now extracts single-column UNIQUE constraints. However, the extracted unique column names are not used inprocessColumns(parameter is prefixed with underscore). This suggests the extraction logic might be incomplete or serving a different purpose that's not yet implemented.Additionally, the function only extracts UNIQUE constraints with exactly one column, which may not capture all unique constraints in the schema.
Consider one of the following approaches:
- If unique column names are not needed: Remove the extraction logic entirely.
-/** - * Extract unique column names from constraints - */ -function extractUniqueColumnNames( - constraints: - | Array<{ - type: string - name: string - columns?: string[] - def: string - referenced_table?: string - referenced_columns?: string[] - }> - | undefined, -): Set<string> { - const uniqueColumns: string[] = [] - - if (constraints) { - const uniqueConstraints = constraints.filter( - (constraint) => - constraint.type === 'UNIQUE' && constraint.columns?.length === 1, - ) - - for (const constraint of uniqueConstraints) { - if (constraint.columns?.[0]) { - uniqueColumns.push(constraint.columns[0]) - } - } - } - - return new Set(uniqueColumns) -}
- If unique column names will be used: Implement the usage in
processColumnsand remove the underscore prefix.
420-420: Replace type assertion with runtime validation.The code uses a type assertion (
as CompatibleTable) which violates the coding guidelines. Use runtime type validation instead.Replace the type assertion with proper runtime validation:
- const [tableName, table] = processTable(tblsTable as CompatibleTable) + // Validate table structure at runtime + if (!isValidTable(tblsTable)) { + errors.push(new Error(`Invalid table structure: ${tblsTable.name}`)) + continue + } + const [tableName, table] = processTable(tblsTable)You'll need to implement the
isValidTablefunction or use a validation library likevalibotas mentioned in the coding guidelines.
🧹 Nitpick comments (1)
frontend/packages/db-structure/src/parser/tbls/parser.ts (1)
349-352: Ensure consistency between function call and parameter usage.The function call and parameter passing appear consistent, but since the unique column names are unused, this entire chain of operations may be unnecessary.
If unique column extraction is not needed, simplify the code:
- const uniqueColumnNames = extractUniqueColumnNames(tblsTable.constraints) - const columns = processColumns(tblsTable.columns, uniqueColumnNames) + const columns = processColumns(tblsTable.columns)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/packages/db-structure/src/parser/__snapshots__/index.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (27)
frontend/internal-packages/agent/src/chat/workflow/workflow.test.ts(1 hunks)frontend/internal-packages/schema-bench/src/evaluate/evaluate.ts(2 hunks)frontend/packages/db-structure/src/deparser/postgresql/operationDeparser.test.ts(2 hunks)frontend/packages/db-structure/src/deparser/postgresql/schemaDeparser.test.ts(10 hunks)frontend/packages/db-structure/src/deparser/postgresql/utils.ts(2 hunks)frontend/packages/db-structure/src/diff/buildSchemaDiff.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnCheckDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnCommentDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnDefaultDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnNameDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/columns/__tests__/buildColumnNotNullDiffItem.test.ts(0 hunks)frontend/packages/db-structure/src/diff/types.ts(0 hunks)frontend/packages/db-structure/src/operation/constants.ts(0 hunks)frontend/packages/db-structure/src/parser/prisma/index.test.ts(1 hunks)frontend/packages/db-structure/src/parser/prisma/parser.ts(0 hunks)frontend/packages/db-structure/src/parser/schemarb/index.test.ts(0 hunks)frontend/packages/db-structure/src/parser/schemarb/parser.ts(0 hunks)frontend/packages/db-structure/src/parser/sql/postgresql/converter.ts(0 hunks)frontend/packages/db-structure/src/parser/sql/postgresql/index.test.ts(0 hunks)frontend/packages/db-structure/src/parser/tbls/index.test.ts(0 hunks)frontend/packages/db-structure/src/parser/tbls/parser.ts(4 hunks)frontend/packages/db-structure/src/schema/factories.ts(0 hunks)frontend/packages/db-structure/src/schema/index.ts(0 hunks)frontend/packages/db-structure/src/schema/mergeSchema.test.ts(5 hunks)frontend/packages/db-structure/src/schema/schema.ts(0 hunks)frontend/packages/db-structure/src/utils/constraintsToRelationships.test.ts(0 hunks)
💤 Files with no reviewable changes (19)
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnNameDiffItem.test.ts
- frontend/packages/db-structure/src/utils/constraintsToRelationships.test.ts
- frontend/packages/db-structure/src/schema/factories.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnNotNullDiffItem.test.ts
- frontend/packages/db-structure/src/schema/index.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnCheckDiffItem.test.ts
- frontend/packages/db-structure/src/parser/sql/postgresql/index.test.ts
- frontend/packages/db-structure/src/diff/buildSchemaDiff.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnCommentDiffItem.test.ts
- frontend/packages/db-structure/src/schema/schema.ts
- frontend/packages/db-structure/src/parser/schemarb/parser.ts
- frontend/packages/db-structure/src/parser/schemarb/index.test.ts
- frontend/packages/db-structure/src/operation/constants.ts
- frontend/packages/db-structure/src/parser/sql/postgresql/converter.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnDefaultDiffItem.test.ts
- frontend/packages/db-structure/src/diff/columns/tests/buildColumnDiffItem.test.ts
- frontend/packages/db-structure/src/diff/types.ts
- frontend/packages/db-structure/src/parser/tbls/index.test.ts
- frontend/packages/db-structure/src/parser/prisma/parser.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- frontend/internal-packages/agent/src/chat/workflow/workflow.test.ts
- frontend/internal-packages/schema-bench/src/evaluate/evaluate.ts
- frontend/packages/db-structure/src/schema/mergeSchema.test.ts
- frontend/packages/db-structure/src/deparser/postgresql/schemaDeparser.test.ts
- frontend/packages/db-structure/src/deparser/postgresql/operationDeparser.test.ts
- frontend/packages/db-structure/src/parser/prisma/index.test.ts
- frontend/packages/db-structure/src/deparser/postgresql/utils.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: Use TypeScript for all components and functions. Avoid using type ass...
**/*.ts: Use TypeScript for all components and functions.
Avoid using type assertions (askeyword) in TypeScript code.
Use runtime type validation withvalibotinstead of type assertions for API responses and external data.
Use type predicates orinstanceofchecks for DOM element type narrowing.
Use descriptive variable and function/const names. Event functions should be named with a 'handle' prefix, like 'handleClick' for onClick and 'handleKeyDown' for onKeyDown.
Use consts instead of functions, for example, 'const toggle = () =>'. Also, define a type if possible.
📄 Source: CodeRabbit Inference Engine (.cursorrules)
List of files the instruction was applied to:
frontend/packages/db-structure/src/parser/tbls/parser.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Supabase Preview
- GitHub Check: frontend-lint
- GitHub Check: frontend-ci
- GitHub Check: codeql / languages (javascript) / Perform CodeQL for javascript
- GitHub Check: Supabase Preview
- Removed primary field from column processing - Removed unused extractPrimaryKeyColumnNames function - Fixed formatting issues 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Issue
Why is this change needed?
The schema had semantic duplication between
column.primaryfield and PRIMARY KEY constraints. This caused:What would you like reviewers to focus on?
isPrimaryKey()functionmigratePrimaryToConstraints()properly handles existing schemasTesting Verification
What was done
🤖 Generated by PR Agent at 529e683
Detailed Changes
21 files
Update mention candidates to use isPrimaryKey functionUse isPrimaryKey for text conversionModify column definition generation for PRIMARY KEYRemove column primary diff item builderDelete column primary diff item builderRemove column primary diff item typeExport isPrimaryKey utility functionRemove column primary path patternRemove primary field from Prisma parserRemove primary field from SchemaRB parserRemove primary field from PostgreSQL converterRemove primary key extraction from tbls parserRemove primary field from column factoryRemove column primary schema exportRemove primary field from column schemaAdd isPrimaryKey utility functionUse isPrimaryKey for column icon displayUse isPrimaryKey for column filteringPass constraints to columns componentUse isPrimaryKey for column item displayPass constraints to columns component17 files
Remove primary field from test schemaUpdate test snapshots for PRIMARY KEY generationUpdate schema deparser test snapshotsRemove primary field from test schemasRemove primary field from test schemasRemove primary field from test schemasRemove primary field from test schemasRemove primary field from test schemasRemove primary field from test schemasDelete entire column primary diff item testRemove primary field from test schemasRemove primary field from parser testsRemove primary field from SchemaRB parser testsRemove primary field from SQL parser testsRemove primary field from tbls parser testsUpdate merge schema tests with constraintsRemove primary field from relationship tests2 files
Update Storybook build commandAdd db-structure dependency to Storybook buildAdditional Notes
This is a breaking change for the schema format, but the
migratePrimaryToConstraints()function is provided to help users migrate existing data.Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests