docs(db): @vertz/db v1 API design plan#145
Conversation
Comprehensive design doc for the thin ORM layer covering schema definitions, type inference, query builder, relations, migrations, error hierarchy, and metadata-only multi-tenancy markers. Based on approved roadmap, POC 1 results (28.5% of budget), and all exploration research. Includes self-review notes from Josh (DX), Ben (feasibility), and PM (scope). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![vertz-advocate[bot]](https://avatars.githubusercontent.com/u/254937586?s=60&v=4){kind=small}
There was a problem hiding this comment.
DX Review: Approved
The API surface is clean and discoverable. Key observations:
- The
dnamespace autocomplete story is strong --d.uuid(),d.text(),d.tenant()are immediately obvious. - Query API mirrors familiar patterns (Prisma-like object syntax) but improves on them with combined
select+include. - Separate
createManyvscreateManyAndReturnis the right call -- overloading return types hurts DX. - The E2E acceptance test doubles as a usage guide, which is excellent for onboarding.
- Error hierarchy with table/column/constraint metadata is exactly what developers need.
One flag for implementation: prioritize the @vertz/db/diagnostic export for type error explanations. First impressions matter.
The naming is consistent, the examples compile, and the design is LLM-friendly. Ship it.
![vertz-dev-core[bot]](https://avatars.githubusercontent.com/in/2828081?s=60&v=4){kind=small}
There was a problem hiding this comment.
Feasibility Review: Approved
Technical assessment:
- POC 1 at 28.5% of budget gives high confidence in the type inference approach. The optimization constraints (interfaces over type aliases, no
inferin hot path, pre-computed visibility, depth-2 cap) are correctly captured. JsonbValidator<T>with{ parse(value: unknown): T }is the right generic interface -- decouples from any specific schema library.select: { not }mutual exclusivity is a clean type-level constraint. Straightforward conditional types.- Plugin ordering semantics ("first non-undefined wins") is simple and sufficient for v1.
- Internal
@vertz/schemadependency is clean -- no leakage to the public API surface.
Implementation notes for Phase planning:
- SQL generation for nested includes is the hardest part. Recommend separate queries with batching first, JOINs as optimization.
- Migration rename detection is inherently heuristic -- the interactive CLI prompt is the right escape valve.
- Error code parser for PostgreSQL (~80 lines) should be thorough -- cover the common constraint violation codes.
Buildable as designed. Ready for implementation planning.
Coverage Report for Schema (packages/schema)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for Compiler (packages/compiler)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for Codegen (packages/codegen)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for CLI (packages/cli)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for CLI Runtime (packages/cli-runtime)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for Fetch (packages/fetch)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for Testing (packages/testing)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
DX Review — Josh (Developer Advocate)
I read every line of this design doc adversarially, cross-referencing against @vertz/core's API design, the approved roadmap, and my own previous gap analysis notes. This is a solid doc. The core API shape is good. But there are real problems that will bite developers on day one if we ship as-is.
Issues Found (Must Fix)
1. db.find() vs db.findOne() — naming collision with a confusing semantic split
The doc shows db.find(users, { where: ... }) returning User | null (Section 1.7, line 1). Then a few lines later, db.findOneOrThrow(users, { where: ... }) exists. But there is no db.findOne() method in any example — only db.find() and db.findOneOrThrow(). Meanwhile db.findMany() is the multi-row method.
This creates a confusing asymmetry: find returns one, findMany returns many, but findOne does not exist as a method. Every Prisma developer will reach for findOne or findUnique first. The naming gap will cause confusion.
Proposal: Either (a) rename find to findOne for consistency with findMany, or (b) explicitly document that find IS findOne and explain the choice. Option (a) is strongly preferred — findOne/findMany/findOneOrThrow is a natural family.
The core API design doc uses
findUniquein its db service example (deps.dbService.user.findUnique). If@vertz/dbships withfindinstead offindOne/findUnique, that is a consistency break within the framework itself.
2. $insert type excludes .hidden() columns — this is wrong and will break real apps
Section 1.3 shows:
type UserInsert = typeof users.$insert;
// { email, name } (no id -- has default, no passwordHash -- hidden in insert context)passwordHash is excluded from $insert because it is .hidden(). But you absolutely need to INSERT a password hash when creating a user. .hidden() means "don't return this in queries" — it should NOT mean "don't accept this in writes." This conflates read visibility with write visibility.
Prisma gets this right: select: false on a field means it is excluded from query results by default, but you can still write to it. Drizzle also keeps read/write types independent.
This is a breaking DX problem. Every user with a passwordHash column will hit this immediately, get a confusing type error, and have no clear path forward.
Proposal: $insert and $update should include ALL writable columns regardless of visibility. Visibility annotations (.sensitive(), .hidden()) should only affect read-side types ($infer, $not_sensitive, $not_hidden). If you want a write-only column pattern, that should be a separate annotation (e.g., .writeOnly()), or just don't apply visibility to write types.
3. The E2E test contradicts the design — $insert type test is wrong
Section 7, line approx. 921:
// @ts-expect-error -- id is required on $infer but has a default, so optional on $insert
const _typeTest3: UserInsert = { email: 'e@x.com', name: 'Alice', organizationId: 'uuid', id: undefined };This @ts-expect-error comment says "id is optional on $insert" — but then passes id: undefined which should be assignable to an optional field. The error description does not match the assertion. If id is optional, then { ..., id: undefined } IS valid and the @ts-expect-error should fire as "unused" (which means the test would FAIL). This is either a bug in the test or a misunderstanding of how @ts-expect-error works.
Additionally, passwordHash is not in this insert, which connects to Issue #2 — if $insert excludes hidden fields, how do you create a user with a password?
Proposal: Fix this test. Write it correctly per the TDD rules. Both positive and negative type tests are needed, and they must be accurate.
4. createDb({ url: process.env.DATABASE_URL }) — url accepts string but process.env.DATABASE_URL is string | undefined
Section 1.1 shows:
const db = createDb({
url: process.env.DATABASE_URL,
...
});But the type signature says url: string (not optional, not nullable). This example would fail TypeScript strict mode out of the box. Every developer copying this example will get a red squiggle.
In @vertz/core, the equivalent pattern uses vertz.env() with schema validation that narrows the type. @vertz/db does not have that integration yet, so the example is misleading.
Proposal: Either (a) show the example with process.env.DATABASE_URL! (non-null assertion — honest about what developers will actually write), or (b) show integration with @vertz/core env: url: env.DATABASE_URL. Option (b) is better because it shows how the packages compose, and it matches the core API design's pattern.
5. No vertz db init onboarding command — 5-minute setup is impossible
The doc describes vertz db migrate dev, vertz db push, etc. But there is no vertz db init command. A developer starting from zero has to:
- Know to create a
schema/directory (never mentioned) - Know the file structure convention (never mentioned)
- Know how to create the
db.tsfile (never mentioned) - Know to set
DATABASE_URL(example shows it but does not guide setup)
Prisma has npx prisma init. Drizzle has drizzle-kit generate. Without an init command, the "zero to working ORM in 5 minutes" promise is dead on arrival.
Proposal: Add vertz db init to the CLI section (1.10). It should: create schema/ directory, generate a starter schema.ts with one example table, generate db.ts with createDb() boilerplate, detect DATABASE_URL in env and warn if missing. This is a DX fundamental, not a nice-to-have.
6. d.email() — what does it actually do at runtime?
The column type table says d.email() maps to PostgreSQL text with a "format hint." But what IS a format hint? Is there runtime validation on insert? Is it just metadata? The doc says @vertz/db depends internally on @vertz/schema for runtime validators (Section 1.2), which implies d.email() does runtime email validation. But this is never stated explicitly.
This matters because:
- If
d.email()validates on insert, developers need to know what error it throws and when - If it is metadata-only, developers will be confused when invalid emails get inserted
- Either way, the behavior must be documented
Proposal: Add a short section or note explaining the runtime validation story for typed column methods (d.email(), d.uuid(), etc.). Does d.email() validate on db.create()? What error does it throw? Or is validation the developer's responsibility?
7. select: { not: 'sensitive' } — the not key name is confusing
db.find(users, { select: { not: 'sensitive' } });not as a key inside select reads ambiguously. Is it "select not-sensitive fields"? Or "do not select sensitive fields"? The double negative (select + not) forces developers to pause and think. Compare to Prisma's omit which is a top-level key, not nested inside select.
Also, not is a common word that could easily collide with a column name. If someone has a table with a column called not, this breaks.
Proposal: Consider renaming to exclude: 'sensitive' or making it a top-level option: db.find(users, { omit: 'sensitive' }). The current syntax is too clever — it requires reading docs to understand, which violates "My LLM nailed it on the first try."
8. No guidance on what happens when db.update() matches zero rows
const updated = await db.update(users, {
where: { id: userId },
data: { name: 'Alice Smith' },
});
// Type: UserReturn type is User, not User | null. But what if the where clause matches nothing? Does it throw NotFoundError? Return null? The doc does not say. db.find() returns null for no match, but db.update() returns User (no null in the type). This implies it throws on no match, but that is not documented.
Same question applies to db.delete().
Proposal: Explicitly document the zero-match behavior for every mutation method. If update and delete throw NotFoundError when no rows match, say so. If they return null, adjust the types. This is the kind of thing that causes hours of debugging.
Suggestions (Nice to Have)
S1. Show @vertz/core + @vertz/db integration example
The core API design uses deps.dbService.query(...) with raw SQL. The db design uses db.find(users, ...). These are two different worlds. A single integration example showing how @vertz/db's createDb() becomes a service in @vertz/core modules would be enormously helpful. This is the happy path for 90% of vertz users — they will use both packages together.
S2. db.$tenantGraph property name uses $ prefix inconsistently
db.$tenantGraph, db.$events, db.$push() — the $ prefix is used for internal/introspection APIs. But @vertz/core does not use this convention anywhere. If $ becomes a vertz-wide convention for "framework internals," document it. If it is db-specific, explain why.
S3. d.ref.many(() => posts).through(() => postTags, 'tagId', 'postId') — argument order is confusing
In the many-to-many example:
posts: d.ref.many(() => posts).through(() => postTags, 'tagId', 'postId'),The second and third arguments to .through() are the join table's FK columns, but it is unclear which points where. Is tagId the column in postTags that points to tags (the "this" side), or to posts (the "other" side)? The order is not self-documenting. Named parameters would be clearer:
.through(() => postTags, { from: 'tagId', to: 'postId' })S4. Aggregate/groupBy API feels bolted on
The _avg, _sum, _count naming with underscore prefix feels like it belongs in a different framework. These are Prisma conventions, not vertz conventions. @vertz/core does not use underscore-prefixed keys anywhere. Consider avg, sum, count as plain keys, or a more explicit API like db.aggregate(posts, { compute: { avgViews: avg('views') } }).
S5. Consider a returning option on updateMany/deleteMany
createMany returns { count } but createManyAndReturn returns rows. The same pattern will be wanted for updateMany and deleteMany. Should there be updateManyAndReturn / deleteManyAndReturn? Or is this a v1.1 concern? Either way, mention it in non-goals if deferring.
S6. Error import path @vertz/db/errors is an extra thing to remember
The E2E test imports errors from @vertz/db/errors:
import { UniqueConstraintError, NotFoundError } from '@vertz/db/errors';But createDb and d come from @vertz/db. Why not re-export errors from the main entrypoint? Two import paths means developers need to know which things come from where. Consider re-exporting from @vertz/db for discoverability, with @vertz/db/errors as an advanced tree-shaking path.
What's Good
-
The
dnamespace is excellent.d.table(),d.uuid(),d.text()— it is clean, discoverable, and the autocomplete story will be great. This is best-in-class schema definition DX. -
Combined
select+includeis a genuine differentiator. Prisma forces you to choose one. This design lets you narrow fields AND include relations in one query. Developers will love this. -
The error hierarchy is well-structured.
UniqueConstraintErrorwith.columnand.valueis exactly what developers need. ThetoJSON()method is a nice touch for API responses. ThedbErrorToHttpErroradapter for@vertz/coreshows good package composition thinking. -
Metadata-only
d.tenant()with honest startup notices is the right call. Shipping metadata that does nothing yet but telling developers honestly is far better than fake runtime enforcement or deferring the API entirely. This sets up v1.1 cleanly. -
The POC results (28.5% of type budget) give real confidence. This is not hand-waving — the team actually measured it. The optimization constraints from Ben are captured in the design. This is how you de-risk a type-heavy feature.
-
findManyAndCountas a dedicated method is great DX. Pagination with total count is the most common pattern in every CRUD app. Having it as a first-class method instead of two separate queries shows good product instinct. -
The cache-readiness primitives are forward-thinking without overcommitting. Mutation event bus, query fingerprinting, result metadata — these are the right hooks for future caching without building a cache layer now.
Verdict
REQUEST CHANGES
Issues #1 (find vs findOne naming), #2 ($insert excluding hidden columns), and #8 (zero-match mutation behavior) are DX landmines that will hit every developer in the first 10 minutes. Issue #5 (no init command) makes the 5-minute onboarding promise impossible. Issue #4 (url type mismatch) means the first code example in the doc does not compile under strict mode.
The foundation is strong — the d namespace, the type inference story, the error hierarchy, the metadata-only tenancy approach — these are all correct. But the issues above need to be resolved before this design is ready for implementation.
Fix the must-fix issues, and I will approve on the next round.
Scope Review — PM (Product Manager)I have cross-referenced every item in the approved roadmap ( Roadmap Alignment Check (v1.0 Tasks)The roadmap defines 15 tasks for v1.0 + 3 pre-release tasks. Checking each against the design doc: v0.1 Pre-release:
v1.0 Initial Release:
Result: All 15 v1.0 tasks and all 3 v0.1 tasks from the roadmap are accounted for in the design doc. Scope Creep Found1. Cache-Readiness Primitives (Section 8) — SCOPE CREEP The design doc includes a full Section 8 describing five cache-readiness primitives:
In the roadmap, these are explicitly a v1.1 task (see roadmap "Cache-readiness primitives" under "v1.1 — RLS, Sessions, Transactions & Tenant Enforcement"). The milestone summary also confirms this: "v1.1 — RLS & Transactions | ... | cache primitives". Nuance: The roadmap's v1.0 query builder task includes one line item: "Mutation event bus integration (cache-readiness primitive 7.1)". So primitive 8.1 (mutation event bus) alone could be argued as v1.0, but primitives 8.2-8.5 are clearly v1.1 scope. Recommendation: Either (a) move Section 8 to the Non-Goals section and note it as v1.1, or (b) explicitly state that Section 8 documents the approved v1.1 design for reference only and is NOT in v1.0 scope. As written, Section 8 reads like it is part of the v1 deliverable. 2. Window Functions in SQL Escape Hatch — BORDERLINE The roadmap lists "Window function support" under the SQL escape hatch task. The design doc shows CTE examples but does not explicitly show window function examples. This is not scope creep (it is missing, not extra), but I flag it here because the Missing from Roadmap1. SQL Generator — Not a Standalone Section The roadmap has "SQL generator" as a standalone P1 task with ~32 hours estimated, covering SELECT/INSERT/UPDATE/DELETE generation, JOIN generation, subquery support, parameter binding, and PostgreSQL-specific syntax. The design doc does not have a dedicated section for the SQL generator. Its existence is implied by the query builder and SQL escape hatch sections, but the specific deliverables (parameter binding, SQL injection prevention, PostgreSQL-specific operators like JSONB/array) are not explicitly called out. Recommendation: Either add a brief section noting the SQL generator as an internal implementation component, or at minimum mention parameter binding and SQL injection prevention explicitly in the query builder section. The security aspect (SQL injection prevention) deserves explicit design attention. 2. Dry-run Mode for Migrations The roadmap lists "Dry-run mode" under the migration runner task. The design doc's migration section (1.10) does not mention dry-run. Minor gap. 3. Josh flagged in the roadmap's open questions that Founder Decision ComplianceChecking all 18 founder decisions from the roadmap:
Auth deep-dive founder decisions (Session 2026-02-09):
Result: All 18 founder decisions are reflected. All auth deep-dive decisions are reflected. Non-Goals AssessmentThe design doc lists 16 non-goals. Cross-referencing:
Issue with Non-Goal #7: It reads: "v1 ships cache-readiness primitives (mutation event bus, query fingerprinting) but no built-in cache." This implies the cache-readiness primitives ARE in v1. Per the roadmap, they are in v1.1 (except possibly the mutation event bus which is mentioned in the v1.0 query builder task). This needs to be corrected or clarified. VerdictREQUEST CHANGES The design doc is very strong — it faithfully reflects 18/18 founder decisions, covers all v1.0 roadmap tasks, and the non-goals are mostly comprehensive. However, there is one clear scope creep issue that needs to be resolved before approval:
These are small fixes. The overall design is roadmap-aligned and founder-decision-compliant. Once the cache-readiness scope is clarified, this gets a product approval. Minor items (not blocking):
|
There was a problem hiding this comment.
Technical Feasibility Review -- Ben (Core Engineer)
I have read the full design doc on docs/db-design, the existing @vertz/schema and @vertz/core source, and mike's gap analysis. This is an adversarial review -- my job is to find the technical problems, not to rubber-stamp.
Blocking Issues (Must Fix Before Implementation)
B1. d.table() returns a value with $infer, $insert, $update, $not_sensitive, $not_hidden -- but the design uses classes for errors
The design declares abstract class DbError extends Error with subclasses like UniqueConstraintError, ForeignKeyError, etc. Core's pattern is functional: createMiddleware() returns a frozen plain object, createApp() returns a builder object, @vertz/schema uses classes for the Schema base but those are internal. The d namespace is shown as a factory (functional), which is correct -- but the error hierarchy introduces 8+ classes into a package that otherwise has zero classes.
This is a consistency issue with core's patterns. I am not saying "no classes ever" -- @vertz/schema uses them internally. But @vertz/core's VertzException is already a class, and the design explicitly states DbError is NOT a subclass of VertzException. That means users catching errors now have two unrelated class hierarchies (VertzException for HTTP errors, DbError for database errors) with no common ancestor.
Fix required: Either (a) make DbError extend VertzException (which the design rejects for standalone usage), or (b) define a common VertzBaseError in a shared package that both extend, or (c) document the explicit dbErrorToHttpError adapter as the ONLY integration path and add a type-level test that proves the mapping is exhaustive. Option (c) is the least invasive but needs a concrete type to guarantee exhaustiveness:
// This must be proven exhaustive at the type level
type DbErrorToHttpMap = {
UNIQUE_VIOLATION: 409;
FOREIGN_KEY_VIOLATION: 422;
NOT_NULL_VIOLATION: 422;
CHECK_VIOLATION: 422;
NOT_FOUND: 404;
CONNECTION_ERROR: 503;
POOL_EXHAUSTED: 503;
};
// If a new DbError subclass is added without updating the map,
// this type fails:
type _Exhaustive = Assert<keyof DbErrorToHttpMap, DbError['code']>;Without this, adding a new DbError subclass silently breaks the adapter. This is a footgun for v1.1 when TransactionError arrives.
B2. FindResult<Table, Options> type with combined select + include will produce unreadable error messages
The design shows:
const postsWithAuthorName = await db.findMany(posts, {
select: { title: true, status: true },
include: { author: { select: { name: true } } },
});
// Type: (Pick<Post, 'title' | 'status'> & { author: Pick<User, 'name'> })[]I validated with POC 1 that the instantiation count is fine. What I did NOT validate is what happens when the developer makes a typo. Consider:
const result = await db.findMany(posts, {
select: { titel: true }, // typo
include: { author: { select: { naem: true } } }, // typo in nested
});TypeScript will produce an error like:
Type '{ titel: true }' is not assignable to type '{ id?: true; authorId?: true; title?: true; content?: true; status?: true; tags?: true; metadata?: true; views?: true; createdAt?: true; updatedAt?: true }'.
Object literal may only specify known properties, and 'titel' does not exist in type '...'
That is manageable for the top level. But for the nested include, the error will reference the full generic resolution path: IncludeResolve<TRelations, TInclude, depth=2> which expands to a monster type. With 10+ columns per table and 3+ levels of generics, the error message will be 20+ lines of noise.
Fix required: The design must specify a strategy for error message quality in the type system. Concrete recommendation: use branded "error message" types as type-level assertions. For example:
type InvalidSelectKey<K extends string, Table extends string> =
`Column '${K}' does not exist on table '${Table}'. Did you mean one of: ${SuggestColumns<K, Table>}?`;This is not hypothetical. Drizzle has this exact problem. Their GitHub issues are full of "I got a 50-line type error and I don't know what's wrong." The design must not repeat this mistake.
This is blocking because the design doc's own manifesto alignment section says "My LLM nailed it on the first try" -- an LLM cannot recover from a 50-line generic expansion error. It needs a clear, short error message.
B3. d.ref.many().through() relation type cannot be inferred without explicit annotation at depth 2
The M:M through-table pattern:
posts: d.ref.many(() => posts).through(() => postTags, 'tagId', 'postId'),When this is used in an include:
db.findMany(tags, {
include: { posts: true },
});The type system must:
- Resolve
postsrelation ontags-> seesd.ref.many().through() - Identify the through-table (
postTags) and the FK columns - Resolve the target table (
posts) - Infer the result type as
Tag & { posts: Post[] }
Step 2 is the problem. The through-table introduces a third generic parameter into IncludeResolve. At depth 2, the type system is now resolving: Tag -> posts (via postTags) -> author (on posts). That is 3 table lookups and 2 JOIN type resolutions. The POC 1 benchmark did NOT test through-table resolution because the POC only had d.ref.one() and d.ref.many() (direct relations).
Fix required: Either (a) add through-table resolution to the POC benchmark and verify it stays within budget, or (b) explicitly exclude through-table includes from the depth-2 cap (through-tables are always depth 1 -- you can include the M:M relation but not nested relations on the target). I recommend (b) as the pragmatic choice.
B4. The select: { not: 'sensitive' } type is a discriminated union that will conflict with select: { id: true }
The design says these are mutually exclusive at the type level. Let me show why this is harder than it sounds. The select option type must be:
type SelectOption<T> =
| { not: 'sensitive' | 'hidden' } // visibility filter
| { [K in keyof T]?: true } // explicit field pickThis is a discriminated union on the not key. But TypeScript's excess property checking does NOT apply to union members the way you'd expect:
// This should be a type error per the design, but...
db.findMany(users, {
select: { not: 'sensitive', id: true }, // does TypeScript reject this?
});If select is typed as the union above, TypeScript will match the first branch ({ not: 'sensitive' | 'hidden' }) because it has the not property -- and silently ignore id: true as an excess property in a union context. Excess property checking on union types is NOT strict in TypeScript -- if ANY branch matches, the remaining properties are allowed.
Fix required: Use a conditional type with never to make the branches truly exclusive:
type SelectOption<T> =
| { not: 'sensitive' | 'hidden'; [K in keyof T]?: never }
| { [K in keyof T]?: true; not?: never }The never on the opposing key is what enforces mutual exclusivity. Without it, the type system will silently accept the illegal combination. This needs a .test-d.ts proving the rejection works.
Technical Concerns (Address During Implementation)
C1. SQL generation for nested includes with depth 2 -- separate queries vs JOINs is an architectural decision that affects the type system
The design says queries return (Post & { author: User })[]. If implemented with JOINs, the SQL result set is flat and needs hydration (grouping joined rows into nested objects). If implemented with separate queries (N+1 with batching), the results arrive pre-shaped but require a second query.
The problem: the type system does not know which strategy is used. But the runtime behavior differs:
- JOINs: One query, but duplicate parent rows when a one-to-many is joined. A
findMany(posts, { include: { comments: true } })with 10 posts and 100 comments returns 100 rows that must be hydrated into 10Post & { comments: Comment[] }objects. The hydration logic needs to deduplicate by primary key. - Separate queries: Two queries, but no hydration needed. The result shape is natural.
Both are valid, but the choice affects: (a) findManyAndCount semantics (does total count pre- or post-dedup?), (b) cursor pagination with includes (cursor on the parent or the joined set?), (c) limit/offset behavior with one-to-many includes.
The design does not specify which approach is used. This MUST be decided before implementation because it affects the query builder's SQL generation and the result type guarantees.
C2. d.tenant(organizations) creates a circular reference between table definitions at the module level
export const users = d.table('users', {
organizationId: d.tenant(organizations), // references `organizations`
// ...
});Unlike d.ref.one(() => users, 'authorId') which uses a lazy function reference, d.tenant(organizations) takes the table directly. If organizations is defined in the same file, this is fine (hoisting). If it is defined in a different file, you get a circular import:
users.tsimportsorganizationsfromorganizations.tsorganizations.tsmight importusersfor ad.ref.many()relation
The design uses lazy references for relations (() => users) to avoid this exact problem. But d.tenant() does NOT use a lazy reference.
Fix: Change d.tenant(organizations) to d.tenant(() => organizations) for consistency with the relation API. This is a small change but it prevents a class of module-resolution bugs.
C3. The $insert type derivation has ambiguous semantics for .hidden() columns
The design says:
type UserInsert = typeof users.$insert;
// { email, name } (no id -- has default, no passwordHash -- hidden in insert context)But wait -- passwordHash is .hidden(), which is described as "excluded from both $not_sensitive and $not_hidden types." The design then says hidden columns are also excluded from $insert. But this is semantically wrong.
passwordHash is a column you absolutely NEED to provide on insert. You cannot create a user without a password hash. The column is "hidden" from query results (you don't want to return it in API responses), but it is required for creation.
If $insert excludes .hidden() columns, how do you create a user? The E2E test actually contradicts this:
// Line from the E2E test:
const user = await db.create(users, {
data: {
id: crypto.randomUUID(),
organizationId: orgId,
email: 'alice@acme.com',
passwordHash: '$2b$10$...', // <-- hidden column IS provided
name: 'Alice',
},
});The data parameter type for create is NOT $insert -- it must include hidden columns. So what is $insert for? The design says it is a derived type helper, but it is misleading if it excludes columns that are required for insertion.
Fix: Clarify that $insert represents the public-facing insert shape (what an API consumer provides), while db.create() accepts the full insert shape (including hidden columns). Or remove $insert as a misleading abstraction and let db.create() use its own derived type.
C4. Enum types are PostgreSQL-specific and will produce confusing migration diffs
plan: d.enum('org_plan', ['free', 'pro', 'enterprise']).default('free'),The enum name 'org_plan' is a PostgreSQL CREATE TYPE name. If two tables use the same enum name with different values, it is a schema error:
// Table A
status: d.enum('status', ['active', 'inactive']),
// Table B
status: d.enum('status', ['open', 'closed', 'resolved']),This creates two columns referencing status type but with different values. PostgreSQL will reject the second CREATE TYPE if the first already exists. The migration differ must detect this conflict.
More subtly: modifying an enum (adding a value) requires ALTER TYPE ... ADD VALUE, which cannot run inside a transaction in PostgreSQL < 12. The design says "Enum type diff" in the migration differ, but does not address this PostgreSQL-specific constraint.
C5. db.$tenantGraph computation at startup is O(tables * relations) -- acceptable, but the design does not specify error handling for cycles
The tenant graph traversal (which tables are directly vs. indirectly tenant-scoped) requires walking the relation graph from tenant-root tables. If the schema has a relation cycle (A -> B -> C -> A), the graph traversal will loop infinitely unless cycle detection is implemented.
This is ~10 lines of code (visited set), but the design should mention it because it is a startup crash if missed.
C6. The casing: 'snake_case' option adds hidden complexity to every query and every migration
When casing: 'snake_case' is active, the ORM translates between TypeScript camelCase property names and SQL snake_case column names. This affects:
- Every
whereclause (mapcreatedAttocreated_at) - Every
selectclause - Every
orderByclause - Every
dataobject in mutations - Every migration column name
- The
_snapshot.jsonformat (does it store camelCase or snake_case?)
The design does not specify:
- When does the casing transform happen? (At query build time? At result hydration time?)
- What if a column is already snake_case in TypeScript? (
user_idin TS ->user_idin SQL, no transform) - Does the snapshot store the TS name or the SQL name?
- What about
sqltagged template literals? Does casing apply inside raw SQL?
This is not blocking but it is a source of bugs if not specified precisely.
C7. Plugin beforeQuery "first non-undefined return wins" semantics are a footgun for composability
If Plugin A modifies the query context and returns it, Plugin B never runs. This is fine for v1 with 0-1 plugins, but it means plugins cannot compose. If a user has both a logging plugin and a tenant-scoping plugin, the first one that returns a modified context "wins" and the other is skipped.
The alternative is pipeline semantics (each plugin receives the output of the previous one). The design should document why "first wins" was chosen and acknowledge the composability limitation.
Implementation Notes
N1. The d namespace should be a frozen object (consistent with core's createMiddleware)
Core uses deepFreeze on middleware definitions. The d namespace and all table definitions returned by d.table() should also be frozen to prevent accidental mutation. This is consistent with core's immutability pattern.
N2. Start with separate queries for includes, not JOINs
As I noted in the previous feasibility review, JOINs with hydration are the complex path. Separate queries (with DataLoader-style batching) are simpler to implement, easier to debug, and produce naturally-shaped results. Optimize to JOINs when benchmarks show it matters.
N3. The POC 1 validation at 28.5% of budget is for the CURRENT design scope
If v1.1 adds d.policy(), d.session(), and RLS types that flow through the query result, the type budget will increase. The 28.5% result has ~71.5% headroom, which is generous, but it should be re-validated when v1.1 types are designed. Do not treat the POC 1 result as permanent validation.
N4. The d.email() column type implies runtime validation but the behavior is unspecified
d.email() maps to PostgreSQL text with a "format hint." Does this mean:
- At insert time, the ORM validates the email format? Using what validator?
@vertz/schema'sEmailSchema? - Or is it metadata-only (the format hint is for documentation/introspection but no runtime check)?
If it does runtime validation, what happens on failure? A CheckConstraintError? A new ValidationError? This is unspecified.
N5. d.textArray() and d.integerArray() are special-cased but d.jsonb<T>() is generic -- inconsistency in array column handling
Why not d.array(d.text()) and d.array(d.integer())? The current design has dedicated array methods for two types but a generic approach for JSON. This asymmetry means adding d.uuidArray() or d.booleanArray() requires adding new methods to d rather than using composition. Consider a d.array(columnType) pattern.
N6. The migration _snapshot.json version field ("version": 1) needs an explicit migration strategy
When v1.1 adds policy metadata to the snapshot, the version bumps to 2. The differ must handle reading a v1 snapshot and comparing against v2 schema definitions. This is a data migration problem inside the migration system -- meta-migrations.
Verdict
REQUEST CHANGES
The design is 80% buildable as written. The POC 1 results give genuine confidence in the type inference approach, and the API surface is clean. However, the four blocking issues must be resolved:
- B1 -- Error hierarchy integration with core needs an exhaustiveness guarantee (type-level).
- B2 -- Type error message quality strategy must be specified (not "we'll figure it out during implementation").
- B3 -- Through-table include resolution must be validated or scoped out of depth-2.
- B4 --
select: { not }mutual exclusivity needs thenever-key pattern; the naive union will silently accept invalid input.
None of these are design-killers. B4 is a 10-line fix to the type definition. B3 is a scoping decision. B1 and B2 require adding sections to the design doc. Once addressed, this is ready for Stage 2.
- Rename find() to findOne() for clarity (Josh #1) - Separate read/write visibility: $insert includes .hidden() columns (Josh #2, Ben C3) - Fix select union type with never-keyed branches for mutual exclusivity (Ben B4) - Move cache-readiness primitives to v1.1 preview section (PM scope creep) - Fix Non-Goal #7 to not claim v1 ships cache primitives (PM) - Add exhaustiveness guarantee for error hierarchy with Assert pattern (Ben B1) - Add type error quality section with branded error messages (Ben B2) - Flag d.ref.many().through() as unvalidated, cap depth at 1 (Ben B3) - Fix E2E type test assertions to match actual type semantics (Josh #3) - Fix first example to compile under strict mode with ! assertion (Josh #4) - Add vertz db init onboarding flow (Josh #5) - Document zero-match behavior for all mutation methods (Josh #8) - Add SQL injection prevention / parameter binding note (PM minor) - Add dry-run mode for migrations (PM minor) - Clarify d.email() is metadata-only, no runtime validation (Josh #6, Ben N4) - Add vertz.env() pattern for type-safe DATABASE_URL access Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
vertz-tech-lead
Bot
dismissed stale reviews from vertz-dev-core[bot] and vertz-advocate[bot]
via
864bc9a
![vertz-dev-dx[bot]](https://avatars.githubusercontent.com/in/2828112?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — all 7 required sections present and substantive. Adversarial review of the 10 fixes from the previous round:
All fixes verified solid:
find()→findOne()rename — consistent throughout, no stale references- Read/write visibility separation — explicitly documented in Section 1.3, validated in E2E type tests
SelectOptionmutual exclusivity —never-keyed branches correctly specified, type flow path #15 mandates.test-d.ts- Cache-readiness → v1.1 preview — Section 8 clearly disclaimed, Non-Goal #7 aligned
- Error exhaustiveness —
Assertpattern in Section 1.9 catches missingDbErrorToHttpMapentries at compile time - Branded error messages — Section 6.3 shows concrete
InvalidSelectKeypattern with good/bad error comparison d.ref.many().through()unvalidated — Unknown U6 flags the risk, depth capped at 1- E2E type assertions —
@ts-expect-erroron$infer(hidden),$not_sensitive, missing$insertfield, narrowed select - Strict mode fix —
DATABASE_URL!with non-null assertion,vertz.env()alternative documented vertz db init— Section 1.10 scaffolds directory structure with next steps
Additional checks passed:
- Zero-match behavior table is complete and consistent (single-row throws, multi-row returns count)
- SQL injection prevention documented (parameterized by default,
sql.raw()explicitly flagged) - Migration dry-run mode present
d.email()metadata-only clarification is clear- 15 type flow paths are concrete and each maps to a
.test-d.tscriterion - E2E acceptance test is compilable, specific, covers happy path + error cases + type safety
No issues found. Ship it.
* docs(db): add @vertz/db v1 API design plan Comprehensive design doc for the thin ORM layer covering schema definitions, type inference, query builder, relations, migrations, error hierarchy, and metadata-only multi-tenancy markers. Based on approved roadmap, POC 1 results (28.5% of budget), and all exploration research. Includes self-review notes from Josh (DX), Ben (feasibility), and PM (scope). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(db-design): address review feedback from Josh, Ben, and PM - Rename find() to findOne() for clarity (Josh #1) - Separate read/write visibility: $insert includes .hidden() columns (Josh #2, Ben C3) - Fix select union type with never-keyed branches for mutual exclusivity (Ben B4) - Move cache-readiness primitives to v1.1 preview section (PM scope creep) - Fix Non-Goal #7 to not claim v1 ships cache primitives (PM) - Add exhaustiveness guarantee for error hierarchy with Assert pattern (Ben B1) - Add type error quality section with branded error messages (Ben B2) - Flag d.ref.many().through() as unvalidated, cap depth at 1 (Ben B3) - Fix E2E type test assertions to match actual type semantics (Josh #3) - Fix first example to compile under strict mode with ! assertion (Josh #4) - Add vertz db init onboarding flow (Josh #5) - Document zero-match behavior for all mutation methods (Josh #8) - Add SQL injection prevention / parameter binding note (PM minor) - Add dry-run mode for migrations (PM minor) - Clarify d.email() is metadata-only, no runtime validation (Josh #6, Ben N4) - Add vertz.env() pattern for type-safe DATABASE_URL access Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: vertz-tech-lead[bot] <2828099+vertz-tech-lead[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: vertz-dev-dx[bot] <260432280+vertz-dev-dx[bot]@users.noreply.github.com>
1 participant
Summary
Comprehensive design doc for
@vertz/dbv1 (thin ORM layer). This is the Stage 1 design deliverable based on:/app/backstage/roadmaps/vertz-db.md)v1 Scope
d.table()/d.column()schema definitions with full column types$infer,$insert,$update,$not_sensitive,$not_hiddend.tenant()/.shared()as metadata-only markersDbError)Self-Review
Includes review notes from three perspectives:
Test plan