diff --git a/crates/diffr-core/src/config/store.rs b/crates/diffr-core/src/config/store.rs index 9961f1866..575beb81d 100644 --- a/crates/diffr-core/src/config/store.rs +++ b/crates/diffr-core/src/config/store.rs @@ -348,10 +348,12 @@ mod tests { shown["plugins"]["bundled"]["deleted-bodies"], serde_json::json!({"enabled": true, "min_lines": 30}) ); - assert!(shown["plugins"]["bundled"]["summarize"]["system_prompt"] - .as_str() - .unwrap() - .starts_with("For each listed fold")); + assert_eq!( + shown["plugins"]["bundled"]["summarize"]["system_prompt"], + crate::plugin::builtin::manifest("summarize") + .unwrap() + .options["system_prompt"]["default"] + ); let text = toml::to_string_pretty(&redacted(&config, false)).unwrap(); assert!(text.contains("[plugins.bundled.summarize]"), "{text}"); assert!(text.contains("system_prompt = "), "{text}"); diff --git a/crates/diffr-core/src/plugin/config.rs b/crates/diffr-core/src/plugin/config.rs index 47cb30dd6..a3fb21ec3 100644 --- a/crates/diffr-core/src/plugin/config.rs +++ b/crates/diffr-core/src/plugin/config.rs @@ -762,39 +762,6 @@ impl PluginsConfig { mod tests { use crate::config::Config; - #[test] - fn the_prompt_description_links_to_its_default_in_the_source() { - let source = include_str!("../../../../plugins/shape/summarize/plugin.toml"); - let lines: Vec<&str> = source.lines().collect(); - let table = lines - .iter() - .position(|line| *line == "[options.system_prompt]") - .unwrap(); - let start = table - + lines[table..] - .iter() - .position(|line| line.starts_with("default = \"\"\"")) - .unwrap(); - let end = start - + lines[start..] - .iter() - .position(|line| line.ends_with("\"\"\"")) - .unwrap(); - let manifest = super::builtin::manifest("summarize").unwrap(); - let description = manifest.options["system_prompt"]["description"] - .as_str() - .unwrap(); - let link = format!( - "https://github.com/devdotfast/diffr/blob/main/plugins/shape/summarize/plugin.toml#L{}-L{}", - start + 1, - end + 1 - ); - assert!( - description.contains(&link), - "{description}\nexpected {link}" - ); - } - #[test] fn defaults_can_follow_another_option() { let summarize = |toml: &str| { @@ -983,10 +950,10 @@ mod tests { let summarize = &plugins["bundled"]["properties"]["summarize"]["properties"]; // A text setting, so settings screens let users edit the prompt. assert!(summarize["system_prompt"].get("x-settings").is_none()); - assert!(summarize["system_prompt"]["default"] - .as_str() - .unwrap() - .starts_with("For each listed fold, rewrite that function body")); + assert_eq!( + summarize["system_prompt"]["default"], + super::builtin::manifest("summarize").unwrap().options["system_prompt"]["default"] + ); let keys: Vec<&String> = plugins.as_object().unwrap().keys().collect(); assert_eq!(keys, ["order", "bundled", "external"]); } @@ -1084,7 +1051,7 @@ mod tests { assert_eq!(classifier["hide_deleted"], false); assert_eq!( classifier["hide"], - serde_json::json!(["generated", "vendored", "test"]) + serde_json::json!(["generated", "vendored"]) ); let summarize = &config.plugins.entries["bundled.summarize"]; assert_eq!( diff --git a/crates/diffr-core/src/plugin/queries.rs b/crates/diffr-core/src/plugin/queries.rs index cf5869f3b..d6907d9c9 100644 --- a/crates/diffr-core/src/plugin/queries.rs +++ b/crates/diffr-core/src/plugin/queries.rs @@ -276,6 +276,7 @@ mod tests { "builtin:shared/queries/rust.scm", "builtin:shared/queries/rust-docstrings.scm", "builtin:deleted-bodies/queries/rust.scm", + "builtin:shared/queries/rust-tests.scm", "builtin:test-bodies/queries/rust.scm", "builtin:removed-runs/queries/rust.scm", "builtin:context/queries/rust.scm", diff --git a/plugins/classify/plugin.toml b/plugins/classify/plugin.toml index 7020cd1dd..cd3727d14 100644 --- a/plugins/classify/plugin.toml +++ b/plugins/classify/plugin.toml @@ -1,13 +1,13 @@ name = "classify" title = "Classify" -description = "Tag each changed file as generated, vendored, docs or test from GitHub Linguist's rules, diffr's test-path rules and git attributes." +description = "Tag each changed file as generated, vendored, docs, test, integration or e2e from GitHub Linguist's rules, diffr's test-path rules and git attributes." [options.hide] type = "array" items = { type = "string" } title = "Tags that hide a file" description = "A file carrying any of these tags starts hidden: it is diffed by line, no shape plugin runs on it, and it is shown collapsed. The first listed tag a file carries names the reason." -default = ["generated", "vendored", "test"] +default = ["generated", "vendored"] [options.hide_deleted] type = "boolean" diff --git a/plugins/classify/plugin.wasm b/plugins/classify/plugin.wasm index 91a8d2db0..b1543a204 100644 Binary files a/plugins/classify/plugin.wasm and b/plugins/classify/plugin.wasm differ diff --git a/plugins/classify/rust/src/lib.rs b/plugins/classify/rust/src/lib.rs index 198b5c115..f025456ea 100644 --- a/plugins/classify/rust/src/lib.rs +++ b/plugins/classify/rust/src/lib.rs @@ -1,5 +1,5 @@ //! The classifier: tags each changed file `generated`, `vendored`, `docs`, -//! `test`, and whatever a repository adds. +//! `test`, `integration`, `e2e`, and whatever a repository adds. //! //! Bundled rules come first and have the lowest precedence: GitHub //! Linguist's `generated.rb` (ported in [`generated`]), its `vendor.yml` and @@ -26,6 +26,8 @@ const GENERATED: &str = "generated"; const VENDORED: &str = "vendored"; const DOCS: &str = "docs"; const TEST: &str = "test"; +const INTEGRATION: &str = "integration"; +const E2E: &str = "e2e"; /// How much of a file the content rules read. const PREFIX_BYTES: usize = 8 * 1024; @@ -52,7 +54,15 @@ static DOCUMENTATION: LazyLock = LazyLock::new(|| { }); /// Directory names, anywhere in the path, that hold tests. -const TEST_DIRS: &[&str] = &["tests", "test", "__tests__", "spec"]; +const TEST_DIRS: &[&str] = &[ + "tests", + "test", + "__tests__", + "spec", + "e2e", + "cypress", + "playwright", +]; fn is_test(path: &str) -> bool { let mut parts = path.split('/').filter(|part| !part.is_empty()); @@ -69,6 +79,49 @@ fn is_test(path: &str) -> bool { || name.contains(".test.") || name.contains(".spec.") || name.contains(".integration.") + || name.contains(".e2e.") +} + +/// `integration` or `e2e` for a test file whose path says which. +fn kind_by_path(path: &str) -> Option<&'static str> { + let segments: Vec<&str> = path.split('/').filter(|part| !part.is_empty()).collect(); + let parts = || { + segments + .iter() + .flat_map(|segment| segment.split(['.', '_', '-'])) + }; + if parts().any(|part| part == "e2e") + || segments + .iter() + .any(|segment| ["cypress", "playwright"].contains(segment)) + { + return Some(E2E); + } + if parts().any(|part| part == "integration") { + return Some(INTEGRATION); + } + let crate_tests = path.ends_with(".rs") + && segments[..segments.len().saturating_sub(1)] + .iter() + .take_while(|segment| **segment != "src") + .any(|segment| *segment == "tests"); + crate_tests.then_some(INTEGRATION) +} + +/// `integration` or `e2e` for a test file whose first lines say which. +fn kind_by_content(text: &str) -> Option<&'static str> { + if text.contains("@playwright/test") { + return Some(E2E); + } + text.lines() + .map(str::trim) + .filter(|line| line.starts_with("//go:build") || line.starts_with("pytestmark")) + .flat_map(|line| line.split(|c: char| !c.is_ascii_alphanumeric())) + .find_map(|word| match word { + "e2e" => Some(E2E), + "integration" => Some(INTEGRATION), + _ => None, + }) } /// Every bundled rule that looks at the repository-relative path alone. @@ -85,6 +138,7 @@ fn from_path(path: &str) -> BTreeSet<&'static str> { } if is_test(path) { tags.insert(TEST); + tags.extend(kind_by_path(path)); } tags } @@ -317,6 +371,11 @@ impl GuestClassifier for Classify { } } } + if bundled.contains(TEST) && !bundled.contains(INTEGRATION) && !bundled.contains(E2E) { + if let Some((bytes, _)) = prefix(side)? { + bundled.extend(kind_by_content(&String::from_utf8_lossy(&bytes))); + } + } let tags: BTreeSet = attributes.resolve(bundled).into_iter().collect(); Ok(Classification { hidden: self.hidden(&file, &tags), diff --git a/plugins/shape/context/plugin.wasm b/plugins/shape/context/plugin.wasm index 2a36e1390..de86807c7 100644 Binary files a/plugins/shape/context/plugin.wasm and b/plugins/shape/context/plugin.wasm differ diff --git a/plugins/shape/removed-runs/plugin.wasm b/plugins/shape/removed-runs/plugin.wasm index baccb748b..c8b43e8d1 100644 Binary files a/plugins/shape/removed-runs/plugin.wasm and b/plugins/shape/removed-runs/plugin.wasm differ diff --git a/plugins/shape/shared/queries/go-tests.scm b/plugins/shape/shared/queries/go-tests.scm new file mode 100644 index 000000000..5b231c998 --- /dev/null +++ b/plugins/shape/shared/queries/go-tests.scm @@ -0,0 +1,9 @@ +; Test bodies, tagged for every plugin that treats tests specially. +((function_declaration name: (identifier) @_name body: (block "{" @fold.open (statement_list . [ + (labeled_statement (label_name) . (_) @fold.indent) + (_) @fold.indent + ]) "}" @fold.close) @fold) + (#not-match? @fold.indent "^[A-Za-z_][A-Za-z0-9_]*:\\s*\n") + (#match? @_name "^Test") + (#set! tag "summarize:test") + (#set! tag "test-bodies:test")) diff --git a/plugins/shape/shared/queries/javascript-tests.scm b/plugins/shape/shared/queries/javascript-tests.scm new file mode 100644 index 000000000..371fc393b --- /dev/null +++ b/plugins/shape/shared/queries/javascript-tests.scm @@ -0,0 +1,10 @@ +; Test bodies, tagged for every plugin that treats tests specially. +((call_expression + function: (identifier) @_name + arguments: (arguments [ + (arrow_function body: (statement_block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) + (function_expression body: (statement_block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) + ])) + (#match? @_name "^(it|test)$") + (#set! tag "summarize:test") + (#set! tag "test-bodies:test")) diff --git a/plugins/shape/shared/queries/python-tests.scm b/plugins/shape/shared/queries/python-tests.scm new file mode 100644 index 000000000..faf2dba60 --- /dev/null +++ b/plugins/shape/shared/queries/python-tests.scm @@ -0,0 +1,5 @@ +; Test bodies, tagged for every plugin that treats tests specially. +((function_definition name: (identifier) @_name ":" @fold.open body: (block . (_) @fold.indent) @fold) + (#match? @_name "^test_") + (#set! tag "summarize:test") + (#set! tag "test-bodies:test")) diff --git a/plugins/shape/shared/queries/rust-tests.scm b/plugins/shape/shared/queries/rust-tests.scm new file mode 100644 index 000000000..2b2b9e991 --- /dev/null +++ b/plugins/shape/shared/queries/rust-tests.scm @@ -0,0 +1,5 @@ +; Test bodies, tagged for every plugin that treats tests specially. +((function_item attributes: (attributes (attribute_item) @_attribute) body: (block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) + (#match? @_attribute "test") + (#set! tag "summarize:test") + (#set! tag "test-bodies:test")) diff --git a/plugins/shape/summarize/plugin.toml b/plugins/shape/summarize/plugin.toml index f4bbf20fe..61bb773d7 100644 --- a/plugins/shape/summarize/plugin.toml +++ b/plugins/shape/summarize/plugin.toml @@ -6,7 +6,7 @@ description = "Pseudocode summaries for large new function bodies and tests." # startup when a plugin that is on cannot be made. [enabled] title = "Summarize functions and tests" -description = "Summarize large new function bodies and right-side tests as pseudocode. Needs an API key: `api_key`, or the provider's variable in the environment: `GEMINI_API_KEY` or `GOOGLE_API_KEY`, `OPENAI_API_KEY`, or `ANTHROPIC_API_KEY`." +description = "Summarize large new function bodies and new tests as pseudocode. Needs an API key: `api_key`, or the provider's variable in the environment: `GEMINI_API_KEY` or `GOOGLE_API_KEY`, `OPENAI_API_KEY`, or `ANTHROPIC_API_KEY`." default = false [options.provider] @@ -62,19 +62,71 @@ default = 3 [options.system_prompt] type = "string" title = "System prompt" -description = "The system instruction sent with every request; diffr builds each request's message from one body's numbered lines and its documentation. Unset uses the default: https://github.com/devdotfast/diffr/blob/main/plugins/shape/summarize/plugin.toml#L66-L72" +description = "The system instruction sent with every request; diffr builds each request's message from one body's numbered lines." default = """\ -For each listed fold, rewrite that function body as short \ -pseudocode. Keep the names. No prose, no comments, no code fences. Use as few lines as \ -possible: about one pseudocode line per five source lines, and never more than a third \ -of the body's lines. When a fold lists a doc, also set "summary" to one sentence copied \ -verbatim from that doc; otherwise leave it empty. Answer with a JSON object whose \ -"summaries" array holds one {"id", "summary", "pseudocode"} object per fold.""" +For each block of code, rewrite it as pseudocode a reviewer can \ +read instead of the code. First decide what the code does. If uncertain, \ +pick the option with the shortest output / explanation. + +Examples below are delimited with code fences, you should not add those to your output. + +A pure algorithm: plain-word steps, nested two spaces per level, for example: +```pseudo +if content is unchanged + return cached result +write new content +return fresh result +``` + +Control flow (mostly calls to other functions, in order): a call tree of the callees, \ +each call's own calls indented under it, for example: +```pseudo +createSession + persistPrompt + launchAgent +navigateToSession +``` + +You can also create notional functions to make the pseudocode read easier in control-flow; +just add a note that you've done so, e.g.: \ +```pseudo +loadSettings +reconcileSessions # summarization; not literal code + resumeRunningSessions + dropExpiredSessions +saveSettings +``` + +UI (renders components or markup): a component tree, with the hooks and state that \ +matter under their component, and a component's module in parentheses when it comes \ +from elsewhere, for example: +```pseudo + + useSessionEvents() + (packages/ui) +``` + +Break a test into setup, action being tested, and assertion phases; summarize setup with the \ +methods above, e.g. control-flow pseudocode. For example: +```pseudo +Setup: + createSession + persistPrompt + stopAgent(session) +Action: + resumed := resumeSession(session.id) +Assertions: + assertEq(resumed.id, session.id) + assertEq(resumed.prompt, session.prompt) + assert agent is running +``` + +Reply with only pseudocode in the format above (code within the 'pseudo' blocks), nothing else.""" [options.tests] type = "boolean" title = "Summarize tests" -description = "Summarize right-side test bodies, including modified and unchanged tests in diffed files." +description = "Summarize new test bodies." default = true [options.test_min_lines] diff --git a/plugins/shape/summarize/plugin.wasm b/plugins/shape/summarize/plugin.wasm index 03f5ea5df..aeb86c431 100644 Binary files a/plugins/shape/summarize/plugin.wasm and b/plugins/shape/summarize/plugin.wasm differ diff --git a/plugins/shape/summarize/queries/go.scm b/plugins/shape/summarize/queries/go.scm index 09118c205..957c399b5 100644 --- a/plugins/shape/summarize/queries/go.scm +++ b/plugins/shape/summarize/queries/go.scm @@ -1,4 +1,4 @@ -; inherits: builtin:shared/queries/go.scm, builtin:shared/queries/go-docstrings.scm +; inherits: builtin:shared/queries/go.scm, builtin:shared/queries/go-docstrings.scm, builtin:shared/queries/go-tests.scm ((function_declaration body: (block "{" @fold.open (statement_list . [ (labeled_statement (label_name) . (_) @fold.indent) (_) @fold.indent @@ -11,10 +11,3 @@ ]) "}" @fold.close) @fold) (#not-match? @fold.indent "^[A-Za-z_][A-Za-z0-9_]*:\\s*\n") (#set! tag "summarize:function")) -((function_declaration name: (identifier) @_name body: (block "{" @fold.open (statement_list . [ - (labeled_statement (label_name) . (_) @fold.indent) - (_) @fold.indent - ]) "}" @fold.close) @fold) - (#not-match? @fold.indent "^[A-Za-z_][A-Za-z0-9_]*:\\s*\n") - (#match? @_name "^Test") - (#set! tag "summarize:test")) diff --git a/plugins/shape/summarize/queries/javascript.scm b/plugins/shape/summarize/queries/javascript.scm index 2d6bb18c0..fae80e401 100644 --- a/plugins/shape/summarize/queries/javascript.scm +++ b/plugins/shape/summarize/queries/javascript.scm @@ -1,4 +1,4 @@ -; inherits: builtin:shared/queries/javascript.scm, builtin:shared/queries/javascript-docstrings.scm +; inherits: builtin:shared/queries/javascript.scm, builtin:shared/queries/javascript-docstrings.scm, builtin:shared/queries/javascript-tests.scm ((function_declaration body: (statement_block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) (#set! tag "summarize:function")) ((generator_function_declaration body: (statement_block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) @@ -12,11 +12,3 @@ (function_expression body: (statement_block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) ]) (#set! tag "summarize:function")) -((call_expression - function: (identifier) @_name - arguments: (arguments [ - (arrow_function body: (statement_block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) - (function_expression body: (statement_block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) - ])) - (#match? @_name "^(it|test)$") - (#set! tag "summarize:test")) diff --git a/plugins/shape/summarize/queries/python.scm b/plugins/shape/summarize/queries/python.scm index 701c1bda7..6f01ea23a 100644 --- a/plugins/shape/summarize/queries/python.scm +++ b/plugins/shape/summarize/queries/python.scm @@ -1,6 +1,3 @@ -; inherits: builtin:shared/queries/python.scm, builtin:shared/queries/python-docstrings.scm +; inherits: builtin:shared/queries/python.scm, builtin:shared/queries/python-docstrings.scm, builtin:shared/queries/python-tests.scm ((function_definition ":" @fold.open body: (block . (_) @fold.indent) @fold) (#set! tag "summarize:function")) -((function_definition name: (identifier) @_name ":" @fold.open body: (block . (_) @fold.indent) @fold) - (#match? @_name "^test_") - (#set! tag "summarize:test")) diff --git a/plugins/shape/summarize/queries/rust.scm b/plugins/shape/summarize/queries/rust.scm index c24b2baaa..5960834f1 100644 --- a/plugins/shape/summarize/queries/rust.scm +++ b/plugins/shape/summarize/queries/rust.scm @@ -1,6 +1,3 @@ -; inherits: builtin:shared/queries/rust.scm, builtin:shared/queries/rust-docstrings.scm +; inherits: builtin:shared/queries/rust.scm, builtin:shared/queries/rust-docstrings.scm, builtin:shared/queries/rust-tests.scm ((function_item body: (block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) (#set! tag "summarize:function")) -((function_item attributes: (attributes (attribute_item) @_attribute) body: (block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) - (#match? @_attribute "test") - (#set! tag "summarize:test")) diff --git a/plugins/shape/summarize/rust/src/lib.rs b/plugins/shape/summarize/rust/src/lib.rs index b27b692d7..adb46ddb0 100644 --- a/plugins/shape/summarize/rust/src/lib.rs +++ b/plugins/shape/summarize/rust/src/lib.rs @@ -1,5 +1,5 @@ -//! The summarizer: large new function bodies become short pseudocode, shown -//! in place of the collapsed body. +//! The summarizer: large new function bodies and new tests become short +//! pseudocode, shown in place of the collapsed body. //! //! It needs an API key for most providers: `new` fails without one, naming //! how to set it or turn the plugin off, which is why the bundled @@ -14,7 +14,7 @@ mod provider; pub use provider::{Details, Provider}; /// The plugin's name, and the tags its queries set: a function body, and a -/// test body, which can be summarized independently of whether it is new. +/// test body. const FUNCTION: &str = "summarize:function"; const TEST: &str = "summarize:test"; @@ -43,62 +43,27 @@ pub struct Summarize { endpoint: String, } -/// One fold to summarize: its region id, 1-based inclusive line range, and -/// its documentation text when it has any. -struct Request { - id: u32, - first_line: u32, - last_line: u32, - doc: Option, -} - -#[derive(Deserialize)] -struct Answer { - id: u32, - #[serde(default)] - summary: String, - pseudocode: String, -} - -/// What the model returned for one fold: an optional sentence quoted from -/// its docstring, and the pseudocode. -struct Summary { - quote: Option, - pseudocode: String, -} - impl Summarize { - fn prompt(&self, path: &str, src: &str, fold: &Request) -> String { + /// One request per selected body: its lines, numbered from `first_line` + /// (1-based). Retried on transient failures; any other failure is a + /// run-level failure. + async fn complete( + &self, + path: &str, + src: &str, + first_line: usize, + ) -> anyhow::Result> { let numbered = src .split_terminator('\n') .enumerate() - .map(|(index, line)| format!("{:5} | {line}", fold.first_line as usize + index)) + .map(|(index, line)| format!("{:5} | {line}", first_line + index)) .collect::>() .join("\n"); - let doc = fold - .doc - .as_ref() - .map(|doc| format!("\n doc: {doc}")) - .unwrap_or_default(); - format!( - "File {path}:\n\n{numbered}\n\nFold:\n- fold {}: lines {}-{}{doc}", - fold.id, fold.first_line, fold.last_line - ) - } - - /// One request per selected body, retried on transient failures. Any other - /// failure is a run-level failure. - async fn complete( - &self, - path: &str, - src: &str, - fold: &Request, - ) -> anyhow::Result> { let provider = self.options.provider; let body = provider.body( &self.options.model, &self.options.system_prompt, - &self.prompt(path, src, fold), + &format!("File {path}:\n\n{numbered}"), 800, ); let url = provider.url(&self.endpoint, &self.options.model); @@ -138,76 +103,12 @@ impl Summarize { http::sleep(Duration::from_millis(250 * (1 << attempt.min(6)))).await; } }; - let content = provider + let pseudocode = provider .text(&text) - .ok_or_else(|| failed("no text in the response".to_owned()))?; - let answers: Vec = provider::answers(content) - .ok_or_else(|| failed(format!("no summaries in the answer: {content}")))?; - if answers.len() > 1 { - return Err(failed("expected at most one summary".into())); - } - let Some(answer) = answers.into_iter().next() else { - return Ok(None); - }; - if answer.id != fold.id { - return Err(failed(format!("answered for unknown fold {}", answer.id))); - } - Ok((!answer.pseudocode.trim().is_empty()).then(|| Summary { - quote: quoted(fold.doc.as_deref(), &answer.summary), - pseudocode: answer.pseudocode.trim().to_owned(), - })) - } -} - -/// The text of the docstring region with this id, with comment markers and -/// quotes stripped, or `None` when it says nothing. -fn documentation(text: &str) -> Option { - let words: Vec<_> = text - .split_terminator('\n') - .map(strip_markers) - .filter(|line| !line.is_empty()) - .collect(); - let text = words.join(" "); - (!text.is_empty()).then_some(text) -} - -fn strip_markers(line: &str) -> &str { - let mut text = line.trim(); - for prefix in [ - "///", "//!", "//", "/**", "/*", "*/", "*", "#", "--", ";;", ";", "%", - ] { - if let Some(rest) = text.strip_prefix(prefix) { - text = rest.trim(); - break; - } - } - for quote in ["\"\"\"", "'''"] { - text = text - .trim_start_matches(quote) - .trim_end_matches(quote) + .ok_or_else(|| failed("no text in the response".to_owned()))? .trim(); + Ok((!pseudocode.is_empty()).then(|| pseudocode.to_owned())) } - text.trim_end_matches("*/").trim() -} - -/// The model's sentence, kept only when it really is a verbatim quote from -/// the docstring: compared with runs of whitespace collapsed. -fn quoted(doc: Option<&str>, summary: &str) -> Option { - let squash = |text: &str| text.split_whitespace().collect::>().join(" "); - let (doc, sentence) = (squash(doc?), squash(summary)); - (!sentence.is_empty() && doc.contains(&sentence)).then_some(sentence) -} - -/// Pseudocode earns its place only when it is clearly shorter than the -/// code: a summary with more than half the body's non-blank lines is -/// dropped; the body retains its original visibility. -fn compresses(summary: &str, body: &[&str]) -> bool { - let summary_lines = summary - .lines() - .filter(|line| !line.trim().is_empty()) - .count(); - let body_lines = body.iter().filter(|line| !line.trim().is_empty()).count(); - summary_lines * 2 <= body_lines } /// The API key: the `api_key` option, or else the first of the provider's @@ -274,44 +175,38 @@ impl GuestPlugin for Summarize { return Ok(true); } let text = cursor.text(data.id)?; - let docstring = cursor.related(data.id, "documentation")?.into_iter().next(); - let doc = docstring - .map(|id| cursor.text(id)) - .transpose()? - .as_deref() - .and_then(documentation); - let lines = data.range.start.line..data.range.end.line; - let request = Request { - id: data.id, - first_line: lines.start + 1, - last_line: lines.end, - doc, - }; let file = cursor.file(); let path = match &file.file { FileSides::Both((_, rhs)) | FileSides::RightOnly(rhs) => &rhs.path, FileSides::LeftOnly(lhs) => &lhs.path, }; - if let Some(summary) = self - .complete(path, &text, &request) + let Some(pseudocode) = self + .complete(path, &text, data.range.start.line as usize + 1) .await .map_err(|e| format!("summarizer: {e:#}"))? - { - if compresses( - &summary.pseudocode, - &text.split_terminator('\n').collect::>(), - ) { - let label = match summary.quote { - Some(quote) => format!("{quote}\n{}", summary.pseudocode), - None => summary.pseudocode, - }; - cursor.set_collapsed(data.id, true)?; - cursor.set_label(data.id, Some(&label))?; - if let Some(docstring) = docstring { - cursor.link(&[data.id, docstring])?; - } - } + else { + return Ok(false); + }; + // Pseudocode earns its place only when it is at most two thirds as + // long as the body; otherwise the body keeps its original visibility. + let size = |text: &str| text.chars().filter(|c| !c.is_whitespace()).count(); + if size(&pseudocode) * 3 > size(&text) * 2 { + return Ok(false); } + cursor.set_collapsed(data.id, true)?; + let docstring = cursor.related(data.id, "documentation")?.into_iter().next(); + let Some(docstring) = docstring else { + cursor.set_label(data.id, Some(&pseudocode))?; + return Ok(false); + }; + let docstring_text = cursor.text(docstring)?; + let first = docstring_text + .lines() + .next() + .ok_or_else(|| format!("empty docstring region {docstring}"))? + .trim(); + cursor.set_label(data.id, Some(&format!("{first}\n{pseudocode}")))?; + cursor.link(&[data.id, docstring])?; Ok(false) } } @@ -323,7 +218,9 @@ impl Summarize { } let count = (data.range.end.line - data.range.start.line) as usize; Ok(if data.tags.iter().any(|tag| tag == TEST) { - self.options.tests && count >= self.options.test_min_lines + self.options.tests + && count >= self.options.test_min_lines + && cursor.is_one_sided(data.id)? } else { data.tags.iter().any(|tag| tag == FUNCTION) && !data.visibility.collapsed diff --git a/plugins/shape/summarize/rust/src/provider.rs b/plugins/shape/summarize/rust/src/provider.rs index 30f0b01b8..2e957b206 100644 --- a/plugins/shape/summarize/rust/src/provider.rs +++ b/plugins/shape/summarize/rust/src/provider.rs @@ -1,6 +1,5 @@ //! Each model API's wire format: where a request goes, how it is //! authenticated, and where the answer's text is. -use serde::de::DeserializeOwned; use serde::Deserialize; use serde_json::{json, Value}; @@ -50,7 +49,6 @@ impl Provider { headers } - /// Every provider constrains the answer with the same schema. /// Only Gemini gets a temperature: current reasoning models reject one. /// OpenAI gets no length limit either, since compatible servers name it /// differently; Anthropic's limit also covers thinking, so it has a floor. @@ -63,8 +61,6 @@ impl Provider { "temperature": 0, "maxOutputTokens": max_tokens, "thinkingConfig": {"thinkingBudget": 0}, - "responseMimeType": "application/json", - "responseJsonSchema": summaries_schema(), }, }), Self::OpenAi => json!({ @@ -73,17 +69,12 @@ impl Provider { {"role": "system", "content": system}, {"role": "user", "content": user}, ], - "response_format": { - "type": "json_schema", - "json_schema": {"name": "summaries", "strict": true, "schema": summaries_schema()}, - }, }), Self::Anthropic => json!({ "model": model, "max_tokens": max_tokens.max(4096), "system": system, "messages": [{"role": "user", "content": user}], - "output_config": {"format": {"type": "json_schema", "schema": summaries_schema()}}, }), } } @@ -104,45 +95,3 @@ impl Provider { } } } - -/// `{"summaries": [{id, summary, pseudocode}]}`, every field required: OpenAI -/// and Anthropic take only an object at the root. -fn summaries_schema() -> Value { - json!({ - "type": "object", - "properties": { - "summaries": { - "type": "array", - "items": { - "type": "object", - "properties": { - "id": {"type": "integer"}, - "summary": {"type": "string"}, - "pseudocode": {"type": "string"}, - }, - "required": ["id", "summary", "pseudocode"], - "additionalProperties": false, - }, - }, - }, - "required": ["summaries"], - "additionalProperties": false, - }) -} - -/// The first JSON array in an answer that holds items, ignoring any prose, -/// fence or reasoning around it, and the `summaries` object wrapping it; -/// an empty array only when none does. -pub fn answers(text: &str) -> Option> { - let mut parsed = text.match_indices('[').filter_map(|(start, _)| { - serde_json::Deserializer::from_str(&text[start..]) - .into_iter::>() - .next()? - .ok() - }); - let first = parsed.next()?; - if !first.is_empty() { - return Some(first); - } - Some(parsed.find(|items| !items.is_empty()).unwrap_or(first)) -} diff --git a/plugins/shape/test-bodies/plugin.wasm b/plugins/shape/test-bodies/plugin.wasm index 932526470..b0b9a07cc 100644 Binary files a/plugins/shape/test-bodies/plugin.wasm and b/plugins/shape/test-bodies/plugin.wasm differ diff --git a/plugins/shape/test-bodies/queries/go.scm b/plugins/shape/test-bodies/queries/go.scm index 3bbca57f8..251aa54e1 100644 --- a/plugins/shape/test-bodies/queries/go.scm +++ b/plugins/shape/test-bodies/queries/go.scm @@ -1,8 +1,6 @@ -; inherits: builtin:shared/queries/go.scm, builtin:shared/queries/go-docstrings.scm -((function_declaration name: (identifier) @_name body: (block "{" @fold.open (statement_list . [ - (labeled_statement (label_name) . (_) @fold.indent) - (_) @fold.indent - ]) "}" @fold.close) @fold) - (#not-match? @fold.indent "^[A-Za-z_][A-Za-z0-9_]*:\\s*\n") +; inherits: builtin:shared/queries/go.scm, builtin:shared/queries/go-docstrings.scm, builtin:shared/queries/go-tests.scm +((function_declaration name: (identifier) @_name body: (block "{" @fold.open (statement_list . + (if_statement condition: (call_expression function: (selector_expression) @_short)) @fold.indent) "}" @fold.close) @fold) (#match? @_name "^Test") - (#set! tag "test-bodies:test")) + (#eq? @_short "testing.Short") + (#set! tag "test-bodies:integration")) diff --git a/plugins/shape/test-bodies/queries/javascript.scm b/plugins/shape/test-bodies/queries/javascript.scm index cc44e7acd..dfe14795b 100644 --- a/plugins/shape/test-bodies/queries/javascript.scm +++ b/plugins/shape/test-bodies/queries/javascript.scm @@ -1,9 +1,9 @@ -; inherits: builtin:shared/queries/javascript.scm, builtin:shared/queries/javascript-docstrings.scm +; inherits: builtin:shared/queries/javascript.scm, builtin:shared/queries/javascript-docstrings.scm, builtin:shared/queries/javascript-tests.scm ((call_expression function: (identifier) @_name arguments: (arguments [ (arrow_function body: (statement_block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) (function_expression body: (statement_block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) ])) - (#match? @_name "^(it|test|describe)$") + (#match? @_name "^describe$") (#set! tag "test-bodies:test")) diff --git a/plugins/shape/test-bodies/queries/python.scm b/plugins/shape/test-bodies/queries/python.scm index 68dbd31cd..668b62bcc 100644 --- a/plugins/shape/test-bodies/queries/python.scm +++ b/plugins/shape/test-bodies/queries/python.scm @@ -1,4 +1,7 @@ -; inherits: builtin:shared/queries/python.scm, builtin:shared/queries/python-docstrings.scm -((function_definition name: (identifier) @_name ":" @fold.open body: (block . (_) @fold.indent) @fold) +; inherits: builtin:shared/queries/python.scm, builtin:shared/queries/python-docstrings.scm, builtin:shared/queries/python-tests.scm +((decorated_definition + (decorator) @_mark + definition: (function_definition name: (identifier) @_name ":" @fold.open body: (block . (_) @fold.indent) @fold)) (#match? @_name "^test_") - (#set! tag "test-bodies:test")) + (#match? @_mark "pytest\\.mark\\.(integration|e2e)\\b") + (#set! tag "test-bodies:integration")) diff --git a/plugins/shape/test-bodies/queries/rust.scm b/plugins/shape/test-bodies/queries/rust.scm index c1a0f6538..08598152b 100644 --- a/plugins/shape/test-bodies/queries/rust.scm +++ b/plugins/shape/test-bodies/queries/rust.scm @@ -1,7 +1,4 @@ -; inherits: builtin:shared/queries/rust.scm, builtin:shared/queries/rust-docstrings.scm -((function_item attributes: (attributes (attribute_item) @_attribute) body: (block "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) - (#match? @_attribute "test") - (#set! tag "test-bodies:test")) +; inherits: builtin:shared/queries/rust.scm, builtin:shared/queries/rust-docstrings.scm, builtin:shared/queries/rust-tests.scm ((mod_item attributes: (attributes (attribute_item) @_attribute) body: (declaration_list "{" @fold.open . (_) @fold.indent "}" @fold.close) @fold) (#match? @_attribute "cfg\\(test\\)") (#set! tag "test-bodies:module")) diff --git a/plugins/shape/test-bodies/rust/src/lib.rs b/plugins/shape/test-bodies/rust/src/lib.rs index aa130f40b..7fe99a1bc 100644 --- a/plugins/shape/test-bodies/rust/src/lib.rs +++ b/plugins/shape/test-bodies/rust/src/lib.rs @@ -3,9 +3,13 @@ use diffr_plugin_sdk::prelude::*; use serde::Deserialize; /// The plugin's name, and the tags its queries set: a test function's body, -/// and a test module's body. +/// a test module's body, and an integration test's body. const TEST: &str = "test-bodies:test"; const MODULE: &str = "test-bodies:module"; +const INTEGRATION: &str = "test-bodies:integration"; + +/// The classifier's tags for a file of integration tests. +const FILE_INTEGRATION: [&str; 2] = ["integration", "e2e"]; #[derive(Deserialize)] #[serde(deny_unknown_fields)] @@ -14,7 +18,8 @@ pub struct Options { } /// Test bodies of at least `min_lines` start collapsed on both sides, paired -/// or not, so a diff reads as the code under test first. A whole test +/// or not, so a diff reads as the code under test first. Integration tests +/// stay open. A whole test /// module, such as a Rust `#[cfg(test)] mod tests`, collapses as one fold /// labelled "test module"; the header stays visible and the fold expands /// like any other. A test body is linked to its docstring, which collapses @@ -36,9 +41,18 @@ impl GuestPlugin for TestBodies { async fn visit(&self, cursor: &Cursor, phase: Visit) -> Result { if phase == Visit::Pre { + if cursor + .file() + .tags + .iter() + .any(|tag| FILE_INTEGRATION.contains(&tag.as_str())) + { + return Ok(false); + } let data = cursor.get(cursor.id())?.data; if !matches!(data.kind, Kind::Fold) || ((data.range.end.line - data.range.start.line) as usize) < self.options.min_lines + || data.tags.iter().any(|tag| tag == INTEGRATION) { return Ok(true); } diff --git a/src/plugin/tests/mod.rs b/src/plugin/tests/mod.rs index 0e4048319..f987e8bcb 100644 --- a/src/plugin/tests/mod.rs +++ b/src/plugin/tests/mod.rs @@ -33,15 +33,6 @@ pub(crate) fn walk(regions: &[Region], visit: &mut impl FnMut(&Region)) { } } -pub(crate) fn walk_mut(regions: &mut [Region], visit: &mut impl FnMut(&mut Region)) { - for region in regions { - visit(region); - if let Node::Fold { children, .. } = &mut region.node { - walk_mut(children, visit); - } - } -} - pub(crate) fn is_fold(region: &Region) -> bool { matches!(region.node, Node::Fold { .. }) } @@ -160,15 +151,6 @@ pub(crate) fn run( shape(&bundled(name, overrides), file, sides).unwrap(); } -/// Run a pipeline and inspect the sides it left, without changing `sides`. -fn edited( - pipeline: &Pipeline, - file: &FileChange, - sides: &Pairing, -) -> anyhow::Result> { - crate::test_runtime().block_on(pipeline.run(file, sides.clone())) -} - /// For each `deleted-bodies:function` body on the after side, the first line /// of the body and the lines of its docstring, as the query relationship identifies it. fn documented(path: &str, after: &str) -> Vec<(u32, Option<(u32, u32)>)> { diff --git a/src/plugin/tests/summarize.rs b/src/plugin/tests/summarize.rs index 74a111689..3acb218cd 100644 --- a/src/plugin/tests/summarize.rs +++ b/src/plugin/tests/summarize.rs @@ -1,7 +1,6 @@ use super::*; use std::io::{BufRead, BufReader, Read, Write}; use std::net::TcpListener; -use std::path::Path; const FUNCTION: &str = "summarize:function"; @@ -106,12 +105,7 @@ fn fold_label(sides: &Pairing) -> String { labels.remove(0) } -fn gemini_answer(items: &[(u32, &str)]) -> String { - let answers: Vec<_> = items - .iter() - .map(|(id, text)| json!({"id": id, "pseudocode": text})) - .collect(); - let text = json!({"summaries": answers}).to_string(); +fn gemini_answer(text: &str) -> String { json!({"candidates": [{"content": {"parts": [{"text": text}]}}]}).to_string() } @@ -130,12 +124,8 @@ fn summarizer_with(overrides: serde_json::Value) -> Pipeline { bundled("summarize", overrides) } -/// Inspect requests emitted by the actual node callbacks. -fn select( - sides: &Pairing, - min_lines: usize, - test_min_lines: Option, -) -> Vec<(u32, u32, u32, Option)> { +/// The first numbered line of each body the summarizer sends. +fn select(sides: &Pairing, min_lines: usize, test_min_lines: Option) -> Vec { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let address = listener.local_addr().unwrap(); let server = std::thread::spawn(move || { @@ -165,17 +155,12 @@ fn select( reader.read_exact(&mut body).unwrap(); let body: serde_json::Value = serde_json::from_slice(&body).unwrap(); let prompt = body["contents"][0]["parts"][0]["text"].as_str().unwrap(); - selected.extend(prompt.lines().filter_map(|line| { - let (id, lines) = line.strip_prefix("- fold ")?.split_once(": lines ")?; - let (start, end) = lines.split_once('-')?; - Some(( - id.parse::().unwrap(), - start.parse::().unwrap(), - end.parse::().unwrap(), - )) - })); - let response = - json!({"candidates":[{"content":{"parts":[{"text":"[]"}]}}]}).to_string(); + let first = prompt + .lines() + .find_map(|line| line.split_once(" | ")?.0.trim().parse::().ok()) + .unwrap(); + selected.push(first); + let response = gemini_answer(""); write!( socket, "HTTP/1.1 200 OK\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", @@ -196,92 +181,20 @@ fn select( let selected = server.join().unwrap(); result.unwrap(); selected - .into_iter() - .map(|(id, start, end)| { - let source = sides.rhs().unwrap(); - let mut doc = None; - walk(source.root.children(), &mut |region| { - if region.id == id { - doc = region - .relations - .iter() - .find(|(name, _)| name == "documentation") - .map(|(_, id)| *id); - } - }); - (id, start, end, doc) - }) - .collect() } #[test] fn selection_takes_new_bodies_of_at_least_min_lines() { let (_, sides) = project("a.py", "", LARGE); - let selected = select(&sides, 3, None); - assert_eq!(selected.len(), 1); - assert_eq!((selected[0].1, selected[0].2), (2, 4)); + assert_eq!(select(&sides, 3, None), [2]); let (_, sides) = project("a.py", LARGE, LARGE); assert!(select(&sides, 3, None).is_empty()); } -#[test] -fn selection_reads_newness_from_the_lines_when_the_match_fell_back() { - let before = "def keep():\n a = 1\n b = 2\n return a + b\n"; - let after = "def keep():\n a = 1\n b = 2\n return a + b\n\ndef fresh():\n x = 1\n y = 2\n return x + y\n"; - let (_, sides) = project_with( - "a.py", - before, - after, - DiffOptions { - graph_limit: 1, - ..DiffOptions::default() - }, - ); - // Nothing matched, so no fold is paired; the lines still are. Only the - // added body, whose lines pair with nothing, is new. - let selected = select(&sides, 3, None); - assert_eq!(selected.len(), 1, "{selected:?}"); - assert_eq!((selected[0].1, selected[0].2), (7, 9)); -} - -#[test] -fn selection_takes_outermost_function_bodies_only() { - // A method inside an impl: the impl's declaration_list is a body but - // not a function, so the method is the outermost selection. - let after = "impl A {\n fn m(&self) {\n a();\n b();\n c();\n let f = || {\n d();\n e();\n g();\n };\n f();\n }\n}\n"; - let (_, sides) = project("a.rs", "", after); - let selected = select(&sides, 3, None); - assert_eq!(selected.len(), 1, "{selected:?}"); - assert_eq!((selected[0].1, selected[0].2), (3, 11)); - // Below the threshold, nothing. - assert!(select(&sides, 30, None).is_empty()); -} - -#[test] -fn selection_skips_test_bodies_and_collapsed_folds() { - let after = "#[test]\nfn t() {\n a();\n b();\n c();\n}\n\nfn f() {\n a();\n b();\n c();\n}\n"; - let (file, mut sides) = project("a.rs", "", after); - let selected = select(&sides, 3, None); - assert_eq!(selected.len(), 1, "{selected:?}"); - assert_eq!((selected[0].1, selected[0].2), (9, 11)); - run("test-bodies", json!({"min_lines": 3}), &file, &mut sides); - let mut sides = sides.clone(); - let (Pairing::Both { rhs, .. } | Pairing::RightOnly { rhs }) = &mut sides else { - panic!("an after side"); - }; - walk_mut(std::slice::from_mut(&mut rhs.root), &mut |region| { - if region.range.start.line == 8 { - region.visibility.collapsed = true; - } - }); - assert!(select(&sides, 3, None).is_empty()); -} - #[test] fn long_summaries_are_discarded_without_changing_initial_folding() { let (file, mut sides) = project("a.py", "", LARGE); - let id = select(&sides, 3, None)[0].0; - let (endpoint, server) = serve(vec![(200, gemini_answer(&[(id, "a()\nb()\nc()")]))]); + let (endpoint, server) = serve(vec![(200, gemini_answer("a()\nb()\nc()"))]); shape(&summarizer(&endpoint, 0), &file, &mut sides).unwrap(); server.join().unwrap(); let mut folds = Vec::new(); @@ -297,14 +210,13 @@ fn long_summaries_are_discarded_without_changing_initial_folding() { #[test] fn summaries_collapse_selected_folds_behind_pseudocode() { let (file, mut sides) = project("a.py", "", LARGE); - let id = select(&sides, 3, None)[0].0; - let (endpoint, server) = serve(vec![(200, gemini_answer(&[(id, "call a, b, c")]))]); + let (endpoint, server) = serve(vec![(200, gemini_answer("a b c"))]); shape(&summarizer(&endpoint, 0), &file, &mut sides).unwrap(); let bodies = server.join().unwrap(); assert!(bodies[0].contains("thinkingBudget")); - assert!(bodies[0].contains(&format!("fold {id}: lines 2-4"))); + assert!(bodies[0].contains(" 2 | ")); // `g` is a one-line function, whose body is not a region. - assert_eq!(fold_label(&sides), "call a, b, c"); + assert_eq!(fold_label(&sides), "a b c"); let mut collapsed = Vec::new(); walk(rhs(&sides).root.children(), &mut |region| { if is_fold(region) && has_tag(region, FUNCTION) { @@ -315,164 +227,25 @@ fn summaries_collapse_selected_folds_behind_pseudocode() { } #[test] -fn a_docstring_is_sent_and_only_a_verbatim_sentence_from_it_is_kept() { +fn a_docstring_heads_the_pseudocode_with_its_first_line() { let after = "/// Sums three numbers.\n/// Used by tests.\nfn total(a: u32, b: u32, c: u32) -> u32 {\n let x = a;\n let y = b;\n let z = c;\n x + y + z\n}\n"; - let answer = |id: u32, summary: &str| { - let answers = vec![json!({"id": id, "summary": summary, "pseudocode": "return a + b + c"})]; - json!({"candidates": [{"content": {"parts": [{"text": serde_json::to_string(&answers).unwrap()}]}}]}) - .to_string() - }; - let body_label = |sides: &Pairing| { - let mut labels = Vec::new(); - walk(std::slice::from_ref(&rhs(sides).root), &mut |region| { - if is_fold(region) && has_tag(region, FUNCTION) { - labels.push(region.visibility.label.clone()); - } - }); - assert_eq!(labels.len(), 1, "{labels:?}"); - labels.remove(0) - }; - let (file, mut sides) = project("a.rs", "", after); - let id = select(&sides, 3, None)[0].0; - let (endpoint, server) = serve(vec![(200, answer(id, "Sums three numbers."))]); - shape(&summarizer(&endpoint, 0), &file, &mut sides).unwrap(); - let bodies = server.join().unwrap(); - assert!( - bodies[0].contains("doc: Sums three numbers. Used by tests."), - "{}", - bodies[0] - ); - assert_eq!(body_label(&sides), "Sums three numbers.\nreturn a + b + c"); - assert_linked(&sides, id); - - // A sentence the docstring does not contain is dropped. let (file, mut sides) = project("a.rs", "", after); - let (endpoint, server) = serve(vec![(200, answer(id, "Adds things up."))]); + let (endpoint, server) = serve(vec![(200, gemini_answer("return a + b + c"))]); shape(&summarizer(&endpoint, 0), &file, &mut sides).unwrap(); server.join().unwrap(); - assert_eq!(body_label(&sides), "return a + b + c"); -} - -/// The after side's docstring shares the summarized body's fold state -/// and starts collapsed with an empty label. -fn assert_linked(sides: &Pairing, body: u32) { - let rhs = rhs(sides); - let mut state = None; - walk(rhs.root.children(), &mut |region| { - if region.id == body { - state = Some(region.fold_state_id); + let (mut body, mut docstring) = (None, None); + walk(rhs(&sides).root.children(), &mut |region| { + if is_fold(region) && has_tag(region, FUNCTION) { + body = Some((region.fold_state_id, region.visibility.label.clone())); } - }); - let mut docstrings = Vec::new(); - walk(rhs.root.children(), &mut |region| { if has_tag(region, "summarize:docstring") { - docstrings.push(( - region.fold_state_id, - region.visibility.collapsed, - region.visibility.label.clone(), - )); + docstring = Some(region.fold_state_id); } }); - assert_eq!(docstrings, [(state.unwrap(), true, String::new())]); -} - -#[test] -fn newness_is_the_lines_inside_the_body() { - // The docstring is unchanged and the signature line still pairs. A fold - // covers its body alone, so that line sits outside it: what decides is - // whether a line of the body itself pairs. - let head = "fn keep() {}\n\n/// Sums three numbers.\n/// Used by tests.\nfn total(a: u32, b: u32, c: u32) -> u32 "; - let one_liner = format!("{head}{{ a }}\n"); - let grown = format!("{head}{{\n let x = a;\n x\n}}\n"); - let after = - format!("{head}{{\n let x = a;\n let y = b;\n let z = c;\n x + y + z\n}}\n"); - let (_, sides) = project("a.rs", &one_liner, &after); - let states = |source: &Source| { - let mut states = Vec::new(); - walk(source.root.children(), &mut |region| { - if is_fold(region) && region.range.start.line == 2 { - states.push(region.fold_state_id); - } - }); - states - }; - let projected = sides.clone(); - let Pairing::Both { lhs, rhs: replaced } = &projected else { - panic!("both sides"); - }; - assert_eq!( - states(lhs), - states(replaced), - "the docstring is matched across sides" - ); - // The one-liner it replaced had no body fold to match, and no line of - // the new body pairs: the body is new. - let mut body = None; - walk(replaced.root.children(), &mut |region| { - if is_fold(region) && has_tag(region, FUNCTION) { - body = Some(region.fold_state_id); - } - }); - let mut lhs_states = Vec::new(); - walk(lhs.root.children(), &mut |region| { - lhs_states.push(region.fold_state_id) - }); - assert!(!lhs_states.contains(&body.expect("a function body on the after side"))); - assert_eq!(select(&projected, 3, None).len(), 1); - // The same body grown from one that already had lines: `let x = a;` - // still pairs, so this is a rewrite rather than a new body. - let (_, sides) = project("a.rs", &grown, &after); - assert!(select(&sides, 3, None).is_empty()); -} - -#[test] -fn the_system_prompt_is_the_configured_one() { - let (file, mut sides) = project("a.py", "", LARGE); - let id = select(&sides, 3, None)[0].0; - let request = |overrides: serde_json::Value, sides: &mut Pairing| { - let (endpoint, server) = serve(vec![(200, gemini_answer(&[(id, "call a, b, c")]))]); - let mut overrides = overrides; - overrides["api_key"] = json!("test-key"); - overrides["endpoint"] = json!(endpoint); - overrides["min_lines"] = json!(3); - shape(&summarizer_with(overrides), &file, sides).unwrap(); - let bodies = server.join().unwrap(); - let body: serde_json::Value = serde_json::from_str(&bodies[0]).unwrap(); - ( - body["systemInstruction"]["parts"][0]["text"].clone(), - body["contents"][0]["parts"][0]["text"].clone(), - ) - }; - let (system, user) = request(json!({"system_prompt": "Answer in haiku."}), &mut sides); - assert_eq!(system, "Answer in haiku."); - let user = user.as_str().unwrap(); - assert!(user.starts_with("File a.py:\n"), "{user}"); - assert!(user.contains(&format!("fold {id}: lines 2-4")), "{user}"); - - let (_, mut sides) = project("a.py", "", LARGE); - let (system, _) = request(json!({}), &mut sides); - let defaults = builtin::manifest("summarize").unwrap().defaults(); - let prompt = defaults["system_prompt"].as_str().unwrap(); - assert_eq!(system, prompt); - assert!(prompt.starts_with( - "For each listed fold, rewrite that function body as short pseudocode. Keep the names." - )); - assert!(!prompt.contains('\n')); -} - -#[test] -fn transient_failures_are_retried_then_succeed() { - let (file, mut sides) = project("a.py", "", LARGE); - let id = select(&sides, 3, None)[0].0; - let (endpoint, server) = serve(vec![ - (503, "{}".to_owned()), - (429, "{}".to_owned()), - (200, gemini_answer(&[(id, "retry ok")])), - ]); - shape(&summarizer(&endpoint, 3), &file, &mut sides).unwrap(); - assert_eq!(server.join().unwrap().len(), 3); - let label = fold_label(&sides); - assert!(label.ends_with("retry ok"), "{label}"); + let (state, label) = body.unwrap(); + assert_eq!(label, "/// Sums three numbers.\nreturn a + b + c"); + // The docstring folds with the body. + assert_eq!(docstring, Some(state)); } #[test] @@ -494,40 +267,7 @@ fn hard_failures_and_exhausted_retries_are_run_failures() { } #[test] -fn small_files_never_call_the_model() { - let (file, sides) = project("a.py", "", "def h():\n e()\n"); - let pipeline = summarizer_with(json!({ - "api_key": "k", - "endpoint": "http://127.0.0.1:1", - "min_lines": 3, - })); - assert_eq!(edited(&pipeline, &file, &sides).unwrap(), sides.clone()); -} - -/// Exercise the same component a user loads from an external plugin folder. -#[test] -fn external_component_summarizes_over_http() { - let (file, mut sides) = project("a.py", "", LARGE); - let id = select(&sides, 3, None)[0].0; - let (endpoint, server) = serve(vec![ - (429, "{}".into()), - (200, gemini_answer(&[(id, "call a, b, c")])), - ]); - let pipeline = super::configured(&format!( - "[plugins]\norder = ['external.summarize']\n[plugins.external.summarize]\nenabled = true\npath = {:?}\n{}", - Path::new(env!("CARGO_MANIFEST_DIR")).join("plugins/shape/summarize"), - super::options( - json!({"api_key": "test-key", "endpoint": endpoint, "min_lines": 3, "retries": 1}) - ) - )) - .unwrap(); - shape(&pipeline, &file, &mut sides).unwrap(); - assert_eq!(fold_label(&sides), "call a, b, c"); - assert_eq!(server.join().unwrap().len(), 2); -} - -#[test] -fn tests_are_selected_when_added_modified_unchanged_or_already_collapsed() { +fn tests_are_selected_only_when_added_even_if_already_collapsed() { for (path, before, after) in [ ( "a.py", @@ -556,154 +296,29 @@ fn tests_are_selected_when_added_modified_unchanged_or_already_collapsed() { let comment = if path.ends_with(".py") { "#" } else { "//" }; let after = format!("{after}\n{comment} changed elsewhere\n"); let (file, mut sides) = project(path, old, &after); - assert_eq!(select(&sides, 3, Some(3)).len(), 1, "{path}: {old}"); + let added = usize::from(old.is_empty()); + assert_eq!(select(&sides, 3, Some(3)).len(), added, "{path}: {old}"); assert!(select(&sides, 3, None).is_empty()); assert!(select(&sides, 3, Some(30)).is_empty()); run("test-bodies", json!({"min_lines": 3}), &file, &mut sides); - assert_eq!(select(&sides, 3, Some(3)).len(), 1); + assert_eq!(select(&sides, 3, Some(3)).len(), added); } } } -#[test] -fn suites_select_individual_tests_and_preserve_nested_summary_folds() { - for (path, after, outer_tag) in [ - ("a.rs", "#[cfg(test)]\nmod tests {\n #[test]\n fn one() {\n setup();\n act();\n check();\n }\n #[test]\n fn two() {\n setup();\n act();\n check();\n }\n}\n", "test-bodies:module"), - ("a.ts", "describe('suite', () => {\n it('one', () => {\n setup();\n act();\n check();\n });\n test('two', () => {\n setup();\n act();\n check();\n });\n});\n", "test-bodies:test"), - ] { - let (file, mut sides) = project(path, "", after); - let selected = select(&sides, 3, Some(3)); - assert_eq!(selected.len(), 2, "{path}: {selected:?}"); - let (endpoint, server) = serve(vec![(200, gemini_answer(&[(selected[0].0, "setup; act; check one")])), (200, gemini_answer(&[(selected[1].0, "setup; act; check two")]))]); - shape(&summarizer_with(json!({"api_key": "test", "endpoint": endpoint, "test_min_lines": 3})), &file, &mut sides).unwrap(); - run("test-bodies", json!({"min_lines": 3}), &file, &mut sides); - server.join().unwrap(); - run("context", json!({"lines": 3}), &file, &mut sides); - let mut found = 0; - let mut outer_state = None; - walk(rhs(&sides).root.children(), &mut |region| { - if has_tag(region, outer_tag) && !selected.iter().any(|s| s.0 == region.id) { - assert!(region.visibility.collapsed); - outer_state = Some(region.fold_state_id); - } - if selected.iter().any(|s| s.0 == region.id) { - assert!(region.visibility.collapsed); - assert!(region.visibility.label.starts_with("setup; act; check")); - assert_ne!(Some(region.fold_state_id), outer_state); - found += 1; - } - }); - assert!(outer_state.is_some()); - assert_eq!(found, 2); - } -} - -#[test] -fn bundled_wasm_summarizer_streams_large_prompts() { - // Exceed the host's outgoing body buffer and close the server immediately - // after replying, exercising backpressure and the response/finish race. - let after = format!( - "def test_it():\n setup()\n act()\n check() # {}\n# outside the selected body\n", - "context ".repeat(16_384) - ); - let (file, mut sides) = project("a.py", "", &after); - let id = select(&sides, 3, Some(3))[0].0; - let (endpoint, server) = serve(vec![(200, gemini_answer(&[(id, "setup; act; check")]))]); - let wasm = summarizer_with(json!({ - "api_key": "test", "endpoint": endpoint, "test_min_lines": 3, "retries": 0, - })); - assert!(builtin::component("summarize").is_some()); - shape(&wasm, &file, &mut sides).unwrap(); - assert_eq!(fold_label(&sides), "setup; act; check"); - let requests = server.join().unwrap(); - assert_eq!(requests.len(), 1); - let request: serde_json::Value = serde_json::from_str(&requests[0]).unwrap(); - let prompt = request["contents"][0]["parts"][0]["text"].as_str().unwrap(); - assert!(prompt.contains(&"context ".repeat(16_384))); - assert!(!prompt.contains("outside the selected body")); - assert!(prompt.contains(&format!("fold {id}: lines 2-4"))); -} - -#[test] -fn missing_or_empty_summaries_leave_the_file_unchanged() { - for empty in [false, true] { - let (file, mut sides) = project("a.py", "", LARGE); - let id = select(&sides, 3, None)[0].0; - let answer = if empty { - gemini_answer(&[(id, "")]) - } else { - gemini_answer(&[]) - }; - let before = sides.clone(); - let (endpoint, server) = serve(vec![(200, answer)]); - shape(&summarizer(&endpoint, 0), &file, &mut sides).unwrap(); - assert_eq!(sides, before); - server.join().unwrap(); - } -} - -#[test] -fn gemini_requests_keep_their_path_and_key_header() { - let (file, mut sides) = project("a.py", "", LARGE); - let id = select(&sides, 3, None)[0].0; - let (endpoint, server) = serve_requests(vec![(200, gemini_answer(&[(id, "call a, b, c")]))]); - shape(&summarizer(&endpoint, 0), &file, &mut sides).unwrap(); - let request = server.join().unwrap().remove(0); - assert_eq!( - request.line, - "POST /v1beta/models/gemini-3.8-flash:generateContent HTTP/1.1" - ); - assert!(request - .headers - .contains(&"x-goog-api-key: test-key".to_owned())); - let body: serde_json::Value = serde_json::from_str(&request.body).unwrap(); - let config = &body["generationConfig"]; - assert!(config.get("responseSchema").is_none()); - assert_eq!(config["responseMimeType"], "application/json"); - assert_summaries_schema(&config["responseJsonSchema"]); - assert!(!request - .headers - .iter() - .any(|header| header.starts_with("authorization"))); -} - -fn answers(items: &[(u32, &str)]) -> String { - let answers: Vec<_> = items - .iter() - .map(|(id, text)| json!({"id": id, "pseudocode": text})) - .collect(); - serde_json::to_string(&answers).unwrap() -} - -/// An object root with every field required and nothing else allowed, as -/// OpenAI's strict mode and Anthropic's structured outputs require. -fn assert_summaries_schema(schema: &serde_json::Value) { - assert_eq!(schema["type"], "object"); - assert_eq!(schema["required"], json!(["summaries"])); - assert_eq!(schema["additionalProperties"], false); - let item = &schema["properties"]["summaries"]["items"]; - assert_eq!(item["required"], json!(["id", "summary", "pseudocode"])); - assert_eq!(item["additionalProperties"], false); -} - #[test] fn each_provider_sends_its_own_request_and_reads_its_own_answer() { - let (_, sides) = project("a.py", "", LARGE); - let id = select(&sides, 3, None)[0].0; - let wrapped = format!("{{\"summaries\": {}}}", answers(&[(id, "call a, b, c")])); - // A compatible server that ignores the schema may wrap its answer. - let fenced = format!("maybe [a] or [b], or []\n```json\n{wrapped}\n```"); for (provider, path, response, auth) in [ ( "openai", "/v1", - json!({"choices": [{"message": {"role": "assistant", "content": fenced}}]}), + json!({"choices": [{"message": {"role": "assistant", "content": "a b c"}}]}), "authorization: bearer test-key", ), ( "anthropic", "", - json!({"content": [{"type": "thinking", "thinking": "[1]"}, {"type": "text", "text": wrapped}]}), + json!({"content": [{"type": "thinking", "thinking": "hmm"}, {"type": "text", "text": "a b c"}]}), "x-api-key: test-key", ), ] { @@ -718,7 +333,7 @@ fn each_provider_sends_its_own_request_and_reads_its_own_answer() { "retries": 0, })); shape(&pipeline, &file, &mut sides).unwrap(); - assert_eq!(fold_label(&sides), "call a, b, c", "{provider}"); + assert_eq!(fold_label(&sides), "a b c", "{provider}"); let request = server.join().unwrap().remove(0); let body: serde_json::Value = serde_json::from_str(&request.body).unwrap(); assert!( @@ -734,12 +349,8 @@ fn each_provider_sends_its_own_request_and_reads_its_own_answer() { assert!(body["messages"][1]["content"] .as_str() .unwrap() - .contains(&format!("fold {id}: lines 2-4"))); + .contains(" 2 | ")); assert!(body.get("temperature").is_none()); - let format = &body["response_format"]; - assert_eq!(format["type"], "json_schema"); - assert_eq!(format["json_schema"]["strict"], true); - assert_summaries_schema(&format["json_schema"]["schema"]); } _ => { assert_eq!(request.line, "POST /v1/messages HTTP/1.1"); @@ -748,56 +359,11 @@ fn each_provider_sends_its_own_request_and_reads_its_own_answer() { .contains(&"anthropic-version: 2023-06-01".to_owned())); assert_eq!(body["max_tokens"], 4096); assert!(body.get("temperature").is_none()); - let format = &body["output_config"]["format"]; - assert_eq!(format["type"], "json_schema"); - assert_summaries_schema(&format["schema"]); - assert!(body["system"] - .as_str() - .unwrap() - .starts_with("For each listed fold")); + assert_eq!( + body["system"], + builtin::manifest("summarize").unwrap().defaults()["system_prompt"] + ); } } } } - -#[test] -fn an_openai_compatible_server_needs_no_key() { - if std::env::var_os("OPENAI_API_KEY").is_some() { - return; - } - let (file, mut sides) = project("a.py", "", LARGE); - let id = select(&sides, 3, None)[0].0; - let response = json!({"choices": [{"message": {"content": answers(&[(id, "call a, b, c")])}}]}); - let (endpoint, server) = serve_requests(vec![(200, response.to_string())]); - let pipeline = summarizer_with(json!({ - "provider": "openai", - "model": "llama", - "endpoint": format!("{endpoint}/v1"), - "min_lines": 3, - })); - shape(&pipeline, &file, &mut sides).unwrap(); - assert_eq!(fold_label(&sides), "call a, b, c"); - let request = server.join().unwrap().remove(0); - assert!(!request - .headers - .iter() - .any(|header| header.starts_with("authorization"))); -} - -#[test] -fn an_unset_model_is_the_providers_default() { - let (file, mut sides) = project("a.py", "", LARGE); - let id = select(&sides, 3, None)[0].0; - let text = json!({"summaries": [{"id": id, "summary": "", "pseudocode": "call a, b, c"}]}); - let response = json!({"content": [{"type": "text", "text": text.to_string()}]}); - let (endpoint, server) = serve_requests(vec![(200, response.to_string())]); - let pipeline = summarizer_with(json!({ - "provider": "anthropic", - "api_key": "test-key", - "endpoint": endpoint, - "min_lines": 3, - })); - shape(&pipeline, &file, &mut sides).unwrap(); - let body: serde_json::Value = serde_json::from_str(&server.join().unwrap()[0].body).unwrap(); - assert_eq!(body["model"], "claude-haiku-4-5"); -} diff --git a/src/plugin/tests/test_bodies.rs b/src/plugin/tests/test_bodies.rs index 5c049be4e..39f9c55dd 100644 --- a/src/plugin/tests/test_bodies.rs +++ b/src/plugin/tests/test_bodies.rs @@ -111,3 +111,38 @@ fn javascript_test_callbacks_collapse() { assert_eq!(collapsed, [1], "{path}"); } } + +/// Whether each test body starts collapsed. +fn collapsed_tests(sides: &Pairing) -> Vec { + let mut collapsed = Vec::new(); + walk(rhs(sides).root.children(), &mut |region| { + if has_tag(region, TEST) { + collapsed.push(region.visibility.collapsed); + } + }); + collapsed +} + +#[test] +fn integration_and_end_to_end_tests_stay_open() { + let after = "#[test]\nfn t() {\n a();\n b();\n c();\n}\n"; + let (mut file, mut sides) = project("tests/a.rs", "", after); + file.tags = vec!["integration".to_owned(), "test".to_owned()]; + run("test-bodies", json!({"min_lines": 3}), &file, &mut sides); + assert_eq!(collapsed_tests(&sides), [false]); + // A marked test stays open beside a unit test that collapses. + for (path, after) in [ + ( + "test_a.py", + "@pytest.mark.integration\ndef test_slow():\n a()\n b()\n c()\n\ndef test_fast():\n a()\n b()\n c()\n", + ), + ( + "a_test.go", + "package a\n\nfunc TestSlow(t *testing.T) {\n\tif testing.Short() {\n\t\tt.Skip()\n\t}\n\ta()\n\tb()\n}\n\nfunc TestFast(t *testing.T) {\n\ta()\n\tb()\n\tc()\n}\n", + ), + ] { + let (file, mut sides) = project(path, "", after); + run("test-bodies", json!({"min_lines": 3}), &file, &mut sides); + assert_eq!(collapsed_tests(&sides), [false, true], "{path}"); + } +} diff --git a/tests/tags.rs b/tests/tags.rs index 1c61d2c78..362ffb51c 100644 --- a/tests/tags.rs +++ b/tests/tags.rs @@ -184,12 +184,19 @@ fn bundled_path_rules_tag_vendored_docs_test_and_generated_files() { ("vendored", &["src/vendors.rs", "distribution/a.py"]), ("docs", &["src/docs/mod.rs", "src/readme_parser.rs"]), ("test", &["testing/helpers.rs", "src/testament.py"]), + ( + "integration", + &["src/plugin/tests/a.rs", "tests/integrations_test.py"], + ), ]; let exactly: &[(&str, &[&str])] = &[ ("tests/streaming/check.py", &["test"]), ("src/App.test.tsx", &["test"]), ("src/review/tests.rs", &["test"]), ("tests/snapshots/Cargo.lock", &["generated", "test"]), + ("crates/core/tests/login.rs", &["integration", "test"]), + ("pkg/db/store_integration_test.go", &["integration", "test"]), + ("web/e2e/checkout.spec.ts", &["e2e", "test"]), ( "node_modules/x/package-lock.json", &["generated", "vendored"], @@ -229,6 +236,41 @@ fn bundled_path_rules_tag_vendored_docs_test_and_generated_files() { } } +#[test] +fn test_content_tags_integration_and_end_to_end_files() { + let files: &[(&str, &str, &[&str])] = &[ + ( + "store_test.go", + "//go:build integration\n\npackage store\n", + &["integration", "test"], + ), + ( + "tests/test_api.py", + "import pytest\n\npytestmark = pytest.mark.e2e\n", + &["e2e", "test"], + ), + ( + "src/checkout.spec.ts", + "import { test } from '@playwright/test';\n", + &["e2e", "test"], + ), + ("tests/test_unit.py", "import pytest\n", &["test"]), + ]; + let fixture = Fixture::new(); + for (path, text, _) in files { + fixture.write(path, text); + } + let base = fixture.commit(); + for (path, text, _) in files { + fixture.write(path, &format!("{text}// changed\n")); + } + let head = fixture.commit(); + let records = records(&fixture.run(&base, &head)); + for (path, _, expected) in files { + assert_eq!(tags_of(&records, path), expected.to_vec(), "{path}"); + } +} + /// Git attributes decide a tag outright, whatever the path rules said. #[test] fn linguist_attributes_set_or_clear_their_tags() { @@ -334,7 +376,7 @@ fn visibility_of<'a>(records: &'a [Value], path: &str) -> Option<&'a Value> { .and_then(|root| root.get("visibility")) } -/// By default the classifier hides generated, vendored and test files, and +/// By default the classifier hides generated and vendored files, and /// deleted ones: each is diffed by line and shown behind its reason. #[test] fn hidden_files_are_diffed_by_line_and_shown_behind_their_reason() { @@ -357,10 +399,7 @@ fn hidden_files_are_diffed_by_line_and_shown_behind_their_reason() { hidden("Vendored file · hidden by default") ); assert_eq!(fallback_of(&defaults, "vendor/lib/a.rs"), Some("hidden")); - assert_eq!( - visibility_of(&defaults, "tests/a.rs").cloned(), - hidden("Test file · hidden by default") - ); + assert_eq!(visibility_of(&defaults, "tests/a.rs"), None); assert_eq!( visibility_of(&defaults, "src/gone.rs").cloned(), hidden("Deleted file · hidden by default") diff --git a/tui/packages/hunk/src/diffr/regions.test.ts b/tui/packages/hunk/src/diffr/regions.test.ts index 4b1222772..25478ffbe 100644 --- a/tui/packages/hunk/src/diffr/regions.test.ts +++ b/tui/packages/hunk/src/diffr/regions.test.ts @@ -98,8 +98,9 @@ test("rows carry fold headers on both layouts and drop hidden lines", () => { const file = createFoldedDiffFile(); const split = rowsForFile(file, 0, "split", dark, new Set([11])).filter((r) => r.right); expect(split.map((r) => r.right!.lineNumber)).toEqual([1, 2, undefined, 5, 6, 7, 8]); - expect(split[2].right!.fold).toEqual({ id: 11, label: "Body", collapsed: true, tint: "neutral" }); - expect(split[2].left!.fold).toEqual({ id: 11, label: "Body", collapsed: true, tint: "neutral" }); + // The body hides the right side's changed line, so both sides take the modification tint. + expect(split[2].right!.fold).toEqual({ id: 11, label: "Body", collapsed: true, tint: "modified" }); + expect(split[2].left!.fold).toEqual({ id: 11, label: "Body", collapsed: true, tint: "modified" }); // The open outer fold marks the first line it covers, so it can be collapsed from there. expect(split[1].right!.fold).toEqual({ id: 10, label: "Body", collapsed: false, tint: "neutral" }); const unified = rowsForFile(file, 0, "unified", dark, new Set([10])).filter((r) => r.cell); @@ -120,6 +121,20 @@ test("a multi-line label uses the declared enclosing indent inside the fold tint const unified = rowsForFile(file, 0, "unified", dark, new Set([11])); expect(unified.filter((r) => r.cell?.foldLabel)).toHaveLength(3); }); +test("a syntax body with a multi-line label is quoted between its opener and closer", () => { + const file = createFoldedDiffFile(); + if (file.diff.type !== "text") throw new Error(); + for (const source of [file.diff.lhs!, file.diff.rhs!]) { + const closure = source.root.children[1].children[1]; + if (closure.kind !== "fold") throw new Error(); + closure.visibility = { collapsed: true, label: "call a\ncall b" }; + closure.syntax = { start: { line: 1, column: 13 }, end: { line: 4, column: 4 } }; + } + const rows = rowsForFile(file, 0, "split", dark, new Set([11])).filter((r) => r.right); + const text = (r: (typeof rows)[number]) => r.right!.spans.map((s) => s.text).join(""); + expect(rows.slice(1, 5).map(text)).toEqual([" inner(|| {", " > call a · 1 line changed", " > call b", " });"]); + expect(rows[2].right!.fold?.id).toBe(11); +}); test("a collapsed leaf is one fold row with the chevron, its label, and no line number", () => { const file = createTestDiffFile(); if (file.diff.type !== "text") throw new Error(); @@ -248,6 +263,13 @@ test("a collapsed fold takes its side's change tint when one-sided and stays neu const both = paired.find((r) => r.left?.fold && r.right?.fold)!; expect([both.left!.fold!.tint, both.right!.fold!.tint]).toEqual(["neutral", "neutral"]); expect(foldBackground(dark, "neutral")).toBe(dark.foldBackground); + // Modified: the same pair hiding a changed line takes the modification tint and counts it. + const changedBody = (id: number) => ({ ...body(id, "Body"), children: [leaf(107, 1, 2, [line(1, 4, 5)])] }); + const modified = rowsFor([changedBody(7)], [{ ...changedBody(8), fold_state_id: 7 }]); + const changedRow = modified.find((r) => r.left?.fold && r.right?.fold)!; + expect([changedRow.left!.fold!.tint, changedRow.right!.fold!.tint]).toEqual(["modified", "modified"]); + expect(changedRow.right!.spans.map((s) => s.text).join("")).toContain("⋯ Body · 1 line changed"); + expect(foldBackground(dark, "modified")).toBe(dark.modification); // A fold state shared only on its own side does not pair a fold. const linked = rowsFor([{ ...body(7, "Body"), children: [{ ...leaf(107, 1, 2), fold_state_id: 7 }] }], [body(8, "Body")]); expect(linked.find((r) => r.left?.fold)!.left!.fold!.tint).toBe("removed"); diff --git a/tui/packages/hunk/src/diffr/regions.ts b/tui/packages/hunk/src/diffr/regions.ts index 1c39f238f..8e541c675 100644 --- a/tui/packages/hunk/src/diffr/regions.ts +++ b/tui/packages/hunk/src/diffr/regions.ts @@ -41,7 +41,7 @@ export interface Fold { parentColumn: number; } /** The change tint of a fold: a one-sided region takes its side's change colour, a paired one stays neutral. */ -export type FoldTint = "inserted" | "removed" | "neutral"; +export type FoldTint = "inserted" | "removed" | "modified" | "neutral"; export interface RowFold { /** The fold-state id: what toggling this header toggles. */ id: number; diff --git a/tui/packages/hunk/src/diffr/rows.ts b/tui/packages/hunk/src/diffr/rows.ts index e35eab5e7..3f2431454 100644 --- a/tui/packages/hunk/src/diffr/rows.ts +++ b/tui/packages/hunk/src/diffr/rows.ts @@ -104,13 +104,13 @@ export function rowsForFile( const { leaves, folds } = flatten(d); const hidden = [hiddenLines(folds[0], collapsed), hiddenLines(folds[1], collapsed)]; // A collapsed fold is a row of its own, where its first line would have been. A syntax fold - // instead joins its opener, label and closing suffix on the opener's line, so its closer is - // masked too. + // with a one-line label instead joins its opener, label and closing suffix on the opener's + // line, so its closer is masked too. const bands = folds.map(side => collapsedFolds(side, collapsed)); const inline = bands.map((sideBands, side) => { const result = new Map(); for (const fold of sideBands.values()) { - if (!fold.syntax) continue; + if (!fold.syntax || fold.label.includes("\n")) continue; sideBands.delete(fold.startLine); result.set(fold.syntax.start.line, fold as SyntaxFold); for (let line = fold.syntax.start.line + 1; line <= fold.syntax.end.line; line++) hidden[side].add(line); @@ -133,6 +133,24 @@ export function rowsForFile( return lines; }); const tintOf = (region: Leaf | Fold) => foldTint(region.id, region.side, paired[region.side]); + const alignments = leaves.map(side => new Set(side.map(leaf => leaf.alignmentId))); + const isChanged = (leaf: Leaf, line: number) => + leaf.changed.has(line) || !alignments[leaf.side ? 0 : 1].has(leaf.alignmentId); + // A paired leaf is changed when its counterpart has change spans. + const spanned = new Set(leaves.flat().filter(leaf => leaf.changed.size).map(leaf => leaf.alignmentId)); + // A collapsed paired region that hides a change is modified, and counts its side's changed lines. + const collapsedTint = (region: Leaf | Fold): { tint: FoldTint; note: string } => { + const tint = tintOf(region); + if (tint !== "neutral") return { tint, note: "" }; + const last = "lastHidden" in region ? region.lastHidden : region.endLine - 1; + const inside = leaves[region.side].filter(leaf => leaf.endLine > region.startLine && leaf.startLine <= last); + let count = 0; + for (const leaf of inside) + for (let line = Math.max(leaf.startLine, region.startLine); line <= Math.min(leaf.endLine - 1, last); line++) + if (isChanged(leaf, line)) count++; + if (!count && !inside.some(leaf => spanned.has(leaf.alignmentId))) return { tint, note: "" }; + return { tint: "modified", note: count ? ` · ${count} line${count === 1 ? "" : "s"} changed` : "" }; + }; const placeholder = (text: string, tint: FoldTint): RenderSpan => ({ text, fg: theme.foldPlaceholder, bg: foldBackground(theme, tint) }); const caches = [new Map(), new Map()]; @@ -145,7 +163,6 @@ export function rowsForFile( } return spans; }; - const alignments = leaves.map(side => new Set(side.map(leaf => leaf.alignmentId))); const cell = (leaf: Leaf | null, line: number | null, side: Side): SplitLineCell => { if (line === null || leaf === null) return empty; let spans = spansOf(side, line, leaf); @@ -156,13 +173,13 @@ export function rowsForFile( const closer = texts[side][end.line]!; const suffix = lineSpans(closer, syntax[side].get(end.line) ?? [], [], side ? "right" : "left", theme); const label = folded.label.replace(/\n/g, " · "); - const tint = tintOf(folded); + const { tint, note } = collapsedTint(folded); spans = [...sliceSpansWindow(spans, 0, byteColumn(texts[side][line]!, start.column)).spans, - placeholder(` ⋯${label ? " " + label : ""} `, tint), + placeholder(` ⋯${label ? " " + label : ""}${note} `, tint), ...sliceSpansWindow(suffix, byteColumn(closer, end.column), Infinity).spans]; fold = { id: folded.foldStateId, label: folded.label, collapsed: true, tint }; } - const changed = leaf.changed.has(line) || !alignments[side ? 0 : 1].has(leaf.alignmentId); + const changed = isChanged(leaf, line); return { kind: changed ? (side ? "addition" : "deletion") : "context", sign: changed ? (side ? "+" : "-") : " ", @@ -179,14 +196,24 @@ export function rowsForFile( }; // A collapsed region is one row: chevron and label, no line number, whether the region is a // fold or a leaf the context plugin cut out. It starts at its parent's indent; a multi-line - // label (pseudocode) hangs under it at that indent, one row per line. + // label (pseudocode) hangs under it at that indent, one row per line. A syntax body with a + // multi-line label is instead quoted between its opener and closer, at the body's indent. const band = (region: Leaf | Fold) => { - const tint = tintOf(region); - const lead = (text: string) => withGuides([{ text: " ".repeat(region.parentColumn) }, placeholder(text, tint)], - guides[region.side].get(region.startLine) ?? [], theme); + const { tint, note } = collapsedTint(region); const multiline = region.label.includes("\n"); + const quoted = "syntax" in region && region.syntax !== undefined && multiline; + const body = texts[region.side][region.startLine]!; + const column = quoted ? byteColumn(body, body.length - body.trimStart().length) : region.parentColumn; + const lead = (text: string) => withGuides([{ text: " ".repeat(column) }, placeholder(text, tint)], + guides[region.side].get(region.startLine) ?? [], theme); + if (quoted) { + const [first, ...rest] = region.label.split("\n"); + const header = { kind: "context" as const, sign: " ", band: tint, spans: lead(`> ${first}${note}`), + fold: { id: region.foldStateId, label: region.label, collapsed: true, tint } }; + return { header, labels: rest.map(text => ({ ...header, foldLabel: true, spans: lead(`> ${text}`), fold: undefined })) }; + } const header = { kind: "context" as const, sign: " ", band: tint, - spans: lead(`⋯${region.label && !multiline ? " " + region.label : ""}`), + spans: lead(`⋯${region.label && !multiline ? " " + region.label : ""}${note}`), fold: { id: region.foldStateId, label: region.label, collapsed: true, tint } }; const labels = multiline ? region.label.split("\n").map(text => ({ ...header, foldLabel: true, spans: lead(text), fold: undefined })) diff --git a/tui/packages/hunk/src/diffr/theme.ts b/tui/packages/hunk/src/diffr/theme.ts index 58f2a3f72..41cc5365a 100644 --- a/tui/packages/hunk/src/diffr/theme.ts +++ b/tui/packages/hunk/src/diffr/theme.ts @@ -28,6 +28,7 @@ export interface Palette { highlight: string; addition: string; deletion: string; + modification: string; addWord: string; deleteWord: string; addedText: string; @@ -121,6 +122,7 @@ export function paletteFromHelix(theme: HelixTheme): Palette { const muted = scopeFg(theme, "ui.linenr") ?? scopeFg(theme, "comment") ?? mix(fg, bg, 0.4); const plus = scopeFg(theme, "diff.plus") ?? (isLight ? "#1a7f37" : "#7ee787"); const minus = scopeFg(theme, "diff.minus") ?? (isLight ? "#cf222e" : "#ffa198"); + const delta = scopeFg(theme, "diff.delta") ?? (isLight ? "#9a6700" : "#e3b341"); const selection = scopeBg(theme, "ui.selection") ?? mix(bg, fg, 0.15); return { name: theme.name, @@ -132,6 +134,7 @@ export function paletteFromHelix(theme: HelixTheme): Palette { highlight: selection, addition: mix(bg, plus, 0.12), deletion: mix(bg, minus, 0.12), + modification: mix(bg, delta, 0.12), addWord: mix(bg, plus, 0.28), deleteWord: mix(bg, minus, 0.28), addedText: plus, @@ -181,5 +184,6 @@ export function themeConfig(show: unknown): { name: string; path: string | null /** Paired folds are neutral; only one-sided folds carry a change tint. */ export function foldBackground(theme: Palette, tint: FoldTint) { - return tint === "inserted" ? theme.addition : tint === "removed" ? theme.deletion : theme.foldBackground; + return tint === "inserted" ? theme.addition : tint === "removed" ? theme.deletion + : tint === "modified" ? theme.modification : theme.foldBackground; }