From 0261982d9a8c8ac5a187e9c60e414d6e5fa63b09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sat, 26 Sep 2026 18:14:36 +0100 Subject: [PATCH] fix(analyze): make --memory-budget set the real heap and report rebuild reasons once (#3386) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(analyze): --memory-budget flag with heap-limit override and worker-pool degradation (#3137) Adds an explicit `--memory-budget ` 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 Co-authored-by: Gergo Magyar --- README.md | 1 + gitnexus/README.md | 43 +- gitnexus/src/cli/analyze-options.ts | 8 + gitnexus/src/cli/analyze-watch.ts | 11 +- gitnexus/src/cli/analyze.ts | 278 ++++++++++-- gitnexus/src/cli/help-i18n.ts | 1 + gitnexus/src/cli/i18n/en.ts | 2 + gitnexus/src/cli/i18n/zh-CN.ts | 2 + gitnexus/src/cli/index.ts | 19 + gitnexus/src/cli/int-option.ts | 65 +++ gitnexus/src/cli/wiki.ts | 28 +- .../ingestion/pipeline-phases/parse-impl.ts | 29 +- .../src/core/ingestion/utils/effective-ram.ts | 54 +++ gitnexus/src/core/rebuild-reasons.ts | 210 +++++++++ gitnexus/src/core/run-analyze.ts | 404 +++++++++++------ gitnexus/src/core/search/fts-crash-marker.ts | 3 + gitnexus/src/storage/repo-meta.ts | 7 + ...external-storage-content-retention.test.ts | 5 +- .../integration/shared-store-analyze.test.ts | 53 ++- .../test/unit/analyze-heap-respawn.test.ts | 350 +++++++++++++++ .../test/unit/analyze-memory-budget.test.ts | 129 ++++++ .../test/unit/analyze-no-stats-bridge.test.ts | 14 +- .../unit/call-summary-schema-version.test.ts | 27 +- gitnexus/test/unit/cli-int-option.test.ts | 76 ++++ .../test/unit/embedding-dims-guard.test.ts | 1 + .../unit/incremental-orchestration.test.ts | 203 ++++++++- .../test/unit/parse-impl-heap-guard.test.ts | 41 +- gitnexus/test/unit/pdg-mode-flip.test.ts | 3 + gitnexus/test/unit/rebuild-reasons.test.ts | 327 ++++++++++++++ .../unit/run-analyze-fts-crash-marker.test.ts | 422 ++++++++++++++++++ .../stream-graph-emit-force-ordering.test.ts | 171 +++++++ 31 files changed, 2701 insertions(+), 286 deletions(-) create mode 100644 gitnexus/src/cli/int-option.ts create mode 100644 gitnexus/src/core/rebuild-reasons.ts create mode 100644 gitnexus/test/unit/analyze-memory-budget.test.ts create mode 100644 gitnexus/test/unit/cli-int-option.test.ts create mode 100644 gitnexus/test/unit/rebuild-reasons.test.ts diff --git a/README.md b/README.md index 2afc65d49..1f4f39d8f 100644 --- a/README.md +++ b/README.md @@ -473,6 +473,7 @@ gitnexus analyze --max-processes # Process-detection process cap (replaces gitnexus analyze --max-entry-point-candidates # Ranked entry-point pool (default 200; raise when the warning names it) gitnexus analyze --spring-actuator ./actuator # Enrich with local Spring Boot Actuator JSON snapshots gitnexus analyze --asyncapi-spec ./docs/asyncapi # Resolve broker addresses from AsyncAPI 3.x documents +gitnexus analyze --memory-budget 3000 # Main-thread V8 heap in MB (>= 200); overrides the auto-sizer and --max-old-space-size gitnexus analyze --wal-checkpoint-threshold 67108864 # LadybugDB WAL auto-checkpoint threshold in bytes # (default 67108864 = 64 MiB; -1 keeps Ladybug stock ~16 MiB) ``` diff --git a/gitnexus/README.md b/gitnexus/README.md index 1d5012a6f..bec30c92b 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -212,25 +212,25 @@ Note that the bundled Graphology path is no longer the slow option it once was: Your AI agent gets **17 tools** (15 per-repo + 2 group) automatically: -| Tool | What It Does | -| ---------------- | ---------------------------------------------------------------------- | -| `list_repos` | Discover all indexed repositories (paginated — `limit`/`offset`) | -| `query` | Process-grouped hybrid search (BM25 + semantic + RRF); optional `chain_depth` expands each result's call chain | +| Tool | What It Does | +| ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | +| `list_repos` | Discover all indexed repositories (paginated — `limit`/`offset`) | +| `query` | Process-grouped hybrid search (BM25 + semantic + RRF); optional `chain_depth` expands each result's call chain | | `context` | 360-degree symbol view — categorized refs, process participation, HTTP routes, `is_entry_point` flag; optional `chain_depth` call-chain expansion | -| `impact` | Blast radius analysis with depth grouping and confidence | -| `trace` | Shortest directed path between two symbols (call + class-member edges) | -| `detect_changes` | Git-diff impact — maps changed lines to affected processes | -| `check` | Read-only structural checks against the indexed graph | -| `rename` | Multi-file coordinated rename with graph + text search | -| `cypher` | Raw Cypher graph queries | -| `route_map` | API route map — which components fetch which endpoints, and handlers | -| `tool_map` | MCP/RPC tool definitions — where they're defined and handled | -| `shape_check` | Validate API response shapes against consumers' property accesses | -| `api_impact` | Pre-change impact report for an API route handler | -| `explain` | Explain persisted taint findings (source→sink flows, `--pdg` indexes) | -| `pdg_query` | Query control/data dependence at statement level (`--pdg` indexes) | -| `group_list` | List configured repository groups | -| `group_sync` | Rebuild a group's Contract Registry and cross-repo links | +| `impact` | Blast radius analysis with depth grouping and confidence | +| `trace` | Shortest directed path between two symbols (call + class-member edges) | +| `detect_changes` | Git-diff impact — maps changed lines to affected processes | +| `check` | Read-only structural checks against the indexed graph | +| `rename` | Multi-file coordinated rename with graph + text search | +| `cypher` | Raw Cypher graph queries | +| `route_map` | API route map — which components fetch which endpoints, and handlers | +| `tool_map` | MCP/RPC tool definitions — where they're defined and handled | +| `shape_check` | Validate API response shapes against consumers' property accesses | +| `api_impact` | Pre-change impact report for an API route handler | +| `explain` | Explain persisted taint findings (source→sink flows, `--pdg` indexes) | +| `pdg_query` | Query control/data dependence at statement level (`--pdg` indexes) | +| `group_list` | List configured repository groups | +| `group_sync` | Rebuild a group's Contract Registry and cross-repo links | > Read-only tools can omit `repo` when one repo is indexed, an MCP default is configured, or the GitNexus process cwd is inside a registered path without crossing into an unindexed nested Git checkout. Otherwise—and for mutating tools with multiple indexed repos and no MCP default—specify it explicitly: `query({search_query: "auth", repo: "my-app"})`. Per-repo tools also take an optional `branch` for indexes pinned with `gitnexus analyze --branch`; omitting it queries the workspace index, which follows your checked-out working tree. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. @@ -279,6 +279,7 @@ gitnexus analyze --spring-actuator ./actuator # Enrich with local Spring Boot A gitnexus analyze --verbose # Log skipped files when parsers are unavailable gitnexus analyze --max-file-size 1024 # Skip files larger than N KB (default: 512, cap: 32768) gitnexus analyze --worker-timeout 60 # Increase worker idle timeout for slow parses +gitnexus analyze --memory-budget 3000 # Main-thread V8 heap in MB (>= 200); overrides the auto-sizer and any --max-old-space-size pin gitnexus analyze --wal-checkpoint-threshold 67108864 # 64 MiB. Control LadybugDB WAL auto-checkpoint threshold (default: 67108864 = 64 MiB; -1 keeps Ladybug stock ~16 MiB) gitnexus auto-sync [init|start|restart|stop|status|reset] # Scheduled remote clone/pull + analyze from GITNEXUS_HOME/watch_config.yml gitnexus mcp # Start MCP server (stdio) — serves all indexed repos @@ -754,6 +755,11 @@ If analyze says the repository doesn't fit, do what the message says: - **The machine is the ceiling**: shrink the scope (exclude generated or vendored directories, below) or use a machine with more RAM. +To set the main-thread heap yourself on a memory-constrained host, pass +`--memory-budget `: analyze re-runs with exactly that V8 heap, overriding +the auto-sizer and any `--max-old-space-size` pin. It sizes the main thread +only; parse workers keep their own caps. + Escape hatches (`GITNEXUS_MEMORY=off` to decline the autopilot, `GITNEXUS_WORKER_HEAP_MB` to size workers yourself) are listed in the environment-variable table below — @@ -839,6 +845,7 @@ Four env vars expose the pool's resilience layers (respawn budget, cumulative-ti | `GITNEXUS_WORKER_READY_TIMEOUT_MS` | `5000` | Startup budget for a parse worker to load its grammar bindings and report `{type:'ready'}`. Slots that miss it are treated as startup crashes. Raise it on a slow or heavily loaded host where a full pool cold-starting concurrently needs more than 5s. | | `GITNEXUS_MEMORY` | `off` | unset (autopilot on) | `off` declines GitNexus's memory autopilot: analyze will neither re-run itself with a RAM-aware heap cap nor abort the parse before V8 enters its ineffective-mark-compact death spiral. Use it when you want to drive memory manually; to simply pin a heap size, pass Node's own `--max-old-space-size`, which is already honoured as your decision. | | `GITNEXUS_WORKER_HEAP_MB` | `clamp(512, RAM/2/poolSize, 4096)` | Per-worker V8 old-generation heap cap (#2649). Bounds pool RSS on large repos; a worker exceeding it dies with a real heap error handled by quarantine/respawn. | +| `GITNEXUS_HEAP_LIMIT_SOURCE` | unset | Set by analyze itself (`budget` or `auto`) to record where the heap limit came from, so out-of-memory advice points at `--memory-budget` rather than a `--max-old-space-size` pin. | Never — internal; set `--memory-budget` instead. | | `GITNEXUS_SERVER_ANALYZE_HEAP_MB` | `min(8192, auto cap)` | Heap for the web/MCP server's forked analyze worker (#2649). Defaults to the historical 8192 MB bounded by the machine/container's RAM-aware auto cap; set an absolute MB value to override. | | `GITNEXUS_CPP_CAPTURE_BUDGET_MS` | `20000` | Per-file wall-clock budget for C++ capture extraction; on breach the file keeps partial captures with a warning (#2432). `0` expires immediately. | diff --git a/gitnexus/src/cli/analyze-options.ts b/gitnexus/src/cli/analyze-options.ts index d778ed1bb..d548484e0 100644 --- a/gitnexus/src/cli/analyze-options.ts +++ b/gitnexus/src/cli/analyze-options.ts @@ -117,6 +117,14 @@ export interface AnalyzeOptions { workerTimeout?: string; /** Control LadybugDB WAL auto-checkpoint threshold during analyze. */ walCheckpointThreshold?: string; + /** + * `--memory-budget ` (#3137): the main-thread V8 heap limit in MB. + * `ensureHeap` applies it through the existing heap respawn, replacing the + * RAM-aware auto cap and any `--max-old-space-size` pin, so the #2649 + * guards read it as the live limit. Parse workers keep their own heap caps. + * Integer, minimum 200; CLI-only (not a `.gitnexusrc` key). + */ + memoryBudget?: string; /** Parse worker pool size (>=1); 0 is rejected (no sequential mode). */ workers?: string; /** Process-detection process cap. Positive integer string; `0` is invalid. */ diff --git a/gitnexus/src/cli/analyze-watch.ts b/gitnexus/src/cli/analyze-watch.ts index 2c3186a24..88080bd06 100644 --- a/gitnexus/src/cli/analyze-watch.ts +++ b/gitnexus/src/cli/analyze-watch.ts @@ -22,6 +22,7 @@ import { import type { AnalyzeOptions } from './analyze-options.js'; import { ensureHeap } from './analyze.js'; import { cliError, cliInfo, cliWarn } from './cli-message.js'; +import { parseIntegerOption } from './int-option.js'; import { formatInvalidProcessDetectionOverride, parseProcessDetectionBudgetStrings, @@ -91,9 +92,7 @@ function positiveInteger( maximum?: number, ): number | undefined { if (value === undefined) return undefined; - const parsed = Number(value); - if (!Number.isInteger(parsed) || parsed < 1) - throw new Error(`${flag} must be a positive integer`); + const parsed = parseIntegerOption(value, flag, { minimum: 1 }); if (maximum !== undefined && parsed > maximum) { throw new Error(`${flag} must not exceed ${maximum}`); } @@ -425,7 +424,11 @@ export async function watchCommandWithRunnerIdentity( inputPath?: string, cliOptions: WatchCliOptions = {}, ): Promise { - if (await ensureHeap({ cleanForwardedTermination: true })) return; + if ( + await ensureHeap({ cleanForwardedTermination: true, memoryBudget: cliOptions.memoryBudget }) + ) { + return; + } const requestedRepoPath = inputPath ? path.resolve(inputPath) : getGitRoot(process.cwd()); if (requestedRepoPath === null || !hasGitDir(requestedRepoPath)) { diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index cb5d58ca6..00e9a9d62 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -64,7 +64,13 @@ import { warnMissingOptionalGrammars, getOptionalGrammarExtensions } from './opt import { glob } from 'glob'; import fs from 'fs/promises'; import { cliError, cliWarn } from './cli-message.js'; -import { heapCapMbFor, memoryAutopilotDisabled } from '../core/ingestion/utils/effective-ram.js'; +import { IntegerOptionError, parseIntegerOption, parseMemoryBudgetMb } from './int-option.js'; +import { + HEAP_LIMIT_SOURCE_ENV, + heapCapMbFor, + memoryAutopilotDisabled, + type HeapLimitSource, +} from '../core/ingestion/utils/effective-ram.js'; import { EMBEDDING_DIMS_ERROR, normalizeEmbeddingDims } from './embedding-dims.js'; import { formatElapsed } from './format-elapsed.js'; import { isHfDownloadFailure } from '../core/embeddings/hf-env.js'; @@ -543,27 +549,23 @@ const RECOMMENDED_WAL_CHECKPOINT_THRESHOLD = 64 * 1024 * 1024; * later-flag-wins semantics when NODE_OPTIONS repeats a flag. */ export function parseMaxOldSpaceMb(nodeOptions: string): number | null { - // V8 accepts `-` and `_` interchangeably in flag names, and Node accepts a - // space-separated value in NODE_OPTIONS — honor every spelling of the pin - // instead of silently overriding it (#2649 review). - const matches = [...nodeOptions.matchAll(/--max[-_]old[-_]space[-_]size(?:=|\s+)(\d+)/g)]; + return parseV8SizeFlagMb(nodeOptions, 'max-old-space-size'); +} + +/** + * Last value (MB) of a V8 `--` size flag in a flag string. V8 accepts + * `-` and `_` interchangeably in flag names, and Node accepts a + * space-separated value — honor every spelling instead of silently + * overriding it (#2649 review). + */ +function parseV8SizeFlagMb(flags: string, name: string): number | null { + const stem = name.split('-').join('[-_]'); + const matches = [...flags.matchAll(new RegExp(`--${stem}(?:=|\\s+)(\\d+)`, 'g'))]; if (matches.length === 0) return null; const mb = Number(matches[matches.length - 1][1]); return Number.isFinite(mb) && mb > 0 ? mb : null; } -/** Re-exec the process with the RAM-aware auto heap cap + larger semi-space/stack - * if we're currently below that. - * - * Heap-source precedence (#2649): - * - an explicit per-invocation `--max-old-space-size` (execArgv) always wins; - * - `GITNEXUS_MEMORY=off` declines the memory autopilot entirely; - * - an ambient NODE_OPTIONS heap >= the auto cap is honored as-is; - * - an ambient NODE_OPTIONS heap BELOW the auto cap is treated as an - * inherited environment default (devcontainers/CI export one for other - * tooling), not a deliberate per-run choice: warn and respawn with the - * auto cap. Pre-#2649 this returned early and large repos then OOM'd on - * whatever heap the environment happened to specify. */ export function forwardedSignalExitCode(signal: NodeJS.Signals, cleanTermination: boolean): number { if (cleanTermination) return 0; if (signal === 'SIGINT') return 130; @@ -571,17 +573,165 @@ export function forwardedSignalExitCode(signal: NodeJS.Signals, cleanTermination return 1; } +/** Old-space and semi-space flags (MB) that give a child a V8 heap of exactly the budget. */ +export interface BudgetHeapSizing { + oldSpaceMb: number; + semiSpaceMb: number; +} + +/** + * Size the respawn flags so the child's `heap_size_limit` equals the budget + * (#3137). V8's limit is old space plus three semi-spaces, and V8 rounds the + * semi-space up to a power of two (measured on Node 22.18: semi 10 → 16), so + * the semi-space is budget/25 rounded up the same way, capped at the usual + * 128MB, and the old space takes the rest: 2000 → 1616 + 3 × 128, 200 → 176 + 3 × 8. + */ +export function budgetHeapSizing(budgetMb: number): BudgetHeapSizing { + const scaled = Math.max(1, Math.floor(budgetMb / 25)); + const semiSpaceMb = Math.min(SEMI_SPACE_MB, 2 ** Math.ceil(Math.log2(scaled))); + return { oldSpaceMb: budgetMb - 3 * semiSpaceMb, semiSpaceMb }; +} + +export interface BudgetHeapInput { + budgetMb: number; + execArgv: readonly string[]; + nodeOptions: string; + /** The RAM-aware auto cap the budget replaces. */ + autoCapMb: number; + /** `GITNEXUS_MEMORY=off`: names Node's default limit as the one replaced. */ + autopilotDisabled: boolean; + /** Inherited `GITNEXUS_HEAP_LIMIT_SOURCE`; `budget` marks a budget-respawned child. */ + inheritedSource: string | undefined; +} + +export interface BudgetHeapDecision { + respawn: boolean; + sizing: BudgetHeapSizing; + /** At most one warning line for the operator; absent in a budget-respawned child. */ + warning?: string; +} + +/** + * The `--memory-budget` heap decision (#3137), kept pure so every branch is + * testable without a process. The effective requested old space is the last + * `--max-old-space-size` in execArgv, else in NODE_OPTIONS (every spelling + * `parseMaxOldSpaceMb` accepts). Only the budget-respawned child keeps + * running; anything else respawns once at the budget. The budget's own flags + * follow the user's in the child, so the child always resolves to "at the + * budget". + */ +export function resolveBudgetHeap(input: BudgetHeapInput): BudgetHeapDecision { + const { budgetMb, autoCapMb } = input; + const sizing = budgetHeapSizing(budgetMb); + const execFlags = input.execArgv.join(' '); + const pinnedMb = parseMaxOldSpaceMb(execFlags) ?? parseMaxOldSpaceMb(input.nodeOptions); + const semiSpaceMb = + parseV8SizeFlagMb(execFlags, 'max-semi-space-size') ?? + parseV8SizeFlagMb(input.nodeOptions, 'max-semi-space-size'); + // Only a budget-respawned child carries both the budget's old space and its + // semi-space, so only it is exactly at the budget. A pin that merely equals + // the budget leaves V8's young generation on top (a 2000MB pin is a 2048MB + // heap), so that process respawns once like any other. The respawning + // parent already logged; the child stays quiet. The env marker alone is not + // proof — it is inherited — so both budget flags must be present too. + if ( + input.inheritedSource === 'budget' && + pinnedMb === sizing.oldSpaceMb && + semiSpaceMb === sizing.semiSpaceMb + ) { + return { respawn: false, sizing }; + } + const swapWarning = + budgetMb > autoCapMb + ? ` --memory-budget ${budgetMb}MB is above the ${autoCapMb}MB this machine's RAM supports — analyze may swap-thrash.\n` + : ''; + const replaced = + pinnedMb !== null + ? `the ${pinnedMb}MB --max-old-space-size heap limit` + : input.autopilotDisabled + ? `Node's default heap limit` + : `the auto-sized ${autoCapMb}MB heap cap`; + return { + respawn: true, + sizing, + warning: + ` --memory-budget replaces ${replaced}: re-running analyze with a ${budgetMb}MB heap.\n` + + swapWarning, + }; +} + +/** Re-exec the process with the RAM-aware auto heap cap + larger semi-space/stack + * if we're currently below that, or at the `--memory-budget` heap (#3137). + * + * Heap-source precedence (#2649, #3137): + * - an explicit `--memory-budget` wins over everything, including + * `GITNEXUS_MEMORY=off` and any `--max-old-space-size` pin; + * - an explicit per-invocation `--max-old-space-size` (execArgv) wins next; + * - `GITNEXUS_MEMORY=off` declines the memory autopilot entirely; + * - an ambient NODE_OPTIONS heap >= the auto cap is honored as-is; + * - an ambient NODE_OPTIONS heap BELOW the auto cap is treated as an + * inherited environment default (devcontainers/CI export one for other + * tooling), not a deliberate per-run choice: warn and respawn with the + * auto cap. Pre-#2649 this returned early and large repos then OOM'd on + * whatever heap the environment happened to specify. + * + * Paths managed by the auto-sizer or `--memory-budget` record the source in + * `GITNEXUS_HEAP_LIMIT_SOURCE` (this process when kept, the child env when + * respawned) so the parse phase's remedy text advises by source. Explicit + * heap pins and `GITNEXUS_MEMORY=off` intentionally leave it unset. */ export async function ensureHeap( - options: { cleanForwardedTermination?: boolean } = {}, + options: { cleanForwardedTermination?: boolean; memoryBudget?: string } = {}, ): Promise { + const nodeOpts = process.env.NODE_OPTIONS || ''; + // The budget is checked BEFORE the GITNEXUS_MEMORY=off return: an explicit + // flag is a deliberate per-run choice, which the opt-out does not cover. + if (options.memoryBudget !== undefined) { + let budgetMb: number; + try { + budgetMb = parseMemoryBudgetMb(options.memoryBudget); + } catch (error) { + // The CLI's preAction hook rejects bad values first; this covers + // programmatic callers. + if (!(error instanceof IntegerOptionError)) throw error; + cliError(` ${error.message}\n`); + process.exitCode = 1; + return true; + } + const decision = resolveBudgetHeap({ + budgetMb, + execArgv: process.execArgv, + nodeOptions: nodeOpts, + autoCapMb: RESPAWN_HEAP_MB, + autopilotDisabled: memoryAutopilotDisabled(), + inheritedSource: process.env[HEAP_LIMIT_SOURCE_ENV], + }); + if (decision.warning) cliWarn(decision.warning); + if (!decision.respawn) { + process.env[HEAP_LIMIT_SOURCE_ENV] = 'budget'; + return false; + } + const { oldSpaceMb, semiSpaceMb } = decision.sizing; + return respawnWithHeap( + `--max-old-space-size=${oldSpaceMb}`, + `--max-semi-space-size=${semiSpaceMb}`, + 'budget', + ` Analysis likely ran out of memory (heap limit set to ${budgetMb}MB by --memory-budget).\n` + + ` This repository's working set exceeds the budget. Raise it, or omit --memory-budget\n` + + ` to use the auto-sized cap (a budget above physical RAM causes swap-thrash — use with care):\n` + + ` gitnexus analyze --memory-budget [your-args]\n` + + ` If this persists, it may be a native crash unrelated to heap size.\n`, + nodeOpts, + options, + ); + } + // Explicit opt-out disables auto-sizing ENTIRELY — both the ambient-pin // override and the default v8-limit respawn — and is honored SILENTLY: // the operator already made the call, and stderr-sensitive consumers // (test harnesses, scripts, supervisors that track a single PID) rely on // a quiet, single-process run. if (memoryAutopilotDisabled()) return false; - const nodeOpts = process.env.NODE_OPTIONS || ''; - if (process.execArgv.some((a) => a.startsWith('--max-old-space-size'))) return false; + if (parseMaxOldSpaceMb(process.execArgv.join(' ')) !== null) return false; const ambientHeapMb = parseMaxOldSpaceMb(nodeOpts); if (ambientHeapMb !== null) { @@ -592,12 +742,39 @@ export async function ensureHeap( ); } else { const v8Heap = v8.getHeapStatistics().heap_size_limit; - if (v8Heap >= HEAP_MB * 1024 * 1024 * 0.9) return false; + if (v8Heap >= HEAP_MB * 1024 * 1024 * 0.9) { + process.env[HEAP_LIMIT_SOURCE_ENV] = 'auto'; + return false; + } } + return respawnWithHeap( + HEAP_FLAG, + SEMI_FLAG, + 'auto', + ` Analysis likely ran out of memory (heap cap auto-sized to ${RESPAWN_HEAP_MB}MB ≈ 0.75x RAM).\n` + + ` This repository's working set exceeds available RAM. Use a machine with more RAM,\n` + + ` or override the cap (a cap above physical RAM causes swap-thrash — use with care):\n` + + ` NODE_OPTIONS="--max-old-space-size=" gitnexus analyze [your-args]\n` + + ` (Windows: set NODE_OPTIONS=--max-old-space-size= && gitnexus analyze [your-args])\n` + + ` If this persists, it may be a native crash unrelated to heap size.\n`, + nodeOpts, + options, + ); +} + +/** Run the analyze child with the given heap flags and map its exit onto this process. */ +async function respawnWithHeap( + heapFlag: string, + semiFlag: string, + source: HeapLimitSource, + oomGuidance: string, + nodeOpts: string, + options: { cleanForwardedTermination?: boolean }, +): Promise { // --stack-size is a V8 flag not allowed in NODE_OPTIONS on Node 24+, so pass it // only as a direct CLI argument. --max-semi-space-size IS allowed in NODE_OPTIONS. - const cliFlags = [HEAP_FLAG, SEMI_FLAG]; + const cliFlags = [heapFlag, semiFlag]; if (!nodeOpts.includes('--stack-size')) cliFlags.push(STACK_FLAG); // Preserve the parent's node flags (execArgv) — dropping them breaks any @@ -611,7 +788,8 @@ export async function ensureHeap( const childArgs = [...preservedExecArgv, ...cliFlags, ...process.argv.slice(1)]; const childEnv = { ...process.env, - NODE_OPTIONS: `${nodeOpts} ${HEAP_FLAG} ${SEMI_FLAG}`.trim(), + NODE_OPTIONS: `${nodeOpts} ${heapFlag} ${semiFlag}`.trim(), + [HEAP_LIMIT_SOURCE_ENV]: source, }; if (shouldBridgeRespawnProgressTty()) childEnv[RESPAWN_PROGRESS_ENV] = '1'; const childExit = await runRespawnedAnalyze(childArgs, childEnv); @@ -624,15 +802,7 @@ export async function ensureHeap( } if (childExit.status !== 0 || childExit.signal) { if (childProcessLikelyOom(childExit)) { - cliError( - ` Analysis likely ran out of memory (heap cap auto-sized to ${RESPAWN_HEAP_MB}MB ≈ 0.75x RAM).\n` + - ` This repository's working set exceeds available RAM. Use a machine with more RAM,\n` + - ` or override the cap (a cap above physical RAM causes swap-thrash — use with care):\n` + - ` NODE_OPTIONS="--max-old-space-size=" gitnexus analyze [your-args]\n` + - ` (Windows: set NODE_OPTIONS=--max-old-space-size= && gitnexus analyze [your-args])\n` + - ` If this persists, it may be a native crash unrelated to heap size.\n`, - { recoveryHint: 'heap-oom-respawn' }, - ); + cliError(oomGuidance, { recoveryHint: 'heap-oom-respawn' }); } else if (childProcessLikelyNativeAbort(childExit)) { cliError( ` Analysis aborted in a native worker or native binding path.\n` + @@ -659,6 +829,7 @@ export async function ensureHeap( * for it in the first place. */ const ANALYZE_CLI_ENV_KEYS = [ + HEAP_LIMIT_SOURCE_ENV, 'GITNEXUS_VERBOSE', 'GITNEXUS_PROFILE_DEFERRED', 'GITNEXUS_PROFILE_DEFERRED_SLOW_MS', @@ -728,7 +899,10 @@ export const analyzeCommand = async ( options?: AnalyzeOptions, runnerIdentityAtBootstrap?: AnalyzerRunnerIdentity, ) => { - if (await ensureHeap()) return; + // Snapshot before ensureHeap: it records GITNEXUS_HEAP_LIMIT_SOURCE on a + // kept process, which must not leak into a later programmatic call. + const envSnap = snapshotAnalyzeEnv(); + if (await ensureHeap({ memoryBudget: options?.memoryBudget })) return; forceHeapOOMForTestIfEnabled(); // Install fatal handlers immediately after re-exec resolution so any @@ -748,7 +922,6 @@ export const analyzeCommand = async ( // exiting, restoration is moot. For early-return paths (validation // errors) and the alreadyUpToDate fast path the finally restores the // pre-call values. - const envSnap = snapshotAnalyzeEnv(); try { await analyzeCommandImpl(inputPath, options, runnerIdentityAtBootstrap); } finally { @@ -940,8 +1113,10 @@ const analyzeCommandImpl = async ( // previous call leaked. let workerPoolSize: number | undefined; if (options.workers !== undefined) { - const parsedWorkers = Number(options.workers); - if (!Number.isInteger(parsedWorkers) || parsedWorkers < 1) { + try { + workerPoolSize = parseIntegerOption(options.workers, '--workers', { minimum: 1 }); + } catch (error) { + if (!(error instanceof IntegerOptionError)) throw error; cliError( ' --workers must be a positive integer (>= 1). ' + 'GitNexus parses through a worker pool only — there is no sequential ' + @@ -950,7 +1125,6 @@ const analyzeCommandImpl = async ( process.exitCode = 1; return; } - workerPoolSize = parsedWorkers; } const processDetectionFromFlags = parseProcessDetectionBudgetStrings( @@ -971,8 +1145,12 @@ const analyzeCommandImpl = async ( // process.exit() leaves the progress bar's hidden cursor uncleared). let embeddingsNodeLimit: number | undefined; if (typeof options.embeddings === 'string') { - const parsed = Number(options.embeddings); - if (!Number.isInteger(parsed) || parsed < 0) { + try { + embeddingsNodeLimit = parseIntegerOption(options.embeddings, '--embeddings', { + minimum: 0, + }); + } catch (error) { + if (!(error instanceof IntegerOptionError)) throw error; cliError( ` --embeddings expects a non-negative integer (got "${options.embeddings}"). ` + `Pass 0 to disable the safety cap, or omit the value to keep the default.\n`, @@ -980,7 +1158,6 @@ const analyzeCommandImpl = async ( process.exitCode = 1; return; } - embeddingsNodeLimit = parsed; } const embeddingsEnabled = !!options.embeddings; @@ -990,13 +1167,14 @@ const analyzeCommandImpl = async ( value: string | undefined, ): boolean => { if (value === undefined) return true; - const parsed = Number(value); - if (!Number.isInteger(parsed) || parsed <= 0) { - cliError(` ${optionName} must be a positive integer.\n`); + try { + process.env[envName] = String(parseIntegerOption(value, optionName, { minimum: 1 })); + } catch (error) { + if (!(error instanceof IntegerOptionError)) throw error; + cliError(` ${error.message}.\n`); process.exitCode = 1; return false; } - process.env[envName] = String(parsed); return true; }; @@ -1207,7 +1385,7 @@ const analyzeCommandImpl = async ( // injection, including community skill writes that `--skills` would normally // produce. Surface the override explicitly so users don't wonder why a // pipeline re-index ran but no skill files appeared. The pipeline still - // re-runs (see `force: options.force || options.skills` below); the warning + // re-runs (`skills` is passed to runFullAnalysis below); the warning // is purely about the dropped post-index write step. if (options.indexOnly && options.skills) { console.log( @@ -1361,10 +1539,12 @@ const analyzeCommandImpl = async ( const skipAgentsMd = skipAll || options.skipAgentsMd; const skipSkills = skipAll || options.skipSkills; const runOptions = { - // Pipeline re-index — OR'd with --skills because skill generation needs - // a fresh pipelineResult, and with --no-parse-cache because bypassing - // parser output is meaningful only when the pipeline runs. - force: options.force || options.skills || options.parseCache === false, + // The user's own --force only. --skills (skill generation needs a fresh + // pipelineResult) and --no-parse-cache (bypassing parser output means + // anything only when the pipeline runs) each force the rebuild inside + // runFullAnalysis under their own named reason (#3137). + force: options.force, + skills: options.skills, useParseCache: options.parseCache !== false, repairFts: options.repairFts, skipFts: options.skipFts, diff --git a/gitnexus/src/cli/help-i18n.ts b/gitnexus/src/cli/help-i18n.ts index d16586102..d8543a62a 100644 --- a/gitnexus/src/cli/help-i18n.ts +++ b/gitnexus/src/cli/help-i18n.ts @@ -73,6 +73,7 @@ const OPTION_DESCRIPTION_KEYS = { 'analyze|--max-file-size ': 'help.option.analyze.maxFileSize', 'analyze|--worker-timeout ': 'help.option.analyze.workerTimeout', 'analyze|--wal-checkpoint-threshold ': 'help.option.analyze.walCheckpointThreshold', + 'analyze|--memory-budget ': 'help.option.analyze.memoryBudget', 'analyze|--workers ': 'help.option.analyze.workers', 'analyze|--max-processes ': 'help.option.analyze.maxProcesses', 'analyze|--max-process-branching ': 'help.option.analyze.maxProcessBranching', diff --git a/gitnexus/src/cli/i18n/en.ts b/gitnexus/src/cli/i18n/en.ts index 0a88908f6..bcd49d8fa 100644 --- a/gitnexus/src/cli/i18n/en.ts +++ b/gitnexus/src/cli/i18n/en.ts @@ -307,6 +307,8 @@ export const en = { 'Worker sub-batch idle timeout before retry/fallback. Default: 30.', 'help.option.analyze.walCheckpointThreshold': 'LadybugDB WAL auto-checkpoint threshold in bytes during analyze (integer >= -1; default: 67108864 = 64 MiB; -1 keeps Ladybug stock ~16 MiB).', + 'help.option.analyze.memoryBudget': + 'Main-thread V8 heap size in MB for analyze (integer >= 200). Re-runs analyze with exactly this heap, overriding the RAM/cgroup auto-sizer and any --max-old-space-size pin; parse workers keep their own heap caps.', 'help.option.analyze.workers': 'Parse worker pool size (>=1). Default: cores-1 capped at 16, auto-sized to the repo.', 'help.option.analyze.maxProcesses': diff --git a/gitnexus/src/cli/i18n/zh-CN.ts b/gitnexus/src/cli/i18n/zh-CN.ts index 766e2f261..96f3f246e 100644 --- a/gitnexus/src/cli/i18n/zh-CN.ts +++ b/gitnexus/src/cli/i18n/zh-CN.ts @@ -280,6 +280,8 @@ export const zhCN = { 'help.option.analyze.workerTimeout': 'Worker 子批次空闲超时，超时后重试/回退。默认：30。', 'help.option.analyze.walCheckpointThreshold': 'analyze 期间 LadybugDB WAL 自动 checkpoint 阈值（字节，整数 >= -1；默认：67108864 = 64 MiB；-1 保持 Ladybug 默认约 16 MiB）。', + 'help.option.analyze.memoryBudget': + 'analyze 主线程 V8 堆大小（MB，整数 >= 200）。以该堆大小重新运行 analyze，覆盖按 RAM/cgroup 自动计算的上限及任何 --max-old-space-size 设置；解析 worker 各自保留独立的堆上限。', 'help.option.analyze.workers': '解析 worker 池大小（>=1）。默认：cores-1，最多 16，按仓库规模自适应。', 'help.option.analyze.maxProcesses': diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index 0563f0c2f..9141c9d85 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -11,6 +11,7 @@ import { createLbugLazyAction, } from './lazy-action.js'; import { EMBEDDING_DIMS_ERROR, normalizeEmbeddingDims } from './embedding-dims.js'; +import { IntegerOptionError, parseMemoryBudgetMb } from './int-option.js'; import { registerGroupCommands } from './group.js'; import { localizeCliHelp } from './help-i18n.js'; import { t } from './i18n/index.js'; @@ -171,6 +172,12 @@ program 'LadybugDB WAL auto-checkpoint threshold in bytes during analyze ' + '(integer >= -1; default: 67108864 = 64 MiB; -1 keeps Ladybug stock ~16 MiB).', ) + .option( + '--memory-budget ', + 'Main-thread V8 heap size in MB for analyze (integer >= 200). Re-runs analyze with ' + + 'exactly this heap, overriding the RAM/cgroup auto-sizer and any --max-old-space-size ' + + 'pin; parse workers keep their own heap caps.', + ) .option( '--workers ', 'Parse worker pool size (>=1). Default: cores-1 capped at 16, auto-sized to the repo.', @@ -231,6 +238,18 @@ program process.stderr.write('\n --debounce requires --watch\n\n'); process.exit(1); } + // Validate --memory-budget here so analyze and --watch both reject a bad + // value before ensureHeap sizes (and possibly respawns) the heap (#3137). + const budgetOpt = analyzeOpts['memoryBudget']; + if (budgetOpt !== undefined) { + try { + parseMemoryBudgetMb(String(budgetOpt)); + } catch (error) { + if (!(error instanceof IntegerOptionError)) throw error; + process.stderr.write(`\n ${error.message}\n\n`); + process.exit(1); + } + } // ONLY GITNEXUS_EMBEDDING_DIMS must be set here: schema.ts reads it at // module-load time during the lazy import('./analyze.js') below (via the // static chain analyze.ts → run-analyze.ts → schema.ts), so deferring to diff --git a/gitnexus/src/cli/int-option.ts b/gitnexus/src/cli/int-option.ts new file mode 100644 index 000000000..21a93403c --- /dev/null +++ b/gitnexus/src/cli/int-option.ts @@ -0,0 +1,65 @@ +/** + * Shared integer parsing for CLI flags. + * + * One digit-only parser with a per-flag minimum, so `1e3`, `0x10`, `1.5`, + * and padded-zero spellings are rejected the same way on every surface. + * Callers choose how to report the typed error: analyze routes it through + * `cliError` + `process.exitCode = 1`, while watch and wiki let it propagate. + */ + +export class IntegerOptionError extends Error { + constructor( + readonly flag: string, + readonly minimum: number, + message: string, + ) { + super(message); + this.name = 'IntegerOptionError'; + } +} + +export interface IntegerOptionBounds { + /** Smallest accepted value (inclusive). */ + minimum: number; + /** + * Divides the safe-integer bound, for values the caller later multiplies + * (wiki's `--timeout` seconds become milliseconds, so it passes 1000). + */ + scale?: number; +} + +/** + * Parse a trimmed, digit-only integer flag value. Throws + * `IntegerOptionError` naming the flag when the value is not a plain + * non-negative integer, is below the minimum, or exceeds + * `MAX_SAFE_INTEGER / scale`. + */ +export function parseIntegerOption( + value: string, + flag: string, + { minimum, scale = 1 }: IntegerOptionBounds, +): number { + const trimmed = value.trim(); + const parsed = /^(0|[1-9]\d*)$/.test(trimmed) ? parseInt(trimmed, 10) : Number.NaN; + if (Number.isNaN(parsed) || parsed < minimum) { + const requirement = + minimum === 1 ? 'must be a positive integer' : `must be an integer >= ${minimum}`; + throw new IntegerOptionError(flag, minimum, `${flag} ${requirement}`); + } + if (parsed > Math.floor(Number.MAX_SAFE_INTEGER / scale)) { + throw new IntegerOptionError(flag, minimum, `${flag} is too large`); + } + return parsed; +} + +/** Smallest `--memory-budget` (MB): below ~200 MB even one parse worker cannot hold a chunk's working set. */ +export const MEMORY_BUDGET_MIN_MB = 200; + +/** + * Parse `--memory-budget ` (#3137). Shared by the commander `preAction` + * hook, which rejects a bad value before any work, and `ensureHeap`, which + * sizes the respawned heap from it. + */ +export function parseMemoryBudgetMb(value: string): number { + return parseIntegerOption(value, '--memory-budget', { minimum: MEMORY_BUDGET_MIN_MB }); +} diff --git a/gitnexus/src/cli/wiki.ts b/gitnexus/src/cli/wiki.ts index e9218c7e5..11325a3c3 100644 --- a/gitnexus/src/cli/wiki.ts +++ b/gitnexus/src/cli/wiki.ts @@ -29,6 +29,7 @@ import { detectCursorCLI } from '../core/wiki/cursor-client.js'; import { detectGrokCLI } from '../core/wiki/grok-client.js'; import { detectLocalCLI } from '../core/wiki/local-cli-client.js'; import { logger } from '../core/logger.js'; +import { parseIntegerOption } from './int-option.js'; export interface WikiCommandOptions { force?: boolean; @@ -48,23 +49,6 @@ export interface WikiCommandOptions { allowInsecureConnection?: string; } -function parsePositiveIntegerOption( - value: string | undefined, - flag: string, - multiplier = 1, -): number | undefined { - if (value === undefined) return undefined; - const trimmed = value.trim(); - if (!/^[1-9]\d*$/.test(trimmed)) { - throw new Error(`${flag} must be a positive integer`); - } - const parsed = parseInt(trimmed, 10); - if (parsed > Math.floor(Number.MAX_SAFE_INTEGER / multiplier)) { - throw new Error(`${flag} is too large`); - } - return parsed; -} - function isLocalProvider( provider: LLMProvider | undefined, ): provider is 'cursor' | 'claude' | 'codex' | 'opencode' | 'grok' { @@ -206,8 +190,14 @@ const wikiCommandImpl = async (inputPath?: string, options?: WikiCommandOptions) let retries: number | undefined; let allowedInsecureHttpHosts: string[] | undefined; try { - timeoutSeconds = parsePositiveIntegerOption(options?.timeout, '--timeout', 1000); - retries = parsePositiveIntegerOption(options?.retries, '--retries'); + timeoutSeconds = + options?.timeout === undefined + ? undefined + : parseIntegerOption(options.timeout, '--timeout', { minimum: 1, scale: 1000 }); + retries = + options?.retries === undefined + ? undefined + : parseIntegerOption(options.retries, '--retries', { minimum: 1 }); allowedInsecureHttpHosts = options?.allowInsecureConnection === undefined ? undefined diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index fa0bbef50..96472e477 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -109,7 +109,7 @@ import { import type { KnowledgeGraph } from '../../graph/types.js'; import type { PipelineOptions } from '../pipeline.js'; import fs from 'node:fs'; -import { effectiveRamBytes, memoryAutopilotDisabled } from '../utils/effective-ram.js'; +import { heapPressureRemedy, memoryAutopilotDisabled } from '../utils/effective-ram.js'; import path from 'node:path'; import v8 from 'node:v8'; import { fileURLToPath, pathToFileURL } from 'node:url'; @@ -177,33 +177,6 @@ export function shouldAbortForHeapPressure(heapUsedBytes: number, heapLimitBytes return heapUsedBytes > heapLimitBytes * HEAP_ABORT_FRACTION; } -/** - * The ONE action a user should take when this repository doesn't fit the - * current heap (#2649). Users hitting memory limits are already frustrated — - * a menu of env knobs at that moment is noise. Branch on whether the machine - * itself has more memory to give: if this process's limit sits well below - * what the RAM-aware auto-sizer would grant (an inherited NODE_OPTIONS pin or - * explicit flag), the fix is to drop the pin — gitnexus sizes itself. - * Otherwise the machine is the ceiling and only scope or hardware helps. - * Escape hatches (GITNEXUS_MEMORY etc.) stay in the README env table. - */ -export function heapPressureRemedy(heapLimitBytes: number): string { - // Effective RAM honors a real cgroup limit — raw os.totalmem() told users - // inside an 8GB-limited container on a 64GB host that "this machine has - // more memory available", an advice loop with no exit (#2649 review). - const autoCapBytes = effectiveRamBytes() * 0.75; - if (heapLimitBytes < autoCapBytes * 0.9) { - return ( - `This machine has more memory available: re-run without the --max-old-space-size ` + - `pin (NODE_OPTIONS or node flag) — gitnexus sizes its heap to the machine automatically.` - ); - } - return ( - `This machine is at its memory ceiling: exclude generated or vendored directories ` + - `via .gitnexusignore, or analyze on a machine with more memory.` - ); -} - /** Max bytes of source content to load per parse cache pack. * * Granularity knob for the parse cache: a single file change invalidates only diff --git a/gitnexus/src/core/ingestion/utils/effective-ram.ts b/gitnexus/src/core/ingestion/utils/effective-ram.ts index c2503ba97..b5ea977c6 100644 --- a/gitnexus/src/core/ingestion/utils/effective-ram.ts +++ b/gitnexus/src/core/ingestion/utils/effective-ram.ts @@ -65,3 +65,57 @@ export function memoryAutopilotDisabled(): boolean { export function autoHeapCapMb(): number { return heapCapMbFor(effectiveRamBytes()); } + +/** + * Env var `ensureHeap` sets to record where the main-thread heap limit came + * from (#3137): `budget` (`--memory-budget`) or `auto` (the RAM-aware cap). + * Unset means a `--max-old-space-size` pin or `GITNEXUS_MEMORY=off` without + * a budget. A budget-respawned child inherits it, which is how it knows not + * to log the heap decision a second time. + */ +export const HEAP_LIMIT_SOURCE_ENV = 'GITNEXUS_HEAP_LIMIT_SOURCE'; + +export type HeapLimitSource = 'budget' | 'auto'; + +/** + * The ONE action a user should take when this repository doesn't fit the + * current heap (#2649). Users hitting memory limits are already frustrated — + * a menu of env knobs at that moment is noise. Branch on whether the machine + * itself has more memory to give: if this process's limit sits well below + * what the RAM-aware auto-sizer would grant, the fix is whatever set the + * smaller limit — raise `--memory-budget` when the budget set it (#3137), + * otherwise drop the NODE_OPTIONS / node-flag pin so gitnexus sizes itself. + * Otherwise the machine is the ceiling and only scope or hardware helps. + * Escape hatches (GITNEXUS_MEMORY etc.) stay in the README env table. + */ +export function heapPressureRemedy( + heapLimitBytes: number, + source: string | undefined = process.env[HEAP_LIMIT_SOURCE_ENV], +): string { + // Effective RAM honors a real cgroup limit — raw os.totalmem() told users + // inside an 8GB-limited container on a 64GB host that "this machine has + // more memory available", an advice loop with no exit (#2649 review). + const autoCapBytes = autoHeapCapMb() * 1024 * 1024; + if (heapLimitBytes < autoCapBytes * 0.9) { + if (source === 'budget') { + return ( + `This machine has more memory available: raise --memory-budget, or omit it ` + + `so gitnexus sizes its heap to the machine automatically.` + ); + } + if (source === undefined && memoryAutopilotDisabled()) { + return ( + `This machine has more memory available: GITNEXUS_MEMORY=off keeps Node's default ` + + `heap — unset it, or pass --memory-budget .` + ); + } + return ( + `This machine has more memory available: re-run without the --max-old-space-size ` + + `pin (NODE_OPTIONS or node flag) — gitnexus sizes its heap to the machine automatically.` + ); + } + return ( + `This machine is at its memory ceiling: exclude generated or vendored directories ` + + `via .gitnexusignore, or analyze on a machine with more memory.` + ); +} diff --git a/gitnexus/src/core/rebuild-reasons.ts b/gitnexus/src/core/rebuild-reasons.ts new file mode 100644 index 000000000..66972c257 --- /dev/null +++ b/gitnexus/src/core/rebuild-reasons.ts @@ -0,0 +1,210 @@ +/** + * Rebuild-reason collector (#3137, PR #3385). + * + * Every path in analyze that forces a full rebuild — and the #2409 escalated + * full write, which is announced but does not force — contributes one + * `{ key, text }` reason here. The rebuild decision (`forced`) and all + * operator-facing output come from this one collector: a single summary + * before the pipeline and at most one follow-up line for reasons added after + * it. Reasons are persisted on the crash-recovery marker as the flattened + * `StoredRebuildReason[]` from `toStored()`, and an interrupted rebuild comes + * back as one `interrupted-rebuild` entry via `recordInterruptedRebuild`. + * + * Pure: no I/O, no logging, no dependency on run-analyze. + */ + +/** One value per distinct cause; paths testing the same predicate share a key. */ +export const REBUILD_REASON_KEYS = [ + 'user-force', + 'skills', + 'parse-cache-bypass', + 'drop-embeddings', + 'interrupted-rebuild', + 'private-graph-unavailable', + 'shared-store-missing-graph', + 'content-retention', + 'pdg-mode', + 'schema-fingerprint', + 'graph-write-collapse', + 'analysis-features', + 'spring-vendor-prefixes', + 'runner-identity', + 'cjk-segmentation', + 'embedding-dims', + 'spring-actuator', + 'asyncapi', + 'escalated-full-write', +] as const; + +export type RebuildReasonKey = (typeof REBUILD_REASON_KEYS)[number]; + +export interface RebuildReason { + readonly key: RebuildReasonKey; + readonly text: string; + /** `false` marks a reason that is announced but does not force a rebuild. */ + readonly forcing?: false; +} + +/** + * Serializable shape persisted on the crash-recovery marker. `key` is a plain + * string so reasons written by a newer build survive a read by an older one. + */ +export interface StoredRebuildReason { + readonly key: string; + readonly text: string; +} + +const INTERRUPTED_KEY: RebuildReasonKey = 'interrupted-rebuild'; + +const isRecord = (value: unknown): value is Record => + typeof value === 'object' && value !== null; + +/** + * A non-forcing reason (the escalated DB write) changes how a run writes, not + * whether it rebuilds, so a block of only non-forcing reasons must not claim a + * full rebuild. + */ +function leadFor(reasons: readonly RebuildReason[], forcingLead: string): string { + return reasons.some((reason) => reason.forcing !== false) ? forcingLead : 'Write plan changed'; +} + +/** + * Validate reasons read back from metadata. Entries with string `key` and + * `text` are kept (unknown keys included); anything else is dropped, and a + * non-array value reads as no reasons recorded. + */ +export const readStoredRebuildReasons = (value: unknown): StoredRebuildReason[] => { + if (!Array.isArray(value)) return []; + const stored: StoredRebuildReason[] = []; + for (const entry of value) { + if (isRecord(entry) && typeof entry.key === 'string' && typeof entry.text === 'string') { + stored.push({ key: entry.key, text: entry.text }); + } + } + return stored; +}; + +const singleLine = (text: string): string => text.replace(/\s*\n\s*/g, ' '); + +export class RebuildReasonCollector { + private readonly entries = new Map(); + private readonly announced = new Set(); + /** Stored reasons of an interrupted rebuild, keyed for merge; `undefined` when none. */ + private interrupted: { stored: Map; details?: string } | undefined; + + /** Add a reason; a key already present keeps its position and takes the newest text. */ + add(reason: RebuildReason): void { + if (reason.key === INTERRUPTED_KEY) { + this.entries.set(reason.key, reason); + return; + } + if (this.interrupted?.stored.has(reason.key)) { + this.interrupted.stored.set(reason.key, reason.text); + this.refreshInterruptedEntry(); + return; + } + this.entries.set(reason.key, reason); + } + + /** + * Contribute the recovery entry for a crashed run. `stored` is flattened + * (nested `interrupted-rebuild` entries are dropped) and any reason already + * collected with a matching key is merged into it — the current run's text + * wins, since it is newer than the stored one. + */ + recordInterruptedRebuild(stored: readonly StoredRebuildReason[], dirtyDetails?: string): void { + const merged = new Map(); + for (const { key, text } of stored) { + if (key !== INTERRUPTED_KEY) merged.set(key, text); + } + for (const key of merged.keys()) { + const current = this.entries.get(key); + if (current) { + merged.set(key, current.text); + this.entries.delete(key); + } + } + this.interrupted = { stored: merged, details: dirtyDetails }; + this.refreshInterruptedEntry(); + } + + /** True when any forcing reason is present. */ + get forced(): boolean { + for (const reason of this.entries.values()) { + if (reason.forcing !== false) return true; + } + return false; + } + + keys(): RebuildReasonKey[] { + return [...this.entries.values()].map((reason) => reason.key); + } + + reasons(): RebuildReason[] { + return [...this.entries.values()]; + } + + /** + * The pre-pipeline summary: inline for one reason, numbered for several, + * `undefined` when empty. Marks every current reason as announced. + */ + formatSummary(): string | undefined { + const reasons = this.takeUnannounced(); + if (reasons.length === 0) return undefined; + const lead = leadFor(reasons, 'Full rebuild required'); + if (reasons.length === 1) return `${lead}: ${reasons[0].text}`; + const numbered = reasons.map((reason, i) => ` ${i + 1}. ${reason.text}`).join('\n'); + return `${lead} (${reasons.length} reasons):\n${numbered}`; + } + + /** + * One line covering only the reasons added since the last summary or + * follow-up; `undefined` when there are none. + */ + formatFollowUp(): string | undefined { + const reasons = this.takeUnannounced(); + if (reasons.length === 0) return undefined; + const lead = leadFor(reasons, 'Full rebuild also required'); + return `${lead}: ${reasons.map((reason) => singleLine(reason.text)).join('; ')}`; + } + + /** + * The flattened reasons to persist on the crash-recovery marker: the + * interrupted rebuild's (merged) stored reasons first, then every other + * collected reason. Never contains an `interrupted-rebuild` entry. + */ + toStored(): StoredRebuildReason[] { + const stored: StoredRebuildReason[] = [...(this.interrupted?.stored ?? [])].map( + ([key, text]) => ({ key, text }), + ); + for (const reason of this.entries.values()) { + if (reason.key !== INTERRUPTED_KEY) stored.push({ key: reason.key, text: reason.text }); + } + return stored; + } + + private takeUnannounced(): RebuildReason[] { + const fresh = [...this.entries.values()].filter((reason) => !this.announced.has(reason.key)); + for (const reason of fresh) this.announced.add(reason.key); + return fresh; + } + + private refreshInterruptedEntry(): void { + if (!this.interrupted) return; + const { stored, details } = this.interrupted; + const texts = [...stored.values()]; + const recorded = + texts.length === 0 + ? 'no reasons recorded' + : texts.length === 1 + ? `interrupted rebuild was for: ${texts[0]}` + : `interrupted rebuild was for: ${texts.map((text, i) => `(${i + 1}) ${text}`).join('; ')}`; + const dirtyState = details === undefined ? '' : `; last dirty state: ${details}`; + this.entries.set(INTERRUPTED_KEY, { + key: INTERRUPTED_KEY, + text: + 'Previous analyze run did not complete cleanly (incrementalInProgress flag set)' + + `${dirtyState}; ${recorded}; forcing full rebuild to restore a known-good index.`, + }); + } +} diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 99c7c3da5..490e2e871 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -308,6 +308,12 @@ import { mintUnverifiedCountCheckpoint, } from './embedding-checkpoint.js'; import type { EmbeddingCheckpoint } from './embedding-checkpoint.js'; +import { + RebuildReasonCollector, + readStoredRebuildReasons, + type RebuildReason, + type RebuildReasonKey, +} from './rebuild-reasons.js'; /** * Strip C0/C1 control characters from a progress/diagnostic message. @@ -422,11 +428,18 @@ export interface AnalyzeOptions { /** * Rebuild the graph and FTS. Parser output is still reused from the * content-addressed parse cache unless `useParseCache` is false. - * Callers may OR this with other flags that imply re-analysis - * (e.g. `--skills`), so the value here is the PIPELINE-force signal, - * NOT the registry-collision bypass. See `allowDuplicateName` below. + * The caller's value is the user's explicit `--force` (the `user-force` + * rebuild reason); other flags that imply re-analysis (`skills`, + * `useParseCache: false`) contribute their own reasons instead of being + * folded in here. NOT the registry-collision bypass — see + * `allowDuplicateName` below. */ force?: boolean; + /** + * `--skills`: skill generation needs a freshly built `pipelineResult`, so + * the run rebuilds (the `skills` rebuild reason). + */ + skills?: boolean; /** * Reuse content-addressed parser output. Defaults to true. When false, * analysis reparses every file and publishes a new parse-cache generation @@ -647,6 +660,13 @@ export interface AnalyzeResult { embeddings?: number; }; alreadyUpToDate?: boolean; + /** + * Keys of every rebuild reason this run collected (#3137), in collection + * order — including the non-forcing `escalated-full-write`. Empty when the + * run took the fast path, ran incrementally, or rebuilt only structurally + * (no git, no stored file hashes, an empty file list). + */ + rebuildReasons: RebuildReasonKey[]; /** The raw pipeline result — only populated when needed by callers (e.g. skill generation). */ pipelineResult?: any; /** True when analyze only repaired FTS indexes and skipped pipeline re-analysis. */ @@ -1132,13 +1152,17 @@ async function resolveWriteTarget(repoPath: string, options: AnalyzeOptions): Pr // are shared across branches (#2106 KTD7). Always re-run requireStoragePath: // a cached path string must not skip ownership (STORAGE_PATH can move to a // foreign slot while the lock is waited out). `--force` may adopt a - // repository-local foreign slot; the non-force set stays ANALYZE_STORAGE. - // A linked-worktree checkout writes its own slot in the shared store - // (#3352); that slot replaces any repository-local `.gitnexus`, which is left - // untouched. - const storageRequirements = options.force - ? ANALYZE_FORCE_STORAGE_REQUIREMENTS - : ANALYZE_STORAGE_REQUIREMENTS; + // repository-local foreign slot; so may `--skills` or a parse-cache bypass — + // each forces the rebuild just as `--force` does (#3137), just under its own + // named reason instead of being folded into `options.force` itself, so this + // check re-derives the same predicate the pre-#3137 folded `force` used. The + // non-force set stays ANALYZE_STORAGE. A linked-worktree checkout writes its + // own slot in the shared store (#3352); that slot replaces any + // repository-local `.gitnexus`, which is left untouched. + const storageRequirements = + options.force || options.skills || options.useParseCache === false + ? ANALYZE_FORCE_STORAGE_REQUIREMENTS + : ANALYZE_STORAGE_REQUIREMENTS; if (options.noShare && resolveSharedStore(repoPath)) { // Fail before any lock or indexing; only an opted-in clone can leave. throw new Error( @@ -1368,6 +1392,55 @@ async function runFullAnalysisInner( const progress = (phase: string, percent: number, message: string) => callbacks.onProgress(phase, percent, message); + // ── rebuild reasons (#3137) ──────────────────────────────────────── + // Every path below that forces a full rebuild adds one reason here instead + // of setting `options.force` and logging on its own. The rebuild decision is + // read back from the collector at two checkpoints — before the pipeline + // (one summary) and after it (at most one follow-up line) — so an upgrade + // that trips several gates names them all once. Structural full builds (no + // git, no stored file hashes, an empty file list) are not reasons. + // + // The caller's `force` is the user's own `--force`; `--skills` and a parse + // cache bypass arrive as their own flags and are named for what they are. + const collector = new RebuildReasonCollector(); + if (options.force === true) { + collector.add({ key: 'user-force', text: 'a full rebuild was requested (--force).' }); + } + if (options.skills === true) { + collector.add({ + key: 'skills', + text: 'skill generation requested (--skills) — it needs a freshly analyzed graph.', + }); + } + if (options.useParseCache === false) { + collector.add({ + key: 'parse-cache-bypass', + text: 'parser cache bypass requested — unchanged files will be re-parsed.', + }); + } + /** + * The collected reasons as the crash-recovery marker's optional `reasons` + * field (#3137 R9): spread into every `incrementalInProgress` writer, and + * omitted when nothing was collected. Read at write time, so each stamp + * carries the reasons known by then. + */ + const storedRebuildReasons = (): Pick< + NonNullable, + 'reasons' + > => { + const reasons = collector.toStored(); + return reasons.length > 0 ? { reasons } : {}; + }; + /** Checkpoint: the collector's verdict becomes the pipeline's `force`. */ + const applyCollectedForce = (): void => { + options = { ...options, force: collector.forced }; + }; + /** Log reasons added after the summary as the run's single follow-up line. */ + const announceFollowUp = (): void => { + const line = collector.formatFollowUp(); + if (line !== undefined) log(line); + }; + // FTS-config validation and the degraded-parse counter reset happen in the // `runFullAnalysis` wrapper (before the lock is taken). @@ -1420,7 +1493,10 @@ async function runFullAnalysisInner( const ensurePrivateGraph = async (copy = true): Promise => { if (!writeTarget.sharedStore || placement.branch) return; if (!(await ensurePrivateSharedGraph(metaDir, log, { copy }))) { - options = { ...options, force: true }; + collector.add({ + key: 'private-graph-unavailable', + text: 'this checkout could not get a private copy of the shared graph — building a fresh one.', + }); } // Later dirty-flag writes spread the in-memory metadata; keep them from // re-recording the pointer this slot just left. @@ -1491,10 +1567,13 @@ async function runFullAnalysisInner( existingMeta && contentRetentionMismatch(existingMeta, contentRetention) ) { - log( - 'content retention or FTS profile changed; forcing a full rebuild before rebuilding search indexes.', - ); - options = { ...options, force: true, repairFts: false }; + // Same predicate and key as the retention gate below, so the two merge + // into one entry; that gate's more specific text replaces this one. + collector.add({ + key: 'content-retention', + text: 'content retention or FTS profile changed — the database is rebuilt before its search indexes.', + }); + options = { ...options, repairFts: false }; } if (options.repairFts) { if (!existingMeta) { @@ -1714,6 +1793,7 @@ async function runFullAnalysisInner( storagePath, stats: existingMeta.stats ?? {}, ftsRepairedOnly: true, + rebuildReasons: collector.keys(), }; } finally { await closeLbug().catch(() => {}); @@ -1748,18 +1828,26 @@ async function runFullAnalysisInner( // embeddings module (#2370 — none loads unless a run actually needs one). // `decideEmbeddingResume` asks for it by aborting on `undefined`, which is // the only abort it can reach without one. - let decision = decideEmbeddingResume(checkpoint, undefined, options); + // + // A rebuild already forced by a collected reason (--force, --skills, a + // parse-cache bypass, a failed shared-graph copy, a retention change under + // --repair-fts) discards the marker, as it did when those set `force`. + const resumeOptions = { ...options, force: collector.forced }; + let decision = decideEmbeddingResume(checkpoint, undefined, resumeOptions); if (decision.action === 'abort') { const { resolveEmbeddingIdentity } = await import('./embeddings/embedding-identity.js'); embeddingIdentityForRun = resolveEmbeddingIdentity(); - decision = decideEmbeddingResume(checkpoint, embeddingIdentityForRun, options); + decision = decideEmbeddingResume(checkpoint, embeddingIdentityForRun, resumeOptions); } if (decision.action === 'abort') throw new Error(decision.error); log(decision.log); if (options.dropEmbeddings) { // --drop-embeddings has always implied a rebuild here; the decision only // covers the marker. - options = { ...options, force: true }; + collector.add({ + key: 'drop-embeddings', + text: 'embeddings are being dropped (--drop-embeddings) along with their resume checkpoint.', + }); } if (decision.action === 'resume') { resumeEmbeddingCheckpoint = true; @@ -1838,17 +1926,16 @@ async function runFullAnalysisInner( ); await persistFtsNativeAbortRecovery(); } else { - log( - // "analyze run", not "incremental run" — since #2099 F1 the flag is a - // generic dirty marker written by BOTH writeback branches. - 'Previous analyze run did not complete cleanly (incrementalInProgress flag set); ' + - `last dirty state: ${dirtyDetails}; ` + - 'forcing full rebuild to restore a known-good index.', + // One `interrupted-rebuild` entry carrying the crashed run's stored + // reasons; a gate below that re-detects one of them merges into it. + // A legacy boolean marker has no `reasons` and reads as none recorded. + collector.recordInterruptedRebuild( + readStoredRebuildReasons(typeof dirty === 'object' ? dirty.reasons : undefined), + dirtyDetails, ); - 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. + // for the post-rebuild meta carry-over, but the collected reason ensures + // the rebuild path executes. // // #2409 defect 2: the crashed writeback's WAL can be poisoned — replaying // it kills the process natively, and the first DB open of this recovery @@ -1888,6 +1975,28 @@ async function runFullAnalysisInner( } } + // ── rebuild-gate collection (#3137) ──────────────────────────────── + // The nine meta-mismatch gates below used to log individually the moment + // each fired and set `force: true` nine times over. On an upgrade that + // trips several gates at once (schema + runner identity + FTS profile is + // the common triple), the operator got a scattered wall of near-identical + // warnings. Each gate now adds its reason to the collector, and the one + // summary before the pipeline prints them together. Every gate is still + // evaluated (none early-returns), and a rebuild happens iff a forcing + // reason was collected. + // + // #3137: the metadata a crashed FIRST build left behind is the slot claim + // (`lastCommit: ''`, no fingerprint, no runner identity), not an index. It + // rebuilds structurally anyway, and measuring it against these gates would + // announce a schema and runner-identity change that never happened. + const priorIsFirstBuildClaim = + existingMeta?.lastCommit === '' && + existingMeta.schemaFingerprint === undefined && + existingMeta.runnerIdentity === undefined; + const addGateReason = (reason: RebuildReason): void => { + if (!priorIsFirstBuildClaim) collector.add(reason); + }; + // ── pdg-mode flip forces full writeback (#2099 F1) ───────────────── // The incremental writeback persists only changed-file nodes, so a pdg // config differing from the one the DB rows were built under cannot be @@ -1895,21 +2004,22 @@ async function runFullAnalysisInner( // layer ("Incremental: changed=0", zero BasicBlock rows), on→off strands // zombie blocks for unchanged files. MUST sit before the alreadyUpToDate // fast path below — a clean-tree flip would otherwise early-return without - // running the pipeline at all. The notice is deliberately NOT gated on - // options.force: --skills implies force with no message of its own, and a - // mode change deserves a diagnostic regardless of why a rebuild happens. + // running the pipeline at all. The reason is collected even when another + // reason already forces the rebuild: a mode change deserves a diagnostic + // regardless of why a rebuild happens. if (existingMeta && pdgModeMismatch(existingMeta.pdg, options)) { const pdgOn = options.pdg === true; const capsOnly = !!existingMeta.pdg && pdgOn; // both-on can only mismatch via caps const was = existingMeta.pdg ? 'with --pdg' : 'without --pdg'; const now = pdgOn ? 'with --pdg' : 'without --pdg'; - log( - `pdg mode changed (index built ${was}, this run is ${now}` + - `${capsOnly ? ', but with different caps' : ''}); forcing a full ` + - `rebuild so the CFG layer is ${pdgOn ? 'fully persisted' : 'fully removed'}. ` + + addGateReason({ + key: 'pdg-mode', + text: + `pdg mode changed (index built ${was}, this run is ${now}` + + `${capsOnly ? ', but with different caps' : ''}) — the CFG layer will be ` + + `${pdgOn ? 'fully persisted' : 'fully removed'}. ` + `Tip: set \`pdg: ${pdgOn}\` in .gitnexusrc to pin the mode across runs.`, - ); - options = { ...options, force: true }; + }); } // Retention controls the DB's persisted text and FTS columns. Incremental @@ -1917,11 +2027,12 @@ async function runFullAnalysisInner( // old source text and index pages behind. Rebuild the database instead. if (existingMeta && contentRetentionMismatch(existingMeta, contentRetention)) { const recorded = existingMeta.contentRetention ?? 'full (legacy)'; - log( - `content retention changed (index built with ${recorded}, this run uses ${contentRetention}); ` + - 'forcing a full rebuild so stored text and FTS indexes are recreated.', - ); - options = { ...options, force: true }; + addGateReason({ + key: 'content-retention', + text: + `content retention changed (index built with ${recorded}, this run uses ${contentRetention}) — ` + + 'stored text and FTS indexes will be recreated.', + }); } // ── schema mismatch forces full rebuild (#2289 P1, #2798) ───────── @@ -1958,11 +2069,12 @@ async function runFullAnalysisInner( stamped === undefined && !repoHasGit ? ' Non-git repositories never record a schema fingerprint, so this run rebuilds regardless.' : ''; - log( - `index schema changed (built by ${origin}, this build is ${SCHEMA_FINGERPRINT}); forcing a ` + - `full re-analyze so the database is recreated from the current schema.${nonGitNote}`, - ); - options = { ...options, force: true }; + addGateReason({ + key: 'schema-fingerprint', + text: + `index schema changed (built by ${origin}, this build is ${SCHEMA_FINGERPRINT}) — ` + + `the database will be recreated from the current schema.${nonGitNote}`, + }); } // ── a recorded graph-write collapse forces a full rebuild ──────── @@ -1982,12 +2094,13 @@ async function runFullAnalysisInner( // same broken index as fresh. if (existingMeta?.graphWriteCollapsed) { const { expected, persisted } = existingMeta.graphWriteCollapsed; - log( - `previous run persisted ${persisted} of ${expected} expected relationships ` + - `(recorded as a graph-write collapse); forcing a full re-analyze rather than ` + + addGateReason({ + key: 'graph-write-collapse', + text: + `previous run persisted ${persisted} of ${expected} expected relationships ` + + `(recorded as a graph-write collapse) — a full re-analyze is required rather than ` + `reporting an index this build already knows is incomplete.`, - ); - options = { ...options, force: true }; + }); } // ── independently-versioned analysis capabilities ──────────────── @@ -2010,14 +2123,13 @@ async function runFullAnalysisInner( expectedPersistedAnalysisFeatures, ) : []; - let analysisFeatureMismatchLogged = false; if (existingMeta && persistedAnalysisFeatureMismatches.length > 0) { - log( - `analysis capabilities changed (${persistedAnalysisFeatureMismatches.join(', ')}); ` + - `forcing a full rebuild so persisted feature evidence is complete.`, - ); - options = { ...options, force: true }; - analysisFeatureMismatchLogged = true; + addGateReason({ + key: 'analysis-features', + text: + `analysis capabilities changed (${persistedAnalysisFeatureMismatches.join(', ')}) — ` + + `persisted feature evidence will be completed by the rebuild.`, + }); } const currentSpringVendorPrefixes = springVendorPrefixesKey(); @@ -2027,11 +2139,12 @@ async function runFullAnalysisInner( persistedRouteBindings === SPRING_ROUTE_BINDINGS_FEATURE.version && existingMeta.springVendorPrefixes !== currentSpringVendorPrefixes ) { - log( - 'Spring vendor mapping prefixes changed; forcing a full rebuild so persisted Route ' + - 'evidence matches the configured aliases.', - ); - options = { ...options, force: true }; + addGateReason({ + key: 'spring-vendor-prefixes', + text: + 'Spring vendor mapping prefixes changed — persisted Route evidence will be rebuilt ' + + 'to match the configured aliases.', + }); } // Analyzer provenance is part of freshness, not merely diagnostics. A @@ -2042,24 +2155,26 @@ async function runFullAnalysisInner( const stampedRunnerSchema = ( existingMeta.runnerIdentity as { schemaVersion?: unknown } | undefined )?.schemaVersion; - log( - `analyzer runner identity changed (stamped schema ${String(stampedRunnerSchema ?? 'missing')}, ` + - `this build uses schema ${runnerIdentity.schemaVersion}); forcing a full rebuild so the ` + - 'index provenance matches the analyzer and dependency/native runtime that produced it.', - ); - options = { ...options, force: true }; + addGateReason({ + key: 'runner-identity', + text: + `analyzer runner identity changed (stamped schema ${String(stampedRunnerSchema ?? 'missing')}, ` + + `this build uses schema ${runnerIdentity.schemaVersion}) — index provenance will be ` + + 'rewritten to match the analyzer and dependency/native runtime that produced it.', + }); } if ( existingMeta && cjkSegmentationModeMismatch(existingMeta.cjkSegmentation, getSearchFTSCjkSegmentation()) ) { - log( - `CJK segmentation mode changed (index built with '${existingMeta.cjkSegmentation ?? 'none'}', ` + - `this run resolves '${getSearchFTSCjkSegmentation()}'); forcing a full rebuild so indexed ` + - `text and query-time segmentation stay in sync.`, - ); - options = { ...options, force: true }; + addGateReason({ + key: 'cjk-segmentation', + text: + `CJK segmentation mode changed (index built with '${existingMeta.cjkSegmentation ?? 'none'}', ` + + `this run resolves '${getSearchFTSCjkSegmentation()}') — indexed text and query-time ` + + `segmentation will be rebuilt in sync.`, + }); } // ── embedding width mismatch forces full rebuild (#2798) ────────── @@ -2094,14 +2209,14 @@ async function runFullAnalysisInner( typeof recordedDims === 'number' && Number.isInteger(recordedDims) && recordedDims > 0 ? `FLOAT[${recordedDims}]` : 'an unrecognized width'; - log( - `embedding dimensions changed (index built with ${built}, this run embeds at ` + - `${EMBEDDING_DIMS}); forcing a full rebuild so the vector column is recreated at the ` + - `new width. Tip: set GITNEXUS_EMBEDDING_DIMS (or --embedding-dims) to pin it across runs.`, - ); - options = { ...options, force: true }; + addGateReason({ + key: 'embedding-dims', + text: + `embedding dimensions changed (index built with ${built}, this run embeds at ` + + `${EMBEDDING_DIMS}) — the vector column will be recreated at the new width. ` + + `Tip: set GITNEXUS_EMBEDDING_DIMS (or --embedding-dims) to pin it across runs.`, + }); } - // Actuator snapshots are external runtime inputs and are intentionally not // hashed or persisted. Rebuild on every enabled run so updated snapshots // cannot hit the git freshness fast path; rebuild once when the option is @@ -2130,10 +2245,10 @@ async function runFullAnalysisInner( ) { retainedActuatorInputs.push(springActuatorRepoRelativeInput); } - if (!options.force) { - log('Spring Actuator runtime enrichment requested; forcing a full rebuild.'); - } - options = { ...options, force: true }; + collector.add({ + key: 'spring-actuator', + text: 'Spring Actuator runtime enrichment requested — runtime snapshots are re-read on every run.', + }); } else if (springActuatorPreviouslyEnabled) { if ( !Array.isArray(previousActuatorInputs) || @@ -2145,8 +2260,10 @@ async function runFullAnalysisInner( 'with the previous --spring-actuator path, then run again without it.', ); } - log('Spring Actuator runtime enrichment disabled; rebuilding to remove runtime evidence.'); - options = { ...options, force: true }; + collector.add({ + key: 'spring-actuator', + text: 'Spring Actuator runtime enrichment disabled — runtime evidence will be removed.', + }); } const springActuatorScanExclusions = retainedActuatorInputs.length === 0 ? undefined : retainedActuatorInputs; @@ -2171,20 +2288,15 @@ async function runFullAnalysisInner( const asyncApiSpecRequested = options.asyncApiSpecPath !== undefined; const asyncApiSpecPreviouslyEnabled = existingMeta?.asyncApiSpec?.enabled === true; if (asyncApiSpecRequested) { - if (!options.force) { - log('AsyncAPI document reading requested; forcing a full rebuild.'); - } - options = { ...options, force: true }; + collector.add({ + key: 'asyncapi', + text: 'AsyncAPI document reading requested — documents are re-read on every run.', + }); } else if (asyncApiSpecPreviouslyEnabled) { - log('AsyncAPI document reading disabled; rebuilding to remove document-derived evidence.'); - options = { ...options, force: true }; - } - - // Programmatic `useParseCache: false` must set force or the up-to-date - // guard returns before the empty-cache construction below. - if (options.useParseCache === false && !options.force) { - log('Parser cache bypass requested; forcing a full rebuild so unchanged files are re-parsed.'); - options = { ...options, force: true }; + collector.add({ + key: 'asyncapi', + text: 'AsyncAPI document reading disabled — document-derived evidence will be removed.', + }); } // Process-detection budget (#3313). Resolve CLI/options then env here so @@ -2216,14 +2328,23 @@ async function runFullAnalysisInner( // writes only changed files into a fresh, empty database) would restore it, // so rebuild. Scoped to store slots: private `.gitnexus` indexes only lose // their graph by hand, and their metadata-only fixtures rely on this path. - if (existingMeta && !options.force && storeRootOfCheckoutSlot(storagePath)) { + // Checked even when another reason already forces the rebuild, so the + // summary names every cause. + if (existingMeta && storeRootOfCheckoutSlot(storagePath)) { const graph = placement.branch ? lbugPath : resolveGraphPath(storagePath); if (!existsSync(graph)) { - log('Shared store: this checkout has no graph; doing a full build.'); - options = { ...options, force: true }; + collector.add({ + key: 'shared-store-missing-graph', + text: 'shared store: this checkout records a commit but has no graph — building a fresh one.', + }); } } + // Checkpoint 1 (pre-pipeline): every reason the fast path must honor is in. + // `ensurePrivateGraph` below can still add one; the checkpoint re-runs after + // it, before any reader that plans the rebuild. + applyCollectedForce(); + // ── Early-return: already up to date ────────────────────────────── if ( existingMeta && @@ -2384,6 +2505,7 @@ async function runFullAnalysisInner( storagePath, stats: existingMeta.stats ?? {}, alreadyUpToDate: true, + rebuildReasons: collector.keys(), ...(ftsDisabledReason ? { ftsSkipped: true, ftsSkipReason: ftsDisabledReason } : {}), ...(!ftsDisabledReason && priorFtsNativeAbort ? { ftsSkipped: true, ftsSkipReason: 'native-abort' } @@ -2402,6 +2524,9 @@ async function runFullAnalysisInner( _deriveEmbeddingMode(options, existingMeta?.stats?.embeddings ?? 0).shouldLoadCache; await ensurePrivateGraph(!options.force || forcedRebuildReadsOldGraph); delete existingMeta?.graphPath; + // Checkpoint 1, final read: a graph copy that just failed forces the rebuild + // before the embedding plan, the parse cache, and the emit mode are decided. + applyCollectedForce(); // ── Cache embeddings from existing index before rebuild ──────────── // Four modes: @@ -2515,16 +2640,17 @@ async function runFullAnalysisInner( // Streamed structural emit (#2680). Resolved ONCE, so the pipeline flag and // the CSV-dir resolution below cannot disagree — and resolved HERE, not at // function entry, because the POSITION is load-bearing: the gate is - // `options.force`, and every freshness guard above REBINDS `options` with - // `force: true` (embedding-checkpoint drop, dirty-flag recovery, pdg-mode - // flip, schema-fingerprint change, analysis-feature drift, runner-identity change, - // CJK-mode change). Resolving before them froze the answer at `false` for + // `options.force`, which only the collector checkpoints above set, from the + // reasons every freshness guard contributed (embedding-checkpoint drop, + // dirty-flag recovery, pdg-mode flip, schema-fingerprint change, + // analysis-feature drift, runner-identity change, CJK-mode change, a failed + // shared-graph copy). Resolving before them froze the answer at `false` for // every rebuild they trigger — including the whole-fleet rebuild an // schema-fingerprint change forces on every existing index at once, // which is exactly when the #2649 memory relief matters most. So this MUST - // stay below the last guard that can set `force` and above its first use. - // (The post-pipeline analysis-feature re-check can also set `force`, but the - // pipeline has already run by then; that run emits non-streamed, precisely as + // stay below the last checkpoint that can set `force` and above its first use. + // (The post-pipeline checkpoint can also set `force`, but the pipeline has + // already run by then; that run emits non-streamed, precisely as // `resolveStreamPdgEmit` — read fresh at the same point — behaves.) const streamGraphEmitActive = resolveStreamGraphEmit(options); @@ -2547,6 +2673,11 @@ async function runFullAnalysisInner( repoHasGit && !schemaFingerprintMismatch(existingMeta.schemaFingerprint); + // The one pre-pipeline announcement (#3137): the reason inline, several as a + // numbered block, nothing for an incremental or structural build. + const rebuildSummary = collector.formatSummary(); + if (rebuildSummary !== undefined) log(rebuildSummary); + // ── Phase 1: Full Pipeline (0–60%) ──────────────────────────────── let pipelineResult; try { @@ -2650,20 +2781,26 @@ async function runFullAnalysisInner( const currentAnalysisFeatureMismatches = existingMeta ? findAnalysisFeatureMismatches(existingMeta.analysisFeatures, currentAnalysisFeatures) : []; - if ( - existingMeta && - currentAnalysisFeatureMismatches.length > 0 && - !analysisFeatureMismatchLogged - ) { + if (existingMeta && currentAnalysisFeatureMismatches.length > 0) { // Covers a repository gaining or losing its first applicable source file: // the persisted file list cannot predict that transition before the // pipeline, but an incremental top-up would leave unchanged rows incomplete. - log( - `analysis capabilities changed (${currentAnalysisFeatureMismatches.join(', ')}); ` + - `forcing a full rebuild so persisted feature evidence is complete.`, - ); - options = { ...options, force: true }; + // Same key as the pre-pipeline gate, so a mismatch that gate already + // announced merges into its entry and prints no second line. + addGateReason({ + key: 'analysis-features', + text: + `analysis capabilities changed (${currentAnalysisFeatureMismatches.join(', ')}) — ` + + `persisted feature evidence will be completed by the rebuild.`, + }); } + // Checkpoint 2 (post-pipeline): the capability re-check is the last reason + // that can force the rebuild. The #2409 escalation below adds a non-forcing + // reason, so `force` is final from here on. + applyCollectedForce(); + // A forced reason added here rules the escalation out (the run is no longer + // incremental), so this and the escalation's line never both print. + announceFollowUp(); // Decide incremental vs full at THIS point (post-pipeline, pre-DB). // All eligibility conditions are checked here against the actual @@ -2800,6 +2937,7 @@ async function runFullAnalysisInner( phase: 'pre-write', toWriteCount: hashDiff.toWrite.length, directWriteCount: hashDiff.toWrite.length, + ...storedRebuildReasons(), }, }); } @@ -2834,6 +2972,7 @@ async function runFullAnalysisInner( updatedAt: now, phase: 'full-rebuild', toWriteCount: 0, + ...storedRebuildReasons(), }, }); } @@ -2987,6 +3126,10 @@ async function runFullAnalysisInner( toWriteCount: writableFiles.size, directWriteCount: directlyChangedCount, ...(droppedImporterChunks > 0 ? { droppedImporterChunks } : {}), + // Rebuilt from the live collector on every call, so the escalation + // reason (added just before the 'escalated-full-write' save) rides + // on that stamp and every later one (#3137). + ...storedRebuildReasons(), ...extra, }, }); @@ -3208,7 +3351,7 @@ async function runFullAnalysisInner( // destroy them. `--drop-embeddings` deliberately leaves `cachedSnapshot` // empty (`deriveEmbeddingMode` returns `shouldLoadCache: false` for it by // construction — see the four-mode comment at the cache-load site), and its - // `options.force = true` conversion sits INSIDE + // `drop-embeddings` rebuild reason is only collected INSIDE // `if (existingMeta?.embeddingCheckpoint)`, so a repo without a checkpoint // stays incremental and arrives here holding exactly the state the rescue // reads as "the index metadata did not account for them" — restoring the N @@ -3396,13 +3539,21 @@ async function runFullAnalysisInner( label === 'FTS' ? resolveFtsVersionPair(inspectPath) : undefined, ).remedy; }); - log( - `Incremental: ${escalationCauses.join('; and ')} — switching to a full DB write ` + - `(wipe + bulk COPY) for this run; file-level incremental bookkeeping is unaffected.` + + // Announced through the collector as its post-pipeline follow-up, but + // non-forcing: `escalatedFullWrite` drives the write plan, and + // `options.force` stays as checkpoint 2 left it. + collector.add({ + key: 'escalated-full-write', + text: + `incremental write escalated: ${escalationCauses.join('; and ')} — switching to a ` + + `full DB write (wipe + bulk COPY) for this run; file-level incremental bookkeeping ` + + `is unaffected.` + (degradedEffects.length > 0 ? ` ${degradedEffects.join(' ')} ${extensionRemedies.join(' ')}` : ''), - ); + forcing: false, + }); + announceFollowUp(); // toWriteCount: 0 is the established full-path dirty-flag sentinel; // the real counters ride along for crash diagnostics. await saveIncrementalDirtyState('escalated-full-write', { @@ -4985,6 +5136,7 @@ async function runFullAnalysisInner( repoPath, storagePath, stats: meta.stats, + rebuildReasons: collector.keys(), pipelineResult, ...(graphWriteCollapsed ? { graphWriteCollapsed } : {}), ftsSkipped: !ftsReady, diff --git a/gitnexus/src/core/search/fts-crash-marker.ts b/gitnexus/src/core/search/fts-crash-marker.ts index 6495583fe..bf0fb32eb 100644 --- a/gitnexus/src/core/search/fts-crash-marker.ts +++ b/gitnexus/src/core/search/fts-crash-marker.ts @@ -111,5 +111,8 @@ export const buildFtsDirtyStamp = (args: { ...(prior?.droppedImporterChunks !== undefined ? { droppedImporterChunks: prior.droppedImporterChunks } : {}), + // Rebuild reasons (#3137) survive the FTS restamp so a crash inside + // CREATE_FTS_INDEX still names why the run rebuilt. + ...(prior?.reasons !== undefined ? { reasons: prior.reasons } : {}), }; }; diff --git a/gitnexus/src/storage/repo-meta.ts b/gitnexus/src/storage/repo-meta.ts index 66540a164..0c412dd7c 100644 --- a/gitnexus/src/storage/repo-meta.ts +++ b/gitnexus/src/storage/repo-meta.ts @@ -33,6 +33,7 @@ import type { NameFallbackSummary } from '../core/ingestion/scope-resolution/nam import type { UndecidedSatisfactionSummary } from '../core/ingestion/scope-resolution/undecided-satisfaction.js'; import { resolveStoragePath } from './storage-resolver.js'; import type { ScopeExtractionFailureSummary } from '../core/ingestion/scope-resolution/scope-extraction-failures.js'; +import type { StoredRebuildReason } from '../core/rebuild-reasons.js'; import { INDEX_METADATA_FILE, LEGACY_METADATA_FILE } from './storage-constants.js'; export { GITNEXUS_DIR, INDEX_METADATA_FILE, LEGACY_METADATA_FILE } from './storage-constants.js'; @@ -459,6 +460,12 @@ export interface RepoMeta { * diagnostics must show whether the write set was already * under-expanded when the run died. */ droppedImporterChunks?: number; + /** + * Why this run rebuilds (#3137), so an interrupted rebuild can name its + * causes on the next run. Untrusted on read: parse it with + * `readStoredRebuildReasons`, since the file is schema-less JSON. + */ + reasons?: StoredRebuildReason[]; }; /** * Durable embedding-resume marker, written in two distinct situations that diff --git a/gitnexus/test/integration/external-storage-content-retention.test.ts b/gitnexus/test/integration/external-storage-content-retention.test.ts index 6fe510913..eea18704c 100644 --- a/gitnexus/test/integration/external-storage-content-retention.test.ts +++ b/gitnexus/test/integration/external-storage-content-retention.test.ts @@ -138,7 +138,7 @@ describe('external storage and content retention', () => { expect(fullGraph.basicBlockCount).toBeGreaterThan(0); process.env.GITNEXUS_CONTENT_RETENTION = 'symbol'; - await runFullAnalysis( + const symbolRun = await runFullAnalysis( repo, { ...options, force: false }, { @@ -149,7 +149,8 @@ describe('external storage and content retention', () => { const symbolMeta = await loadMeta(storage); const symbolGraph = await readGraph(lbugPath); const symbolDatabaseSize = await recursiveSize(lbugPath); - expect(logs.join('\n')).toContain('forcing a full rebuild'); + // The retention change is what forced this rebuild (the caller passed no --force). + expect(symbolRun.rebuildReasons).toContain('content-retention'); expect(symbolMeta).toMatchObject({ contentRetention: 'symbol', contentRetentionSchemaVersion: 1, diff --git a/gitnexus/test/integration/shared-store-analyze.test.ts b/gitnexus/test/integration/shared-store-analyze.test.ts index b42c5dd6d..5c5df84f9 100644 --- a/gitnexus/test/integration/shared-store-analyze.test.ts +++ b/gitnexus/test/integration/shared-store-analyze.test.ts @@ -6,6 +6,7 @@ import { pathToFileURL } from 'url'; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { CLASS_FRAMEWORK_ANNOTATIONS_FEATURE } from '../../src/core/analysis-features.js'; import { resolveAnalyzerRunnerIdentity } from '../../src/core/analyzer-identity.js'; +import { RebuildReasonCollector } from '../../src/core/rebuild-reasons.js'; import type { EmbeddingCheckpoint } from '../../src/core/embedding-checkpoint.js'; import { SCHEMA_FINGERPRINT } from '../../src/core/lbug/schema.js'; import { @@ -536,7 +537,10 @@ describe('up-to-date fast path over a missing shared graph (#3374)', () => { await tmpHome.cleanup(); }); - it('rebuilds a slot whose metadata is at HEAD but whose graph is gone', async () => { + /** A linked-worktree checkout slot whose metadata is current in every stamp. */ + const currentCheckoutSlot = async ( + overrides: Partial = {}, + ): Promise<{ wt: string; slot: string }> => { const root = await fs.realpath(tmpRepo.dbPath); const main = path.join(root, 'main'); await fs.mkdir(main); @@ -569,14 +573,20 @@ describe('up-to-date fast path over a missing shared graph (#3374)', () => { runnerIdentity: resolveAnalyzerRunnerIdentity( pathToFileURL(path.resolve(__dirname, '../../src/core/run-analyze.ts')).href, ), - // Same FTS mode as the run below, so only the missing graph can - // decide against the fast path. + // Same FTS mode as the runs below, so only the graph can decide + // against the fast path. capabilities: { graph: { provider: 'ladybugdb', status: 'available' }, fts: { provider: 'ladybugdb-fts', status: 'unavailable', skipReason: 'disabled-by-flag' }, vectorSearch: { provider: 'exact-scan', status: 'unavailable', exactScanLimit: 0 }, }, + ...overrides, }); + return { wt, slot }; + }; + + it('rebuilds a slot whose metadata is at HEAD but whose graph is gone', async () => { + const { wt, slot } = await currentCheckoutSlot(); const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); const result = await runFullAnalysis( @@ -586,8 +596,45 @@ describe('up-to-date fast path over a missing shared graph (#3374)', () => { ); expect(result.alreadyUpToDate).not.toBe(true); + expect(result.rebuildReasons).toEqual(['shared-store-missing-graph']); expect(existsSync(resolveGraphPath(slot))).toBe(true); }, 120_000); + + it('names a failed shared-graph copy in the summary, not in a follow-up (#3137)', async () => { + // The slot records an older commit, so the run reaches the copy, and + // points at a published commit graph that exists but cannot be copied (a + // directory where the graph file belongs). + const olderCommit = '0'.repeat(40); + const { wt, slot } = await currentCheckoutSlot({ lastCommit: olderCommit }); + const unreadableGraph = path.join( + commitGraphDir(layoutOf(wt), olderCommit, 'deadbeefdeadbeef'), + 'lbug', + ); + await fs.mkdir(unreadableGraph, { recursive: true }); + const meta = await loadMeta(slot); + if (meta === null) throw new Error('slot metadata missing'); + await saveMeta(slot, { ...meta, graphPath: unreadableGraph }); + expect(resolveGraphPath(slot)).toBe(unreadableGraph); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const logs: string[] = []; + const result = await runFullAnalysis( + wt, + { skipAgentsMd: true, skipSkills: true, skipFts: true }, + { onProgress: () => {}, onLog: (m) => logs.push(m) }, + ); + + expect(result.rebuildReasons).toEqual(['private-graph-unavailable']); + const single = new RebuildReasonCollector(); + single.add({ key: 'private-graph-unavailable', text: '' }); + const summaryPrefix = single.formatSummary() ?? ''; + const late = new RebuildReasonCollector(); + late.formatSummary(); + late.add({ key: 'private-graph-unavailable', text: '' }); + const followUpPrefix = late.formatFollowUp() ?? ''; + expect(logs.filter((m) => m.startsWith(summaryPrefix))).toHaveLength(1); + expect(logs.filter((m) => m.startsWith(followUpPrefix))).toEqual([]); + }, 120_000); }); describe('listStoreMetaRoots', () => { diff --git a/gitnexus/test/unit/analyze-heap-respawn.test.ts b/gitnexus/test/unit/analyze-heap-respawn.test.ts index 7176fb46d..150cbb554 100644 --- a/gitnexus/test/unit/analyze-heap-respawn.test.ts +++ b/gitnexus/test/unit/analyze-heap-respawn.test.ts @@ -414,6 +414,356 @@ describe('analyzeCommand heap respawn', () => { }); }); +const MB = 1024 * 1024; + +/** Run `body` with `process.execArgv` replaced, restoring it afterwards. */ +const withExecArgv = async (execArgv: string[], body: () => Promise): Promise => { + const execArgvDesc = Object.getOwnPropertyDescriptor(process, 'execArgv'); + Object.defineProperty(process, 'execArgv', { configurable: true, value: execArgv }); + try { + await body(); + } finally { + if (execArgvDesc) Object.defineProperty(process, 'execArgv', execArgvDesc); + } +}; + +/** Pin the cgroup limit so the auto cap resolves to `floor(0.8 × mb)`. */ +const constrainTo = (mb: number): void => { + Object.defineProperty(process, 'constrainedMemory', { + configurable: true, + value: () => mb * MB, + }); +}; + +describe('ensureHeap --memory-budget (#3137)', () => { + let initialNodeOptions: string | undefined; + let initialSource: string | undefined; + let initialMemory: string | undefined; + let stdoutWriteSpy: ReturnType; + let stderrWriteSpy: ReturnType; + let restoreConstrainedMemory: (() => void) | undefined; + + beforeEach(() => { + initialNodeOptions = process.env.NODE_OPTIONS; + initialSource = process.env.GITNEXUS_HEAP_LIMIT_SOURCE; + initialMemory = process.env.GITNEXUS_MEMORY; + delete process.env.NODE_OPTIONS; + delete process.env.GITNEXUS_MEMORY; + delete process.env.GITNEXUS_HEAP_LIMIT_SOURCE; + vi.resetModules(); + spawnMock.mockReset(); + getHeapStatisticsMock.mockReset(); + getHeapStatisticsMock.mockReturnValue({ heap_size_limit: 512 * MB }); + process.exitCode = undefined; + const cmDesc = Object.getOwnPropertyDescriptor(process, 'constrainedMemory'); + // Unconstrained: the mocked 16GB totalmem gives an auto cap of 13107MB. + Object.defineProperty(process, 'constrainedMemory', { configurable: true, value: () => 0 }); + restoreConstrainedMemory = () => { + if (cmDesc) Object.defineProperty(process, 'constrainedMemory', cmDesc); + else delete (process as { constrainedMemory?: unknown }).constrainedMemory; + }; + stdoutWriteSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + stderrWriteSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + }); + + afterEach(() => { + restoreConstrainedMemory?.(); + restoreConstrainedMemory = undefined; + stdoutWriteSpy.mockRestore(); + stderrWriteSpy.mockRestore(); + if (initialNodeOptions === undefined) delete process.env.NODE_OPTIONS; + else process.env.NODE_OPTIONS = initialNodeOptions; + if (initialSource === undefined) delete process.env.GITNEXUS_HEAP_LIMIT_SOURCE; + else process.env.GITNEXUS_HEAP_LIMIT_SOURCE = initialSource; + if (initialMemory === undefined) delete process.env.GITNEXUS_MEMORY; + else process.env.GITNEXUS_MEMORY = initialMemory; + process.exitCode = undefined; + }); + + it('replaces an 8192MB execArgv pin with a 2000MB heap, logging one line that names the pin', async () => { + mockSpawnExit(); + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + await withExecArgv(['--max-old-space-size=8192'], async () => { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand(undefined, { memoryBudget: '2000' }); + }); + cap.restore(); + + expect(spawnMock).toHaveBeenCalledTimes(1); + const [, args, opts] = spawnMock.mock.calls[0]; + const records = cap.records(); + expect({ + // The budget flags follow the user's pin, so V8's later-flag-wins + // semantics resolve to old 1616 + 3 × semi 128 = 2000MB. + budgetFlagsAfterPin: + args.indexOf('--max-old-space-size=1616') > args.indexOf('--max-old-space-size=8192'), + semi: args.includes('--max-semi-space-size=128'), + nodeOptions: opts.env.NODE_OPTIONS, + source: opts.env.GITNEXUS_HEAP_LIMIT_SOURCE, + recordCount: records.length, + namesPin: records[0]?.msg.includes('8192MB'), + namesBudget: records[0]?.msg.includes('2000MB'), + }).toEqual({ + budgetFlagsAfterPin: true, + semi: true, + nodeOptions: '--max-old-space-size=1616 --max-semi-space-size=128', + source: 'budget', + recordCount: 1, + namesPin: true, + namesBudget: true, + }); + }); + + it('raises a 1024MB NODE_OPTIONS pin to a 3000MB heap, without the below-cap warning', async () => { + process.env.NODE_OPTIONS = '--max-old-space-size=1024'; + mockSpawnExit(); + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand(undefined, { memoryBudget: '3000' }); + cap.restore(); + + expect(spawnMock).toHaveBeenCalledTimes(1); + const [, args] = spawnMock.mock.calls[0]; + const records = cap.records(); + expect({ + // 3000 / 25 = 120 rounds up to V8's power-of-two semi-space 128. + old: args.includes('--max-old-space-size=2616'), + semi: args.includes('--max-semi-space-size=128'), + recordCount: records.length, + namesPin: records[0]?.msg.includes('1024MB'), + namesAutoCap: records[0]?.msg.includes('13107MB'), + }).toEqual({ old: true, semi: true, recordCount: 1, namesPin: true, namesAutoCap: false }); + }); + + it('a 12000MB budget above a 6000MB auto cap respawns at 12000 with a swap-risk warning', async () => { + // 7500MB cgroup limit -> auto cap floor(0.8 × 7500) = 6000MB. + constrainTo(7500); + mockSpawnExit(); + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand(undefined, { memoryBudget: '12000' }); + cap.restore(); + + expect(spawnMock).toHaveBeenCalledTimes(1); + const [, args] = spawnMock.mock.calls[0]; + const records = cap.records(); + expect({ + old: args.includes('--max-old-space-size=11616'), + recordCount: records.length, + namesAutoCap: records[0]?.msg.includes('6000MB'), + warnsSwap: /swap/i.test(records[0]?.msg ?? ''), + }).toEqual({ old: true, recordCount: 1, namesAutoCap: true, warnsSwap: true }); + }); + + it('a budget-respawned child (8192 pin, then the budget flags) proceeds silently', async () => { + process.env.GITNEXUS_HEAP_LIMIT_SOURCE = 'budget'; + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + let respawned: boolean | undefined; + await withExecArgv( + ['--max-old-space-size=8192', '--max-old-space-size=1616', '--max-semi-space-size=128'], + async () => { + const { ensureHeap } = await import('../../src/cli/analyze.js'); + respawned = await ensureHeap({ memoryBudget: '2000' }); + }, + ); + cap.restore(); + + expect({ respawned, spawns: spawnMock.mock.calls.length, records: cap.records() }).toEqual({ + respawned: false, + spawns: 0, + records: [], + }); + }); + + it('a pin equal to the budget is not the exact budget heap, so the process respawns once', async () => { + mockSpawnExit(); + constrainTo(7500); + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + let respawned: boolean | undefined; + await withExecArgv(['--max-old-space-size=12000'], async () => { + const { ensureHeap } = await import('../../src/cli/analyze.js'); + respawned = await ensureHeap({ memoryBudget: '12000' }); + }); + cap.restore(); + + const [, args, opts] = spawnMock.mock.calls[0]; + const records = cap.records(); + expect({ + respawned, + flags: args.filter( + (a: string) => a.startsWith('--max-old-space') || a.startsWith('--max-semi'), + ), + source: opts.env.GITNEXUS_HEAP_LIMIT_SOURCE, + recordCount: records.length, + warnsSwap: /swap/i.test(records[0]?.msg ?? ''), + }).toEqual({ + respawned: true, + flags: [ + '--max-old-space-size=12000', + '--max-old-space-size=11616', + '--max-semi-space-size=128', + ], + source: 'budget', + recordCount: 1, + warnsSwap: true, + }); + }); + + it('a programmatic caller passing an invalid budget exits 1 without respawning', async () => { + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand(undefined, { memoryBudget: '199' }); + cap.restore(); + + expect({ + exitCode: process.exitCode, + spawns: spawnMock.mock.calls.length, + namesMinimum: cap.records().some((r) => /--memory-budget.*200/.test(String(r.msg ?? ''))), + }).toEqual({ exitCode: 1, spawns: 0, namesMinimum: true }); + }); + + it('a kept process does not leak its heap source into a later analyzeCommand call', async () => { + getHeapStatisticsMock.mockReturnValue({ heap_size_limit: 13107 * MB }); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + // An invalid --workers returns right after ensureHeap kept the process and + // recorded its source, so the env restore runs without a full analyze. + await analyzeCommand(undefined, { workers: '0' }); + expect(process.env.GITNEXUS_HEAP_LIMIT_SOURCE).toBeUndefined(); + }); + + it('budget 200 scales the semi-space down so old + 3 × semi equals 200', async () => { + mockSpawnExit(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand(undefined, { memoryBudget: '200' }); + + const [, args] = spawnMock.mock.calls[0]; + expect( + args.filter((a: string) => a.startsWith('--max-old-space') || a.startsWith('--max-semi')), + ).toEqual(['--max-old-space-size=176', '--max-semi-space-size=8']); + }); + + it('a forged budget marker without the budget semi-space flag still respawns', async () => { + mockSpawnExit(); + process.env.GITNEXUS_HEAP_LIMIT_SOURCE = 'budget'; + let respawned: boolean | undefined; + await withExecArgv(['--max-old-space-size=1616'], async () => { + const { ensureHeap } = await import('../../src/cli/analyze.js'); + respawned = await ensureHeap({ memoryBudget: '2000' }); + }); + expect({ respawned, spawns: spawnMock.mock.calls.length }).toEqual({ + respawned: true, + spawns: 1, + }); + }); + + it('a budget child whose old-space pin is spelled with underscores and a space skips the respawn', async () => { + process.env.GITNEXUS_HEAP_LIMIT_SOURCE = 'budget'; + let respawned: boolean | undefined; + await withExecArgv(['--max_old_space_size', '1616', '--max-semi-space-size=128'], async () => { + const { ensureHeap } = await import('../../src/cli/analyze.js'); + respawned = await ensureHeap({ memoryBudget: '2000' }); + }); + + expect({ + respawned, + spawns: spawnMock.mock.calls.length, + source: process.env.GITNEXUS_HEAP_LIMIT_SOURCE, + }).toEqual({ respawned: false, spawns: 0, source: 'budget' }); + }); + + it('GITNEXUS_MEMORY=off still respawns at an explicit budget', async () => { + process.env.GITNEXUS_MEMORY = 'off'; + mockSpawnExit(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand(undefined, { memoryBudget: '2000' }); + + expect(spawnMock).toHaveBeenCalledTimes(1); + const [, args, opts] = spawnMock.mock.calls[0]; + expect({ + old: args.includes('--max-old-space-size=1616'), + source: opts.env.GITNEXUS_HEAP_LIMIT_SOURCE, + }).toEqual({ old: true, source: 'budget' }); + }); + + it('without a budget the auto respawn keeps its exact argv and records source=auto in the child env', async () => { + mockSpawnExit(); + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand(undefined, {}); + cap.restore(); + + const [, args, opts] = spawnMock.mock.calls[0]; + const preserved = process.execArgv.filter((a) => !a.startsWith('--inspect')); + expect({ args, source: opts.env.GITNEXUS_HEAP_LIMIT_SOURCE, records: cap.records() }).toEqual({ + args: [ + ...preserved, + '--max-old-space-size=13107', + '--max-semi-space-size=128', + '--stack-size=4096', + ...process.argv.slice(1), + ], + source: 'auto', + records: [], + }); + }); + + it('a process already at the auto cap records source=auto for itself', async () => { + getHeapStatisticsMock.mockReturnValue({ heap_size_limit: 13107 * MB }); + const { ensureHeap } = await import('../../src/cli/analyze.js'); + const respawned = await ensureHeap(); + + expect({ respawned, source: process.env.GITNEXUS_HEAP_LIMIT_SOURCE }).toEqual({ + respawned: false, + source: 'auto', + }); + }); + + it('the respawn OOM message names the budget and --memory-budget instead of NODE_OPTIONS', async () => { + mockSpawnExit({ status: null, signal: 'SIGABRT' }); + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + await analyzeCommand(undefined, { memoryBudget: '2000' }); + cap.restore(); + + const oom = cap.records().find((r) => r.recoveryHint === 'heap-oom-respawn'); + expect({ + exitCode: process.exitCode, + namesBudget: oom?.msg.includes('2000MB'), + namesFlag: oom?.msg.includes('--memory-budget'), + mentionsNodeOptions: oom?.msg.includes('NODE_OPTIONS'), + }).toEqual({ exitCode: 1, namesBudget: true, namesFlag: true, mentionsNodeOptions: false }); + }); + + it('analyze --watch --memory-budget 2000 goes through the same budget respawn', async () => { + mockSpawnExit(); + const { analyzeOrWatchCommandWithRunnerIdentity } = await import('../../src/cli/analyze.js'); + const { resolveAnalyzerRunnerIdentity } = await import('../../src/core/analyzer-identity.js'); + const identity = resolveAnalyzerRunnerIdentity( + new URL('../../src/cli/analyze.ts', import.meta.url).href, + ); + await analyzeOrWatchCommandWithRunnerIdentity(identity, undefined, { + watch: true, + memoryBudget: '2000', + }); + + expect(spawnMock).toHaveBeenCalledTimes(1); + const [, args, opts] = spawnMock.mock.calls[0]; + expect({ + old: args.includes('--max-old-space-size=1616'), + source: opts.env.GITNEXUS_HEAP_LIMIT_SOURCE, + exitCode: process.exitCode, + }).toEqual({ old: true, source: 'budget', exitCode: undefined }); + }); +}); + describe('computeHeapCapMb (RAM-aware auto heap cap)', () => { const GB = 1024 * 1024 * 1024; diff --git a/gitnexus/test/unit/analyze-memory-budget.test.ts b/gitnexus/test/unit/analyze-memory-budget.test.ts new file mode 100644 index 000000000..cb4f2319e --- /dev/null +++ b/gitnexus/test/unit/analyze-memory-budget.test.ts @@ -0,0 +1,129 @@ +/** + * `--memory-budget` (#3137) at its two real boundaries: the commander entry, + * which rejects a bad value before any heap work, and a real child process + * launched with the budget's heap flags. The respawn decision itself is + * covered through `ensureHeap` in `analyze-heap-respawn.test.ts`. + */ +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const ensureHeapMock = vi.fn(async () => true); +const analyzeOrWatchMock = vi.fn(async () => undefined); + +vi.mock('../../src/cli/analyze.js', () => ({ + ensureHeap: ensureHeapMock, + analyzeOrWatchCommandWithRunnerIdentity: analyzeOrWatchMock, +})); + +vi.mock('../../src/cli/update-notice.js', () => ({ + runProcessCliUpdateNotice: vi.fn(), +})); + +class ExitCalled extends Error { + constructor(readonly code: number | string | null | undefined) { + super(`process.exit(${String(code)})`); + } +} + +describe('--memory-budget validation at the CLI entry', () => { + const initialArgv = process.argv; + let stderrChunks: string[]; + + beforeEach(() => { + vi.resetModules(); + ensureHeapMock.mockClear(); + analyzeOrWatchMock.mockClear(); + stderrChunks = []; + vi.spyOn(process.stderr, 'write').mockImplementation((chunk: string | Uint8Array) => { + stderrChunks.push(String(chunk)); + return true; + }); + vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + vi.spyOn(process, 'exit').mockImplementation((code?: number | string | null) => { + throw new ExitCalled(code); + }); + }); + + afterEach(() => { + process.argv = initialArgv; + vi.restoreAllMocks(); + }); + + const cases = ['abc', '0', '199', '1.5', '-5'].flatMap((value) => [ + { label: `analyze ${value}`, argv: ['analyze', '--memory-budget', value] }, + { label: `analyze --watch ${value}`, argv: ['analyze', '--watch', '--memory-budget', value] }, + ]); + + it.each(cases)('$label exits 1 naming the flag and its minimum', async ({ argv }) => { + process.argv = [process.execPath, 'gitnexus', ...argv]; + + await expect(import('../../src/cli/index.js')).rejects.toMatchObject({ code: 1 }); + + const stderr = stderrChunks.join(''); + expect({ + namesFlag: stderr.includes('--memory-budget'), + namesMinimum: stderr.includes('200'), + ensureHeapCalls: ensureHeapMock.mock.calls.length, + actionCalls: analyzeOrWatchMock.mock.calls.length, + }).toEqual({ namesFlag: true, namesMinimum: true, ensureHeapCalls: 0, actionCalls: 0 }); + }); +}); + +describe('--memory-budget is CLI-only', () => { + it('a .gitnexusrc memoryBudget key is rejected as an unknown key', async () => { + const { GitNexusRcError, loadAnalyzeConfig } = await import('../../src/cli/analyze-config.js'); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-budget-rc-')); + try { + fs.writeFileSync(path.join(dir, '.gitnexusrc'), JSON.stringify({ memoryBudget: 2000 })); + let thrown: unknown; + try { + loadAnalyzeConfig(dir); + } catch (error) { + thrown = error; + } + expect({ + isRcError: thrown instanceof GitNexusRcError, + namesKey: String(thrown).includes('memoryBudget'), + }).toEqual({ isRcError: true, namesKey: true }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +describe('--memory-budget real-child smoke (256MB)', () => { + it('a child launched with the budget heap flags reports a 256MB limit and would not respawn again', async () => { + // Imported for real: this block needs the production sizing, not the mock. + const { budgetHeapSizing, resolveBudgetHeap } = await vi.importActual< + typeof import('../../src/cli/analyze.js') + >('../../src/cli/analyze.js'); + const { oldSpaceMb, semiSpaceMb } = budgetHeapSizing(256); + const out = execFileSync( + process.execPath, + [ + `--max-old-space-size=${oldSpaceMb}`, + `--max-semi-space-size=${semiSpaceMb}`, + '-e', + 'process.stdout.write(JSON.stringify({ limit: require("v8").getHeapStatistics().heap_size_limit, execArgv: process.execArgv }))', + ], + { encoding: 'utf8', env: { ...process.env, NODE_OPTIONS: '' } }, + ); + const child = JSON.parse(out) as { limit: number; execArgv: string[] }; + const decision = resolveBudgetHeap({ + budgetMb: 256, + execArgv: child.execArgv, + nodeOptions: '', + autoCapMb: 13107, + autopilotDisabled: false, + inheritedSource: 'budget', + }); + + expect({ limitMb: child.limit / (1024 * 1024), respawn: decision.respawn }).toEqual({ + limitMb: 256, + respawn: false, + }); + }); +}); diff --git a/gitnexus/test/unit/analyze-no-stats-bridge.test.ts b/gitnexus/test/unit/analyze-no-stats-bridge.test.ts index bab7eb100..a63a6fa5a 100644 --- a/gitnexus/test/unit/analyze-no-stats-bridge.test.ts +++ b/gitnexus/test/unit/analyze-no-stats-bridge.test.ts @@ -144,7 +144,19 @@ describe('analyzeCommand commander → runFullAnalysis noStats bridge (#1477)', const opts = runFullAnalysisMock.mock.calls[0][1]; expect(opts.useParseCache).toBe(false); - expect(opts.force).toBe(true); + // Not folded into --force: runFullAnalysis names the bypass as its own + // rebuild reason (#3137), so no run announces a --force nobody passed. + expect(opts.force).toBeFalsy(); + }); + + it('passes --skills through without folding it into --force', async () => { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { skills: true }); + + const opts = runFullAnalysisMock.mock.calls[0][1]; + expect(opts.skills).toBe(true); + expect(opts.force).toBeFalsy(); }); it('reuses parser output by default', async () => { diff --git a/gitnexus/test/unit/call-summary-schema-version.test.ts b/gitnexus/test/unit/call-summary-schema-version.test.ts index e92e5db77..4bde5accd 100644 --- a/gitnexus/test/unit/call-summary-schema-version.test.ts +++ b/gitnexus/test/unit/call-summary-schema-version.test.ts @@ -24,9 +24,10 @@ * of those bumps changed NO DDL — they were semantic. A DDL digest cannot fire * on any of them. The runner-identity receipt is their only remaining cover, so * this file names that split instead of leaving it implicit: it owns the - * DDL-blind half (the fingerprint below) plus a source anchor proving - * run-analyze.ts still consults the receipt. The receipt predicate's own - * behaviour is asserted against the real function in analyzer-identity.test.ts. + * DDL-blind half (the fingerprint below). That run-analyze.ts still consults + * the receipt is pinned behaviourally in incremental-orchestration.test.ts (a + * moved receipt returns the `runner-identity` rebuild reason), and the receipt + * predicate's own behaviour in analyzer-identity.test.ts. */ import { describe, it, expect } from 'vitest'; @@ -49,8 +50,6 @@ import { const here = path.dirname(fileURLToPath(import.meta.url)); const repoRoot = path.resolve(here, '..', '..'); -const runAnalyzeSource = readFileSync(path.join(repoRoot, 'src', 'core', 'run-analyze.ts'), 'utf8'); - describe('CALL_SUMMARY relation-type exclusion (U-C1)', () => { it('is NOT in VALID_RELATION_TYPES (never enters impact symbol-space traversal)', () => { expect(VALID_RELATION_TYPES.has('CALL_SUMMARY')).toBe(false); @@ -127,21 +126,3 @@ describe('incremental reuse gate — schema fingerprint (U-C5, #2798)', () => { ); }); }); - -describe('semantic and id-shape changes ride the runner-identity receipt (#2798/#3041)', () => { - it('run-analyze.ts still forces a full rebuild when the stamped runner identity differs', () => { - // The invariant the INCREMENTAL_SCHEMA_VERSION ladder used to backstop. It - // is implicit nowhere else: no other gate observes analyzer code that emits - // no DDL. Deleting this block silently re-opens same-commit top-ups across - // an analyzer that changed how the graph is shaped. - // - // Source-anchored on purpose: the wiring has no extracted predicate to call, - // so the only way to assert the gate still exists is to read run-analyze.ts. - // The predicate's OWN behaviour — a moved build digest with unmoved DDL, an - // absent/null/legacy/malformed receipt, an alternate diagnostic entrypoint — - // is asserted against the real function in analyzer-identity.test.ts. - expect(runAnalyzeSource).toMatch( - /!analyzerRunnerIdentitiesEqual\(\s*existingMeta\.runnerIdentity,\s*runnerIdentity,?\s*\)[\s\S]{0,900}?options = \{ \.\.\.options, force: true \};/, - ); - }); -}); diff --git a/gitnexus/test/unit/cli-int-option.test.ts b/gitnexus/test/unit/cli-int-option.test.ts new file mode 100644 index 000000000..d509cd4c4 --- /dev/null +++ b/gitnexus/test/unit/cli-int-option.test.ts @@ -0,0 +1,76 @@ +/** + * Unit tests for the shared CLI integer flag parser (`src/cli/int-option.ts`). + */ +import { describe, it, expect } from 'vitest'; +import { IntegerOptionError, parseIntegerOption } from '../../src/cli/int-option.js'; + +describe('parseIntegerOption', () => { + it.each([ + ['1', 1, 1], + ['42', 1, 42], + [' 7 ', 1, 7], + ['200', 200, 200], + ['4096', 200, 4096], + ['0', 0, 0], + ['5', 0, 5], + ])('parses %j with minimum %i to %i', (value, minimum, expected) => { + expect(parseIntegerOption(value, '--flag', { minimum })).toBe(expected); + }); + + it.each(['abc', '1.5', '-5', '', ' ', '1e3', '0x10', '01'])( + 'rejects %j with the positive-integer message when the minimum is 1', + (value) => { + expect(() => parseIntegerOption(value, '--workers', { minimum: 1 })).toThrow( + new IntegerOptionError('--workers', 1, '--workers must be a positive integer'), + ); + }, + ); + + it('rejects 0 with the positive-integer message when the minimum is 1', () => { + expect(() => parseIntegerOption('0', '--retries', { minimum: 1 })).toThrow( + '--retries must be a positive integer', + ); + }); + + it.each(['abc', '1.5', '-5', '', '199', '0'])( + 'rejects %j with a message naming the flag and a minimum of 200', + (value) => { + expect(() => parseIntegerOption(value, '--memory-budget', { minimum: 200 })).toThrow( + '--memory-budget must be an integer >= 200', + ); + }, + ); + + it('rejects -1 with a message naming the flag and a minimum of 0', () => { + expect(() => parseIntegerOption('-1', '--embeddings', { minimum: 0 })).toThrow( + '--embeddings must be an integer >= 0', + ); + }); + + it('throws an IntegerOptionError carrying the flag and minimum', () => { + const thrown = (() => { + try { + parseIntegerOption('abc', '--memory-budget', { minimum: 200 }); + return undefined; + } catch (error) { + return error; + } + })(); + expect(thrown).toBeInstanceOf(IntegerOptionError); + expect(thrown).toMatchObject({ flag: '--memory-budget', minimum: 200 }); + }); + + it('rejects a value above the safe-integer bound divided by the scale', () => { + const bound = Math.floor(Number.MAX_SAFE_INTEGER / 1000); + expect(parseIntegerOption(String(bound), '--timeout', { minimum: 1, scale: 1000 })).toBe(bound); + expect(() => + parseIntegerOption(String(bound + 1), '--timeout', { minimum: 1, scale: 1000 }), + ).toThrow('--timeout is too large'); + }); + + it('rejects a value above the safe-integer bound without a scale', () => { + expect(() => parseIntegerOption('9007199254740992', '--workers', { minimum: 1 })).toThrow( + '--workers is too large', + ); + }); +}); diff --git a/gitnexus/test/unit/embedding-dims-guard.test.ts b/gitnexus/test/unit/embedding-dims-guard.test.ts index eb13177a3..918cfbd87 100644 --- a/gitnexus/test/unit/embedding-dims-guard.test.ts +++ b/gitnexus/test/unit/embedding-dims-guard.test.ts @@ -190,6 +190,7 @@ describe('run-analyze embedding-dims guard (#2798)', () => { // Pipeline actually ran (embeddingDims mismatch -> force=true), the // notice names both widths, and the rebuild stamped the live one. expect(result.alreadyUpToDate).toBeUndefined(); + expect(result.rebuildReasons).toContain('embedding-dims'); expect(logs.join('\n')).toContain( `embedding dimensions changed (index built with FLOAT[${stale}], this run embeds at ${EMBEDDING_DIMS})`, ); diff --git a/gitnexus/test/unit/incremental-orchestration.test.ts b/gitnexus/test/unit/incremental-orchestration.test.ts index 742aac8d8..4825406bb 100644 --- a/gitnexus/test/unit/incremental-orchestration.test.ts +++ b/gitnexus/test/unit/incremental-orchestration.test.ts @@ -42,6 +42,7 @@ import { stampEmbeddingCount, } from '../helpers/embedding-seed.js'; import { CLASS_FRAMEWORK_ANNOTATIONS_FEATURE } from '../../src/core/analysis-features.js'; +import { RebuildReasonCollector } from '../../src/core/rebuild-reasons.js'; import { SCHEMA_FINGERPRINT } from '../../src/core/lbug/schema.js'; import { isLanguageAvailable, @@ -84,6 +85,32 @@ const gitCommitAll = (cwd: string, message: string): void => { const SPRING_SERVICE = 'org.springframework.stereotype.Service'; +/** + * The collector's own output shapes, taken from the real formatter rather than + * re-typed: a single-reason summary, a numbered summary's header for `count` + * reasons, and the post-pipeline follow-up line. A reason with empty text + * reduces each to its fixed part. + */ +const rebuildLineShapes = (count: number) => { + const single = new RebuildReasonCollector(); + single.add({ key: 'user-force', text: '' }); + const numbered = new RebuildReasonCollector(); + const keys = ['user-force', 'skills', 'parse-cache-bypass', 'drop-embeddings'] as const; + for (const key of keys.slice(0, count)) numbered.add({ key, text: '' }); + const late = new RebuildReasonCollector(); + late.formatSummary(); + late.add({ key: 'user-force', text: '' }); + const lateNonForcing = new RebuildReasonCollector(); + lateNonForcing.formatSummary(); + lateNonForcing.add({ key: 'escalated-full-write', text: '', forcing: false }); + return { + singleSummary: single.formatSummary() ?? '', + numberedHeader: (numbered.formatSummary() ?? '').split('\n')[0], + followUp: late.formatFollowUp() ?? '', + nonForcingFollowUp: lateNonForcing.formatFollowUp() ?? '', + }; +}; + function withoutAnalysisFeature(meta: RepoMeta, featureId: string): RepoMeta { return { ...meta, @@ -602,7 +629,10 @@ describe('runFullAnalysis — incremental orchestration', () => { expect(cold.alreadyUpToDate).toBeUndefined(); expect(cold.pipelineResult?.parseCacheHitFileCount ?? 0).toBe(0); expect(cold.pipelineResult?.reparsedFileCount).toBe(7); - expect(logs.join('\n')).toContain('Parser cache bypass requested'); + // Its own reason, not a borrowed --force. + expect(cold.rebuildReasons).toEqual(['parse-cache-bypass']); + const { singleSummary } = rebuildLineShapes(1); + expect(logs.filter((m) => m.startsWith(singleSummary))).toHaveLength(1); } finally { await repo.cleanup(); } @@ -636,6 +666,7 @@ describe('runFullAnalysis — incremental orchestration', () => { { onProgress: () => {} }, ); expect(enabled.alreadyUpToDate).toBeUndefined(); + expect(enabled.rebuildReasons).toEqual(['spring-actuator']); const { storagePath } = getStoragePaths(repo.dbPath); const enabledMeta = await loadMeta(storagePath); @@ -674,9 +705,10 @@ describe('runFullAnalysis — incremental orchestration', () => { { onProgress: () => {}, onLog: (message) => disableLogs.push(message) }, ); expect(disabled.alreadyUpToDate).toBeUndefined(); - expect(disableLogs.join('\n')).toContain( - 'Spring Actuator runtime enrichment disabled; rebuilding to remove runtime evidence.', - ); + expect(disabled.rebuildReasons).toEqual(['spring-actuator']); + expect( + disableLogs.filter((m) => m.startsWith(rebuildLineShapes(1).singleSummary)), + ).toHaveLength(1); expect((await loadMeta(storagePath))?.springActuator).toEqual({ enabled: false, repoRelativeInputs: [runtimeInput], @@ -699,6 +731,7 @@ describe('runFullAnalysis — incremental orchestration', () => { { onProgress: () => {}, onLog: (message) => forceLogs.push(message) }, ); expect(forcedSteady.alreadyUpToDate).toBeUndefined(); + expect(forcedSteady.rebuildReasons).toEqual(['user-force']); expect(forceLogs.join('\n')).toContain( 'Rebuilt the graph and FTS while reusing cached parser output', ); @@ -881,6 +914,7 @@ describe('runFullAnalysis — incremental orchestration', () => { ); expect(rebuilt.alreadyUpToDate).toBeUndefined(); + expect(rebuilt.rebuildReasons).toContain('spring-vendor-prefixes'); expect(logs.join('\n')).toContain('Spring vendor mapping prefixes changed'); expect((await loadMeta(storagePath))?.springVendorPrefixes).toBe(springVendorPrefixesKey()); @@ -982,13 +1016,19 @@ describe('runFullAnalysis — incremental orchestration', () => { gitCommitAll(repo.dbPath, 'add first JVM source file'); const logs: string[] = []; - await runFullAnalysis( + const result = await runFullAnalysis( repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {}, onLog: (message) => logs.push(message) }, ); - expect(logs.join('\n')).toContain(`missing:${SPRING_BEAN_INVENTORY_FEATURE.id}`); + // Found only after the pipeline: no summary, exactly one follow-up line. + expect(result.rebuildReasons).toEqual(['analysis-features']); + const { singleSummary, followUp } = rebuildLineShapes(1); + const followUps = logs.filter((m) => m.startsWith(followUp)); + expect(followUps).toHaveLength(1); + expect(followUps[0]).toContain(`missing:${SPRING_BEAN_INVENTORY_FEATURE.id}`); + expect(logs.filter((m) => m.startsWith(singleSummary))).toEqual([]); expect(logs.join('\n')).not.toContain('Incremental:'); expect((await loadMeta(storagePath))!.analysisFeatures).toEqual({ [CLASS_FRAMEWORK_ANNOTATIONS_FEATURE.id]: CLASS_FRAMEWORK_ANNOTATIONS_FEATURE.version, @@ -1344,6 +1384,17 @@ describe('runFullAnalysis — incremental orchestration', () => { // The importer expansion fired AND the valve rerouted the write plan. expect(joined).toContain('importer(s) added to writable set'); expect(joined).toContain('switching to a full DB write'); + // Announced as the one follow-up line, carrying the escalation's own + // text, and non-forcing: the run stayed on the incremental branch + // (`incrementalStats` exists only there) and wrote the full plan. + expect(incremental.rebuildReasons).toEqual(['escalated-full-write']); + const { singleSummary, followUp, nonForcingFollowUp } = rebuildLineShapes(1); + const followUps = logs.filter((m) => m.startsWith(nonForcingFollowUp)); + expect(followUps).toHaveLength(1); + expect(followUps[0]).toContain('switching to a full DB write'); + expect(logs.filter((m) => m.startsWith(followUp))).toEqual([]); + expect(logs.filter((m) => m.startsWith(singleSummary))).toEqual([]); + expect(incremental.incrementalStats?.writeMode).toBe('full'); const { storagePath } = getStoragePaths(repo.dbPath); const escalatedMeta = await loadMeta(storagePath); @@ -1504,6 +1555,7 @@ describe('runFullAnalysis — incremental orchestration', () => { // explicitly cannot fire because the dirty-flag check rewrote // `options.force` to true. expect(recovered.alreadyUpToDate).toBeUndefined(); + expect(recovered.rebuildReasons).toEqual(['interrupted-rebuild']); const after = await loadMeta(storagePath); expect(after!.incrementalInProgress).toBeUndefined(); @@ -1515,6 +1567,95 @@ describe('runFullAnalysis — incremental orchestration', () => { } }, 300_000); + // #2798/#3041: the invariant the INCREMENTAL_SCHEMA_VERSION ladder used to + // backstop. It is implicit nowhere else: no other gate observes analyzer code + // that emits no DDL, so dropping the runner-identity reason silently re-opens + // same-commit top-ups across an analyzer that changed how the graph is + // shaped. Pinned on the returned reasons (it used to be a source regex over + // run-analyze.ts). The predicate's OWN behaviour — a moved build digest with + // unmoved DDL, an absent/null/legacy/malformed receipt, an alternate + // diagnostic entrypoint — is asserted in analyzer-identity.test.ts. + it('a stamped runner identity that differs forces a full rebuild with the runner-identity reason', 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); + if (meta?.runnerIdentity === undefined) throw new Error('first run stamped no identity'); + // Same commit, clean tree, same DDL: only the analyzer build digest moved. + const { invokedArtifact } = meta.runnerIdentity; + await saveMeta(storagePath, { + ...meta, + runnerIdentity: { + ...meta.runnerIdentity, + invokedArtifact: { ...invokedArtifact, digest: '0'.repeat(64) }, + }, + }); + + const reanalyzed = await runFullAnalysis( + repo.dbPath, + { skipAgentsMd: true }, + { onProgress: () => {} }, + ); + + expect(reanalyzed.alreadyUpToDate).toBeUndefined(); + expect(reanalyzed.rebuildReasons).toEqual(['runner-identity']); + } finally { + await repo.cleanup(); + } + }, 300_000); + + // an upgrade that trips several reasons at once names them in ONE + // numbered block and prints no other rebuild line. + it('schema, runner identity, and a new Actuator request print one numbered block of three', async () => { + const repo = await setupMiniRepo(); + const runtimeInput = 'runtime-actuator'; + try { + await mkdir(path.join(repo.dbPath, runtimeInput), { recursive: true }); + await writeFile( + path.join(repo.dbPath, runtimeInput, 'env.json'), + JSON.stringify({ propertySources: [] }), + 'utf-8', + ); + gitCommitAll(repo.dbPath, 'add actuator runtime snapshot'); + 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); + if (meta?.runnerIdentity === undefined) throw new Error('first run stamped no identity'); + const { invokedArtifact } = meta.runnerIdentity; + await saveMeta(storagePath, { + ...meta, + schemaFingerprint: 'b1c2d3e4f5a6', + runnerIdentity: { + ...meta.runnerIdentity, + invokedArtifact: { ...invokedArtifact, digest: '0'.repeat(64) }, + }, + }); + + const logs: string[] = []; + const upgraded = await runFullAnalysis( + repo.dbPath, + { skipAgentsMd: true, springActuatorPath: runtimeInput }, + { onProgress: () => {}, onLog: (message) => logs.push(message) }, + ); + + expect(upgraded.rebuildReasons).toEqual([ + 'schema-fingerprint', + 'runner-identity', + 'spring-actuator', + ]); + const { singleSummary, numberedHeader, followUp } = rebuildLineShapes(3); + const blocks = logs.filter((m) => m.startsWith(numberedHeader)); + expect(blocks).toHaveLength(1); + expect(blocks[0].split('\n')).toHaveLength(4); + expect(logs.filter((m) => m.startsWith(singleSummary) || m.startsWith(followUp))).toEqual([]); + } finally { + await repo.cleanup(); + } + }, 300_000); + // An index carrying a schema stamp that is not this build's must not take the // alreadyUpToDate fast path. The schema mismatch guard runs before lastCommit // equality can short-circuit the pipeline, so node-identity migrations receive @@ -1547,6 +1688,7 @@ describe('runFullAnalysis — incremental orchestration', () => { // Pipeline actually ran (schemaFingerprint mismatch → force=true), and the // notice names the stamp it rejected rather than a generic placeholder. expect(reanalyzed.alreadyUpToDate).toBeUndefined(); + expect(reanalyzed.rebuildReasons).toEqual(['schema-fingerprint']); expect(logs.join('\n')).toContain('index schema changed (built by b1c2d3e4f5a6,'); // And the rebuild restamped this build's digest (that path runs saveMeta). const restamped = await loadMeta(storagePath); @@ -1556,6 +1698,40 @@ describe('runFullAnalysis — incremental orchestration', () => { } }, 300_000); + // A recorded graph-write collapse at an unchanged commit must not take the + // alreadyUpToDate fast path: the gate forces a full rebuild and names it. + it('a recorded graphWriteCollapsed stamp forces a full rebuild on an unchanged-commit re-analyze', 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?.graphWriteCollapsed).toBeUndefined(); + expect(meta).toBeTruthy(); + + // Same commit, clean tree, same DDL: only the collapse stamp differs. + const collapsed: RepoMeta = { + ...(meta as RepoMeta), + graphWriteCollapsed: { expected: 500, persisted: 3 }, + }; + await saveMeta(storagePath, collapsed); + + const logs: string[] = []; + const reanalyzed = await runFullAnalysis( + repo.dbPath, + { skipAgentsMd: true }, + { onProgress: () => {}, onLog: (message) => logs.push(message) }, + ); + + expect(reanalyzed.alreadyUpToDate).toBeUndefined(); + expect(reanalyzed.rebuildReasons).toContain('graph-write-collapse'); + expect(logs.join('\n')).toContain('previous run persisted 3 of 500 expected relationships'); + } finally { + await repo.cleanup(); + } + }, 300_000); + // #2331/#2339: mirrors the schema-fingerprint mismatch test above, but for // the CJK segmentation mode stamp. Uses a non-default mode ('bigram') rather // than 'none' — with the default, (undefined ?? 'none') !== 'none' is @@ -1585,6 +1761,7 @@ describe('runFullAnalysis — incremental orchestration', () => { ); // Pipeline actually ran (cjkSegmentation mismatch → force=true). expect(reanalyzed.alreadyUpToDate).toBeUndefined(); + expect(reanalyzed.rebuildReasons).toContain('cjk-segmentation'); // And the meta is restamped to the live resolved mode. const restamped = await loadMeta(storagePath); expect(restamped!.cjkSegmentation).toBe('bigram'); @@ -2082,9 +2259,10 @@ describe('runFullAnalysis — AsyncAPI document reading', () => { { onProgress: () => {}, onLog: (message) => enabledLogs.push(message) }, ); expect(enabled.alreadyUpToDate).toBeUndefined(); - expect(enabledLogs.join('\n')).toContain( - 'AsyncAPI document reading requested; forcing a full rebuild.', - ); + expect(enabled.rebuildReasons).toEqual(['asyncapi']); + expect( + enabledLogs.filter((m) => m.startsWith(rebuildLineShapes(1).singleSummary)), + ).toHaveLength(1); // The forward into PipelineOptions is what puts this node in the graph; // without it the flag parses and nothing else happens. expect(await readDocumentDestinations(repo.dbPath)).toEqual([ @@ -2113,9 +2291,10 @@ describe('runFullAnalysis — AsyncAPI document reading', () => { { onProgress: () => {}, onLog: (message) => disableLogs.push(message) }, ); expect(disabled.alreadyUpToDate).toBeUndefined(); - expect(disableLogs.join('\n')).toContain( - 'AsyncAPI document reading disabled; rebuilding to remove document-derived evidence.', - ); + expect(disabled.rebuildReasons).toEqual(['asyncapi']); + expect( + disableLogs.filter((m) => m.startsWith(rebuildLineShapes(1).singleSummary)), + ).toHaveLength(1); expect(await readDocumentDestinations(repo.dbPath)).toEqual([]); expect((await loadMeta(storagePath))?.asyncApiSpec).toBeUndefined(); diff --git a/gitnexus/test/unit/parse-impl-heap-guard.test.ts b/gitnexus/test/unit/parse-impl-heap-guard.test.ts index 89a535d41..64a0ae8c3 100644 --- a/gitnexus/test/unit/parse-impl-heap-guard.test.ts +++ b/gitnexus/test/unit/parse-impl-heap-guard.test.ts @@ -9,10 +9,10 @@ vi.mock('os', async () => { }); import { - heapPressureRemedy, projectParseHeapNeedBytes, shouldAbortForHeapPressure, } from '../../src/core/ingestion/pipeline-phases/parse-impl.js'; +import { heapPressureRemedy } from '../../src/core/ingestion/utils/effective-ram.js'; const GB = 1024 * 1024 * 1024; @@ -27,11 +27,14 @@ const setConstrainedMemory = (value: number): (() => void) => { describe('#2649 parse-phase heap guardrails', () => { let initialGuard: string | undefined; + let initialSource: string | undefined; let restoreConstrained: (() => void) | undefined; beforeEach(() => { initialGuard = process.env.GITNEXUS_MEMORY; + initialSource = process.env.GITNEXUS_HEAP_LIMIT_SOURCE; delete process.env.GITNEXUS_MEMORY; + delete process.env.GITNEXUS_HEAP_LIMIT_SOURCE; // Unconstrained by default so the mocked 32GB totalmem governs. restoreConstrained = setConstrainedMemory(0); }); @@ -39,6 +42,8 @@ describe('#2649 parse-phase heap guardrails', () => { afterEach(() => { if (initialGuard === undefined) delete process.env.GITNEXUS_MEMORY; else process.env.GITNEXUS_MEMORY = initialGuard; + if (initialSource === undefined) delete process.env.GITNEXUS_HEAP_LIMIT_SOURCE; + else process.env.GITNEXUS_HEAP_LIMIT_SOURCE = initialSource; restoreConstrained?.(); restoreConstrained = undefined; }); @@ -84,4 +89,38 @@ describe('#2649 parse-phase heap guardrails', () => { restoreConstrained = setConstrainedMemory(8 * GB); expect(heapPressureRemedy(6.5 * GB)).toContain('.gitnexusignore'); }); + + it('a --memory-budget heap is never told to drop a --max-old-space-size pin (#3137)', () => { + // 4GB budget on a 32GB machine: the budget, not a pin, set the limit. + process.env.GITNEXUS_HEAP_LIMIT_SOURCE = 'budget'; + const remedy = heapPressureRemedy(4 * GB); + expect({ + mentionsPin: remedy.includes('--max-old-space-size'), + mentionsBudgetFlag: remedy.includes('--memory-budget'), + }).toEqual({ mentionsPin: false, mentionsBudgetFlag: true }); + }); + + it('GITNEXUS_MEMORY=off without a pin is not told to drop a --max-old-space-size pin', () => { + process.env.GITNEXUS_MEMORY = 'off'; + const remedy = heapPressureRemedy(4 * GB); + expect({ + mentionsPin: remedy.includes('--max-old-space-size'), + mentionsMemoryOff: remedy.includes('GITNEXUS_MEMORY=off'), + }).toEqual({ mentionsPin: false, mentionsMemoryOff: true }); + }); + + it('measures pressure against the auto-sized cap, not a flat 0.75 × RAM', () => { + // 7500MB cgroup: the auto cap is 6000MB (0.80 × RAM), so a 5300MB pin is + // below 90% of it and still gets the drop-the-pin advice. + restoreConstrained?.(); + restoreConstrained = setConstrainedMemory(7500 * 1024 * 1024); + expect(heapPressureRemedy(5300 * 1024 * 1024)).toContain('--max-old-space-size'); + }); + + it('a --memory-budget heap at the machine ceiling gets the scope-or-hardware advice', () => { + process.env.GITNEXUS_HEAP_LIMIT_SOURCE = 'budget'; + const budgetRemedy = heapPressureRemedy(23 * GB); + delete process.env.GITNEXUS_HEAP_LIMIT_SOURCE; + expect(budgetRemedy).toBe(heapPressureRemedy(23 * GB)); + }); }); diff --git a/gitnexus/test/unit/pdg-mode-flip.test.ts b/gitnexus/test/unit/pdg-mode-flip.test.ts index 888f5de09..fb3b398b6 100644 --- a/gitnexus/test/unit/pdg-mode-flip.test.ts +++ b/gitnexus/test/unit/pdg-mode-flip.test.ts @@ -305,6 +305,7 @@ describe('runFullAnalysis — pdg-mode flip (#2099 F1)', () => { logs.length = 0; const flipOn = await runFullAnalysis(repo.dbPath, { skipAgentsMd: true, pdg: true }, cb); expect(flipOn.alreadyUpToDate).toBeUndefined(); + expect(flipOn.rebuildReasons).toContain('pdg-mode'); expect(logs.some((m) => m.includes('pdg mode changed'))).toBe(true); expect(await countBasicBlocks(repo.dbPath)).toBeGreaterThan(0); const stamped = await loadMeta(storagePath); @@ -336,6 +337,7 @@ describe('runFullAnalysis — pdg-mode flip (#2099 F1)', () => { logs.length = 0; const flipOff = await runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, cb); expect(flipOff.alreadyUpToDate).toBeUndefined(); + expect(flipOff.rebuildReasons).toContain('pdg-mode'); expect(logs.some((m) => m.includes('pdg mode changed'))).toBe(true); expect(await countBasicBlocks(repo.dbPath)).toBe(0); expect((await loadMeta(storagePath))!.pdg).toBeUndefined(); @@ -365,6 +367,7 @@ describe('runFullAnalysis — pdg-mode flip (#2099 F1)', () => { cb, ); expect(capChange.alreadyUpToDate).toBeUndefined(); + expect(capChange.rebuildReasons).toContain('pdg-mode'); expect(logs.some((m) => m.includes('different caps'))).toBe(true); expect((await loadMeta(storagePath))!.pdg).toEqual({ maxFunctionLines: 2000, diff --git a/gitnexus/test/unit/rebuild-reasons.test.ts b/gitnexus/test/unit/rebuild-reasons.test.ts new file mode 100644 index 000000000..5f6848efc --- /dev/null +++ b/gitnexus/test/unit/rebuild-reasons.test.ts @@ -0,0 +1,327 @@ +import { existsSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; +import { + REBUILD_REASON_KEYS, + RebuildReasonCollector, + readStoredRebuildReasons, + type RebuildReason, + type RebuildReasonKey, +} from '../../src/core/rebuild-reasons.js'; + +const schema: RebuildReason = { key: 'schema-fingerprint', text: 'index schema changed (A → B)' }; +const runner: RebuildReason = { key: 'runner-identity', text: 'analyzer runner identity changed' }; +const actuator: RebuildReason = { + key: 'spring-actuator', + text: 'Spring Actuator runtime enrichment requested', +}; +const escalation: RebuildReason = { + key: 'escalated-full-write', + text: 'incremental write escalated to a full write; re-run with --force if it fails', + forcing: false, +}; + +describe('RebuildReasonCollector summary', () => { + it('formats a single reason inline', () => { + const collector = new RebuildReasonCollector(); + collector.add(schema); + expect(collector.formatSummary()).toBe(`Full rebuild required: ${schema.text}`); + }); + + it('formats three reasons as one numbered block', () => { + const collector = new RebuildReasonCollector(); + collector.add(schema); + collector.add(runner); + collector.add(actuator); + expect(collector.formatSummary()).toBe( + 'Full rebuild required (3 reasons):\n' + + ` 1. ${schema.text}\n` + + ` 2. ${runner.text}\n` + + ` 3. ${actuator.text}`, + ); + expect(collector.keys()).toEqual(['schema-fingerprint', 'runner-identity', 'spring-actuator']); + }); + + it('reports not forced and formats nothing when empty', () => { + const collector = new RebuildReasonCollector(); + expect(collector.forced).toBe(false); + expect(collector.keys()).toEqual([]); + expect(collector.formatSummary()).toBeUndefined(); + expect(collector.formatFollowUp()).toBeUndefined(); + }); + + it('merges the same key into one entry with the newest text, keeping its position', () => { + const collector = new RebuildReasonCollector(); + collector.add({ key: 'content-retention', text: 'repair-fts retention force' }); + collector.add(schema); + collector.add({ key: 'content-retention', text: 'content retention changed' }); + expect(collector.reasons()).toEqual([ + { key: 'content-retention', text: 'content retention changed' }, + schema, + ]); + }); + + it('reports forced once any forcing reason is present', () => { + const collector = new RebuildReasonCollector(); + collector.add(runner); + expect(collector.forced).toBe(true); + }); + + it('does not claim a full rebuild in a follow-up that carries only non-forcing reasons', () => { + const collector = new RebuildReasonCollector(); + collector.formatSummary(); + collector.add(escalation); + const followUp = collector.formatFollowUp() ?? ''; + expect(followUp).toContain(escalation.text); + expect(followUp).not.toMatch(/full rebuild/i); + }); + + it('lists a non-forcing reason without reporting forced', () => { + const collector = new RebuildReasonCollector(); + collector.add(escalation); + expect(collector.forced).toBe(false); + expect(collector.keys()).toEqual(['escalated-full-write']); + const summary = collector.formatSummary() ?? ''; + expect(summary).toContain(escalation.text); + expect(summary).not.toMatch(/full rebuild/i); + }); +}); + +describe('RebuildReasonCollector follow-up', () => { + it('formats two late reasons as one line covering only the unannounced ones', () => { + const collector = new RebuildReasonCollector(); + collector.add(schema); + collector.formatSummary(); + collector.add({ key: 'analysis-features', text: 'analysis capabilities changed' }); + collector.add(escalation); + const followUp = collector.formatFollowUp(); + expect(followUp?.split('\n')).toHaveLength(1); + expect(followUp).toContain('analysis capabilities changed'); + expect(followUp).toContain(escalation.text); + expect(followUp).not.toContain(schema.text); + }); + + it('keeps a follow-up on one line when a late reason text spans lines', () => { + const collector = new RebuildReasonCollector(); + collector.formatSummary(); + collector.add({ key: 'analysis-features', text: 'first line\nsecond line' }); + expect(collector.formatFollowUp()?.split('\n')).toHaveLength(1); + }); + + it('formats nothing when no reason arrived after the summary', () => { + const collector = new RebuildReasonCollector(); + collector.add(schema); + collector.formatSummary(); + expect(collector.formatFollowUp()).toBeUndefined(); + }); + + it('does not repeat a follow-up once it was announced', () => { + const collector = new RebuildReasonCollector(); + collector.formatSummary(); + collector.add(escalation); + collector.formatFollowUp(); + expect(collector.formatFollowUp()).toBeUndefined(); + }); + + it('treats a late re-add of an announced key as already announced', () => { + const collector = new RebuildReasonCollector(); + collector.add(schema); + collector.formatSummary(); + collector.add({ key: 'schema-fingerprint', text: 'index schema changed again' }); + expect(collector.formatFollowUp()).toBeUndefined(); + }); +}); + +describe('RebuildReasonCollector interrupted-rebuild recovery', () => { + it('names the interrupted rebuild and its stored reason in one forcing entry', () => { + const collector = new RebuildReasonCollector(); + collector.recordInterruptedRebuild([schema], 'phase=load-graph'); + expect(collector.keys()).toEqual(['interrupted-rebuild']); + expect(collector.forced).toBe(true); + const text = collector.reasons()[0].text; + expect(text).toContain('did not complete cleanly'); + expect(text).toContain('phase=load-graph'); + expect(text).toContain(schema.text); + }); + + it('merges a re-detected reason with a matching key into the entry', () => { + const collector = new RebuildReasonCollector(); + collector.recordInterruptedRebuild([schema]); + collector.add({ key: 'schema-fingerprint', text: 'index schema changed (B → C)' }); + collector.add(runner); + const summary = collector.formatSummary() ?? ''; + expect(collector.keys()).toEqual(['interrupted-rebuild', 'runner-identity']); + expect(summary.split('index schema changed')).toHaveLength(2); + expect(summary).toContain('index schema changed (B → C)'); + expect(summary).not.toContain(schema.text); + }); + + it('does not re-announce the interrupted-rebuild entry when a matching reason merges after the summary', () => { + const collector = new RebuildReasonCollector(); + collector.recordInterruptedRebuild([schema]); + collector.formatSummary(); + collector.add({ key: 'schema-fingerprint', text: 'index schema changed (B → C)' }); + expect(collector.formatFollowUp()).toBeUndefined(); + expect(collector.toStored()).toEqual([ + { key: 'schema-fingerprint', text: 'index schema changed (B → C)' }, + ]); + }); + + it('absorbs a matching reason that was added before the recovery entry', () => { + const collector = new RebuildReasonCollector(); + collector.add({ key: 'schema-fingerprint', text: 'index schema changed (B → C)' }); + collector.recordInterruptedRebuild([schema]); + expect(collector.keys()).toEqual(['interrupted-rebuild']); + expect(collector.reasons()[0].text).toContain('index schema changed (B → C)'); + }); + + it('flattens stored reasons that already contain an interrupted-rebuild entry', () => { + const collector = new RebuildReasonCollector(); + collector.recordInterruptedRebuild([ + { key: 'interrupted-rebuild', text: 'Previous analyze run did not complete cleanly' }, + schema, + ]); + const text = collector.reasons()[0].text; + expect(text.split('did not complete cleanly')).toHaveLength(2); + expect(collector.toStored()).toEqual([schema]); + }); + + it('describes a marker with no stored reasons as having no reasons recorded', () => { + const collector = new RebuildReasonCollector(); + collector.recordInterruptedRebuild([]); + expect(collector.reasons()[0].text).toContain('no reasons recorded'); + expect(collector.forced).toBe(true); + }); + + it('numbers each stored cause inside a multi-cause interrupted-rebuild entry', () => { + const collector = new RebuildReasonCollector(); + collector.recordInterruptedRebuild([schema, { key: 'future-key', text: 'from a newer build' }]); + expect(collector.reasons()[0].text).toContain(`(1) ${schema.text}; (2) from a newer build`); + }); + + it('persists the interrupted-rebuild reasons before reasons collected earlier in this run', () => { + const collector = new RebuildReasonCollector(); + collector.add({ key: 'user-force', text: 'forced' }); + collector.recordInterruptedRebuild([schema]); + expect(collector.toStored().map((r) => r.key)).toEqual(['schema-fingerprint', 'user-force']); + }); + + it('persists the flattened, merged reasons with no interrupted-rebuild entry', () => { + const collector = new RebuildReasonCollector(); + collector.recordInterruptedRebuild([schema, { key: 'future-key', text: 'from a newer build' }]); + collector.add({ key: 'schema-fingerprint', text: 'index schema changed (B → C)' }); + collector.add(escalation); + expect(collector.toStored()).toEqual([ + { key: 'schema-fingerprint', text: 'index schema changed (B → C)' }, + { key: 'future-key', text: 'from a newer build' }, + { key: escalation.key, text: escalation.text }, + ]); + }); + + it('keeps flattening across two consecutive crashes', () => { + const first = new RebuildReasonCollector(); + first.recordInterruptedRebuild([schema]); + first.add(runner); + const second = new RebuildReasonCollector(); + second.recordInterruptedRebuild(readStoredRebuildReasons(first.toStored())); + expect(second.toStored()).toEqual([schema, runner]); + expect(second.reasons()[0].text.split('did not complete cleanly')).toHaveLength(2); + }); +}); + +describe('readStoredRebuildReasons', () => { + it('keeps entries with string key and text, including unknown keys', () => { + expect( + readStoredRebuildReasons([ + schema, + { key: 'future-key', text: 'from a newer build' }, + { key: 7, text: 'bad key' }, + { key: 'runner-identity' }, + null, + 'loose string', + ]), + ).toEqual([ + { key: 'schema-fingerprint', text: schema.text }, + { key: 'future-key', text: 'from a newer build' }, + ]); + }); + + it('drops extra fields so the result is the serializable shape', () => { + expect(readStoredRebuildReasons([{ ...escalation, extra: 1 }])).toEqual([ + { key: escalation.key, text: escalation.text }, + ]); + }); + + it.each([ + ['undefined', undefined], + ['a boolean legacy marker', true], + ['an object', { key: 'schema-fingerprint', text: 'x' }], + ['a string', 'schema-fingerprint'], + ])('reads %s as no reasons recorded', (_label, value) => { + const stored = readStoredRebuildReasons(value); + expect(stored).toEqual([]); + const collector = new RebuildReasonCollector(); + collector.recordInterruptedRebuild(stored); + expect(collector.reasons()[0].text).toContain('no reasons recorded'); + }); + + it('keeps an unknown stored key text inside the recovery entry', () => { + const collector = new RebuildReasonCollector(); + collector.recordInterruptedRebuild( + readStoredRebuildReasons([{ key: 'future-key', text: 'from a newer build' }]), + ); + expect(collector.reasons()[0].text).toContain('from a newer build'); + }); +}); + +/** + * R15 coverage table: every `RebuildReasonKey` paired with the test file that + * drives it through `runFullAnalysis` and asserts the key. The exhaustiveness + * check below fails `tsc` when a key is added to the union without a row here. + */ +const REBUILD_REASON_COVERAGE = [ + { key: 'user-force', file: 'test/unit/incremental-orchestration.test.ts' }, + { key: 'skills', file: 'test/unit/stream-graph-emit-force-ordering.test.ts' }, + { key: 'parse-cache-bypass', file: 'test/unit/incremental-orchestration.test.ts' }, + { key: 'drop-embeddings', file: 'test/unit/stream-graph-emit-force-ordering.test.ts' }, + { key: 'interrupted-rebuild', file: 'test/unit/incremental-orchestration.test.ts' }, + { key: 'private-graph-unavailable', file: 'test/integration/shared-store-analyze.test.ts' }, + { key: 'shared-store-missing-graph', file: 'test/integration/shared-store-analyze.test.ts' }, + { + key: 'content-retention', + file: 'test/integration/external-storage-content-retention.test.ts', + }, + { key: 'pdg-mode', file: 'test/unit/pdg-mode-flip.test.ts' }, + { key: 'schema-fingerprint', file: 'test/unit/incremental-orchestration.test.ts' }, + { key: 'graph-write-collapse', file: 'test/unit/incremental-orchestration.test.ts' }, + { key: 'analysis-features', file: 'test/unit/incremental-orchestration.test.ts' }, + { key: 'spring-vendor-prefixes', file: 'test/unit/incremental-orchestration.test.ts' }, + { key: 'runner-identity', file: 'test/unit/incremental-orchestration.test.ts' }, + { key: 'cjk-segmentation', file: 'test/unit/incremental-orchestration.test.ts' }, + { key: 'embedding-dims', file: 'test/unit/embedding-dims-guard.test.ts' }, + { key: 'spring-actuator', file: 'test/unit/incremental-orchestration.test.ts' }, + { key: 'asyncapi', file: 'test/unit/incremental-orchestration.test.ts' }, + { key: 'escalated-full-write', file: 'test/unit/incremental-orchestration.test.ts' }, +] as const satisfies readonly { readonly key: RebuildReasonKey; readonly file: string }[]; + +const packageRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..'); + +describe('rebuild reason coverage table (R15)', () => { + it('lists every key exactly once', () => { + const keys: readonly RebuildReasonKey[] = REBUILD_REASON_COVERAGE.map(({ key }) => key); + // Runtime check against the exported key list: CI does not type-check + // test files, so a compile-time-only gate would never fire. + expect([...keys].sort()).toEqual([...REBUILD_REASON_KEYS].sort()); + expect(new Set(keys).size).toBe(keys.length); + }); + + // The runtime check only proves each named driver file exists; the + // behavior proof is the driver test itself, which runs runFullAnalysis and + // asserts the returned key. Grepping the file for the key would pass on a + // stale string, so it is deliberately not asserted here. + it.each(REBUILD_REASON_COVERAGE)('$key is driven by $file', ({ file }) => { + const absolute = path.join(packageRoot, file); + expect(existsSync(absolute)).toBe(true); + }); +}); diff --git a/gitnexus/test/unit/run-analyze-fts-crash-marker.test.ts b/gitnexus/test/unit/run-analyze-fts-crash-marker.test.ts index 7164caada..6997a0604 100644 --- a/gitnexus/test/unit/run-analyze-fts-crash-marker.test.ts +++ b/gitnexus/test/unit/run-analyze-fts-crash-marker.test.ts @@ -28,6 +28,7 @@ import { PROCESS_DETECTION_ENV, } from '../../src/core/ingestion/process-detection-budget.js'; import { getSearchFTSCjkSegmentation } from '../../src/core/search/cjk-segmentation.js'; +import { RebuildReasonCollector } from '../../src/core/rebuild-reasons.js'; import { FTS_DIRTY_PHASE, allowsFtsCrashWalPark, @@ -296,6 +297,27 @@ describe('FTS crash-marker policy (characterization)', () => { }); }); + it('carries the prior marker rebuild reasons through the FTS stamp', () => { + const reasons = [{ key: 'schema-fingerprint', text: 'schema moved' }]; + const stamp = buildFtsDirtyStamp({ + prior: { startedAt: 7, toWriteCount: 0, phase: 'full-rebuild', reasons }, + writePlan: 'in-place', + checkpointSucceeded: false, + now: 42, + }); + expect(stamp).toMatchObject({ startedAt: 7, phase: FTS_DIRTY_PHASE, reasons }); + }); + + it('omits reasons from the FTS stamp when the prior marker has none', () => { + const stamp = buildFtsDirtyStamp({ + prior: { startedAt: 7, toWriteCount: 0, phase: 'pre-write' }, + writePlan: 'in-place', + checkpointSucceeded: true, + now: 42, + }); + expect(Object.keys(stamp)).not.toContain('reasons'); + }); + it('stamps after the escalation valve in source order', () => { const src = readFileSync(RUN_ANALYZE_SRC, 'utf8'); const valve = src.indexOf("saveIncrementalDirtyState('escalated-full-write'"); @@ -1449,3 +1471,403 @@ describe('runFullAnalysis FTS crash marker', () => { } }); }); + +/** + * #3137: the collected rebuild reasons ride on every + * `incrementalInProgress` writer, so an interrupted rebuild can name its + * causes, and every clearing path drops them with the marker. + */ +describe('runFullAnalysis rebuild reasons on the crash marker', () => { + type MarkerWrite = { dir: string; marker: RepoMeta['incrementalInProgress'] }; + const FOREIGN_SCHEMA = 'b1c2d3e4f5a6'; + const BUMPED_SCHEMA = 'c2d3e4f5a6b7'; + + beforeEach(() => { + for (const key of Object.values(PROCESS_DETECTION_ENV)) { + vi.stubEnv(key, undefined); + } + // FTS is mocked here, so the FTS phase (and its stamp) must run. + vi.stubEnv('GITNEXUS_SKIP_FTS', undefined); + }); + afterEach(() => { + vi.doUnmock('../../src/core/lbug/lbug-adapter.js'); + vi.doUnmock('../../src/core/search/fts-indexes.js'); + vi.doUnmock('../../src/core/ingestion/pipeline.js'); + vi.doUnmock('../../src/storage/repo-manager.js'); + vi.doUnmock('../../src/core/incremental/escalation-gate.js'); + vi.restoreAllMocks(); + vi.resetModules(); + vi.clearAllMocks(); + vi.unstubAllEnvs(); + }); + + /** Mocks the storage and pipeline edges; records every marker write with its target dir. */ + const mockRun = ({ + buildSearchIndexes = vi.fn(async () => ({ ok: true })), + inPlaceEscalation = false, + } = {}) => { + const writes: MarkerWrite[] = []; + const wipeLbugDbFiles = vi.fn(async () => undefined); + vi.doMock('../../src/core/lbug/wal-checkpoint-driver.js', async (importActual) => ({ + ...(await importActual()), + checkpointOnce: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/lbug-adapter.js', async () => { + const adapter = await mockLbugAdapter(); + // An unreadable catalog keeps a size escalation in place (no staging + // swap), which is the only POSIX route to an in-place FTS stamp that + // carries a reason. + const catalog = [[], adapter.INDEX_CATALOG_UNREADABLE][Number(inPlaceEscalation)]; + return { + ...adapter, + wipeLbugDbFiles, + readIndexCatalogSnapshot: vi.fn(async () => catalog), + }; + }); + vi.doMock('../../src/core/incremental/escalation-gate.js', async (importActual) => ({ + ...(await importActual()), + shouldEscalateIncrementalWrite: () => inPlaceEscalation, + })); + vi.doMock('../../src/core/search/fts-indexes.js', async (importActual) => ({ + ...(await importActual()), + initialiseSearchFTSStemmer: vi.fn(() => 'porter'), + missingSearchFTSIndexTables: vi.fn(async () => []), + dropSearchFTSIndexes: vi.fn(async () => undefined), + buildSearchIndexesOrDegrade: buildSearchIndexes, + })); + vi.doMock('../../src/core/ingestion/pipeline.js', () => ({ + runPipelineFromRepo: vi.fn(async (repoPath: string) => ({ repoPath, graph: fileGraph() })), + })); + vi.doMock('../../src/storage/repo-manager.js', async (importActual) => { + const actual = await importActual(); + return { + ...actual, + saveMeta: async (...args: Parameters) => { + writes.push({ dir: args[0], marker: args[1].incrementalInProgress }); + return actual.saveMeta(...args); + }, + }; + }); + return { writes, wipeLbugDbFiles }; + }; + + const markersInPhase = (writes: MarkerWrite[], phase: string) => + writes.filter((write) => write.marker?.phase === phase); + + /** A fully indexed meta at HEAD whose schema stamp is not this build's. */ + const schemaForeignMeta = (repoPath: string, schemaFingerprint: string): RepoMeta => ({ + ...incrementalMeta(repoPath), + lastCommit: headCommit(repoPath), + schemaFingerprint, + }); + + const summaryPrefix = (): string => { + const shape = new RebuildReasonCollector(); + shape.add({ key: 'user-force', text: '' }); + return shape.formatSummary() ?? ''; + }; + + const occurrences = (haystack: string, needle: string): number => + haystack.split(needle).length - 1; + + it('stores the schema reason on the full-rebuild stamp and clears it on success', async () => { + const { writes } = mockRun(); + const tmpRepo = await createTempDir('gitnexus-reasons-full-stamp-'); + try { + await seedGitFile(tmpRepo.dbPath); + const { storagePath, lbugPath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + await saveMeta(storagePath, schemaForeignMeta(tmpRepo.dbPath, FOREIGN_SCHEMA)); + await createPlaceholderGraphStore(lbugPath); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis( + tmpRepo.dbPath, + { skipAgentsMd: true, skipSkills: true }, + { onProgress: () => {}, onLog: () => {} }, + ); + + expect(result.rebuildReasons).toEqual(['schema-fingerprint']); + const stamps = markersInPhase(writes, 'full-rebuild'); + expect(stamps).toHaveLength(1); + expect(stamps[0].marker?.reasons?.map((reason) => reason.key)).toEqual([ + 'schema-fingerprint', + ]); + const finalMeta = await loadMeta(storagePath); + expect(finalMeta?.incrementalInProgress).toBeUndefined(); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('a schema rebuild killed at the wipe names the interrupted rebuild and the schema change once', async () => { + const { wipeLbugDbFiles } = mockRun(); + wipeLbugDbFiles.mockRejectedValueOnce(new Error('simulated kill at the wipe')); + const tmpRepo = await createTempDir('gitnexus-reasons-ae5-'); + try { + await seedGitFile(tmpRepo.dbPath); + const { storagePath, lbugPath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + await saveMeta(storagePath, schemaForeignMeta(tmpRepo.dbPath, FOREIGN_SCHEMA)); + await createPlaceholderGraphStore(lbugPath); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + await expect( + runFullAnalysis( + tmpRepo.dbPath, + { skipAgentsMd: true, skipSkills: true }, + { onProgress: () => {}, onLog: () => {} }, + ), + ).rejects.toThrow('simulated kill at the wipe'); + const crashed = await loadMeta(storagePath); + expect(crashed?.incrementalInProgress?.phase).toBe('full-rebuild'); + const stored = crashed?.incrementalInProgress?.reasons ?? []; + expect(stored.map((reason) => reason.key)).toEqual(['schema-fingerprint']); + + const logs: string[] = []; + const recovered = await runFullAnalysis( + tmpRepo.dbPath, + { skipAgentsMd: true, skipSkills: true }, + { onProgress: () => {}, onLog: (message) => logs.push(message) }, + ); + + expect(recovered.rebuildReasons).toEqual(['interrupted-rebuild']); + const summaries = logs.filter((message) => message.startsWith(summaryPrefix())); + expect(summaries).toHaveLength(1); + expect(occurrences(summaries[0], stored[0].text)).toBe(1); + expect(occurrences(logs.join('\n'), stored[0].text)).toBe(1); + const finalMeta = await loadMeta(storagePath); + expect(finalMeta?.incrementalInProgress).toBeUndefined(); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('two consecutive crashes across a schema bump store flat reasons with no nested entry', async () => { + const { wipeLbugDbFiles } = mockRun(); + wipeLbugDbFiles + .mockRejectedValueOnce(new Error('first kill')) + .mockRejectedValueOnce(new Error('second kill')); + const tmpRepo = await createTempDir('gitnexus-reasons-double-crash-'); + try { + await seedGitFile(tmpRepo.dbPath); + const { storagePath, lbugPath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + await saveMeta(storagePath, schemaForeignMeta(tmpRepo.dbPath, FOREIGN_SCHEMA)); + await createPlaceholderGraphStore(lbugPath); + const options = { skipAgentsMd: true, skipSkills: true }; + const callbacks = { onProgress: () => {}, onLog: () => {} }; + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + await expect(runFullAnalysis(tmpRepo.dbPath, options, callbacks)).rejects.toThrow( + 'first kill', + ); + const first = await loadMeta(storagePath); + expect(first).toBeTruthy(); + await saveMeta(storagePath, { ...(first as RepoMeta), schemaFingerprint: BUMPED_SCHEMA }); + await expect(runFullAnalysis(tmpRepo.dbPath, options, callbacks)).rejects.toThrow( + 'second kill', + ); + + const second = await loadMeta(storagePath); + const firstReasons = first?.incrementalInProgress?.reasons ?? []; + const secondReasons = second?.incrementalInProgress?.reasons ?? []; + expect(secondReasons.map((reason) => reason.key)).toEqual(['schema-fingerprint']); + expect(secondReasons[0].text).toContain(BUMPED_SCHEMA); + expect(secondReasons[0].text).not.toBe(firstReasons[0].text); + + const recovered = await runFullAnalysis(tmpRepo.dbPath, options, callbacks); + expect(recovered.rebuildReasons).toEqual(['interrupted-rebuild']); + expect((await loadMeta(storagePath))?.incrementalInProgress).toBeUndefined(); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('keeps the escalation reason on the escalated-full-write stamp and through the FTS stamp', async () => { + const { writes, wipeLbugDbFiles } = mockRun({ inPlaceEscalation: true }); + wipeLbugDbFiles.mockRejectedValueOnce(new Error('simulated kill in the escalated wipe')); + const tmpRepo = await createTempDir('gitnexus-reasons-escalated-'); + try { + await seedGitFile(tmpRepo.dbPath); + const { storagePath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + await saveMeta(storagePath, incrementalMeta(tmpRepo.dbPath)); + const options = { skipAgentsMd: true, skipSkills: true }; + const callbacks = { onProgress: () => {}, onLog: () => {} }; + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + await expect(runFullAnalysis(tmpRepo.dbPath, options, callbacks)).rejects.toThrow( + 'simulated kill in the escalated wipe', + ); + const crashed = await loadMeta(storagePath); + expect(crashed?.incrementalInProgress?.phase).toBe('escalated-full-write'); + expect(crashed?.incrementalInProgress?.reasons?.map((reason) => reason.key)).toEqual([ + 'escalated-full-write', + ]); + expect(markersInPhase(writes, 'pre-write')[0].marker?.reasons).toBeUndefined(); + + // A clean rerun with a live escalation hands the reason to the FTS stamp. + await saveMeta(storagePath, incrementalMeta(tmpRepo.dbPath)); + writes.length = 0; + await runFullAnalysis(tmpRepo.dbPath, options, callbacks); + const escalated = markersInPhase(writes, 'escalated-full-write'); + const ftsStamps = markersInPhase(writes, FTS_DIRTY_PHASE); + expect(ftsStamps).toHaveLength(1); + expect(ftsStamps[0].marker?.reasons).toEqual(escalated[0].marker?.reasons); + expect((await loadMeta(storagePath))?.incrementalInProgress).toBeUndefined(); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('an FTS-phase crash keeps the reasons the escalated write stored', async () => { + const buildSearchIndexes = vi.fn(async () => { + throw new Error('simulated kill in CREATE_FTS_INDEX'); + }); + mockRun({ buildSearchIndexes, inPlaceEscalation: true }); + const tmpRepo = await createTempDir('gitnexus-reasons-fts-crash-'); + try { + await seedGitFile(tmpRepo.dbPath); + const { storagePath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + await saveMeta(storagePath, incrementalMeta(tmpRepo.dbPath)); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + await expect( + runFullAnalysis( + tmpRepo.dbPath, + { skipAgentsMd: true, skipSkills: true }, + { onProgress: () => {}, onLog: () => {} }, + ), + ).rejects.toThrow('simulated kill in CREATE_FTS_INDEX'); + + const crashed = await loadMeta(storagePath); + expect(crashed?.incrementalInProgress?.phase).toBe(FTS_DIRTY_PHASE); + expect(crashed?.incrementalInProgress?.reasons?.map((reason) => reason.key)).toEqual([ + 'escalated-full-write', + ]); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('under --branch the reasons land only in the branch slot, and success clears them', async () => { + const { writes } = mockRun(); + const tmpRepo = await createTempDir('gitnexus-reasons-branch-'); + try { + await seedGitFile(tmpRepo.dbPath); + const flat = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(flat.storagePath, { recursive: true }); + await saveMeta(flat.storagePath, { + ...incrementalMeta(tmpRepo.dbPath), + lastCommit: headCommit(tmpRepo.dbPath), + branch: 'main-owner', + }); + execSync('git checkout -q -b feature-x', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + const branchSlot = getStoragePaths(tmpRepo.dbPath, 'feature-x'); + const branchDir = path.dirname(branchSlot.metaPath); + await fs.mkdir(branchDir, { recursive: true }); + await saveMeta(branchDir, { + ...schemaForeignMeta(tmpRepo.dbPath, FOREIGN_SCHEMA), + branch: 'feature-x', + }); + writes.length = 0; + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis( + tmpRepo.dbPath, + { branch: 'feature-x', skipAgentsMd: true, skipSkills: true }, + { onProgress: () => {}, onLog: () => {} }, + ); + + expect(result.rebuildReasons).toEqual(['schema-fingerprint']); + const withReasons = writes.filter((write) => (write.marker?.reasons ?? []).length > 0); + expect(withReasons.length).toBeGreaterThan(0); + expect(new Set(withReasons.map((write) => write.dir))).toEqual(new Set([branchDir])); + expect((await loadMeta(branchDir))?.incrementalInProgress).toBeUndefined(); + expect((await loadMeta(flat.storagePath))?.incrementalInProgress).toBeUndefined(); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('an FTS-park recovery clears the marker and its reasons without forcing', async () => { + const { wipeLbugDbFiles } = mockRun(); + const tmpRepo = await createTempDir('gitnexus-reasons-fts-park-'); + try { + await seedGitFile(tmpRepo.dbPath); + const { storagePath, lbugPath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + await saveMeta(storagePath, { + ...incrementalMeta(tmpRepo.dbPath), + lastCommit: headCommit(tmpRepo.dbPath), + incrementalInProgress: { + ...ftsInPlaceDirty, + reasons: [{ key: 'escalated-full-write', text: 'stored escalation' }], + }, + }); + await fs.writeFile(lbugPath, GRAPH_BYTES); + await fs.writeFile(`${lbugPath}.wal`, WAL_PATTERN); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis( + tmpRepo.dbPath, + { skipAgentsMd: true, skipSkills: true }, + { onProgress: () => {}, onLog: () => {} }, + ); + + expect(result.alreadyUpToDate).toBe(true); + expect(result.rebuildReasons).toEqual([]); + expect(wipeLbugDbFiles).not.toHaveBeenCalled(); + expect((await loadMeta(storagePath))?.incrementalInProgress).toBeUndefined(); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('a legacy boolean marker recovers with no reasons recorded', async () => { + mockRun(); + const tmpRepo = await createTempDir('gitnexus-reasons-legacy-marker-'); + try { + await seedGitFile(tmpRepo.dbPath); + const { storagePath, lbugPath, metaPath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + await saveMeta(storagePath, { + ...incrementalMeta(tmpRepo.dbPath), + lastCommit: headCommit(tmpRepo.dbPath), + }); + const raw = JSON.parse(await fs.readFile(metaPath, 'utf8')) as Record; + await fs.writeFile(metaPath, JSON.stringify({ ...raw, incrementalInProgress: true })); + await createPlaceholderGraphStore(lbugPath); + + // The "no reasons recorded" tail, taken from the real collector: the part + // of an empty recovery entry that differs from one carrying a reason. + const entryText = (stored: { key: string; text: string }[]): string => { + const shape = new RebuildReasonCollector(); + shape.recordInterruptedRebuild(stored); + return shape.reasons()[0].text; + }; + const empty = entryText([]); + const withReason = entryText([{ key: 'k', text: 'K' }]); + const shared = [...empty].findIndex((char, i) => char !== withReason[i]); + const noReasonsTail = empty.slice(shared); + + const logs: string[] = []; + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis( + tmpRepo.dbPath, + { skipAgentsMd: true, skipSkills: true }, + { onProgress: () => {}, onLog: (message) => logs.push(message) }, + ); + + expect(result.rebuildReasons).toEqual(['interrupted-rebuild']); + const summaries = logs.filter((message) => message.startsWith(summaryPrefix())); + expect(summaries).toHaveLength(1); + expect(summaries[0].endsWith(noReasonsTail)).toBe(true); + expect((await loadMeta(storagePath))?.incrementalInProgress).toBeUndefined(); + } finally { + await tmpRepo.cleanup(); + } + }); +}); diff --git a/gitnexus/test/unit/stream-graph-emit-force-ordering.test.ts b/gitnexus/test/unit/stream-graph-emit-force-ordering.test.ts index 36e787e62..d75f2e026 100644 --- a/gitnexus/test/unit/stream-graph-emit-force-ordering.test.ts +++ b/gitnexus/test/unit/stream-graph-emit-force-ordering.test.ts @@ -22,6 +22,8 @@ import fsp from 'node:fs/promises'; import path from 'node:path'; import { getStoragePaths, saveMeta } from '../../src/storage/repo-manager.js'; +import type { RepoMeta } from '../../src/storage/repo-meta.js'; +import { RebuildReasonCollector, type RebuildReasonKey } from '../../src/core/rebuild-reasons.js'; import { createTempDir } from '../helpers/test-db.js'; type PipelineModule = typeof import('../../src/core/ingestion/pipeline.js'); @@ -50,6 +52,7 @@ vi.mock('../../src/core/ingestion/pipeline.js', async (importOriginal) => { afterEach(() => { captured.options.length = 0; vi.unstubAllEnvs(); + vi.restoreAllMocks(); }); describe('streamGraphEmit is resolved after the force-mutating freshness guards', () => { @@ -104,3 +107,171 @@ describe('streamGraphEmit is resolved after the force-mutating freshness guards' } }, 120_000); }); + +/** What the collector held, and printed, at the pre-pipeline summary. */ +interface SummaryCall { + keys: RebuildReasonKey[]; + summary: string | undefined; +} + +/** + * Record every pre-pipeline summary the real collector formats. The pipeline + * mock ends the run before `runFullAnalysis` can return its keys, so the + * summary checkpoint is the seam: what was collected, and the exact text it + * printed, without re-typing any reason. + */ +const recordSummaries = (): SummaryCall[] => { + const calls: SummaryCall[] = []; + const formatSummary = RebuildReasonCollector.prototype.formatSummary; + vi.spyOn(RebuildReasonCollector.prototype, 'formatSummary').mockImplementation(function ( + this: RebuildReasonCollector, + ) { + const keys = this.keys(); + const summary = formatSummary.call(this); + calls.push({ keys, summary }); + return summary; + }); + return calls; +}; + +/** Run to the (mocked) pipeline and return the log lines. */ +const runToPipeline = async ( + repoPath: string, + options: { + useParseCache?: boolean; + skills?: boolean; + repairFts?: boolean; + dropEmbeddings?: boolean; + }, +): Promise => { + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const logs: string[] = []; + await expect( + runFullAnalysis( + repoPath, + { skipAgentsMd: true, ...options }, + { onProgress: () => {}, onLog: (m: string) => logs.push(m) }, + ), + ).rejects.toThrow(PIPELINE_REACHED); + return logs; +}; + +const writeMeta = async (repoPath: string, meta: Omit): Promise => { + const metaDir = path.dirname(getStoragePaths(repoPath).metaPath); + await fsp.mkdir(metaDir, { recursive: true }); + await saveMeta(metaDir, { repoPath, ...meta }); +}; + +describe('pre-pipeline rebuild reasons (#3137)', () => { + it.each([ + { flag: 'skills', options: { skills: true }, key: 'skills' }, + { flag: 'useParseCache: false', options: { useParseCache: false }, key: 'parse-cache-bypass' }, + ] as const)( + '$flag alone contributes only its own reason, once', + async ({ options, key }) => { + vi.stubEnv('GITNEXUS_STREAM_GRAPH_EMIT', '1'); + const summaries = recordSummaries(); + const tmpRepo = await createTempDir('gitnexus-rebuild-reason-flag-'); + try { + const logs = await runToPipeline(tmpRepo.dbPath, options); + + expect(summaries.map(({ keys }) => keys)).toEqual([[key]]); + const [{ summary }] = summaries; + expect(logs.filter((m) => m === summary)).toHaveLength(1); + // The flag no longer masquerades as --force: it forces the rebuild itself. + expect(captured.options[0]).toMatchObject({ streamGraphEmit: true }); + } finally { + await tmpRepo.cleanup(); + } + }, + 120_000, + ); + + it('--repair-fts after a retention change yields one content-retention entry', async () => { + const summaries = recordSummaries(); + const tmpRepo = await createTempDir('gitnexus-rebuild-reason-retention-'); + try { + // Built under a different retention than this run's default (`full`): + // both the --repair-fts conversion and the retention gate fire. + await writeMeta(tmpRepo.dbPath, { + lastCommit: '', + indexedAt: new Date(0).toISOString(), + schemaFingerprint: 'a0b1c2d3e4f5', + contentRetention: 'none', + }); + + await runToPipeline(tmpRepo.dbPath, { repairFts: true }); + + expect(summaries).toHaveLength(1); + expect(summaries[0].keys.filter((k) => k === 'content-retention')).toHaveLength(1); + // The repair was converted into the rebuild instead of returning early. + expect(captured.options).toHaveLength(1); + } finally { + await tmpRepo.cleanup(); + } + }, 120_000); + + it('--drop-embeddings over a stored embedding checkpoint contributes the drop-embeddings reason', async () => { + const summaries = recordSummaries(); + const tmpRepo = await createTempDir('gitnexus-rebuild-reason-drop-embeddings-'); + try { + // The reason is collected only where a checkpoint is being discarded. + await writeMeta(tmpRepo.dbPath, { + lastCommit: '', + indexedAt: new Date(0).toISOString(), + embeddingCheckpoint: { + at: new Date(0).toISOString(), + nodesProcessed: 0, + totalNodes: 1, + chunksProcessed: 0, + model: 'test-model', + dimensions: 384, + provider: 'local', + pendingNodeIds: [], + }, + }); + + const logs = await runToPipeline(tmpRepo.dbPath, { dropEmbeddings: true }); + + expect(summaries).toHaveLength(1); + expect(summaries[0].keys).toContain('drop-embeddings'); + expect(logs).toContain('Discarding the embedding checkpoint (--drop-embeddings).'); + } finally { + await tmpRepo.cleanup(); + } + }, 120_000); + + it('a first-build claim retry names no schema or runner-identity change', async () => { + const summaries = recordSummaries(); + const tmpRepo = await createTempDir('gitnexus-rebuild-reason-claim-'); + try { + // Exactly what the slot claim writes before a first build that then crashed. + await writeMeta(tmpRepo.dbPath, { + storagePath: path.dirname(getStoragePaths(tmpRepo.dbPath).metaPath), + lastCommit: '', + indexedAt: new Date(0).toISOString(), + }); + + await runToPipeline(tmpRepo.dbPath, {}); + + expect(summaries).toEqual([{ keys: [], summary: undefined }]); + } finally { + await tmpRepo.cleanup(); + } + }, 120_000); + + it('a first build of a non-git folder rebuilds structurally with no summary', async () => { + vi.stubEnv('GITNEXUS_STREAM_GRAPH_EMIT', '1'); + const summaries = recordSummaries(); + const tmpRepo = await createTempDir('gitnexus-rebuild-reason-nongit-'); + try { + await runToPipeline(tmpRepo.dbPath, {}); + + expect(summaries).toEqual([{ keys: [], summary: undefined }]); + // Structural, not collected: the pipeline sees no forced rebuild. + expect(captured.options[0]).toMatchObject({ streamGraphEmit: false }); + } finally { + await tmpRepo.cleanup(); + } + }, 120_000); +});