diff --git a/webviews/src/agent-session/acpmux/Localizable.xcstrings b/webviews/src/agent-session/acpmux/Localizable.xcstrings index d1e06d05913c..0f3887afc074 100644 --- a/webviews/src/agent-session/acpmux/Localizable.xcstrings +++ b/webviews/src/agent-session/acpmux/Localizable.xcstrings @@ -55563,6 +55563,33 @@ } } }, + "turn.retryChangedFiles": { + "comment": "Agent pane: Retry warns when the response turn changed files, so retrying is visible alongside the reply (ChatGPT parity).", + "extractionState": "manual", + "localizations": { + "ar": { "stringUnit": { "state": "needs_review", "value": "إعادة محاولة الرد (غيّرت هذه الجولة ملفات)" } }, + "bs": { "stringUnit": { "state": "needs_review", "value": "Ponovi odgovor (ova runda je promijenila datoteke)" } }, + "da": { "stringUnit": { "state": "needs_review", "value": "Prøv svaret igen (denne tur ændrede filer)" } }, + "de": { "stringUnit": { "state": "needs_review", "value": "Antwort wiederholen (dieser Durchlauf hat Dateien geändert)" } }, + "en": { "stringUnit": { "state": "translated", "value": "Retry response (this turn changed files)" } }, + "es": { "stringUnit": { "state": "needs_review", "value": "Reintentar la respuesta (este turno cambió archivos)" } }, + "fr": { "stringUnit": { "state": "needs_review", "value": "Réessayer la réponse (ce tour a modifié des fichiers)" } }, + "it": { "stringUnit": { "state": "needs_review", "value": "Riprova la risposta (questo turno ha modificato dei file)" } }, + "ja": { "stringUnit": { "state": "needs_review", "value": "応答を再試行（このターンでファイルが変更されました）" } }, + "km": { "stringUnit": { "state": "needs_review", "value": "ព្យាយាមចម្លើយម្ដងទៀត (វេននេះបានផ្លាស់ប្តូរឯកសារ)" } }, + "ko": { "stringUnit": { "state": "needs_review", "value": "응답 다시 시도 (이 턴에서 파일이 변경됨)" } }, + "nb": { "stringUnit": { "state": "needs_review", "value": "Prøv svaret på nytt (denne runden endret filer)" } }, + "pl": { "stringUnit": { "state": "needs_review", "value": "Ponów odpowiedź (ta tura zmieniła pliki)" } }, + "pt-BR": { "stringUnit": { "state": "needs_review", "value": "Tentar resposta novamente (esta rodada alterou arquivos)" } }, + "ru": { "stringUnit": { "state": "needs_review", "value": "Повторить ответ (этот ход изменил файлы)" } }, + "th": { "stringUnit": { "state": "needs_review", "value": "ลองคำตอบอีกครั้ง (เทิร์นนี้เปลี่ยนไฟล์)" } }, + "tr": { "stringUnit": { "state": "needs_review", "value": "Yanıtı yeniden dene (bu tur dosyaları değiştirdi)" } }, + "uk": { "stringUnit": { "state": "needs_review", "value": "Повторити відповідь (цей хід змінив файли)" } }, + "vi": { "stringUnit": { "state": "needs_review", "value": "Thử lại phản hồi (lượt này đã thay đổi tệp)" } }, + "zh-Hans": { "stringUnit": { "state": "needs_review", "value": "重试回复（此轮更改了文件）" } }, + "zh-Hant": { "stringUnit": { "state": "needs_review", "value": "重試回覆（此輪變更了檔案）" } } + } + }, "turn.retryLabel": { "comment": "Agent pane", "extractionState": "manual", diff --git a/webviews/src/agent-session/acpmux/conversation/TurnFooter.test.tsx b/webviews/src/agent-session/acpmux/conversation/TurnFooter.test.tsx index cbc265ebf928..abc852a08916 100644 --- a/webviews/src/agent-session/acpmux/conversation/TurnFooter.test.tsx +++ b/webviews/src/agent-session/acpmux/conversation/TurnFooter.test.tsx @@ -140,6 +140,89 @@ test("the chat's last turn that ended on work has a row of its own, 8 under the expect(isEmptyFooter(later.find((row) => row.kind === "turnSummary")!)).toBe(true); }); +test("the retry hint only marks a completed edit with a known non-empty patch", () => { + const prompt = { id: "p", version: 1, at: 0, kind: "user", text: "Update the file" } as AcpmuxRow; + const summary = { id: "s", version: 1, at: 3, kind: "turnSummary", status: "completed" } as AcpmuxRow; + const edit = ( + status: string, + oldText?: string, + newText = "after", + fragment?: true, + canonical?: string, + path = "/repo/file.ts", + ): AcpmuxRow => ({ + id: `edit-${status}-${oldText ?? "unknown"}`, + version: 1, + at: 2, + kind: "activity", + items: [ + { + kind: "tool", + text: "", + tool: { + id: "tool", + title: "Edit file.ts", + kind: "edit", + status, + diffs: [ + { + path, + ...(oldText === undefined ? {} : { oldText }), + newText, + ...(fragment && { fragment }), + ...(canonical && { canonical }), + }, + ], + }, + }, + ], + }); + const changed = turnView([prompt, edit("completed", "before"), summary], new Set(), { now: 10_000 }).find( + (row) => row.kind === "turnSummary", + ); + expect(changed?.changedFiles).toBe(true); + + for (const row of [ + turnView([prompt, edit("completed", "same", "same"), summary], new Set()).find( + (item) => item.kind === "turnSummary", + ), + turnView([prompt, edit("failed", "before"), summary], new Set()).find((item) => item.kind === "turnSummary"), + turnView([prompt, edit("pending", "before"), summary], new Set()).find((item) => item.kind === "turnSummary"), + turnView([prompt, edit("completed", undefined, "new file"), summary], new Set()).find( + (item) => item.kind === "turnSummary", + ), + turnView( + [prompt, edit("completed", "before", "after"), edit("completed", "after", "before"), summary], + new Set(), + ).find((item) => item.kind === "turnSummary"), + turnView( + [prompt, edit("completed", "before", "after"), edit("completed", undefined, "before"), summary], + new Set(), + ).find((item) => item.kind === "turnSummary"), + turnView( + [prompt, edit("completed", undefined, "after"), edit("completed", "after", "before"), summary], + new Set(), + ).find((item) => item.kind === "turnSummary"), + turnView( + [prompt, edit("completed", "before", "after"), edit("completed", "after", "before", true), summary], + new Set(), + ).find((item) => item.kind === "turnSummary"), + turnView( + [ + prompt, + edit("completed", "before", "after", undefined, undefined, "file.ts"), + edit("completed", "after", "before", undefined, "/repo/file.ts", "file.ts"), + summary, + ], + new Set(), + ).find((item) => item.kind === "turnSummary"), + turnView([prompt, edit("completed", undefined, "partial", true), summary], new Set()).find( + (item) => item.kind === "turnSummary", + ), + ]) + expect(row?.changedFiles).toBeUndefined(); +}); + test("a work-only last turn that said nothing has nothing to copy and stays empty", () => { expect(isEmptyFooter(footer({ retryable: true }))).toBe(true); expect(isEmptyFooter(footer({ retryable: true, turnCopy: [{ text: "I'll check." }] }))).toBe(false); diff --git a/webviews/src/agent-session/acpmux/conversation/TurnRows.tsx b/webviews/src/agent-session/acpmux/conversation/TurnRows.tsx index b0de52c7acae..696e0f7d934e 100644 --- a/webviews/src/agent-session/acpmux/conversation/TurnRows.tsx +++ b/webviews/src/agent-session/acpmux/conversation/TurnRows.tsx @@ -379,6 +379,23 @@ export function TurnFooter({ row }: { row: AcpmuxRow }) { : retry && prompt && row.retryRowId ? () => retry(row.retryRowId!, prompt) : undefined; + // The last completed turn keeps its prompt and retry target on the footer as well as under the + // prompt bubble. ChatGPT leaves the action with the answer, which makes a finished reply + // recoverable without scrolling back to its prompt. `retryable` is only set on that last turn + // (turns.ts), so older successful replies never gain another action row. + const lastTurnRetry = + !failed && retry && row.retryable === true && prompt && row.retryRowId + ? { rowId: row.retryRowId, prompt, run: () => retry(row.retryRowId!, prompt) } + : undefined; + const retryAction = failedRetry ?? lastTurnRetry?.run; + const retryTitle = row.changedFiles ? t("turn.retryChangedFiles") : t("turn.retryLabel"); + // The model picker follows the same rule as Retry: it is available for the last turn, whether + // that turn ended successfully or failed. A view-only turn has no retry callback, so it stays + // out of both controls. + const modelRetry = + retryWith && retryWith.models.length > 1 && retry && row.retryable === true && prompt && row.retryRowId + ? { retryWith, rowId: row.retryRowId, prompt } + : undefined; // The card's lead (errorRecovery.ts): Continue when the turn had done work, Retry when nothing // ran; it shares the footer Retry's one-at-a-time state and refusal note. A sign-in failure's comes // only after its Sign in again, so only the card offers it (signInRetry), never the native command. @@ -434,22 +451,22 @@ export function TurnFooter({ row }: { row: AcpmuxRow }) { {!messages && copy && } - {failedRetry && ( + {retryAction && ( )} - {failed && retry && prompt && retryWith && retryWith.models.length > 1 && ( + {modelRetry && ( retryWith.retry(row.retryRowId ?? "", prompt, model)} + retryWith={modelRetry.retryWith} + onPick={(model) => modelRetry.retryWith.retry(modelRetry.rowId, modelRetry.prompt, model)} /> )} {!messages && fork && seq !== undefined && (forkAny || seq === forkSeq) && ( diff --git a/webviews/src/agent-session/acpmux/conversation/footerActions.test.tsx b/webviews/src/agent-session/acpmux/conversation/footerActions.test.tsx index f562dcd541e6..d16164b08837 100644 --- a/webviews/src/agent-session/acpmux/conversation/footerActions.test.tsx +++ b/webviews/src/agent-session/acpmux/conversation/footerActions.test.tsx @@ -4,9 +4,11 @@ // reply that opened pull requests draws the same row as one that did not, and no PR link joins it. import { expect, test } from "bun:test"; import { renderToStaticMarkup } from "react-dom/server"; +import { translate } from "../i18n"; import type { AcpmuxRow } from "../model"; import { ReplyActions } from "./MessageActions"; import { TurnFooter } from "./TurnRows"; +import { TurnActionsContext } from "./turnActions"; const summary: AcpmuxRow = { id: "summary", version: 1, at: 1_000, kind: "turnSummary", status: "completed" }; const reply = (text: string) => ({ id: "reply", version: 1, at: 1_000, kind: "assistant", text }) as AcpmuxRow; @@ -28,3 +30,71 @@ test("a reply's action row starts with its buttons and draws the same whatever t expect(named).not.toContain("acpmux-pr"); expect(named.replace(/>[^<]*<")).toBe(plain.replace(/>[^<]*<")); }); + +test("the latest completed turn keeps Retry beside its reply", () => { + const row: AcpmuxRow = { + ...summary, + text: "Done.", + prompt: "Finish the task", + retryRowId: "prompt-1", + retryable: true, + }; + const markup = renderToStaticMarkup( + undefined }}> + + , + ); + expect(markup).toContain(`aria-label="${translate("turn.retry")}"`); +}); + +test("Retry calls out a known file change without changing the action", () => { + const row: AcpmuxRow = { + ...summary, + text: "Done.", + prompt: "Finish the task", + retryRowId: "prompt-1", + retryable: true, + changedFiles: true, + }; + const markup = renderToStaticMarkup( + undefined }}> + + , + ); + expect(markup).toContain(`title="${translate("turn.retryChangedFiles")}"`); +}); + +test("an earlier completed turn does not gain the latest-turn Retry", () => { + const row: AcpmuxRow = { ...summary, text: "Done.", prompt: "Finish the task", retryRowId: "prompt-1" }; + const markup = renderToStaticMarkup( + undefined }}> + + , + ); + expect(markup).not.toContain(`aria-label="${translate("turn.retry")}"`); +}); + +test("a failed turn keeps its Retry and Retry with model actions", () => { + const retryWith = { + models: [ + { id: "claude-sonnet", name: "Claude Sonnet" }, + { id: "claude-opus", name: "Claude Opus" }, + ], + retry: () => undefined, + }; + const row: AcpmuxRow = { + ...summary, + status: "failed", + error: "The request failed", + prompt: "Finish the task", + retryRowId: "prompt-1", + retryable: true, + }; + const markup = renderToStaticMarkup( + undefined, retryWith }}> + + , + ); + expect(markup).toContain(`aria-label="${translate("turn.retry")}"`); + expect(markup).toContain(`aria-label="${translate("turn.retryWith")}"`); +}); diff --git a/webviews/src/agent-session/acpmux/conversation/turns.ts b/webviews/src/agent-session/acpmux/conversation/turns.ts index 6f47940fa6ab..310df18aa0ef 100644 --- a/webviews/src/agent-session/acpmux/conversation/turns.ts +++ b/webviews/src/agent-session/acpmux/conversation/turns.ts @@ -7,7 +7,14 @@ // Stop-gap seam: acpmux owns the turn structure (spec acp-ui.md, OWNERSHIP-PRINCIPLES.md). When // acpmux serves turn-structured rows (`acp.view.subscribe`, `_acpmux/view`), `turnView` becomes // a field-for-field mapping of them and nothing else in the pane changes. -import { TURN_RETRY, type AcpmuxActivity, type AcpmuxPlanEntry, type AcpmuxRow } from "../model"; +import { + pathKey, + TURN_RETRY, + type AcpmuxActivity, + type AcpmuxFileDiff, + type AcpmuxPlanEntry, + type AcpmuxRow, +} from "../model"; import type { Folds } from "./viewFolds"; import { continuesTurn, opensTurn } from "../turnStart"; import { findRowIndex } from "./chatFind"; @@ -150,6 +157,57 @@ export const endsChatOnWork = (row: AcpmuxRow) => row.retryable === true && (row const isEditTool = (tool: { kind?: string } | undefined) => tool?.kind === "edit" || tool?.kind === "fileChange"; const isEdit = (row: AcpmuxRow) => row.kind === "activity" && (row.items ?? []).some((item) => isEditTool(item.tool)); +/// A completed whole-file tool diff proves a file changed only when both before and after text are +/// known. A path-only or unknown-old write can be a no-op (even when its replacement text is +/// non-empty), so it stays neutral for the reply Retry hint. Comparing the first before and final +/// after also keeps an edit that is reverted within one turn neutral (A -> B -> A). Fragment edits +/// are deliberately excluded because their net file contents are not available here. +const turnChangedFiles = (rows: readonly AcpmuxRow[]) => { + const byPath = new Map(); + const keysByBasename = new Map>(); + for (const row of rows) { + if (row.kind !== "activity") continue; + for (const item of row.items ?? []) { + const tool = item.tool; + if (tool?.status !== "completed" || !isEditTool(tool)) continue; + for (const diff of tool.diffs ?? []) { + const key = pathKey(diff); + const entry = byPath.get(key) ?? { diffs: [], uncertain: false }; + // An unknown-old or fragment write can hide a no-op or a later revert. Keep the path + // marked uncertain instead of filtering that event out and claiming a net change from + // the remaining diffs. + if (diff.fragment === true || diff.oldText === undefined) entry.uncertain = true; + else entry.diffs.push(diff); + byPath.set(key, entry); + const basename = diff.path.slice(diff.path.lastIndexOf("/") + 1); + const keys = keysByBasename.get(basename) ?? new Set(); + keys.add(key); + keysByBasename.set(basename, keys); + } + } + } + // A path can gain its canonical spelling after an earlier event (`file.ts` then + // `/repo/file.ts` with canonical `/repo/file.ts`). If a basename has multiple path keys, keep + // all of those histories neutral: the net chain cannot be proven without a path resolver. + for (const keys of keysByBasename.values()) { + if (keys.size < 2) continue; + for (const key of keys) { + const entry = byPath.get(key); + if (entry) entry.uncertain = true; + } + } + return [...byPath.values()].some(({ diffs, uncertain }) => { + if (uncertain) return false; + const first = diffs[0]; + const last = diffs[diffs.length - 1]; + if (!first || !last || first.oldText === undefined) return false; + // A missing chain link means the file's net contents are not proven by this snapshot. + for (let index = 1; index < diffs.length; index++) + if (diffs[index]!.oldText !== diffs[index - 1]!.newText) return false; + return first.oldText !== last.newText; + }); +}; + /// The rows to draw. `expanded` holds the ids of open disclosures; `working` says the last /// turn is still running (the snapshot's `isWorking`); `now` dates the turns (timestamps.ts); /// `clockOffset` (the snapshot's `clockOffsetMs`) moves the running turn's start onto this @@ -955,6 +1013,9 @@ function shapeTurn( const noticeStarted = user.incoming?.kind === "notice" || user.incoming?.kind === "inbox"; const continuable = offers && lastSummary < 0 && isFailed(summary) && (noticeStarted || body.some((row) => row.kind !== "user")); + // The hint is only rendered for a settled retry target. Avoid scanning the turn body on each + // live snapshot/token while the turn is still running. + const changedFiles = retryable && !live ? turnChangedFiles(body) : false; shaped.push({ ...summary, folded, @@ -964,6 +1025,7 @@ function shapeTurn( ...(resumable && { resumable }), ...(retryable && { retryable }), ...(continuable && { continuable }), + ...(changedFiles && { changedFiles: true as const }), ...(answer && { answerId: answer.id }), // Retry sends the prompt as the agent got it: an incoming message with its sender. A // notification, or acpmux's inbox notice, is not a prompt to send again (incomingMessage.ts). @@ -976,7 +1038,7 @@ function shapeTurn( retryAttachments: user.retryAttachments, }), // A turn is either stopped or failed, so Continue's bit serves both. - version: version * 4 + (retryable ? 1 : 0) + (resumable || continuable ? 2 : 0), + version: version * 8 + (retryable ? 1 : 0) + (resumable || continuable ? 2 : 0) + (changedFiles ? 4 : 0), }); // Anything after the summary (late tool updates, or a turn the agent started on its own) // draws as it came. diff --git a/webviews/src/agent-session/acpmux/model.ts b/webviews/src/agent-session/acpmux/model.ts index 7e263f68d32c..42076ecde790 100644 --- a/webviews/src/agent-session/acpmux/model.ts +++ b/webviews/src/agent-session/acpmux/model.ts @@ -487,6 +487,9 @@ export type AcpmuxRow = { /// A turn summary of the chat's last turn, which a failed turn's Retry asks acpmux to retry /// (turns.ts), even when its prompt is an attempt acpmux sent and draws nothing. retryable?: boolean; + /// The turn's completed edit diff proves it changed files. Retry keeps this hint in its tooltip; + /// missing or incomplete checkpoint data stays neutral (conversation/turns.ts). + changedFiles?: true; /// A turn footer's answer row (turns.ts), whose right-click menu the footer's More opens. answerId?: string; /// A turn footer's whole turn for its Copy (conversation/turnCopy.ts): the agent's replies and each