From ca497bef1740b9cdc2badb7a036c6ee1052dcf3c Mon Sep 17 00:00:00 2001 From: Troy Hernandez Date: Thu, 14 May 2026 14:05:35 -0500 Subject: [PATCH 1/4] Render colored inline diffs for file-edit tool calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the agent ran replace_in_file or write_file the user saw just "⎿ 1 line in 12ms" and had no idea what actually changed. Now the display shows a Claude-Code-style inline diff: an "Added N, removed M" summary followed by colored unified-diff hunks (red minus, green plus, cyan hunk header, dim metadata), so the change is visible at a glance. Implementation: - New R/diff-render.R::compute_unified_diff(old, new, path) shells out to `diff -u`. Returns NULL when the inputs are identical (display silently skips), an all-green payload for new files (old == ""), and a fallback summary when `diff` isn't on PATH so the tool call still succeeds. - New ok_with_diff(text, diff) in R/utils.R is a thin builder; ok() itself is untouched so the diff-extension doesn't bleed into the package's general tool-result contract. - tool_replace_in_file passes its existing `original`/`updated` strings; tool_write_file reads prior content before the write and diffs against the post-write content (taking append mode into account). - turn.R threads the diff payload through outcome_text -> event; observer_progress() renders via render_tool_diff() when present. - inst/bin/corteza tool_handler does the same render. - colorize_diff() in R/cli-colors.R is the shared line painter; also wired into the /diff slash command so its output is colored too. - LLM-facing result text is unchanged. The model already knows what it asked for; the diff is purely for the human reading the terminal. 27 new tinytests cover identical / new file / emptied / multi-hunk / missing trailing newline / `diff`-not-on-PATH fallback. --- NEWS.md | 14 +++ R/cli-colors.R | 43 +++++++ R/diff-render.R | 189 +++++++++++++++++++++++++++++++ R/tool-impl.R | 32 +++++- R/turn.R | 16 ++- R/utils.R | 23 ++++ inst/bin/corteza | 21 ++-- inst/tinytest/test_diff_render.R | 70 ++++++++++++ 8 files changed, 392 insertions(+), 16 deletions(-) create mode 100644 R/diff-render.R create mode 100644 inst/tinytest/test_diff_render.R diff --git a/NEWS.md b/NEWS.md index 3bd0490..58e6758 100644 --- a/NEWS.md +++ b/NEWS.md @@ -18,6 +18,20 @@ intercepts Ctrl+C for copy). In the terminal `~/bin/corteza` CLI it's **Ctrl+C** — terminals send raw `^[` for Esc, which is not a signal. +## Inline diffs on file edits + +* `replace_in_file` and `write_file` now attach a unified-diff payload + to their MCP result. The CLI and `corteza::chat()` render it inline + in the tool-call output (`Added N, removed M`, then colored hunks) + instead of the prior `N lines in Xms` summary, so it's obvious what + the agent actually changed. The LLM-facing result text is unchanged + — the diff is for the human reading the terminal. +* Diff generation shells out to the system `diff -u`. If `diff` isn't + on `PATH` the tool degrades to a one-line size summary rather than + failing. +* The `/diff` slash command's output is now ANSI-colored (same + red/green/cyan palette as `git diff --color=always`). + ## Other * `load_saber_briefing()` now wraps `saber::briefing()` in diff --git a/R/cli-colors.R b/R/cli-colors.R index 66fad39..fa9c18a 100644 --- a/R/cli-colors.R +++ b/R/cli-colors.R @@ -46,3 +46,46 @@ ansi_colors <- function() { bright_magenta = "\033[95m", bright_cyan = "\033[96m") } +#' Colorize a unified-diff string for terminal display. +#' +#' Matches `git diff` / `git --color=always`'s palette: green additions, +#' red deletions, cyan hunk headers, bold file headers, dim metadata +#' lines. The `+++ ` / `--- ` file-header check has to run before the +#' bare `+` / `-` check or the file-header lines would be colored as +#' add/delete rows. +#' +#' Returns `text` unchanged when the terminal doesn't support ANSI. +#' @param text Character scalar, raw diff output. +#' @param palette Optional palette from \code{ansi_colors()}; tests pass +#' a forced palette to assert escape sequences are emitted. +#' @return Character scalar with ANSI escapes interleaved. +#' @noRd +colorize_diff <- function(text, palette = ansi_colors()) { + if (!is.character(text) || length(text) != 1L || !nzchar(text)) { + return(text) + } + if (!nzchar(palette$reset)) { + return(text) + } + lines <- strsplit(text, "\n", fixed = TRUE)[[1]] + paint <- function(ln) { + if (startsWith(ln, "diff --git ") || startsWith(ln, "index ") || + startsWith(ln, "similarity ") || startsWith(ln, "rename ") || + startsWith(ln, "new file") || startsWith(ln, "deleted file")) { + paste0(palette$dim, ln, palette$reset) + } else if (startsWith(ln, "+++ ") || startsWith(ln, "--- ")) { + paste0(palette$bold, ln, palette$reset) + } else if (startsWith(ln, "@@")) { + paste0(palette$cyan, ln, palette$reset) + } else if (startsWith(ln, "+")) { + paste0(palette$green, ln, palette$reset) + } else if (startsWith(ln, "-")) { + paste0(palette$red, ln, palette$reset) + } else { + ln + } + } + paste(vapply(lines, paint, character(1L), USE.NAMES = FALSE), + collapse = "\n") +} + diff --git a/R/diff-render.R b/R/diff-render.R new file mode 100644 index 0000000..6406c51 --- /dev/null +++ b/R/diff-render.R @@ -0,0 +1,189 @@ +# Compute a unified diff between two text scalars for display in the +# CLI / chat output. Shells out to `diff -u` because writing a correct +# unified-diff algorithm in pure R is significantly more code than the +# rest of this feature combined; if `diff` isn't on PATH we degrade to +# a one-line fallback rather than fail the tool call. The output is +# uncolored — coloring happens at render time via colorize_diff() so +# the same payload can be re-rendered when ANSI is unavailable. + +#' Locate the system `diff` binary. +#' +#' Returns "" when no binary is found. Cached for the process lifetime +#' since PATH doesn't change during a corteza session. +#' @noRd +.diff_binary_cache <- new.env(parent = emptyenv()) +.diff_binary <- function() { + if (!is.null(.diff_binary_cache$value)) { + return(.diff_binary_cache$value) + } + bin <- Sys.which("diff") + .diff_binary_cache$value <- if (is.na(bin)) "" else unname(bin) + .diff_binary_cache$value +} + +#' Count added and removed lines in a unified-diff body. +#' +#' Ignores file headers (`+++ `, `--- `) and hunk headers (`@@`). +#' @noRd +.diff_summary_counts <- function(lines) { + added <- 0L + removed <- 0L + for (ln in lines) { + if (startsWith(ln, "+++ ") || startsWith(ln, "--- ") || + startsWith(ln, "@@") || + startsWith(ln, "diff --git") || + startsWith(ln, "index ")) { + next + } + if (startsWith(ln, "+")) { + added <- added + 1L + } else if (startsWith(ln, "-")) { + removed <- removed + 1L + } + } + list(added = added, removed = removed) +} + +#' Build a one-line summary like "Added 3 lines, removed 1 line". +#' @noRd +.diff_summary_line <- function(added, removed) { + pl <- function(n, w) sprintf("%d %s%s", n, w, if (n == 1L) "" else "s") + if (added == 0L && removed == 0L) { + "No textual change" + } else if (added == 0L) { + sprintf("Removed %s", pl(removed, "line")) + } else if (removed == 0L) { + sprintf("Added %s", pl(added, "line")) + } else { + sprintf("Added %s, removed %s", + pl(added, "line"), pl(removed, "line")) + } +} + +#' Compute a unified diff for terminal display. +#' +#' Returns NULL when the two inputs are byte-identical (signal to the +#' caller that no diff display is warranted). When `diff` isn't on PATH, +#' returns a fallback payload describing the size of the change without +#' the per-line content. +#' +#' @param old_text Character scalar, prior file contents. Empty string +#' means "new file". +#' @param new_text Character scalar, new file contents. +#' @param path Character scalar, the file path the diff describes; used +#' for the `+++` / `---` header labels. +#' @return NULL if identical, else a list with: +#' \itemize{ +#' \item \code{path}: input path +#' \item \code{summary}: one-line summary string +#' \item \code{lines}: character vector of uncolored diff lines +#' (header + hunks). May be empty when only the fallback +#' summary is available. +#' \item \code{fallback}: logical TRUE when `diff` was unavailable +#' and the payload is summary-only. +#' } +#' @noRd +compute_unified_diff <- function(old_text, new_text, path) { + old_text <- old_text %||% "" + new_text <- new_text %||% "" + path <- path %||% "(unnamed)" + + if (identical(old_text, new_text)) { + return(NULL) + } + + bin <- .diff_binary() + if (!nzchar(bin)) { + # Fallback: approximate added/removed by line count delta. Not + # accurate for arbitrary edits, but it's only used when the + # user has no `diff` available, so we communicate the size of + # the change rather than nothing. + old_n <- if (nzchar(old_text)) { + length(strsplit(old_text, "\n", fixed = TRUE)[[1]]) + } else 0L + new_n <- if (nzchar(new_text)) { + length(strsplit(new_text, "\n", fixed = TRUE)[[1]]) + } else 0L + delta <- new_n - old_n + summary <- if (delta == 0L) { + sprintf("Content changed (%d lines, diff binary unavailable)", + new_n) + } else if (delta > 0L) { + sprintf("Net +%d line(s), diff binary unavailable", delta) + } else { + sprintf("Net %d line(s), diff binary unavailable", delta) + } + return(list(path = path, summary = summary, lines = character(), + fallback = TRUE)) + } + + old_file <- tempfile("corteza-old-") + new_file <- tempfile("corteza-new-") + on.exit({ + unlink(old_file, force = TRUE) + unlink(new_file, force = TRUE) + }, add = TRUE) + + # writeBin avoids platform line-ending translation; we want the + # bytes diff sees to match the bytes that were written. + writeBin(charToRaw(old_text), old_file) + writeBin(charToRaw(new_text), new_file) + + res <- suppressWarnings(system2( + bin, + args = c("-u", + "--label", shQuote(path), + "--label", shQuote(path), + shQuote(old_file), + shQuote(new_file)), + stdout = TRUE, stderr = TRUE + )) + # diff exits 0 (identical, handled above), 1 (differ), or 2 (error). + status <- attr(res, "status") %||% 0L + if (!identical(status, 0L) && !identical(status, 1L)) { + return(list(path = path, + summary = sprintf("diff failed (status %d)", status), + lines = character(), + fallback = TRUE)) + } + + counts <- .diff_summary_counts(res) + list(path = path, + summary = .diff_summary_line(counts$added, counts$removed), + lines = as.character(res), + fallback = FALSE) +} + +#' Render a diff payload to the terminal. +#' +#' Used by both the CLI tool_handler in `inst/bin/corteza` and the +#' `observer_progress()` printer in `R/turn.R` so file-edit tool calls +#' look the same regardless of which entry point the user launched. +#' Skips quietly when the payload is NULL (i.e., the underlying texts +#' were identical and \code{compute_unified_diff()} returned nothing). +#' +#' @param diff Payload from \code{compute_unified_diff()}, or NULL. +#' @param palette Optional ANSI palette; tests force a specific palette. +#' @param indent Leading indent string for each printed line; matches +#' the surrounding tool-call output. +#' @return Invisibly TRUE if anything was printed, FALSE otherwise. +#' @noRd +render_tool_diff <- function(diff, palette = ansi_colors(), indent = " ") { + if (is.null(diff)) { + return(invisible(FALSE)) + } + summary <- diff$summary %||% "" + if (nzchar(summary)) { + cat(sprintf("%s%s⎿ %s%s\n", + indent, palette$dim %||% "", summary, palette$reset %||% "")) + } + if (isTRUE(diff$fallback) || length(diff$lines) == 0L) { + return(invisible(TRUE)) + } + body <- colorize_diff(paste(diff$lines, collapse = "\n"), palette) + body_lines <- strsplit(body, "\n", fixed = TRUE)[[1]] + for (ln in body_lines) { + cat(sprintf("%s%s\n", indent, ln)) + } + invisible(TRUE) +} diff --git a/R/tool-impl.R b/R/tool-impl.R index d197fe8..faed02f 100644 --- a/R/tool-impl.R +++ b/R/tool-impl.R @@ -252,6 +252,15 @@ tool_write_file <- function(path, content, append = FALSE, create_dirs = TRUE) { content <- content %||% "" append <- isTRUE(append) + # Read prior content so we can show the user a diff after the + # write. Empty string when the file is new or unreadable; that + # path produces an all-green diff, which is what we want. + old_content <- "" + if (file.exists(path) && !dir.exists(path)) { + old_content <- tryCatch(tool_read_text(path), + error = function(e) "") + } + write_error <- tryCatch({ tool_write_text(path, content, append = append) NULL @@ -260,8 +269,16 @@ tool_write_file <- function(path, content, append = FALSE, create_dirs = TRUE) { return(err(paste("Write error:", write_error))) } - ok(sprintf("%s %d byte(s) to %s", if (append) "Appended" else "Wrote", - nchar(content, type = "bytes"), path)) + summary <- sprintf("%s %d byte(s) to %s", + if (append) "Appended" else "Wrote", + nchar(content, type = "bytes"), path) + # Append mode writes after existing content; the on-disk file now + # has old_content + content. Reflect that in the displayed diff so + # the user sees what was actually written, not a misleading + # whole-file overwrite preview. + new_for_diff <- if (append) paste0(old_content, content) else content + diff <- compute_unified_diff(old_content, new_for_diff, path) + ok_with_diff(summary, diff) } #' Replace exact text in a file without rewriting the whole file manually. @@ -336,10 +353,13 @@ tool_replace_in_file <- function(path, old_text, new_text, all = FALSE, return(err(paste("Write error:", write_error))) } - ok(sprintf("Updated %s (%d replacement%s)", - path, - if (replace_all) match_count else 1L, - if ((if (replace_all) match_count else 1L) == 1L) "" else "s")) + replacements <- if (replace_all) match_count else 1L + summary <- sprintf("Updated %s (%d replacement%s)", + path, + replacements, + if (replacements == 1L) "" else "s") + diff <- compute_unified_diff(original, updated, path) + ok_with_diff(summary, diff) } # Search ---- diff --git a/R/turn.R b/R/turn.R index e71c0a2..44c1ef8 100644 --- a/R/turn.R +++ b/R/turn.R @@ -198,7 +198,7 @@ new_session <- function(channel = c("cli", "console", "matrix"), start <- Sys.time() - outcome_text <- function(kind, text, success) { + outcome_text <- function(kind, text, success, diff = NULL) { event <- list( call = call, decision = decision, @@ -208,7 +208,8 @@ new_session <- function(channel = c("cli", "console", "matrix"), elapsed_ms = as.numeric( difftime(Sys.time(), start, units = "secs") ) * 1000, - turn_number = session$turn_number + turn_number = session$turn_number, + diff = diff ) .fire_observers(session, event) text @@ -253,7 +254,8 @@ new_session <- function(channel = c("cli", "console", "matrix"), if (identical(internal_name, "exit_plan_mode") && isTRUE(success)) { session$plan_mode <- FALSE } - outcome_text("ran", .flatten_mcp_result(raw), success) + outcome_text("ran", .flatten_mcp_result(raw), success, + diff = raw$diff) } } @@ -366,6 +368,14 @@ observer_progress <- function() { return(invisible()) } + # File-edit tools attach a diff payload to their result. When + # present, render the colored hunks in place of the usual + # one-line "N lines" summary so the user can see what changed. + if (!is.null(event$diff)) { + render_tool_diff(event$diff) + return(invisible()) + } + summary <- cli_event_summary(event, width = 84L) detail <- if (length(summary$detail_lines) > 0L) { summary$detail_lines[1] diff --git a/R/utils.R b/R/utils.R index d228299..2006864 100644 --- a/R/utils.R +++ b/R/utils.R @@ -18,6 +18,29 @@ ok <- function(text) { list(content = list(list(type = "text", text = text))) } +#' Successful tool response with an attached human-facing diff payload. +#' +#' Thin builder used only by write-side tools (`write_file`, +#' `replace_in_file`). The base MCP result shape is unchanged so +#' external clients see the same `content` they always did; the extra +#' `diff` field is read by corteza's own CLI / chat display layer for +#' inline diff rendering. We deliberately keep `ok()` itself unaware of +#' diffs so the extension doesn't bleed into every tool author's +#' mental model of what a "successful" result looks like. +#' @param text LLM-facing summary string. +#' @param diff Diff payload from \code{compute_unified_diff()}, or NULL +#' to skip the field entirely (callers can pass through whatever +#' \code{compute_unified_diff()} returned without branching). +#' @return List formatted as MCP tool result with optional `diff` field. +#' @noRd +ok_with_diff <- function(text, diff = NULL) { + res <- ok(text) + if (!is.null(diff)) { + res$diff <- diff + } + res +} + #' Create error MCP tool response #' @param text Error message #' @return List formatted as MCP error result diff --git a/inst/bin/corteza b/inst/bin/corteza index cc90023..78f1da7 100755 --- a/inst/bin/corteza +++ b/inst/bin/corteza @@ -1026,12 +1026,19 @@ run_agent <- function(opts) { elapsed_ms <- as.numeric(difftime(Sys.time(), start_time, units = "secs")) * 1000 if (!failed) { - lines <- length(strsplit(text, "\n")[[1]]) - cat(sprintf(" %s⎿%s %d line%s in %dms\n", - color$dim, color$reset, - lines, - if (identical(lines, 1L)) "" else "s", - round(elapsed_ms))) + # File-edit tools (replace_in_file, write_file) attach a diff + # payload alongside the LLM-facing text. Render it inline so the + # user sees what changed instead of just a "N lines" summary. + if (!is.null(result$diff)) { + corteza:::render_tool_diff(result$diff) + } else { + lines <- length(strsplit(text, "\n")[[1]]) + cat(sprintf(" %s⎿%s %d line%s in %dms\n", + color$dim, color$reset, + lines, + if (identical(lines, 1L)) "" else "s", + round(elapsed_ms))) + } # Store in buffer for /last command tool_buffer_add(name, args, text) } @@ -1185,7 +1192,7 @@ run_agent <- function(opts) { } else { tool_buffer_add("git_diff", list(ref = material$target), material$diff) cat(sprintf("\n%sDiff against %s%s\n", color$cyan, material$target, color$reset)) - cat(material$diff, "\n") + cat(corteza:::colorize_diff(material$diff), "\n") } next } else if (cmd == "/review") { diff --git a/inst/tinytest/test_diff_render.R b/inst/tinytest/test_diff_render.R new file mode 100644 index 0000000..f260f84 --- /dev/null +++ b/inst/tinytest/test_diff_render.R @@ -0,0 +1,70 @@ +library(tinytest) + +# Identical inputs return NULL so the display layer can skip silently. +expect_null(corteza:::compute_unified_diff("a\nb\n", "a\nb\n", "x.R")) +expect_null(corteza:::compute_unified_diff("", "", "x.R")) + +# New file: empty old, non-empty new. Every content line should appear +# as an addition. +new_file <- corteza:::compute_unified_diff("", "hello\nworld\n", "new.R") +expect_false(is.null(new_file)) +expect_identical(new_file$path, "new.R") +expect_true(any(grepl("^\\+hello$", new_file$lines))) +expect_true(any(grepl("^\\+world$", new_file$lines))) +# Counts in summary reflect two additions, zero removals. +expect_true(grepl("Added 2 lines", new_file$summary, fixed = TRUE)) +expect_false(grepl("removed", new_file$summary, fixed = TRUE)) + +# File emptied: all removals. +emptied <- corteza:::compute_unified_diff("a\nb\n", "", "empty.R") +expect_false(is.null(emptied)) +expect_true(grepl("Removed 2 lines", emptied$summary, fixed = TRUE)) +expect_true(any(grepl("^-a$", emptied$lines))) +expect_true(any(grepl("^-b$", emptied$lines))) + +# Single-line change inside a longer file. +edited <- corteza:::compute_unified_diff( + "a\nb\nc\nd\n", + "a\nB\nc\nd\n", + "x.R" +) +expect_false(is.null(edited)) +expect_true(grepl("Added 1 line", edited$summary, fixed = TRUE)) +expect_true(grepl("removed 1 line", edited$summary, fixed = TRUE)) +expect_true(any(grepl("^@@", edited$lines))) +expect_true(any(grepl("^-b$", edited$lines))) +expect_true(any(grepl("^\\+B$", edited$lines))) + +# Headers carry the path we passed in, not the temp-file paths. +expect_true(any(grepl("^--- ", edited$lines))) +expect_true(any(grepl("^\\+\\+\\+ ", edited$lines))) +expect_true(any(grepl("x.R", edited$lines, fixed = TRUE))) + +# Missing trailing newline doesn't crash the diff; we get a payload back. +no_newline <- corteza:::compute_unified_diff("a\nb", "a\nB", "x.R") +expect_false(is.null(no_newline)) +expect_true(length(no_newline$lines) > 0L) + +# Fallback when `diff` is not on PATH: poison the binary cache so +# .diff_binary() returns "" without touching the package namespace +# (locked bindings prevent shimming the function itself). The result +# should still be a non-NULL payload, just with empty lines and a +# fallback flag. +cache <- corteza:::.diff_binary_cache +saved <- if (exists("value", envir = cache, inherits = FALSE)) { + get("value", envir = cache, inherits = FALSE) +} else NULL +on.exit({ + if (is.null(saved)) { + suppressWarnings(rm(list = "value", envir = cache)) + } else { + assign("value", saved, envir = cache) + } +}, add = TRUE) +assign("value", "", envir = cache) + +fb <- corteza:::compute_unified_diff("a\nb\nc\n", "a\nB\nc\n", "x.R") +expect_false(is.null(fb)) +expect_true(isTRUE(fb$fallback)) +expect_identical(fb$lines, character(0L)) +expect_true(nzchar(fb$summary)) From d0bf67c6aec9bb7b292a43c7cde63ff56b149b76 Mon Sep 17 00:00:00 2001 From: Troy Hernandez Date: Thu, 14 May 2026 14:20:26 -0500 Subject: [PATCH 2/4] Cap inline diff payload at 200 lines / 20000 chars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex flagged that compute_unified_diff() was unbounded: a 1000-line new file dumped 1003 lines into chat and CLI scrollback (and shipped the same volume across the callr worker boundary). Cap the lines vector with a "[diff truncated: N more lines]" marker so the user can see the size of the change without drowning in it. Summary counts still reflect the full diff so "Added 1000 lines" stays accurate. The new max_lines / max_chars params default to 200 / 20000 — tighter than the 300 / 60000 used by /diff because inline tool diffs live in chat scrollback rather than a one-shot command output. Both budgets are tunable; pass Inf to disable. --- R/diff-render.R | 67 +++++++++++++++++++++++++++++--- inst/tinytest/test_diff_render.R | 33 ++++++++++++++++ 2 files changed, 94 insertions(+), 6 deletions(-) diff --git a/R/diff-render.R b/R/diff-render.R index 6406c51..ab7a4f0 100644 --- a/R/diff-render.R +++ b/R/diff-render.R @@ -65,25 +65,38 @@ #' Returns NULL when the two inputs are byte-identical (signal to the #' caller that no diff display is warranted). When `diff` isn't on PATH, #' returns a fallback payload describing the size of the change without -#' the per-line content. +#' the per-line content. Large diffs are truncated to keep the payload +#' bounded — both for serialization across the callr worker boundary +#' and for chat scrollback hygiene — but the `summary` counts always +#' reflect the full diff. #' #' @param old_text Character scalar, prior file contents. Empty string #' means "new file". #' @param new_text Character scalar, new file contents. #' @param path Character scalar, the file path the diff describes; used #' for the `+++` / `---` header labels. +#' @param max_lines Cap on the number of diff lines retained for +#' display. Beyond this, a `[diff truncated: N more lines]` marker is +#' appended in place of the rest. Set to `Inf` to disable. +#' @param max_chars Cap on total characters across retained lines. +#' Tripped if a small number of long lines blow past the budget even +#' though `max_lines` hasn't. #' @return NULL if identical, else a list with: #' \itemize{ #' \item \code{path}: input path -#' \item \code{summary}: one-line summary string +#' \item \code{summary}: one-line summary string (always reflects +#' the full diff, not the truncated lines) #' \item \code{lines}: character vector of uncolored diff lines #' (header + hunks). May be empty when only the fallback -#' summary is available. +#' summary is available, or truncated for large diffs. #' \item \code{fallback}: logical TRUE when `diff` was unavailable #' and the payload is summary-only. +#' \item \code{truncated}: logical TRUE when `lines` was clipped. #' } #' @noRd -compute_unified_diff <- function(old_text, new_text, path) { +compute_unified_diff <- function(old_text, new_text, path, + max_lines = 200L, + max_chars = 20000L) { old_text <- old_text %||% "" new_text <- new_text %||% "" path <- path %||% "(unnamed)" @@ -148,10 +161,52 @@ compute_unified_diff <- function(old_text, new_text, path) { } counts <- .diff_summary_counts(res) + full_lines <- as.character(res) + clipped <- .clip_diff_lines(full_lines, max_lines, max_chars) list(path = path, summary = .diff_summary_line(counts$added, counts$removed), - lines = as.character(res), - fallback = FALSE) + lines = clipped$lines, + fallback = FALSE, + truncated = clipped$truncated) +} + +#' Clip a diff-line vector to the configured budgets. +#' +#' Returns a list with the (possibly clipped) `lines` and a `truncated` +#' flag. When the budget is busted we keep the first N lines and append +#' a `[diff truncated: N more lines]` marker so the reader knows there +#' was more. +#' @noRd +.clip_diff_lines <- function(lines, max_lines, max_chars) { + total <- length(lines) + if (total == 0L) { + return(list(lines = lines, truncated = FALSE)) + } + + keep <- min(total, as.integer(max_lines)) + head <- lines[seq_len(keep)] + + # Character budget: walk the kept lines until we'd exceed max_chars, + # then drop the rest. Counts newlines so the budget matches what + # the user actually sees. + if (is.finite(max_chars)) { + widths <- nchar(head, type = "bytes") + 1L + running <- cumsum(widths) + within <- which(running <= as.integer(max_chars)) + keep_chars <- if (length(within) == 0L) 0L else max(within) + if (keep_chars < length(head)) { + head <- head[seq_len(keep_chars)] + } + } + + truncated <- length(head) < total + if (truncated) { + dropped <- total - length(head) + head <- c(head, + sprintf("[diff truncated: %d more line%s]", + dropped, if (dropped == 1L) "" else "s")) + } + list(lines = head, truncated = truncated) } #' Render a diff payload to the terminal. diff --git a/inst/tinytest/test_diff_render.R b/inst/tinytest/test_diff_render.R index f260f84..21b6c8f 100644 --- a/inst/tinytest/test_diff_render.R +++ b/inst/tinytest/test_diff_render.R @@ -68,3 +68,36 @@ expect_false(is.null(fb)) expect_true(isTRUE(fb$fallback)) expect_identical(fb$lines, character(0L)) expect_true(nzchar(fb$summary)) + +# Restore the diff-binary cache so the truncation tests below run +# against the real `diff` again. +if (is.null(saved)) { + suppressWarnings(rm(list = "value", envir = cache)) +} else { + assign("value", saved, envir = cache) +} + +# Truncation: a big new file gets capped so chat scrollback / callr +# serialization don't blow up. Counts in summary still reflect the +# full diff. +big_new <- paste(sprintf("line %04d", seq_len(1000)), collapse = "\n") +big <- corteza:::compute_unified_diff("", big_new, "big.R", + max_lines = 50L) +expect_false(is.null(big)) +expect_true(isTRUE(big$truncated)) +# 50 retained lines plus the truncation marker. +expect_equal(length(big$lines), 51L) +expect_true(any(grepl("^\\[diff truncated:", big$lines))) +expect_true(grepl("Added 1000 lines", big$summary, fixed = TRUE)) + +# Char-budget trip: lots of small lines but tight max_chars. +tight <- corteza:::compute_unified_diff("", big_new, "big.R", + max_lines = 1000L, + max_chars = 200L) +expect_false(is.null(tight)) +expect_true(isTRUE(tight$truncated)) +expect_true(length(tight$lines) < 50L) + +# Default budgets leave a small diff untouched. +small <- corteza:::compute_unified_diff("a\nb\nc\n", "a\nB\nc\n", "x.R") +expect_false(isTRUE(small$truncated)) From fc9788acac14850f7f1406d9216637e6b9255bf9 Mon Sep 17 00:00:00 2001 From: Troy Hernandez Date: Thu, 14 May 2026 15:12:39 -0500 Subject: [PATCH 3/4] Consolidate console color/diff layout, render inline diffs with line numbers Codex flagged that the CLI script and corteza::chat() were making separate decisions about color support and inline-diff layout, which is exactly how the "CLI is colored but chat() isn't" drift just hit the user. Centralize the shared bits before the gap widens. Color policy is now sourced from R/cli-colors.R only: - ansi_supported() learns NO_COLOR / FORCE_COLOR / RSTUDIO so RStudio's R console (which is not a tty) and override env vars both work. - inst/bin/corteza deletes its private .ansi_supported() and inline 16-entry color list; both surfaces call corteza:::ansi_colors(). Inline diffs rendered through the same renderer everywhere: - render_tool_diff() now drops the redundant `--- /path` and `+++ /path` lines (the path is already in the tool-call title) and the `@@` hunk headers. It parses each hunk header for starting line numbers, walks the body, and emits one row per line as `NNNN +|-| content` with red on removals, green on additions, default on context. Truncation marker passes through dim. - Tool labels: replace_in_file -> "Update", write_file -> "Write" to match Claude Code phrasing and match the rendered diff context. This is the scoped step Codex suggested; a full console_ui() module that owns every cat() in the CLI is a follow-up, filed separately. --- R/cli-colors.R | 15 ++++ R/cli-ui.R | 8 +- R/diff-render.R | 127 +++++++++++++++++++++++++++++-- inst/bin/corteza | 51 ++----------- inst/tinytest/test_diff_render.R | 53 +++++++++++++ 5 files changed, 199 insertions(+), 55 deletions(-) diff --git a/R/cli-colors.R b/R/cli-colors.R index fa9c18a..5167f99 100644 --- a/R/cli-colors.R +++ b/R/cli-colors.R @@ -12,6 +12,21 @@ #' @return Single logical. #' @noRd ansi_supported <- function() { + # NO_COLOR / FORCE_COLOR are the conventional overrides; they win + # over auto-detection so users can force either side. RStudio's R + # console pane is not a tty (isatty(stdout()) is FALSE) but it + # does render ANSI escape sequences — without the RSTUDIO check, + # corteza::chat() would emit plain text in RStudio while the + # terminal CLI gets colored output. + if (nzchar(Sys.getenv("NO_COLOR"))) { + return(FALSE) + } + if (nzchar(Sys.getenv("FORCE_COLOR"))) { + return(TRUE) + } + if (identical(Sys.getenv("RSTUDIO"), "1")) { + return(TRUE) + } if (.Platform$OS.type == "windows") { return(any(nzchar(Sys.getenv(c("WT_SESSION", "ConEmuANSI", "TERM_PROGRAM"))))) diff --git a/R/cli-ui.R b/R/cli-ui.R index b6039d9..5407df0 100644 --- a/R/cli-ui.R +++ b/R/cli-ui.R @@ -119,9 +119,9 @@ cli_tool_label <- function(tool_name, long = FALSE) { run_r_script = "Run R Script", read_file = "Read File", "base::readLines" = "Read File", - write_file = "Write File", - "base::writeLines" = "Write File", - replace_in_file = "Replace in File", + write_file = "Write", + "base::writeLines" = "Write", + replace_in_file = "Update", list_files = "List Files", "base::list.files" = "List Files", grep_files = "Grep Files", @@ -144,7 +144,7 @@ cli_tool_label <- function(tool_name, long = FALSE) { run_r = "Run R code", run_r_script = "Run R script", read_file = "Read file", "base::readLines" = "Read file", write_file = "Write file", "base::writeLines" = "Write file", - replace_in_file = "Replace in file", list_files = "List files", + replace_in_file = "Update file", list_files = "List files", "base::list.files" = "List files", grep_files = "Search files", web_search = "Web search", fetch_url = "Fetch URL", git_status = "Git status", git_diff = "Git diff", diff --git a/R/diff-render.R b/R/diff-render.R index ab7a4f0..67da398 100644 --- a/R/diff-render.R +++ b/R/diff-render.R @@ -209,6 +209,113 @@ compute_unified_diff <- function(old_text, new_text, path, list(lines = head, truncated = truncated) } +#' Parse `@@ -A,B +X,Y @@` into the four integers, with sensible +#' defaults for omitted counts. Returns NULL if the header doesn't +#' match. Exposed as an internal helper so the renderer's hunk-walking +#' state can be unit-tested in isolation. +#' @noRd +.parse_hunk_header <- function(line) { + m <- regmatches(line, regexec( + "^@@ -([0-9]+)(?:,([0-9]+))? \\+([0-9]+)(?:,([0-9]+))? @@", + line))[[1]] + if (length(m) < 5L) { + return(NULL) + } + list(old_start = as.integer(m[2]), + old_count = if (nzchar(m[3])) as.integer(m[3]) else 1L, + new_start = as.integer(m[4]), + new_count = if (nzchar(m[5])) as.integer(m[5]) else 1L) +} + +#' Walk a unified-diff body and emit one rendered line per hunk row, +#' annotated with file-relative line numbers and colored +/-. +#' +#' Drops the `---` / `+++` file headers and the `@@` hunk headers — the +#' line numbers we inject make those redundant and the path already +#' appears in the surrounding tool-call header. Truncation marker lines +#' from \code{compute_unified_diff()} pass through dim. +#' @noRd +.format_diff_with_line_numbers <- function(diff_lines, palette) { + # Find the largest line number we'll need to print so we can right- + # align everything to a consistent width. Width 4 is the minimum so + # small files still produce a tidy two-digit-on-left look. + max_line <- 1L + for (ln in diff_lines) { + if (startsWith(ln, "@@")) { + h <- .parse_hunk_header(ln) + if (!is.null(h)) { + max_line <- max(max_line, + h$old_start + h$old_count - 1L, + h$new_start + h$new_count - 1L) + } + } + } + width <- max(nchar(as.character(max_line)), 4L) + pad_num <- function(n) formatC(n, width = width, flag = "") + + out <- character(0L) + old_line <- 0L + new_line <- 0L + in_hunk <- FALSE + + for (ln in diff_lines) { + if (startsWith(ln, "--- ") || startsWith(ln, "+++ ")) { + next + } + if (startsWith(ln, "diff --git") || startsWith(ln, "index ") || + startsWith(ln, "similarity ") || startsWith(ln, "rename ") || + startsWith(ln, "new file") || startsWith(ln, "deleted file")) { + next + } + if (startsWith(ln, "@@")) { + h <- .parse_hunk_header(ln) + if (!is.null(h)) { + old_line <- h$old_start + new_line <- h$new_start + in_hunk <- TRUE + } + next + } + if (startsWith(ln, "[diff truncated:")) { + out <- c(out, sprintf("%s%s%s", + palette$dim %||% "", + ln, + palette$reset %||% "")) + next + } + if (!in_hunk) { + next + } + if (startsWith(ln, "\\")) { + # `\ No newline at end of file` — drop, not useful for a + # human display. + next + } + body <- if (nchar(ln) >= 1L) substring(ln, 2L) else "" + prefix <- substring(ln, 1L, 1L) + rendered <- if (identical(prefix, "+")) { + new_no <- new_line + new_line <- new_line + 1L + sprintf("%s%s + %s%s", + palette$green %||% "", pad_num(new_no), body, + palette$reset %||% "") + } else if (identical(prefix, "-")) { + old_no <- old_line + old_line <- old_line + 1L + sprintf("%s%s - %s%s", + palette$red %||% "", pad_num(old_no), body, + palette$reset %||% "") + } else { + new_no <- new_line + old_line <- old_line + 1L + new_line <- new_line + 1L + sprintf("%s %s", pad_num(new_no), body) + } + out <- c(out, rendered) + } + out +} + #' Render a diff payload to the terminal. #' #' Used by both the CLI tool_handler in `inst/bin/corteza` and the @@ -217,6 +324,13 @@ compute_unified_diff <- function(old_text, new_text, path, #' Skips quietly when the payload is NULL (i.e., the underlying texts #' were identical and \code{compute_unified_diff()} returned nothing). #' +#' The rendered shape mirrors Claude Code's inline diff display: a +#' summary on the `⎿` line, then one row per kept line of the form +#' \code{NNNN [+|-| ] content} with red/green coloring on `+` and `-`. +#' The `---` / `+++` path headers and the `@@` hunk markers are +#' dropped — the line numbers replace the latter and the path already +#' appears in the surrounding tool-call title. +#' #' @param diff Payload from \code{compute_unified_diff()}, or NULL. #' @param palette Optional ANSI palette; tests force a specific palette. #' @param indent Leading indent string for each printed line; matches @@ -229,16 +343,17 @@ render_tool_diff <- function(diff, palette = ansi_colors(), indent = " ") { } summary <- diff$summary %||% "" if (nzchar(summary)) { - cat(sprintf("%s%s⎿ %s%s\n", - indent, palette$dim %||% "", summary, palette$reset %||% "")) + cat(sprintf("%s%s⎿ %s%s\n", + indent, palette$dim %||% "", summary, + palette$reset %||% "")) } if (isTRUE(diff$fallback) || length(diff$lines) == 0L) { return(invisible(TRUE)) } - body <- colorize_diff(paste(diff$lines, collapse = "\n"), palette) - body_lines <- strsplit(body, "\n", fixed = TRUE)[[1]] - for (ln in body_lines) { - cat(sprintf("%s%s\n", indent, ln)) + rendered <- .format_diff_with_line_numbers(diff$lines, palette) + body_indent <- paste0(indent, " ") + for (ln in rendered) { + cat(sprintf("%s%s\n", body_indent, ln)) } invisible(TRUE) } diff --git a/inst/bin/corteza b/inst/bin/corteza index 78f1da7..a65c745 100755 --- a/inst/bin/corteza +++ b/inst/bin/corteza @@ -241,51 +241,12 @@ cli_worker_close <- function(worker) { # Display helpers # ============================================================================ -# Detect whether the terminal interprets ANSI escape sequences. Modern -# Linux/macOS terminals and Windows Terminal / ConEmu / VS Code's -# integrated terminal do; the classic powershell.exe and cmd.exe -# consoles do not unless VT mode is explicitly enabled. Emitting -# escapes into a non-VT console leaves "[32m" garbage on screen, so -# we default OFF for classic Windows consoles and let NO_COLOR / -# FORCE_COLOR override either way. -.ansi_supported <- function() { - if (nzchar(Sys.getenv("NO_COLOR"))) return(FALSE) - if (nzchar(Sys.getenv("FORCE_COLOR"))) return(TRUE) - if (.Platform$OS.type != "windows") return(TRUE) - # Windows Terminal, ConEmu, VS Code, and anything that advertises a - # real terminal type all set one of these. - any(nzchar(Sys.getenv(c("WT_SESSION", "ConEmuANSI", "TERM_PROGRAM")))) -} - -.ansi <- .ansi_supported() - -color <- if (.ansi) { - list( - reset = "\033[0m", - bold = "\033[1m", - dim = "\033[2m", - red = "\033[31m", - green = "\033[32m", - yellow = "\033[33m", - blue = "\033[34m", - magenta = "\033[35m", - cyan = "\033[36m", - white = "\033[37m", - bright_red = "\033[91m", - bright_green = "\033[92m", - bright_yellow = "\033[93m", - bright_blue = "\033[94m", - bright_magenta = "\033[95m", - bright_cyan = "\033[96m" - ) -} else { - # No-op escapes for terminals that don't interpret VT sequences. - stats::setNames(as.list(rep("", 16L)), - c("reset", "bold", "dim", - "red", "green", "yellow", "blue", "magenta", "cyan", "white", - "bright_red", "bright_green", "bright_yellow", - "bright_blue", "bright_magenta", "bright_cyan")) -} +# Source of truth for ANSI support + the color palette is the package +# (R/cli-colors.R). The CLI used to keep its own copies of both, which +# meant chat() and the CLI could disagree about whether to emit colors +# (e.g. RStudio's R console is not a tty but does render ANSI). The +# duplicate is gone; both surfaces now share one decision. +color <- corteza:::ansi_colors() print_banner <- function(session_id = NULL) { cat(sprintf("\n%s corteza%s", color$cyan, color$reset)) diff --git a/inst/tinytest/test_diff_render.R b/inst/tinytest/test_diff_render.R index 21b6c8f..78d606c 100644 --- a/inst/tinytest/test_diff_render.R +++ b/inst/tinytest/test_diff_render.R @@ -101,3 +101,56 @@ expect_true(length(tight$lines) < 50L) # Default budgets leave a small diff untouched. small <- corteza:::compute_unified_diff("a\nb\nc\n", "a\nB\nc\n", "x.R") expect_false(isTRUE(small$truncated)) + +# Hunk-header parser: integer extraction with and without explicit +# counts. +h1 <- corteza:::.parse_hunk_header("@@ -10,5 +12,7 @@") +expect_identical(h1$old_start, 10L) +expect_identical(h1$old_count, 5L) +expect_identical(h1$new_start, 12L) +expect_identical(h1$new_count, 7L) + +h2 <- corteza:::.parse_hunk_header("@@ -3 +3 @@ extra context here") +expect_identical(h2$old_start, 3L) +expect_identical(h2$old_count, 1L) +expect_identical(h2$new_start, 3L) +expect_identical(h2$new_count, 1L) + +expect_null(corteza:::.parse_hunk_header("not a hunk header")) + +# .format_diff_with_line_numbers: drops --- / +++ / @@ rows, tags each +# remaining line with its file-relative number, and signs +/-/space. +# Use a no-op palette so the assertions don't have to mention ANSI +# escapes. +no_color <- stats::setNames(as.list(rep("", 16L)), + c("reset", "bold", "dim", "red", "green", + "yellow", "blue", "magenta", "cyan", "white", + "bright_red", "bright_green", "bright_yellow", + "bright_blue", "bright_magenta", "bright_cyan")) + +# Single-line replacement at line 2 of a 4-line file: expect one +# context line, then `- old`, `+ new`, then more context. +edited2 <- corteza:::compute_unified_diff("a\nb\nc\nd\n", + "a\nB\nc\nd\n", "x.R") +rendered <- corteza:::.format_diff_with_line_numbers(edited2$lines, no_color) +# No `---`, `+++`, or `@@` lines survive. +expect_false(any(grepl("^---|^\\+\\+\\+|^@@", rendered))) +# Body lines exist. +expect_true(length(rendered) > 0L) +# Line 2 appears with `-` for old and `+` for new, with matching line +# numbers (both at position 2 in their respective files). +expect_true(any(grepl("^\\s*2 - b$", rendered))) +expect_true(any(grepl("^\\s*2 \\+ B$", rendered))) +# Surrounding context lines show their new-file line numbers with no +# prefix sign (three spaces between the number and content). +expect_true(any(grepl("^\\s*1 a$", rendered))) +expect_true(any(grepl("^\\s*3 c$", rendered))) + +# New-file path: every body line is an addition, numbered from 1. +new_lines <- corteza:::compute_unified_diff("", "alpha\nbeta\ngamma\n", "n.R") +rn <- corteza:::.format_diff_with_line_numbers(new_lines$lines, no_color) +expect_true(any(grepl("^\\s*1 \\+ alpha$", rn))) +expect_true(any(grepl("^\\s*2 \\+ beta$", rn))) +expect_true(any(grepl("^\\s*3 \\+ gamma$", rn))) +# No context, no removals. +expect_false(any(grepl(" - ", rn))) From a4cbaee9574e23a2160d266a8969c8acc2ca4377 Mon Sep 17 00:00:00 2001 From: Troy Hernandez Date: Thu, 14 May 2026 15:26:32 -0500 Subject: [PATCH 4/4] Bump version to 0.6.6.2 --- DESCRIPTION | 2 +- NEWS.md | 41 +++++++++++++++++++++++++++-------------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index e0f0fa9..4fb94d8 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: corteza Title: AI Agent Runtime -Version: 0.6.6.1 +Version: 0.6.6.2 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), email = "troy@cornball.ai", diff --git a/NEWS.md b/NEWS.md index 58e6758..d52b906 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,30 @@ +# corteza 0.6.6.2 + +## Inline diffs on file edits + +* `replace_in_file` and `write_file` now attach a unified-diff payload + to their MCP result. The CLI and `corteza::chat()` render it inline + in the tool-call output as `⎿ Added N, removed M` followed by one + row per kept line (`NNNN +|-| content` with red/green color) instead + of the prior `N lines in Xms` summary. The LLM-facing result text is + unchanged — the diff is for the human reading the terminal. +* Tool labels renamed for clarity: `replace_in_file` → "Update", + `write_file` → "Write" (matches the inline-diff phrasing). +* Diff generation shells out to the system `diff -u`. If `diff` isn't + on `PATH` the tool degrades to a one-line size summary rather than + failing. Diff payload is capped at 200 lines / 20000 chars with a + `[diff truncated: N more lines]` marker so big writes don't dump + thousands of lines into chat scrollback. +* The `/diff` slash command's output is also ANSI-colored. + +## Console color policy is shared + +* `ansi_supported()` / `ansi_colors()` in the package are now the + single source of truth for both `corteza::chat()` and the + `~/bin/corteza` CLI. RStudio's R console (which is not a tty) is now + correctly detected as ANSI-capable, and `NO_COLOR` / `FORCE_COLOR` + overrides work in both surfaces. + # corteza 0.6.6.1 ## Interrupt key @@ -18,20 +45,6 @@ intercepts Ctrl+C for copy). In the terminal `~/bin/corteza` CLI it's **Ctrl+C** — terminals send raw `^[` for Esc, which is not a signal. -## Inline diffs on file edits - -* `replace_in_file` and `write_file` now attach a unified-diff payload - to their MCP result. The CLI and `corteza::chat()` render it inline - in the tool-call output (`Added N, removed M`, then colored hunks) - instead of the prior `N lines in Xms` summary, so it's obvious what - the agent actually changed. The LLM-facing result text is unchanged - — the diff is for the human reading the terminal. -* Diff generation shells out to the system `diff -u`. If `diff` isn't - on `PATH` the tool degrades to a one-line size summary rather than - failing. -* The `/diff` slash command's output is now ANSI-colored (same - red/green/cyan palette as `git diff --color=always`). - ## Other * `load_saber_briefing()` now wraps `saber::briefing()` in