fix(kei-model-router): close 2 HIGH regressions from re-audit
Re-audit (codex + critic-bug) on commit 3b03fb7b 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
3b03fb7bc3
commit
2631766ff7
3 changed files with 81 additions and 30 deletions
|
|
@ -1,11 +1,5 @@
|
|||
//! kei-model-router CLI.
|
||||
//!
|
||||
//! 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
|
||||
//! kei-model-router CLI — model selection for Claude Code Agent spawns.
|
||||
//! Subcommands: pricing | select <agent> [--prompt P] | calibrate | --help
|
||||
|
||||
use kei_model_router::{
|
||||
calibrate, pick, select, DecisionInput, KernelWeights, Model, Registry,
|
||||
|
|
@ -68,28 +62,19 @@ fn cmd_select(args: &[String]) {
|
|||
std::process::exit(2);
|
||||
});
|
||||
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
|
||||
// becomes the fallback rather than an early-return shortcut.
|
||||
let mut input = DecisionInput::new(synthetic_dna.clone(), prompt.clone());
|
||||
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;
|
||||
}
|
||||
}
|
||||
if let Some((prov, model_id)) = non_claude {
|
||||
print_non_claude(agent, &prov, &model_id);
|
||||
return;
|
||||
}
|
||||
|
||||
let conn = match open_ledger() {
|
||||
Some(c) => c,
|
||||
None => {
|
||||
eprintln!("warning: ledger not available; falling back to default");
|
||||
print_decision_no_ledger(&synthetic_dna, &prompt);
|
||||
print_decision_no_ledger(&input, &dna);
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
|
@ -98,7 +83,17 @@ fn cmd_select(args: &[String]) {
|
|||
Ok(d) => d,
|
||||
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!("model: {}", d.model.slug());
|
||||
println!("expected_cost ${:.4} (microcents={})",
|
||||
|
|
@ -107,6 +102,31 @@ fn cmd_select(args: &[String]) {
|
|||
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 {
|
||||
let mut i = 1;
|
||||
while i < args.len() {
|
||||
|
|
@ -118,12 +138,12 @@ fn parse_prompt_flag(args: &[String]) -> String {
|
|||
String::new()
|
||||
}
|
||||
|
||||
fn print_decision_no_ledger(dna: &str, prompt: &str) {
|
||||
let inp = DecisionInput::new(dna.to_string(), prompt.to_string());
|
||||
/// FIX NEW-2: takes &DecisionInput so the profile-resolved fallback survives.
|
||||
fn print_decision_no_ledger(input: &DecisionInput, dna: &str) {
|
||||
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",
|
||||
inp.fallback.slug(), est.tau);
|
||||
input.fallback.slug(), est.tau);
|
||||
}
|
||||
|
||||
fn cmd_calibrate() {
|
||||
|
|
@ -151,7 +171,6 @@ fn open_ledger() -> Option<Connection> {
|
|||
let path = if let Ok(p) = std::env::var("KEI_LEDGER_DB") {
|
||||
p
|
||||
} else {
|
||||
// Finding 8: HOME unset → emit warning and bail; don't open a garbled path.
|
||||
let home = std::env::var("HOME").unwrap_or_default();
|
||||
if home.is_empty() {
|
||||
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")
|
||||
};
|
||||
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.busy_timeout(std::time::Duration::from_secs(5)).ok();
|
||||
Some(conn)
|
||||
|
|
|
|||
|
|
@ -158,4 +158,37 @@ mod tests {
|
|||
// Unknown profile always None (existing test, but adds explicit assertion).
|
||||
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
|
||||
|
||||
> 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:
|
||||
|
||||
| Type | Count |
|
||||
|
|
|
|||
Loading…
Reference in a new issue