From d79aeed81bed5afb850149df0994b760ecaf7f86 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 11 Mar 2026 10:14:37 +1100 Subject: [PATCH 1/2] fix(mark): strip wrapper from fallback note titles Project-level chat sessions prepend an ... block to the stored session prompt. When the assistant's note lacks an H1 heading, the fallback title was derived from the first 80 characters of this wrapped prompt, causing raw XML to appear as the note title. Add strip_action_wrapper() to remove the injected block before generating fallback titles for both regular and project notes. Co-Authored-By: Claude Opus 4.6 --- apps/mark/src-tauri/src/session_runner.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/apps/mark/src-tauri/src/session_runner.rs b/apps/mark/src-tauri/src/session_runner.rs index 20754009..e9645bb9 100644 --- a/apps/mark/src-tauri/src/session_runner.rs +++ b/apps/mark/src-tauri/src/session_runner.rs @@ -555,8 +555,9 @@ fn run_post_completion_hooks( .ok() .flatten() .map(|s| { - let t: String = s.prompt.chars().take(80).collect(); - if s.prompt.len() > 80 { + let prompt = strip_action_wrapper(&s.prompt); + let t: String = prompt.chars().take(80).collect(); + if prompt.len() > 80 { format!("{t}…") } else { t @@ -597,8 +598,9 @@ fn run_post_completion_hooks( .ok() .flatten() .map(|s| { - let t: String = s.prompt.chars().take(80).collect(); - if s.prompt.len() > 80 { + let prompt = strip_action_wrapper(&s.prompt); + let t: String = prompt.chars().take(80).collect(); + if prompt.len() > 80 { format!("{t}…") } else { t @@ -804,6 +806,18 @@ fn line_byte_offsets(text: &str) -> impl Iterator { }) } +/// Strip leading `...` blocks from a prompt so that fallback +/// title generation uses the user's actual text rather than injected XML. +fn strip_action_wrapper(prompt: &str) -> &str { + let trimmed = prompt.trim_start(); + if let Some(rest) = trimmed.strip_prefix("") { + if let Some(end) = rest.find("") { + return rest[end + "".len()..].trim_start(); + } + } + prompt +} + /// Extract a title (leading `# H1`) from note content. /// /// Returns `(title, body_without_title)`. If no H1 is found, title is empty From 6ba2f10bfe7a4b1dd0b5f751ae6f66df3ca7e37c Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 11 Mar 2026 10:28:11 +1100 Subject: [PATCH 2/2] refactor(mark): consolidate note extraction and add strip_action_wrapper tests Extract resolve_note_title_and_body() to deduplicate the identical title/body extraction + fallback-title logic that was copy-pasted between the repo-note and project-note paths in run_post_completion_hooks. Both paths now share a single loop over NoteTarget variants, scanning assistant messages only once. Add 6 unit tests for strip_action_wrapper covering: no wrapper, normal wrapper, leading whitespace, missing closing tag, whitespace-only remainder, and nested action tags. Co-Authored-By: Claude Opus 4.6 --- apps/mark/src-tauri/src/session_runner.rs | 219 ++++++++++++++-------- 1 file changed, 144 insertions(+), 75 deletions(-) diff --git a/apps/mark/src-tauri/src/session_runner.rs b/apps/mark/src-tauri/src/session_runner.rs index e9645bb9..0ad1967e 100644 --- a/apps/mark/src-tauri/src/session_runner.rs +++ b/apps/mark/src-tauri/src/session_runner.rs @@ -536,53 +536,47 @@ fn run_post_completion_hooks( } } - // --- Note extraction --- - if let Ok(Some(note)) = store.get_note_by_session(session_id) { - // Collect all assistant messages for this session and find note content. - if let Ok(messages) = store.get_session_messages(session_id) { - let note_content = messages - .iter() - .rev() - .filter(|m| m.role == MessageRole::Assistant) - .find_map(|m| extract_note_content(&m.content)); - - if let Some(note_content) = note_content { - let (title, body) = extract_note_title(¬e_content); - let final_title = if title.is_empty() { - // Fallback title from the session prompt - store - .get_session(session_id) - .ok() - .flatten() - .map(|s| { - let prompt = strip_action_wrapper(&s.prompt); - let t: String = prompt.chars().take(80).collect(); - if prompt.len() > 80 { - format!("{t}…") - } else { - t - } - }) - .unwrap_or_else(|| "Untitled Note".to_string()) - } else { - title - }; - let is_amendment = !note.content.is_empty(); - log::info!( - "Session {session_id}: {} note \"{final_title}\"", - if is_amendment { "amended" } else { "extracted" } - ); - if let Err(e) = store.update_note_title_and_content(¬e.id, &final_title, &body) { - log::error!("Failed to update note content: {e}"); - } - } else { - log::warn!("Session {session_id}: note session completed but no --- found in assistant output"); - } - } + // --- Note extraction (repo notes and project notes) --- + // + // Both paths share the same message-scanning and title-resolution logic + // via `resolve_note_title_and_body`, differing only in which DB record + // they read/write. + struct NoteTarget { + id: String, + is_amendment: bool, + kind: NoteKind, + } + enum NoteKind { + Repo, + Project, } - // --- Project note extraction --- - if let Ok(Some(note)) = store.get_project_note_by_session(session_id) { + let note_targets: Vec = [ + store + .get_note_by_session(session_id) + .ok() + .flatten() + .map(|n| NoteTarget { + id: n.id, + is_amendment: !n.content.is_empty(), + kind: NoteKind::Repo, + }), + store + .get_project_note_by_session(session_id) + .ok() + .flatten() + .map(|n| NoteTarget { + id: n.id, + is_amendment: !n.content.is_empty(), + kind: NoteKind::Project, + }), + ] + .into_iter() + .flatten() + .collect(); + + if !note_targets.is_empty() { + // Scan assistant messages once for all note targets. if let Ok(messages) = store.get_session_messages(session_id) { let note_content = messages .iter() @@ -590,38 +584,38 @@ fn run_post_completion_hooks( .filter(|m| m.role == MessageRole::Assistant) .find_map(|m| extract_note_content(&m.content)); - if let Some(note_content) = note_content { - let (title, body) = extract_note_title(¬e_content); - let final_title = if title.is_empty() { - store - .get_session(session_id) - .ok() - .flatten() - .map(|s| { - let prompt = strip_action_wrapper(&s.prompt); - let t: String = prompt.chars().take(80).collect(); - if prompt.len() > 80 { - format!("{t}…") - } else { - t - } - }) - .unwrap_or_else(|| "Untitled Note".to_string()) - } else { - title + for target in ¬e_targets { + let label = match target.kind { + NoteKind::Repo => "note", + NoteKind::Project => "project note", }; - let is_amendment = !note.content.is_empty(); - log::info!( - "Session {session_id}: {} project note \"{final_title}\"", - if is_amendment { "amended" } else { "extracted" } - ); - if let Err(e) = - store.update_project_note_title_and_content(¬e.id, &final_title, &body) - { - log::error!("Failed to update project note content: {e}"); + if let Some(ref note_content) = note_content { + let (final_title, body) = + resolve_note_title_and_body(note_content, store, session_id); + log::info!( + "Session {session_id}: {} {label} \"{final_title}\"", + if target.is_amendment { + "amended" + } else { + "extracted" + } + ); + let result = match target.kind { + NoteKind::Repo => { + store.update_note_title_and_content(&target.id, &final_title, &body) + } + NoteKind::Project => store.update_project_note_title_and_content( + &target.id, + &final_title, + &body, + ), + }; + if let Err(e) = result { + log::error!("Failed to update {label} content: {e}"); + } + } else { + log::warn!("Session {session_id}: {label} session completed but no --- found in assistant output"); } - } else { - log::warn!("Session {session_id}: project note session completed but no --- found in assistant output"); } } } @@ -839,6 +833,37 @@ fn extract_note_title(content: &str) -> (String, String) { } } +/// Parse note content into a final `(title, body)` pair, using the session +/// prompt as a fallback title when the note has no H1 heading. +/// +/// Shared by both repo-note and project-note extraction paths. +fn resolve_note_title_and_body( + note_content: &str, + store: &Store, + session_id: &str, +) -> (String, String) { + let (title, body) = extract_note_title(note_content); + let final_title = if title.is_empty() { + store + .get_session(session_id) + .ok() + .flatten() + .map(|s| { + let prompt = strip_action_wrapper(&s.prompt); + let t: String = prompt.chars().take(80).collect(); + if prompt.len() > 80 { + format!("{t}…") + } else { + t + } + }) + .unwrap_or_else(|| "Untitled Note".to_string()) + } else { + title + }; + (final_title, body) +} + /// Extract review comments from assistant output. /// /// Looks for ```review-comments fenced blocks and parses the JSON array inside. @@ -1435,4 +1460,48 @@ Second batch: assert!(title.is_empty()); assert_eq!(body, "No heading here.\nJust text."); } + + // ── strip_action_wrapper ──────────────────────────────────────────── + + #[test] + fn strip_action_no_wrapper() { + assert_eq!(strip_action_wrapper("plain prompt"), "plain prompt"); + } + + #[test] + fn strip_action_normal_wrapper() { + let input = "\nSome injected context\n\nActual user prompt"; + assert_eq!(strip_action_wrapper(input), "Actual user prompt"); + } + + #[test] + fn strip_action_wrapper_with_leading_whitespace() { + let input = " \ninjected user text"; + assert_eq!(strip_action_wrapper(input), "user text"); + } + + #[test] + fn strip_action_missing_closing_tag() { + // If the closing tag is absent, return the original prompt unchanged. + let input = "unclosed block\nuser text"; + assert_eq!(strip_action_wrapper(input), input); + } + + #[test] + fn strip_action_whitespace_only_after_stripping() { + // After stripping the wrapper, only whitespace remains — should trim to empty. + let input = "stuff \n "; + assert_eq!(strip_action_wrapper(input), ""); + } + + #[test] + fn strip_action_nested_action_tags() { + // The function uses the *first* it finds, so a nested + // inside the wrapper content would split there. + let input = "outer inner leftover\nreal prompt"; + assert_eq!( + strip_action_wrapper(input), + "leftover\nreal prompt" + ); + } }