diff --git a/desktop/main/agent-client.ts b/desktop/main/agent-client.ts index 4b71ed010..4ef35ad43 100644 --- a/desktop/main/agent-client.ts +++ b/desktop/main/agent-client.ts @@ -1731,7 +1731,7 @@ export class AgentClient extends EventEmitter { * agent without the route answers its unknown-route 404 with no code and is * reported as unsupported, so the window falls back to resending. */ - async rollbackLastTurn(sessionId: string, expectText?: string, expectPriorQuestions?: number): Promise<{ + async rollbackLastTurn(sessionId: string, expectText?: string, expectPriorQuestions?: number, expectUnanswered?: boolean): Promise<{ ok: boolean; supported: boolean; text?: string; code?: string; error?: string; }> { try { @@ -1746,6 +1746,9 @@ export class AgentClient extends EventEmitter { body: JSON.stringify({ ...(typeof expectText === "string" && expectText ? { expectText } : {}), ...(Number.isInteger(expectPriorQuestions) && (expectPriorQuestions as number) >= 0 ? { expectPriorQuestions } : {}), + // ATO-1420: the window shows no answer to it, so a stored question + // with one is not it (an agent without the field ignores it). + ...(expectUnanswered === true ? { expectUnanswered: true } : {}), }), signal: AbortSignal.timeout(30_000), }); diff --git a/desktop/main/main.ts b/desktop/main/main.ts index ed9953450..3f816851c 100644 --- a/desktop/main/main.ts +++ b/desktop/main/main.ts @@ -2421,12 +2421,13 @@ function wireIpc(client: AgentClient): void { }); // ATO-1299: "Regenerate" under the last reply takes the turn back first. ipcMain.handle("agent:rollbackLastTurn", (_event, payload: unknown) => { - const { sessionId, expectText, expectPriorQuestions } = (payload ?? {}) as { sessionId?: unknown; expectText?: unknown; expectPriorQuestions?: unknown }; + const { sessionId, expectText, expectPriorQuestions, expectUnanswered } = (payload ?? {}) as { sessionId?: unknown; expectText?: unknown; expectPriorQuestions?: unknown; expectUnanswered?: unknown }; if (typeof sessionId !== "string" || !sessionId) return { ok: false, supported: true, code: "session_not_found", error: "This chat is not saved yet" }; return client.rollbackLastTurn( sessionId, typeof expectText === "string" ? expectText : undefined, typeof expectPriorQuestions === "number" ? expectPriorQuestions : undefined, + expectUnanswered === true, ); }); diff --git a/desktop/preload/preload.ts b/desktop/preload/preload.ts index 0d4d7b228..697f5f750 100644 --- a/desktop/preload/preload.ts +++ b/desktop/preload/preload.ts @@ -444,8 +444,8 @@ contextBridge.exposeInMainWorld("atomic", { /** ATO-259: summarize the chat's older history now (POST /api/sessions/{id}/compact). */ compactSession: (sessionId: string) => ipcRenderer.invoke("agent:compactSession", { sessionId }), /** ATO-1299: remove the chat's last question and answer (POST /api/sessions/{id}/rollback-last-turn). */ - rollbackLastTurn: (sessionId: string, expectText?: string, expectPriorQuestions?: number) => - ipcRenderer.invoke("agent:rollbackLastTurn", { sessionId, expectText, expectPriorQuestions }), + rollbackLastTurn: (sessionId: string, expectText?: string, expectPriorQuestions?: number, expectUnanswered?: boolean) => + ipcRenderer.invoke("agent:rollbackLastTurn", { sessionId, expectText, expectPriorQuestions, expectUnanswered }), /** Item 7A — add a model from Hugging Face (the port lives in main/huggingface.ts). */ hfResolve: (ref: string) => ipcRenderer.invoke("cli:hfResolve", ref), diff --git a/desktop/renderer/renderer.js b/desktop/renderer/renderer.js index a6cc85f4c..44ac5e3ff 100644 --- a/desktop/renderer/renderer.js +++ b/desktop/renderer/renderer.js @@ -3190,7 +3190,7 @@ function resendUser(id) { /* ATO-1299: whether the agent can take a turn back. Left true until an agent answers with its unknown-route 404; then every Regenerate resends. */ -const ROLLBACK = {supported: true, pending: false, stash: new Map()}; +const ROLLBACK = {supported: true, pending: false, stash: new Map(), cleared: {sid: null, n: 0}}; /* A question the agent took back while its chat was not on screen (the person switched chats during the call): it lives only here until that chat is opened again, then waits in the composer (openSession). */ @@ -3206,9 +3206,43 @@ function rollbackStashBack(sid) { ctxDraftChanged(); toast('Your question is back in the box', 'Its old answer was taken back. Send it when you are ready.'); } -/** How many questions (steers not counted) come before `m` in the log. */ +/* ATO-1420: Clear Transcript empties the window, not the agent's chat, so + the questions it took off screen still come before the next ones there. + Kept per chat in ROLLBACK.cleared and added by priorQuestions. A question + that never reached the agent is counted too: a count too high only makes + the agent refuse (and the window resend), never take back another one. + openSession draws the whole chat again and starts it from 0. */ +function rollbackNoteCleared(before, after, sid) { + const asks = (log) => (log || []).filter((x) => x.k === 'user' && !x.steered).length; + const hidden = Math.max(0, asks(before) - asks(after)); + const was = ROLLBACK.cleared; + const key = sid || null; + ROLLBACK.cleared = {sid: key, n: (was.sid === key ? was.n : 0) + hidden}; +} +/* Clear Transcript's own step (act 'clear'): the window empties (a turn + running on screen keeps its rows) and the questions it took off screen are + noted for priorQuestions, as the agent still has them (ATO-1420). */ +function clearTranscriptNow() { + const before = S.log; + S.log = liveClearedLog(); S.history = []; + rollbackNoteCleared(before, S.log, S.agentSession); +} +/* openSession's part of the ATO-1420 count: the whole chat is drawn again, so + it starts from 0. A Clear Transcript that landed while the chat loaded + (ATO-131) clears it as act 'clear' does (a running turn's own rows stay, + `rebuilt`) and notes what it hid. */ +function rollbackOnSessionOpened(sid, clearedWhileLoading, rebuilt) { + ROLLBACK.cleared = {sid: null, n: 0}; + if (!clearedWhileLoading) return; + const loaded = S.log; + S.log = rebuilt ? liveClearedLog() : []; + rollbackNoteCleared(loaded, S.log, sid); +} +/** How many questions (steers not counted) come before `m` in the agent's + chat: the ones in the log, and the ones Clear Transcript hid (ATO-1420). */ function priorQuestions(m) { - let count = 0; + const c = ROLLBACK.cleared; + let count = c.sid && c.sid === S.agentSession ? c.n : 0; for (const x of S.log) { if (x === m) return count; if (x.k === 'user' && !x.steered) count++; @@ -3275,7 +3309,11 @@ async function regenerateReply(id, fromFailure) { agent never stored must not match an earlier "continue" with a real answer. The count is of the questions before this one, so a question that never reached the agent cannot line up with the one before it. */ - try { r = await BR.rollbackLastTurn(sid, String(m.text || ''), priorQuestions(m)); } + /* ATO-1420: and, when the window shows no answer to it (its turn + failed), a stored question that has one is never it, whatever the + count says. */ + const unanswered = !S.log.slice(at + 1).some((x) => x.k === 'assistant' && !x.placeholder && String(x.text || '').trim()); + try { r = await BR.rollbackLastTurn(sid, String(m.text || ''), priorQuestions(m), unanswered); } catch (err) { r = {ok: false, supported: true, error: String((err && err.message) || err)}; } finally { ROLLBACK.pending = false; } if (r && r.supported === false) { ROLLBACK.supported = false; resendUser(id); return; } @@ -7031,7 +7069,8 @@ function act(a) { agent keeps this chat and opening it again brings it back. An empty new chat has nothing for the agent to remember, so it says only that. */ const kept = !!(S.agentSession || S.busy); - S.log = liveClearedLog(); S.history = []; render(); chatToast('Window cleared', kept ? 'The agent still remembers this chat' : ''); return; } + clearTranscriptNow(); // ATO-1420: notes the questions the agent still has + render(); chatToast('Window cleared', kept ? 'The agent still remembers this chat' : ''); return; } if (a === 'stop') { close(); abort(); return; } if (a === 'sessmodel:apply') { applySessionModelStamp(); return; } /* Item 7C — the menu's `Steer the running turn`. There is no separate @@ -20623,7 +20662,9 @@ async function openSession(id) { S.streamId = null; } // ATO-131: cleared while this loaded: as Clear leaves a chat that is open (its running turn's own rows stay). - if (OPEN_CLEARED === seq) { OPEN_CLEARED = 0; S.log = rebuilt ? liveClearedLog() : []; } + const clearedWhileLoading = OPEN_CLEARED === seq; + if (clearedWhileLoading) OPEN_CLEARED = 0; + rollbackOnSessionOpened(id, clearedWhileLoading, rebuilt); // ATO-1420: the whole chat is drawn again // item 6: a turn of this session is still running, but its stream is not in // this log any more (the user left and came back). The desktop cannot replay // a stream, so it shows the stored snapshot and says what is still happening diff --git a/desktop/test/rollback-last-turn.test.mjs b/desktop/test/rollback-last-turn.test.mjs index c137dca45..f39ae7858 100644 --- a/desktop/test/rollback-last-turn.test.mjs +++ b/desktop/test/rollback-last-turn.test.mjs @@ -46,6 +46,16 @@ test("a rollback hands back the removed question", async () => { } finally { await s.close(); } }); +test("a question shown without an answer goes along as expectUnanswered (ATO-1420)", async () => { + const s = await standIn(() => [409, { error: { message: "not that one", code: "turn_mismatch" } }]); + try { + await s.client.rollbackLastTurn("s1", "continue", 2, true); + assert.equal(s.seen[0].body, JSON.stringify({ expectText: "continue", expectPriorQuestions: 2, expectUnanswered: true })); + await s.client.rollbackLastTurn("s1", "continue", 2, false); + assert.equal(s.seen[1].body, JSON.stringify({ expectText: "continue", expectPriorQuestions: 2 })); + } finally { await s.close(); } +}); + test("the question on screen goes along as expectText", async () => { const s = await standIn(() => [409, { error: { message: "not that one", code: "turn_mismatch" } }]); try { @@ -111,16 +121,16 @@ function constLine(name) { return source.slice(at + 1, source.indexOf("\n", at + 1) + 1); } -function windowWith({ log, answer, busy = false, draft = "", voice = "idle", sid = "ses-1" }) { +function windowWith({ log, answer, busy = false, draft = "", voice = "idle", sid = "ses-1", liveRows = null }) { const calls = { rollback: [], resend: [], submit: [], toasts: [], opened: [] }; const entry = { value: draft, focus() {}, setSelectionRange() {} }; const c = vm.createContext({ - S: { log, busy, pending: null, draft, agentSession: sid, stick: false }, + S: { log, busy, pending: null, draft, agentSession: sid, stick: false, history: [] }, RUNNING: new Map(), VOICE: { state: voice }, ATT: { items: [] }, ANX: { messageAction() {} }, - BR: { rollbackLastTurn: async (id, expectText, prior) => { calls.rollback.push(id); calls.expect = expectText; calls.prior = prior; return answer; } }, + BR: { rollbackLastTurn: async (id, expectText, prior, unanswered) => { calls.rollback.push(id); calls.expect = expectText; calls.prior = prior; calls.unanswered = unanswered; return typeof answer === "function" ? answer(expectText, prior, unanswered) : answer; } }, $: () => entry, toast: (t, s) => calls.toasts.push([t, s]), attSplitMessage: () => null, @@ -130,12 +140,15 @@ function windowWith({ log, answer, busy = false, draft = "", voice = "idle", sid ctxDraftChanged() {}, openSession: async (id) => { calls.opened.push(id); }, resendUser: (id) => calls.resend.push(id), + // A turn running on screen keeps its own rows through a clear. + liveClearedLog: () => (liveRows ? liveRows.slice() : []), calls, entry, }); vm.runInContext( - constLine("ROLLBACK") + block("function rollbackStashBack(") + block("function priorQuestions(") + block("function rollbackRefusalText(") + block("async function regenerateReply(") - + "globalThis.regenerateReply = regenerateReply; globalThis.ROLLBACK = ROLLBACK;" + constLine("ROLLBACK") + block("function rollbackStashBack(") + block("function rollbackNoteCleared(") + block("function clearTranscriptNow(") + block("function rollbackOnSessionOpened(") + block("function priorQuestions(") + block("function rollbackRefusalText(") + block("async function regenerateReply(") + + "globalThis.regenerateReply = regenerateReply; globalThis.ROLLBACK = ROLLBACK; globalThis.rollbackNoteCleared = rollbackNoteCleared;" + + "globalThis.clearTranscriptNow = clearTranscriptNow; globalThis.rollbackOnSessionOpened = rollbackOnSessionOpened;" + "globalThis.submit = () => calls.submit.push(S.draft);", c, ); @@ -255,3 +268,122 @@ test("a question taken back while another chat was opened waits for its own chat assert.equal(w.ROLLBACK.stash.has("ses-1"), false); }); + +/* ATO-1420: the agent's own check, as planLastTurnRollback makes it, on a + chat whose window was cleared: "Hi" answered before Clear Transcript, then + "continue" answered for real, then a second "continue" that failed and + never reached the agent's store. */ +function agentChat() { + const turns = [ + { kind: "user", text: "Hi" }, { kind: "assistant_reply", text: "Hello" }, + { kind: "user", text: "continue" }, { kind: "assistant_reply", text: "real answer" }, + ]; + const answer = (expectText, prior, unanswered) => { + let start = -1; + for (let i = turns.length - 1; i >= 0; i--) if (turns[i].kind === "user") { start = i; break; } + const q = turns[start]; + const before = turns.slice(0, start).filter((t) => t.kind === "user").length; + const replied = turns.slice(start + 1).some((t) => t.kind === "assistant_reply"); + if (q.text !== expectText || before !== prior || (unanswered === true && replied)) { + return { ok: false, supported: true, code: "turn_mismatch", error: "not that one" }; + } + turns.splice(start); + return { ok: true, supported: true, text: q.text }; + }; + return { turns, answer }; +} + +test("after Clear Transcript, the hidden question still counts toward the position (ATO-1420)", async () => { + const agent = agentChat(); + const w = windowWith({ log: [{ id: "q0", k: "user", text: "Hi" }, { id: "a0", k: "assistant", text: "Hello" }], answer: agent.answer }); + w.clearTranscriptNow(); // the 'clear' action's own step + assert.equal(w.S.log.length, 0); + w.S.log.push( + { id: "q1", k: "user", text: "continue" }, { id: "a1", k: "assistant", text: "real answer" }, + { id: "q2", k: "user", text: "continue" }, { id: "f2", k: "system", text: "turn failed" }, + ); + await w.regenerateReply("q2", true); + assert.equal(w.calls.prior, 2, "the cleared question counts"); + assert.equal(w.calls.unanswered, true, "the window shows no answer to it"); + assert.deepEqual(agent.turns.map((t) => t.text), ["Hi", "Hello", "continue", "real answer"], "the real answer stays"); + assert.deepEqual(w.calls.resend, ["q2"], "the question is sent again as a new turn"); + assert.equal(w.S.log.length, 4, "nothing removed on screen"); +}); + +test("a failed repeat with a wrong count is refused only for its missing answer (ATO-1420)", async () => { + const agent = agentChat(); + // The cleared question is not known to the window (say, cleared before the chat was saved). + const w = windowWith({ + log: [ + { id: "q1", k: "user", text: "continue" }, { id: "a1", k: "assistant", text: "real answer" }, + { id: "q2", k: "user", text: "continue" }, { id: "f2", k: "system", text: "turn failed" }, + ], + answer: agent.answer, + }); + // Off by one the way the bug was: position 1 is the stored "continue". + await w.regenerateReply("q2", true); + assert.equal(w.calls.prior, 1); + assert.equal(w.calls.unanswered, true); + assert.equal(agent.turns.length, 4, "the real answer stays"); + assert.deepEqual(w.calls.resend, ["q2"]); + // The same count without the flag is the bug: the agent's check alone takes the real answer back. + assert.equal(agent.answer("continue", 1, false).ok, true, "only expectUnanswered saved it"); +}); + +test("after Clear Transcript, Regenerate on the last answered question takes it back (ATO-1420)", async () => { + const agent = agentChat(); + const w = windowWith({ log: [{ id: "q0", k: "user", text: "Hi" }, { id: "a0", k: "assistant", text: "Hello" }], answer: agent.answer }); + w.clearTranscriptNow(); + w.S.log.push({ id: "q1", k: "user", text: "continue" }, { id: "a1", k: "assistant", text: "real answer" }); + await w.regenerateReply("q1", false); + assert.equal(w.calls.prior, 1); + assert.equal(w.calls.unanswered, false); + assert.deepEqual(agent.turns.map((t) => t.text), ["Hi", "Hello"]); + assert.deepEqual(w.calls.submit, ["continue"]); +}); + +test("the cleared count belongs to its chat", () => { + const w = windowWith({ log: [], answer: { ok: false, supported: true } }); + w.rollbackNoteCleared([{ k: "user", text: "a" }, { k: "user", text: "b", steered: true }], [], "ses-1"); + w.rollbackNoteCleared([{ k: "user", text: "c" }], [], "ses-1"); + assert.deepEqual({ ...w.ROLLBACK.cleared }, { sid: "ses-1", n: 2 }); + w.rollbackNoteCleared([{ k: "user", text: "d" }], [], "ses-2"); + assert.deepEqual({ ...w.ROLLBACK.cleared }, { sid: "ses-2", n: 1 }); +}); + +test("a turn running on screen keeps its rows through Clear, and only the hidden questions count (ATO-1420)", () => { + const live = [{ id: "q2", k: "user", text: "still going" }]; + const w = windowWith({ log: [{ id: "q1", k: "user", text: "a" }, { id: "a1", k: "assistant", text: "b" }, ...live], liveRows: live, answer: { ok: false, supported: true } }); + w.clearTranscriptNow(); + assert.deepEqual([...w.S.log].map((x) => x.id), ["q2"]); + assert.deepEqual({ ...w.ROLLBACK.cleared }, { sid: "ses-1", n: 1 }); + assert.equal(w.priorQuestions(w.S.log[0]), 1); +}); + +test("opening a chat draws all of it again, so the cleared count starts over (ATO-1420)", async () => { + const agent = agentChat(); + const w = windowWith({ log: [{ id: "q0", k: "user", text: "Hi" }, { id: "a0", k: "assistant", text: "Hello" }], answer: agent.answer }); + w.clearTranscriptNow(); + // The chat is opened again: the stored transcript, "Hi" included, is the log. + w.S.log = [ + { id: "q0", k: "user", text: "Hi" }, { id: "a0", k: "assistant", text: "Hello" }, + { id: "q1", k: "user", text: "continue" }, { id: "a1", k: "assistant", text: "real answer" }, + ]; + w.rollbackOnSessionOpened("ses-1", false, false); + assert.deepEqual({ ...w.ROLLBACK.cleared }, { sid: null, n: 0 }); + await w.regenerateReply("q1", false); + assert.equal(w.calls.prior, 1, "Hi is counted once, from the log"); + assert.deepEqual(w.calls.submit, ["continue"]); +}); + +test("a Clear that landed while the chat loaded empties it and counts what it hid (ATO-1420)", () => { + const w = windowWith({ log: [{ id: "q0", k: "user", text: "Hi" }, { id: "a0", k: "assistant", text: "Hello" }], answer: { ok: false, supported: true } }); + w.rollbackOnSessionOpened("ses-1", true, false); + assert.equal(w.S.log.length, 0); + assert.deepEqual({ ...w.ROLLBACK.cleared }, { sid: "ses-1", n: 1 }); + const live = [{ id: "q1", k: "user", text: "running" }]; + const r = windowWith({ log: [{ id: "q0", k: "user", text: "Hi" }, ...live], liveRows: live, answer: { ok: false, supported: true } }); + r.rollbackOnSessionOpened("ses-1", true, true); + assert.deepEqual([...r.S.log].map((x) => x.id), ["q1"], "a running turn's own rows stay"); + assert.deepEqual({ ...r.ROLLBACK.cleared }, { sid: "ses-1", n: 1 }); +});