fix(cli): clarify local-source and docs guidance in generated README#720
Merged
fix(cli): clarify local-source and docs guidance in generated README#720
Conversation
|
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:
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "ztd feature tests scaffold"
participant Entry as "entryspec.ts"
participant QuerySpec as "queryspec.ts"
participant SQL as "query.sql"
participant Generator as "Scaffold Generator"
participant Harness as "tests/ztd/harness.ts"
participant DB as "Postgres"
User->>CLI: run --feature users-insert
CLI->>Entry: parse entryspec (request fields)
CLI->>QuerySpec: parse queryspec (input/output types)
CLI->>SQL: parse SQL (tables, RETURNING cols)
CLI->>Generator: compute fixture shapes, TEST_PLAN.md, analysis.json, query types
Generator->>CLI: write thin Vitest entrypoint + generated files under tests/generated/
User->>Harness: run Vitest entrypoint (loads cases from tests/cases/)
Harness->>DB: verifier truncates & seeds beforeDb (RESTART IDENTITY CASCADE)
Harness->>DB: execute query (via execute callback)
DB-->>Harness: returns rows
Harness->>Harness: assert output and afterDb subset matches (unordered per-row)
DB->>DB: ROLLBACK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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: 5
🧹 Nitpick comments (10)
packages/ztd-cli/tests/cliCommands.test.ts (1)
257-270: Also pin the new top-level help example.This covers the subcommand contract, but Line 108 in
packages/ztd-cli/src/index.tsadds a newGetting startedexample that can still drift without a matchingrunCli(['--help'])assertion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/tests/cliCommands.test.ts` around lines 257 - 270, Add a new assertion that calls runCli(['--help']) and verifies the top-level help output includes the new "Getting started" example string introduced in index.ts; use assertCliSuccess(result, '--help') (same pattern as the existing test) and an expect(result.stdout).toContain('Getting started') to pin that top-level example so it cannot drift, referencing runCli and assertCliSuccess to locate where to add the check.packages/ztd-cli/templates/src/features/smoke/tests/README.md (1)
7-8: Clarify whichtests/paths are repo-root vs feature-local.From inside
src/features/smoke/tests/README.md,tests/ztd/harness.tsreads like a sibling path whiletests/cases/reads feature-local. A small qualifier like “repo-roottests/ztd/harness.ts” would make the layout unambiguous.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/templates/src/features/smoke/tests/README.md` around lines 7 - 8, Clarify the README text to disambiguate repo-root vs feature-local test paths: state that the fixed app-level ZTD runner is at the repo-root path tests/ztd/harness.ts (i.e., “repo-root tests/ztd/harness.ts”) and that query-local cases live under the feature-local tests/cases/ directory; likewise keep the note about adding a thin <query>.queryspec.ztd.test.ts next to query-local tests and a feature-root <feature>.entryspec.test.ts for the mock-based lane.packages/core/src/models/TableDefinitionModel.ts (1)
104-105: Keep identity fallback separate from DDL defaults.
defaultValueis documented as DDL-derived metadata, but this now serializes a syntheticrow_number() over ()into the shared model and generated fixture manifest. An explicitisIdentity/generatedKindflag would preserve the schema model and let the rewrite layer synthesize its deterministic stand-in only when it actually needs one.As per coding guidelines, code in
packages/coreMUST NOT implement query rewrite responsibilities owned bytestkit-core.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/models/TableDefinitionModel.ts` around lines 104 - 105, The model currently injects a synthetic 'row_number() over ()' into defaultValue (in TableDefinitionModel where defaultValue: defaultConstraint?.defaultValue ?? (hasIdentity ? 'row_number() over ()' : null)), which mixes query-rewrite concerns into the schema; change defaultValue to only reflect DDL (defaultConstraint?.defaultValue ?? null) and introduce an explicit identity/generation flag (e.g., isIdentity or generatedKind) set from hasIdentity so callers can detect generated columns; leave synthesis of any deterministic stand-in (row_number()) to testkit-core/rewrite layer instead of packages/core.packages/ztd-cli/templates/src/features/README.md (1)
10-12: Clarify queryspec paths as query-local to avoid ambiguity.Line 10 and Line 12 currently read like queryspec assets live directly under
src/features/<feature>/tests/. Consider making the<query>/tests/...prefix explicit so this file matches the rest of the scaffold docs.📘 Suggested wording update
-- `tests`: feature-local checks that keep the slice honest, including a thin `tests/<feature>.entryspec.test.ts` Vitest entrypoint for the mock-based lane and per-query `tests/<query>.queryspec.ztd.test.ts` Vitest entrypoints for the ZTD lane +- `tests`: feature-local checks that keep the slice honest, including a thin `tests/<feature>.entryspec.test.ts` Vitest entrypoint for the mock-based lane and per-query `<query>/tests/<query>.queryspec.ztd.test.ts` Vitest entrypoints for the ZTD lane -Use `ztd feature tests scaffold --feature <feature-name>` after SQL and DTO edits to refresh `tests/generated/TEST_PLAN.md` and `analysis.json`, keep the thin `tests/<query-name>.queryspec.ztd.test.ts` entrypoint in sync, and add persistent cases under `tests/cases/` with the fixed app-level ZTD runner. +Use `ztd feature tests scaffold --feature <feature-name>` after SQL and DTO edits to refresh `<query>/tests/generated/TEST_PLAN.md` and `analysis.json`, keep the thin `<query>/tests/<query-name>.queryspec.ztd.test.ts` entrypoint in sync, and add persistent cases under `<query>/tests/cases/` with the fixed app-level ZTD runner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/templates/src/features/README.md` around lines 10 - 12, Update the README wording to clarify that queryspec test files are query-local (i.e., live under each query's folder) rather than directly under the feature tests folder: change occurrences of tests/<query-name>.queryspec.ztd.test.ts and the phrase "thin tests/<query>.entryspec.test.ts" to make the prefix explicit as <query>/tests/<query-name>.queryspec.ztd.test.ts (and <query>/tests/<query>.entryspec.test.ts), and ensure the instruction for using ztd feature tests scaffold references the query-local paths and keeping <query>/tests/cases/ in sync with tests/generated/TEST_PLAN.md and analysis.json.packages/ztd-cli/tests/init.command.test.ts (1)
175-182: Remove duplicate README assertion in starter test.Line 175 and Line 181 assert the same sentence; keeping one is enough.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/tests/init.command.test.ts` around lines 175 - 182, Remove the duplicated README assertion in the test file by deleting one of the expect(...) calls that checks for "Copy `.env.example` to `.env` and update `ZTD_DB_PORT` if 5432 is already in use." in packages/ztd-cli/tests/init.command.test.ts; keep a single expect(readNormalizedFile(path.join(workspace, 'README.md'))).toContain('Copy `.env.example` to `.env` and update `ZTD_DB_PORT` if 5432 is already in use.'); and run tests to ensure no other duplicates remain.packages/ztd-cli/templates/README.md (1)
49-56: Troubleshooting bullets repeat the same checks with different wording.Line 49–Line 56 can be condensed; several bullets duplicate the same
null id, local-source resolution, andafterDbcomparison guidance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/templates/README.md` around lines 49 - 56, Condense the repetitive troubleshooting bullets into a single concise list that keeps the unique checks only: (1) inspect fixture manifest/metadata and rewrite path when a DB-backed case shows user_id: null, (2) compare the direct DB INSERT ... RETURNING result with the ZTD result to isolate whether the DB, manifest, or rewrite path is the culprit, (3) when a dogfood workspace should reflect source changes ensure the local rawsql-ts checkout is resolved rather than a registry copy, (4) explain afterDb semantics (row-as-unordered-multiset, ignore row order, allow volatile columns to be omitted) and how to debug false negatives (comparison path, multiset handling, omitted volatile columns), and (5) note that AI-authored ZTD tests failing may also indicate bugs in ztd-cli or rawsql-ts; remove duplicate bullets about null id, local-source resolution, and afterDb checks and merge overlapping guidance into these five points.packages/ztd-cli/README.md (1)
80-80: Consider hyphenating compound modifiers for clarity.The static analysis flagged two grammar issues on this line:
- "CLI owned" → "CLI-owned"
- "human/AI owned" → "human/AI-owned"
These hyphens help clarify the compound modifiers when they precede the noun.
📝 Suggested fix
-After you finish the SQL and DTO edits, run `npx ztd feature tests scaffold --feature <feature-name>` to refresh `src/features/<feature-name>/<query-name>/tests/generated/TEST_PLAN.md` and `analysis.json`. That command also creates the thin Vitest entrypoint `src/features/<feature-name>/<query-name>/tests/<query-name>.queryspec.ztd.test.ts`, which stays checked in as a small adapter around the fixed app-level harness. `generated/*` is CLI owned and refreshable, `cases/*` is human/AI owned and kept, and the thin entrypoint is kept. +After you finish the SQL and DTO edits, run `npx ztd feature tests scaffold --feature <feature-name>` to refresh `src/features/<feature-name>/<query-name>/tests/generated/TEST_PLAN.md` and `analysis.json`. That command also creates the thin Vitest entrypoint `src/features/<feature-name>/<query-name>/tests/<query-name>.queryspec.ztd.test.ts`, which stays checked in as a small adapter around the fixed app-level harness. `generated/*` is CLI-owned and refreshable, `cases/*` is human/AI-owned and kept, and the thin entrypoint is kept.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/README.md` at line 80, In the README sentence beginning "That command also creates..." update the two compound modifiers by hyphenating them: change "CLI owned" to "CLI-owned" and "human/AI owned" to "human/AI-owned" so the compounds preceding the noun are clear; search for the exact phrases "CLI owned" and "human/AI owned" in the README and replace them accordingly.packages/ztd-cli/src/commands/feature.ts (1)
21-36: Minor: Inconsistent indentation in layout description.Lines 27-28 use a different indentation pattern (missing leading spaces for the string array element) compared to the surrounding lines. This doesn't affect functionality but makes the layout description harder to maintain.
' <query-name>/', - ' queryspec.ts', - ' <query-name>.sql', + ' queryspec.ts', + ' <query-name>.sql', ' tests/',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/src/commands/feature.ts` around lines 21 - 36, The FIXED_LAYOUT_DESCRIPTION array contains two elements (' queryspec.ts' and ' <query-name>.sql') with inconsistent leading space compared to the surrounding lines; update those two string entries in FIXED_LAYOUT_DESCRIPTION so their indentation matches the other nested lines (add the missing leading spaces) to make the layout description consistent and maintainable.packages/ztd-cli/templates/tests/ztd/verifier.ts (1)
28-46: Consider pool reuse for test performance.Creating a new
Poolfor each test case (line 28) adds connection overhead. For test suites with many cases, consider accepting a pool from the caller or using a singleton within a test run.That said, the current approach provides good isolation and the cleanup pattern (BEGIN/ROLLBACK in try/finally) is correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/templates/tests/ztd/verifier.ts` around lines 28 - 46, The test verifier currently creates a new Pool for each run (Pool, pool.connect) which is costly; change the verifier function to accept an optional Pool parameter (e.g., pool?: Pool) so callers can pass a shared pool or singleton for the whole test suite, and only create a new Pool when none is provided; when using an injected pool, do not call pool.end() at the end (only call pool.end() for pools you created locally), but continue using the same transaction pattern (client, BEGIN/ROLLBACK, resetFixtureTables, seedFixture, execute via createQuerySpecExecutor, expectAfterDbState) and release the client in the finally block as before.packages/ztd-cli/src/commands/featureTests.ts (1)
575-588: Low-risk ReDoS: Inputs are currently controlled.Static analysis flagged dynamic regex construction from
schemaName. While the current usage only passes hardcoded strings ('RequestSchema','QueryParamsSchema','QueryResultSchema'), the function signature accepts arbitrary strings.Consider adding validation or using a lookup-based approach if this function might be called with user-controlled input in the future:
🛡️ Optional defensive validation
function extractSchemaFields(source: string, schemaName: string): string[] { + if (!/^[A-Za-z][A-Za-z0-9]*$/.test(schemaName)) { + throw new Error(`Invalid schema name: ${schemaName}`); + } const match = source.match(new RegExp(`const\\s+${schemaName}\\s*=\\s*z\\.object\\(\\{([\\s\\S]*?)\\}\\)\\.strict\\(\\);`));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/src/commands/featureTests.ts` around lines 575 - 588, The dynamic RegExp in extractSchemaFields uses the raw schemaName which can lead to ReDoS if uncontrolled; update extractSchemaFields to either validate schemaName against a safe whitelist (e.g., only allow alphanumerics and underscores or the known schema names 'RequestSchema','QueryParamsSchema','QueryResultSchema') or escape any special regex characters in schemaName before constructing the RegExp (use a robust escape routine) so the pattern is never built from arbitrary input; reference extractSchemaFields and the RegExp construction to locate where to add the validation/escaping or switch to a lookup-based approach that maps allowed schema names to precompiled regexes.
🤖 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/README.md`:
- Around line 16-19: Update the README wording to clarify that while
tool-managed workspace files live under `.ztd/generated/` (and other `.ztd/*`
metadata), scaffolded query artifacts are placed next to the query (e.g.,
`src/features/<feature>/<query>/tests/{generated,cases}`) and the shared test
runner is at `tests/ztd/`; also keep the note that `ztd.config.json` controls
the tool-managed test workspace but does not relocate feature-local tests. Use
the exact paths `.ztd/generated/`, `.ztd/tests/`,
`src/features/<feature>/<query>/tests/{generated,cases}`, `tests/ztd/`, and
`ztd.config.json` when updating the sentences to remove the implication that all
test artifacts are under `.ztd/tests/`.
In `@packages/ztd-cli/templates/tests/ztd/case-types.ts`:
- Line 1: The exported type QuerySpecZtdResultCardinality is currently unused;
either remove it or integrate it into the template by adding a result
cardinality field to the QuerySpecZtdCase shape. Update the QuerySpecZtdCase
definition to include a property (e.g., resultCardinality:
QuerySpecZtdResultCardinality) so the type is referenced and documents expected
usage, or if you choose to keep the template minimal, delete the standalone
export of QuerySpecZtdResultCardinality until a concrete generated-spec use
requires it.
In `@packages/ztd-cli/templates/tests/ztd/README.md`:
- Line 12: Update the README wording for afterDb to match the actual semantics:
replace the phrase that says it compares "exact rows" with a concise description
that afterDb performs a subset match per expected row (each expected row's
keys/values must be present in an actual row), treats rows as an unordered
multiset (row order ignored but multiplicity considered), and normalizes object
key order before comparison; reference the fixture names beforeDb and afterDb in
the sentence so it's clear which files follow this behavior.
In `@packages/ztd-cli/tests/init.command.test.ts`:
- Around line 384-385: The two assertions that check promptDogfood currently
reference a feature-local path 'src/features/<feature-name>/.ztd/generated/...'
which is incorrect; update both expectations to reference the workspace-root
path '.ztd/generated/ztd-fixture-manifest.generated.ts' (and adjust any
surrounding text accordingly) so the assertions match the scaffolded manifest
location — locate the expectations that use promptDogfood in
tests/init.command.test.ts and replace the
'src/features/<feature-name>/.ztd/generated/...' path with
'.ztd/generated/ztd-fixture-manifest.generated.ts' and ensure the second
assertion still mentions the pure fixture skeletons ('beforeDb' and 'afterDb')
without changing its intent.
In `@packages/ztd-cli/tests/sqlFirstTutorial.docs.test.ts`:
- Line 73: The test's assertion on the variable tutorial hardcodes an incorrect
manifest path; in the expect(...) string that references the fixture manifest,
replace the occurrence of
"src/features/users-insert/.ztd/generated/ztd-fixture-manifest.generated.ts"
with the root path ".ztd/generated/ztd-fixture-manifest.generated.ts" so the
tutorial text matches the intended scaffold guidance (leave the rest of the
expected string unchanged).
---
Nitpick comments:
In `@packages/core/src/models/TableDefinitionModel.ts`:
- Around line 104-105: The model currently injects a synthetic 'row_number()
over ()' into defaultValue (in TableDefinitionModel where defaultValue:
defaultConstraint?.defaultValue ?? (hasIdentity ? 'row_number() over ()' :
null)), which mixes query-rewrite concerns into the schema; change defaultValue
to only reflect DDL (defaultConstraint?.defaultValue ?? null) and introduce an
explicit identity/generation flag (e.g., isIdentity or generatedKind) set from
hasIdentity so callers can detect generated columns; leave synthesis of any
deterministic stand-in (row_number()) to testkit-core/rewrite layer instead of
packages/core.
In `@packages/ztd-cli/README.md`:
- Line 80: In the README sentence beginning "That command also creates..."
update the two compound modifiers by hyphenating them: change "CLI owned" to
"CLI-owned" and "human/AI owned" to "human/AI-owned" so the compounds preceding
the noun are clear; search for the exact phrases "CLI owned" and "human/AI
owned" in the README and replace them accordingly.
In `@packages/ztd-cli/src/commands/feature.ts`:
- Around line 21-36: The FIXED_LAYOUT_DESCRIPTION array contains two elements ('
queryspec.ts' and ' <query-name>.sql') with inconsistent leading space
compared to the surrounding lines; update those two string entries in
FIXED_LAYOUT_DESCRIPTION so their indentation matches the other nested lines
(add the missing leading spaces) to make the layout description consistent and
maintainable.
In `@packages/ztd-cli/src/commands/featureTests.ts`:
- Around line 575-588: The dynamic RegExp in extractSchemaFields uses the raw
schemaName which can lead to ReDoS if uncontrolled; update extractSchemaFields
to either validate schemaName against a safe whitelist (e.g., only allow
alphanumerics and underscores or the known schema names
'RequestSchema','QueryParamsSchema','QueryResultSchema') or escape any special
regex characters in schemaName before constructing the RegExp (use a robust
escape routine) so the pattern is never built from arbitrary input; reference
extractSchemaFields and the RegExp construction to locate where to add the
validation/escaping or switch to a lookup-based approach that maps allowed
schema names to precompiled regexes.
In `@packages/ztd-cli/templates/README.md`:
- Around line 49-56: Condense the repetitive troubleshooting bullets into a
single concise list that keeps the unique checks only: (1) inspect fixture
manifest/metadata and rewrite path when a DB-backed case shows user_id: null,
(2) compare the direct DB INSERT ... RETURNING result with the ZTD result to
isolate whether the DB, manifest, or rewrite path is the culprit, (3) when a
dogfood workspace should reflect source changes ensure the local rawsql-ts
checkout is resolved rather than a registry copy, (4) explain afterDb semantics
(row-as-unordered-multiset, ignore row order, allow volatile columns to be
omitted) and how to debug false negatives (comparison path, multiset handling,
omitted volatile columns), and (5) note that AI-authored ZTD tests failing may
also indicate bugs in ztd-cli or rawsql-ts; remove duplicate bullets about null
id, local-source resolution, and afterDb checks and merge overlapping guidance
into these five points.
In `@packages/ztd-cli/templates/src/features/README.md`:
- Around line 10-12: Update the README wording to clarify that queryspec test
files are query-local (i.e., live under each query's folder) rather than
directly under the feature tests folder: change occurrences of
tests/<query-name>.queryspec.ztd.test.ts and the phrase "thin
tests/<query>.entryspec.test.ts" to make the prefix explicit as
<query>/tests/<query-name>.queryspec.ztd.test.ts (and
<query>/tests/<query>.entryspec.test.ts), and ensure the instruction for using
ztd feature tests scaffold references the query-local paths and keeping
<query>/tests/cases/ in sync with tests/generated/TEST_PLAN.md and
analysis.json.
In `@packages/ztd-cli/templates/src/features/smoke/tests/README.md`:
- Around line 7-8: Clarify the README text to disambiguate repo-root vs
feature-local test paths: state that the fixed app-level ZTD runner is at the
repo-root path tests/ztd/harness.ts (i.e., “repo-root tests/ztd/harness.ts”) and
that query-local cases live under the feature-local tests/cases/ directory;
likewise keep the note about adding a thin <query>.queryspec.ztd.test.ts next to
query-local tests and a feature-root <feature>.entryspec.test.ts for the
mock-based lane.
In `@packages/ztd-cli/templates/tests/ztd/verifier.ts`:
- Around line 28-46: The test verifier currently creates a new Pool for each run
(Pool, pool.connect) which is costly; change the verifier function to accept an
optional Pool parameter (e.g., pool?: Pool) so callers can pass a shared pool or
singleton for the whole test suite, and only create a new Pool when none is
provided; when using an injected pool, do not call pool.end() at the end (only
call pool.end() for pools you created locally), but continue using the same
transaction pattern (client, BEGIN/ROLLBACK, resetFixtureTables, seedFixture,
execute via createQuerySpecExecutor, expectAfterDbState) and release the client
in the finally block as before.
In `@packages/ztd-cli/tests/cliCommands.test.ts`:
- Around line 257-270: Add a new assertion that calls runCli(['--help']) and
verifies the top-level help output includes the new "Getting started" example
string introduced in index.ts; use assertCliSuccess(result, '--help') (same
pattern as the existing test) and an expect(result.stdout).toContain('Getting
started') to pin that top-level example so it cannot drift, referencing runCli
and assertCliSuccess to locate where to add the check.
In `@packages/ztd-cli/tests/init.command.test.ts`:
- Around line 175-182: Remove the duplicated README assertion in the test file
by deleting one of the expect(...) calls that checks for "Copy `.env.example` to
`.env` and update `ZTD_DB_PORT` if 5432 is already in use." in
packages/ztd-cli/tests/init.command.test.ts; keep a single
expect(readNormalizedFile(path.join(workspace, 'README.md'))).toContain('Copy
`.env.example` to `.env` and update `ZTD_DB_PORT` if 5432 is already in use.');
and run tests to ensure no other duplicates remain.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 77443da7-a819-46c4-ab3a-e0c3ab98f8d0
📒 Files selected for processing (28)
docs/guide/sql-first-end-to-end-tutorial.mdpackages/core/src/models/TableDefinitionModel.tspackages/core/tests/models/TableDefinitionModel.test.tspackages/ztd-cli/README.mdpackages/ztd-cli/src/commands/feature.tspackages/ztd-cli/src/commands/featureTests.tspackages/ztd-cli/src/commands/init.tspackages/ztd-cli/src/index.tspackages/ztd-cli/templates/AGENTS.mdpackages/ztd-cli/templates/PROMPT_DOGFOOD.mdpackages/ztd-cli/templates/README.mdpackages/ztd-cli/templates/src/features/AGENTS.mdpackages/ztd-cli/templates/src/features/README.mdpackages/ztd-cli/templates/src/features/smoke/tests/README.mdpackages/ztd-cli/templates/tests/AGENTS.mdpackages/ztd-cli/templates/tests/support/postgres-testkit.tspackages/ztd-cli/templates/tests/ztd/README.mdpackages/ztd-cli/templates/tests/ztd/case-types.tspackages/ztd-cli/templates/tests/ztd/harness.tspackages/ztd-cli/templates/tests/ztd/verifier.tspackages/ztd-cli/tests/cliCommands.test.tspackages/ztd-cli/tests/directoryFinding.docs.test.tspackages/ztd-cli/tests/featureScaffold.unit.test.tspackages/ztd-cli/tests/featureTestsScaffold.unit.test.tspackages/ztd-cli/tests/init.command.test.tspackages/ztd-cli/tests/sqlFirstTutorial.docs.test.tspackages/ztd-cli/tests/ztdConfig.unit.test.tsscripts/verify-published-package-mode.mjs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/README.md`:
- Line 82: The README contains a duplicate "Troubleshooting" heading that
triggers markdownlint MD024; either remove or consolidate this second
"Troubleshooting" heading into the existing "Troubleshooting" section (the
earlier "Troubleshooting" heading) or rename this heading to something unique
(e.g., "ZTD Test Troubleshooting" or "Feature Test Debugging") so the headings
are distinct; update the heading text in the README accordingly and move any
content under the duplicate into the original troubleshooting section if
consolidating.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 061e4e68-5b20-4fbf-8e41-29249fe4f446
📒 Files selected for processing (7)
packages/ztd-cli/README.mdpackages/ztd-cli/src/commands/feature.tspackages/ztd-cli/templates/README.mdpackages/ztd-cli/templates/src/features/README.mdpackages/ztd-cli/tests/cliCommands.test.tspackages/ztd-cli/tests/init.command.test.tspackages/ztd-cli/tests/sqlFirstTutorial.docs.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/ztd-cli/templates/src/features/README.md
- packages/ztd-cli/tests/init.command.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ztd-cli/tests/cliCommands.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/ztd-cli/templates/README.md (1)
46-46: Consider breaking up the long workflow paragraph for readability.Line 46 is a single ~1300-character paragraph explaining the
ztd feature tests scaffoldworkflow, fixture semantics, and execution steps. While the content is accurate, breaking it into smaller paragraphs or bullet points would improve scannability for both human developers and AI agents following these instructions.📝 Suggested restructuring
-After the SQL and DTO edits settle, run `ztd feature tests scaffold --feature <feature-name>` to refresh `tests/generated/TEST_PLAN.md` and `analysis.json`, create the thin `tests/<query-name>.queryspec.ztd.test.ts` Vitest entrypoint if it is missing, and keep `tests/cases/` as human/AI-owned persistent cases around the fixed app-level ZTD runner. `generated/*` is CLI-owned and refreshable, while the thin entrypoint is kept. If `ztd-config` has already run, use `.ztd/generated/ztd-fixture-manifest.generated.ts` as the source for `tableDefinitions` and any fixture-shape hints the case needs. `beforeDb` and `afterDb` are pure fixture skeletons with schema-qualified table keys. `afterDb` is subset-based per row, rows are treated as an unordered multiset, and row order itself is ignored. The verifier truncates tables named in `beforeDb` with `restart identity cascade` before seeding. When the cases are ready, run `npx vitest run src/features/<feature-name>/<query-name>/tests/<query-name>.queryspec.ztd.test.ts` to execute the ZTD query test. +After the SQL and DTO edits settle, run `ztd feature tests scaffold --feature <feature-name>` to refresh `tests/generated/TEST_PLAN.md` and `analysis.json`, create the thin `tests/<query-name>.queryspec.ztd.test.ts` Vitest entrypoint if it is missing, and keep `tests/cases/` as human/AI-owned persistent cases around the fixed app-level ZTD runner. + +`generated/*` is CLI-owned and refreshable, while the thin entrypoint is kept. If `ztd-config` has already run, use `.ztd/generated/ztd-fixture-manifest.generated.ts` as the source for `tableDefinitions` and any fixture-shape hints the case needs. + +`beforeDb` and `afterDb` are pure fixture skeletons with schema-qualified table keys. `afterDb` is subset-based per row, rows are treated as an unordered multiset, and row order itself is ignored. The verifier truncates tables named in `beforeDb` with `restart identity cascade` before seeding. + +When the cases are ready, run `npx vitest run src/features/<feature-name>/<query-name>/tests/<query-name>.queryspec.ztd.test.ts` to execute the ZTD query test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/templates/README.md` at line 46, Split the long single paragraph into 3–4 short paragraphs or a brief bulleted list covering: (1) the command and purpose (mention `ztd feature tests scaffold --feature <feature-name>` and the generated files `tests/generated/TEST_PLAN.md` and `analysis.json`), (2) what the scaffold creates/owns vs what is persistent (note `tests/<query-name>.queryspec.ztd.test.ts`, `generated/*` and `tests/cases/`), (3) fixture semantics (`.ztd/generated/ztd-fixture-manifest.generated.ts`, `beforeDb`, `afterDb` and that `afterDb` is subset-based, unordered multiset and verifier truncates `beforeDb` tables with `restart identity cascade`), and (4) how to run the test (`npx vitest run src/features/<feature-name>/<query-name>/tests/<query-name>.queryspec.ztd.test.ts`); keep each paragraph/point one to two sentences for readability and preserve the exact command/file names shown.
🤖 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/README.md`:
- Line 70: Replace the misleading phrase "use the linked source-repository
guides" in the README.md sentence so it does not imply clickable hyperlinks;
update the sentence that currently reads "If you need the deeper change
scenarios, use the linked source-repository guides when you are working from the
monorepo checkout; this generated workspace may not contain `docs/`." to
something like "If you need deeper change scenarios, consult the
source-repository guides from the monorepo checkout, since this generated
workspace may not contain `docs/`," or otherwise remove the word "linked" so it
accurately reflects there are no hyperlinks.
- Line 30: Clarify the distinction between the two test directories by editing
the sentence that mentions `.ztd/tests/` and `tests/ztd/`: explicitly state that
`tests/ztd/` contains shared runner code and reusable test harnesses (e.g.,
common fixtures, test runner helpers), while `.ztd/tests/` is reserved for
tool-managed support files (e.g., generated metadata, internal state files) and
should not be edited by users; add a short parenthetical example after each
directory name to make the roles unmistakable.
---
Nitpick comments:
In `@packages/ztd-cli/templates/README.md`:
- Line 46: Split the long single paragraph into 3–4 short paragraphs or a brief
bulleted list covering: (1) the command and purpose (mention `ztd feature tests
scaffold --feature <feature-name>` and the generated files
`tests/generated/TEST_PLAN.md` and `analysis.json`), (2) what the scaffold
creates/owns vs what is persistent (note
`tests/<query-name>.queryspec.ztd.test.ts`, `generated/*` and `tests/cases/`),
(3) fixture semantics (`.ztd/generated/ztd-fixture-manifest.generated.ts`,
`beforeDb`, `afterDb` and that `afterDb` is subset-based, unordered multiset and
verifier truncates `beforeDb` tables with `restart identity cascade`), and (4)
how to run the test (`npx vitest run
src/features/<feature-name>/<query-name>/tests/<query-name>.queryspec.ztd.test.ts`);
keep each paragraph/point one to two sentences for readability and preserve the
exact command/file names shown.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0d97de3-0746-40cc-bbd1-974edeb46153
📒 Files selected for processing (9)
docs/guide/sql-first-end-to-end-tutorial.mdpackages/ztd-cli/README.mdpackages/ztd-cli/templates/PROMPT_DOGFOOD.mdpackages/ztd-cli/templates/README.mdpackages/ztd-cli/templates/src/features/README.mdpackages/ztd-cli/templates/tests/ztd/README.mdpackages/ztd-cli/templates/tests/ztd/case-types.tspackages/ztd-cli/tests/init.command.test.tspackages/ztd-cli/tests/sqlFirstTutorial.docs.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/ztd-cli/templates/tests/ztd/README.md
- packages/ztd-cli/templates/PROMPT_DOGFOOD.md
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/ztd-cli/templates/tests/ztd/case-types.ts
- packages/ztd-cli/README.md
- docs/guide/sql-first-end-to-end-tutorial.md
- packages/ztd-cli/tests/sqlFirstTutorial.docs.test.ts
- packages/ztd-cli/templates/src/features/README.md
- packages/ztd-cli/tests/init.command.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ztd-cli/templates/README.md (1)
52-57: Consider deduplicating repeated troubleshooting bullets.Line 52/56 and Line 53/57 repeat the same guidance. Consolidating each pair would make the section easier to scan without changing behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/templates/README.md` around lines 52 - 57, Two troubleshooting bullets are duplicated; consolidate the repeated guidance into single bullets to improve scanning: remove the duplicate line advising to verify the local `rawsql-ts` checkout (keep one bullet referencing "rawsql-ts" or "local `rawsql-ts` checkout") and merge the two `afterDb`/false-negative bullets into one that mentions "afterDb", multiset/row-order semantics, and volatile columns. Edit the README bullet list so each unique tip appears once, keeping the exact phrasing for keys like `afterDb` and `user_id: null` to make locating and verifying the entries straightforward.
🤖 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/README.md`:
- Line 17: Update the README phrase that currently groups
`tests/{generated,cases}` as “generated assets”: change the wording so it
clearly distinguishes `tests/generated/` as regenerated artifacts and
`tests/cases/` as persistent human/AI-owned test cases (e.g., “keep query-local
ZTD assets under src/features/<feature>/<query>/tests with regenerated artifacts
in `tests/generated/` and persistent human/AI-owned test cases in
`tests/cases/`). Ensure the terms `tests/generated` and `tests/cases` are used
to make the distinction explicit.
---
Nitpick comments:
In `@packages/ztd-cli/templates/README.md`:
- Around line 52-57: Two troubleshooting bullets are duplicated; consolidate the
repeated guidance into single bullets to improve scanning: remove the duplicate
line advising to verify the local `rawsql-ts` checkout (keep one bullet
referencing "rawsql-ts" or "local `rawsql-ts` checkout") and merge the two
`afterDb`/false-negative bullets into one that mentions "afterDb",
multiset/row-order semantics, and volatile columns. Edit the README bullet list
so each unique tip appears once, keeping the exact phrasing for keys like
`afterDb` and `user_id: null` to make locating and verifying the entries
straightforward.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d992aa18-e182-48ea-863a-c837ae952d2a
📒 Files selected for processing (2)
packages/ztd-cli/templates/README.mdpackages/ztd-cli/tests/furtherReading.docs.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/ztd-cli/tests/furtherReading.docs.test.ts
|
|
||
| If you need the repository telemetry seam that comes with the starter, use [Repository Telemetry Setup](../../../docs/guide/repository-telemetry-setup.md) for the edit points, logging examples, and queryId-based flow. | ||
| - keep feature-root entryspec tests under `src/features/<feature>/tests/` | ||
| - keep query-local ZTD generated assets under `src/features/<feature>/<query>/tests/{generated,cases}` alongside the thin entrypoint |
There was a problem hiding this comment.
tests/cases is mislabeled as generated assets.
Line 17 groups {generated,cases} under “generated assets,” but Line 46 correctly treats tests/cases/ as persistent human/AI-owned files. Please reword to avoid implying cases/ is regenerated.
✏️ Suggested wording tweak
-- keep query-local ZTD generated assets under `src/features/<feature>/<query>/tests/{generated,cases}` alongside the thin entrypoint
+- keep query-local ZTD test assets under `src/features/<feature>/<query>/tests/{generated,cases}` alongside the thin entrypoint (`generated/` is CLI-owned; `cases/` is persistent)📝 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.
Suggested change
| - keep query-local ZTD generated assets under `src/features/<feature>/<query>/tests/{generated,cases}` alongside the thin entrypoint | |
| - keep query-local ZTD test assets under `src/features/<feature>/<query>/tests/{generated,cases}` alongside the thin entrypoint (`generated/` is CLI-owned; `cases/` is persistent) |
🧰 Tools
🪛 LanguageTool
[grammar] ~17-~17: Use a hyphen to join words.
Context: .../tests/- keep query-local ZTD generated assets undersrc/features/<fe...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ztd-cli/templates/README.md` at line 17, Update the README phrase
that currently groups `tests/{generated,cases}` as “generated assets”: change
the wording so it clearly distinguishes `tests/generated/` as regenerated
artifacts and `tests/cases/` as persistent human/AI-owned test cases (e.g.,
“keep query-local ZTD assets under src/features/<feature>/<query>/tests with
regenerated artifacts in `tests/generated/` and persistent human/AI-owned test
cases in `tests/cases/`). Ensure the terms `tests/generated` and `tests/cases`
are used to make the distinction explicit.
This was referenced Apr 5, 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
file:dependencies in the generatedpackage.json, rather than by a hard-coded relative path example.Verification
pnpm --filter @rawsql-ts/ztd-cli buildpnpm --filter @rawsql-ts/ztd-cli test -- tests/init.command.test.tspnpm typecheckC:\Users\mssgm\CodexApp\tmp\artifact-docs-checkNotes
Summary by CodeRabbit
New Features
Documentation
Behavior Change
Tests