clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-17 01:32:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

codex: address apply_patch grammar review feedback

Browse Source 
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
starr-openai
2026-05-07 18:28:31 -07:00
parent 07866e78d9
commit 101eabf6af
4 changed files with 66 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
codex-rs/apply-patch/src/streaming_parser.rs
View File

@@ -19,8 +19,6 @@ use ParseError::*;
#[derive(Debug, Default, Clone)]
pub struct StreamingPatchParser {
allow_environment_id: bool,
saw_environment_id: bool,
line_buffer: String,
state: StreamingParserState,
line_number: usize,
@@ -32,11 +30,15 @@ struct StreamingParserState {
hunks: Vec<Hunk>,
}
#[derive(Debug, Default, Clone)]
#[derive(Debug, Clone)]
enum StreamingParserMode {
#[default]
NotStarted,
StartedPatch,
NotStarted {
allow_environment_id: bool,
},
StartedPatch {
allow_environment_id: bool,
saw_environment_id: bool,
},
AddFile,
DeleteFile,
UpdateFile {
@@ -45,9 +47,29 @@ enum StreamingParserMode {
EndedPatch,
}
impl Default for StreamingParserMode {
fn default() -> Self {
Self::NotStarted {
allow_environment_id: false,
}
}
}
impl StreamingPatchParser {
pub fn set_allow_environment_id(&mut self, allow_environment_id: bool) {
self.allow_environment_id = allow_environment_id;
match &mut self.state.mode {
StreamingParserMode::NotStarted {
allow_environment_id: current,
}
| StreamingParserMode::StartedPatch {
allow_environment_id: current,
..
} => *current = allow_environment_id,
StreamingParserMode::AddFile
| StreamingParserMode::DeleteFile
| StreamingParserMode::UpdateFile { .. }
| StreamingParserMode::EndedPatch => {}
}
}
fn ensure_update_hunk_is_not_empty(&self, line: &str) -> Result<(), ParseError> {
@@ -158,18 +180,26 @@ impl StreamingPatchParser {
fn process_line(&mut self, line: &str) -> Result<(), ParseError> {
let trimmed = line.trim();
match self.state.mode.clone() {
StreamingParserMode::NotStarted => {
StreamingParserMode::NotStarted {
allow_environment_id,
} => {
if trimmed == BEGIN_PATCH_MARKER {
self.state.mode = StreamingParserMode::StartedPatch;
self.state.mode = StreamingParserMode::StartedPatch {
allow_environment_id,
saw_environment_id: false,
};
return Ok(());
}
Err(InvalidPatchError(
"The first line of the patch must be '*** Begin Patch'".to_string(),
))
}
StreamingParserMode::StartedPatch => {
if self.allow_environment_id
&& !self.saw_environment_id
StreamingParserMode::StartedPatch {
allow_environment_id,
saw_environment_id,
} => {
if allow_environment_id
&& !saw_environment_id
&& let Some(environment_id) = trimmed.strip_prefix(ENVIRONMENT_ID_MARKER)
{
if environment_id.trim().is_empty() {
@@ -177,7 +207,10 @@ impl StreamingPatchParser {
"Environment ID cannot be empty".to_string(),
));
}
self.saw_environment_id = true;
self.state.mode = StreamingParserMode::StartedPatch {
allow_environment_id,
saw_environment_id: true,
};
return Ok(());
}
if self.handle_hunk_headers_and_end_patch(trimmed)? {

3
codex-rs/core/src/tools/handlers/apply_patch.lark
View File

@@ -1,6 +1,5 @@
start: begin_patch environment_id? hunk+ end_patch
start: begin_patch hunk+ end_patch
begin_patch: "*** Begin Patch" LF
environment_id: "*** Environment ID: " /(.+)/ LF
end_patch: "*** End Patch" LF?
hunk: add_hunk | delete_hunk | update_hunk

12
codex-rs/core/src/tools/handlers/apply_patch_spec.rs
View File

@@ -10,6 +10,7 @@ use std::collections::BTreeMap;
const APPLY_PATCH_LARK_GRAMMAR: &str = include_str!("apply_patch.lark");
const APPLY_PATCH_WITH_ENV_START: &str = "start: begin_patch environment_id? hunk+ end_patch";
const APPLY_PATCH_STRICT_START: &str = "start: begin_patch hunk+ end_patch";
const APPLY_PATCH_BEGIN_PATCH_RULE: &str = "begin_patch: \"*** Begin Patch\" LF\n";
const APPLY_PATCH_ENVIRONMENT_RULE: &str = "environment_id: \"*** Environment ID: \" /(.+)/ LF\n";
const APPLY_PATCH_JSON_TOOL_DESCRIPTION: &str = r#"Use the `apply_patch` tool to edit files.
@@ -109,11 +110,14 @@ pub fn create_apply_patch_freeform_tool(multi_environment: bool) -> ToolSpec {
r#type: "grammar".to_string(),
syntax: "lark".to_string(),
definition: if multi_environment {
APPLY_PATCH_LARK_GRAMMAR.to_string()
} else {
APPLY_PATCH_LARK_GRAMMAR
.replace(APPLY_PATCH_WITH_ENV_START, APPLY_PATCH_STRICT_START)
.replace(APPLY_PATCH_ENVIRONMENT_RULE, "")
.replace(APPLY_PATCH_STRICT_START, APPLY_PATCH_WITH_ENV_START)
.replace(
APPLY_PATCH_BEGIN_PATCH_RULE,
&format!("{APPLY_PATCH_BEGIN_PATCH_RULE}{APPLY_PATCH_ENVIRONMENT_RULE}"),
)
} else {
APPLY_PATCH_LARK_GRAMMAR.to_string()
},
},
})

16
codex-rs/core/src/tools/handlers/apply_patch_spec_tests.rs
View File

@@ -3,6 +3,15 @@ use codex_tools::JsonSchema;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
fn multi_environment_apply_patch_grammar() -> String {
APPLY_PATCH_LARK_GRAMMAR
.replace(APPLY_PATCH_STRICT_START, APPLY_PATCH_WITH_ENV_START)
.replace(
APPLY_PATCH_BEGIN_PATCH_RULE,
&format!("{APPLY_PATCH_BEGIN_PATCH_RULE}{APPLY_PATCH_ENVIRONMENT_RULE}"),
)
}
#[test]
fn create_apply_patch_freeform_tool_matches_expected_spec() {
assert_eq!(
@@ -13,7 +22,7 @@ fn create_apply_patch_freeform_tool_matches_expected_spec() {
format: FreeformToolFormat {
r#type: "grammar".to_string(),
syntax: "lark".to_string(),
definition: APPLY_PATCH_LARK_GRAMMAR.to_string(),
definition: multi_environment_apply_patch_grammar(),
},
})
);
@@ -59,10 +68,7 @@ fn strict_apply_patch_tools_do_not_advertise_multi_environment() {
let ToolSpec::Freeform(freeform) = freeform else {
panic!("expected freeform tool");
};
let expected_strict = APPLY_PATCH_LARK_GRAMMAR
.replace(APPLY_PATCH_WITH_ENV_START, APPLY_PATCH_STRICT_START)
.replace(APPLY_PATCH_ENVIRONMENT_RULE, "");
assert_eq!(freeform.format.definition, expected_strict);
assert_eq!(freeform.format.definition, APPLY_PATCH_LARK_GRAMMAR);
assert_eq!(freeform.description, APPLY_PATCH_FREEFORM_TOOL_DESCRIPTION);
let json_tool = create_apply_patch_json_tool(/*multi_environment*/ false);