Skip to content

Commit 4ebe883

Browse files
cdeustclaude
andcommitted
fix(search): pass index dir as a parameter — delete AA_SEARCH_INDEX_DIR global (0.2.2)
Root-cause for the stage3d_hybrid_search flake. v0.2.1 serialized the tests with a mutex (band-aid). The structural cause: do_search_codebase handed the search-index dir to search::search_graph through the PROCESS-GLOBAL env var AA_SEARCH_INDEX_DIR — a hidden channel between two functions that should communicate by argument. Parallel callers/tests stomped it; build_search_index wiped+rebuilt the dir mid-read → tantivy FileDoesNotExist. search_graph now takes index_dir: Option<&Path>. find_search_index_dir and the env var are deleted; main.rs passes graph_path.parent()/search_index; prd_input + the four integration tests pass None. The test mutex is removed — the four hybrid-search tests run fully parallel, each passing its own index dir. Verified: full suite green; hybrid+integration search tests 3× green under parallelism with no mutex. source: dijkstra root-cause audit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e4cb033 commit 4ebe883

9 files changed

Lines changed: 59 additions & 68 deletions

File tree

.claude-plugin/marketplace.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@
66
},
77
"metadata": {
88
"description": "automatised-pipeline — Rust MCP that indexes ANY codebase (every file type: source in 10 languages with full AST, plus docs/config/binaries as File nodes) into a LadybugDB property graph and exposes 24 tools (index_codebase, query_graph, get_impact, get_context, semantic-diff) to agents.",
9-
"version": "0.2.1"
9+
"version": "0.2.2"
1010
},
1111
"plugins": [
1212
{
1313
"name": "automatised-pipeline",
1414
"source": "./",
1515
"description": "24 MCP tools · all-file indexing (every file is a File node; .js→Imports, Markdown→References) · LadybugDB property graph · Leiden communities · BM25+TF-IDF+RRF search",
16-
"version": "0.2.1",
16+
"version": "0.2.2",
1717
"author": {
1818
"name": "Clement Deust",
1919
"email": "admin@ai-architect.tools"

.claude-plugin/plugin.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "automatised-pipeline",
33
"description": "Rust MCP server that indexes ANY codebase into a LadybugDB property graph — source in 10 languages with full AST, plus docs/config/binaries as File nodes (.js→Imports, Markdown→References). Resolves imports and call chains, detects communities via Leiden, traces execution flows from entry points, and exposes 24 MCP tools to agents — index_codebase, query_graph, get_symbol, get_impact, get_context, semantic_diff, and more.",
4-
"version": "0.2.1",
4+
"version": "0.2.2",
55
"author": {
66
"name": "Clement Deust",
77
"email": "admin@ai-architect.tools"

CHANGELOG.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,21 @@ adheres to [Semantic Versioning](https://semver.org/).
66

77
## [Unreleased]
88

9+
## [0.2.2] — Remove the search-index env-var channel (flaky-test root cause)
10+
11+
### Fixed
12+
13+
- **Root-caused the `stage3d_hybrid_search` flake.** v0.2.1 serialized the
14+
tests with a mutex — a band-aid. The structural cause was that
15+
`do_search_codebase` passed the search-index directory to
16+
`search::search_graph` through the PROCESS-GLOBAL env var
17+
`AA_SEARCH_INDEX_DIR`, a hidden channel that races across any parallel
18+
callers (and was wiped+rebuilt mid-read → tantivy `FileDoesNotExist`).
19+
`search_graph` now takes `index_dir: Option<&Path>` as an explicit
20+
parameter; the env var and `find_search_index_dir` are deleted. The test
21+
mutex is removed — the four tests run fully parallel, each passing its own
22+
index dir (verified 3× green). source: dijkstra root-cause audit.
23+
924
## [0.2.1] — Release hygiene + flaky-test fix
1025

1126
### Fixed

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ members = [".", "benches/harness", "crates/zera"]
99

1010
[package]
1111
name = "ai-architect-mcp"
12-
version = "0.2.1"
12+
version = "0.2.2"
1313
edition = "2021"
1414
description = "Stage-by-stage rewrite of the ai-architect pipeline as an MCP server. One tool per stage. Grown by zetetic + genius agents."
1515

src/main.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2443,14 +2443,13 @@ fn do_search_codebase(arguments: &Value) -> Result<Value, String> {
24432443
return Err(format!("graph_path does not exist: {graph_str}"));
24442444
}
24452445

2446-
// Set search index dir hint for hybrid search.
2447-
// Convention: search_index/ is a sibling of graph/ under the output dir.
2448-
if let Some(parent) = graph_path.parent() {
2449-
let search_index_dir = parent.join("search_index");
2450-
if search_index_dir.exists() {
2451-
std::env::set_var("AA_SEARCH_INDEX_DIR", search_index_dir.to_string_lossy().as_ref());
2452-
}
2453-
}
2446+
// The search index lives in a sibling ``search_index/`` of the graph dir.
2447+
// Pass it explicitly to search_graph — no process-global env hand-off
2448+
// (that channel raced across parallel callers; see search::search_graph).
2449+
let search_index_dir = graph_path
2450+
.parent()
2451+
.map(|p| p.join("search_index"))
2452+
.filter(|p| p.exists());
24542453

24552454
let start = std::time::Instant::now();
24562455
let store = graph_store::GraphStore::open_or_create(graph_path)?;
@@ -2459,7 +2458,8 @@ fn do_search_codebase(arguments: &Value) -> Result<Value, String> {
24592458
label_filter,
24602459
min_score: 0.01,
24612460
};
2462-
let results = search::search_graph(&store, query, &options)?;
2461+
let results =
2462+
search::search_graph(&store, query, &options, search_index_dir.as_deref())?;
24632463
let elapsed_ms = start.elapsed().as_millis() as u64;
24642464

24652465
let items: Vec<Value> = results.iter().map(|r| json!({

src/prd_input.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ fn search_and_enrich(store: &GraphStore, tokens: &[String]) -> Vec<MatchedSymbol
242242
label_filter: None,
243243
min_score: 0.0,
244244
};
245-
let hits = match search::search_graph(store, token, &opts) {
245+
let hits = match search::search_graph(store, token, &opts, None) {
246246
Ok(h) => h,
247247
Err(_) => continue,
248248
};

src/search/mod.rs

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ pub fn search_graph(
142142
store: &GraphStore,
143143
query: &str,
144144
options: &SearchOptions,
145+
index_dir: Option<&std::path::Path>,
145146
) -> Result<Vec<SearchResult>, String> {
146147
let _start = Instant::now();
147148
let query_lower = query.to_lowercase();
@@ -150,9 +151,13 @@ pub fn search_graph(
150151
return Ok(Vec::new());
151152
}
152153

153-
// Try to find search index directory relative to the graph path.
154-
// Convention: search_index/ is a sibling of graph/ under the output_dir.
155-
let index_dir = find_search_index_dir(store);
154+
// The search-index directory is passed by the caller (sibling
155+
// ``search_index/`` of the graph dir). It used to be smuggled through the
156+
// process-global env var ``AA_SEARCH_INDEX_DIR``, which raced across
157+
// parallel callers/tests (tantivy FileDoesNotExist). Passing it as an
158+
// argument removes that hidden global channel entirely. source: dijkstra
159+
// root-cause audit of the stage3d_hybrid_search flake.
160+
let index_dir = index_dir.map(|p| p.to_path_buf());
156161

157162
let has_bm25 = index_dir.as_ref()
158163
.map(|d| d.join("bm25").exists())
@@ -517,21 +522,6 @@ fn find_name_candidates(store: &GraphStore, name: &str, limit: usize) -> Vec<Str
517522
// Internal: find search index directory
518523
// ---------------------------------------------------------------------------
519524

520-
fn find_search_index_dir(_store: &GraphStore) -> Option<std::path::PathBuf> {
521-
// The GraphStore doesn't expose its path, but we can probe known locations.
522-
// Convention: search_index/ is a sibling of graph/ under the output dir.
523-
// The caller passes graph_path when opening the store. We use a probe:
524-
// check if ../search_index/ exists relative to the DB path.
525-
//
526-
// Since GraphStore doesn't expose its path, we use an env-var hint
527-
// set by the search tool handler, or probe common locations.
528-
if let Ok(hint) = std::env::var("AA_SEARCH_INDEX_DIR") {
529-
let p = std::path::PathBuf::from(hint);
530-
if p.exists() { return Some(p); }
531-
}
532-
None
533-
}
534-
535525
// ---------------------------------------------------------------------------
536526
// Internal: candidate fetching (for substring fallback)
537527
// ---------------------------------------------------------------------------

tests/stage3d_hybrid_search.rs

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,13 @@ use ai_architect_mcp::search;
1111
use std::fs;
1212
use std::path::PathBuf;
1313
use std::sync::atomic::{AtomicU64, Ordering};
14-
use std::sync::{Mutex, MutexGuard, OnceLock};
15-
16-
// search_graph reads the index location from the PROCESS-GLOBAL env var
17-
// `AA_SEARCH_INDEX_DIR`. cargo runs the tests in this binary on parallel
18-
// threads, so without serialization two tests stomp each other's env var
19-
// (and `build_search_index` wipes+rebuilds its dir), producing a tantivy
20-
// `FileDoesNotExist` on the BM25 store. Hold this lock for the whole of
21-
// each test so the env var + index belong to exactly one test at a time.
22-
// source: observed CI flake (stage3d_hybrid_search.rs, run 26824494088).
23-
fn search_test_guard() -> MutexGuard<'static, ()> {
24-
static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
25-
LOCK.get_or_init(|| Mutex::new(()))
26-
.lock()
27-
.unwrap_or_else(|poisoned| poisoned.into_inner())
28-
}
14+
15+
// These tests run in parallel (cargo default). Each builds its own index in a
16+
// per-test temp dir and passes that dir EXPLICITLY to search_graph, so there
17+
// is no shared global state to race — the previous flake came from smuggling
18+
// the index dir through the process-global env var AA_SEARCH_INDEX_DIR, now
19+
// removed at the root (search_graph takes index_dir as a parameter).
20+
// source: dijkstra root-cause audit (run 26824494088).
2921

3022
const FIXTURE_MAIN: &str = r#"
3123
fn main() {
@@ -58,12 +50,9 @@ pub fn route_incoming(path: &str) -> String {
5850

5951
static COUNTER: AtomicU64 = AtomicU64::new(0);
6052

61-
fn setup_with_search_index(
62-
test_name: &str,
63-
) -> (MutexGuard<'static, ()>, PathBuf, GraphStore) {
64-
// Acquire BEFORE touching the shared env var / index dir; the guard is
65-
// returned so the caller holds it for the whole test.
66-
let guard = search_test_guard();
53+
/// Returns (tmp_root, store, index_dir). The index_dir is passed explicitly to
54+
/// search_graph by each test — no shared env var, so the tests are parallel-safe.
55+
fn setup_with_search_index(test_name: &str) -> (PathBuf, GraphStore, PathBuf) {
6756
let n = COUNTER.fetch_add(1, Ordering::SeqCst);
6857
let tmp_root = std::env::temp_dir().join(format!(
6958
"stage3d_hybrid_{}_{n}_{}", test_name, std::process::id()
@@ -88,22 +77,19 @@ fn setup_with_search_index(
8877
assert!(si.bm25_doc_count > 0, "should index BM25 docs");
8978
assert!(si.vector_doc_count > 0, "should index vector docs");
9079

91-
// Set env hint for search_graph to find the index
9280
let idx_dir = tmp_root.join("search_index");
93-
std::env::set_var("AA_SEARCH_INDEX_DIR", idx_dir.to_string_lossy().as_ref());
94-
95-
(guard, tmp_root, store)
81+
(tmp_root, store, idx_dir)
9682
}
9783

9884
#[test]
9985
fn test_hybrid_bm25_keyword_search() {
100-
let (_guard, tmp_root, store) = setup_with_search_index("bm25");
86+
let (tmp_root, store, idx_dir) = setup_with_search_index("bm25");
10187
let opts = search::SearchOptions {
10288
limit: 10,
10389
label_filter: None,
10490
min_score: 0.0,
10591
};
106-
let results = search::search_graph(&store, "handle tool", &opts).unwrap();
92+
let results = search::search_graph(&store, "handle tool", &opts, Some(&idx_dir)).unwrap();
10793
assert!(!results.is_empty(), "BM25 should find 'handle tool'");
10894
let found = results.iter().any(|r| r.name.contains("handle_tool"));
10995
assert!(found, "should find handle_tool_call via BM25: {:?}",
@@ -114,15 +100,15 @@ fn test_hybrid_bm25_keyword_search() {
114100

115101
#[test]
116102
fn test_hybrid_semantic_search() {
117-
let (_guard, tmp_root, store) = setup_with_search_index("semantic");
103+
let (tmp_root, store, idx_dir) = setup_with_search_index("semantic");
118104
let opts = search::SearchOptions {
119105
limit: 10,
120106
label_filter: None,
121107
min_score: 0.0,
122108
};
123109
// "process incoming requests" should find process_request or route_incoming
124110
// via TF-IDF even though the exact phrase doesn't appear
125-
let results = search::search_graph(&store, "process incoming requests", &opts).unwrap();
111+
let results = search::search_graph(&store, "process incoming requests", &opts, Some(&idx_dir)).unwrap();
126112
assert!(!results.is_empty(),
127113
"semantic search should find results for 'process incoming requests'");
128114

@@ -141,13 +127,13 @@ fn test_hybrid_semantic_search() {
141127

142128
#[test]
143129
fn test_rrf_fusion_combines_rankings() {
144-
let (_guard, tmp_root, store) = setup_with_search_index("rrf");
130+
let (tmp_root, store, idx_dir) = setup_with_search_index("rrf");
145131
let opts = search::SearchOptions {
146132
limit: 10,
147133
label_filter: None,
148134
min_score: 0.0,
149135
};
150-
let results = search::search_graph(&store, "handle", &opts).unwrap();
136+
let results = search::search_graph(&store, "handle", &opts, Some(&idx_dir)).unwrap();
151137
assert!(!results.is_empty(), "should find results for 'handle'");
152138

153139
// RRF scores are in the range ~0.01-0.03 (1/(60+rank))
@@ -163,7 +149,7 @@ fn test_rrf_fusion_combines_rankings() {
163149

164150
#[test]
165151
fn test_build_search_index_creates_files() {
166-
let (_guard, tmp_root, _store) = setup_with_search_index("files");
152+
let (tmp_root, _store, _idx_dir) = setup_with_search_index("files");
167153

168154
let bm25_dir = tmp_root.join("search_index/bm25");
169155
assert!(bm25_dir.exists(), "BM25 index directory should exist");

tests/stage3d_integration.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ fn test_search_exact_name() {
9595
label_filter: None,
9696
min_score: 0.0,
9797
};
98-
let results = search::search_graph(&store, "main", &opts).unwrap();
98+
let results = search::search_graph(&store, "main", &opts, None).unwrap();
9999
assert!(!results.is_empty(), "search for 'main' should return results");
100100
assert_eq!(results[0].name, "main", "top result should be exact match");
101101
assert!(
@@ -114,7 +114,7 @@ fn test_search_partial_name() {
114114
label_filter: None,
115115
min_score: 0.0,
116116
};
117-
let results = search::search_graph(&store, "handle", &opts).unwrap();
117+
let results = search::search_graph(&store, "handle", &opts, None).unwrap();
118118
assert!(!results.is_empty(), "search for 'handle' should find handle_request");
119119
let found = results.iter().any(|r| r.name.contains("handle"));
120120
assert!(found, "should find a symbol containing 'handle'");
@@ -129,7 +129,7 @@ fn test_search_label_filter() {
129129
label_filter: Some("Function".to_string()),
130130
min_score: 0.0,
131131
};
132-
let results = search::search_graph(&store, "process", &opts).unwrap();
132+
let results = search::search_graph(&store, "process", &opts, None).unwrap();
133133
for r in &results {
134134
assert_eq!(r.label, "Function", "label filter should be respected");
135135
}
@@ -144,7 +144,7 @@ fn test_search_results_have_context() {
144144
label_filter: None,
145145
min_score: 0.0,
146146
};
147-
let results = search::search_graph(&store, "main", &opts).unwrap();
147+
let results = search::search_graph(&store, "main", &opts, None).unwrap();
148148
let main_result = results.iter().find(|r| r.name == "main").unwrap();
149149
assert!(!main_result.file_path.is_empty(), "should have file_path");
150150
assert!(main_result.community_id.is_some(), "main should have a community");

0 commit comments

Comments
 (0)