valknar/llmx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: always send full instructions when using the Responses API (#1207)

Browse Source 
This fixes a longstanding error in the Rust CLI where `codex.rs`
contained an errant `is_first_turn` check that would exclude the user
instructions for subsequent "turns" of a conversation when using the
responses API (i.e., when `previous_response_id` existed).

While here, renames `Prompt.instructions` to `Prompt.user_instructions`
since we now have quite a few levels of instructions floating around.
Also removed an unnecessary use of `clone()` in
`Prompt.get_full_instructions()`.
This commit is contained in:
Michael Bolin
2025-06-03 09:40:19 -07:00
committed by GitHub
parent 6fcc528a43
commit c6fcec55fe
3 changed files with 22 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
codex-rs/core/src/client_common.rs
View File

@@ -25,7 +25,7 @@ pub struct Prompt {
pub prev_id: Option<String>,
/// Optional instructions from the user to amend to the built-in agent
/// instructions.
pub instructions: Option<String>,
pub user_instructions: Option<String>,
/// Whether to store response on server side (disable_response_storage = !store).
pub store: bool,
@@ -37,21 +37,14 @@ pub struct Prompt {
impl Prompt {
pub(crate) fn get_full_instructions(&self, model: &str) -> Cow<str> {
[
Some(Cow::Borrowed(BASE_INSTRUCTIONS)),
self.instructions.as_ref().map(|s| Cow::Owned(s.clone())),
if model.starts_with("gpt-4.1") {
Some(Cow::Borrowed(APPLY_PATCH_TOOL_INSTRUCTIONS))
} else {
None
},
]
.iter()
.filter_map(|s| s.as_ref())
.map(|cow| cow.as_ref())
.collect::<Vec<_>>()
.join("\n")
.into()
let mut sections: Vec<&str> = vec![BASE_INSTRUCTIONS];
if let Some(ref user) = self.user_instructions {
sections.push(user);
}
if model.starts_with("gpt-4.1") {
sections.push(APPLY_PATCH_TOOL_INSTRUCTIONS);
}
Cow::Owned(sections.join("\n"))
}
}

17
codex-rs/core/src/codex.rs
View File

@@ -59,7 +59,7 @@ use crate::models::ReasoningItemReasoningSummary;
use crate::models::ResponseInputItem;
use crate::models::ResponseItem;
use crate::models::ShellToolCallParams;
use crate::project_doc::create_full_instructions;
use crate::project_doc::get_user_instructions;
use crate::protocol::AgentMessageEvent;
use crate::protocol::AgentReasoningEvent;
use crate::protocol::ApplyPatchApprovalRequestEvent;
@@ -104,7 +104,7 @@ impl Codex {
let (tx_sub, rx_sub) = async_channel::bounded(64);
let (tx_event, rx_event) = async_channel::bounded(64);
let instructions = create_full_instructions(&config).await;
let instructions = get_user_instructions(&config).await;
let configure_session = Op::ConfigureSession {
provider: config.model_provider.clone(),
model: config.model.clone(),
@@ -990,9 +990,8 @@ async fn run_turn(
input: Vec<ResponseItem>,
) -> CodexResult<Vec<ProcessedResponseItem>> {
// Decide whether to use server-side storage (previous_response_id) or disable it
let (prev_id, store, is_first_turn) = {
let (prev_id, store) = {
let state = sess.state.lock().unwrap();
let is_first_turn = state.previous_response_id.is_none();
let store = state.zdr_transcript.is_none();
let prev_id = if store {
state.previous_response_id.clone()
@@ -1001,20 +1000,14 @@ async fn run_turn(
// back, but trying to use it results in a 400.
None
};
(prev_id, store, is_first_turn)
};
let instructions = if is_first_turn {
sess.instructions.clone()
} else {
None
(prev_id, store)
};
let extra_tools = sess.mcp_connection_manager.list_all_tools();
let prompt = Prompt {
input,
prev_id,
instructions,
user_instructions: sess.instructions.clone(),
store,
extra_tools,
};

16
codex-rs/core/src/project_doc.rs
View File

@@ -25,7 +25,7 @@ const PROJECT_DOC_SEPARATOR: &str = "\n\n--- project-doc ---\n\n";
/// Combines `Config::instructions` and `AGENTS.md` (if present) into a single
/// string of instructions.
pub(crate) async fn create_full_instructions(config: &Config) -> Option<String> {
pub(crate) async fn get_user_instructions(config: &Config) -> Option<String> {
match find_project_doc(config).await {
Ok(Some(project_doc)) => match &config.instructions {
Some(original_instructions) => Some(format!(
@@ -168,7 +168,7 @@ mod tests {
async fn no_doc_file_returns_none() {
let tmp = tempfile::tempdir().expect("tempdir");
let res = create_full_instructions(&make_config(&tmp, 4096, None)).await;
let res = get_user_instructions(&make_config(&tmp, 4096, None)).await;
assert!(
res.is_none(),
"Expected None when AGENTS.md is absent and no system instructions provided"
@@ -182,7 +182,7 @@ mod tests {
let tmp = tempfile::tempdir().expect("tempdir");
fs::write(tmp.path().join("AGENTS.md"), "hello world").unwrap();
let res = create_full_instructions(&make_config(&tmp, 4096, None))
let res = get_user_instructions(&make_config(&tmp, 4096, None))
.await
.expect("doc expected");
@@ -201,7 +201,7 @@ mod tests {
let huge = "A".repeat(LIMIT * 2); // 2 KiB
fs::write(tmp.path().join("AGENTS.md"), &huge).unwrap();
let res = create_full_instructions(&make_config(&tmp, LIMIT, None))
let res = get_user_instructions(&make_config(&tmp, LIMIT, None))
.await
.expect("doc expected");
@@ -233,7 +233,7 @@ mod tests {
let mut cfg = make_config(&repo, 4096, None);
cfg.cwd = nested;
let res = create_full_instructions(&cfg).await.expect("doc expected");
let res = get_user_instructions(&cfg).await.expect("doc expected");
assert_eq!(res, "root level doc");
}
@@ -243,7 +243,7 @@ mod tests {
let tmp = tempfile::tempdir().expect("tempdir");
fs::write(tmp.path().join("AGENTS.md"), "something").unwrap();
let res = create_full_instructions(&make_config(&tmp, 0, None)).await;
let res = get_user_instructions(&make_config(&tmp, 0, None)).await;
assert!(
res.is_none(),
"With limit 0 the function should return None"
@@ -259,7 +259,7 @@ mod tests {
const INSTRUCTIONS: &str = "base instructions";
let res = create_full_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS)))
let res = get_user_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS)))
.await
.expect("should produce a combined instruction string");
@@ -276,7 +276,7 @@ mod tests {
const INSTRUCTIONS: &str = "some instructions";
let res = create_full_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS))).await;
let res = get_user_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS))).await;
assert_eq!(res, Some(INSTRUCTIONS.to_string()));
}