Render whole-file patches from change metadata
This commit is contained in:
1 parent
06bf1c8f81
commit
3d1b1e304d
4 files changed
+57
-18
No files matched your search
@@ -15,7 +15,9 @@ use std::sync::{Arc, Mutex};
|
|||||||
|
|
||||||
use serde_json::{Value, json};
|
use serde_json::{Value, json};
|
||||||
|
|
||||||
use super::super::driver::{Event, QuestionOption, SessionStatus, context_tokens, patch_start};
|
use super::super::driver::{
|
||||||
|
Event, QuestionOption, SessionStatus, context_tokens, patch_start, prefixed_lines,
|
||||||
|
};
|
||||||
use super::super::subagent::Subagents;
|
use super::super::subagent::Subagents;
|
||||||
|
|
||||||
/// Whether this line is the CLI opening a fresh model call.
|
/// Whether this line is the CLI opening a fresh model call.
|
||||||
@@ -1169,18 +1171,6 @@ fn replacement_diff(input: &Value) -> String {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn prefixed_lines(prefix: char, text: &str) -> String {
|
|
||||||
text.split_inclusive('\n')
|
|
||||||
.map(|line| {
|
|
||||||
if line.ends_with('\n') {
|
|
||||||
format!("{prefix}{line}")
|
|
||||||
} else {
|
|
||||||
format!("{prefix}{line}\n")
|
|
||||||
}
|
|
||||||
})
|
|
||||||
.collect()
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Whether this line is a subagent's *own* turn ending -- the only thing
|
/// Whether this line is a subagent's *own* turn ending -- the only thing
|
||||||
/// that does, per `SUBAGENTS.md`: not the parent's `tool_result`, which for
|
/// that does, per `SUBAGENTS.md`: not the parent's `tool_result`, which for
|
||||||
/// a background Task arrives at launch rather than at completion.
|
/// a background Task arrives at launch rather than at completion.
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ use std::sync::Arc;
|
|||||||
|
|
||||||
use serde_json::{Value, json};
|
use serde_json::{Value, json};
|
||||||
|
|
||||||
use super::super::driver::{Event, SessionStatus, patch_start};
|
use super::super::driver::{Event, SessionStatus, patch_start, prefixed_lines};
|
||||||
use super::super::subagent::Subagents;
|
use super::super::subagent::Subagents;
|
||||||
|
|
||||||
#[derive(Default)]
|
#[derive(Default)]
|
||||||
@@ -741,12 +741,17 @@ fn file_change_diff(item: &Value) -> String {
|
|||||||
Some(Value::Object(changes)) => changes
|
Some(Value::Object(changes)) => changes
|
||||||
.iter()
|
.iter()
|
||||||
.map(|(path, change)| {
|
.map(|(path, change)| {
|
||||||
|
let kind = change.get("type").and_then(Value::as_str);
|
||||||
render_file_diff(
|
render_file_diff(
|
||||||
path,
|
path,
|
||||||
change.get("type").and_then(Value::as_str),
|
kind,
|
||||||
change.get("move_path").and_then(Value::as_str),
|
change.get("move_path").and_then(Value::as_str),
|
||||||
change
|
change
|
||||||
.get("unified_diff")
|
.get(if matches!(kind, Some("add" | "delete")) {
|
||||||
|
"content"
|
||||||
|
} else {
|
||||||
|
"unified_diff"
|
||||||
|
})
|
||||||
.and_then(Value::as_str)
|
.and_then(Value::as_str)
|
||||||
.unwrap_or_default(),
|
.unwrap_or_default(),
|
||||||
)
|
)
|
||||||
@@ -759,7 +764,9 @@ fn file_change_diff(item: &Value) -> String {
|
|||||||
|
|
||||||
fn render_file_diff(path: &str, kind: Option<&str>, moved: Option<&str>, body: &str) -> String {
|
fn render_file_diff(path: &str, kind: Option<&str>, moved: Option<&str>, body: &str) -> String {
|
||||||
// Some protocol revisions carry complete unified diffs and some carry only their hunks.
|
// Some protocol revisions carry complete unified diffs and some carry only their hunks.
|
||||||
if body.starts_with("--- ") || body.starts_with("diff --git ") {
|
if !matches!(kind, Some("add" | "delete"))
|
||||||
|
&& (body.starts_with("--- ") || body.starts_with("diff --git "))
|
||||||
|
{
|
||||||
return body.to_string();
|
return body.to_string();
|
||||||
}
|
}
|
||||||
let from = if kind == Some("add") {
|
let from = if kind == Some("add") {
|
||||||
@@ -772,6 +779,14 @@ fn render_file_diff(path: &str, kind: Option<&str>, moved: Option<&str>, body: &
|
|||||||
} else {
|
} else {
|
||||||
moved.unwrap_or(path)
|
moved.unwrap_or(path)
|
||||||
};
|
};
|
||||||
|
let body = match kind {
|
||||||
|
// Add and delete carry file content, not a unified diff. Prefixing it here makes the
|
||||||
|
// protocol's change kind authoritative: a '-' that belongs to a Markdown bullet can never
|
||||||
|
// be mistaken for diff syntax.
|
||||||
|
Some("add") => prefixed_lines('+', body),
|
||||||
|
Some("delete") => prefixed_lines('-', body),
|
||||||
|
_ => body.to_string(),
|
||||||
|
};
|
||||||
format!("--- {from}\n+++ {to}\n{body}")
|
format!("--- {from}\n+++ {to}\n{body}")
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1069,6 +1084,28 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn whole_file_changes_get_markers_from_their_kind_not_their_content() {
|
||||||
|
let item = line(
|
||||||
|
r#"{"changes":[{"path":"added.md","kind":{"type":"add"},"diff":"--- a rule\n- a bullet\nplain\n"},{"path":"deleted.md","kind":{"type":"delete"},"diff":"- another bullet\nplain\n"}]}"#,
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
file_change_diff(&item),
|
||||||
|
"--- /dev/null\n+++ added.md\n+--- a rule\n+- a bullet\n+plain\n\n--- deleted.md\n+++ /dev/null\n-- another bullet\n-plain\n"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn legacy_whole_file_changes_show_the_content_they_changed() {
|
||||||
|
let item = line(
|
||||||
|
r#"{"changes":{"added.md":{"type":"add","content":"new\n"},"deleted.md":{"type":"delete","content":"old\n"}}}"#,
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
file_change_diff(&item),
|
||||||
|
"--- /dev/null\n+++ added.md\n+new\n\n--- deleted.md\n+++ /dev/null\n-old\n"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn native_app_server_completion_corrects_provisional_streaming() {
|
fn native_app_server_completion_corrects_provisional_streaming() {
|
||||||
let mut translator = Translator::default();
|
let mut translator = Translator::default();
|
||||||
|
|||||||
@@ -470,6 +470,18 @@ pub(super) fn patch_start(id: String, diff: String) -> Event {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(super) fn prefixed_lines(prefix: char, text: &str) -> String {
|
||||||
|
text.split_inclusive('\n')
|
||||||
|
.map(|line| {
|
||||||
|
if line.ends_with('\n') {
|
||||||
|
format!("{prefix}{line}")
|
||||||
|
} else {
|
||||||
|
format!("{prefix}{line}\n")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
/// How much the model was holding, from the three figures a turn reports:
|
/// How much the model was holding, from the three figures a turn reports:
|
||||||
/// the input side only, prompt plus both cache figures. A cached token is
|
/// the input side only, prompt plus both cache figures. A cached token is
|
||||||
/// cheaper but it is still one the model was given; output is what the turn
|
/// cheaper but it is still one the model was given; output is what the turn
|
||||||
|
|||||||
@@ -820,7 +820,7 @@ impl EchoDriver {
|
|||||||
let id = format!("p-{}", super::random_hex());
|
let id = format!("p-{}", super::random_hex());
|
||||||
send(patch_start(
|
send(patch_start(
|
||||||
id.clone(),
|
id.clone(),
|
||||||
"--- src/example.rs\n+++ src/example.rs\n@@ -1,3 +1,3 @@\n fn answer() -> u8 {\n- 41\n+ 42\n }\n"
|
"--- src/example.rs\n+++ src/example.rs\n@@ -1,3 +1,3 @@\n fn answer() -> u8 {\n- 41\n+ 42\n }\n\n--- /dev/null\n+++ notes.md\n+- added bullet\n+added prose\n\n--- old-notes.md\n+++ /dev/null\n-- removed bullet\n-removed prose\n"
|
||||||
.to_string(),
|
.to_string(),
|
||||||
));
|
));
|
||||||
tokio::time::sleep(DELTA_DELAY).await;
|
tokio::time::sleep(DELTA_DELAY).await;
|
||||||
|
|||||||
Reference in new issue
Block a user