refactor(db): convert the two dynamic queries to Kysely and retire the S2077 hotspots #308

New Issue

2026-09-04T16:11:56-05:00

bermudalamb commented

2026-09-04 16:11:56 -05:00

The work #305 made possible and deliberately did not do. #305 swapped the builder and reconverted one file; this converts the queries that were the reason for wanting a builder at all.

The target

Two call sites, and they are the only ones where S2077 has a real point:

backend/src/routes/admin.ts:146 — `${ADMIN_ITEM_SELECT} ${where} ORDER BY i.created_at DESC`
backend/src/routes/items.ts:95 — `${PUBLIC_ITEM_SELECT} WHERE ${where} ORDER BY i.created_at DESC`

Both interpolate a where composed at run time by buildItemFilterSql in src/itemFilters.ts. #294 removed the other seven interpolation sites by making each fixed-shape query a named constant; these two genuinely build SQL at run time and could not be fixed that way.

They are safe today, and the argument is written out at admin.ts:132-143: the clause fragments are string literals in itemFilters.ts, the only things interpolated into them are placeholder indices ($${next}), and every value goes into params. That is sound — and it is still an argument, something a reader has to follow and a future edit could quietly break. Converting turns it into a property of the type system, which is the whole point of #202 and the reason #180's hotspots have sat reviewed rather than closed.

Why this is smaller than it looks

The hard part is already written and verified. docs/superpowers/specs/2026-09-04-kysely-spike.ts expresses buildItemFilterSql in Kysely — all six optional clauses, the recursive CTE, the ANY(...::int[]) tag match with its count equality, and the array parameters — and the emitted SQL is quoted in docs/superpowers/specs/2026-09-04-kysely-vs-drizzle.md. src/db-kysely/CONVENTIONS.md carries the same example as the worked reference.

So this is not a design problem. It is the mechanical work of moving that into src/itemFilters.ts, plus the two call sites, plus proving nothing changed.

What makes it non-trivial anyway

buildItemFilterSql returns { clauses, params } and both callers splice that into their own query string. Kysely composes differently — the filters become expressions applied to a query builder, not fragments handed back as text. So the function's signature changes, and both call sites change with it. Decide the shape before converting: whether it returns an array of expressions, or takes a query builder and returns it filtered.

ADMIN_ITEM_SELECT and PUBLIC_ITEM_SELECT in src/itemSelect.ts carry image aggregates and a favourites join, and ADMIN_IMAGES_SUBQUERY deliberately exposes original_image_path to admin only. That distinction must survive.

Done when

Both routes build their query through Kysely, buildItemFilterSql no longer returns text to be interpolated, the existing filter integration tests pass unchanged, and SonarQube's S2077 hotspots on those two lines are gone rather than re-reviewed.

Not in scope

The other ~236 raw pool.query sites. They stay raw pg, file by file, and most of them have no reason to move at all — a fixed-shape query in a named constant is already safe, which is what #294 established.

Refs #305, #297, #294, #202, #180

The work #305 made possible and deliberately did not do. #305 swapped the builder and reconverted one file; this converts the queries that were the reason for wanting a builder at all. ## The target Two call sites, and they are the only ones where S2077 has a real point: - `backend/src/routes/admin.ts:146` — `` `${ADMIN_ITEM_SELECT} ${where} ORDER BY i.created_at DESC` `` - `backend/src/routes/items.ts:95` — `` `${PUBLIC_ITEM_SELECT} WHERE ${where} ORDER BY i.created_at DESC` `` Both interpolate a `where` composed at run time by `buildItemFilterSql` in `src/itemFilters.ts`. #294 removed the other seven interpolation sites by making each fixed-shape query a named constant; these two genuinely build SQL at run time and could not be fixed that way. They are safe today, and the argument is written out at `admin.ts:132-143`: the clause fragments are string literals in `itemFilters.ts`, the only things interpolated into them are placeholder *indices* (`$${next}`), and every value goes into `params`. That is sound — and it is still an argument, something a reader has to follow and a future edit could quietly break. Converting turns it into a property of the type system, which is the whole point of #202 and the reason #180's hotspots have sat reviewed rather than closed. ## Why this is smaller than it looks The hard part is already written and verified. `docs/superpowers/specs/2026-09-04-kysely-spike.ts` expresses `buildItemFilterSql` in Kysely — all six optional clauses, the recursive CTE, the `ANY(...::int[])` tag match with its count equality, and the array parameters — and the emitted SQL is quoted in `docs/superpowers/specs/2026-09-04-kysely-vs-drizzle.md`. `src/db-kysely/CONVENTIONS.md` carries the same example as the worked reference. So this is not a design problem. It is the mechanical work of moving that into `src/itemFilters.ts`, plus the two call sites, plus proving nothing changed. ## What makes it non-trivial anyway `buildItemFilterSql` returns `{ clauses, params }` and both callers splice that into their own query string. Kysely composes differently — the filters become expressions applied to a query builder, not fragments handed back as text. So the function's signature changes, and both call sites change with it. Decide the shape before converting: whether it returns an array of expressions, or takes a query builder and returns it filtered. `ADMIN_ITEM_SELECT` and `PUBLIC_ITEM_SELECT` in `src/itemSelect.ts` carry image aggregates and a favourites join, and `ADMIN_IMAGES_SUBQUERY` deliberately exposes `original_image_path` to admin only. That distinction must survive. ## Done when Both routes build their query through Kysely, `buildItemFilterSql` no longer returns text to be interpolated, the existing filter integration tests pass unchanged, and SonarQube's S2077 hotspots on those two lines are gone rather than re-reviewed. ## Not in scope The other ~236 raw `pool.query` sites. They stay raw `pg`, file by file, and most of them have no reason to move at all — a fixed-shape query in a named constant is already safe, which is what #294 established. Refs #305, #297, #294, #202, #180

bermudalamb referenced this issue

2026-09-04 17:22:23 -05:00

chore(sonar): clean up after the rotation and Kysely branches #307

bermudalamb referenced this issue from a commit

2026-09-04 17:42:12 -05:00

docs(db): design converting the two dynamic queries to Kysely (#308)

bermudalamb referenced this issue from a commit

2026-09-04 17:42:12 -05:00

docs(db): plan converting the item queries to Kysely (#308)

bermudalamb referenced this issue from a commit

2026-09-04 17:42:12 -05:00

refactor(db): build the item queries through Kysely (#308)