From 526b6f249b7681c3adea58ddc97e4b589d76027a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Mon, 5 Oct 2026 13:49:07 +0100 Subject: [PATCH] test(search): guard native lookup and Unicode FTS after COPY (#3477) --- .../incremental-write-integrity/README.md | 3 + .../reproduce-search.cjs | 251 ++++++++++++++++++ .../search-root-cause.md | 100 +++++++ gitnexus/scripts/cross-platform-tests.ts | 1 + .../lbug-incremental-search.test.ts | 62 +++++ gitnexus/vitest.config.ts | 2 + 6 files changed, 419 insertions(+) create mode 100644 gitnexus/bench/incremental-write-integrity/reproduce-search.cjs create mode 100644 gitnexus/bench/incremental-write-integrity/search-root-cause.md create mode 100644 gitnexus/test/integration/lbug-incremental-search.test.ts diff --git a/gitnexus/bench/incremental-write-integrity/README.md b/gitnexus/bench/incremental-write-integrity/README.md index b5b14d881..a75861500 100644 --- a/gitnexus/bench/incremental-write-integrity/README.md +++ b/gitnexus/bench/incremental-write-integrity/README.md @@ -1,5 +1,8 @@ # Incremental node identity reconciliation +For the native cause of #3421/#3423 and regressions covering both missing +context results and Unicode Property FTS, see [search-root-cause.md](search-root-cause.md). + From `gitnexus/`: ```sh diff --git a/gitnexus/bench/incremental-write-integrity/reproduce-search.cjs b/gitnexus/bench/incremental-write-integrity/reproduce-search.cjs new file mode 100644 index 000000000..8e4c2d2e9 --- /dev/null +++ b/gitnexus/bench/incremental-write-integrity/reproduce-search.cjs @@ -0,0 +1,251 @@ +#!/usr/bin/env node +/** Native search contracts for #3421/#3423. See search-root-cause.md. */ +const assert = require('node:assert/strict'); +const fs = require('node:fs/promises'); +const os = require('node:os'); +const path = require('node:path'); +// Diagnostic overrides allow the identical workload to test an older native engine. +const lbug = require(process.env.LBUG_MODULE || '@ladybugdb/core'); +const scenario = process.argv[2]; +assert.ok( + ['context', 'property'].includes(scenario), + 'Usage: reproduce-search.cjs context|property [--keep]', +); + +(async () => { + const directory = await fs.mkdtemp(path.join(os.tmpdir(), 'ladybug-search-')); + const checks = []; + let database; + let connection; + const record = async (name, fn) => { + try { + checks.push({ name, ok: true, ...(await fn()) }); + } catch (error) { + checks.push({ name, ok: false, error: String(error) }); + } + }; + const close = async () => { + const activeConnection = connection; + const activeDatabase = database; + connection = undefined; + database = undefined; + const errors = []; + try { + await activeConnection?.close(); + } catch (error) { + errors.push(error); + } + try { + await activeDatabase?.close(); + } catch (error) { + errors.push(error); + } + if (errors.length) throw new AggregateError(errors, 'Native cleanup failed'); + }; + const query = async (sql, params) => { + const raw = params + ? await connection.execute(await connection.prepare(sql), params) + : await connection.query(sql); + const rows = []; + const errors = []; + // FTS expands into multiple statements: consume and close every result, + // including when an earlier statement reports a deferred native error. + for (const result of Array.isArray(raw) ? raw : [raw]) { + try { + rows.push(...(await result.getAll())); + } catch (error) { + errors.push(error); + } + try { + await result.close(); + } catch (error) { + errors.push(error); + } + } + if (errors.length) throw new AggregateError(errors, errors.map(String).join('; ')); + return rows; + }; + const table = scenario === 'context' ? 'Function' : 'Property'; + const columns = ['id', 'name', 'filePath', 'startLine', 'endLine']; + if (scenario === 'property') columns.push('content', 'description'); + const indices = Array.from({ length: 8192 }, (_, i) => i); + const changed = (i) => i < 32 || i >= 8192 - 32; + const tuple = (i) => + scenario === 'context' + ? [ + `Function:src/owner${Math.floor(i / 32)}.ts:unchangedSwiftTarget${i}`, + `unchangedSwiftTarget${i}`, // Out-of-line name, unlike the short fn64 fixture. + `src/owner${Math.floor(i / 32)}.ts`, + (i % 32) * 4, + (i % 32) * 4 + 2, + ] + : [ + `Property:src/owner${Math.floor(i / 32)}.swift:property${i}`, + `unchangedPropertyValue${i}${'王'.repeat(i % 17)}`, + `src/owner${Math.floor(i / 32)}.swift`, + i, + i + 1, + `var unchangedPropertyValue${i} = "Valid UTF8 Å王 café 東京 ${i} ${'Résumé Ελληνικά Привет 🙂 '.repeat(i % 53)}"${i === 64 ? ' retainedsearchsentinel' : ''}`, + `Documentation for property ${i} ${'正常な日本語 naïve façade Å王 '.repeat(i % 37)}`, + ]; + const csv = (selected) => + columns.join(',') + + '\n' + + selected + .map((i) => + tuple(i) + .map((value) => '"' + String(value).replaceAll('"', '""') + '"') + .join(','), + ) + .join('\n') + + '\n'; + const literal = (value) => "'" + value.replace(/\\/g, '/').replaceAll("'", "\\'") + "'"; + const copy = (file) => + query( + `COPY ${table} FROM ${literal(path.join(directory, file))} ` + + `(HEADER=true, ESCAPE='"', DELIM=',', QUOTE='"', PARALLEL=false, auto_detect=false)`, + ); + const extension = + process.env.FTS_LIBRARY || + path.resolve( + __dirname, + '../../vendor/lbug-fts/prebuilds', + `${process.platform}-${process.arch}`, + 'libfts.lbug_extension', + ); + const open = async (readOnly) => { + database = new lbug.Database( + path.join(directory, 'db'), + 256 * 1024 * 1024, + false, + readOnly, + 4 * 1024 ** 3, + true, + 64 * 1024 * 1024, + true, + true, + ); + connection = new lbug.Connection(database); + if (scenario === 'property') await query(`LOAD ${literal(extension)}`); + }; + const scan = async (stage, selected = indices) => + record(`${stage}:scan`, async () => { + const rows = await query( + `MATCH (n:${table}) RETURN ${columns.map((c) => `n.${c} AS ${c}`).join(', ')}`, + ); + const expected = new Set(selected.map((i) => JSON.stringify(tuple(i)))); + const actual = rows.map((row) => JSON.stringify(columns.map((c) => row[c]))); + const actualSet = new Set(actual); + const wrong = actual.filter((row) => !expected.has(row)).length; + const missing = [...expected].filter((row) => !actualSet.has(row)).length; + assert.deepEqual( + { rows: rows.length, wrong, missing }, + { rows: selected.length, wrong: 0, missing: 0 }, + ); + return { rows: rows.length, wrong, missing }; + }); + const lookup = async (stage) => { + const target = tuple(64); // Retained throughout DELETE/COPY. + const select = + 'n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine'; + for (const [kind, sql, params] of [ + ['primary-key', `MATCH (n:Function {id: $uid}) RETURN ${select} LIMIT 1`, { uid: target[0] }], + // Match LocalBackend.resolveContextSymbol's label-free UID/name queries. + ['uid', `MATCH (n {id: $uid}) RETURN ${select} LIMIT 1`, { uid: target[0] }], + [ + 'name', + `MATCH (n) WHERE n.name = $symName RETURN ${select} ORDER BY n.id LIMIT 20`, + { symName: target[1] }, + ], + ]) + await record(`${stage}:${kind}`, async () => { + const rows = await query(sql, params); + assert.deepEqual( + rows.map((row) => columns.map((column) => row[column])), + [target], + ); + return { ids: rows.map((row) => row.id) }; + }); + }; + const build = () => + query("CALL CREATE_FTS_INDEX('Property', 'property_fts', ['name', 'content', 'description'])"); + const drop = () => query("CALL DROP_FTS_INDEX('Property', 'property_fts')"); + const search = async (stage) => { + await record(`${stage}:catalog`, async () => { + const indexes = await query('CALL SHOW_INDEXES() RETURN *'); + const matches = indexes.filter( + (row) => row.table_name === 'Property' && row.index_name === 'property_fts', + ); + assert.equal(matches.length, 1); + assert.deepEqual(matches[0].property_names, ['name', 'content', 'description']); + return { indexes: matches.length }; + }); + await record(`${stage}:fts-query`, async () => { + const rows = await query( + "CALL QUERY_FTS_INDEX('Property', 'property_fts', 'retainedsearchsentinel') RETURN node, score ORDER BY score DESC, node.id LIMIT 20", + ); + const ids = rows.map((row) => row.node.id); + assert.deepEqual(ids, [tuple(64)[0]]); + return { ids }; + }); + for (const column of ['name', 'content', 'description']) + await record(`${stage}:lower-${column}`, async () => { + const rows = await query(`MATCH (n:Property) RETURN LOWER(n.${column}) AS value`); + assert.equal(rows.length, indices.length); + return { rows: rows.length }; + }); + }; + const check = async (stage) => { + await scan(stage); + if (scenario === 'context') await lookup(stage); + else await search(stage); + }; + try { + await fs.writeFile(path.join(directory, 'full.csv'), csv(indices)); + await fs.writeFile(path.join(directory, 'delta.csv'), csv(indices.filter(changed))); + await open(false); + const schema = columns.map((c) => `${c} ${c.endsWith('Line') ? 'INT64' : 'STRING'}`).join(', '); + await query(`CREATE NODE TABLE ${table}(${schema}, PRIMARY KEY(id))`); + // Multiple labels make the UID query use a filtered scan, as in GitNexus. + if (scenario === 'context') await query(`CREATE NODE TABLE Method(${schema}, PRIMARY KEY(id))`); + await copy('full.csv'); + if (scenario === 'property') await record('baseline:build', build); + await query('CHECKPOINT'); + await check('baseline'); + if (scenario === 'property') await drop(); + const suffix = scenario === 'context' ? 'ts' : 'swift'; + await query( + `MATCH (n:${table}) WHERE n.filePath IN ['src/owner0.${suffix}', 'src/owner255.${suffix}'] DETACH DELETE n`, + ); + await scan( + 'after-delete', + indices.filter((i) => !changed(i)), + ); + await copy('delta.csv'); + // Build first: scanning strings beforehand can change the old engine's error manifestation. + if (scenario === 'property') await record('after-copy:build', build); + await check('after-copy'); + if (scenario === 'property') { + await record('repair:drop', drop); + await record('repair:build', build); + await check('repair'); + } + await query('CHECKPOINT'); + await check('after-checkpoint'); + await close(); + await open(true); + await check('reopened'); + } catch (error) { + checks.push({ name: 'setup-or-write', ok: false, error: String(error) }); + } finally { + await record('close', close); + if (!process.argv.includes('--keep')) { + await record('remove-fixture', () => fs.rm(directory, { recursive: true, force: true })); + } + } + console.log(JSON.stringify({ scenario, native: lbug.VERSION, directory, checks })); + if (checks.some((check) => !check.ok)) process.exitCode = 1; +})().catch((error) => { + console.error(error); + process.exitCode = 1; +}); diff --git a/gitnexus/bench/incremental-write-integrity/search-root-cause.md b/gitnexus/bench/incremental-write-integrity/search-root-cause.md new file mode 100644 index 000000000..fc09ee6dd --- /dev/null +++ b/gitnexus/bench/incremental-write-integrity/search-root-cause.md @@ -0,0 +1,100 @@ +# Incremental search failures: native cause and regression contracts + +Issues [#3421](https://github.com/abhigyanpatwari/GitNexus/issues/3421) and +[#3423](https://github.com/abhigyanpatwari/GitNexus/issues/3423) reported two +symptoms after a small incremental write: valid source strings caused Property +FTS to fail in `LOWER`, and unchanged symbols stopped resolving by both name and +UID. A full rebuild restored service in both reports. + +## Cause and effective fix + +LadybugDB 0.18.3's `StringColumn::scanFiltered` compared a position in the output +vector with a segment-local length. When a scan crossed segments after selective +DELETE/COPY, it could skip valid string positions. The native scan could return +blank or stale string values while a labeled primary-key lookup still returned +the correct tuple. This establishes an incorrect read, not physical byte loss. + +Upstream [LadybugDB #737](https://github.com/LadybugDB/ladybug/pull/737), commit +`a10cecbc76e05f993af6c6f4a57edbbf438bb376`, passes the current segment's scan length +into `scanFiltered` and bounds positions by +`offsetInResult <= pos < offsetInResult + numValuesToScan`. The local read offset +is `startOffsetInChunk + pos - offsetInResult`. The correction first shipped in +LadybugDB 0.19.0. + +GitNexus already includes it through the 0.21.1 dependency in +[#3442](https://github.com/abhigyanpatwari/GitNexus/pull/3442), commit +`412446408d0e3f286b9e461fc451c25bf6283b3c`. Keep that native correction and the +existing publication reconciliation. Changing only the UID query to a labeled +lookup would hide one symptom while leaving name scans and FTS vulnerable. + +## Controlled causal check + +On Linux x64, a clean build of upstream tag `v0.18.3` reproduced both symptoms via +the native C API, without GitNexus, its parser, or the Node binding. Applying +**only the two production-file changes from #737** to that same checkout and +rebuilding removed both failures using the same CSV input and FTS extension +(`v0.18.1/linux_amd64`, the extension ABI used by core 0.18.3). + +| Observation after selective COPY | Untouched 0.18.3 | 0.18.3 + #737 only | +| --- | --- | --- | +| Label-free retained UID lookup | 0 rows | Correct row | +| Label-free retained name lookup | 0 rows | Correct row | +| Labeled primary-key control | Correct row | Correct row | +| Function scan with blank ID/name | 1,984 rows | 0 rows | +| Property FTS creation | `LOWER: Invalid UTF-8` | Success | +| Property scan with blank ID/name | 1,984 rows | 0 rows | +| Property `LOWER(content)` | `Invalid UTF-8` | Success | +| Drop/recreate Property FTS | Missing index / orphan table | Success | + +The Node fixtures also fail on published core 0.18.3 and pass on 0.21.1. Incorrect +tuple counts vary with the fixture and scan shape (1,984 or 3,936 observed). +The UTF-8 exception itself varies between runs: a failed string scan can return +blank values without throwing. Therefore the regression oracle compares every +source field and checks lookup results, rather than requiring an exception or +treating a successful FTS build as sufficient. + +FTS creation expands into multiple statements in the extension's +`create_fts_index.cpp`: it creates internal tables, tokenizes property values +with `LOWER`, then registers the index. A failure during tokenization can leave +`0_property_fts_appears_info` without a catalog index. Dropping by index name then +fails, and creating again can encounter the orphan table. FTS-only repair cannot +correct the underlying string scan. The experiment reproduced this failure +chain, but not #3421's exact missing-index message without an appended native +error. Neither reporter's overwritten private database was available, so this +is a controlled reproduction of both reported symptoms, not a historical replay. + +## Permanent regressions + +From `gitnexus/`: + +```sh +node bench/incremental-write-integrity/reproduce-search.cjs context +node bench/incremental-write-integrity/reproduce-search.cjs property +npx vitest run test/integration/lbug-incremental-search.test.ts +``` + +Each workload loads 8,192 rows, checkpoints, deletes the first and last owners +(64 rows), then copies back exactly those rows. Row 64 remains untouched. + +- `context` uses out-of-line ASCII names and a second node table. Those details + matter: a single table can turn the UID query into a healthy primary-key scan, + and short names can stay readable while their IDs are blank. It runs the + label-free UID/name query shapes from `LocalBackend.resolveContextSymbol` plus + the labeled primary-key control. +- `property` uses valid, variable-length Unicode in name/content/description, + builds all three FTS columns, checks the catalog and a retained unique search + term, exercises `LOWER` on each column, and drops/recreates the index. It uses + the production FTS result shape (`RETURN node, score`). + +Both compare the complete expected tuple set before/after the write, after an +explicit checkpoint, and after read-only reopen. The Property fixture also +checks the repair cycle. The Vitest wrapper runs each native workload in a child +process, requires every expected phase, and rejects nonzero exits and signals. +It belongs to the serialized `lbug-db` project and the Windows/macOS native test list. + +The harness requires no network access. It defaults to the pinned core and +vendored platform FTS library. For a diagnostic comparison, set `LBUG_MODULE` +to an absolute path to another installed `@ladybugdb/core` and `FTS_LIBRARY` +to its matching native extension. Core 0.18.3 is expected to exit nonzero; do not +skip the test on that version. `--keep` retains only the synthetic fixture files; +otherwise cleanup errors are reported and fail the command. diff --git a/gitnexus/scripts/cross-platform-tests.ts b/gitnexus/scripts/cross-platform-tests.ts index 037109640..3fdb26ee0 100644 --- a/gitnexus/scripts/cross-platform-tests.ts +++ b/gitnexus/scripts/cross-platform-tests.ts @@ -167,6 +167,7 @@ const LBUG_NATIVE = [ 'test/integration/lbug-close-handle-release.test.ts', 'test/integration/lbug-orphan-sidecar-recovery.test.ts', 'test/integration/lbug-interrupted-checkpoint-recovery.test.ts', + 'test/integration/lbug-incremental-search.test.ts', 'test/integration/lbug-readonly-init.test.ts', 'test/integration/lbug-non-ascii-path.test.ts', // Cross-repo trace e2e: builds two real lbug indexes + a real bridge and diff --git a/gitnexus/test/integration/lbug-incremental-search.test.ts b/gitnexus/test/integration/lbug-incremental-search.test.ts new file mode 100644 index 000000000..ac94170be --- /dev/null +++ b/gitnexus/test/integration/lbug-incremental-search.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; + +const reproducer = fileURLToPath( + new URL('../../bench/incremental-write-integrity/reproduce-search.cjs', import.meta.url), +); +interface Check { + name: string; + ok: boolean; + error?: string; + rows?: number; + wrong?: number; + missing?: number; + ids?: string[]; + indexes?: number; +} + +describe('native incremental search (#3421, #3423)', () => { + for (const scenario of ['context', 'property'] as const) { + it(`preserves retained ${scenario} strings and search through COPY, checkpoint and reopen`, () => { + // Own process: isolate native lifetimes and allow old-engine diagnostic runs. + const result = spawnSync(process.execPath, [reproducer, scenario], { + encoding: 'utf8', + timeout: 120_000, + maxBuffer: 2 * 1024 * 1024, + }); + expect(result.error, result.stderr).toBeUndefined(); + expect(result.signal, result.stderr).toBeNull(); + expect(result.status, result.stdout + result.stderr).toBe(0); + const report = JSON.parse(result.stdout.trim()) as { checks: Check[] }; + expect(report.checks.filter((check) => !check.ok)).toEqual([]); + const check = (name: string) => report.checks.find((entry) => entry.name === name); + expect(check('after-delete:scan')).toMatchObject({ rows: 8128, wrong: 0, missing: 0 }); + const stages = ['baseline', 'after-copy', 'after-checkpoint', 'reopened']; + if (scenario === 'property') stages.push('repair'); + for (const stage of stages) { + expect(check(`${stage}:scan`)).toMatchObject({ rows: 8192, wrong: 0, missing: 0 }); + if (scenario === 'context') { + for (const lookup of ['primary-key', 'uid', 'name']) { + expect(check(`${stage}:${lookup}`)?.ids).toEqual([ + 'Function:src/owner2.ts:unchangedSwiftTarget64', + ]); + } + } else { + expect(check(`${stage}:catalog`)?.indexes).toBe(1); + expect(check(`${stage}:fts-query`)?.ids).toEqual([ + 'Property:src/owner2.swift:property64', + ]); + for (const column of ['name', 'content', 'description']) { + expect(check(`${stage}:lower-${column}`)?.rows).toBe(8192); + } + } + } + if (scenario === 'property') { + for (const name of ['baseline:build', 'after-copy:build', 'repair:drop', 'repair:build']) { + expect(check(name)?.ok).toBe(true); + } + } + }, 150_000); + } +}); diff --git a/gitnexus/vitest.config.ts b/gitnexus/vitest.config.ts index 37c5889f3..53e9f0ba1 100644 --- a/gitnexus/vitest.config.ts +++ b/gitnexus/vitest.config.ts @@ -98,6 +98,7 @@ export default defineConfig({ 'test/integration/class-impact-all-languages.test.ts', 'test/integration/lbug-orphan-sidecar-recovery.test.ts', 'test/integration/lbug-interrupted-checkpoint-recovery.test.ts', + 'test/integration/lbug-incremental-search.test.ts', 'test/integration/lbug-readonly-init.test.ts', // Shared sibling store (#3352): each file runs real analyses and opens // the resulting LadybugDB graphs. @@ -191,6 +192,7 @@ export default defineConfig({ 'test/integration/class-impact-all-languages.test.ts', 'test/integration/lbug-orphan-sidecar-recovery.test.ts', 'test/integration/lbug-interrupted-checkpoint-recovery.test.ts', + 'test/integration/lbug-incremental-search.test.ts', 'test/integration/lbug-readonly-init.test.ts', 'test/integration/analyze-wal-checkpoint-failure.test.ts', 'test/integration/lbug-non-ascii-path.test.ts',