One task, and that is a decision rather than a shortcut. itemSelect.ts, itemFilters.ts, both routes and the unit test file are coupled — the exports the routes call are the ones being replaced, and #298 put the test file under a tsconfig that type-checks it, so any partial commit is a red build. Every line of it was verified by probe against the real generated schema before it was written, not sketched. The projections type-check, jsonArrayFrom correlates through whereRef, the mixed array of sql templates and builder expressions composes under eb.and, and the emitted SQL is quoted in the steps so a wrong result is caught at the step that produces it rather than three steps later. The probe also settled the question the spec left open with a fallback: the row type is assignable to the hand-written contract, so no cast is needed. The two invariant tests are rewritten rather than ported. They used to read the clause strings the builder returned; they now compile the expressions against the same items-and-categories shape the real queries use and assert on the SQL Kysely emits, with the hostile value present in the parameters and absent from the text. Building a narrower query in the helper would have needed a cast, and a cast in that test would be testing the cast. The step that verifies the conversion is the one that runs the integration suite unedited. Those tests are the contract — same JSON, same ordering, same statuses — so the plan says plainly that a test needing an edit means the query changed behaviour and the query is what to fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
29 KiB
Dynamic Queries to Kysely Implementation Plan
For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (
- [ ]) syntax for tracking.
Goal: Build the item list and by-id queries through Kysely so no value or clause is ever interpolated into query text, retiring the last two S2077 hotspots.
Architecture: itemSelect.ts stops exporting SQL strings and exports two query builders instead, with the image and tag aggregates built by jsonArrayFrom. itemFilters.ts stops returning { clauses, params } and returns an array of Kysely expressions. The four call sites compose the two.
Tech Stack: Express 4 + TypeScript, Kysely 0.28 (jsonArrayFrom from kysely/helpers/postgres), pg, Jest + supertest.
Spec: docs/superpowers/specs/2026-09-04-dynamic-queries-kysely-design.md
Global Constraints
- No value and no clause may reach query text. Every filter value is a bind parameter. This is the entire point of the change.
- The API contract does not change. Same columns, same JSON keys, same ordering, same statuses. If a response changes, the conversion is wrong.
AdminItemRowandPublicItemRowstay exported and stay hand-written. The queries are assigned to them so a drift becomes a compile error. Do not replace them with inferred types.ADMIN_IMAGES_SUBQUERY's distinction survives:original_image_pathappears in the admin projection and never in the public one (#293).parseItemFiltersis untouched, and so is every test over it.- This is one task.
itemSelect.ts,itemFilters.ts, both routes and the unit tests are coupled — the exports the routes use are the ones being replaced, and the test file is type-checked bytsconfig.test.json, so any partial commit is a red build. - All SQL parameterized; every Express handler stays wrapped in
asyncRoute. - Commit subject ends with
(#308). Commit bodies are never hard-wrapped — one long line per paragraph. End withCo-Authored-By: Claude Opus 5 <noreply@anthropic.com>. - Do not push.
- Do not run
scripts/start-local.ps1orscripts/run-tests.ps1— they prompt for UAC and hang. - Node 20 is required. Prepend
export PATH="/c/Users/tlamb/AppData/Local/nvm/v20.20.2:$PATH"to every command; the machine default is 18.x and Jest fails on it.
File Structure
| File | Change |
|---|---|
backend/src/itemSelect.ts |
The four SQL string constants become adminItemQuery() and publicItemQuery(). Row types kept. |
backend/src/itemFilters.ts |
buildItemFilterSql → itemFilterExpressions. BuiltFilter deleted. Parser untouched. |
backend/src/routes/admin.ts |
Three call sites: the list, and two by-id reads. |
backend/src/routes/items.ts |
Two call sites: the list, and the by-id read. |
backend/tests/unit/itemFilters.test.ts |
The two buildItemFilterSql describe blocks rewritten against compiled SQL. The parseItemFilters block untouched. |
Everything below was verified by probe against the real generated schema before this plan was written — it compiles, and the SQL it emits is quoted in the steps. It is not a sketch.
Task 1: The conversion
Files:
- Modify:
backend/src/itemSelect.ts,backend/src/itemFilters.ts,backend/src/routes/admin.ts,backend/src/routes/items.ts,backend/tests/unit/itemFilters.test.ts
Interfaces:
-
Consumes:
dbfrom../db,DBfrom../db-kysely/schema, both from #305. -
Produces:
adminItemQuery(),publicItemQuery(),ItemContext,AdminItemRow,PublicItemRowfromitemSelect.ts;itemFilterExpressions(eb, filters, favoritesCustomerId)fromitemFilters.ts. -
Step 1: Rewrite
itemSelect.ts
Replace everything from the top of the file down to and including export const ADMIN_ITEM_BY_ID = ... with the following. Keep every interface below that line exactly as it is — ItemRowBase, AdminItemRow, PublicItemRow and anything else — they are the contract this change is checked against.
// Shared item query shapes for the public and admin routes, and the row types
// they return.
//
// The types live here rather than in types.ts because they describe a
// projection, not a table. adminItemQuery takes every column of items and
// publicItemQuery names its columns, so the storefront never sees
// paypal_order_id or reserved_until — typing both as "an items row" would
// quietly re-admit exactly the columns that projection was written to exclude.
//
// These were SQL string constants until #308. They had to be kept in step with
// their row types by hand, because `pool.query<T>` asserts a shape and never
// checks it against the SQL, so dropping a column from a select without
// dropping it from its type compiled cleanly and went undefined at run time —
// and only the integration suite ever caught it. Built through Kysely, that is
// a compile error, because the row type now follows from the projection.
//
// Images and tags are pulled as aggregate subqueries rather than LEFT JOIN +
// GROUP BY. Joining two one-to-many relations in the same query multiplies
// their rows together — an item with 2 images and 3 tags would aggregate 6
// rows, silently repeating every image three times. Subqueries keep each
// aggregate independent and drop the GROUP BY entirely. `jsonArrayFrom` emits
// `coalesce(json_agg(agg), '[]')`, which is what these hand-wrote before.
import { ExpressionBuilder } from 'kysely';
import { jsonArrayFrom } from 'kysely/helpers/postgres';
import { db } from './db';
import { DB } from './db-kysely/schema';
import { ItemStatus, ItemImage, ItemTag } from './types';
/**
* The aliases every item query and every filter clause is written against.
*
* `i` and `c` are kept from the SQL these replaced. Not because short names are
* better, but because the filter clauses, the subquery correlations and the
* ORDER BY all reference them, and renaming them in the same change that moved
* the builder would have made the diff unreadable against the SQL it replaces.
*/
export type ItemContext = ExpressionBuilder<
DB & { i: DB['items']; c: DB['categories'] },
'i' | 'c'
>;
/** The public image fields. Correlated to the outer item by `whereRef`. */
function imagesFor(eb: ItemContext) {
return jsonArrayFrom(
eb
.selectFrom('item_images as img')
.select(['img.id', 'img.image_path', 'img.sort_order'])
.whereRef('img.item_id', '=', 'i.id')
.orderBy('img.sort_order')
).as('images');
}
/**
* Admin-only images, carrying `original_image_path` alongside the public
* fields — the field the inventory screen needs to know whether a photo has a
* cut-out to restore (#293).
*
* A separate function rather than a flag on `imagesFor`, for the same reason
* `publicItemQuery` names its columns instead of taking them all: an original
* filename is internal, nobody's business on the storefront, and a boolean in
* the middle of the thing that keeps it off the public API is one edit away
* from being passed wrongly.
*/
function adminImagesFor(eb: ItemContext) {
return jsonArrayFrom(
eb
.selectFrom('item_images as img')
.select(['img.id', 'img.image_path', 'img.sort_order', 'img.original_image_path'])
.whereRef('img.item_id', '=', 'i.id')
.orderBy('img.sort_order')
).as('images');
}
function tagsFor(eb: ItemContext) {
return jsonArrayFrom(
eb
.selectFrom('item_tags as it')
.innerJoin('tags as t', 't.id', 'it.tag_id')
.select(['t.id', 't.name', 't.color'])
.whereRef('it.item_id', '=', 'i.id')
.orderBy('t.name')
).as('tags');
}
/**
* The storefront's projection — an explicit column list, because it has no
* business seeing paypal_order_id or reserved_until.
*
* A function rather than a constant so each caller gets a fresh builder. Kysely
* builders are immutable, so sharing one would be safe, but a function makes it
* obvious that adding a `where` does not affect anyone else.
*/
export function publicItemQuery() {
return db
.selectFrom('items as i')
.leftJoin('categories as c', 'c.id', 'i.category_id')
.select([
'i.id',
'i.name',
'i.description',
'i.price_cents',
'i.status',
'i.created_at',
'i.category_id',
'c.name as category_name'
])
.select(imagesFor)
.select(tagsFor);
}
/** The admin projection — every item column, plus the admin image fields. */
export function adminItemQuery() {
return db
.selectFrom('items as i')
.leftJoin('categories as c', 'c.id', 'i.category_id')
.selectAll('i')
.select('c.name as category_name')
.select(adminImagesFor)
.select(tagsFor);
}
If ItemStatus, ItemImage or ItemTag end up unused by the file after this, leave the import of whichever the interfaces below still use and drop only the genuinely unused ones — npm run lint will say which.
- Step 2: Verify the projections compile and match
cd backend
export PATH="/c/Users/tlamb/AppData/Local/nvm/v20.20.2:$PATH"
npm run build 2>&1 | grep -E "itemSelect|error TS" | head
Expected at this point: errors only in admin.ts, items.ts and itemFilters.ts, which still reference the deleted constants. Errors inside itemSelect.ts itself mean the projection does not type-check against the generated schema — stop and report which column.
- Step 3: Convert the filter builder
In backend/src/itemFilters.ts: delete the BuiltFilter interface, and replace the whole of buildItemFilterSql — its long comment block included — with the following. Every clause keeps its own comment; those record decisions, not descriptions.
// Composes the filter clauses as Kysely expressions.
//
// This returned `{ clauses: string[]; params: unknown[] }` until #308, and both
// callers spliced the clauses straight into query text. The invariant that made
// that safe — only a placeholder index may ever be interpolated into a clause,
// never a value — was a sixteen-line comment and two tests standing between an
// edit and a live injection on a route reachable without signing in.
//
// It is now a property of the type system. `${value}` inside a Kysely `sql`
// template emits a bind parameter, never text, and the builder expressions
// cannot express interpolation at all. The two tests at the bottom of
// itemFilters.test.ts still exist and now assert against the SQL Kysely
// actually emits, which is a stronger claim than the one they used to make.
//
// `startIndex` is gone with the splicing it existed for.
//
// `favoritesCustomerId` is required rather than optional so a caller has to say
// whose favorites it means, even when it means nobody's. Both routes already
// reject a favorites filter they cannot satisfy, so reaching the throw below is
// a programming error — but it is here so that a future caller which forgets
// the guard fails loudly instead of quietly ignoring the filter and listing the
// whole catalogue.
export function itemFilterExpressions(
eb: ItemContext,
filters: ItemFilters,
favoritesCustomerId: number | null
): Expression<SqlBool>[] {
const clauses: Expression<SqlBool>[] = [];
if (filters.categoryIds.length) {
// Selecting a category means "and everything filed beneath it", so walk the
// tree down from each chosen node. A recursive CTE keeps the tree
// un-denormalized: reparenting stays a single UPDATE with no stored paths
// to rewrite.
//
// Seeded with `= ANY(...)` rather than one id, so every selected root is
// walked in the same recursion. That also gives the OR for free: the union
// of the subtrees is exactly "filed under any of these", and an item filed
// under two selected branches appears once because IN is a set test.
//
// Still a `sql` template, because the builder expresses a recursive CTE no
// better than this does. `${filters.categoryIds}` is one bind parameter
// holding the whole array — not a placeholder list — which is why no
// sql.param() ceremony appears here. See src/db-kysely/CONVENTIONS.md.
clauses.push(sql<SqlBool>`i.category_id IN (
WITH RECURSIVE subtree AS (
SELECT id FROM categories WHERE id = ANY(${filters.categoryIds}::int[])
UNION ALL
SELECT c.id FROM categories c JOIN subtree s ON c.parent_id = s.id
)
SELECT id FROM subtree
)`);
}
if (filters.tagIds.length) {
// AND, not OR: the item must carry every selected tag. Matching with
// `tag_id = ANY(...)` alone would return items holding just one of them, so
// the count of matched rows has to equal the number requested.
clauses.push(sql<SqlBool>`(SELECT COUNT(*) FROM item_tags it
WHERE it.item_id = i.id AND it.tag_id = ANY(${filters.tagIds}::int[])) = ${filters.tagIds.length}`);
}
if (filters.minPriceCents !== null) {
clauses.push(eb('i.price_cents', '>=', filters.minPriceCents));
}
if (filters.maxPriceCents !== null) {
clauses.push(eb('i.price_cents', '<=', filters.maxPriceCents));
}
if (filters.status !== null) {
// `in` replaces the `= ANY($n::text[])` this used to build. Kysely emits
// the placeholder list itself, so one status and several use the same
// expression and the explicit ::text[] cast is no longer needed.
clauses.push(eb('i.status', 'in', filters.status));
}
if (filters.favoritesOnly) {
if (favoritesCustomerId === null) {
throw new Error('favorites filter requires a customer id');
}
// EXISTS rather than a join: an item is favorited by a customer at most
// once, but joining would still risk multiplying rows if that ever changed,
// and this reads as the membership test it is.
clauses.push(
eb.exists(
eb
.selectFrom('favorites as f')
.select('f.item_id')
.whereRef('f.item_id', '=', 'i.id')
.where('f.customer_id', '=', favoritesCustomerId)
)
);
}
return clauses;
}
Add to that file's imports:
import { Expression, SqlBool, sql } from 'kysely';
import { ItemContext } from './itemSelect';
- Step 4: Convert the admin route
In backend/src/routes/admin.ts, change the import on line 4 from ADMIN_ITEM_SELECT, ADMIN_ITEM_BY_ID to adminItemQuery, keeping AdminItemRow and ItemRecord, and change buildItemFilterSql to itemFilterExpressions in the itemFilters import.
Replace the whole S2077 comment block and the three lines after it (the buildItemFilterSql call, the where assembly, and the pool.query) with:
// No interpolation, and nothing to argue about. Until #308 this assembled
// `${ADMIN_ITEM_SELECT} ${where}` from clauses composed at run time, and
// sixteen lines in itemFilters.ts explained why that was safe. The clauses
// are Kysely expressions now: a value cannot reach the SQL text, because the
// types do not let it.
const rows: AdminItemRow[] = await adminItemQuery()
.where((eb) => eb.and(itemFilterExpressions(eb, filters, null)))
.orderBy('i.created_at', 'desc')
.execute();
res.json(rows);
Then replace both by-id reads. At the two places currently reading pool.query<AdminItemRow>(ADMIN_ITEM_BY_ID, [item.id]) and pool.query<AdminItemRow>(ADMIN_ITEM_BY_ID, [itemId]), the surrounding code destructures { rows: full } and uses full[0]. Replace each with a single-row read, keeping the surrounding logic:
const full = await adminItemQuery().where('i.id', '=', item.id).execute();
and
const full = await adminItemQuery().where('i.id', '=', itemId).execute();
full is now the array directly rather than { rows }, so remove the destructuring at both sites and leave every use of full[0] as it is.
- Step 5: Convert the storefront route
In backend/src/routes/items.ts, change the import to publicItemQuery (keeping PublicItemRow) and buildItemFilterSql to itemFilterExpressions.
EXCLUDE_PENDING and PUBLIC_ITEM_BY_ID both go. Replace them with:
/**
* Pending items are excluded everywhere, not only from the list. A pending item
* that stayed fetchable by id would be hidden from the catalogue and still
* reachable by anyone who guessed or kept a link.
*
* An expression rather than the SQL literal this was until #308, so it composes
* with the filter clauses through `eb.and` instead of being joined into a
* string. That join used to need its own argument about why AND could not
* weaken it; `and` cannot re-associate anything.
*/
function notPending(eb: ItemContext) {
return eb('i.status', '!=', 'pending');
}
with ItemContext added to the itemSelect import.
Replace the buildItemFilterSql call, the S2077 comment, the where assembly and the pool.query with:
const rows: PublicItemRow[] = await publicItemQuery()
.where((eb) =>
eb.and([
notPending(eb),
...itemFilterExpressions(eb, effectiveFilters, req.customerId ?? null)
])
)
.orderBy('i.created_at', 'desc')
.execute();
res.json(rows);
And replace the by-id read:
const rows = await publicItemQuery()
.where('i.id', '=', Number(req.params.id))
.where((eb) => notPending(eb))
.execute();
if (!rows.length) return res.status(404).json({ error: 'not found' });
res.json(rows[0]);
Number(req.params.id) rather than the raw string, because the column is an integer and Kysely types it that way. A non-numeric id becomes NaN, which matches no row and yields the same 404 the old query gave — verify that in Step 7.
- Step 6: Rewrite the two filter test blocks
In backend/tests/unit/itemFilters.test.ts, leave the entire describe('parseItemFilters', ...) block untouched. Replace the describe('buildItemFilterSql', ...) block and the describe('buildItemFilterSql keeps every value out of the SQL text', ...) block with the following, and add these imports at the top:
import { db } from '../../src/db';
import { itemFilterExpressions, ItemFilters } from '../../src/itemFilters';
ItemFilters is already exported from itemFilters.ts; check the name against the file and use whatever it actually exports for the parsed-filters shape.
/**
* Compiles the filter clauses on their own, with no projection around them.
*
* The expressions are what this file is about, and Kysely compiles without a
* connection — so these assert on the SQL and parameters actually emitted,
* rather than on the intermediate strings the old builder returned. That is a
* stronger claim than the one these tests used to make.
*/
function compileFilters(filters: ItemFilters, customerId: number | null = null) {
// The same `items as i` + `categories as c` shape both real queries use, so
// the expression builder handed to the callback is exactly the ItemContext
// the filters are written against. Building a narrower query here would need
// a cast, and a cast in the test would be testing the cast.
const { sql, parameters } = db
.selectFrom('items as i')
.leftJoin('categories as c', 'c.id', 'i.category_id')
.select('i.id')
.where((eb) => eb.and(itemFilterExpressions(eb, filters, customerId)))
.compile();
return { sql, parameters: [...parameters] };
}
const NO_FILTERS = {
categoryIds: [],
tagIds: [],
minPriceCents: null,
maxPriceCents: null,
status: null,
favoritesOnly: false
};
describe('itemFilterExpressions', () => {
it('adds no condition when nothing is filtered', () => {
const { sql, parameters } = compileFilters(NO_FILTERS);
// `select ... from` with no `where` at all — Kysely emits nothing for an
// empty `and`, matching the old `clauses.length ? ... : ''`.
expect(sql).not.toContain('where');
expect(parameters).toEqual([]);
});
it('matches a category and all of its descendants', () => {
const { sql, parameters } = compileFilters({ ...NO_FILTERS, categoryIds: [4] });
expect(sql).toContain('WITH RECURSIVE subtree');
expect(parameters).toEqual([[4]]);
});
// One bind parameter holding the whole array, not a placeholder list. This is
// the property that made the array trap in the previous builder impossible
// here — see #297 and src/db-kysely/CONVENTIONS.md.
it('seeds the descendant walk with every selected category, as one parameter', () => {
const { parameters } = compileFilters({ ...NO_FILTERS, categoryIds: [4, 9] });
expect(parameters).toEqual([[4, 9]]);
});
it('requires every listed tag rather than any of them', () => {
const { sql, parameters } = compileFilters({ ...NO_FILTERS, tagIds: [2, 5] });
expect(sql).toContain('SELECT COUNT(*) FROM item_tags');
expect(parameters).toEqual([[2, 5], 2]);
});
it('filters on a price range', () => {
const { parameters } = compileFilters({
...NO_FILTERS,
minPriceCents: 1000,
maxPriceCents: 5000
});
expect(parameters).toEqual([1000, 5000]);
});
it('filters on several statuses with one expression', () => {
const { sql, parameters } = compileFilters({
...NO_FILTERS,
status: ['available', 'reserved']
});
expect(sql).toContain('"i"."status" in');
expect(parameters).toEqual(['available', 'reserved']);
});
it('restricts to the favorites of the given customer', () => {
const { sql, parameters } = compileFilters({ ...NO_FILTERS, favoritesOnly: true }, 7);
expect(sql).toContain('exists');
expect(parameters).toEqual([7]);
});
it('does not restrict to favorites when the flag is off, even given a customer', () => {
const { sql, parameters } = compileFilters(NO_FILTERS, 7);
expect(sql).not.toContain('exists');
expect(parameters).toEqual([]);
});
it('throws rather than ignore a favorites filter with no customer', () => {
expect(() => compileFilters({ ...NO_FILTERS, favoritesOnly: true }, null)).toThrow(
/favorites filter requires a customer id/
);
});
it('composes several filters together', () => {
const { parameters } = compileFilters(
{
categoryIds: [4],
tagIds: [2],
minPriceCents: 1000,
maxPriceCents: null,
status: ['available'],
favoritesOnly: true
},
7
);
expect(parameters).toEqual([[4], [2], 1, 1000, 'available', 7]);
});
});
// The invariant, and it is load-bearing: the storefront call site is reachable
// without signing in, so a filter value reaching the SQL text is SQL injection
// rather than a style problem. These two made that fail a build rather than
// relying on someone reading a comment, and they still do — but they now check
// the SQL Kysely actually emits rather than the strings the old builder
// returned. See #202, #180 for the S2077 review, and #308 for the conversion.
describe('itemFilterExpressions keeps every value out of the SQL text', () => {
// Built by hand rather than through parseItemFilters, because the claim is
// that the expressions are safe with no parser at all. These values could
// never survive parsing, which is the point: the parser is defence in depth,
// not the reason this holds.
const HOSTILE = "1); DROP TABLE items; --";
it('never lets a filter value reach the SQL, even one the parser would reject', () => {
const { sql, parameters } = compileFilters(
{
categoryIds: [HOSTILE],
tagIds: [HOSTILE],
minPriceCents: HOSTILE,
maxPriceCents: HOSTILE,
status: [HOSTILE],
favoritesOnly: true
} as unknown as ItemFilters,
HOSTILE as unknown as number
);
expect(sql).not.toContain('DROP TABLE');
expect(JSON.stringify(parameters)).toContain('DROP TABLE');
});
it('produces byte-identical SQL for two completely different filter sets', () => {
const first = compileFilters(
{
categoryIds: [1],
tagIds: [2],
minPriceCents: 3,
maxPriceCents: 4,
status: ['available'],
favoritesOnly: true
},
5
);
const second = compileFilters(
{
categoryIds: [99],
tagIds: [98],
minPriceCents: 97,
maxPriceCents: 96,
status: ['sold'],
favoritesOnly: true
},
95
);
expect(first.sql).toBe(second.sql);
expect(first.parameters).not.toEqual(second.parameters);
});
});
- Step 7: Run everything
cd backend
export PATH="/c/Users/tlamb/AppData/Local/nvm/v20.20.2:$PATH"
npm run build
npm run lint
npx jest -c jest.unit.config.js
npm run db:test:up
npx jest -c jest.integration.config.js --runInBand
Expected: build clean, lint 0 errors, unit passing, integration 445/445 with no test file edited. The integration suite is the contract: every filter test, the sold-filter suite, the favorites suite and the pending-status suite must pass exactly as written. If any integration test needs editing to pass, the conversion changed behaviour — fix the query, not the test, and report what differed.
Pay particular attention to pendingStatus.integration.test.ts and any test fetching an item by a non-numeric id, since Step 5 changed that path from a string comparison to Number(...).
- Step 8: Confirm the interpolation is actually gone
cd backend
grep -n "ITEM_SELECT\|ITEM_BY_ID\|buildItemFilterSql\|BuiltFilter" src/ tests/ -r
Expected: no output. Every one of those names is deleted by this task; a survivor means a call site was missed.
- Step 9: Commit
git add -A backend
git commit -F- <<'EOF'
refactor(db): build the item queries through Kysely (#308)
The two queries the builder was ever wanted for. #294 removed interpolation from seven sites by hoisting each fixed-shape query into a named constant; these two genuinely composed their WHERE at run time and could not be fixed that way, which is why they are the last S2077 hotspots. They were safe, and itemFilters.ts spent sixteen lines explaining why — that the clause fragments are literals, that the only things interpolated into them are placeholder indices, and that every value goes onto params. That argument was correct and it was still an argument, guarded by a comment and two tests, on a route reachable without signing in.
All four call sites moved rather than only the two flagged ones. The by-id constants carried no hotspot, but they were built by interpolating the same projection strings the list queries used, so converting only the list queries would have left itemSelect.ts holding a Kysely builder and a raw string that had to produce an identical projection — two spellings to keep in step by hand where the file's own header already warned about one.
The second thing this buys may matter more than the first. pool.query<T> asserts a shape TypeScript never checks against the SQL, which is why that header said the selects and their row types are kept in step by hand and the integration suite was the only thing that caught a drop. The projections are built with jsonArrayFrom now, which emits the same coalesce(json_agg(agg), '[]') they hand-wrote, so the row type follows from the projection and a dropped column is a compile error.
The row types stay hand-written and exported rather than being inferred from the query. Inference would be tidier and is refused deliberately: these are the shape the frontend reads, and a type that silently becomes whatever the query happens to return is a contract that can change without anyone deciding to change it.
The two invariant tests survive and got stronger. They used to inspect the clause strings the builder returned; they now compile the expressions and assert on the SQL Kysely actually emits, with the hostile value appearing in the parameters and nowhere else — the same claim, tested against the real artefact instead of an intermediate one.
Closes #308
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EOF
Self-Review
Spec coverage:
| Spec requirement | Step |
|---|---|
| All four call sites convert | 4, 5 |
Filter builder returns expressions, startIndex gone |
3 |
Aggregates become jsonArrayFrom |
1 |
| Row types kept, hand-written, assigned | 1, 4, 5 |
i / c aliases kept |
1 |
| Admin images stay separate from public | 1 |
| The six clauses keep their comments | 3 |
favoritesCustomerId keeps its throw |
3 |
No-filters case emits no where |
6 (first test) |
| Existing integration tests pass unedited | 7 |
| The two invariant tests re-pointed, not deleted | 6 |
parseItemFilters untouched |
6 (explicitly) |
No gaps.
Placeholder scan: none. Every code step carries literal code; every command step carries the command and its expected output.
Type consistency: ItemContext is defined once in itemSelect.ts (Step 1) and imported by itemFilters.ts (Step 3), items.ts (Step 5) and the test (Step 6). itemFilterExpressions(eb, filters, favoritesCustomerId) has that argument order at its definition and at all three call sites. adminItemQuery() and publicItemQuery() are functions, called with () everywhere. AdminItemRow and PublicItemRow keep their existing names and are the annotation on the two list results.
One thing the implementer must not paper over: Step 1's row types and Step 4/5's AdminItemRow[] / PublicItemRow[] annotations are the whole point of the change. If the assignment fails to type-check, that is information — the projection and the contract disagree. The spec permits $castTo as a fallback only where the types genuinely differ (a json_agg timestamp arriving as a string is the expected case), and requires saying which column disagreed. Silently widening a row type to any would remove the only thing this change adds over the old code.