diff --git a/.changeset/row-identity-ref-probe-sql.md b/.changeset/row-identity-ref-probe-sql.md new file mode 100644 index 0000000000..c57ab0606e --- /dev/null +++ b/.changeset/row-identity-ref-probe-sql.md @@ -0,0 +1,15 @@ +--- +id: row-identity-ref-probe-sql +tasks: [T13261, T12341] +kind: fix +summary: "Row identity: the steady-open ref probe runs no JS hash per dangling reference" +--- + +`rowIdentityFillPending` asks, on every fill-on open, whether a NULL stored +reference (`ac_uid`, `ac_text_hash`) has become resolvable. For +`ac_text_hash` it evaluated the JS AC-text hash on every NULL row, so a store +with many legitimately dangling bindings paid one JS call per row on every +open. The probe now checks the same condition in plain SQL: the hash is +non-NULL exactly when the referenced criterion's `text` is a string, +`typeof(text) = 'text'`. The fill pass itself is unchanged. A reference that +becomes resolvable is still found, and a test pins both behaviours. diff --git a/packages/core/src/store/__tests__/row-identity-steady-open.test.ts b/packages/core/src/store/__tests__/row-identity-steady-open.test.ts index 08f6c012d5..2b0d386a82 100644 --- a/packages/core/src/store/__tests__/row-identity-steady-open.test.ts +++ b/packages/core/src/store/__tests__/row-identity-steady-open.test.ts @@ -19,8 +19,10 @@ import { join } from 'node:path'; import { DatabaseSync } from 'node:sqlite'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { + AC_TEXT_HASH_SQL_FUNCTION, missingRowIdentitySchema, prepareRowIdentity, + registerRowUidFunction, rowIdentityFillPending, } from '../row-identity.js'; import { getNativeTasksDb } from '../sqlite.js'; @@ -61,6 +63,37 @@ describe('row identity on a completed store (T12341 C1)', () => { return db; } + it('dangling AC refs cost no JS hash in the pending probe; a ref that becomes resolvable is found (T13261)', () => { + const db = native(); + prepareRowIdentity(db, 'project'); + expect(rowIdentityFillPending(db, 'project')).toEqual([]); + // Bindings whose criterion is gone: their stored refs stay NULL for good. + db.exec('PRAGMA foreign_keys = OFF'); + const bind = db.prepare( + "INSERT INTO tasks_evidence_ac_bindings (id, evidence_atom_id, ac_id, binding_type) VALUES (?, ?, 'AC-gone', 'direct')", + ); + for (let i = 0; i < 40; i++) bind.run(`bind-${i}`, `atom-${i}`); + db.exec('PRAGMA foreign_keys = ON'); + let hashes = 0; + db.function(AC_TEXT_HASH_SQL_FUNCTION, { deterministic: true }, (text) => { + hashes++; + return typeof text === 'string' ? 'h' : null; + }); + expect(rowIdentityFillPending(db, 'project')).toEqual([]); + expect(hashes).toBe(0); + registerRowUidFunction(db, 'project'); + // The criterion appears: both stored refs of those bindings now resolve. + db.prepare( + "INSERT INTO tasks_task_acceptance_criteria (id, task_id, ordinal, text, kind, source_key) VALUES ('AC-gone', 'T001', 9, 'tests pass', 'text', 'text:9:x')", + ).run(); + expect(rowIdentityFillPending(db, 'project')).toEqual([ + 'ref:tasks_evidence_ac_bindings.ac_uid', + 'ref:tasks_evidence_ac_bindings.ac_text_hash', + ]); + prepareRowIdentity(db, 'project'); + expect(rowIdentityFillPending(db, 'project')).toEqual([]); + }); + it('a completed store reports nothing pending and the pass writes nothing', () => { const db = native(); prepareRowIdentity(db, 'project'); // whatever the seed left (the recipe marker) diff --git a/packages/core/src/store/row-identity.ts b/packages/core/src/store/row-identity.ts index 7d7f49559a..7d0d2feeeb 100644 --- a/packages/core/src/store/row-identity.ts +++ b/packages/core/src/store/row-identity.ts @@ -659,6 +659,27 @@ function birthFpCallSql(scope: TableScope, spec: RowIdentitySpec, row: string): return `${ROW_BIRTH_FP_SQL_FUNCTION}(${[`'${spec.table}'`, birth, ...facts].join(', ')})`; } +/** + * SQL that is true exactly when {@link storedRefSql} would be NOT NULL for + * `row`, without calling a JS function: the AC text hash is non-NULL iff the + * referenced row's text is a string, which SQLite says as `typeof = 'text'`. + * The pending probe uses it, so a store with many legitimately dangling refs + * opens without one JS hash per dangling row (T13261). + */ +function storedRefPresentSql( + scope: TableScope, + ref: NonNullable[number], + row: string, +): string { + if (ref.source === 'text_hash') { + return ( + `(SELECT typeof(_r.${q('text')}) FROM main.${q(ref.table)} AS _r ` + + `WHERE _r.${q(targetKey(scope, ref.table))} = ${row}.${q(ref.from)}) = 'text'` + ); + } + return `${refUidSql(scope, { column: ref.from, table: ref.table }, row)} IS NOT NULL`; +} + /** SQL for a stored reference fact (`ac_uid`, `ac_text_hash`) read from `row`. */ function storedRefSql( scope: TableScope, @@ -2171,12 +2192,7 @@ export function rowIdentityFillPending(db: DatabaseSync, scope: TableScope): str } const row = `main.${q(spec.table)}`; for (const ref of spec.storedRefUids ?? []) { - if ( - any( - spec.table, - `${q(ref.column)} IS NULL AND (${storedRefSql(scope, ref, row)}) IS NOT NULL`, - ) - ) { + if (any(spec.table, `${q(ref.column)} IS NULL AND ${storedRefPresentSql(scope, ref, row)}`)) { pending.push(`ref:${spec.table}.${ref.column}`); } }