From 65922f83f304edcb0d628560b03e5787840bf992 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 2 Apr 2026 13:37:02 +1100 Subject: [PATCH 1/2] fix(staged): stabilize timeline ordering by completion time Add write-once completion timestamps for notes, reviews, and project notes so timeline ordering reflects when items actually finished instead of when they were queued or later touched. Also align displayed times with completion sorting, restamp adopted auto reviews when they become visible, and cover the new behavior with migration and store tests. --- apps/staged/src-tauri/src/lib.rs | 2 + apps/staged/src-tauri/src/note_commands.rs | 1 + apps/staged/src-tauri/src/session_commands.rs | 2 +- apps/staged/src-tauri/src/session_runner.rs | 12 ++ .../src-tauri/src/store/migration_tests.rs | 2 +- .../migrations/0007-add-completed-at/up.sql | 19 ++++ .../0008-add-project-note-completed-at/up.sql | 9 ++ apps/staged/src-tauri/src/store/models.rs | 14 +++ apps/staged/src-tauri/src/store/notes.rs | 34 ++++-- .../src-tauri/src/store/project_notes.rs | 35 ++++-- apps/staged/src-tauri/src/store/reviews.rs | 57 +++++++--- apps/staged/src-tauri/src/store/tests.rs | 103 ++++++++++++++++++ apps/staged/src-tauri/src/timeline.rs | 2 + .../features/projects/ProjectSection.svelte | 4 +- .../features/timeline/BranchTimeline.svelte | 11 +- apps/staged/src/lib/types.ts | 3 + apps/staged/tsconfig.app.json | 3 +- 17 files changed, 268 insertions(+), 45 deletions(-) create mode 100644 apps/staged/src-tauri/src/store/migrations/0007-add-completed-at/up.sql create mode 100644 apps/staged/src-tauri/src/store/migrations/0008-add-project-note-completed-at/up.sql diff --git a/apps/staged/src-tauri/src/lib.rs b/apps/staged/src-tauri/src/lib.rs index 7ee71a5f..861b7227 100644 --- a/apps/staged/src-tauri/src/lib.rs +++ b/apps/staged/src-tauri/src/lib.rs @@ -139,6 +139,7 @@ pub struct NoteTimelineItem { pub completion_reason: Option, pub created_at: i64, pub updated_at: i64, + pub completed_at: Option, } /// Review with session status resolved. @@ -155,6 +156,7 @@ pub struct ReviewTimelineItem { pub comment_count: usize, pub created_at: i64, pub updated_at: i64, + pub completed_at: Option, } /// Image with session status resolved. diff --git a/apps/staged/src-tauri/src/note_commands.rs b/apps/staged/src-tauri/src/note_commands.rs index ef4ccdde..544f292a 100644 --- a/apps/staged/src-tauri/src/note_commands.rs +++ b/apps/staged/src-tauri/src/note_commands.rs @@ -24,6 +24,7 @@ pub fn create_note( completion_reason: None, created_at: note.created_at, updated_at: note.updated_at, + completed_at: note.completed_at, }) } diff --git a/apps/staged/src-tauri/src/session_commands.rs b/apps/staged/src-tauri/src/session_commands.rs index 58fcbd83..af238695 100644 --- a/apps/staged/src-tauri/src/session_commands.rs +++ b/apps/staged/src-tauri/src/session_commands.rs @@ -2025,7 +2025,7 @@ fn project_note_timeline_entries( let content = format_project_note_for_context(¬e.id, ¬e.title, ¬e.content, workspace_name); entries.push(TimelineEntry { - timestamp: note.created_at / 1000, + timestamp: note.completed_at.unwrap_or(note.created_at) / 1000, order: 0, content, }); diff --git a/apps/staged/src-tauri/src/session_runner.rs b/apps/staged/src-tauri/src/session_runner.rs index caa2d0ad..f3960a45 100644 --- a/apps/staged/src-tauri/src/session_runner.rs +++ b/apps/staged/src-tauri/src/session_runner.rs @@ -675,6 +675,13 @@ fn run_post_completion_hooks( } } else { log::warn!("Session {session_id}: {label} session completed but no --- found in assistant output"); + let result = match target.kind { + NoteKind::Repo => store.mark_note_completed(&target.id), + NoteKind::Project => store.mark_project_note_completed(&target.id), + }; + if let Err(e) = result { + log::error!("Failed to mark {label} completed: {e}"); + } } } } @@ -699,6 +706,11 @@ fn run_post_completion_hooks( } } else { log::warn!("Session {session_id}: review session completed but no review-title block found"); + // Still mark the review as completed so completed_at is set + // for timeline sorting, even without a title. + if let Err(e) = store.mark_review_completed(&review.id) { + log::error!("Failed to mark review completed: {e}"); + } } } diff --git a/apps/staged/src-tauri/src/store/migration_tests.rs b/apps/staged/src-tauri/src/store/migration_tests.rs index 6e61ca9d..379557fd 100644 --- a/apps/staged/src-tauri/src/store/migration_tests.rs +++ b/apps/staged/src-tauri/src/store/migration_tests.rs @@ -133,7 +133,7 @@ fn test_store_bootstraps_fresh_database_with_baseline_migration() { ) .unwrap(); - assert_eq!(version, 6); + assert_eq!(version, 8); assert_eq!(app_version, super::APP_VERSION); assert!(table_exists(&conn, "projects")); assert!(table_exists(&conn, "project_notes")); diff --git a/apps/staged/src-tauri/src/store/migrations/0007-add-completed-at/up.sql b/apps/staged/src-tauri/src/store/migrations/0007-add-completed-at/up.sql new file mode 100644 index 00000000..7cef0c5a --- /dev/null +++ b/apps/staged/src-tauri/src/store/migrations/0007-add-completed-at/up.sql @@ -0,0 +1,19 @@ +-- Add completed_at column to notes and reviews. +-- This records when the AI session finished producing the item, giving us a +-- stable timestamp for timeline sorting that won't shift on later edits +-- (unlike updated_at which bumps on every user interaction). +-- +-- Commits don't need this column because their created_at is already set to +-- the git commit timestamp (i.e. the completion time). +-- +-- NULL means the item hasn't completed yet (still queued/generating). +-- For existing rows we backfill with updated_at, which is the best +-- approximation we have. + +ALTER TABLE notes ADD COLUMN completed_at INTEGER; +UPDATE notes SET completed_at = updated_at WHERE content != ''; + +ALTER TABLE reviews ADD COLUMN completed_at INTEGER; +UPDATE reviews SET completed_at = updated_at + WHERE title IS NOT NULL + OR id IN (SELECT DISTINCT review_id FROM comments); diff --git a/apps/staged/src-tauri/src/store/migrations/0008-add-project-note-completed-at/up.sql b/apps/staged/src-tauri/src/store/migrations/0008-add-project-note-completed-at/up.sql new file mode 100644 index 00000000..40d73e21 --- /dev/null +++ b/apps/staged/src-tauri/src/store/migrations/0008-add-project-note-completed-at/up.sql @@ -0,0 +1,9 @@ +-- Add completed_at to project_notes so queued project notes sort by when the +-- session finished producing them, not when they were queued. +-- +-- NULL means the note hasn't completed yet (still queued/generating). +-- For existing rows we backfill with updated_at, which is the best +-- approximation we have. + +ALTER TABLE project_notes ADD COLUMN completed_at INTEGER; +UPDATE project_notes SET completed_at = updated_at WHERE content != ''; diff --git a/apps/staged/src-tauri/src/store/models.rs b/apps/staged/src-tauri/src/store/models.rs index fec8c7cb..4ec65a68 100644 --- a/apps/staged/src-tauri/src/store/models.rs +++ b/apps/staged/src-tauri/src/store/models.rs @@ -688,11 +688,15 @@ pub struct Note { pub content: String, pub created_at: i64, pub updated_at: i64, + /// When the AI session finished producing this note's content. + /// `None` while the session is still running. + pub completed_at: Option, } impl Note { pub fn new(branch_id: &str, title: &str, content: &str) -> Self { let now = now_timestamp(); + let has_content = !content.is_empty(); Self { id: Uuid::new_v4().to_string(), branch_id: branch_id.to_string(), @@ -701,6 +705,7 @@ impl Note { content: content.to_string(), created_at: now, updated_at: now, + completed_at: if has_content { Some(now) } else { None }, } } @@ -729,11 +734,15 @@ pub struct ProjectNote { pub content: String, pub created_at: i64, pub updated_at: i64, + /// When the AI session finished producing this project note's content. + /// `None` while the session is still running. + pub completed_at: Option, } impl ProjectNote { pub fn new(project_id: &str, title: &str, content: &str) -> Self { let now = now_timestamp(); + let has_content = !content.is_empty(); Self { id: Uuid::new_v4().to_string(), project_id: project_id.to_string(), @@ -742,6 +751,7 @@ impl ProjectNote { content: content.to_string(), created_at: now, updated_at: now, + completed_at: if has_content { Some(now) } else { None }, } } @@ -983,6 +993,9 @@ pub struct Review { pub reference_files: Vec, pub created_at: i64, pub updated_at: i64, + /// When the AI session finished producing this review. + /// `None` while the session is still running. + pub completed_at: Option, } impl Review { @@ -1001,6 +1014,7 @@ impl Review { reference_files: Vec::new(), created_at: now, updated_at: now, + completed_at: None, } } diff --git a/apps/staged/src-tauri/src/store/notes.rs b/apps/staged/src-tauri/src/store/notes.rs index 0eb5e44d..ab3b6d96 100644 --- a/apps/staged/src-tauri/src/store/notes.rs +++ b/apps/staged/src-tauri/src/store/notes.rs @@ -9,8 +9,8 @@ impl Store { pub fn create_note(&self, note: &Note) -> Result<(), StoreError> { let conn = self.conn.lock().unwrap(); conn.execute( - "INSERT INTO notes (id, branch_id, session_id, title, content, created_at, updated_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)", + "INSERT INTO notes (id, branch_id, session_id, title, content, created_at, updated_at, completed_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", params![ note.id, note.branch_id, @@ -19,6 +19,7 @@ impl Store { note.content, note.created_at, note.updated_at, + note.completed_at, ], )?; Ok(()) @@ -27,7 +28,7 @@ impl Store { pub fn get_note(&self, id: &str) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); conn.query_row( - "SELECT id, branch_id, session_id, title, content, created_at, updated_at + "SELECT id, branch_id, session_id, title, content, created_at, updated_at, completed_at FROM notes WHERE id = ?1", params![id], Self::row_to_note, @@ -39,7 +40,7 @@ impl Store { pub fn list_notes_for_branch(&self, branch_id: &str) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); let mut stmt = conn.prepare( - "SELECT id, branch_id, session_id, title, content, created_at, updated_at + "SELECT id, branch_id, session_id, title, content, created_at, updated_at, completed_at FROM notes WHERE branch_id = ?1 ORDER BY created_at DESC", )?; let rows = stmt.query_map(params![branch_id], Self::row_to_note)?; @@ -50,7 +51,7 @@ impl Store { pub fn get_note_by_session(&self, session_id: &str) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); conn.query_row( - "SELECT id, branch_id, session_id, title, content, created_at, updated_at + "SELECT id, branch_id, session_id, title, content, created_at, updated_at, completed_at FROM notes WHERE session_id = ?1", params![session_id], Self::row_to_note, @@ -63,7 +64,7 @@ impl Store { pub fn get_empty_note_by_session(&self, session_id: &str) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); conn.query_row( - "SELECT id, branch_id, session_id, title, content, created_at, updated_at + "SELECT id, branch_id, session_id, title, content, created_at, updated_at, completed_at FROM notes WHERE session_id = ?1 AND content = ''", params![session_id], Self::row_to_note, @@ -80,18 +81,30 @@ impl Store { content: &str, ) -> Result<(), StoreError> { let conn = self.conn.lock().unwrap(); + let now = now_timestamp(); conn.execute( - "UPDATE notes SET title = ?1, content = ?2, updated_at = ?3 WHERE id = ?4", - params![title, content, now_timestamp(), id], + "UPDATE notes SET title = ?1, content = ?2, updated_at = ?3, completed_at = COALESCE(completed_at, ?4) WHERE id = ?5", + params![title, content, now, now, id], )?; Ok(()) } pub fn update_note_content(&self, id: &str, content: &str) -> Result<(), StoreError> { let conn = self.conn.lock().unwrap(); + let now = now_timestamp(); conn.execute( - "UPDATE notes SET content = ?1, updated_at = ?2 WHERE id = ?3", - params![content, now_timestamp(), id], + "UPDATE notes SET content = ?1, updated_at = ?2, completed_at = COALESCE(completed_at, ?3) WHERE id = ?4", + params![content, now, now, id], + )?; + Ok(()) + } + + pub fn mark_note_completed(&self, id: &str) -> Result<(), StoreError> { + let conn = self.conn.lock().unwrap(); + let now = now_timestamp(); + conn.execute( + "UPDATE notes SET completed_at = COALESCE(completed_at, ?1) WHERE id = ?2", + params![now, id], )?; Ok(()) } @@ -111,6 +124,7 @@ impl Store { content: row.get(4)?, created_at: row.get(5)?, updated_at: row.get(6)?, + completed_at: row.get(7)?, }) } } diff --git a/apps/staged/src-tauri/src/store/project_notes.rs b/apps/staged/src-tauri/src/store/project_notes.rs index 041da9c5..8df94b10 100644 --- a/apps/staged/src-tauri/src/store/project_notes.rs +++ b/apps/staged/src-tauri/src/store/project_notes.rs @@ -9,8 +9,8 @@ impl Store { pub fn create_project_note(&self, note: &ProjectNote) -> Result<(), StoreError> { let conn = self.conn.lock().unwrap(); conn.execute( - "INSERT INTO project_notes (id, project_id, session_id, title, content, created_at, updated_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)", + "INSERT INTO project_notes (id, project_id, session_id, title, content, created_at, updated_at, completed_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", params![ note.id, note.project_id, @@ -19,6 +19,7 @@ impl Store { note.content, note.created_at, note.updated_at, + note.completed_at, ], )?; Ok(()) @@ -27,7 +28,7 @@ impl Store { pub fn get_project_note(&self, id: &str) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); conn.query_row( - "SELECT id, project_id, session_id, title, content, created_at, updated_at + "SELECT id, project_id, session_id, title, content, created_at, updated_at, completed_at FROM project_notes WHERE id = ?1", params![id], Self::row_to_project_note, @@ -39,8 +40,10 @@ impl Store { pub fn list_project_notes(&self, project_id: &str) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); let mut stmt = conn.prepare( - "SELECT id, project_id, session_id, title, content, created_at, updated_at - FROM project_notes WHERE project_id = ?1 ORDER BY created_at DESC", + "SELECT id, project_id, session_id, title, content, created_at, updated_at, completed_at + FROM project_notes + WHERE project_id = ?1 + ORDER BY COALESCE(completed_at, created_at) DESC, created_at DESC", )?; let rows = stmt.query_map(params![project_id], Self::row_to_project_note)?; rows.collect::, _>>().map_err(Into::into) @@ -53,7 +56,7 @@ impl Store { ) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); conn.query_row( - "SELECT id, project_id, session_id, title, content, created_at, updated_at + "SELECT id, project_id, session_id, title, content, created_at, updated_at, completed_at FROM project_notes WHERE session_id = ?1", params![session_id], Self::row_to_project_note, @@ -69,7 +72,7 @@ impl Store { ) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); conn.query_row( - "SELECT id, project_id, session_id, title, content, created_at, updated_at + "SELECT id, project_id, session_id, title, content, created_at, updated_at, completed_at FROM project_notes WHERE session_id = ?1 AND content = ''", params![session_id], Self::row_to_project_note, @@ -85,9 +88,22 @@ impl Store { content: &str, ) -> Result<(), StoreError> { let conn = self.conn.lock().unwrap(); + let now = now_timestamp(); conn.execute( - "UPDATE project_notes SET title = ?1, content = ?2, updated_at = ?3 WHERE id = ?4", - params![title, content, now_timestamp(), id], + "UPDATE project_notes + SET title = ?1, content = ?2, updated_at = ?3, completed_at = COALESCE(completed_at, ?4) + WHERE id = ?5", + params![title, content, now, now, id], + )?; + Ok(()) + } + + pub fn mark_project_note_completed(&self, id: &str) -> Result<(), StoreError> { + let conn = self.conn.lock().unwrap(); + let now = now_timestamp(); + conn.execute( + "UPDATE project_notes SET completed_at = COALESCE(completed_at, ?1) WHERE id = ?2", + params![now, id], )?; Ok(()) } @@ -107,6 +123,7 @@ impl Store { content: row.get(4)?, created_at: row.get(5)?, updated_at: row.get(6)?, + completed_at: row.get(7)?, }) } } diff --git a/apps/staged/src-tauri/src/store/reviews.rs b/apps/staged/src-tauri/src/store/reviews.rs index eab2a065..1e3e7723 100644 --- a/apps/staged/src-tauri/src/store/reviews.rs +++ b/apps/staged/src-tauri/src/store/reviews.rs @@ -12,8 +12,8 @@ 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, title, is_auto, created_at, updated_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)", + "INSERT INTO reviews (id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)", params![ review.id, review.branch_id, @@ -24,6 +24,7 @@ impl Store { review.is_auto, review.created_at, review.updated_at, + review.completed_at, ], )?; Ok(()) @@ -49,7 +50,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, title, is_auto, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at FROM reviews WHERE branch_id = ?1 AND commit_sha = ?2 AND scope = ?3 AND is_auto = 0 ORDER BY created_at DESC LIMIT 1", @@ -66,8 +67,8 @@ 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, title, is_auto, created_at, updated_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)", + "INSERT INTO reviews (id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)", params![ review.id, review.branch_id, @@ -78,6 +79,7 @@ impl Store { review.is_auto, review.created_at, review.updated_at, + review.completed_at, ], )?; Ok(review) @@ -100,7 +102,7 @@ impl Store { // find_fresh_auto_review and should not be returned here. let existing: Option = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at FROM reviews WHERE branch_id = ?1 AND commit_sha = ?2 AND scope = ?3 AND is_auto = 0 ORDER BY created_at DESC LIMIT 1", @@ -123,7 +125,7 @@ impl Store { let conn = self.conn.lock().unwrap(); let review = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at FROM reviews WHERE id = ?1", params![id], Self::row_to_review_header, @@ -143,7 +145,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, title, is_auto, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_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)?; @@ -264,7 +266,7 @@ impl Store { let conn = self.conn.lock().unwrap(); let review = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at FROM reviews WHERE session_id = ?1", params![session_id], Self::row_to_review_header, @@ -297,7 +299,7 @@ impl Store { ) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); let mut stmt = conn.prepare( - "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at FROM reviews WHERE branch_id = ?1 AND created_at >= ?2 ORDER BY created_at ASC", @@ -306,12 +308,25 @@ impl Store { rows.collect::, _>>().map_err(Into::into) } + /// Mark a review as completed without changing the title. + /// Used when the review session finishes but title extraction fails. + pub fn mark_review_completed(&self, id: &str) -> Result<(), StoreError> { + let conn = self.conn.lock().unwrap(); + let now = now_timestamp(); + conn.execute( + "UPDATE reviews SET completed_at = COALESCE(completed_at, ?1) WHERE id = ?2", + params![now, id], + )?; + 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(); + let now = now_timestamp(); conn.execute( - "UPDATE reviews SET title = ?1, updated_at = ?2 WHERE id = ?3", - params![title, now_timestamp(), id], + "UPDATE reviews SET title = ?1, updated_at = ?2, completed_at = COALESCE(completed_at, ?3) WHERE id = ?4", + params![title, now, now, id], )?; Ok(()) } @@ -319,9 +334,10 @@ impl Store { /// Update the `commit_sha` of a review. pub fn update_review_commit_sha(&self, id: &str, commit_sha: &str) -> Result<(), StoreError> { let conn = self.conn.lock().unwrap(); + let now = now_timestamp(); conn.execute( "UPDATE reviews SET commit_sha = ?1, updated_at = ?2 WHERE id = ?3", - params![commit_sha, now_timestamp(), id], + params![commit_sha, now, id], )?; Ok(()) } @@ -329,9 +345,17 @@ impl Store { /// Update the `is_auto` flag on a review. pub fn set_review_auto(&self, id: &str, is_auto: bool) -> Result<(), StoreError> { let conn = self.conn.lock().unwrap(); + let now = now_timestamp(); conn.execute( - "UPDATE reviews SET is_auto = ?1, updated_at = ?2 WHERE id = ?3", - params![is_auto, now_timestamp(), id], + "UPDATE reviews + SET is_auto = ?1, + updated_at = ?2, + completed_at = CASE + WHEN is_auto = 1 AND ?1 = 0 AND completed_at IS NOT NULL THEN ?2 + ELSE completed_at + END + WHERE id = ?3", + params![is_auto, now, id], )?; Ok(()) } @@ -355,7 +379,7 @@ impl Store { let conn = self.conn.lock().unwrap(); let review: Option = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at FROM reviews WHERE branch_id = ?1 AND is_auto = 1 AND created_at >= MAX( @@ -405,6 +429,7 @@ impl Store { reference_files: Vec::new(), created_at: row.get(7)?, updated_at: row.get(8)?, + completed_at: row.get(9)?, }) } diff --git a/apps/staged/src-tauri/src/store/tests.rs b/apps/staged/src-tauri/src/store/tests.rs index 05ca37ad..c59c8b18 100644 --- a/apps/staged/src-tauri/src/store/tests.rs +++ b/apps/staged/src-tauri/src/store/tests.rs @@ -69,6 +69,66 @@ fn test_list_projects() { assert_eq!(projects.len(), 2); } +#[test] +fn test_project_note_sets_completed_at_when_created_with_content() { + let note = ProjectNote::new("project-1", "Title", "Body"); + assert_eq!(note.completed_at, Some(note.created_at)); +} + +#[test] +fn test_project_note_completion_is_write_once() { + let store = Store::in_memory().unwrap(); + let project = Project::new("test-owner/test-repo"); + store.create_project(&project).unwrap(); + + let note = ProjectNote::new(&project.id, "", ""); + store.create_project_note(¬e).unwrap(); + + let before = store.get_project_note(¬e.id).unwrap().unwrap(); + assert!(before.completed_at.is_none()); + + store + .update_project_note_title_and_content(¬e.id, "First", "Initial content") + .unwrap(); + let completed = store.get_project_note(¬e.id).unwrap().unwrap(); + let first_completed_at = completed.completed_at.unwrap(); + + std::thread::sleep(std::time::Duration::from_millis(2)); + store + .update_project_note_title_and_content(¬e.id, "Second", "Updated content") + .unwrap(); + let updated = store.get_project_note(¬e.id).unwrap().unwrap(); + + assert_eq!(updated.completed_at, Some(first_completed_at)); + assert!(updated.updated_at >= completed.updated_at); +} + +#[test] +fn test_list_project_notes_orders_by_completion_time() { + let store = Store::in_memory().unwrap(); + let project = Project::new("test-owner/test-repo"); + store.create_project(&project).unwrap(); + + let older = ProjectNote::new(&project.id, "", "").with_session("session-older"); + store.create_project_note(&older).unwrap(); + std::thread::sleep(std::time::Duration::from_millis(2)); + + let newer = ProjectNote::new(&project.id, "", "").with_session("session-newer"); + store.create_project_note(&newer).unwrap(); + + store + .update_project_note_title_and_content(&newer.id, "Newer", "Completed first") + .unwrap(); + std::thread::sleep(std::time::Duration::from_millis(2)); + store + .update_project_note_title_and_content(&older.id, "Older", "Completed second") + .unwrap(); + + let notes = store.list_project_notes(&project.id).unwrap(); + let ordered_ids: Vec<_> = notes.iter().map(|note| note.id.as_str()).collect(); + assert_eq!(ordered_ids, vec![older.id.as_str(), newer.id.as_str()]); +} + #[test] fn test_delete_project_cascades() { let store = Store::in_memory().unwrap(); @@ -952,6 +1012,49 @@ fn test_review_with_comments_and_files() { assert!(after_remove.reference_files.is_empty()); } +#[test] +fn test_set_review_auto_restamps_completed_at_when_made_visible() { + let store = Store::in_memory().unwrap(); + let project = Project::new("test-owner/test-repo"); + store.create_project(&project).unwrap(); + let branch = Branch::new(&project.id, "feature", "main"); + store.create_branch(&branch).unwrap(); + + let review = Review::new(&branch.id, "abc123", ReviewScope::Branch).with_auto(); + store.create_review(&review).unwrap(); + store + .update_review_title(&review.id, "Auto review") + .unwrap(); + + let auto_review = store.get_review(&review.id).unwrap().unwrap(); + let original_completed_at = auto_review.completed_at.unwrap(); + + std::thread::sleep(std::time::Duration::from_millis(2)); + store.set_review_auto(&review.id, false).unwrap(); + + let visible_review = store.get_review(&review.id).unwrap().unwrap(); + assert!(!visible_review.is_auto); + assert!(visible_review.completed_at.unwrap() > original_completed_at); +} + +#[test] +fn test_set_review_auto_leaves_incomplete_review_uncompleted() { + let store = Store::in_memory().unwrap(); + let project = Project::new("test-owner/test-repo"); + store.create_project(&project).unwrap(); + let branch = Branch::new(&project.id, "feature", "main"); + store.create_branch(&branch).unwrap(); + + let review = Review::new(&branch.id, "abc123", ReviewScope::Branch).with_auto(); + store.create_review(&review).unwrap(); + + store.set_review_auto(&review.id, false).unwrap(); + + let visible_review = store.get_review(&review.id).unwrap().unwrap(); + assert!(!visible_review.is_auto); + assert!(visible_review.completed_at.is_none()); +} + #[test] fn test_list_reviews_for_branch() { let store = Store::in_memory().unwrap(); diff --git a/apps/staged/src-tauri/src/timeline.rs b/apps/staged/src-tauri/src/timeline.rs index 1ba25d27..651cd7bf 100644 --- a/apps/staged/src-tauri/src/timeline.rs +++ b/apps/staged/src-tauri/src/timeline.rs @@ -160,6 +160,7 @@ fn build_branch_timeline(store: &Arc, branch_id: &str) -> Result, branch_id: &str) -> Result 0 ? badges : undefined, deleting: isDeleting, - timestamp: Math.floor(review.createdAt / 1000), + // Use completedAt so completed reviews sort by completion time, not queue time + timestamp: Math.floor((review.completedAt ?? review.createdAt) / 1000), order: 0, sessionId: review.sessionId ?? undefined, reviewId: review.id, diff --git a/apps/staged/src/lib/types.ts b/apps/staged/src/lib/types.ts index 7df1b147..b7cb386b 100644 --- a/apps/staged/src/lib/types.ts +++ b/apps/staged/src/lib/types.ts @@ -113,6 +113,7 @@ export interface NoteTimelineItem { completionReason: string | null; createdAt: number; updatedAt: number; + completedAt: number | null; } export interface ReviewTimelineItem { @@ -127,6 +128,7 @@ export interface ReviewTimelineItem { isAuto?: boolean; createdAt: number; updatedAt: number; + completedAt: number | null; } export interface ImageTimelineItem { @@ -207,6 +209,7 @@ export interface ProjectNote { content: string; createdAt: number; updatedAt: number; + completedAt: number | null; } export interface ProjectSessionResponse { diff --git a/apps/staged/tsconfig.app.json b/apps/staged/tsconfig.app.json index 31c18cfd..f2420284 100644 --- a/apps/staged/tsconfig.app.json +++ b/apps/staged/tsconfig.app.json @@ -17,5 +17,6 @@ "checkJs": true, "moduleDetection": "force" }, - "include": ["src/**/*.ts", "src/**/*.js", "src/**/*.svelte"] + "include": ["src/**/*.ts", "src/**/*.js", "src/**/*.svelte"], + "exclude": ["src/**/*.test.ts", "src/**/*.spec.ts"] } From d834f02a8e1e4e1120efc45d550e1669b4981c72 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 2 Apr 2026 15:15:28 +1100 Subject: [PATCH 2/2] fix(staged): resolve timeline ordering review comments --- apps/staged/src-tauri/src/session_commands.rs | 6 ++-- apps/staged/src-tauri/src/store/notes.rs | 3 +- apps/staged/src-tauri/src/store/tests.rs | 28 +++++++++++++++++++ apps/staged/tsconfig.app.json | 3 +- 4 files changed, 34 insertions(+), 6 deletions(-) diff --git a/apps/staged/src-tauri/src/session_commands.rs b/apps/staged/src-tauri/src/session_commands.rs index af238695..e749b373 100644 --- a/apps/staged/src-tauri/src/session_commands.rs +++ b/apps/staged/src-tauri/src/session_commands.rs @@ -1982,7 +1982,7 @@ fn note_timeline_entries( format_note_for_context(¬e.id, ¬e.title, ¬e.content, workspace_name) { entries.push(TimelineEntry { - timestamp: note.created_at / 1000, + timestamp: note.completed_at.unwrap_or(note.created_at) / 1000, order: 0, content, }); @@ -2104,8 +2104,8 @@ fn review_timeline_entries( continue; } let short_sha = &review.commit_sha[..review.commit_sha.len().min(7)]; - let review_ts_secs = review.created_at / 1000; - let is_old = max_commit_ts.is_some_and(|ts| review_ts_secs < ts); + let review_ts_secs = review.completed_at.unwrap_or(review.created_at) / 1000; + let is_old = max_commit_ts.is_some_and(|ts| review.created_at / 1000 < ts); let heading_title = match review.title.as_deref() { Some(title) => format!("Code review: {} — {}", title, short_sha), diff --git a/apps/staged/src-tauri/src/store/notes.rs b/apps/staged/src-tauri/src/store/notes.rs index ab3b6d96..61dffee7 100644 --- a/apps/staged/src-tauri/src/store/notes.rs +++ b/apps/staged/src-tauri/src/store/notes.rs @@ -41,7 +41,8 @@ impl Store { let conn = self.conn.lock().unwrap(); let mut stmt = conn.prepare( "SELECT id, branch_id, session_id, title, content, created_at, updated_at, completed_at - FROM notes WHERE branch_id = ?1 ORDER BY created_at DESC", + FROM notes WHERE branch_id = ?1 + ORDER BY COALESCE(completed_at, created_at) DESC, created_at DESC", )?; let rows = stmt.query_map(params![branch_id], Self::row_to_note)?; rows.collect::, _>>().map_err(Into::into) diff --git a/apps/staged/src-tauri/src/store/tests.rs b/apps/staged/src-tauri/src/store/tests.rs index c59c8b18..9263b553 100644 --- a/apps/staged/src-tauri/src/store/tests.rs +++ b/apps/staged/src-tauri/src/store/tests.rs @@ -839,6 +839,34 @@ fn test_notes() { assert!(notes[0].content.contains("Design")); } +#[test] +fn test_list_notes_for_branch_orders_by_completion_time() { + let store = Store::in_memory().unwrap(); + let project = Project::new("test-owner/test-repo"); + store.create_project(&project).unwrap(); + let branch = Branch::new(&project.id, "feature", "main"); + store.create_branch(&branch).unwrap(); + + let older = Note::new(&branch.id, "", "").with_session("session-older"); + store.create_note(&older).unwrap(); + std::thread::sleep(std::time::Duration::from_millis(2)); + + let newer = Note::new(&branch.id, "", "").with_session("session-newer"); + store.create_note(&newer).unwrap(); + + store + .update_note_title_and_content(&newer.id, "Newer", "Completed first") + .unwrap(); + std::thread::sleep(std::time::Duration::from_millis(2)); + store + .update_note_title_and_content(&older.id, "Older", "Completed second") + .unwrap(); + + let notes = store.list_notes_for_branch(&branch.id).unwrap(); + let ordered_ids: Vec<_> = notes.iter().map(|note| note.id.as_str()).collect(); + assert_eq!(ordered_ids, vec![older.id.as_str(), newer.id.as_str()]); +} + // ============================================================================= // Repo Actions // ============================================================================= diff --git a/apps/staged/tsconfig.app.json b/apps/staged/tsconfig.app.json index f2420284..31c18cfd 100644 --- a/apps/staged/tsconfig.app.json +++ b/apps/staged/tsconfig.app.json @@ -17,6 +17,5 @@ "checkJs": true, "moduleDetection": "force" }, - "include": ["src/**/*.ts", "src/**/*.js", "src/**/*.svelte"], - "exclude": ["src/**/*.test.ts", "src/**/*.spec.ts"] + "include": ["src/**/*.ts", "src/**/*.js", "src/**/*.svelte"] }