From 93f42321345f88a78aa036eb3f4a9666dba9c759 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 1 May 2026 15:42:23 +1000 Subject: [PATCH 1/5] feat(staged): surface chat updates after linked notes Signed-off-by: Test --- .../lib/features/branches/BranchCard.svelte | 45 ++++-- .../src/lib/features/notes/NoteModal.svelte | 50 ++++++- .../features/projects/ProjectSection.svelte | 38 +++-- .../lib/features/sessions/SessionModal.svelte | 77 ++++++++-- .../features/sessions/noteFreshness.test.ts | 134 ++++++++++++++++++ .../lib/features/sessions/noteFreshness.ts | 91 ++++++++++++ .../features/timeline/BranchTimeline.svelte | 18 ++- 7 files changed, 419 insertions(+), 34 deletions(-) create mode 100644 apps/staged/src/lib/features/sessions/noteFreshness.test.ts create mode 100644 apps/staged/src/lib/features/sessions/noteFreshness.ts diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index d3a4c119..18cc2c6a 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -50,6 +50,7 @@ import { sessionRegistry } from '../../stores/sessionRegistry.svelte'; import { getPreferredAgent } from '../settings/preferences.svelte'; import { agentState, REMOTE_AGENTS } from '../agents/agent.svelte'; + import type { LinkedNoteContext } from '../sessions/noteFreshness'; interface Props { branch: Branch; @@ -416,6 +417,7 @@ title: string; content: string; sessionId?: string; + noteUpdatedAt?: number; nextSteps?: { commitStep: string | null; noteStep: string | null } | null; } | null>(null); @@ -736,20 +738,37 @@ // ========================================================================= /** Look up note info from timeline data by session ID (for cross-modal navigation). */ - function findNoteForSession( - sessionId: string - ): { id: string; title: string; content: string } | null { - const note = timeline?.notes.find((n) => n.sessionId === sessionId && n.content?.trim()); + function findNoteForSession(sessionId: string): LinkedNoteContext | null { + const note = timeline?.notes.find((n) => n.sessionId === sessionId); if (!note) return null; - return { id: note.id, title: note.title, content: note.content }; + return { + id: note.id, + title: note.title, + content: note.content, + updatedAt: note.updatedAt, + hasParsedNote: !!note.content.trim(), + }; } function handleCommitClick(sha: string) { commitDiffSha = sha; } - function handleNoteClick(noteId: string, title: string, content: string, sessionId?: string) { - openNote = { noteId, title, content, sessionId, nextSteps: computeNoteNextSteps(noteId) }; + function handleNoteClick( + noteId: string, + title: string, + content: string, + sessionId: string | undefined, + noteUpdatedAt: number + ) { + openNote = { + noteId, + title, + content, + sessionId, + noteUpdatedAt, + nextSteps: computeNoteNextSteps(noteId), + }; } async function handleReviewClick(reviewId: string) { @@ -1300,6 +1319,7 @@ title={openNote.title} content={openNote.content} sessionId={openNote.sessionId} + noteUpdatedAt={openNote.noteUpdatedAt} nextSteps={openNote.nextSteps} onClose={() => (openNote = null)} onOpenSession={(sid) => { @@ -1379,15 +1399,16 @@ branchId={branch.id} projectId={branch.projectId} noteInfo={findNoteForSession(sessionMgr.openSessionId)} - onOpenNote={(noteId, title, content) => { + onOpenNote={(note) => { const sid = sessionMgr.openSessionId; sessionMgr.openSessionId = null; openNote = { - noteId, - title, - content, + noteId: note.id, + title: note.title, + content: note.content, sessionId: sid ?? undefined, - nextSteps: computeNoteNextSteps(noteId), + noteUpdatedAt: note.updatedAt, + nextSteps: computeNoteNextSteps(note.id), }; }} onClose={async () => { diff --git a/apps/staged/src/lib/features/notes/NoteModal.svelte b/apps/staged/src/lib/features/notes/NoteModal.svelte index 751ae56a..ee69f011 100644 --- a/apps/staged/src/lib/features/notes/NoteModal.svelte +++ b/apps/staged/src/lib/features/notes/NoteModal.svelte @@ -10,10 +10,11 @@ import { marked } from 'marked'; import { sanitize } from '../../shared/sanitize'; import { createBackdropDismissHandlers } from '../../shared/backdropDismiss'; - import { handleExternalLinkClick } from '../../api/commands'; + import { getSessionMessages, handleExternalLinkClick } from '../../api/commands'; import InContentSearch from '../../shared/InContentSearch.svelte'; import { highlightMatches, clearHighlights, scrollToMatch } from '../../shared/textHighlight'; import { registerSearchShortcutTarget } from '../keyboard/searchTargets'; + import { countAssistantMessagesAfterNote } from '../sessions/noteFreshness'; marked.setOptions({ breaks: true, gfm: true }); @@ -23,6 +24,7 @@ onClose: () => void; /** When set, shows a button to open the associated chat session. */ sessionId?: string | null; + noteUpdatedAt?: number | null; onOpenSession?: (sessionId: string) => void; /** Suggested next steps to show as action buttons at the bottom. */ nextSteps?: { commitStep: string | null; noteStep: string | null } | null; @@ -30,11 +32,27 @@ onStartSession?: (mode: 'commit' | 'note', prefill: string) => void; } - let { title, content, onClose, sessionId, onOpenSession, nextSteps, onStartSession }: Props = - $props(); + let { + title, + content, + onClose, + sessionId, + noteUpdatedAt, + onOpenSession, + nextSteps, + onStartSession, + }: Props = $props(); let copied = $state(false); const backdropDismiss = createBackdropDismissHandlers({ onDismiss: () => onClose() }); + let assistantMessagesAfterNote = $state(0); + let chatButtonLabel = $derived( + assistantMessagesAfterNote === 1 + ? '1 message after note in chat' + : assistantMessagesAfterNote > 1 + ? `${assistantMessagesAfterNote} messages after note in chat` + : 'View chat' + ); // Search state let searchVisible = $state(false); @@ -57,6 +75,30 @@ unregisterSearchTarget?.(); }); + $effect(() => { + const sid = sessionId; + const updatedAt = noteUpdatedAt; + if (!sid || typeof updatedAt !== 'number') { + assistantMessagesAfterNote = 0; + return; + } + + let stale = false; + getSessionMessages(sid) + .then((messages) => { + if (!stale) { + assistantMessagesAfterNote = countAssistantMessagesAfterNote(messages, updatedAt); + } + }) + .catch(() => { + if (!stale) assistantMessagesAfterNote = 0; + }); + + return () => { + stale = true; + }; + }); + function renderMarkdown(text: string): string { return sanitize(marked.parse(text) as string); } @@ -202,7 +244,7 @@ onclick={() => onOpenSession?.(sessionId!)} title="Open chat session" > - View chat + {chatButtonLabel} {/if} {/if} @@ -1223,6 +1231,23 @@ Thinking… {/if} + + {#if noteFollowupLabel} +
+ +
+ {/if} {/if} @@ -1899,6 +1924,42 @@ padding: 4px 0; } + .note-followup-row { + display: flex; + justify-content: center; + padding: 4px 0; + } + + .note-followup-btn { + display: inline-flex; + align-items: center; + justify-content: center; + gap: 6px; + min-height: 30px; + padding: 6px 12px; + border: 1px solid var(--border-muted); + border-radius: 6px; + background: var(--note-bg); + color: var(--note-color); + font-size: var(--size-xs); + font-weight: 500; + cursor: pointer; + transition: + background-color 0.1s, + border-color 0.1s, + color 0.1s; + } + + .note-followup-btn:hover:not(:disabled) { + border-color: var(--note-color); + background: var(--note-bg-emphasis); + } + + .note-followup-btn:disabled { + cursor: default; + opacity: 0.65; + } + /* Error banner */ .error-banner { display: flex; diff --git a/apps/staged/src/lib/features/sessions/noteFreshness.test.ts b/apps/staged/src/lib/features/sessions/noteFreshness.test.ts new file mode 100644 index 00000000..0799b43e --- /dev/null +++ b/apps/staged/src/lib/features/sessions/noteFreshness.test.ts @@ -0,0 +1,134 @@ +import { describe, expect, it } from 'vitest'; +import type { Session, SessionMessage, SessionStatus, CompletionReason } from '../../types'; +import { + buildNoteFollowupMessage, + countAssistantMessagesAfterNote, + getNoteFollowupLabel, + latestAssistantMessage, + type LinkedNoteContext, +} from './noteFreshness'; + +function session( + status: SessionStatus = 'completed', + completionReason: CompletionReason | null = 'turn_complete' +): Session { + return { + id: 'session-1', + prompt: 'Write a note', + status, + agentId: null, + errorMessage: null, + completionReason, + createdAt: 1000, + updatedAt: 2000, + }; +} + +function message( + id: number, + role: SessionMessage['role'], + createdAt: number, + content = `${role} ${id}` +): SessionMessage { + return { + id, + sessionId: 'session-1', + role, + content, + createdAt, + }; +} + +function note(overrides: Partial = {}): LinkedNoteContext { + return { + id: 'note-1', + title: 'Note', + content: '# Note\n\nBody', + updatedAt: 2000, + hasParsedNote: true, + ...overrides, + }; +} + +describe('note freshness', () => { + it('does not show a note follow-up CTA without a linked note', () => { + const messages = [message(1, 'assistant', 3000)]; + + expect(getNoteFollowupLabel(session(), messages, null)).toBeNull(); + }); + + it.each([ + ['queued', null], + ['running', null], + ['cancelled', 'interrupted'], + ['error', 'crashed'], + ['completed', 'interrupted'], + ] satisfies [SessionStatus, CompletionReason | null][])( + 'does not show a note follow-up CTA for %s sessions with %s completion', + (status, completionReason) => { + const messages = [message(1, 'assistant', 3000)]; + + expect(getNoteFollowupLabel(session(status, completionReason), messages, note())).toBeNull(); + } + ); + + it('asks for a note to be written when an empty linked note has a later assistant message', () => { + const messages = [message(1, 'assistant', 3000)]; + const emptyNote = note({ content: '', hasParsedNote: false }); + + expect(getNoteFollowupLabel(session(), messages, emptyNote)).toBe( + 'Ask for a note to be written' + ); + }); + + it('asks for the note to be updated when a parsed linked note has a later assistant message', () => { + const messages = [message(1, 'assistant', 3000)]; + + expect(getNoteFollowupLabel(session(), messages, note())).toBe( + 'Ask for the note to be updated' + ); + }); + + it('ignores assistant messages before the note updated timestamp', () => { + const messages = [ + message(1, 'assistant', 1500), + message(2, 'user', 3000), + message(3, 'tool_result', 3500), + ]; + + expect(countAssistantMessagesAfterNote(messages, 2000)).toBe(0); + expect(getNoteFollowupLabel(session(), messages, note())).toBeNull(); + }); + + it('counts multiple assistant messages after the note updated timestamp', () => { + const messages = [ + message(1, 'assistant', 1500), + message(2, 'assistant', 2500), + message(3, 'user', 3000), + message(4, 'assistant', 3500), + ]; + + expect(countAssistantMessagesAfterNote(messages, 2000)).toBe(2); + }); + + it('uses the newest assistant message by timestamp and id', () => { + const messages = [ + message(1, 'assistant', 3000), + message(2, 'assistant', 2500), + message(3, 'assistant', 3000), + ]; + + expect(latestAssistantMessage(messages)?.id).toBe(3); + }); + + it('builds a structured follow-up prompt with a readable visible request', () => { + const updateMessage = buildNoteFollowupMessage(true); + const writeMessage = buildNoteFollowupMessage(false); + + expect(updateMessage.startsWith('')).toBe(true); + expect(updateMessage).toContain('```suggested-next-steps'); + expect(updateMessage).toContain('\n---\n# '); + expect(updateMessage).toContain('Please update the note to reflect the latest chat.'); + expect(writeMessage).toContain('Please write the note for this session.'); + }); +}); diff --git a/apps/staged/src/lib/features/sessions/noteFreshness.ts b/apps/staged/src/lib/features/sessions/noteFreshness.ts new file mode 100644 index 00000000..478dd203 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/noteFreshness.ts @@ -0,0 +1,91 @@ +import type { Session, SessionMessage } from '../../types'; + +export interface LinkedNoteContext { + id: string; + title: string; + content: string; + updatedAt: number; + hasParsedNote: boolean; +} + +export function countAssistantMessagesAfterNote( + messages: SessionMessage[], + noteUpdatedAt: number | null | undefined +): number { + if (typeof noteUpdatedAt !== 'number') return 0; + return messages.filter( + (message) => message.role === 'assistant' && message.createdAt > noteUpdatedAt + ).length; +} + +export function latestAssistantMessage(messages: SessionMessage[]): SessionMessage | null { + let latest: SessionMessage | null = null; + for (const message of messages) { + if (message.role !== 'assistant') continue; + if ( + !latest || + message.createdAt > latest.createdAt || + (message.createdAt === latest.createdAt && message.id > latest.id) + ) { + latest = message; + } + } + return latest; +} + +export function shouldAskForNoteUpdate( + session: Session | null, + messages: SessionMessage[], + noteContext: LinkedNoteContext | null | undefined +): boolean { + if (!session || !noteContext) return false; + if (session.status !== 'completed' || session.completionReason !== 'turn_complete') return false; + + const latestAssistant = latestAssistantMessage(messages); + return !!latestAssistant && latestAssistant.createdAt > noteContext.updatedAt; +} + +export function getNoteFollowupLabel( + session: Session | null, + messages: SessionMessage[], + noteContext: LinkedNoteContext | null | undefined +): string | null { + if (!shouldAskForNoteUpdate(session, messages, noteContext)) return null; + return noteContext?.hasParsedNote + ? 'Ask for the note to be updated' + : 'Ask for a note to be written'; +} + +export function buildNoteFollowupMessage(hasParsedNote: boolean): string { + const visibleRequest = hasParsedNote + ? 'Please update the note to reflect the latest chat.' + : 'Please write the note for this session.'; + + return `<action> +The user is asking you to ${hasParsedNote ? 'update the linked note' : 'write the linked note'} from the latest chat history. + +Use the existing conversation context. Do not create commits. + +Your final response must include a suggested-next-steps fenced block followed by the note content after a horizontal rule: + +\`\`\`suggested-next-steps +{"suggestedNextCommitStep": null, "suggestedNextNoteStep": null} +\`\`\` + +--- +# <Title> +<Body> + +Formatting requirements: +- The opening fence line for suggested-next-steps must be exactly: \`\`\`suggested-next-steps +- The closing fence line must be exactly: \`\`\` +- Put only a JSON object inside the suggested-next-steps block. +- Include both nullable string fields: suggestedNextCommitStep and suggestedNextNoteStep. +- Keep suggested next steps concise; use null when there is no clear next action. +- The \`---\` separator must be on its own line. +- The note content must start immediately after \`---\` with a markdown H1. +- Do not wrap the note in code fences. +</action> + +${visibleRequest}`; +} diff --git a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte index 1602cb4f..11f8c291 100644 --- a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte +++ b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte @@ -53,7 +53,13 @@ onSessionClick?: (sessionId: string) => void; onResumeClick?: (sessionId: string) => void; onCommitClick?: (sha: string) => void; - onNoteClick?: (noteId: string, title: string, content: string, sessionId?: string) => void; + onNoteClick?: ( + noteId: string, + title: string, + content: string, + sessionId: string | undefined, + updatedAt: number + ) => void; onReviewClick?: (reviewId: string) => void; onImageClick?: (imageId: string) => void; onDeleteCommit?: (sha: string, sessionId?: string, opts?: { altKey: boolean }) => void; @@ -199,6 +205,7 @@ noteId?: string; noteTitle?: string; noteContent?: string; + noteUpdatedAt?: number; reviewId?: string; imageId?: string; imageFilename?: string; @@ -336,6 +343,7 @@ noteId: note.id, noteTitle: stripXmlTags(note.title), noteContent: note.content, + noteUpdatedAt: note.updatedAt, deleteDisabledReason: isDeleting ? 'Deleting...' : undefined, completionReason: note.completionReason, hashtagRef: type === 'note' ? `#note:${note.id}` : undefined, @@ -498,7 +506,13 @@ if (item.type === 'commit' && item.commitSha && onCommitClick) { onCommitClick(item.commitSha); } else if (item.type === 'note' && item.noteId && onNoteClick) { - onNoteClick(item.noteId, item.noteTitle ?? '', item.noteContent ?? '', item.sessionId); + onNoteClick( + item.noteId, + item.noteTitle ?? '', + item.noteContent ?? '', + item.sessionId, + item.noteUpdatedAt ?? 0 + ); } else if (item.type === 'review' && item.reviewId && onReviewClick) { onReviewClick(item.reviewId); } else if (item.type === 'image' && item.imageId && onImageClick) { From cf0b474835b5147a5d56aed525a86d3ae2d32472 Mon Sep 17 00:00:00 2001 From: Matt Toohey <contact@matttoohey.com> Date: Fri, 1 May 2026 15:55:14 +1000 Subject: [PATCH 2/5] fix(staged): use SQL COUNT for note freshness and skip feature when updatedAt missing Resolve code review comments on 2f3ef35: - Replace client-side fetch of all session messages in NoteModal with a new count_assistant_messages_after Tauri command that performs a SQL COUNT(role='assistant' AND created_at > timestamp) query, avoiding transferring the full message payload just to count. - Change BranchTimeline noteUpdatedAt fallback from `?? 0` to `undefined`, so notes without a populated updatedAt field skip the freshness feature entirely rather than showing a misleading count. - Update onNoteClick type signature and BranchCard handler to accept number | undefined for noteUpdatedAt. Signed-off-by: Test <test@example.com> --- apps/staged/src-tauri/src/lib.rs | 1 + apps/staged/src-tauri/src/session_commands.rs | 11 ++++++ apps/staged/src-tauri/src/store/messages.rs | 16 +++++++++ apps/staged/src-tauri/src/store/tests.rs | 35 +++++++++++++++++++ apps/staged/src/lib/commands.ts | 7 ++++ .../lib/features/branches/BranchCard.svelte | 2 +- .../src/lib/features/notes/NoteModal.svelte | 9 +++-- .../features/timeline/BranchTimeline.svelte | 4 +-- 8 files changed, 77 insertions(+), 8 deletions(-) diff --git a/apps/staged/src-tauri/src/lib.rs b/apps/staged/src-tauri/src/lib.rs index 7d3d3ff3..7e8b3271 100644 --- a/apps/staged/src-tauri/src/lib.rs +++ b/apps/staged/src-tauri/src/lib.rs @@ -1855,6 +1855,7 @@ pub fn run() { session_commands::get_session, session_commands::get_session_messages, session_commands::get_session_messages_since, + session_commands::count_assistant_messages_after, session_commands::start_session, session_commands::resume_session, session_commands::cancel_session, diff --git a/apps/staged/src-tauri/src/session_commands.rs b/apps/staged/src-tauri/src/session_commands.rs index 52eb20e2..90aae1bc 100644 --- a/apps/staged/src-tauri/src/session_commands.rs +++ b/apps/staged/src-tauri/src/session_commands.rs @@ -120,6 +120,17 @@ pub fn get_session_messages_since( .map_err(|e| e.to_string()) } +#[tauri::command] +pub fn count_assistant_messages_after( + store: tauri::State<'_, Mutex<Option<Arc<Store>>>>, + session_id: String, + after_timestamp: i64, +) -> Result<i64, String> { + get_store(&store)? + .count_assistant_messages_after(&session_id, after_timestamp) + .map_err(|e| e.to_string()) +} + // ============================================================================= // Lifecycle commands // ============================================================================= diff --git a/apps/staged/src-tauri/src/store/messages.rs b/apps/staged/src-tauri/src/store/messages.rs index 8b8bf0c1..56b0ecfe 100644 --- a/apps/staged/src-tauri/src/store/messages.rs +++ b/apps/staged/src-tauri/src/store/messages.rs @@ -87,6 +87,22 @@ impl Store { Ok(()) } + /// Count assistant messages created after a given timestamp. + pub fn count_assistant_messages_after( + &self, + session_id: &str, + after_timestamp: i64, + ) -> Result<i64, StoreError> { + let conn = self.conn.lock().unwrap(); + let count: i64 = conn.query_row( + "SELECT COUNT(*) FROM session_messages + WHERE session_id = ?1 AND role = 'assistant' AND created_at > ?2", + params![session_id, after_timestamp], + |row| row.get(0), + )?; + Ok(count) + } + /// Get messages with id >= since_id (inclusive — re-fetches the last known /// message so the caller picks up streaming content updates). pub fn get_session_messages_since( diff --git a/apps/staged/src-tauri/src/store/tests.rs b/apps/staged/src-tauri/src/store/tests.rs index 60e6efe5..12ff4676 100644 --- a/apps/staged/src-tauri/src/store/tests.rs +++ b/apps/staged/src-tauri/src/store/tests.rs @@ -623,6 +623,41 @@ fn test_session_messages() { assert_eq!(since[1].id, id2); } +#[test] +fn test_count_assistant_messages_after() { + let store = Store::in_memory().unwrap(); + + let session = Session::new_running("test", Path::new("/tmp")); + store.create_session(&session).unwrap(); + + // Add messages with different roles — timestamps are auto-set via now_timestamp() + // so we use a timestamp of 0 to count all assistant messages. + store + .add_session_message(&session.id, MessageRole::User, "hello") + .unwrap(); + store + .add_session_message(&session.id, MessageRole::Assistant, "hi there") + .unwrap(); + store + .add_session_message(&session.id, MessageRole::User, "more") + .unwrap(); + store + .add_session_message(&session.id, MessageRole::Assistant, "reply") + .unwrap(); + + // All assistant messages are after timestamp 0 + let count = store + .count_assistant_messages_after(&session.id, 0) + .unwrap(); + assert_eq!(count, 2); + + // No assistant messages after a far-future timestamp + let count = store + .count_assistant_messages_after(&session.id, i64::MAX) + .unwrap(); + assert_eq!(count, 0); +} + // ============================================================================= // Workdirs // ============================================================================= diff --git a/apps/staged/src/lib/commands.ts b/apps/staged/src/lib/commands.ts index 9e9f7e14..24a9c285 100644 --- a/apps/staged/src/lib/commands.ts +++ b/apps/staged/src/lib/commands.ts @@ -537,6 +537,13 @@ export function getSessionMessagesSince( return invoke('get_session_messages_since', { sessionId, sinceId }); } +export function countAssistantMessagesAfter( + sessionId: string, + afterTimestamp: number +): Promise<number> { + return invoke('count_assistant_messages_after', { sessionId, afterTimestamp }); +} + /** Create a session and immediately start the agent. */ export function startSession( prompt: string, diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index 18cc2c6a..f316ac95 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -759,7 +759,7 @@ title: string, content: string, sessionId: string | undefined, - noteUpdatedAt: number + noteUpdatedAt: number | undefined ) { openNote = { noteId, diff --git a/apps/staged/src/lib/features/notes/NoteModal.svelte b/apps/staged/src/lib/features/notes/NoteModal.svelte index ee69f011..22e646f9 100644 --- a/apps/staged/src/lib/features/notes/NoteModal.svelte +++ b/apps/staged/src/lib/features/notes/NoteModal.svelte @@ -10,11 +10,10 @@ import { marked } from 'marked'; import { sanitize } from '../../shared/sanitize'; import { createBackdropDismissHandlers } from '../../shared/backdropDismiss'; - import { getSessionMessages, handleExternalLinkClick } from '../../api/commands'; + import { countAssistantMessagesAfter, handleExternalLinkClick } from '../../api/commands'; import InContentSearch from '../../shared/InContentSearch.svelte'; import { highlightMatches, clearHighlights, scrollToMatch } from '../../shared/textHighlight'; import { registerSearchShortcutTarget } from '../keyboard/searchTargets'; - import { countAssistantMessagesAfterNote } from '../sessions/noteFreshness'; marked.setOptions({ breaks: true, gfm: true }); @@ -84,10 +83,10 @@ } let stale = false; - getSessionMessages(sid) - .then((messages) => { + countAssistantMessagesAfter(sid, updatedAt) + .then((count) => { if (!stale) { - assistantMessagesAfterNote = countAssistantMessagesAfterNote(messages, updatedAt); + assistantMessagesAfterNote = count; } }) .catch(() => { diff --git a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte index 11f8c291..0b88386f 100644 --- a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte +++ b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte @@ -58,7 +58,7 @@ title: string, content: string, sessionId: string | undefined, - updatedAt: number + updatedAt: number | undefined ) => void; onReviewClick?: (reviewId: string) => void; onImageClick?: (imageId: string) => void; @@ -511,7 +511,7 @@ item.noteTitle ?? '', item.noteContent ?? '', item.sessionId, - item.noteUpdatedAt ?? 0 + item.noteUpdatedAt ); } else if (item.type === 'review' && item.reviewId && onReviewClick) { onReviewClick(item.reviewId); From 9b71cd0347c185a8613aeefbe949228a709fc191 Mon Sep 17 00:00:00 2001 From: Matt Toohey <contact@matttoohey.com> Date: Fri, 1 May 2026 16:04:40 +1000 Subject: [PATCH 3/5] refactor(staged): address code review feedback on note freshness feature - Extract singular/plural chat button label logic from NoteModal into formatChatButtonLabel() in noteFreshness.ts, co-locating all note-freshness display logic. - Fix recurring CTA loop: after the user clicks the note-followup button, the CTA would reappear because the new assistant response has a createdAt after the updated note's updatedAt. Now shouldAskForNoteUpdate checks hasNoteFollowupBeenSent() which detects the marker text in any prior user message, suppressing the CTA once a followup has already been sent. - Refactor onNoteClick callback from 5 positional parameters to a single NoteClickInfo object parameter for readability and safer future additions. - Add tests for formatChatButtonLabel and hasNoteFollowupBeenSent. Signed-off-by: Test <test@example.com> --- .../lib/features/branches/BranchCard.svelte | 22 ++++------- .../src/lib/features/notes/NoteModal.svelte | 9 +---- .../features/sessions/noteFreshness.test.ts | 37 +++++++++++++++++++ .../lib/features/sessions/noteFreshness.ts | 31 ++++++++++++++++ .../features/timeline/BranchTimeline.svelte | 23 +++++------- 5 files changed, 87 insertions(+), 35 deletions(-) diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index f316ac95..f30e297a 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -50,7 +50,7 @@ import { sessionRegistry } from '../../stores/sessionRegistry.svelte'; import { getPreferredAgent } from '../settings/preferences.svelte'; import { agentState, REMOTE_AGENTS } from '../agents/agent.svelte'; - import type { LinkedNoteContext } from '../sessions/noteFreshness'; + import type { LinkedNoteContext, NoteClickInfo } from '../sessions/noteFreshness'; interface Props { branch: Branch; @@ -754,20 +754,14 @@ commitDiffSha = sha; } - function handleNoteClick( - noteId: string, - title: string, - content: string, - sessionId: string | undefined, - noteUpdatedAt: number | undefined - ) { + function handleNoteClick(note: NoteClickInfo) { openNote = { - noteId, - title, - content, - sessionId, - noteUpdatedAt, - nextSteps: computeNoteNextSteps(noteId), + noteId: note.noteId, + title: note.title, + content: note.content, + sessionId: note.sessionId, + noteUpdatedAt: note.updatedAt, + nextSteps: computeNoteNextSteps(note.noteId), }; } diff --git a/apps/staged/src/lib/features/notes/NoteModal.svelte b/apps/staged/src/lib/features/notes/NoteModal.svelte index 22e646f9..eb9150d3 100644 --- a/apps/staged/src/lib/features/notes/NoteModal.svelte +++ b/apps/staged/src/lib/features/notes/NoteModal.svelte @@ -11,6 +11,7 @@ import { sanitize } from '../../shared/sanitize'; import { createBackdropDismissHandlers } from '../../shared/backdropDismiss'; import { countAssistantMessagesAfter, handleExternalLinkClick } from '../../api/commands'; + import { formatChatButtonLabel } from '../sessions/noteFreshness'; import InContentSearch from '../../shared/InContentSearch.svelte'; import { highlightMatches, clearHighlights, scrollToMatch } from '../../shared/textHighlight'; import { registerSearchShortcutTarget } from '../keyboard/searchTargets'; @@ -45,13 +46,7 @@ let copied = $state(false); const backdropDismiss = createBackdropDismissHandlers({ onDismiss: () => onClose() }); let assistantMessagesAfterNote = $state(0); - let chatButtonLabel = $derived( - assistantMessagesAfterNote === 1 - ? '1 message after note in chat' - : assistantMessagesAfterNote > 1 - ? `${assistantMessagesAfterNote} messages after note in chat` - : 'View chat' - ); + let chatButtonLabel = $derived(formatChatButtonLabel(assistantMessagesAfterNote)); // Search state let searchVisible = $state(false); diff --git a/apps/staged/src/lib/features/sessions/noteFreshness.test.ts b/apps/staged/src/lib/features/sessions/noteFreshness.test.ts index 0799b43e..fd403249 100644 --- a/apps/staged/src/lib/features/sessions/noteFreshness.test.ts +++ b/apps/staged/src/lib/features/sessions/noteFreshness.test.ts @@ -3,7 +3,9 @@ import type { Session, SessionMessage, SessionStatus, CompletionReason } from '. import { buildNoteFollowupMessage, countAssistantMessagesAfterNote, + formatChatButtonLabel, getNoteFollowupLabel, + hasNoteFollowupBeenSent, latestAssistantMessage, type LinkedNoteContext, } from './noteFreshness'; @@ -131,4 +133,39 @@ describe('note freshness', () => { expect(updateMessage).toContain('Please update the note to reflect the latest chat.'); expect(writeMessage).toContain('Please write the note for this session.'); }); + + it('suppresses note followup CTA when a followup was already sent', () => { + const followupContent = buildNoteFollowupMessage(true); + const messages = [ + message(1, 'assistant', 1500), + message(2, 'user', 2500, followupContent), + message(3, 'assistant', 3000), + ]; + + expect(hasNoteFollowupBeenSent(messages)).toBe(true); + expect(getNoteFollowupLabel(session(), messages, note())).toBeNull(); + }); + + it('does not suppress CTA when no followup has been sent', () => { + const messages = [message(1, 'user', 1500, 'Can you help me?'), message(2, 'assistant', 3000)]; + + expect(hasNoteFollowupBeenSent(messages)).toBe(false); + expect(getNoteFollowupLabel(session(), messages, note())).toBe( + 'Ask for the note to be updated' + ); + }); +}); + +describe('formatChatButtonLabel', () => { + it('returns singular label for 1 message', () => { + expect(formatChatButtonLabel(1)).toBe('1 message after note in chat'); + }); + + it('returns plural label for multiple messages', () => { + expect(formatChatButtonLabel(5)).toBe('5 messages after note in chat'); + }); + + it('returns generic label for 0 messages', () => { + expect(formatChatButtonLabel(0)).toBe('View chat'); + }); }); diff --git a/apps/staged/src/lib/features/sessions/noteFreshness.ts b/apps/staged/src/lib/features/sessions/noteFreshness.ts index 478dd203..74372f38 100644 --- a/apps/staged/src/lib/features/sessions/noteFreshness.ts +++ b/apps/staged/src/lib/features/sessions/noteFreshness.ts @@ -8,6 +8,15 @@ export interface LinkedNoteContext { hasParsedNote: boolean; } +/** Info passed when a note is clicked in the timeline. */ +export interface NoteClickInfo { + noteId: string; + title: string; + content: string; + sessionId?: string; + updatedAt?: number; +} + export function countAssistantMessagesAfterNote( messages: SessionMessage[], noteUpdatedAt: number | null | undefined @@ -18,6 +27,16 @@ export function countAssistantMessagesAfterNote( ).length; } +/** + * Formats a label for the "view chat" button in the note modal. + * Shows the count of assistant messages after the note was last updated. + */ +export function formatChatButtonLabel(messagesAfterNote: number): string { + if (messagesAfterNote === 1) return '1 message after note in chat'; + if (messagesAfterNote > 1) return `${messagesAfterNote} messages after note in chat`; + return 'View chat'; +} + export function latestAssistantMessage(messages: SessionMessage[]): SessionMessage | null { let latest: SessionMessage | null = null; for (const message of messages) { @@ -33,6 +52,17 @@ export function latestAssistantMessage(messages: SessionMessage[]): SessionMessa return latest; } +/** Marker text embedded in the note followup action block. */ +const NOTE_FOLLOWUP_MARKER = 'The user is asking you to'; + +/** + * Returns true if any user message in the session already contains the + * note-followup prompt. Used to suppress the CTA after it has been clicked. + */ +export function hasNoteFollowupBeenSent(messages: SessionMessage[]): boolean { + return messages.some((m) => m.role === 'user' && m.content.includes(NOTE_FOLLOWUP_MARKER)); +} + export function shouldAskForNoteUpdate( session: Session | null, messages: SessionMessage[], @@ -40,6 +70,7 @@ export function shouldAskForNoteUpdate( ): boolean { if (!session || !noteContext) return false; if (session.status !== 'completed' || session.completionReason !== 'turn_complete') return false; + if (hasNoteFollowupBeenSent(messages)) return false; const latestAssistant = latestAssistantMessage(messages); return !!latestAssistant && latestAssistant.createdAt > noteContext.updatedAt; diff --git a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte index 0b88386f..2271510d 100644 --- a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte +++ b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte @@ -12,6 +12,7 @@ import { FileText, GitCommitVertical, FileSearch, Plus } from 'lucide-svelte'; import { isResumableReason } from '../../types'; import type { BranchTimeline as BranchTimelineData, HashtagItem } from '../../types'; + import type { NoteClickInfo } from '../sessions/noteFreshness'; import TimelineRow from './TimelineRow.svelte'; import type { TimelineItemType, TimelineBadge } from './TimelineRow.svelte'; import TimelineContextMenu from './TimelineContextMenu.svelte'; @@ -53,13 +54,7 @@ onSessionClick?: (sessionId: string) => void; onResumeClick?: (sessionId: string) => void; onCommitClick?: (sha: string) => void; - onNoteClick?: ( - noteId: string, - title: string, - content: string, - sessionId: string | undefined, - updatedAt: number | undefined - ) => void; + onNoteClick?: (note: NoteClickInfo) => void; onReviewClick?: (reviewId: string) => void; onImageClick?: (imageId: string) => void; onDeleteCommit?: (sha: string, sessionId?: string, opts?: { altKey: boolean }) => void; @@ -506,13 +501,13 @@ if (item.type === 'commit' && item.commitSha && onCommitClick) { onCommitClick(item.commitSha); } else if (item.type === 'note' && item.noteId && onNoteClick) { - onNoteClick( - item.noteId, - item.noteTitle ?? '', - item.noteContent ?? '', - item.sessionId, - item.noteUpdatedAt - ); + onNoteClick({ + noteId: item.noteId, + title: item.noteTitle ?? '', + content: item.noteContent ?? '', + sessionId: item.sessionId, + updatedAt: item.noteUpdatedAt, + }); } else if (item.type === 'review' && item.reviewId && onReviewClick) { onReviewClick(item.reviewId); } else if (item.type === 'image' && item.imageId && onImageClick) { From 1a4a28be6e8ec5a9abd1d9bfa3f0d799de07e56a Mon Sep 17 00:00:00 2001 From: Matt Toohey <contact@matttoohey.com> Date: Mon, 4 May 2026 10:38:19 +1000 Subject: [PATCH 4/5] fix(staged): only suppress note followup CTA for markers sent after note update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hasNoteFollowupBeenSent previously scanned all user messages in the session, so a single earlier note-followup would suppress the CTA forever — even after the note was successfully updated and new assistant messages arrived. Now the function accepts a noteUpdatedAt parameter and only considers marker messages with createdAt > noteUpdatedAt. This allows the CTA to reappear when the note is updated (advancing updatedAt) and subsequent assistant turns make it stale again. Add test covering the case where a followup sent before the note update does not suppress the CTA. Signed-off-by: Test <test@example.com> --- .../features/sessions/noteFreshness.test.ts | 22 ++++++++++++++++--- .../lib/features/sessions/noteFreshness.ts | 18 ++++++++++----- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/apps/staged/src/lib/features/sessions/noteFreshness.test.ts b/apps/staged/src/lib/features/sessions/noteFreshness.test.ts index fd403249..1b747d73 100644 --- a/apps/staged/src/lib/features/sessions/noteFreshness.test.ts +++ b/apps/staged/src/lib/features/sessions/noteFreshness.test.ts @@ -134,7 +134,7 @@ describe('note freshness', () => { expect(writeMessage).toContain('Please write the note for this session.'); }); - it('suppresses note followup CTA when a followup was already sent', () => { + it('suppresses note followup CTA when a followup was already sent after note updatedAt', () => { const followupContent = buildNoteFollowupMessage(true); const messages = [ message(1, 'assistant', 1500), @@ -142,14 +142,30 @@ describe('note freshness', () => { message(3, 'assistant', 3000), ]; - expect(hasNoteFollowupBeenSent(messages)).toBe(true); + expect(hasNoteFollowupBeenSent(messages, 2000)).toBe(true); expect(getNoteFollowupLabel(session(), messages, note())).toBeNull(); }); it('does not suppress CTA when no followup has been sent', () => { const messages = [message(1, 'user', 1500, 'Can you help me?'), message(2, 'assistant', 3000)]; - expect(hasNoteFollowupBeenSent(messages)).toBe(false); + expect(hasNoteFollowupBeenSent(messages, 2000)).toBe(false); + expect(getNoteFollowupLabel(session(), messages, note())).toBe( + 'Ask for the note to be updated' + ); + }); + + it('does not suppress CTA when followup was sent before note was updated', () => { + // A followup was sent at t=1500, but the note was updated at t=2000 (after the followup). + // New assistant messages at t=3000 should still trigger the CTA. + const followupContent = buildNoteFollowupMessage(true); + const messages = [ + message(1, 'assistant', 1000), + message(2, 'user', 1500, followupContent), + message(3, 'assistant', 3000), + ]; + + expect(hasNoteFollowupBeenSent(messages, 2000)).toBe(false); expect(getNoteFollowupLabel(session(), messages, note())).toBe( 'Ask for the note to be updated' ); diff --git a/apps/staged/src/lib/features/sessions/noteFreshness.ts b/apps/staged/src/lib/features/sessions/noteFreshness.ts index 74372f38..414c74de 100644 --- a/apps/staged/src/lib/features/sessions/noteFreshness.ts +++ b/apps/staged/src/lib/features/sessions/noteFreshness.ts @@ -56,11 +56,19 @@ export function latestAssistantMessage(messages: SessionMessage[]): SessionMessa const NOTE_FOLLOWUP_MARKER = 'The user is asking you to'; /** - * Returns true if any user message in the session already contains the - * note-followup prompt. Used to suppress the CTA after it has been clicked. + * Returns true if any user message created after `noteUpdatedAt` contains the + * note-followup prompt. Only markers sent after the note was last updated + * suppress the CTA, so that a subsequent note update (which advances updatedAt) + * re-enables the prompt if new assistant messages arrive. */ -export function hasNoteFollowupBeenSent(messages: SessionMessage[]): boolean { - return messages.some((m) => m.role === 'user' && m.content.includes(NOTE_FOLLOWUP_MARKER)); +export function hasNoteFollowupBeenSent( + messages: SessionMessage[], + noteUpdatedAt: number +): boolean { + return messages.some( + (m) => + m.role === 'user' && m.createdAt > noteUpdatedAt && m.content.includes(NOTE_FOLLOWUP_MARKER) + ); } export function shouldAskForNoteUpdate( @@ -70,7 +78,7 @@ export function shouldAskForNoteUpdate( ): boolean { if (!session || !noteContext) return false; if (session.status !== 'completed' || session.completionReason !== 'turn_complete') return false; - if (hasNoteFollowupBeenSent(messages)) return false; + if (hasNoteFollowupBeenSent(messages, noteContext.updatedAt)) return false; const latestAssistant = latestAssistantMessage(messages); return !!latestAssistant && latestAssistant.createdAt > noteContext.updatedAt; From 3cfaae9a4943ecfe77342509738cddfa0d9e1409 Mon Sep 17 00:00:00 2001 From: Matt Toohey <contact@matttoohey.com> Date: Mon, 4 May 2026 17:15:56 +1000 Subject: [PATCH 5/5] chore(staged): rebase note freshness branch onto main Rebase the branch onto origin/main after fetching origin and using the requested merge-base flow. Main now carries migrations through 0014-add-pipeline; this branch has no migration diff remaining to renumber. Signed-off-by: Matt Toohey <contact@matttoohey.com> Signed-off-by: Test <test@example.com> --- apps/staged/src/lib/features/sessions/noteFreshness.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/staged/src/lib/features/sessions/noteFreshness.test.ts b/apps/staged/src/lib/features/sessions/noteFreshness.test.ts index 1b747d73..90b2d934 100644 --- a/apps/staged/src/lib/features/sessions/noteFreshness.test.ts +++ b/apps/staged/src/lib/features/sessions/noteFreshness.test.ts @@ -18,6 +18,7 @@ function session( id: 'session-1', prompt: 'Write a note', status, + provider: null, agentId: null, errorMessage: null, completionReason,