Fix six open issues: stale imports, MCP validation, import-graph gaps, dup importers, I/O errors, scope docs

- pipeline: re-resolve imports for every file on update (not just changed),
  so add/remove/rename of a target fixes unchanged importers; propagate hash
  and read errors instead of treating unreadable files as empty
- discovery: report unreadable walk entries to stderr instead of dropping them
- imports: strip `pub` visibility so `pub use` re-exports resolve; share
  extension/module-entry probe lists (adds cc,cxx,hpp,hh,hxx,mts,cts,jsx,mjs,cjs)
- store: DISTINCT + ORDER in importers_of to dedup repeated edges
- mcp: validate required tool args (explicit isError), emit JSON-RPC -32700
  parse errors and -32600 invalid-request instead of silently skipping
- README: document the 13-tool surface vs toktoken's 27 and known gaps
- tests: regression coverage for update re-resolution, pub-use re-exports,
  importer dedup, and MCP error reporting

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
endergebnisandCopilot committed 2026-09-04 18:49:06 +02:00
1 parent 6875998ab6
commit 4338e28ce0
7 files changed
+271 -49

No files matched your search

+23
View File
@@ -138,6 +138,29 @@ Notes:
- Hardware cycle counters were not available (`perf` tools missing on this - Hardware cycle counters were not available (`perf` tools missing on this
kernel), so cycle figures are estimates: CPU time × 3.2 GHz nominal. kernel), so cycle figures are estimates: CPU time × 3.2 GHz nominal.
## Scope and known gaps
`rustytokens` covers the same *core* retrieval surface for Java/Python/Rust/
C/C++/HTML as `toktoken`, but intentionally implements a smaller tool set: 13
tools vs `toktoken`'s 27. The following `toktoken` tools have **no equivalent
here yet** (deliberately, not an oversight):
- `index_file` (reindex a single file)
- `find_callers`, `find_references` (cross-reference search)
- `inspect_bundle`, `inspect_dependencies`, `inspect_hierarchy` (graph views)
- `search_similar`, `search_cooccurrence` (similarity search)
- GitHub indexing and centrality ranking
Add them only if the agent workflow actually needs them; they are not required
for the "roughly cover Java/Python/Rust/C/C++/HTML" goal.
Known limits that are by design, not bugs:
- `.css` files are discovered and import-tracked, but `ctags` has no CSS
parser — `search symbols` returns nothing for `.css`.
- `find dead` over-counts for C/C++ header-only entry points (documented
conservatism); treat results as candidates, not verdicts.
## Development ## Development
```bash ```bash
+9 -1
View File
@@ -32,10 +32,15 @@ pub fn discover(project: &Path) -> Result<Vec<SourceFile>> {
}); });
let mut out = Vec::new(); let mut out = Vec::new();
let mut skipped = Vec::new();
for entry in builder.build() { for entry in builder.build() {
let entry = match entry { let entry = match entry {
Ok(e) => e, Ok(e) => e,
Err(_) => continue, Err(e) => {
// Report unreadable entries instead of dropping them silently.
skipped.push(e.to_string());
continue;
}
}; };
let path = entry.path(); let path = entry.path();
if entry.file_type().is_some_and(|t| t.is_dir()) { if entry.file_type().is_some_and(|t| t.is_dir()) {
@@ -54,6 +59,9 @@ pub fn discover(project: &Path) -> Result<Vec<SourceFile>> {
language, language,
}); });
} }
for s in &skipped {
eprintln!("rustytokens: skipped unreadable entry: {s}");
}
out.sort_by(|a, b| a.rel.cmp(&b.rel)); out.sort_by(|a, b| a.rel.cmp(&b.rel));
Ok(out) Ok(out)
} }
+28 -5
View File
@@ -75,10 +75,22 @@ fn extract_python(content: &str) -> Vec<Import> {
out out
} }
/// Strip a leading visibility modifier (`pub`, `pub(crate)`, `pub(super)`, …)
/// so `pub use`/`pub mod` parse the same as their private counterparts.
fn strip_visibility(l: &str) -> &str {
if let Some(r) = l.strip_prefix("pub ") {
r
} else if let Some(r) = l.strip_prefix("pub(") {
r.find(") ").map(|i| &r[i + 2..]).unwrap_or(l)
} else {
l
}
}
fn extract_rust(content: &str) -> Vec<Import> { fn extract_rust(content: &str) -> Vec<Import> {
let mut out = Vec::new(); let mut out = Vec::new();
for line in content.lines() { for line in content.lines() {
let l = line.trim(); let l = strip_visibility(line.trim());
if let Some(rest) = l.strip_prefix("use ") { if let Some(rest) = l.strip_prefix("use ") {
let rest = rest let rest = rest
.trim_end_matches(';') .trim_end_matches(';')
@@ -293,6 +305,17 @@ fn clean(target: &str) -> Option<String> {
Some(t.to_string()) Some(t.to_string())
} }
/// Code file extensions probed when resolving extensionless import targets.
/// Mirrors `config::language_for` for code languages (html/css assets are
/// always imported with an explicit extension, so they need no probing).
const RESOLVE_EXTS: &[&str] = &[
"rs", "py", "ts", "tsx", "mts", "cts", "js", "jsx", "mjs", "cjs", "c", "h", "cc", "cpp", "cxx",
"hpp", "hh", "hxx", "java",
];
/// Module-directory entry files tried for `dir/mod` style resolution.
const MODULE_ENTRIES: &[&str] = &["mod.rs", "__init__.py", "index.ts", "index.js", "index.tsx"];
impl Resolver { impl Resolver {
pub fn build(files: Vec<(String, i64)>, crate_name: Option<&str>) -> Self { pub fn build(files: Vec<(String, i64)>, crate_name: Option<&str>) -> Self {
let mut by_path = HashMap::new(); let mut by_path = HashMap::new();
@@ -380,12 +403,12 @@ impl Resolver {
if let Some(id) = self.by_path.get(&format!("{base}{t}")) { if let Some(id) = self.by_path.get(&format!("{base}{t}")) {
return Some(*id); return Some(*id);
} }
for ext in ["rs", "py", "ts", "tsx", "js", "cpp", "c", "h", "java"] { for ext in RESOLVE_EXTS {
if let Some(id) = self.by_path.get(&format!("{base}{t}.{ext}")) { if let Some(id) = self.by_path.get(&format!("{base}{t}.{ext}")) {
return Some(*id); return Some(*id);
} }
} }
for entry in ["mod.rs", "__init__.py", "index.ts", "index.js", "index.tsx"] { for entry in MODULE_ENTRIES {
if let Some(id) = self.by_path.get(&format!("{base}{t}/{entry}")) { if let Some(id) = self.by_path.get(&format!("{base}{t}/{entry}")) {
return Some(*id); return Some(*id);
} }
@@ -394,12 +417,12 @@ impl Resolver {
} }
fn try_module(&self, base: &str) -> Option<i64> { fn try_module(&self, base: &str) -> Option<i64> {
for ext in ["rs", "py", "ts", "tsx", "js", "cpp", "c", "h", "java"] { for ext in RESOLVE_EXTS {
if let Some(id) = self.by_path.get(&format!("{base}.{ext}")) { if let Some(id) = self.by_path.get(&format!("{base}.{ext}")) {
return Some(*id); return Some(*id);
} }
} }
for entry in ["mod.rs", "__init__.py", "index.ts", "index.js", "index.tsx"] { for entry in MODULE_ENTRIES {
if let Some(id) = self.by_path.get(&format!("{base}/{entry}")) { if let Some(id) = self.by_path.get(&format!("{base}/{entry}")) {
return Some(*id); return Some(*id);
} }
+23 -10
View File
@@ -56,15 +56,17 @@ pub fn run(project: &Path, db_path: &Path, recreate: bool) -> Result<IndexStats>
.map(|f| (f.path.clone(), (f.id, f.content_hash.clone()))) .map(|f| (f.path.clone(), (f.id, f.content_hash.clone())))
.collect(); .collect();
// Hash all files in parallel. // Hash all files in parallel. Propagate read errors instead of silently
// treating an unreadable file as an empty (all-zero) hash.
let hashed: Vec<(String, String, i64, &'static str)> = files let hashed: Vec<(String, String, i64, &'static str)> = files
.par_iter() .par_iter()
.map(|f| { .map(|f| {
let h = hashing::hash_file(&f.path).unwrap_or_else(|_| "0000000000000000".to_string()); let h = hashing::hash_file(&f.path)
.with_context(|| format!("cannot hash {}", f.path.display()))?;
let m = hashing::mtime_secs(&f.path); let m = hashing::mtime_secs(&f.path);
(f.rel.clone(), h, m, f.language) Ok((f.rel.clone(), h, m, f.language))
}) })
.collect(); .collect::<Result<Vec<_>>>()?;
let by_rel: HashMap<&str, &SourceFile> = files.iter().map(|f| (f.rel.as_str(), f)).collect(); let by_rel: HashMap<&str, &SourceFile> = files.iter().map(|f| (f.rel.as_str(), f)).collect();
let abs_to_rel: HashMap<String, String> = files let abs_to_rel: HashMap<String, String> = files
@@ -112,11 +114,27 @@ pub fn run(project: &Path, db_path: &Path, recreate: bool) -> Result<IndexStats>
read_crate_name(project).as_deref(), read_crate_name(project).as_deref(),
); );
// Symbols + content FTS: only files whose content changed. Skipping ctags
// for unchanged files is the whole point of the hash optimization.
for f in &changed { for f in &changed {
let fid = file_ids[&f.rel]; let fid = file_ids[&f.rel];
let content = std::fs::read_to_string(&f.path).unwrap_or_default(); let content = std::fs::read_to_string(&f.path)
.with_context(|| format!("cannot read {}", f.path.display()))?;
let tags = tags_by_rel.get(&f.rel).cloned().unwrap_or_default(); let tags = tags_by_rel.get(&f.rel).cloned().unwrap_or_default();
store::replace_symbols(&conn, fid, &tags_to_symbols(&tags, &content))?; store::replace_symbols(&conn, fid, &tags_to_symbols(&tags, &content))?;
conn.execute("DELETE FROM content_fts WHERE rowid = ?1", [fid])?;
conn.execute(
"INSERT INTO content_fts(rowid, path, content) VALUES (?1, ?2, ?3)",
params![fid, f.rel, content],
)?;
}
// Imports: re-resolve for EVERY file. Adding/removing/renaming a file can
// change whether an unchanged importer resolves, so resolution must cover
// the whole file set, not just the changed set. (Cheap: read + regex, no
// ctags subprocess.)
for f in &files {
let fid = file_ids[&f.rel];
store::clear_imports_for_file(&conn, fid)?; store::clear_imports_for_file(&conn, fid)?;
let dir = f let dir = f
.rel .rel
@@ -131,11 +149,6 @@ pub fn run(project: &Path, db_path: &Path, recreate: bool) -> Result<IndexStats>
store::insert_import(&conn, fid, to, &imp.import_type)?; store::insert_import(&conn, fid, to, &imp.import_type)?;
} }
} }
conn.execute("DELETE FROM content_fts WHERE rowid = ?1", [fid])?;
conn.execute(
"INSERT INTO content_fts(rowid, path, content) VALUES (?1, ?2, ?3)",
params![fid, f.rel, content],
)?;
} }
conn.execute_batch("COMMIT;")?; conn.execute_batch("COMMIT;")?;
+3 -1
View File
@@ -200,7 +200,9 @@ pub fn symbols_for_file(conn: &Connection, file_id: i64) -> Result<Vec<SymbolRow
} }
pub fn importers_of(conn: &Connection, file_id: i64) -> Result<Vec<i64>> { pub fn importers_of(conn: &Connection, file_id: i64) -> Result<Vec<i64>> {
let mut stmt = conn.prepare("SELECT from_file_id FROM imports WHERE to_file_id = ?1")?; let mut stmt = conn.prepare(
"SELECT DISTINCT from_file_id FROM imports WHERE to_file_id = ?1 ORDER BY from_file_id",
)?;
let rows = stmt.query_map([file_id], |r| r.get(0))?; let rows = stmt.query_map([file_id], |r| r.get(0))?;
Ok(rows.collect::<std::result::Result<Vec<_>, _>>()?) Ok(rows.collect::<std::result::Result<Vec<_>, _>>()?)
} }
+54 -31
View File
@@ -22,7 +22,22 @@ pub fn serve(project: &Path) -> Result<()> {
} }
let msg: Value = match serde_json::from_str(&line) { let msg: Value = match serde_json::from_str(&line) {
Ok(m) => m, Ok(m) => m,
Err(_) => continue, Err(_) => {
// Unparseable input is a JSON-RPC parse error, not something to
// drop silently.
let mut out = stdout.lock();
serde_json::to_writer(
&mut out,
&json!({
"jsonrpc": "2.0",
"id": null,
"error": { "code": -32700, "message": "parse error" }
}),
)?;
out.write_all(b"\n")?;
out.flush()?;
continue;
}
}; };
if let Some(resp) = handle(&msg, project) { if let Some(resp) = handle(&msg, project) {
let mut out = stdout.lock(); let mut out = stdout.lock();
@@ -35,12 +50,20 @@ pub fn serve(project: &Path) -> Result<()> {
} }
fn handle(msg: &Value, project: &Path) -> Option<Value> { fn handle(msg: &Value, project: &Path) -> Option<Value> {
let method = msg.get("method")?.as_str()?;
let id = msg.get("id").cloned(); let id = msg.get("id").cloned();
// Notifications have no id -> no response. // Notifications have no id -> no response.
let id = id.as_ref()?; let id = id.as_ref()?;
// Requests must carry a string method name.
let Some(method) = msg.get("method").and_then(|m| m.as_str()) else {
return Some(json!({
"jsonrpc": "2.0",
"id": id,
"error": { "code": -32600, "message": "invalid request" }
}));
};
match method { match method {
"initialize" => Some(json!({ "initialize" => Some(json!({
"jsonrpc": "2.0", "jsonrpc": "2.0",
@@ -93,38 +116,38 @@ fn call_tool(name: &str, args: &Value, project: &Path) -> Value {
fn dispatch(name: &str, args: &Value, project: &Path) -> Result<Value> { fn dispatch(name: &str, args: &Value, project: &Path) -> Result<Value> {
use crate::ops; use crate::ops;
let s = |k: &str| args.get(k).and_then(|v| v.as_str()).unwrap_or("");
fn req_str<'a>(args: &'a Value, key: &str, tool: &str) -> Result<&'a str> {
args.get(key)
.and_then(|v| v.as_str())
.filter(|s| !s.is_empty())
.ok_or_else(|| anyhow::anyhow!("{tool}: missing or invalid argument '{key}'"))
}
fn req_i64(args: &Value, key: &str, tool: &str) -> Result<i64> {
args.get(key)
.and_then(|v| v.as_i64())
.ok_or_else(|| anyhow::anyhow!("{tool}: missing or invalid argument '{key}'"))
}
let opt_str = |k: &str| args.get(k).and_then(|v| v.as_str());
let opt_depth = |k: &str| args.get(k).and_then(|v| v.as_u64()).map(|d| d as usize);
match name { match name {
"index_create" => ops::index_create(Path::new(s("path"))), "index_create" => ops::index_create(Path::new(req_str(args, "path", name)?)),
"index_update" => ops::index_update(Path::new(s("path"))), "index_update" => ops::index_update(Path::new(req_str(args, "path", name)?)),
"search_symbols" => ops::search_symbols(project, s("query")), "search_symbols" => ops::search_symbols(project, req_str(args, "query", name)?),
"search_text" => ops::search_text(project, s("query")), "search_text" => ops::search_text(project, req_str(args, "query", name)?),
"inspect_symbol" => ops::inspect_symbol( "inspect_symbol" => ops::inspect_symbol(project, req_i64(args, "id", name)?),
project, "inspect_outline" => ops::inspect_outline(project, req_str(args, "file", name)?),
args.get("id").and_then(|v| v.as_i64()).unwrap_or(0), "inspect_file" => {
), ops::inspect_file(project, req_str(args, "file", name)?, opt_str("lines"))
"inspect_outline" => ops::inspect_outline(project, s("file")), }
"inspect_file" => ops::inspect_file( "inspect_tree" => ops::inspect_tree(project, opt_depth("depth")),
project, "find_importers" => ops::find_importers(project, req_str(args, "file", name)?),
s("file"),
args.get("lines").and_then(|v| v.as_str()),
),
"inspect_tree" => ops::inspect_tree(
project,
args.get("depth")
.and_then(|v| v.as_u64())
.map(|d| d as usize),
),
"find_importers" => ops::find_importers(project, s("file")),
"find_dead" => ops::find_dead(project), "find_dead" => ops::find_dead(project),
"inspect_cycles" => ops::inspect_cycles(project), "inspect_cycles" => ops::inspect_cycles(project),
"inspect_blast_radius" => ops::inspect_blast_radius( "inspect_blast_radius" => {
project, ops::inspect_blast_radius(project, req_str(args, "file", name)?, opt_depth("depth"))
s("file"), }
args.get("depth")
.and_then(|v| v.as_u64())
.map(|d| d as usize),
),
"stats" => ops::stats(project), "stats" => ops::stats(project),
_ => anyhow::bail!("unknown tool: {name}"), _ => anyhow::bail!("unknown tool: {name}"),
} }
+131 -1
View File
@@ -144,7 +144,10 @@ fn inspect_blast_radius_finds_affected_files() {
.collect(); .collect();
// lib.rs imports util.rs directly; main.rs reaches it via the crate name. // lib.rs imports util.rs directly; main.rs reaches it via the crate name.
assert!(affected.contains(&"src/lib.rs"), "lib.rs imports util.rs"); assert!(affected.contains(&"src/lib.rs"), "lib.rs imports util.rs");
assert!(affected.contains(&"src/main.rs"), "main.rs → lib.rs → util.rs"); assert!(
affected.contains(&"src/main.rs"),
"main.rs → lib.rs → util.rs"
);
} }
#[test] #[test]
@@ -496,3 +499,130 @@ fn index_update_reindexes_changed_files() {
"index update picks up the new symbol" "index update picks up the new symbol"
); );
} }
#[test]
fn index_update_reimports_unchanged_file_and_dedups() {
let tmp = cache_dir().join("update-reimport");
let _ = std::fs::remove_dir_all(&tmp);
let src = tmp.join("src");
std::fs::create_dir_all(&src).unwrap();
// Two `use` lines targeting the same (not-yet-existing) module.
std::fs::write(
src.join("main.rs"),
"use util::hello;\nuse util::world;\n\nfn main() {}\n",
)
.unwrap();
run_json(&["index", "create", tmp.to_str().unwrap()]);
std::fs::write(
src.join("util.rs"),
"pub fn hello() {}\npub fn world() {}\n",
)
.unwrap();
run_json(&["index", "update", tmp.to_str().unwrap()]);
let after = run_json(&[
"--project",
tmp.to_str().unwrap(),
"find",
"importers",
"src/util.rs",
]);
let names: Vec<&str> = after
.as_array()
.unwrap()
.iter()
.map(|v| v.as_str().unwrap())
.collect();
assert_eq!(
names,
vec!["src/main.rs"],
"unchanged main.rs re-resolves to the new util.rs, deduplicated"
);
}
#[test]
fn rust_pub_use_re_exports_resolve() {
let tmp = cache_dir().join("pub-use");
let _ = std::fs::remove_dir_all(&tmp);
let src = tmp.join("src");
std::fs::create_dir_all(&src).unwrap();
std::fs::write(src.join("inner.rs"), "pub fn x() {}\n").unwrap();
std::fs::write(src.join("re.rs"), "pub use inner;\n").unwrap();
run_json(&["index", "create", tmp.to_str().unwrap()]);
let importers = run_json(&[
"--project",
tmp.to_str().unwrap(),
"find",
"importers",
"src/inner.rs",
]);
let names: Vec<&str> = importers
.as_array()
.unwrap()
.iter()
.map(|v| v.as_str().unwrap())
.collect();
assert!(
names.contains(&"src/re.rs"),
"pub use in re.rs re-exports inner.rs"
);
}
#[test]
fn mcp_reports_invalid_params_and_parse_errors() {
let src = fixture("rustdemo");
let tmp = cache_dir().join("mcp-errors");
let _ = std::fs::remove_dir_all(&tmp);
copy_dir(&src, &tmp);
run_json(&["index", "create", tmp.to_str().unwrap()]);
let mut child = Command::new(bin())
.args(["--project", tmp.to_str().unwrap(), "serve"])
.env("XDG_CACHE_HOME", cache_dir())
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::null())
.spawn()
.expect("spawn serve");
{
let mut stdin = child.stdin.take().unwrap();
// Missing required `query` argument -> tool-level isError.
writeln!(
stdin,
r#"{{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{{"name":"search_symbols","arguments":{{}}}}}}"#
)
.unwrap();
// Malformed JSON -> JSON-RPC parse error (id null, code -32700).
writeln!(stdin, "this is not json").unwrap();
}
let out = child.wait_with_output().unwrap();
let responses: Vec<Value> = String::from_utf8_lossy(&out.stdout)
.lines()
.filter_map(|l| serde_json::from_str(l).ok())
.collect();
let call = responses
.iter()
.find(|r| r["id"] == 2)
.expect("tools/call response");
assert_eq!(call["result"]["isError"], true, "missing arg is an error");
assert!(
call["result"]["content"][0]["text"]
.as_str()
.unwrap()
.contains("query"),
"error names the missing argument"
);
let parse = responses
.iter()
.find(|r| r["error"]["code"] == -32700)
.expect("parse error response");
assert_eq!(parse["error"]["message"], "parse error");
}