diff --git a/apps/mark/src-tauri/src/lib.rs b/apps/mark/src-tauri/src/lib.rs index d6dbe5d7..79921c5a 100644 --- a/apps/mark/src-tauri/src/lib.rs +++ b/apps/mark/src-tauri/src/lib.rs @@ -242,6 +242,7 @@ pub struct ReviewTimelineItem { pub scope: String, pub session_id: Option, pub session_status: Option, + pub title: Option, pub comment_count: usize, pub created_at: i64, pub updated_at: i64, @@ -1413,6 +1414,7 @@ fn build_branch_timeline(store: &Arc, branch_id: &str) -> Result, branch_id: &str) -> Vec format!( + "### {} — {} ({} scope)\n", + title, + short_sha, + review.scope.as_str(), + ), + None => format!( + "### Code Review of {} ({} scope)\n", + short_sha, + review.scope.as_str(), + ), + }; + let mut content = heading; // Group comments by file path let mut by_path: std::collections::BTreeMap<&str, Vec<&crate::store::models::Comment>> = @@ -1425,7 +1434,18 @@ Reserve `\"warning\"` and `\"issue\"` for genuine concerns. ## Output format -Return your review as exactly one fenced JSON block with this structure: +Your response must start directly with the review-title fenced block below — \ +do not output any preamble, commentary, or thinking before it. + +Provide a single-sentence title (max 15 words) that conveys your overall \ +confidence level in the changes. Do not describe what the changes do — instead focus on \ +how confident you are that they are correct and safe. Wrap it in a fenced block: + +```review-title +Looks solid overall with one minor edge case worth checking +``` + +Then return your review comments as exactly one fenced JSON block: ```review-comments [ @@ -1445,10 +1465,12 @@ Return your review as exactly one fenced JSON block with this structure: ``` Formatting requirements: -- The opening fence line must be exactly: ```review-comments -- The closing fence line must be exactly: ``` -- Put only the JSON array between the fences (no prose or markdown inside). -- Do not wrap this block in any additional code fences. +- The opening fence line for the title must be exactly: ```review-title +- The opening fence line for comments must be exactly: ```review-comments +- Each closing fence line must be exactly: ``` +- Put only plain text (no markdown) inside the review-title block. +- Put only the JSON array inside the review-comments block (no prose or markdown). +- Do not wrap these blocks in any additional code fences. Rules: - `span` uses 0-indexed line numbers from the \"after\" side of the diff (exclusive end). @@ -1483,11 +1505,17 @@ mod tests { &BranchSessionType::Review, ); + assert!( + prompt.contains("Then return your review comments as exactly one fenced JSON block:") + ); assert!(prompt - .contains("Return your review as exactly one fenced JSON block with this structure:")); - assert!(prompt.contains("The opening fence line must be exactly: ```review-comments")); - assert!(prompt.contains("The closing fence line must be exactly: ```")); + .contains("The opening fence line for the title must be exactly: ```review-title")); assert!(prompt - .contains("Put only the JSON array between the fences (no prose or markdown inside).")); + .contains("The opening fence line for comments must be exactly: ```review-comments")); + assert!(prompt.contains("Each closing fence line must be exactly: ```")); + assert!(prompt.contains( + "Put only the JSON array inside the review-comments block (no prose or markdown)." + )); + assert!(prompt.contains("do not output any preamble, commentary, or thinking before it")); } } diff --git a/apps/mark/src-tauri/src/session_runner.rs b/apps/mark/src-tauri/src/session_runner.rs index 91bbfafb..8b7665b5 100644 --- a/apps/mark/src-tauri/src/session_runner.rs +++ b/apps/mark/src-tauri/src/session_runner.rs @@ -518,17 +518,30 @@ fn run_post_completion_hooks( } } - // --- Review comment extraction --- + // --- Review comment and title extraction --- if let Ok(Some(review)) = store.get_review_by_session(session_id) { - if review.comments.is_empty() { - if let Ok(messages) = store.get_session_messages(session_id) { - let full_text: String = messages - .iter() - .filter(|m| m.role == MessageRole::Assistant) - .map(|m| m.content.as_str()) - .collect::>() - .join("\n"); + if let Ok(messages) = store.get_session_messages(session_id) { + let full_text: String = messages + .iter() + .filter(|m| m.role == MessageRole::Assistant) + .map(|m| m.content.as_str()) + .collect::>() + .join("\n"); + + // Extract and save review title (always attempt, even if comments exist) + if review.title.is_none() { + if let Some(title) = extract_review_title(&full_text) { + log::info!("Session {session_id}: extracted review title: {title}"); + if let Err(e) = store.update_review_title(&review.id, &title) { + log::error!("Failed to update review title: {e}"); + } + } else { + log::warn!("Session {session_id}: review session completed but no review-title block found"); + } + } + // Extract and save review comments + if review.comments.is_empty() { let comments = extract_review_comments(&full_text); if comments.is_empty() { log::warn!("Session {session_id}: review session completed but no review-comments block found"); @@ -715,8 +728,9 @@ fn extract_review_comments(text: &str) -> Vec { let marker_start = "```review-comments"; let mut search_from = 0; - while let Some(start_pos) = text[search_from..].find(marker_start) { - let block_start = search_from + start_pos + marker_start.len(); + while let Some(rel_pos) = find_opening_fence(&text[search_from..], marker_start) { + let start_pos = search_from + rel_pos; + let block_start = start_pos + marker_start.len(); // Skip to the next line after the opening marker let json_start = match text[block_start..].find('\n') { Some(pos) => block_start + pos + 1, @@ -768,6 +782,42 @@ fn extract_review_comments(text: &str) -> Vec { comments } +/// Extract the review title from assistant output. +/// +/// Looks for a ```review-title fenced block and returns the trimmed text inside. +/// The opening fence must appear at the start of a line to avoid matching the +/// marker when it appears inside regular prose (e.g. the LLM discussing the +/// extraction logic itself). +fn extract_review_title(text: &str) -> Option { + let marker = "```review-title"; + let start_pos = find_opening_fence(text, marker)?; + let block_start = start_pos + marker.len(); + let content_start = block_start + text[block_start..].find('\n')? + 1; + let end_pos = find_closing_fence(&text[content_start..])?; + let title = text[content_start..content_start + end_pos].trim(); + if title.is_empty() { + None + } else { + Some(title.to_string()) + } +} + +/// Find an opening fence marker (e.g. ` ```review-title `) that appears at the +/// start of a line (position 0 or immediately after `\n`). Returns the byte +/// offset of the marker within `text`, or `None` if no line-start match exists. +fn find_opening_fence(text: &str, marker: &str) -> Option { + let mut pos = 0; + while pos < text.len() { + let candidate = text[pos..].find(marker)?; + let abs = pos + candidate; + if abs == 0 || text.as_bytes()[abs - 1] == b'\n' { + return Some(abs); + } + pos = abs + marker.len(); + } + None +} + /// Find the closing ``` fence for a code block. /// /// A closing fence must appear at the start of a line (after a newline) and @@ -888,6 +938,111 @@ mod tests { assert_eq!(find_closing_fence(text), Some(5)); } + // ── extract_review_title ────────────────────────────────────────── + + #[test] + fn extract_title_simple() { + let text = "Here is my review:\n\n```review-title\nSolid refactor with one edge case\n```\n\nAnd comments..."; + assert_eq!( + extract_review_title(text), + Some("Solid refactor with one edge case".to_string()) + ); + } + + #[test] + fn extract_title_trims_whitespace() { + let text = "```review-title\n Clean changes, no concerns \n```\n"; + assert_eq!( + extract_review_title(text), + Some("Clean changes, no concerns".to_string()) + ); + } + + #[test] + fn extract_title_none_when_missing() { + let text = "Just a normal message with no review title."; + assert_eq!(extract_review_title(text), None); + } + + #[test] + fn extract_title_none_when_empty() { + let text = "```review-title\n\n```\n"; + assert_eq!(extract_review_title(text), None); + } + + #[test] + fn extract_title_with_review_comments() { + let text = r#"Here is my review: + +```review-title +Risky changes to auth flow need closer look +``` + +```review-comments +[ + { + "path": "src/auth.rs", + "span": { "start": 10, "end": 15 }, + "content": "Missing validation." + } +] +``` +"#; + assert_eq!( + extract_review_title(text), + Some("Risky changes to auth flow need closer look".to_string()) + ); + } + + #[test] + fn extract_title_ignores_llm_preamble() { + let text = r#"I now have a complete picture of the changes. Let me produce the review. + +```review-title +Clean, well-tested feature addition with good backward compatibility +``` + +```review-comments +[ + { + "path": "src/foo.rs", + "span": { "start": 10, "end": 15 }, + "type": "suggestion", + "content": "Consider adding a test." + } +] +``` +"#; + assert_eq!( + extract_review_title(text), + Some( + "Clean, well-tested feature addition with good backward compatibility".to_string() + ) + ); + } + + #[test] + fn extract_title_marker_mentioned_in_preamble() { + // The LLM discusses the ```review-title marker in its preamble before + // actually producing the fenced block. The extractor must skip the + // mid-line mention and find the real opening fence. + let text = r#"Let me check whether `"```review-title"` would match inside `"```review-title-v2"`: +I now have a complete picture. Let me produce the review. + +```review-title +Solid changes with minor nit +``` + +```review-comments +[] +``` +"#; + assert_eq!( + extract_review_title(text), + Some("Solid changes with minor nit".to_string()) + ); + } + // ── extract_review_comments ───────────────────────────────────────── #[test] diff --git a/apps/mark/src-tauri/src/store/mod.rs b/apps/mark/src-tauri/src/store/mod.rs index a98b2b07..59e5cc27 100644 --- a/apps/mark/src-tauri/src/store/mod.rs +++ b/apps/mark/src-tauri/src/store/mod.rs @@ -62,7 +62,7 @@ impl From for StoreError { /// /// Bump this whenever the schema changes. /// Many app versions may share the same schema version. -pub const SCHEMA_VERSION: i64 = 17; +pub const SCHEMA_VERSION: i64 = 18; /// Oldest schema version we can migrate forward from. /// @@ -373,6 +373,7 @@ impl Store { commit_sha TEXT NOT NULL, scope TEXT NOT NULL, session_id TEXT, + title TEXT, created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL ); @@ -518,8 +519,13 @@ impl Store { } // v15→v16 and v16→v17 previously added a workstation_id column that - // has been removed. The migrations are no-ops now; SCHEMA_VERSION stays - // at 17 so existing databases are simply stamped forward. + // has been removed. The migrations are no-ops now. + + if db_version < 18 { + // v17 → v18: add title column to reviews + conn.execute_batch("ALTER TABLE reviews ADD COLUMN title TEXT DEFAULT NULL;") + .ok(); // Ignore error if column already exists (fresh DB) + } // Stamp the current schema version so future opens skip applied migrations. conn.execute( diff --git a/apps/mark/src-tauri/src/store/models.rs b/apps/mark/src-tauri/src/store/models.rs index 6ae55a7b..acd16ff1 100644 --- a/apps/mark/src-tauri/src/store/models.rs +++ b/apps/mark/src-tauri/src/store/models.rs @@ -822,6 +822,8 @@ pub struct Review { pub commit_sha: String, pub scope: ReviewScope, pub session_id: Option, + /// AI-generated one-sentence title summarising the review's confidence. + pub title: Option, /// Paths that have been marked as reviewed. pub reviewed: Vec, /// Comments attached to specific locations. @@ -841,6 +843,7 @@ impl Review { commit_sha: commit_sha.to_string(), scope, session_id: None, + title: None, reviewed: Vec::new(), comments: Vec::new(), reference_files: Vec::new(), diff --git a/apps/mark/src-tauri/src/store/reviews.rs b/apps/mark/src-tauri/src/store/reviews.rs index 3525df4e..3b6a0b3c 100644 --- a/apps/mark/src-tauri/src/store/reviews.rs +++ b/apps/mark/src-tauri/src/store/reviews.rs @@ -12,14 +12,15 @@ impl Store { pub fn create_review(&self, review: &Review) -> Result<(), StoreError> { let conn = self.conn.lock().unwrap(); conn.execute( - "INSERT INTO reviews (id, branch_id, commit_sha, scope, session_id, created_at, updated_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)", + "INSERT INTO reviews (id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", params![ review.id, review.branch_id, review.commit_sha, review.scope.as_str(), review.session_id, + review.title, review.created_at, review.updated_at, ], @@ -47,7 +48,7 @@ impl Store { // when multiple reviews share the same (branch, commit, scope) triple. let existing: Option = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at FROM reviews WHERE branch_id = ?1 AND commit_sha = ?2 AND scope = ?3 ORDER BY created_at DESC LIMIT 1", @@ -64,14 +65,15 @@ impl Store { // Create new let review = Review::new(branch_id, commit_sha, scope); conn.execute( - "INSERT INTO reviews (id, branch_id, commit_sha, scope, session_id, created_at, updated_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)", + "INSERT INTO reviews (id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", params![ review.id, review.branch_id, review.commit_sha, review.scope.as_str(), review.session_id, + review.title, review.created_at, review.updated_at, ], @@ -94,7 +96,7 @@ impl Store { // reviews share the same (branch, commit, scope) triple. let existing: Option = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at FROM reviews WHERE branch_id = ?1 AND commit_sha = ?2 AND scope = ?3 ORDER BY created_at DESC LIMIT 1", @@ -117,7 +119,7 @@ impl Store { let conn = self.conn.lock().unwrap(); let review = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at FROM reviews WHERE id = ?1", params![id], Self::row_to_review_header, @@ -137,7 +139,7 @@ impl Store { pub fn list_reviews_for_branch(&self, branch_id: &str) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); let mut stmt = conn.prepare( - "SELECT id, branch_id, commit_sha, scope, session_id, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at FROM reviews WHERE branch_id = ?1 ORDER BY created_at ASC", )?; let rows = stmt.query_map(params![branch_id], Self::row_to_review_header)?; @@ -258,7 +260,7 @@ impl Store { let conn = self.conn.lock().unwrap(); let review = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at FROM reviews WHERE session_id = ?1", params![session_id], Self::row_to_review_header, @@ -281,6 +283,16 @@ impl Store { Ok(()) } + /// Update the title of a review. + pub fn update_review_title(&self, id: &str, title: &str) -> Result<(), StoreError> { + let conn = self.conn.lock().unwrap(); + conn.execute( + "UPDATE reviews SET title = ?1, updated_at = ?2 WHERE id = ?3", + params![title, now_timestamp(), id], + )?; + Ok(()) + } + // ========================================================================= // Internal helpers // ========================================================================= @@ -301,11 +313,12 @@ impl Store { commit_sha: row.get(2)?, scope: ReviewScope::parse(&scope_str).unwrap_or(ReviewScope::Commit), session_id: row.get(4)?, + title: row.get(5)?, reviewed: Vec::new(), comments: Vec::new(), reference_files: Vec::new(), - created_at: row.get(5)?, - updated_at: row.get(6)?, + created_at: row.get(6)?, + updated_at: row.get(7)?, }) } diff --git a/apps/mark/src/lib/features/timeline/BranchTimeline.svelte b/apps/mark/src/lib/features/timeline/BranchTimeline.svelte index b0bfc8af..85c40090 100644 --- a/apps/mark/src/lib/features/timeline/BranchTimeline.svelte +++ b/apps/mark/src/lib/features/timeline/BranchTimeline.svelte @@ -245,7 +245,7 @@ all.push({ key: `review-${review.id}`, type, - title: `Code Review`, + title: review.title || 'Code Review', meta: countParts.length > 0 ? countParts.join(' + ') : undefined, secondaryMeta: isDeleting ? 'Deleting...' : secondaryMeta, deleting: isDeleting, diff --git a/apps/mark/src/lib/features/timeline/liveSessionHints.ts b/apps/mark/src/lib/features/timeline/liveSessionHints.ts index 876bd868..55dc1cff 100644 --- a/apps/mark/src/lib/features/timeline/liveSessionHints.ts +++ b/apps/mark/src/lib/features/timeline/liveSessionHints.ts @@ -65,7 +65,13 @@ function formatToolCallHint(content: string, repoDir?: string | null): string | } function formatAssistantHint(content: string): string | undefined { - const lines = stripXmlTags(content) + const stripped = stripXmlTags(content); + + // When the response contains review fenced blocks, the surrounding text is just + // LLM preamble (e.g. "I now have a complete picture…") — not a useful hint. + if (/```review-(?:title|comments)/.test(stripped)) return undefined; + + const lines = stripped .replace(/```[\s\S]*?```/g, ' ') .split(/\r?\n/) .map((line) => diff --git a/apps/mark/src/lib/types.ts b/apps/mark/src/lib/types.ts index 893fb204..7d208b11 100644 --- a/apps/mark/src/lib/types.ts +++ b/apps/mark/src/lib/types.ts @@ -115,6 +115,7 @@ export interface ReviewTimelineItem { scope: string; sessionId: string | null; sessionStatus: string | null; + title: string | null; commentCount: number; createdAt: number; updatedAt: number;