Skip to content

Commit 9179c9d

Browse files
authored
Merge Modelfamily into modelinfo (#8763)
- Merge ModelFamily into ModelInfo - Remove logic for adding instructions to apply patch - Add compaction limit and visible context window to `ModelInfo`
1 parent a1e8118 commit 9179c9d

File tree

28 files changed

+965
-778
lines changed

28 files changed

+965
-778
lines changed

codex-rs/app-server-protocol/src/protocol/v2.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,7 @@ pub struct ThreadTokenUsageUpdatedNotification {
12471247
pub struct ThreadTokenUsage {
12481248
pub total: TokenUsageBreakdown,
12491249
pub last: TokenUsageBreakdown,
1250+
// TODO(aibrahim): make this not optional
12501251
#[ts(type = "number | null")]
12511252
pub model_context_window: Option<i64>,
12521253
}

codex-rs/app-server/tests/common/models_cache.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo {
1515
slug: preset.id.clone(),
1616
display_name: preset.display_name.clone(),
1717
description: Some(preset.description.clone()),
18-
default_reasoning_level: preset.default_reasoning_effort,
18+
default_reasoning_level: Some(preset.default_reasoning_effort),
1919
supported_reasoning_levels: preset.supported_reasoning_efforts.clone(),
2020
shell_type: ConfigShellToolType::ShellCommand,
2121
visibility: if preset.show_in_picker {
@@ -26,14 +26,16 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo {
2626
supported_in_api: true,
2727
priority,
2828
upgrade: preset.upgrade.as_ref().map(|u| u.id.clone()),
29-
base_instructions: None,
29+
base_instructions: "base instructions".to_string(),
3030
supports_reasoning_summaries: false,
3131
support_verbosity: false,
3232
default_verbosity: None,
3333
apply_patch_tool_type: None,
3434
truncation_policy: TruncationPolicyConfig::bytes(10_000),
3535
supports_parallel_tool_calls: false,
36-
context_window: None,
36+
context_window: Some(272_000),
37+
auto_compact_token_limit: None,
38+
effective_context_window_percent: 95,
3739
experimental_supported_tools: Vec::new(),
3840
}
3941
}

codex-rs/codex-api/src/endpoint/models.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,14 +215,14 @@ mod tests {
215215
"supported_in_api": true,
216216
"priority": 1,
217217
"upgrade": null,
218-
"base_instructions": null,
218+
"base_instructions": "base instructions",
219219
"supports_reasoning_summaries": false,
220220
"support_verbosity": false,
221221
"default_verbosity": null,
222222
"apply_patch_tool_type": null,
223223
"truncation_policy": {"mode": "bytes", "limit": 10_000},
224224
"supports_parallel_tool_calls": false,
225-
"context_window": null,
225+
"context_window": 272_000,
226226
"experimental_supported_tools": [],
227227
}))
228228
.unwrap(),

codex-rs/codex-api/tests/models_integration.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ async fn models_client_hits_models_endpoint() {
5656
slug: "gpt-test".to_string(),
5757
display_name: "gpt-test".to_string(),
5858
description: Some("desc".to_string()),
59-
default_reasoning_level: ReasoningEffort::Medium,
59+
default_reasoning_level: Some(ReasoningEffort::Medium),
6060
supported_reasoning_levels: vec![
6161
ReasoningEffortPreset {
6262
effort: ReasoningEffort::Low,
@@ -76,14 +76,16 @@ async fn models_client_hits_models_endpoint() {
7676
supported_in_api: true,
7777
priority: 1,
7878
upgrade: None,
79-
base_instructions: None,
79+
base_instructions: "base instructions".to_string(),
8080
supports_reasoning_summaries: false,
8181
support_verbosity: false,
8282
default_verbosity: None,
8383
apply_patch_tool_type: None,
8484
truncation_policy: TruncationPolicyConfig::bytes(10_000),
8585
supports_parallel_tool_calls: false,
86-
context_window: None,
86+
context_window: Some(272_000),
87+
auto_compact_token_limit: None,
88+
effective_context_window_percent: 95,
8789
experimental_supported_tools: Vec::new(),
8890
}],
8991
};

codex-rs/core/prompt_with_apply_patch_instructions.md

Lines changed: 386 additions & 0 deletions
Large diffs are not rendered by default.

codex-rs/core/src/client.rs

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use codex_otel::otel_manager::OtelManager;
2222
use codex_protocol::ThreadId;
2323
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
2424
use codex_protocol::models::ResponseItem;
25+
use codex_protocol::openai_models::ModelInfo;
2526
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
2627
use codex_protocol::protocol::SessionSource;
2728
use eventsource_stream::Event;
@@ -49,15 +50,14 @@ use crate::features::FEATURES;
4950
use crate::flags::CODEX_RS_SSE_FIXTURE;
5051
use crate::model_provider_info::ModelProviderInfo;
5152
use crate::model_provider_info::WireApi;
52-
use crate::models_manager::model_family::ModelFamily;
5353
use crate::tools::spec::create_tools_json_for_chat_completions_api;
5454
use crate::tools::spec::create_tools_json_for_responses_api;
5555

5656
#[derive(Debug, Clone)]
5757
pub struct ModelClient {
5858
config: Arc<Config>,
5959
auth_manager: Option<Arc<AuthManager>>,
60-
model_family: ModelFamily,
60+
model_info: ModelInfo,
6161
otel_manager: OtelManager,
6262
provider: ModelProviderInfo,
6363
conversation_id: ThreadId,
@@ -71,7 +71,7 @@ impl ModelClient {
7171
pub fn new(
7272
config: Arc<Config>,
7373
auth_manager: Option<Arc<AuthManager>>,
74-
model_family: ModelFamily,
74+
model_info: ModelInfo,
7575
otel_manager: OtelManager,
7676
provider: ModelProviderInfo,
7777
effort: Option<ReasoningEffortConfig>,
@@ -82,7 +82,7 @@ impl ModelClient {
8282
Self {
8383
config,
8484
auth_manager,
85-
model_family,
85+
model_info,
8686
otel_manager,
8787
provider,
8888
conversation_id,
@@ -93,11 +93,11 @@ impl ModelClient {
9393
}
9494

9595
pub fn get_model_context_window(&self) -> Option<i64> {
96-
let model_family = self.get_model_family();
97-
let effective_context_window_percent = model_family.effective_context_window_percent;
98-
model_family
99-
.context_window
100-
.map(|w| w.saturating_mul(effective_context_window_percent) / 100)
96+
let model_info = self.get_model_info();
97+
let effective_context_window_percent = model_info.effective_context_window_percent;
98+
model_info.context_window.map(|context_window| {
99+
context_window.saturating_mul(effective_context_window_percent) / 100
100+
})
101101
}
102102

103103
pub fn config(&self) -> Arc<Config> {
@@ -146,8 +146,8 @@ impl ModelClient {
146146
}
147147

148148
let auth_manager = self.auth_manager.clone();
149-
let model_family = self.get_model_family();
150-
let instructions = prompt.get_full_instructions(&model_family).into_owned();
149+
let model_info = self.get_model_info();
150+
let instructions = prompt.get_full_instructions(&model_info).into_owned();
151151
let tools_json = create_tools_json_for_chat_completions_api(&prompt.tools)?;
152152
let api_prompt = build_api_prompt(prompt, instructions, tools_json);
153153
let conversation_id = self.conversation_id.to_string();
@@ -200,13 +200,14 @@ impl ModelClient {
200200
}
201201

202202
let auth_manager = self.auth_manager.clone();
203-
let model_family = self.get_model_family();
204-
let instructions = prompt.get_full_instructions(&model_family).into_owned();
203+
let model_info = self.get_model_info();
204+
let instructions = prompt.get_full_instructions(&model_info).into_owned();
205205
let tools_json: Vec<Value> = create_tools_json_for_responses_api(&prompt.tools)?;
206206

207-
let reasoning = if model_family.supports_reasoning_summaries {
207+
let default_reasoning_effort = model_info.default_reasoning_level;
208+
let reasoning = if model_info.supports_reasoning_summaries {
208209
Some(Reasoning {
209-
effort: self.effort.or(model_family.default_reasoning_effort),
210+
effort: self.effort.or(default_reasoning_effort),
210211
summary: if self.summary == ReasoningSummaryConfig::None {
211212
None
212213
} else {
@@ -223,15 +224,13 @@ impl ModelClient {
223224
vec![]
224225
};
225226

226-
let verbosity = if model_family.support_verbosity {
227-
self.config
228-
.model_verbosity
229-
.or(model_family.default_verbosity)
227+
let verbosity = if model_info.support_verbosity {
228+
self.config.model_verbosity.or(model_info.default_verbosity)
230229
} else {
231230
if self.config.model_verbosity.is_some() {
232231
warn!(
233232
"model_verbosity is set but ignored as the model does not support verbosity: {}",
234-
model_family.family
233+
model_info.slug
235234
);
236235
}
237236
None
@@ -298,12 +297,11 @@ impl ModelClient {
298297

299298
/// Returns the currently configured model slug.
300299
pub fn get_model(&self) -> String {
301-
self.get_model_family().get_model_slug().to_string()
300+
self.model_info.slug.clone()
302301
}
303302

304-
/// Returns the currently configured model family.
305-
pub fn get_model_family(&self) -> ModelFamily {
306-
self.model_family.clone()
303+
pub fn get_model_info(&self) -> ModelInfo {
304+
self.model_info.clone()
307305
}
308306

309307
/// Returns the current reasoning effort setting.
@@ -340,7 +338,7 @@ impl ModelClient {
340338
.with_telemetry(Some(request_telemetry));
341339

342340
let instructions = prompt
343-
.get_full_instructions(&self.get_model_family())
341+
.get_full_instructions(&self.get_model_info())
344342
.into_owned();
345343
let payload = ApiCompactionInput {
346344
model: &self.get_model(),

codex-rs/core/src/client_common.rs

Lines changed: 19 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
use crate::client_common::tools::ToolSpec;
22
use crate::error::Result;
3-
use crate::models_manager::model_family::ModelFamily;
43
pub use codex_api::common::ResponseEvent;
5-
use codex_apply_patch::APPLY_PATCH_TOOL_INSTRUCTIONS;
64
use codex_protocol::models::ResponseItem;
5+
use codex_protocol::openai_models::ModelInfo;
76
use futures::Stream;
87
use serde::Deserialize;
98
use serde_json::Value;
109
use std::borrow::Cow;
1110
use std::collections::HashSet;
12-
use std::ops::Deref;
1311
use std::pin::Pin;
1412
use std::task::Context;
1513
use std::task::Poll;
@@ -44,28 +42,12 @@ pub struct Prompt {
4442
}
4543

4644
impl Prompt {
47-
pub(crate) fn get_full_instructions<'a>(&'a self, model: &'a ModelFamily) -> Cow<'a, str> {
48-
let base = self
49-
.base_instructions_override
50-
.as_deref()
51-
.unwrap_or(model.base_instructions.deref());
52-
// When there are no custom instructions, add apply_patch_tool_instructions if:
53-
// - the model needs special instructions (4.1)
54-
// AND
55-
// - there is no apply_patch tool present
56-
let is_apply_patch_tool_present = self.tools.iter().any(|tool| match tool {
57-
ToolSpec::Function(f) => f.name == "apply_patch",
58-
ToolSpec::Freeform(f) => f.name == "apply_patch",
59-
_ => false,
60-
});
61-
if self.base_instructions_override.is_none()
62-
&& model.needs_special_apply_patch_instructions
63-
&& !is_apply_patch_tool_present
64-
{
65-
Cow::Owned(format!("{base}\n{APPLY_PATCH_TOOL_INSTRUCTIONS}"))
66-
} else {
67-
Cow::Borrowed(base)
68-
}
45+
pub(crate) fn get_full_instructions<'a>(&'a self, model: &'a ModelInfo) -> Cow<'a, str> {
46+
Cow::Borrowed(
47+
self.base_instructions_override
48+
.as_deref()
49+
.unwrap_or(model.base_instructions.as_str()),
50+
)
6951
}
7052

7153
pub(crate) fn get_formatted_input(&self) -> Vec<ResponseItem> {
@@ -277,6 +259,8 @@ mod tests {
277259
let prompt = Prompt {
278260
..Default::default()
279261
};
262+
let prompt_with_apply_patch_instructions =
263+
include_str!("../prompt_with_apply_patch_instructions.md");
280264
let test_cases = vec![
281265
InstructionsTestCase {
282266
slug: "gpt-3.5",
@@ -317,19 +301,16 @@ mod tests {
317301
];
318302
for test_case in test_cases {
319303
let config = test_config();
320-
let model_family =
321-
ModelsManager::construct_model_family_offline(test_case.slug, &config);
322-
let expected = if test_case.expects_apply_patch_instructions {
323-
format!(
324-
"{}\n{}",
325-
model_family.clone().base_instructions,
326-
APPLY_PATCH_TOOL_INSTRUCTIONS
327-
)
328-
} else {
329-
model_family.clone().base_instructions
330-
};
331-
332-
let full = prompt.get_full_instructions(&model_family);
304+
let model_info = ModelsManager::construct_model_info_offline(test_case.slug, &config);
305+
if test_case.expects_apply_patch_instructions {
306+
assert_eq!(
307+
model_info.base_instructions.as_str(),
308+
prompt_with_apply_patch_instructions
309+
);
310+
}
311+
312+
let expected = model_info.base_instructions.as_str();
313+
let full = prompt.get_full_instructions(&model_info);
333314
assert_eq!(full, expected);
334315
}
335316
}

0 commit comments

Comments
 (0)