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); +});