From 4fa40e9881531b5a5d4c11459b188f872b6cb843 Mon Sep 17 00:00:00 2001 From: Abhigyan Patwari <126312502+abhigyanpatwari@users.noreply.github.com> Date: Tue, 12 May 2026 13:14:56 +0100 Subject: [PATCH] feat(analyze): incremental indexing (parse cache + DB writeback + scope-res short-circuit) (#1479) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs: incremental indexing design spec Captures the design agreed in brainstorming on 2026-05-10: - Transitive importer closure with public-surface-change optimization - Git-only change detection (non-git repos: full rebuild as today) - New default behavior; --force opts out - New hydratePhase + loadGraphFromLbug primitive - Iterative closure expansion with parseCache reuse - incrementalInProgress dirty flag for crash recovery Prior art: PR #592 (zenprocess), PR #533 (davidbeesley), PR #1146 (azeemshaik025) — referenced and credited. Co-Authored-By: Claude Opus 4.7 (1M context) * feat(communities): seed Leiden RNG for deterministic community detection The vendored Leiden algorithm defaults to Math.random for tie-breaking and randomized walks, which produces non-deterministic community assignments and modularity values across runs on the same graph. Pass a seeded mulberry32 RNG (LEIDEN_SEED=0xC0DE) so: - The same graph always produces the same partition - Modularity values are reproducible - Equivalence tests for incremental indexing can compare community assignments byte-for-byte This is foundational for the upcoming incremental-indexing feature (see docs/superpowers/specs/2026-05-10-incremental-indexing-design.md) where the correctness contract is incremental output ≡ full rebuild output. Co-Authored-By: Claude Opus 4.7 (1M context) * feat(incremental): change-detection, surface signatures, closure expansion Three new modules supporting the incremental-indexing pipeline: * core/incremental/git-diff.ts — getChangedFilesSinceCommit() unions 'git diff lastCommit HEAD' (committed) with 'git status --porcelain' (dirty tree). Renames flattened to delete(orig) + add(new). Throws LastCommitMissingError when lastCommit is gone (caller falls back to full rebuild). * core/incremental/surface.ts — extractSurfaceSignature() produces a stable hash of a file's publicly-visible symbols (functions, classes, methods, interfaces, types, heritage). Body-only edits → same hash. Signature/heritage changes → different hash. Drives the closure scoping optimization. * core/incremental/closure.ts — computeImporterClosure() iterative fixpoint: parse each closure file, extract surface, query DB importers, expand. Uses a parseCache so each file is parsed once. Generic over TParseResult so closure logic is decoupled from the pipeline's parse representation. 32 unit tests across the three modules. Tests cover edge cases: clean tree, dirty-only, mixed, renames, deletes, multi-hop cascade, cycle termination, surface invariance, etc. Co-Authored-By: Claude Opus 4.7 (1M context) * feat(lbug): loadGraphFromLbug, queryImporters, deleteAllCommunitiesAndProcesses Three new primitives in lbug-adapter.ts to support incremental indexing: * loadGraphFromLbug(graph, unchangedFilePaths) — streams all nodes for files in the set across every hydratable node table (excludes Community/Process — graph-wide, regenerated downstream). Then loads edges where both endpoints belong to loaded nodes, excluding MEMBER_OF / STEP_IN_PROCESS edges (also graph-wide). FilePaths chunked at 200 per query to keep statement size bounded on huge repos. Endpoint-level join filters by source-side filePath in the query, target-side checked JS-side via the loadedNodeIds set. * queryImporters(targetFilePath) — returns DISTINCT a.filePath where a -[IMPORTS]-> b and b.filePath = target. Powers closure expansion: when a changed file's surface signature changes, all its importers must be re-parsed. * deleteAllCommunitiesAndProcesses() — drops Community/Process nodes (and their edges via DETACH DELETE) at the start of each incremental run so the communities/processes phases regenerate them from the fully-merged graph. Required for the 'Leiden runs on full graph' correctness invariant. Co-Authored-By: Claude Opus 4.7 (1M context) * feat(pipeline): hydrate phase + parse-filter for incremental indexing Wires the incremental-indexing infrastructure into the phase-based pipeline. Three coordinated changes: * New hydratePhase (deps: structure) — loads node/edge state for files OUTSIDE ctx.options.filesToParse from the existing LadybugDB index. Runs before parse so the parse phase can produce a partial graph while downstream phases (mro, communities, processes) still see the full graph. No-op in full-rebuild mode (filesToParse unset). * PipelineOptions.filesToParse: optional ReadonlySet. When set, parse phase filters scanned files to this set; hydrate fills the complement. Set by runFullAnalysis when it detects an eligible incremental run; never set by callers directly. * gitnexus-shared PipelinePhase enum: 'hydrate' added so progress callbacks can report the new phase distinctly from 'structure'. Phase order: scan → structure → hydrate → markdown,cobol → parse → routes,tools,orm → crossFile → scopeResolution → mro → communities → processes. Communities (Leiden) still runs on the full graph, satisfying the correctness invariant. Co-Authored-By: Claude Opus 4.7 (1M context) * feat(analyze): incremental orchestrator branch + meta schema Wires incremental indexing into runFullAnalysis. Highlights: * RepoMeta schema extended: schemaVersion, surfaceSignatures, and incrementalInProgress fields. INCREMENTAL_SCHEMA_VERSION = 1. * core/incremental/file-hash.ts — v1 surface signature: SHA-256 of file content. v2 will switch to a true surface-only signature (defined in surface.ts) so body-only edits don't expand the closure. The plumbing is signature-agnostic so the swap is local. * core/incremental/orchestrator.ts — eligibility check, closure computation (uses file-hash as the surface signal), dirty-flag management, subgraph extraction, signature merge. * run-analyze.ts adds: - hasDirtyTree() check on the existing 'lastCommit==HEAD' early-exit so an uncommitted edit triggers re-index (was a coarse equality check before). - incremental branch: try incremental first; fall through to full rebuild on any setup failure or eligibility miss. - runIncrementalBranch() — opens existing DB, deletes closure-file rows + Community/Process, runs pipeline with filesToParse, writes only the changed-subgraph back, refreshes FTS, updates meta with new surfaceSignatures and clears the dirty flag. - Full-rebuild path now populates surfaceSignatures + schemaVersion in meta.json so the next run is eligible for incremental. Crash recovery: incrementalInProgress is set BEFORE any DB mutation and cleared on success by overwriting meta.json. A crash anywhere in between leaves the flag set, and the next analyze run forces a full rebuild (cheapest path back to a known-good index). v1 limitation documented: body-only edits trigger 1-hop closure expansion (content-hash signal). True surface-only optimization is deferred to v2 — see design doc for the integration path. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(incremental): drop invalid --no-renames=false from git diff The flag --no-renames=false isn't valid git syntax (it's parsed as a file path). Git's default rename detection is on; removing the flag keeps that behavior. Caught while running an end-to-end smoke test against a small fixture repo: incremental setup failed with 'Command failed: git diff --name-status -z --no-renames=false ...'. After the fix, the incremental path runs cleanly: closure is computed, hydrate phase loads unchanged-file state from DB, parse phase only re-parses files in closure, and the writeback updates only changed nodes/edges. Co-Authored-By: Claude Opus 4.7 (1M context) * Revert v1 incremental indexing (5 commits) Reverts the v1 design that parsed only closure files into a fresh graph and tried to hydrate the rest from DB. Real-repo equivalence test failed: cross-file resolution operates on partial parse data (closure files only), so CALLS edges that resolve through unchanged files silently fall off. Diff against full rebuild on the same edited state: -50 nodes, -425 edges, -5 communities, -48 processes. Architecture pivot: switch to PR #533-style content-addressed parse cache. Pipeline parses every file (cache-served when possible), giving cross-file resolution full data, with DB writeback then restricted to changed-file rows. Reverts: d4b9de47 fix(incremental): drop invalid --no-renames=false f35f7634 feat(analyze): incremental orchestrator branch + meta schema bc039686 feat(pipeline): hydrate phase + parse-filter 98bb893d feat(lbug): loadGraphFromLbug, queryImporters, ... aa8d7ae3 feat(incremental): change-detection, surface signatures, closure Kept: d9e340b0 feat(communities): seed Leiden RNG (foundational) 8235ca36 docs: incremental indexing design spec (will be revised) Co-Authored-By: Claude Opus 4.7 (1M context) * feat(analyze): incremental DB writeback (Option B) Equivalence-preserving incremental analyze. The pipeline still parses every file (correctness invariant: cross-file resolution / scope resolution / MRO / community detection all need full graph data); the saving comes from selectively replacing only changed-file rows in LadybugDB instead of wiping and reloading the whole graph. How it works: * On every analyze, we hash all source files (SHA-256 of content) and store the map in meta.json.fileHashes alongside schemaVersion. * The next run loads the prior map and diffs: - changed: content hash differs → file's DB rows replaced. - added: not in prior map → file's DB rows inserted. - deleted: in prior map but not on disk → file's DB rows dropped. * If the diff is non-empty AND no --force / no schema mismatch / no dirty flag, take the incremental path: - Set incrementalInProgress dirty flag (BEFORE any DB mutation). - Open existing DB (no wipe). - deleteNodesForFile() for each changed/added/deleted file. - deleteAllCommunitiesAndProcesses() — Leiden regenerates these. - extractChangedSubgraph() from the in-memory ctx.graph: nodes whose filePath is in the writable set + Community + Process + edges with at least one endpoint in the writable set (edges entirely between hydrated unchanged nodes are skipped — already in DB). - loadGraphToLbug() on the subgraph. Unchanged-file rows in DB untouched. - Recreate FTS indexes. - Update meta with new fileHashes; clear dirty flag. * Otherwise full-rebuild path runs as before. Crash recovery: incrementalInProgress is the dirty flag. Set before destructive ops; cleared on success. Set on next-run startup → forces full rebuild (cheapest path back to known-good). Other changes: * Dirty-tree gate on the existing 'lastCommit==HEAD' early-return: uncommitted edits no longer slip through as 'already up to date'. * deleteAllCommunitiesAndProcesses helper in lbug-adapter. * Skip the embedding cache+restore cycle when willTryIncremental is true — embeddings stay in DB; re-inserting them would PK-conflict. End-to-end equivalence verified on this repo (993 files, 24K nodes): incremental run produces byte-identical {nodes, edges, clusters, flows} to a full rebuild from the same edited state. Speedup is currently modest (~5% on this repo) because the parse phase still runs in full. Parse-cache integration is a separate follow-up that composes cleanly on top of this work. See docs/superpowers/specs/2026-05-10-incremental-indexing-design.md. Co-Authored-By: Claude Opus 4.7 (1M context) * feat(analyze): chunk-level parse cache for full incremental speedup Composes with the incremental DB writeback (commit 27f3b49d) to deliver the major-speedup half of incremental indexing. Previously, the parse phase ran in full on every analyze; the speedup came purely from selective DB rewriting. With this commit the parse phase also reuses prior tree-sitter output for chunks whose contents haven't changed. How it works: * Cache layer (gitnexus/src/storage/parse-cache.ts): - File: /.gitnexus/parse-cache.json. Versioned, atomic write. - Key: chunk content hash = sha256(sorted(filePath:fileContentHash for each file in chunk)). - Value: ParseWorkerResult[] (raw worker output for the chunk, pre-merge). - Granularity: per chunk (~20MB byte-budget). A change to one file invalidates only its chunk — typically 1 of ~50 on a 1000-file repo (~98% cache hit ratio on a small edit). * Worker contract (gitnexus/src/core/ingestion/parsing-processor.ts): - Extracted the chunk-result merge loop into a public mergeChunkResults() so the same logic applies to live worker output AND replayed cache entries. - processParsingWithWorkers / processParsing accept an optional outRawResults out-parameter that captures worker output before merging — used by parse-impl to populate the cache after a miss. * Parse phase wiring (parse-impl.ts): - For each chunk, compute its content hash (after reading file contents). Cache hit → mergeChunkResults() on cached results, skip the worker dispatch entirely. Cache miss → run workers normally, capture raw results, store under the chunk hash. - Cache mutations happen in-place on the ParseCache passed via PipelineOptions.parseCache. * Lifecycle (run-analyze.ts): - loadParseCache() before pipeline runs. - Cache passed via runPipelineFromRepo's PipelineOptions. - saveParseCache() after the pipeline + DB writeback succeed. Equivalence verified on this repo (993 files, 24K nodes): Cold (no cache, full work): 141.1s Warm cache + 1-file edit, incremental: 63.6s ← 55% speedup Warm cache + 1-file edit, --force: 71.6s ← 49% speedup All three runs produce byte-identical {nodes, edges, clusters, flows}. The cache survives --force (content-addressed = always correct), so even forced rebuilds get the parse-skip benefit. Why chunk-level rather than per-file: workers process sub-batches and emit aggregated ParseWorkerResults. Per-file granularity would require restructuring the worker contract; chunk-level captures most of the practical speedup with no worker-side changes. Co-Authored-By: Claude Opus 4.7 (1M context) * perf(parse-impl): smaller default chunk budget (20MB→2MB) for cache granularity The parse cache is keyed at chunk granularity. With the previous 20MB budget, a typical mid-size repo (e.g. this worktree at 9MB total parseable source) fits in a single chunk — meaning ANY file change invalidates the whole chunk and re-parses every file. 2MB default produces ~5x more chunks on the same input, so a one-file edit invalidates ~1/N of cached chunks instead of the whole thing. Cold-run overhead from more chunks is <5% (one extra serialization pass per chunk). Override via GITNEXUS_CHUNK_BYTE_BUDGET env var for benchmarking. Measured on this repo (~9MB / 887 parseable files): Cold (no cache): 143s Warm cache, no source changes: 2s (early-return) Warm cache + 1-file edit: 81s (~43% off cold) Speedup is bounded by the scopeResolution phase (~58s flat regardless of parse cache) and by GitNexus's own auto-writes during analyze (AGENTS.md / .claude/skills/ etc. mutate between runs and invalidate chunks containing them). Both are addressable in follow-ups. Co-Authored-By: Claude Opus 4.7 (1M context) * perf(scope-resolution): reuse worker-produced ParsedFile + stabilize chunk order Two compounding optimizations that drop warm-cache analyze from ~134s to ~38s on a 1000-file repo (72% faster), and cold rebuild from ~143s to ~86s (40% faster) by short-circuiting work that was previously re-done. 1. SCOPE-RESOLUTION: REUSE WORKER PARSEDFILE Previously, the scope-resolution phase re-parsed every file with tree-sitter on the main thread (~58s on a 1000-file repo) because worker-produced tree-sitter Trees can't cross the worker MessageChannel. But the worker ALSO produces a artifact via , which structured-clones fine — and it's exactly what scope-resolution would re-derive. Threading those ParsedFiles through the parse phase () into ( map) lets scope- resolution skip its extract loop on a per-file basis. The fast path is bounded only by per file (cheap graph mutation). On this repo: scopeResolution went from 58s → 5s. 2. MAP-PRESERVING PARSE-CACHE SERIALIZATION is a which JSON.stringify collapses to . The first attempt at threading parsedFiles through the parse cache crashed at runtime with "importerModule.typeBindings is not iterable" because cached entries came back as plain objects. Added a JSON replacer/reviver pair in parse-cache.ts that round-trips Map and Set instances through tagged plain objects (). Symmetric: save uses replacer, load uses reviver. 3. STABLE CHUNK ORDERING The byte-budget chunker walked files in filesystem-scan order, which on Windows isn't guaranteed to be stable across runs. Even with identical source content, two scans could place files in different chunks, shifting chunk hashes and causing 100% parse-cache misses. Added a deterministic alphabetical sort on before chunking. Chunk membership is now stable across runs, so a single-file edit invalidates exactly one chunk, not all of them. Measured on this repo (993 files, 24K nodes): Cold rebuild: 86s (was 143s) Warm cache, no source changes: 3s (early-return) Warm cache + 1-file edit: 38s (was 134s) Co-Authored-By: Claude Opus 4.7 (1M context) * docs(incremental): update spec + AGENTS.md + GUARDRAILS.md for shipped design - Rewrite docs/superpowers/specs/2026-05-10-incremental-indexing-design.md to describe the architecture that actually shipped (parse cache + incremental DB writeback + scope-resolution short-circuit), with the v1 hydrate-phase post-mortem preserved as historical context. - AGENTS.md "Keeping the Index Fresh" section: note that incremental is the new default and --force is the explicit opt-out; mention the parse-cache file location and that it's safe to delete. - GUARDRAILS.md Signs: add an "Index seems corrupt or incremental is misbehaving" entry pointing users to --force as the manual escape hatch (the dirty flag handles automatic recovery). Co-Authored-By: Claude Opus 4.7 (1M context) * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(incremental): bugbot review + CI test failures Bugbot (PR #1479): - Medium: pruneCache was exported but never called -> cache grew unbounded. Wire pruneCache into run-analyze before saveParseCache, using a transient usedKeys Set on ParseCache that the parse phase populates as it processes chunks. - Low: willTryIncremental (pre-pipeline) and isIncremental (post-pipeline) could desync, silently dropping embeddings on mispredicted runs. Removed the prediction; the embedding cache now loads unconditionally when shouldLoadCache is true. The re-insert step gates on the actual isIncremental value to avoid PK-conflicts when the incremental-writeback path keeps DB rows. CI test failures: - cli-e2e #1169 + run-analyze.test.ts #1233: my dirty-tree gate on the lastCommit==HEAD early-return saw GitNexus's own auto-generated outputs (.claude/, .cursor/, AGENTS.md, CLAUDE.md) as dirty, perpetually defeating the up-to-date fast path. Extended the pathspec exclusion to cover all auto-gen outputs, not just .gitnexus/. - ruby field-type disambig: my chunk-stability sort exposed a pre-existing order-dependency in Ruby cross-file resolution (`user.address.save -> Address#save` only resolves correctly when user.rb parses before address.rb in some configurations). Removed the sort. Filesystem ordering is stable enough in practice that the parse cache still hits the common case; the pre-existing fragility is left for a separate fix. - pipeline-graph-golden: regenerated. Seeded Leiden RNG produces a partition different from the previous Math.random snapshot. - staleness `parallel calls` was a CI timing flake; passes locally. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(incremental): re-insert cached embeddings on incremental path Bugbot re-review caught: deleteNodesForFile cascades to the CodeEmbedding table (DELETE WHERE e.nodeId STARTS WITH ...), so changed-file embedding rows are wiped along with their nodes. The previous fix gated re-insert on `!isIncremental`, which silently dropped those embeddings — a regression versus the full-rebuild path's "preserve embeddings by default" guarantee. Remove the `!isIncremental` gate. The per-batch try/catch already handles the unchanged-file PK-conflict case ("some may fail if node was removed, that's fine") with the same semantics, so re-inserting the full cached set on incremental works: - changed-file rows: deleted, then re-inserted from cache (preserved) - unchanged-file rows: still in DB, re-insert PK-conflicts and is silently ignored (existing rows are correct) Cost: re-inserting ~24K embeddings on incremental when only a few files changed — most are no-op conflicts. Bounded by batch size of 200; ~3-5s overhead. Worth it for correctness. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(incremental): address Claude+Bugbot review findings + remove design doc Addresses CHANGES_REQUESTED review on PR #1479: 1. Remove docs/superpowers/specs/2026-05-10-incremental-indexing-design.md per maintainer request. 2. BLOCKER (Claude Finding 1, Bugbot Round 3): Stale cross-file edges between unchanged files. extractChangedSubgraph excluded edges where both endpoints were unchanged-file nodes — when a barrel/re-export file changes, cross-file resolution may update CALLS edges between two unchanged files that would then be silently lost. Fix: 1-hop importer-closure expansion of the writable set in run-analyze.ts. Before deleting/rewriting rows, query DB for importers of every changed/deleted file and add them to the writable set. Their nodes get deleted+rewritten too, so cross-file's refined edges land in the DB. Re-added queryImporters to lbug-adapter.ts. 3. BLOCKER (Claude Finding 3): Parse cache key omitted parser version. After a GitNexus upgrade, the cache silently replays pre-upgrade ParseWorkerResults against the new schema → wrong CALLS/IMPORTS/ scope edges with no visible signal. Fix: PARSE_CACHE_VERSION now embeds the gitnexus npm package version (read at module load via createRequire on package.json). Format: `${SCHEMA_BUMP}+${PKG_VERSION}` e.g. "1+1.6.4". Any release that bumps package.json automatically invalidates the on-disk cache. Mismatched versions fall through to an empty cache (next save overwrites with the new version baked in). 4. BLOCKER (Claude Finding 2): No automated tests for incremental behavior. Added 28 unit tests across 3 files: - incremental-file-hash.test.ts (10 tests) diffFileHashes classification, computeFileHash determinism, computeFileHashes batch / missing-file tolerance, sorted output. - incremental-parse-cache.test.ts (12 tests) computeChunkHash stability and order-independence, version prefix format, pruneCache, load/save round-trip on empty / missing / corrupt / version-mismatched files, AND a Map/Set round-trip test that pins the JSON replacer/reviver behaviour (without it, ParsedFile.scopes[*].typeBindings collapses to {} and downstream `.get()` / iteration throws). - incremental-subgraph-extract.test.ts (6 tests) writable-set node inclusion, Community/Process always kept, edge inclusion when at least one endpoint is writable, MEMBER_OF edges via graph-wide endpoints, empty subgraph case. 5. Medium (Claude Finding 6): AGENTS.md "Keeping the Index Fresh" said "only changed files are re-parsed." Imprecise — the pipeline parses every file every run; the cache skips tree-sitter for chunks whose contents haven't changed. Reworded to match the design doc. Test plan still expects: [x] Typecheck clean [x] All 28 new unit tests pass [x] All previously-failing tests still pass on the rebased branch [x] Equivalence verified locally (incremental ≡ --force, byte-identical stats on this repo) Co-Authored-By: Claude Opus 4.7 (1M context) * fix(incremental): round 3 review feedback — bounded BFS, atomic meta, integration test, docs Addresses remaining findings on PR #1479 from Claude's re-review of commit ad7bd31 + verifies the outstanding Bugbot HIGH severity. 1. F1 — Transitive importer expansion (Claude, was Medium-but-noted). Previous 1-hop importer expansion missed barrel re-export chains (A imports C, C re-exports B; when B changes, only C was pulled in — A was left with potentially-stale CALLS edges to refined targets). Replaced the single pass with a bounded BFS over the IMPORTS graph (depth ≤ 4). Catches nested barrel pyramids without ballooning into a near-full rebuild on monorepos with deep re-export trees. `--force` remains the escape hatch documented in GUARDRAILS.md for cases that exceed the bound. 2. F2 — Integration test for incremental orchestration (Claude, BLOCKER, DoD §2.7). The unit tests added in ad7bd31 covered `diffFileHashes`, `extractChangedSubgraph`, `computeChunkHash`, `pruneCache`, and the Map/Set JSON round-trip — but none of them exercised the real `runFullAnalysis` orchestration. Added gitnexus/test/unit/ incremental-orchestration.test.ts with four end-to-end tests against a real git-initialized fixture repo + real LadybugDB: a. First run populates fileHashes + schemaVersion and clears incrementalInProgress on success. b. Second run on unchanged state takes the alreadyUpToDate fast path (early-return). c. Second run after a source edit takes the incremental path (not full rebuild) and rotates fileHashes for the touched file while keeping the dirty flag cleared. d. A pre-set incrementalInProgress flag forces a full rebuild that clears it (crash-recovery wire). These would catch any regression that wires `isIncremental` from a pre-pipeline prediction (the Bugbot finding from commit 5eb0597) or accidentally re-gates the embedding re-insert on `!isIncremental` (the Bugbot finding from commit 60c10f1). 3. F3 — GUARDRAILS.md docs accuracy (Claude, Low). Line 33 still said "only changed files are re-parsed" — AGENTS.md was already corrected in ad7bd31 but GUARDRAILS.md was missed. Reworded to match. 4. F5 — Atomic saveMeta (Claude, Medium; vvladescu-tb fork). The dirty flag (`incrementalInProgress`) travels through meta.json. A crash mid-write would leave a corrupt meta.json that `loadMeta` would silently treat as "no prior index", losing the flag and skipping recovery. Switched to tmp-file + rename matching saveParseCache. 5. Bugbot's "Subgraph edges reference nodes absent from subgraph" (HIGH severity). Verified as FALSE POSITIVE: `getNodeLabel` in lbug-adapter.ts derives labels from the node-ID string (parses the table prefix), not from the in-memory graph. The CSV generator writes (src_id, dst_id, type) rows without consulting node objects; `splitRelCsvByLabelPair` routes by ID-derived label; `COPY ... (from=X, to=Y)` resolves both endpoints against the live LadybugDB where unchanged-file nodes still exist. No fix needed. All 213 tests pass locally (including the 4 new integration tests and the previously-failing CI tests). Co-Authored-By: Claude Opus 4.7 (1M context) * fix(incremental): address Bugbot round-4 findings (added-file shadow seed + dedupe) Bugbot review on commit e23e4400 surfaced two new findings against the incremental writeback in run-analyze.ts: HIGH — Incremental BFS misses importers of newly added files. queryImporters() reads the pre-pipeline DB. For a NEWLY ADDED file there are no IMPORTS rows pointing to it yet, so unchanged files whose pre-existing import statements now resolve to the newcomer keep stale CALLS edges pointing at the OLD resolution target. LOW — Deleted files double-counted in filesToDelete. hashDiff.deleted entries can reappear in writableFiles via the BFS expansion (queryImporters can return a now-deleted path), so deleteNodesForFile() ran twice for the same file. Fixes: - Add gitnexus/src/core/incremental/shadow-candidates.ts: derive the pre-existing file paths whose JS/TS module-resolution claim an added file can steal. Pattern catalogue: same-basename/ different-extension, bare-file-beats-directory-index, and directory-index-beats-bare-file. Emit both POSIX and Windows separators because the prior fileHashes map may have been written from either OS. - In run-analyze.ts, seed the BFS frontier with shadow candidates that exist in the prior meta.fileHashes. Their importers — found via queryImporters — get pulled into the writable set so their CALLS edges re-resolve against the new file. - Dedupe filesToDelete via Set to avoid the double-call. Tests: gitnexus/test/unit/incremental-shadow-candidates.test.ts — 8 cases covering each shadow pattern, separator handling, .d.ts as a single extension token, deduplication, and the no-self-shadow invariant. All 40 incremental tests (file-hash, parse-cache, subgraph-extract, shadow-candidates, orchestration) pass locally. Note on the third Bugbot finding ("Subgraph edges reference nodes absent from subgraph"): re-anchored from a prior review pass — the code at subgraph-extract.ts:48 is unchanged. Already verified as a false positive: getNodeLabel parses labels from ID strings, CSV write is by ID, and COPY resolves against the live DB. * chore(autofix): apply prettier + eslint fixes via /autofix command * test(incremental): exact-equality stats invariant + analyze ≡ analyze --force Addresses the only remaining Claude production-readiness review finding on PR #1479 (Low-Medium, test-quality only — Claude itself said it does NOT block merge, but the central PR claim "incremental ≡ full rebuild" deserves explicit CI coverage rather than implicit trust). Changes to gitnexus/test/unit/incremental-orchestration.test.ts: 1) Tighten the existing "comment-only edit takes incremental path" test. - Replace toBeGreaterThan(0) bounds assertions on stats.files and stats.nodes with exact toBe(firstMeta) per-field equality across files / nodes / edges / communities / processes. DoD §2.7 calls out bounds-only assertions as masking regressions that drop half the graph; this swap closes that gap. - Rationale: a comment-only edit must change the file content hash (driving the incremental path) without changing any graph data. Therefore every stat MUST be identical to the first run. Anything else is a regression. 2) New test: incremental output is byte-equivalent to a full rebuild. - Run analyze → comment-only edit → analyze (incremental writeback) → analyze --force (full rebuild from same on-disk state). - Assert files / nodes / edges / communities / processes are exactly equal across the incremental and the --force passes. - This is the PR's central correctness contract, now proven by a test that exercises the real runtime path end-to-end against a real on-disk LadybugDB. All 5 orchestration tests pass locally (52s), including the new equivalence test — every stat field matches exactly between incremental and --force on the mini-repo fixture. tsc --noEmit clean. * fix(incremental): F1 cross-file edge consistency + F4 stable chunk sort + unit coverage (#1511) Patch addressing two of the still-open changes-requested findings on PR #1479, rebased onto the current feat/incremental-indexing head. F3 (parser fingerprint in the cache key), F5 (atomic saveMeta), and F6 (AGENTS.md phrasing) were already handled on the branch, so the corresponding parts of the original patch were dropped as redundant. F1 (Blocker) — Cross-file edges between unchanged files Adds `computeEffectiveWriteSet(graph, toWriteSet)` to subgraph-extract.ts: a single pass over the new graph's edges that pulls the unchanged-side file of every writable-boundary-crossing edge into the write set. run-analyze composes it ON TOP of the existing importer-BFS expansion and feeds the combined set to BOTH `deleteNodesForFile` and `extractChangedSubgraph`, so the delete cascade and the writeback subgraph cover identical files (asymmetry would leave stale rows or PK-conflict at COPY time). The BFS reads IMPORTS from the pre-pipeline DB (catches files that *stopped* importing a changed file); the edge walk reads the new graph (catches refined CALLS edges the pre-run DB couldn't predict, e.g. a barrel re-export shifting a symbol from B to D). `extractChangedSubgraph` stays a pure filter — all expansion is the orchestrator's job. F4 (Medium) — Restore alphabetical chunk sort `parseableScanned` is sorted before chunking. Filesystem-scan order isn't stable enough across runs/platforms (notably macOS APFS) to keep chunk hashes consistent, so the parse cache thrashes without it. The pre-existing Ruby cross-file resolution order-dependency the old comment cited is independent — the sort surfaces it but doesn't cause it; tracked separately rather than leaving the cache cold. Tests — incremental-subgraph-extract.test.ts Locks the F1 invariants: `extractChangedSubgraph` is a pure filter (includes only the set it's given, plus graph-wide nodes; edges fire on one writable endpoint), and `computeEffectiveWriteSet` covers the barrel-re-export scenario, the symmetric edge-into- changed-file case, the no-boundary-crossed no-op, graph-wide-node edges, and input-immutability. Supersedes the prior extractChangedSubgraph-only test file on the branch. Co-authored-by: Val Vladescu * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(call-processor): register properties in pre-pass to fix order-dependent field type disambiguation + regenerate golden snapshot Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2d66666f-861c-432e-a4b0-11f2aefca98a Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(call-processor): port worker-path property enrichment into the sequential pre-pass Copilot's pre-pass in 8184439 fixed the Ruby attr_accessor order-dependence, but it copied the OLD in-loop registration logic, not the canonical worker path in parse-worker.ts. That left the sequential and worker paths emitting non-identical Property nodes/symbols for the same source — silently breaking the `incremental ≡ --force` invariant the moment a repo crosses the worker threshold between runs. Two concrete divergences are closed here: * Node id: worker keys Property as `${file}:${className}.${propName}` (qualified). Pre-pass was using `${file}:${propName}` (unqualified). Same source produced different graph ids depending on which path ran. * Field metadata: worker enriches each routed property with `provider.fieldExtractor` + `getFieldInfo`, falling back to `routedFieldInfo.type` for `declaredType` when the routing payload lacks one (e.g. types discovered from `@address = Address.new` ctor assignments rather than YARD `@return [Type]`), and propagates `visibility` / `isStatic` / `isReadonly`. Pre-pass did none of this, so on the sequential path `resolveFieldAccessType` failed to walk chains where the type only came from the FieldExtractor. The pre-pass now mirrors parse-worker.ts:1803-1898 verbatim, with one deliberate difference: the FieldInfo cache is scoped to a single `processCalls` invocation rather than module-level (the worker process is short-lived; the main thread is not, and a module-level cache would leak state between analyze runs). Also drops the now-stale "Defer resolution: Ruby attr_accessor properties are registered during this same loop" comment on `pendingWrites.push` — the rationale is no longer accurate after Copilot's pre-pass, but the deferral is still needed so write-access tracking sees inference that completes during the main loop. Comment updated to reflect that. Verification: * `tsc --noEmit`: 0 errors * test/unit (call-processor, call-routing, field-extraction, ruby-self-call): 224 passing * test/integration (ruby, ruby-sequential-mixin, pipeline-graph-golden): 137 passing Co-Authored-By: Claude Opus 4.7 (1M context) * fix(call-processor): key fieldInfoCache by filePath:startIndex, not raw byte offset Claude's review of 255bdf6 caught a real collision in the FieldInfoCache I added: keying by `classNode.startIndex` alone is a per-file byte offset, so two files that both begin with a class at byte 0 — extremely common in Ruby / Python, where files frequently open with `class Foo`, `module Foo` — collide on the same cache entry. The second file's `getFieldInfo` then returns the first file's FieldInfo map, producing wrong `declaredType` / `visibility` / `isReadonly` on its properties. Same shape as the bug that already exists in parse-worker.ts:377 (also keyed by `classNode.startIndex` in a module-level map, persistent across files processed by the same worker). Fixing the symmetric pre-existing leak in parse-worker.ts is a separate, scoped follow-up — left out of this commit to keep the fix minimal and reviewable. Cache map and key are now both string-typed. Composite key `${context.filePath}:${classNode.startIndex}` keeps the within-file hit rate (one FieldExtractor.extract() per class regardless of how many `attr_accessor` lines it has) while eliminating cross-file aliasing. Verification on the patched HEAD: * `tsc --noEmit`: 0 errors * test/unit (call-processor, call-routing, field-extraction, ruby-self-call): 224 passing * test/integration (ruby, ruby-sequential-mixin, pipeline-graph-golden): 137 passing Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) Co-authored-by: Gergő Magyar Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Val Vladescu Co-authored-by: Val Vladescu Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> --- AGENTS.md | 7 +- GUARDRAILS.md | 8 +- .../src/core/incremental/shadow-candidates.ts | 76 ++++ .../src/core/incremental/subgraph-extract.ts | 123 ++++++ gitnexus/src/core/ingestion/call-processor.ts | 224 ++++++++-- .../src/core/ingestion/community-processor.ts | 19 + .../src/core/ingestion/parsing-processor.ts | 177 ++++---- .../ingestion/pipeline-phases/parse-impl.ts | 175 ++++++-- .../core/ingestion/pipeline-phases/parse.ts | 14 + gitnexus/src/core/ingestion/pipeline.ts | 13 + .../scope-resolution/pipeline/phase.ts | 15 +- .../scope-resolution/pipeline/run.ts | 51 ++- gitnexus/src/core/lbug/lbug-adapter.ts | 71 ++++ gitnexus/src/core/run-analyze.ts | 383 ++++++++++++++++-- gitnexus/src/storage/file-hash.ts | 104 +++++ gitnexus/src/storage/parse-cache.ts | 213 ++++++++++ gitnexus/src/storage/repo-manager.ts | 47 ++- .../mini-repo/expected-graph.json | 2 +- .../test/unit/incremental-file-hash.test.ts | 124 ++++++ .../unit/incremental-orchestration.test.ts | 263 ++++++++++++ .../test/unit/incremental-parse-cache.test.ts | 243 +++++++++++ .../incremental-shadow-candidates.test.ts | 75 ++++ .../unit/incremental-subgraph-extract.test.ts | 169 ++++++++ 23 files changed, 2409 insertions(+), 187 deletions(-) create mode 100644 gitnexus/src/core/incremental/shadow-candidates.ts create mode 100644 gitnexus/src/core/incremental/subgraph-extract.ts create mode 100644 gitnexus/src/storage/file-hash.ts create mode 100644 gitnexus/src/storage/parse-cache.ts create mode 100644 gitnexus/test/unit/incremental-file-hash.test.ts create mode 100644 gitnexus/test/unit/incremental-orchestration.test.ts create mode 100644 gitnexus/test/unit/incremental-parse-cache.test.ts create mode 100644 gitnexus/test/unit/incremental-shadow-candidates.test.ts create mode 100644 gitnexus/test/unit/incremental-subgraph-extract.test.ts diff --git a/AGENTS.md b/AGENTS.md index 60317d73d..1346facc9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -149,11 +149,16 @@ Indexed as **GitNexus** (4325 symbols, 10556 relationships, 300 execution flows) ## Keeping the Index Fresh ```bash -npx gitnexus analyze # basic refresh; preserves any existing embeddings +npx gitnexus analyze # incremental by default; preserves embeddings +npx gitnexus analyze --force # full rebuild from scratch (opt out of incremental) npx gitnexus analyze --embeddings # also generate embeddings for new/changed nodes npx gitnexus analyze --drop-embeddings # explicit opt-in to wipe existing embeddings ``` +`analyze` runs **incrementally by default**. The pipeline still parses every file every run (cross-file resolution requires it), but tree-sitter parsing is **served from a content-addressed cache** at `.gitnexus/parse-cache.json` for chunks whose file contents haven't changed since the last run. Only changed-file rows (and their importers) are rewritten in LadybugDB; unchanged-file rows are preserved. Output is byte-equivalent to a full rebuild. Pass `--force` to wipe and re-index from scratch (e.g., to recover from a corrupt index, or after upgrading GitNexus). + +The parse cache key is **content-addressed and version-tagged**: it survives `--force` runs, and is automatically invalidated by a `gitnexus` package upgrade (so a new tree-sitter grammar doesn't silently replay stale parse output). Safe to delete `.gitnexus/parse-cache.json` at any time — it'll be rebuilt on the next analyze. + Check `.gitnexus/meta.json` `stats.embeddings` (0 = none). A plain `analyze` no longer drops existing vectors — pass `--drop-embeddings` to wipe. > Claude Code: PostToolUse hook detects a stale index after `git commit` and `git merge` and prompts the agent to run `analyze`. The hook does not invoke `analyze` itself. diff --git a/GUARDRAILS.md b/GUARDRAILS.md index 1cc032759..c09f0319a 100644 --- a/GUARDRAILS.md +++ b/GUARDRAILS.md @@ -30,9 +30,15 @@ Format: **Trigger → Instruction → Reason**. Append new Signs when the same m ### Stale graph after edits - **Trigger:** MCP warns index is behind `HEAD`, or search doesn't match latest commit. -- **Do:** `npx gitnexus analyze` (plus `--embeddings` if used). +- **Do:** `npx gitnexus analyze` (plus `--embeddings` if used). Runs incrementally by default — the pipeline parses every file every run (cross-file resolution requires it), but tree-sitter dispatch is skipped for unchanged file chunks via the content-addressed cache, and only changed-file rows (plus their importers, transitively) are rewritten in LadybugDB. - **Why:** Tools query LadybugDB from last analyze; git changes are invisible until re-indexed. +### Index seems corrupt or "incremental" is misbehaving + +- **Trigger:** `analyze` produces unexpected results, or `meta.json.incrementalInProgress` is set, or the index is in a half-state after a crash. +- **Do:** `npx gitnexus analyze --force` to rebuild from scratch. The dirty-flag check forces this automatically when a previous incremental run didn't complete cleanly, but `--force` is the manual escape hatch. Safe to delete `.gitnexus/parse-cache.json` at any time — content-addressed, will be regenerated. +- **Why:** Incremental writeback is selective DB row replacement; if the on-disk state is inconsistent for any reason, a full rebuild is the cheapest path back to a known-good index. + ### Embeddings vanished after analyze - **Trigger:** Semantic search quality drops; `stats.embeddings` in `meta.json` is 0 after refresh. diff --git a/gitnexus/src/core/incremental/shadow-candidates.ts b/gitnexus/src/core/incremental/shadow-candidates.ts new file mode 100644 index 000000000..415a6d9df --- /dev/null +++ b/gitnexus/src/core/incremental/shadow-candidates.ts @@ -0,0 +1,76 @@ +/** + * Shadow-candidate path derivation for incremental indexing. + * + * Background — Bugbot review on PR #1479: + * queryImporters() on a NEWLY ADDED file returns 0 importers in the + * pre-pipeline DB, because the new file's IMPORTS rows haven't been + * written yet. But pre-existing files may have IMPORTS edges that + * *resolved to a sibling path*, and the newcomer can now steal that + * resolution under standard JS/TS module-resolution rules. Without + * pulling those pre-existing files into the writable set, their + * stale CALLS edges remain pointing at the OLD resolution target. + * + * Given an added file path, this helper enumerates the pre-existing + * file paths whose import-resolution claim the newcomer can steal. + * Caller filters the candidates against the prior-run `fileHashes` + * map so we only query importers of paths that actually existed. + * + * Shadow patterns covered (resolution-priority-aware): + * + * (a) Same basename, different extension — + * added `foo/bar.ts` shadows `foo/bar.{tsx,js,jsx,mjs,cjs,d.ts}`. + * (b) Bare-file beats directory-style index — + * added `foo/bar.ts` shadows `foo/bar/index.{ts,tsx,...}`. + * (c) Directory-index beats bare-file — + * added `foo/index.ts` shadows `foo.{ts,tsx,...}` (rare but real, + * e.g. converting a single-file module into a directory module). + * + * Resolution-order priority is conservatively wide: we enumerate ALL + * common extensions because we don't know which the importer actually + * specified, and over-seeding is harmless (extra BFS work, but the + * subgraph extract still gates write-back by file membership). + * + * Cross-platform path separators: candidates are emitted with both `/` + * and `\` for shadow pattern (b), since the caller's prior fileHashes + * map may use either depending on the OS that wrote it. + */ + +const SHADOW_EXTS = ['.d.ts', '.tsx', '.ts', '.jsx', '.js', '.mjs', '.cjs']; + +/** + * Enumerate pre-existing paths whose import-resolution `added` can steal. + * + * @param added — repo-relative path of a newly-added file + * @returns deduplicated list of candidate paths (NOT filtered against + * any known-files set — caller does that) + */ +export const shadowCandidatesFor = (added: string): string[] => { + const ext = SHADOW_EXTS.find((e) => added.endsWith(e)); + if (!ext) return []; + + const noExt = added.slice(0, -ext.length); + const out = new Set(); + + // (a) Same basename, different extension. + for (const alt of SHADOW_EXTS) { + if (alt !== ext) out.add(noExt + alt); + } + + // (b) Bare file beats sibling directory-style index. + for (const idx of SHADOW_EXTS) { + out.add(`${noExt}/index${idx}`); + out.add(`${noExt}\\index${idx}`); + } + + // (c) New `foo/index.ext` shadows old `foo.ext`. + const idxSuffixSlash = '/index'; + const idxSuffixBack = '\\index'; + let dir: string | null = null; + if (noExt.endsWith(idxSuffixSlash)) dir = noExt.slice(0, -idxSuffixSlash.length); + else if (noExt.endsWith(idxSuffixBack)) dir = noExt.slice(0, -idxSuffixBack.length); + if (dir !== null) { + for (const alt of SHADOW_EXTS) out.add(dir + alt); + } + + return [...out]; +}; diff --git a/gitnexus/src/core/incremental/subgraph-extract.ts b/gitnexus/src/core/incremental/subgraph-extract.ts new file mode 100644 index 000000000..71fe656be --- /dev/null +++ b/gitnexus/src/core/incremental/subgraph-extract.ts @@ -0,0 +1,123 @@ +/** + * Subgraph extraction for incremental DB writeback. + * + * Given the FULL ctx.graph produced by the pipeline (all files parsed, + * all phases run) and the set of file paths whose DB rows must be + * replaced, produce a smaller KnowledgeGraph that contains: + * + * - Every node whose `properties.filePath` is in `toWriteSet`. + * - Every graph-wide node (Community, Process) — these are regenerated + * each run by the communities/processes phases and must be fully + * rewritten. + * - Every relationship where AT LEAST ONE endpoint is in the writable + * set above. Relationships entirely between unchanged-file nodes + * are skipped — their rows are still in the DB and re-inserting + * them would PK-conflict at COPY time. + * + * The resulting subgraph is what gets passed to `loadGraphToLbug` after + * the orchestrator has deleted the corresponding DB rows. Hydrated + * unchanged-file rows are never touched in the DB. + * + * # Cross-file edge consistency (Finding 1) + * + * `extractChangedSubgraph` intentionally does NOT expand the set it is + * given — expansion is the orchestrator's job, so the SAME expanded set + * can be fed to both `deleteNodesForFile` and this function (asymmetry + * between the delete set and the write set silently corrupts the DB). + * `computeEffectiveWriteSet` below performs the boundary-crossing 1-hop + * walk; the orchestrator composes it with its importer-BFS expansion and + * passes the result here. + * + * Why the 1-hop walk is needed: consider a barrel re-export change — + * file C (a barrel) shifts `export { foo } from './b'` to + * `export { foo } from './d'`. After scope resolution, file A's CALLS + * edge to `foo` resolves to D instead of B, even though A's content is + * byte-for-byte identical: + * + * - Old A→B edge survives in DB (neither A nor B is changed → not deleted) + * - New A→D edge is missing (neither A nor D in writable set → skipped) + * + * Pulling the unchanged-side file of every writable-boundary-crossing + * edge into the write set fixes both halves: the orchestrator's + * `DETACH DELETE` cleans up the stale unchanged-side rows, and the new + * cross-file edges land because at least one endpoint is now writable. + * + * Limitation (documented): if a file X *stopped* importing from a + * changed file C, X has no edge to C in the new graph, so this 1-hop + * walk doesn't catch it. The orchestrator's importer-BFS (which reads + * IMPORTS from the pre-pipeline DB) covers that case instead. + */ + +import type { GraphNode, GraphRelationship } from 'gitnexus-shared'; +import { createKnowledgeGraph } from '../graph/graph.js'; +import type { KnowledgeGraph } from '../graph/types.js'; + +const isGraphWide = (label: string): boolean => label === 'Community' || label === 'Process'; + +/** + * Build a Map for every File-bound node in the graph. + * Graph-wide nodes (Community/Process) have no filePath and are filtered. + */ +const indexNodeFilePaths = (fullGraph: KnowledgeGraph): Map => { + const idx = new Map(); + fullGraph.forEachNode((n: GraphNode) => { + const fp = n.properties?.filePath as string | undefined; + if (fp) idx.set(n.id, fp); + }); + return idx; +}; + +export const extractChangedSubgraph = ( + fullGraph: KnowledgeGraph, + toWriteSet: ReadonlySet, +): KnowledgeGraph => { + const sub = createKnowledgeGraph(); + const writableNodeIds = new Set(); + + fullGraph.forEachNode((n: GraphNode) => { + const filePath = n.properties?.filePath as string | undefined; + const include = (filePath && toWriteSet.has(filePath)) || isGraphWide(n.label); + if (include) { + sub.addNode(n); + writableNodeIds.add(n.id); + } + }); + + fullGraph.forEachRelationship((r: GraphRelationship) => { + if (writableNodeIds.has(r.sourceId) || writableNodeIds.has(r.targetId)) { + sub.addRelationship(r); + } + }); + + return sub; +}; + +/** + * Public — derive the EFFECTIVE write-set: `toWriteSet` expanded by one + * hop along every edge in the new graph that crosses the writable + * boundary (one endpoint in a writable file, the other in an unchanged + * file). The unchanged-side file is pulled in so its stale rows are + * deleted + rewritten in lockstep with the changed side. + * + * Single pass over the edge list. Does NOT mutate `toWriteSet`. The + * orchestrator MUST feed the returned set to both `deleteNodesForFile` + * and `extractChangedSubgraph` — feeding the unexpanded set to either + * one leaves stale rows or PK-conflicts at COPY time. + */ +export const computeEffectiveWriteSet = ( + fullGraph: KnowledgeGraph, + toWriteSet: ReadonlySet, +): Set => { + const nodeFilePaths = indexNodeFilePaths(fullGraph); + const expanded = new Set(toWriteSet); + fullGraph.forEachRelationship((r: GraphRelationship) => { + const sourcePath = nodeFilePaths.get(r.sourceId); + const targetPath = nodeFilePaths.get(r.targetId); + if (!sourcePath || !targetPath) return; // skip edges to graph-wide nodes + const sourceWritable = toWriteSet.has(sourcePath); + const targetWritable = toWriteSet.has(targetPath); + if (sourceWritable && !targetWritable) expanded.add(targetPath); + else if (targetWritable && !sourceWritable) expanded.add(sourcePath); + }); + return expanded; +}; diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index 9fa6c1ae5..b45478f7a 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -42,12 +42,14 @@ import { isVerboseIngestionEnabled } from './utils/verbose.js'; import { yieldToEventLoop } from './utils/event-loop.js'; import { parseSourceSafe } from '../tree-sitter/safe-parse.js'; import { + CLASS_CONTAINER_TYPES, FUNCTION_NODE_TYPES, - findEnclosingClassId, findEnclosingClassInfo, genericFuncName, inferFunctionLabel, } from './utils/ast-helpers.js'; +import type { FieldInfo, FieldExtractorContext } from './field-types.js'; +import type { LanguageProvider } from './language-provider.js'; import { typeTagForId, constTagForId, buildCollisionGroups } from './utils/method-props.js'; import type { MethodInfo } from './method-types.js'; import { @@ -77,6 +79,62 @@ import type { LiteralTypeInferrer } from './type-extractors/types.js'; import type { SyntaxNode } from './utils/ast-helpers.js'; import { logger } from '../logger.js'; + +// ── Property-prepass helpers (parity with parse-worker.ts) ── +// These mirror the sequential-path equivalents in parse-worker.ts so the main- +// thread `processCalls` pre-pass produces byte-identical Property nodes/symbols +// to the worker pool. Drift between the two paths breaks the +// `incremental ≡ --force` invariant the moment a repo crosses the worker +// threshold between runs. + +/** Walk up to the nearest enclosing class/struct/interface AST node. */ +const findEnclosingClassNode = (node: SyntaxNode): SyntaxNode | null => { + let current = node.parent; + while (current) { + if (CLASS_CONTAINER_TYPES.has(current.type)) return current; + current = current.parent; + } + return null; +}; + +/** No-op SymbolTable stub for FieldExtractorContext — matches parse-worker. */ +const NOOP_SYMBOL_TABLE: SymbolTableReader = { + lookupExact: () => undefined, + lookupExactFull: () => undefined, + lookupExactAll: () => [], + lookupCallableByName: () => [], + getFiles: () => [][Symbol.iterator](), + getStats: () => ({ fileCount: 0 }), +}; + +/** + * Extract (and cache) field info for a class node. Cache is passed in so it + * stays scoped to a single `processCalls` invocation rather than leaking + * across analyze runs (worker uses module-level caching because each worker + * process is short-lived; the main thread is not). + * + * Cache key is `${filePath}:${classNode.startIndex}` — startIndex alone is a + * per-file byte offset, so almost every Ruby/Python file's leading class lands + * at byte 0 and would collide across files in the shared map. + */ +const getFieldInfo = ( + classNode: SyntaxNode, + provider: LanguageProvider, + context: FieldExtractorContext, + cache: Map>, +): Map | undefined => { + if (!provider.fieldExtractor) return undefined; + const cacheKey = `${context.filePath}:${classNode.startIndex}`; + const cached = cache.get(cacheKey); + if (cached) return cached; + const result = provider.fieldExtractor.extract(classNode, context); + if (!result?.fields?.length) return undefined; + const map = new Map(); + for (const field of result.fields) map.set(field.name, field); + cache.set(cacheKey, map); + return map; +}; + /** Per-file resolved type bindings for exported symbols. * Populated during call processing, consumed by Phase 14 re-resolution pass. */ export type ExportedTypeMap = Map>; @@ -860,6 +918,120 @@ export const processCalls = async ( prepared.push({ file, language, provider, tree, matches, parentMap, typeEnv }); } + // ── Property-registration pre-pass ── + // Register all routed properties (e.g. Ruby attr_accessor) BEFORE the + // resolution loop so cross-file field-type lookups (e.g. + // `user.address.save → Address#save`) succeed regardless of file + // processing order. This MUST stay in lockstep with the equivalent + // worker-path block in parse-worker.ts (kind === 'properties') — any + // divergence between the two paths breaks the `incremental ≡ --force` + // invariant once a repo crosses the worker threshold between runs. + const fieldInfoCache = new Map>(); + for (const { file, language, provider, matches, typeEnv } of prepared) { + const callRouter = provider.callRouter; + if (!callRouter) continue; + matches.forEach((match) => { + const captureMap: Record = {}; + match.captures.forEach((c) => (captureMap[c.name] = c.node)); + if (!captureMap['call']) return; + const callNameNode = captureMap['call.name']; + if (!callNameNode) return; + const routed = callRouter(callNameNode.text, captureMap['call']); + if (!routed || routed.kind !== 'properties') return; + + const propEnclosingInfo = findEnclosingClassInfo( + captureMap['call'], + file.path, + provider.resolveEnclosingOwner, + ); + const propEnclosingClassId = propEnclosingInfo?.classId ?? null; + + // Enrich routed properties with FieldExtractor metadata so types + // discovered from constructor assignments (e.g. `@address = Address.new`) + // are propagated even when the routing payload itself lacks declaredType. + let routedFieldMap: Map | undefined; + if (provider.fieldExtractor && typeEnv) { + const classNode = findEnclosingClassNode(captureMap['call']); + if (classNode) { + routedFieldMap = getFieldInfo( + classNode, + provider, + { + typeEnv, + symbolTable: NOOP_SYMBOL_TABLE, + filePath: file.path, + language, + }, + fieldInfoCache, + ); + } + } + + const fileId = generateId('File', file.path); + for (const item of routed.items) { + const routedFieldInfo = routedFieldMap?.get(item.propName); + const propQualifiedName = propEnclosingInfo + ? `${propEnclosingInfo.className}.${item.propName}` + : item.propName; + const nodeId = generateId('Property', `${file.path}:${propQualifiedName}`); + graph.addNode({ + id: nodeId, + label: 'Property', + properties: { + name: item.propName, + filePath: file.path, + startLine: item.startLine, + endLine: item.endLine, + language, + isExported: true, + description: item.accessorType, + ...(item.declaredType + ? { declaredType: item.declaredType } + : routedFieldInfo?.type + ? { declaredType: routedFieldInfo.type } + : {}), + ...(routedFieldInfo?.visibility !== undefined + ? { visibility: routedFieldInfo.visibility } + : {}), + ...(routedFieldInfo?.isStatic !== undefined + ? { isStatic: routedFieldInfo.isStatic } + : {}), + ...(routedFieldInfo?.isReadonly !== undefined + ? { isReadonly: routedFieldInfo.isReadonly } + : {}), + }, + }); + ctx.model.symbols.add(file.path, item.propName, nodeId, 'Property', { + ...(propEnclosingClassId ? { ownerId: propEnclosingClassId } : {}), + ...(item.declaredType + ? { declaredType: item.declaredType } + : routedFieldInfo?.type + ? { declaredType: routedFieldInfo.type } + : {}), + }); + const relId = generateId('DEFINES', `${fileId}->${nodeId}`); + graph.addRelationship({ + id: relId, + sourceId: fileId, + targetId: nodeId, + type: 'DEFINES', + confidence: 1.0, + reason: '', + }); + if (propEnclosingClassId) { + graph.addRelationship({ + id: generateId('HAS_PROPERTY', `${propEnclosingClassId}->${nodeId}`), + sourceId: propEnclosingClassId, + targetId: nodeId, + type: 'HAS_PROPERTY', + confidence: 1.0, + reason: '', + }); + } + } + }); + } + // ── Resolution loop: verify constructor bindings and resolve calls ── // The accumulator (if present) is now fully populated from the preparation // loop above, so verifyConstructorBindings sees all provider bindings @@ -930,9 +1102,10 @@ export const processCalls = async ( provider, ); const srcId = enclosing || generateId('File', file.path); - // Defer resolution: Ruby attr_accessor properties are registered during - // this same loop, so cross-file lookups fail if the declaring file hasn't - // been processed yet. Collect now, resolve after all files are done. + // Defer resolution so write-access tracking sees the FINAL graph + // state — properties from the pre-pass are present, but receiver-type + // resolution can still depend on inference that completes during the + // main loop. Resolve after all files have been processed. pendingWrites.push({ receiverTypeName, propertyName, filePath: file.path, srcId }); } // Assignment-only capture (no @call sibling): skip the rest of this @@ -1053,47 +1226,8 @@ export const processCalls = async ( return; case 'properties': { - const fileId = generateId('File', file.path); - const propEnclosingClassId = findEnclosingClassId(captureMap['call'], file.path); - for (const item of routed.items) { - const nodeId = generateId('Property', `${file.path}:${item.propName}`); - graph.addNode({ - id: nodeId, - label: 'Property', - properties: { - name: item.propName, - filePath: file.path, - startLine: item.startLine, - endLine: item.endLine, - language, - isExported: true, - description: item.accessorType, - }, - }); - ctx.model.symbols.add(file.path, item.propName, nodeId, 'Property', { - ...(propEnclosingClassId ? { ownerId: propEnclosingClassId } : {}), - ...(item.declaredType ? { declaredType: item.declaredType } : {}), - }); - const relId = generateId('DEFINES', `${fileId}->${nodeId}`); - graph.addRelationship({ - id: relId, - sourceId: fileId, - targetId: nodeId, - type: 'DEFINES', - confidence: 1.0, - reason: '', - }); - if (propEnclosingClassId) { - graph.addRelationship({ - id: generateId('HAS_PROPERTY', `${propEnclosingClassId}->${nodeId}`), - sourceId: propEnclosingClassId, - targetId: nodeId, - type: 'HAS_PROPERTY', - confidence: 1.0, - reason: '', - }); - } - } + // Properties already registered in the pre-pass above. + // Skip to avoid duplicate nodes/edges. return; } diff --git a/gitnexus/src/core/ingestion/community-processor.ts b/gitnexus/src/core/ingestion/community-processor.ts index 9913e4a3f..ac8f068fa 100644 --- a/gitnexus/src/core/ingestion/community-processor.ts +++ b/gitnexus/src/core/ingestion/community-processor.ts @@ -41,6 +41,24 @@ interface LeidenDetailedResult { modularity: number; } +/** + * Deterministic PRNG (mulberry32) seed for the vendored Leiden algorithm. + * Vendored Leiden defaults `rng: Math.random`, which makes community + * assignment non-deterministic across runs. Passing a seeded RNG gives us + * reproducible community/modularity output, which is required for the + * incremental-indexing equivalence test (incremental ≡ full rebuild). + */ +const LEIDEN_SEED = 0xc0de; +function createSeededRng(seed: number): () => number { + let s = seed >>> 0; + return () => { + s = (s + 0x6d2b79f5) >>> 0; + let t = Math.imul(s ^ (s >>> 15), 1 | s); + t = (t + Math.imul(t ^ (t >>> 7), 61 | t)) ^ t; + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; +} + // ============================================================================ // TYPES // ============================================================================ @@ -150,6 +168,7 @@ export const processCommunities = async ( leiden.detailed(graph, { resolution: isLarge ? 2.0 : 1.0, maxIterations: isLarge ? 3 : 0, + rng: createSeededRng(LEIDEN_SEED), }), ), new Promise((_, reject) => diff --git a/gitnexus/src/core/ingestion/parsing-processor.ts b/gitnexus/src/core/ingestion/parsing-processor.ts index 7559b26bc..04a17db4f 100644 --- a/gitnexus/src/core/ingestion/parsing-processor.ts +++ b/gitnexus/src/core/ingestion/parsing-processor.ts @@ -82,6 +82,88 @@ export interface WorkerExtractedData { // Worker-based parallel parsing // ============================================================================ +/** + * Merge a list of `ParseWorkerResult`s into the running graph + symbol + * table state and produce the chunk-aggregated `WorkerExtractedData`. + * + * Extracted from `processParsingWithWorkers` so the same merge logic can + * be applied to both freshly-parsed worker output AND cached worker + * output replayed during incremental analyze. Idempotent on the + * accumulator fields (push-only); idempotent on graph if the caller + * starts from a clean graph (otherwise duplicate `addNode` calls are + * silently no-op'd by `KnowledgeGraph`). + */ +export const mergeChunkResults = ( + graph: KnowledgeGraph, + symbolTable: SymbolTableWriter, + chunkResults: readonly ParseWorkerResult[], +): WorkerExtractedData => { + const allImports: ExtractedImport[] = []; + const allCalls: ExtractedCall[] = []; + const allAssignments: ExtractedAssignment[] = []; + const allHeritage: ExtractedHeritage[] = []; + const allRoutes: ExtractedRoute[] = []; + const allFetchCalls: ExtractedFetchCall[] = []; + const allDecoratorRoutes: ExtractedDecoratorRoute[] = []; + const allToolDefs: ExtractedToolDef[] = []; + const allORMQueries: ExtractedORMQuery[] = []; + const allConstructorBindings: FileConstructorBindings[] = []; + const fileScopeBindingsByFile: FileScopeBindings[] = []; + const allParsedFiles: ParsedFile[] = []; + + for (const result of chunkResults) { + for (const node of result.nodes) { + graph.addNode({ + id: node.id, + label: node.label as NodeLabel, + properties: node.properties, + }); + } + for (const rel of result.relationships) { + graph.addRelationship(rel); + } + for (const sym of result.symbols) { + symbolTable.add(sym.filePath, sym.name, sym.nodeId, sym.type, { + parameterCount: sym.parameterCount, + requiredParameterCount: sym.requiredParameterCount, + parameterTypes: sym.parameterTypes, + returnType: sym.returnType, + declaredType: sym.declaredType, + ownerId: sym.ownerId, + qualifiedName: sym.qualifiedName, + }); + } + for (const item of result.imports) allImports.push(item); + for (const item of result.calls) allCalls.push(item); + for (const item of result.assignments) allAssignments.push(item); + for (const item of result.heritage) allHeritage.push(item); + for (const item of result.routes) allRoutes.push(item); + for (const item of result.fetchCalls) allFetchCalls.push(item); + for (const item of result.decoratorRoutes) allDecoratorRoutes.push(item); + for (const item of result.toolDefs) allToolDefs.push(item); + if (result.ormQueries) for (const item of result.ormQueries) allORMQueries.push(item); + for (const item of result.constructorBindings) allConstructorBindings.push(item); + if (result.fileScopeBindings) + for (const item of result.fileScopeBindings) fileScopeBindingsByFile.push(item); + if (result.parsedFiles) for (const item of result.parsedFiles) allParsedFiles.push(item); + } + + return { + imports: allImports, + calls: allCalls, + assignments: allAssignments, + heritage: allHeritage, + routes: allRoutes, + fetchCalls: allFetchCalls, + decoratorRoutes: allDecoratorRoutes, + toolDefs: allToolDefs, + ormQueries: allORMQueries, + constructorBindings: allConstructorBindings, + fileScopeBindings: fileScopeBindingsByFile, + parsedFiles: allParsedFiles, + }; +}; + const processParsingWithWorkers = async ( graph: KnowledgeGraph, files: { path: string; content: string }[], @@ -89,6 +171,14 @@ const processParsingWithWorkers = async ( astCache: ASTCache, workerPool: WorkerPool, onFileProgress?: FileProgressCallback, + /** + * When provided, populated with the raw worker results before merging. + * Used by the incremental-indexing parse cache to capture the per-chunk + * worker output for caching across runs. The mutation happens in-place + * so the caller (parse-impl) can keep a reference. See + * `gitnexus/src/storage/parse-cache.ts`. + */ + outRawResults?: ParseWorkerResult[], ): Promise => { // Filter to parseable files only const parseableFiles: ParseWorkerInput[] = []; @@ -123,63 +213,16 @@ const processParsingWithWorkers = async ( }, ); - // Merge results from all workers into graph and symbol table - const allImports: ExtractedImport[] = []; - const allCalls: ExtractedCall[] = []; - const allAssignments: ExtractedAssignment[] = []; - const allHeritage: ExtractedHeritage[] = []; - const allRoutes: ExtractedRoute[] = []; - const allFetchCalls: ExtractedFetchCall[] = []; - const allDecoratorRoutes: ExtractedDecoratorRoute[] = []; - const allToolDefs: ExtractedToolDef[] = []; - const allORMQueries: ExtractedORMQuery[] = []; - const allConstructorBindings: FileConstructorBindings[] = []; - const fileScopeBindingsByFile: FileScopeBindings[] = []; - const allParsedFiles: ParsedFile[] = []; - for (const result of chunkResults) { - for (const node of result.nodes) { - graph.addNode({ - id: node.id, - label: node.label as NodeLabel, - properties: node.properties, - }); - } - - for (const rel of result.relationships) { - graph.addRelationship(rel); - } - - for (const sym of result.symbols) { - symbolTable.add(sym.filePath, sym.name, sym.nodeId, sym.type, { - parameterCount: sym.parameterCount, - requiredParameterCount: sym.requiredParameterCount, - parameterTypes: sym.parameterTypes, - returnType: sym.returnType, - declaredType: sym.declaredType, - ownerId: sym.ownerId, - qualifiedName: sym.qualifiedName, - }); - } - - for (const item of result.imports) allImports.push(item); - for (const item of result.calls) allCalls.push(item); - for (const item of result.assignments) allAssignments.push(item); - for (const item of result.heritage) allHeritage.push(item); - for (const item of result.routes) allRoutes.push(item); - for (const item of result.fetchCalls) allFetchCalls.push(item); - for (const item of result.decoratorRoutes) allDecoratorRoutes.push(item); - for (const item of result.toolDefs) allToolDefs.push(item); - if (result.ormQueries) for (const item of result.ormQueries) allORMQueries.push(item); - for (const item of result.constructorBindings) allConstructorBindings.push(item); - if (result.fileScopeBindings) - for (const item of result.fileScopeBindings) fileScopeBindingsByFile.push(item); - // RFC #909 Ring 2: aggregate per-file scope artifacts. Tolerant of - // workers that don't emit the field yet (older worker builds or - // partial rollouts), since the additive contract means undefined = - // "this worker produced no ParsedFiles for this chunk". - if (result.parsedFiles) for (const item of result.parsedFiles) allParsedFiles.push(item); + // Capture the raw chunk results for the incremental parse cache before + // merging — the cache stores the unmerged worker output so a future run + // can re-merge them into a fresh graph state. + if (outRawResults) { + for (const r of chunkResults) outRawResults.push(r); } + // Merge results from all workers into graph and symbol table. + const merged = mergeChunkResults(graph, symbolTable, chunkResults); + // Merge and log skipped languages from workers const skippedLanguages = new Map(); for (const result of chunkResults) { @@ -196,20 +239,7 @@ const processParsingWithWorkers = async ( // Final progress onFileProgress?.(total, total, 'done'); - return { - imports: allImports, - calls: allCalls, - assignments: allAssignments, - heritage: allHeritage, - routes: allRoutes, - fetchCalls: allFetchCalls, - decoratorRoutes: allDecoratorRoutes, - toolDefs: allToolDefs, - ormQueries: allORMQueries, - constructorBindings: allConstructorBindings, - fileScopeBindings: fileScopeBindingsByFile, - parsedFiles: allParsedFiles, - }; + return merged; }; // ============================================================================ @@ -732,6 +762,14 @@ export const processParsing = async ( scopeTreeCache: ASTCache | undefined, onFileProgress?: FileProgressCallback, workerPool?: WorkerPool, + /** + * Optional out-parameter for the incremental parse cache. When + * provided AND the worker-pool path runs successfully, populated + * with the raw `ParseWorkerResult[]` from the workers (pre-merge). + * Stays empty for the sequential fallback path (no per-chunk + * artifact to cache there). See `gitnexus/src/storage/parse-cache.ts`. + */ + outRawResults?: ParseWorkerResult[], ): Promise => { let lastProgress = 0; const reportProgress: FileProgressCallback | undefined = onFileProgress @@ -759,6 +797,7 @@ export const processParsing = async ( astCache, workerPool, reportProgress, + outRawResults, ); } catch (err) { const message = err instanceof Error ? err.message : String(err); diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index bd39a4330..17cfaab3f 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -17,7 +17,10 @@ import { enrichExportedTypeMap, type BindingEntry, } from '../binding-accumulator.js'; -import { processParsing } from '../parsing-processor.js'; +import { processParsing, mergeChunkResults } from '../parsing-processor.js'; +import { fileContentHash, computeChunkHash } from '../../../storage/parse-cache.js'; +import type { ParseWorkerResult } from '../workers/parse-worker.js'; +import type { WorkerExtractedData } from '../parsing-processor.js'; import { processImports, processImportsFromExtracted, @@ -72,8 +75,21 @@ import { extractORMQueriesInline } from './orm-extraction.js'; import { logger } from '../../logger.js'; // ── Constants ────────────────────────────────────────────────────────────── -/** Max bytes of source content to load per parse chunk. */ -const CHUNK_BYTE_BUDGET = 20 * 1024 * 1024; // 20MB +/** Max bytes of source content to load per parse chunk. + * + * Memory bound for the worker pool dispatch + a granularity knob for + * the parse cache. A single file change invalidates only its enclosing + * chunk, so smaller budgets → finer-grained invalidation. + * + * Override via GITNEXUS_CHUNK_BYTE_BUDGET (bytes) — the default of 2MB + * gives a useful invalidation floor (~1/N chunks on a multi-MB repo) + * while keeping worker dispatch overhead under 5% on cold runs. + */ +const CHUNK_BYTE_BUDGET = (() => { + const env = Number(process.env.GITNEXUS_CHUNK_BYTE_BUDGET); + if (Number.isFinite(env) && env > 0) return env; + return 2 * 1024 * 1024; +})(); // ── Main parse + resolve function ────────────────────────────────────────── @@ -119,6 +135,11 @@ export async function runChunkedParseAndResolve( * source. See plan * docs/plans/2026-04-20-002-perf-parse-heritage-mro-plan.md (Unit 4). */ scopeTreeCache: ASTCache; + /** Worker-produced ParsedFile artifacts aggregated across chunks. + * Threaded into scope-resolution as a re-extract cache so the warm- + * cache analyze run can skip the dominant `extractParsedFile` cost + * (otherwise ~58s on a 1000-file repo). */ + parsedFiles: import('gitnexus-shared').ParsedFile[]; }> { const ctx = createResolutionContext(); const symbolTable = ctx.model.symbols; @@ -142,6 +163,15 @@ export async function runChunkedParseAndResolve( ); } + // Sort parseableScanned alphabetically for stable chunk membership + // across runs (Finding 4). Without this, filesystem-scan order can + // shift between runs (notably on macOS APFS where directory entry + // order can change after modifications) — different files in the + // same chunk → different chunk hash → cache miss even when no file + // content changed. The cache also becomes platform-specific: a + // Linux-built cache misses on macOS for the same repo. + parseableScanned.sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)); + const totalParseable = parseableScanned.length; if (totalParseable === 0) { @@ -271,6 +301,20 @@ export async function runChunkedParseAndResolve( const deferredWorkerHeritage: ExtractedHeritage[] = []; const deferredConstructorBindings: FileConstructorBindings[] = []; const deferredAssignments: ExtractedAssignment[] = []; + // Aggregated per-file ParsedFile artifacts produced by workers' calls + // to `extractParsedFile`. Threaded through to the scope-resolution + // phase so it can SKIP its own re-extraction on cache hits — this is + // the second-half of the parse-cache speedup since scope-resolution's + // re-parse otherwise dominates the warm-cache wall-clock time. + const allParsedFiles: import('gitnexus-shared').ParsedFile[] = []; + + // Incremental parse cache (Option B): chunk-level content-addressed. + // When the chunk's (filePath, content-hash) signature matches a prior + // run's, replay the cached ParseWorkerResult[] instead of dispatching + // to workers. See gitnexus/src/storage/parse-cache.ts. + const parseCache = options?.parseCache; + let chunkCacheHits = 0; + let chunkCacheMisses = 0; try { for (let chunkIdx = 0; chunkIdx < numChunks; chunkIdx++) { @@ -281,29 +325,89 @@ export async function runChunkedParseAndResolve( .filter((p) => chunkContents.has(p)) .map((p) => ({ path: p, content: chunkContents.get(p)! })); - const chunkWorkerData = await processParsing( - graph, - chunkFiles, - symbolTable, - astCache, - scopeTreeCache, - (current, _total, filePath) => { - const globalCurrent = filesParsedSoFar + current; - const parsingProgress = 20 + (globalCurrent / totalParseable) * 62; - onProgress({ - phase: 'parsing', - percent: Math.round(parsingProgress), - message: `Parsing chunk ${chunkIdx + 1}/${numChunks}...`, - detail: filePath, - stats: { - filesProcessed: globalCurrent, - totalFiles: totalParseable, - nodesCreated: graph.nodeCount, - }, - }); - }, - workerPool, - ); + // Compute the chunk's content-hash signature (if cache available). + let chunkHash: string | null = null; + if (parseCache) { + const entries = chunkFiles.map((f) => ({ + filePath: f.path, + contentHash: fileContentHash(f.content), + })); + chunkHash = computeChunkHash(entries); + } + + let chunkWorkerData: WorkerExtractedData | null; + const cachedRaw = chunkHash ? parseCache!.entries.get(chunkHash) : undefined; + + // Track every chunk hash we touched so the orchestrator can + // prune stale entries (chunks whose composition no longer + // corresponds to a live chunk in the current scan) before saving. + if (parseCache && chunkHash) parseCache.usedKeys.add(chunkHash); + + if (cachedRaw && cachedRaw.length > 0) { + // Cache hit: replay the cached worker output through the same + // merge logic the live worker path uses. + chunkCacheHits++; + chunkWorkerData = mergeChunkResults(graph, symbolTable, cachedRaw); + if (isDev) { + logger.info( + `📦 parse-cache HIT: chunk ${chunkIdx + 1}/${numChunks} (${chunkFiles.length} files, ${chunkHash!.slice(0, 8)})`, + ); + } + // Progress update so UI advances even on a cache hit. + const cachedFiles = chunkFiles.length; + onProgress({ + phase: 'parsing', + percent: Math.round(20 + ((filesParsedSoFar + cachedFiles) / totalParseable) * 62), + message: `Parsing chunk ${chunkIdx + 1}/${numChunks} (cache)...`, + stats: { + filesProcessed: filesParsedSoFar + cachedFiles, + totalFiles: totalParseable, + nodesCreated: graph.nodeCount, + }, + }); + } else { + // Cache miss: dispatch to workers, capture the raw results, store + // them under the chunk hash for the next run. + chunkCacheMisses++; + const rawResults: ParseWorkerResult[] = []; + chunkWorkerData = await processParsing( + graph, + chunkFiles, + symbolTable, + astCache, + scopeTreeCache, + (current, _total, filePath) => { + const globalCurrent = filesParsedSoFar + current; + const parsingProgress = 20 + (globalCurrent / totalParseable) * 62; + onProgress({ + phase: 'parsing', + percent: Math.round(parsingProgress), + message: `Parsing chunk ${chunkIdx + 1}/${numChunks}...`, + detail: filePath, + stats: { + filesProcessed: globalCurrent, + totalFiles: totalParseable, + nodesCreated: graph.nodeCount, + }, + }); + }, + workerPool, + // Capture raw results only when we have a cache to write to — + // otherwise we'd retain extra arrays for nothing. + parseCache && chunkHash ? rawResults : undefined, + ); + // Persist the raw results for this chunk hash. Sequential path + // doesn't populate rawResults (it writes directly to graph), so + // small repos without worker pool simply don't cache. That's fine. + if (parseCache && chunkHash && rawResults.length > 0) { + parseCache.entries.set(chunkHash, rawResults); + if (isDev) { + logger.info( + `📦 parse-cache MISS+store: chunk ${chunkIdx + 1}/${numChunks} (${chunkFiles.length} files, ${chunkHash.slice(0, 8)})`, + ); + } + } + } const chunkBasePercent = 20 + (filesParsedSoFar / totalParseable) * 62; @@ -349,6 +453,12 @@ export async function runChunkedParseAndResolve( for (const item of chunkWorkerData.heritage) deferredWorkerHeritage.push(item); for (const item of chunkWorkerData.constructorBindings) deferredConstructorBindings.push(item); + // Aggregate worker-produced ParsedFile artifacts so scope- + // resolution can use them as a re-extraction cache (skips its + // own tree-sitter re-parse on warm runs). + if (chunkWorkerData.parsedFiles?.length) { + for (const item of chunkWorkerData.parsedFiles) allParsedFiles.push(item); + } if (chunkWorkerData.assignments?.length) { for (const item of chunkWorkerData.assignments) deferredAssignments.push(item); } @@ -422,6 +532,12 @@ export async function runChunkedParseAndResolve( astCache.clear(); } + if (isDev && parseCache && (chunkCacheHits > 0 || chunkCacheMisses > 0)) { + logger.info( + `📦 parse-cache summary: ${chunkCacheHits} chunk hit(s), ${chunkCacheMisses} miss(es) across ${numChunks} chunk(s)`, + ); + } + const fullWorkerHeritageMap = deferredWorkerHeritage.length > 0 ? buildHeritageMap(deferredWorkerHeritage, ctx, getHeritageStrategyForLanguage) @@ -621,5 +737,12 @@ export async function runChunkedParseAndResolve( // chunk-local `astCache` above is intentionally NOT exposed // because parse-impl clears it between chunks. scopeTreeCache, + // Per-file ParsedFile artifacts produced by workers' calls to + // `extractParsedFile`. Empty when only the sequential path ran + // (sequential doesn't go through the worker, and extracts ParsedFile + // inline rather than emitting it). Consumed by scope-resolution as + // a re-extraction cache: when the file's ParsedFile is here, + // scope-resolution skips its own `extractParsedFile` call. + parsedFiles: allParsedFiles, }; } diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse.ts index a20d1e4b0..a3fa81be7 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse.ts @@ -20,6 +20,7 @@ import type { PipelinePhase, PipelineContext, PhaseResult } from './types.js'; import { getPhaseOutput } from './types.js'; import type { StructureOutput } from './structure.js'; import type { BindingAccumulator } from '../binding-accumulator.js'; +import type { ParsedFile } from 'gitnexus-shared'; import type { ExtractedFetchCall, ExtractedRoute, @@ -81,6 +82,19 @@ export interface ParseOutput { * `scopeTreeCache.clear()` after its extract loop finishes. */ readonly scopeTreeCache: ASTCache; + /** + * Per-file `ParsedFile` artifacts produced by workers' calls to + * `extractParsedFile`. Threaded through to `scopeResolutionPhase` + * as a re-extraction cache: when a file's ParsedFile is present here, + * scope-resolution can skip its own `extractParsedFile` (which would + * otherwise re-parse the file with tree-sitter on the main thread, + * costing ~58s on a 1000-file repo). + * + * Empty for files that went through the sequential parse fallback — + * sequential doesn't emit ParsedFile artifacts; scope-resolution + * falls back to a fresh extract for those. + */ + readonly parsedFiles: readonly ParsedFile[]; } export const parsePhase: PipelinePhase = { diff --git a/gitnexus/src/core/ingestion/pipeline.ts b/gitnexus/src/core/ingestion/pipeline.ts index c220ea224..1ee8e102f 100644 --- a/gitnexus/src/core/ingestion/pipeline.ts +++ b/gitnexus/src/core/ingestion/pipeline.ts @@ -55,6 +55,19 @@ export interface PipelineOptions { minFiles?: number; minBytes?: number; }; + /** + * Incremental-indexing parse cache. When provided: + * - The parse phase looks up each chunk's content hash in + * `parseCache.entries`. On hit, it replays the cached + * `ParseWorkerResult[]` instead of dispatching to workers. + * - On miss, it runs the workers as today and stores the new + * results in `parseCache.entries` keyed by chunk hash. + * The caller (`run-analyze.ts`) is responsible for loading the cache + * before the pipeline runs and persisting it after. Cache survives + * `--force` because keys are content-addressed. + * See `gitnexus/src/storage/parse-cache.ts`. + */ + parseCache?: import('../../storage/parse-cache.js').ParseCache; } // ── Phase registry ───────────────────────────────────────────────────────── diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts index c2fda9777..98a9f8994 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts @@ -93,13 +93,25 @@ export const scopeResolutionPhase: PipelinePhase = { // Worker-mode parses leave the cache empty for those files; they // also fall back to a fresh parse — no correctness impact. const parseOutput = getPhaseOutput(deps, 'parse'); - const { scopeTreeCache, resolutionContext } = parseOutput; + const { scopeTreeCache, resolutionContext, parsedFiles: workerParsedFiles } = parseOutput; // SemanticModel populated during `parse`: scope-resolution consumes // TypeRegistry / MethodRegistry / SymbolTable lookups instead of // rebuilding parallel indexes. See ARCHITECTURE.md § "Semantic-model // source of truth". const model = resolutionContext.model; + // Build a per-file lookup of ParsedFile artifacts the workers (or + // sequential extracts) already produced. Threading this into + // `runScopeResolution` lets the per-language extract loop short- + // circuit `extractParsedFile` — the dominant cost on the warm-cache + // path, since workers can't return tree-sitter Trees across the + // MessageChannel and scope-resolution would otherwise re-parse + // every file from scratch on the main thread. + const preExtractedByPath = new Map(); + for (const pf of workerParsedFiles) { + preExtractedByPath.set(pf.filePath, pf); + } + let totalFiles = 0; let totalImports = 0; let totalRefs = 0; @@ -143,6 +155,7 @@ export const scopeResolutionPhase: PipelinePhase = { files, treeCache: scopeTreeCache, resolutionConfig, + preExtractedParsedFiles: preExtractedByPath, onWarn: (msg) => { if (isSemanticModelValidatorEnabled()) { logger.warn(`[scope-resolution:${lang}] ${msg}`); diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts index e2c734a43..31a58cdfa 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts @@ -72,6 +72,22 @@ interface RunScopeResolutionInput { * provider doesn't supply a config loader. */ readonly resolutionConfig?: unknown; + /** + * Pre-extracted ParsedFile artifacts keyed by file path. When a + * file is present here, the extract loop reuses it directly and + * skips `extractParsedFile` (which would re-parse the file with + * tree-sitter on the main thread). Only files matching the + * provider's language are honored — the loop verifies this + * implicitly by language filter at the call-site (scopeResolution + * phase). + * + * Worker-mode parses produce these ParsedFile artifacts as a side + * effect of `extractParsedFile` running inside the worker; threading + * them here is what lets the warm-cache analyze run skip the ~58s + * scope-resolution re-parse loop on a multi-thousand-file repo. + * Cache miss is safe — falls back to fresh extract. + */ + readonly preExtractedParsedFiles?: ReadonlyMap; } interface RunScopeResolutionStats { @@ -104,22 +120,37 @@ export function runScopeResolution( const parsedFiles: ParsedFile[] = []; let filesSkipped = 0; const treeCache = input.treeCache; + const preExtracted = input.preExtractedParsedFiles; + let preExtractedHits = 0; for (const file of files) { - const cachedTree = treeCache?.get(file.path); - const parsed = extractParsedFile( - provider.languageProvider, - file.content, - file.path, - onWarn, - cachedTree, - ); + let parsed: ParsedFile | undefined; + // Fast path: a worker (during the parse phase) already produced a + // ParsedFile for this file via `extractParsedFile`. Reuse it + // directly — skips a tree-sitter re-parse on the main thread. + if (preExtracted !== undefined) { + parsed = preExtracted.get(file.path); + if (parsed !== undefined) preExtractedHits++; + } if (parsed === undefined) { - filesSkipped++; - continue; + const cachedTree = treeCache?.get(file.path); + parsed = extractParsedFile( + provider.languageProvider, + file.content, + file.path, + onWarn, + cachedTree, + ); + if (parsed === undefined) { + filesSkipped++; + continue; + } } provider.populateOwners(parsed); parsedFiles.push(parsed); } + if (PROF && preExtracted !== undefined) { + logger.warn(`[scope-resolution prof] pre-extracted hits: ${preExtractedHits}/${files.length}`); + } provider.populateWorkspaceOwners?.(parsedFiles, { fileContents: getFileContents() }); // Reconcile scope-resolution's ownership view into the SemanticModel. diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index fe831cd43..caa7a58a1 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -1204,6 +1204,77 @@ export const deleteNodesForFile = async ( export const getEmbeddingTableName = (): string => EMBEDDING_TABLE_NAME; +/** + * Return the distinct repo-relative paths of files that import + * `targetFilePath` according to the IMPORTS edges currently in the + * DB. Used by the incremental writeback path to expand the + * "files-to-rewrite" set so that files importing a changed file get + * their edges (which may have been refined by cross-file resolution) + * re-emitted, rather than left stale in the DB. + * + * The DB query reads the *previous* run's state — pre-pipeline, before + * any nodes are deleted — so the returned importers are "files that + * USED TO import the target". That's the right set to invalidate: + * those are the files whose edges in the DB might no longer match + * what cross-file resolution produces given the changed file's new + * exports. + */ +export const queryImporters = async (targetFilePath: string): Promise => { + if (!conn) { + throw new Error('LadybugDB not initialized. Call initLbug first.'); + } + const escaped = targetFilePath.replace(/'/g, "''"); + const cypher = ` + MATCH (a)-[r:${REL_TABLE_NAME}]->(b) + WHERE r.type = 'IMPORTS' AND b.filePath = '${escaped}' + RETURN DISTINCT a.filePath AS importer + `; + try { + const queryResult = await conn.query(cypher); + const result = Array.isArray(queryResult) ? queryResult[0] : queryResult; + const rows = await result.getAll(); + const out: string[] = []; + for (const row of rows) { + const v = (row as { importer?: unknown }).importer; + if (typeof v === 'string' && v.length > 0) out.push(v); + } + return out; + } catch { + return []; + } +}; + +/** + * Drop every Community and Process node (and their MEMBER_OF / + * STEP_IN_PROCESS edges via DETACH DELETE). Used at the start of an + * incremental run so the communities and processes phases regenerate + * them from scratch on the merged graph — required for the + * "Leiden runs on the FULL graph" correctness invariant. + */ +export const deleteAllCommunitiesAndProcesses = async (): Promise<{ + nodesDeleted: number; +}> => { + if (!conn) { + throw new Error('LadybugDB not initialized. Call initLbug first.'); + } + let nodesDeleted = 0; + for (const label of ['Community', 'Process']) { + try { + const countResult = await conn.query(`MATCH (n:${label}) RETURN count(n) AS cnt`); + const result = Array.isArray(countResult) ? countResult[0] : countResult; + const rows = await result.getAll(); + const count = Number(rows[0]?.cnt ?? rows[0]?.[0] ?? 0); + if (count > 0) { + await conn.query(`MATCH (n:${label}) DETACH DELETE n`); + nodesDeleted += count; + } + } catch { + // Table may not exist yet on a freshly-initialized DB — fine. + } + } + return { nodesDeleted }; +}; + // ============================================================================ // Full-Text Search (FTS) Functions // ============================================================================ diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index fa2757f45..425f18f9a 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -11,6 +11,7 @@ import path from 'path'; import fs from 'fs/promises'; +import { execFileSync } from 'child_process'; import { runPipelineFromRepo } from './ingestion/pipeline.js'; import { initLbug, @@ -20,6 +21,9 @@ import { executeWithReusedStatement, closeLbug, loadCachedEmbeddings, + deleteNodesForFile, + deleteAllCommunitiesAndProcesses, + queryImporters, } from './lbug/lbug-adapter.js'; import { createSearchFTSIndexes } from './search/fts-indexes.js'; import { @@ -29,7 +33,15 @@ import { ensureGitNexusIgnored, registerRepo, cleanupOldKuzuFiles, + INCREMENTAL_SCHEMA_VERSION, } from '../storage/repo-manager.js'; +import { computeFileHashes, diffFileHashes } from '../storage/file-hash.js'; +import { + extractChangedSubgraph, + computeEffectiveWriteSet, +} from './incremental/subgraph-extract.js'; +import { shadowCandidatesFor } from './incremental/shadow-candidates.js'; +import { loadParseCache, saveParseCache, pruneCache } from '../storage/parse-cache.js'; import { getCurrentCommit, getRemoteUrl, @@ -178,23 +190,81 @@ export async function runFullAnalysis( const currentCommit = repoHasGit ? getCurrentCommit(repoPath) : ''; const existingMeta = await loadMeta(storagePath); + // ── Crash recovery: dirty flag forces full rebuild ──────────────── + // If the previous incremental run set incrementalInProgress and didn't + // clear it, the on-disk index may be in a half-state. Cheapest path + // back to a known-good index is to wipe + rebuild from scratch. + if (existingMeta?.incrementalInProgress) { + log( + 'Previous incremental run did not complete cleanly (incrementalInProgress flag set); ' + + 'forcing full rebuild to restore a known-good index.', + ); + options = { ...options, force: true }; + // Reload meta after clearing the flag in-memory; we still want fileHashes + // for the post-rebuild meta carry-over, but force=true ensures the + // rebuild path executes. + } + // ── Early-return: already up to date ────────────────────────────── if (existingMeta && !options.force && existingMeta.lastCommit === currentCommit) { // Non-git folders have currentCommit = '' — always rebuild since we can't detect changes if (currentCommit !== '') { - await ensureGitNexusIgnored(repoPath); - return { - // `resolveRepoIdentityRoot` collapses worktree roots to the - // canonical repo basename (#1259) but leaves arbitrary subdirs - // and `--skip-git` paths unchanged (#1232/#1233 intent preserved). - repoName: - options.registryName ?? - getInferredRepoName(repoPath) ?? - path.basename(resolveRepoIdentityRoot(repoPath)), - repoPath, - stats: existingMeta.stats ?? {}, - alreadyUpToDate: true, - }; + // For git repos, even if HEAD matches lastCommit, the working tree + // may have uncommitted changes. Only short-circuit when the working + // tree is also clean — otherwise fall through to the incremental + // path which will hash-diff and update only changed files. + // + // We exclude paths that GitNexus itself writes during analyze: + // .gitnexus/ — db / parse cache / meta.json + // .claude/, .cursor/ — auto-generated agent skill files + // AGENTS.md, CLAUDE.md — auto-updated stats blocks + // Counting them as dirty would perpetually defeat the up-to-date + // fast path because the previous analyze just wrote them + // (regression vs PR #1233 behavior). + const dirty = (() => { + try { + const out = execFileSync( + 'git', + [ + 'status', + '--porcelain', + '--', + '.', + ':(exclude).gitnexus', + ':(exclude).gitnexus/**', + ':(exclude).claude', + ':(exclude).claude/**', + ':(exclude).cursor', + ':(exclude).cursor/**', + ':(exclude)AGENTS.md', + ':(exclude)CLAUDE.md', + ], + { + cwd: repoPath, + stdio: ['ignore', 'pipe', 'ignore'], + encoding: 'utf8', + }, + ); + return out.trim().length > 0; + } catch { + return true; // conservative on git failure + } + })(); + if (!dirty) { + await ensureGitNexusIgnored(repoPath); + return { + // `resolveRepoIdentityRoot` collapses worktree roots to the + // canonical repo basename (#1259) but leaves arbitrary subdirs + // and `--skip-git` paths unchanged (#1232/#1233 intent preserved). + repoName: + options.registryName ?? + getInferredRepoName(repoPath) ?? + path.basename(resolveRepoIdentityRoot(repoPath)), + repoPath, + stats: existingMeta.stats ?? {}, + alreadyUpToDate: true, + }; + } } } @@ -243,6 +313,14 @@ export async function runFullAnalysis( ); } + // We *always* load the embedding cache when one is requested (regardless + // of the predicted `willTryIncremental`). The post-pipeline branch may + // disagree with the prediction (e.g. when the pipeline produces zero + // File nodes, `isIncremental` flips false and the full-rebuild path + // wipes the DB) — loading unconditionally is cheap insurance against + // silently dropping embeddings on a mispredicted run. The re-insert + // step gates itself on the actual `isIncremental` value to avoid + // PK-conflicts when the incremental writeback path keeps the rows. if (shouldLoadCache && existingMeta) { try { progress('embeddings', 0, 'Caching embeddings...'); @@ -270,24 +348,89 @@ export async function runFullAnalysis( } } + // ── Load incremental parse cache ────────────────────────────────── + // Content-addressed: safe to reuse across `--force` runs (chunks whose + // file contents haven't changed produce identical worker output). + // Loaded into a single ParseCache object that the pipeline mutates + // in-place (cache hits leave entries unchanged; misses add new ones). + const parseCache = await loadParseCache(storagePath); + // ── Phase 1: Full Pipeline (0–60%) ──────────────────────────────── - const pipelineResult = await runPipelineFromRepo(repoPath, (p) => { - const phaseLabel = PHASE_LABELS[p.phase] || p.phase; - const scaled = Math.round(p.percent * 0.6); - const message = p.detail ? `${p.message || phaseLabel} (${p.detail})` : p.message || phaseLabel; - progress(p.phase, scaled, message); - }); + const pipelineResult = await runPipelineFromRepo( + repoPath, + (p) => { + const phaseLabel = PHASE_LABELS[p.phase] || p.phase; + const scaled = Math.round(p.percent * 0.6); + const message = p.detail + ? `${p.message || phaseLabel} (${p.detail})` + : p.message || phaseLabel; + progress(p.phase, scaled, message); + }, + { parseCache }, + ); // ── Phase 2: LadybugDB (60–85%) ────────────────────────────────── progress('lbug', 60, 'Loading into LadybugDB...'); - await closeLbug(); - const lbugFiles = [lbugPath, `${lbugPath}.wal`, `${lbugPath}.lock`]; - for (const f of lbugFiles) { - try { - await fs.rm(f, { recursive: true, force: true }); - } catch { - /* swallow */ + // Compute current per-file content hashes from the pipeline's File nodes. + // Used both to drive the incremental DB writeback (when eligible) and to + // populate meta.json.fileHashes for the next run. + const allFilePaths: string[] = []; + pipelineResult.graph.forEachNode((n) => { + if (n.label === 'File') { + const fp = n.properties?.filePath as string | undefined; + if (fp) allFilePaths.push(fp); + } + }); + const newFileHashes = await computeFileHashes(repoPath, allFilePaths); + + // Decide incremental vs full at THIS point (post-pipeline, pre-DB). + // All eligibility conditions are checked here against the actual + // pipeline output — no separate pre-pipeline prediction to desync from + // (Bugbot review on PR #1479: a prediction that flipped post-pipeline + // could skip the embedding cache load and then take the full-rebuild + // path, silently losing embeddings). + const isIncremental = + !options.force && + !!existingMeta && + existingMeta.schemaVersion === INCREMENTAL_SCHEMA_VERSION && + !!existingMeta.fileHashes && + Object.keys(existingMeta.fileHashes).length > 0 && + repoHasGit && + allFilePaths.length > 0; + + const hashDiff = isIncremental + ? diffFileHashes(newFileHashes, existingMeta!.fileHashes) + : undefined; + + if (isIncremental && hashDiff) { + log( + `Incremental: changed=${hashDiff.changed.length}, ` + + `added=${hashDiff.added.length}, ` + + `deleted=${hashDiff.deleted.length} ` + + `(skipping wipe + ${ + allFilePaths.length - hashDiff.toWrite.length + } unchanged file rows preserved)`, + ); + // Set the dirty flag BEFORE any destructive DB mutation. Cleared on + // success at the meta-save step. + await saveMeta(storagePath, { + ...existingMeta!, + incrementalInProgress: { + startedAt: Date.now(), + toWriteCount: hashDiff.toWrite.length, + }, + }); + } else { + // Full rebuild path: wipe DB files first. + await closeLbug(); + const lbugFiles = [lbugPath, `${lbugPath}.wal`, `${lbugPath}.lock`]; + for (const f of lbugFiles) { + try { + await fs.rm(f, { recursive: true, force: true }); + } catch { + /* swallow */ + } } } @@ -298,11 +441,145 @@ export async function runFullAnalysis( // must be released to avoid blocking subsequent invocations. let lbugMsgCount = 0; - await loadGraphToLbug(pipelineResult.graph, pipelineResult.repoPath, storagePath, (msg) => { - lbugMsgCount++; - const pct = Math.min(84, 60 + Math.round((lbugMsgCount / (lbugMsgCount + 10)) * 24)); - progress('lbug', pct, msg); - }); + if (isIncremental && hashDiff) { + // ── Incremental DB writeback ─────────────────────────────────── + // 0. Expand the writable set with transitive importers of + // changed/deleted files (bounded BFS). + // + // Reason (Bugbot/Claude review on PR #1479): when a barrel / + // re-export file C changes, cross-file resolution may update + // CALLS edges between two unchanged files A and B (A imports + // from C, C re-exports something from B). Those refined edges + // live in `ctx.graph` but would be excluded from the subgraph + // if neither endpoint is in the changed set. To catch this, + // files that imported (directly OR transitively, through + // other unchanged intermediaries) any changed file get pulled + // into the writable set so their rows are deleted + rewritten + // against the refined edges. + // + // BFS bound: MAX_IMPORTER_BFS_DEPTH. Practically sized to + // catch nested barrel chains (e.g. `index.ts → submodule/index.ts + // → submodule/impl.ts`) without ballooning into a near-full- + // rebuild on monorepos with deep re-export pyramids. Beyond + // this depth, the "incremental ≡ full-rebuild" invariant is + // self-acknowledged as best-effort; `--force` remains the + // escape hatch documented in GUARDRAILS.md. + // + // `queryImporters` reads `IMPORTS` from the pre-pipeline DB + // state, so the result is "files that USED TO import the + // target" — exactly the set whose previously-stored edges may + // no longer match what cross-file resolution produces this run. + const MAX_IMPORTER_BFS_DEPTH = 4; + const writableFiles = new Set(hashDiff.toWrite); + const directlyChangedCount = writableFiles.size; + + // Shadow-seed: for ADDED files, queryImporters returns 0 (the new + // file has no IMPORTS rows in the pre-pipeline DB yet). But pre- + // existing unchanged files may have IMPORTS edges whose module- + // resolution claim the newcomer can steal under standard JS/TS + // resolution (Bugbot review on PR #1479). For each added file we + // derive the shadow candidates and, if the candidate was a known + // file in the prior meta, seed it into the BFS frontier so its + // importers — surfaced via queryImporters — get their CALLS edges + // re-resolved against the new file. See shadow-candidates.ts for + // the full pattern catalogue. + const priorFileSet = new Set( + existingMeta?.fileHashes ? Object.keys(existingMeta.fileHashes) : [], + ); + const shadowSeed: string[] = []; + for (const added of hashDiff.added) { + for (const cand of shadowCandidatesFor(added)) { + if (priorFileSet.has(cand) && !writableFiles.has(cand)) { + shadowSeed.push(cand); + } + } + } + + { + let frontier: string[] = [...hashDiff.toWrite, ...hashDiff.deleted, ...shadowSeed]; + for (let depth = 0; depth < MAX_IMPORTER_BFS_DEPTH && frontier.length > 0; depth++) { + const nextFrontier: string[] = []; + for (const f of frontier) { + try { + const importers = await queryImporters(f); + for (const i of importers) { + if (!writableFiles.has(i)) { + writableFiles.add(i); + nextFrontier.push(i); + } + } + } catch { + /* per-file importer query failure → skip; correctness degrades on + that branch, but DB stays writable. */ + } + } + frontier = nextFrontier; + } + } + const importerExpansion = writableFiles.size - directlyChangedCount; + if (importerExpansion > 0) { + log( + `Incremental: +${importerExpansion} importer(s) added to writable set ` + + `(BFS depth ≤ ${MAX_IMPORTER_BFS_DEPTH}` + + (shadowSeed.length > 0 ? `, ${shadowSeed.length} shadow-seed(s)` : '') + + `)`, + ); + } + + // 1. Compute the EFFECTIVE write-set (Finding 1). Two layers, + // composed: + // (a) `writableFiles` — toWrite ∪ transitive importers of + // changed/deleted files (the bounded BFS above, reading + // IMPORTS from the pre-pipeline DB). + // (b) `computeEffectiveWriteSet` — walks the NEW graph's + // edges and pulls in any unchanged-side file that sits + // on a writable-boundary-crossing edge (catches refined + // cross-file CALLS edges that the pre-run DB couldn't + // predict, e.g. a barrel re-export shifting `foo` from + // B to D). + // The composed set is the input to BOTH deleteNodesForFile + // and extractChangedSubgraph — asymmetry between the two would + // leave stale rows or PK-conflict at COPY time. + const effectiveWriteSet = computeEffectiveWriteSet(pipelineResult.graph, writableFiles); + // Deduped: deleted entries may already appear via importer-BFS + // expansion (queryImporters can return a now-deleted path), which + // would otherwise call deleteNodesForFile twice for the same file + // (Bugbot LOW finding on PR #1479). + const filesToDelete = [...new Set([...effectiveWriteSet, ...hashDiff.deleted])]; + for (let i = 0; i < filesToDelete.length; i++) { + const f = filesToDelete[i]; + try { + await deleteNodesForFile(f); + } catch { + /* file may not have rows (e.g. an unparseable file) — fine */ + } + if (i % 20 === 0) { + progress('lbug', 62, `Removing rows for changed files (${i}/${filesToDelete.length})...`); + } + } + // 2. Drop graph-wide nodes (Community, Process). They'll be re-inserted + // from the fresh pipeline output below. Required for the + // "Leiden runs on the FULL graph" correctness invariant. + await deleteAllCommunitiesAndProcesses(); + + // 3. Extract the changed subgraph from the FULL ctx.graph and write + // only that. Unchanged-file rows in the DB stay untouched. Pass + // the SAME effectiveWriteSet so the subgraph and the deletes + // cover identical files (asymmetry would silently corrupt). + const subgraph = extractChangedSubgraph(pipelineResult.graph, effectiveWriteSet); + await loadGraphToLbug(subgraph, pipelineResult.repoPath, storagePath, (msg) => { + lbugMsgCount++; + const pct = Math.min(84, 65 + Math.round((lbugMsgCount / (lbugMsgCount + 10)) * 19)); + progress('lbug', pct, msg); + }); + } else { + // ── Full rebuild ─────────────────────────────────────────────── + await loadGraphToLbug(pipelineResult.graph, pipelineResult.repoPath, storagePath, (msg) => { + lbugMsgCount++; + const pct = Math.min(84, 60 + Math.round((lbugMsgCount / (lbugMsgCount + 10)) * 24)); + progress('lbug', pct, msg); + }); + } // ── Phase 3: FTS (85–90%) ───────────────────────────────────────── progress('fts', 85, 'Creating search indexes...'); @@ -310,6 +587,19 @@ export async function runFullAnalysis( progress('fts', 90, 'Search indexes ready'); // ── Phase 3.5: Re-insert cached embeddings ──────────────────────── + // Runs on BOTH the full-rebuild path and the incremental path: + // - Full rebuild: DB was wiped, every cached row needs to come back. + // - Incremental: changed-file rows were just deleted by + // deleteNodesForFile (which cascades to their + // embedding rows) — so their cached vectors need + // to come back too. Unchanged-file rows still + // exist; re-inserting their cached vectors would + // PK-conflict, but the per-batch try/catch below + // silently ignores those (matches the existing + // "some may fail if node was removed, that's + // fine" semantics). Bugbot review on PR #1479 + // flagged that gating this on `!isIncremental` + // silently lost changed-file embeddings. if (cachedEmbeddings.length > 0) { const cachedDims = cachedEmbeddings[0].embedding.length; const { EMBEDDING_DIMS } = await import('./lbug/schema.js'); @@ -456,6 +746,12 @@ export async function runFullAnalysis( const effectiveSemanticMode = semanticMode ?? (runtimeCapabilities.semanticMode === 'vector-index' ? 'vector-index' : 'exact-scan'); + + // Convert the post-run file-hash map to the on-disk Record + // shape consumed by RepoMeta.fileHashes. + const newFileHashesRecord: Record = {}; + for (const [k, v] of newFileHashes) newFileHashesRecord[k] = v; + const meta = { repoPath, lastCommit: currentCommit, @@ -485,8 +781,33 @@ export async function runFullAnalysis( reason: runtimeCapabilities.reason, }, }, + // Incremental-indexing fields. Populated for git repos so the next + // analyze run can take the incremental DB-writeback path. Setting + // incrementalInProgress to undefined explicitly clears any prior + // dirty flag (full and incremental success paths converge here). + schemaVersion: hasGitDir(repoPath) ? INCREMENTAL_SCHEMA_VERSION : undefined, + fileHashes: hasGitDir(repoPath) ? newFileHashesRecord : undefined, + incrementalInProgress: undefined as { startedAt: number; toWriteCount: number } | undefined, }; await saveMeta(storagePath, meta); + + // Persist the incremental parse cache for the next run. Wraps in + // try/catch so a cache-write failure never breaks an otherwise + // successful indexing run. Prune stale chunk-hash entries first so + // the cache file size stays bounded across runs (chunks whose + // composition no longer matches anything in the current scan are + // dead weight; the parse phase populates `usedKeys` as it processes + // chunks). + try { + const pruned = pruneCache(parseCache, parseCache.usedKeys); + if (pruned > 0) { + log(`Parse cache: pruned ${pruned} stale chunk entries`); + } + await saveParseCache(storagePath, parseCache); + } catch (e) { + log(`Warning: could not save parse cache (${(e as Error).message}); continuing.`); + } + // Forward the --name alias and the registry-collision bypass bit. // `allowDuplicateName` is its own concern — independent from the // pipeline `force` above. The CLI maps it from diff --git a/gitnexus/src/storage/file-hash.ts b/gitnexus/src/storage/file-hash.ts new file mode 100644 index 000000000..b39111815 --- /dev/null +++ b/gitnexus/src/storage/file-hash.ts @@ -0,0 +1,104 @@ +/** + * Per-file content hashing for incremental DB writeback. + * + * On every analyze run we compute SHA-256 of every file's content and + * store the map in meta.json. The next run compares disk against the + * stored map and produces: + * - `changed` — content differs (re-emit DB rows for this file) + * - `added` — file is new on disk (insert DB rows) + * - `deleted` — file was in last meta but no longer on disk (drop rows) + * + * The pipeline still parses every file (correctness invariant: cross-file + * resolution needs full data). What this enables is a SELECTIVE DB + * writeback: instead of wipe-and-reload of the whole graph (~50s of CSV + * COPY on a 25K-node repo), we only delete-and-rewrite rows for the + * changed/added/deleted set. + * + * See docs/superpowers/specs/2026-05-10-incremental-indexing-design.md + * (Option B revision). + */ + +import { createHash } from 'crypto'; +import fs from 'fs/promises'; +import path from 'path'; + +/** + * Compute SHA-256 of a single file. Returns null when the file can't be + * read — caller treats that as "no signature, assume changed". + */ +export const computeFileHash = async (absPath: string): Promise => { + try { + const buf = await fs.readFile(absPath); + return createHash('sha256').update(buf).digest('hex'); + } catch { + return null; + } +}; + +/** + * Compute SHA-256 hashes for many files in parallel batches. Files that + * fail to read are omitted from the result map. + */ +export const computeFileHashes = async ( + repoPath: string, + relPaths: readonly string[], +): Promise> => { + const out = new Map(); + const BATCH = 100; + for (let i = 0; i < relPaths.length; i += BATCH) { + const batch = relPaths.slice(i, i + BATCH); + const results = await Promise.all( + batch.map(async (rel) => { + const h = await computeFileHash(path.join(repoPath, rel)); + return h ? ([rel, h] as const) : null; + }), + ); + for (const r of results) if (r) out.set(r[0], r[1]); + } + return out; +}; + +/** Result of comparing the current on-disk hashes against stored ones. */ +export interface FileHashDiff { + /** Files whose content hash differs from stored. */ + changed: string[]; + /** Files in the current scan that weren't in the stored map. */ + added: string[]; + /** Files in the stored map that aren't in the current scan. */ + deleted: string[]; + /** All files whose DB rows must be replaced (changed ∪ added). */ + toWrite: string[]; +} + +/** + * Diff a current hash map against a previously stored one. + * + * Sorted output so two runs produce identical diff arrays for the same + * changes — useful for stable logging / equivalence checks. + */ +export const diffFileHashes = ( + current: ReadonlyMap, + stored: Readonly> | undefined, +): FileHashDiff => { + const storedMap = new Map(stored ? Object.entries(stored) : []); + const changed: string[] = []; + const added: string[] = []; + for (const [p, h] of current) { + const prev = storedMap.get(p); + if (prev === undefined) added.push(p); + else if (prev !== h) changed.push(p); + } + const deleted: string[] = []; + for (const p of storedMap.keys()) { + if (!current.has(p)) deleted.push(p); + } + changed.sort(); + added.sort(); + deleted.sort(); + return { + changed, + added, + deleted, + toWrite: [...changed, ...added].sort(), + }; +}; diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts new file mode 100644 index 000000000..a1abf76fa --- /dev/null +++ b/gitnexus/src/storage/parse-cache.ts @@ -0,0 +1,213 @@ +/** + * Chunk-level content-addressed parse cache. + * + * The pipeline always parses every file (correctness invariant: cross-file + * resolution and downstream phases need full graph data). What this cache + * does is skip the tree-sitter worker dispatch when a chunk's contents + * haven't changed since the last run. + * + * Granularity: chunk-level. The parse phase chunks files into ~20MB byte + * budgets. The cache key is `sha256(joined(filePath:contentHash for each + * file in the chunk, sorted))`. A change to a single file invalidates only + * that file's chunk — typically 1 of ~50 chunks on a 1000-file repo. + * + * Why not per-file: + * - Workers process sub-batches and emit aggregated `ParseWorkerResult`s. + * Splitting back to per-file would require reworking the worker contract. + * - Chunk-level invalidation gives a useful speedup floor (98% on a single + * 1-of-50 invalidated chunk) without touching the worker. + * + * Survives `--force` because it's content-addressed: the same bytes always + * produce the same key. `--force` only matters for the LadybugDB writeback; + * the cache itself is always safe to reuse. + */ + +import { createHash } from 'crypto'; +import { createRequire } from 'module'; +import fs from 'fs/promises'; +import path from 'path'; +import { fileURLToPath } from 'url'; +import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.js'; + +/** + * Cache version composed of: + * - A schema bump knob (`SCHEMA_BUMP`) for hand-controlled invalidation + * when ParseWorkerResult shape or upstream parse semantics change. + * - The current `gitnexus` npm package version, read at module load. + * Any release that ships an updated tree-sitter grammar or revised + * extractor logic implies a version bump in package.json, which + * automatically invalidates the on-disk cache. Without this, a user + * running `npm i -g gitnexus@latest` after a parser-affecting + * release would silently replay pre-upgrade ParseWorkerResults + * against the new graph schema (Bugbot/Claude review on #1479). + * + * On version mismatch, `loadParseCache` returns an empty cache and the + * next save overwrites the on-disk file with the new version baked in. + */ +const SCHEMA_BUMP = 1; +const GITNEXUS_PKG_VERSION = (() => { + try { + // package.json sits at gitnexus/package.json — two levels up from + // gitnexus/src/storage/parse-cache.ts (or its dist/ equivalent). + const here = path.dirname(fileURLToPath(import.meta.url)); + const candidates = [ + path.join(here, '..', '..', 'package.json'), // src/storage → gitnexus/ + path.join(here, '..', '..', '..', 'package.json'), // dist/storage → gitnexus/ + ]; + const requireCJS = createRequire(import.meta.url); + for (const c of candidates) { + try { + const pkg = requireCJS(c); + if (typeof pkg?.version === 'string') return pkg.version; + } catch { + /* try next candidate */ + } + } + } catch { + /* fall through to fallback */ + } + return '0.0.0-unknown'; +})(); +export const PARSE_CACHE_VERSION = `${SCHEMA_BUMP}+${GITNEXUS_PKG_VERSION}`; + +const CACHE_FILENAME = 'parse-cache.json'; + +/** On-disk shape. */ +interface ParseCacheFile { + version: string; + /** key = chunk hash (hex) → cached chunk result list. */ + entries: Record; +} + +/** Runtime view: keyed Map for fast lookup; mutated in place during a run. */ +export interface ParseCache { + version: string; + entries: Map; + /** + * Hashes referenced (hit OR miss-and-stored) by the current run. + * The parse phase populates this as it processes chunks; the orchestrator + * uses it as input to `pruneCache` before saving so entries that no + * longer correspond to any chunk in the current scan are discarded. + * Transient — never serialized to disk. + */ + usedKeys: Set; +} + +/** SHA-256 hex of a single string or buffer. */ +const sha256Hex = (input: Buffer | string): string => + createHash('sha256') + .update(typeof input === 'string' ? Buffer.from(input) : input) + .digest('hex'); + +/** Stable hash of a single file's contents — used by callers to compose a chunk hash. */ +export const fileContentHash = (content: Buffer | string): string => sha256Hex(content); + +/** + * Compute the canonical cache key for a chunk's contents. + * + * `entries` is the list of (filePath, file content hash) for every file + * in the chunk. We sort by filePath before hashing so chunks composed of + * the same files in different order produce the same key. + */ +export const computeChunkHash = ( + entries: Array<{ filePath: string; contentHash: string }>, +): string => { + const sorted = [...entries].sort((a, b) => (a.filePath < b.filePath ? -1 : 1)); + const joined = sorted.map((e) => `${e.filePath}:${e.contentHash}`).join('\n'); + return sha256Hex(joined); +}; + +/** + * JSON replacer that round-trips Map/Set instances through plain JSON. + * + * `ParseWorkerResult.parsedFiles[*].scopes[*].typeBindings` is a + * `ReadonlyMap`; without this transform it serializes + * to `{}` and downstream code that iterates / `.get()`s on it crashes + * with "is not iterable". Applied symmetrically by `mapReviver` on + * load so the in-memory shape stays Map-typed. + */ +const MAP_TAG = '__$mapEntries$__'; +const SET_TAG = '__$setValues$__'; + +const mapReplacer = (_key: string, value: unknown): unknown => { + if (value instanceof Map) return { [MAP_TAG]: Array.from(value.entries()) }; + if (value instanceof Set) return { [SET_TAG]: Array.from(value.values()) }; + return value; +}; + +const mapReviver = (_key: string, value: unknown): unknown => { + if (value && typeof value === 'object') { + const v = value as Record; + if (Array.isArray(v[MAP_TAG])) return new Map(v[MAP_TAG] as [unknown, unknown][]); + if (Array.isArray(v[SET_TAG])) return new Set(v[SET_TAG] as unknown[]); + } + return value; +}; + +/** + * Load the parse cache. Returns an empty cache on any failure (missing + * file, corrupt JSON, version mismatch). Never throws on a normal load. + */ +export const loadParseCache = async (storagePath: string): Promise => { + const cachePath = path.join(storagePath, CACHE_FILENAME); + try { + const raw = await fs.readFile(cachePath, 'utf-8'); + const data = JSON.parse(raw, mapReviver) as ParseCacheFile; + if ( + typeof data !== 'object' || + data === null || + data.version !== PARSE_CACHE_VERSION || + typeof data.entries !== 'object' || + data.entries === null + ) { + return emptyCache(); + } + const entries = new Map(); + for (const [k, v] of Object.entries(data.entries)) { + if (Array.isArray(v)) entries.set(k, v as ParseWorkerResult[]); + } + return { version: PARSE_CACHE_VERSION, entries, usedKeys: new Set() }; + } catch { + return emptyCache(); + } +}; + +/** + * Persist the cache to disk atomically (write-and-rename) so a crash + * mid-write doesn't leave a corrupt file. + */ +export const saveParseCache = async (storagePath: string, cache: ParseCache): Promise => { + await fs.mkdir(storagePath, { recursive: true }); + const cachePath = path.join(storagePath, CACHE_FILENAME); + const tmpPath = `${cachePath}.tmp`; + const out: ParseCacheFile = { + version: cache.version, + entries: Object.fromEntries(cache.entries), + }; + // Compact JSON; this file can be tens of MB on a large repo and pretty- + // printing roughly doubles size for no value. + await fs.writeFile(tmpPath, JSON.stringify(out, mapReplacer), 'utf-8'); + await fs.rename(tmpPath, cachePath); +}; + +/** + * Drop entries whose hashes are not in `usedHashes`. Called at the end + * of a run so chunks that no longer correspond to any current chunk + * don't keep their stale entries forever. + */ +export const pruneCache = (cache: ParseCache, usedHashes: ReadonlySet): number => { + let removed = 0; + for (const k of cache.entries.keys()) { + if (!usedHashes.has(k)) { + cache.entries.delete(k); + removed++; + } + } + return removed; +}; + +const emptyCache = (): ParseCache => ({ + version: PARSE_CACHE_VERSION, + entries: new Map(), + usedKeys: new Set(), +}); diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 8c0bda95f..456a6c143 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -71,8 +71,40 @@ export interface RepoMeta { processes?: number; embeddings?: number; }; + /** + * Bumped whenever incremental-indexing invariants change in an + * incompatible way (delete-and-rewrite logic, subgraph extraction, + * graph-wide node handling). On mismatch, runFullAnalysis forces a + * full rebuild rather than risk an inconsistent incremental update. + */ + schemaVersion?: number; + /** + * SHA-256 of every file's content at the time of the last successful + * indexing run. The next run computes current hashes and diffs against + * this map to determine which files' DB rows must be replaced. + * Map keys are repo-relative paths. + */ + fileHashes?: Record; + /** + * Crash-recovery dirty flag. Written to meta.json BEFORE any + * destructive DB mutation in an incremental run; cleared on success + * by overwriting meta.json. If a run crashes between, the next run + * sees the flag and forces a full rebuild — the cheapest path back + * to a known-good index. + */ + incrementalInProgress?: { + /** When the incremental run started (epoch ms). */ + startedAt: number; + /** Number of files in the writable set, for diagnostic logs. */ + toWriteCount: number; + }; } +/** + * Bumped whenever incremental-indexing invariants change incompatibly. + */ +export const INCREMENTAL_SCHEMA_VERSION = 1; + export interface IndexedRepo { repoPath: string; storagePath: string; @@ -186,12 +218,23 @@ export const loadMeta = async (storagePath: string): Promise => }; /** - * Save metadata to storage + * Save metadata to storage. + * + * Atomic via tmp-file + rename (matches `saveParseCache`'s pattern). The + * `incrementalInProgress` dirty flag travels through this file — a crash + * mid-write would leave a corrupt `meta.json` that the next run's + * `loadMeta` would silently treat as "no prior index", losing the dirty + * flag and skipping the recovery full-rebuild. Write-and-rename rules + * that out: the rename is atomic on POSIX and on Windows (`fs.rename` + * on `node:fs/promises` uses `MoveFileEx(REPLACE_EXISTING)`), so either + * the old or the new file is observed at every moment. */ export const saveMeta = async (storagePath: string, meta: RepoMeta): Promise => { await fs.mkdir(storagePath, { recursive: true }); const metaPath = path.join(storagePath, 'meta.json'); - await fs.writeFile(metaPath, JSON.stringify(meta, null, 2), 'utf-8'); + const tmpPath = `${metaPath}.tmp`; + await fs.writeFile(tmpPath, JSON.stringify(meta, null, 2), 'utf-8'); + await fs.rename(tmpPath, metaPath); }; /** diff --git a/gitnexus/test/fixtures/pipeline-golden/mini-repo/expected-graph.json b/gitnexus/test/fixtures/pipeline-golden/mini-repo/expected-graph.json index 0d66fd3a7..34f2a6106 100644 --- a/gitnexus/test/fixtures/pipeline-golden/mini-repo/expected-graph.json +++ b/gitnexus/test/fixtures/pipeline-golden/mini-repo/expected-graph.json @@ -25,5 +25,5 @@ "MEMBER_OF": 12, "STEP_IN_PROCESS": 12 }, - "edgeDigest": "a418debec537cf959fe56fd1fbbbfb59a640398cdb3c61ce0bcb8056c1f45110" + "edgeDigest": "6f414427a20c037df3e336f055c83f987e7d381c9bfa73b4d2be690cb8103302" } diff --git a/gitnexus/test/unit/incremental-file-hash.test.ts b/gitnexus/test/unit/incremental-file-hash.test.ts new file mode 100644 index 000000000..0f59dfb09 --- /dev/null +++ b/gitnexus/test/unit/incremental-file-hash.test.ts @@ -0,0 +1,124 @@ +import { describe, it, expect } from 'vitest'; +import { mkdtemp, writeFile, rm } from 'fs/promises'; +import { tmpdir } from 'os'; +import path from 'path'; +import { computeFileHash, computeFileHashes, diffFileHashes } from '../../src/storage/file-hash.js'; + +describe('diffFileHashes', () => { + it('classifies files into changed / added / deleted / toWrite', () => { + const stored = { a: 'h-a', b: 'h-b', c: 'h-c' }; + const current = new Map([ + ['a', 'h-a'], // unchanged + ['b', 'h-b-NEW'], // changed + ['d', 'h-d'], // added + // 'c' is gone → deleted + ]); + const diff = diffFileHashes(current, stored); + expect(diff.changed).toEqual(['b']); + expect(diff.added).toEqual(['d']); + expect(diff.deleted).toEqual(['c']); + // toWrite is the union of changed ∪ added (rows to be (re)written) + expect(diff.toWrite.sort()).toEqual(['b', 'd']); + }); + + it('treats no stored map as "everything is added"', () => { + const current = new Map([ + ['x', 'h1'], + ['y', 'h2'], + ]); + const diff = diffFileHashes(current, undefined); + expect(diff.added.sort()).toEqual(['x', 'y']); + expect(diff.changed).toEqual([]); + expect(diff.deleted).toEqual([]); + expect(diff.toWrite.sort()).toEqual(['x', 'y']); + }); + + it('returns sorted arrays for stable cross-platform comparison', () => { + const stored = { z: 'h', a: 'h', m: 'h' }; + const current = new Map([ + ['z', 'h2'], + ['a', 'h2'], + ['m', 'h2'], + ]); + const diff = diffFileHashes(current, stored); + expect(diff.changed).toEqual(['a', 'm', 'z']); + expect(diff.toWrite).toEqual(['a', 'm', 'z']); + }); + + it('handles empty current map (all stored files become deleted)', () => { + const stored = { a: 'h1', b: 'h2' }; + const diff = diffFileHashes(new Map(), stored); + expect(diff.deleted).toEqual(['a', 'b']); + expect(diff.changed).toEqual([]); + expect(diff.added).toEqual([]); + }); +}); + +describe('computeFileHash', () => { + it('produces a stable SHA-256 hex digest', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'gnx-fh-')); + try { + const f = path.join(dir, 'a.txt'); + await writeFile(f, 'hello world\n', 'utf-8'); + const h1 = await computeFileHash(f); + const h2 = await computeFileHash(f); + expect(h1).toBe(h2); + expect(h1).toMatch(/^[a-f0-9]{64}$/); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it('returns null on missing file (caller treats as "no signature")', async () => { + const h = await computeFileHash('/definitely/does/not/exist/here.xyz'); + expect(h).toBeNull(); + }); + + it('different content → different hash', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'gnx-fh-')); + try { + const a = path.join(dir, 'a.txt'); + const b = path.join(dir, 'b.txt'); + await writeFile(a, 'hello', 'utf-8'); + await writeFile(b, 'goodbye', 'utf-8'); + const ha = await computeFileHash(a); + const hb = await computeFileHash(b); + expect(ha).not.toBeNull(); + expect(hb).not.toBeNull(); + expect(ha).not.toBe(hb); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); + +describe('computeFileHashes', () => { + it('hashes a small batch of files in parallel', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'gnx-fh-')); + try { + await writeFile(path.join(dir, 'one.txt'), 'A', 'utf-8'); + await writeFile(path.join(dir, 'two.txt'), 'B', 'utf-8'); + await writeFile(path.join(dir, 'three.txt'), 'C', 'utf-8'); + const map = await computeFileHashes(dir, ['one.txt', 'two.txt', 'three.txt']); + expect(map.size).toBe(3); + expect(map.get('one.txt')).toMatch(/^[a-f0-9]{64}$/); + // All distinct since contents differ + const hashes = [...map.values()]; + expect(new Set(hashes).size).toBe(3); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it('omits files that fail to read (no entry in result)', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'gnx-fh-')); + try { + await writeFile(path.join(dir, 'real.txt'), 'X', 'utf-8'); + const map = await computeFileHashes(dir, ['real.txt', 'phantom.txt']); + expect(map.has('real.txt')).toBe(true); + expect(map.has('phantom.txt')).toBe(false); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/gitnexus/test/unit/incremental-orchestration.test.ts b/gitnexus/test/unit/incremental-orchestration.test.ts new file mode 100644 index 000000000..3d0d244af --- /dev/null +++ b/gitnexus/test/unit/incremental-orchestration.test.ts @@ -0,0 +1,263 @@ +/** + * Integration coverage for the `runFullAnalysis` incremental-orchestration + * wiring (Claude PR-review Finding 2). + * + * These tests exercise the *real runtime path* — they call + * `runFullAnalysis` against a real on-disk git repo backed by a real + * LadybugDB at `/.gitnexus/`, and assert behaviours that pure + * unit tests on `diffFileHashes` / `extractChangedSubgraph` cannot + * catch: + * + * - the `isIncremental` decision (post-pipeline eligibility check) + * - `incrementalInProgress` dirty-flag set-before-mutation and + * clear-on-success + * - the importer-closure expansion (1-hop reached via the writable + * set, transitive reachable via bounded BFS) + * - the "forced full rebuild on dirty-flag-from-prior-crash" path + * + * Each test creates a temporary git repo, runs the analyzer, and asserts + * on the resulting `meta.json` and graph state. Cleanup is best-effort + * (Windows LadybugDB handle release can lag; `cleanupTempDir` retries). + */ + +import { execSync } from 'child_process'; +import { writeFile, readFile, copyFile, mkdir } from 'fs/promises'; +import path from 'path'; +import { fileURLToPath } from 'url'; +import { describe, it, expect } from 'vitest'; +import { + getStoragePaths, + saveMeta, + loadMeta, + INCREMENTAL_SCHEMA_VERSION, + type RepoMeta, +} from '../../src/storage/repo-manager.js'; +import { createTempDir } from '../helpers/test-db.js'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const FIXTURE_SRC = path.resolve(HERE, '..', 'fixtures', 'mini-repo', 'src'); + +/** + * Copy the mini-repo fixture into a fresh git-initialized temp directory. + * Returns the temp handle so the caller owns cleanup. + */ +async function setupMiniRepo(): Promise<{ dbPath: string; cleanup: () => Promise }> { + const tmp = await createTempDir('gitnexus-incr-orch-'); + const dest = path.join(tmp.dbPath, 'src'); + await mkdir(dest, { recursive: true }); + // Copy mini-repo fixture files + const names = [ + 'index.ts', + 'handler.ts', + 'validator.ts', + 'formatter.ts', + 'middleware.ts', + 'logger.ts', + 'db.ts', + ]; + for (const n of names) { + await copyFile(path.join(FIXTURE_SRC, n), path.join(dest, n)); + } + execSync('git init', { cwd: tmp.dbPath, stdio: 'pipe' }); + execSync('git -c user.name=test -c user.email=t@t -c commit.gpgsign=false add -A', { + cwd: tmp.dbPath, + stdio: 'pipe', + }); + execSync('git -c user.name=test -c user.email=t@t -c commit.gpgsign=false commit -q -m initial', { + cwd: tmp.dbPath, + stdio: 'pipe', + }); + return tmp; +} + +describe('runFullAnalysis — incremental orchestration', () => { + it('first run populates fileHashes + schemaVersion and clears incrementalInProgress on success', async () => { + const repo = await setupMiniRepo(); + try { + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + await runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }); + + const { storagePath } = getStoragePaths(repo.dbPath); + const meta = await loadMeta(storagePath); + expect(meta).not.toBeNull(); + expect(meta!.schemaVersion).toBe(INCREMENTAL_SCHEMA_VERSION); + expect(meta!.fileHashes).toBeDefined(); + expect(Object.keys(meta!.fileHashes ?? {}).length).toBeGreaterThan(0); + // Dirty flag MUST be cleared after a successful run. + expect(meta!.incrementalInProgress).toBeUndefined(); + } finally { + await repo.cleanup(); + } + }, 180_000); + + it('second run on unchanged state takes the alreadyUpToDate fast path', async () => { + const repo = await setupMiniRepo(); + try { + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const first = await runFullAnalysis( + repo.dbPath, + { skipAgentsMd: true }, + { onProgress: () => {} }, + ); + expect(first.alreadyUpToDate).toBeUndefined(); + + const second = await runFullAnalysis( + repo.dbPath, + { skipAgentsMd: true }, + { onProgress: () => {} }, + ); + // lastCommit==HEAD && working tree clean (mod GitNexus output) → + // early-return fast path. + expect(second.alreadyUpToDate).toBe(true); + } finally { + await repo.cleanup(); + } + }, 300_000); + + it('second run after a comment-only edit takes the incremental path, clears the dirty flag, and preserves graph stats exactly', async () => { + const repo = await setupMiniRepo(); + try { + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + await runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }); + const { storagePath } = getStoragePaths(repo.dbPath); + const firstMeta = await loadMeta(storagePath); + + // Modify a source file with a COMMENT-ONLY edit — by construction + // this changes the content hash (driving the incremental code path) + // without changing any symbol, scope binding, call edge, import, + // or community membership. Therefore every graph-stat invariant + // (files / nodes / edges / communities / processes) MUST be + // bit-identical to the first run. Anything else is a regression. + const target = path.join(repo.dbPath, 'src', 'logger.ts'); + const before = await readFile(target, 'utf-8'); + await writeFile(target, before + '\n// touched by test\n', 'utf-8'); + + const second = await runFullAnalysis( + repo.dbPath, + { skipAgentsMd: true }, + { onProgress: () => {} }, + ); + // The early-return alreadyUpToDate path must NOT fire (the dirty + // tree should kick the run through to incremental writeback). + expect(second.alreadyUpToDate).toBeUndefined(); + + const secondMeta = await loadMeta(storagePath); + expect(secondMeta).not.toBeNull(); + // Dirty flag must be cleared on success. + expect(secondMeta!.incrementalInProgress).toBeUndefined(); + // fileHashes[logger.ts] must have rotated to the new content. + expect(secondMeta!.fileHashes?.['src/logger.ts']).toBeDefined(); + expect(secondMeta!.fileHashes?.['src/logger.ts']).not.toBe( + firstMeta!.fileHashes?.['src/logger.ts'], + ); + // Exact-equality stats invariant. DoD §2.7: avoid bounds-only + // assertions that would mask a regression dropping half the graph. + expect(secondMeta!.stats?.files).toBe(firstMeta!.stats?.files); + expect(secondMeta!.stats?.nodes).toBe(firstMeta!.stats?.nodes); + expect(secondMeta!.stats?.edges).toBe(firstMeta!.stats?.edges); + expect(secondMeta!.stats?.communities).toBe(firstMeta!.stats?.communities); + expect(secondMeta!.stats?.processes).toBe(firstMeta!.stats?.processes); + } finally { + await repo.cleanup(); + } + }, 300_000); + + it('incremental output is byte-equivalent to a full rebuild (incremental ≡ --force on the same repo state)', async () => { + // The central correctness contract of this PR: an incremental run + // and a full rebuild from the same repo state must produce identical + // graph stats. We exercise it end-to-end: + // + // 1. setup mini-repo + run analyze (populates the index) + // 2. edit one source file (comment-only — same graph) + // 3. run incremental analyze → record secondMeta + // 4. run analyze --force from the same state → record forceMeta + // 5. assert every stats invariant is exactly equal. + // + // Steps 3 and 4 share the same on-disk file contents, so any + // divergence is purely an artifact of the writeback strategy. If + // any invariant differs, the PR's load-bearing claim is violated. + const repo = await setupMiniRepo(); + try { + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + + // Step 1: initial index. + await runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }); + + // Step 2: comment-only edit, same as the test above. + const target = path.join(repo.dbPath, 'src', 'logger.ts'); + const original = await readFile(target, 'utf-8'); + await writeFile(target, original + '\n// equivalence test touch\n', 'utf-8'); + + // Step 3: incremental writeback for the edited file. + const incremental = await runFullAnalysis( + repo.dbPath, + { skipAgentsMd: true }, + { onProgress: () => {} }, + ); + expect(incremental.alreadyUpToDate).toBeUndefined(); + const { storagePath } = getStoragePaths(repo.dbPath); + const secondMeta = await loadMeta(storagePath); + expect(secondMeta).not.toBeNull(); + + // Step 4: force a full rebuild from the SAME on-disk file state. + const forced = await runFullAnalysis( + repo.dbPath, + { skipAgentsMd: true, force: true }, + { onProgress: () => {} }, + ); + expect(forced.alreadyUpToDate).toBeUndefined(); + const forceMeta = await loadMeta(storagePath); + expect(forceMeta).not.toBeNull(); + + // Step 5: exact-equality across every stat. `toEqual` would also + // work but `toBe` per-field makes a failure pinpoint the field. + expect(secondMeta!.stats?.files).toBe(forceMeta!.stats?.files); + expect(secondMeta!.stats?.nodes).toBe(forceMeta!.stats?.nodes); + expect(secondMeta!.stats?.edges).toBe(forceMeta!.stats?.edges); + expect(secondMeta!.stats?.communities).toBe(forceMeta!.stats?.communities); + expect(secondMeta!.stats?.processes).toBe(forceMeta!.stats?.processes); + } finally { + await repo.cleanup(); + } + }, 600_000); + + it('a stale incrementalInProgress flag at startup forces a full rebuild that clears it', async () => { + const repo = await setupMiniRepo(); + try { + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + // First run lays down a normal index. + await runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }); + + // Manually corrupt meta.json with a stale dirty flag — simulates + // a crashed previous incremental run. + const { storagePath } = getStoragePaths(repo.dbPath); + const meta = await loadMeta(storagePath); + expect(meta).not.toBeNull(); + const tampered: RepoMeta = { + ...meta!, + incrementalInProgress: { + startedAt: Date.now() - 60_000, + toWriteCount: 3, + }, + }; + await saveMeta(storagePath, tampered); + + // Next run must detect the flag, force a full rebuild (which + // overwrites meta), and clear the flag. + const recovered = await runFullAnalysis( + repo.dbPath, + { skipAgentsMd: true }, + { onProgress: () => {} }, + ); + // A full rebuild was taken — the alreadyUpToDate fast path + // explicitly cannot fire because the dirty-flag check rewrote + // `options.force` to true. + expect(recovered.alreadyUpToDate).toBeUndefined(); + + const after = await loadMeta(storagePath); + expect(after!.incrementalInProgress).toBeUndefined(); + } finally { + await repo.cleanup(); + } + }, 300_000); +}); diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts new file mode 100644 index 000000000..757b9cf3a --- /dev/null +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -0,0 +1,243 @@ +import { describe, it, expect } from 'vitest'; +import { mkdtemp, rm } from 'fs/promises'; +import { tmpdir } from 'os'; +import path from 'path'; +import { + PARSE_CACHE_VERSION, + computeChunkHash, + fileContentHash, + loadParseCache, + saveParseCache, + pruneCache, + type ParseCache, +} from '../../src/storage/parse-cache.js'; +import type { ParseWorkerResult } from '../../src/core/ingestion/workers/parse-worker.js'; + +const minimalResult = (overrides: Partial = {}): ParseWorkerResult => ({ + nodes: [], + relationships: [], + symbols: [], + imports: [], + calls: [], + assignments: [], + heritage: [], + routes: [], + fetchCalls: [], + decoratorRoutes: [], + toolDefs: [], + ormQueries: [], + constructorBindings: [], + fileScopeBindings: [], + parsedFiles: [], + skippedLanguages: {}, + fileCount: 0, + ...overrides, +}); + +describe('computeChunkHash', () => { + it('produces a stable hex hash for a fixed set of (filePath, contentHash) entries', () => { + const entries = [ + { filePath: 'a.ts', contentHash: 'h-a' }, + { filePath: 'b.ts', contentHash: 'h-b' }, + { filePath: 'c.ts', contentHash: 'h-c' }, + ]; + const h1 = computeChunkHash(entries); + const h2 = computeChunkHash(entries); + expect(h1).toBe(h2); + expect(h1).toMatch(/^[a-f0-9]{64}$/); + }); + + it('is order-independent (same files in different order → same hash)', () => { + const order1 = [ + { filePath: 'a.ts', contentHash: 'h-a' }, + { filePath: 'b.ts', contentHash: 'h-b' }, + ]; + const order2 = [ + { filePath: 'b.ts', contentHash: 'h-b' }, + { filePath: 'a.ts', contentHash: 'h-a' }, + ]; + expect(computeChunkHash(order1)).toBe(computeChunkHash(order2)); + }); + + it('changes when any file content changes', () => { + const before = [ + { filePath: 'a.ts', contentHash: 'h-a' }, + { filePath: 'b.ts', contentHash: 'h-b' }, + ]; + const after = [ + { filePath: 'a.ts', contentHash: 'h-a' }, + { filePath: 'b.ts', contentHash: 'h-b-NEW' }, // b.ts content changed + ]; + expect(computeChunkHash(before)).not.toBe(computeChunkHash(after)); + }); + + it('changes when chunk membership changes (file added or removed)', () => { + const small = [ + { filePath: 'a.ts', contentHash: 'h-a' }, + { filePath: 'b.ts', contentHash: 'h-b' }, + ]; + const bigger = [...small, { filePath: 'c.ts', contentHash: 'h-c' }]; + expect(computeChunkHash(small)).not.toBe(computeChunkHash(bigger)); + }); +}); + +describe('fileContentHash', () => { + it('hashes a string deterministically', () => { + expect(fileContentHash('hello')).toBe(fileContentHash('hello')); + expect(fileContentHash('hello')).not.toBe(fileContentHash('hello!')); + expect(fileContentHash('hello')).toMatch(/^[a-f0-9]{64}$/); + }); + + it('handles Buffer input identical to its string form', () => { + const s = 'sentinel'; + expect(fileContentHash(Buffer.from(s))).toBe(fileContentHash(s)); + }); +}); + +describe('PARSE_CACHE_VERSION', () => { + it('embeds the gitnexus package version (so upgrades invalidate the cache)', () => { + // Looks like "1+1.6.4" — schema bump prefix + actual gitnexus version + expect(PARSE_CACHE_VERSION).toMatch(/^\d+\+\d+\.\d+\.\d+/); + }); +}); + +describe('pruneCache', () => { + it('drops entries whose hashes are not in the used-set', () => { + const cache: ParseCache = { + version: PARSE_CACHE_VERSION, + entries: new Map([ + ['hash-A', [minimalResult()]], + ['hash-B', [minimalResult()]], + ['hash-C', [minimalResult()]], + ]), + usedKeys: new Set(['hash-A']), + }; + const removed = pruneCache(cache, cache.usedKeys); + expect(removed).toBe(2); + expect([...cache.entries.keys()].sort()).toEqual(['hash-A']); + }); + + it('returns 0 when every entry is in use', () => { + const cache: ParseCache = { + version: PARSE_CACHE_VERSION, + entries: new Map([ + ['hash-A', [minimalResult()]], + ['hash-B', [minimalResult()]], + ]), + usedKeys: new Set(['hash-A', 'hash-B']), + }; + expect(pruneCache(cache, cache.usedKeys)).toBe(0); + expect(cache.entries.size).toBe(2); + }); +}); + +describe('loadParseCache / saveParseCache (round-trip)', () => { + it('round-trips an empty cache', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'gnx-pc-')); + try { + const cache: ParseCache = { + version: PARSE_CACHE_VERSION, + entries: new Map(), + usedKeys: new Set(), + }; + await saveParseCache(dir, cache); + const loaded = await loadParseCache(dir); + expect(loaded.version).toBe(PARSE_CACHE_VERSION); + expect(loaded.entries.size).toBe(0); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it('returns an empty cache when the file is missing', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'gnx-pc-')); + try { + const loaded = await loadParseCache(dir); + expect(loaded.entries.size).toBe(0); + expect(loaded.usedKeys.size).toBe(0); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it('returns an empty cache on version mismatch (next-run regen)', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'gnx-pc-')); + try { + // Write a cache file with a different version directly + const fs = await import('fs/promises'); + await fs.writeFile( + path.join(dir, 'parse-cache.json'), + JSON.stringify({ version: 'foreign-99', entries: { h: [] } }), + 'utf-8', + ); + const loaded = await loadParseCache(dir); + expect(loaded.entries.size).toBe(0); // mismatch → empty + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it('returns an empty cache on corrupt JSON', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'gnx-pc-')); + try { + const fs = await import('fs/promises'); + await fs.writeFile(path.join(dir, 'parse-cache.json'), '{not-json', 'utf-8'); + const loaded = await loadParseCache(dir); + expect(loaded.entries.size).toBe(0); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it('round-trips Map and Set values through the JSON replacer/reviver', async () => { + // ParsedFile.scopes[*].typeBindings is a ReadonlyMap. + // Without the replacer/reviver pair, JSON.stringify collapses Maps to + // {} and downstream code that does .get() / iterates entries crashes + // with "is not iterable". This test pins the round-trip behaviour. + const dir = await mkdtemp(path.join(tmpdir(), 'gnx-pc-')); + try { + const innerMap = new Map([ + ['k1', 'v1'], + ['k2', 'v2'], + ]); + const innerSet = new Set(['s1', 's2']); + // Stash the live Map/Set inside a synthetic ParseWorkerResult — we + // only need the serializer to traverse them. Casting to bypass the + // strict shape isn't a problem here: this test is about JSON + // round-tripping of arbitrary nested Map/Set values, not full + // ParseWorkerResult contents. + const fake = minimalResult({ + parsedFiles: [ + { + filePath: 't.ts', + // Cast through unknown to satisfy the readonly Scope shape + // while still smuggling a live Map into the serializer's + // traversal path — see comment block above. + scopes: [{ id: 's1', typeBindings: innerMap, extras: innerSet }], + } as unknown as ParseWorkerResult['parsedFiles'][number], + ], + }); + + const cache: ParseCache = { + version: PARSE_CACHE_VERSION, + entries: new Map([['chunk-h', [fake]]]), + usedKeys: new Set(['chunk-h']), + }; + await saveParseCache(dir, cache); + const loaded = await loadParseCache(dir); + const reloaded = loaded.entries.get('chunk-h')?.[0]; + expect(reloaded).toBeDefined(); + const scope = (reloaded as ParseWorkerResult).parsedFiles[0]?.scopes[0] as unknown as { + typeBindings?: unknown; + extras?: unknown; + }; + expect(scope.typeBindings).toBeInstanceOf(Map); + expect((scope.typeBindings as Map).get('k1')).toBe('v1'); + expect((scope.typeBindings as Map).size).toBe(2); + expect(scope.extras).toBeInstanceOf(Set); + expect((scope.extras as Set).has('s2')).toBe(true); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/gitnexus/test/unit/incremental-shadow-candidates.test.ts b/gitnexus/test/unit/incremental-shadow-candidates.test.ts new file mode 100644 index 000000000..207cc0b3c --- /dev/null +++ b/gitnexus/test/unit/incremental-shadow-candidates.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect } from 'vitest'; +import { shadowCandidatesFor } from '../../src/core/incremental/shadow-candidates.js'; + +describe('shadowCandidatesFor', () => { + it('returns an empty list when the input has no recognised module extension', () => { + expect(shadowCandidatesFor('README.md')).toEqual([]); + expect(shadowCandidatesFor('src/foo')).toEqual([]); + expect(shadowCandidatesFor('binary.so')).toEqual([]); + }); + + it('enumerates same-basename / different-extension candidates (pattern a)', () => { + const out = shadowCandidatesFor('src/foo/bar.ts'); + // All non-.ts module extensions on the same path should appear. + expect(out).toContain('src/foo/bar.tsx'); + expect(out).toContain('src/foo/bar.js'); + expect(out).toContain('src/foo/bar.jsx'); + expect(out).toContain('src/foo/bar.mjs'); + expect(out).toContain('src/foo/bar.cjs'); + expect(out).toContain('src/foo/bar.d.ts'); + // ...but NOT the same .ts (you can't shadow yourself). + expect(out).not.toContain('src/foo/bar.ts'); + }); + + it('enumerates directory-style index candidates (pattern b) for both path separators', () => { + const out = shadowCandidatesFor('src/foo/bar.ts'); + // POSIX form + expect(out).toContain('src/foo/bar/index.ts'); + expect(out).toContain('src/foo/bar/index.tsx'); + expect(out).toContain('src/foo/bar/index.js'); + // Windows form + expect(out).toContain('src/foo/bar\\index.ts'); + expect(out).toContain('src/foo/bar\\index.js'); + }); + + it('enumerates bare-file shadows when the added file is a directory index (pattern c)', () => { + const out = shadowCandidatesFor('src/foo/index.ts'); + // Adding foo/index.ts can shadow foo.{ext} (rare but real — converting + // a single-file module into a directory module). + expect(out).toContain('src/foo.ts'); + expect(out).toContain('src/foo.tsx'); + expect(out).toContain('src/foo.js'); + expect(out).toContain('src/foo.jsx'); + expect(out).toContain('src/foo.mjs'); + expect(out).toContain('src/foo.cjs'); + }); + + it('also handles the Windows-separator form of `foo\\index.ts`', () => { + const out = shadowCandidatesFor('src\\foo\\index.ts'); + expect(out).toContain('src\\foo.ts'); + expect(out).toContain('src\\foo.tsx'); + expect(out).toContain('src\\foo.js'); + }); + + it('handles `.d.ts` as a single extension token (not `.ts`)', () => { + // The longest-match scan in shadowCandidatesFor puts `.d.ts` first. + // For `foo.d.ts`, the noExt portion is "foo" (not "foo.d"), so the + // pattern (a) candidates should be the non-.d.ts module variants. + const out = shadowCandidatesFor('types/foo.d.ts'); + expect(out).toContain('types/foo.ts'); + expect(out).toContain('types/foo.tsx'); + expect(out).toContain('types/foo.js'); + // Not the .d.ts itself. + expect(out).not.toContain('types/foo.d.ts'); + }); + + it('deduplicates output (no candidate appears twice)', () => { + const out = shadowCandidatesFor('src/foo/bar.ts'); + expect(out.length).toBe(new Set(out).size); + }); + + it('never includes the input path itself', () => { + const input = 'src/foo/bar.ts'; + expect(shadowCandidatesFor(input)).not.toContain(input); + }); +}); diff --git a/gitnexus/test/unit/incremental-subgraph-extract.test.ts b/gitnexus/test/unit/incremental-subgraph-extract.test.ts new file mode 100644 index 000000000..dc720fd9e --- /dev/null +++ b/gitnexus/test/unit/incremental-subgraph-extract.test.ts @@ -0,0 +1,169 @@ +/** + * Tests for incremental DB writeback subgraph extraction. + * + * Locks the Finding 1 fix (PR #1479 review): cross-file edges between + * two unchanged files MUST land in the writeback subgraph when a third + * (changed) file alters their cross-file resolution. The pre-fix + * behaviour silently dropped those edges, leaving stale rows in the DB. + * + * These tests use synthetic graphs constructed via createKnowledgeGraph + * directly — they don't run the parser, so they're cheap and stable. + */ + +import { describe, it, expect } from 'vitest'; +import type { GraphNode, GraphRelationship } from 'gitnexus-shared'; +import { createKnowledgeGraph } from '../../src/core/graph/graph.js'; +import { + extractChangedSubgraph, + computeEffectiveWriteSet, +} from '../../src/core/incremental/subgraph-extract.js'; + +const makeFileNode = (id: string, filePath: string, label = 'Function'): GraphNode => + ({ + id, + label, + properties: { filePath, name: id }, + }) as unknown as GraphNode; + +const makeWideNode = (id: string, label: 'Community' | 'Process'): GraphNode => + ({ + id, + label, + properties: {}, + }) as unknown as GraphNode; + +const makeRel = ( + id: string, + sourceId: string, + targetId: string, + type = 'CALLS', +): GraphRelationship => + ({ + id, + sourceId, + targetId, + type, + properties: {}, + }) as unknown as GraphRelationship; + +describe('extractChangedSubgraph', () => { + it('includes nodes whose filePath is in the explicit toWriteSet', () => { + const g = createKnowledgeGraph(); + g.addNode(makeFileNode('a', '/repo/a.ts')); + g.addNode(makeFileNode('c', '/repo/c.ts')); + + const sub = extractChangedSubgraph(g, new Set(['/repo/c.ts'])); + + expect(sub.nodes.map((n) => n.id).sort()).toEqual(['c']); + }); + + it('always includes graph-wide nodes (Community, Process)', () => { + const g = createKnowledgeGraph(); + g.addNode(makeFileNode('a', '/repo/a.ts')); + g.addNode(makeWideNode('comm-1', 'Community')); + g.addNode(makeWideNode('proc-1', 'Process')); + + const sub = extractChangedSubgraph(g, new Set([])); // no files changed + + expect(sub.nodes.map((n) => n.id).sort()).toEqual(['comm-1', 'proc-1']); + }); + + it('includes a relationship when at least one endpoint is writable', () => { + const g = createKnowledgeGraph(); + g.addNode(makeFileNode('a:fn', '/repo/a.ts')); + g.addNode(makeFileNode('c:fn', '/repo/c.ts')); + g.addRelationship(makeRel('e1', 'a:fn', 'c:fn', 'CALLS')); + + // toWriteSet already includes A (the orchestrator expanded it via + // computeEffectiveWriteSet) — both endpoints writable, edge fires. + const sub = extractChangedSubgraph(g, new Set(['/repo/a.ts', '/repo/c.ts'])); + + expect(sub.nodes.map((n) => n.id).sort()).toEqual(['a:fn', 'c:fn']); + expect(sub.relationships.map((r) => r.id)).toEqual(['e1']); + }); + + it('skips a relationship entirely between unchanged files', () => { + const g = createKnowledgeGraph(); + g.addNode(makeFileNode('x:fn', '/repo/x.ts')); + g.addNode(makeFileNode('y:fn', '/repo/y.ts')); + g.addRelationship(makeRel('e1', 'x:fn', 'y:fn', 'CALLS')); + + const sub = extractChangedSubgraph(g, new Set(['/repo/c.ts'])); + + expect(sub.nodes).toEqual([]); + expect(sub.relationships).toEqual([]); + }); +}); + +describe('computeEffectiveWriteSet (Finding 1)', () => { + it('barrel re-export — expands the writable set to the consumer file', () => { + // Scenario: file C (a barrel) used to re-export from B; now re-exports + // from D. File A is unchanged byte-wise but its CALLS to foo() now + // resolve to D instead of B. Both A and D are unchanged at the file + // level — but A's edges have shifted. + // + // Pre-fix: toWriteSet={C} → A's nodes not deleted, A→D edge not + // inserted (neither endpoint writable). DB ends up with + // stale A→B and missing A→D. + // Post-fix: the new graph has A→C (A still imports the barrel), so + // A crosses the writable boundary and joins the effective + // write set. deleteNodesForFile(A) then clears the stale + // rows and the subgraph carries the new A→D edge. + const g = createKnowledgeGraph(); + g.addNode(makeFileNode('a:fn', '/repo/a.ts')); + g.addNode(makeFileNode('b:fn', '/repo/b.ts')); + g.addNode(makeFileNode('c:re-export', '/repo/c.ts')); + g.addNode(makeFileNode('d:fn', '/repo/d.ts')); + g.addRelationship(makeRel('e1', 'a:fn', 'c:re-export', 'IMPORTS')); + g.addRelationship(makeRel('e2', 'a:fn', 'd:fn', 'CALLS')); + + const effective = computeEffectiveWriteSet(g, new Set(['/repo/c.ts'])); + + expect([...effective].sort()).toEqual(['/repo/a.ts', '/repo/c.ts']); + }); + + it('picks up edges pointing INTO the changed file (symmetric case)', () => { + const g = createKnowledgeGraph(); + g.addNode(makeFileNode('b:fn', '/repo/b.ts')); + g.addNode(makeFileNode('c:fn', '/repo/c.ts')); + g.addRelationship(makeRel('e1', 'b:fn', 'c:fn', 'CALLS')); + + const effective = computeEffectiveWriteSet(g, new Set(['/repo/c.ts'])); + + expect([...effective].sort()).toEqual(['/repo/b.ts', '/repo/c.ts']); + }); + + it('does not expand when no edge crosses the writable boundary', () => { + const g = createKnowledgeGraph(); + g.addNode(makeFileNode('x:fn', '/repo/x.ts')); + g.addNode(makeFileNode('y:fn', '/repo/y.ts')); + g.addRelationship(makeRel('e1', 'x:fn', 'y:fn', 'CALLS')); + + const effective = computeEffectiveWriteSet(g, new Set(['/repo/c.ts'])); + + expect([...effective].sort()).toEqual(['/repo/c.ts']); + }); + + it('ignores edges to graph-wide nodes (no filePath)', () => { + const g = createKnowledgeGraph(); + g.addNode(makeFileNode('a:fn', '/repo/a.ts')); + g.addNode(makeWideNode('comm-1', 'Community')); + g.addRelationship(makeRel('e1', 'a:fn', 'comm-1', 'BELONGS_TO')); + + const effective = computeEffectiveWriteSet(g, new Set(['/repo/a.ts'])); + + expect([...effective].sort()).toEqual(['/repo/a.ts']); + }); + + it('does not mutate the input set', () => { + const g = createKnowledgeGraph(); + g.addNode(makeFileNode('a:fn', '/repo/a.ts')); + g.addNode(makeFileNode('c:fn', '/repo/c.ts')); + g.addRelationship(makeRel('e1', 'a:fn', 'c:fn', 'CALLS')); + + const input = new Set(['/repo/c.ts']); + computeEffectiveWriteSet(g, input); + + expect([...input]).toEqual(['/repo/c.ts']); + }); +});