Skip to content

refactor(driver-sql): move the raw-execution path out of sql-driver.ts into sql-driver-raw-execution.ts (#25521 cut 2) - #25630

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-25521-sql-driver-cut-2
Oct 24, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-25521-sql-driver-cut-2

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Part of #25521
Clause-②: no
Metadata-protocol: none
Ruling-ref: 6270097840
Ruling-ref: 6348891160

Cut 2 of the sql-driver.ts split (#24586 ruling A, recorded in 6270097840). The claim is 6383192755. Three SqlDriver method bodies move unedited into a new module, packages/drivers/driver-sql/src/sql-driver-raw-execution.ts: the public execute, its protected refusal terminal rawStatementFault, and the protected noteRuntimeIndexDdl, which records the index DDL execute ran.

It is move-only, and the card stays open for the later cuts. Branch claude/issue-25521-sql-driver-cut-2, cut from origin/main at f7b029896 (cut 1's merge).

Serial with the file's in-flight PRs. At claim the PM read no open PR touching sql-driver.ts or driver-turso/src. Re-measured after my first push (14 open PRs, each PR's file list read in full from its head, merge base to head) and again just before this PR was opened (13 open; #25617 had merged). None touches packages/drivers/driver-sql/src/** or packages/drivers/driver-turso/src/**. origin/main moved three commits past the base meanwhile (#25612, #25615, #25617, now cc10e74fe). None of them touches driver-sql, driver-turso, driver-sqlite-wasm or a gate script this diff derives, so main is not merged in.

Seam table (fresh, at f7b029896)

sql-driver.ts is 21,675 lines. 30-day lines are git blame --line-porcelain lines whose committer time is on or after 2026-09-24T10:03:29Z, 30 days before the base commit. History was deepened to 2026-09-15 first, and scripts/pm/git-history.mjs count proves 68 commits on the file in that window (19 in the last 7 days). In all, 3,855 lines (17.8%) are from the last 30 days. The class SqlDriver spans lines 5482-21600 with 347 members: 251 methods (49 public), 32 statics. No public method has an inferred return type; the one inferred public member is the getter supports, in the class head. Sections are the class's own banner comments, cut by contiguous line range. "Public" counts public methods, plus public accessors where there are any:

Lines Section Lines 30-day lines Members (public)
1-5481 Module preamble: imports, types, top-level helpers (200 statements) 5,481 1,405 -
5482-7073 Class head: fields, statics, constructor, capability getters (supports) 1,592 290 88 (1 + 2 accessors)
7074-7505 Lifecycle 432 155 15 (3)
7506-10184 CRUD (IDataDriver core) 2,679 691 45 (7)
10185-10796 Bulk and batch operations 612 140 9 (6)
10797-10922 Raw execution (this cut) 126 12 4 (1)
10923-11000 Transactions 78 0 7 (5)
11001-11532 Aggregation 532 61 5 (1)
11533-11820 Distinct 288 20 3 (1)
11821-11936 Window functions 116 4 1 (1)
11937-12035 Query plan analysis 99 1 2 (2)
12036-15790 Schema sync (syncSchema / init) 3,755 373 62 (14 + 3 accessors)
15791-15926 Schema introspection (listPhysicalColumns, introspectSchema) 136 65 2 (2)
15927-21600 Internal helpers, including cut 1's five introspection stubs 5,674 638 104 (5)
21601-21675 After the class: UniqueConstraintMember, singleColumnUniqueColumns 75 0 -

The limits per area are cut 1's (PR #25614), re-read on this tree:

  • getBuilder builders. Aggregation, Distinct, Window functions, Query plan analysis, CRUD and Bulk build through getBuilder, so a cut of any of them must first carry the new module into check:tenant-chokepoint's SCAN_FILES, or the gate goes silently narrower. None of them is cut here.
  • Protected statics. The hash-shadow family and the row-size explainers (Internal helpers) read protected statics, so those bodies stay.
  • this.FIELD.delete/insert/update(. Transactions has one, releaseTransaction (this.openTransactions.delete(key)), and Schema sync has three. Those bodies stay. Raw execution has none: noteRuntimeIndexDdl calls .delete( on a local Set, and calls .get(, .set( and .values() on the ledger.
  • insertOnlyUpsertColumns stays in sql-driver.ts, because drivers(sql): upsert's empty-merge-set fallback is merge-ALL, which re-admits every insert-only column — currently unreachable, but it is the wrong shape for "nothing to merge" #8740's restart line names that symbol in that file. This cut does not touch it.

Proposed cut 3: Transactions less releaseTransaction (10923-11000, 78 lines, 0 from the last 30 days). That is beginTransaction, assertBareKnexSafe, commit, rollback, commitTransaction and rollbackTransaction. Every return type is annotated, and none reads getBuilder or a static. releaseTransaction stays, for its this.openTransactions.delete( site. The bodies call it through a late-bound closure, as execute calls rawStatementFault here. Schema introspection (136 lines, 65 from 30 days) is the next candidate after that.

Why this area for cut 2

Cut 1 proposed it, and it re-measures clean: 126 lines, 12 from the last 30 days. Its one public member, execute, has an annotated return type (a Promise of unknown, pinned by sql-driver-doors-declared-types.test.ts). It has no static and no getBuilder, and no this.FIELD.delete/insert/update( site. Between them, the three bodies read only knex, logger, runtimeCreatedIndexes and each other (execute calls rawStatementFault and noteRuntimeIndexDdl). The field runtimeCreatedIndexes and its docblock stay on the class: schema sync also reads the ledger (the index-drift diff in the Schema sync section). Both subclasses reach the moved bodies. TursoDriver overrides execute, calls super.execute, and calls rawStatementFault at eight sites. SqliteWasmDriver inherits all three.

The shape (#23050 shape B, as cut 1 applies it)

  • The class keeps each method with its name, signature, docblock and position, because tsup emits members with their JSDoc into the .d.ts. Each body is now one host literal plus fn.call(host, ...), marked [#25521 cut 2]. execute drops async and returns the moved function's promise. The other two were never async.
  • RawExecutionHost is new. It types every member as the class member itself (an indexed access on SqlDriver). Each moved function types its this as the Pick of the host over exactly the members its body reads. The generator lists them, and they were checked against the this. reads of each base body. Each stub annotates its literal as the ThisParameterType of the moved function. There is no cast, and no member becomes public.
  • knex, logger and runtimeCreatedIndexes go as getters, so each read happens when the body makes it. rawStatementFault and noteRuntimeIndexDdl go as late-bound closures. So a subclass override is still the one execute reaches: sql-driver-21385-refusal-log-line-redaction.test.ts and turso-driver-16019-remote-raw-statement-fault-envelope.test.ts override rawStatementFault and call super. A spy on the instance is reached the same way.
  • this.logger.warn(...) keeps its receiver: the getter hands back the class's logger, and .warn is called on it.
  • Module-level declarations. The bodies read seven top-level declarations of sql-driver.ts: OWN_STATEMENT_FAULT, INDEX_DDL_PREFIX, CREATE_INDEX_DDL, DROP_INDEX_DDL, unquoteSqlIdentifier, lastIdentifierSegment and rawStatementFaultError. All seven stay where they are, each gains a leading export, and the new module imports them back. They stay because OWN_STATEMENT_FAULT has readers that did not move (four other refusal lines). They also stay because tests in four other packages (types, cli, metadata, metadata-protocol) cite rawStatementFaultError "verbatim (sql-driver.ts)" by path. Every value import is read only inside a function body, so the module cycle is evaluation-safe, as in cut 1. index.ts re-exports nothing new, so the published surface does not move (proof below). This is the one place the cut differs from cut 1's shape: cut 1 needed no new export. It is recorded as an open question in the report.
  • The move was produced by a script from the source text, not retyped: each method's parameter list, return type and body lines, re-indented two columns. The three bodies hold no multi-line template literal.

The published .d.ts does not change

Under the series bar (ruling 6348891160, letter A), measured on base f7b029896 and on head a92731ec. Both were built against the same dependency tree: turbo run build --filter=@objectstack/driver-turso... --filter=@objectstack/driver-sqlite-wasm... at base, 9/9 tasks, 0 cached (remote caching disabled). Then pnpm --filter @objectstack/driver-sql build at head.

  • This package emits dist/index.d.ts and dist/index.d.mts (there is no .d.cts). The two are identical to each other at both ends.
  • Raw sha256 of both files, base and head, all four the same: 93da45168e82136b7613944525e47402d5dc6f9f8daa9efa4bdceeaa11b52a89 (cut 1's head hash, unchanged). The raw diff is 0 lines.
  • Normalised with the ruling's one-liner verbatim (double-quoted unions), base = head = 93da45168e82136b7613944525e47402d5dc6f9f8daa9efa4bdceeaa11b52a89. This file has 0 double-quoted unions, so that normaliser rewrites nothing.
  • Normalised with the same one-liner, with the quote character swapped to a single quote (21 lines of this file carry a single-quoted union), base = head = 118e992ad8aad590044eb31509dca65e6b321e3410ce846e1e5f5dd34b956e95. With a raw diff of 0 lines, the proof rests on raw identity. The single-quote pass is reported only because the dispatch asked for it.
  • Positive control on a copy of the head dist/index.d.ts: line 4217, protected rawStatementFault(command: string, error: unknown): Error;, made public. That gives raw and double-quote-normalised 47c79477965c2473c4a079afd8c4c8f47bc9f36213ac7dcfa2df99be7f081688 and single-quote-normalised 464658c31cd8f7ee5c779054428fe3f8d3c520610061252de5567e05a480ccd2, each different from the real hash.
  • Export tables: the ESM and CJS bundles each export the same 56 names at base and head, loaded at runtime (import() and require of the built bundles). The sorted list hashes to d61a9dd3a02661602b1bb2dbe18152ac43986148f36b2746f40c0884bd8ffdec on all four.
  • The head build carries the move: dist/index.mjs and dist/index.js each contain 8 host .call( sites (cut 1's 5 plus these 3).

No changeset: skip-changeset, per ruling 6348891160 item 4.

Proof of move-only

  • Bodies, by AST token (recursive getChildren leaves). Each moved function's parameters (less the added this), return type and body have the same token sequence as the base method, and async matches on all three. 404 tokens compared, 0 differ.
  • Lines: 52 body lines compared (16 + 21 + 15, each body after its opening brace). Each one is the base line with two leading spaces removed.
  • Negative control: changing one character inside a string literal of rawStatementFault in a copy of the module makes the check report 1 token and 1 line different, and it exits PROOF FAILED.
  • Class side: 347 members at base and head. The 344 members that did not move are byte-identical to base, leading trivia included. Each of the 3 stubs keeps its docblock and signature byte-identical, less async on execute.
  • Top level of sql-driver.ts: 200 base statements other than the class. 193 are byte-identical. 7 are byte-identical except for a leading export. They stay in order. The one new statement is the import of the new module.
  • Size: +166 / -57 over 2 files, far under the 3,000-line manual-merge line. sql-driver.ts goes from 21,675 to 21,651 lines, and sql-driver-raw-execution.ts has 133.

Readers of sql-driver.ts by path, checked against the moved text

None needed an edit. Each was read against the moved region (10797-10922) and run at both ends where it is a gate or test:

  • ADR anchors. scripts/adr-anchors/packages__drivers__driver-sql__src__sql-driver.ts.json carries ADR-0053 (formatOutput, presentReadValue) and ADR-0120 D4. Neither paragraph describes a moved body, so per card ask 3 the new module gets no entry of its own. The moved bodies cite no ADR at all. The docblocks that cite ADR-0048 and ADR-0112 stay on the class. check:adr-anchors: 85 anchored files at base and head. Decision-letter citations on the anchored surface: 1,871 at base and 1,871 at head.
  • Symbol anchors. No ADR or platform-checklist anchor names execute, rawStatementFault, noteRuntimeIndexDdl or the seven module-level declarations in sql-driver.ts, and the stubs keep every member name anyway. check:adr-symbol-anchors: 2,384 anchors resolve at both ends.
  • check:tenant-chokepoint reads sql-driver.ts for getBuilder bindings and for IS NULL arms in raw fragments. The moved text has no getBuilder. Its one raw call, this.knex.raw(command, ...), passes a variable, which that gate does not read (its header names this case as outside its view). So the gate saw nothing in the moved text, and it still covers the same population: 24 getBuilder() bindings across 3 files at both ends.
  • Logger receivers. logger-receiver-detach.test.ts ③ scans sql-driver.ts for detach-then-call shapes. The moved this.logger.warn( is a direct call, not a finding. The repo-wide check:logger-receiver-detach walks every packages/** source, so it covers the new module: 0 detaches at head.
  • Other source scans, none of whose patterns occurs in the moved text: sql-driver-compile-refusal-seam.test.ts (no INVALID_FILTER builder), the five packages/cli generate-*.pin.test.ts files (createColumn and the varchar constants), check-keyed-text-bounds (varcharColumnChars), check-object-def-param-keys (no (obj as any).KEY read).
  • Path lists that name the file for cache inputs or fixtures (turbo.json, scripts/cross-package-test-inputs.mjs, scripts/pm/check-half-states.mjs, scripts/check-changeset-no-major.mjs, packages/spec/liveness/*.json evidence). The readers they serve read createColumn, the varchar constants or other unmoved symbols.
  • Comment citations in other packages ("rawStatementFaultError … (sql-driver.ts)") stay true, because that function did not move.
  • driver-turso and driver-sqlite-wasm: their readers of moved members reach the stubs unchanged. Both suites ran at both ends (below).
  • check:driver-conformance at base and head: 55 covered, 0 in the DEBT ledger, 0 exempt, both times.

Verification

  • tsc --noEmit (driver-sql) exit 0. eslint --no-inline-config on both source files exit 0.
  • packages/drivers/driver-sql suite: base 3,999 tests (3,787 passed, 0 failed, 212 skipped; 247 files), head the same. Compared per test name and status from vitest's JSON reporter: no test lost, none changed status, none added.
  • packages/drivers/driver-turso suite, run against the rebuilt head dist: base 2,526 tests (2,492 passed, 0 failed, 34 skipped; 95 files), head the same. Per test name and status there is no difference (2,513 distinct names; 13 names repeat).
  • packages/drivers/driver-sqlite-wasm suite, against the rebuilt head dist: base 704 tests (704 passed; 39 files), head the same, per test name no difference.
  • dispatch-gates --commands on this diff, at head a92731ec: 55 derived commands, each run with its exit code. --ran reconciles 55 derived, 54 run, 1 NOT MEASURED, 0 unrun. The NOT MEASURED one is check:dual-build-cjs-loads (exit 3, PREREQUISITE NOT MET: it needs every package's dist), which CI runs. Two gates first answered exit 3 for prerequisites and then exit 0: check-plugin-teardown-shape --self-test after its pinned fixture commit 621a4876 was fetched, and check:lean-entry-closure after objectql was built.
  • No reverse verification or ablation: the diff changes no tested behaviour.

Acceptance notes

  • Seven top-level declarations of sql-driver.ts gain export, so the moved bodies can import them without a second copy. The package's public surface does not change (raw .d.ts identity, export tables). Moving the five declarations only the moved bodies read into the new module was the alternative. It was not taken: other packages cite rawStatementFaultError in sql-driver.ts by path, and OWN_STATEMENT_FAULT would need the export anyway.
  • TursoDriver's execute override still calls super.execute, which now returns the moved function's promise instead of an async method's promise. A caller awaits the same settlement.

Generated by Claude Code

…s into sql-driver-raw-execution.ts (#25521 cut 2)

Move-only. The bodies of SqlDriver#execute, #rawStatementFault and
#noteRuntimeIndexDdl move unedited into a new module and reach the class
through one named host (RawExecutionHost): knex, logger and
runtimeCreatedIndexes as getters, rawStatementFault and noteRuntimeIndexDdl as
late-bound closures so subclass overrides and spies still dispatch. The class
keeps each method's name, signature, docblock and position. The seven
module-level declarations the bodies read stay in sql-driver.ts and gain an
`export` so the new module can import them; index.ts re-exports nothing new.

Claude-Session: https://claude.ai/code/session_01RvoeGVyNRC3zkVrNsLbm8M
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet objectstack-fleet Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Oct 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

14 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json cc10e74fefc6ca104b0d9159b23783b94499a119 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 9b394818531587e01f18beed385a0337c52c4ce9 — the merge of head a92731ec28863774f1f5ba42d269f6c136c9952a into base cc10e74fefc6ca104b0d9159b23783b94499a119, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9b394818531587e01f18beed385a0337c52c4ce9 && git checkout 9b394818531587e01f18beed385a0337c52c4ce9
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cc10e74fefc6ca104b0d9159b23783b94499a119 a92731ec28863774f1f5ba42d269f6c136c9952a && git checkout -B drift-repro cc10e74fefc6ca104b0d9159b23783b94499a119 && git merge --no-ff a92731ec28863774f1f5ba42d269f6c136c9952a

node scripts/docs-audit/affected-docs.mjs --json cc10e74fefc6ca104b0d9159b23783b94499a119

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · domain:server seat 2 · sunhaolin · session_01JVTd3PprStGa26aez5omLB · 2026-10-24T11:50Z · head a92731ec28 · claim 6383192755 · report 6383606915

Checklist conclusions (read on GitHub, not from the report):

  • Shape: draft, base main, author objectstack-fleet[bot], assignee sunhaolin. The first line is Part of #25521; no closing keyword in the body. Clause-②: no and Metadata-protocol: none are line-initial. skip-changeset under ruling 6348891160.
  • Scope: 2 files in packages/drivers/driver-sql/src/ (sql-driver.ts +33/-57, sql-driver-raw-execution.ts new). Not governed; 223 changed lines.
  • Move-only, re-measured by the seat: I extracted the three bodies at base (sql-driver.ts:10822 rawStatementFault, :10863 execute, :10906 noteRuntimeIndexDdl) and in the new module (:75, :94, :118). Whitespace-insensitive comparison: all three identical (743 / 536 / 556 significant chars). rawStatementFault and noteRuntimeIndexDdl reach the host as late-bound closures, so driver-turso's overrides still dispatch.
  • The seven new exports (open question 1, option A): src/index.ts re-exports ./sql-driver.js by name only (no export *), so the added exports do not reach the published surface. That matches the report's .d.ts block: raw base = head for index.d.ts / .d.mts, with a positive control, and exports 56/56. Option A is accepted: it moves only method bodies and keeps every path citation true.
  • logger-receiver-detach (open question 2, option A): accepted. The repo-wide check:logger-receiver-detach walks the new module (0 detaches at head), and the moved line is a direct call, not a finding. Card ask 3's "must not go silently empty" holds: test ③'s population still has its file.
  • Tests: identical counts and names at base and head for driver-sql (3787 / 212 skipped), driver-turso (2492 / 34) and driver-sqlite-wasm (704; the dev added it, since SqliteWasmDriver also extends SqlDriver). These are local readings; CI decides.
  • Writes: mcp_calls 8, all GitHub MCP reads; no write tool.
  • drivers(sql): upsert's empty-merge-set fallback is merge-ALL, which re-admits every insert-only column — currently unreachable, but it is the wrong shape for "nothing to merge" #8740: insertOnlyUpsertColumns was not moved.

Deviations accepted: as reported (trailers in the AGENTS.md form; the platform-created branch; deepened history; main not merged in, since the 3 new main commits touch none of the three drivers or a derived gate; the merge queue builds on current main).

Carried to cut 3: the dev proposes Transactions less releaseTransaction (78 lines, 0 from 30 days; releaseTransaction stays for its this.openTransactions.delete( site). The getBuilder, statics and #8740 constraints carry forward.

Landing: pr_ready + automerge_enable through the relay once every check on head a92731ec28 is green. After it merges, #25521 goes back to pm:queue for cut 3, and the cloud session session_01RvoeGVyNRC3zkVrNsLbm8M is archived.


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 24, 2026 12:00
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 24, 2026 12:00
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit b694e56 Oct 24, 2026
47 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-25521-sql-driver-cut-2 branch October 24, 2026 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants