diff --git a/CHANGELOG.md b/CHANGELOG.md index 73432e52..cd1552cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ they care about is running `embarc-data`, which is unaffected. - **The eval harness** and the model-call trace reader. - Query leaves for the tables that went: `offerings`, `crowdfunding`, `reg-a`, `persons`. +- The `better-sqlite3` devDependency and its `trustedDependencies` entry, + unused since `@workglow/sqlite` moved to `node:sqlite`. ### Added @@ -46,6 +48,18 @@ they care about is running `embarc-data`, which is unaffected. run next. Bare `sec` runs it. - **`sec read`** — a filing as markdown, from the database or straight off a local HTML file. `--trace` replaces the `verify` group. +- **`engines: {"node": ">=24", "bun": ">=1.4.0"}`.** The floor `node:sqlite` + needs was documented and not declared, so `npm i -g @workglow/sec` on Node 22 + installed cleanly and failed later, reading as a storage bug rather than a + version error. +- **`release` derives the bump.** It cut a patch unconditionally, and on a 0.x + line the minor is the break slot — so a heading like this one would have gone + out as 0.1.6 and resolved for anyone on `^0.1.5`. It now runs `bunset --auto` + behind a shared `release-checks` gate set, which reads the bump off the + commits and off a diff of `package.json` against the last tag. The `engines` + field above is exactly what that diff is for: a runtime floor that appeared is + a break no commit message describes, and it raises this release to the minor + on its own. ### Changed @@ -67,6 +81,11 @@ they care about is running `embarc-data`, which is unaffected. - `typecheck` covers src, scripts and tests in one pass; `typecheck-tests` and `tsconfig.test.json` are gone. CI runs it before the build. - `sec fetch golden-fixtures` is now `bun run check-fixtures`. +- `isUniqueConstraintError` matches `node:sqlite`'s numeric `errcode` (2067 + UNIQUE, 1555 PRIMARY KEY). That driver reports every failure as + `code: "ERR_SQLITE_ERROR"`, so the `"SQLITE_CONSTRAINT_UNIQUE"` string — a + `better-sqlite3` spelling — never matched, leaving SQLite with the error + message as its only signal. - **`--dry-run` no longer creates the knowledge-base tables.** Those three are built lazily against the `getDb()` connection rather than through `createStorage`, so no `ReadOnlyTabularStorage` wrapper stood between a dry diff --git a/README.md b/README.md index d94bb3de..15644d05 100644 --- a/README.md +++ b/README.md @@ -123,8 +123,38 @@ bun run lint # oxlint + type-aware rules bun run format # oxfmt ``` -Node 24 and Bun 1.4+. `node:sqlite` is what backs the SQLite storage, and it is -neither stable nor unflagged below those. +Node 24 and Bun 1.4+, declared as `engines` so a lower Node fails at install +rather than later inside `node:sqlite`. `node:sqlite` is what backs the SQLite +storage, and it is neither stable nor unflagged below those. + +### The package is two binaries, not a library + +`package.json` carries `bin` and no `exports`, `main` or `types`. That is the +shape on purpose: the re-founding retired the library surface, both binaries +bundle their dependencies, and `import ... from "@workglow/sec"` is not something +this package offers. A manifest with no import entry point normally reads as a +field someone deleted by mistake, so `src/packageManifest.test.ts` asserts the +shape — including that it stays binary-only. + +### Cutting a release + +`bun run release-checks` is the gate set (format, lint, typecheck, build, packed +contents). One script runs it and then bumps: + +```sh +bun run release +``` + +The bump is derived, not chosen. `bunset --auto` reads it off the commits since +the last tag — and off a diff of `package.json` against the one at that tag, +which is what catches the break no commit message describes: a lost `exports` +subpath or `bin` entry, or an `engines` floor that appeared or moved up. On a +0.x line a break lands in the **minor**, because `^0.1.5` already admits only +`0.1.x`; a feature lands in the patch, so the two stay distinguishable in the +number. + +A break recorded *only* as a changelog heading is still invisible to it — mark +the commit (`feat!:`, or a `BREAKING CHANGE:` footer) when there is one. See `ARCHITECTURE.md` for the pipeline end to end, and `docs/fetch-and-storage.md` for the fetch layer. diff --git a/bun.lock b/bun.lock index 3f14d721..8e575d0c 100644 --- a/bun.lock +++ b/bun.lock @@ -26,8 +26,7 @@ "@types/bun": "1.4.0", "@types/pg": "^8.23.1", "@types/xml2js": "^0.4.14", - "better-sqlite3": "^13.0.3", - "bunset": "1.0.15", + "bunset": "1.1.1", "concurrently": "^10.0.5", "oxfmt": "0.66.0", "oxlint": "^1.81.0", @@ -41,7 +40,6 @@ }, }, "trustedDependencies": [ - "better-sqlite3", "onnxruntime-node", "@huggingface/transformers", ], @@ -537,8 +535,6 @@ "base64-js": ["base64-js@1.5.1", "", {}, "sha512-AKpaYlHn8t4SVbOHCy+b5+KKgvR4vrsD8vbvrbiQJps7fKDTkjkDry6ji0rUJjC0kzbNePLwzxq8iypo41qeWA=="], - "better-sqlite3": ["better-sqlite3@13.0.3", "", { "dependencies": { "node-addon-api": "^8.0.0" } }, "sha512-RbOBxmLBG8uvFUc15X9+9SFemKcQ0WBuISBVkpuiaUB2qblC8UWlHEjdWVoZ8AdhSwmoEgsiXKfopX0CQxaACQ=="], - "bignumber.js": ["bignumber.js@9.3.1", "", {}, "sha512-Ko0uX15oIUS7wJ3Rb30Fs6SkVbLmPBAKdlm7q9+ak9bbIeFf0MwuBsQV6z7+X768/cHsfg+WlysDWJcmthjsjQ=="], "body-parser": ["body-parser@2.3.0", "", { "dependencies": { "bytes": "^3.1.2", "content-type": "^2.0.0", "debug": "^4.4.3", "http-errors": "^2.0.1", "iconv-lite": "^0.7.2", "on-finished": "^2.4.1", "qs": "^6.15.2", "raw-body": "^3.0.2", "type-is": "^2.1.0" } }, "sha512-2cGmJupaNgg+QUwVLAucDuWuoMZ6EX9iHDRswZ5lsNYEmwPaRknMPCLZz07yTzVq/83p4o/wzbDZbBrTvGGTIw=="], @@ -551,7 +547,7 @@ "bun-types": ["bun-types@1.4.0", "", { "dependencies": { "@types/node": "*" } }, "sha512-iIKw23BspnQQYd3prITOBxeUsxBHnwzX6YJfGMuNOZzeNcMmVqzIIVGRm1l69ogaPQmb4wB6BN8mA5bE9YuC5Q=="], - "bunset": ["bunset@1.0.15", "", { "bin": { "bunset": "src/index.ts" } }, "sha512-hqkm9dKdf1tauHBiStnrDXiMel2EaRnNi0pR3J1sMDo/CffNkpVhjgWRMqP3k0gbaM5WEl9ZboFihWY9/JI9Og=="], + "bunset": ["bunset@1.1.1", "", { "bin": { "bunset": "src/index.ts" } }, "sha512-BiTmmA1OncSh44/9Fen6u/ZvLEVPowxFYMYrtFsmxFnYxKp9cvmDPy5HV0ZXcCM1qpKrBIW84j84jtVgkvq5RQ=="], "bytes": ["bytes@3.1.2", "", {}, "sha512-/Nf7TyzTx6S3yRJObOAV7956r8cr2+Oj8AC5dt8wSP3BQAoeX58NoHyCU8P8zGkNXStjTSi6fzO6F0pBdcYbEg=="], diff --git a/package.json b/package.json index 93480e7f..c74c193f 100644 --- a/package.json +++ b/package.json @@ -12,8 +12,8 @@ "homepage": "https://workglow.dev", "description": "A worked example of the Workglow libraries: SEC EDGAR and Form ADV ingestion, filing-to-markdown conversion, and retrieval-grounded question answering over what filings say.", "scripts": { - "bunset": "bunset --patch --push --commit --tag", - "release": "bun run format && bun run lint && bun run typecheck && bun run build && bun run prepack-check && bun run bunset", + "release-checks": "bun run format && bun run lint && bun run typecheck && bun run build && bun run prepack-check", + "release": "bun run release-checks && bunset --auto --push --commit --tag", "prepack-check": "bun ./scripts/checkPackedContents.ts", "dev": "concurrently -c 'auto' -n 'sec:' 'bun:dev-*'", "dev-sec": "bun build --watch --target=bun --sourcemap=external --packages=external --outfile ./dist/sec.js ./src/sec.ts", @@ -46,6 +46,10 @@ "dist", "!**/*.map" ], + "engines": { + "node": ">=24", + "bun": ">=1.4.0" + }, "dependencies": { "@huggingface/transformers": "^4.2.0", "@modelcontextprotocol/sdk": "^1.30.0", @@ -68,8 +72,7 @@ "@types/bun": "1.4.0", "@types/pg": "^8.23.1", "@types/xml2js": "^0.4.14", - "better-sqlite3": "^13.0.3", - "bunset": "1.0.15", + "bunset": "1.1.1", "concurrently": "^10.0.5", "oxfmt": "0.66.0", "oxlint": "^1.81.0", @@ -79,7 +82,6 @@ }, "trustedDependencies": [ "@huggingface/transformers", - "better-sqlite3", "onnxruntime-node" ], "private": false, diff --git a/src/packageManifest.test.ts b/src/packageManifest.test.ts new file mode 100644 index 00000000..34f26329 --- /dev/null +++ b/src/packageManifest.test.ts @@ -0,0 +1,121 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; + +interface Manifest { + readonly engines?: Record; + readonly dependencies?: Record; + readonly devDependencies?: Record; + readonly peerDependencies?: Record; + readonly trustedDependencies?: readonly string[]; + readonly scripts?: Record; + readonly bin?: Record; + readonly exports?: unknown; + readonly main?: unknown; + readonly types?: unknown; +} + +const manifest = JSON.parse( + readFileSync(join(import.meta.dirname, "..", "package.json"), "utf8") +) as Manifest; + +/** + * The runtime floor is asserted in three places that can drift apart: this + * manifest, `.claude/CLAUDE.md`, and `@workglow/sqlite`'s own `engines`. Only + * the manifest is enforced at install time — the other two are prose. A floor + * that is documented but not declared installs cleanly on Node 22 and fails + * later, inside `node:sqlite`, reading as a storage bug rather than a version + * error. + */ +describe("runtime floor", () => { + it("declares the Node floor node:sqlite needs", () => { + expect(manifest.engines?.node).toBe(">=24"); + }); + + it("declares the Bun floor", () => { + expect(manifest.engines?.bun).toBe(">=1.4.0"); + }); +}); + +/** + * `@workglow/sqlite` moved off `better-sqlite3` onto the built-in + * `node:sqlite`. Nothing here loads a native SQLite driver any more, so a + * reappearance is a stale copy-paste rather than a dependency — and its + * `trustedDependencies` entry would run that package's install scripts. + */ +describe("no native SQLite driver", () => { + const maps = [ + ["dependencies", manifest.dependencies], + ["devDependencies", manifest.devDependencies], + ["peerDependencies", manifest.peerDependencies], + ] as const; + + it.each(maps)("keeps better-sqlite3 out of %s", (_name, map) => { + expect(Object.keys(map ?? {})).not.toContain("better-sqlite3"); + }); + + it("keeps better-sqlite3 out of trustedDependencies", () => { + expect(manifest.trustedDependencies ?? []).not.toContain("better-sqlite3"); + }); +}); + +/** + * The package is binary-only by intent, not by accident: the re-founding + * removed the library surface, so there is no `exports` map, no `main` and no + * `types` — just the two `bin` entries. Asserting the shape is what separates + * "decided" from "a field someone deleted by mistake". + */ +describe("binary-only distribution", () => { + it("ships the two binaries", () => { + expect(Object.keys(manifest.bin ?? {}).sort()).toEqual(["sec", "sec-base"]); + }); + + it("exposes no import entry point", () => { + expect(manifest.exports).toBeUndefined(); + expect(manifest.main).toBeUndefined(); + expect(manifest.types).toBeUndefined(); + }); +}); + +/** + * On a 0.x line the minor is the break slot, so a consumer on `^0.1.5` resolves + * a patch on their next install. `release` used to cut a patch unconditionally + * and offer the minor as a second script, which left the number to whoever + * remembered which one to type. + * + * `--auto` derives it instead — from the commits, and from a diff of this + * manifest against the one at the last tag, which raises the bump to the break + * slot when the package lost an entry point or gained a runtime floor. That is + * the class of break no commit message describes, because nothing about it + * looks like a breaking edit. + */ +describe("release scripts", () => { + it("derives the bump rather than naming one", () => { + const release = manifest.scripts?.release ?? ""; + expect(release).toContain("--auto"); + // A second script that names a level is the choice `--auto` removes, back + // in the place it was made from. + expect(release).not.toMatch(/--(patch|minor|major)\b/); + expect(Object.keys(manifest.scripts ?? {})).not.toContain("release-minor"); + }); + + it("runs the gates before the bump", () => { + expect(manifest.scripts?.release).toContain("release-checks"); + const checks = manifest.scripts?.["release-checks"] ?? ""; + for (const gate of ["format", "lint", "typecheck", "build", "prepack-check"]) { + expect(checks).toContain(gate); + } + }); + + it("pins a bunset that has `--auto`", () => { + // `--auto` arrived in 1.1.0 and the 0.x bump table it uses was corrected in + // 1.1.1. An older pin does not fail loudly — it takes the flag as unknown. + expect(manifest.devDependencies?.bunset).toBe("1.1.1"); + }); +}); diff --git a/src/util/isUniqueConstraintError.test.ts b/src/util/isUniqueConstraintError.test.ts index 23ce19f7..a25fc261 100644 --- a/src/util/isUniqueConstraintError.test.ts +++ b/src/util/isUniqueConstraintError.test.ts @@ -23,6 +23,25 @@ describe("isUniqueConstraintError", () => { expect(isUniqueConstraintError({ code: "SQLITE_CONSTRAINT_UNIQUE" })).toBe(true); }); + it("matches node:sqlite's extended result code for a UNIQUE index", () => { + // node:sqlite reports every failure as code "ERR_SQLITE_ERROR" and puts + // the specific one in `errcode`. Verified against Node 24.20: + // UNIQUE index -> 2067, message "UNIQUE constraint failed: t.a" + expect(isUniqueConstraintError({ code: "ERR_SQLITE_ERROR", errcode: 2067 })).toBe(true); + }); + + it("matches node:sqlite's extended result code for a PRIMARY KEY", () => { + // 1555, and SQLite words it "UNIQUE constraint failed: pk.a" as well, so + // the code path has to accept what the message path already accepts. + expect(isUniqueConstraintError({ code: "ERR_SQLITE_ERROR", errcode: 1555 })).toBe(true); + }); + + it("matches on errcode alone, without the message", () => { + // The point of carrying both signals: a wrapper that keeps the code and + // drops the message must not turn a UNIQUE violation into a hard error. + expect(isUniqueConstraintError({ errcode: 2067 })).toBe(true); + }); + it("is case-insensitive on the SQLite/InMemory message", () => { expect(isUniqueConstraintError(new Error("unique constraint failed: foo"))).toBe(true); expect(isUniqueConstraintError(new Error("Unique Constraint Failed: foo"))).toBe(true); @@ -72,6 +91,17 @@ describe("isUniqueConstraintError", () => { }); describe("rejects unrelated errors", () => { + it("rejects node:sqlite errcodes for other constraint kinds", () => { + // Same Node 24.20 run: CHECK -> 275, NOT NULL -> 1299. Both arrive as + // code "ERR_SQLITE_ERROR" too, so the discriminator has to be `errcode`. + expect(isUniqueConstraintError({ code: "ERR_SQLITE_ERROR", errcode: 275 })).toBe(false); + expect(isUniqueConstraintError({ code: "ERR_SQLITE_ERROR", errcode: 1299 })).toBe(false); + }); + + it("rejects a bare ERR_SQLITE_ERROR carrying no errcode", () => { + expect(isUniqueConstraintError({ code: "ERR_SQLITE_ERROR" })).toBe(false); + }); + it("rejects unrelated Postgres SQLSTATE codes", () => { expect(isUniqueConstraintError({ code: "23503" })).toBe(false); // FK violation expect(isUniqueConstraintError({ code: "23502" })).toBe(false); // NOT NULL violation diff --git a/src/util/isUniqueConstraintError.ts b/src/util/isUniqueConstraintError.ts index ddd36fdc..feb1c82a 100644 --- a/src/util/isUniqueConstraintError.ts +++ b/src/util/isUniqueConstraintError.ts @@ -4,29 +4,45 @@ * SPDX-License-Identifier: Apache-2.0 */ +/** + * SQLite's extended result codes for the two ways a UNIQUE index is violated. + * SQLite words both as `"UNIQUE constraint failed: ."`, so the + * code path has to accept whatever the message path accepts. + */ +const SQLITE_CONSTRAINT_UNIQUE = 2067; +const SQLITE_CONSTRAINT_PRIMARYKEY = 1555; + /** * Detects a UNIQUE-index violation thrown by `@workglow/storage` backends. * * Three backends in production today: * - InMemory / SQLite — surface the violation as an `Error` whose message - * starts (case-insensitively) with `"UNIQUE constraint failed"`. SQLite - * additionally carries `code: "SQLITE_CONSTRAINT_UNIQUE"` on the native - * `better-sqlite3` error. + * starts (case-insensitively) with `"UNIQUE constraint failed"`. The + * built-in `node:sqlite` driver reports every failure as + * `code: "ERR_SQLITE_ERROR"` and puts the specific one in a numeric + * `errcode`, so the code that discriminates is `errcode`, not `code`. * - Postgres — propagates the raw `pg.DatabaseError` unmodified through * `PostgresTabularStorage._putInternal`. It carries `code: "23505"` * (SQLSTATE `unique_violation`) and a message of the form - * `"duplicate key value violates unique constraint \"\""`. We match - * BOTH signals so the helper still fires if a future wrapper layer - * strips the SQLSTATE but preserves the message (or vice versa). + * `"duplicate key value violates unique constraint \"\""`. + * + * Every backend is matched on BOTH a code and a message signal, so the helper + * still fires if a wrapper layer strips one but preserves the other. A driver + * swap is what makes that worth stating: the string `"SQLITE_CONSTRAINT_UNIQUE"` + * came from a native driver this package no longer installs, and matching only + * that left SQLite with the message as its sole signal. * * We deliberately avoid `instanceof pg.DatabaseError` / `instanceof SqliteError` - * — neither `pg` nor `better-sqlite3` is a direct dependency of `@workglow/sec`, - * and string/code matching is robust to wrapped or re-thrown errors. + * — `pg` is not the only Postgres path and `node:sqlite` exports no error class + * to test against — and string/code matching is robust to wrapped or re-thrown + * errors. */ export function isUniqueConstraintError(err: unknown): boolean { if (err === null || typeof err !== "object") return false; const code = (err as { code?: unknown }).code; if (code === "23505" || code === "SQLITE_CONSTRAINT_UNIQUE") return true; + const errcode = (err as { errcode?: unknown }).errcode; + if (errcode === SQLITE_CONSTRAINT_UNIQUE || errcode === SQLITE_CONSTRAINT_PRIMARYKEY) return true; const msg = (err as { message?: unknown }).message; if (typeof msg !== "string") return false; const lower = msg.toLowerCase(); diff --git a/vitest.config.ts b/vitest.config.ts index 9928b2a5..b0983599 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -73,7 +73,7 @@ export default defineConfig({ // fork pool selected below.) isolate: true, // Run each test file in its own forked process (not a worker thread). Forks - // give better isolation for native modules (better-sqlite3, onnxruntime) + // give better isolation for native modules (onnxruntime) // and match how a real `sec` CLI subprocess sees the module graph, at the // cost of slower cold starts. pool: "forks",