feat(cli): add --introspect option for studio command for bootstrapping without schema - #2775
feat(cli): add --introspect option for studio command for bootstrapping without schema #2775jiashengguo wants to merge 8 commits into
Conversation
…schema file is found
…r options to PullOptions and exporting related functions
|
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 CLI adds project-aware database driver loading and an optional ChangesCLI introspection flow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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.
Pull request overview
This PR adds an --introspect option to the CLI’s Studio/proxy command flow to bootstrap a project when no ZModel schema file is present, including auto-installing the correct DB driver and generating a schema via the existing pull/introspection pipeline.
Changes:
- Adds
--introspectflag to the CLI command definition and routes schema resolution through a newresolveSchemahelper. - Introduces shared optional-dependency helpers (
isPackageInstalled,loadPackage) and uses them across DB introspection providers and proxy dialect creation. - Extends
runPullto accept explicitprovider/databaseUrloverrides to support schema generation from an introspected database.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/index.ts | Adds the --introspect CLI option to the command. |
| packages/cli/src/actions/proxy.ts | Implements --introspect schema resolution, driver installation, schema generation, and generation follow-up. |
| packages/cli/src/actions/db.ts | Exports runPull and allows provider/URL overrides for introspection-driven generation. |
| packages/cli/src/actions/action-utils.ts | Adds isPackageInstalled and loadPackage helpers for optional dependency loading. |
| packages/cli/src/actions/pull/provider/sqlite.ts | Switches SQLite introspection to loadPackage and adds file URL handling. |
| packages/cli/src/actions/pull/provider/postgresql.ts | Switches PostgreSQL introspection to loadPackage. |
| packages/cli/src/actions/pull/provider/mysql.ts | Switches MySQL introspection to loadPackage. |
| packages/cli/test/proxy.test.ts | Adds unit tests for getProviderFromUrl and resolveSchema behaviors. |
| packages/cli/test/action-utils.test.ts | Adds tests for isPackageInstalled and loadPackage. |
Comments suppressed due to low confidence (1)
packages/cli/test/proxy.test.ts:862
- This assertion looks for
zen studio --introspect, butresolveSchemaemitsnpx @zenstackhq/cli studio --introspect .... Usingstudio --introspectas the substring match should make the test align with the actual message formatting.
).rejects.toThrow('zen studio --introspect');
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { Client } = await import('pg'); | ||
| const client = new Client({ connectionString }); | ||
| const pg = await loadPackage('pg'); | ||
| const ClientClass: typeof PgClient = pg?.Client || pg?.default?.Client; |
| if (!path.isAbsolute(filePath)) { | ||
| resolvedUrl = path.join(process.cwd(), filePath); | ||
| } |
| port: 3000, | ||
| signatureToleranceSecs: 60, | ||
| }), | ||
| ).rejects.toThrow('zen studio --introspect -d <url>'); |
| urlFromEnv: boolean, | ||
| ): Promise<string> { | ||
| // Write a minimal datasource-only schema so runPull has something to load | ||
| const urlExpr = urlFromEnv ? 'env("DATABASE_URL")' : `'${databaseUrl}'`; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
packages/cli/src/actions/action-utils.ts (1)
386-415: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared
createRequire(package.json)logic.The same
createRequire(path.resolve(process.cwd(), 'package.json'))pattern is repeated 3 times acrossisPackageInstalledandloadPackage. A small shared helper (getProjectRequire()) would reduce duplication and keep the two functions in sync if the resolution base ever changes.🤖 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 `@packages/cli/src/actions/action-utils.ts` around lines 386 - 415, Extract the repeated createRequire(path.resolve(process.cwd(), 'package.json')) construction into a shared getProjectRequire() helper. Update isPackageInstalled and both fallback branches in loadPackage to reuse this helper, preserving their existing resolution and error-handling behavior.
🤖 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 `@packages/cli/src/actions/action-utils.ts`:
- Around line 396-415: Update loadPackage so it preserves and propagates the
underlying error when the package cannot be loaded, rather than returning null
after all import and require fallbacks fail. Track the original and fallback
failures through the nested dynamic import, projectRequire.resolve, and
projectRequire calls, then throw a diagnostic error that retains the relevant
cause for callers such as createDialect and the database providers.
In `@packages/cli/src/actions/db.ts`:
- Around line 38-39: Add a warning in the introspection flow when the resulting
tables and enums are both empty, regardless of provider or schema selection.
Update the logic around allSchemas and the PostgreSQL introspect result so the
warning clearly indicates that no database objects were found, while preserving
successful empty-result behavior.
In `@packages/cli/src/actions/proxy.ts`:
- Around line 517-530: Restrict the existing default-schema fallback around
getSchemaFile to cases where options.schema was not explicitly provided. When an
explicit --schema path fails, propagate the original CliError instead of
returning zenstack/schema.zmodel; preserve the fallback for missing schemas in
default locations.
- Around line 619-649: Update introspectAndGenerateSchema so the generated
datasource always references env("DATABASE_URL") instead of embedding
databaseUrl when urlFromEnv is false. Preserve the existing provider and runPull
behavior, and ensure the literal connection string is never written to
schema.zmodel.
- Around line 542-544: Update the runGenerate invocation in the introspection
flow to pass options.output as the code generation output path. Preserve the
existing schema, silent, and watch arguments so generated files and subsequent
imports use the same custom output directory resolved by resolveSchema().
In `@packages/cli/src/actions/pull/provider/sqlite.ts`:
- Around line 144-151: Align SQLite relative file URL resolution between the
introspection path and createDialect. Update the resolution logic surrounding
the SQLite constructor and createDialect’s SQLite branch to use the same base
directory, preserving identical database targeting for the --introspect
bootstrap flow; prefer the existing shared resolution behavior rather than
introducing divergent path handling.
In `@packages/cli/test/action-utils.test.ts`:
- Around line 60-62: Update the successful-load test around loadPackage to
assert that the returned module is not null, replacing the insufficient
toBeDefined assertion. Keep the test focused on rejecting loadPackage’s
documented null failure result.
In `@packages/cli/test/proxy.test.ts`:
- Around line 836-846: Update the expected error substrings in the tests around
resolveSchema to match its actual command text: assert the npx `@zenstackhq/cli`
studio prefix and the -d <databaseUrl> placeholder. Apply the same correction to
both affected assertions while preserving the existing test setup and
error-checking behavior.
---
Nitpick comments:
In `@packages/cli/src/actions/action-utils.ts`:
- Around line 386-415: Extract the repeated
createRequire(path.resolve(process.cwd(), 'package.json')) construction into a
shared getProjectRequire() helper. Update isPackageInstalled and both fallback
branches in loadPackage to reuse this helper, preserving their existing
resolution and error-handling behavior.
🪄 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: Pro Plus
Run ID: 312bca78-7c2c-4751-8fea-9074176dca5b
📒 Files selected for processing (9)
packages/cli/src/actions/action-utils.tspackages/cli/src/actions/db.tspackages/cli/src/actions/proxy.tspackages/cli/src/actions/pull/provider/mysql.tspackages/cli/src/actions/pull/provider/postgresql.tspackages/cli/src/actions/pull/provider/sqlite.tspackages/cli/src/index.tspackages/cli/test/action-utils.test.tspackages/cli/test/proxy.test.ts
| // Resolve database URL | ||
| const databaseUrl = options.databaseUrl ?? process.env['DATABASE_URL']; | ||
| if (!databaseUrl) { | ||
| throw new CliError('--introspect requires a database connection — pass -d <url> or set DATABASE_URL.'); | ||
| } | ||
|
|
||
| // If the default schema file already exists, skip introspection | ||
| const defaultSchemaPath = path.resolve('zenstack', 'schema.zmodel'); | ||
| if (fs.existsSync(defaultSchemaPath)) { | ||
| console.log( | ||
| colors.gray('Note: --introspect ignored because a schema file already exists at ' + defaultSchemaPath), | ||
| ); | ||
| return defaultSchemaPath; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Default-schema fallback can mask a genuinely wrong --schema path.
This branch is reached for any CliError thrown by getSchemaFile, including the case where the user passed an explicit --schema <wrong-path> that doesn't exist. If zenstack/schema.zmodel happens to already exist from a prior run, this silently returns that stale file instead of surfacing the original "Schema file not found: <wrong-path>" error — the user never learns their --schema flag was wrong.
Consider only taking this fallback path when the failure was specifically the "not found in default locations" case (e.g., check when options.schema was not explicitly provided), rather than for every CliError from getSchemaFile.
🤖 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 `@packages/cli/src/actions/proxy.ts` around lines 517 - 530, Restrict the
existing default-schema fallback around getSchemaFile to cases where
options.schema was not explicitly provided. When an explicit --schema path
fails, propagate the original CliError instead of returning
zenstack/schema.zmodel; preserve the fallback for missing schemas in default
locations.
| it('loads installed package successfully', async () => { | ||
| const mod = await loadPackage('better-sqlite3'); | ||
| expect(mod).toBeDefined(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the successful-load assertion reject null.
loadPackage returns null on failure, but toBeDefined() passes for null; this test stays green when package loading fails.
Proposed fix
const mod = await loadPackage('better-sqlite3');
- expect(mod).toBeDefined();
+ expect(mod).toBeTruthy();📝 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.
| it('loads installed package successfully', async () => { | |
| const mod = await loadPackage('better-sqlite3'); | |
| expect(mod).toBeDefined(); | |
| it('loads installed package successfully', async () => { | |
| const mod = await loadPackage('better-sqlite3'); | |
| expect(mod).toBeTruthy(); |
🤖 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 `@packages/cli/test/action-utils.test.ts` around lines 60 - 62, Update the
successful-load test around loadPackage to assert that the returned module is
not null, replacing the insufficient toBeDefined assertion. Keep the test
focused on rejecting loadPackage’s documented null failure result.
| it('should throw helpful error without --introspect when DATABASE_URL is not set', async () => { | ||
| const originalEnv = process.env['DATABASE_URL']; | ||
| delete process.env['DATABASE_URL']; | ||
| try { | ||
| await expect( | ||
| resolveSchema({ | ||
| schema: '/nonexistent/path/schema.zmodel', | ||
| port: 3000, | ||
| signatureToleranceSecs: 60, | ||
| }), | ||
| ).rejects.toThrow('zen studio --introspect -d <url>'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Match the asserted commands to resolveSchema’s actual error text.
The implementation emits npx @zenstackhq/cli studio ... (and -d <databaseUrl>), not zen studio ...; both assertions currently fail.
Proposed fix
- ).rejects.toThrow('zen studio --introspect -d <url>');
+ ).rejects.toThrow('npx `@zenstackhq/cli` studio --introspect -d <databaseUrl>');
- ).rejects.toThrow('zen studio --introspect');
+ ).rejects.toThrow('npx `@zenstackhq/cli` studio --introspect');Also applies to: 852-862
🤖 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 `@packages/cli/test/proxy.test.ts` around lines 836 - 846, Update the expected
error substrings in the tests around resolveSchema to match its actual command
text: assert the npx `@zenstackhq/cli` studio prefix and the -d <databaseUrl>
placeholder. Apply the same correction to both affected assertions while
preserving the existing test setup and error-checking behavior.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/cli/src/actions/pull/provider/sqlite.ts (1)
131-143: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winSQLite
file:/sqlite:scheme stripping is now split across files and only partially implemented. The root cause:sqlite.ts'sintrospect()used to normalizefile:-prefixed connection strings itself; that logic was removed and only partially reimplemented indb.ts'srunPull, covering just thefile:prefix and only the explicit--introspectoverride path.
packages/cli/src/actions/pull/provider/sqlite.ts#L131-L143: reinstate scheme stripping (file:/sqlite:→ raw path) directly beforenew SQLite(connectionString, ...)as a defensive last line of normalization, since callers cannot be relied upon to always pre-resolve the URL.packages/cli/src/actions/db.ts#L137-L149: extend the existingdatabaseUrl.startsWith('file:')branch to also coversqlite:, and consider applying equivalent normalization in the non-overridegetDatasource(model)branch too, since the documented ZenStack SQLite format (url = 'file:./dev.db') needs the same treatment there.packages/cli/src/actions/proxy.ts#L607-L631: either confirmsqlite:is a genuinely supported input format and ensurerunPull/introspect()honor it end-to-end, or drop thesqlite:branch here if it's dead/speculative handling.🤖 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 `@packages/cli/src/actions/pull/provider/sqlite.ts` around lines 131 - 143, Normalize SQLite connection schemes consistently: in packages/cli/src/actions/pull/provider/sqlite.ts lines 131-143, strip file: and sqlite: immediately before new SQLite in introspect(); in packages/cli/src/actions/db.ts lines 137-149, extend runPull’s normalization to sqlite: and apply equivalent handling in the non-override getDatasource(model) path; in packages/cli/src/actions/proxy.ts lines 607-631, verify sqlite: is supported end-to-end and retain the branch only if runPull and introspect honor it, otherwise remove it.packages/cli/src/actions/proxy.ts (1)
209-230: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse the schema directory as the SQLite URL base when the path is relative.
outputPathresolves tozenstack/withgetOutputPath(), so relative SQLite URLs likefile:database.sqlite,file:./database.sqlite, orsqlite:database.sqliteare incorrectly resolved from the cwd (/app/database.sqlite) while the generateddatasource.urlpreserves them as relative tozenstack/(/app/database.sqlitefromzenstack/schema.zmodel) or intentionally up one level. Resolvefile:/sqlite:relative URLs againstpath.resolve('zenstack'), not by joining withoutputPath.🤖 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 `@packages/cli/src/actions/proxy.ts` around lines 209 - 230, Update createDialect’s SQLite URL resolution to use path.resolve('zenstack') as the base for relative file: and sqlite: URLs, preserving intentional relative paths such as ./ and ../. Do not use outputPath as the URL base; retain absolute URL handling and Windows normalization.
🤖 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.
Outside diff comments:
In `@packages/cli/src/actions/proxy.ts`:
- Around line 209-230: Update createDialect’s SQLite URL resolution to use
path.resolve('zenstack') as the base for relative file: and sqlite: URLs,
preserving intentional relative paths such as ./ and ../. Do not use outputPath
as the URL base; retain absolute URL handling and Windows normalization.
In `@packages/cli/src/actions/pull/provider/sqlite.ts`:
- Around line 131-143: Normalize SQLite connection schemes consistently: in
packages/cli/src/actions/pull/provider/sqlite.ts lines 131-143, strip file: and
sqlite: immediately before new SQLite in introspect(); in
packages/cli/src/actions/db.ts lines 137-149, extend runPull’s normalization to
sqlite: and apply equivalent handling in the non-override getDatasource(model)
path; in packages/cli/src/actions/proxy.ts lines 607-631, verify sqlite: is
supported end-to-end and retain the branch only if runPull and introspect honor
it, otherwise remove it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 98c2175a-b357-4c8e-8bfe-ec5f3d6a9336
📒 Files selected for processing (5)
packages/cli/src/actions/action-utils.tspackages/cli/src/actions/db.tspackages/cli/src/actions/proxy.tspackages/cli/src/actions/pull/provider/sqlite.tspackages/cli/test/proxy.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/src/actions/action-utils.ts
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 `@packages/cli/src/actions/pull/provider/sqlite.ts`:
- Line 361: Update the column nullability mapping around the `nullable` property
to preserve SQLite’s explicit `c.notnull` value instead of treating every
primary-key column as non-nullable. Only override nullability when the table
schema confirms SQLite enforces it, such as an `INTEGER PRIMARY KEY` rowid alias
or applicable `WITHOUT ROWID`/strict tables, and add regression coverage for
TEXT and composite primary keys.
🪄 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: Pro Plus
Run ID: cf8d702a-03ad-4664-9471-f6e37904c166
📒 Files selected for processing (1)
packages/cli/src/actions/pull/provider/sqlite.ts
| pk: !!c.pk, | ||
| computed: isGenerated, | ||
| nullable: c.notnull !== 1, | ||
| nullable: c.notnull !== 1 && !c.pk, // SQLite treats PRIMARY KEY columns as NOT NULL |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
node <<'NODE'
const Database = require('better-sqlite3');
const db = new Database(':memory:');
db.exec(`
CREATE TABLE single_pk (id TEXT PRIMARY KEY);
CREATE TABLE composite_pk (a TEXT, b TEXT, PRIMARY KEY (a, b));
INSERT INTO single_pk (id) VALUES (NULL);
INSERT INTO composite_pk (a, b) VALUES (NULL, NULL);
`);
console.log(db.prepare("PRAGMA table_xinfo('single_pk')").all());
console.log(db.prepare("PRAGMA table_xinfo('composite_pk')").all());
NODERepository: zenstackhq/zenstack
Length of output: 1020
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant file and surrounding code.
if [ -f packages/cli/src/actions/pull/provider/sqlite.ts ]; then
wc -l packages/cli/src/actions/pull/provider/sqlite.ts
sed -n '320,380p' packages/cli/src/actions/pull/provider/sqlite.ts
else
fd -a sqlite.ts packages/cli/src/actions/pull | sed -n '1,20p'
fi
# Search for this inferred field and similar table_xinfo parsing.
rg -n "nullable:|table_xinfo|notnull|pk" packages/cli/src/actions/pull/provider/sqlite.tsRepository: zenstackhq/zenstack
Length of output: 3928
🌐 Web query:
SQLite PRAGMA table_xinfo notnull pk rowidtable TEXT PRIMARY KEY NULL composite primary key documentation
💡 Result:
In SQLite, PRAGMA table_xinfo provides detailed metadata for table columns, including those omitted by the standard PRAGMA table_info, such as generated and hidden columns [1][2]. The output columns for PRAGMA table_xinfo include: cid: The 0-based column ID [3][2]. name: The name of the column [1][3]. type: The declared data type [1][3]. notnull: A flag indicating if the column prohibits NULL values (0 for allowed, 1 for NOT NULL) [1][3]. dflt_value: The default value for the column [1][2]. pk: Indicates the column's role in the primary key. If the value is 0, the column is not part of the primary key. If greater than 0, it represents the 1-based index of that column within the primary key (e.g., 1 for the first column, 2 for the second) [1][2][4]. hidden: Indicates column visibility: 0 for normal columns, 1 for hidden columns in virtual tables, and 2 or 3 for generated columns [1][2][4]. Regarding your specific keywords: Composite Primary Keys: When a table has a composite primary key, the pk column in the output will contain integers greater than 0, reflecting the order of each column within that key [4]. NULLs in Primary Keys: While standard SQL implies PRIMARY KEY columns should be NOT NULL, SQLite historically allows NULL values in primary key columns unless the column is an INTEGER PRIMARY KEY, part of a WITHOUT ROWID table, declared NOT NULL, or the table is a STRICT table [5][6]. Rowid Tables: Standard SQLite tables (rowid tables) possess an implicit, unique 64-bit signed integer rowid [7][5]. The PRIMARY KEY defined in a rowid table is not necessarily the true primary key (the rowid) unless it is defined as INTEGER PRIMARY KEY, which aliases the rowid [7]. If a table is created with WITHOUT ROWID, the specified PRIMARY KEY becomes the true primary key, and the table does not have an implicit rowid [7][5].
Citations:
- 1: https://sqlite.org/pragma.html
- 2: https://docs.devart.com/odbc-driver-for-sqlite/using-the-driver/metadata/metadata-pragma-statements.html
- 3: https://www.sqlitetutor.com/pragma-table_xinfo/
- 4: https://thelinuxcode.com/sqlite-describe-table-practical-modern-ways-to-inspect-schema-in-2026/
- 5: https://www2.sqlite.org/matrix/lang_createtable.html
- 6: https://www.sqlite.org/lang_createtable.html
- 7: https://sqlite.org/rowidtable.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the source around this inference as a data file to ensure the PRAGMA parsing is complete.
sed -n '160,250p' packages/cli/src/actions/pull/provider/sqlite.ts
sed -n '290,375p' packages/cli/src/actions/pull/provider/sqlite.ts
# Deterministically model the current inference for representative SQLite table_xinfo shapes.
python3 - <<'PY'
def inferred_nullable(notnull, pk):
return not (notnull == 1 or pk != 0)
cases = [
("ordinary TEXT PRIMARY KEY rowid table", 0, 1),
("composite PK column 'a' ordinary table", 0, 1),
("composite PK column 'b' ordinary table", 0, 2),
("INTEGER PRIMARY KEY rowid alias", 0, 1),
("nullable after NOT NULL explicit", 1, 0),
("PK with explicit NOT NULL", 1, 1),
]
for name, notnull, pk in cases:
print(f"{name}: notnull={notnull}, pk={pk} -> inferred_nullable={inferred_nullable(notnull, pk)}")
PYRepository: zenstackhq/zenstack
Length of output: 10305
Do not infer non-nullability from all SQLite primary key columns.
SQLite rowid tables allow NULL in non-INTEGER PRIMARY KEY columns, including first/other columns of a composite primary key, unless the explicit NOT NULL flag or WITHOUT ROWID/strict semantics apply. This current inference can introspect an existing valid table with ordinary TEXT PRIMARY KEY or composite keys and write back a schema that rejects those rows.
Preserve c.notnull here, and only switch to false in cases where SQLite actually enforces non-nullability (for example, INTEGER ... PRIMARY KEY rowid aliases or WITHOUT ROWID/strict primary key support). Add regression coverage for TEXT PRIMARY KEY and composite primary keys.
🤖 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 `@packages/cli/src/actions/pull/provider/sqlite.ts` at line 361, Update the
column nullability mapping around the `nullable` property to preserve SQLite’s
explicit `c.notnull` value instead of treating every primary-key column as
non-nullable. Only override nullability when the table schema confirms SQLite
enforces it, such as an `INTEGER PRIMARY KEY` rowid alias or applicable `WITHOUT
ROWID`/strict tables, and add regression coverage for TEXT and composite primary
keys.
Source: MCP tools
Summary by CodeRabbit
New Features
--introspectoption to generate a schema from your database when no schema file is available.Enhancements
--introspectvs missing schema, including smarterDATABASE_URLhandling for SQLite (relativefile:rewriting).--providerand--database-urlsupport (including improved SQLite path normalization).Bug Fixes
Tests