Refactor ztd-cli to reduce dependencies and support named parameters#433
Refactor ztd-cli to reduce dependencies and support named parameters#433
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughIntroduces SQL-first named-parameter binding via a new shared binder package ( Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as ztd init Command
participant Prompter
participant Templates as Template Generator
participant FS as File System
participant PackageJSON as package.json Manager
User->>CLI: Run ztd init with options
CLI->>Prompter: Gather project details (path, validator backend)
Prompter->>User: Prompt for input, schema workflow (empty/demo/pg_dump)
User-->>Prompter: Provide answers (validator: zod/arktype, workflow choice)
Prompter-->>CLI: Return gathered config + optional features
CLI->>Templates: Resolve template paths (agents, readmes, specs, runtime)
Templates->>FS: Write multi-folder structure
FS-->>Templates: Confirm write with overwrite policy (force/interactive)
alt Validator Backend Selected
CLI->>Templates: Inject validator dependencies (ZOD_DEPENDENCY or ARKTYPE_DEPENDENCY)
Templates->>FS: Write validator-specific spec (_smoke.spec.zod.ts or .arktype.ts)
end
alt Schema Workflow = pg_dump
CLI->>CLI: Execute pg_dump to pull schema
CLI->>FS: Write pulled schema to ztd/ddl/<schema>.sql
else Workflow = demo
CLI->>FS: Write demo.sql with sample tables
else Workflow = empty
CLI->>FS: Write empty template placeholder
end
CLI->>PackageJSON: Inject scripts, devDependencies, formatting hooks
PackageJSON->>FS: Update/create package.json
CLI->>FS: Write per-folder AGENTS.md (runtime, specs, repos, sql, etc.)
CLI->>User: Return summary with next steps and validator info
sequenceDiagram
participant Runtime as Generated Runtime
participant Driver as SQL Driver
participant Binder as `@rawsql-ts/_shared/binder`
participant Adapter as adapter-node-pg
participant Contract as sql-contract
Runtime->>Runtime: Prepare named SQL query with :paramName syntax
Runtime->>Adapter: Call query(sql, { paramName: value, ... })
Adapter->>Adapter: Detect NamedParams object input
Adapter->>Binder: compileNamedParameters(sql, params, 'pg-indexed')
Binder->>Binder: Tokenize SQL (strings, identifiers, comments)
Binder->>Binder: Extract :name placeholders
Binder->>Binder: Replace with $1, $2, ... placeholders
Binder-->>Adapter: Return { sql: compiled, values: ordered[], orderedNames: [] }
Adapter->>Driver: query(compiledSQL, orderedValues)
Driver-->>Adapter: rows: { id, createdAt: Date | string, ... }[]
Adapter->>Runtime: Return raw rows
Runtime->>Contract: normalizeTimestamp(row.createdAt, 'createdAt')
alt Input is Date
Contract->>Contract: Validate non-Invalid Date
Contract-->>Runtime: Return same Date instance
else Input is string
Contract->>Contract: Parse via Date.parse
Contract-->>Runtime: Return Date instance
else Invalid input
Contract->>Contract: Throw with field context
end
Runtime->>Runtime: Apply validator (zod/arktype) to normalized DTO
Runtime-->>Runtime: Return typed result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/ztd-cli/src/commands/init.ts (1)
273-283:⚠️ Potential issue | 🟡 MinorAPP_INTERFACE_SECTION hardcodes "zod v4" — misleading when ArkType is chosen.
Line 279 states
"Input validation relies on zod v4 or later", but this guidance is appended regardless of which validator backend the user selected. When ArkType is chosen, this advice is incorrect.Consider parameterizing this section based on
optionalFeatures.validator, or making the statement validator-agnostic (e.g., "Input validation relies on the configured validator backend").Proposed fix
-3. Input validation relies on zod v4 or later and happens at the repository boundary before any SQL runs. +3. Input validation happens at the repository boundary before any SQL runs (using the project's configured validator).packages/ztd-cli/src/commands/lint.ts (1)
174-219:⚠️ Potential issue | 🟡 MinorContainer cleanup error is unhandled on Line 218.
container?.stop()is awaited but not guarded with.catch(), unlikeclient?.end()on Line 217. If the Testcontainers stop call throws (network timeout, Docker daemon issues), the error will propagate out of thefinallyblock and mask any original error from thetrybody.🛡️ Proposed fix
await client?.end().catch(() => undefined); - await container?.stop(); + await container?.stop().catch(() => undefined);packages/ztd-cli/src/commands/ztdConfig.ts (1)
82-82:⚠️ Potential issue | 🟡 MinorAdd JSDoc to exported
snapshotTableMetadatafunction.This exported function in
src/commands/ztdConfig.tsis missing JSDoc documentation required by the coding guidelines for all exported functions insrc/. Document what the function does, what it expects fromsourcesand the optionalresolverparameter, and whatTableMetadata[]contains.Note: The
TableNameResolvertype dependency is not a concern—this function is used only internally within ztd-cli, and the parameter is optional, allowing callers likegenEntities.tsto omit it entirely.
🤖 Fix all issues with AI agents
In `@docs/recipes/sql-contract.md`:
- Around line 56-58: Update the "What’s next" section: replace the ambiguous
phrase "the Zod or ArkType recipe below" with explicit links to the separate
recipe files for Zod and ArkType (e.g., link labels "Zod recipe" and "ArkType
recipe") so readers know those are separate documents; keep the rest of the
sentence about wiring/runtime validation and ZTD projects intact.
In `@docs/recipes/validation-arktype.md`:
- Around line 11-13: The install command currently uses `-D` which makes
`arktype` a devDependency but the example shows application code
(`listCustomers`) implying production use; either change the install line to
`pnpm add `@rawsql-ts/sql-contract` arktype` to install `arktype` as a runtime
dependency, or explicitly state in the recipe that validation is test-only and
keep `pnpm add -D ...`; update the docs around the `pnpm add -D
`@rawsql-ts/sql-contract` arktype` snippet and reference the `listCustomers`
example so readers know which approach applies.
- Around line 34-41: The code incorrectly infers Customer with ReturnType and
redundantly casts after assertion; update the Customer type to use typeof
CustomerSchema.infer and in listCustomers' reader.validator callback return the
result of CustomerSchema.assert(value) directly (remove the "as Customer" cast)
so the validated value carries the proper inferred type from CustomerSchema;
target symbols: CustomerSchema, Customer type alias, listCustomers, and
reader.validator.
In `@docs/recipes/validation-zod.md`:
- Around line 19-42: The example calls validator() directly on the object
returned by createReader(), but createReader() returns a Mapper (no validator);
fix by creating a mapping via rowMapping(...) (e.g., customerMapping) and then
call bind(customerMapping) on the mapper returned from createReader() to obtain
a MapperReader before calling validator(); update references so you use const
mapper = createReader(executor) and then
mapper.bind(customerMapping).validator(CustomerSchema).list(...).
In `@packages/_shared/binder/src/compileNamedParameters.ts`:
- Around line 120-131: When entering the single-quote state in
compileNamedParameters, remember if the quote was opened with an E prefix (e.g.,
E'...') by setting a flag (e.g., singleQuoteIsEScape) when you see an 'E'
immediately before the opening single quote; then, inside the single_quote
branch (where compiledSql, state, char, next, index are handled), treat a single
quote preceded by a backslash as escaped when singleQuoteIsEScape is true (do
not close the string or exit the single_quote state), and clear that flag when
you transition out of single_quote; this prevents misinterpreting
backslash-escaped quotes like E'\'' as string terminators and avoids rewriting
named params inside such strings.
In `@packages/adapters/adapter-node-pg/package.json`:
- Around line 34-39: The public package `@rawsql-ts/adapter-node-pg` lists a
dependency on the private package `@rawsql-ts/_shared/binder` using "workspace:^",
which will fail for npm consumers because the binder package is
private/unpublished; fix by removing the private binder dependency from
adapter-node-pg and either (A) make `@rawsql-ts/_shared/binder` publishable
(remove private: true and assign a version) and update package.json to reference
its published version instead of workspace:^, or (B) inline/bundle the binder
code into the adapter at build time so `@rawsql-ts/_shared/binder` is not a
runtime dependency, or (C) move the binder utilities into an already-published
package (e.g., rawsql-ts or `@rawsql-ts/testkit-core`) and update the dependency
to that package; ensure package.json no longer contains
"@rawsql-ts/_shared/binder": "workspace:^" when publishing.
In `@packages/adapters/adapter-node-pg/src/types.ts`:
- Around line 34-36: The local duplicate NamedParams type in types.ts must be
removed and replaced with an import from the canonical package; update the file
so PgQueryInput continues to use NamedParams but imports NamedParams from
"@rawsql-ts/_shared/binder" (matching the pattern used in PgTestkitClient.ts) to
eliminate the local definition and ensure a single source of truth for
NamedParams.
In `@packages/ztd-cli/src/utils/optionalDependencies.ts`:
- Around line 25-28: Add JSDoc blocks for the exported functions
clearOptionalDependencyCache, ensureTestkitCoreModule,
ensureAdapterNodePgModule, ensurePgModule, and ensurePostgresContainerModule:
for each function include a one-line summary, descriptions of parameters (if
any) and their types, the exact return type (or Promise<...>), and note thrown
errors (e.g., when a dependency cannot be resolved) and side effects (like
clearing cache); ensure the JSDoc is in plain English, placed immediately above
each exported function, and follows the existing project style (`@param`,
`@returns`, and `@throws` tags where applicable) so documentation and tooling pick
up the signatures.
In `@packages/ztd-cli/templates/src/catalog/specs/_smoke.spec.arktype.ts`:
- Around line 8-13: The type alias SmokeOutput should use ArkType's .infer
rather than ReturnType; replace the current ReturnType usage for the exported
SmokeOutput with the proper ArkType extraction using typeof
smokeOutputSchema.infer so the inferred validated shape from smokeOutputSchema
is used (refer to smokeOutputSchema and SmokeOutput).
In `@packages/ztd-cli/templates/src/jobs/AGENTS.md`:
- Line 10: Update the compound adjective in the AGENTS.md content: change the
phrase "Temporary-table driven workflows" to "Temporary-table-driven workflows"
so the compound modifier is correctly hyphenated; locate the string
"Temporary-table driven" in the AGENTS.md template and replace it with
"Temporary-table-driven".
In `@packages/ztd-cli/tests/init.command.test.ts`:
- Around line 20-35: Export the normalization and sanitization logic from the
production module and import them in the test instead of duplicating them: in
init.ts export the existing normalizeSchemaName function and rename/re-export
the schemaFileName logic as sanitizeSchemaFileName (keep its behavior of
trimming, removing surrounding quotes, lowercasing, replacing non [a-z0-9_-]
chars with '_' and trimming leading/trailing underscores, returning 'schema'
when empty), then update the test to import normalizeSchemaName and
sanitizeSchemaFileName and remove the duplicated implementations so the test
reuses the single source of truth.
🧹 Nitpick comments (17)
packages/ztd-cli/templates/ztd/ddl/AGENTS.md (1)
12-12: Nit: examples use PostgreSQL reserved words.
"user"and"order"are reserved keywords in PostgreSQL and require quoting when used as identifiers. Consider using non-reserved examples (e.g.,account,purchase_order) to avoid misleading newcomers, or add a note about quoting.packages/ztd-cli/templates/ztd/ddl/demo.sql (1)
45-50: Consider adding foreign key constraints totask_assignment.
task_idanduser_idreferencetaskand"user"respectively (per column comments), but noREFERENCESconstraints are declared. For a demo scaffold that guides users on best practices, explicit FK constraints would reinforce referential integrity patterns.♻️ Suggested addition
create table task_assignment ( task_assignment_id serial8 primary key, - task_id bigint not null, - user_id bigint not null, + task_id bigint not null references task(task_id), + user_id bigint not null references "user"(user_id), assigned_at timestamptz not null default current_timestamp );packages/_shared/binder/tsconfig.json (1)
1-14:tsconfig.jsonandtsconfig.build.jsonare identical.Both files in
packages/_shared/binder/have the exact same content. Typicallytsconfig.jsonserves the IDE (and may include tests), whiletsconfig.build.jsonis referenced by the build script. If this package ever adds its own tests,tsconfig.jsonshould include them. Consider havingtsconfig.jsonextendtsconfig.build.jsonand add test paths, or remove the duplicate if one config suffices.packages/_shared/binder/README.md (1)
1-3: Consider adding a brief description of the package's purpose.The import guidance is clear, but a one-liner explaining what the binder does (e.g., named-parameter → positional-placeholder compilation) would help newcomers orient quickly without reading source.
packages/ztd-cli/templates/tests/support/global-setup.ts (1)
8-14: Consider failing early whenDATABASE_URLis absent.The setup now warns but still lets the test suite run. Every SQL-backed test will then fail individually with opaque connection errors, making it harder to diagnose the root cause. Throwing here (or returning a non-zero exit) would surface the problem immediately.
If warn-only is intentional (e.g. to allow non-SQL unit tests in the same suite), a brief inline comment documenting that rationale would help future maintainers of scaffolded projects.
💡 Optional: fail-fast alternative
if (!configuredUrl) { - console.warn( - 'DATABASE_URL is not configured. Install a database adapter or set DATABASE_URL before running SQL-backed tests.', - ); + throw new Error( + 'DATABASE_URL is not configured. Install a database adapter or set DATABASE_URL before running SQL-backed tests.', + ); }packages/_shared/binder/src/compileNamedParameters.ts (1)
223-225: Throwing when no named parameters are found may be too strict for some callers.If a caller passes SQL that legitimately has no parameters (e.g.
SELECT 1), this function throws. The caller must know to avoid callingcompileNamedParametersfor parameterless queries, which pushes validation upstream. Consider whether returning{ sql, values: [], orderedNames: [] }would be more ergonomic, or document this precondition prominently.packages/ztd-cli/src/commands/init.ts (5)
192-222:gatherOptionalFeaturesaccepts an unused_dependenciesparameter.The
_dependenciesparameter is prefixed to suppress lint, but passing it through adds noise at the call site (line 528). If there's no planned use, consider removing it now and adding it later via a signature change when needed.
531-919: Consider extracting the repetitive template-writing block into a data-driven loop.Lines 531–919 follow an identical pattern for ~25 template files: call
writeTemplateFile, check the result, store insummaries. This could be reduced to a configuration array mappingFileKey → templateNameiterated in a loop.This isn't blocking, but it would significantly reduce the ~400 lines of boilerplate and make adding new templates a one-line change.
Sketch of a data-driven approach
const templateEntries: Array<{ key: FileKey; template: string; allowFallback?: boolean }> = [ { key: 'readme', template: README_TEMPLATE, allowFallback: true }, { key: 'ztdDocsAgent', template: ZTD_AGENTS_TEMPLATE }, { key: 'ztdDocsReadme', template: ZTD_README_TEMPLATE }, // ... all other entries ]; for (const { key, template, allowFallback } of templateEntries) { const summary = await writeTemplateFile( rootDir, absolutePaths[key], relativePath(key), template, dependencies, prompter, overwritePolicy, allowFallback ); if (summary) { summaries[key] = summary; } }
979-984: Unsafe type assertionsummaries as Record<FileKey, FileSummary>.
summariesisPartial<Record<FileKey, FileSummary>>, butbuildSummaryLinesdeclares its first parameter as the non-partialRecord<FileKey, FileSummary>. The function body (line 1725) safely handles missing entries withsummaries[key]and a truthiness check, so the runtime is fine — but the signature is misleading. Either keep the assertion and updatebuildSummaryLinesto acceptPartial<...>, or leave the assertion as-is with a comment.Align the signature with actual usage
function buildSummaryLines( - summaries: Record<FileKey, FileSummary>, + summaries: Partial<Record<FileKey, FileSummary>>, optionalFeatures: OptionalFeatures, nextSteps: string[] ): string[] {
1750-1769: Missing--non-interactiveCLI flag — currently inferred from TTY only.Line 1764 sets
nonInteractive: !process.stdin.isTTY, which is a reasonable default. However, there's no explicit--non-interactiveflag for cases where stdin is a TTY but the user wants non-interactive behavior (e.g., scripted CI runners that allocate a pseudo-TTY). Consider adding--non-interactivealongside--yesfor full control.This is low-priority since
--yesalready covers the common CI use case.
1750-1751: ExportedregisterInitCommandis missing JSDoc.As per coding guidelines, all exported functions in
src/must include JSDoc. A one-liner would suffice.Add JSDoc
+/** Register the `ztd init` sub-command on the given Commander program. */ export function registerInitCommand(program: Command): void {packages/ztd-cli/tests/init.command.test.ts (1)
220-241: Consider verifying demo-specific scaffolding artifacts.The empty-scaffold test explicitly asserts that demo files (e.g.
src/sql/example-user/,src/repositories/views/user-profiles.ts) are absent. This test should mirror that by asserting the demo-specific files are present, confirming the two workflows diverge as intended.docs/recipes/sql-contract.md (1)
24-26: Combine the two imports from the same module in the example.Lines 25–26 import
createReaderandrowMappingseparately from the same path. Combining them makes the example cleaner for readers.📝 Suggested fix
-import { createReader } from '@rawsql-ts/sql-contract/mapper'; -import { rowMapping } from '@rawsql-ts/sql-contract/mapper'; +import { createReader, rowMapping } from '@rawsql-ts/sql-contract/mapper';packages/sql-contract/tests/utils/coercions.test.ts (1)
4-37: Solid core coverage; consider addingnulland empty-string edge cases.The five tests cover the main success and error branches well. The implementation has distinct error messages for
nullvalues ("received null") and empty strings ("non-empty timestamp string") that aren't currently exercised. Adding those two cases would complete the branch coverage.🧪 Optional: additional edge-case tests
+ it('throws for null values', () => { + expect(() => timestampFromDriver(null, 'ts')).toThrow( + 'Expected Date or timestamp string for "ts", received null.' + ) + }) + + it('throws for empty strings', () => { + expect(() => timestampFromDriver('', 'ts')).toThrow( + 'Expected a non-empty timestamp string' + ) + })packages/adapters/adapter-node-pg/src/types.ts (1)
31-32: Re-exportNamedParamsfrom@rawsql-ts/_shared/binderinstead of redeclaring it locally.The shared binder package already exports this type publicly. Since
adapter-node-pgalready imports from the binder elsewhere, importingNamedParamsfrom there maintains a single source of truth and eliminates unnecessary duplication.packages/adapters/adapter-node-pg/src/driver/PgTestkitClient.ts (1)
56-76: Internal cast on Line 63 dropsNamedParamsfrom the union.The first overload (Line 50) accepts
values?: unknown[] | NamedParams, but the implementation narrows tounknown[] | undefinedon Line 63. At runtime this is fine becausenormalizeQuerydistinguishes viaArray.isArray, but the cast silently discards theRecordbranch from the type system.Consider widening the assertion so the types stay honest:
♻️ Proposed type alignment
- const values = typeof valuesOrCallback === 'function' ? undefined : (valuesOrCallback as unknown[] | undefined); + const values = typeof valuesOrCallback === 'function' ? undefined : (valuesOrCallback as unknown[] | NamedParams | undefined);packages/ztd-cli/src/utils/optionalDependencies.ts (1)
30-64: Missing JSDoc on all exported functions.Per coding guidelines, all exported functions in
src/must include English JSDoc explaining purpose, inputs, and return value. The five public loaders andclearOptionalDependencyCacheare exported without documentation. The function names are descriptive, but the guideline is explicit.As per coding guidelines:
packages/*/src/**/*.ts: "All exported classes, functions, and types in src/ must include clear English JSDoc explaining what the API does, what it consumes, and what it returns"📝 Example JSDoc for one loader
+/** + * Lazily load `@rawsql-ts/testkit-core`, throwing a descriptive error with an + * install hint when the package is not available. + * `@returns` The testkit-core module namespace. + */ export async function ensureTestkitCoreModule(): Promise<TestkitCoreModule> {
| ```bash | ||
| pnpm add -D @rawsql-ts/sql-contract arktype | ||
| ``` |
There was a problem hiding this comment.
-D installs as a dev dependency — is that intended for production code?
The example snippet defines a listCustomers function that looks like application (not test) code. If this recipe targets production usage, the install command should use pnpm add (without -D) for arktype. If it's test-only validation, the current -D is correct but the example should clarify that context.
🤖 Prompt for AI Agents
In `@docs/recipes/validation-arktype.md` around lines 11 - 13, The install command
currently uses `-D` which makes `arktype` a devDependency but the example shows
application code (`listCustomers`) implying production use; either change the
install line to `pnpm add `@rawsql-ts/sql-contract` arktype` to install `arktype`
as a runtime dependency, or explicitly state in the recipe that validation is
test-only and keep `pnpm add -D ...`; update the docs around the `pnpm add -D
`@rawsql-ts/sql-contract` arktype` snippet and reference the `listCustomers`
example so readers know which approach applies.
| type Customer = ReturnType<typeof CustomerSchema>; | ||
|
|
||
| export async function listCustomers(): Promise<Customer[]> { | ||
| return reader | ||
| .validator((value) => { | ||
| CustomerSchema.assert(value); | ||
| return value as Customer; | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ArkType type.infer property type inference
💡 Result:
In ArkType, .infer is an inference-only property (it’s undefined at runtime) that gives you the output TypeScript type of a type(...) definition. (arktype.io)
Infer the whole object type
import { type } from "arktype"
const User = type({
name: "string",
"age?": "number"
})
type UserT = typeof User.inferInfer a property type from the inferred object
Use normal TypeScript indexed access on the inferred type:
type UserNameT = (typeof User.infer)["name"] // string
type UserAgeT = (typeof User.infer)["age"] // number | undefinedIf you need the input type (e.g., when morphs/pipes transform)
Use .inferIn for the input side:
const ParsedNumber = type("string").pipe(s => Number.parseInt(s))
type Out = typeof ParsedNumber.infer // number
type In = typeof ParsedNumber.inferIn // stringCitations:
- 1: https://arktype.io/docs/type-api?utm_source=openai
- 2: https://arktype.io/docs/intro/your-first-type?utm_source=openai
- 3: https://arktype.io/docs/type-api?utm_source=openai
🌐 Web query:
ArkType assert method return type validation
💡 Result:
In ArkType, .assert(...) is the “throwing” validation API: it validates the value and returns the (possibly transformed) output, otherwise it throws (a TraversalError). (arktype.io)
That means the TypeScript return type of T.assert(x) is the Type’s output type (i.e., what you’d see as typeof T.infer, including any morph/pipe transformations). (arktype.io)
Validating a function’s return value
You typically just assert at the return site:
import { type } from "arktype"
const Out = type({ id: "number" })
export function makeOut(): unknown {
const v = JSON.parse('{"id":1}') // unknown
return Out.assert(v) // returns { id: number } or throws
}Validating transformed returns (pipelines)
If your function produces a value you want validated as an output step, use a pipeline and/or .to(...) (which pipes into an output validator Type): (arktype.io)
const ParseOut = type("string").pipe(JSON.parse).to({ id: "number" })
const out = ParseOut.assert('{"id":1}') // { id: number } or throwsCitations:
- 1: https://arktype.io/docs/type-api
- 2: https://arktype.io/docs/type-api
- 3: https://arktype.io/docs/expressions?utm_source=openai
Use ArkType's .infer property for type inference and leverage the return value from .assert().
The correct way to infer a type from an ArkType Type object is typeof CustomerSchema.infer, not ReturnType. Additionally, CustomerSchema.assert(value) already returns the validated & typed value, so the redundant cast can be removed.
📝 Suggested fix
-type Customer = ReturnType<typeof CustomerSchema>;
+type Customer = typeof CustomerSchema.infer; .validator((value) => {
- CustomerSchema.assert(value);
- return value as Customer;
+ return CustomerSchema.assert(value);
})🤖 Prompt for AI Agents
In `@docs/recipes/validation-arktype.md` around lines 34 - 41, The code
incorrectly infers Customer with ReturnType and redundantly casts after
assertion; update the Customer type to use typeof CustomerSchema.infer and in
listCustomers' reader.validator callback return the result of
CustomerSchema.assert(value) directly (remove the "as Customer" cast) so the
validated value carries the proper inferred type from CustomerSchema; target
symbols: CustomerSchema, Customer type alias, listCustomers, and
reader.validator.
| ```ts | ||
| import { z } from 'zod'; | ||
| import { createReader } from '@rawsql-ts/sql-contract/mapper'; | ||
| import { getSqlClient } from '../support/sql-client-factory'; | ||
|
|
||
| const executor = async (sql: string, params: readonly unknown[]) => { | ||
| const client = await getSqlClient(); | ||
| return client.query(sql, params); | ||
| }; | ||
|
|
||
| const reader = createReader(executor); | ||
|
|
||
| const CustomerSchema = z.object({ | ||
| customerId: z.number(), | ||
| customerName: z.string(), | ||
| }); | ||
|
|
||
| export async function listCustomers() { | ||
| return reader.validator(CustomerSchema).list( | ||
| 'SELECT customer_id, customer_name FROM public.user_account', | ||
| [] | ||
| ); | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and display the sql-contract package.json
find . -type f -name "package.json" | grep -E "packages/sql-contract/package\.json$" | head -1 | xargs catRepository: mk3008/rawsql-ts
Length of output: 1850
🏁 Script executed:
#!/bin/bash
# Check the mapper/index.ts file to verify createReader and its API
find . -path "*/packages/sql-contract/src/mapper/index.ts" -exec cat {} \;Repository: mk3008/rawsql-ts
Length of output: 40150
🏁 Script executed:
#!/bin/bash
# Check if Mapper class has a validator method
grep -A 5 "class Mapper" packages/sql-contract/src/mapper/index.ts | grep -E "validator|async query"
# Also check the full Mapper interface/class structure
sed -n '/^export class Mapper/,/^}/p' packages/sql-contract/src/mapper/index.ts | grep -E "^\s+(async |[a-z]+\()" | head -10Repository: mk3008/rawsql-ts
Length of output: 191
🏁 Script executed:
#!/bin/bash
# Verify the Mapper class does NOT have validator method
sed -n '/^export class Mapper/,/^ }/p' packages/sql-contract/src/mapper/index.ts | grep -c "validator"
# Check if validator is only on MapperReader
grep -n "interface MapperReader" packages/sql-contract/src/mapper/index.ts -A 7Repository: mk3008/rawsql-ts
Length of output: 377
🏁 Script executed:
#!/bin/bash
# Confirm that Mapper.bind() returns MapperReader with validator support
grep -n "bind<T>" packages/sql-contract/src/mapper/index.ts -A 2Repository: mk3008/rawsql-ts
Length of output: 201
The example code will fail at runtime — createReader() returns a Mapper, which does not expose the validator() method.
The validator() method only exists on MapperReader, which must be obtained by calling bind(mapping) first:
Corrected example
import { z } from 'zod';
import { createReader, rowMapping } from '@rawsql-ts/sql-contract/mapper';
import { getSqlClient } from '../support/sql-client-factory';
const executor = async (sql: string, params: readonly unknown[]) => {
const client = await getSqlClient();
return client.query(sql, params);
};
const mapper = createReader(executor);
const CustomerSchema = z.object({
customerId: z.number(),
customerName: z.string(),
});
const customerMapping = rowMapping({
name: 'Customer',
key: 'customerId',
prefix: 'customer_',
});
export async function listCustomers() {
return mapper.bind(customerMapping).validator(CustomerSchema).list(
'SELECT customer_id, customer_name FROM public.user_account',
[]
);
}🤖 Prompt for AI Agents
In `@docs/recipes/validation-zod.md` around lines 19 - 42, The example calls
validator() directly on the object returned by createReader(), but
createReader() returns a Mapper (no validator); fix by creating a mapping via
rowMapping(...) (e.g., customerMapping) and then call bind(customerMapping) on
the mapper returned from createReader() to obtain a MapperReader before calling
validator(); update references so you use const mapper = createReader(executor)
and then mapper.bind(customerMapping).validator(CustomerSchema).list(...).
| if (state === 'single_quote') { | ||
| compiledSql += char; | ||
| if (char === "'" && next === "'") { | ||
| compiledSql += next; | ||
| index += 1; | ||
| continue; | ||
| } | ||
| if (char === "'") { | ||
| state = 'normal'; | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
PostgreSQL E'...' escape strings with backslash escapes are not handled.
Inside single_quote state, only '' (doubled quote) is handled as an escape. PostgreSQL's E'\'' (backslash-escaped single quote inside escape strings) would prematurely close the string state, potentially causing a named param inside the string body to be rewritten.
This is a minor edge case — E'...' syntax is uncommon in modern PostgreSQL, and the AGENTS.md discourages non-standard patterns. However, if this binder is used broadly, it could bite someone.
🤖 Prompt for AI Agents
In `@packages/_shared/binder/src/compileNamedParameters.ts` around lines 120 -
131, When entering the single-quote state in compileNamedParameters, remember if
the quote was opened with an E prefix (e.g., E'...') by setting a flag (e.g.,
singleQuoteIsEScape) when you see an 'E' immediately before the opening single
quote; then, inside the single_quote branch (where compiledSql, state, char,
next, index are handled), treat a single quote preceded by a backslash as
escaped when singleQuoteIsEScape is true (do not close the string or exit the
single_quote state), and clear that flag when you transition out of
single_quote; this prevents misinterpreting backslash-escaped quotes like E'\''
as string terminators and avoids rewriting named params inside such strings.
| export type PgQueryInput = | ||
| | string | ||
| | (Omit<QueryConfig<unknown[]>, 'values'> & { values?: unknown[] | NamedParams; params?: NamedParams }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for PgTestkitClient implementation and how it processes query input
find packages/adapters/adapter-node-pg/src -name "*.ts" | head -20Repository: mk3008/rawsql-ts
Length of output: 345
🏁 Script executed:
# Check the PgTestkitClient class for query handling logic
rg -n -A 20 "class PgTestkitClient" packages/adapters/adapter-node-pg/src/Repository: mk3008/rawsql-ts
Length of output: 2216
🏁 Script executed:
# Look for how values and params are processed
rg -n -B 5 -A 10 "values.*params|params.*values" packages/adapters/adapter-node-pg/src/ -t tsRepository: mk3008/rawsql-ts
Length of output: 10097
🏁 Script executed:
# Check where NamedParams is defined and exported
rg -n "NamedParams" packages/adapters/adapter-node-pg/src/ -t tsRepository: mk3008/rawsql-ts
Length of output: 679
🏁 Script executed:
# Look at the full types.ts file to understand the complete context
cat -n packages/adapters/adapter-node-pg/src/types.tsRepository: mk3008/rawsql-ts
Length of output: 3335
🏁 Script executed:
# Check if NamedParams is exported from shared binder and should be imported instead
rg -n "NamedParams" packages/_shared/binder/src/ -t ts
# Verify the shared binder module location and what it exports
cat -n packages/_shared/binder/src/index.ts | head -50Repository: mk3008/rawsql-ts
Length of output: 769
🏁 Script executed:
# Check all imports of NamedParams across the codebase to understand the pattern
rg -n "import.*NamedParams|from.*NamedParams" packages/ -t tsRepository: mk3008/rawsql-ts
Length of output: 212
Remove ambiguity claim; fix NamedParams duplication.
The resolution order between values and params is explicit via the ?? operator (line 70, line 99) and unambiguous—no issue here.
However, NamedParams is incorrectly defined locally in types.ts (line 32) when it should be imported from @rawsql-ts/_shared/binder, which is the canonical source. Replace the local definition with an import to maintain a single source of truth and match the pattern used in PgTestkitClient.ts.
🤖 Prompt for AI Agents
In `@packages/adapters/adapter-node-pg/src/types.ts` around lines 34 - 36, The
local duplicate NamedParams type in types.ts must be removed and replaced with
an import from the canonical package; update the file so PgQueryInput continues
to use NamedParams but imports NamedParams from "@rawsql-ts/_shared/binder"
(matching the pattern used in PgTestkitClient.ts) to eliminate the local
definition and ensure a single source of truth for NamedParams.
| export type TestkitCoreModule = typeof import('@rawsql-ts/testkit-core'); | ||
| export type AdapterNodePgModule = typeof import('@rawsql-ts/adapter-node-pg'); | ||
| export type PgModule = typeof import('pg'); | ||
| export type PostgresContainerModule = typeof import('@testcontainers/postgresql'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check ztd-cli package.json for optional module declarations
cat packages/ztd-cli/package.json | jq '{
deps: (.dependencies // {} | keys),
devDeps: (.devDependencies // {} | keys),
peerDeps: (.peerDependencies // {} | keys)
}'Repository: mk3008/rawsql-ts
Length of output: 466
🏁 Script executed:
# Check tsconfig files in ztd-cli
echo "=== tsconfig.json ===" && \
cat packages/ztd-cli/tsconfig.json | jq '.compilerOptions.paths // empty' 2>/dev/null || echo "No paths config or file not found"
echo "=== tsconfig.build.json ===" && \
cat packages/ztd-cli/tsconfig.build.json | jq '.compilerOptions.paths // empty' 2>/dev/null || echo "No paths config or file not found"Repository: mk3008/rawsql-ts
Length of output: 198
🏁 Script executed:
# Check the actual optionalDependencies.ts file
cat -n packages/ztd-cli/src/utils/optionalDependencies.ts | head -70Repository: mk3008/rawsql-ts
Length of output: 2654
Missing JSDoc on exported utility functions.
Five exported functions in optionalDependencies.ts lack JSDoc documentation: clearOptionalDependencyCache, ensureTestkitCoreModule, ensureAdapterNodePgModule, ensurePgModule, and ensurePostgresContainerModule. Per coding guidelines, all exported functions in src/ must include clear English JSDoc explaining what they do, what they consume, and what they return. Add JSDoc blocks to each function.
🧰 Tools
🪛 GitHub Actions: PR Check
[error] 26-26: Cannot find module '@rawsql-ts/adapter-node-pg' or its corresponding type declarations.
🤖 Prompt for AI Agents
In `@packages/ztd-cli/src/utils/optionalDependencies.ts` around lines 25 - 28, Add
JSDoc blocks for the exported functions clearOptionalDependencyCache,
ensureTestkitCoreModule, ensureAdapterNodePgModule, ensurePgModule, and
ensurePostgresContainerModule: for each function include a one-line summary,
descriptions of parameters (if any) and their types, the exact return type (or
Promise<...>), and note thrown errors (e.g., when a dependency cannot be
resolved) and side effects (like clearing cache); ensure the JSDoc is in plain
English, placed immediately above each exported function, and follows the
existing project style (`@param`, `@returns`, and `@throws` tags where applicable) so
documentation and tooling pick up the signatures.
| export const smokeOutputSchema = type({ | ||
| id: 'number.integer', | ||
| createdAt: 'Date' | ||
| }); | ||
|
|
||
| export type SmokeOutput = ReturnType<typeof smokeOutputSchema>; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ArkType type() ReturnType inference number.integer
💡 Result:
In ArkType, you generally don’t use ReturnType<> to get the “validated type”. Instead, you extract it from the Type’s inference-only property .infer.
import { type } from "arktype"
const Integer = type("number.integer")
type IntegerOut = typeof Integer.infer // number
type IntegerIn = typeof Integer.inferIn // unknown (what the validator accepts)number.integer is a constraint on number, so the inferred TypeScript type is still number (TypeScript can’t represent “integer” as a distinct primitive type). [1]
If you do this instead:
type R = ReturnType<typeof Integer>R corresponds to what calling the validator returns at runtime: either the validated value or an ArkErrors instance (the docs show checking out instanceof type.errors). [2]
Sources: ArkType Type API (infer, inferIn) [1], ArkType “Your First Type” (validator returns value or type.errors) [2].
Use .infer property instead of ReturnType<typeof> for ArkType type extraction.
ReturnType<typeof smokeOutputSchema> returns the validator's runtime return type (value | ArkErrors), not the inferred TypeScript type. Use typeof smokeOutputSchema.infer to extract the validated type shape:
export type SmokeOutput = typeof smokeOutputSchema.infer;While 'number.integer' does correctly infer as number in TypeScript, the .infer pattern is the proper ArkType API for type extraction.
🤖 Prompt for AI Agents
In `@packages/ztd-cli/templates/src/catalog/specs/_smoke.spec.arktype.ts` around
lines 8 - 13, The type alias SmokeOutput should use ArkType's .infer rather than
ReturnType; replace the current ReturnType usage for the exported SmokeOutput
with the proper ArkType extraction using typeof smokeOutputSchema.infer so the
inferred validated shape from smokeOutputSchema is used (refer to
smokeOutputSchema and SmokeOutput).
| - Maintenance jobs | ||
| - Data backfills | ||
| - Batch operations | ||
| - Temporary-table driven workflows |
There was a problem hiding this comment.
Minor: hyphenate compound adjective.
"Temporary-table driven" should be "Temporary-table-driven" when used as a compound modifier before a noun.
Proposed fix
-- Temporary-table driven workflows
+- Temporary-table-driven workflows📝 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.
| - Temporary-table driven workflows | |
| - Temporary-table-driven workflows |
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Use a hyphen to join words.
Context: ...lls - Batch operations - Temporary-table driven workflows ## Safety rules - Be ...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In `@packages/ztd-cli/templates/src/jobs/AGENTS.md` at line 10, Update the
compound adjective in the AGENTS.md content: change the phrase "Temporary-table
driven workflows" to "Temporary-table-driven workflows" so the compound modifier
is correctly hyphenated; locate the string "Temporary-table driven" in the
AGENTS.md template and replace it with "Temporary-table-driven".
mk3008
changed the title
1 participant
Summary by CodeRabbit
Release Notes
New Features
:user_id) with automatic compilation to driver-specific placeholders.Documentation
Chores