From 7d4948d2117649fb4c0cb802130c51d08fbcdafd Mon Sep 17 00:00:00 2001 From: Sutu Sebastian Date: Mon, 10 Aug 2026 23:35:26 +0300 Subject: [PATCH 1/3] fix: bind recipe boolean params as SQLite 0/1 better-sqlite3 rejects JS booleans; coerce type:boolean params to integers at resolve time so defaulted recipes (e.g. churn-complexity-hotspots) run on Node. Narrow QueryBindValue; add CI Node smoke. --- .changeset/boolean-sqlite-bind.md | 5 + .github/workflows/ci.yml | 8 + docs/plans/boolean-sqlite-bind.md | 141 ++++++++++++++++++ scripts/agent-eval/probe-tokens.ts | 4 +- scripts/agent-eval/run-probes.ts | 4 +- scripts/query-golden/resolve-golden-query.ts | 4 +- .../apply-command-template.test.ts | 5 +- src/application/query-engine.ts | 11 +- src/application/recipe-params.test.ts | 31 +++- src/application/recipe-params.ts | 42 ++++-- src/application/tool-handlers.ts | 7 +- src/cli/cmd-query.test.ts | 2 +- src/cli/cmd-query.ts | 12 +- 13 files changed, 234 insertions(+), 42 deletions(-) create mode 100644 .changeset/boolean-sqlite-bind.md create mode 100644 docs/plans/boolean-sqlite-bind.md diff --git a/.changeset/boolean-sqlite-bind.md b/.changeset/boolean-sqlite-bind.md new file mode 100644 index 00000000..b00cafa3 --- /dev/null +++ b/.changeset/boolean-sqlite-bind.md @@ -0,0 +1,5 @@ +--- +"@stainless-code/codemap": patch +--- + +Fix recipe `boolean` params failing at SQLite bind time on Node (`better-sqlite3`). Values now bind as `0` / `1`, so recipes like `churn-complexity-hotspots` and `stale-imports` run with defaults. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 42c6aef9..5644d7f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -153,6 +153,14 @@ jobs: timeout 45 node dist/index.mjs --full node dist/index.mjs query "SELECT COUNT(*) AS files FROM files" + # Boolean recipe params must bind as 0/1 β€” better-sqlite3 rejects JS booleans. + # Bun dogfood can mask this; empty rows are fine (bind must not throw). + - name: Node smoke (boolean recipe params bind) + run: | + set -euo pipefail + export CODEMAP_ROOT="$GITHUB_WORKSPACE/fixtures/minimal" + node dist/index.mjs query --json --recipe churn-complexity-hotspots + check-pack: name: πŸ“¦ Pack validation needs: skip-ci diff --git a/docs/plans/boolean-sqlite-bind.md b/docs/plans/boolean-sqlite-bind.md new file mode 100644 index 00000000..69f5a9f1 --- /dev/null +++ b/docs/plans/boolean-sqlite-bind.md @@ -0,0 +1,141 @@ +# Recipe boolean β†’ SQLite bind β€” plan + +> **Status:** open Β· **Priority:** P0 (shipped recipes broken on Node / better-sqlite3) Β· **Effort:** S (~1 tracer slice) +> +> **Motivator:** Bundled recipes with `type: boolean` params fail at bind time with `SQLite3 can only bind numbers, strings, bigints, buffers, and null`. Confirmed on `@stainless-code/codemap@0.11.4` (Node / better-sqlite3) via `churn-complexity-hotspots` / `stale-imports` β€” defaults alone suffice. Found during an external repo survey. +> +> **Grilled:** 2026-08-10 β€” decisions below are locked. Delete this plan on merge (no architecture lift unless maintainers want a durable bind note). + +--- + +## Agent start here + +**Branch from default (`main`), not** `chore/blume-*`. **PR-only** (no GitHub issue). Changeset + tests required. + +### Key touchpoints + +| Area | Path | +| ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | +| Coerce | `src/application/recipe-params.ts` (`coerceParamValue` boolean arm) | +| Unit tests | `src/application/recipe-params.test.ts` | +| Bind types | `src/application/query-engine.ts` (`QueryBindValue`) vs `src/sqlite-db.ts` (`BindValues`) | +| Bind call sites | `query-engine` `executeQueryOnDb`; `index-engine` `queryRows` / `printQueryResult`; `query-baseline` β€” **do not** add a second coerce here | +| Action templates | `apply-command-template.ts` + `cmd-query.test.ts` / `apply-command-template.test.ts` β€” expect `0`/`1` after coerce | +| Affected recipes | `templates/recipes/{churn-complexity-hotspots,stale-imports,migrate-deprecated,rename-preview}.{md,sql}` β€” SQL already uses `= 0` / `!= 0` | +| CI gate | `.github/workflows/ci.yml` Node smoke β€” extend after minimal index | + +### Architecture (bug path) + +```text +CLI/MCP/HTTP params + β†’ resolveRecipeParams / coerceParamValue + boolean arm β†’ 1 | 0 (numbers) ← fix here + β†’ values[] β†’ db.query(sql).all(...values) + better-sqlite3 accepts numbers +SQL recipes already: … = 0 / != 0 +``` + +### Tracer bullet + +1. Red: unit test that resolved boolean binds are `0`/`1`. +2. Green: coerce boolean β†’ `1`/`0` in `coerceParamValue`; update expectations + action-template pins. +3. Verify: unit tests + CI Node smoke running one boolean-default recipe. + +### Out of scope + +- Symbol-table `bindings` / `bindings-engine` (unrelated domain noun). +- MCP `NODE_MODULE_VERSION` mismatch (separate Core bug if filed). +- Merchant-dashboard survey followups / substrate work. +- Recipe SQL rewrites (already integer-shaped). +- New schema / `SCHEMA_VERSION` bump. +- Agent-content / skill narrative edits. +- Display map for pretty `true`/`false` in action templates. +- Defensive coerce at `queryRows` / `executeQueryOnDb`. +- GitHub Core bug issue (PR is the tracker). + +### Verification + +```bash +bun test src/application/recipe-params.test.ts +bun test src/application/apply-command-template.test.ts +bun test src/cli/cmd-query.test.ts +# local Node path (mirrors CI): +node dist/index.mjs query --json --recipe churn-complexity-hotspots --root fixtures/minimal +# (after build + index of fixtures/minimal; empty rows OK β€” bind must not throw) +``` + +--- + +## Pre-locked decisions (grilled) + +| # | Decision | Source | +| ---- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------ | +| L.1 | **Coerce at resolve time** β€” `coerceParamValue` for `type: "boolean"` returns `1` \| `0`, never JS `boolean`. Input still accepts `true`/`false`/`1`/`0` (string or number). | Grill Β· SQLite / better-sqlite3 bind types | +| L.2 | **One `values[]`** β€” no display map. Action `{{param}}` / `formatParamsCli` render `0`/`1`. Update tests that pinned `true`/`false`. | Grill | +| L.3 | **Narrow `QueryBindValue`** β€” drop `boolean` (align with `BindValues` + `bigint` if still needed). | Type matches runtime | +| L.4 | **`RecipeParamValue` still accepts input `boolean`** β€” MCP `z.boolean()` / CLI; resolved boolean-param slots are numbers. | Call surface unchanged | +| L.5 | **No recipe SQL / frontmatter type changes** β€” `type: boolean` stays. | Predicate-as-API | +| L.6 | **Changeset (patch)** β€” user-visible: boolean recipe params no longer throw at bind. | Consumer surface | +| L.7 | **No second coerce at DB edge** β€” resolver is the single contract. | Grill | +| L.8 | **PR-only** β€” no Core bug issue. | Grill | +| L.9 | **No agent-content edits** β€” caller types remain `string \| number \| boolean`; wire `0`/`1` is impl detail. | Grill Β· docs Rule 10 | +| L.10 | **Regression gate** β€” unit contract **and** CI Node smoke runs one boolean-default recipe (prefer `churn-complexity-hotspots` after existing `fixtures/minimal` Node full index; empty rows OK). Bun-only recipe tests are not sufficient. | Grill | + +Inspiration cite: [SQLite bind API](https://www.sqlite.org/c3ref/bind_blob.html) + better-sqlite3 accepted types β€” not peer indexers. + +--- + +## Implementation slices (tracer bullets) + +### Slice 1 β€” coerce + unit contract (must ship) + +1. Change boolean arm in `coerceParamValue` to return `1` / `0`. +2. Update `recipe-params.test.ts` expectations (`false` β†’ `0`, `true` β†’ `1`). +3. Assert resolved values never include `typeof === "boolean"`. +4. Narrow `QueryBindValue` (drop `boolean`); fix type fallout. +5. Update action-template / cmd-query tests that pin `include_type_only=true|false` β†’ `0|1`. + +**Done when:** unit tests green; `resolveRecipeParams` never puts a boolean in `values[]`. + +### Slice 2 β€” CI Node smoke (same PR) + +1. In `.github/workflows/ci.yml`, after Node full index of `fixtures/minimal`, run e.g. + `node dist/index.mjs query --json --recipe churn-complexity-hotspots` + (with `CODEMAP_ROOT` / `--root` as the existing step). Exit 0 is enough; do not require non-empty rows. +2. Optionally also `stale-imports` if cheap; one recipe is the locked minimum. + +**Done when:** CI Node job would have failed on 0.11.4’s boolean bind error. + +### Slice 3 β€” ship hygiene + +1. Changeset (patch). +2. On merge: delete this plan; **no** architecture/glossary lift unless a durable β€œboolean params bind as INTEGER 0/1” note is wanted (default: lift nowhere β€” bug fix). + +--- + +## Acceptance + +- [ ] `coerceParamValue` / `resolveRecipeParams` emit `0`/`1` for boolean params (all input spellings) +- [ ] `QueryBindValue` has no `boolean` +- [ ] Action template tests updated for `0`/`1` +- [ ] CI Node smoke runs a boolean-default recipe against `fixtures/minimal` via `node dist/index.mjs` +- [ ] Changeset present; no schema bump; no agent-content edit; no GitHub issue required + +--- + +## Risks / non-goals + +| Risk | Mitigation | +| --------------------------------------- | ----------------------------------------------- | +| Bun dogfood hides the bug | L.10 CI Node smoke | +| Action templates change `false` β†’ `0` | CLI accepts both; update pinned tests | +| Future path feeds raw boolean to `.all` | L.3 type narrowing; no silent edge coerce (L.7) | + +**Non-goals:** display map; bind-boundary coerce; recipe YAML/SQL rewrites; MCP native-module ABI; agent-content; filing a Core bug issue. + +--- + +## Dependencies + +- Runtime split: [packaging.md Β§ Node vs Bun](../packaging.md#node-vs-bun) +- Tenet: predicate-as-API β€” recipes must run; bind wire format is an implementation detail diff --git a/scripts/agent-eval/probe-tokens.ts b/scripts/agent-eval/probe-tokens.ts index 523299b2..dc009ea6 100644 --- a/scripts/agent-eval/probe-tokens.ts +++ b/scripts/agent-eval/probe-tokens.ts @@ -1,4 +1,4 @@ -import type { RecipeParamValue } from "../../src/application/recipe-params"; +import type { ResolvedRecipeParamValue } from "../../src/application/recipe-params"; import { estimateTokens, jsonCharLength } from "./metrics"; /** Probe-mode token budget: prompt + payload chars, then chars/4 (plan L.4). */ @@ -12,7 +12,7 @@ export function estimateProbeTokens( export function mcpOnPayloadChars( sql: string, rows: unknown[], - bindValues: RecipeParamValue[] = [], + bindValues: ResolvedRecipeParamValue[] = [], ): number { return ( Buffer.byteLength(sql, "utf-8") + diff --git a/scripts/agent-eval/run-probes.ts b/scripts/agent-eval/run-probes.ts index d06c9004..0277c46c 100644 --- a/scripts/agent-eval/run-probes.ts +++ b/scripts/agent-eval/run-probes.ts @@ -4,7 +4,7 @@ import { fileURLToPath } from "node:url"; import { createCodemap } from "../../src/api"; import { queryRows } from "../../src/application/index-engine"; -import type { RecipeParamValue } from "../../src/application/recipe-params"; +import type { ResolvedRecipeParamValue } from "../../src/application/recipe-params"; import { resolveCodemapConfig } from "../../src/config"; import { resolveGoldenQuery } from "../query-golden/resolve-golden-query"; import { runGoldenSetup } from "../query-golden/run-setup"; @@ -154,7 +154,7 @@ function parseArgs(argv: string[]) { function runMcpOnArm( prompt: string, sql: string, - bindValues: RecipeParamValue[], + bindValues: ResolvedRecipeParamValue[], ): ArmRunMetrics { const t0 = performance.now(); const rows = queryRows(sql, bindValues) as unknown[]; diff --git a/scripts/query-golden/resolve-golden-query.ts b/scripts/query-golden/resolve-golden-query.ts index 5215c513..733d41d2 100644 --- a/scripts/query-golden/resolve-golden-query.ts +++ b/scripts/query-golden/resolve-golden-query.ts @@ -3,12 +3,12 @@ import { getQueryRecipeSql, } from "../../src/application/query-recipes"; import { resolveRecipeParams } from "../../src/application/recipe-params"; -import type { RecipeParamValue } from "../../src/application/recipe-params"; +import type { ResolvedRecipeParamValue } from "../../src/application/recipe-params"; import type { GoldenScenario } from "./schema"; export function resolveGoldenQuery(s: GoldenScenario): { sql: string; - bindValues: RecipeParamValue[]; + bindValues: ResolvedRecipeParamValue[]; } { if (s.sql !== undefined) { if (s.params !== undefined) { diff --git a/src/application/apply-command-template.test.ts b/src/application/apply-command-template.test.ts index 7962cc04..e3b1f019 100644 --- a/src/application/apply-command-template.test.ts +++ b/src/application/apply-command-template.test.ts @@ -60,10 +60,11 @@ describe("renderRecipeActionCommands", () => { "codemap apply stale-imports --params in_file={{in_file}},include_type_only={{include_type_only}} --dry-run --force", }, ], - { in_file: "src/widget", include_type_only: true }, + // Resolved boolean params are 0/1 (see resolveRecipeParams). + { in_file: "src/widget", include_type_only: 1 }, ); expect(out?.[0]?.command).toBe( - "codemap apply stale-imports --params in_file=src/widget,include_type_only=true --dry-run --force", + "codemap apply stale-imports --params in_file=src/widget,include_type_only=1 --dry-run --force", ); }); }); diff --git a/src/application/query-engine.ts b/src/application/query-engine.ts index b1272563..c01831bb 100644 --- a/src/application/query-engine.ts +++ b/src/application/query-engine.ts @@ -11,12 +11,13 @@ import type { Bucketizer, GroupByMode } from "../group-by"; import type { CodemapDatabase } from "../sqlite-db"; /** - * SQLite bind value β€” the union accepted by `db.query(sql).all(...values)`. - * Kept here at the DB boundary so `executeQuery` doesn't depend on any - * recipe-layer type. Recipe coercion lives in `application/recipe-params.ts` - * and produces values assignable to this union. + * SQLite bind value β€” the union accepted by `db.query(sql).all(...values)` + * (better-sqlite3 / bun:sqlite). Kept here at the DB boundary so + * `executeQuery` doesn't depend on any recipe-layer type. Recipe coercion + * lives in `application/recipe-params.ts` and produces values assignable to + * this union (`type: "boolean"` params become `0` / `1`, never JS boolean). */ -export type QueryBindValue = string | number | bigint | boolean | null; +export type QueryBindValue = string | number | bigint | null; /** * Pure, transport-agnostic query execution. Mirrors the layering of diff --git a/src/application/recipe-params.test.ts b/src/application/recipe-params.test.ts index 466a479e..89d9cf93 100644 --- a/src/application/recipe-params.test.ts +++ b/src/application/recipe-params.test.ts @@ -47,7 +47,7 @@ describe("resolveRecipeParams", () => { include_tests: "false", }, }); - expect(r).toEqual({ ok: true, values: ["function", 42, false] }); + expect(r).toEqual({ ok: true, values: ["function", 42, 0] }); }); it("uses defaults for omitted optional params", () => { @@ -56,7 +56,30 @@ describe("resolveRecipeParams", () => { declared, provided: { kind: "function" }, }); - expect(r).toEqual({ ok: true, values: ["function", 80, true] }); + expect(r).toEqual({ ok: true, values: ["function", 80, 1] }); + }); + + it("never puts JS booleans in bind values", () => { + const r = resolveRecipeParams({ + recipeId: "example", + declared, + provided: { + kind: "function", + include_tests: false, + }, + }); + expect(r.ok).toBe(true); + if (!r.ok) return; + for (const v of r.values) { + expect(typeof v === "boolean").toBe(false); + expect( + typeof v === "string" || + typeof v === "number" || + typeof v === "bigint" || + v === null, + ).toBe(true); + } + expect(r.values).toEqual(["function", 80, 0]); }); it("binds omitted optional params without defaults as null", () => { @@ -97,14 +120,14 @@ describe("resolveRecipeParams", () => { declared, provided: { kind: "function", include_tests: 1 }, }); - expect(truthy).toEqual({ ok: true, values: ["function", 80, true] }); + expect(truthy).toEqual({ ok: true, values: ["function", 80, 1] }); const falsy = resolveRecipeParams({ recipeId: "example", declared, provided: { kind: "function", include_tests: 0 }, }); - expect(falsy).toEqual({ ok: true, values: ["function", 80, false] }); + expect(falsy).toEqual({ ok: true, values: ["function", 80, 0] }); }); it("rejects malformed numbers and booleans", () => { diff --git a/src/application/recipe-params.ts b/src/application/recipe-params.ts index 88154ceb..4d4be9e1 100644 --- a/src/application/recipe-params.ts +++ b/src/application/recipe-params.ts @@ -3,7 +3,7 @@ import type { RecipeParam } from "./recipes-loader"; /** Map positional bind values back to param names for action command templates. */ export function recipeParamValuesFromResolved( declared: RecipeParam[] | undefined, - values: RecipeParamValue[], + values: ResolvedRecipeParamValue[], ): RecipeParamValues { const out: RecipeParamValues = {}; for (let i = 0; i < (declared ?? []).length; i++) { @@ -15,20 +15,28 @@ export function recipeParamValuesFromResolved( } /** - * One bound parameter value. `null` is internal-only β€” callers may not pass - * `null` directly; the resolver assigns it for declared optional params that - * the caller omitted, so positional `?` placeholders stay aligned with the - * declaration order in the recipe. + * One parameter value from a caller. May include JS `boolean` for + * `type: "boolean"` params; {@link resolveRecipeParams} coerces those to + * SQLite-safe `1` / `0` in {@link ResolvedRecipeParamValue}. `null` is + * internal-only on the resolved list β€” callers may not pass `null` directly; + * the resolver assigns it for declared optional params that the caller + * omitted, so positional `?` placeholders stay aligned with declaration order. */ export type RecipeParamValue = string | number | boolean | null; +/** + * Bind-ready value after {@link resolveRecipeParams} β€” never JS `boolean` + * (better-sqlite3 rejects them). Assignable to `QueryBindValue`. + */ +export type ResolvedRecipeParamValue = string | number | null; + /** Loose `key: value` map of params provided to a recipe by the caller. */ export type RecipeParamValues = Record; /** Successful resolution; `values` are positional in declaration order. */ export interface ResolveRecipeParamsOk { ok: true; - values: RecipeParamValue[]; + values: ResolvedRecipeParamValue[]; } /** Resolution failure; `error` carries a single human-readable message. */ @@ -68,8 +76,9 @@ export function mergeParams( /** * Validate `provided` against `declared` and produce positional bind values * in declaration order. Strict on missing required, unknown keys, and type - * mismatches; coerces `string | number` into the declared `number` / - * `boolean` types where the value is unambiguous. + * mismatches; coerces `string | number` into the declared `number` type and + * `boolean` / `true`/`false` / `1`/`0` into INTEGER `1` / `0` (better-sqlite3 + * rejects JS booleans at bind time; recipe SQL compares with `= 0` / `!= 0`). */ export function resolveRecipeParams(opts: { recipeId: string; @@ -97,7 +106,7 @@ export function resolveRecipeParams(opts: { } } - const values: RecipeParamValue[] = []; + const values: ResolvedRecipeParamValue[] = []; for (const param of declared) { const raw = provided[param.name]; if (raw === undefined) { @@ -132,7 +141,7 @@ function coerceParamValue( raw: RecipeParamValue, recipeId: string, ): - | { ok: true; value: Exclude } + | { ok: true; value: Exclude } | ResolveRecipeParamsError { if (param.type === "string") { return { ok: true, value: String(raw) }; @@ -153,15 +162,16 @@ function coerceParamValue( } return { ok: true, value: n }; } - if (typeof raw === "boolean") return { ok: true, value: raw }; - // Accept numeric `1`/`0` as well as their string forms β€” MCP / HTTP callers - // hit this path because `query_recipe.params` accepts `z.number()` and the - // CLI / HTTP layers don't pre-coerce numeric booleans. + // Bind as INTEGER 0/1 β€” better-sqlite3 rejects JS booleans; recipe SQL + // already uses `= 0` / `!= 0`. Accept numeric `1`/`0` and their string + // forms β€” MCP / HTTP callers hit this path because `query_recipe.params` + // accepts `z.number()` and the CLI / HTTP layers don't pre-coerce. + if (typeof raw === "boolean") return { ok: true, value: raw ? 1 : 0 }; if (raw === "true" || raw === "1" || raw === 1) { - return { ok: true, value: true }; + return { ok: true, value: 1 }; } if (raw === "false" || raw === "0" || raw === 0) { - return { ok: true, value: false }; + return { ok: true, value: 0 }; } return { ok: false, diff --git a/src/application/tool-handlers.ts b/src/application/tool-handlers.ts index 30f74e46..0f584cf1 100644 --- a/src/application/tool-handlers.ts +++ b/src/application/tool-handlers.ts @@ -80,7 +80,10 @@ import { getQueryRecipeSql, } from "./query-recipes"; import { resolveRecipeParams } from "./recipe-params"; -import type { RecipeParamValue, RecipeParamValues } from "./recipe-params"; +import type { + RecipeParamValues, + ResolvedRecipeParamValue, +} from "./recipe-params"; import { tryRecordRecipeRun } from "./recipe-recency"; import { runCodemapIndex } from "./run-index"; import { buildShowResult, buildSnippetResult } from "./show-engine"; @@ -1357,7 +1360,7 @@ function runFormattedQuery(args: { recipeId: string | undefined; recipeActions: ReadonlyArray | undefined; changedFiles: Set | undefined; - bindValues?: RecipeParamValue[] | undefined; + bindValues?: ResolvedRecipeParamValue[] | undefined; format: | "sarif" | "annotations" diff --git a/src/cli/cmd-query.test.ts b/src/cli/cmd-query.test.ts index fa2f0793..9538ea37 100644 --- a/src/cli/cmd-query.test.ts +++ b/src/cli/cmd-query.test.ts @@ -1149,7 +1149,7 @@ describe("getQueryRecipeActionsRendered β€” auditβ†’apply pairs (C.6)", () => { in_file: "src/widget", }); expect(actions?.[0]?.command).toBe( - "codemap apply stale-imports --params in_file=src/widget,include_type_only=false --dry-run --force", + "codemap apply stale-imports --params in_file=src/widget,include_type_only=0 --dry-run --force", ); }); diff --git a/src/cli/cmd-query.ts b/src/cli/cmd-query.ts index c1796106..7b460477 100644 --- a/src/cli/cmd-query.ts +++ b/src/cli/cmd-query.ts @@ -34,8 +34,8 @@ import { resolveRecipeParams, } from "../application/recipe-params"; import type { - RecipeParamValue, RecipeParamValues, + ResolvedRecipeParamValue, } from "../application/recipe-params"; import { enrichWithRecency, @@ -1011,7 +1011,7 @@ function resolveRecipeBindValues(opts: { recipeId: string | undefined; params: RecipeParamValues | undefined; json: boolean; -}): { values: RecipeParamValue[] } | { error: true } { +}): { values: ResolvedRecipeParamValue[] } | { error: true } { if (opts.recipeId === undefined) return { values: [] }; const resolved = resolveRecipeParams({ recipeId: opts.recipeId, @@ -1112,7 +1112,7 @@ function printFormattedQuery( format: Exclude; recipeId: string | undefined; changedFiles: Set | undefined; - bindValues: RecipeParamValue[] | undefined; + bindValues: ResolvedRecipeParamValue[] | undefined; /** `--ci`: suppress no-locatable-rows warning + exit 1 on `rows.length > 0`. */ ci?: boolean; badgeStyle: BadgeStyle; @@ -1215,7 +1215,7 @@ function runGroupedQuery(opts: { groupBy: GroupByMode; changedFiles: Set | undefined; recipeActions: ReadonlyArray | undefined; - bindValues: RecipeParamValue[] | undefined; + bindValues: ResolvedRecipeParamValue[] | undefined; root: string; }) { let bucketize: Bucketizer; @@ -1308,7 +1308,7 @@ function runSaveBaseline(opts: { recipeId: string | undefined; baselineName: string; changedFiles: Set | undefined; - bindValues: RecipeParamValue[] | undefined; + bindValues: ResolvedRecipeParamValue[] | undefined; }) { let rows: unknown[]; try { @@ -1368,7 +1368,7 @@ function runBaselineDiff(opts: { baselineName: string; changedFiles: Set | undefined; recipeActions: ReadonlyArray | undefined; - bindValues: RecipeParamValue[] | undefined; + bindValues: ResolvedRecipeParamValue[] | undefined; }) { const result = compareQueryBaseline({ baselineName: opts.baselineName, From cb31d7b81a71a34fe6a8417d468f63c1209fd2ed Mon Sep 17 00:00:00 2001 From: Sutu Sebastian Date: Mon, 10 Aug 2026 23:37:38 +0300 Subject: [PATCH 2/3] harden: retire boolean-bind plan; scrub changeset --- .changeset/boolean-sqlite-bind.md | 2 +- docs/plans/boolean-sqlite-bind.md | 141 ------------------------------ 2 files changed, 1 insertion(+), 142 deletions(-) delete mode 100644 docs/plans/boolean-sqlite-bind.md diff --git a/.changeset/boolean-sqlite-bind.md b/.changeset/boolean-sqlite-bind.md index b00cafa3..84b16602 100644 --- a/.changeset/boolean-sqlite-bind.md +++ b/.changeset/boolean-sqlite-bind.md @@ -2,4 +2,4 @@ "@stainless-code/codemap": patch --- -Fix recipe `boolean` params failing at SQLite bind time on Node (`better-sqlite3`). Values now bind as `0` / `1`, so recipes like `churn-complexity-hotspots` and `stale-imports` run with defaults. +Fix recipes with `boolean` params failing on Node when run with defaults. Boolean params now bind as `0` / `1`, so recipes like `churn-complexity-hotspots` and `stale-imports` work again. diff --git a/docs/plans/boolean-sqlite-bind.md b/docs/plans/boolean-sqlite-bind.md deleted file mode 100644 index 69f5a9f1..00000000 --- a/docs/plans/boolean-sqlite-bind.md +++ /dev/null @@ -1,141 +0,0 @@ -# Recipe boolean β†’ SQLite bind β€” plan - -> **Status:** open Β· **Priority:** P0 (shipped recipes broken on Node / better-sqlite3) Β· **Effort:** S (~1 tracer slice) -> -> **Motivator:** Bundled recipes with `type: boolean` params fail at bind time with `SQLite3 can only bind numbers, strings, bigints, buffers, and null`. Confirmed on `@stainless-code/codemap@0.11.4` (Node / better-sqlite3) via `churn-complexity-hotspots` / `stale-imports` β€” defaults alone suffice. Found during an external repo survey. -> -> **Grilled:** 2026-08-10 β€” decisions below are locked. Delete this plan on merge (no architecture lift unless maintainers want a durable bind note). - ---- - -## Agent start here - -**Branch from default (`main`), not** `chore/blume-*`. **PR-only** (no GitHub issue). Changeset + tests required. - -### Key touchpoints - -| Area | Path | -| ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | -| Coerce | `src/application/recipe-params.ts` (`coerceParamValue` boolean arm) | -| Unit tests | `src/application/recipe-params.test.ts` | -| Bind types | `src/application/query-engine.ts` (`QueryBindValue`) vs `src/sqlite-db.ts` (`BindValues`) | -| Bind call sites | `query-engine` `executeQueryOnDb`; `index-engine` `queryRows` / `printQueryResult`; `query-baseline` β€” **do not** add a second coerce here | -| Action templates | `apply-command-template.ts` + `cmd-query.test.ts` / `apply-command-template.test.ts` β€” expect `0`/`1` after coerce | -| Affected recipes | `templates/recipes/{churn-complexity-hotspots,stale-imports,migrate-deprecated,rename-preview}.{md,sql}` β€” SQL already uses `= 0` / `!= 0` | -| CI gate | `.github/workflows/ci.yml` Node smoke β€” extend after minimal index | - -### Architecture (bug path) - -```text -CLI/MCP/HTTP params - β†’ resolveRecipeParams / coerceParamValue - boolean arm β†’ 1 | 0 (numbers) ← fix here - β†’ values[] β†’ db.query(sql).all(...values) - better-sqlite3 accepts numbers -SQL recipes already: … = 0 / != 0 -``` - -### Tracer bullet - -1. Red: unit test that resolved boolean binds are `0`/`1`. -2. Green: coerce boolean β†’ `1`/`0` in `coerceParamValue`; update expectations + action-template pins. -3. Verify: unit tests + CI Node smoke running one boolean-default recipe. - -### Out of scope - -- Symbol-table `bindings` / `bindings-engine` (unrelated domain noun). -- MCP `NODE_MODULE_VERSION` mismatch (separate Core bug if filed). -- Merchant-dashboard survey followups / substrate work. -- Recipe SQL rewrites (already integer-shaped). -- New schema / `SCHEMA_VERSION` bump. -- Agent-content / skill narrative edits. -- Display map for pretty `true`/`false` in action templates. -- Defensive coerce at `queryRows` / `executeQueryOnDb`. -- GitHub Core bug issue (PR is the tracker). - -### Verification - -```bash -bun test src/application/recipe-params.test.ts -bun test src/application/apply-command-template.test.ts -bun test src/cli/cmd-query.test.ts -# local Node path (mirrors CI): -node dist/index.mjs query --json --recipe churn-complexity-hotspots --root fixtures/minimal -# (after build + index of fixtures/minimal; empty rows OK β€” bind must not throw) -``` - ---- - -## Pre-locked decisions (grilled) - -| # | Decision | Source | -| ---- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------ | -| L.1 | **Coerce at resolve time** β€” `coerceParamValue` for `type: "boolean"` returns `1` \| `0`, never JS `boolean`. Input still accepts `true`/`false`/`1`/`0` (string or number). | Grill Β· SQLite / better-sqlite3 bind types | -| L.2 | **One `values[]`** β€” no display map. Action `{{param}}` / `formatParamsCli` render `0`/`1`. Update tests that pinned `true`/`false`. | Grill | -| L.3 | **Narrow `QueryBindValue`** β€” drop `boolean` (align with `BindValues` + `bigint` if still needed). | Type matches runtime | -| L.4 | **`RecipeParamValue` still accepts input `boolean`** β€” MCP `z.boolean()` / CLI; resolved boolean-param slots are numbers. | Call surface unchanged | -| L.5 | **No recipe SQL / frontmatter type changes** β€” `type: boolean` stays. | Predicate-as-API | -| L.6 | **Changeset (patch)** β€” user-visible: boolean recipe params no longer throw at bind. | Consumer surface | -| L.7 | **No second coerce at DB edge** β€” resolver is the single contract. | Grill | -| L.8 | **PR-only** β€” no Core bug issue. | Grill | -| L.9 | **No agent-content edits** β€” caller types remain `string \| number \| boolean`; wire `0`/`1` is impl detail. | Grill Β· docs Rule 10 | -| L.10 | **Regression gate** β€” unit contract **and** CI Node smoke runs one boolean-default recipe (prefer `churn-complexity-hotspots` after existing `fixtures/minimal` Node full index; empty rows OK). Bun-only recipe tests are not sufficient. | Grill | - -Inspiration cite: [SQLite bind API](https://www.sqlite.org/c3ref/bind_blob.html) + better-sqlite3 accepted types β€” not peer indexers. - ---- - -## Implementation slices (tracer bullets) - -### Slice 1 β€” coerce + unit contract (must ship) - -1. Change boolean arm in `coerceParamValue` to return `1` / `0`. -2. Update `recipe-params.test.ts` expectations (`false` β†’ `0`, `true` β†’ `1`). -3. Assert resolved values never include `typeof === "boolean"`. -4. Narrow `QueryBindValue` (drop `boolean`); fix type fallout. -5. Update action-template / cmd-query tests that pin `include_type_only=true|false` β†’ `0|1`. - -**Done when:** unit tests green; `resolveRecipeParams` never puts a boolean in `values[]`. - -### Slice 2 β€” CI Node smoke (same PR) - -1. In `.github/workflows/ci.yml`, after Node full index of `fixtures/minimal`, run e.g. - `node dist/index.mjs query --json --recipe churn-complexity-hotspots` - (with `CODEMAP_ROOT` / `--root` as the existing step). Exit 0 is enough; do not require non-empty rows. -2. Optionally also `stale-imports` if cheap; one recipe is the locked minimum. - -**Done when:** CI Node job would have failed on 0.11.4’s boolean bind error. - -### Slice 3 β€” ship hygiene - -1. Changeset (patch). -2. On merge: delete this plan; **no** architecture/glossary lift unless a durable β€œboolean params bind as INTEGER 0/1” note is wanted (default: lift nowhere β€” bug fix). - ---- - -## Acceptance - -- [ ] `coerceParamValue` / `resolveRecipeParams` emit `0`/`1` for boolean params (all input spellings) -- [ ] `QueryBindValue` has no `boolean` -- [ ] Action template tests updated for `0`/`1` -- [ ] CI Node smoke runs a boolean-default recipe against `fixtures/minimal` via `node dist/index.mjs` -- [ ] Changeset present; no schema bump; no agent-content edit; no GitHub issue required - ---- - -## Risks / non-goals - -| Risk | Mitigation | -| --------------------------------------- | ----------------------------------------------- | -| Bun dogfood hides the bug | L.10 CI Node smoke | -| Action templates change `false` β†’ `0` | CLI accepts both; update pinned tests | -| Future path feeds raw boolean to `.all` | L.3 type narrowing; no silent edge coerce (L.7) | - -**Non-goals:** display map; bind-boundary coerce; recipe YAML/SQL rewrites; MCP native-module ABI; agent-content; filing a Core bug issue. - ---- - -## Dependencies - -- Runtime split: [packaging.md Β§ Node vs Bun](../packaging.md#node-vs-bun) -- Tenet: predicate-as-API β€” recipes must run; bind wire format is an implementation detail From bb4735df34584b38a04a0b5e31c343935d9d0592 Mon Sep 17 00:00:00 2001 From: Sutu Sebastian Date: Mon, 10 Aug 2026 23:41:41 +0300 Subject: [PATCH 3/3] fix: reject explicit null in recipe param values Callers must omit optional keys; Number(null)/String(null) were silently becoming 0/"null". Keep resolver-assigned null on the bind list. --- src/application/recipe-params.test.ts | 11 +++++++++++ src/application/recipe-params.ts | 22 +++++++++++++++------- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/application/recipe-params.test.ts b/src/application/recipe-params.test.ts index 89d9cf93..55618ca1 100644 --- a/src/application/recipe-params.test.ts +++ b/src/application/recipe-params.test.ts @@ -94,6 +94,17 @@ describe("resolveRecipeParams", () => { expect(r).toEqual({ ok: true, values: ["x", null] }); }); + it("rejects explicit null from callers (omit key instead)", () => { + const r = resolveRecipeParams({ + recipeId: "example", + declared: [{ name: "min_coverage", type: "number", required: true }], + // Cast: production maps are typed without null; runtime JSON can still send it. + provided: { min_coverage: null as unknown as number }, + }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toContain("must not be null"); + }); + it("rejects missing required params", () => { const r = resolveRecipeParams({ recipeId: "example", diff --git a/src/application/recipe-params.ts b/src/application/recipe-params.ts index 4d4be9e1..2a18dea9 100644 --- a/src/application/recipe-params.ts +++ b/src/application/recipe-params.ts @@ -17,16 +17,16 @@ export function recipeParamValuesFromResolved( /** * One parameter value from a caller. May include JS `boolean` for * `type: "boolean"` params; {@link resolveRecipeParams} coerces those to - * SQLite-safe `1` / `0` in {@link ResolvedRecipeParamValue}. `null` is - * internal-only on the resolved list β€” callers may not pass `null` directly; - * the resolver assigns it for declared optional params that the caller - * omitted, so positional `?` placeholders stay aligned with declaration order. + * SQLite-safe `1` / `0` in {@link ResolvedRecipeParamValue}. Callers must + * not pass `null` β€” omit the key for optional params; the resolver assigns + * `null` on the resolved list so positional `?` placeholders stay aligned. */ -export type RecipeParamValue = string | number | boolean | null; +export type RecipeParamValue = string | number | boolean; /** * Bind-ready value after {@link resolveRecipeParams} β€” never JS `boolean` - * (better-sqlite3 rejects them). Assignable to `QueryBindValue`. + * (better-sqlite3 rejects them). `null` is resolver-only for omitted optional + * params. Assignable to `QueryBindValue`. */ export type ResolvedRecipeParamValue = string | number | null; @@ -138,11 +138,19 @@ export function resolveRecipeParams(opts: { function coerceParamValue( param: RecipeParam, - raw: RecipeParamValue, + raw: RecipeParamValue | null, recipeId: string, ): | { ok: true; value: Exclude } | ResolveRecipeParamsError { + // Runtime guard β€” `RecipeParamValue` excludes null, but `provided` maps and + // JSON/MCP edges can still deliver it; Number(null)β†’0 / String(null)β†’"null". + if (raw === null) { + return { + ok: false, + error: `${prefix(recipeId)} --params ${param.name} must not be null (omit the key for optional params).`, + }; + } if (param.type === "string") { return { ok: true, value: String(raw) }; }