Skip to content

Commit 436791b

Browse files
authored
fix: address the retroactive review findings on the v1.6.1 port (#291)
The retroactive kirocodex review of the v1.6.1 port PRs found four port defects, and this fixes them. - Same-line Vue script functions get distinct ids (#1349). - An exported store initializer's own calls (create, persist, their options) stay with the store. - A default import binds the `export default NAME` binding before an exported component. - Explore trim notes say gap markers name what room allowed, instead of claiming they name every elided symbol. The extraction version moves to 18. The UPSTREAM.md entry records the review, and the findings kept at upstream behavior with upstream's reasons. BEGIN_COMMIT_OVERRIDE fix(extract): give same-line Vue script functions distinct ids (#1349) fix(extract): keep a store initializer's own calls on the store fix(resolve): prefer an explicit default binding over an exported component fix(mcp): never claim that gap markers name every elided symbol END_COMMIT_OVERRIDE
1 parent c00aa2b commit 436791b

12 files changed

Lines changed: 626 additions & 22 deletions

File tree

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
//! The trim note never claims a gap marker named what it could not afford to
2+
//! (#1711, #2077).
3+
//!
4+
//! A gap between two rendered windows names the indexed symbols it hides, but
5+
//! only from the budget the file has spare; a name that costs more stays out
6+
//! and the marker is a bare `... (gap) ...`. The note closing the response
7+
//! then said the gap markers named what was elided. Here an index-only helper
8+
//! whose name no budget affords sits between the two windows.
9+
10+
use std::path::{Path, PathBuf};
11+
use std::process::Command;
12+
13+
fn bin() -> PathBuf {
14+
PathBuf::from(env!("CARGO_BIN_EXE_codegraph"))
15+
}
16+
17+
struct TestDir {
18+
path: PathBuf,
19+
}
20+
21+
impl TestDir {
22+
fn new(label: &str) -> Self {
23+
let path = std::env::temp_dir().join(format!(
24+
"codegraph-cli-unnamed-gap-{label}-{}-{}",
25+
std::process::id(),
26+
std::time::SystemTime::now()
27+
.duration_since(std::time::UNIX_EPOCH)
28+
.unwrap()
29+
.as_nanos()
30+
));
31+
std::fs::create_dir_all(&path).unwrap();
32+
Self { path }
33+
}
34+
}
35+
36+
impl Drop for TestDir {
37+
fn drop(&mut self) {
38+
let _ = std::fs::remove_dir_all(&self.path);
39+
}
40+
}
41+
42+
fn run_in(cwd: &Path, args: &[&str]) -> String {
43+
let output = Command::new(bin())
44+
.args(args)
45+
.current_dir(cwd)
46+
.env("CODEGRAPH_NO_DAEMON", "1")
47+
.output()
48+
.expect("run codegraph binary");
49+
assert!(
50+
output.status.success(),
51+
"{args:?} failed: {}{}",
52+
String::from_utf8_lossy(&output.stdout),
53+
String::from_utf8_lossy(&output.stderr)
54+
);
55+
String::from_utf8_lossy(&output.stdout).into_owned()
56+
}
57+
58+
/// What follows the last source fence: the epilogue.
59+
fn epilogue_of(text: &str) -> &str {
60+
text.rfind("```").map_or(text, |i| &text[i + 3..])
61+
}
62+
63+
/// `src/big.ts`: `alphaEntry` at the top, then (when `helper_x` is given) an
64+
/// index-only helper named `helper` plus that many X's, then `betaEntry`, whose
65+
/// body runs `beta_lines` lines. `padding` small files lift the project into
66+
/// the large tiers.
67+
fn project(
68+
label: &str,
69+
helper_x: Option<usize>,
70+
beta_lines: usize,
71+
padding: usize,
72+
) -> (TestDir, PathBuf) {
73+
let dir = TestDir::new(label);
74+
let root = dir.path.join("app");
75+
let src = root.join("src");
76+
std::fs::create_dir_all(&src).unwrap();
77+
let mut body: Vec<String> = (1..10).map(|i| format!("// header {i}")).collect();
78+
body.push("export function alphaEntry(): number {".to_string());
79+
body.extend((0..19).map(|i| format!(" const a{i} = {i};")));
80+
body.push(" return betaEntry(1);".to_string());
81+
body.push("}".to_string());
82+
while body.len() < 99 {
83+
body.push(format!("// filler {}", body.len() + 1));
84+
}
85+
if let Some(x) = helper_x {
86+
let helper = format!("helper{}", "X".repeat(x));
87+
body.push(format!("function {helper}(): number {{ return 1; }}"));
88+
}
89+
while body.len() < 199 {
90+
body.push(format!("// filler {}", body.len() + 1));
91+
}
92+
body.push("export function betaEntry(v: number): number {".to_string());
93+
body.extend((0..beta_lines).map(|i| format!(" v = v + {i}; // beta stage {i}")));
94+
body.push(" return v;".to_string());
95+
body.push("}".to_string());
96+
while body.len() < 400 {
97+
body.push(format!("// filler {}", body.len() + 1));
98+
}
99+
std::fs::write(src.join("big.ts"), body.join("\n")).unwrap();
100+
if padding > 0 {
101+
let pad = root.join("pad");
102+
std::fs::create_dir_all(&pad).unwrap();
103+
for i in 0..padding {
104+
std::fs::write(
105+
pad.join(format!("pad{i}.ts")),
106+
format!("export const pad{i} = {i};\n"),
107+
)
108+
.unwrap();
109+
}
110+
}
111+
run_in(&dir.path, &["init", root.to_str().unwrap()]);
112+
(dir, root)
113+
}
114+
115+
#[test]
116+
fn small_tier_note_does_not_claim_an_unaffordable_gap_name() {
117+
// No per-file budget affords an 8000-character name.
118+
let (_dir, root) = project("small", Some(8000), 19, 0);
119+
let text = run_in(&root, &["explore", "alphaEntry betaEntry"]);
120+
// The fixture does what it is for: both windows render around a bare gap.
121+
assert!(text.contains("export function alphaEntry"), "{text}");
122+
assert!(text.contains("export function betaEntry"), "{text}");
123+
assert!(text.contains("... (gap) ..."), "{text}");
124+
assert!(!text.contains("XXXXXXXX"), "{text}");
125+
let epilogue = epilogue_of(&text);
126+
assert!(
127+
epilogue.contains("Some file sections were trimmed for size"),
128+
"{epilogue}"
129+
);
130+
assert!(
131+
!epilogue.contains("Elided symbols are named inside gap markers"),
132+
"{epilogue}"
133+
);
134+
assert!(epilogue.contains("only where room allowed"), "{epilogue}");
135+
}
136+
137+
#[test]
138+
fn large_tier_note_does_not_claim_an_unaffordable_gap_name() {
139+
let (_dir, root) = project("large", Some(8000), 900, 520);
140+
let text = run_in(&root, &["explore", "alphaEntry betaEntry"]);
141+
assert!(text.contains("export function alphaEntry"), "{text}");
142+
assert!(text.contains("... (gap) ..."), "{text}");
143+
assert!(!text.contains("XXXXXXXX"), "{text}");
144+
let epilogue = epilogue_of(&text);
145+
// `betaEntry` is windowed, so the section is reported trimmed.
146+
assert!(epilogue.contains("Verbatim source for"), "{epilogue}");
147+
assert!(!epilogue.contains("name what was elided"), "{epilogue}");
148+
assert!(epilogue.contains("name what room allowed"), "{epilogue}");
149+
}
150+
151+
/// The section a windowed `betaEntry` leaves ends in a bare tail marker: what
152+
/// it cut is never named in a gap marker, whatever the budget. Nothing indexed
153+
/// sits between the two functions, so no inner gap had a name to withhold.
154+
fn assert_trailing_bare_gap(text: &str) {
155+
assert!(text.contains("export function betaEntry"), "{text}");
156+
assert!(text.contains("... (gap) ...\n```"), "{text}");
157+
assert!(!text.contains("(gap: "), "{text}");
158+
}
159+
160+
#[test]
161+
fn small_tier_note_does_not_claim_names_for_a_trailing_gap() {
162+
let (_dir, root) = project("small-tail", None, 900, 0);
163+
let text = run_in(&root, &["explore", "alphaEntry betaEntry"]);
164+
assert_trailing_bare_gap(&text);
165+
let epilogue = epilogue_of(&text);
166+
assert!(
167+
epilogue.contains("Some file sections were trimmed for size"),
168+
"{epilogue}"
169+
);
170+
assert!(
171+
!epilogue.contains("Elided symbols are named inside gap markers"),
172+
"{epilogue}"
173+
);
174+
}
175+
176+
#[test]
177+
fn large_tier_note_does_not_claim_names_for_a_trailing_gap() {
178+
let (_dir, root) = project("large-tail", None, 900, 520);
179+
let text = run_in(&root, &["explore", "alphaEntry betaEntry"]);
180+
assert_trailing_bare_gap(&text);
181+
let epilogue = epilogue_of(&text);
182+
assert!(epilogue.contains("Verbatim source for"), "{epilogue}");
183+
assert!(!epilogue.contains("name what was elided"), "{epilogue}");
184+
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
//! Same-line namesakes survive the store, not just extraction (#1349).
2+
//!
3+
//! Nodes are upserted by id, so two declarations sharing one id leave a single
4+
//! row behind. The Vue extractor mints its script functions itself; this
5+
//! indexes a real project and reads the rows back.
6+
7+
use std::fs;
8+
use std::path::{Path, PathBuf};
9+
use std::process::Command;
10+
use std::time::{Duration, Instant};
11+
12+
use codegraph_core::IndexPaths;
13+
use codegraph_core::types::NodeKind;
14+
use codegraph_store::Store;
15+
16+
struct TestDir {
17+
path: PathBuf,
18+
}
19+
20+
impl TestDir {
21+
fn new(label: &str) -> Self {
22+
let path = std::env::temp_dir().join(format!(
23+
"codegraph-cli-same-line-identity-{label}-{}-{}",
24+
std::process::id(),
25+
std::time::SystemTime::now()
26+
.duration_since(std::time::UNIX_EPOCH)
27+
.unwrap()
28+
.as_nanos()
29+
));
30+
fs::create_dir_all(&path).unwrap();
31+
Self { path }
32+
}
33+
}
34+
35+
impl Drop for TestDir {
36+
fn drop(&mut self) {
37+
let _ = fs::remove_dir_all(&self.path);
38+
}
39+
}
40+
41+
fn cli(args: &[&str]) {
42+
let output = Command::new(env!("CARGO_BIN_EXE_codegraph"))
43+
.args(args)
44+
.env("CODEGRAPH_NO_DAEMON", "1")
45+
.output()
46+
.expect("run codegraph binary");
47+
assert!(
48+
output.status.success(),
49+
"codegraph {args:?} failed: stdout={} stderr={}",
50+
String::from_utf8_lossy(&output.stdout),
51+
String::from_utf8_lossy(&output.stderr)
52+
);
53+
}
54+
55+
fn store(project: &Path) -> Store {
56+
let paths = IndexPaths::resolve(project, None).expect("resolve index paths");
57+
Store::open_for_read(&paths, Instant::now() + Duration::from_secs(30), || false)
58+
.expect("open the index for reading")
59+
}
60+
61+
#[test]
62+
fn same_line_vue_functions_are_two_rows() {
63+
let dir = TestDir::new("vue");
64+
let project = dir.path.join("proj");
65+
fs::create_dir_all(&project).unwrap();
66+
fs::write(
67+
project.join("Widget.vue"),
68+
"<template><div/></template>\n<script>\nfunction f() {} function f() {}\n</script>\n",
69+
)
70+
.unwrap();
71+
let p = project.to_str().unwrap();
72+
cli(&["init", p]);
73+
let rows = |project: &Path| {
74+
let mut rows = store(project)
75+
.all_nodes()
76+
.unwrap()
77+
.into_iter()
78+
.filter(|node| node.kind == NodeKind::Function && node.name == "f")
79+
.map(|node| (node.id, node.start_line, node.start_column))
80+
.collect::<Vec<_>>();
81+
rows.sort();
82+
rows
83+
};
84+
let indexed = rows(&project);
85+
assert_eq!(indexed.len(), 2, "{indexed:#?}");
86+
assert_ne!(indexed[0].0, indexed[1].0);
87+
cli(&["sync", p]);
88+
assert_eq!(rows(&project), indexed, "sync keeps both rows");
89+
cli(&["index", "--force", p]);
90+
assert_eq!(rows(&project), indexed, "index --force keeps both rows");
91+
}

‎crates/codegraph-extract/src/embedded/vue.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use codegraph_core::node_id::generate_node_id;
1+
use codegraph_core::node_id::{NodeIdAllocator, generate_node_id, utf16_column};
22
use codegraph_core::types::{
33
Edge, EdgeKind, ExtractionResult, Language, Node, NodeKind, UnresolvedRef,
44
};
@@ -51,6 +51,8 @@ pub struct VueExtractor<'a> {
5151
edges: Vec<Edge>,
5252
unresolved_references: Vec<UnresolvedRef>,
5353
errors: Vec<String>,
54+
/// Same-line namesakes keep distinct ids across every script block (#1349).
55+
node_ids: NodeIdAllocator,
5456
}
5557

5658
impl<'a> VueExtractor<'a> {
@@ -62,6 +64,7 @@ impl<'a> VueExtractor<'a> {
6264
edges: Vec::new(),
6365
unresolved_references: Vec::new(),
6466
errors: Vec::new(),
67+
node_ids: NodeIdAllocator::default(),
6568
}
6669
}
6770

@@ -204,11 +207,12 @@ impl<'a> VueExtractor<'a> {
204207
let start_pos = node.start_position();
205208
let end_pos = node.end_position();
206209

207-
let id = generate_node_id(
210+
let id = self.node_ids.generate(
208211
self.file_path,
209212
NodeKind::Function,
210213
&name,
211214
start_pos.row as u32 + 1,
215+
utf16_column(&block.content, node.start_byte()),
212216
);
213217
let qualified_name = format!("{}::{}", self.file_path, name);
214218

0 commit comments

Comments
 (0)