From 5854ee169230b5386de94e9ad49c1a22a5ea9a9f Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 13 Jul 2026 12:32:26 -0400 Subject: [PATCH 1/7] fix(kg): resolve canonical entity names that hybrid search ranks first kg.resolve returned NotFound for canonical names its own hybrid fallback ranked top-1. Fix at the resolution stage without introducing silent-picking on genuine ambiguity. Fixes #908 Co-Authored-By: Claude Fable 5 --- .../khive-runtime/src/reference_resolution.rs | 185 +++++++++++++++++- 1 file changed, 183 insertions(+), 2 deletions(-) diff --git a/crates/khive-runtime/src/reference_resolution.rs b/crates/khive-runtime/src/reference_resolution.rs index 081d6cb13..aede75fc7 100644 --- a/crates/khive-runtime/src/reference_resolution.rs +++ b/crates/khive-runtime/src/reference_resolution.rs @@ -88,6 +88,29 @@ const SEARCH_SCORE_FLOOR: f64 = 0.0; /// "search picked this out" by confidence alone; the raw RRF value is still /// preserved in `ReferenceCandidate.score` for `Ambiguous` listings. const SEARCH_RESOLVED_CONFIDENCE: f64 = 0.6; +/// Floor on the retrieval depth stage 4 asks `hybrid_search` for, independent +/// of the caller's requested `limit`. +/// +/// `KhiveRuntime::hybrid_search` truncates its returned hit list to exactly +/// the `limit` it is called with (its final step is `fused.truncate(limit as +/// usize)`), so that `limit` doubles as both "how deep to search" and "how +/// many hits to hand back" — for `resolve`, whose caller-facing default +/// `limit` is 5 (`khive_pack_kg::handlers::resolve::DEFAULT_LIMIT`), a +/// caller asking for a short candidate list was silently also asking for a +/// shallow search. A genuine canonical-name match ranked, say, 6th-to-10th — +/// exactly where a qualified natural-language ref (a project prefix ahead of +/// a short canonical name) lands when it beats the exact-name stage above +/// but only partially matches the text leg and is corroborated by the vector +/// leg — was truncated out of the stage-4 candidate set entirely even though +/// the same query against a wider `limit` (e.g. the `search` verb's own +/// default of 10) surfaces it as the top hit (#908). Retrieving at this +/// floor decouples "how deep to search" from "how many candidates the caller +/// asked for" — the caller's own `limit` still governs the id-string/ring +/// stages above and is clamped to a max of 20 +/// (`khive_pack_kg::handlers::resolve::MAX_LIMIT`) by the handler, so this +/// floor never returns more candidates than a caller could have asked for +/// outright. +const STAGE4_MIN_SEARCH_LIMIT: u32 = 20; /// Resolve one natural-language reference for `token`'s actor. /// @@ -220,13 +243,18 @@ pub async fn resolve_reference( return Ok(resolution); } - // Stage 4: hybrid-search fallback over the namespace. + // Stage 4: hybrid-search fallback over the namespace. Search deeper than + // the caller's requested `limit` (see `STAGE4_MIN_SEARCH_LIMIT`) so a + // genuine match ranked just outside a small `limit` isn't truncated out + // of the pool before it's even ranked against the alternatives; the + // caller's `limit` still bounds how many candidates get rendered below. + let search_limit = limit.max(STAGE4_MIN_SEARCH_LIMIT); let hits = runtime .hybrid_search( token, trimmed, None, - limit.max(1), + search_limit, entity_kind, None, &[], @@ -613,4 +641,157 @@ mod tests { other => panic!("expected Ambiguous driven by storage total (11), got {other:?}"), } } + + // Regression for #908: a canonical-name entity ranked outside the + // caller's small default `limit` (resolve's default is 5) used to be + // dropped entirely, because the old code called `hybrid_search` with + // that same small `limit` and `hybrid_search`'s own final step is + // `fused.truncate(limit as usize)` — so a genuine match ranked, say, + // 8th, never survived to reach `resolve_reference`'s own candidate list + // at all. Twelve decoy entities that also satisfy the (trigram, plain + // AND-mode) FTS query out-rank the target via heavier term repetition, + // pushing the target below position 5 but keeping it within + // `STAGE4_MIN_SEARCH_LIMIT` (20). The target must still surface in the + // stage-4 result — as the `Ambiguous` listing's tail if not decisively + // first — rather than vanish outright. + #[tokio::test] + async fn fallback_stage_recovers_canonical_name_ranked_below_default_limit() { + let rt = KhiveRuntime::memory().expect("in-memory runtime"); + let token = actor_token("resolver-test"); + let ring = ReferenceRing::new(); + + let target = rt + .create_entity( + &token, + "concept", + None, + "ADR-040", + Some("khive ADR-040"), + None, + vec![], + ) + .await + .expect("create target entity"); + + // Decoys out-rank the target by repeating the shared query terms + // many times (FTS5 bm25 rewards term frequency); none share the + // target's canonical name, so stage 3 (exact-name) never fires and + // stage 4 is genuinely reached. + for i in 0..12 { + rt.create_entity( + &token, + "concept", + None, + &format!("Filler{i}"), + Some("khive ADR-040 khive ADR-040 khive ADR-040 khive ADR-040 khive ADR-040"), + None, + vec![], + ) + .await + .expect("create decoy entity"); + } + + let resolution = resolve_reference(&rt, &ring, &token, "khive ADR-040", 5, None) + .await + .expect("resolve_reference"); + + let found_target = match resolution { + ReferenceResolution::Resolved { id, .. } => id == target.id, + ReferenceResolution::Ambiguous { candidates } => { + candidates.iter().any(|c| c.id == target.id) + } + ReferenceResolution::NotFound => false, + }; + assert!( + found_target, + "stage 4 must surface a canonical-name match ranked below the \ + caller's default `limit` instead of dropping it (#908)" + ); + } + + // Regression for #908: genuine ambiguity — two entities with the exact + // same canonical name — must still return `Ambiguous`, never a silent + // pick, after widening stage 4's retrieval floor. + #[tokio::test] + async fn fallback_stage_still_reports_ambiguous_on_genuine_tie() { + let rt = KhiveRuntime::memory().expect("in-memory runtime"); + let token = actor_token("resolver-test"); + let ring = ReferenceRing::new(); + + let a = rt + .create_entity( + &token, + "concept", + None, + "Twin Record", + Some("khive Twin Record document"), + None, + vec![], + ) + .await + .expect("create entity a"); + let b = rt + .create_entity( + &token, + "concept", + None, + "Twin Record", + Some("khive Twin Record document"), + None, + vec![], + ) + .await + .expect("create entity b"); + + // Exact byte-identical names hit stage 3 (exact-name storage + // lookup), not stage 4 — but the same "must not silently pick" + // contract applies at both stages, and stage 3 is a cheaper, + // deterministic way to pin it. + let resolution = resolve_reference(&rt, &ring, &token, "Twin Record", 5, None) + .await + .expect("resolve_reference"); + + match resolution { + ReferenceResolution::Ambiguous { candidates } => { + let ids: std::collections::HashSet = + candidates.iter().map(|c| c.id).collect(); + assert!(ids.contains(&a.id) && ids.contains(&b.id)); + } + other => panic!("expected Ambiguous on a genuine name tie, got {other:?}"), + } + } + + // Regression for #908: garbage input that matches nothing must still be + // `NotFound` after widening stage 4's retrieval floor — the widened pool + // must not turn "nothing relevant exists" into a spurious pick. + #[tokio::test] + async fn fallback_stage_still_not_found_on_garbage() { + let rt = KhiveRuntime::memory().expect("in-memory runtime"); + let token = actor_token("resolver-test"); + let ring = ReferenceRing::new(); + + rt.create_entity( + &token, + "concept", + None, + "ADR-040", + Some("khive ADR-040"), + None, + vec![], + ) + .await + .expect("create unrelated entity"); + + let resolution = resolve_reference( + &rt, + &ring, + &token, + "zzqxw completely unrelated garbage nonsense", + 5, + None, + ) + .await + .expect("resolve_reference"); + assert_eq!(resolution, ReferenceResolution::NotFound); + } } From ddfdbb1f3a48c2aae4305b82e67f8411c46191ab Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 13 Jul 2026 17:49:45 -0400 Subject: [PATCH 2/7] test(mcp): keep schedule routing regression hermetic --- crates/khive-mcp/src/serve.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/khive-mcp/src/serve.rs b/crates/khive-mcp/src/serve.rs index 6116fac2f..a9fd7a163 100644 --- a/crates/khive-mcp/src/serve.rs +++ b/crates/khive-mcp/src/serve.rs @@ -5048,11 +5048,21 @@ backend = "kg-backend" ); use clap::Parser; - let args = Args::parse_from(["mcp", "--config", config_path.to_str().expect("utf8 path")]); + let args = Args::parse_from([ + "mcp", + "--config", + config_path.to_str().expect("utf8 path"), + "--no-embed", + ]); let (server, schedule_rt) = build_server(&args).expect("build_server must succeed"); // No `[packs.schedule]` entry above, so it defaults to "main". let rt = schedule_rt.expect("schedule pack is loaded by default"); + assert!( + rt.config().embedding_model.is_none() + && rt.config().additional_embedding_models.is_empty(), + "the backend-routing fixture must remain independent of external embedding models" + ); let marker = "adr106-multi-backend-dispatch-marker"; let action_dsl = format!("create(kind=\"observation\", content=\"{marker}\")"); From 7871897f1dde987658144b9a30e6ed37b94dfe46 Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 13 Jul 2026 17:02:23 -0400 Subject: [PATCH 3/7] test(khive-db): serialize writer-task tests on tx_registry to fix flaky tx-age sweep run_writer_task registers a writer_task_tx handle in the process-wide tx_registry singleton; the checkpoint tx_age_sweep_* tests read that singleton under #[serial(tx_registry)]. The writer_task.rs tests that spawn a writer task were missing from the group, leaking a longer-lived writer_task_tx into the sweep's oldest() read and intermittently failing the assertion on main CI. Join them to the serial group. No prod change. --- crates/khive-db/src/writer_task.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/khive-db/src/writer_task.rs b/crates/khive-db/src/writer_task.rs index c13590065..17738a9a5 100644 --- a/crates/khive-db/src/writer_task.rs +++ b/crates/khive-db/src/writer_task.rs @@ -420,6 +420,7 @@ async fn run_writer_task( mod tests { use super::*; use crate::pool::PoolConfig; + use serial_test::serial; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::time::Duration; @@ -432,7 +433,14 @@ mod tests { ConnectionPool::new(cfg).expect("pool open") } + // `#[serial(tx_registry)]`: `run_writer_task` registers a `writer_task_tx` + // handle in the process-wide `tx_registry` singleton for the life of each + // `BEGIN IMMEDIATE`. Tests that observe the registry (the checkpoint + // `tx_age_sweep_*` group) read `tx_registry::oldest()`; an un-serialized + // spawning test here would leak a longer-lived `writer_task_tx` into that + // read and make the sweep name the wrong transaction. Share the key. #[tokio::test] + #[serial(tx_registry)] async fn begin_immediate_failure_replies_error_without_running_op() { // Real lock contention, not a simulation: hold the database-level // write lock from the pool's own writer connection (the unmigrated @@ -504,7 +512,11 @@ mod tests { ); } + // `#[serial(tx_registry)]`: shares the key with the checkpoint + // `tx_age_sweep_*` tests — see the note on + // `begin_immediate_failure_replies_error_without_running_op`. #[tokio::test] + #[serial(tx_registry)] async fn writer_task_executes_op_and_commits() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("writer_task_commit.db"); @@ -627,7 +639,13 @@ mod tests { first.abort(); } + // `#[serial(tx_registry)]`: this test deliberately keeps a request (and + // thus its `writer_task_tx` registry handle) alive past a timeout, so it is + // the worst polluter of the checkpoint `tx_age_sweep_*` reads if left + // un-serialized. Shares the key — see the note on + // `begin_immediate_failure_replies_error_without_running_op`. #[tokio::test] + #[serial(tx_registry)] async fn send_with_timeout_returns_op_result_when_op_outlives_the_timeout() { // `send_with_timeout`'s timeout must bound ONLY the enqueue step — // never the reply-wait. An accepted request (channel not full) must From 1e789bb75d19bdef651fe5f9b8ecd2b7cc8ee5d3 Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 13 Jul 2026 17:56:25 -0400 Subject: [PATCH 4/7] test(kkernel): make code-ingest vector test hermetic --- crates/kkernel/src/code_ingest.rs | 81 +++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 3 deletions(-) diff --git a/crates/kkernel/src/code_ingest.rs b/crates/kkernel/src/code_ingest.rs index 4d0bf5d5e..2d49613eb 100644 --- a/crates/kkernel/src/code_ingest.rs +++ b/crates/kkernel/src/code_ingest.rs @@ -104,6 +104,16 @@ pub async fn run_code_ingest(args: CodeIngestArgs) -> Result<()> { /// after validation has already succeeded and a real (non-dry-run) write is /// about to occur. async fn code_ingest_batch(args: CodeIngestArgs) -> Result { + code_ingest_batch_with_runtime_setup(args, |_| Ok(())).await +} + +async fn code_ingest_batch_with_runtime_setup( + args: CodeIngestArgs, + runtime_setup: F, +) -> Result +where + F: FnOnce(&KhiveRuntime) -> Result<()>, +{ let bytes = std::fs::read(&args.findings) .with_context(|| format!("failed to read {}", args.findings.display()))?; @@ -161,6 +171,7 @@ async fn code_ingest_batch(args: CodeIngestArgs) -> Result { } let runtime = KhiveRuntime::new(cfg).map_err(|e| anyhow::anyhow!("{e}"))?; + runtime_setup(&runtime)?; let resolved_ns = runtime.config().default_namespace.clone(); let token = runtime .authorize(resolved_ns) @@ -479,10 +490,63 @@ fn wal_sidecar_path(db_path: &Path) -> PathBuf { #[cfg(test)] mod tests { + use std::sync::Arc; + + use async_trait::async_trait; + use khive_runtime::{EmbedderProvider, RuntimeError}; + use lattice_embed::{EmbedError, EmbeddingModel, EmbeddingService}; use serial_test::serial; use super::*; + struct FixedEmbeddingService { + dimensions: usize, + } + + #[async_trait] + impl EmbeddingService for FixedEmbeddingService { + async fn embed( + &self, + texts: &[String], + _model: EmbeddingModel, + ) -> std::result::Result>, EmbedError> { + Ok(texts + .iter() + .map(|_| vec![1.0_f32; self.dimensions]) + .collect()) + } + + fn supports_model(&self, _model: EmbeddingModel) -> bool { + true + } + + fn name(&self) -> &'static str { + "code-ingest-test" + } + } + + struct FixedEmbeddingProvider { + name: String, + dimensions: usize, + } + + #[async_trait] + impl EmbedderProvider for FixedEmbeddingProvider { + fn name(&self) -> &str { + &self.name + } + + fn dimensions(&self) -> usize { + self.dimensions + } + + async fn build(&self) -> std::result::Result, RuntimeError> { + Ok(Arc::new(FixedEmbeddingService { + dimensions: self.dimensions, + })) + } + } + fn base_args(findings: PathBuf, db: PathBuf) -> CodeIngestArgs { CodeIngestArgs { findings, @@ -847,9 +911,20 @@ mod tests { let findings = write_valid_findings(tmp.path()); let db = tmp.path().join("scratch.db"); - code_ingest_batch(base_args(findings, db.clone())) - .await - .expect("ingest must succeed"); + code_ingest_batch_with_runtime_setup(base_args(findings, db.clone()), |runtime| { + let model_names = runtime.registered_embedding_model_names(); + assert!( + !model_names.is_empty(), + "test requires at least one configured embedding model" + ); + for name in model_names { + let dimensions = runtime.resolve_embedding_model(Some(&name))?.dimensions(); + runtime.register_embedder(FixedEmbeddingProvider { name, dimensions }); + } + Ok(()) + }) + .await + .expect("ingest must succeed"); let cfg = resolve_runtime_config(RuntimeConfigInputs { db: Some(db.to_str().expect("utf8 path")), From 09e3ea9d64a2129e19896acd41e4ae07dac58017 Mon Sep 17 00:00:00 2001 From: Leo Date: Tue, 14 Jul 2026 09:03:47 -0400 Subject: [PATCH 5/7] fix(runtime): bound resolve ambiguity candidates (#970) --- crates/khive-pack-kg/src/dispatch.rs | 58 +++++++++++++++ .../khive-runtime/src/reference_resolution.rs | 73 +++++++++++-------- implementation_notes.md | 16 ++++ 3 files changed, 117 insertions(+), 30 deletions(-) create mode 100644 implementation_notes.md diff --git a/crates/khive-pack-kg/src/dispatch.rs b/crates/khive-pack-kg/src/dispatch.rs index a3bf9c083..ce68e0d3f 100644 --- a/crates/khive-pack-kg/src/dispatch.rs +++ b/crates/khive-pack-kg/src/dispatch.rs @@ -1301,6 +1301,64 @@ mod tests { assert_eq!(candidates.len(), 2); } + /// Stage 4 searches deeper than the caller-facing candidate limit so its + /// decisiveness check sees enough alternatives, but the serialized + /// ambiguity payload must still honor the requested (or default) limit. + #[tokio::test] + async fn resolve_stage4_ambiguous_candidates_respect_wire_limit() { + let rt = KhiveRuntime::memory().expect("in-memory runtime"); + let token = rt.authorize(Namespace::local()).unwrap(); + + for i in 0..8 { + rt.create_entity( + &token, + "concept", + None, + &format!("Limit Candidate {i}"), + Some("stage four bounded ambiguity"), + None, + vec![], + ) + .await + .expect("create candidate entity"); + } + + let mut builder = VerbRegistryBuilder::new(); + builder.with_default_namespace("local"); + builder.register(KgPack::new(rt)); + let registry = builder.build().expect("registry build"); + + for (limit, expected) in [(Some(1_u32), 1_usize), (None, 5_usize)] { + let mut params = json!({"refs": ["stage four bounded ambiguity"]}); + if let Some(limit) = limit { + params["limit"] = json!(limit); + } + + let result = registry + .dispatch("resolve", params) + .await + .expect("resolve must succeed"); + let resolution = &result + .get("results") + .and_then(Value::as_array) + .expect("resolve must return results")[0]; + assert_eq!( + resolution.get("status").and_then(Value::as_str), + Some("ambiguous"), + "the deeper search pool must keep a tied result ambiguous" + ); + let candidates = resolution + .get("candidates") + .and_then(Value::as_array) + .expect("ambiguous result must carry candidates"); + assert_eq!( + candidates.len(), + expected, + "the serialized candidate count must honor limit={limit:?}" + ); + } + } + /// A ref that matches nothing in the ring or hybrid search is `NotFound`. #[tokio::test] async fn resolve_not_found_when_nothing_matches() { diff --git a/crates/khive-runtime/src/reference_resolution.rs b/crates/khive-runtime/src/reference_resolution.rs index aede75fc7..6d9d80e87 100644 --- a/crates/khive-runtime/src/reference_resolution.rs +++ b/crates/khive-runtime/src/reference_resolution.rs @@ -105,17 +105,15 @@ const SEARCH_RESOLVED_CONFIDENCE: f64 = 0.6; /// the same query against a wider `limit` (e.g. the `search` verb's own /// default of 10) surfaces it as the top hit (#908). Retrieving at this /// floor decouples "how deep to search" from "how many candidates the caller -/// asked for" — the caller's own `limit` still governs the id-string/ring -/// stages above and is clamped to a max of 20 -/// (`khive_pack_kg::handlers::resolve::MAX_LIMIT`) by the handler, so this -/// floor never returns more candidates than a caller could have asked for -/// outright. +/// asked for". The full pool is retained for the decisiveness check, while an +/// `Ambiguous` payload is truncated back to the caller's `limit` before it is +/// returned. const STAGE4_MIN_SEARCH_LIMIT: u32 = 20; /// Resolve one natural-language reference for `token`'s actor. /// /// `limit` bounds the hybrid-search fallback candidate count (Layer-0 stage -/// 3); it has no effect on the id-string or ring stages, which are always +/// 4); it has no effect on the id-string or ring stages, which are always /// exact-or-nothing / small in-memory scans. `entity_kind`, if set, restricts /// stage 3 to that entity kind (e.g. `"concept"`); the id-string and ring /// stages are kind-agnostic by construction (a ring entry or an explicit id @@ -248,7 +246,8 @@ pub async fn resolve_reference( // genuine match ranked just outside a small `limit` isn't truncated out // of the pool before it's even ranked against the alternatives; the // caller's `limit` still bounds how many candidates get rendered below. - let search_limit = limit.max(STAGE4_MIN_SEARCH_LIMIT); + let candidate_limit = limit.max(1); + let search_limit = candidate_limit.max(STAGE4_MIN_SEARCH_LIMIT); let hits = runtime .hybrid_search( token, @@ -291,6 +290,8 @@ pub async fn resolve_reference( confidence: SEARCH_RESOLVED_CONFIDENCE, }) } else { + let mut candidates = candidates; + candidates.truncate(candidate_limit as usize); Ok(ReferenceResolution::Ambiguous { candidates }) } } @@ -642,20 +643,12 @@ mod tests { } } - // Regression for #908: a canonical-name entity ranked outside the - // caller's small default `limit` (resolve's default is 5) used to be - // dropped entirely, because the old code called `hybrid_search` with - // that same small `limit` and `hybrid_search`'s own final step is - // `fused.truncate(limit as usize)` — so a genuine match ranked, say, - // 8th, never survived to reach `resolve_reference`'s own candidate list - // at all. Twelve decoy entities that also satisfy the (trigram, plain - // AND-mode) FTS query out-rank the target via heavier term repetition, - // pushing the target below position 5 but keeping it within - // `STAGE4_MIN_SEARCH_LIMIT` (20). The target must still surface in the - // stage-4 result — as the `Ambiguous` listing's tail if not decisively - // first — rather than vanish outright. + // Regression for #908 and the public candidate-limit contract: stage 4 + // retains a deeper decision pool even when the caller requests five + // candidates, but below-limit hits from that pool must not leak into the + // ambiguity payload. #[tokio::test] - async fn fallback_stage_recovers_canonical_name_ranked_below_default_limit() { + async fn fallback_stage_bounds_payload_after_deep_search() { let rt = KhiveRuntime::memory().expect("in-memory runtime"); let token = actor_token("resolver-test"); let ring = ReferenceRing::new(); @@ -691,22 +684,42 @@ mod tests { .expect("create decoy entity"); } + let deep_hits = rt + .hybrid_search( + &token, + "khive ADR-040", + None, + STAGE4_MIN_SEARCH_LIMIT, + None, + None, + &[], + None, + ) + .await + .expect("deep hybrid search"); + let target_rank = deep_hits + .iter() + .position(|hit| hit.entity_id == target.id) + .expect("the widened decision pool must include the target"); + assert!( + target_rank >= 5, + "fixture target must rank below the caller-facing limit" + ); + let resolution = resolve_reference(&rt, &ring, &token, "khive ADR-040", 5, None) .await .expect("resolve_reference"); - let found_target = match resolution { - ReferenceResolution::Resolved { id, .. } => id == target.id, + match resolution { ReferenceResolution::Ambiguous { candidates } => { - candidates.iter().any(|c| c.id == target.id) + assert_eq!(candidates.len(), 5); + assert!( + candidates.iter().all(|candidate| candidate.id != target.id), + "a rank-below-limit hit must remain outside the public payload" + ); } - ReferenceResolution::NotFound => false, - }; - assert!( - found_target, - "stage 4 must surface a canonical-name match ranked below the \ - caller's default `limit` instead of dropping it (#908)" - ); + other => panic!("expected bounded Ambiguous result, got {other:?}"), + } } // Regression for #908: genuine ambiguity — two entities with the exact diff --git a/implementation_notes.md b/implementation_notes.md new file mode 100644 index 000000000..789fad941 --- /dev/null +++ b/implementation_notes.md @@ -0,0 +1,16 @@ +# Implementation notes + +- Kept stage 4's widened hybrid-search pool for decisiveness checks, then truncated non-decisive `Ambiguous` candidates to the effective caller limit before returning from `resolve_reference`. +- Added an end-to-end `khive-pack-kg` dispatch regression proving serialized stage-4 candidate arrays honor both `limit=1` and the default limit of 5. +- Reconciled the existing deep-search regression with the public API contract: it now verifies that a below-limit target exists in the widened decision pool but does not leak into the bounded ambiguity payload. + +## Verification + +- `cargo fmt --all -- --check` — passed. +- `cargo test -p khive-runtime --lib` — 851 passed, 5 ignored. +- `cargo test -p khive-pack-kg --lib` — 145 passed. +- `cargo clippy -p khive-runtime -p khive-pack-kg --all-targets -- -D warnings` — passed. +- `cargo check --workspace` — passed. +- A combined all-target package test attempt was killed by the host with exit 137 during linking; the affected lib suites and focused resolver/dispatch regressions were then run separately and passed. + +Domain utility: MEDIUM — repository-local interface and regression-testing guidance reinforced testing the serialized boundary, while the code and documented `limit` contract determined the implementation. From 5fabcc1b489467d179e2b0eca0e3f496202c3e84 Mon Sep 17 00:00:00 2001 From: Leo Date: Tue, 14 Jul 2026 10:38:38 -0400 Subject: [PATCH 6/7] chore: remove workflow scratch notes from tree Merge main and drop implementation_notes.md, a stray process-notes file that does not belong in the source tree. --- implementation_notes.md | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 implementation_notes.md diff --git a/implementation_notes.md b/implementation_notes.md deleted file mode 100644 index 789fad941..000000000 --- a/implementation_notes.md +++ /dev/null @@ -1,16 +0,0 @@ -# Implementation notes - -- Kept stage 4's widened hybrid-search pool for decisiveness checks, then truncated non-decisive `Ambiguous` candidates to the effective caller limit before returning from `resolve_reference`. -- Added an end-to-end `khive-pack-kg` dispatch regression proving serialized stage-4 candidate arrays honor both `limit=1` and the default limit of 5. -- Reconciled the existing deep-search regression with the public API contract: it now verifies that a below-limit target exists in the widened decision pool but does not leak into the bounded ambiguity payload. - -## Verification - -- `cargo fmt --all -- --check` — passed. -- `cargo test -p khive-runtime --lib` — 851 passed, 5 ignored. -- `cargo test -p khive-pack-kg --lib` — 145 passed. -- `cargo clippy -p khive-runtime -p khive-pack-kg --all-targets -- -D warnings` — passed. -- `cargo check --workspace` — passed. -- A combined all-target package test attempt was killed by the host with exit 137 during linking; the affected lib suites and focused resolver/dispatch regressions were then run separately and passed. - -Domain utility: MEDIUM — repository-local interface and regression-testing guidance reinforced testing the serialized boundary, while the code and documented `limit` contract determined the implementation. From 1a5e6c8769bac1d6719d2d9bc0935ab80f4579b7 Mon Sep 17 00:00:00 2001 From: Leo Date: Fri, 17 Jul 2026 05:35:26 -0400 Subject: [PATCH 7/7] test(runtime): assert resolve() exact-name determinism, document candidate-limit contract (#970) Respond to the review of the stage-4 candidate-limit test. The exact-name short-circuit (stage 3, #849) already resolves an exact canonical name to its id before the stage-4 hybrid truncation, so no resolution logic changes; this round corrects the test and documents the contract. - Replace the candidate-limit regression test with exact_name_resolves_ahead_of_competing_hybrid_matches: a target owning an exact name resolves at EXACT_NAME_CONFIDENCE even at limit=1 against many competing hybrid matches, proving stage 3 short-circuits regardless of the target's hybrid rank. - Add fallback_stage_bounds_non_exact_payload_to_limit: a non-exact ref over near-equal matches returns an Ambiguous sample bounded to limit, with no entity framed as a withheld canonical answer. - Comment the truncation site: the bound is correct because any exact identity resolves upstream in stage 3. - Document the exact-name determinism and the bounded non-exact contract on the resolve verb help text and crates/khive-pack-kg/docs/api/resolve-verb.md. Co-authored-by: Leo Co-Authored-By: Claude Opus 4.8 --- crates/khive-pack-kg/docs/api/resolve-verb.md | 9 +- crates/khive-pack-kg/src/handler_defs.rs | 35 ++++-- .../khive-runtime/src/reference_resolution.rs | 111 ++++++++++++------ 3 files changed, 105 insertions(+), 50 deletions(-) diff --git a/crates/khive-pack-kg/docs/api/resolve-verb.md b/crates/khive-pack-kg/docs/api/resolve-verb.md index b8cf1bb61..6bcdca14b 100644 --- a/crates/khive-pack-kg/docs/api/resolve-verb.md +++ b/crates/khive-pack-kg/docs/api/resolve-verb.md @@ -44,8 +44,13 @@ filter every real match out (#849) — `entities.kind` only ever holds a granula baked into `query_entities`); two entities sharing an exact name resolve as `Ambiguous`, mirroring the ring's contract. 4. **Hybrid search fallback** — a ref with no exact-name match falls through to hybrid search - at a confidence below the exact-name stage's 0.98. A ref matching nothing at any stage is - `NotFound`. + at a confidence below the exact-name stage's 0.98. The fallback searches deeper than the + caller's `limit` so a match ranked just outside a small `limit` is still ranked against the + alternatives. When the result stays ambiguous, the returned `candidates` are a bounded sample + capped at `limit`. That bound is intentional. Outside exact matches there is no oracle for a + single "canonical" candidate (an exact identity would have resolved at stage 3), so `resolve` + returns a bounded ambiguous sample instead of silently picking one, and raising `limit` + surfaces deeper-ranked matches. A ref matching nothing at any stage is `NotFound`. `resolve`'s `kind` param is entity-only: a note kind is rejected with a clear error rather than silently over-filtering to zero matches. `resolve` is registered as a public verb and diff --git a/crates/khive-pack-kg/src/handler_defs.rs b/crates/khive-pack-kg/src/handler_defs.rs index 843c7fc67..106d5c45c 100644 --- a/crates/khive-pack-kg/src/handler_defs.rs +++ b/crates/khive-pack-kg/src/handler_defs.rs @@ -852,13 +852,20 @@ pub(crate) static KG_HANDLERS: [HandlerDef; 18] = [ HandlerDef { name: "resolve", description: "Resolve natural-language references to ids. Each ref in \ - `refs` is resolved through: (1) id-string passthrough \ - (UUID / 8+ hex prefix) via the existing by-ID path; \ - (2) this actor's recently-referenced ring; (3) hybrid \ - search over the namespace. Returns one of \ - Resolved{id,confidence} | Ambiguous{candidates} | \ - NotFound per ref — never a silent pick among close \ - candidates. Read-only: performs no mutation.", + `refs` is resolved through, in order: (1) id-string \ + passthrough (UUID / 8+ hex prefix) via the by-ID path; \ + (2) this actor's recently-referenced ring; (3) an exact, \ + case-sensitive entity-name match, which resolves \ + deterministically regardless of search rank (one match \ + -> Resolved; several identically-named entities -> \ + Ambiguous over exactly that set); (4) hybrid search over \ + the namespace. Returns one of Resolved{id,confidence} | \ + Ambiguous{candidates} | NotFound per ref — never a silent \ + pick among close candidates. For a non-exact ref that \ + stays ambiguous, `candidates` is a bounded sample capped \ + at `limit` (raise `limit` to surface deeper-ranked \ + matches); an exact-name match is an identity and is \ + exempt from that bound. Read-only: performs no mutation.", visibility: Visibility::Verb, category: VerbCategory::Assertive, params: &[ @@ -874,16 +881,20 @@ pub(crate) static KG_HANDLERS: [HandlerDef; 18] = [ name: "kind", param_type: "string", required: false, - description: "Restrict the hybrid-search fallback (stage 3) \ - to an entity kind (e.g. \"concept\", \"project\"). \ - Has no effect on the id-string or ring stages.", + description: "Restrict the exact-name (stage 3) and \ + hybrid-search (stage 4) stages to an entity kind \ + (e.g. \"concept\", \"project\"). Has no effect on \ + the id-string or ring stages.", }, ParamDef { name: "limit", param_type: "integer", required: false, - description: "Max candidates returned per ref from the \ - hybrid-search fallback. Default 5, max 20.", + description: "Max candidates in a non-exact ref's Ambiguous \ + payload from the hybrid-search fallback; raise it \ + to surface deeper-ranked matches. An exact-name \ + match resolves to a single id and ignores this \ + bound. Default 5, max 20.", }, ], }, diff --git a/crates/khive-runtime/src/reference_resolution.rs b/crates/khive-runtime/src/reference_resolution.rs index 6d9d80e87..67fce103e 100644 --- a/crates/khive-runtime/src/reference_resolution.rs +++ b/crates/khive-runtime/src/reference_resolution.rs @@ -290,6 +290,12 @@ pub async fn resolve_reference( confidence: SEARCH_RESOLVED_CONFIDENCE, }) } else { + // Non-exact ambiguity: bound the payload to the caller's + // `limit`. Any deterministic identity (an exact canonical + // name) was already resolved by stage 3 above, so nothing + // withheld here is "the" answer — outside exact matches there + // is no oracle for a single canonical candidate. Raising + // `limit` surfaces deeper ranks (#970; resolve() contract). let mut candidates = candidates; candidates.truncate(candidate_limit as usize); Ok(ReferenceResolution::Ambiguous { candidates }) @@ -643,12 +649,17 @@ mod tests { } } - // Regression for #908 and the public candidate-limit contract: stage 4 - // retains a deeper decision pool even when the caller requests five - // candidates, but below-limit hits from that pool must not leak into the - // ambiguity payload. + // Regression for #908 / #970: an exact canonical-name ref resolves to a + // single id through the stage-3 exact-name lookup, which short-circuits + // BEFORE the stage-4 hybrid fallback and its `limit` bound. Many other + // entities are strong hybrid matches for the same term — enough that + // hybrid alone would rank them close together and return `Ambiguous` — yet + // the exact name resolves deterministically to its owner. The returned + // `EXACT_NAME_CONFIDENCE` (not the stage-4 `SEARCH_RESOLVED_CONFIDENCE`) + // proves the result came from stage 3, regardless of the target's hybrid + // rank. An exact name is an identity, not a ranked candidate. #[tokio::test] - async fn fallback_stage_bounds_payload_after_deep_search() { + async fn exact_name_resolves_ahead_of_competing_hybrid_matches() { let rt = KhiveRuntime::memory().expect("in-memory runtime"); let token = actor_token("resolver-test"); let ring = ReferenceRing::new(); @@ -666,59 +677,87 @@ mod tests { .await .expect("create target entity"); - // Decoys out-rank the target by repeating the shared query terms - // many times (FTS5 bm25 rewards term frequency); none share the - // target's canonical name, so stage 3 (exact-name) never fires and - // stage 4 is genuinely reached. + // Competitors that also match "ADR-040" strongly in hybrid search but + // do NOT carry it as an exact name; only the target owns the exact + // name, so only the target satisfies the stage-3 lookup. for i in 0..12 { rt.create_entity( &token, "concept", None, - &format!("Filler{i}"), - Some("khive ADR-040 khive ADR-040 khive ADR-040 khive ADR-040 khive ADR-040"), + &format!("ADR-040 companion {i}"), + Some("ADR-040 ADR-040 ADR-040 ADR-040 ADR-040"), None, vec![], ) .await - .expect("create decoy entity"); + .expect("create competing entity"); } - let deep_hits = rt - .hybrid_search( + // Even at limit = 1 — the tightest bound — the exact name resolves to + // the target, at the stage-3 confidence, never a ranked hybrid pick. + let resolution = resolve_reference(&rt, &ring, &token, "ADR-040", 1, None) + .await + .expect("resolve_reference"); + assert_eq!( + resolution, + ReferenceResolution::Resolved { + id: target.id, + confidence: EXACT_NAME_CONFIDENCE, + } + ); + } + + // The complement of the exact-name contract: a NON-exact ref (no entity + // carries it as an exact name) that stays ambiguous returns a bounded + // sample capped at the caller's `limit`. Outside exact matches there is no + // oracle for a single "canonical" candidate, so the bound is the + // intentional, documented resolve() contract (raise `limit` to surface + // deeper ranks) — not a withheld identity. Any deterministic identity + // would have resolved upstream in stage 3. + #[tokio::test] + async fn fallback_stage_bounds_non_exact_payload_to_limit() { + let rt = KhiveRuntime::memory().expect("in-memory runtime"); + let token = actor_token("resolver-test"); + let ring = ReferenceRing::new(); + + // Twelve near-equal matches, each with a distinct name and the same + // descriptive text — none is the "right" answer, so the result is a + // genuinely bounded ambiguous sample, not a suppressed target. + for i in 0..12 { + rt.create_entity( &token, - "khive ADR-040", - None, - STAGE4_MIN_SEARCH_LIMIT, - None, + "concept", None, - &[], + &format!("Retrieval Fusion Note {i}"), + Some("khive retrieval fusion ranking note"), None, + vec![], ) .await - .expect("deep hybrid search"); - let target_rank = deep_hits - .iter() - .position(|hit| hit.entity_id == target.id) - .expect("the widened decision pool must include the target"); - assert!( - target_rank >= 5, - "fixture target must rank below the caller-facing limit" - ); + .expect("create entity"); + } - let resolution = resolve_reference(&rt, &ring, &token, "khive ADR-040", 5, None) - .await - .expect("resolve_reference"); + let resolution = resolve_reference( + &rt, + &ring, + &token, + "khive retrieval fusion ranking", + 5, + None, + ) + .await + .expect("resolve_reference"); match resolution { ReferenceResolution::Ambiguous { candidates } => { - assert_eq!(candidates.len(), 5); - assert!( - candidates.iter().all(|candidate| candidate.id != target.id), - "a rank-below-limit hit must remain outside the public payload" + assert_eq!( + candidates.len(), + 5, + "a non-exact ref's ambiguity payload is bounded to the caller's limit" ); } - other => panic!("expected bounded Ambiguous result, got {other:?}"), + other => panic!("expected a bounded Ambiguous sample, got {other:?}"), } }