diff --git a/crates/codegraph-cli/tests/explore_unnamed_gap_note.rs b/crates/codegraph-cli/tests/explore_unnamed_gap_note.rs new file mode 100644 index 00000000..22cb079c --- /dev/null +++ b/crates/codegraph-cli/tests/explore_unnamed_gap_note.rs @@ -0,0 +1,184 @@ +//! The trim note never claims a gap marker named what it could not afford to +//! (#1711, #2077). +//! +//! A gap between two rendered windows names the indexed symbols it hides, but +//! only from the budget the file has spare; a name that costs more stays out +//! and the marker is a bare `... (gap) ...`. The note closing the response +//! then said the gap markers named what was elided. Here an index-only helper +//! whose name no budget affords sits between the two windows. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +fn bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_codegraph")) +} + +struct TestDir { + path: PathBuf, +} + +impl TestDir { + fn new(label: &str) -> Self { + let path = std::env::temp_dir().join(format!( + "codegraph-cli-unnamed-gap-{label}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(&path).unwrap(); + Self { path } + } +} + +impl Drop for TestDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.path); + } +} + +fn run_in(cwd: &Path, args: &[&str]) -> String { + let output = Command::new(bin()) + .args(args) + .current_dir(cwd) + .env("CODEGRAPH_NO_DAEMON", "1") + .output() + .expect("run codegraph binary"); + assert!( + output.status.success(), + "{args:?} failed: {}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).into_owned() +} + +/// What follows the last source fence: the epilogue. +fn epilogue_of(text: &str) -> &str { + text.rfind("```").map_or(text, |i| &text[i + 3..]) +} + +/// `src/big.ts`: `alphaEntry` at the top, then (when `helper_x` is given) an +/// index-only helper named `helper` plus that many X's, then `betaEntry`, whose +/// body runs `beta_lines` lines. `padding` small files lift the project into +/// the large tiers. +fn project( + label: &str, + helper_x: Option, + beta_lines: usize, + padding: usize, +) -> (TestDir, PathBuf) { + let dir = TestDir::new(label); + let root = dir.path.join("app"); + let src = root.join("src"); + std::fs::create_dir_all(&src).unwrap(); + let mut body: Vec = (1..10).map(|i| format!("// header {i}")).collect(); + body.push("export function alphaEntry(): number {".to_string()); + body.extend((0..19).map(|i| format!(" const a{i} = {i};"))); + body.push(" return betaEntry(1);".to_string()); + body.push("}".to_string()); + while body.len() < 99 { + body.push(format!("// filler {}", body.len() + 1)); + } + if let Some(x) = helper_x { + let helper = format!("helper{}", "X".repeat(x)); + body.push(format!("function {helper}(): number {{ return 1; }}")); + } + while body.len() < 199 { + body.push(format!("// filler {}", body.len() + 1)); + } + body.push("export function betaEntry(v: number): number {".to_string()); + body.extend((0..beta_lines).map(|i| format!(" v = v + {i}; // beta stage {i}"))); + body.push(" return v;".to_string()); + body.push("}".to_string()); + while body.len() < 400 { + body.push(format!("// filler {}", body.len() + 1)); + } + std::fs::write(src.join("big.ts"), body.join("\n")).unwrap(); + if padding > 0 { + let pad = root.join("pad"); + std::fs::create_dir_all(&pad).unwrap(); + for i in 0..padding { + std::fs::write( + pad.join(format!("pad{i}.ts")), + format!("export const pad{i} = {i};\n"), + ) + .unwrap(); + } + } + run_in(&dir.path, &["init", root.to_str().unwrap()]); + (dir, root) +} + +#[test] +fn small_tier_note_does_not_claim_an_unaffordable_gap_name() { + // No per-file budget affords an 8000-character name. + let (_dir, root) = project("small", Some(8000), 19, 0); + let text = run_in(&root, &["explore", "alphaEntry betaEntry"]); + // The fixture does what it is for: both windows render around a bare gap. + assert!(text.contains("export function alphaEntry"), "{text}"); + assert!(text.contains("export function betaEntry"), "{text}"); + assert!(text.contains("... (gap) ..."), "{text}"); + assert!(!text.contains("XXXXXXXX"), "{text}"); + let epilogue = epilogue_of(&text); + assert!( + epilogue.contains("Some file sections were trimmed for size"), + "{epilogue}" + ); + assert!( + !epilogue.contains("Elided symbols are named inside gap markers"), + "{epilogue}" + ); + assert!(epilogue.contains("only where room allowed"), "{epilogue}"); +} + +#[test] +fn large_tier_note_does_not_claim_an_unaffordable_gap_name() { + let (_dir, root) = project("large", Some(8000), 900, 520); + let text = run_in(&root, &["explore", "alphaEntry betaEntry"]); + assert!(text.contains("export function alphaEntry"), "{text}"); + assert!(text.contains("... (gap) ..."), "{text}"); + assert!(!text.contains("XXXXXXXX"), "{text}"); + let epilogue = epilogue_of(&text); + // `betaEntry` is windowed, so the section is reported trimmed. + assert!(epilogue.contains("Verbatim source for"), "{epilogue}"); + assert!(!epilogue.contains("name what was elided"), "{epilogue}"); + assert!(epilogue.contains("name what room allowed"), "{epilogue}"); +} + +/// The section a windowed `betaEntry` leaves ends in a bare tail marker: what +/// it cut is never named in a gap marker, whatever the budget. Nothing indexed +/// sits between the two functions, so no inner gap had a name to withhold. +fn assert_trailing_bare_gap(text: &str) { + assert!(text.contains("export function betaEntry"), "{text}"); + assert!(text.contains("... (gap) ...\n```"), "{text}"); + assert!(!text.contains("(gap: "), "{text}"); +} + +#[test] +fn small_tier_note_does_not_claim_names_for_a_trailing_gap() { + let (_dir, root) = project("small-tail", None, 900, 0); + let text = run_in(&root, &["explore", "alphaEntry betaEntry"]); + assert_trailing_bare_gap(&text); + let epilogue = epilogue_of(&text); + assert!( + epilogue.contains("Some file sections were trimmed for size"), + "{epilogue}" + ); + assert!( + !epilogue.contains("Elided symbols are named inside gap markers"), + "{epilogue}" + ); +} + +#[test] +fn large_tier_note_does_not_claim_names_for_a_trailing_gap() { + let (_dir, root) = project("large-tail", None, 900, 520); + let text = run_in(&root, &["explore", "alphaEntry betaEntry"]); + assert_trailing_bare_gap(&text); + let epilogue = epilogue_of(&text); + assert!(epilogue.contains("Verbatim source for"), "{epilogue}"); + assert!(!epilogue.contains("name what was elided"), "{epilogue}"); +} diff --git a/crates/codegraph-cli/tests/same_line_identity.rs b/crates/codegraph-cli/tests/same_line_identity.rs new file mode 100644 index 00000000..fe7ad425 --- /dev/null +++ b/crates/codegraph-cli/tests/same_line_identity.rs @@ -0,0 +1,91 @@ +//! Same-line namesakes survive the store, not just extraction (#1349). +//! +//! Nodes are upserted by id, so two declarations sharing one id leave a single +//! row behind. The Vue extractor mints its script functions itself; this +//! indexes a real project and reads the rows back. + +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::time::{Duration, Instant}; + +use codegraph_core::IndexPaths; +use codegraph_core::types::NodeKind; +use codegraph_store::Store; + +struct TestDir { + path: PathBuf, +} + +impl TestDir { + fn new(label: &str) -> Self { + let path = std::env::temp_dir().join(format!( + "codegraph-cli-same-line-identity-{label}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&path).unwrap(); + Self { path } + } +} + +impl Drop for TestDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.path); + } +} + +fn cli(args: &[&str]) { + let output = Command::new(env!("CARGO_BIN_EXE_codegraph")) + .args(args) + .env("CODEGRAPH_NO_DAEMON", "1") + .output() + .expect("run codegraph binary"); + assert!( + output.status.success(), + "codegraph {args:?} failed: stdout={} stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + +fn store(project: &Path) -> Store { + let paths = IndexPaths::resolve(project, None).expect("resolve index paths"); + Store::open_for_read(&paths, Instant::now() + Duration::from_secs(30), || false) + .expect("open the index for reading") +} + +#[test] +fn same_line_vue_functions_are_two_rows() { + let dir = TestDir::new("vue"); + let project = dir.path.join("proj"); + fs::create_dir_all(&project).unwrap(); + fs::write( + project.join("Widget.vue"), + "\n\n", + ) + .unwrap(); + let p = project.to_str().unwrap(); + cli(&["init", p]); + let rows = |project: &Path| { + let mut rows = store(project) + .all_nodes() + .unwrap() + .into_iter() + .filter(|node| node.kind == NodeKind::Function && node.name == "f") + .map(|node| (node.id, node.start_line, node.start_column)) + .collect::>(); + rows.sort(); + rows + }; + let indexed = rows(&project); + assert_eq!(indexed.len(), 2, "{indexed:#?}"); + assert_ne!(indexed[0].0, indexed[1].0); + cli(&["sync", p]); + assert_eq!(rows(&project), indexed, "sync keeps both rows"); + cli(&["index", "--force", p]); + assert_eq!(rows(&project), indexed, "index --force keeps both rows"); +} diff --git a/crates/codegraph-extract/src/embedded/vue.rs b/crates/codegraph-extract/src/embedded/vue.rs index 90c63150..7bd7e806 100644 --- a/crates/codegraph-extract/src/embedded/vue.rs +++ b/crates/codegraph-extract/src/embedded/vue.rs @@ -1,4 +1,4 @@ -use codegraph_core::node_id::generate_node_id; +use codegraph_core::node_id::{NodeIdAllocator, generate_node_id, utf16_column}; use codegraph_core::types::{ Edge, EdgeKind, ExtractionResult, Language, Node, NodeKind, UnresolvedRef, }; @@ -51,6 +51,8 @@ pub struct VueExtractor<'a> { edges: Vec, unresolved_references: Vec, errors: Vec, + /// Same-line namesakes keep distinct ids across every script block (#1349). + node_ids: NodeIdAllocator, } impl<'a> VueExtractor<'a> { @@ -62,6 +64,7 @@ impl<'a> VueExtractor<'a> { edges: Vec::new(), unresolved_references: Vec::new(), errors: Vec::new(), + node_ids: NodeIdAllocator::default(), } } @@ -204,11 +207,12 @@ impl<'a> VueExtractor<'a> { let start_pos = node.start_position(); let end_pos = node.end_position(); - let id = generate_node_id( + let id = self.node_ids.generate( self.file_path, NodeKind::Function, &name, start_pos.row as u32 + 1, + utf16_column(&block.content, node.start_byte()), ); let qualified_name = format!("{}::{}", self.file_path, name); diff --git a/crates/codegraph-extract/src/walker.rs b/crates/codegraph-extract/src/walker.rs index 1223bed1..2df3504d 100644 --- a/crates/codegraph-extract/src/walker.rs +++ b/crates/codegraph-extract/src/walker.rs @@ -344,6 +344,19 @@ pub struct TreeSitterWalker<'a, 'tree> { /// Latched once a child would exceed [`MAX_WALK_DEPTH`]. All remaining /// descent stops and `extract` discards the partial graph. walk_aborted: bool, + /// Set while an exported store's initializer is walked for the store's + /// own calls; see [`StoreWalk`]. + store_walk: Option, +} + +/// The parts of a store initializer that its walk must not treat as the +/// store's own code. +struct StoreWalk { + /// The factory closure whose body returns the action object. It is walked + /// as the store's body, never bound as a function named after the store. + factory: Option, + /// The action values that became function nodes of their own. + actions: Vec, } impl<'a, 'tree> TreeSitterWalker<'a, 'tree> { @@ -372,6 +385,7 @@ impl<'a, 'tree> TreeSitterWalker<'a, 'tree> { cfml_consumed_until: 0, walk_depth: 0, walk_aborted: false, + store_walk: None, } } @@ -3113,10 +3127,23 @@ impl<'a, 'tree> TreeSitterWalker<'a, 'tree> { // An exported store — `create((set, get) => ({ reset: () => // set({}) }))`, middleware wrappers included — owns the // inline actions of the object its initializer returns - // (upstream #647/#1862). The initializer is not walked as a - // whole: that would re-attribute every action's calls to - // the store itself. - self.extract_js_object_literal_members(actions, owner_id); + // (upstream #647/#1862). + let actions_ids = self.extract_js_object_literal_members(actions, owner_id); + // The rest of the initializer runs when the store is built: + // the `create`/`persist` wrappers, their options, a block + // factory's setup. Those calls stay the store's (#693), as + // they were before the actions became nodes; upstream skips + // the whole initializer here and drops them. Only the + // extracted actions are skipped, so no action's calls are + // counted for the store as well. + let previous = self.store_walk.replace(StoreWalk { + factory: nearest_function_ancestor(actions).map(|factory| factory.id()), + actions: actions_ids, + }); + self.node_stack.push(owner_id.to_string()); + self.visit_function_body(value); + self.node_stack.pop(); + self.store_walk = previous; } else { match declared_id { Some(id) => { @@ -3152,7 +3179,14 @@ impl<'a, 'tree> TreeSitterWalker<'a, 'tree> { /// range is inside that holder. Nested objects are deliberately not recursed /// into, which lets resolution distinguish `api.run()` from /// `api.nested.run()` by direct range containment. - fn extract_js_object_literal_members(&mut self, object: SyntaxNode<'tree>, owner_id: &str) { + /// + /// Returns the tree node ids of the extracted member values. + fn extract_js_object_literal_members( + &mut self, + object: SyntaxNode<'tree>, + owner_id: &str, + ) -> Vec { + let mut extracted = Vec::new(); self.node_stack.push(owner_id.to_string()); for member in object.named_children(&mut object.walk()) { if member.kind() == "method_definition" { @@ -3163,6 +3197,7 @@ impl<'a, 'tree> TreeSitterWalker<'a, 'tree> { continue; }; self.extract_function(member, Some(name)); + extracted.push(member.id()); continue; } if member.kind() != "pair" { @@ -3179,6 +3214,7 @@ impl<'a, 'tree> TreeSitterWalker<'a, 'tree> { && let Some(bound) = self.curried_wrapper_bound_name(function) { self.extract_function(function, Some(bound)); + extracted.push(function.id()); } continue; } @@ -3197,8 +3233,10 @@ impl<'a, 'tree> TreeSitterWalker<'a, 'tree> { continue; }; self.extract_function(value, Some(name)); + extracted.push(value.id()); } self.node_stack.pop(); + extracted } fn extract_python_assignment(&mut self, node: SyntaxNode<'tree>) { @@ -4309,6 +4347,13 @@ impl<'a, 'tree> TreeSitterWalker<'a, 'tree> { if self.walk_aborted { return; } + if self + .store_walk + .as_ref() + .is_some_and(|walk| walk.actions.contains(&node.id())) + { + return; + } if self.walk_depth >= MAX_WALK_DEPTH { self.walk_aborted = true; return; @@ -4367,7 +4412,12 @@ impl<'a, 'tree> TreeSitterWalker<'a, 'tree> { self.maybe_cpp_construction(node); } - if has_type(self.spec.function_types(), node_type) { + if has_type(self.spec.function_types(), node_type) + && self + .store_walk + .as_ref() + .is_none_or(|walk| walk.factory != Some(node.id())) + { let nested_name = self.extract_name(node); if !nested_name.is_empty() && nested_name != "" { self.extract_function(node, None); @@ -5231,6 +5281,21 @@ fn initializer_returned_object(call: SyntaxNode<'_>, depth: usize) -> Option) -> Option> { + let mut current = node.parent(); + while let Some(candidate) = current { + if matches!( + candidate.kind(), + "arrow_function" | "function_expression" | "generator_function" + ) { + return Some(candidate); + } + current = candidate.parent(); + } + None +} + /// The object literal a function returns: the `=> ({ … })` arrow form or a /// top-level `return { … }` of a block body. fn function_returned_object(function: SyntaxNode<'_>) -> Option> { diff --git a/crates/codegraph-extract/tests/js_store_initializers.rs b/crates/codegraph-extract/tests/js_store_initializers.rs new file mode 100644 index 00000000..3f39e128 --- /dev/null +++ b/crates/codegraph-extract/tests/js_store_initializers.rs @@ -0,0 +1,94 @@ +//! A store initializer's own calls belong to the store (#693, #1862). +//! +//! `export const useStore = create(persist((set) => ({ reset: () => set({}) }), +//! opts))` makes each inline action a function node of its own. The rest of +//! the initializer, the `create`/`persist` wrappers and their options, runs +//! when the store is built, so its calls stay attributed to the store, as they +//! were before the actions became nodes. An action's calls are never counted +//! for the store as well. + +use codegraph_core::types::{EdgeKind, ExtractionResult, Language, NodeKind}; +use codegraph_extract::extract_source; + +fn extract(source: &str) -> ExtractionResult { + let result = extract_source("store.ts", source, Some(Language::TypeScript)); + assert!(result.errors.is_empty(), "{:?}", result.errors); + result +} + +/// Sorted names of the `calls` refs made by the node named by `qualified`. +fn calls_from(result: &ExtractionResult, qualified: &str) -> Vec { + let ids = result + .nodes + .iter() + .filter(|node| node.qualified_name == qualified) + .map(|node| node.id.as_str()) + .collect::>(); + assert_eq!(ids.len(), 1, "{qualified}: {:#?}", result.nodes); + let mut names = result + .unresolved_references + .iter() + .filter(|reference| { + reference.reference_kind == EdgeKind::Calls && reference.from_node_id == ids[0] + }) + .map(|reference| reference.reference_name.clone()) + .collect::>(); + names.sort(); + names +} + +#[test] +fn wrapper_and_option_calls_stay_with_the_store() { + let result = extract( + "import { create } from 'zustand'; +import { persist, createJSONStorage } from 'zustand/middleware'; +export const useStore = create( + persist( + (set, get) => ({ + count: initialCount(), + reset: () => set({ count: 0 }), + bump() { get().reset(); }, + }), + { name: 'app', storage: createJSONStorage(() => localStorage) }, + ), +); +", + ); + assert_eq!( + calls_from(&result, "useStore"), + vec!["create", "createJSONStorage", "initialCount", "persist"] + ); + assert_eq!(calls_from(&result, "useStore::reset"), vec!["set"]); + assert_eq!( + calls_from(&result, "useStore::bump"), + vec!["get", "get().reset"] + ); +} + +#[test] +fn a_curried_store_mints_no_function_for_its_factory() { + let result = extract( + "import { create } from 'zustand'; +interface S { bump(): void } +export const useCurried = create()((set) => { + const seed = loadSeed(); + return { bump: () => set({}) }; +}); +", + ); + assert!( + !result + .nodes + .iter() + .any(|node| node.name == "useCurried" && node.kind == NodeKind::Function), + "{:#?}", + result.nodes + ); + // The outer application is named by its callee text, as any curried + // call is. + assert_eq!( + calls_from(&result, "useCurried"), + vec!["create", "create()", "loadSeed"] + ); + assert_eq!(calls_from(&result, "useCurried::bump"), vec!["set"]); +} diff --git a/crates/codegraph-extract/tests/node_identity.rs b/crates/codegraph-extract/tests/node_identity.rs index b8ac6606..0aa8cad5 100644 --- a/crates/codegraph-extract/tests/node_identity.rs +++ b/crates/codegraph-extract/tests/node_identity.rs @@ -127,3 +127,37 @@ fn repeated_same_line_liquid_and_cfml_declarations_survive() { assert_eq!(methods.len(), 2, "{:#?}", cfml.nodes); assert_ne!(methods[0].id, methods[1].id); } + +#[test] +fn same_line_vue_script_functions_survive() { + // The Vue extractor mints its own script-block functions; ids use the + // block-relative line and column, as upstream's delegated extractor does. + let source = + "\n\n"; + let result = extract("Widget.vue", source, Language::Vue); + let functions = named(&result, "f", NodeKind::Function); + assert_eq!(functions.len(), 2, "{:#?}", result.nodes); + let legacy = generate_node_id("Widget.vue", NodeKind::Function, "f", 2); + let block_line = "function f() {} function f() {}"; + let second = utf16_column(block_line, block_line.rfind("function").unwrap()); + let mut ids = functions + .iter() + .map(|node| node.id.clone()) + .collect::>(); + ids.sort(); + assert_eq!(ids, vec![legacy.clone(), format!("{legacy}:{second}")]); + for node in functions { + assert!( + result + .edges + .iter() + .any(|edge| edge.kind == EdgeKind::Contains && edge.target == node.id), + "{} has no contains edge", + node.id + ); + } + assert_eq!( + extract("Widget.vue", source, Language::Vue).nodes, + result.nodes + ); +} diff --git a/crates/codegraph-mcp/src/engine.rs b/crates/codegraph-mcp/src/engine.rs index 9093d626..6904119c 100644 --- a/crates/codegraph-mcp/src/engine.rs +++ b/crates/codegraph-mcp/src/engine.rs @@ -5411,7 +5411,12 @@ const COMPLETENESS_RULE: &str = "---"; /// The trim note emitted at tiers that gate the completeness signal OFF. It is /// only reached when a file was actually trimmed, but `any_file_trimmed` is /// loop-determined, so the pre-loop reserve has to assume it. -const TRIMMED_NOTE: &str = "> Some file sections were trimmed for size. Elided symbols are named inside gap markers as `name (file:line)` and preferred in the file header — run another `codegraph_explore` (or `codegraph_node`) with those exact names for their source."; +/// +/// It never says every elided symbol is named: a gap marker names what it +/// hides only from the budget its file has spare (#2057), the tail marker after +/// a dropped or windowed cluster names nothing, and a dropped file appears only +/// in the pointer list. Upstream's wording claims the markers name them all. +const TRIMMED_NOTE: &str = "> Some file sections were trimmed for size. Gap markers name elided symbols as `name (file:line)` only where room allowed, and the file header prefers them — run another `codegraph_explore` (or `codegraph_node`) for their source."; /// One symbol a file section set out to deliver: a cluster member, or a node /// of a section that renders no source (#2077). Completeness is judged against @@ -5529,7 +5534,8 @@ fn complete_source_note(files: &str) -> String { ) } -const TRIMMED_NOTE_WHAT: &str = "gap markers and file headers name what was elided"; +/// Never "name what was elided", as upstream says: see [`TRIMMED_NOTE`]. +const TRIMMED_NOTE_WHAT: &str = "gap markers and headers name what room allowed"; const TRIMMED_NOTE_TAIL: &str = "For those, or anything under \"Not shown above\", make ANOTHER codegraph_explore with those exact names instead of reading the files — it returns their source with line numbers."; fn trimmed_note_head(files: &str) -> String { @@ -6547,6 +6553,21 @@ mod tests { assert!(completeness_note_floor(8) >= completeness_notes(0, &trimmed, &known)[2].len()); } + #[test] + fn trim_notes_never_claim_every_elided_symbol_is_named() { + let trimmed = vec![( + "src/a.ts".to_string(), + vec![wanted("f", 1, 9, 10, "function")], + )]; + let mut notes = completeness_notes(1, &trimmed, &["src/a.ts".to_string()]); + notes.push(TRIMMED_NOTE.to_string()); + for note in ¬es { + assert!(!note.contains("name what was elided"), "{note}"); + assert!(!note.contains("Elided symbols are named"), "{note}"); + assert!(note.contains("room allowed"), "{note}"); + } + } + #[test] fn the_truncation_note_drops_complete_for_a_trimmed_response() { let (complete, trimmed) = (truncation_note(false), truncation_note(true)); diff --git a/crates/codegraph-resolve/src/import_resolver.rs b/crates/codegraph-resolve/src/import_resolver.rs index e5ee6fee..d652dcae 100644 --- a/crates/codegraph-resolve/src/import_resolver.rs +++ b/crates/codegraph-resolve/src/import_resolver.rs @@ -1999,11 +1999,15 @@ fn find_exported_symbol( // Direct hit (import-resolver.ts:1829-1853). if want.is_default { - // A component file IS its default export; otherwise the binding an - // `export default NAME` statement names — a function, class, component, - // or a namespace object such as `const Api = { upload }` that is not - // exported at its declaration — beats the first-exported-function - // guess (upstream F13 / `defaultExportBindingNode`). + // The binding an `export default NAME` statement names — a function, + // class, component, or a namespace object such as `const Api = { upload + // }` that is not exported at its declaration — is the default export + // (upstream F13 / `defaultExportBindingNode`). Without one, a component + // file IS its default export, and otherwise the first exported function + // or class is the guess. Upstream tries the component first, but the + // React resolver mints component nodes inside ordinary modules too, and + // one of those is not the module's default export when a statement says + // which binding is. let bound_name = re_exports.iter().find_map(|rex| match rex { ReExport::LocalAlias { exported_name, @@ -2028,10 +2032,12 @@ fn find_exported_symbol( }) .min_by_key(|n| (n.start_line, n.start_column)) }; - if let Some(direct) = nodes_in_file - .iter() - .find(|n| n.is_exported && n.kind == NodeKind::Component) - .or_else(bound) + if let Some(direct) = bound() + .or_else(|| { + nodes_in_file + .iter() + .find(|n| n.is_exported && n.kind == NodeKind::Component) + }) .or_else(|| { nodes_in_file.iter().find(|n| { n.is_exported && matches!(n.kind, NodeKind::Function | NodeKind::Class) diff --git a/crates/codegraph-resolve/tests/js_receiver_evidence.rs b/crates/codegraph-resolve/tests/js_receiver_evidence.rs index 5bf1d9ac..89f7bd11 100644 --- a/crates/codegraph-resolve/tests/js_receiver_evidence.rs +++ b/crates/codegraph-resolve/tests/js_receiver_evidence.rs @@ -1417,3 +1417,36 @@ fn a_default_import_of_a_later_exported_const_finds_that_const() { // first exported function (`unrelated`). assert_eq!(project.callees("consume", None), vec!["useStore::read"]); } + +#[test] +fn an_explicit_default_binding_beats_an_exported_component() { + let project = resolve_project(&files(&[ + ( + "package.json", + "{\"name\":\"app\",\"dependencies\":{\"react\":\"18\"}}", + ), + ( + "src/upload.js", + "export function upload(uri) {\n return uri\n}\n", + ), + ( + "src/screen.js", + "import { upload } from './upload'\n\ + import { memo } from 'react'\n\ + export function Screen() {\n return
\n}\n\ + export const Card = memo(() =>
)\n\ + const Api = {\n upload,\n}\n\ + export default Api\n", + ), + ( + "src/use.js", + "import Api from './screen'\nexport function consume(uri) {\n return Api.upload(uri)\n}\n", + ), + ])); + // The React resolver makes `Screen` and `Card` component nodes, but a + // module's exported component is not its default export: the `export + // default NAME` statement says which binding is. Upstream prefers the + // first exported component, which made `consume` call `Screen` and lose + // its `upload` edge. + assert_eq!(project.callees("consume", None), vec!["upload"]); +} diff --git a/crates/codegraph-store/src/index_state.rs b/crates/codegraph-store/src/index_state.rs index bf082c18..c3be5473 100644 --- a/crates/codegraph-store/src/index_state.rs +++ b/crates/codegraph-store/src/index_state.rs @@ -115,7 +115,7 @@ pub const CURRENT_STORAGE_PROTOCOL: u64 = 2; /// The extraction-pipeline version this binary produces. This is the single /// source of truth: no other crate may define its own copy. -pub const CURRENT_EXTRACTION_VERSION: u64 = 17; +pub const CURRENT_EXTRACTION_VERSION: u64 = 18; /// The `project_metadata` key under which a built index records the extraction /// version it was produced with. Single source of truth for the key spelling. diff --git a/crates/codegraph-store/tests/index_state.rs b/crates/codegraph-store/tests/index_state.rs index 65e5e7cf..cc6a465e 100644 --- a/crates/codegraph-store/tests/index_state.rs +++ b/crates/codegraph-store/tests/index_state.rs @@ -119,7 +119,7 @@ fn assert_corrupt( fn index_state_constants_and_canonical_payload_are_exact() { /// The extraction version the immediately preceding release wrote. Read from /// `index_state.rs` at implementation time; see plan decision C1 step 0. - const PREVIOUS_EXTRACTION_VERSION: u64 = 16; + const PREVIOUS_EXTRACTION_VERSION: u64 = 17; assert_eq!(CURRENT_STORAGE_PROTOCOL, 2); assert_eq!(CURRENT_EXTRACTION_VERSION, PREVIOUS_EXTRACTION_VERSION + 1); diff --git a/docs/upstream-sync/UPSTREAM.md b/docs/upstream-sync/UPSTREAM.md index 94fe7c36..f619b278 100644 --- a/docs/upstream-sync/UPSTREAM.md +++ b/docs/upstream-sync/UPSTREAM.md @@ -59,7 +59,11 @@ below remain immutable historical evidence. > - **KEEP-RUST divergences** are recorded row by row in the audit — among them > a parameter-pack constructor declines its overload set (upstream admits it > unbounded), a self-closing `` is never a statement-map root, and a -> `this.a.b.m()` chain keeps its last segment. +> `this.a.b.m()` chain keeps its last segment. Three more came out of the +> retroactive review, in #291: a store initializer's own calls stay with the +> store, an explicit `export default NAME` binding beats an exported +> component, and a trim note never claims that gap markers name every elided +> symbol. See the 2026-10-01 retroactive-review entry below. > - The `1.5.0 → 1.6.0` and older caveats below carry forward unchanged. > **1.5.0 → 1.6.0 sync: COMPLETE** (codegraph-rs `v0.48.2` → `v0.50.1`). @@ -126,6 +130,74 @@ below remain immutable historical evidence. ## Sync log +### 2026-10-01 — Retroactive review of the `v1.6.1` port PRs: four port defects FIXED (#291) + +The owner asked for a kirocodex review of the large PRs that merged before +large changes needed one. The goal gate reviewed each merged commit against its +parent at `high` effort, with the PR description as the claims under review. +Then every blocking item was checked against `main` (`c00aa2b`) and upstream +`v1.6.1` (`f4ddf50`). + +| PR | Verdict | Blocking items | Outcome | +| ---- | ------- | -------------- | --------------------------------------------------------------------------- | +| #271 | PASS | 0 | — | +| #275 | PASS | 0 | — | +| #277 | FAIL | 2 | one fixed, one kept at upstream behavior | +| #278 | FAIL | 4 | two fixed, one kept at upstream behavior, one about evidence only | +| #279 | PASS | 0 | its one defect, the full-sync scope, was already fixed by #286 | +| #281 | FAIL | 1 | fixed | +| #282 | FAIL | 3 | all three kept at upstream behavior | + +**Fixed in #291, extraction version 18:** + +- **#1349 gap.** The Vue extractor mints its own script-block functions and + still called `generate_node_id` directly. Two same-name declarations on one + line therefore shared an id, and the store kept one row. Upstream reaches its + allocator by delegating Vue scripts to its tree-sitter extractor. The port now + uses `NodeIdAllocator` there, with the same block-relative line and UTF-16 + column. +- **KEEP-RUST: a store initializer's own calls stay with the store.** After + #277, an exported store's inline actions were function nodes, and the rest of + the initializer went unwalked. `create`, `persist` and their options' calls + were lost; the port had recorded them before #277. Upstream skips the + initializer the same way (`membersExtractedSeparately`). The walk now skips + only the extracted action values, and it treats the factory closure as the + store's body, so a curried `create()(...)` mints no function named after + the store. +- **KEEP-RUST: an explicit `export default NAME` binding beats an exported + component.** The React resolver mints component nodes inside `.js` and `.ts` + modules. Upstream's lookup (`defaultComponent ?? defaultExportBindingNode ?? + defaultFnClass`) bound a default import to the module's first exported + component. In a module exporting `function Screen() { return
}` + beside `const Api = { upload }; export default Api`, `Api.upload()` became a + call to `Screen`, and the `upload` edge was lost. +- **KEEP-RUST: the trim note never claims that gap markers name every elided + symbol.** A gap marker names what it hides only from the budget its file has + spare. The tail marker after a dropped or windowed cluster names nothing, and + a dropped file appears only in the pointer list. Both tiers' notes, like + upstream's (`tools.ts:1302`, `:7272`), still said the markers named what was + elided. They now say the markers name what room allowed. + +**Kept at upstream behavior.** Each of these review findings describes +upstream's own design, and in each case the upstream source says why: + +- **#277:** `.getState().member()` binds for a store built by any factory, not + only Zustand's `create`. Upstream's `matchStoreAccessorChain` checks Zustand + provenance only for selectors. The member must still sit in that holder's own + object literal. A provenance gate would also drop `zustand/vanilla`, wrapped + and locally re-exported stores. +- **#278:** an unknown receiver falls back to a project-unique method name + ("Unknown receivers retain the old unique-or-drop discipline"). This + predates #278. +- **#282:** `#if` reads a literal, one `defined()` or a bare name, so a compound + condition is unknown and keeps the call. `#ifndef X` followed by an empty + `#define X` reads as the guard idiom (`guardsItself`). Any `initializer_list` + constructor declines its overload set, because the reference carries arity, + not the brace or paren form. + +The fourth #278 item asked for an exact-head evidence artifact. #278 merged +after CI run `36798356332` passed on a tree-identical head. + ### 2026-10-01 — `v0.52.0` RELEASED: #286, #288 and #289 shipped Release-please PR [#287](https://github.com/sunerpy/codegraph-rust/pull/287)