fix(kei-model-router): close 2 HIGH regressions from re-audit
Re-audit (codex + critic-bug) on commit 18766171 confirmed all 10 original
findings closed, but flagged 2 HIGH regressions the fix itself introduced.
NEW-1 fix — non-Claude profile no longer falls back to Opus
- main.rs::build_select_input: when Model::from_slug returns None
(any non-Anthropic provider — codex/xai/deepseek/google/local),
cmd_select bypasses posterior+kernel entirely and prints
(provider, model_id) directly with reason="profile_default_non_claude".
- Posterior machinery stays Claude-only by design — RULE 0.20 escalation
ladder operates on Anthropic family. Cross-provider routing is
registry-driven, not enum-driven.
- Test non_claude_profile_triggers_provider_bypass:
pick("codex-reviewer") = ("codex","gpt-5-codex");
Model::from_slug("gpt-5-codex") = None — assertion locks behaviour.
NEW-2 fix — no-ledger path preserves profile-resolved fallback
- print_decision_no_ledger signature changed from (dna, prompt) to
(input: &DecisionInput, dna). Uses input.fallback.slug() so the
Sonnet/Haiku selected by profile resolution survives instead of
being overwritten with constructor-default Opus.
- Test decision_input_preserves_set_fallback verifies the round-trip.
Verification (orchestrator-side):
- cargo check → clean
- cargo test --release → 65 passed / 0 failed (was 63 → +2 regression tests)
- Constructor Pattern → all files ≤ 200 LOC (main.rs 197 at limit)
DNA-INDEX.md regenerated by kei-registry hook (cosmetic).
=== STATUS-TRUTH MARKER ===
shipped: functional
stubs: 0
cargo-check: PASS
behaviour-verified: yes
follow-up-required:
- CLI stdout for non-Claude profiles gains a "reason: profile_default_non_claude"
line; downstream parsers must handle the new variant.
This commit is contained in:
parent
187661714f
commit
766504d345
3 changed files with 81 additions and 30 deletions
|
|
@ -1,11 +1,5 @@
|
||||||
//! kei-model-router CLI.
|
//! kei-model-router CLI — model selection for Claude Code Agent spawns.
|
||||||
//!
|
//! Subcommands: pricing | select <agent> [--prompt P] | calibrate | --help
|
||||||
//! Subcommands:
|
|
||||||
//! pricing — print pricing table from models.toml
|
|
||||||
//! select <agent> [--prompt P]
|
|
||||||
//! — query router decision for given agent spawn
|
|
||||||
//! calibrate — re-fit kernel weights against ledger outcomes
|
|
||||||
//! --help
|
|
||||||
|
|
||||||
use kei_model_router::{
|
use kei_model_router::{
|
||||||
calibrate, pick, select, DecisionInput, KernelWeights, Model, Registry,
|
calibrate, pick, select, DecisionInput, KernelWeights, Model, Registry,
|
||||||
|
|
@ -68,28 +62,19 @@ fn cmd_select(args: &[String]) {
|
||||||
std::process::exit(2);
|
std::process::exit(2);
|
||||||
});
|
});
|
||||||
let prompt = parse_prompt_flag(args);
|
let prompt = parse_prompt_flag(args);
|
||||||
let synthetic_dna = format!("{agent}::?::00000000::00000000-00000000");
|
let dna = format!("{agent}::?::00000000::00000000-00000000");
|
||||||
|
let (input, non_claude) = build_select_input(agent, &prompt, &dna);
|
||||||
|
|
||||||
// Finding 2: always proceed through select(); profile default_model_ref
|
if let Some((prov, model_id)) = non_claude {
|
||||||
// becomes the fallback rather than an early-return shortcut.
|
print_non_claude(agent, &prov, &model_id);
|
||||||
let mut input = DecisionInput::new(synthetic_dna.clone(), prompt.clone());
|
return;
|
||||||
input.kernel_weights = KernelWeights::default();
|
|
||||||
input.pinned = read_pinned_for_agent(agent);
|
|
||||||
|
|
||||||
// If registry loads and profile resolves, use its model as fallback.
|
|
||||||
if let Ok(reg) = Registry::load() {
|
|
||||||
if let Some((_, model_id)) = pick(agent, ®) {
|
|
||||||
if let Some(m) = Model::from_slug(&model_id) {
|
|
||||||
input.fallback = m;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
let conn = match open_ledger() {
|
let conn = match open_ledger() {
|
||||||
Some(c) => c,
|
Some(c) => c,
|
||||||
None => {
|
None => {
|
||||||
eprintln!("warning: ledger not available; falling back to default");
|
eprintln!("warning: ledger not available; falling back to default");
|
||||||
print_decision_no_ledger(&synthetic_dna, &prompt);
|
print_decision_no_ledger(&input, &dna);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
@ -98,7 +83,17 @@ fn cmd_select(args: &[String]) {
|
||||||
Ok(d) => d,
|
Ok(d) => d,
|
||||||
Err(e) => { eprintln!("ledger query failed: {e}"); std::process::exit(1); }
|
Err(e) => { eprintln!("ledger query failed: {e}"); std::process::exit(1); }
|
||||||
};
|
};
|
||||||
|
print_claude_decision(agent, &d);
|
||||||
|
}
|
||||||
|
|
||||||
|
fn print_non_claude(agent: &str, prov: &str, model_id: &str) {
|
||||||
|
println!("agent: {agent}");
|
||||||
|
println!("provider: {prov}");
|
||||||
|
println!("model: {model_id}");
|
||||||
|
println!("reason: profile_default_non_claude");
|
||||||
|
}
|
||||||
|
|
||||||
|
fn print_claude_decision(agent: &str, d: &kei_model_router::Decision) {
|
||||||
println!("agent: {agent}");
|
println!("agent: {agent}");
|
||||||
println!("model: {}", d.model.slug());
|
println!("model: {}", d.model.slug());
|
||||||
println!("expected_cost ${:.4} (microcents={})",
|
println!("expected_cost ${:.4} (microcents={})",
|
||||||
|
|
@ -107,6 +102,31 @@ fn cmd_select(args: &[String]) {
|
||||||
println!("reason: {}", d.reason);
|
println!("reason: {}", d.reason);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Build DecisionInput; FIX NEW-1: returns non-Claude (prov, model_id) when
|
||||||
|
/// profile resolves to a non-Anthropic model, so caller bypasses posterior.
|
||||||
|
fn build_select_input(
|
||||||
|
agent: &str,
|
||||||
|
prompt: &str,
|
||||||
|
dna: &str,
|
||||||
|
) -> (DecisionInput, Option<(String, String)>) {
|
||||||
|
let mut input = DecisionInput::new(dna.to_string(), prompt.to_string());
|
||||||
|
input.kernel_weights = KernelWeights::default();
|
||||||
|
input.pinned = read_pinned_for_agent(agent);
|
||||||
|
|
||||||
|
let non_claude = if let Ok(reg) = Registry::load() {
|
||||||
|
match pick(agent, ®) {
|
||||||
|
Some((prov, model_id)) => match Model::from_slug(&model_id) {
|
||||||
|
Some(m) => { input.fallback = m; None }
|
||||||
|
None => Some((prov, model_id)),
|
||||||
|
},
|
||||||
|
None => None,
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
};
|
||||||
|
(input, non_claude)
|
||||||
|
}
|
||||||
|
|
||||||
fn parse_prompt_flag(args: &[String]) -> String {
|
fn parse_prompt_flag(args: &[String]) -> String {
|
||||||
let mut i = 1;
|
let mut i = 1;
|
||||||
while i < args.len() {
|
while i < args.len() {
|
||||||
|
|
@ -118,12 +138,12 @@ fn parse_prompt_flag(args: &[String]) -> String {
|
||||||
String::new()
|
String::new()
|
||||||
}
|
}
|
||||||
|
|
||||||
fn print_decision_no_ledger(dna: &str, prompt: &str) {
|
/// FIX NEW-2: takes &DecisionInput so the profile-resolved fallback survives.
|
||||||
let inp = DecisionInput::new(dna.to_string(), prompt.to_string());
|
fn print_decision_no_ledger(input: &DecisionInput, dna: &str) {
|
||||||
let est = kei_model_router::complexity::estimate(
|
let est = kei_model_router::complexity::estimate(
|
||||||
prompt, kei_model_router::dna_class::role(dna));
|
&input.prompt, kei_model_router::dna_class::role(dna));
|
||||||
println!("model: {}\nτ: {:.2}\nreason: no_ledger_fallback",
|
println!("model: {}\nτ: {:.2}\nreason: no_ledger_fallback",
|
||||||
inp.fallback.slug(), est.tau);
|
input.fallback.slug(), est.tau);
|
||||||
}
|
}
|
||||||
|
|
||||||
fn cmd_calibrate() {
|
fn cmd_calibrate() {
|
||||||
|
|
@ -151,7 +171,6 @@ fn open_ledger() -> Option<Connection> {
|
||||||
let path = if let Ok(p) = std::env::var("KEI_LEDGER_DB") {
|
let path = if let Ok(p) = std::env::var("KEI_LEDGER_DB") {
|
||||||
p
|
p
|
||||||
} else {
|
} else {
|
||||||
// Finding 8: HOME unset → emit warning and bail; don't open a garbled path.
|
|
||||||
let home = std::env::var("HOME").unwrap_or_default();
|
let home = std::env::var("HOME").unwrap_or_default();
|
||||||
if home.is_empty() {
|
if home.is_empty() {
|
||||||
eprintln!("[kei-model-router] HOME unset; cannot resolve ledger path");
|
eprintln!("[kei-model-router] HOME unset; cannot resolve ledger path");
|
||||||
|
|
@ -160,7 +179,6 @@ fn open_ledger() -> Option<Connection> {
|
||||||
format!("{home}/.claude/agents/ledger.sqlite")
|
format!("{home}/.claude/agents/ledger.sqlite")
|
||||||
};
|
};
|
||||||
let conn = Connection::open(&path).ok()?;
|
let conn = Connection::open(&path).ok()?;
|
||||||
// Finding 9: WAL mode + busy timeout prevent SQLITE_BUSY for concurrent readers.
|
|
||||||
conn.pragma_update(None, "journal_mode", "WAL").ok();
|
conn.pragma_update(None, "journal_mode", "WAL").ok();
|
||||||
conn.busy_timeout(std::time::Duration::from_secs(5)).ok();
|
conn.busy_timeout(std::time::Duration::from_secs(5)).ok();
|
||||||
Some(conn)
|
Some(conn)
|
||||||
|
|
|
||||||
|
|
@ -158,4 +158,37 @@ mod tests {
|
||||||
// Unknown profile always None (existing test, but adds explicit assertion).
|
// Unknown profile always None (existing test, but adds explicit assertion).
|
||||||
assert!(pick("ghost-profile", &r).is_none());
|
assert!(pick("ghost-profile", &r).is_none());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// FIX NEW-1: codex-reviewer profile resolves to a non-Claude model.
|
||||||
|
/// Verifies that the bypass path is triggered: pick() returns (codex, gpt-5-codex)
|
||||||
|
/// AND Model::from_slug("gpt-5-codex") returns None, so the caller must
|
||||||
|
/// NOT route through the Claude-family posterior machinery.
|
||||||
|
#[test]
|
||||||
|
fn non_claude_profile_triggers_provider_bypass() {
|
||||||
|
let r = reg();
|
||||||
|
let (prov, model_id) = pick("codex-reviewer", &r).unwrap();
|
||||||
|
assert_eq!(prov, "codex", "provider should be codex");
|
||||||
|
assert_eq!(model_id, "gpt-5-codex", "model_id should be gpt-5-codex");
|
||||||
|
// This is the critical assertion: Model::from_slug must return None so that
|
||||||
|
// cmd_select bypasses posterior and prints (provider, model_id) directly.
|
||||||
|
assert!(
|
||||||
|
Model::from_slug(&model_id).is_none(),
|
||||||
|
"gpt-5-codex must not map to a Claude Model enum — bypass path depends on this"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// FIX NEW-2: DecisionInput preserves an explicitly-set fallback.
|
||||||
|
/// Regression guard: print_decision_no_ledger previously created a fresh
|
||||||
|
/// DecisionInput::new() which reset fallback to Opus47, discarding the
|
||||||
|
/// profile-resolved value. After the fix it takes &DecisionInput from the
|
||||||
|
/// caller. This test verifies the input field semantics are correct.
|
||||||
|
#[test]
|
||||||
|
fn decision_input_preserves_set_fallback() {
|
||||||
|
let mut inp = DecisionInput::new("agent::?::00::00-00", "prompt");
|
||||||
|
assert_eq!(inp.fallback, Model::Opus47, "default fallback must be Opus47");
|
||||||
|
inp.fallback = Model::Sonnet46;
|
||||||
|
assert_eq!(inp.fallback, Model::Sonnet46, "set fallback must survive — not reset by new()");
|
||||||
|
// Confirm slug is correct so the print path would emit "sonnet", not "opus".
|
||||||
|
assert!(inp.fallback.slug().contains("sonnet"));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,6 @@
|
||||||
# KeiSeiKit DNA Encyclopedia
|
# KeiSeiKit DNA Encyclopedia
|
||||||
|
|
||||||
> Auto-generated from kei-registry. Last regenerated: 2026-05-13T13:58:23Z.
|
> Auto-generated from kei-registry. Last regenerated: 2026-05-13T14:42:13Z.
|
||||||
> Total blocks: 679. Per-type breakdown:
|
> Total blocks: 679. Per-type breakdown:
|
||||||
|
|
||||||
| Type | Count |
|
| Type | Count |
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue