diff --git a/packages/coding-agent/docs/session-tree-plan.md b/packages/coding-agent/docs/session-tree-plan.md index 6e45dd85..c862162b 100644 --- a/packages/coding-agent/docs/session-tree-plan.md +++ b/packages/coding-agent/docs/session-tree-plan.md @@ -78,11 +78,18 @@ Questions to resolve: - [ ] Design and implement branch summarizer - [ ] Add tests for `branchWithSummary()` flow -### Entry Labels +### Entry Labels ✅ -- [ ] Add optional `label?: string` field to `SessionEntryBase` -- [ ] Allow users to label any entry -- [ ] Display labels in UI (tree view, path view) +- [x] Add `LabelEntry` type with `targetId` and `label` fields +- [x] Add `labelsById: Map` private field +- [x] Build labels map in `_buildIndex()` via linear scan +- [x] Add `getLabel(id)` method +- [x] Add `appendLabelChange(targetId, label)` method (undefined clears) +- [x] Update `createBranchedSession()` to filter out LabelEntry and recreate from resolved map +- [x] `buildSessionContext()` already ignores LabelEntry (only handles message types) +- [x] Add `label?: string` to `SessionTreeNode`, populated by `getTree()` +- [ ] Display labels in UI (tree view, path view) - deferred to UI phase +- [ ] `/label` command - deferred to UI phase ### HTML Export @@ -110,10 +117,6 @@ Design new commands based on refactored SessionManager: - [ ] Allow switching between branches (move leaf pointer) - [ ] Show current position in tree -**`/label`** - Label entries (new, if labels implemented) -- [ ] Allow labeling current or selected entry -- [ ] Display in tree view - --- ## Notes diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index 148eec05..9f724372 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -68,6 +68,13 @@ export interface CustomEntry extends SessionEntryBase { data?: unknown; } +/** Label entry for user-defined bookmarks/markers on entries. */ +export interface LabelEntry extends SessionEntryBase { + type: "label"; + targetId: string; + label: string | undefined; +} + /** Session entry - has id/parentId for tree structure (returned by "read" methods in SessionManager) */ export type SessionEntry = | SessionMessageEntry @@ -75,7 +82,8 @@ export type SessionEntry = | ModelChangeEntry | CompactionEntry | BranchSummaryEntry - | CustomEntry; + | CustomEntry + | LabelEntry; /** Raw file entry (includes header) */ export type FileEntry = SessionHeader | SessionEntry; @@ -84,6 +92,8 @@ export type FileEntry = SessionHeader | SessionEntry; export interface SessionTreeNode { entry: SessionEntry; children: SessionTreeNode[]; + /** Resolved label for this entry, if any */ + label?: string; } export interface SessionContext { @@ -407,6 +417,7 @@ export class SessionManager { private flushed: boolean = false; private fileEntries: FileEntry[] = []; private byId: Map = new Map(); + private labelsById: Map = new Map(); private leafId: string = ""; private constructor(cwd: string, sessionDir: string, sessionFile: string | null, persist: boolean) { @@ -466,11 +477,19 @@ export class SessionManager { private _buildIndex(): void { this.byId.clear(); + this.labelsById.clear(); this.leafId = ""; for (const entry of this.fileEntries) { if (entry.type === "session") continue; this.byId.set(entry.id, entry); this.leafId = entry.id; + if (entry.type === "label") { + if (entry.label) { + this.labelsById.set(entry.targetId, entry.label); + } else { + this.labelsById.delete(entry.targetId); + } + } } } @@ -608,6 +627,39 @@ export class SessionManager { return this.byId.get(id); } + /** + * Get the label for an entry, if any. + */ + getLabel(id: string): string | undefined { + return this.labelsById.get(id); + } + + /** + * Set or clear a label on an entry. + * Labels are user-defined markers for bookmarking/navigation. + * Pass undefined or empty string to clear the label. + */ + appendLabelChange(targetId: string, label: string | undefined): string { + if (!this.byId.has(targetId)) { + throw new Error(`Entry ${targetId} not found`); + } + const entry: LabelEntry = { + type: "label", + id: generateId(this.byId), + parentId: this.leafId || null, + timestamp: new Date().toISOString(), + targetId, + label, + }; + this._appendEntry(entry); + if (label) { + this.labelsById.set(targetId, label); + } else { + this.labelsById.delete(targetId); + } + return entry.id; + } + /** * Walk from entry to root, returning all entries in path order. * Includes all entry types (messages, compaction, model changes, etc.). @@ -658,9 +710,10 @@ export class SessionManager { const nodeMap = new Map(); const roots: SessionTreeNode[] = []; - // Create nodes + // Create nodes with resolved labels for (const entry of entries) { - nodeMap.set(entry.id, { entry, children: [] }); + const label = this.labelsById.get(entry.id); + nodeMap.set(entry.id, { entry, children: [], label }); } // Build tree @@ -731,6 +784,9 @@ export class SessionManager { throw new Error(`Entry ${leafId} not found`); } + // Filter out LabelEntry from path - we'll recreate them from the resolved map + const pathWithoutLabels = path.filter((e) => e.type !== "label"); + const newSessionId = randomUUID(); const timestamp = new Date().toISOString(); const fileTimestamp = timestamp.replace(/[:.]/g, "-"); @@ -745,16 +801,55 @@ export class SessionManager { branchedFrom: this.persist ? this.sessionFile : undefined, }; + // Collect labels for entries in the path + const pathEntryIds = new Set(pathWithoutLabels.map((e) => e.id)); + const labelsToWrite: Array<{ targetId: string; label: string }> = []; + for (const [targetId, label] of this.labelsById) { + if (pathEntryIds.has(targetId)) { + labelsToWrite.push({ targetId, label }); + } + } + if (this.persist) { appendFileSync(newSessionFile, `${JSON.stringify(header)}\n`); - for (const entry of path) { + for (const entry of pathWithoutLabels) { appendFileSync(newSessionFile, `${JSON.stringify(entry)}\n`); } + // Write fresh label entries at the end + const lastEntryId = pathWithoutLabels[pathWithoutLabels.length - 1]?.id || null; + let parentId = lastEntryId; + for (const { targetId, label } of labelsToWrite) { + const labelEntry: LabelEntry = { + type: "label", + id: generateId(new Set(pathEntryIds)), + parentId, + timestamp: new Date().toISOString(), + targetId, + label, + }; + appendFileSync(newSessionFile, `${JSON.stringify(labelEntry)}\n`); + pathEntryIds.add(labelEntry.id); + parentId = labelEntry.id; + } return newSessionFile; } - // In-memory mode: replace current session with the path - this.fileEntries = [header, ...path]; + // In-memory mode: replace current session with the path + labels + const labelEntries: LabelEntry[] = []; + let parentId = pathWithoutLabels[pathWithoutLabels.length - 1]?.id || null; + for (const { targetId, label } of labelsToWrite) { + const labelEntry: LabelEntry = { + type: "label", + id: generateId(new Set([...pathEntryIds, ...labelEntries.map((e) => e.id)])), + parentId, + timestamp: new Date().toISOString(), + targetId, + label, + }; + labelEntries.push(labelEntry); + parentId = labelEntry.id; + } + this.fileEntries = [header, ...pathWithoutLabels, ...labelEntries]; this.sessionId = newSessionId; this._buildIndex(); return null; diff --git a/packages/coding-agent/test/session-manager/labels.test.ts b/packages/coding-agent/test/session-manager/labels.test.ts new file mode 100644 index 00000000..e349aa10 --- /dev/null +++ b/packages/coding-agent/test/session-manager/labels.test.ts @@ -0,0 +1,178 @@ +import { describe, expect, it } from "vitest"; +import { type LabelEntry, SessionManager } from "../../src/core/session-manager.js"; + +describe("SessionManager labels", () => { + it("sets and gets labels", () => { + const session = SessionManager.inMemory(); + + const msgId = session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); + + // No label initially + expect(session.getLabel(msgId)).toBeUndefined(); + + // Set a label + const labelId = session.appendLabelChange(msgId, "checkpoint"); + expect(session.getLabel(msgId)).toBe("checkpoint"); + + // Label entry should be in entries + const entries = session.getEntries(); + const labelEntry = entries.find((e) => e.type === "label") as LabelEntry; + expect(labelEntry).toBeDefined(); + expect(labelEntry.id).toBe(labelId); + expect(labelEntry.targetId).toBe(msgId); + expect(labelEntry.label).toBe("checkpoint"); + }); + + it("clears labels with undefined", () => { + const session = SessionManager.inMemory(); + + const msgId = session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); + + session.appendLabelChange(msgId, "checkpoint"); + expect(session.getLabel(msgId)).toBe("checkpoint"); + + // Clear the label + session.appendLabelChange(msgId, undefined); + expect(session.getLabel(msgId)).toBeUndefined(); + }); + + it("last label wins", () => { + const session = SessionManager.inMemory(); + + const msgId = session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); + + session.appendLabelChange(msgId, "first"); + session.appendLabelChange(msgId, "second"); + session.appendLabelChange(msgId, "third"); + + expect(session.getLabel(msgId)).toBe("third"); + }); + + it("labels are included in tree nodes", () => { + const session = SessionManager.inMemory(); + + const msg1Id = session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); + const msg2Id = session.appendMessage({ + role: "assistant", + content: [{ type: "text", text: "hi" }], + api: "anthropic-messages", + provider: "anthropic", + model: "test", + usage: { + input: 1, + output: 1, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 2, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: 2, + }); + + session.appendLabelChange(msg1Id, "start"); + session.appendLabelChange(msg2Id, "response"); + + const tree = session.getTree(); + + // Find the message nodes (skip label entries) + const msg1Node = tree.find((n) => n.entry.id === msg1Id); + expect(msg1Node?.label).toBe("start"); + + // msg2 is a child of msg1 + const msg2Node = msg1Node?.children.find((n) => n.entry.id === msg2Id); + expect(msg2Node?.label).toBe("response"); + }); + + it("labels are preserved in createBranchedSession", () => { + const session = SessionManager.inMemory(); + + const msg1Id = session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); + const msg2Id = session.appendMessage({ + role: "assistant", + content: [{ type: "text", text: "hi" }], + api: "anthropic-messages", + provider: "anthropic", + model: "test", + usage: { + input: 1, + output: 1, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 2, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: 2, + }); + + session.appendLabelChange(msg1Id, "important"); + session.appendLabelChange(msg2Id, "also-important"); + + // Branch from msg2 (in-memory mode returns null, but updates internal state) + session.createBranchedSession(msg2Id); + + // Labels should be preserved + expect(session.getLabel(msg1Id)).toBe("important"); + expect(session.getLabel(msg2Id)).toBe("also-important"); + + // New label entries should exist + const entries = session.getEntries(); + const labelEntries = entries.filter((e) => e.type === "label") as LabelEntry[]; + expect(labelEntries).toHaveLength(2); + }); + + it("labels not on path are not preserved in createBranchedSession", () => { + const session = SessionManager.inMemory(); + + const msg1Id = session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); + const msg2Id = session.appendMessage({ + role: "assistant", + content: [{ type: "text", text: "hi" }], + api: "anthropic-messages", + provider: "anthropic", + model: "test", + usage: { + input: 1, + output: 1, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 2, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: 2, + }); + const msg3Id = session.appendMessage({ role: "user", content: "followup", timestamp: 3 }); + + // Label all messages + session.appendLabelChange(msg1Id, "first"); + session.appendLabelChange(msg2Id, "second"); + session.appendLabelChange(msg3Id, "third"); + + // Branch from msg2 (excludes msg3) + session.createBranchedSession(msg2Id); + + // Only labels for msg1 and msg2 should be preserved + expect(session.getLabel(msg1Id)).toBe("first"); + expect(session.getLabel(msg2Id)).toBe("second"); + expect(session.getLabel(msg3Id)).toBeUndefined(); + }); + + it("labels are not included in buildSessionContext", () => { + const session = SessionManager.inMemory(); + + const msgId = session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); + session.appendLabelChange(msgId, "checkpoint"); + + const ctx = session.buildSessionContext(); + expect(ctx.messages).toHaveLength(1); + expect(ctx.messages[0].role).toBe("user"); + }); + + it("throws when labeling non-existent entry", () => { + const session = SessionManager.inMemory(); + + expect(() => session.appendLabelChange("non-existent", "label")).toThrow("Entry non-existent not found"); + }); +});