Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions webviews/src/agent-session/acpmux/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
83 changes: 83 additions & 0 deletions webviews/src/agent-session/acpmux/conversation/TurnFooter.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
29 changes: 23 additions & 6 deletions webviews/src/agent-session/acpmux/conversation/TurnRows.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -434,22 +451,22 @@ export function TurnFooter({ row }: { row: AcpmuxRow }) {
<MessageFooterRow className="cv-turn-footer-row">
<span className="cv-turn-action-group">
{!messages && copy && <MessageCopy text={copy} />}
{failedRetry && (
{retryAction && (
<button
type="button"
className="cv-iconbtn cv-iconbtn--compact"
aria-label={t("turn.retry")}
title={t("turn.retryLabel")}
title={retryTitle}
disabled={retrying}
onClick={failedRetry}
onClick={retryAction}
>
<Retry size={ROW_ICON} />
</button>
)}
{failed && retry && prompt && retryWith && retryWith.models.length > 1 && (
{modelRetry && (
<RetryModelMenu
retryWith={retryWith}
onPick={(model) => 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) && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(/>[^<]*</g, "><")).toBe(plain.replace(/>[^<]*</g, "><"));
});

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(
<TurnActionsContext.Provider value={{ retry: () => undefined }}>
<TurnFooter row={row} />
</TurnActionsContext.Provider>,
);
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(
<TurnActionsContext.Provider value={{ retry: () => undefined }}>
<TurnFooter row={row} />
</TurnActionsContext.Provider>,
);
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(
<TurnActionsContext.Provider value={{ retry: () => undefined }}>
<TurnFooter row={row} />
</TurnActionsContext.Provider>,
);
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(
<TurnActionsContext.Provider value={{ retry: () => undefined, retryWith }}>
<TurnFooter row={row} />
</TurnActionsContext.Provider>,
);
expect(markup).toContain(`aria-label="${translate("turn.retry")}"`);
expect(markup).toContain(`aria-label="${translate("turn.retryWith")}"`);
});
66 changes: 64 additions & 2 deletions webviews/src/agent-session/acpmux/conversation/turns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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<string, { diffs: AcpmuxFileDiff[]; uncertain: boolean }>();
const keysByBasename = new Map<string, Set<string>>();
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<string>();
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
Expand Down Expand Up @@ -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,
Expand All @@ -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).
Expand All @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions webviews/src/agent-session/acpmux/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading