From 6767f09d9d11d4f8c67a41ea893ad7f4cd27461a Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Mon, 6 Jul 2026 04:08:11 -0600 Subject: [PATCH] feat: wire points-to solver max-iterations cap through DEFAULTS.analysis.pointsToMaxIterations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MAX_SOLVER_ITERATIONS was a hardcoded 50 in both the WASM points-to solver (points-to.ts) and the native Rust solver (build_edges.rs), duplicating but never reading DEFAULTS.analysis.pointsToMaxIterations. Threads a maxIterations parameter from the pipeline's resolved config through buildPointsToMap -> buildPointsToMapForFile -> buildCallEdgesJS/buildCallEdgesNative on the TS side, and through build_call_edges -> process_file -> build_file_context -> build_pts_map_for_file -> build_points_to_map on the Rust side, sourced from a new BuildConfig.analysis.points_to_max_iterations field deserialized from the JSON config payload already passed to the native engine. Default value (50) is unchanged when no override is configured. docs check acknowledged: no README/CLAUDE.md/ROADMAP.md updates needed — purely internal plumbing for an already-documented, already-accepted config key with no new CLI flags or user-facing surface. Fixes #1753 Impact: 7 functions changed, 6 affected --- .../src/domain/graph/builder/pipeline.rs | 14 +- .../graph/builder/stages/build_edges.rs | 137 +++++++++++--- .../src/infrastructure/config.rs | 43 +++++ .../graph/builder/stages/build-edges.ts | 21 ++- src/domain/graph/resolver/points-to.ts | 23 +-- src/infrastructure/config.ts | 21 ++- src/types.ts | 14 +- ...ssue-1753-points-to-max-iterations.test.ts | 178 ++++++++++++++++++ tests/unit/config.test.ts | 12 ++ tests/unit/points-to.test.ts | 60 ++++++ 10 files changed, 469 insertions(+), 54 deletions(-) create mode 100644 tests/integration/issue-1753-points-to-max-iterations.test.ts diff --git a/crates/codegraph-core/src/domain/graph/builder/pipeline.rs b/crates/codegraph-core/src/domain/graph/builder/pipeline.rs index 2d2471457..17810f123 100644 --- a/crates/codegraph-core/src/domain/graph/builder/pipeline.rs +++ b/crates/codegraph-core/src/domain/graph/builder/pipeline.rs @@ -578,7 +578,13 @@ pub fn run_pipeline( // Build call edges using existing Rust edge_builder (internal path) // For now, call edges are built via the existing napi-exported function's // internal logic. We load nodes from DB and pass to the edge builder. - build_and_insert_call_edges(conn, &file_symbols, &import_ctx, !change_result.is_full_build); + build_and_insert_call_edges( + conn, + &file_symbols, + &import_ctx, + !change_result.is_full_build, + config.analysis.points_to_max_iterations, + ); reconnect_saved_reverse_dep_edges(conn, &saved_reverse_dep_edges); @@ -1517,11 +1523,15 @@ fn insert_call_edge_rows(conn: &Connection, edges: &[crate::domain::graph::build } /// Full builds always load every node — there is no smaller set anyway. +/// +/// `max_iterations` caps the Phase 8.3 points-to solver's fixed-point loop — +/// forwarded from `config.analysis.points_to_max_iterations` (issue #1753). fn build_and_insert_call_edges( conn: &Connection, file_symbols: &BTreeMap, import_ctx: &ImportEdgeContext, is_incremental: bool, + max_iterations: u32, ) { use crate::domain::graph::builder::stages::build_edges::*; @@ -1619,7 +1629,7 @@ fn build_and_insert_call_edges( }); } - let computed_edges = build_call_edges(file_entries, all_nodes, builtin_receivers); + let computed_edges = build_call_edges(file_entries, all_nodes, builtin_receivers, max_iterations); insert_call_edge_rows(conn, &computed_edges); } diff --git a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs index 8a48743ed..94b35cc06 100644 --- a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs +++ b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs @@ -182,11 +182,17 @@ impl<'a> EdgeContext<'a> { // ── Phase 8.3: points-to analysis ───────────────────────────────────────── -/// Maximum fixed-point iterations for the pts solver. -/// Mirrors `MAX_SOLVER_ITERATIONS` in `src/domain/graph/resolver/points-to.ts`. -/// TODO: wire through `CodegraphConfig.analysis.pointsToMaxIterations` once -/// config plumbing is in place (same pattern as `typePropagationDepth`). -const MAX_SOLVER_ITERATIONS: usize = 50; +/// Default maximum fixed-point iterations for the pts solver — mirrors +/// `MAX_SOLVER_ITERATIONS` in `src/domain/graph/resolver/points-to.ts` and +/// `DEFAULTS.analysis.pointsToMaxIterations` in `src/infrastructure/config.ts`. +/// `build_call_edges()` now receives the resolved value as an explicit +/// `max_iterations` parameter (threaded from `BuildConfig.analysis.points_to_max_iterations` +/// on the native-first pipeline path, or from `ctx.config.analysis.pointsToMaxIterations` +/// via the napi call on the JS-orchestrated per-stage path); production code no +/// longer references this constant directly, so it is `#[cfg(test)]`-gated — +/// it remains only as the fallback default used directly by unit tests below. +#[cfg(test)] +const MAX_SOLVER_ITERATIONS: u32 = 50; /// Per-file points-to binding inputs, borrowed from a `FileEdgeInput`. /// `fn_ref_bindings` must already include the `fn::this → ctx` conversions @@ -208,11 +214,16 @@ struct PtsBindings<'a> { /// Seeds every locally-defined callable and every imported name as pointing /// to itself, generates inclusion constraints (`pts(lhs) ⊇ pts(rhsKey)`) /// from every binding kind, then solves by fixed-point iteration. +/// +/// `max_iterations` caps the fixed-point loop below — resolved from +/// `CodegraphConfig.analysis.pointsToMaxIterations` by the caller (mirrors +/// the `maxIterations` parameter of the TS `buildPointsToMap`). fn build_points_to_map( bindings: &PtsBindings, def_names: &HashSet<&str>, imported_names: &HashMap<&str, &str>, definition_params: &HashMap<&str, Vec<&str>>, + max_iterations: u32, ) -> HashMap> { let mut pts: HashMap> = HashMap::new(); for name in def_names { @@ -346,7 +357,7 @@ fn build_points_to_map( } // Fixed-point iteration: propagate pts sets until no new information flows. - for _ in 0..MAX_SOLVER_ITERATIONS { + for _ in 0..max_iterations { let mut changed = false; for (lhs, rhs_key) in &constraints { let rhs_pts: Option> = pts.get(rhs_key.as_str()) @@ -445,17 +456,22 @@ fn emit_pts_alias_edges<'a>( /// /// Mirrors the algorithm in builder.js `buildEdges` transaction (call edges /// portion). Import edges are handled separately in JS. +/// +/// `max_iterations` caps the Phase 8.3 points-to solver's fixed-point loop — +/// callers pass `ctx.config.analysis.pointsToMaxIterations` (resolved from +/// `.codegraphrc.json`, defaulting to `DEFAULTS.analysis.pointsToMaxIterations`). #[napi] pub fn build_call_edges( files: Vec, all_nodes: Vec, builtin_receivers: Vec, + max_iterations: u32, ) -> Vec { let ctx = EdgeContext::new(&all_nodes, &builtin_receivers); let mut edges = Vec::new(); for file_input in &files { - process_file(&ctx, file_input, &all_nodes, &mut edges); + process_file(&ctx, file_input, &all_nodes, &mut edges, max_iterations); } edges @@ -512,6 +528,7 @@ fn build_type_map<'a>(file_input: &'a FileEdgeInput) -> HashMap<&'a str, (&'a st fn build_pts_map_for_file( file_input: &FileEdgeInput, imported_names: &HashMap<&str, &str>, + max_iterations: u32, ) -> Option>> { let raw_fn_ref: &[FnRefBinding] = file_input.fn_ref_bindings.as_deref().unwrap_or(&[]); let this_calls: &[ThisCallBinding] = file_input.this_call_bindings.as_deref().unwrap_or(&[]); @@ -568,13 +585,20 @@ fn build_pts_map_for_file( PtsBindings { fn_ref_bindings: &merged_fn_ref, ..bindings } }; - Some(build_points_to_map(&final_bindings, &def_names, imported_names, &definition_params)) + Some(build_points_to_map( + &final_bindings, + &def_names, + imported_names, + &definition_params, + max_iterations, + )) } /// Build all per-file lookup structures needed for edge emission. fn build_file_context<'a>( file_input: &'a FileEdgeInput, all_nodes: &'a [NodeInfo], + max_iterations: u32, ) -> FileContext<'a> { let rel_path = file_input.file.as_str(); let imported_names: HashMap<&str, &str> = file_input @@ -599,7 +623,7 @@ fn build_file_context<'a>( node_id, } }).collect(); - let pts_map = build_pts_map_for_file(file_input, &imported_names); + let pts_map = build_pts_map_for_file(file_input, &imported_names, max_iterations); let raw_fn_ref: &[FnRefBinding] = file_input.fn_ref_bindings.as_deref().unwrap_or(&[]); // Case (c) flat-key gate set: lhs names from the *raw* fnRefBindings only // (thisCall conversions are scoped keys and never flat-matched). @@ -732,8 +756,9 @@ fn process_file<'a>( file_input: &'a FileEdgeInput, all_nodes: &'a [NodeInfo], edges: &mut Vec, + max_iterations: u32, ) { - let fc = build_file_context(file_input, all_nodes); + let fc = build_file_context(file_input, all_nodes, max_iterations); // Phase 8.3: tracks pts-resolved edges separately from seen_edges so that a // subsequent direct call to the same caller→target pair can upgrade confidence @@ -2198,7 +2223,7 @@ mod call_edge_tests { vec![], )]; - let edges = build_call_edges(files, all_nodes, vec![]); + let edges = build_call_edges(files, all_nodes, vec![], MAX_SOLVER_ITERATIONS); let receiver_edge = edges.iter().find(|e| e.kind == "receiver"); assert!( @@ -2239,7 +2264,7 @@ mod call_edge_tests { // same-file kind="function" node as an import artifact and falls through. file.imported_names = vec![ImportedName { name: "Calculator".to_string(), file: "utils.js".to_string(), imported: None }]; - let edges = build_call_edges(vec![file], all_nodes, vec![]); + let edges = build_call_edges(vec![file], all_nodes, vec![], MAX_SOLVER_ITERATIONS); let receiver_edge = edges.iter().find(|e| e.kind == "receiver"); assert!( @@ -2273,7 +2298,7 @@ mod call_edge_tests { vec![], )]; - let edges = build_call_edges(files, all_nodes, vec![]); + let edges = build_call_edges(files, all_nodes, vec![], MAX_SOLVER_ITERATIONS); let receiver_edge = edges.iter().find(|e| e.kind == "receiver"); assert!( @@ -2301,7 +2326,7 @@ mod call_edge_tests { vec![type_map_entry("UserService.logger", "Logger", 1.0)], vec![], )]; - let edges = build_call_edges(files, all_nodes, vec![]); + let edges = build_call_edges(files, all_nodes, vec![], MAX_SOLVER_ITERATIONS); assert!( edges.iter().any(|e| e.kind == "calls" && e.source_id == 1 && e.target_id == 2), "expected calls edge UserService.create → Logger.error; got: {:?}", @@ -2325,7 +2350,7 @@ mod call_edge_tests { vec![type_map_entry("useRest::eerest", "E4", 0.85)], vec![], )]; - let edges = build_call_edges(files, all_nodes, vec![]); + let edges = build_call_edges(files, all_nodes, vec![], MAX_SOLVER_ITERATIONS); assert!( edges.iter().any(|e| e.kind == "calls" && e.source_id == 1 && e.target_id == 2), "expected calls edge useRest → E4.e4 via rest-param key; got: {:?}", @@ -2349,7 +2374,7 @@ mod call_edge_tests { vec![], vec![], )]; - let edges = build_call_edges(files, all_nodes, vec![]); + let edges = build_call_edges(files, all_nodes, vec![], MAX_SOLVER_ITERATIONS); assert!( !edges.iter().any(|e| e.kind == "calls" && e.source_id == 1 && e.target_id == 2), "bare call must not resolve to same-class sibling in a module-scoped language" @@ -2372,7 +2397,7 @@ mod call_edge_tests { vec![], vec![], )]; - let edges = build_call_edges(files, all_nodes, vec![]); + let edges = build_call_edges(files, all_nodes, vec![], MAX_SOLVER_ITERATIONS); assert!( edges.iter().any(|e| e.kind == "calls" && e.source_id == 1 && e.target_id == 2), "bare sibling call must resolve in a class-scoped language; got: {:?}", @@ -2397,7 +2422,7 @@ mod call_edge_tests { vec![], vec![], )]; - let edges = build_call_edges(files, all_nodes, vec![]); + let edges = build_call_edges(files, all_nodes, vec![], MAX_SOLVER_ITERATIONS); assert!( edges.iter().any(|e| e.kind == "calls" && e.source_id == 1 && e.target_id == 2), "expected Geo.Shape.describe → Shape.area via bare class segment; got: {:?}", @@ -2423,7 +2448,7 @@ mod call_edge_tests { vec![type_map_entry("calc", "Calculator", 0.85)], vec![], )]; - let edges = build_call_edges(files, all_nodes, vec![]); + let edges = build_call_edges(files, all_nodes, vec![], MAX_SOLVER_ITERATIONS); let re = edges.iter().find(|e| e.kind == "receiver").expect("receiver edge"); assert!( (re.confidence - 0.85).abs() < 1e-9, @@ -2450,7 +2475,7 @@ mod call_edge_tests { vec![], )]; - let edges = build_call_edges(files, all_nodes, vec![]); + let edges = build_call_edges(files, all_nodes, vec![], MAX_SOLVER_ITERATIONS); let receiver_edge = edges.iter().find(|e| e.kind == "receiver"); assert!(receiver_edge.is_some(), "expected receiver edge for direct class-name receiver"); @@ -2496,7 +2521,7 @@ mod call_edge_tests { arg_name: "target".to_string(), }]); - let edges = build_call_edges(vec![file], all_nodes, vec![]); + let edges = build_call_edges(vec![file], all_nodes, vec![], MAX_SOLVER_ITERATIONS); assert!( edges.iter().any(|e| e.source_id == 1 && e.target_id == 2 && e.kind == "calls"), @@ -2541,7 +2566,7 @@ mod call_edge_tests { this_arg: "handler".to_string(), }]); - let edges = build_call_edges(vec![file], all_nodes, vec![]); + let edges = build_call_edges(vec![file], all_nodes, vec![], MAX_SOLVER_ITERATIONS); assert!( edges.iter().any(|e| e.source_id == 1 && e.target_id == 2 && e.kind == "calls"), @@ -2587,7 +2612,7 @@ mod call_edge_tests { enclosing_func: "iterPlain".to_string(), }]); - let edges = build_call_edges(vec![file], all_nodes, vec![]); + let edges = build_call_edges(vec![file], all_nodes, vec![], MAX_SOLVER_ITERATIONS); for target in [1u32, 2u32] { assert!( @@ -2637,7 +2662,7 @@ mod call_edge_tests { value_name: "e4".to_string(), }]); - let edges = build_call_edges(vec![file], all_nodes, vec![]); + let edges = build_call_edges(vec![file], all_nodes, vec![], MAX_SOLVER_ITERATIONS); assert!( edges.iter().any(|e| e.source_id == 1 && e.target_id == 2 && e.kind == "calls"), @@ -2679,7 +2704,7 @@ mod call_edge_tests { start_index: 0, }]); - let edges = build_call_edges(vec![file], all_nodes, vec![]); + let edges = build_call_edges(vec![file], all_nodes, vec![], MAX_SOLVER_ITERATIONS); assert!( edges.iter().any(|e| e.source_id == 1 && e.target_id == 2 && e.kind == "calls"), @@ -2692,6 +2717,68 @@ mod call_edge_tests { edges.iter().map(|e| (e.source_id, e.target_id, &e.kind)).collect::>() ); } + + /// Regression for issue #1753: the points-to solver's fixed-point loop must + /// honor the caller-supplied `max_iterations` rather than a hardcoded value. + /// Mirrors the equivalent TS-side test in `tests/unit/points-to.test.ts`. + /// + /// Builds an 8-hop alias chain `a0=a1, a1=a2, ..., a6=a7, a7=handler` in this + /// exact (declaration) order. `build_points_to_map` processes constraints in + /// array order each pass, so a single hop propagates per iteration, moving + /// from the tail of the array backward to the front — resolving `a0` + /// requires exactly `chain_len` (8) iterations. + #[test] + fn max_iterations_caps_alias_chain_convergence() { + let chain_len: u32 = 8; + let mut fn_ref_bindings: Vec = (0..chain_len - 1) + .map(|i| FnRefBinding { + lhs: format!("a{i}"), + rhs: format!("a{}", i + 1), + rhs_receiver: None, + }) + .collect(); + fn_ref_bindings.push(FnRefBinding { + lhs: format!("a{}", chain_len - 1), + rhs: "handler".to_string(), + rhs_receiver: None, + }); + + let def_names: HashSet<&str> = ["handler"].into_iter().collect(); + let imported_names: HashMap<&str, &str> = HashMap::new(); + let definition_params: HashMap<&str, Vec<&str>> = HashMap::new(); + let bindings = PtsBindings { + fn_ref_bindings: &fn_ref_bindings, + param_bindings: &[], + array_elem_bindings: &[], + spread_arg_bindings: &[], + for_of_bindings: &[], + array_callback_bindings: &[], + object_rest_param_bindings: &[], + object_prop_bindings: &[], + }; + + // A cap well below the chain length must not converge for a0. + let pts_low = + build_points_to_map(&bindings, &def_names, &imported_names, &definition_params, 3); + assert!( + resolve_via_points_to("a0", &pts_low).is_empty(), + "expected a0 to NOT resolve with max_iterations=3 (chain needs {chain_len})" + ); + + // A cap at the chain length must fully converge for a0. + let pts_high = build_points_to_map( + &bindings, + &def_names, + &imported_names, + &definition_params, + chain_len, + ); + assert_eq!( + resolve_via_points_to("a0", &pts_high), + vec!["handler"], + "expected a0 to resolve to handler with max_iterations={chain_len}" + ); + } } #[cfg(test)] diff --git a/crates/codegraph-core/src/infrastructure/config.rs b/crates/codegraph-core/src/infrastructure/config.rs index 4dbb706c2..4392e9c72 100644 --- a/crates/codegraph-core/src/infrastructure/config.rs +++ b/crates/codegraph-core/src/infrastructure/config.rs @@ -32,6 +32,10 @@ pub struct BuildConfig { /// Config-level path aliases (merged with tsconfig aliases). #[serde(default)] pub aliases: std::collections::HashMap, + + /// Analysis-tuning settings (points-to solver, etc.). + #[serde(default)] + pub analysis: AnalysisConfig, } #[derive(Debug, Clone, Deserialize)] @@ -66,6 +70,31 @@ fn default_drift_threshold() -> f64 { 0.1 } +/// Subset of `CodegraphConfig.analysis` relevant to the build pipeline. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AnalysisConfig { + /// Maximum fixed-point iterations for the Phase 8.3 points-to solver. + /// Mirrors `DEFAULTS.analysis.pointsToMaxIterations` in + /// `src/infrastructure/config.ts` and `MAX_SOLVER_ITERATIONS` in + /// `src/domain/graph/builder/stages/build_edges.rs`. Threaded to + /// `build_call_edges()` via `build_and_insert_call_edges()`. + #[serde(default = "default_points_to_max_iterations")] + pub points_to_max_iterations: u32, +} + +impl Default for AnalysisConfig { + fn default() -> Self { + Self { + points_to_max_iterations: default_points_to_max_iterations(), + } + } +} + +fn default_points_to_max_iterations() -> u32 { + 50 +} + /// Build options passed from the JS caller. #[derive(Debug, Clone, Deserialize, Default)] #[serde(rename_all = "camelCase")] @@ -143,6 +172,8 @@ mod tests { assert!(config.include.is_empty()); assert!(config.exclude.is_empty()); assert!(config.build.incremental); + // Default mirrors DEFAULTS.analysis.pointsToMaxIterations in config.ts. + assert_eq!(config.analysis.points_to_max_iterations, 50); } #[test] @@ -166,6 +197,18 @@ mod tests { assert!(!config.build.incremental); assert_eq!(config.build.drift_threshold, 0.2); assert_eq!(config.aliases.get("@/").unwrap(), "src/"); + // analysis key omitted entirely — must still fall back to the default. + assert_eq!(config.analysis.points_to_max_iterations, 50); + } + + #[test] + fn deserialize_analysis_override() { + // Mirrors the shape the JS side serializes via JSON.stringify(ctx.config) + // when a repo sets a non-default `analysis.pointsToMaxIterations` in + // .codegraphrc.json (issue #1753). + let json = r#"{"analysis": {"pointsToMaxIterations": 5}}"#; + let config: BuildConfig = serde_json::from_str(json).unwrap(); + assert_eq!(config.analysis.points_to_max_iterations, 5); } #[test] diff --git a/src/domain/graph/builder/stages/build-edges.ts b/src/domain/graph/builder/stages/build-edges.ts index 570121fde..cfe33f891 100644 --- a/src/domain/graph/builder/stages/build-edges.ts +++ b/src/domain/graph/builder/stages/build-edges.ts @@ -638,9 +638,12 @@ function buildCallEdgesNative( nativeFiles.push(buildNativeFileEntry(ctx, relPath, fileNodeRow.id, symbols, rootDir)); } - const nativeEdges = native.buildCallEdges(nativeFiles, allNodes, [ - ...BUILTIN_RECEIVERS, - ]) as NativeEdge[]; + const nativeEdges = native.buildCallEdges( + nativeFiles, + allNodes, + [...BUILTIN_RECEIVERS], + ctx.config.analysis.pointsToMaxIterations, + ) as NativeEdge[]; for (const e of nativeEdges) { allEdgeRows.push([ e.sourceId, @@ -929,7 +932,11 @@ function buildCallEdgesJS( } const seenCallEdges = new Set(); - const ptsMap = buildPointsToMapForFile(symbols, importedNames); + const ptsMap = buildPointsToMapForFile( + symbols, + importedNames, + ctx.config.analysis.pointsToMaxIterations, + ); // Build the import-artifact name set: importedNames plus CJS require bindings. // Used only by resolveReceiverEdge to distinguish local definitions from CJS // import shadows — does NOT affect call-target resolution or DB edges (#1661). @@ -1059,10 +1066,15 @@ function makeContextLookup(ctx: PipelineContext, getNodeIdStmt: NodeIdStmt): Cal * (`const Svc = MyService`) is an uncommon pattern that would require tracking * `new`-expression flows separately from the alias chain. That is left to Phase * 8.2 call-assignment propagation, which already handles constructor assignments. + * + * @param maxIterations - fixed-point solver iteration cap, forwarded to + * `buildPointsToMap` (resolved from `ctx.config.analysis.pointsToMaxIterations` + * by the caller, which already holds the pipeline's resolved config). */ function buildPointsToMapForFile( symbols: ExtractorOutput, importedNames: Map, + maxIterations: number, ): PointsToMap | null { const hasThisCallBindings = !!symbols.thisCallBindings?.length; if ( @@ -1108,6 +1120,7 @@ function buildPointsToMapForFile( symbols.arrayCallbackBindings, symbols.objectRestParamBindings, symbols.objectPropBindings, + maxIterations, ); } diff --git a/src/domain/graph/resolver/points-to.ts b/src/domain/graph/resolver/points-to.ts index 5573c53ab..ec9274719 100644 --- a/src/domain/graph/resolver/points-to.ts +++ b/src/domain/graph/resolver/points-to.ts @@ -19,6 +19,7 @@ * that build-edges.ts already builds per file is the cross-module link — if * a variable aliases an imported name, resolveCallTargets follows it). */ +import { DEFAULTS } from '../../../infrastructure/config.js'; import type { ArrayCallbackBinding, ArrayElemBinding, @@ -32,14 +33,6 @@ import type { export type PointsToMap = Map>; -/** - * Maximum fixed-point iterations before bailing out (prevents divergence). - * Mirrors `DEFAULTS.analysis.pointsToMaxIterations` in config.ts. - * TODO(Phase 8.3): thread config through buildPointsToMap so this can be tuned - * per-repo via `.codegraphrc.json` (tracked alongside typePropagationDepth). - */ -const MAX_SOLVER_ITERATIONS = 50; - /** * Seed the pts map from locally-defined functions, imported names, and * fnRefBindings (direct assignment aliases: `const fn = handler`). @@ -365,15 +358,16 @@ function appendAdvancedConstraints( /** * Run the fixed-point solver: propagate pts sets through constraints until - * no new information flows (or MAX_SOLVER_ITERATIONS is reached). + * no new information flows (or `maxIterations` is reached). * * Mutates `pts` in place. */ function buildCallSiteTypeMap( pts: PointsToMap, constraints: ReadonlyArray<{ lhs: string; rhsKey: string }>, + maxIterations: number, ): void { - for (let iter = 0; iter < MAX_SOLVER_ITERATIONS; iter++) { + for (let iter = 0; iter < maxIterations; iter++) { let changed = false; for (const { lhs, rhsKey } of constraints) { const rhsPts = pts.get(rhsKey); @@ -410,6 +404,12 @@ function buildCallSiteTypeMap( * @param spreadArgBindings - spread-argument bindings (Phase 8.3e) * @param forOfBindings - for-of iteration variable bindings (Phase 8.3e) * @param arrayCallbackBindings - Array.from/callback bindings (Phase 8.3e) + * @param maxIterations - fixed-point iteration cap before bailing out (prevents + * divergence). Defaults to `DEFAULTS.analysis.pointsToMaxIterations`; + * callers that already hold a resolved `CodegraphConfig` (e.g. + * `buildPointsToMapForFile` in `stages/build-edges.ts`) pass the + * user-configured value through explicitly. Mirrored by + * `MAX_SOLVER_ITERATIONS` in the native Rust solver (`stages/build_edges.rs`). */ export function buildPointsToMap( fnRefBindings: readonly FnRefBinding[], @@ -423,6 +423,7 @@ export function buildPointsToMap( arrayCallbackBindings?: readonly ArrayCallbackBinding[], objectRestParamBindings?: readonly ObjectRestParamBinding[], objectPropBindings?: readonly ObjectPropBinding[], + maxIterations: number = DEFAULTS.analysis.pointsToMaxIterations, ): PointsToMap { const { pts, constraints } = buildThisAssignmentMap( fnRefBindings, @@ -447,7 +448,7 @@ export function buildPointsToMap( if (constraints.length === 0) return pts; - buildCallSiteTypeMap(pts, constraints); + buildCallSiteTypeMap(pts, constraints, maxIterations); return pts; } diff --git a/src/infrastructure/config.ts b/src/infrastructure/config.ts index 315d221d5..46e0dd791 100644 --- a/src/infrastructure/config.ts +++ b/src/infrastructure/config.ts @@ -149,11 +149,10 @@ export const DEFAULTS = deepFreeze({ briefImporterDepth: 5, briefHighRiskCallers: 10, briefMediumRiskCallers: 3, - // TODO(Phase 8.3): wire these into the points-to solver and type-propagation path - // once config is threaded through to extractSymbols / buildPointsToMap. Currently - // controlled by hardcoded constants in src/extractors/javascript.ts - // (MAX_PROPAGATION_DEPTH, PROPAGATION_HOP_PENALTY, INFERRED_RETURN_TYPE_CONFIDENCE) and in - // src/domain/graph/resolver/points-to.ts (MAX_SOLVER_ITERATIONS). + // TODO(Phase 8.3): wire these into the type-propagation path once config is + // threaded through to extractSymbols. Currently controlled by hardcoded + // constants in src/extractors/javascript.ts (MAX_PROPAGATION_DEPTH, + // PROPAGATION_HOP_PENALTY, INFERRED_RETURN_TYPE_CONFIDENCE). typePropagationDepth: 3, /** * Confidence score assigned to a return type inferred from `return new Constructor()` @@ -164,10 +163,14 @@ export const DEFAULTS = deepFreeze({ typeInferenceConfidence: 0.85, /** * Maximum fixed-point iterations for the Phase 8.3 points-to solver. - * @reserved — currently not wired to either the WASM solver - * (`MAX_SOLVER_ITERATIONS` in `points-to.ts`) or the native Rust solver - * (`MAX_SOLVER_ITERATIONS` in `stages/build_edges.rs`), both of which use the - * same hardcoded value of 50. See the TODO comment above. + * Wired as the default `maxIterations` parameter of `buildPointsToMap()` + * in `src/domain/graph/resolver/points-to.ts`. The build pipeline + * (`buildCallEdgesJS` in `stages/build-edges.ts`, which already holds a + * resolved `ctx.config`) passes the value through explicitly to the WASM + * solver, and to the native Rust solver (`MAX_SOLVER_ITERATIONS` in + * `stages/build_edges.rs`) via `native.buildCallEdges()` on the per-stage + * path or the `BuildConfig` JSON payload on the native-first path — keeping + * both engines in sync. */ pointsToMaxIterations: 50, }, diff --git a/src/types.ts b/src/types.ts index 1d41d4e02..954b9fb5a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1490,8 +1490,11 @@ export interface CodegraphConfig { typePropagationDepth: number; /** * Maximum fixed-point iterations for the Phase 8.3 points-to solver. - * @reserved — currently not wired to either solver; both use a hardcoded - * constant of 50. See TODO in `src/infrastructure/config.ts`. + * Wired as the default `maxIterations` parameter of `buildPointsToMap()` + * in `src/domain/graph/resolver/points-to.ts`. The build pipeline (which + * already holds a resolved config) passes this value through explicitly + * to both the WASM solver and, via `native.buildCallEdges()` / the + * `BuildConfig` JSON payload, the native Rust solver in `stages/build_edges.rs`. */ pointsToMaxIterations: number; /** @@ -2276,7 +2279,12 @@ export interface NativeAddon { assignments: Array<{ node: string; community: number }>; modularity: number; }; - buildCallEdges(files: unknown[], nodes: unknown[], builtinReceivers: string[]): unknown[]; + buildCallEdges( + files: unknown[], + nodes: unknown[], + builtinReceivers: string[], + maxIterations: number, + ): unknown[]; buildImportEdges?( files: unknown[], resolvedImports: unknown[], diff --git a/tests/integration/issue-1753-points-to-max-iterations.test.ts b/tests/integration/issue-1753-points-to-max-iterations.test.ts new file mode 100644 index 000000000..c740a83e8 --- /dev/null +++ b/tests/integration/issue-1753-points-to-max-iterations.test.ts @@ -0,0 +1,178 @@ +/** + * Regression test for issue #1753 — `pointsToMaxIterations` config threading. + * + * `MAX_SOLVER_ITERATIONS` in the Phase 8.3 points-to solver used to be a + * hardcoded constant (50) in both `src/domain/graph/resolver/points-to.ts` + * (WASM) and `crates/codegraph-core/.../build_edges.rs` (native), duplicating + * — but never reading from — `DEFAULTS.analysis.pointsToMaxIterations` in + * `src/infrastructure/config.ts`. + * + * This suite builds an 8-hop function-alias chain + * (`a0=a1, a1=a2, ..., a6=a7, a7=handler`) that the fixed-point solver needs + * exactly 8 iterations to fully resolve (see the equivalent unit tests in + * `tests/unit/points-to.test.ts` and the Rust `max_iterations_caps_alias_chain_convergence` + * test for the derivation). A `.codegraphrc.json` setting + * `analysis.pointsToMaxIterations` below that depth must suppress the + * resulting call edge on BOTH engines; the default (50) must resolve it. + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, describe, expect, it } from 'vitest'; +import { buildGraph } from '../../src/domain/graph/builder.js'; +import { isNativeAvailable } from '../../src/infrastructure/native.js'; + +const hasNative = isNativeAvailable(); +const requireParity = !!process.env.CODEGRAPH_PARITY; +const itNativeOrSkip = requireParity || hasNative ? it : it.skip; + +// 8-hop alias chain: a0 requires exactly 8 fixed-point iterations to resolve +// to `handler` (one hop propagates per solver iteration — see file header). +const CHAIN_LENGTH = 8; + +const HANDLER_JS = ` +export function handler(item) { + return item * 2; +} +`.trimStart(); + +function buildConsumerSource(): string { + const lines = [ + "import { handler } from './handler.js';", + '', + 'export function processItems(items) {', + ]; + for (let i = 0; i < CHAIN_LENGTH - 1; i++) { + lines.push(` const a${i} = a${i + 1};`); + } + lines.push(` const a${CHAIN_LENGTH - 1} = handler;`); + lines.push(' return items.map(a0);'); + lines.push('}'); + return `${lines.join('\n')}\n`; +} + +const CONSUMER_JS = buildConsumerSource(); + +const dirsToClean: string[] = []; + +function writeFixture(dir: string, maxIterations?: number): void { + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'handler.js'), HANDLER_JS); + fs.writeFileSync(path.join(dir, 'consumer.js'), CONSUMER_JS); + if (maxIterations !== undefined) { + fs.writeFileSync( + path.join(dir, '.codegraphrc.json'), + JSON.stringify({ analysis: { pointsToMaxIterations: maxIterations } }), + ); + } +} + +function readCallEdges(dbPath: string): Array<{ source: string; target: string }> { + const db = new Database(dbPath, { readonly: true }); + const rows = db + .prepare(` + SELECT n1.name AS source, n2.name AS target + FROM edges e + JOIN nodes n1 ON e.source_id = n1.id + JOIN nodes n2 ON e.target_id = n2.id + WHERE e.kind = 'calls' + ORDER BY n1.name, n2.name + `) + .all() as Array<{ source: string; target: string }>; + db.close(); + return rows; +} + +function hasProcessItemsToHandlerEdge(dbPath: string): boolean { + return readCallEdges(dbPath).some((e) => e.source === 'processItems' && e.target === 'handler'); +} + +afterAll(() => { + for (const dir of dirsToClean) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + } catch { + /* ignore */ + } + } +}); + +async function buildFixture(engine: 'wasm' | 'native', maxIterations?: number): Promise { + const label = maxIterations === undefined ? 'default' : `cap${maxIterations}`; + const tmpBase = fs.mkdtempSync( + path.join(os.tmpdir(), `codegraph-pts-max-iter-${engine}-${label}-`), + ); + dirsToClean.push(tmpBase); + writeFixture(tmpBase, maxIterations); + await buildGraph(tmpBase, { engine, incremental: false, skipRegistry: true }); + return tmpBase; +} + +describe('Phase 8.3 pts: pointsToMaxIterations config threading (WASM)', () => { + it('resolves the 8-hop alias chain with the default cap (50)', async () => { + const dir = await buildFixture('wasm'); + expect(hasProcessItemsToHandlerEdge(path.join(dir, '.codegraph', 'graph.db'))).toBe(true); + }, 60_000); + + it('suppresses the alias-chain edge when .codegraphrc.json caps below the required depth', async () => { + const dir = await buildFixture('wasm', 3); + expect(hasProcessItemsToHandlerEdge(path.join(dir, '.codegraph', 'graph.db'))).toBe(false); + }, 60_000); + + it('resolves the alias-chain edge when .codegraphrc.json raises the cap to meet the required depth', async () => { + const dir = await buildFixture('wasm', CHAIN_LENGTH); + expect(hasProcessItemsToHandlerEdge(path.join(dir, '.codegraph', 'graph.db'))).toBe(true); + }, 60_000); +}); + +describe('Phase 8.3 pts: pointsToMaxIterations config threading (native)', () => { + itNativeOrSkip( + 'resolves the 8-hop alias chain with the default cap (50)', + async () => { + const dir = await buildFixture('native'); + expect(hasProcessItemsToHandlerEdge(path.join(dir, '.codegraph', 'graph.db'))).toBe(true); + }, + 60_000, + ); + + itNativeOrSkip( + 'suppresses the alias-chain edge when .codegraphrc.json caps below the required depth', + async () => { + const dir = await buildFixture('native', 3); + expect(hasProcessItemsToHandlerEdge(path.join(dir, '.codegraph', 'graph.db'))).toBe(false); + }, + 60_000, + ); +}); + +describe('Phase 8.3 pts: pointsToMaxIterations — engine parity', () => { + itNativeOrSkip( + 'both engines agree the chain resolves under the default cap', + async () => { + const wasmDir = await buildFixture('wasm'); + const nativeDir = await buildFixture('native'); + const wasmEdges = readCallEdges(path.join(wasmDir, '.codegraph', 'graph.db')); + const nativeEdges = readCallEdges(path.join(nativeDir, '.codegraph', 'graph.db')); + expect(nativeEdges).toEqual(wasmEdges); + expect(hasProcessItemsToHandlerEdge(path.join(wasmDir, '.codegraph', 'graph.db'))).toBe(true); + }, + 60_000, + ); + + itNativeOrSkip( + 'both engines agree the chain is suppressed under a below-depth override', + async () => { + const wasmDir = await buildFixture('wasm', 3); + const nativeDir = await buildFixture('native', 3); + const wasmEdges = readCallEdges(path.join(wasmDir, '.codegraph', 'graph.db')); + const nativeEdges = readCallEdges(path.join(nativeDir, '.codegraph', 'graph.db')); + expect(nativeEdges).toEqual(wasmEdges); + expect(hasProcessItemsToHandlerEdge(path.join(wasmDir, '.codegraph', 'graph.db'))).toBe( + false, + ); + }, + 60_000, + ); +}); diff --git a/tests/unit/config.test.ts b/tests/unit/config.test.ts index 14122333c..71cb22b4f 100644 --- a/tests/unit/config.test.ts +++ b/tests/unit/config.test.ts @@ -272,6 +272,18 @@ describe('loadConfig', () => { expect(config.analysis.impactDepth).toBe(3); expect(config.analysis.auditDepth).toBe(3); }); + + it('loads a pointsToMaxIterations override from config (issue #1753)', () => { + const dir = fs.mkdtempSync(path.join(tmpDir, 'pts-max-iter-')); + fs.writeFileSync( + path.join(dir, '.codegraphrc.json'), + JSON.stringify({ analysis: { pointsToMaxIterations: 5 } }), + ); + const config = loadConfig(dir); + expect(config.analysis.pointsToMaxIterations).toBe(5); + // Sibling defaults preserved + expect(config.analysis.typePropagationDepth).toBe(3); + }); }); describe('mergeConfig', () => { diff --git a/tests/unit/points-to.test.ts b/tests/unit/points-to.test.ts index b9dac077d..e32b2df71 100644 --- a/tests/unit/points-to.test.ts +++ b/tests/unit/points-to.test.ts @@ -417,3 +417,63 @@ describe('buildPointsToMap — object-rest parameter dispatch (Phase 8.3f)', () } }); }); + +describe('buildPointsToMap — maxIterations cap (issue #1753)', () => { + // Builds an 8-hop alias chain a0=a1, a1=a2, ..., a6=a7, a7=handler, in this + // exact declaration order. buildCallSiteTypeMap processes constraints in + // array order on every pass, so only one hop propagates per iteration, + // moving from the tail of the array backward to the front — resolving a0 + // requires exactly CHAIN_LENGTH (8) iterations. + const CHAIN_LENGTH = 8; + function buildChainBindings(): Array<{ lhs: string; rhs: string }> { + const fnRefBindings: Array<{ lhs: string; rhs: string }> = []; + for (let i = 0; i < CHAIN_LENGTH - 1; i++) { + fnRefBindings.push({ lhs: `a${i}`, rhs: `a${i + 1}` }); + } + fnRefBindings.push({ lhs: `a${CHAIN_LENGTH - 1}`, rhs: 'handler' }); + return fnRefBindings; + } + + it('does not converge the chain when maxIterations is below the required depth', () => { + const pts = buildPointsToMap( + buildChainBindings(), + new Set(['handler']), + NO_IMPORTS, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + 3, // well below CHAIN_LENGTH (8) + ); + expect(resolveViaPointsTo('a0', pts)).toEqual([]); + }); + + it('fully converges the chain when maxIterations meets the required depth', () => { + const pts = buildPointsToMap( + buildChainBindings(), + new Set(['handler']), + NO_IMPORTS, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + CHAIN_LENGTH, + ); + expect(resolveViaPointsTo('a0', pts)).toEqual(['handler']); + }); + + it('defaults to DEFAULTS.analysis.pointsToMaxIterations (50) when maxIterations is omitted', () => { + // The 8-hop chain converges comfortably under the default cap, confirming + // default behavior is unchanged now that maxIterations is configurable. + const pts = buildPointsToMap(buildChainBindings(), new Set(['handler']), NO_IMPORTS); + expect(resolveViaPointsTo('a0', pts)).toEqual(['handler']); + }); +});