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:
1 parent
6875998ab6
commit
4338e28ce0
7 files changed
+271
-49
No files matched your search
@@ -138,6 +138,29 @@ Notes:
|
||||
- Hardware cycle counters were not available (`perf` tools missing on this
|
||||
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
|
||||
|
||||
```bash
|
||||
|
||||
@@ -32,10 +32,15 @@ pub fn discover(project: &Path) -> Result<Vec<SourceFile>> {
|
||||
});
|
||||
|
||||
let mut out = Vec::new();
|
||||
let mut skipped = Vec::new();
|
||||
for entry in builder.build() {
|
||||
let entry = match entry {
|
||||
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();
|
||||
if entry.file_type().is_some_and(|t| t.is_dir()) {
|
||||
@@ -54,6 +59,9 @@ pub fn discover(project: &Path) -> Result<Vec<SourceFile>> {
|
||||
language,
|
||||
});
|
||||
}
|
||||
for s in &skipped {
|
||||
eprintln!("rustytokens: skipped unreadable entry: {s}");
|
||||
}
|
||||
out.sort_by(|a, b| a.rel.cmp(&b.rel));
|
||||
Ok(out)
|
||||
}
|
||||
+28
-5
@@ -75,10 +75,22 @@ fn extract_python(content: &str) -> Vec<Import> {
|
||||
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> {
|
||||
let mut out = Vec::new();
|
||||
for line in content.lines() {
|
||||
let l = line.trim();
|
||||
let l = strip_visibility(line.trim());
|
||||
if let Some(rest) = l.strip_prefix("use ") {
|
||||
let rest = rest
|
||||
.trim_end_matches(';')
|
||||
@@ -293,6 +305,17 @@ fn clean(target: &str) -> Option<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 {
|
||||
pub fn build(files: Vec<(String, i64)>, crate_name: Option<&str>) -> Self {
|
||||
let mut by_path = HashMap::new();
|
||||
@@ -380,12 +403,12 @@ impl Resolver {
|
||||
if let Some(id) = self.by_path.get(&format!("{base}{t}")) {
|
||||
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}")) {
|
||||
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}")) {
|
||||
return Some(*id);
|
||||
}
|
||||
@@ -394,12 +417,12 @@ impl Resolver {
|
||||
}
|
||||
|
||||
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}")) {
|
||||
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}")) {
|
||||
return Some(*id);
|
||||
}
|
||||
|
||||
+23
-10
@@ -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())))
|
||||
.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
|
||||
.par_iter()
|
||||
.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);
|
||||
(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 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(),
|
||||
);
|
||||
|
||||
// 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 {
|
||||
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();
|
||||
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)?;
|
||||
let dir = f
|
||||
.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)?;
|
||||
}
|
||||
}
|
||||
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;")?;
|
||||
|
||||
+3
-1
@@ -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>> {
|
||||
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))?;
|
||||
Ok(rows.collect::<std::result::Result<Vec<_>, _>>()?)
|
||||
}
|
||||
|
||||
+54
-31
@@ -22,7 +22,22 @@ pub fn serve(project: &Path) -> Result<()> {
|
||||
}
|
||||
let msg: Value = match serde_json::from_str(&line) {
|
||||
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) {
|
||||
let mut out = stdout.lock();
|
||||
@@ -35,12 +50,20 @@ pub fn serve(project: &Path) -> Result<()> {
|
||||
}
|
||||
|
||||
fn handle(msg: &Value, project: &Path) -> Option<Value> {
|
||||
let method = msg.get("method")?.as_str()?;
|
||||
let id = msg.get("id").cloned();
|
||||
|
||||
// Notifications have no id -> no response.
|
||||
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 {
|
||||
"initialize" => Some(json!({
|
||||
"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> {
|
||||
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 {
|
||||
"index_create" => ops::index_create(Path::new(s("path"))),
|
||||
"index_update" => ops::index_update(Path::new(s("path"))),
|
||||
"search_symbols" => ops::search_symbols(project, s("query")),
|
||||
"search_text" => ops::search_text(project, s("query")),
|
||||
"inspect_symbol" => ops::inspect_symbol(
|
||||
project,
|
||||
args.get("id").and_then(|v| v.as_i64()).unwrap_or(0),
|
||||
),
|
||||
"inspect_outline" => ops::inspect_outline(project, s("file")),
|
||||
"inspect_file" => ops::inspect_file(
|
||||
project,
|
||||
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")),
|
||||
"index_create" => ops::index_create(Path::new(req_str(args, "path", name)?)),
|
||||
"index_update" => ops::index_update(Path::new(req_str(args, "path", name)?)),
|
||||
"search_symbols" => ops::search_symbols(project, req_str(args, "query", name)?),
|
||||
"search_text" => ops::search_text(project, req_str(args, "query", name)?),
|
||||
"inspect_symbol" => ops::inspect_symbol(project, req_i64(args, "id", name)?),
|
||||
"inspect_outline" => ops::inspect_outline(project, req_str(args, "file", name)?),
|
||||
"inspect_file" => {
|
||||
ops::inspect_file(project, req_str(args, "file", name)?, opt_str("lines"))
|
||||
}
|
||||
"inspect_tree" => ops::inspect_tree(project, opt_depth("depth")),
|
||||
"find_importers" => ops::find_importers(project, req_str(args, "file", name)?),
|
||||
"find_dead" => ops::find_dead(project),
|
||||
"inspect_cycles" => ops::inspect_cycles(project),
|
||||
"inspect_blast_radius" => ops::inspect_blast_radius(
|
||||
project,
|
||||
s("file"),
|
||||
args.get("depth")
|
||||
.and_then(|v| v.as_u64())
|
||||
.map(|d| d as usize),
|
||||
),
|
||||
"inspect_blast_radius" => {
|
||||
ops::inspect_blast_radius(project, req_str(args, "file", name)?, opt_depth("depth"))
|
||||
}
|
||||
"stats" => ops::stats(project),
|
||||
_ => anyhow::bail!("unknown tool: {name}"),
|
||||
}
|
||||
|
||||
+131
-1
@@ -144,7 +144,10 @@ fn inspect_blast_radius_finds_affected_files() {
|
||||
.collect();
|
||||
// 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/main.rs"), "main.rs → lib.rs → util.rs");
|
||||
assert!(
|
||||
affected.contains(&"src/main.rs"),
|
||||
"main.rs → lib.rs → util.rs"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -496,3 +499,130 @@ fn index_update_reindexes_changed_files() {
|
||||
"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");
|
||||
}
|
||||
Reference in new issue
Block a user