fix(llm): the preflight found two broken links on its first live run, one its own

fallback chain (6 link(s), 4 usable):
    claude-opus-4-8            ok
    claude-sonnet-4-6          ok
    claude-haiku-4-5-20251001  ok
    kimi:kimi-k2.7-code        BROKEN: 400 ... role 'system' must not be empty
    glm:glm-4.7                ok
    local:ornith-fleet:9b      UNREGISTERED — resolves to the DEFAULT provider

Neither link was actually broken.

The probe sent an EMPTY system prompt, which Kimi rejects outright. A probe has
to look like the traffic it stands in for, or it measures itself.

The second is the one worth keeping. `resolve_provider` returns a spec unchanged
when it does not recognise the provider, and the part after the FIRST colon when
it does — so the obvious test, "does the model half still contain a colon",
reads correctly and is wrong the moment a model id has one. `ornith-fleet:9b`
has one. The probe reported a provider the server had just finished registering
as UNREGISTERED.

`evaluator::cross_provider_judge` had the identical check, and would therefore
have refused a local judge as "not independent" — silently falling back to a
same-family one, which is the exact claim that path exists to make honestly.
Both now compare against the whole spec.

That bug was written into the codebase before a model name with a colon existed,
was correct at the time, and became wrong when one arrived. Nothing would have
reported it; a boot-time probe of every link did, on its first run.

Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
Omar Sobh
2026-08-09 14:42:28 -07:00
co-authored by Claude Opus 5
parent c3ad5672fc
commit d9c5300859
2 changed files with 62 additions and 8 deletions
+49 -4
View File
@@ -257,15 +257,33 @@ pub async fn preflight(runtime: &cm_runtime::Runtime, head: &str) -> Vec<(String
// A qualified spec whose provider is missing resolves to the default —
// detected the same way `cross_provider_judge` does it, by asking what
// the model half came back as.
if let Some((name, _)) = spec.split_once(':') {
if spec.contains(':') {
// Unrouted specs come back WHOLE; routed ones come back as the part
// after the FIRST colon. Testing "does it still contain a colon"
// reads the same and is wrong: `local:ornith-fleet:9b` resolves
// correctly to model `ornith-fleet:9b`, which does. This probe
// reported a provider the server had just registered as
// UNREGISTERED on its first live run, which is how the same latent
// bug was found in `evaluator::cross_provider_judge`.
let (_, resolved) = runtime.resolve_provider(&spec);
if resolved.contains(':') || resolved == spec {
let _ = name;
if resolved == spec {
out.push((spec.clone(), LinkStatus::Unregistered));
continue;
}
}
let status = match complete_or(runtime, "", "Reply with exactly: OK", &spec, 8, false).await
// A non-empty system prompt. Kimi rejects an empty one outright —
// `400 the message at position 0 with role 'system' must not be empty` —
// so an empty probe reported a healthy provider as BROKEN on the first
// live run. The probe must look like the traffic it stands in for.
let status = match complete_or(
runtime,
"You are a reachability probe.",
"Reply with exactly: OK",
&spec,
8,
false,
)
.await
{
Ok(_) => LinkStatus::Answered,
Err(e) if is_capacity_failure(&e) => LinkStatus::Throttled(e),
@@ -502,6 +520,33 @@ mod tests {
}
}
/// A model name may contain a colon, and "unregistered" must not mean that.
///
/// `resolve_provider` returns the spec unchanged when it does not recognise
/// the provider and the part after the FIRST colon when it does. The obvious
/// test — "does the model half still contain a colon" — reads the same and
/// is wrong the moment a model id has one. `ornith-fleet:9b` has one, and
/// the live preflight reported a provider the server had just registered as
/// UNREGISTERED. The identical bug was in `cross_provider_judge`, where it
/// would have refused a perfectly good independent judge.
#[test]
fn a_colon_in_the_model_name_is_not_a_missing_provider() {
// What `resolve_provider` returns, in both cases.
let routed = |spec: &str| spec.split_once(':').map(|(_, m)| m).unwrap_or(spec);
let unrouted = |spec: &str| spec;
for spec in ["local:ornith-fleet:9b", "glm:glm-4.7", "kimi:kimi-k2.7-code"] {
assert_ne!(
routed(spec),
spec,
"{spec} routed must not equal the whole spec"
);
assert_eq!(unrouted(spec), spec, "{spec} unrouted comes back whole");
}
// And the one that made the naive check look correct for so long.
assert!(routed("local:ornith-fleet:9b").contains(':'));
}
/// A throttled link is usable; an unregistered one is not.
///
/// The second is the dangerous one and the reason `preflight` checks