2 commits

Author	SHA1	Message	Date
Parafee41	316aaed928	fix(indexing): keep full text file content searchable (#2323) ... * fix(indexing): keep full text file content searchable * fix(indexing): flush CSV chunks by byte size * fix(fts): flatten newlines/tabs in indexed content so multiline files are searchable (#2317) The end-to-end FTS test (review follow-up F2) exposed that removing the 10KB cap alone does NOT fix #2317: Ladybug's FTS tokenizer splits ONLY on the space character — \n, \r, and \t are not delimiters. So multiline file/symbol content indexes as a few giant cross-line tokens that no word query matches; full content is stored but stays unsearchable. (Verified: identical 8KB content is fully searchable when space-separated and entirely unsearchable when newline-separated.) The existing fts-description-search test never caught this because all its seed content is single-line. Collapse \r\n\t -> single space in the FTS-indexed text (extractContent's File and snippet content, plus the description column) via normalizeFtsText. This rewrites the stored column too, so File content returned via the graph API is space-flattened — an accepted trade for making file/symbol text searchable. Add the real end-to-end guard test/integration/fts-fullfile-search.test.ts: write a >16KB file, load it through the real streamAllCSVsToDisk -> COPY -> createSearchFTSIndexes path, and assert searchFTSFromLbug returns a needle past 10KB (plus a short-content no-regression and a stored-cell-not-truncated guard). It drives the COPY path a Cypher-seed test would bypass, reusing withTestLbugDB's FTS-availability gating via a new before-FTS load hook. * docs(lbug): note the deliberate File-unbounded / snippet-capped asymmetry The File branch returns full content (whitespace-normalized for FTS, bounded upstream by the walker cap) while the symbol snippet path 11 lines down stays MAX_SNIPPET-capped. Comment the intent so the uncapped File branch doesn't read as a forgotten guard. No behavior change. * test(lbug): update #2203 overlap round-trip for FTS whitespace normalization The newline/tab→space normalization (a170915a, #2317) flattens stored File content, so the #2203 overlap test's "File content == original multiline source" assertion no longer holds. The test's actual invariant — overlap path == serial path, byte-for-byte — is unchanged and still asserted; BasicBlock text (not FTS-indexed) still round-trips raw. Update only the File-content expectation to the whitespace-flattened form and document why. * fix(lbug): collapse CSV flush to a single byte threshold BufferedCSVWriter flushed on row-count (FLUSH_EVERY=500) OR byte-count (FLUSH_BYTES=8MB) — two independent triggers for one job. Byte count is the only one tied to the actual risk (an unbounded buffer.join('\n') string), so drop FLUSH_EVERY and make shouldFlushCSVBuffer single-arg. Rather than tune FLUSH_BYTES by guesswork or expose it as an env knob, derive its safety margin from constants the codebase already hard-enforces: a single row is capped at TREE_SITTER_MAX_BUFFER (32MB, clamped regardless of GITNEXUS_MAX_FILE_SIZE) and at most doubled by escapeCSVField's quote-escaping, so the worst-case joined chunk (FLUSH_BYTES + 2 * TREE_SITTER_MAX_BUFFER ≈ 72MB) sits >7x under Node's MAX_STRING_LENGTH (~512MB) — the ceiling that throws RangeError: Invalid string length. A new test pins that margin numerically so it can't erode unnoticed, which covers the "configurable" alternative better than a knob would: there's no evidence any deployment needs a different value, and an unbounded env var would let an operator silently walk the margin back into the danger zone. Also updates the two tests tied to the removed row-count path: the FLUSH_EVERY-boundary integration test now crosses FLUSH_BYTES with real oversized File content instead of relying on row count, and the shouldFlushCSVBuffer unit test drops to the new single-arg signature. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-07-01 08:27:09 +01:00
Gergő Magyar	b895a20415	perf(lbug): overlap node COPY with relationship emit (#2203) (#2226) ... * test(lbug): lock PARALLEL=false as a tested correctness invariant (#2203) The parallel CSV reader (Kuzu-derived, default PARALLEL=true) cannot parse quoted fields with embedded newlines (kuzudb/kuzu#5778); our content/text columns hold source code, so PARALLEL=false is mandatory for correctness. Add a live-DB multiline-quoted round-trip that fails if it is ever flipped, plus a static guard on the generated COPY queries. Export COPY_CSV_OPTS / getCopyQuery for the static assertion; document the invariant at the source. * feat(lbug): expose node/rel phase boundary via onNodePhaseComplete hook (#2203) streamAllCSVsToDisk now fires an optional onNodePhaseComplete(nodeFiles) callback right after node CSVs are flushed and before the relationship pass writes any rel_*.csv — the boundary the COPY-overlap leg needs. The node-file manifest construction is hoisted above the rel pass and reused in the return, so output is byte-for-byte identical when no callback is supplied (verified by the emit bench fingerprint and the splitRelCsvByLabelPair differential oracle). The callback is not awaited, so the rel pass runs concurrently with the caller's node COPY. * perf(lbug): overlap node COPY with relationship emit (#2203) The deferred parallelism leg of #2203. LadybugDB is single-writer and its parallel CSV reader is unsafe for our multiline content (kuzudb/kuzu#5778), so the only safe parallelism is pipeline-overlap: node COPY (uses conn, never the rel files) runs concurrently with the relationship emit pass (writes rel_*.csv, never conn). Node COPY now starts at streamAllCSVsToDisk's onNodePhaseComplete boundary while the rel pass keeps writing; the relationship COPY still waits for node COPY (FK precondition), so DB load order and content are unchanged. - Extract copyNodeCSVs; start it in the hook (overlap) or after emit (serial). - GITNEXUS_SERIAL_LBUG_LOAD=1 forces the legacy strictly-sequential path (operator escape hatch + differential-test oracle). - Settle the in-flight node-COPY promise on emit failure (no unhandled rejection); rethrow node-COPY errors at the FK barrier. - Preserve the PDG manifest merge + collision guards (node merge at the hook, rel merge before rel COPY) and all retry/fallback/cleanup behavior. - PROF_LBUG_LOAD gains mode=overlap|serial; copy-nodes becomes the residual node-COPY time after emit (trends to 0 as overlap hides it). * test(lbug): differential gate — overlap load === serial load (#2203) Loads one fixture (multiple node tables, multiple edge pairs, multiline File content + BasicBlock text) into two fresh DBs — once via the default node-COPY ‖ rel-emit overlap, once via GITNEXUS_SERIAL_LBUG_LOAD=1 — and asserts the two databases are content-equivalent: identical per-table node counts, per-type edge counts, byte-for-byte multiline content/text, and identical insertedRels/skippedRels/warnings. This is the issue's byte-identical-content acceptance gate for the parallelism leg. * fix(review): apply autofix feedback - csv-generator: onNodePhaseComplete doc-contract now matches reality (a sync throw is allowed and is how loadGraphToLbug surfaces the manifest collision guard) — drops the inaccurate 'must not throw synchronously' line. - lbug-adapter: copyNodeCSVs totalSteps is the node-table count (drop the +1 rel-step holdover; the rel COPY has its own progress line). - lbug-adapter: on emit+node-COPY double-failure, log the swallowed node-COPY error before rethrowing the emit error (diagnosability). - lbug-load-prof test: assert mode=overlap on the default path. * fix(test): use mkdtemp for secure temp dirs (CodeQL js/insecure-temporary-file) CodeQL flagged lbug-load-overlap.test.ts writing a file into a predictable os.tmpdir() path. Create the base temp dir with fs.mkdtemp (atomic, random suffix) in both new live-DB tests, and switch to the gitnexus-lbug- prefix that TEST_FIXTURE_PREFIXES recognizes so the Windows stale-sidecar sweep covers these fixtures. * fix(lbug): check PDG manifest rel-pair collision before node COPY (#2203) Found by Codex in tri-review. The manifest rel-pair collision guard ran after the FK barrier (after node COPY committed), so on that should-never-happen error branch the overlap path left orphan node rows AND the GITNEXUS_SERIAL_LBUG_LOAD escape hatch diverged from the legacy 'validate manifest before any COPY' behavior. Move the rel merge + collision check ahead of beginNodeCopy/the barrier: the serial path now detects a collision before committing any node rows (legacy parity restored — the escape hatch is a faithful oracle again), and the overlap path detects it as early as csvResult is available. The node-collision guard already ran before node COPY (in the hook). * test(lbug): cover rel-emit failure with node COPY in flight (#2203) Resolves a P1 review gap on PR #2226: the overlap's catch(emitErr) branch (settle the in-flight node-COPY promise, then rethrow the emit error) was untested. Fault-injects via a vi.mock of streamAllCSVsToDisk that fires onNodePhaseComplete (starting a real node COPY on a live DB) then throws, asserting loadGraphToLbug rejects with the emit error and no unhandled rejection leaks. Also covers the both-fail case (node COPY error is logged, emit error still wins). Listener removed in finally; macrotask queue flushed before the assertion so it can't pass vacuously. * test(lbug): cover node-COPY hard-failure rethrow at the FK barrier (#2203) Resolves the second P1 review gap on PR #2226. Mocks emit to fire onNodePhaseComplete with a nodeFiles entry pointing at a missing CSV (a bind-time COPY error that IGNORE_ERRORS does not suppress) and otherwise succeed, so copyNodeCSVs throws, the error is captured in nodeCopyError, and loadGraphToLbug rethrows it at the FK barrier — asserted via rejects /COPY failed for File/. * test(lbug): cover PDG manifest rel-pair collision in overlap + serial (#2203) Resolves the P2 gap behind the Codex tri-review finding: the manifest rel-pair collision guard (moved ahead of node COPY in ad195582) had no test. A leaky graph with a structural BasicBlock->BasicBlock edge (routed by id-prefix, no BasicBlock nodes — isolating the rel-pair clash from the node-CSV one) plus a PdgEmitSink manifest declaring the same pair makes loadGraphToLbug reject with the rel-pair collision error, asserted on both the overlap (default) and serial (GITNEXUS_SERIAL_LBUG_LOAD=1) paths. * perf(lbug): yield the event loop periodically during relationship emit (#2203) Resolves a P2 review finding on PR #2226: the relationship-emit loop ran long synchronous stretches between write-stream drain awaits, which could starve the overlapped node-COPY callbacks on fast I/O and erode the node-COPY-||-rel-emit overlap. Yield via setImmediate every REL_YIELD_EVERY (5000) edges so the node COPY and drains get scheduling time. Scheduling-only — emit bench fingerprint unchanged (byte-identical), csv-pipeline determinism + overlap differential green. * refactor(lbug): extract shared copyCsvWithRetry helper (#2203) Resolves a P2 maintainability finding on PR #2226: the COPY + IGNORE_ERRORS retry block was duplicated in copyNodeCSVs and the inline relationship-COPY loop. Extract copyCsvWithRetry(conn, query, onError); the callback receives the RAW retry error so each site keeps its own message shape + slice length (node throws, slices 200; relationship warns + records the failed pair, slices 80). Behavior-preserving — guarded by the live-DB round-trips plus the new node-COPY-failure and overlap error-path tests. * docs(lbug): document loadGraphToLbug non-transactionality (#2203) Resolves the advisory review finding on PR #2226: loadGraphToLbug runs independent COPYs with no surrounding transaction, so a mid-load failure leaves a partial DB and recovery is a --force re-analyze. Make that contract explicit on the function so callers don't assume atomicity.	2026-06-16 10:57:26 +01:00