mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-03 02:21:44 +00:00
2 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
0261982d9a
|
fix(analyze): make --memory-budget set the real heap and report rebuild reasons once (#3386)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* feat(analyze): --memory-budget flag with heap-limit override and worker-pool degradation (#3137) Adds an explicit `--memory-budget <mb>` CLI flag that overrides the RAM/cgroup auto-sized main-thread heap ceiling for the parse phase: - CLI validation (integer >= 200 MB) before bar.start(), matching the --workers pattern - Threaded CLI → runFullAnalysis → PipelineOptions → parse-impl as memoryBudgetBytes - parse-impl resolves the heap limit as budget ?? v8.heap_size_limit, so both the preflight projection warning and the #2649 mid-loop abort probe honor the budget - Graceful degradation: when the projected heap need exceeds the budget at the computed pool size, the pool shrinks (never below 1, never above the operator's --workers) before sub-batch math and pool construction, so all downstream consumers see the degraded size Omitting the flag keeps the auto-sizer path byte-identical. Refs #3137 * feat(analyze): collapse rebuild-gate log into one summary + persist needsFullRebuild verdict (#3137) The nine meta-mismatch rebuild gates (pdg mode, content retention, schema fingerprint, graph-write collapse, analysis features, Spring vendor prefixes, runner identity, FTS CJK mode, embedding dims) each logged individually and set force:true independently. An upgrade that trips several at once printed a scattered wall of near-identical warnings. - Gates now collect into rebuildReasons[]; a single summary block prints them (inline for one, numbered for many) and sets force once. Per-gate Tip text is preserved verbatim inside the entries. - The verdict persists to meta (needsFullRebuild: {reasons, recordedAt}) BEFORE the rebuild starts. If the rebuild is interrupted, the next run announces the recorded reasons up front instead of quietly attempting an incremental write on a half-rebuilt index — the gates may not all re-fire against a wiped DB. - The verdict is cleared on the next successful completion (the final meta does not carry the field forward). Semantics unchanged: every gate was already evaluated (none early-returns), force is idempotent, and a rebuild happens iff at least one reason fired. Refs #3137 * refactor(cli): share one integer flag parser across analyze, watch, and wiki Replace the duplicated Number.isInteger checks for --workers, --embeddings, the positive env-backed analyze flags, the watch interval flags, and wiki's --timeout/--retries with parseIntegerOption (per-flag minimum, optional scale for the safe-integer bound). User-facing messages are unchanged. * fix(analyze): make --memory-budget set the real V8 heap through the respawn The budget now drives ensureHeap's existing respawn instead of a parse-phase override, so the #2649 preflight, mid-loop abort, remedy text, and GC pacing all see one heap limit. The respawn sizes old space plus three semi-spaces to equal the budget, the child resolves as already at the budget (no second respawn), and GITNEXUS_HEAP_LIMIT_SOURCE drives budget-aware OOM advice. Budget validation moves to the preAction hook so analyze and watch reject a bad value before any respawn. Removes the pool-shrink block and the memoryBudgetBytes plumbing through PipelineOptions and run-analyze. * docs(analyze): describe --memory-budget accurately and translate its help The help text claimed graceful worker-pool degradation, which no longer exists; it now says the flag sets the main-thread V8 heap and that parse workers keep their own caps. Wires the option through the help i18n map with en and zh-CN strings, and documents it in both READMEs and the out-of-memory troubleshooting section. * feat(analyze): add a pure rebuild-reason collector One collector per run holds keyed rebuild reasons, merges by key, flattens reasons stored by an interrupted rebuild into one recovery entry, validates stored reasons on read, and formats the single up-front summary plus one follow-up line for reasons added after the pipeline. * fix(analyze): route every forced rebuild through one reason collector Every path that forces a full rebuild (the nine meta gates, --force, --skills, --no-parse-cache, --drop-embeddings, --repair-fts retention, Spring Actuator, AsyncAPI, shared-store graph gaps, dirty-flag recovery, the post-pipeline capability gate, and the #2409 escalation) now adds a keyed reason to one collector. The rebuild decision is applied from the collector at fixed checkpoints, one summary prints right before the pipeline, and late reasons print one follow-up line. The escalation stays non-forcing. runFullAnalysis returns the collected keys, which replaces the runner-identity source-regex test with a behavior test. Removes the separate needsFullRebuild field and its announcement, and stops folding --skills and --no-parse-cache into --force. * fix(analyze): persist rebuild reasons on the existing crash marker Every incrementalInProgress writer (the full-rebuild stamp before the wipe, the incremental pre-write, saveIncrementalDirtyState including the #2409 escalation, and buildFtsDirtyStamp) now carries the collected reasons into the active slot's metaDir, so an interrupted rebuild explains itself on the next run through one merged recovery entry. A successful run still clears the marker and its reasons; the FTS-park recovery clears them without forcing. * test(analyze): cover every rebuild-reason key through runFullAnalysis Add a coverage table that the typechecker keeps complete: every RebuildReasonKey maps to a test file that drives it through runFullAnalysis and asserts the returned key. Adds the missing graph-write-collapse and drop-embeddings drivers, asserts the key in the existing pdg-mode, spring-vendor-prefixes, cjk-segmentation, and embedding-dims tests, and removes plan-local IDs from test names and comments. * fix(review): apply review findings - A --max-old-space-size pin equal to --memory-budget no longer counts as the exact budget heap (V8 adds the young generation on top); only the budget-respawned child skips the respawn, so the limit really equals the budget. - Snapshot the analyze env before ensureHeap and restore GITNEXUS_HEAP_LIMIT_SOURCE, so a kept process does not leak its heap source into a later programmatic analyzeCommand call. - --skills and --no-parse-cache keep the forced storage requirements they had before force stopped being folded from them. - Merge the duplicated follow-up announcement into one helper and fix a stale --drop-embeddings comment. - The rebuild-reason coverage table no longer greps driver files for the key string; add tests for a programmatic invalid budget and the multi-cause interrupted-rebuild text. * fix(review): don't announce the escalated write as a full rebuild The #2409 escalation is a non-forcing reason, but its follow-up line used the 'Full rebuild also required' lead. A follow-up that carries only non-forcing reasons now leads with 'Write plan changed'. * docs(analyze): document GITNEXUS_HEAP_LIMIT_SOURCE in the env table CONTRIBUTING requires every new GITNEXUS_* variable to have a row; this one is internal (set by analyze itself) and exists so OOM advice points at --memory-budget. * fix(review): address GitNexus review threads on #3386 - heapPressureRemedy measures pressure against the real auto-sized cap (heapCapMbFor) instead of a flat 0.75 x RAM, and no longer tells a GITNEXUS_MEMORY=off run with no pin to drop a pin that does not exist. - toStored() persists the interrupted rebuild's reasons first, as documented. - ensureHeap's doc names which paths leave GITNEXUS_HEAP_LIMIT_SOURCE unset. - The heap-respawn suite restores the caller's GITNEXUS_MEMORY. - The non-forcing follow-up test rejects any 'full rebuild' wording. * fix(review): require both budget flags and check key coverage at runtime - A budget-respawned child is recognized only when the inherited heap-source marker comes with both the budget's old-space and semi-space flags; the marker alone is an inherited env var, not proof. The old-space parser is generalized to any V8 size flag instead of copying its regex. - REBUILD_REASON_KEYS is exported and RebuildReasonKey derives from it, so the coverage table is checked at runtime (CI does not type-check test files). * fix(review): don't claim a full rebuild in a non-forcing summary formatSummary and formatFollowUp now share one leadFor helper, so a block of only non-forcing reasons reads 'Write plan changed' in both. --------- Co-authored-by: ChunxueLi <mecoloud@users.noreply.gitee.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> |
||
|
|
7468cc915b
|
fix(analyze): replace the hand-incremented schema version with a derived DDL fingerprint (#2798) (#2808)
* feat(schema): derive a fingerprint from the DDL this build creates `SCHEMA_FINGERPRINT` is a sha256 digest of the node and relation DDL that `runSchemaCreationQueries` actually executes, in the same shape as the existing `taintModelVersion` stamp (hex, sliced to 12). It exists because `INCREMENTAL_SCHEMA_VERSION` is hand-picked and has to *predict* whether an on-disk database matches this build's DDL. That number has collided with `main` eight times, twice exactly — and an exact clash is the quiet one, because the reuse gate is a strict `===`. `EMBEDDING_SCHEMA` is deliberately excluded: its `FLOAT[N]` width comes from `GITNEXUS_EMBEDDING_DIMS` at module load, so folding it in would make the digest a function of the environment rather than of code, and two runs of the same build under different env would thrash full rebuilds. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(storage): record the DDL fingerprint in RepoMeta `RepoMeta.schemaFingerprint` stores the digest of the DDL an index's tables were actually created from. It is the derived companion to `schemaVersion`, not its replacement: both are compared, and both must match. Absent means mismatch, deliberately. Grandfathering a missing fingerprint would let an incremental top-up stamp a fresh one onto a database whose DDL was never verified, permanently certifying exactly the wrong-shaped index the field exists to catch. The cost is one full rebuild per pre-existing index. The version ladder gains a note that its "re-check against origin/main before merge" ritual now only guards *semantic* bumps. v25, v26, v30, v31 and v34 all changed emitted ids, edges or wire formats while leaving the DDL byte-identical, and the fingerprint cannot see any of them — but DDL collisions no longer need renumbering. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(analyze): gate index reuse on the DDL fingerprint, not just the version (#2798) `INCREMENTAL_SCHEMA_VERSION` is a hand-incremented integer that has to predict a derived fact: whether the on-disk DDL matches the code's DDL. It has collided with `main` eight times, and twice the collision was *exact*. An exact clash is the silent one. Two builds stamp the same number over different DDL, the `===` reuse gate reads the index as current, every `CREATE ... TABLE` is then skipped as "already exists" (suppressed in `runSchemaCreationQueries`), and the edges whose endpoint pair the live database cannot hold are dropped by `fallbackRelationshipInserts`' bare `catch`. The result is a wrong graph, with no error anywhere. Reuse now requires the version AND the DDL fingerprint to match, in both the pre-pipeline force-rebuild guard and the `isIncremental` predicate, and the fingerprint is stamped alongside the version at the end of a run. Both conditions are necessary. The fingerprint does not replace the integer: most entries in the version ladder change emitted ids, edges or wire formats while the DDL stays byte-identical, and a fingerprint-only gate would stop forcing rebuilds for all of them. What it does buy is that two branches picking the same number no longer need renumbering. The new branch sits above the `alreadyUpToDate` fast path for the same reason the version guard does — a clean tree at an unchanged commit would otherwise early-return before either check ran. Closes #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(analyze): pin the DDL fingerprint gate and its two failure cases `schema-fingerprint.test.ts` pins the properties the gate rests on: the digest covers exactly the node and relation DDL that gets executed (recomputed from the exported lists, so adding a table or a FROM/TO pair without the fingerprint moving is impossible), it excludes the environment-derived embedding DDL, and it moves when any covered string moves. The two `incremental-orchestration` cases exercise the production path rather than modelling it: an index carrying the *current* version with a foreign fingerprint, and one with no fingerprint at all. Both were run against the pre-fix tree first and both failed there with `alreadyUpToDate === true` — the fast path swallowing the mismatch, which is the #2798 symptom exactly. `call-summary-schema-version.test.ts` widens its gate model to two equalities. The second argument defaults to the current fingerprint so all 33 existing version cases read unchanged, and a new case covers the collision, the legacy absence, and the semantic bump the fingerprint cannot see. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(review-skill): point the schema-constant check at the fingerprint, not the deleted integer All four `gitnexus-review` SKILL.md mirrors told reviewers to verify `INCREMENTAL_SCHEMA_VERSION` "was bumped or regenerated". That constant no longer exists, so the instruction sent every future reviewer looking for something they could not find — and, worse, past its replacement. The check for graph DDL is now derived: `SCHEMA_FINGERPRINT` moves on its own, so the question is whether the diff changed a string in `NODE_SCHEMA_QUERIES` / `REL_SCHEMA_QUERIES`, and whether a newly added DDL array was folded into the fingerprint at all — the one way the derived gate can still be bypassed. What did NOT change is called out explicitly: the parse-store `SCHEMA_BUMP` and the bench fingerprint sets are still hand-maintained and still need the re-check-against-base ritual, and semantic changes that leave the DDL untouched fall outside the fingerprint entirely — those rely on the analyzer runner-identity receipt. Found by the review swarm's docs lane. The original plan for #2798 claimed no documentation mentioned the constant; that sweep covered five root docs and never looked at `.claude/skills/**` or the three mirrors. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(migration): record the one-time rebuild the fingerprint switch costs Replacing `schemaVersion` with `schemaFingerprint` means every index written by an earlier GitNexus carries no fingerprint, reads as a mismatch, and is rebuilt once. That is deliberate — grandfathering absence would stamp a fresh fingerprint onto a database whose DDL was never verified — but until now it was undocumented, so a user's first post-upgrade analyze would announce a full re-analyze with nothing to explain it. MIGRATION.md already sets the precedent: PR #2363's meta.json → gitnexus.json rename was equally automatic and equally in need of an entry. This follows that shape, and is explicit about the parts that are easy to undersell: - the cost is per INDEX, and branch-scoped slots (#2106) each pay separately; on a large repository a full re-analyze is substantial, not a blip; - rollback is safe — an older binary sees no `schemaVersion` and forces its own rebuild, which is a cost, never a stale graph; - alternating between an old and a new binary rebuilds on every switch, because the end-of-run meta is written as a fresh literal so neither field survives the other's run. The retired ladder's per-version rationale is pointed at in git history rather than reproduced: `git show 561f913a3:.../repo-manager.ts`. That commit is an ancestor of origin/main, so the pointer survives this branch being squash-merged. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(identity): cover workspace-linked packages in the analyzer dependency digest `dependencyNames` enumerated `dependencies`, `optionalDependencies` and `peerDependencies` only. `gitnexus-shared` is declared as a devDependency (`file:../gitnexus-shared`), and in a source-mode run the build root is the gitnexus package tree, which does not contain that sibling. So a change to gitnexus-shared moved neither `build.digest` nor `dependencyRuntime.digest`. That gap matters more since #2798 deleted `INCREMENTAL_SCHEMA_VERSION`. A DDL-affecting edit there is still caught by `SCHEMA_FINGERPRINT`, but a SEMANTIC-only edit — a new `REL_TYPES` member, say, where the relation table carries a bare `type STRING` column so no CREATE statement moves — was covered by nothing at all. Roughly thirty of the retired ladder's entries were exactly that change class, and the runner-identity receipt is what now carries them. Only checkout-local specifiers are added: `file:`, `link:`, `workspace:`, `portal:` and npm's bare local-path shorthands. Pulling in every devDependency was rejected — vitest, eslint and typescript would enter the digest and force a full re-analyze on unrelated tool bumps, which is worse than the hole. Scanning the linked sibling for the first time exposed a latent throw: `collectArtifacts` honoured `PRUNED_RUNTIME_DIRECTORIES` only for a real directory, so a SYMLINKED `node_modules` fell through to the payload branch and died with "Analyzer identity input is not a file". Worktree-style dev layouts and pnpm shared stores hit this immediately — verified in this worktree, where `gitnexus-shared/node_modules` is such a symlink. Pruning it loses nothing: packages beneath are still reached through `resolveDependencyPackageRoot`. Verified: a real `analyze` in this worktree succeeds with `packageCount` 259; editing the linked package's source moves the digest, bumping an installed registry devDependency does not, and removing the link moves it. `DEPENDENCY_RUNTIME_CANONICALIZATION` is deliberately not bumped — freshness compares digests, not the label, and the input-set change already moves them. Follow-up worth having: no fixture in the suite declares `devDependencies`, so this has no regression test yet. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(analyze)!: delete INCREMENTAL_SCHEMA_VERSION, gate reuse on the DDL fingerprint alone The integer and its ~180-line version ladder are gone, along with `RepoMeta.schemaVersion`. Index reuse is now decided solely by `SCHEMA_FINGERPRINT`; a mismatch — including the absent stamp every pre-existing index carries — warns and forces a full re-analyze, which wipes and recreates the database so the tables are built from the current DDL. Deleting the integer is safe because it was already redundant: the runner-identity guard deep-compares the whole schema-v4 receipt, including a digest over the build tree, and forces a rebuild on ANY analyzer delta. Verified empirically — a comment-only edit to logger.ts, with the fingerprint byte identical, produced "runner identity changed ... forcing a full rebuild". The fingerprint is not thereby redundant. It fires where that guard cannot: a DDL-affecting change in `gitnexus-shared`, which is a workspace-linked devDependency and so sat outside both digests until the companion commit closed that gap. Review findings folded in, each correcting a line this rewrite itself introduced and never published: - B1: two assertions matched a log string the rewrite had renamed; both tests failed. They now assert what production emits. - B2: the pre-existing downgrade test perturbed `schemaVersion: 7`, a field this change deletes, so the spread carried a valid fingerprint, every guard passed, and the run legitimately took the fast path. It perturbs the fingerprint now, restoring the only integration coverage of the gate-above-the-fast-path ordering invariant. - N5: duplicate `schemaFingerprint` keys silently collapsed two assertions into one (TS1117). - N6: the absent-stamp message told non-git repositories their index was "built by an older GitNexus version" — on every run, about an index this exact build had just written. Non-git repos never record a fingerprint, and now the message says so. - N9: the on-disk stamp is shape-checked before being echoed, so a crafted gitnexus.json cannot push ANSI escapes through the CLI log. - N7: a test case that re-computed the same digest expression with its operands swapped, mislabelled as a randomness check on a module-level const. - N10: comments claiming the digest "cannot collide" (it is 48 bits), pointing at a vector-column gate that does not exist, and asserting storage/ is free of a core/ dependency two lines below a core/ value import. None of these were caught by `tsc -p tsconfig.json`, which covers src only, nor by eslint, where no-dupe-keys is off. `tsconfig.test.json` reports all three test defects and is not currently wired into CI. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(schema): pin that the fingerprint covers every DDL statement init executes `SCHEMA_QUERIES` is the list `runSchemaCreationQueries` iterates — the DDL that actually runs. The fingerprint hashes only two of its three members, and until now no test imported `SCHEMA_QUERIES` at all, so nothing tied the two together. A fourth member appended to that array — the one literally named for what init executes — would have been invisible to the gate. Every existing test would still pass, because they all recompute the digest from the same two arrays the fingerprint already uses. An index whose gate passed would then run `initLbug` over the old database, where `runSchemaCreationQueries` suppresses "already exists", so the new table would never be created and its edges would be dropped by `fallbackRelationshipInserts`' bare catch. A wrong graph, no error — exactly the failure #2798 exists to end. The check is a pure predicate over (executed, fingerprinted, documented exclusions) rather than a positional `toEqual`, so `EMBEDDING_SCHEMA` is named as an exclusion with its reason — its FLOAT[N] width is environment-derived — rather than sitting in a list where a future reader might "fix" it by folding it in. It asserts both directions and is order-insensitive, leaving ordering to the digest assertion that already pins it. The negative case is pinned in CI rather than checked by hand once: the same predicate over a synthetic fourth member must report it. If a refactor ever makes the predicate vacuous, that case fails even though the positive one would not. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(analyze): name the invariant the version deletion now rests on Deleting `INCREMENTAL_SCHEMA_VERSION` moved a load-bearing guarantee into an implicit one. Roughly thirty of the retired ladder's entries changed no DDL at all — node ids, wire formats, resolution tiers — and the fingerprint is structurally incapable of firing on any of them. Their only remaining cover is the analyzer runner-identity receipt, and nothing in the suite said so. This adds a table over the real `analyzerRunnerIdentitiesEqual` with a well-formed schema-v4 receipt: byte-identical reuses; an entrypoint-only difference reuses (CLI vs analyze worker); a moved build digest with unchanged DDL forces — that case IS the invariant, commented as such; and a dependency change, an ABI change, undefined, null, a schema-v3 legacy receipt, a missing build section and a non-sha256 digest all fail closed. The deleted `expect(INCREMENTAL_SCHEMA_VERSION).toBe(35)` pin is also worth naming: it failed CI on every bump by design, which is what made an author stop and think. Nothing replaced it. This does not restore that — a digest has no literal to pin — but it does make the mechanism that took over the job visible to the next person who reads the file. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(spring): pin CLASS_SCHEMA's membership in the fingerprinted DDL set When `INCREMENTAL_SCHEMA_VERSION` went away, its sibling in basicblock-callee-ids-schema.test.ts got a replacement assertion tying BASICBLOCK_SCHEMA to the fingerprint's input set. This file's `>= 23` floor was deleted with nothing put in its place. The file still asserts CLASS_SCHEMA's CONTENT — that the `frameworkAnnotations` column exists — but not that CLASS_SCHEMA is part of what the digest covers, and the second is what makes an index built before that column carry a different fingerprint and get rebuilt. Mirrors the sibling so the two read the same way. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(identity): stop a symlinked directory from aborting the whole analyze `collectArtifacts` fused two orthogonal facts into one condition: that four directory names never carry runtime payload, and that a symlink where a real directory was assumed falls through to the payload branch, where `snapshotReadableFile` stats the target, sees a directory, and throws "Analyzer identity input is not a file". The second was only fixed for those four names. Every other symlinked directory in a scanned package root still aborted the run — `dist -> build`, a vendored grammar link, anything inside a linked sibling checkout. Newly reachable, because making workspace-linked packages scannable pointed the scanner at a live checkout instead of an immutable registry tarball for the first time. Split along the actual seam: prune on the NAME alone, and give symlinks their own branch in the type dispatch, ahead of the payload branch. Link text is recorded rather than followed. Following was rejected on three grounds, each checked in source: the traversal is a stack with no visited set, so a self-referential link would recurse to `runtimeDepth` — which throws, trading one hard abort for another; `snapshotDirectory` rejects a symlink outright, so the directory guard could not accept one without a realpath rewrite of its canonical-path identity; and a link into an already-scanned tree double-counts against `runtimeEntries`/`runtimeBytes`, which also throw. The cost is stated in code: a link out of the package contributes its text, not its target's content. Links resolving to a regular file keep the existing content digest. The new `'unfollowed-symlink'` kind is threaded through every consumer, including the cache validator — which re-probes with `mode: 'link'`, since the readable-file probe resolves the target and would return null for exactly this kind, silently failing every warm validation. No canonicalization or cache-schema bump. Digest content changes only for trees that previously crashed: a delta scan over all 258 scanned roots of this install found no regular file bearing a pruned name and no symlink failing to resolve to a file, so `dependencyRuntime.digest` is byte-identical here. Six of the eight new tests fail against the unfixed tree with the exact production error; all eight pass after. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(analyze): give the reuse gate a real seam and sanitize logs at the funnel Cleanup pass over the #2798 branch. Net -183 lines. The gate had no extracted predicate, so its own test asserted it by regex-matching run-analyze.ts SOURCE TEXT. That pinned production formatting: one pattern froze three back-to-back single-name imports from './lbug/schema.js', so merging them — the obvious tidy-up — failed a test named "still imports the DDL digest itself". `schemaFingerprintMismatch` and `isSchemaFingerprintShaped` now live in core/lbug/schema.ts beside the constant. Not in run-analyze.ts next to `pdgModeMismatch`, because storage/ must stay off the analyze pipeline and mcp/resources.ts is a plausible second consumer — the same reasoning that puts `cjkSegmentationModeMismatch` in core/search/. The regex block is gone; the test calls the predicate. The three imports are merged. ANSI sanitation moved from one field to the funnel. The per-field guard's own comment stated the general hazard — gitnexus.json is parsed with no runtime shape validation and the notice reaches console.log — while two sibling guards twelve lines away echoed `runnerIdentity.schemaVersion` and `cjkSegmentation` from that same file raw into the same log. `log()` now strips C0/C1 controls, covering all seven guard messages and any written later. Also: - Deleted a duplicate integration test. After the downgrade test was repointed at `schemaFingerprint` it became the same scenario as the new one, differing only by an extra log assertion — which is now folded into the survivor. Saves a fixture and two full pipeline runs per CI pass. - Replaced a 3-parameter set-difference helper with one set equality. Its doc was false at one call site (arguments semantically swapped) and it needed a fourth test purely to prove itself non-vacuous; set equality cannot go vacuous. - Removed ~115 lines of runner-identity table that duplicated analyzer-identity.test.ts. The three genuinely uncovered cases moved there, and the #2798 invariant — build digest moved while the DDL did not — now asserts against a REAL analyzer-build-tree edit rather than a hand-built literal, which is strictly stronger than what it replaces. - MIGRATION.md quoted a log line the code cannot emit; it was written before the placeholder changed. - Restored the rationale on the `capabilities` docstring, which a previous pass replaced with its consequence — leaving a maintainer reading "duplicated by hand" as a wart to fix by importing, which is what the original forbade. - Marked the `isIncremental` conjunct as belt-and-braces: `!options.force` short-circuits before it, so it cannot decide anything. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(analyze): force a rebuild when the vector column width changes `CodeEmbedding.embedding` is declared `FLOAT[EMBEDDING_DIMS]`, resolved from `GITNEXUS_EMBEDDING_DIMS` at module load. Nothing gated it. Flip the variable on a same-commit clean tree and no guard fired at all: `alreadyUpToDate` returned over a `FLOAT[384]` table while the process embedded at 768. The only reaction anywhere discards the embedding CACHE and re-embeds — into a column whose type it never revisits. This predates #2798; `INCREMENTAL_SCHEMA_VERSION` never covered dims either. It surfaced because the fingerprint work had to reason about why `EMBEDDING_SCHEMA` must stay OUT of the digest: its width is environment-derived, so folding it in would make the same build disagree with itself and thrash rebuilds. That exclusion is correct, and it leaves the width needing its own guard. Modelled on `cjkSegmentation`, the closest sibling: an env-resolved scalar stamped at write time and compared by a small exported predicate that forces on mismatch. `embeddingDimsMismatch` sits in core/lbug/schema.ts beside `EMBEDDING_DIMS`, so the query side can adopt it without importing the analyze pipeline — mcp/local/local-backend.ts already warns on a cjkSegmentation disagreement and has the identical claim here, since the query path embeds at the live width against a table of unknown width with no validation at all today. ABSENCE IS NOT A MISMATCH, deliberately. Forcing on it would be dead code: `embeddingDims` and `schemaFingerprint` ship together, and a missing fingerprint already forces exactly one rebuild — which is where this stamp lands. Absence also carries no signal here, unlike the fingerprint: a missing fingerprint means "DDL this build cannot vouch for" and ships WITH a DDL change, whereas a missing dims stamp means only "written before the field existed", and that run's table agreed with that run's width. Drift requires the env to change, which absence says nothing about. The `cjkSegmentation` trick of folding absence into the default was unavailable — there is no width that is safe to assume for an existing table — so the stamp is instead written unconditionally, giving absence exactly one meaning. Malformed values are not grandfathered: null, '384', NaN and objects all read as a mismatch and err toward a rebuild. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(mcp): warn when the served index's vector width differs from the query embedder's The analyze side now forces a rebuild when the vector column width changes. The query side had no equivalent: a serving process embeds a query at its own width and searches a table whose width was fixed when the index was built. Disagree and the user gets wrong or missing semantic results with nothing explaining why. Mirrors the cjkSegmentation drift warning immediately above it — same warnings[] array, same per-query recomputation, agent-visible in the tool response, and it warns rather than refuses. A width mismatch degrades the semantic lane only; keyword results are unaffected, so `partial` is deliberately not set. Compares against `getEmbeddingDims()` — the width the query embedder actually produces — NOT schema.ts's `EMBEDDING_DIMS`. The two diverge exactly when GITNEXUS_EMBEDDING_DIMS is set on a server that embeds LOCALLY: the query path ignores that variable and embeds at 384, so comparing against the env-derived constant would report drift on a lane that works fine. The recorded width is what the vector CAST actually binds. `embeddingDimsMismatch` is imported from core/lbug/schema.js rather than restated, so "absent is not a mismatch" cannot drift between the analyze and query sides. That predicate was placed in schema.ts precisely so this consumer could reach it without importing the analyze pipeline. Two gates keep it quiet when it would be noise: it fires only for a repo where this process actually produced a query vector, so an index analyzed without --embeddings (or a server whose embedder is unavailable) never carries it. An untrusted recorded value — meta.json is schema-less JSON — is reported as "an unrecognized width" rather than echoed. `loadMeta` is hoisted out of the neighbouring try so both diagnostics share one read and an invalid GITNEXUS_FTS_CJK_SEGMENTATION cannot take this one down with it. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(identity): detect an npm-linked dev dependency the specifier cannot see `isLocallyLinkedSpecifier` admits a devDependency whose SPECIFIER is checkout-local. `npm link <pkg>` leaves the specifier a registry range while the node_modules entry symlinks to a checkout — locally linked, invisible to a specifier check, so a semantic-only edit there still moves neither digest. The obvious placement is unaffordable, measured rather than assumed: probing every dev-only name inside collectRuntimePackages costs 1998 resolutions, not the ~8 it looks like, because dependencyNames runs for every package in the BFS and published tarballs retain their devDependencies. Persisted path guards go 2221 -> 11050 (+398%), and every guard is re-probed on each warm validation — the path `status` takes. Scoped to the root package instead. The declared-intent half is untouched and still enumerated everywhere: it alone can emit the `<missing>` edge for a declared link whose checkout is absent, where resolution returns null and cannot distinguish that from an uninstalled dev tool. The new resolved-location half runs only when `parent.root === packageRoot`, resolves through the existing resolver so its path guards are recorded, and admits a name iff the realpath'd root carries no node_modules segment. Bounded against mis-fire by EXPANSION. "Not under node_modules" is a proxy for "checkout-local"; under a relocated pnpm virtual store every dev dep passes it and the whole dev tree folds into the receipt — against limits that THROW, so a legitimate install would abort. Measured here: uncapped, that shape takes 259 -> 347 packages and 2250 -> 3786 guards. The cap admits at most four and DROPS THE WHOLE CHANNEL on overflow rather than an arbitrary prefix, because the abort comes from the transitive payload of whichever trees get folded in — four of a mis-fired thirteen is still unbounded, and a sorted-prefix receipt would be arbitrary. Overflow falls back to the specifier-only receipt that ships today. Cost on this install: 259 packages unchanged, 13 dev names resolved, guards 2221 -> 2250 (+29, +1.3%). Verified against the real implementation, not just a replay: validation guards 16295 -> 16324, packageCount and artifactCount unchanged, and `dependencyRuntime.digest` byte-identical — so this forces no re-analysis for anyone. Each test fails on the defect it targets: disabling the channel kills the npm-link and cap cases; dropping the root-only scope makes the differential guard-count case fail at 2.8x guards; removing the specifier half kills the `<missing>` case. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |