chore(personality) new schema with fallbacks (#10147)

## Summary Let's dial in this api contract in a bit more with more robust fallback behavior when model_instructions_template is false. Switches to a more explicit template / variables structure, with more fallbacks. ## Testing - [x] Adding unit tests - [x] Tested locally
2026-04-30 01:16:54 +00:00 · 2026-01-30 00:10:12 -07:00
parent d550fbf41a
commit e3ab0bd973
13 changed files with 607 additions and 115 deletions
--- a/codex-rs/protocol/src/openai_models.rs
+++ b/codex-rs/protocol/src/openai_models.rs
@@ -1,4 +1,3 @@
-use std::collections::BTreeMap;
 use std::collections::HashMap;
 use std::collections::HashSet;

@@ -14,7 +13,7 @@ use ts_rs::TS;
 use crate::config_types::Personality;
 use crate::config_types::Verbosity;

-const PERSONALITY_PLACEHOLDER: &str = "{{ personality_message }}";
+const PERSONALITY_PLACEHOLDER: &str = "{{ personality }}";

 /// See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning
 #[derive(
@@ -189,7 +188,7 @@ pub struct ModelInfo {
    pub upgrade: Option<ModelInfoUpgrade>,
    pub base_instructions: String,
    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub model_instructions_template: Option<ModelInstructionsTemplate>,
+    pub model_messages: Option<ModelMessages>,
    pub supports_reasoning_summaries: bool,
    pub support_verbosity: bool,
    pub default_verbosity: Option<Verbosity>,
@@ -218,26 +217,25 @@ impl ModelInfo {
    }

    pub fn supports_personality(&self) -> bool {
-        self.model_instructions_template
+        self.model_messages
            .as_ref()
-            .is_some_and(ModelInstructionsTemplate::supports_personality)
+            .is_some_and(ModelMessages::supports_personality)
    }

    pub fn get_model_instructions(&self, personality: Option<Personality>) -> String {
-        if let Some(personality) = personality
-            && let Some(template) = &self.model_instructions_template
-            && template.has_personality_placeholder()
-            && let Some(personality_messages) = &template.personality_messages
-            && let Some(personality_message) = personality_messages.0.get(&personality)
+        if let Some(model_messages) = &self.model_messages
+            && let Some(template) = &model_messages.instructions_template
        {
-            template
-                .template
-                .replace(PERSONALITY_PLACEHOLDER, personality_message.as_str())
+            // if we have a template, always use it
+            let personality_message = model_messages
+                .get_personality_message(personality)
+                .unwrap_or_default();
+            template.replace(PERSONALITY_PLACEHOLDER, personality_message.as_str())
        } else if let Some(personality) = personality {
            warn!(
                model = %self.slug,
                %personality,
-                "Model personality requested but model_instructions_template is invalid, falling back to base instructions."
+                "Model personality requested but model_messages is missing, falling back to base instructions."
            );
            self.base_instructions.clone()
        } else {
@@ -246,31 +244,62 @@ impl ModelInfo {
    }
 }

-/// A strongly-typed template for assembling model instructions. If populated and valid, will override
-/// base_instructions.
+/// A strongly-typed template for assembling model instructions and developer messages. If
+/// instructions_* is populated and valid, it will override base_instructions.
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
-pub struct ModelInstructionsTemplate {
-    pub template: String,
-    pub personality_messages: Option<PersonalityMessages>,
+pub struct ModelMessages {
+    pub instructions_template: Option<String>,
+    pub instructions_variables: Option<ModelInstructionsVariables>,
 }

-impl ModelInstructionsTemplate {
+impl ModelMessages {
    fn has_personality_placeholder(&self) -> bool {
-        self.template.contains(PERSONALITY_PLACEHOLDER)
+        self.instructions_template
+            .as_ref()
+            .map(|spec| spec.contains(PERSONALITY_PLACEHOLDER))
+            .unwrap_or(false)
    }

    fn supports_personality(&self) -> bool {
        self.has_personality_placeholder()
-            && self.personality_messages.as_ref().is_some_and(|messages| {
-                Personality::iter().all(|personality| messages.0.contains_key(&personality))
-            })
+            && self
+                .instructions_variables
+                .as_ref()
+                .is_some_and(ModelInstructionsVariables::is_complete)
+    }
+
+    pub fn get_personality_message(&self, personality: Option<Personality>) -> Option<String> {
+        self.instructions_variables
+            .as_ref()
+            .and_then(|variables| variables.get_personality_message(personality))
    }
 }

-// serializes as a dictionary from personality to message
-#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, TS, JsonSchema)]
-#[serde(transparent)]
-pub struct PersonalityMessages(pub BTreeMap<Personality, String>);
+#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
+pub struct ModelInstructionsVariables {
+    pub personality_default: Option<String>,
+    pub personality_friendly: Option<String>,
+    pub personality_pragmatic: Option<String>,
+}
+
+impl ModelInstructionsVariables {
+    pub fn is_complete(&self) -> bool {
+        self.personality_default.is_some()
+            && self.personality_friendly.is_some()
+            && self.personality_pragmatic.is_some()
+    }
+
+    pub fn get_personality_message(&self, personality: Option<Personality>) -> Option<String> {
+        if let Some(personality) = personality {
+            match personality {
+                Personality::Friendly => self.personality_friendly.clone(),
+                Personality::Pragmatic => self.personality_pragmatic.clone(),
+            }
+        } else {
+            self.personality_default.clone()
+        }
+    }
+}

 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
 pub struct ModelInfoUpgrade {
@@ -407,7 +436,7 @@ mod tests {
    use super::*;
    use pretty_assertions::assert_eq;

-    fn test_model(template: Option<ModelInstructionsTemplate>) -> ModelInfo {
+    fn test_model(spec: Option<ModelMessages>) -> ModelInfo {
        ModelInfo {
            slug: "test-model".to_string(),
            display_name: "Test Model".to_string(),
@@ -420,7 +449,7 @@ mod tests {
            priority: 1,
            upgrade: None,
            base_instructions: "base".to_string(),
-            model_instructions_template: template,
+            model_messages: spec,
            supports_reasoning_summaries: false,
            support_verbosity: false,
            default_verbosity: None,
@@ -434,18 +463,19 @@ mod tests {
        }
    }

-    fn personality_messages() -> PersonalityMessages {
-        PersonalityMessages(BTreeMap::from([(
-            Personality::Friendly,
-            "friendly".to_string(),
-        )]))
+    fn personality_variables() -> ModelInstructionsVariables {
+        ModelInstructionsVariables {
+            personality_default: Some("default".to_string()),
+            personality_friendly: Some("friendly".to_string()),
+            personality_pragmatic: Some("pragmatic".to_string()),
+        }
    }

    #[test]
    fn get_model_instructions_uses_template_when_placeholder_present() {
-        let model = test_model(Some(ModelInstructionsTemplate {
-            template: "Hello {{ personality_message }}".to_string(),
-            personality_messages: Some(personality_messages()),
+        let model = test_model(Some(ModelMessages {
+            instructions_template: Some("Hello {{ personality }}".to_string()),
+            instructions_variables: Some(personality_variables()),
        }));

        let instructions = model.get_model_instructions(Some(Personality::Friendly));
@@ -454,14 +484,116 @@ mod tests {
    }

    #[test]
-    fn get_model_instructions_falls_back_when_placeholder_missing() {
-        let model = test_model(Some(ModelInstructionsTemplate {
-            template: "Hello there".to_string(),
-            personality_messages: Some(personality_messages()),
+    fn get_model_instructions_always_strips_placeholder() {
+        let model = test_model(Some(ModelMessages {
+            instructions_template: Some("Hello\n{{ personality }}".to_string()),
+            instructions_variables: Some(ModelInstructionsVariables {
+                personality_default: None,
+                personality_friendly: Some("friendly".to_string()),
+                personality_pragmatic: None,
+            }),
+        }));
+        assert_eq!(
+            model.get_model_instructions(Some(Personality::Friendly)),
+            "Hello\nfriendly"
+        );
+        assert_eq!(
+            model.get_model_instructions(Some(Personality::Pragmatic)),
+            "Hello\n"
+        );
+        assert_eq!(model.get_model_instructions(None), "Hello\n");
+
+        let model_no_personality = test_model(Some(ModelMessages {
+            instructions_template: Some("Hello\n{{ personality }}".to_string()),
+            instructions_variables: Some(ModelInstructionsVariables {
+                personality_default: None,
+                personality_friendly: None,
+                personality_pragmatic: None,
+            }),
+        }));
+        assert_eq!(
+            model_no_personality.get_model_instructions(Some(Personality::Friendly)),
+            "Hello\n"
+        );
+        assert_eq!(
+            model_no_personality.get_model_instructions(Some(Personality::Pragmatic)),
+            "Hello\n"
+        );
+        assert_eq!(model_no_personality.get_model_instructions(None), "Hello\n");
+    }
+
+    #[test]
+    fn get_model_instructions_falls_back_when_template_is_missing() {
+        let model = test_model(Some(ModelMessages {
+            instructions_template: None,
+            instructions_variables: Some(ModelInstructionsVariables {
+                personality_default: None,
+                personality_friendly: None,
+                personality_pragmatic: None,
+            }),
        }));

        let instructions = model.get_model_instructions(Some(Personality::Friendly));

        assert_eq!(instructions, "base");
    }
+
+    #[test]
+    fn get_personality_message_returns_default_when_personality_is_none() {
+        let personality_template = personality_variables();
+        assert_eq!(
+            personality_template.get_personality_message(None),
+            Some("default".to_string())
+        );
+    }
+
+    #[test]
+    fn get_personality_message() {
+        let personality_variables = personality_variables();
+        assert_eq!(
+            personality_variables.get_personality_message(Some(Personality::Friendly)),
+            Some("friendly".to_string())
+        );
+        assert_eq!(
+            personality_variables.get_personality_message(Some(Personality::Pragmatic)),
+            Some("pragmatic".to_string())
+        );
+        assert_eq!(
+            personality_variables.get_personality_message(None),
+            Some("default".to_string())
+        );
+
+        let personality_variables = ModelInstructionsVariables {
+            personality_default: Some("default".to_string()),
+            personality_friendly: None,
+            personality_pragmatic: None,
+        };
+        assert_eq!(
+            personality_variables.get_personality_message(Some(Personality::Friendly)),
+            None
+        );
+        assert_eq!(
+            personality_variables.get_personality_message(Some(Personality::Pragmatic)),
+            None
+        );
+        assert_eq!(
+            personality_variables.get_personality_message(None),
+            Some("default".to_string())
+        );
+
+        let personality_variables = ModelInstructionsVariables {
+            personality_default: None,
+            personality_friendly: Some("friendly".to_string()),
+            personality_pragmatic: Some("pragmatic".to_string()),
+        };
+        assert_eq!(
+            personality_variables.get_personality_message(Some(Personality::Friendly)),
+            Some("friendly".to_string())
+        );
+        assert_eq!(
+            personality_variables.get_personality_message(Some(Personality::Pragmatic)),
+            Some("pragmatic".to_string())
+        );
+        assert_eq!(personality_variables.get_personality_message(None), None);
+    }
 }