From f6a8e7da0585f4ea238f43cb61f80ecff1f754ca Mon Sep 17 00:00:00 2001 From: vera-bot Date: Wed, 30 Sep 2026 00:29:28 -0400 Subject: [PATCH] refactor(retrieval): remove rejected experiments and guard old indexes --- crates/vera-cli/src/commands/config.rs | 64 +- crates/vera-core/src/config.rs | 627 +----------------- crates/vera-core/src/indexing/freshness.rs | 78 ++- crates/vera-core/src/indexing/update.rs | 107 +++ crates/vera-core/src/parsing/chunker.rs | 228 ------- crates/vera-core/src/parsing/mod.rs | 31 +- crates/vera-core/src/retrieval/bm25.rs | 6 +- .../src/retrieval/graph_augmentation.rs | 416 ------------ crates/vera-core/src/retrieval/hybrid.rs | 193 +----- crates/vera-core/src/retrieval/mod.rs | 13 +- crates/vera-core/src/retrieval/ranking/mod.rs | 40 +- .../vera-core/src/retrieval/ranking/score.rs | 116 +--- .../vera-core/src/retrieval/ranking/tests.rs | 179 ----- crates/vera-core/src/retrieval/references.rs | 4 +- .../vera-core/src/retrieval/regex_search.rs | 4 +- .../vera-core/src/retrieval/search_service.rs | 98 +-- crates/vera-core/src/retrieval/structural.rs | 3 +- .../vera-core/src/retrieval/type_relations.rs | 4 +- crates/vera-core/src/retrieval/vector.rs | 4 + crates/vera-core/src/types.rs | 4 +- docs/adr/000-decision-summary.md | 2 +- docs/adr/006-ranking-signals.md | 2 +- docs/adr/007-ranking-hypotheses.md | 110 +-- docs/benchmarks-history.md | 16 +- docs/configuration.md | 15 - docs/how-it-works.md | 2 + docs/migration-v2.md | 28 + docs/whats-new.md | 8 +- eval/src/lanes.rs | 3 - eval/src/vera_adapter.rs | 43 +- 30 files changed, 374 insertions(+), 2074 deletions(-) delete mode 100644 crates/vera-core/src/retrieval/graph_augmentation.rs create mode 100644 docs/migration-v2.md diff --git a/crates/vera-cli/src/commands/config.rs b/crates/vera-cli/src/commands/config.rs index 0b31ae4d..329ef89b 100644 --- a/crates/vera-cli/src/commands/config.rs +++ b/crates/vera-cli/src/commands/config.rs @@ -104,10 +104,6 @@ fn print_human_config(config: &vera_core::config::VeraConfig) { " max_chunk_bytes {}", config.indexing.max_chunk_bytes ); - println!( - " chunk_max_chars {}", - config.indexing.chunk_max_chars - ); println!(); println!(" Retrieval:"); println!( @@ -167,14 +163,6 @@ fn print_human_config(config: &vera_core::config::VeraConfig) { " reranker_return_documents {:?}", config.retrieval.reranker_return_documents ); - println!( - " ranking_multiplicative_path_penalty {}", - config.retrieval.ranking_multiplicative_path_penalty - ); - println!( - " ranking_candidate_pool_multiplier {}", - config.retrieval.ranking_candidate_pool_multiplier - ); println!(); println!(" Embedding:"); println!( @@ -240,9 +228,6 @@ pub fn get_config_value( "indexing.max_chunk_bytes" => Some(serde_json::Value::Number( config.indexing.max_chunk_bytes.into(), )), - "indexing.chunk_max_chars" | "indexing.max_chunk_chars" => Some(serde_json::Value::Number( - config.indexing.chunk_max_chars.into(), - )), "retrieval.default_limit" => Some(serde_json::Value::Number( config.retrieval.default_limit.into(), )), @@ -300,16 +285,6 @@ pub fn get_config_value( | "retrieval.return_documents" => { serde_json::to_value(config.retrieval.reranker_return_documents).ok() } - "retrieval.ranking_multiplicative_path_penalty" - | "retrieval.ranking_path_penalty" - | "retrieval.ranking_multiplicative_penalty" => Some(serde_json::Value::Bool( - config.retrieval.ranking_multiplicative_path_penalty, - )), - "retrieval.ranking_candidate_pool_multiplier" - | "retrieval.ranking_pool_multiplier" - | "retrieval.ranking_candidate_pool_size_multiplier" => Some(serde_json::Value::Bool( - config.retrieval.ranking_candidate_pool_multiplier, - )), "embedding.batch_size" => Some(serde_json::Value::Number( config.embedding.batch_size.into(), )), @@ -364,11 +339,6 @@ fn set_config_value( "indexing.max_chunk_bytes" => { config.indexing.max_chunk_bytes = parse_value(key, value)?; } - "indexing.chunk_max_chars" - | "indexing.max_chunk_chars" - | "indexing.max_chunk_characters" => { - config.indexing.chunk_max_chars = parse_value(key, value)?; - } "retrieval.default_limit" => { config.retrieval.default_limit = parse_positive(key, value)?; } @@ -448,16 +418,6 @@ fn set_config_value( | "retrieval.return_documents" => { config.retrieval.reranker_return_documents = parse_optional_bool(key, value)?; } - "retrieval.ranking_multiplicative_path_penalty" - | "retrieval.ranking_path_penalty" - | "retrieval.ranking_multiplicative_penalty" => { - config.retrieval.ranking_multiplicative_path_penalty = parse_value(key, value)?; - } - "retrieval.ranking_candidate_pool_multiplier" - | "retrieval.ranking_pool_multiplier" - | "retrieval.ranking_candidate_pool_size_multiplier" => { - config.retrieval.ranking_candidate_pool_multiplier = parse_value(key, value)?; - } "embedding.batch_size" => { config.embedding.batch_size = parse_positive(key, value)?; } @@ -588,6 +548,30 @@ fn parse_optional_u64(key: &str, value: &str) -> anyhow::Result> { mod tests { use super::*; + #[test] + fn retired_experiment_keys_are_unknown_to_get_and_set() { + let mut config = vera_core::config::VeraConfig::default(); + for key in [ + "indexing.chunk_max_chars", + "indexing.max_chunk_chars", + "indexing.max_chunk_characters", + "retrieval.ranking_multiplicative_path_penalty", + "retrieval.ranking_path_penalty", + "retrieval.ranking_multiplicative_penalty", + "retrieval.ranking_candidate_pool_multiplier", + "retrieval.ranking_pool_multiplier", + "retrieval.ranking_candidate_pool_size_multiplier", + ] { + assert!(get_config_value(&config, key).is_none()); + assert!( + set_config_value(&mut config, key, "1") + .unwrap_err() + .to_string() + .contains("unknown configuration key") + ); + } + } + #[test] fn config_set_rejects_zero_default_limit() { let mut config = vera_core::config::VeraConfig::default(); diff --git a/crates/vera-core/src/config.rs b/crates/vera-core/src/config.rs index 04da006e..3bee992f 100644 --- a/crates/vera-core/src/config.rs +++ b/crates/vera-core/src/config.rs @@ -42,17 +42,6 @@ pub struct IndexingConfig { /// (measured on the Semble suite, see issue #67), not a model limit. #[serde(default = "default_max_chunk_bytes")] pub max_chunk_bytes: usize, - /// Maximum chunk size in characters. When non-zero, chunks exceeding this - /// are split at line boundaries. 0 disables char-based splitting (default - /// OFF). Mechanism: 750-char windows give finer embedding locality than - /// the 24 KB byte cap, reducing concept blur within long chunks while - /// preserving symbol coherence. - #[serde( - default = "default_chunk_max_chars", - alias = "max_chunk_chars", - alias = "max_chunk_characters" - )] - pub chunk_max_chars: usize, } /// Read a `usize` config override from an environment variable, falling back @@ -151,33 +140,6 @@ fn default_max_chunk_bytes() -> usize { env_usize("VERA_MAX_CHUNK_BYTES", 24_576) } -fn default_chunk_max_chars() -> usize { - // Alias set and precedence match `IndexingConfig::chunk_max_chars_effective` - // (per alias-discipline convention): first wins, identical order. - for key in [ - "VERA_INDEXING_CHUNK_MAX_CHARS", - "VERA_INDEXING_MAX_CHUNK_CHARS", - "VERA_MAX_CHUNK_CHARS", - "VERA_CHUNK_MAX_CHARS", - ] { - if let Ok(value) = std::env::var(key) { - return match value.parse::() { - Ok(parsed) => parsed, - Err(error) => { - tracing::warn!( - key, - value = %value, - error = %error, - "invalid numeric environment override; using default 0" - ); - 0 - } - }; - } - } - 0 -} - impl Default for IndexingConfig { fn default() -> Self { Self { @@ -197,47 +159,10 @@ impl Default for IndexingConfig { no_ignore: false, no_default_excludes: false, max_chunk_bytes: default_max_chunk_bytes(), - chunk_max_chars: default_chunk_max_chars(), } } } -impl IndexingConfig { - /// Effective char-budget for chunk splitting, with env override. - /// 0 means disabled (DEFAULT OFF). Env var `VERA_INDEXING_CHUNK_MAX_CHARS` - /// (and alias `VERA_INDEXING_MAX_CHUNK_CHARS` / `VERA_MAX_CHUNK_CHARS`) - /// authoritatively overrides the persisted value. - pub fn chunk_max_chars_effective(&self) -> usize { - for key in [ - "VERA_INDEXING_CHUNK_MAX_CHARS", - "VERA_INDEXING_MAX_CHUNK_CHARS", - "VERA_MAX_CHUNK_CHARS", - "VERA_CHUNK_MAX_CHARS", - ] { - if let Ok(value) = std::env::var(key) { - return match value.parse::() { - Ok(parsed) => parsed, - Err(error) => { - tracing::warn!( - key, - value = %value, - error = %error, - "invalid numeric environment override; using stored value" - ); - self.chunk_max_chars - } - }; - } - } - self.chunk_max_chars - } - - /// Whether the ~750-char chunking hypothesis is active. - pub fn chunk_max_chars_enabled(&self) -> bool { - self.chunk_max_chars_effective() != 0 - } -} - /// Reranker wire protocol / capability selection. /// /// `Generic` covers SiliconFlow, Jina, Cohere and other OpenAI-style @@ -390,38 +315,6 @@ pub struct RetrievalConfig { /// promote it. #[serde(default = "default_ranking_recall_pool_expansion")] pub ranking_recall_pool_expansion: bool, - /// Multiplicative path penalty for test/compat/example directories. - /// - /// When enabled, results whose file path lies in `tests/`, `compat/`, - /// `examples/` (or classified as `Test` / `Example` / `Bench`) are - /// demoted multiplicatively (~0.3×) when the query does not explicitly - /// ask for those directories. Mechanism: boilerplate, fixture and example - /// directories are keyword-dense but rarely contain the implementation a - /// developer is seeking; a multiplicative penalty scales with retrieval - /// confidence, preserving ordering among non-penalized candidates while - /// consistently demoting penalized ones. Respects the existing - /// boost-directory gating (definition boost's `wants_*` checks) so explicit - /// requests for test/compat/example paths are not penalized. - /// Default OFF (0.3 factor only when enabled). - #[serde(default = "default_ranking_multiplicative_path_penalty")] - pub ranking_multiplicative_path_penalty: bool, - /// Candidate-pool multiplier for broader ranking consideration. - /// - /// When enabled, the retrieval fetch limit (`top_k` driven) is multiplied - /// by ~5× so that more low-ranked but correct candidates enter the - /// reranking/ranking stage. Mechanism: ranking signals can only promote - /// files that are present in the pool; a broader pool trades a modest - /// increase in candidates for higher recall of the correct file. - /// This is distinct from `ranking_recall_pool_expansion` which applies a - /// table-driven expansion gated on structural/NL intent (full-suite - /// delta 0.54% reported in #239); the new knob is a bare 5× `top_k` - /// multiplier applied uniformly after the table, not duplicating the - /// conditional recall expansion. Default OFF. - #[serde( - default = "default_ranking_candidate_pool_multiplier", - alias = "ranking_pool_multiplier" - )] - pub ranking_candidate_pool_multiplier: bool, /// Filter-during-scan optimization for filtered flat-vector queries. /// /// When enabled, filtered queries on the flat backend avoid hydrating the whole index: @@ -538,53 +431,6 @@ fn default_ranking_recall_pool_expansion() -> bool { env_bool("VERA_RANKING_RECALL_POOL_EXPANSION", true) } -fn default_ranking_multiplicative_path_penalty() -> bool { - // Default OFF — only enabled when env is truthy or config explicitly true. - // Alias set and precedence match `ranking_multiplicative_path_penalty_enabled` - // (per alias-discipline convention): first wins, identical order. - for key in [ - "VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY", - "VERA_RANKING_PATH_PENALTY", - "VERA_RANKING_MULTIPLICATIVE_PENALTY", - ] { - if std::env::var(key).is_ok() { - return env_bool(key, false); - } - } - false -} - -fn default_ranking_candidate_pool_multiplier() -> bool { - // Default OFF. Accepts boolean truthy values plus numeric "5" as truthy for flexibility, - // because the hypothesis is described as "5x" and validators may set env to "5". - for key in [ - "VERA_RANKING_CANDIDATE_POOL_MULTIPLIER", - "VERA_RANKING_CANDIDATE_POOL_SIZE_MULTIPLIER", - "VERA_RANKING_POOL_MULTIPLIER", - "VERA_RANKING_CANDIDATE_POOL_MULT", - ] { - if let Ok(value) = std::env::var(key) { - // Numeric >0 enables (covers "5"), bool parsing covers "1"/"true" - if let Ok(num) = value.parse::() { - return num != 0; - } - match value.to_ascii_lowercase().as_str() { - "1" | "true" | "yes" | "on" => return true, - "0" | "false" | "no" | "off" => return false, - _ => { - tracing::warn!( - key, - value = %value, - "invalid boolean/numeric environment override; using default false" - ); - return false; - } - } - } - } - false -} - fn default_vector_filter_during_scan() -> bool { // Default ON since the r5 evidence-backed flip (issue #197). The r5 // preregistered decision round at 98e6e50 (3 flag-on + 3 flag-off @@ -627,8 +473,6 @@ impl Default for RetrievalConfig { default_ranking_filename_stem_skip_symbol_queries(), ranking_definition_boost: default_ranking_definition_boost(), ranking_recall_pool_expansion: default_ranking_recall_pool_expansion(), - ranking_multiplicative_path_penalty: default_ranking_multiplicative_path_penalty(), - ranking_candidate_pool_multiplier: default_ranking_candidate_pool_multiplier(), vector_filter_during_scan: default_vector_filter_during_scan(), } } @@ -695,50 +539,6 @@ impl RetrievalConfig { } } - /// Multiplicative path penalty enabled, with env-var override. - /// Checks multiple aliases for validator flexibility; numeric "5" counts as true. - pub fn ranking_multiplicative_path_penalty_enabled(&self) -> bool { - for key in [ - "VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY", - "VERA_RANKING_PATH_PENALTY", - "VERA_RANKING_MULTIPLICATIVE_PENALTY", - ] { - if std::env::var(key).is_ok() { - return env_bool(key, self.ranking_multiplicative_path_penalty); - } - } - self.ranking_multiplicative_path_penalty - } - - /// Candidate-pool multiplier enabled, with env-var override. - /// Numeric env values like "5" are treated as enabled (non-zero). - pub fn ranking_candidate_pool_multiplier_enabled(&self) -> bool { - for key in [ - "VERA_RANKING_CANDIDATE_POOL_MULTIPLIER", - "VERA_RANKING_CANDIDATE_POOL_SIZE_MULTIPLIER", - "VERA_RANKING_POOL_MULTIPLIER", - "VERA_RANKING_CANDIDATE_POOL_MULT", - ] { - if let Ok(value) = std::env::var(key) { - if let Ok(num) = value.parse::() { - return num != 0; - } - return env_bool(key, self.ranking_candidate_pool_multiplier); - } - } - self.ranking_candidate_pool_multiplier - } - - /// Candidate-pool multiplier factor (5 when enabled, 1 otherwise). - /// Used by `compute_fetch_limit_with_config` to scale the fetch limit. - pub fn candidate_pool_multiplier_factor(&self) -> usize { - if self.ranking_candidate_pool_multiplier_enabled() { - 5 - } else { - 1 - } - } - /// Filter-during-scan optimization enabled, with env-var override. /// Default ON since the r5 evidence-backed flip (issue #197); /// env `VERA_VECTOR_FILTER_DURING_SCAN` authoritative. @@ -934,21 +734,6 @@ pub fn is_local_mode() -> bool { .unwrap_or(false) } -/// Whether experimental structural graph augmentation is enabled. -/// -/// This is intentionally an environment-only ablation switch rather than a -/// persisted retrieval setting. Accepted truthy values are `1`, `true`, and -/// `yes`, case-insensitively. -pub fn graph_augmentation_enabled() -> bool { - std::env::var("VERA_GRAPH_AUGMENT") - .map(|value| { - value.eq_ignore_ascii_case("1") - || value.eq_ignore_ascii_case("true") - || value.eq_ignore_ascii_case("yes") - }) - .unwrap_or(false) -} - fn backend_from_env() -> Option { std::env::var("VERA_BACKEND") .ok() @@ -1370,43 +1155,6 @@ mod tests { ); } - #[test] - fn graph_augmentation_env_accepts_only_truthy_values() { - for value in ["1", "true", "TRUE", "yes", "YeS"] { - run_env_test( - "config::tests::graph_augmentation_truthy_probe", - &[("VERA_GRAPH_AUGMENT", Some(value))], - ); - } - - for value in ["0", "false", "no", "", "on"] { - run_env_test( - "config::tests::graph_augmentation_falsey_probe", - &[("VERA_GRAPH_AUGMENT", Some(value))], - ); - } - } - - #[test] - #[ignore = "driven by graph_augmentation_env_accepts_only_truthy_values"] - fn graph_augmentation_truthy_probe() { - let value = std::env::var("VERA_GRAPH_AUGMENT").unwrap(); - assert!( - graph_augmentation_enabled(), - "{value} should enable the flag" - ); - } - - #[test] - #[ignore = "driven by graph_augmentation_env_accepts_only_truthy_values"] - fn graph_augmentation_falsey_probe() { - let value = std::env::var("VERA_GRAPH_AUGMENT").unwrap(); - assert!( - !graph_augmentation_enabled(), - "{value} should disable the flag" - ); - } - #[test] fn embedding_parallelism_clamps_batch_and_concurrency() { let config = EmbeddingConfig { @@ -2156,371 +1904,28 @@ card0, 1073741824, 4294967296\n"; assert!(RetrievalConfig::default().ranking_recall_pool_expansion_enabled()); } - // ── Issue #196 hypotheses (feat/issue196-hypotheses): three remaining knobs ── - // Each hypothesis is a toggleable knob, DEFAULT OFF, with env override. - // These tests pin the default-off contract and independent flip behavior. - #[test] - fn issue196_hypotheses_default_off() { - // Ensure no env leakage: this test must run with all three new envs unset. - // We do not use run_env_test here so it also validates that Default - // itself (which reads env) sees OFF when env absent. Caller must ensure - // env is clean; the later matrix tests cover the env-flip cases. - // To be deterministic, we explicitly check the underlying default - // functions return OFF when env absent via a probe subprocess. - // Inline check: VeraConfig::default() should have OFF for all three. - // If env is set from a prior test, this would flake; run_env_test probes - // avoid that by isolating env. So we delegate to a subprocess probe. - run_env_test( - "config::tests::issue196_hypotheses_default_off_probe", - &[ - ("VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY", None), - ("VERA_RANKING_CANDIDATE_POOL_MULTIPLIER", None), - ("VERA_RANKING_POOL_MULTIPLIER", None), - ("VERA_INDEXING_CHUNK_MAX_CHARS", None), - ("VERA_MAX_CHUNK_CHARS", None), - ("VERA_INDEXING_MAX_CHUNK_CHARS", None), - ], - ); - } - - #[test] - #[ignore = "driven by issue196_hypotheses_default_off"] - fn issue196_hypotheses_default_off_probe() { + fn legacy_experiment_settings_load_without_being_serialized() { + let mut legacy = serde_json::to_value(VeraConfig::default()).unwrap(); + legacy["indexing"]["chunk_max_chars"] = serde_json::json!(750); + legacy["indexing"]["max_chunk_chars"] = serde_json::json!(750); + legacy["indexing"]["max_chunk_characters"] = serde_json::json!(750); for key in [ - "VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY", - "VERA_RANKING_CANDIDATE_POOL_MULTIPLIER", - "VERA_RANKING_POOL_MULTIPLIER", - "VERA_INDEXING_CHUNK_MAX_CHARS", - "VERA_MAX_CHUNK_CHARS", - "VERA_INDEXING_MAX_CHUNK_CHARS", + "ranking_multiplicative_path_penalty", + "ranking_path_penalty", + "ranking_multiplicative_penalty", + "ranking_candidate_pool_multiplier", + "ranking_candidate_pool_size_multiplier", + "ranking_pool_multiplier", ] { - assert!( - std::env::var(key).is_err(), - "{key} should be unset for default-off probe" - ); + legacy["retrieval"][key] = serde_json::json!(true); } - let cfg = VeraConfig::default(); - assert!( - !cfg.retrieval.ranking_multiplicative_path_penalty, - "multiplicative penalty must default OFF" - ); - assert!( - !cfg.retrieval.ranking_multiplicative_path_penalty_enabled(), - "multiplicative penalty enabled() must be false when OFF" - ); - assert!( - !cfg.retrieval.ranking_candidate_pool_multiplier, - "candidate-pool multiplier must default OFF (false / 1×)" - ); - assert!( - !cfg.retrieval.ranking_candidate_pool_multiplier_enabled(), - "candidate-pool multiplier enabled() must be false when OFF" - ); - assert_eq!( - cfg.retrieval.candidate_pool_multiplier_factor(), - 1, - "pool multiplier factor must be 1 when OFF" - ); - assert_eq!( - cfg.indexing.chunk_max_chars, 0, - "chunk_max_chars must default 0 (OFF)" - ); - assert_eq!( - cfg.indexing.chunk_max_chars_effective(), - 0, - "chunk_max_chars_effective must be 0 when OFF" - ); - assert!( - !cfg.indexing.chunk_max_chars_enabled(), - "chunk_max_chars_enabled must be false when OFF" - ); - // Legacy deserialization must also default OFF for all three - let legacy = r#"{ - "default_limit": 5, - "rrf_k": 60.0, - "rerank_candidates": 50, - "reranking_enabled": false, - "max_rerank_batch": 20, - "max_output_chars": 0 - }"#; - let rcfg: RetrievalConfig = serde_json::from_str(legacy).unwrap(); - assert!(!rcfg.ranking_multiplicative_path_penalty); - assert!(!rcfg.ranking_candidate_pool_multiplier); - let vera_legacy = r#"{"indexing":{"max_chunk_lines":200,"default_excludes":[],"max_file_size_bytes":1000000,"max_chunk_bytes":24576},"retrieval":{"default_limit":5,"rrf_k":60.0,"rerank_candidates":50,"reranking_enabled":false,"max_rerank_batch":20,"max_output_chars":0},"embedding":{"batch_size":128,"max_concurrent_requests":8,"timeout_secs":60,"max_retries":3,"max_stored_dim":1024}}"#; - let vcfg: VeraConfig = serde_json::from_str(vera_legacy).unwrap(); - assert_eq!(vcfg.indexing.chunk_max_chars, 0); - assert!(!vcfg.retrieval.ranking_multiplicative_path_penalty); - assert!(!vcfg.retrieval.ranking_candidate_pool_multiplier); - } - - #[test] - fn issue196_hypotheses_env_flips_independently() { - // Each env flips only its knob; setting one must not affect the others. - run_env_test( - "config::tests::issue196_penalty_env_flip_probe", - &[("VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY", Some("1"))], - ); - run_env_test( - "config::tests::issue196_pool_env_flip_probe", - &[("VERA_RANKING_CANDIDATE_POOL_MULTIPLIER", Some("1"))], - ); - // Also verify numeric "5" style flips the pool multiplier (hypothesis is 5×) - run_env_test( - "config::tests::issue196_pool_env_flip_numeric_5_probe", - &[("VERA_RANKING_CANDIDATE_POOL_MULTIPLIER", Some("5"))], - ); - run_env_test( - "config::tests::issue196_chunk_env_flip_probe", - &[("VERA_INDEXING_CHUNK_MAX_CHARS", Some("750"))], - ); - // Alias envs also flip - run_env_test( - "config::tests::issue196_chunk_env_flip_alias_probe", - &[("VERA_MAX_CHUNK_CHARS", Some("750"))], - ); - run_env_test( - "config::tests::issue196_pool_env_flip_alias_pool_multiplier_probe", - &[("VERA_RANKING_POOL_MULTIPLIER", Some("1"))], - ); - } - - #[test] - #[ignore = "driven by issue196_hypotheses_env_flips_independently"] - fn issue196_penalty_env_flip_probe() { - assert_eq!( - std::env::var("VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY").unwrap(), - "1" - ); - let cfg = VeraConfig::default(); - assert!( - cfg.retrieval.ranking_multiplicative_path_penalty_enabled(), - "penalty should be enabled when VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY=1" - ); - // Other knobs must remain OFF — independent flipping - assert!( - !cfg.retrieval.ranking_candidate_pool_multiplier_enabled(), - "pool multiplier must stay OFF when only penalty env is set" - ); - assert_eq!( - cfg.indexing.chunk_max_chars_effective(), - 0, - "chunk knob must stay OFF when penalty env is set" - ); - } - - #[test] - #[ignore = "driven by issue196_hypotheses_env_flips_independently"] - fn issue196_pool_env_flip_probe() { - assert!(std::env::var("VERA_RANKING_CANDIDATE_POOL_MULTIPLIER").is_ok()); - let cfg = VeraConfig::default(); - assert!( - cfg.retrieval.ranking_candidate_pool_multiplier_enabled(), - "pool multiplier should be enabled when env=1" - ); - assert_eq!(cfg.retrieval.candidate_pool_multiplier_factor(), 5); - assert!( - !cfg.retrieval.ranking_multiplicative_path_penalty_enabled(), - "penalty must stay OFF when only pool env is set" - ); - assert_eq!(cfg.indexing.chunk_max_chars_effective(), 0); - } - - #[test] - #[ignore = "driven by issue196_hypotheses_env_flips_independently"] - fn issue196_pool_env_flip_numeric_5_probe() { + let config: VeraConfig = serde_json::from_value(legacy).unwrap(); + let serialized = serde_json::to_value(config).unwrap(); assert_eq!( - std::env::var("VERA_RANKING_CANDIDATE_POOL_MULTIPLIER").unwrap(), - "5" - ); - let cfg = VeraConfig::default(); - assert!( - cfg.retrieval.ranking_candidate_pool_multiplier_enabled(), - "pool multiplier should be enabled when env=5" + serialized, + serde_json::to_value(VeraConfig::default()).unwrap() ); - assert_eq!(cfg.retrieval.candidate_pool_multiplier_factor(), 5); - } - - #[test] - #[ignore = "driven by issue196_hypotheses_env_flips_independently"] - fn issue196_chunk_env_flip_probe() { - assert_eq!( - std::env::var("VERA_INDEXING_CHUNK_MAX_CHARS").unwrap(), - "750" - ); - let cfg = VeraConfig::default(); - assert_eq!(cfg.indexing.chunk_max_chars_effective(), 750); - assert!(cfg.indexing.chunk_max_chars_enabled()); - assert_eq!(cfg.indexing.chunk_max_chars, 750); - // Retrieval knobs must stay OFF - assert!(!cfg.retrieval.ranking_multiplicative_path_penalty_enabled()); - assert!(!cfg.retrieval.ranking_candidate_pool_multiplier_enabled()); - } - - #[test] - #[ignore = "driven by issue196_hypotheses_env_flips_independently"] - fn issue196_chunk_env_flip_alias_probe() { - assert_eq!(std::env::var("VERA_MAX_CHUNK_CHARS").unwrap(), "750"); - let cfg = VeraConfig::default(); - assert_eq!(cfg.indexing.chunk_max_chars_effective(), 750); - assert!(cfg.indexing.chunk_max_chars_enabled()); - } - - #[test] - #[ignore = "driven by issue196_hypotheses_env_flips_independently"] - fn issue196_pool_env_flip_alias_pool_multiplier_probe() { - assert_eq!(std::env::var("VERA_RANKING_POOL_MULTIPLIER").unwrap(), "1"); - let cfg = VeraConfig::default(); - assert!(cfg.retrieval.ranking_candidate_pool_multiplier_enabled()); - } - - #[test] - #[allow(clippy::field_reassign_with_default)] - fn issue196_hypotheses_round_trip() { - let mut cfg = RetrievalConfig::default(); - cfg.ranking_multiplicative_path_penalty = true; - cfg.ranking_candidate_pool_multiplier = true; - let json = serde_json::to_string(&cfg).unwrap(); - let back: RetrievalConfig = serde_json::from_str(&json).unwrap(); - assert!(back.ranking_multiplicative_path_penalty); - assert!(back.ranking_candidate_pool_multiplier); - assert!(back.ranking_multiplicative_path_penalty_enabled()); - assert!(back.ranking_candidate_pool_multiplier_enabled()); - assert_eq!(back.candidate_pool_multiplier_factor(), 5); - - let mut icfg = IndexingConfig::default(); - icfg.chunk_max_chars = 750; - let j2 = serde_json::to_string(&icfg).unwrap(); - let back2: IndexingConfig = serde_json::from_str(&j2).unwrap(); - assert_eq!(back2.chunk_max_chars, 750); - assert_eq!(back2.chunk_max_chars_effective(), 750); - - // Alias deserialization: max_chunk_chars should also populate chunk_max_chars - let alias_json = r#"{"max_chunk_lines":200,"default_excludes":[],"max_file_size_bytes":1000000,"max_chunk_bytes":24576,"max_chunk_chars":750}"#; - let alias_cfg: IndexingConfig = serde_json::from_str(alias_json).unwrap(); - assert_eq!(alias_cfg.chunk_max_chars, 750); - } - - #[test] - fn alias_precedence_parity() { - // Chunk aliases: default* vs effective* must have identical set/order. - run_env_test( - "config::tests::chunk_alias_precedence_probe", - &[ - ("VERA_INDEXING_CHUNK_MAX_CHARS", Some("111")), - ("VERA_INDEXING_MAX_CHUNK_CHARS", Some("222")), - ("VERA_MAX_CHUNK_CHARS", Some("333")), - ("VERA_CHUNK_MAX_CHARS", Some("444")), - ], - ); - run_env_test( - "config::tests::chunk_alias_second_precedence_probe", - &[ - ("VERA_INDEXING_CHUNK_MAX_CHARS", None), - ("VERA_INDEXING_MAX_CHUNK_CHARS", Some("222")), - ("VERA_MAX_CHUNK_CHARS", Some("333")), - ("VERA_CHUNK_MAX_CHARS", Some("444")), - ], - ); - // Penalty aliases: default* vs effective* must be identical. - run_env_test( - "config::tests::penalty_alias_precedence_probe", - &[ - ("VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY", Some("0")), - ("VERA_RANKING_PATH_PENALTY", Some("1")), - ("VERA_RANKING_MULTIPLICATIVE_PENALTY", Some("1")), - ], - ); - run_env_test( - "config::tests::penalty_alias_second_precedence_probe", - &[ - ("VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY", None), - ("VERA_RANKING_PATH_PENALTY", Some("1")), - ("VERA_RANKING_MULTIPLICATIVE_PENALTY", Some("0")), - ], - ); - } - - #[test] - #[ignore = "driven by alias_precedence_parity"] - fn chunk_alias_precedence_probe() { - // All four chunk aliases set; first in precedence must win for both helpers. - assert_eq!( - std::env::var("VERA_INDEXING_CHUNK_MAX_CHARS").unwrap(), - "111" - ); - let cfg = VeraConfig::default(); - assert_eq!( - cfg.indexing.chunk_max_chars, 111, - "default_chunk_max_chars must respect first alias" - ); - assert_eq!( - cfg.indexing.chunk_max_chars_effective(), - 111, - "chunk_max_chars_effective must respect same first alias" - ); - // Verify with a distinct stored value that effective still prefers env first. - let stored = IndexingConfig { - chunk_max_chars: 999, - ..Default::default() - }; - assert_eq!( - stored.chunk_max_chars_effective(), - 111, - "effective must prefer env over stored value with same precedence" - ); - } - - #[test] - #[ignore = "driven by alias_precedence_parity"] - fn chunk_alias_second_precedence_probe() { - assert!(std::env::var("VERA_INDEXING_CHUNK_MAX_CHARS").is_err()); - assert_eq!( - std::env::var("VERA_INDEXING_MAX_CHUNK_CHARS").unwrap(), - "222" - ); - let cfg = VeraConfig::default(); - assert_eq!(cfg.indexing.chunk_max_chars, 222); - assert_eq!(cfg.indexing.chunk_max_chars_effective(), 222); - } - - #[test] - #[ignore = "driven by alias_precedence_parity"] - fn penalty_alias_precedence_probe() { - // All three penalty aliases set with conflicting values; first must win. - assert_eq!( - std::env::var("VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY").unwrap(), - "0" - ); - let cfg = VeraConfig::default(); - // default helper should be false (first alias 0 wins) - assert!( - !cfg.retrieval.ranking_multiplicative_path_penalty, - "default helper must respect first penalty alias (0=false)" - ); - assert!( - !cfg.retrieval.ranking_multiplicative_path_penalty_enabled(), - "effective helper must respect same first alias (0=false)" - ); - // With stored true, effective must still be false due to env first. - let rcfg = RetrievalConfig { - ranking_multiplicative_path_penalty: true, - ..Default::default() - }; - assert!( - !rcfg.ranking_multiplicative_path_penalty_enabled(), - "effective must prefer env first alias even when stored:true" - ); - } - - #[test] - #[ignore = "driven by alias_precedence_parity"] - fn penalty_alias_second_precedence_probe() { - assert!(std::env::var("VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY").is_err()); - assert_eq!(std::env::var("VERA_RANKING_PATH_PENALTY").unwrap(), "1"); - let cfg = VeraConfig::default(); - assert!(cfg.retrieval.ranking_multiplicative_path_penalty); - assert!(cfg.retrieval.ranking_multiplicative_path_penalty_enabled()); } // ── Filter-during-scan knob (#197) ── diff --git a/crates/vera-core/src/indexing/freshness.rs b/crates/vera-core/src/indexing/freshness.rs index e39ca890..2bb9d5ed 100644 --- a/crates/vera-core/src/indexing/freshness.rs +++ b/crates/vera-core/src/indexing/freshness.rs @@ -4,7 +4,7 @@ use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; -use anyhow::{Context, Result}; +use anyhow::{Context, Result, bail}; use rayon::prelude::*; use tracing::warn; @@ -36,6 +36,41 @@ pub fn index_format_is_current(metadata_store: &MetadataStore) -> bool { ) } +/// Reject indexes built with the retired character-cap chunking experiment. +/// +/// Missing or zero caps preserve compatibility with ordinary indexes. Inspect +/// the raw metadata before deserialization can discard historical aliases. +pub fn ensure_index_chunking_compatible( + metadata_store: &MetadataStore, + repo_path: &Path, +) -> Result<()> { + let rebuild = || { + format!( + "Run `vera index {}` to rebuild the full index.", + repo_path.display() + ) + }; + let Some(encoded) = metadata_store + .get_index_meta(INDEXING_CONFIG_KEY) + .context("failed to read saved indexing config")? + else { + return Ok(()); + }; + let value: serde_json::Value = serde_json::from_str(&encoded) + .with_context(|| format!("Invalid saved indexing config. {}", rebuild()))?; + for key in ["chunk_max_chars", "max_chunk_chars", "max_chunk_characters"] { + if let Some(cap) = value.get(key) + && cap.as_u64() != Some(0) + { + bail!( + "Index uses retired character-cap chunking ({key}={cap}). {}", + rebuild() + ); + } + } + Ok(()) +} + /// Summary of drift between the working tree and the current index. #[derive(Debug, Clone, Default, PartialEq, Eq, serde::Serialize)] pub struct IndexFreshness { @@ -126,6 +161,8 @@ pub fn detect_staleness( let metadata_store = MetadataStore::open(&metadata_path).context("failed to open metadata store")?; + ensure_index_chunking_compatible(&metadata_store, &repo_root)?; + let indexing_config = load_indexing_config(&metadata_store, fallback_config); let discovery = discovery::discover_files(&repo_root, &indexing_config) .context("failed to discover files for freshness scan")?; @@ -293,6 +330,45 @@ mod tests { assert_eq!(IndexFreshness::default().stale_warning(), None); } + #[test] + fn retired_character_caps_require_a_full_rebuild_under_every_alias() { + let metadata = MetadataStore::open_in_memory().unwrap(); + let root = Path::new("fixture-repo"); + ensure_index_chunking_compatible(&metadata, root).unwrap(); + for encoded in [ + r#"{}"#, + r#"{"chunk_max_chars":0,"max_chunk_chars":0,"max_chunk_characters":0}"#, + ] { + metadata + .set_index_meta(INDEXING_CONFIG_KEY, encoded) + .unwrap(); + ensure_index_chunking_compatible(&metadata, root).unwrap(); + } + for key in ["chunk_max_chars", "max_chunk_chars", "max_chunk_characters"] { + let mut encoded = serde_json::json!({ + "chunk_max_chars": 0, + "max_chunk_chars": 0, + "max_chunk_characters": 0, + }); + encoded[key] = serde_json::json!(750); + metadata + .set_index_meta(INDEXING_CONFIG_KEY, &encoded.to_string()) + .unwrap(); + let error = ensure_index_chunking_compatible(&metadata, root) + .unwrap_err() + .to_string(); + assert!(error.contains(key)); + assert!(error.contains("vera index fixture-repo")); + } + metadata + .set_index_meta(INDEXING_CONFIG_KEY, "invalid-json") + .unwrap(); + let error = ensure_index_chunking_compatible(&metadata, root) + .unwrap_err() + .to_string(); + assert!(error.contains("vera index fixture-repo")); + } + #[test] fn detects_added_modified_and_deleted_files() { let dir = tempdir().unwrap(); diff --git a/crates/vera-core/src/indexing/update.rs b/crates/vera-core/src/indexing/update.rs index d3a164a2..96b6176a 100644 --- a/crates/vera-core/src/indexing/update.rs +++ b/crates/vera-core/src/indexing/update.rs @@ -335,6 +335,7 @@ where let metadata_path = idx_dir.join("metadata.db"); let metadata_store = MetadataStore::open(&metadata_path).context("failed to open metadata store")?; + crate::indexing::freshness::ensure_index_chunking_compatible(&metadata_store, &repo_root)?; // Index format version must match: legacy suffixed rows (v1) are never silently reused. if !crate::indexing::freshness::index_format_is_current(&metadata_store) { @@ -946,6 +947,7 @@ mod regression_tests { update_repository_with_options_and_progress, }; use crate::storage::bm25::Bm25Index; + use crate::storage::metadata::MetadataStore; use crate::types::Language; use tempfile::tempdir; @@ -973,6 +975,111 @@ mod regression_tests { assert_eq!(content_hash(&preprocessed), update_hash); } + #[tokio::test] + async fn retired_character_cap_blocks_search_and_update_without_replacing_chunks() { + let dir = tempdir().unwrap(); + std::fs::write(dir.path().join("main.rs"), "fn old_name() {}\n").unwrap(); + let provider = MockProvider::new(8); + let config = VeraConfig::default(); + index_repository(dir.path(), &provider, &config, "mock-model") + .await + .unwrap(); + let idx = index_dir(&dir.path().canonicalize().unwrap()); + let store = MetadataStore::open(&idx.join("metadata.db")).unwrap(); + let bm25 = Bm25Index::open(&idx.join("bm25")).unwrap(); + let saved = store + .get_index_meta(crate::indexing::freshness::INDEXING_CONFIG_KEY) + .unwrap() + .unwrap(); + let context = crate::retrieval::search_service::SearchContext::bm25_only(); + let filters = crate::types::SearchFilters::default(); + assert!( + !context + .search(&idx, "old_name", None, &config, &filters, 5) + .await + .unwrap() + .0 + .is_empty() + ); + std::fs::write(dir.path().join("main.rs"), "fn new_name() {}\n").unwrap(); + for key in ["chunk_max_chars", "max_chunk_chars", "max_chunk_characters"] { + let mut metadata: serde_json::Value = serde_json::from_str(&saved).unwrap(); + metadata[key] = serde_json::json!(750); + store + .set_index_meta( + crate::indexing::freshness::INDEXING_CONFIG_KEY, + &metadata.to_string(), + ) + .unwrap(); + let error = context + .search(&idx, "old_name", None, &config, &filters, 5) + .await + .unwrap_err(); + assert!(error.to_string().contains("vera index")); + let error = crate::retrieval::search_bm25(&idx, "old_name", 5).unwrap_err(); + assert!(error.to_string().contains("vera index")); + for result in [ + crate::retrieval::search_bm25_with_stores(&bm25, &store, "old_name", 5), + crate::retrieval::search_bm25_with_stores_and_filters( + &bm25, + &store, + "old_name", + &crate::types::SearchFilters { + language: Some("rust".to_string()), + ..Default::default() + }, + 5, + ), + ] { + assert!(result.unwrap_err().to_string().contains("vera index")); + } + for result in [ + crate::retrieval::search_regex(&idx, "old_name", 5, false, 0, &filters), + crate::retrieval::search_structural( + &idx, + crate::retrieval::StructuralSearchKind::Definitions, + Some("old_name"), + 5, + &filters, + ), + crate::retrieval::search_callers(&idx, "old_name", 5, &filters), + crate::retrieval::type_relations::search_explicit_implementations( + &idx, "old_name", 5, &filters, + ), + ] { + assert!(result.unwrap_err().to_string().contains("vera index")); + } + let error = crate::retrieval::search_hybrid( + &idx, &provider, "old_name", "old_name", &filters, 5, 60.0, 8, 50, + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("vera index")); + let error = update_repository(dir.path(), &provider, &config, "mock-model") + .await + .unwrap_err(); + assert!(error.to_string().contains("vera index")); + let chunks = store.get_chunks_by_file("main.rs").unwrap(); + assert!( + chunks + .iter() + .any(|chunk| chunk.content.contains("old_name")) + ); + assert!( + chunks + .iter() + .all(|chunk| !chunk.content.contains("new_name")) + ); + } + store + .set_index_meta(crate::indexing::freshness::INDEXING_CONFIG_KEY, &saved) + .unwrap(); + let summary = update_repository(dir.path(), &provider, &config, "mock-model") + .await + .unwrap(); + assert_eq!(summary.files_modified, 1); + } + #[tokio::test] async fn update_removes_old_chunks_when_modified_file_has_no_chunks() { let dir = tempdir().unwrap(); diff --git a/crates/vera-core/src/parsing/chunker.rs b/crates/vera-core/src/parsing/chunker.rs index 8beaf78a..02e805f4 100644 --- a/crates/vera-core/src/parsing/chunker.rs +++ b/crates/vera-core/src/parsing/chunker.rs @@ -630,82 +630,6 @@ pub fn split_oversized_chunks(chunks: Vec, max_bytes: usize) -> Vec, max_chars: usize) -> Vec { - if max_chars == 0 { - return chunks; - } - - let mut result = Vec::with_capacity(chunks.len()); - for chunk in chunks { - let char_count = chunk.content.chars().count(); - if char_count <= max_chars { - result.push(chunk); - continue; - } - - let lines: Vec<&str> = chunk.content.lines().collect(); - let total = lines.len() as u32; - let mut current: u32 = 0; - let mut part = 1u32; - - while current < total { - let mut lo = current; - let mut hi = (total - 1).min(current + 500); - while lo < hi { - let mid = lo + (hi - lo).div_ceil(2); - let candidate: usize = lines[current as usize..=mid as usize] - .iter() - .map(|l| l.chars().count() + 1) - .sum(); - if candidate <= max_chars { - lo = mid; - } else { - hi = mid - 1; - } - } - - let end = lo.max(current); - let sub_content = lines[current as usize..=end as usize].join("\n"); - let bare_name = chunk.symbol_name.clone(); - let is_single = part == 1 && end + 1 >= total; - let part_index = if bare_name.is_none() || is_single { - None - } else { - Some(part) - }; - let id = if is_single { - chunk.id.clone() - } else { - format!("{}:{part}", chunk.id) - }; - - result.push(Chunk { - id, - file_path: chunk.file_path.clone(), - line_start: chunk.line_start + current, - line_end: chunk.line_start + end, - content: sub_content, - language: chunk.language, - symbol_type: chunk.symbol_type, - symbol_name: bare_name, - part_index, - }); - - part += 1; - current = end + 1; - } - } - result -} - /// Join lines from `start_row` to `end_row` (inclusive, 0-based) into a string. fn join_lines(lines: &[&str], start_row: u32, end_row: u32) -> String { let start = start_row as usize; @@ -1387,156 +1311,4 @@ mod tests { } // ── Issue #196 hypothesis: ~750-char chunks (DEFAULT OFF, byte-identical when off) ── - - #[test] - fn chunk_max_chars_off_is_byte_identical() { - // Gating: when chunk_max_chars is 0 (DEFAULT OFF), char splitting must be - // a no-op and produce byte-identical output vs master (no extra splits). - let content = "a".repeat(2000); - let chunk = Chunk { - id: "src/large.rs:0".to_string(), - file_path: "src/large.rs".to_string(), - line_start: 1, - line_end: 1, - content: content.clone(), - language: Language::Rust, - symbol_type: Some(SymbolType::Function), - symbol_name: Some("Large".to_string()), - part_index: None, - }; - let unsplit = split_oversized_chunks_by_chars(vec![chunk.clone()], 0); - assert_eq!( - unsplit.len(), - 1, - "max_chars=0 must not split (DEFAULT OFF, byte-identical)" - ); - assert_eq!(unsplit[0].content, content); - assert_eq!(unsplit[0].id, chunk.id); - assert_eq!(unsplit[0].part_index, None); - - // Also verify the high-level config gating: default IndexingConfig - // (chunk_max_chars=0) produces same chunks as master. - let default_cfg = IndexingConfig::default(); - assert_eq!(default_cfg.chunk_max_chars, 0); - assert_eq!(default_cfg.chunk_max_chars_effective(), 0); - assert!(!default_cfg.chunk_max_chars_enabled()); - // Simulate a file that would be split by the 750-char knob but not by - // the 24 KB byte cap. With knob off, it must stay single-part. - let line = "x".repeat(100); - let src = (0..10).map(|_| line.clone()).collect::>().join("\n"); - let chunks_byte_only = split_oversized_chunks( - vec![Chunk { - id: "src/file.rs:0".to_string(), - file_path: "src/file.rs".to_string(), - line_start: 1, - line_end: 10, - content: src.clone(), - language: Language::Rust, - symbol_type: Some(SymbolType::Function), - symbol_name: Some("Func".to_string()), - part_index: None, - }], - 24_576, - ); - assert_eq!( - chunks_byte_only.len(), - 1, - "24 KB cap should not split 1 KB file" - ); - let chunks_gated = split_oversized_chunks_by_chars( - chunks_byte_only.clone(), - default_cfg.chunk_max_chars_effective(), - ); - assert_eq!( - chunks_gated.len(), - chunks_byte_only.len(), - "char knob OFF must be byte-identical (no extra splits)" - ); - assert_eq!(chunks_gated[0].content, chunks_byte_only[0].content); - } - - #[test] - fn chunk_max_chars_on_splits_finer_than_byte_cap() { - // When enabled at ~750 chars, a long chunk should be split into - // finer pieces than the 24 KB byte cap would. - let content = "line content with some text\n".repeat(50); // ~1400 chars - let chunk = Chunk { - id: "src/large.rs:0".to_string(), - file_path: "src/large.rs".to_string(), - line_start: 1, - line_end: 50, - content: content.clone(), - language: Language::Rust, - symbol_type: Some(SymbolType::Function), - symbol_name: Some("Large".to_string()), - part_index: None, - }; - let unsplit_bytes = split_oversized_chunks(vec![chunk.clone()], 24_576); - assert_eq!( - unsplit_bytes.len(), - 1, - "24 KB byte cap should not split this" - ); - let split_chars = split_oversized_chunks_by_chars(vec![chunk], 750); - assert!( - split_chars.len() >= 2, - "750-char budget should split into ≥2 parts, got {}", - split_chars.len() - ); - for c in &split_chars { - assert!( - c.content.chars().count() <= 750, - "sub-chunk exceeds 750 chars: {} chars", - c.content.chars().count() - ); - assert_eq!(c.symbol_name.as_deref(), Some("Large")); - } - // part_index and bare-name shape mirrors byte path - for (idx, c) in split_chars.iter().enumerate() { - assert_eq!(c.part_index, Some((idx as u32) + 1)); - assert_eq!(c.id, format!("src/large.rs:0:{}", idx + 1)); - } - // Single-part unsplit case must keep bare name and no part_index - let small = Chunk { - id: "src/small.rs:0".to_string(), - file_path: "src/small.rs".to_string(), - line_start: 1, - line_end: 1, - content: "small".to_string(), - language: Language::Rust, - symbol_type: Some(SymbolType::Function), - symbol_name: Some("Small".to_string()), - part_index: None, - }; - let unsplit_small = split_oversized_chunks_by_chars(vec![small.clone()], 750); - assert_eq!(unsplit_small.len(), 1); - assert_eq!(unsplit_small[0].part_index, None); - assert_eq!(unsplit_small[0].id, small.id); - } - - #[test] - fn chunk_max_chars_split_preserves_no_gaps() { - // No content gaps when splitting by chars (same guarantee as bytes). - let lines: Vec = (0..20).map(|i| format!("line {i} content")).collect(); - let content = lines.join("\n"); - let chunk = Chunk { - id: "src/file.rs:0".to_string(), - file_path: "src/file.rs".to_string(), - line_start: 1, - line_end: 20, - content: content.clone(), - language: Language::Rust, - symbol_type: Some(SymbolType::Function), - symbol_name: Some("Func".to_string()), - part_index: None, - }; - let split = split_oversized_chunks_by_chars(vec![chunk], 50); - assert!(split.len() > 1); - let reconstructed = split - .iter() - .map(|c| c.content.clone()) - .collect::>() - .join("\n"); - assert_eq!(reconstructed, content); - } } diff --git a/crates/vera-core/src/parsing/mod.rs b/crates/vera-core/src/parsing/mod.rs index 36f46ccc..172c3f4b 100644 --- a/crates/vera-core/src/parsing/mod.rs +++ b/crates/vera-core/src/parsing/mod.rs @@ -35,21 +35,6 @@ pub struct ParseDiagnostics { pub used_tier0_fallback: bool, } -/// Apply both byte- and char-budget splits, gated behind the ~750-char knob. -/// -/// Byte splitting is always applied (max_chunk_bytes default 24576). Char -/// splitting is gated by `chunk_max_chars_effective()` so default behavior -/// remains byte-identical when the knob is off (DEFAULT OFF). -fn apply_splits(chunks: Vec, config: &IndexingConfig) -> Vec { - let chunks = chunker::split_oversized_chunks(chunks, config.max_chunk_bytes); - let max_chars = config.chunk_max_chars_effective(); - if max_chars != 0 { - chunker::split_oversized_chunks_by_chars(chunks, max_chars) - } else { - chunks - } -} - /// Parse a source file and return both code chunks and parser diagnostics. pub fn parse_file_with_diagnostics( source: &str, @@ -61,19 +46,23 @@ pub fn parse_file_with_diagnostics( if language == Language::Markdown { let chunks = chunker::markdown_section_chunks(source, file_path); return Ok(( - apply_splits(chunks, config), + chunker::split_oversized_chunks(chunks, config.max_chunk_bytes), Vec::new(), ParseDiagnostics::default(), )); } if language == Language::Rst { let (chunks, diagnostics) = parse_rst_section_chunks(source, file_path)?; - return Ok((apply_splits(chunks, config), Vec::new(), diagnostics)); + return Ok(( + chunker::split_oversized_chunks(chunks, config.max_chunk_bytes), + Vec::new(), + diagnostics, + )); } if language.prefers_file_chunking() { let chunks = chunker::whole_file_chunk(source, file_path, language); return Ok(( - apply_splits(chunks, config), + chunker::split_oversized_chunks(chunks, config.max_chunk_bytes), Vec::new(), ParseDiagnostics::default(), )); @@ -81,7 +70,7 @@ pub fn parse_file_with_diagnostics( if uses_indexing_tier0_fallback(language) { let chunks = chunker::tier0_line_chunks(source, file_path, language); return Ok(( - apply_splits(chunks, config), + chunker::split_oversized_chunks(chunks, config.max_chunk_bytes), Vec::new(), ParseDiagnostics { used_tier0_fallback: true, @@ -95,7 +84,7 @@ pub fn parse_file_with_diagnostics( None => { let chunks = chunker::tier0_line_chunks(source, file_path, language); return Ok(( - apply_splits(chunks, config), + chunker::split_oversized_chunks(chunks, config.max_chunk_bytes), Vec::new(), ParseDiagnostics { used_tier0_fallback: true, @@ -129,7 +118,7 @@ pub fn parse_file_with_diagnostics( }; Ok(( - apply_splits(chunks, config), + chunker::split_oversized_chunks(chunks, config.max_chunk_bytes), refs, ParseDiagnostics { tree_has_error, diff --git a/crates/vera-core/src/retrieval/bm25.rs b/crates/vera-core/src/retrieval/bm25.rs index 979868bd..d63d196c 100644 --- a/crates/vera-core/src/retrieval/bm25.rs +++ b/crates/vera-core/src/retrieval/bm25.rs @@ -34,11 +34,8 @@ const BM25_HYDRATION_PAGE_MAX: usize = 900; /// A vector of `SearchResult` with full chunk metadata, sorted by score descending. pub fn search_bm25(index_dir: &Path, query: &str, limit: usize) -> Result> { let bm25_dir = index_dir.join("bm25"); - let metadata_path = index_dir.join("metadata.db"); - let bm25_index = Bm25Index::open(&bm25_dir).context("failed to open BM25 index for search")?; - let metadata_store = - MetadataStore::open(&metadata_path).context("failed to open metadata store for search")?; + let metadata_store = super::open_search_metadata(index_dir)?; search_bm25_with_stores(&bm25_index, &metadata_store, query, limit) } @@ -97,6 +94,7 @@ fn search_bm25_with_stores_inner( raw_limit: usize, filters: Option<&SearchFilters>, ) -> Result> { + crate::indexing::freshness::ensure_index_chunking_compatible(metadata_store, Path::new("."))?; if limit == 0 { return Ok(Vec::new()); } diff --git a/crates/vera-core/src/retrieval/graph_augmentation.rs b/crates/vera-core/src/retrieval/graph_augmentation.rs deleted file mode 100644 index b4845b37..00000000 --- a/crates/vera-core/src/retrieval/graph_augmentation.rs +++ /dev/null @@ -1,416 +0,0 @@ -//! Experimental structural graph augmentation for retrieval. -//! -//! The graph lookups are intentionally supplemental. A failed lookup is -//! treated as an empty result so older or partially-built indexes do not make -//! ordinary search fail. - -use std::collections::HashSet; -use std::path::Path; - -use tracing::debug; - -use crate::retrieval::references::search_callers; -use crate::retrieval::type_relations::search_explicit_implementations; -use crate::types::{SearchFilters, SearchResult, SymbolType}; - -const MAX_SEEDS: usize = 5; -const MAX_LOOKUP_RESULTS: usize = 5; -const MAX_AUGMENTED_CANDIDATES: usize = 20; -const RRF_K: f64 = 60.0; - -const CALLER_DENYLIST: &[&str] = &[ - "new", - "default", - "fmt", - "display", - "debug", - "clone", - "drop", - "to_string", - "from", - "into", - "len", - "get", - "set", - "main", - "run", - "test", - "init", - "new_instance", -]; - -/// Expand a fused pool with caller and explicit implementation results. -/// -/// The synchronous graph APIs perform bounded SQLite and source-file reads. -/// They are called directly here because the fixed seed and per-lookup caps -/// bound the amount of supplemental work. There is deliberately no timeout: -/// cancelling a blocking lookup would require detached work and could leave -/// an index read running after the search has returned. -pub(crate) fn augment_pool( - index_dir: &Path, - pool: &mut Vec, - filters: &SearchFilters, -) -> usize { - let seed_symbols = select_seed_symbols(pool); - if seed_symbols.is_empty() { - return 0; - } - - let mut candidates = Vec::with_capacity(seed_symbols.len() * MAX_LOOKUP_RESULTS * 2); - for symbol in seed_symbols { - if !caller_lookup_is_skipped(&symbol) { - match search_callers(index_dir, &symbol, MAX_LOOKUP_RESULTS, filters) { - Ok(results) => candidates.extend(results.into_iter().take(MAX_LOOKUP_RESULTS)), - Err(error) => debug!( - symbol = %symbol, - error = %error, - "graph callers lookup failed; skipping lookup" - ), - } - } - - match search_explicit_implementations(index_dir, &symbol, MAX_LOOKUP_RESULTS, filters) { - Ok(results) => candidates.extend(results.into_iter().take(MAX_LOOKUP_RESULTS)), - Err(error) => debug!( - symbol = %symbol, - error = %error, - "graph implementations lookup failed; skipping lookup" - ), - } - } - - inject_augmented_candidates(pool, candidates) -} - -/// Select up to five distinct eligible symbols from the highest-scoring pool -/// results. The original pool order is not changed. -fn select_seed_symbols(pool: &[SearchResult]) -> Vec { - let mut ranked: Vec<&SearchResult> = pool.iter().collect(); - ranked.sort_by(|left, right| right.score.total_cmp(&left.score)); - - let mut seen = HashSet::new(); - ranked - .into_iter() - .filter_map(|result| { - let symbol_type = result.symbol_type?; - if !is_seed_symbol_type(symbol_type) { - return None; - } - - let symbol = result.symbol_name.as_deref()?; - if symbol.is_empty() { - return None; - } - seen.insert(symbol.to_string()).then(|| symbol.to_string()) - }) - .take(MAX_SEEDS) - .collect() -} - -fn is_seed_symbol_type(symbol_type: SymbolType) -> bool { - matches!( - symbol_type, - SymbolType::Function - | SymbolType::Method - | SymbolType::Struct - | SymbolType::Enum - | SymbolType::Class - | SymbolType::Trait - | SymbolType::Interface - ) -} - -fn caller_lookup_is_skipped(symbol: &str) -> bool { - symbol.chars().count() <= 2 - || CALLER_DENYLIST - .iter() - .any(|denied| symbol.eq_ignore_ascii_case(denied)) -} - -/// Append graph candidates after deduplicating them against the organic pool. -/// -/// This pure step is kept separate from the SQLite lookups so score, dedup, -/// and cap behavior can be tested without constructing a full index fixture. -fn inject_augmented_candidates( - pool: &mut Vec, - candidates: impl IntoIterator, -) -> usize { - let pool_len = pool.len(); - let sentinel_score = 1.0 / (RRF_K + pool_len as f64 + 1.0); - let mut seen: HashSet<(String, u32)> = pool - .iter() - .map(|result| (result.file_path.clone(), result.line_start)) - .collect(); - let mut added = 0; - - for mut candidate in candidates { - if added >= MAX_AUGMENTED_CANDIDATES { - break; - } - - let key = (candidate.file_path.clone(), candidate.line_start); - if !seen.insert(key) { - continue; - } - - candidate.score = sentinel_score; - pool.push(candidate); - added += 1; - } - - added -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::types::Language; - - fn result( - file_path: &str, - line_start: u32, - score: f64, - symbol_name: Option<&str>, - symbol_type: Option, - ) -> SearchResult { - SearchResult { - file_path: file_path.to_string(), - line_start, - line_end: line_start + 2, - content: format!("content for {file_path}"), - language: Language::Rust, - score, - symbol_name: symbol_name.map(str::to_string), - symbol_type, - part_index: None, - } - } - - #[test] - fn selects_only_eligible_symbols_from_top_five() { - let pool = vec![ - result("block.rs", 1, 1.0, None, Some(SymbolType::Block)), - result( - "module.rs", - 1, - 0.9, - Some("module"), - Some(SymbolType::Module), - ), - result( - "function.rs", - 1, - 0.8, - Some("function_name"), - Some(SymbolType::Function), - ), - result( - "trait.rs", - 1, - 0.7, - Some("TraitName"), - Some(SymbolType::Trait), - ), - result( - "class.rs", - 1, - 0.6, - Some("ClassName"), - Some(SymbolType::Class), - ), - result( - "fifth-eligible.rs", - 1, - 0.5, - Some("fifth_eligible"), - Some(SymbolType::Function), - ), - ]; - - assert_eq!( - select_seed_symbols(&pool), - vec![ - "function_name".to_string(), - "TraitName".to_string(), - "ClassName".to_string(), - "fifth_eligible".to_string(), - ] - ); - } - - #[test] - fn caller_relation_adds_definition_chunk_with_sentinel_score() { - use crate::parsing::references::RawReference; - use crate::storage::metadata::MetadataStore; - use crate::types::Chunk; - use tempfile::tempdir; - - let repo_dir = tempdir().unwrap(); - let index_dir = repo_dir.path().join(".vera"); - std::fs::create_dir_all(&index_dir).unwrap(); - std::fs::write( - repo_dir.path().join("caller.rs"), - "fn caller() {\n target();\n}\n", - ) - .unwrap(); - - let store = MetadataStore::open(&index_dir.join("metadata.db")).unwrap(); - store - .insert_chunks(&[ - Chunk { - id: "seed:0".to_string(), - file_path: "seed.rs".to_string(), - line_start: 1, - line_end: 3, - content: "fn target() {}".to_string(), - language: Language::Rust, - symbol_type: Some(SymbolType::Function), - symbol_name: Some("target".to_string()), - part_index: None, - }, - Chunk { - id: "caller:0".to_string(), - file_path: "caller.rs".to_string(), - line_start: 1, - line_end: 3, - content: "fn caller() {\n target();\n}".to_string(), - language: Language::Rust, - symbol_type: Some(SymbolType::Function), - symbol_name: Some("caller".to_string()), - part_index: None, - }, - ]) - .unwrap(); - let references = [RawReference { - callee: "target".to_string(), - caller: Some("caller".to_string()), - qualifier: None, - line: 2, - }]; - store - .insert_parse_artifacts_batch_borrowed(&[("caller.rs", &references)], &[]) - .unwrap(); - - let mut pool = vec![result( - "seed.rs", - 1, - 0.5, - Some("target"), - Some(SymbolType::Function), - )]; - - assert_eq!( - augment_pool(&index_dir, &mut pool, &SearchFilters::default()), - 1 - ); - assert_eq!(pool.len(), 2); - assert_eq!(pool[1].file_path, "caller.rs"); - assert_eq!(pool[1].line_start, 1); - assert_eq!(pool[1].symbol_name.as_deref(), Some("caller")); - assert_eq!(pool[1].score, 1.0 / 62.0); - } - - #[test] - fn injects_with_sentinel_and_deduplicates_by_file_and_start_line() { - let mut pool = vec![result( - "organic.rs", - 10, - 0.5, - Some("seed"), - Some(SymbolType::Function), - )]; - let candidates = vec![ - result( - "organic.rs", - 10, - 1.0, - Some("seed"), - Some(SymbolType::Function), - ), - result( - "caller.rs", - 20, - 1.0, - Some("caller"), - Some(SymbolType::Function), - ), - result( - "caller.rs", - 20, - 0.8, - Some("caller"), - Some(SymbolType::Function), - ), - ]; - - assert_eq!(inject_augmented_candidates(&mut pool, candidates), 1); - assert_eq!(pool.len(), 2); - assert_eq!(pool[0].score, 0.5); - assert_eq!(pool[1].file_path, "caller.rs"); - assert_eq!(pool[1].score, 1.0 / 62.0); - } - - #[test] - fn caps_augmented_candidates_at_twenty() { - let mut pool = vec![result( - "seed.rs", - 1, - 1.0, - Some("seed"), - Some(SymbolType::Function), - )]; - let candidates = (0..25).map(|index| { - result( - &format!("caller-{index}.rs"), - 1, - 1.0, - Some("caller"), - Some(SymbolType::Function), - ) - }); - - assert_eq!(inject_augmented_candidates(&mut pool, candidates), 20); - assert_eq!(pool.len(), 21); - } - - #[test] - fn skips_callers_for_short_and_ubiquitous_names() { - for symbol in ["new", "DEFAULT", "fmt", "x", "ab"] { - assert!( - caller_lookup_is_skipped(symbol), - "{symbol} should be skipped" - ); - } - assert!(!caller_lookup_is_skipped("authenticate_user")); - } - - #[test] - fn normalizes_split_chunk_seed_names() { - let pool = vec![ - SearchResult { - file_path: "large.rs".to_string(), - line_start: 1, - line_end: 3, - content: "content".to_string(), - language: Language::Rust, - score: 1.0, - symbol_name: Some("LargeFunction".to_string()), - symbol_type: Some(SymbolType::Function), - part_index: Some(1), - }, - SearchResult { - file_path: "large.rs".to_string(), - line_start: 20, - line_end: 22, - content: "content".to_string(), - language: Language::Rust, - score: 0.9, - symbol_name: Some("LargeFunction".to_string()), - symbol_type: Some(SymbolType::Function), - part_index: Some(2), - }, - ]; - - assert_eq!(select_seed_symbols(&pool), vec!["LargeFunction"]); - } -} diff --git a/crates/vera-core/src/retrieval/hybrid.rs b/crates/vera-core/src/retrieval/hybrid.rs index 3f017bd2..495cc61b 100644 --- a/crates/vera-core/src/retrieval/hybrid.rs +++ b/crates/vera-core/src/retrieval/hybrid.rs @@ -18,7 +18,6 @@ use tracing::{debug, info, warn}; use crate::embedding::EmbeddingProvider; use crate::retrieval::bm25::search_bm25_with_stores_and_filters; -use crate::retrieval::graph_augmentation::augment_pool; use crate::retrieval::query_classifier::{QueryType, classify_query, params_for_query_type}; use crate::retrieval::query_utils::result_key; use crate::retrieval::ranking::is_path_weighted_query; @@ -210,8 +209,7 @@ impl SearchStores { let bm25 = Bm25Index::open(&index_dir.join("bm25")) .context("failed to open BM25 index for search")?; let metadata_path = index_dir.join("metadata.db"); - let bm25_metadata = MetadataStore::open(&metadata_path) - .context("failed to open metadata store for search")?; + let bm25_metadata = super::open_search_metadata(index_dir)?; let vector_metadata = MetadataStore::open(&metadata_path) .context("failed to open vector metadata store for search")?; let open_stamp = metadata_db_stamp(&metadata_path); @@ -357,7 +355,6 @@ impl SearchStores { } #[cfg(test)] - #[allow(dead_code)] pub(crate) fn eligibility_cached(&self) -> bool { self.eligibility .lock() @@ -365,12 +362,6 @@ impl SearchStores { .unwrap_or(false) } - #[cfg(test)] - #[allow(dead_code)] - pub(crate) fn invalidate_eligibility_for_test(&self) { - let _ = self.eligibility.lock().map(|mut g| *g = None); - } - pub(crate) fn vector_store(&self, dim: usize) -> Result>> { let mut cached = self .vector @@ -487,40 +478,6 @@ pub async fn search_hybrid( .await } -/// Perform hybrid search using stores retained by a reusable search context. -#[allow(clippy::too_many_arguments)] -#[allow(dead_code)] -pub(crate) async fn search_hybrid_with_stores( - index_dir: &Path, - provider: &impl EmbeddingProvider, - bm25_query: &str, - vector_query: &str, - filters: &SearchFilters, - limit: usize, - rrf_k: f64, - stored_dim: usize, - vector_candidates: usize, - stores: Arc, -) -> Result<(Vec, HybridTimings), HybridSearchError> { - let filter_during_scan_enabled = crate::config::VeraConfig::default() - .retrieval - .vector_filter_during_scan_enabled(); - search_hybrid_inner( - index_dir, - provider, - bm25_query, - vector_query, - filters, - limit, - rrf_k, - stored_dim, - vector_candidates, - filter_during_scan_enabled, - Some(stores), - ) - .await -} - /// Perform hybrid search using stores + explicit filter-during-scan flag. /// SearchContext uses this to respect file-config + env precedence. #[allow(clippy::too_many_arguments)] @@ -567,6 +524,9 @@ async fn search_hybrid_inner( filter_during_scan_enabled: bool, stores: Option>, ) -> Result<(Vec, HybridTimings), HybridSearchError> { + if stores.is_none() { + super::open_search_metadata(index_dir)?; + } let query_type = classify_query(bm25_query); let query_params = params_for_query_type(query_type); let bm25_candidates = compute_bm25_candidates(bm25_query, limit); @@ -942,43 +902,6 @@ pub async fn search_hybrid_reranked( stored_dim: usize, rerank_candidates: usize, vector_candidates: usize, -) -> Result<(Vec, HybridTimings), HybridSearchError> { - search_hybrid_reranked_with_augmentation( - index_dir, - provider, - reranker, - bm25_query, - vector_query, - filters, - fetch_limit, - result_limit, - rrf_k, - stored_dim, - rerank_candidates, - vector_candidates, - false, - ) - .await -} - -/// Perform reranked hybrid search using stores retained by a reusable context. -#[allow(clippy::too_many_arguments)] -#[allow(dead_code)] -pub(crate) async fn search_hybrid_reranked_with_stores( - index_dir: &Path, - provider: &impl EmbeddingProvider, - reranker: &impl Reranker, - bm25_query: &str, - vector_query: &str, - filters: &SearchFilters, - fetch_limit: usize, - result_limit: usize, - rrf_k: f64, - stored_dim: usize, - rerank_candidates: usize, - vector_candidates: usize, - graph_augmentation_enabled: bool, - stores: Arc, ) -> Result<(Vec, HybridTimings), HybridSearchError> { search_hybrid_reranked_inner( index_dir, @@ -993,86 +916,6 @@ pub(crate) async fn search_hybrid_reranked_with_stores( stored_dim, rerank_candidates, vector_candidates, - graph_augmentation_enabled, - crate::config::VeraConfig::default() - .retrieval - .vector_filter_during_scan_enabled(), - Some(stores), - ) - .await -} - -/// Flagged variant respecting config-override for filter-during-scan. -#[allow(clippy::too_many_arguments)] -#[allow(dead_code)] -pub(crate) async fn search_hybrid_reranked_with_stores_and_flag( - index_dir: &Path, - provider: &impl EmbeddingProvider, - reranker: &impl Reranker, - bm25_query: &str, - vector_query: &str, - filters: &SearchFilters, - fetch_limit: usize, - result_limit: usize, - rrf_k: f64, - stored_dim: usize, - rerank_candidates: usize, - vector_candidates: usize, - graph_augmentation_enabled: bool, - stores: Arc, - filter_during_scan_enabled: bool, -) -> Result<(Vec, HybridTimings), HybridSearchError> { - search_hybrid_reranked_inner( - index_dir, - provider, - reranker, - bm25_query, - vector_query, - filters, - fetch_limit, - result_limit, - rrf_k, - stored_dim, - rerank_candidates, - vector_candidates, - graph_augmentation_enabled, - filter_during_scan_enabled, - Some(stores), - ) - .await -} - -/// Perform hybrid search with optional experimental graph augmentation. -#[allow(clippy::too_many_arguments)] -pub(crate) async fn search_hybrid_reranked_with_augmentation( - index_dir: &Path, - provider: &impl EmbeddingProvider, - reranker: &impl Reranker, - bm25_query: &str, - vector_query: &str, - filters: &SearchFilters, - fetch_limit: usize, - result_limit: usize, - rrf_k: f64, - stored_dim: usize, - rerank_candidates: usize, - vector_candidates: usize, - graph_augmentation_enabled: bool, -) -> Result<(Vec, HybridTimings), HybridSearchError> { - search_hybrid_reranked_inner( - index_dir, - provider, - reranker, - bm25_query, - vector_query, - filters, - fetch_limit, - result_limit, - rrf_k, - stored_dim, - rerank_candidates, - vector_candidates, - graph_augmentation_enabled, crate::config::VeraConfig::default() .retrieval .vector_filter_during_scan_enabled(), @@ -1081,8 +924,9 @@ pub(crate) async fn search_hybrid_reranked_with_augmentation( .await } +/// Reranked search with retained stores and an explicit filter-scan setting. #[allow(clippy::too_many_arguments)] -pub(crate) async fn search_hybrid_reranked_with_augmentation_and_flag( +pub(crate) async fn search_hybrid_reranked_with_stores_and_flag( index_dir: &Path, provider: &impl EmbeddingProvider, reranker: &impl Reranker, @@ -1095,7 +939,6 @@ pub(crate) async fn search_hybrid_reranked_with_augmentation_and_flag( stored_dim: usize, rerank_candidates: usize, vector_candidates: usize, - graph_augmentation_enabled: bool, stores: Arc, filter_during_scan_enabled: bool, ) -> Result<(Vec, HybridTimings), HybridSearchError> { @@ -1112,7 +955,6 @@ pub(crate) async fn search_hybrid_reranked_with_augmentation_and_flag( stored_dim, rerank_candidates, vector_candidates, - graph_augmentation_enabled, filter_during_scan_enabled, Some(stores), ) @@ -1133,13 +975,12 @@ async fn search_hybrid_reranked_inner( stored_dim: usize, rerank_candidates: usize, vector_candidates: usize, - graph_augmentation_enabled: bool, filter_during_scan_enabled: bool, stores: Option>, ) -> Result<(Vec, HybridTimings), HybridSearchError> { let fusion_limit = rerank_candidates.max(fetch_limit); - let (mut hybrid_results, mut timings) = match stores { + let (hybrid_results, mut timings) = match stores { Some(stores) => { search_hybrid_with_stores_and_flag( index_dir, @@ -1178,29 +1019,16 @@ async fn search_hybrid_reranked_inner( } }; - let augmented_count = if graph_augmentation_enabled { - augment_pool(index_dir, &mut hybrid_results, filters) - } else { - 0 - }; - if hybrid_results.is_empty() { return Ok((hybrid_results, timings)); } - if hybrid_results.len() <= result_limit && augmented_count == 0 { + if hybrid_results.len() <= result_limit { return Ok((hybrid_results, timings)); } let rerank_start = Instant::now(); - // When graph candidates were appended, score the whole expanded pool so - // candidates beyond the normal rerank prefix still compete semantically. - let rerank_limit = if augmented_count > 0 { - hybrid_results.len() - } else { - rerank_candidates - }; - match rerank_results(reranker, vector_query, &hybrid_results, rerank_limit).await { + match rerank_results(reranker, vector_query, &hybrid_results, rerank_candidates).await { Ok(mut reranked) => { timings.reranking = Some(rerank_start.elapsed()); info!( @@ -1209,7 +1037,8 @@ async fn search_hybrid_reranked_inner( reranked = reranked.len(), "reranking complete" ); - reranked = merge_reranked_results(reranked, hybrid_results, rerank_limit, fetch_limit); + reranked = + merge_reranked_results(reranked, hybrid_results, rerank_candidates, fetch_limit); Ok((reranked, timings)) } Err(rerank_err) => { diff --git a/crates/vera-core/src/retrieval/mod.rs b/crates/vera-core/src/retrieval/mod.rs index d7dfe3a5..e572a0c8 100644 --- a/crates/vera-core/src/retrieval/mod.rs +++ b/crates/vera-core/src/retrieval/mod.rs @@ -10,7 +10,6 @@ pub mod bm25; pub(crate) mod exact_matches; -pub(crate) mod graph_augmentation; pub mod hybrid; pub mod query_classifier; pub mod ranking; @@ -52,6 +51,18 @@ use crate::config::{DEFAULT_MAX_FILE_SIZE_BYTES, IndexingConfig}; use crate::storage::metadata::MetadataStore; use crate::types::{SearchFilters, SearchResult}; +/// Open search metadata only after checking the stored chunking contract. +pub(crate) fn open_search_metadata(index_dir: &std::path::Path) -> anyhow::Result { + use anyhow::Context; + let store = MetadataStore::open(&index_dir.join("metadata.db")) + .context("failed to open metadata store for search")?; + crate::indexing::freshness::ensure_index_chunking_compatible( + &store, + index_dir.parent().unwrap_or(index_dir), + )?; + Ok(store) +} + /// The file-size cap query-time source reads must honor. /// /// Indexing records its `IndexingConfig` alongside the index, and retrieval diff --git a/crates/vera-core/src/retrieval/ranking/mod.rs b/crates/vera-core/src/retrieval/ranking/mod.rs index 71faa5f3..48d0c6a4 100644 --- a/crates/vera-core/src/retrieval/ranking/mod.rs +++ b/crates/vera-core/src/retrieval/ranking/mod.rs @@ -65,26 +65,8 @@ pub(crate) fn apply_query_ranking_with_filters_and_config( finish_ranking(results, scores, wants_diversity) } -/// Multi-query ranking: each subquery carries its own identifier or filename -/// target, so score the pool under every subquery's features and keep each -/// result's best score. A single joined query would promote only the first -/// subquery's exact match and crowd out the rest (issue #121). -#[allow(dead_code)] -pub(crate) fn apply_query_ranking_multi_query( - queries: &[String], - results: Vec, - stage: RankingStage, - filters: &SearchFilters, -) -> Vec { - apply_query_ranking_multi_query_with_config( - queries, - results, - stage, - filters, - &VeraConfig::default(), - ) -} - +/// Score under each subquery's features and keep each result's best score. +/// Joining queries would promote only the first target and crowd out the rest. pub(crate) fn apply_query_ranking_multi_query_with_config( queries: &[String], results: Vec, @@ -110,19 +92,8 @@ pub(crate) fn apply_query_ranking_multi_query_with_config( finish_ranking(results, scores, wants_diversity) } -/// Score every pool entry under one query's features: retrieval position -/// (base rank), additive priors, then pool-relative boosts scaled by the -/// pool's best combined score so signal strength tracks retrieval confidence. -#[allow(dead_code)] -fn score_pool( - features: &QueryFeatures, - stage: RankingStage, - filters: &SearchFilters, - results: &[SearchResult], -) -> Vec { - score_pool_with_config(features, stage, filters, results, &VeraConfig::default()) -} - +/// Combine retrieval position and additive priors, then apply pool-relative +/// boosts in their established order so signal strength tracks confidence. fn score_pool_with_config( features: &QueryFeatures, stage: RankingStage, @@ -150,9 +121,6 @@ fn score_pool_with_config( if retrieval.ranking_definition_boost_enabled() { apply_content_symbol_boost(features, &mut scores, results, max_score); } - if retrieval.ranking_multiplicative_path_penalty_enabled() { - apply_multiplicative_path_penalty(features, &mut scores, results); - } scores } diff --git a/crates/vera-core/src/retrieval/ranking/score.rs b/crates/vera-core/src/retrieval/ranking/score.rs index 652d2434..f9b5c057 100644 --- a/crates/vera-core/src/retrieval/ranking/score.rs +++ b/crates/vera-core/src/retrieval/ranking/score.rs @@ -1,7 +1,7 @@ //! Prior scoring and result-shaping heuristics. use crate::chunk_text::file_name; -use crate::config::{RetrievalConfig, VeraConfig}; +use crate::config::RetrievalConfig; use crate::corpus::{ContentClass, classify_content}; use crate::retrieval::query_classifier::QueryType; use crate::retrieval::query_utils::{ @@ -87,22 +87,6 @@ pub(super) const CONTENT_SYMBOL_WEIGHT_EMBEDDED: f64 = 1.5; /// Content-definition weight for identifier queries. pub(super) const CONTENT_SYMBOL_WEIGHT_IDENT: f64 = 3.0; -#[allow(dead_code)] -pub(super) fn score_prior( - features: &QueryFeatures, - result: &SearchResult, - stage: RankingStage, - filters: &SearchFilters, -) -> f64 { - score_prior_with_config( - features, - result, - stage, - filters, - &VeraConfig::default().retrieval, - ) -} - pub(super) fn score_prior_with_config( features: &QueryFeatures, result: &SearchResult, @@ -593,23 +577,6 @@ pub(super) fn apply_keyword_path_boost( } } -/// Backward-compatible wrapper for tests that don't pass config (defaults preserved). -#[allow(dead_code)] -pub(super) fn apply_keyword_path_boost_legacy( - features: &QueryFeatures, - scores: &mut [f64], - results: &[SearchResult], - max_score: f64, -) { - apply_keyword_path_boost( - features, - scores, - results, - max_score, - &RetrievalConfig::default(), - ) -} - /// Pool-relative content-based symbol definition boost. A chunk whose /// text actually defines the queried symbol ("class Session", /// "CREATE TABLE sessions") is the definition site regardless of what symbol @@ -687,14 +654,6 @@ pub(super) fn apply_content_symbol_boost( } } -/// Multiplicative path penalty factor for test/compat/example directories. -/// ~0.3× as hypothesized in #196: boilerplate / fixture / example directories -/// are keyword-dense but rarely contain the implementation sought. Multiplying -/// (rather than subtracting) demotes proportionally to retrieval confidence, -/// preserving ordering among non-penalized candidates while consistently -/// demoting penalized ones. -pub(super) const MULTIPLICATIVE_PATH_PENALTY_FACTOR: f64 = 0.3; - /// Shared directory-classification constants (single definition to prevent drift). const TEST_DIRS: &[&str] = &[ "t", @@ -721,79 +680,6 @@ const EXAMPLE_DIRS: &[&str] = &[ "benchmarks", ]; -/// Apply the ~0.3× multiplicative penalty for test/compat/example paths. -/// -/// Respects the existing boost-directory gating (definition boost's -/// `wants_*` checks) so explicit requests for those paths are not penalized. -/// A file is penalized if its path lies in `tests/`, `compat/`, or -/// `examples/` (or is classified as `Test`/`Example`/`Bench`) and the query -/// does not ask for that category. The penalty multiplies the score, so two -/// equally-scoring candidates (`src/` vs `tests/`) will rank `src/` higher -/// when the knob is on. -pub(super) fn apply_multiplicative_path_penalty( - features: &QueryFeatures, - scores: &mut [f64], - results: &[SearchResult], -) { - for (score, result) in scores.iter_mut().zip(results) { - let lower = result.file_path.to_ascii_lowercase(); - let role = classify_content(&result.file_path, result.language, &result.content); - let mut penalized = false; - - // Test / fixture penalization: mirrors the additive penalty's - // ContentClass gate plus the definition-boost directory gating so - // both signals respect the same explicit-request logic. - if !features.wants_test_paths - && (matches!(role, ContentClass::Test) - || definition_site_role_blocked_is_test(features, result)) - { - penalized = true; - } - if !penalized - && !features.wants_example_paths - && (matches!(role, ContentClass::Example | ContentClass::Bench) - || definition_site_role_blocked_is_example(features, result)) - { - penalized = true; - } - if !penalized && !features.wants_compat_paths && is_compat_path(&lower) { - penalized = true; - } - - if penalized { - *score *= MULTIPLICATIVE_PATH_PENALTY_FACTOR; - } - } -} - -fn definition_site_role_blocked_is_test(features: &QueryFeatures, result: &SearchResult) -> bool { - let lower = result.file_path.to_ascii_lowercase(); - let mut parts = lower.rsplit('/'); - let filename = parts.next().unwrap_or(""); - let in_test_dir = parts.any(|dir| TEST_DIRS.contains(&dir)); - if in_test_dir || is_test_filename(filename) { - return !features.wants_test_paths; - } - false -} - -fn definition_site_role_blocked_is_example( - features: &QueryFeatures, - result: &SearchResult, -) -> bool { - let lower = result.file_path.to_ascii_lowercase(); - let mut parts = lower.rsplit('/'); - let filename = parts.next().unwrap_or(""); - let in_example_dir = parts.clone().any(|dir| EXAMPLE_DIRS.contains(&dir)); - // is_test_filename part already handled in test check; example only cares about dir - if in_example_dir { - return !features.wants_example_paths; - } - // also treat example-like filenames conservatively? filename alone not penalized - let _ = filename; - false -} - /// A chunk counts as a definition site for the content-symbol boost only /// when its path marks it as source-like. Definitions in test, example, and /// bench trees are fixtures or usage samples; they qualify only when the diff --git a/crates/vera-core/src/retrieval/ranking/tests.rs b/crates/vera-core/src/retrieval/ranking/tests.rs index 7adfc369..22f7e235 100644 --- a/crates/vera-core/src/retrieval/ranking/tests.rs +++ b/crates/vera-core/src/retrieval/ranking/tests.rs @@ -818,185 +818,6 @@ fn file_coherence_boost_survives_definition_flag_toggle() { ); } -// ── Issue #196 hypotheses: multiplicative path penalty (DEFAULT OFF, 0.3×) ── - -#[test] -fn multiplicative_path_penalty_is_toggleable() { - // Two files with equal base rank and neutral content; one is in tests/. - // With penalty disabled (DEFAULT OFF) the input order holds. With it - // enabled, the tests/ fixture must be demoted below src/ — multiplicative - // 0.3× preserves ordering among non-penalized while demoting penalized. - let neutral = "pub fn helper() {}"; - - // DEFAULT OFF: first result (tests/) should stay first because scores are - // base_rank + prior where prior's additive test penalty (-0.95) already - // applies, but the multiplicative gate is OFF. To isolate the multiplicative - // effect, we use a query that does NOT trigger additive test penalty? - // Actually additive penalty always applies for tests/ when not wanting tests. - // The multiplicative is extra; with it OFF, ordering is determined by base - // rank + additive only. Since first has higher base_rank (1.0 vs 0.5), it - // may still outrank src/ depending on additive. Instead we place src/ first - // so that additive alone keeps src/ first, and multiplicative is the extra - // guarantee. Simplify: src/ first, tests/ second — with additive, src/ - // already wins; multiplicative keeps that. - let src_first = vec![ - make_result( - "src/helper.rs", - Some("helper"), - Some(SymbolType::Function), - neutral, - ), - make_result( - "tests/fixtures/helper.rs", - Some("helper"), - Some(SymbolType::Function), - neutral, - ), - ]; - - let mut disabled = VeraConfig::default(); - disabled.retrieval.ranking_multiplicative_path_penalty = false; - let ranked_off = apply_query_ranking_with_filters_and_config( - "helper utility", - src_first.clone(), - RankingStage::Initial, - &SearchFilters::default(), - &disabled, - ); - assert_eq!( - ranked_off[0].file_path, "src/helper.rs", - "with penalty OFF, src/ should still beat tests/ (additive already)" - ); - - let mut enabled = VeraConfig::default(); - enabled.retrieval.ranking_multiplicative_path_penalty = true; - let ranked_on = apply_query_ranking_with_filters_and_config( - "helper utility", - src_first, - RankingStage::Initial, - &SearchFilters::default(), - &enabled, - ); - assert_eq!( - ranked_on[0].file_path, "src/helper.rs", - "with penalty ON, src/ must still beat tests/ (multiplicative reinforces)" - ); - - // More direct: place tests/ first — even though it starts with higher - // base_rank, the multiplicative penalty must demote it below src/. - let tests_first = vec![ - make_result( - "tests/fixtures/helper.rs", - Some("helper"), - Some(SymbolType::Function), - neutral, - ), - make_result( - "src/helper.rs", - Some("helper"), - Some(SymbolType::Function), - neutral, - ), - ]; - let ranked_tests_first_on = apply_query_ranking_with_filters_and_config( - "helper utility", - tests_first.clone(), - RankingStage::Initial, - &SearchFilters::default(), - &enabled, - ); - assert_eq!( - ranked_tests_first_on[0].file_path, "src/helper.rs", - "multiplicative 0.3× must demote tests/ below equally-scoring src/ even when tests/ starts first" - ); - - // Respects gating: when query explicitly wants tests, penalty must not fire. - // Directly verify the multiplicative penalty respects wants_test_paths - // gating — other ranking signals (Source bonus etc.) may still order src - // before tests, so we check the multiplicative layer alone. - { - let wants_features = QueryFeatures::from_query("helper utility tests"); - assert!( - wants_features.wants_test_paths, - "query 'helper utility tests' should want test paths" - ); - let mut gated_scores = vec![1.0, 1.0]; - let gated_results = vec![tests_first[0].clone(), tests_first[1].clone()]; - apply_multiplicative_path_penalty(&wants_features, &mut gated_scores, &gated_results); - assert_eq!( - gated_scores, - vec![1.0, 1.0], - "when query wants tests, multiplicative penalty must not apply" - ); - } -} - -#[test] -fn multiplicative_path_penalty_is_multiplicative_not_additive() { - // Directly verify the factor is multiplicative: apply the penalty to known - // scores and check ratio is ~0.3, not a constant subtract. - let features = QueryFeatures::from_query("helper utility"); - assert!( - !features.wants_test_paths, - "query should not want test paths for this probe" - ); - let results = vec![ - make_result( - "tests/fixtures/helper.rs", - Some("helper"), - Some(SymbolType::Function), - "pub fn helper() {}", - ), - make_result( - "src/helper.rs", - Some("helper"), - Some(SymbolType::Function), - "pub fn helper() {}", - ), - ]; - let mut scores = vec![10.0, 10.0]; - apply_multiplicative_path_penalty(&features, &mut scores, &results); - assert!( - (scores[0] - 3.0).abs() < 1e-6, - "tests/ score should be 10.0 * 0.3 = 3.0, got {}", - scores[0] - ); - assert!( - (scores[1] - 10.0).abs() < 1e-6, - "src/ score should stay 10.0, got {}", - scores[1] - ); - - // Compat and example also penalized with same factor - let compat_results = vec![make_result( - "src/compat/session.rs", - Some("session"), - Some(SymbolType::Function), - "pub fn session() {}", - )]; - let mut compat_scores = vec![7.0]; - apply_multiplicative_path_penalty(&features, &mut compat_scores, &compat_results); - assert!( - (compat_scores[0] - 2.1).abs() < 1e-6, - "compat path should be 7.0 * 0.3 = 2.1, got {}", - compat_scores[0] - ); - - let example_results = vec![make_result( - "examples/demo.rs", - Some("demo"), - Some(SymbolType::Function), - "pub fn demo() {}", - )]; - let mut example_scores = vec![4.0]; - apply_multiplicative_path_penalty(&features, &mut example_scores, &example_results); - assert!( - (example_scores[0] - 1.2).abs() < 1e-6, - "example path should be 4.0 * 0.3 = 1.2, got {}", - example_scores[0] - ); -} - // ── Stem-boost gating knobs (#196) — VAL-196-001..010 ── #[test] diff --git a/crates/vera-core/src/retrieval/references.rs b/crates/vera-core/src/retrieval/references.rs index 5eef1395..45013156 100644 --- a/crates/vera-core/src/retrieval/references.rs +++ b/crates/vera-core/src/retrieval/references.rs @@ -10,7 +10,6 @@ use crate::path_containment::canonical_project_root; use crate::retrieval::file_scan::{ allows_class, language_for_path, line_context_snippet, symbol_for_line, }; -use crate::storage::metadata::MetadataStore; use crate::types::{SearchFilters, SearchResult}; /// Search exact call sites of `symbol` using the persisted call graph. @@ -39,8 +38,7 @@ pub fn search_callers_through( anyhow::bail!("limit must be greater than zero"); } - let metadata_path = index_dir.join("metadata.db"); - let store = MetadataStore::open(&metadata_path)?; + let store = super::open_search_metadata(index_dir)?; let repo_root = canonical_project_root(index_dir)?; let root_dir = crate::discovery::open_root_dir(&repo_root)?; let max_file_size_bytes = super::configured_max_file_size_bytes(&store); diff --git a/crates/vera-core/src/retrieval/regex_search.rs b/crates/vera-core/src/retrieval/regex_search.rs index 18c18659..7c5d94ee 100644 --- a/crates/vera-core/src/retrieval/regex_search.rs +++ b/crates/vera-core/src/retrieval/regex_search.rs @@ -17,7 +17,6 @@ use crate::retrieval::file_scan::{ symbol_for_line, }; use crate::retrieval::ranking::{RankingStage, apply_query_ranking_with_filters}; -use crate::storage::metadata::MetadataStore; use crate::types::{Chunk, SearchFilters, SearchResult}; /// Search indexed files for a regex pattern. @@ -38,8 +37,7 @@ pub fn search_regex( .build() .map_err(|e| anyhow::anyhow!("Invalid regex pattern: {e}"))?; - let metadata_path = index_dir.join("metadata.db"); - let store = MetadataStore::open(&metadata_path)?; + let store = super::open_search_metadata(index_dir)?; let mut files = store.indexed_files()?; sort_files_by_scan_priority(&mut files, filters); diff --git a/crates/vera-core/src/retrieval/search_service.rs b/crates/vera-core/src/retrieval/search_service.rs index 52cc44f0..a75f76c5 100644 --- a/crates/vera-core/src/retrieval/search_service.rs +++ b/crates/vera-core/src/retrieval/search_service.rs @@ -385,15 +385,7 @@ impl SearchContext { let filter_flag = config.retrieval.vector_filter_during_scan_enabled(); let (results, hybrid_timings) = if let Some(reranker) = reranker { - // Reranked path: temporary direct call that respects the flag via - // the default-config fallback (env authoritative). Full flag threading - // for reranked path will be added if needed; for now the non-reranked - // path is the differential surface covered by VAL-197. - // To keep the flag authoritative through SearchContext, we route the - // non-reranked filtered path via the flagged variant and leave the - // reranked variant on the legacy default-flag path unless the config - // flag is carried through the reranked wrapper in a follow-up. - crate::retrieval::hybrid::search_hybrid_reranked_with_augmentation_and_flag( + crate::retrieval::hybrid::search_hybrid_reranked_with_stores_and_flag( index_dir, provider, reranker, @@ -406,7 +398,6 @@ impl SearchContext { stored_dim, rerank_candidates, vector_candidates, - crate::config::graph_augmentation_enabled(), Arc::clone(&stores), filter_flag, ) @@ -500,7 +491,7 @@ pub(crate) fn build_query_with_intent<'a>( /// scores placed outside the requested result window. Gated by the /// `ranking_recall_pool_expansion` signal (issue #196) so ablations can /// measure its contribution without changing other ranking logic. -#[allow(dead_code)] +#[cfg(test)] fn compute_fetch_limit(query: &str, filters: &SearchFilters, result_limit: usize) -> usize { compute_fetch_limit_with_config(query, filters, result_limit, &VeraConfig::default()) } @@ -527,21 +518,7 @@ pub(crate) fn compute_fetch_limit_with_config( fetch_limit = fetch_limit.max(result_limit.saturating_mul(12).max(result_limit + 200)); } - // Candidate-pool multiplier (issue #196 second hypothesis): bare 5× top_k. - // Distinct from the table-driven `ranking_recall_pool_expansion` (which - // gates structural 8× and NL 3× on query shape; reported full-suite delta - // 0.54% in #239). The bare multiplier is applied uniformly when enabled, - // before the conditional recall expansion so both knobs compose without - // duplicating the conditional logic. - if config.retrieval.ranking_candidate_pool_multiplier_enabled() { - let five_x = result_limit.saturating_mul(5).max(result_limit + 50); - fetch_limit = fetch_limit.max(five_x); - } - - // Recall-oriented expansion for NL queries is the #196 signal. When disabled, - // only filter-driven (and the bare 5× above) expansion applies, keeping - // the pool tight so the ablation can measure the recall contribution of - // the broader pool. + // Preserve filter-driven overfetch when recall expansion is disabled. if !config.retrieval.ranking_recall_pool_expansion_enabled() { return fetch_limit; } @@ -1151,75 +1128,6 @@ mod tests { assert_eq!(ident_collapsed, 20); } - #[test] - fn candidate_pool_multiplier_is_toggleable_and_independent() { - let filters = SearchFilters::default(); - let mut disabled = VeraConfig::default(); - disabled.retrieval.ranking_candidate_pool_multiplier = false; - disabled.retrieval.ranking_recall_pool_expansion = false; - // DEFAULT OFF: fetch limit stays at result_limit for neutral queries - assert_eq!( - compute_fetch_limit_with_config("Config", &filters, 20, &disabled), - 20, - "DEFAULT OFF must not inflate neutral filter-less query" - ); - // Bare 5× when enabled: even an identifier query (which never triggers - // the recall table) must be inflated to at least 5× top_k. - let mut bare_enabled = VeraConfig::default(); - bare_enabled.retrieval.ranking_candidate_pool_multiplier = true; - bare_enabled.retrieval.ranking_recall_pool_expansion = false; - let bare = compute_fetch_limit_with_config("Config", &filters, 20, &bare_enabled); - assert!( - bare >= 100, - "candidate-pool multiplier ON must inflate Config to at least 5×20=100, got {bare}" - ); - assert_eq!( - bare, 100, - "bare 5× should be 100 (max(5*20,20+50)) without recall duplication, got {bare}" - ); - // NL query with multiplier ON should be at least 5×, distinct from recall's 3×/8× - let nl_bare = compute_fetch_limit_with_config( - "file type detection and filtering", - &filters, - 20, - &bare_enabled, - ); - assert!( - nl_bare >= 100, - "NL with bare multiplier must be at least 5×, got {nl_bare}" - ); - // When both knobs are ON, they compose (bare 5× plus recall table). - // For a structural NL query (≥4 words, not path-weighted), recall would - // give 8× (160). With bare 5×, the result should be at least 100 but - // not duplicate the recall logic: recall still applies after bare. - let mut both = VeraConfig::default(); - both.retrieval.ranking_candidate_pool_multiplier = true; - both.retrieval.ranking_recall_pool_expansion = true; - let both_limit = compute_fetch_limit_with_config( - "file type detection and filtering", - &filters, - 20, - &both, - ); - assert!( - both_limit >= 100, - "both knobs ON should still be at least 5×, got {both_limit}" - ); - // Filter-driven max must still work with bare multiplier independent of recall - let filtered = SearchFilters { - path_glob: vec!["src/**".to_string()], - ..Default::default() - }; - let filtered_bare = compute_fetch_limit_with_config("Config", &filtered, 5, &bare_enabled); - assert!( - filtered_bare >= 50, - "path_glob + bare multiplier should be large, got {filtered_bare}" - ); - // Ensure pool multiplier factor helper reports 5 when enabled - assert_eq!(bare_enabled.retrieval.candidate_pool_multiplier_factor(), 5); - assert_eq!(disabled.retrieval.candidate_pool_multiplier_factor(), 1); - } - #[test] fn exact_identifier_queries_skip_reranking() { assert!(should_skip_reranking("Config", &SearchFilters::default())); diff --git a/crates/vera-core/src/retrieval/structural.rs b/crates/vera-core/src/retrieval/structural.rs index 3cb57d17..d51c187b 100644 --- a/crates/vera-core/src/retrieval/structural.rs +++ b/crates/vera-core/src/retrieval/structural.rs @@ -64,8 +64,7 @@ pub fn search_structural( bail!("limit must be greater than zero"); } - let metadata_path = index_dir.join("metadata.db"); - let store = MetadataStore::open(&metadata_path)?; + let store = super::open_search_metadata(index_dir)?; let repo_root = canonical_project_root(index_dir)?; let root_dir = crate::discovery::open_root_dir(&repo_root)?; let max_file_size_bytes = super::configured_max_file_size_bytes(&store); diff --git a/crates/vera-core/src/retrieval/type_relations.rs b/crates/vera-core/src/retrieval/type_relations.rs index e3683071..19fd161d 100644 --- a/crates/vera-core/src/retrieval/type_relations.rs +++ b/crates/vera-core/src/retrieval/type_relations.rs @@ -12,7 +12,6 @@ use crate::retrieval::file_scan::{ allows_class, language_for_path, line_context_snippet, smallest_symbol_chunk_for_line, symbol_for_line, }; -use crate::storage::metadata::MetadataStore; use crate::types::{SearchFilters, SearchResult, SymbolType}; pub fn search_explicit_implementations( @@ -25,8 +24,7 @@ pub fn search_explicit_implementations( anyhow::bail!("limit must be greater than zero"); } - let metadata_path = index_dir.join("metadata.db"); - let store = MetadataStore::open(&metadata_path)?; + let store = super::open_search_metadata(index_dir)?; let repo_root = canonical_project_root(index_dir)?; let root_dir = crate::discovery::open_root_dir(&repo_root)?; let max_file_size_bytes = super::configured_max_file_size_bytes(&store); diff --git a/crates/vera-core/src/retrieval/vector.rs b/crates/vera-core/src/retrieval/vector.rs index ae2c50ea..5b1aaf44 100644 --- a/crates/vera-core/src/retrieval/vector.rs +++ b/crates/vera-core/src/retrieval/vector.rs @@ -72,6 +72,10 @@ pub async fn search_vector_with_stores_timed( query: &str, limit: usize, ) -> Result<(Vec, Duration), VectorSearchError> { + crate::indexing::freshness::ensure_index_chunking_compatible( + metadata_store, + std::path::Path::new("."), + )?; if limit == 0 { return Ok((Vec::new(), Duration::ZERO)); } diff --git a/crates/vera-core/src/types.rs b/crates/vera-core/src/types.rs index eb710e4b..8741b56f 100644 --- a/crates/vera-core/src/types.rs +++ b/crates/vera-core/src/types.rs @@ -1038,7 +1038,9 @@ pub struct SearchResult { pub content: String, /// Programming language. pub language: Language, - /// Relevance score (higher is better). + /// Pipeline-specific ranking value, which may be rank-normalized. + /// Returned ordering is authoritative. Scores are not probabilities and + /// are not comparable across queries. pub score: f64, /// Symbol name (`null` if the result doesn't correspond to a named symbol). pub symbol_name: Option, diff --git a/docs/adr/000-decision-summary.md b/docs/adr/000-decision-summary.md index ac0cc2b8..c5d939e2 100644 --- a/docs/adr/000-decision-summary.md +++ b/docs/adr/000-decision-summary.md @@ -10,7 +10,7 @@ These are the main technical choices behind Vera's current architecture. Earlier | Chunking | Symbol-aware via tree-sitter AST | [004](004-chunking-strategy.md) | | Retrieval | BM25 + Vector + RRF + Query-aware ranking + optional reranking | [005](005-query-aware-retrieval.md) | | Ranking signals | Toggleable ranking signals, implemented separately from measurement | [006](006-ranking-signals.md) | -| Ranking hypotheses | Candidate-pool, path-penalty, and chunk-size hypotheses (tested, kept default-off) | [007](007-ranking-hypotheses.md) | +| Ranking hypotheses | Candidate-pool, path-penalty, and chunk-size hypotheses (rejected, removed in v2) | [007](007-ranking-hypotheses.md) | | Filter-scan default | Filtered vector scans enabled by default after profiling | [008](008-filter-during-scan-default.md) | | Filter-scan profiling | Persistent-index query-latency profile and bounded resident state | [009](009-filter-scan-profiling.md) | | Reranker batching | Client-side batch setting retained as the batching contract | [010](010-reranker-server-batching.md) | diff --git a/docs/adr/006-ranking-signals.md b/docs/adr/006-ranking-signals.md index 87a210d1..151229fc 100644 --- a/docs/adr/006-ranking-signals.md +++ b/docs/adr/006-ranking-signals.md @@ -16,7 +16,7 @@ Implement three ranking signals in `crates/vera-core` behind individually toggle - `retrieval.ranking_definition_boost` (`VERA_RANKING_DEFINITION_BOOST`) - `retrieval.ranking_recall_pool_expansion` (`VERA_RANKING_RECALL_POOL_EXPANSION`) -Config plumbing threads `VeraConfig` through `score_prior`, `score_pool`, `apply_query_ranking`, `compute_fetch_limit`, and exact-match augmentation. Wrappers preserve backward-compatible signatures using `VeraConfig::default()` (which already respects env overrides). +Config plumbing threads `VeraConfig` through ranking priors, pool scoring, fetch-limit computation, and exact-match augmentation. Existing public entry points retain their signatures and defaults. ## Mechanism Rationale Per Signal diff --git a/docs/adr/007-ranking-hypotheses.md b/docs/adr/007-ranking-hypotheses.md index 97c73698..d39ee8b7 100644 --- a/docs/adr/007-ranking-hypotheses.md +++ b/docs/adr/007-ranking-hypotheses.md @@ -1,109 +1,25 @@ -# ADR 007: Ranking Hypotheses: Multiplicative Path Penalty, Candidate-Pool Multiplier, 750-Char Chunks (DEFAULT OFF) +# ADR 007: Rejected Ranking and Chunking Experiments -Status: implemented behind toggleable knobs, DEFAULT OFF; measurement owned by `ranking-hypotheses-measurement` (separate PR) - -## Context - -After ADR 006 / #239 (three ranking signals shipped toggleable, default `true` with env overrides), three remaining #196 hypotheses remain untested. The Semble full-suite gap (Vera 0.8451 vs 0.8514 nDCG@10, 0.74% relative) concentrates in `intent` and `cross_file` with worst-case repos (nvm/zig/rails/redis/nlohmann-json/redux/axum). Failure taxonomy: recall misses (correct file never reaches ranking), ranking misses (correct file in pool but outranked by docs/tests/fixtures), noise. No new benchmark tuning may occur in the implementation PR; mechanism-first rationales only. This ADR follows the #239 / ADR-006 pattern exactly. +Status: rejected after measurement; implementations and controls removed in v2.0.0. ## Decision -Implement the three remaining #196 hypotheses as individually toggleable config knobs + env overrides, **DEFAULT OFF**, with mechanism-first rationales, strictly separated from measurement (no result JSONs in this PR): - -- `retrieval.ranking_multiplicative_path_penalty` (`VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY`, aliases `VERA_RANKING_PATH_PENALTY`): bool, default `false` (OFF), factor `0.3×` when enabled -- `retrieval.ranking_candidate_pool_multiplier` (`VERA_RANKING_CANDIDATE_POOL_MULTIPLIER`, aliases `VERA_RANKING_POOL_MULTIPLIER` / `VERA_RANKING_CANDIDATE_POOL_SIZE_MULTIPLIER`, numeric `5` also accepted as truthy): bool, default `false` (OFF), factor `5×` `top_k` when enabled -- `indexing.chunk_max_chars` (`VERA_INDEXING_CHUNK_MAX_CHARS`, aliases `VERA_MAX_CHUNK_CHARS` / `VERA_CHUNK_MAX_CHARS` / `VERA_INDEXING_MAX_CHUNK_CHARS`, serde alias `max_chunk_chars`): `usize`, default `0` (OFF, 0 means disabled), `750` when enabled - -Naming follows the existing `retrieval.ranking_*` / `indexing.max_chunk_*` / `VERA_RANKING_*` / `VERA_INDEXING_*` conventions. `VeraConfig` threads through `score_prior` / `score_pool` / `apply_query_ranking` / `compute_fetch_limit_with_config` and through `parsing::chunker` plus `indexing_config` identity gates. Wrappers preserve compatibility via `VeraConfig::default()` (which already respects env overrides). Env overrides are authoritative over file values. - -Implementation lives on branch `feat/issue196-hypotheses` (worktree) and merges only on stable + MSRV (1.88) green at the exact head. - -## Mechanism Rationale Per Hypothesis - -Parameters are carried from mechanism reasoning only; no benchmark tuning in this PR. - -### 1. Multiplicative path penalties for tests/compat/examples at ~0.3× - -**Current state:** `retrieval/ranking/score.rs` penalizes `tests/` / `compat` / `examples` additively: `Test -0.95`, `compat -0.65`, `Example|Bench -0.55`, `Docs -0.55/-0.95`, `Archive -0.85`, `Generated -0.95`, etc., plus `definition_site_role_blocked` gating for definition boosts. Additive penalties subtract a constant regardless of retrieval confidence. - -**Hypothesis:** boilerplate, fixture, and example directories (`tests/`, `testdata/`, `__tests__/`, `spec/`, `fixture(s)`, `example(s)`, `demo(s)`, `bench(es)`, `benchmark(s)`, `sample(s)`, `compat`/`legacy`/`shim`/`polyfill`) contain keyword-dense non-implementation content (copied snippets, mocked configs, usage samples) that matches many NL queries incidentally. A **multiplicative** `0.3×` penalty demotes these candidates **proportionally to retrieval confidence**, preserving ordering among non-penalized candidates (all keep their relative scores) while consistently demoting penalized ones by the same factor. Two equally-scoring candidates, `src/session.rs` vs `tests/fixtures/session.rs`, retain `src/` > `tests/` by factor 3.3, rather than by a fixed subtract that may be washed out at high scores or dominate at low scores. - -**Gating:** must respect the existing boost-directory gating used by `ranking_definition_boost` (`definition_site_role_blocked` / `wants_test_paths` / `wants_example_paths` / `wants_compat_paths`). When the query explicitly asks for tests/compat/examples (e.g. “legacy compat session”), the penalty does not fire. This mirrors the definition boost's directory checks so explicit requests are not penalized. - -**Implementation:** new `apply_multiplicative_path_penalty` in `score.rs` multiplies `scores[i] *= 0.3` for `ContentClass::Test` / `Example` / `Bench` or `definition_site_role_blocked`-detected test/example dirs, or `is_compat_path`, when the corresponding `wants_*` is false. Called from `score_pool_with_config` after `apply_coherence_boost` / `apply_keyword_path_boost` / `apply_content_symbol_boost`, gated by `ranking_multiplicative_path_penalty_enabled()`. Additive penalties remain unchanged; when the knob is on, both additive and multiplicative apply (distinct mechanisms, additive -0.95 plus ×0.3). When OFF (default), behavior is unchanged. - -No weight was tuned: `0.3` is carried from the mechanism hypothesis (strong enough to demote below `src/` for equally-scoring candidates, `1/0.3 ≈ 3.3`, without collapsing test fixtures to zero) and is not derived from ground-truth or score chasing. - -### 2. Larger candidate-pool multiplier: the 5× `top_k` hypothesis - -**Current state:** `compute_fetch_limit_with_config` computes `fetch_limit` from filters (`result_limit`, `3×`/`10×`/`12×` for `path_glob`/`exact_paths`) and, when `ranking_recall_pool_expansion` is enabled, a **table-driven** recall expansion: `needs_structural_overfetch` (NL ≥4 words, not path-weighted, no filters) → `8×`, else broad NL → `3×`. Full-suite delta for that signal was **0.54%** (reported in #239). Vector candidate pooling (`candidate_pool`) further bounds the fetch via the KNN cap (4427 flat / KNN selector for extreme limits). - -**Hypothesis:** a **bare** `5×` `top_k` pool multiplier gives ranking signals more correct-but-low-ranked files to promote, without the conditional table. Intent and cross-file queries are open-ended; BM25/vector may place the correct file at rank 40 while the fetch limit only keeps 5–20 candidates. Expanding the pool uniformly (not gated on `needs_structural_overfetch`) trades a modest, bounded increase in candidates (5×) for higher recall that ranking can then refine. This is the issue's `5× top_k` hypothesis, now wired as a pool-size knob. - -**Non-duplication:** the new knob does **not** duplicate `ranking_recall_pool_expansion`. The existing signal is **table-driven and conditional** (structural 8× gated on ≥4 words and exact query shape; full-suite delta 0.54% documented in #239). The new knob is a **bare 5× `top_k`** applied **uniformly before** the conditional table (`fetch_limit.max(result_limit*5 + 50)`) so both knobs compose without duplicating logic: when both are on, the fetch limit is the max of `5×`, `8×`, and filter-driven expansions; when only the new knob is on, NL structural queries still expand to 5× even if the recall table is off. The ADR explicitly cross-references the 0.54% delta to keep the distinction measurable in the separate measurement PR. - -**Implementation:** in `retrieval/search_service.rs` `compute_fetch_limit_with_config`, after filter-driven `fetch_limit` is computed, if `ranking_candidate_pool_multiplier_enabled()` then `fetch_limit = fetch_limit.max(result_limit*5)` (with `+50` minimum headroom, `result_limit*5`). This runs **before** the `ranking_recall_pool_expansion` early return, so the bare multiplier survives even when recall expansion is disabled. `candidate_pool_multiplier_factor()` returns `5` when enabled, `1` otherwise. Gated DEFAULT OFF. - -Parameter `5` is carried from the mechanism hypothesis (5× top_k as named in the issue) and not re-tuned to benchmark scores. - -### 3. ~750-character chunks: finer embedding locality (touches index format / identity) - -**Prior negative (#67):** two larger-window / larger-cap experiments were run and kept as negative results with cost columns: - -- **window-2048 on jina:** `-0.23%` nDCG for `+88%` index time (stored, hypothesis rejected) -- **2048-byte cap on potion:** `-1.24%` subset / `-0.21%` full nDCG, `+24%` index time, `+15%` storage (kept status quo `512/24576`) - -The status quo remains `max_chunk_lines 200` / `max_chunk_bytes 24576` (24 KB, ~6–7K tokens; local embedders see first 512 tokens). - -**Hypothesis (opposite direction):** **smaller** `~750` character chunks give **finer embedding locality** than the 24 KB byte cap. Long chunks blur multiple concepts (imports + several functions + trailing prose) into one embedding; a retrieval query matching one concept must compete with noise from the rest of the chunk. Splitting at `~750` chars (line-boundary aware) isolates each concept into its own retrievable unit while preserving symbol coherence (the same `bare_name` + `part_index` shape as the byte path). This is the opposite direction of #67's 2048, so it is expected to have different trade-offs; cost columns (index time, storage) must be reported alongside quality in the measurement PR. - -**Index format / identity:** this touches the index format. `chunk_max_chars` is wired through the **content-affecting** `indexing_config` identity keys: the `max_chunk_lines` / `max_chunk_bytes` pattern already exists in `eval/src/lanes.rs` provenance gates and `eval/src/vera_adapter.rs` `indexing_config_matches`. `freshness.rs` stores the full `IndexingConfig` JSON (including `chunk_max_chars`) via `record_index_snapshot`; `vera_adapter.rs` checks `chunk_max_chars` (with `max_chunk_chars` / `max_chunk_characters` aliases) and treats missing as `0` (DEFAULT OFF) for backward compatibility so existing indexes reuse when the knob is off. Throughput-only keys (`batch_size`, `max_concurrent_requests`, `timeout_secs`, etc.) are intentionally **not** part of the identity. - -**Gating for byte-identical default:** the chunker change is **gated** behind the knob so default behavior is **byte-identical when off**. `parsing/chunker.rs` adds `split_oversized_chunks_by_chars(chunks, max_chars)` (mirrors `split_oversized_chunks` but uses `char` counts, `0` is a no-op). `parsing/mod.rs` introduces `apply_splits()` which does `split_oversized_chunks` (always) then `split_oversized_chunks_by_chars` only when `config.chunk_max_chars_effective() != 0`. When `chunk_max_chars == 0` (DEFAULT OFF), the char path is not taken, producing identical chunk boundaries to master. - -Parameter `750` is carried from the mechanism hypothesis (finer locality than 24 KB, smaller than #67's 2048) and not tuned to benchmark scores. - -## What Was NOT Done (Implementation / Measurement Separation) - -Per VAL-ISSUE-025 / VAL-ISSUE-026: - -- No `benchmarks/results/*.json` was committed in this PR; no measured quality claim (subset, full-suite, or independent) appears here. -- No with/without ablation was executed to tune knobs; every new knob defaults **OFF** so the signals-off reference (post-m8 rebaseline, `ranking-signals-measurement` will evaluate hypotheses against that baseline on the 320-task subset and 180-task independent set at named commits with `vera_git_sha` provenance). -- No heuristic constants (0.3, 5, 750) were derived from ground-truth inspection or score-chasing; they are carried from the issue's mechanism reasoning. -- No tuning of existing signals (`KEYWORD_PATH_WEIGHT`, `COVERAGE_WEIGHT`, `FILE_SATURATION`, etc.) occurred. -- The PR is implementation-only; measurement artifacts (result JSONs with `index_time_secs` / `storage_size_bytes` cost columns, and the chunk-arm's explicit cross-reference to #67's `-0.23%` / `+88%` and `-1.24%/ -0.21%` / `+24%` / `+15%` numbers) belong to the separate measurement PR which also satisfies VAL-ISSUE-027 through VAL-ISSUE-030. - -## Toggleability and Ablation Cost - -All three knobs are individually toggleable via config file and env var (env authoritative over file): - -- `retrieval.ranking_multiplicative_path_penalty`: `VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY` (bool); covers `tests/` / `compat` / `examples`; 0.3× multiplicative; respects `wants_*` gating -- `retrieval.ranking_candidate_pool_multiplier`: `VERA_RANKING_CANDIDATE_POOL_MULTIPLIER` (bool, numeric `5` also truthy; aliases `VERA_RANKING_POOL_MULTIPLIER`, `VERA_RANKING_CANDIDATE_POOL_SIZE_MULTIPLIER`); bare 5× `top_k`; interacts with `compute_fetch_limit_with_config`; distinct from the 0.54% recall-pool signal -- `indexing.chunk_max_chars`: `VERA_INDEXING_CHUNK_MAX_CHARS` (aliases `VERA_MAX_CHUNK_CHARS` / `VERA_CHUNK_MAX_CHARS` / `VERA_INDEXING_MAX_CHUNK_CHARS`; serde aliases `max_chunk_chars`); char-budget split; content-affecting identity; DEFAULT OFF gives byte-identical chunking +Remove the multiplicative path penalty, uniform candidate-pool multiplier, and character-cap chunking experiment. Their full-suite results did not meet the 0.5% aggregate quality bar. Retaining dormant runtime branches, configuration aliases, and environment overrides adds maintenance cost without a supported use case. -Tests prove the contracts: +The accepted filename-stem boost, definition-content boost, and query-dependent recall-pool expansion remain unchanged. Their mechanisms are recorded in [ADR 006](006-ranking-signals.md). The rejected uniform multiplier was a separate signal from that recall expansion. -- `config::tests::issue196_hypotheses_default_off`: fresh `VeraConfig::default()` keeps all three OFF (bool false, `chunk_max_chars == 0`); legacy JSON without the keys deserializes to OFF -- `config::tests::issue196_hypotheses_env_flips_independently`: each `VERA_*` env flips only its knob (`penalty` `1` → `0.3×` enabled but pool/chunk stay OFF; `pool` `1`/`5` → `5×` enabled but others stay OFF; `chunk` `750` → char budget enabled but retrieval stays OFF); alias envs also flip -- `parsing::chunker::tests::chunk_max_chars_off_is_byte_identical`: `split_oversized_chunks_by_chars(..., 0)` is identity; default `IndexingConfig` produces identical chunks to the 24 KB path -- `parsing::chunker::tests::chunk_max_chars_on_splits_finer_than_byte_cap`: `750` splits into ≥2 sub-chunks each ≤750 chars, with `bare_name` + `part_index` shape mirroring the byte path, and the suffix is verbatim (`display_symbol_name` is single source) -- `retrieval::ranking::tests::multiplicative_path_penalty_is_toggleable`: disabled keeps base rank (penalty OFF), enabled demotes `tests/fixtures/` below `src/` even when `tests/` starts first, and respects `wants_test_paths` gating (`"tests"` in query → no demotion) -- `retrieval::ranking::tests::multiplicative_path_penalty_is_multiplicative_not_additive`: direct `apply_multiplicative_path_penalty` multiplies `10.0 → 3.0`, `7.0 → 2.1` (compat), `4.0 → 1.2` (example), not a subtract -- `retrieval::search_service::tests::candidate_pool_multiplier_is_toggleable_and_independent`: DEFAULT OFF leaves `compute_fetch_limit("Config", 20)` at `20`; bare `5×` inflates even identifier queries to `≥100`; NL similarly `≥100`; both knobs compose; filter-driven `path_glob` still maximal +## Mechanisms Tested -## Ground-Truth Non-Inspection Statement +- A 0.3× path penalty tried to demote keyword-dense test, compatibility, and example content proportionally to retrieval confidence. Explicit requests for those categories bypassed the penalty. It added no full-suite value beyond the existing additive priors. +- A uniform 5× candidate-pool multiplier tried to expose more low-ranked candidates to ranking on every query. Its gain was below the acceptance bar; the supported pool expansion remains query-dependent. +- A 750-character cap tried to isolate concepts inside long symbols while preserving line boundaries and split-symbol identities. It fragmented context and increased indexing, storage, and query costs while regressing the full suite. -No rule, constant, or weight in this PR was derived from inspecting benchmark ground-truth answers or expected file paths. Hypotheses were formed from the failure taxonomy (recall vs ranking vs noise) and from general code-search intuition (module labels, definition anchors, open-ended query recall, chunk locality vs #67). No hardcoded repo or file-path lists matching benchmark expectations were introduced. +Parameters came from mechanism hypotheses, not benchmark answer inspection or heuristic retuning. The experiments were evaluated with and without each signal on the subset, independent set, and full suite. [Benchmark history](../benchmarks-history.md#rejected-ranking-and-chunking-hypotheses-2026-09-01) preserves the measurements, source commit, model, and decision. -## Consequences +Structural graph augmentation was also removed. Its bounded caller and implementation expansion gained too little quality for its reranking latency cost; see the [historical full-pipeline ablations](../benchmarks-history.md#ablations). Reference lookup and its receiver-disambiguation evaluation remain supported. -- New unit tests pin toggle + byte-identical + multiplicative behavior; existing ranking/chunking tests continue to pass with defaults. -- No benchmark-derived tuning debt is introduced; measurement can proceed independently and per-hypothesis dual-set (subset + independent) ablations with cost columns can be attributed cleanly before any default flip (gates per VAL-ISSUE-027 through VAL-ISSUE-030, and the 0.5% bar). -- Benchmark-integrity position: **implementation PR contains no result JSONs and no measured quality claims; parameters are from mechanism reasoning only** (VAL-ISSUE-026). Measurement PR will name its evaluated `vera_git_sha` and report `index_time_secs` + `storage_size_bytes` for the chunk arm with explicit #67 prior cross-reference. +## Compatibility -## References +Legacy configuration files still load through unknown-field tolerance. Removed settings disappear from configuration output and setters, and their environment variables have no effect. -- #196 gap and failure taxonomy; #239 / ADR 006 (toggleable-signal pattern, `VeraConfig` threading, `compute_fetch_limit` gating, `Lanes` provenance `VERA_RANKING_*` + `host.cpu_model`) -- #67 prior negatives: `window-2048` / `2048-byte cap` with index-time / storage cost columns -- #243 hardware caveat (7600X3D → 9800X3D warm 9.93 → 16.6 ms p50; rebaseline target `072c725` re-measured same-host) -- Implementation files: `crates/vera-core/src/config.rs` (knobs + `env_bool`/`env_usize` + `enabled()` helpers), `crates/vera-core/src/retrieval/ranking/score.rs` + `mod.rs` (0.3×), `crates/vera-core/src/retrieval/search_service.rs` (5×), `crates/vera-core/src/parsing/chunker.rs` + `mod.rs` (750-char), `eval/src/vera_adapter.rs` + `eval/src/lanes.rs` (identity + provenance) +Ordinary indexes keep their format and remain reusable. Any nonzero historical character-cap metadata, including either old alias or conflicting aliases, requires a full rebuild before search or incremental update. This check reads raw metadata so deserialization cannot discard the incompatibility. See [v2 migration](../migration-v2.md). diff --git a/docs/benchmarks-history.md b/docs/benchmarks-history.md index dc77ee19..2302f2c9 100644 --- a/docs/benchmarks-history.md +++ b/docs/benchmarks-history.md @@ -2,6 +2,20 @@ Historical benchmark snapshots, ablations, and comparisons moved from the current results page. See [Benchmarks](benchmarks.md#current-results) for the maintained comparison. +## Rejected Ranking and Chunking Hypotheses (2026-09-01) + +Measured at `6e01956fb644b9cce2d96d7e799b0b0798855c46` on the full 1,251-task Semble v0.5.5 suite (`921849164e2632dd4f0e1c1370f82cfe15ed6d6c`), local Potion Code v2 revision `e9d2a44ca6a05ac6685f3b23709ea57eb7352d5b`, no reranker, AMD Ryzen 7 9800X3D. Each arm rebuilt its indexes. The 320-task subset and 180-task independent set were also measured. [The measurement report](https://github.com/VeraTools/vera/issues/196#issuecomment-5486933078) contains those pairs and provenance. + +| Hypothesis | Full nDCG without | Full nDCG with | Relative delta | Index time without → with | Storage without → with | Decision | +|---|---:|---:|---:|---:|---:|---| +| Multiplicative 0.3× path penalty | 0.843879 | 0.841765 | -0.251% | 110.37 s → 111.95 s | 4.69 GB → 4.69 GB | Rejected: regresses | +| Uniform 5× candidate pool | 0.843480 | 0.846349 | +0.340% | 106.74 s → 106.64 s | 4.69 GB → 4.69 GB | Rejected: below 0.5% bar | +| 750-character chunks | 0.843937 | 0.841982 | -0.232% | 105.74 s → 154.28 s | 4.69 GB → 6.55 GB | Rejected: regresses with higher cost | + +The character-cap arm also increased full-suite p50 from 11.37 ms to 18.96 ms and p95 from 128 ms to 287 ms. Earlier chunk-size experiments had also failed: a 2048-token Jina window regressed nDCG by 0.23% for 88% more indexing time; a 2048-byte Potion cap regressed full nDCG by 0.21% for 24% more indexing time and 15% more storage. + +The three controls stayed off after measurement and were removed in v2.0.0. [ADR 007](adr/007-ranking-hypotheses.md) records the mechanisms and compatibility decision. These historical measurements do not claim a new quality improvement from removing dormant code. + ## Historical v1.0 Full-Pipeline Benchmark The v1.0.0-rc run used the full Semble v0.5.5 task set on the `vera-cuda` lane: hybrid BM25+vector retrieval, RRF fusion, and a local ONNX cross-encoder reranker on CUDA, measured 2026-08-16 against the v1.0.0 release candidate. The older "BM25 scoped filters" full-suite row (`nDCG@10` 0.7267, measured in May 2026) was a BM25-only lane using an older harness. Cross-date comparisons are approximate. @@ -124,7 +138,7 @@ All ablations were measured on the full 1,251-task suite against the stated base |----------|-----------------|------------|-----------|----------------|----------| | C1: rerank no-surplus skip | Skip reranking when the fused pool has no surplus over the requested limit | -0.0002 | +0.0000 | -2.4 ms mean | Shipped | | C2: rerank path-glob searches | Always rerank path-scoped searches (removes the old skip heuristic) | +0.0113 | +0.0132 | scoped queries now pay rerank cost | Shipped | -| GA: structural graph augmentation | Append bounded caller/implementation chunks into the rerank pool (`VERA_GRAPH_AUGMENT=1`) | +0.0047 | +0.0080 | +2018 ms mean (reranks the expanded pool) | Merged off by default: gain too small for the latency, R@5 did not move | +| GA: structural graph augmentation | Append bounded caller/implementation chunks into the rerank pool (`VERA_GRAPH_AUGMENT=1`) | +0.0047 | +0.0080 | +2018 ms mean (reranks the expanded pool) | Rejected: gain too small for the latency, R@5 did not move; implementation removed in v2.0.0 | Artifacts: diff --git a/docs/configuration.md b/docs/configuration.md index 7d66ca6c..3e8dcb18 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -72,8 +72,6 @@ | `retrieval.ranking_filename_stem_skip_symbol_queries` | `false` | Skips filename-stem boosting for symbol queries. | | `retrieval.ranking_definition_boost` | `true` | Boosts matching definitions. | | `retrieval.ranking_recall_pool_expansion` | `true` | Expands the recall pool before final ranking. | -| `retrieval.ranking_multiplicative_path_penalty` (`ranking_path_penalty`, `ranking_multiplicative_penalty`) | `false` | Enables the multiplicative path penalty. | -| `retrieval.ranking_candidate_pool_multiplier` (`ranking_pool_multiplier`, `ranking_candidate_pool_size_multiplier`) | `false` | Enables the 5x candidate-pool multiplier. | | `retrieval.vector_filter_during_scan` | `true` | Applies eligible path and language filters during vector scanning. | ### Environment variables @@ -91,13 +89,6 @@ | `VERA_RANKING_FILENAME_STEM_SKIP_SYMBOL_QUERIES` | `false` | Skips that boost for symbol queries. | | `VERA_RANKING_DEFINITION_BOOST` | `true` | Enables definition-content boosting. | | `VERA_RANKING_RECALL_POOL_EXPANSION` | `true` | Enables recall-pool expansion. | -| `VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY` | `false` | Enables multiplicative path penalties. | -| `VERA_RANKING_PATH_PENALTY` | `false` | Alias for the path-penalty switch. | -| `VERA_RANKING_MULTIPLICATIVE_PENALTY` | `false` | Alias for the path-penalty switch. | -| `VERA_RANKING_CANDIDATE_POOL_MULTIPLIER` | `false` | Enables the candidate-pool multiplier. | -| `VERA_RANKING_CANDIDATE_POOL_SIZE_MULTIPLIER` | `false` | Alias for the candidate-pool multiplier. | -| `VERA_RANKING_POOL_MULTIPLIER` | `false` | Alias for the candidate-pool multiplier. | -| `VERA_RANKING_CANDIDATE_POOL_MULT` | `false` | Alias for the candidate-pool multiplier. | | `VERA_VECTOR_FILTER_DURING_SCAN` | `true` | Enables filtering during flat vector scans. | | `VERA_VECTOR_SCAN` | flat | Selects the flat SIMD scan or `vec0` fallback. | @@ -114,7 +105,6 @@ | `indexing.no_ignore` | `false` | Disables `.gitignore` and `.veraignore` parsing. | | `indexing.no_default_excludes` | `false` | Disables smart default exclusions. | | `indexing.max_chunk_bytes` | 24576 | Splits oversized embedding chunks at line boundaries; `0` disables this cap. | -| `indexing.chunk_max_chars` (`max_chunk_chars`, `max_chunk_characters`) | 0 | Splits chunks by character count; `0` disables this cap. | ### Environment variables @@ -122,11 +112,6 @@ |---|---:|---| | `VERA_MAX_CHUNK_BYTES` | 24576 | Overrides the byte chunk cap. | | `VERA_MAX_IN_FLIGHT_INPUTS` | 0 | Caps the number of embedding inputs held in flight. | -| `VERA_INDEXING_CHUNK_MAX_CHARS` | 0 | Overrides the character chunk cap. | -| `VERA_INDEXING_MAX_CHUNK_CHARS` | 0 | Alias for the character chunk cap. | -| `VERA_MAX_CHUNK_CHARS` | 0 | Alias for the character chunk cap. | -| `VERA_CHUNK_MAX_CHARS` | 0 | Alias for the character chunk cap. | -| `VERA_GRAPH_AUGMENT` | unset | Enables graph-based indexing augmentation where supported. | | `VERA_OVERCAP_FIXTURE` | unset | Selects the filter-scan over-cap test fixture. | ## Runtime/misc diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 42af8e4a..52ff03f1 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -58,6 +58,8 @@ For broad intent queries, Vera also keeps a deeper fused candidate pool before f This is also where Vera adds a small amount of query-aware candidate expansion, such as pulling in related implementation blocks or same-file structural context when the initial hit is too narrow. +The `score` returned in JSON is a pipeline-specific ranking value and may be rank-normalized. Use the returned ordering. Scores are not probabilities and cannot be compared across queries. + ## Reranking: Cross-Encoder Reranking is opt-in through `retrieval.reranking_enabled` and is off by default. When enabled, the top fused candidates are sent to a cross-encoder reranker. Unlike embeddings (which encode query and document separately), the cross-encoder reads the query and each candidate together as a single pair, scoring relevance jointly. diff --git a/docs/migration-v2.md b/docs/migration-v2.md new file mode 100644 index 00000000..1c2e8860 --- /dev/null +++ b/docs/migration-v2.md @@ -0,0 +1,28 @@ +# Migrating to Vera v2 + +Upgrade the binary or wrapper as usual. Ordinary default indexes remain compatible, and the search-result JSON fields are unchanged. + +## Retired Experiments + +V2 removes four rejected experiments: + +| Experiment | Removed configuration keys | Removed environment variables | +|---|---|---| +| Multiplicative path penalty | `retrieval.ranking_multiplicative_path_penalty`, `retrieval.ranking_path_penalty`, `retrieval.ranking_multiplicative_penalty` | `VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY`, `VERA_RANKING_PATH_PENALTY`, `VERA_RANKING_MULTIPLICATIVE_PENALTY` | +| Uniform candidate-pool multiplier | `retrieval.ranking_candidate_pool_multiplier`, `retrieval.ranking_candidate_pool_size_multiplier`, `retrieval.ranking_pool_multiplier` | `VERA_RANKING_CANDIDATE_POOL_MULTIPLIER`, `VERA_RANKING_CANDIDATE_POOL_SIZE_MULTIPLIER`, `VERA_RANKING_POOL_MULTIPLIER`, `VERA_RANKING_CANDIDATE_POOL_MULT` | +| Character-cap chunking | `indexing.chunk_max_chars`, `indexing.max_chunk_chars`, `indexing.max_chunk_characters` | `VERA_INDEXING_CHUNK_MAX_CHARS`, `VERA_INDEXING_MAX_CHUNK_CHARS`, `VERA_MAX_CHUNK_CHARS`, `VERA_CHUNK_MAX_CHARS` | +| Structural graph augmentation | None | `VERA_GRAPH_AUGMENT` | + +Existing configuration files still load. Removed keys are ignored and disappear when configuration is saved; `vera config get` and `vera config set` report them as unknown keys. Removed environment variables no longer affect behavior. The supported query-dependent recall-pool expansion and explicit `references` queries remain available. See [ADR 007](adr/007-ranking-hypotheses.md) for the decision and evidence. + +If an index was built with a nonzero character cap, search and incremental update require a full rebuild: + +```bash +vera index /path/to/repository +``` + +Indexes with missing or zero character-cap metadata continue to work. Running `vera update` cannot convert an incompatible index because unchanged files would retain the old chunk boundaries. + +## Search Scores + +Use the returned result ordering. The JSON `score` remains a pipeline-specific ranking value that may be rank-normalized; it is neither a probability nor comparable across queries. [How it works](how-it-works.md) explains the pipeline. diff --git a/docs/whats-new.md b/docs/whats-new.md index 61e941f3..bc9201cb 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,10 @@ Release highlights from v1.0 onward. For the current benchmark tables and methodology, see [benchmarks.md](benchmarks.md). For the full command surface, see [features.md](features.md). +## v2.0.0 + +Rejected ranking, character-cap chunking, and structural graph-augmentation experiments have been removed. Default indexes remain compatible; experimental character-capped indexes require a full rebuild. See [v2 migration](migration-v2.md) for the removed controls and score contract. + ## v1.4.1 ### Agent ergonomics and correctness @@ -54,7 +58,7 @@ A four-arm sweep ran GLM-5.3 (high effort) against 10 cross-file Flask questions ### Ranking and retrieval - Three ranking signals for issue #196 are now toggleable with mechanism-first rationales: filename-stem boost, definition boost, and recall-pool expansion. Each has a config knob and `VERA_RANKING_*` env override, implemented separately from measurement and proven by dual-set ablations on the 320-task subset and 180-task independent set with full-suite confirmation before any quality claim. -- Three additional hypotheses (multiplicative path penalties, candidate-pool multiplier, 750-char chunks) are implemented as default-off knobs with correct index-identity wiring. Dual-set ablations on the 320-task subset and 180-task independent set plus full 1,251-task confirmation showed each below the 0.5% full-suite aggregate bar or with regression, so all three stay default off with negative results recorded. The chunk arm cites the prior 2048 window and cap negatives and reports its own index-time and storage cost. +- Three additional hypotheses (multiplicative path penalties, candidate-pool multiplier, 750-char chunks) are implemented as default-off knobs with correct index-identity wiring. Dual-set ablations on the 320-task subset and 180-task independent set plus full 1,251-task confirmation showed each below the 0.5% full-suite aggregate bar or with regression, so all three stayed default off. Their implementations and controls were removed in v2.0.0; the negative results remain recorded. The chunk arm cites the prior 2048 window and cap negatives and reports its own index-time and storage cost. - Reranker protocol now cleanly separates generic (`top_n` / `results`) from Voyage (`top_k` / `data`) with explicit config override over hostname auto-detection, and resilience covers permanent 4xx no-retry, capped `Retry-After` and `X-RateLimit-Reset` waits, cancellation, and graceful degradation. ### Setup and first-run @@ -191,7 +195,7 @@ The gains came from a reworked default retrieval pipeline: BM25 stemming and ide |--------|------------------| | C1: rerank no-surplus skip | Shipped as a latency guard. Vera skips reranking when the fused pool has no surplus over the requested result limit, with a `-0.0002` nDCG delta and `-2.4 ms` mean latency effect. | | C2: rerank path-glob searches | Shipped. Path-scoped searches remain eligible for reranking, improving full-suite nDCG by `+0.0113`. | -| Structural graph augmentation | Merged as an experimental opt-in under `VERA_GRAPH_AUGMENT=1`. It adds bounded caller and implementation chunks to the rerank pool, gaining `+0.0047` nDCG at roughly `+83%` mean latency. It is off by default because the latency cost outweighs the gain and Recall@5 did not move. | +| Structural graph augmentation | Merged as an experimental opt-in under `VERA_GRAPH_AUGMENT=1`. It adds bounded caller and implementation chunks to the rerank pool, gaining `+0.0047` nDCG at roughly `+83%` mean latency. The latency cost outweighed the gain and Recall@5 did not move; the opt-in implementation was removed in v2.0.0. | ### Agent-level benchmark diff --git a/eval/src/lanes.rs b/eval/src/lanes.rs index 8c9f26c6..891a43cd 100644 --- a/eval/src/lanes.rs +++ b/eval/src/lanes.rs @@ -41,13 +41,10 @@ const PROVENANCE_ENV_KEYS: &[&str] = &[ "RERANKER_MODEL_BASE_URL", "RERANKER_MODEL_ID", "VERA_BACKEND", - "VERA_INDEXING_CHUNK_MAX_CHARS", "VERA_LOCAL", "VERA_MAX_RERANK_BATCH", - "VERA_RANKING_CANDIDATE_POOL_MULTIPLIER", "VERA_RANKING_DEFINITION_BOOST", "VERA_RANKING_FILENAME_STEM_BOOST", - "VERA_RANKING_MULTIPLICATIVE_PATH_PENALTY", "VERA_RANKING_RECALL_POOL_EXPANSION", ]; diff --git a/eval/src/vera_adapter.rs b/eval/src/vera_adapter.rs index 8a26298e..8c208e4f 100644 --- a/eval/src/vera_adapter.rs +++ b/eval/src/vera_adapter.rs @@ -257,6 +257,11 @@ fn embedding_dim_matches(store: &MetadataStore, provider: } fn indexing_config_matches(store: &MetadataStore, config: &VeraConfig) -> bool { + if vera_core::indexing::freshness::ensure_index_chunking_compatible(store, Path::new(".")) + .is_err() + { + return false; + } // Content-affecting indexing keys: max_chunk_lines, max_chunk_bytes, // max_file_size_bytes, and embedding max_length. Any mismatch forces a // full re-index because chunk boundaries or file inclusion change. @@ -283,19 +288,6 @@ fn indexing_config_matches(store: &MetadataStore, config: &VeraConfig) -> bool { { return false; } - // Chunk char budget is content-affecting (~750-char hypothesis). Stored - // under "chunk_max_chars" (new) with alias "max_chunk_chars" for compat. - // Old indexes lack the key; treat missing as 0 (DEFAULT OFF) so - // byte-identical default chunking still reuses old indexes. - let stored_chunk_chars = value - .get("chunk_max_chars") - .or_else(|| value.get("max_chunk_chars")) - .or_else(|| value.get("max_chunk_characters")) - .and_then(|v| v.as_u64()) - .unwrap_or(0); - if stored_chunk_chars != config.indexing.chunk_max_chars_effective() as u64 { - return false; - } if value.get("max_file_size_bytes").and_then(|v| v.as_u64()) != Some(config.indexing.max_file_size_bytes) { @@ -547,6 +539,31 @@ mod tests { MetadataStore::open(&path).unwrap() } + #[test] + fn reuse_rejects_every_retired_character_cap_alias_but_accepts_zero() { + let store = MetadataStore::open_in_memory().unwrap(); + let config = VeraConfig::default(); + let original = serde_json::to_value(&config.indexing).unwrap(); + store + .set_index_meta(INDEXING_CONFIG_KEY, &original.to_string()) + .unwrap(); + assert!(indexing_config_matches(&store, &config)); + for key in ["chunk_max_chars", "max_chunk_chars", "max_chunk_characters"] { + let mut stored = original.clone(); + stored["chunk_max_chars"] = serde_json::json!(0); + stored[key] = serde_json::json!(750); + store + .set_index_meta(INDEXING_CONFIG_KEY, &stored.to_string()) + .unwrap(); + assert!(!indexing_config_matches(&store, &config)); + stored[key] = serde_json::json!(0); + store + .set_index_meta(INDEXING_CONFIG_KEY, &stored.to_string()) + .unwrap(); + assert!(indexing_config_matches(&store, &config)); + } + } + #[test] fn vera_bm25_indexes_and_searches_small_repo() { let dir = tempfile::tempdir().unwrap();