From c6f979e2f2219175dda54121bfb3345bf7b21968 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 20 Mar 2026 18:55:16 -0400 Subject: [PATCH] Resolve model aliases to canonical IDs in workflow graph nodes ProviderInferenceTransform only inferred the provider but passed the raw alias (e.g. "gpt-54") to the LLM API, causing request failures. Rename to ModelResolutionTransform and resolve aliases via the model catalog so the canonical ID (e.g. "gpt-5.4") is used in API calls. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-workflows/src/transform.rs | 72 ++++++++++++++++++--- lib/crates/fabro-workflows/src/workflow.rs | 7 +- 2 files changed, 66 insertions(+), 13 deletions(-) diff --git a/lib/crates/fabro-workflows/src/transform.rs b/lib/crates/fabro-workflows/src/transform.rs index 80ea396ab..9f8898705 100644 --- a/lib/crates/fabro-workflows/src/transform.rs +++ b/lib/crates/fabro-workflows/src/transform.rs @@ -112,10 +112,10 @@ impl Transform for StylesheetApplicationTransform { } } -/// For nodes with `model` but no `provider`, infer the provider from the model catalog. -pub struct ProviderInferenceTransform; +/// Resolves model aliases to canonical IDs and infers the provider from the model catalog. +pub struct ModelResolutionTransform; -impl Transform for ProviderInferenceTransform { +impl Transform for ModelResolutionTransform { fn apply(&self, graph: &mut Graph) { for node in graph.nodes.values_mut() { let model = node @@ -124,8 +124,13 @@ impl Transform for ProviderInferenceTransform { .and_then(AttrValue::as_str) .map(String::from); if let Some(model) = model { - if !node.attrs.contains_key("provider") { - if let Some(info) = fabro_model::get_model_info(&model) { + if let Some(info) = fabro_model::get_model_info(&model) { + // Resolve alias to canonical model ID + if model != info.id { + node.attrs + .insert("model".to_string(), AttrValue::String(info.id.clone())); + } + if !node.attrs.contains_key("provider") { node.attrs .insert("provider".to_string(), AttrValue::String(info.provider)); } @@ -637,7 +642,7 @@ mod tests { } // ----------------------------------------------------------------------- - // ProviderInferenceTransform tests + // ModelResolutionTransform tests // ----------------------------------------------------------------------- #[test] @@ -650,7 +655,7 @@ mod tests { ); graph.nodes.insert("a".to_string(), node); - ProviderInferenceTransform.apply(&mut graph); + ModelResolutionTransform.apply(&mut graph); assert_eq!( graph.nodes["a"] @@ -675,7 +680,7 @@ mod tests { ); graph.nodes.insert("a".to_string(), node); - ProviderInferenceTransform.apply(&mut graph); + ModelResolutionTransform.apply(&mut graph); assert_eq!( graph.nodes["a"] @@ -696,7 +701,7 @@ mod tests { ); graph.nodes.insert("a".to_string(), node); - ProviderInferenceTransform.apply(&mut graph); + ModelResolutionTransform.apply(&mut graph); assert_eq!(graph.nodes["a"].attrs.get("provider"), None); } @@ -707,11 +712,58 @@ mod tests { let node = Node::new("a"); graph.nodes.insert("a".to_string(), node); - ProviderInferenceTransform.apply(&mut graph); + ModelResolutionTransform.apply(&mut graph); assert_eq!(graph.nodes["a"].attrs.get("provider"), None); } + #[test] + fn model_resolution_resolves_alias_to_canonical_id() { + let mut graph = Graph::new("test"); + let mut node = Node::new("a"); + node.attrs + .insert("model".to_string(), AttrValue::String("gpt-54".to_string())); + graph.nodes.insert("a".to_string(), node); + + ModelResolutionTransform.apply(&mut graph); + + assert_eq!( + graph.nodes["a"] + .attrs + .get("model") + .and_then(AttrValue::as_str), + Some("gpt-5.4") + ); + assert_eq!( + graph.nodes["a"] + .attrs + .get("provider") + .and_then(AttrValue::as_str), + Some("openai") + ); + } + + #[test] + fn model_resolution_keeps_canonical_id_unchanged() { + let mut graph = Graph::new("test"); + let mut node = Node::new("a"); + node.attrs.insert( + "model".to_string(), + AttrValue::String("gpt-5.4".to_string()), + ); + graph.nodes.insert("a".to_string(), node); + + ModelResolutionTransform.apply(&mut graph); + + assert_eq!( + graph.nodes["a"] + .attrs + .get("model") + .and_then(AttrValue::as_str), + Some("gpt-5.4") + ); + } + // ----------------------------------------------------------------------- // resolve_file_ref tests // ----------------------------------------------------------------------- diff --git a/lib/crates/fabro-workflows/src/workflow.rs b/lib/crates/fabro-workflows/src/workflow.rs index b2b386fb1..8dd168715 100644 --- a/lib/crates/fabro-workflows/src/workflow.rs +++ b/lib/crates/fabro-workflows/src/workflow.rs @@ -2,7 +2,7 @@ use std::path::Path; use crate::error::FabroError; use crate::transform::{ - FileInliningTransform, ProviderInferenceTransform, StylesheetApplicationTransform, Transform, + FileInliningTransform, ModelResolutionTransform, StylesheetApplicationTransform, Transform, VariableExpansionTransform, }; use fabro_graphviz::graph::Graph; @@ -61,7 +61,7 @@ impl WorkflowBuilder { // Built-in transforms (PreambleTransform moved to engine execution time) VariableExpansionTransform.apply(&mut graph); StylesheetApplicationTransform.apply(&mut graph); - ProviderInferenceTransform.apply(&mut graph); + ModelResolutionTransform.apply(&mut graph); // File inlining when base_dir is provided if let Some(dir) = base_dir { @@ -158,9 +158,10 @@ mod tests { start -> work -> exit }"#; let graph = prepare_from_source(dot).unwrap(); + // "sonnet" alias is resolved to canonical ID "claude-sonnet-4-6" assert_eq!( graph.nodes["work"].attrs.get("model"), - Some(&AttrValue::String("sonnet".into())) + Some(&AttrValue::String("claude-sonnet-4-6".into())) ); }