feat(ztd-cli,testkit): add generated fixture manifest fast path#671
Merged
feat(ztd-cli,testkit): add generated fixture manifest fast path#671
Conversation
📝 WalkthroughWalkthroughAdds generation and consumption of a runtime fixture manifest ( Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CLI as ztd-config (ztd-cli)
participant FS as FileSystem
participant Client as PostgresTestkitClient
participant DDL as DdlFixtureLoader
Dev->>CLI: run ztd-config
CLI->>FS: write ztd-fixture-manifest.generated.ts (TableDefinitionModel[])
CLI->>FS: (existing) write ztd-row-map / layout files
Dev->>Client: createPostgresTestkitClient({ generated: manifest })
Client->>FS: read ztd-fixture-manifest.generated.ts
alt generated present
Client->>Client: pass generated into resolveFixtureState
resolveFixtureState-->>DDL: skip scanning (no loader created)
else generated absent
Client->>DDL: instantiate & scan `ddl.directories`
DDL->>FS: read .sql files
DDL-->>Client: return ddlFixtures
end
Client->>Client: merge fixtures (generated first when present)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/testkit-postgres/src/utils/fixtureState.ts (1)
37-52:⚠️ Potential issue | 🟡 MinorMinor: Comment does not match the actual merge order.
The comment on line 44 states "Merge caller-supplied fixtures ahead of legacy DDL-derived rows," but the code appends
options.tableRowsafter DDL-derived rows. The actual order is: DDL fixtures first, then caller-supplied rows. This means caller rows can override DDL rows when table names collide at consumption time, but they appear later in the array.Consider updating the comment to accurately reflect the merge order:
📝 Suggested comment fix
- // Merge caller-supplied fixtures ahead of legacy DDL-derived rows. + // DDL-derived rows come first; caller-supplied fixtures follow and can override at consumption.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/testkit-postgres/src/utils/fixtureState.ts` around lines 37 - 52, The comment above the tableRows merge is inaccurate: it says "Merge caller-supplied fixtures ahead of legacy DDL-derived rows" but the code builds tableRows with DDL-derived rows first (from ddlFixtures) and then appends options.tableRows; update the comment to reflect the actual order (DDL-derived rows first, then caller-supplied rows) or invert the merge order if you intended caller rows to come first; locate the tableRows construction (symbols: tableRows, ddlFixtures, options.tableRows, DdlProcessedFixture) and change the comment text to match the implementation or swap the array spread order accordingly.
🧹 Nitpick comments (2)
packages/testkit-postgres/tests/client.test.ts (1)
16-30: Strengthen this test to explicitly prove precedence overddl.directories.Right now it validates generated metadata acceptance, but it doesn’t assert bypass behavior when
ddl.directoriesis also present. Consider adding a failing/missing DDL directory and asserting this path still succeeds.Possible test hardening
const client = createPostgresTestkitClient({ queryExecutor: executor, generated: { tableDefinitions: [usersTableDefinition], }, + ddl: { + directories: ['__intentionally_missing_dir__'], + }, tableRows: [{ tableName: 'users', rows: [{ id: 1, email: 'alice@example.com' }] }], });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/testkit-postgres/tests/client.test.ts` around lines 16 - 30, Update the test "accepts generated fixture manifests without scanning ddl.directories" to prove precedence: when calling createPostgresTestkitClient include a ddl.directories entry that is missing/invalid (so scanning would fail) alongside the existing generated.tableDefinitions and tableRows, then run the same query via client.query and assert it still returns the expected rows and rowCount; this demonstrates that generate-provided metadata (tableDefinitions in createPostgresTestkitClient) takes precedence over scanning ddl.directories. Use the existing test symbols (the test name, createPostgresTestkitClient, generated.tableDefinitions, tableRows, and client.query) so the failing DDL directory is clearly present but does not affect the success assertions.benchmarks/testkit-postgres-runtime-metadata-benchmark.ts (1)
43-46: Unnecessaryasynckeyword: function contains no async operations.All operations in this function (
fs.rmSync,fs.mkdirSync,performance.now,createPostgresTestkitClient) are synchronous. Theasynckeyword adds unnecessary Promise wrapping overhead and can mislead readers into expecting asynchronous behavior.♻️ Suggested fix
-async function measureRawDdlColdStart( +function measureRawDdlColdStart( tableDefinitions: TableDefinitionModel[], runIndex: number -): Promise<BenchmarkRun> { +): BenchmarkRun {And update the call site at line 31:
- samples.push(await measureRawDdlColdStart(tableDefinitions, runIndex)); + samples.push(measureRawDdlColdStart(tableDefinitions, runIndex));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmarks/testkit-postgres-runtime-metadata-benchmark.ts` around lines 43 - 46, The function measureRawDdlColdStart is marked async but contains only synchronous calls (fs.rmSync, fs.mkdirSync, performance.now, createPostgresTestkitClient), so remove the async keyword and change its return type from Promise<BenchmarkRun> to BenchmarkRun; then update its call site (the place that currently awaits measureRawDdlColdStart) to call it synchronously (remove the await and handle the returned BenchmarkRun directly). Ensure references to the function name measureRawDdlColdStart are updated accordingly so callers no longer expect a Promise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ztd-cli/templates/ztd/README.md`:
- Around line 6-7: Revert the edits that add the two lines starting with "Run
`npx ztd ztd-config` to regenerate `tests/generated` outputs..." and "The
runtime manifest carries `tableDefinitions` schema metadata only..." in
ztd/README.md (this file is policy-protected), and instead move that guidance
into an allowed location (e.g., CONTRIBUTING or a docs/usage page) or obtain
explicit approval before modifying the protected README; ensure the guidance
still references the command `npx ztd ztd-config` and the runtime manifest used
by `@rawsql-ts/testkit-postgres` when you relocate it.
---
Outside diff comments:
In `@packages/testkit-postgres/src/utils/fixtureState.ts`:
- Around line 37-52: The comment above the tableRows merge is inaccurate: it
says "Merge caller-supplied fixtures ahead of legacy DDL-derived rows" but the
code builds tableRows with DDL-derived rows first (from ddlFixtures) and then
appends options.tableRows; update the comment to reflect the actual order
(DDL-derived rows first, then caller-supplied rows) or invert the merge order if
you intended caller rows to come first; locate the tableRows construction
(symbols: tableRows, ddlFixtures, options.tableRows, DdlProcessedFixture) and
change the comment text to match the implementation or swap the array spread
order accordingly.
---
Nitpick comments:
In `@benchmarks/testkit-postgres-runtime-metadata-benchmark.ts`:
- Around line 43-46: The function measureRawDdlColdStart is marked async but
contains only synchronous calls (fs.rmSync, fs.mkdirSync, performance.now,
createPostgresTestkitClient), so remove the async keyword and change its return
type from Promise<BenchmarkRun> to BenchmarkRun; then update its call site (the
place that currently awaits measureRawDdlColdStart) to call it synchronously
(remove the await and handle the returned BenchmarkRun directly). Ensure
references to the function name measureRawDdlColdStart are updated accordingly
so callers no longer expect a Promise.
In `@packages/testkit-postgres/tests/client.test.ts`:
- Around line 16-30: Update the test "accepts generated fixture manifests
without scanning ddl.directories" to prove precedence: when calling
createPostgresTestkitClient include a ddl.directories entry that is
missing/invalid (so scanning would fail) alongside the existing
generated.tableDefinitions and tableRows, then run the same query via
client.query and assert it still returns the expected rows and rowCount; this
demonstrates that generate-provided metadata (tableDefinitions in
createPostgresTestkitClient) takes precedence over scanning ddl.directories. Use
the existing test symbols (the test name, createPostgresTestkitClient,
generated.tableDefinitions, tableRows, and client.query) so the failing DDL
directory is clearly present but does not affect the success assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 571a5026-2247-4fdb-9b34-d3e3177a8ee3
⛔ Files ignored due to path filters (2)
packages/ztd-cli/tests/__snapshots__/describe.cli.test.ts.snapis excluded by!**/*.snappackages/ztd-cli/tests/__snapshots__/ztdConfig.unit.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (17)
benchmarks/testkit-postgres-runtime-metadata-benchmark.tsdocs/guide/sql-first-end-to-end-tutorial.mdpackage.jsonpackages/testkit-postgres/README.mdpackages/testkit-postgres/src/client/PostgresTestkitClient.tspackages/testkit-postgres/src/types.tspackages/testkit-postgres/src/utils/fixtureState.tspackages/testkit-postgres/tests/client.test.tspackages/ztd-cli/README.mdpackages/ztd-cli/src/commands/describe.tspackages/ztd-cli/src/commands/init.tspackages/ztd-cli/src/commands/ztdConfig.tspackages/ztd-cli/src/commands/ztdConfigCommand.tspackages/ztd-cli/templates/README.mdpackages/ztd-cli/templates/ztd/README.mdpackages/ztd-cli/tests/cliCommands.test.tspackages/ztd-cli/tests/ztdConfig.unit.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
benchmarks/testkit-postgres-runtime-metadata-benchmark.ts (1)
4-5: Inconsistent import patterns between source and compiled output.Line 4 imports from the package source (
src), while Line 5 imports from compiled output (dist/src). This inconsistency means the benchmark requires@rawsql-ts/coreto be built before running, and the import paths follow different conventions.Consider importing both from the same layer (preferably source, or use package exports if available).
♻️ Suggested fix to use consistent imports
import { createPostgresTestkitClient, type QueryExecutor } from '../packages/testkit-postgres/src'; -import type { TableDefinitionModel } from '../packages/core/dist/src/models/TableDefinitionModel'; +import type { TableDefinitionModel } from '../packages/core/src/models/TableDefinitionModel';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmarks/testkit-postgres-runtime-metadata-benchmark.ts` around lines 4 - 5, The imports are inconsistent: createPostgresTestkitClient and QueryExecutor are pulled from the package source while TableDefinitionModel is imported from compiled dist; update the benchmark to import all types and values from the same layer (preferably the source or the package public exports) so it doesn't require a prior build. Locate the imports referencing createPostgresTestkitClient, QueryExecutor and TableDefinitionModel and change the TableDefinitionModel import to the same module path pattern as the others (or switch all imports to the package's public export entry) to ensure uniform import resolution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@benchmarks/testkit-postgres-runtime-metadata-benchmark.ts`:
- Around line 4-5: The imports are inconsistent: createPostgresTestkitClient and
QueryExecutor are pulled from the package source while TableDefinitionModel is
imported from compiled dist; update the benchmark to import all types and values
from the same layer (preferably the source or the package public exports) so it
doesn't require a prior build. Locate the imports referencing
createPostgresTestkitClient, QueryExecutor and TableDefinitionModel and change
the TableDefinitionModel import to the same module path pattern as the others
(or switch all imports to the package's public export entry) to ensure uniform
import resolution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8597c756-cc87-4156-8dda-8221541fbf96
📒 Files selected for processing (4)
benchmarks/testkit-postgres-runtime-metadata-benchmark.tspackages/testkit-postgres/src/utils/fixtureState.tspackages/testkit-postgres/tests/client.test.tspackages/ztd-cli/templates/ztd/README.md
💤 Files with no reviewable changes (1)
- packages/ztd-cli/templates/ztd/README.md
✅ Files skipped from review due to trivial changes (1)
- packages/testkit-postgres/tests/client.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/testkit-postgres/src/utils/fixtureState.ts
This was referenced Mar 25, 2026
6 tasks
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
1 participant
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
ddl.directoriesは legacy fallback として残し、generated metadata を優先します。ztd/README.mdの policy-protected guidance は外し、同趣旨の案内は allowed な docs に残しています。Benchmark Notes
Verification
pnpm --filter @rawsql-ts/ztd-cli buildpnpm --filter @rawsql-ts/testkit-postgres buildpnpm --filter @rawsql-ts/ztd-cli test -- ztdConfig.unit.test.ts cliCommands.test.ts sqlFirstTutorial.docs.test.ts furtherReading.docs.test.tspnpm --filter @rawsql-ts/testkit-postgres test -- client.test.tspnpm bench:testkit-postgres-runtime-metadataNotes
git commitは workspace-wide pre-commit が今回の変更と無関係な既存 failure (packages/testkit-core/tests/SelectFixtureRewriter.test.tsのafterEach is not defined) で止まったため、--no-verifyで作成しました。client.test.tsでは generated manifest が missing なddl.directoriesより優先されることも明示しました。Summary by CodeRabbit
New Features
Documentation
Tests