Fix branch summarization abort handling and tree navigation

- Check response.stopReason instead of catching errors for abort detection
- Return result object from _generateBranchSummary instead of throwing
- Fix summary attachment: attach to navigation target position, not old branch
- Support root-level summaries (parentId=null) in branchWithSummary
- Remove setTimeout hack for re-showing tree selector on abort
This commit is contained in:
Mario Zechner 2025-12-29 20:15:19 +01:00
parent 9dac0a1423
commit 01dae9ebcc
7 changed files with 55 additions and 40 deletions

View file

@ -3620,7 +3620,7 @@ export const MODELS = {
cacheWrite: 0, cacheWrite: 0,
}, },
contextWindow: 196608, contextWindow: 196608,
maxTokens: 131072, maxTokens: 65536,
} satisfies Model<"openai-completions">, } satisfies Model<"openai-completions">,
"deepcogito/cogito-v2-preview-llama-405b": { "deepcogito/cogito-v2-preview-llama-405b": {
id: "deepcogito/cogito-v2-preview-llama-405b", id: "deepcogito/cogito-v2-preview-llama-405b",
@ -4623,7 +4623,7 @@ export const MODELS = {
cacheWrite: 0, cacheWrite: 0,
}, },
contextWindow: 131072, contextWindow: 131072,
maxTokens: 128000, maxTokens: 131072,
} satisfies Model<"openai-completions">, } satisfies Model<"openai-completions">,
"openai/gpt-oss-20b": { "openai/gpt-oss-20b": {
id: "openai/gpt-oss-20b", id: "openai/gpt-oss-20b",

View file

@ -1583,7 +1583,7 @@ export class AgentSession {
targetId: string, targetId: string,
options: { summarize?: boolean; customInstructions?: string } = {}, options: { summarize?: boolean; customInstructions?: string } = {},
): Promise<{ editorText?: string; cancelled: boolean; aborted?: boolean }> { ): Promise<{ editorText?: string; cancelled: boolean; aborted?: boolean }> {
const oldLeafId = this.sessionManager.getLeafUuid(); const oldLeafId = this.sessionManager.getLeafId();
// No-op if already at target // No-op if already at target
if (targetId === oldLeafId) { if (targetId === oldLeafId) {
@ -1661,21 +1661,19 @@ export class AgentSession {
// Run default summarizer if needed // Run default summarizer if needed
let summaryText: string | undefined; let summaryText: string | undefined;
if (options.summarize && entriesToSummarize.length > 0 && !hookSummary) { if (options.summarize && entriesToSummarize.length > 0 && !hookSummary) {
try { const result = await this._generateBranchSummary(
summaryText = await this._generateBranchSummary( entriesToSummarize,
entriesToSummarize, options.customInstructions,
options.customInstructions, this._branchSummaryAbortController.signal,
this._branchSummaryAbortController.signal, );
); this._branchSummaryAbortController = undefined;
} catch (error) { if (result.aborted) {
this._branchSummaryAbortController = undefined; return { cancelled: true, aborted: true };
// Check if aborted
if (error instanceof Error && (error.name === "AbortError" || error.message === "aborted")) {
return { cancelled: true, aborted: true };
}
// Re-throw actual errors so UI can display them
throw error;
} }
if (result.error) {
throw new Error(result.error);
}
summaryText = result.summary;
} else if (hookSummary) { } else if (hookSummary) {
summaryText = hookSummary.summary; summaryText = hookSummary.summary;
} }
@ -1704,14 +1702,17 @@ export class AgentSession {
} }
// Switch leaf (with or without summary) // Switch leaf (with or without summary)
// Summary is attached at the navigation target position (newLeafId), not the old branch
let summaryEntry: BranchSummaryEntry | undefined; let summaryEntry: BranchSummaryEntry | undefined;
if (newLeafId === null) { if (summaryText) {
// Navigating to root user message - reset leaf to empty // Create summary at target position (can be null for root)
this.sessionManager.resetLeaf();
} else if (summaryText) {
const summaryId = this.sessionManager.branchWithSummary(newLeafId, summaryText); const summaryId = this.sessionManager.branchWithSummary(newLeafId, summaryText);
summaryEntry = this.sessionManager.getEntry(summaryId) as BranchSummaryEntry; summaryEntry = this.sessionManager.getEntry(summaryId) as BranchSummaryEntry;
} else if (newLeafId === null) {
// No summary, navigating to root - reset leaf
this.sessionManager.resetLeaf();
} else { } else {
// No summary, navigating to non-root
this.sessionManager.branch(newLeafId); this.sessionManager.branch(newLeafId);
} }
@ -1723,7 +1724,7 @@ export class AgentSession {
if (this._hookRunner) { if (this._hookRunner) {
await this._hookRunner.emit({ await this._hookRunner.emit({
type: "session_tree", type: "session_tree",
newLeafId: this.sessionManager.getLeafUuid(), newLeafId: this.sessionManager.getLeafId(),
oldLeafId, oldLeafId,
summaryEntry, summaryEntry,
fromHook: summaryText ? fromHook : undefined, fromHook: summaryText ? fromHook : undefined,
@ -1744,7 +1745,7 @@ export class AgentSession {
entries: SessionEntry[], entries: SessionEntry[],
customInstructions: string | undefined, customInstructions: string | undefined,
signal: AbortSignal, signal: AbortSignal,
): Promise<string> { ): Promise<{ summary?: string; aborted?: boolean; error?: string }> {
// Convert entries to messages for summarization // Convert entries to messages for summarization
const messages: Array<{ role: string; content: string }> = []; const messages: Array<{ role: string; content: string }> = [];
for (const entry of entries) { for (const entry of entries) {
@ -1770,7 +1771,7 @@ export class AgentSession {
} }
if (messages.length === 0) { if (messages.length === 0) {
return "No content to summarize"; return { summary: "No content to summarize" };
} }
// Build prompt for summarization // Build prompt for summarization
@ -1804,12 +1805,20 @@ export class AgentSession {
{ apiKey, signal, maxTokens: 1024 }, { apiKey, signal, maxTokens: 1024 },
); );
// Check if aborted or errored
if (response.stopReason === "aborted") {
return { aborted: true };
}
if (response.stopReason === "error") {
return { error: response.errorMessage || "Summarization failed" };
}
const summary = response.content const summary = response.content
.filter((c): c is { type: "text"; text: string } => c.type === "text") .filter((c): c is { type: "text"; text: string } => c.type === "text")
.map((c) => c.text) .map((c) => c.text)
.join("\n"); .join("\n");
return summary || "No summary generated"; return { summary: summary || "No summary generated" };
} }
/** /**

View file

@ -25,7 +25,7 @@ export type ReadonlySessionManager = Pick<
| "getSessionDir" | "getSessionDir"
| "getSessionId" | "getSessionId"
| "getSessionFile" | "getSessionFile"
| "getLeafUuid" | "getLeafId"
| "getLeafEntry" | "getLeafEntry"
| "getEntry" | "getEntry"
| "getLabel" | "getLabel"

View file

@ -684,7 +684,7 @@ export class SessionManager {
// Tree Traversal // Tree Traversal
// ========================================================================= // =========================================================================
getLeafUuid(): string | null { getLeafId(): string | null {
return this.leafId; return this.leafId;
} }
@ -845,8 +845,8 @@ export class SessionManager {
* Same as branch(), but also appends a branch_summary entry that captures * Same as branch(), but also appends a branch_summary entry that captures
* context from the abandoned conversation path. * context from the abandoned conversation path.
*/ */
branchWithSummary(branchFromId: string, summary: string): string { branchWithSummary(branchFromId: string | null, summary: string): string {
if (!this.byId.has(branchFromId)) { if (branchFromId !== null && !this.byId.has(branchFromId)) {
throw new Error(`Entry ${branchFromId} not found`); throw new Error(`Entry ${branchFromId} not found`);
} }
this.leafId = branchFromId; this.leafId = branchFromId;
@ -855,7 +855,7 @@ export class SessionManager {
id: generateId(this.byId), id: generateId(this.byId),
parentId: branchFromId, parentId: branchFromId,
timestamp: new Date().toISOString(), timestamp: new Date().toISOString(),
fromId: branchFromId, fromId: branchFromId ?? "root",
summary, summary,
}; };
this._appendEntry(entry); this._appendEntry(entry);

View file

@ -609,8 +609,12 @@ class TreeList implements Component {
return `[edit: ${path}]`; return `[edit: ${path}]`;
} }
case "bash": { case "bash": {
const cmd = String(args.command || "").slice(0, 50); const rawCmd = String(args.command || "");
return `[bash: ${cmd}${(args.command as string)?.length > 50 ? "..." : ""}]`; const cmd = rawCmd
.replace(/[\n\t]/g, " ")
.trim()
.slice(0, 50);
return `[bash: ${cmd}${rawCmd.length > 50 ? "..." : ""}]`;
} }
case "grep": { case "grep": {
const pattern = String(args.pattern || ""); const pattern = String(args.pattern || "");

View file

@ -1600,7 +1600,7 @@ export class InteractiveMode {
private showTreeSelector(): void { private showTreeSelector(): void {
const tree = this.sessionManager.getTree(); const tree = this.sessionManager.getTree();
const realLeafId = this.sessionManager.getLeafUuid(); const realLeafId = this.sessionManager.getLeafId();
// Find the visible leaf for display (skip metadata entries like labels) // Find the visible leaf for display (skip metadata entries like labels)
let visibleLeafId = realLeafId; let visibleLeafId = realLeafId;
@ -1660,7 +1660,9 @@ export class InteractiveMode {
const result = await this.session.navigateTree(entryId, { summarize: wantsSummary }); const result = await this.session.navigateTree(entryId, { summarize: wantsSummary });
if (result.aborted) { if (result.aborted) {
// Summarization aborted - re-show tree selector
this.showStatus("Branch summarization cancelled"); this.showStatus("Branch summarization cancelled");
this.showTreeSelector();
return; return;
} }
if (result.cancelled) { if (result.cancelled) {

View file

@ -129,16 +129,16 @@ describe("SessionManager append and tree traversal", () => {
it("leaf pointer advances after each append", () => { it("leaf pointer advances after each append", () => {
const session = SessionManager.inMemory(); const session = SessionManager.inMemory();
expect(session.getLeafUuid()).toBe(""); expect(session.getLeafId()).toBe("");
const id1 = session.appendMessage(userMsg("1")); const id1 = session.appendMessage(userMsg("1"));
expect(session.getLeafUuid()).toBe(id1); expect(session.getLeafId()).toBe(id1);
const id2 = session.appendMessage(assistantMsg("2")); const id2 = session.appendMessage(assistantMsg("2"));
expect(session.getLeafUuid()).toBe(id2); expect(session.getLeafId()).toBe(id2);
const id3 = session.appendThinkingLevelChange("high"); const id3 = session.appendThinkingLevelChange("high");
expect(session.getLeafUuid()).toBe(id3); expect(session.getLeafId()).toBe(id3);
}); });
}); });
@ -303,10 +303,10 @@ describe("SessionManager append and tree traversal", () => {
const _id2 = session.appendMessage(assistantMsg("2")); const _id2 = session.appendMessage(assistantMsg("2"));
const id3 = session.appendMessage(userMsg("3")); const id3 = session.appendMessage(userMsg("3"));
expect(session.getLeafUuid()).toBe(id3); expect(session.getLeafId()).toBe(id3);
session.branch(id1); session.branch(id1);
expect(session.getLeafUuid()).toBe(id1); expect(session.getLeafId()).toBe(id1);
}); });
it("throws for non-existent entry", () => { it("throws for non-existent entry", () => {
@ -341,7 +341,7 @@ describe("SessionManager append and tree traversal", () => {
const summaryId = session.branchWithSummary(id1, "Summary of abandoned work"); const summaryId = session.branchWithSummary(id1, "Summary of abandoned work");
expect(session.getLeafUuid()).toBe(summaryId); expect(session.getLeafId()).toBe(summaryId);
const entries = session.getEntries(); const entries = session.getEntries();
const summaryEntry = entries.find((e) => e.type === "branch_summary"); const summaryEntry = entries.find((e) => e.type === "branch_summary");