From fcf2f6c62469230f6ca555e5bc279e00b0068625 Mon Sep 17 00:00:00 2001 From: Cooper Maruyama Date: Sat, 1 Aug 2026 03:50:48 -0700 Subject: [PATCH] refactor(db): remove redundant data, significantly simplify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit did a pass on the sqlite schema, replaced it with an equivalent but much smaller and simpler schema, and realized the downstream simplifications that became possible as a result. Replace the speculative-changeset store (changesets.rs, commits.rs, store_{bare,whole_diff}_changeset.rs, summarize/group_existing.rs and migrations 02/03) with a content-addressed schema keyed by sha256 of sorted change hashes: - db/keys.rs — snapshot_key / group_key helpers - db/snapshots.rs — snapshots table (cached commit message, its id is the changeset_id plumbed through evolve/build) - db/summaries.rs — patch_summaries (singles) + summary_groups - migrations/01_initial/up.sql rewritten to define the new tables - migrations 02-restore-commits, 03-drop-queued-summaries dropped summarize::find_existing now returns a SemanticChangeMap directly: greedily selects non-overlapping summary_groups (members ⊔ live hashes), falls back to patch_summaries for uncovered hashes, then loads the snapshot. --- .../src-tauri/examples/specta_gen_ts.rs | 6 +- .../src-tauri/migrations/01-initial/up.sql | 100 ++--- .../migrations/02-restore-commits/up.sql | 4 - .../03-drop-queued-summaries/up.sql | 5 - apps/native/src-tauri/src/README.md | 9 +- apps/native/src-tauri/src/commands/git.rs | 31 +- apps/native/src-tauri/src/db/changesets.rs | 400 ------------------ apps/native/src-tauri/src/db/commits.rs | 132 ------ apps/native/src-tauri/src/db/evolutions.rs | 6 +- apps/native/src-tauri/src/db/keys.rs | 69 +++ apps/native/src-tauri/src/db/mod.rs | 58 ++- apps/native/src-tauri/src/db/schema.rs | 2 +- apps/native/src-tauri/src/db/snapshots.rs | 118 ++++++ .../src-tauri/src/db/store_bare_changeset.rs | 31 -- .../src/db/store_whole_diff_changeset.rs | 113 ----- apps/native/src-tauri/src/db/summaries.rs | 272 ++++++++++++ apps/native/src-tauri/src/db/tables.rs | 73 +--- apps/native/src-tauri/src/git/exec.rs | 1 + apps/native/src-tauri/src/git/query.rs | 130 ++++-- .../src-tauri/src/history/get_history.rs | 33 +- .../src/managed_edits/managed_edit.rs | 8 +- apps/native/src-tauri/src/shared_types/git.rs | 24 +- apps/native/src-tauri/src/sqlite_types.rs | 27 -- .../native/src-tauri/src/state/build_state.rs | 32 +- apps/native/src-tauri/src/state/watcher.rs | 5 +- .../src-tauri/src/summarize/build_prompt.rs | 48 +-- .../src-tauri/src/summarize/find_existing.rs | 291 +++++++++---- .../src-tauri/src/summarize/group_existing.rs | 211 --------- apps/native/src-tauri/src/summarize/mod.rs | 93 +--- .../src-tauri/src/summarize/model_calls.rs | 66 ++- .../src/summarize/pipelines/commit_message.rs | 31 +- .../src/summarize/pipelines/history.rs | 42 +- .../src/summarize/pipelines/whole_diff.rs | 337 ++++++++++----- apps/native/src-tauri/src/summarize/sumlog.rs | 43 -- apps/native/src/ipc/sqlite.ts | 10 +- apps/native/src/ipc/types.ts | 34 -- 36 files changed, 1247 insertions(+), 1648 deletions(-) delete mode 100644 apps/native/src-tauri/migrations/02-restore-commits/up.sql delete mode 100644 apps/native/src-tauri/migrations/03-drop-queued-summaries/up.sql delete mode 100644 apps/native/src-tauri/src/db/changesets.rs delete mode 100644 apps/native/src-tauri/src/db/commits.rs create mode 100644 apps/native/src-tauri/src/db/keys.rs create mode 100644 apps/native/src-tauri/src/db/snapshots.rs delete mode 100644 apps/native/src-tauri/src/db/store_bare_changeset.rs delete mode 100644 apps/native/src-tauri/src/db/store_whole_diff_changeset.rs create mode 100644 apps/native/src-tauri/src/db/summaries.rs delete mode 100644 apps/native/src-tauri/src/summarize/group_existing.rs diff --git a/apps/native/src-tauri/examples/specta_gen_ts.rs b/apps/native/src-tauri/examples/specta_gen_ts.rs index ed5b48165..b0c8686af 100644 --- a/apps/native/src-tauri/examples/specta_gen_ts.rs +++ b/apps/native/src-tauri/examples/specta_gen_ts.rs @@ -23,10 +23,8 @@ fn main() { let types = collection .register::() .register::() - .register::() .register::() - .register::() - .register::(); + .register::(); let manifest_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")); let output_path = manifest_dir.join("../src/ipc/sqlite.ts"); @@ -68,8 +66,6 @@ fn main() { .register::() .register::() .register::() - .register::() - .register::() .register::() .register::() .register::() diff --git a/apps/native/src-tauri/migrations/01-initial/up.sql b/apps/native/src-tauri/migrations/01-initial/up.sql index 51e2f255e..8487b3fe4 100644 --- a/apps/native/src-tauri/migrations/01-initial/up.sql +++ b/apps/native/src-tauri/migrations/01-initial/up.sql @@ -1,79 +1,61 @@ -CREATE TABLE IF NOT EXISTS commits ( - id INTEGER PRIMARY KEY, - hash TEXT NOT NULL UNIQUE, - tree_hash TEXT NOT NULL, - message TEXT, +-- Content-addressed summary schema. +-- +-- Summaries describe *what a patch does*, keyed only by the content hash of the +-- change(s) they cover. Git remains the source of truth for diffs; these tables +-- are a local cache that can always be rebuilt by re-summarizing. + +-- Per-change summary. Used for singletons / fallback (a change the model +-- described on its own, or a one-member group). +CREATE TABLE IF NOT EXISTS patch_summaries ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + change_hash TEXT NOT NULL UNIQUE, + title TEXT NOT NULL DEFAULT '', + description TEXT NOT NULL DEFAULT '', + status TEXT NOT NULL DEFAULT 'DONE' CHECK(status IN ('QUEUED', 'DONE', 'FAILED', 'CANCELLED')), created_at INTEGER NOT NULL ); -CREATE TABLE IF NOT EXISTS evolutions ( +-- First-class group summary. `group_key` content-addresses the exact set of +-- member change hashes (sha256 of the sorted hashes), so a group's identity is +-- its membership. +CREATE TABLE IF NOT EXISTS summary_groups ( id INTEGER PRIMARY KEY AUTOINCREMENT, - origin_branch TEXT NOT NULL, - merged INTEGER NOT NULL DEFAULT 0, - builds INTEGER NOT NULL DEFAULT 0 -); - -CREATE TABLE IF NOT EXISTS change_summaries ( - id INTEGER PRIMARY KEY, + group_key TEXT NOT NULL UNIQUE, title TEXT NOT NULL DEFAULT '', description TEXT NOT NULL DEFAULT '', - status TEXT NOT NULL DEFAULT 'QUEUED' CHECK(status IN ('QUEUED', 'DONE', 'FAILED', 'CANCELLED')), + status TEXT NOT NULL DEFAULT 'DONE' CHECK(status IN ('QUEUED', 'DONE', 'FAILED', 'CANCELLED')), created_at INTEGER NOT NULL ); -CREATE TABLE IF NOT EXISTS changes ( - id INTEGER PRIMARY KEY, - hash TEXT NOT NULL UNIQUE, - filename TEXT NOT NULL, - diff TEXT NOT NULL, - line_count INTEGER NOT NULL, - created_at INTEGER NOT NULL, - own_summary_id INTEGER REFERENCES change_summaries(id) +-- Membership of a group. Content-addressed by `group_key`. +CREATE TABLE IF NOT EXISTS summary_group_members ( + group_key TEXT NOT NULL REFERENCES summary_groups(group_key), + change_hash TEXT NOT NULL, + PRIMARY KEY (group_key, change_hash) ); -CREATE TABLE IF NOT EXISTS group_summaries ( - change_id INTEGER NOT NULL REFERENCES changes(id), - change_summary_id INTEGER NOT NULL REFERENCES change_summaries(id) +-- Thin evolution record — just the origin branch an evolution started from. +CREATE TABLE IF NOT EXISTS evolutions ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + origin_branch TEXT NOT NULL ); -CREATE TABLE IF NOT EXISTS change_sets ( +-- A snapshot caches the generated commit message for an exact set of change +-- hashes. `snapshot_key` = sha256(sorted change hashes). The integer `id` is the +-- value historically plumbed as `changeset_id` throughout evolve/build state. +CREATE TABLE IF NOT EXISTS snapshots ( id INTEGER PRIMARY KEY AUTOINCREMENT, - commit_id INTEGER REFERENCES commits(id), - base_commit_id INTEGER NOT NULL REFERENCES commits(id), - commit_message TEXT, + snapshot_key TEXT NOT NULL UNIQUE, generated_commit_message TEXT, - created_at INTEGER NOT NULL, - evolution_id INTEGER REFERENCES evolutions(id) -); - -CREATE TABLE IF NOT EXISTS set_changes ( - change_set_id INTEGER NOT NULL REFERENCES change_sets(id), - change_id INTEGER NOT NULL REFERENCES changes(id), - PRIMARY KEY (change_set_id, change_id) -); - -CREATE TABLE IF NOT EXISTS queued_summaries ( - id INTEGER PRIMARY KEY, - status TEXT NOT NULL DEFAULT 'QUEUED' CHECK(status IN ('QUEUED', 'DONE', 'FAILED', 'CANCELLED')), - attempted_count INTEGER NOT NULL DEFAULT 0, - prompt TEXT NOT NULL, - model_response TEXT, - group_summary_id INTEGER REFERENCES change_summaries(id), - hash_own_summary_id_pairs TEXT, - type TEXT NOT NULL CHECK(type IN ('NEW_SINGLE', 'NEW_GROUP', 'EVOLVED_GROUP')) + evolution_id INTEGER REFERENCES evolutions(id), + created_at INTEGER NOT NULL ); -CREATE TABLE IF NOT EXISTS prompts ( - id INTEGER PRIMARY KEY, - text TEXT NOT NULL, - commit_id INTEGER REFERENCES commits(id) ON DELETE SET NULL, - created_at INTEGER NOT NULL +-- Records which commits were created by a restore operation and their origin. +CREATE TABLE IF NOT EXISTS restore_commits ( + commit_hash TEXT PRIMARY KEY, + origin_hash TEXT NOT NULL ); -CREATE INDEX IF NOT EXISTS idx_commits_tree_hash ON commits(tree_hash); CREATE INDEX IF NOT EXISTS idx_evolutions_origin_branch ON evolutions(origin_branch); -CREATE INDEX IF NOT EXISTS idx_prompts_commit ON prompts(commit_id); -CREATE INDEX IF NOT EXISTS idx_change_sets_commit ON change_sets(commit_id); -CREATE INDEX IF NOT EXISTS idx_change_sets_base ON change_sets(base_commit_id); -CREATE INDEX IF NOT EXISTS idx_set_changes_change ON set_changes(change_id); -CREATE INDEX IF NOT EXISTS idx_queued_summaries_status ON queued_summaries(status); +CREATE INDEX IF NOT EXISTS idx_summary_group_members_hash ON summary_group_members(change_hash); diff --git a/apps/native/src-tauri/migrations/02-restore-commits/up.sql b/apps/native/src-tauri/migrations/02-restore-commits/up.sql deleted file mode 100644 index 8aff4ddef..000000000 --- a/apps/native/src-tauri/migrations/02-restore-commits/up.sql +++ /dev/null @@ -1,4 +0,0 @@ -CREATE TABLE IF NOT EXISTS restore_commits ( - commit_hash TEXT PRIMARY KEY, - origin_hash TEXT NOT NULL -); diff --git a/apps/native/src-tauri/migrations/03-drop-queued-summaries/up.sql b/apps/native/src-tauri/migrations/03-drop-queued-summaries/up.sql deleted file mode 100644 index d8feef490..000000000 --- a/apps/native/src-tauri/migrations/03-drop-queued-summaries/up.sql +++ /dev/null @@ -1,5 +0,0 @@ --- The queued_summaries table backed the per-hunk summarization queue worker --- that PR #330 deleted. Nothing reads or writes the table anymore, so drop it --- and its status index to keep fresh databases lean. -DROP INDEX IF EXISTS idx_queued_summaries_status; -DROP TABLE IF EXISTS queued_summaries; diff --git a/apps/native/src-tauri/src/README.md b/apps/native/src-tauri/src/README.md index 9bf06f7b9..66d731bf6 100644 --- a/apps/native/src-tauri/src/README.md +++ b/apps/native/src-tauri/src/README.md @@ -51,9 +51,10 @@ Each file (apply, cli_tool, config, debug, editor, evolve, evolve_state, feedbac - `schema.rs` — Runs migrations - `pool.rs`, `tables.rs` — Diesel r2d2 connection pool and `table!` declarations -- `commits.rs`, `evolutions.rs` — CRUD for their respective tables -- `changesets.rs` — Shared insert helpers for changeset tables -- `store_whole_diff_changeset.rs`, `store_bare_changeset.rs` — Persist summarization pipeline results +- `keys.rs` — Content-addressing helpers (`snapshot_key` / `group_key` = sha256 of sorted change hashes) +- `summaries.rs` — Store + lookup for content-addressed `patch_summaries` (singles) and `summary_groups` +- `snapshots.rs` — Upsert/get for `snapshots` (caches the generated commit message; its `id` is the `changeset_id` plumbed through evolve/build) +- `evolutions.rs` — CRUD for the thin `evolutions` table (`id`, `origin_branch`) - `restore_commits.rs` — Tracks restore-origin provenance **Called by:** history, summarize, evolve/lifecycle, managed_edits, state/watcher @@ -147,7 +148,7 @@ grouping scheme). - `mod.rs` — Top-level `new_changeset` / `summarize_since` flow - `find_existing.rs` — Queries DB for existing summarized changesets -- `group_existing.rs` — Builds SemanticChangeMap from found changesets +- `find_existing.rs` — Content-addressed lookup: greedily selects non-overlapping `summary_groups` (`members ⊆ live hashes`), falls back to `patch_summaries` for uncovered hashes, loads the `snapshot`, and produces a `SemanticChangeMap` - `model_calls.rs` — AI API call for the whole-diff summary - `build_prompt.rs` — Constructs the whole-diff prompt - `token_budgets.rs` — Computes input/output token allocations diff --git a/apps/native/src-tauri/src/commands/git.rs b/apps/native/src-tauri/src/commands/git.rs index 9c029f0c1..804704a82 100644 --- a/apps/native/src-tauri/src/commands/git.rs +++ b/apps/native/src-tauri/src/commands/git.rs @@ -2,7 +2,7 @@ use super::helpers::capture_err; use crate::state::{build_state, evolve_state}; use crate::storage::store; use crate::{db, git, shared_types}; -use tauri::{AppHandle, Manager, State}; +use tauri::{AppHandle, State}; pub async fn fetch_file_diff_contents( app: AppHandle, @@ -22,7 +22,6 @@ pub async fn create_commit( app: AppHandle, message: String, ) -> Result { - let db_pool = app.state::(); let dir = store::ensure_git_repo_folder(&app).map_err(|e| capture_err("git_commit", e))?; let commit_info = git::commit_all(&dir, &message).map_err(|e| capture_err("git_commit", e))?; @@ -35,22 +34,6 @@ pub async fn create_commit( log::warn!("[git_commit] Failed to tag commit: {}", e); } - let now = crate::utils::unix_now(); - match db::commits::upsert_commit( - &db_pool, - &commit_info.hash, - &commit_info.tree_hash, - Some(&message), - now, - ) { - Ok(id) => log::info!( - "[git_commit] Saved commit to database (id={}, hash={})", - id, - &commit_info.hash[..8] - ), - Err(e) => log::error!("[git_commit] Failed to save commit: {}", e), - } - if let Ok(current_build_state) = build_state::get(&app) { let updated = build_state::BuildState { head_commit_hash: Some(commit_info.hash.clone()), @@ -97,7 +80,6 @@ pub async fn commit_single_file( filename: String, message: String, ) -> Result { - let db_pool = app.state::(); let dir = store::ensure_git_repo_folder(&app).map_err(|e| capture_err("git_commit_file", e))?; let commit_info = git::commit_file(&dir, &filename, &message) .map_err(|e| capture_err("git_commit_file", e))?; @@ -111,17 +93,6 @@ pub async fn commit_single_file( log::warn!("[git_commit_file] Failed to tag commit: {}", e); } - let now = crate::utils::unix_now(); - if let Err(e) = db::commits::upsert_commit( - &db_pool, - &commit_info.hash, - &commit_info.tree_hash, - Some(&message), - now, - ) { - log::error!("[git_commit_file] Failed to save commit: {}", e); - } - if let Ok(current_build_state) = build_state::get(&app) { let updated = build_state::BuildState { head_commit_hash: Some(commit_info.hash.clone()), diff --git a/apps/native/src-tauri/src/db/changesets.rs b/apps/native/src-tauri/src/db/changesets.rs deleted file mode 100644 index 95bef5d0b..000000000 --- a/apps/native/src-tauri/src/db/changesets.rs +++ /dev/null @@ -1,400 +0,0 @@ -//! Change-set persistence helpers. -//! -//! All helpers take `&mut diesel::SqliteConnection`. Atomic groups are wrapped -//! at the caller via `conn.transaction(|conn| ...)`. - -use anyhow::Result; -use diesel::prelude::*; -use diesel::sql_query; -use diesel::sql_types::{BigInt, Nullable, Text}; - -use crate::db::tables::{change_sets, change_summaries, changes, group_summaries, set_changes}; -use crate::shared_types::{SummarizedChange, SummarizedChangeSet}; -use crate::sqlite_types::{Change, ChangeSet, ChangeSummary}; - -pub fn insert_change_summary( - conn: &mut SqliteConnection, - title: &str, - description: &str, - status: &str, - created_at: i64, -) -> Result { - diesel::insert_into(change_summaries::table) - .values(( - change_summaries::title.eq(title), - change_summaries::description.eq(description), - change_summaries::status.eq(status), - change_summaries::created_at.eq(created_at), - )) - .execute(conn)?; - last_insert_rowid(conn) -} - -pub fn upsert_change( - conn: &mut SqliteConnection, - change: &Change, - own_summary_id: Option, -) -> Result<()> { - diesel::insert_into(changes::table) - .values(( - changes::hash.eq(&change.hash), - changes::filename.eq(&change.filename), - changes::diff.eq(&change.diff), - changes::line_count.eq(change.line_count), - changes::created_at.eq(change.created_at), - changes::own_summary_id.eq(own_summary_id), - )) - .on_conflict(changes::hash) - .do_nothing() - .execute(conn)?; - diesel::update(changes::table.filter(changes::hash.eq(&change.hash))) - .set(changes::own_summary_id.eq(own_summary_id)) - .execute(conn)?; - Ok(()) -} - -pub fn insert_change_or_ignore( - conn: &mut SqliteConnection, - change: &Change, - own_summary_id: Option, -) -> Result { - diesel::insert_into(changes::table) - .values(( - changes::hash.eq(&change.hash), - changes::filename.eq(&change.filename), - changes::diff.eq(&change.diff), - changes::line_count.eq(change.line_count), - changes::created_at.eq(change.created_at), - changes::own_summary_id.eq(own_summary_id), - )) - .on_conflict(changes::hash) - .do_nothing() - .execute(conn)?; - Ok(changes::table - .filter(changes::hash.eq(&change.hash)) - .select(changes::id) - .first::(conn)?) -} - -pub fn link_change_to_group_summary( - conn: &mut SqliteConnection, - change_id: i64, - change_summary_id: i64, -) -> Result<()> { - diesel::insert_into(group_summaries::table) - .values(( - group_summaries::change_id.eq(change_id), - group_summaries::change_summary_id.eq(change_summary_id), - )) - .execute(conn)?; - Ok(()) -} - -pub fn insert_change_set( - conn: &mut SqliteConnection, - commit_id: Option, - base_commit_id: i64, - commit_message: Option<&str>, - generated_commit_message: Option<&str>, - created_at: i64, - evolution_id: Option, -) -> Result { - diesel::insert_into(change_sets::table) - .values(( - change_sets::commit_id.eq(commit_id), - change_sets::base_commit_id.eq(base_commit_id), - change_sets::commit_message.eq(commit_message), - change_sets::generated_commit_message.eq(generated_commit_message), - change_sets::created_at.eq(created_at), - change_sets::evolution_id.eq(evolution_id), - )) - .execute(conn)?; - last_insert_rowid(conn) -} - -pub fn get_change_id_by_hash(conn: &mut SqliteConnection, hash: &str) -> Result { - Ok(changes::table - .filter(changes::hash.eq(hash)) - .select(changes::id) - .first::(conn)?) -} - -pub fn link_change_to_set( - conn: &mut SqliteConnection, - change_set_id: i64, - change_id: i64, -) -> Result<()> { - diesel::insert_into(set_changes::table) - .values(( - set_changes::change_set_id.eq(change_set_id), - set_changes::change_id.eq(change_id), - )) - .on_conflict((set_changes::change_set_id, set_changes::change_id)) - .do_nothing() - .execute(conn)?; - Ok(()) -} - -// ── Big aliased row used by the JOIN read queries ──────────────────────────── - -#[derive(QueryableByName)] -struct SummarizedChangeRow { - #[diesel(sql_type = BigInt)] - c_id: i64, - #[diesel(sql_type = Text)] - c_hash: String, - #[diesel(sql_type = Text)] - c_filename: String, - #[diesel(sql_type = Text)] - c_diff: String, - #[diesel(sql_type = BigInt)] - c_line_count: i64, - #[diesel(sql_type = BigInt)] - c_created_at: i64, - #[diesel(sql_type = Nullable)] - c_own_summary_id: Option, - #[diesel(sql_type = Nullable)] - os_id: Option, - #[diesel(sql_type = Nullable)] - os_title: Option, - #[diesel(sql_type = Nullable)] - os_description: Option, - #[diesel(sql_type = Nullable)] - os_status: Option, - #[diesel(sql_type = Nullable)] - os_created_at: Option, - #[diesel(sql_type = Nullable)] - gs_id: Option, - #[diesel(sql_type = Nullable)] - gs_title: Option, - #[diesel(sql_type = Nullable)] - gs_description: Option, - #[diesel(sql_type = Nullable)] - gs_status: Option, - #[diesel(sql_type = Nullable)] - gs_created_at: Option, -} - -impl From for SummarizedChange { - fn from(row: SummarizedChangeRow) -> Self { - let change = Change { - id: row.c_id, - hash: row.c_hash, - filename: row.c_filename, - diff: row.c_diff, - line_count: row.c_line_count, - created_at: row.c_created_at, - own_summary_id: row.c_own_summary_id, - }; - let own_summary = row.os_id.map(|id| ChangeSummary { - id, - title: row.os_title.unwrap_or_default(), - description: row.os_description.unwrap_or_default(), - status: row.os_status.unwrap_or_default(), - created_at: row.os_created_at.unwrap_or(0), - }); - let group_summary = row.gs_id.map(|id| ChangeSummary { - id, - title: row.gs_title.unwrap_or_default(), - description: row.gs_description.unwrap_or_default(), - status: row.gs_status.unwrap_or_default(), - created_at: row.gs_created_at.unwrap_or(0), - }); - SummarizedChange { - change, - own_summary, - group_summary, - } - } -} - -const CHANGE_SELECT: &str = "SELECT \ - c.id AS c_id, c.hash AS c_hash, c.filename AS c_filename, c.diff AS c_diff, \ - c.line_count AS c_line_count, c.created_at AS c_created_at, \ - c.own_summary_id AS c_own_summary_id, \ - os.id AS os_id, os.title AS os_title, os.description AS os_description, \ - os.status AS os_status, os.created_at AS os_created_at, \ - gs.id AS gs_id, gs.title AS gs_title, gs.description AS gs_description, \ - gs.status AS gs_status, gs.created_at AS gs_created_at"; - -#[derive(QueryableByName)] -struct ChangeSetRow { - #[diesel(sql_type = BigInt)] - id: i64, - #[diesel(sql_type = Nullable)] - commit_id: Option, - #[diesel(sql_type = BigInt)] - base_commit_id: i64, - #[diesel(sql_type = Nullable)] - commit_message: Option, - #[diesel(sql_type = Nullable)] - generated_commit_message: Option, - #[diesel(sql_type = BigInt)] - created_at: i64, - #[diesel(sql_type = Nullable)] - evolution_id: Option, -} - -impl From for ChangeSet { - fn from(row: ChangeSetRow) -> Self { - Self { - id: row.id, - commit_id: row.commit_id, - base_commit_id: row.base_commit_id, - commit_message: row.commit_message, - generated_commit_message: row.generated_commit_message, - created_at: row.created_at, - evolution_id: row.evolution_id, - } - } -} - -#[allow(dead_code)] -pub fn query_change_set_for_commit_pair( - conn: &mut SqliteConnection, - commit_id: i64, - base_commit_id: i64, -) -> Result> { - let cs: Option = sql_query( - "SELECT id, commit_id, base_commit_id, commit_message, generated_commit_message, \ - created_at, evolution_id \ - FROM change_sets WHERE commit_id = ?1 AND base_commit_id = ?2 \ - ORDER BY created_at DESC LIMIT 1", - ) - .bind::(commit_id) - .bind::(base_commit_id) - .get_result(conn) - .optional()?; - - let Some(cs) = cs else { return Ok(None) }; - let change_set: ChangeSet = cs.into(); - let change_set_id = change_set.id; - - // Subquery picks at most one group summary per change: only summaries whose - // entire member set is present in this change_set (no orphaned members). - let rows: Vec = sql_query(format!( - "{CHANGE_SELECT} - FROM set_changes sc - JOIN changes c ON c.id = sc.change_id - LEFT JOIN change_summaries os ON os.id = c.own_summary_id - LEFT JOIN ( - SELECT g.change_id, MAX(g.change_summary_id) AS change_summary_id - FROM group_summaries g - WHERE NOT EXISTS ( - SELECT 1 FROM group_summaries g2 - WHERE g2.change_summary_id = g.change_summary_id - AND g2.change_id NOT IN ( - SELECT change_id FROM set_changes WHERE change_set_id = ?1 - ) - ) - GROUP BY g.change_id - ) best_gs ON best_gs.change_id = c.id - LEFT JOIN change_summaries gs ON gs.id = best_gs.change_summary_id - WHERE sc.change_set_id = ?1" - )) - .bind::(change_set_id) - .load(conn)?; - - let changes = rows.into_iter().map(Into::into).collect(); - Ok(Some(SummarizedChangeSet { - change_set, - changes, - missed_hashes: vec![], - })) -} - -pub fn query_change_set_for_base_with_hashes( - conn: &mut SqliteConnection, - base_commit_id: i64, - hashes: &[String], -) -> Result> { - let cs: Option = sql_query( - "SELECT id, commit_id, base_commit_id, commit_message, generated_commit_message, \ - created_at, evolution_id FROM change_sets WHERE base_commit_id = ?1 \ - ORDER BY created_at DESC LIMIT 1", - ) - .bind::(base_commit_id) - .get_result(conn) - .optional()?; - - let Some(cs) = cs else { return Ok(None) }; - let change_set: ChangeSet = cs.into(); - - let matched = query_changes_by_hashes_for_base(conn, base_commit_id, hashes)?; - let matched_set: std::collections::HashSet<&str> = - matched.iter().map(|sc| sc.change.hash.as_str()).collect(); - let missed_hashes = hashes - .iter() - .filter(|h| !matched_set.contains(h.as_str())) - .cloned() - .collect(); - - Ok(Some(SummarizedChangeSet { - change_set, - changes: matched, - missed_hashes, - })) -} - -fn query_changes_by_hashes_for_base( - conn: &mut SqliteConnection, - base_commit_id: i64, - hashes: &[String], -) -> Result> { - if hashes.is_empty() { - return Ok(vec![]); - } - - // Numbered params (?2, ?3, …) are bound once and referenced twice in the - // generated SQL, so the bind list is base_commit_id followed by the hashes. - let placeholders = (2..=hashes.len() + 1) - .map(|i| format!("?{i}")) - .collect::>() - .join(", "); - let sql = format!( - "{CHANGE_SELECT} - FROM changes c - JOIN set_changes sc ON sc.change_id = c.id - JOIN change_sets cs ON cs.id = sc.change_set_id - LEFT JOIN change_summaries os ON os.id = c.own_summary_id - LEFT JOIN ( - SELECT g.change_id, MAX(g.change_summary_id) AS change_summary_id - FROM group_summaries g - WHERE NOT EXISTS ( - SELECT 1 FROM group_summaries g2 - WHERE g2.change_summary_id = g.change_summary_id - AND g2.change_id NOT IN ( - SELECT id FROM changes WHERE hash IN ({placeholders}) - ) - ) - GROUP BY g.change_id - ) best_gs ON best_gs.change_id = c.id - LEFT JOIN change_summaries gs ON gs.id = best_gs.change_summary_id - WHERE cs.base_commit_id = ?1 AND c.hash IN ({placeholders})" - ); - - let mut q = sql_query(sql).into_boxed::(); - q = q.bind::(base_commit_id); - for hash in hashes { - q = q.bind::(hash.clone()); - } - let rows: Vec = q.load(conn)?; - Ok(rows.into_iter().map(Into::into).collect()) -} - -/// Fetch the change hashes stored in a changeset. -pub fn fetch_hashes_for_changeset( - conn: &mut SqliteConnection, - changeset_id: i64, -) -> Result> { - let hashes = changes::table - .inner_join(set_changes::table.on(set_changes::change_id.eq(changes::id))) - .filter(set_changes::change_set_id.eq(changeset_id)) - .select(changes::hash) - .load::(conn)?; - Ok(hashes) -} - -fn last_insert_rowid(conn: &mut SqliteConnection) -> Result { - Ok(diesel::select(diesel::dsl::sql::("last_insert_rowid()")).get_result(conn)?) -} diff --git a/apps/native/src-tauri/src/db/commits.rs b/apps/native/src-tauri/src/db/commits.rs deleted file mode 100644 index f062a1fe6..000000000 --- a/apps/native/src-tauri/src/db/commits.rs +++ /dev/null @@ -1,132 +0,0 @@ -//! Commit persistence operations. - -use anyhow::Result; -use diesel::prelude::*; - -use crate::db::DbPool; -use crate::db::tables::commits; - -#[derive(Debug, Insertable)] -#[diesel(table_name = commits)] -struct NewCommit<'a> { - hash: &'a str, - tree_hash: &'a str, - message: Option<&'a str>, - created_at: i64, -} - -#[derive(Debug, Queryable, Selectable)] -#[diesel(table_name = commits)] -struct CommitRow { - id: i64, - hash: String, - tree_hash: String, - message: Option, - created_at: i64, -} - -impl From for crate::sqlite_types::Commit { - fn from(row: CommitRow) -> Self { - Self { - id: row.id, - hash: row.hash, - tree_hash: row.tree_hash, - message: row.message, - created_at: row.created_at, - } - } -} - -/// Insert a commit through the managed Diesel pool, returning its id. -pub fn upsert_commit( - pool: &DbPool, - hash: &str, - tree_hash: &str, - message: Option<&str>, - created_at: i64, -) -> Result { - let mut conn = pool.get()?; - - use commits::dsl; - - match dsl::commits - .filter(dsl::hash.eq(hash)) - .select(dsl::id) - .first::(&mut conn) - { - Ok(existing_id) => return Ok(existing_id), - Err(diesel::result::Error::NotFound) => {} - Err(e) => return Err(e.into()), - } - - diesel::insert_into(commits::table) - .values(NewCommit { - hash, - tree_hash, - message, - created_at, - }) - .execute(&mut conn)?; - - Ok(dsl::commits - .filter(dsl::hash.eq(hash)) - .select(dsl::id) - .first::(&mut conn)?) -} - -/// Returns the full commit row for a given hash through the managed Diesel pool. -pub fn get_commit_by_hash( - pool: &DbPool, - hash: &str, -) -> Result> { - let mut conn = pool.get()?; - let row = commits::table - .filter(commits::hash.eq(hash)) - .select(CommitRow::as_select()) - .first::(&mut conn) - .optional()?; - - Ok(row.map(Into::into)) -} - -/// Passes through `existing` if `Some`; otherwise resolves HEAD from git and upserts it. -pub fn store_head_commit( - pool: &DbPool, - config_dir: &str, - existing: Option, -) -> Result> { - if let Some(id) = existing { - return Ok(Some(id)); - } - let Some(hash) = crate::git::get_ref_sha(config_dir, "HEAD") else { - return Ok(None); - }; - let Some(tree_hash) = crate::git::get_ref_sha(config_dir, "HEAD^{tree}") else { - return Ok(None); - }; - let now = crate::utils::unix_now(); - Ok(Some(upsert_commit(pool, &hash, &tree_hash, None, now)?)) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[tokio::test] - async fn pool_backed_commit_helpers_upsert_and_fetch_commit_rows() { - let temp_dir = tempfile::tempdir().unwrap(); - let db_path = temp_dir.path().join("nixmac.db"); - let pool = crate::db::init_pool_at_path(&db_path).await.unwrap(); - - let first_id = upsert_commit(&pool, "abc123", "tree123", Some("message"), 123).unwrap(); - let second_id = upsert_commit(&pool, "abc123", "tree123", Some("message"), 123).unwrap(); - let commit = get_commit_by_hash(&pool, "abc123").unwrap().unwrap(); - - assert_eq!(first_id, second_id); - assert_eq!(commit.id, first_id); - assert_eq!(commit.hash, "abc123"); - assert_eq!(commit.tree_hash, "tree123"); - assert_eq!(commit.message.as_deref(), Some("message")); - assert_eq!(commit.created_at, 123); - } -} diff --git a/apps/native/src-tauri/src/db/evolutions.rs b/apps/native/src-tauri/src/db/evolutions.rs index a1072c6c8..07aafbfb5 100644 --- a/apps/native/src-tauri/src/db/evolutions.rs +++ b/apps/native/src-tauri/src/db/evolutions.rs @@ -24,11 +24,7 @@ pub fn upsert(pool: &DbPool, existing_id: Option, origin_branch: &str) -> R } diesel::insert_into(evolutions::table) - .values(( - evolutions::origin_branch.eq(origin_branch), - evolutions::merged.eq(0), - evolutions::builds.eq(0), - )) + .values(evolutions::origin_branch.eq(origin_branch)) .execute(&mut conn)?; let id = diesel::select(diesel::dsl::sql::( diff --git a/apps/native/src-tauri/src/db/keys.rs b/apps/native/src-tauri/src/db/keys.rs new file mode 100644 index 000000000..ed5defda1 --- /dev/null +++ b/apps/native/src-tauri/src/db/keys.rs @@ -0,0 +1,69 @@ +//! Content-addressing helpers for summaries and snapshots. +//! +//! A "content key" is the sha256 hex digest of a set of change hashes: the +//! hashes are sorted and de-duplicated, then joined with a NUL separator before +//! hashing. This mirrors `git::hunk_hash` (also sha2/sha256) so identity is +//! stable and order-independent across summarization runs. + +use sha2::{Digest, Sha256}; + +/// Compute the content key for a set of change hashes. +/// +/// Sorting + dedup make the key a pure function of the *set* of members, so the +/// same group / snapshot is recognized regardless of the order the hashes were +/// discovered in. +pub fn content_key(hashes: &[String]) -> String { + let mut sorted: Vec<&str> = hashes.iter().map(String::as_str).collect(); + sorted.sort_unstable(); + sorted.dedup(); + + let mut hasher = Sha256::new(); + for (i, hash) in sorted.iter().enumerate() { + if i > 0 { + hasher.update(b"\0"); + } + hasher.update(hash.as_bytes()); + } + format!("{:x}", hasher.finalize()) +} + +/// Key identifying an exact set of change hashes for a cached snapshot. +pub fn snapshot_key(hashes: &[String]) -> String { + content_key(hashes) +} + +/// Key identifying the exact membership of a summary group. +pub fn group_key(hashes: &[String]) -> String { + content_key(hashes) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn content_key_is_order_independent() { + let a = content_key(&["b".into(), "a".into(), "c".into()]); + let b = content_key(&["c".into(), "b".into(), "a".into()]); + assert_eq!(a, b); + } + + #[test] + fn content_key_ignores_duplicates() { + let a = content_key(&["a".into(), "b".into()]); + let b = content_key(&["a".into(), "b".into(), "a".into()]); + assert_eq!(a, b); + } + + #[test] + fn content_key_differs_by_membership() { + let a = content_key(&["a".into(), "b".into()]); + let b = content_key(&["a".into(), "c".into()]); + assert_ne!(a, b); + } + + #[test] + fn empty_set_has_a_stable_key() { + assert_eq!(content_key(&[]), content_key(&[])); + } +} diff --git a/apps/native/src-tauri/src/db/mod.rs b/apps/native/src-tauri/src/db/mod.rs index 34cbb5cea..58d0f80b4 100644 --- a/apps/native/src-tauri/src/db/mod.rs +++ b/apps/native/src-tauri/src/db/mod.rs @@ -1,13 +1,12 @@ -//! SQLite database for persisting evolution history, summaries, and prompts. +//! SQLite database for content-addressed summaries, snapshots, and app metadata. -pub mod changesets; -pub mod commits; pub mod evolutions; +pub mod keys; pub mod pool; pub mod restore_commits; mod schema; -pub mod store_bare_changeset; -pub mod store_whole_diff_changeset; +pub mod snapshots; +pub mod summaries; pub(crate) mod tables; use anyhow::Result; @@ -67,7 +66,7 @@ mod tests { let pool = init_pool_at_path(&db_path).await.unwrap(); let mut conn = pool.get().unwrap(); - let count = crate::db::tables::commits::table + let count = crate::db::tables::snapshots::table .select(count_star()) .first::(&mut conn) .unwrap(); @@ -75,24 +74,45 @@ mod tests { } #[tokio::test] - async fn migration_03_drops_queued_summaries_table() { - // PR #330 removed the queued summary pipeline; migration 03 then drops - // the table the worker used to drain. New databases shouldn't contain - // it after init. + async fn schema_creates_content_addressed_tables_and_no_legacy_tables() { let temp_dir = tempfile::tempdir().unwrap(); let db_path = temp_dir.path().join("nixmac.db"); let pool = init_pool_at_path(&db_path).await.unwrap(); let mut conn = pool.get().unwrap(); - let surviving = diesel::select(diesel::dsl::sql::( - "COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'queued_summaries'", - )) - .get_result::(&mut conn) - .unwrap(); - assert_eq!( - surviving, 0, - "queued_summaries should be dropped by 03-drop-queued-summaries" - ); + for present in [ + "patch_summaries", + "summary_groups", + "summary_group_members", + "snapshots", + "evolutions", + "restore_commits", + ] { + let count = diesel::select(diesel::dsl::sql::(&format!( + "COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = '{present}'" + ))) + .get_result::(&mut conn) + .unwrap(); + assert_eq!(count, 1, "{present} should exist"); + } + + for absent in [ + "queued_summaries", + "commits", + "changes", + "change_sets", + "set_changes", + "change_summaries", + "group_summaries", + "prompts", + ] { + let count = diesel::select(diesel::dsl::sql::(&format!( + "COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = '{absent}'" + ))) + .get_result::(&mut conn) + .unwrap(); + assert_eq!(count, 0, "{absent} should not exist"); + } } } diff --git a/apps/native/src-tauri/src/db/schema.rs b/apps/native/src-tauri/src/db/schema.rs index 694218421..56f46c0d5 100644 --- a/apps/native/src-tauri/src/db/schema.rs +++ b/apps/native/src-tauri/src/db/schema.rs @@ -8,7 +8,7 @@ use diesel::sqlite::SqliteConnection; use diesel_migrations::{EmbeddedMigrations, MigrationHarness, embed_migrations}; use std::path::Path; -const SCHEMA_VERSION: i64 = 1; +const SCHEMA_VERSION: i64 = 2; const MIGRATIONS: EmbeddedMigrations = embed_migrations!("./migrations"); /// Initialize schema. diff --git a/apps/native/src-tauri/src/db/snapshots.rs b/apps/native/src-tauri/src/db/snapshots.rs new file mode 100644 index 000000000..10068040b --- /dev/null +++ b/apps/native/src-tauri/src/db/snapshots.rs @@ -0,0 +1,118 @@ +//! Snapshot persistence — caches the generated commit message for an exact set +//! of change hashes, content-addressed by `snapshot_key`. +//! +//! The integer `id` is the value historically plumbed as `changeset_id` +//! throughout evolve/build state, so callers can keep using an `i64` handle. + +use anyhow::Result; +use diesel::prelude::*; + +use crate::db::DbPool; +use crate::db::tables::snapshots; + +/// A cached snapshot row. +pub struct Snapshot { + pub id: i64, + pub generated_commit_message: Option, +} + +/// Upsert a snapshot by `snapshot_key`, returning its id. +/// +/// When the key already exists, the row is reused and its +/// `generated_commit_message` is updated only when a non-empty message is +/// supplied — a bare (build-check) upsert must not clobber a real message. +pub fn upsert( + pool: &DbPool, + snapshot_key: &str, + generated_commit_message: Option<&str>, + evolution_id: Option, + created_at: i64, +) -> Result { + let mut conn = pool.get()?; + + let existing: Option = snapshots::table + .filter(snapshots::snapshot_key.eq(snapshot_key)) + .select(snapshots::id) + .first::(&mut conn) + .optional()?; + + if let Some(id) = existing { + let has_message = generated_commit_message.is_some_and(|m| !m.trim().is_empty()); + if has_message { + diesel::update(snapshots::table.filter(snapshots::id.eq(id))) + .set(( + snapshots::generated_commit_message.eq(generated_commit_message), + snapshots::evolution_id.eq(evolution_id), + )) + .execute(&mut conn)?; + } + return Ok(id); + } + + diesel::insert_into(snapshots::table) + .values(( + snapshots::snapshot_key.eq(snapshot_key), + snapshots::generated_commit_message.eq(generated_commit_message), + snapshots::evolution_id.eq(evolution_id), + snapshots::created_at.eq(created_at), + )) + .execute(&mut conn)?; + + Ok(snapshots::table + .filter(snapshots::snapshot_key.eq(snapshot_key)) + .select(snapshots::id) + .first::(&mut conn)?) +} + +/// Fetch a snapshot by its content key. +pub fn get_by_key(pool: &DbPool, snapshot_key: &str) -> Result> { + let mut conn = pool.get()?; + Ok(snapshots::table + .filter(snapshots::snapshot_key.eq(snapshot_key)) + .select((snapshots::id, snapshots::generated_commit_message)) + .first::<(i64, Option)>(&mut conn) + .optional()? + .map(|(id, generated_commit_message)| Snapshot { + id, + generated_commit_message, + })) +} + +/// Fetch the content key for a snapshot id, used to verify build state without +/// re-storing membership. +pub fn get_key_by_id(pool: &DbPool, id: i64) -> Result> { + let mut conn = pool.get()?; + Ok(snapshots::table + .filter(snapshots::id.eq(id)) + .select(snapshots::snapshot_key) + .first::(&mut conn) + .optional()?) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn upsert_reuses_key_and_preserves_existing_message() { + let temp_dir = tempfile::tempdir().unwrap(); + let db_path = temp_dir.path().join("nixmac.db"); + let pool = crate::db::init_pool_at_path(&db_path).await.unwrap(); + + let first = upsert(&pool, "key-a", Some("feat: thing"), None, 1).unwrap(); + // Bare upsert (null message) must not clobber the stored message. + let same = upsert(&pool, "key-a", None, None, 2).unwrap(); + assert_eq!(first, same); + + let snap = get_by_key(&pool, "key-a").unwrap().unwrap(); + assert_eq!(snap.id, first); + assert_eq!( + snap.generated_commit_message.as_deref(), + Some("feat: thing") + ); + assert_eq!( + get_key_by_id(&pool, first).unwrap().as_deref(), + Some("key-a") + ); + } +} diff --git a/apps/native/src-tauri/src/db/store_bare_changeset.rs b/apps/native/src-tauri/src/db/store_bare_changeset.rs deleted file mode 100644 index 7819ca199..000000000 --- a/apps/native/src-tauri/src/db/store_bare_changeset.rs +++ /dev/null @@ -1,31 +0,0 @@ -//! Lightweight changeset persistence — no AI, no queued summaries. - -use anyhow::Result; -use diesel::connection::Connection; - -use crate::db::DbPool; -use crate::db::changesets::{insert_change_or_ignore, insert_change_set, link_change_to_set}; -use crate::sqlite_types::Change; -use crate::utils::unix_now; - -/// Insert `changes` as bare rows where absent, create a new changeset. -pub fn store(pool: &DbPool, base_commit_id: i64, changes: &[Change]) -> Result { - let mut conn = pool.get()?; - let now = unix_now(); - - conn.transaction::(|conn| { - let mut change_ids = Vec::with_capacity(changes.len()); - for change in changes { - let id = insert_change_or_ignore(conn, change, None)?; - change_ids.push(id); - } - - let change_set_id = insert_change_set(conn, None, base_commit_id, None, None, now, None)?; - - for id in change_ids { - link_change_to_set(conn, change_set_id, id)?; - } - - Ok(change_set_id) - }) -} diff --git a/apps/native/src-tauri/src/db/store_whole_diff_changeset.rs b/apps/native/src-tauri/src/db/store_whole_diff_changeset.rs deleted file mode 100644 index d39d885ec..000000000 --- a/apps/native/src-tauri/src/db/store_whole_diff_changeset.rs +++ /dev/null @@ -1,113 +0,0 @@ -//! Persists a whole-diff summarization result — one or more groups, each with -//! its own commit message, where every change is linked to its group's summary. - -use anyhow::Result; -use diesel::connection::Connection; - -use crate::db::DbPool; -use crate::db::changesets::{ - get_change_id_by_hash, insert_change_set, insert_change_summary, link_change_to_group_summary, - link_change_to_set, upsert_change, -}; -use crate::sqlite_types::Change; -use crate::summarize::pipelines::whole_diff::GroupedChange; - -#[allow(clippy::too_many_arguments)] -pub fn store( - pool: &DbPool, - groups: &[GroupedChange], - generated_commit_message: &str, - commit_id: Option, - base_commit_id: i64, - commit_message: Option<&str>, - evolution_id: Option, -) -> Result { - let mut conn = pool.get()?; - let now = crate::utils::unix_now(); - - conn.transaction::(|conn| { - let mut change_ids: Vec = Vec::with_capacity(groups.len()); - - // One group_summary row per distinct summary string; multiple changes - // sharing a summary are linked to the same row. Summary equality is - // intentionally string-based — two groups with identical text are - // collapsed, mirroring how `group_existing` reconstructs groups by - // `group_summary.id`. - let mut summary_id_by_text: std::collections::HashMap = - std::collections::HashMap::new(); - - for grouped in groups { - let change = &grouped.change; - let description = grouped.summary.trim(); - let title = description.lines().next().unwrap_or(description).trim(); - - let group_summary_id = if let Some(id) = summary_id_by_text.get(description) { - *id - } else { - let id = insert_change_summary(conn, title, description, "DONE", now)?; - summary_id_by_text.insert(description.to_string(), id); - id - }; - - let own_title = std::path::Path::new(&change.filename) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(&change.filename); - // Per-change own_summary mirrors the group summary; the read path - // treats a change without an own_summary as "unsummarized" and - // surfaces it in `unsummarized_hashes`, so we set it to keep the - // change off the re-summarize list. - let own_summary_id = insert_change_summary(conn, own_title, description, "DONE", now)?; - upsert_change(conn, change, Some(own_summary_id))?; - let change_id = get_change_id_by_hash(conn, &change.hash)?; - link_change_to_group_summary(conn, change_id, group_summary_id)?; - change_ids.push(change_id); - } - - let change_set_id = insert_change_set( - conn, - commit_id, - base_commit_id, - commit_message, - Some(generated_commit_message), - now, - evolution_id, - )?; - - for change_id in change_ids { - link_change_to_set(conn, change_set_id, change_id)?; - } - - Ok(change_set_id) - }) -} - -/// Convenience wrapper for callers that still produce a single message for -/// the entire changeset (legacy single-group path). -#[allow(dead_code)] -pub fn store_single( - pool: &DbPool, - changes: &[Change], - message: &str, - commit_id: Option, - base_commit_id: i64, - commit_message: Option<&str>, - evolution_id: Option, -) -> Result { - let groups: Vec = changes - .iter() - .map(|c| GroupedChange { - change: c.clone(), - summary: message.to_string(), - }) - .collect(); - store( - pool, - &groups, - message, - commit_id, - base_commit_id, - commit_message, - evolution_id, - ) -} diff --git a/apps/native/src-tauri/src/db/summaries.rs b/apps/native/src-tauri/src/db/summaries.rs new file mode 100644 index 000000000..e800742f0 --- /dev/null +++ b/apps/native/src-tauri/src/db/summaries.rs @@ -0,0 +1,272 @@ +//! Summary persistence — content-addressed patch (single) and group summaries. +//! +//! A summary describes *what a patch does*. It is keyed only by the content +//! hash(es) of the change(s) it covers — never by a base commit. Groups are +//! first-class and identified by `group_key = hash(sorted member hashes)`. + +use std::collections::{HashMap, HashSet}; + +use anyhow::Result; +use diesel::prelude::*; + +use crate::db::DbPool; +use crate::db::keys; +use crate::db::tables::{patch_summaries, summary_group_members, summary_groups}; + +/// A stored per-change (single / fallback) summary. +#[derive(Clone)] +#[allow(dead_code)] // `change_hash`/`created_at` retained for completeness / future use. +pub struct PatchRow { + pub change_hash: String, + pub title: String, + pub description: String, + pub status: String, + pub created_at: i64, +} + +/// A stored group summary together with its exact membership. +#[derive(Clone)] +pub struct GroupRow { + pub id: i64, + pub title: String, + pub description: String, + pub status: String, + pub created_at: i64, + pub members: Vec, +} + +/// Upsert a per-change summary keyed by `change_hash`. +pub fn store_patch( + pool: &DbPool, + change_hash: &str, + title: &str, + description: &str, + status: &str, + created_at: i64, +) -> Result<()> { + let mut conn = pool.get()?; + diesel::insert_into(patch_summaries::table) + .values(( + patch_summaries::change_hash.eq(change_hash), + patch_summaries::title.eq(title), + patch_summaries::description.eq(description), + patch_summaries::status.eq(status), + patch_summaries::created_at.eq(created_at), + )) + .on_conflict(patch_summaries::change_hash) + .do_update() + .set(( + patch_summaries::title.eq(title), + patch_summaries::description.eq(description), + patch_summaries::status.eq(status), + patch_summaries::created_at.eq(created_at), + )) + .execute(&mut conn)?; + Ok(()) +} + +/// Upsert a group summary keyed by the content hash of its members, replacing +/// membership so the group's identity always matches its exact member set. +pub fn store_group( + pool: &DbPool, + member_hashes: &[String], + title: &str, + description: &str, + status: &str, + created_at: i64, +) -> Result { + let group_key = keys::group_key(member_hashes); + let mut conn = pool.get()?; + + conn.transaction::<_, anyhow::Error, _>(|conn| { + diesel::insert_into(summary_groups::table) + .values(( + summary_groups::group_key.eq(&group_key), + summary_groups::title.eq(title), + summary_groups::description.eq(description), + summary_groups::status.eq(status), + summary_groups::created_at.eq(created_at), + )) + .on_conflict(summary_groups::group_key) + .do_update() + .set(( + summary_groups::title.eq(title), + summary_groups::description.eq(description), + summary_groups::status.eq(status), + summary_groups::created_at.eq(created_at), + )) + .execute(conn)?; + + diesel::delete( + summary_group_members::table.filter(summary_group_members::group_key.eq(&group_key)), + ) + .execute(conn)?; + + let mut unique: Vec<&String> = member_hashes.iter().collect(); + unique.sort_unstable(); + unique.dedup(); + for hash in unique { + diesel::insert_into(summary_group_members::table) + .values(( + summary_group_members::group_key.eq(&group_key), + summary_group_members::change_hash.eq(hash), + )) + .on_conflict(( + summary_group_members::group_key, + summary_group_members::change_hash, + )) + .do_nothing() + .execute(conn)?; + } + Ok(()) + })?; + + Ok(group_key) +} + +/// Load every group whose full membership is contained in `live_hashes`. +/// +/// A group only qualifies when *all* of its members are live, so partially +/// present groups never surface (their identity is exact). +pub fn groups_within(pool: &DbPool, live_hashes: &[String]) -> Result> { + if live_hashes.is_empty() { + return Ok(vec![]); + } + let live_set: HashSet<&str> = live_hashes.iter().map(String::as_str).collect(); + let mut conn = pool.get()?; + + // Candidate group keys: any group that has at least one live member. + let candidate_keys: Vec = summary_group_members::table + .filter(summary_group_members::change_hash.eq_any(live_hashes)) + .select(summary_group_members::group_key) + .distinct() + .load::(&mut conn)?; + + if candidate_keys.is_empty() { + return Ok(vec![]); + } + + // Full membership of every candidate group. + let member_rows: Vec<(String, String)> = summary_group_members::table + .filter(summary_group_members::group_key.eq_any(&candidate_keys)) + .select(( + summary_group_members::group_key, + summary_group_members::change_hash, + )) + .load::<(String, String)>(&mut conn)?; + + let mut members_by_key: HashMap> = HashMap::new(); + for (key, hash) in member_rows { + members_by_key.entry(key).or_default().push(hash); + } + + // Keep only groups whose entire membership is live. + let qualifying: Vec = members_by_key + .iter() + .filter(|(_, members)| members.iter().all(|h| live_set.contains(h.as_str()))) + .map(|(key, _)| key.clone()) + .collect(); + + if qualifying.is_empty() { + return Ok(vec![]); + } + + let group_rows: Vec<(i64, String, String, String, String, i64)> = summary_groups::table + .filter(summary_groups::group_key.eq_any(&qualifying)) + .select(( + summary_groups::id, + summary_groups::group_key, + summary_groups::title, + summary_groups::description, + summary_groups::status, + summary_groups::created_at, + )) + .load(&mut conn)?; + + Ok(group_rows + .into_iter() + .map(|(id, key, title, description, status, created_at)| { + let members = members_by_key.remove(&key).unwrap_or_default(); + GroupRow { + id, + title, + description, + status, + created_at, + members, + } + }) + .collect()) +} + +/// Load per-change summaries for the given change hashes, keyed by hash. +pub fn patches_for(pool: &DbPool, hashes: &[String]) -> Result> { + if hashes.is_empty() { + return Ok(HashMap::new()); + } + let mut conn = pool.get()?; + let rows: Vec<(String, String, String, String, i64)> = patch_summaries::table + .filter(patch_summaries::change_hash.eq_any(hashes)) + .select(( + patch_summaries::change_hash, + patch_summaries::title, + patch_summaries::description, + patch_summaries::status, + patch_summaries::created_at, + )) + .load(&mut conn)?; + + Ok(rows + .into_iter() + .map(|(change_hash, title, description, status, created_at)| { + ( + change_hash.clone(), + PatchRow { + change_hash, + title, + description, + status, + created_at, + }, + ) + }) + .collect()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn group_only_matches_when_all_members_live() { + let temp_dir = tempfile::tempdir().unwrap(); + let db_path = temp_dir.path().join("nixmac.db"); + let pool = crate::db::init_pool_at_path(&db_path).await.unwrap(); + + store_group(&pool, &["a".into(), "b".into()], "title", "desc", "DONE", 0).unwrap(); + + // Both members live → group surfaces. + let found = groups_within(&pool, &["a".into(), "b".into(), "c".into()]).unwrap(); + assert_eq!(found.len(), 1); + assert_eq!(found[0].members.len(), 2); + + // Only one member live → group is excluded (exact identity). + let partial = groups_within(&pool, &["a".into(), "c".into()]).unwrap(); + assert!(partial.is_empty()); + } + + #[tokio::test] + async fn store_patch_upserts_by_hash() { + let temp_dir = tempfile::tempdir().unwrap(); + let db_path = temp_dir.path().join("nixmac.db"); + let pool = crate::db::init_pool_at_path(&db_path).await.unwrap(); + + store_patch(&pool, "h1", "t1", "d1", "DONE", 0).unwrap(); + store_patch(&pool, "h1", "t2", "d2", "DONE", 5).unwrap(); + + let map = patches_for(&pool, &["h1".into()]).unwrap(); + let row = map.get("h1").unwrap(); + assert_eq!(row.title, "t2"); + assert_eq!(row.description, "d2"); + } +} diff --git a/apps/native/src-tauri/src/db/tables.rs b/apps/native/src-tauri/src/db/tables.rs index 5ad90dddf..d1a92dde9 100644 --- a/apps/native/src-tauri/src/db/tables.rs +++ b/apps/native/src-tauri/src/db/tables.rs @@ -1,27 +1,16 @@ //! Diesel table declarations for query-builder-backed database code. -diesel::table! { - commits (id) { - id -> BigInt, - hash -> Text, - tree_hash -> Text, - message -> Nullable, - created_at -> BigInt, - } -} - diesel::table! { evolutions (id) { id -> BigInt, origin_branch -> Text, - merged -> Integer, - builds -> Integer, } } diesel::table! { - change_summaries (id) { + patch_summaries (id) { id -> BigInt, + change_hash -> Text, title -> Text, description -> Text, status -> Text, @@ -30,48 +19,29 @@ diesel::table! { } diesel::table! { - changes (id) { + summary_groups (id) { id -> BigInt, - hash -> Text, - filename -> Text, - diff -> Text, - line_count -> BigInt, + group_key -> Text, + title -> Text, + description -> Text, + status -> Text, created_at -> BigInt, - own_summary_id -> Nullable, } } diesel::table! { - group_summaries (change_id, change_summary_id) { - change_id -> BigInt, - change_summary_id -> BigInt, + summary_group_members (group_key, change_hash) { + group_key -> Text, + change_hash -> Text, } } diesel::table! { - change_sets (id) { + snapshots (id) { id -> BigInt, - commit_id -> Nullable, - base_commit_id -> BigInt, - commit_message -> Nullable, + snapshot_key -> Text, generated_commit_message -> Nullable, - created_at -> BigInt, evolution_id -> Nullable, - } -} - -diesel::table! { - set_changes (change_set_id, change_id) { - change_set_id -> BigInt, - change_id -> BigInt, - } -} - -diesel::table! { - prompts (id) { - id -> BigInt, - text -> Text, - commit_id -> Nullable, created_at -> BigInt, } } @@ -83,22 +53,13 @@ diesel::table! { } } -diesel::joinable!(changes -> change_summaries (own_summary_id)); -diesel::joinable!(group_summaries -> changes (change_id)); -diesel::joinable!(group_summaries -> change_summaries (change_summary_id)); -diesel::joinable!(change_sets -> evolutions (evolution_id)); -diesel::joinable!(set_changes -> change_sets (change_set_id)); -diesel::joinable!(set_changes -> changes (change_id)); -diesel::joinable!(prompts -> commits (commit_id)); +diesel::joinable!(snapshots -> evolutions (evolution_id)); diesel::allow_tables_to_appear_in_same_query!( - change_sets, - change_summaries, - changes, - commits, evolutions, - group_summaries, - prompts, + patch_summaries, restore_commits, - set_changes, + snapshots, + summary_group_members, + summary_groups, ); diff --git a/apps/native/src-tauri/src/git/exec.rs b/apps/native/src-tauri/src/git/exec.rs index 34715c6da..ef102f2b9 100644 --- a/apps/native/src-tauri/src/git/exec.rs +++ b/apps/native/src-tauri/src/git/exec.rs @@ -108,6 +108,7 @@ pub fn intent_add_untracked(dir: &str) -> Result<()> { /// Info about a created commit. pub struct CommitInfo { pub hash: String, + #[allow(dead_code)] // No longer mirrored to the DB, but cheap to keep populated. pub tree_hash: String, } diff --git a/apps/native/src-tauri/src/git/query.rs b/apps/native/src-tauri/src/git/query.rs index 07705ce56..55bcaceb4 100644 --- a/apps/native/src-tauri/src/git/query.rs +++ b/apps/native/src-tauri/src/git/query.rs @@ -12,12 +12,17 @@ use crate::{ /// Used to track the state of a file as we build up its diff, since git2 processing /// uses the metadata and content in separate passes. struct FileState { - diff: String, - line_count: i64, + hunks: Vec, is_binary: bool, last_hunk_key: Option<(usize, usize)>, } +/// A single independently reviewable portion of a file diff. +struct FileHunk { + diff: String, + line_count: i64, +} + /// Interhunk lines controls whether nearby changes are grouped together in the same hunk. /// It's normally 0 by default in the git CLI but we'll use 1 to be more aggressive about grouping /// nearby changes together, which should help with summarization quality (our main use case). @@ -511,8 +516,7 @@ fn run_diff_engine(diff: git2::Diff) -> Result> { .unwrap_or_default(); let entry = state.entry(filename).or_insert(FileState { - diff: String::new(), - line_count: 0, + hunks: Vec::new(), last_hunk_key: None, is_binary: delta.flags().contains(git2::DiffFlags::BINARY), }); @@ -527,36 +531,40 @@ fn run_diff_engine(diff: git2::Diff) -> Result> { if entry.last_hunk_key != Some(key) { entry.last_hunk_key = Some(key); - - entry.diff.push_str(&format!( - "@@ -{},{} +{},{} @@\n", - h.old_start(), - h.old_lines(), - h.new_start(), - h.new_lines() - )); + entry.hunks.push(FileHunk { + diff: format!( + "@@ -{},{} +{},{} @@\n", + h.old_start(), + h.old_lines(), + h.new_start(), + h.new_lines() + ), + line_count: 0, + }); } } // ----------------------------------- // Actual line content (with origin prefix if necessary) // ----------------------------------- - match line.origin() { - '+' => { - entry.diff.push('+'); - entry.diff.push_str(content); - entry.line_count += 1; - } - '-' => { - entry.diff.push('-'); - entry.diff.push_str(content); - entry.line_count += 1; + if let Some(hunk) = entry.hunks.last_mut() { + match line.origin() { + '+' => { + hunk.diff.push('+'); + hunk.diff.push_str(content); + hunk.line_count += 1; + } + '-' => { + hunk.diff.push('-'); + hunk.diff.push_str(content); + hunk.line_count += 1; + } + ' ' => { + hunk.diff.push(' '); + hunk.diff.push_str(content); + } + _ => {} } - ' ' => { - entry.diff.push(' '); - entry.diff.push_str(content); - } - _ => {} } true @@ -564,7 +572,8 @@ fn run_diff_engine(diff: git2::Diff) -> Result> { // ------------------------- // 3. Merge and filter-sensitive files pass - // Now merge the structured delta info with the patch text and line counts to produce final FileDiffs. + // Now merge the structured delta info with each patch hunk. Keeping hunks + // separate preserves their semantic identity even when they share a path. // ------------------------- let mut result = Vec::new(); @@ -576,16 +585,23 @@ fn run_diff_engine(diff: git2::Diff) -> Result> { .unwrap_or(""); if let Some(s) = state.remove(key) { - if is_sensitive_or_opaque(key, &s.diff, s.is_binary) { + let full_diff = s + .hunks + .iter() + .map(|hunk| hunk.diff.as_str()) + .collect::(); + if is_sensitive_or_opaque(key, &full_diff, s.is_binary) { continue; } - result.push(FileDiff { - old_path: f.old_path, - new_path: f.new_path, - diff: s.diff, - line_count: s.line_count, - }); + for hunk in s.hunks { + result.push(FileDiff { + old_path: f.old_path.clone(), + new_path: f.new_path.clone(), + diff: hunk.diff, + line_count: hunk.line_count, + }); + } } } @@ -1158,6 +1174,50 @@ mod tests { assert!(diffs[0].diff.contains("+{ inputs = {}; }")); } + #[test] + fn changes_since_ref_keeps_separate_hunks_in_the_same_file() { + let temp_dir = TempDir::new().unwrap(); + let repo_dir = temp_dir.path().join("repo"); + let repo_dir_str = repo_dir.to_string_lossy().to_string(); + + init_repo(&repo_dir_str).unwrap(); + fs::write( + repo_dir.join("configuration.nix"), + "line 1\nline 2\nline 3\nline 4\nline 5\nline 6\nline 7\nline 8\nline 9\nline 10\nline 11\nline 12\nline 13\nline 14\nline 15\nline 16\nline 17\nline 18\nline 19\nline 20\n", + ) + .unwrap(); + crate::git::commit_all(&repo_dir_str, "initial").unwrap(); + + let repo = git2::Repository::discover(&repo_dir_str).unwrap(); + let head = repo.head().unwrap().peel_to_commit().unwrap(); + repo.branch("evolution-start", &head, false).unwrap(); + + fs::write( + repo_dir.join("configuration.nix"), + "line 1\nupdated line 2\nline 3\nline 4\nline 5\nline 6\nline 7\nline 8\nline 9\nline 10\nline 11\nline 12\nline 13\nline 14\nline 15\nline 16\nline 17\nline 18\nupdated line 19\nline 20\n", + ) + .unwrap(); + + let diffs = changes_since_ref(&repo_dir_str, "evolution-start").unwrap(); + + assert_eq!(diffs.len(), 2); + assert!( + diffs + .iter() + .all(|diff| diff.new_path.as_deref() == Some("configuration.nix")) + ); + assert!( + diffs + .iter() + .any(|diff| diff.diff.contains("+updated line 2")) + ); + assert!( + diffs + .iter() + .any(|diff| diff.diff.contains("+updated line 19")) + ); + } + #[test] fn changes_since_ref_includes_untracked_files_since_base_ref() { let temp_dir = TempDir::new().unwrap(); diff --git a/apps/native/src-tauri/src/history/get_history.rs b/apps/native/src-tauri/src/history/get_history.rs index 45d0b105b..6b1464418 100644 --- a/apps/native/src-tauri/src/history/get_history.rs +++ b/apps/native/src-tauri/src/history/get_history.rs @@ -45,15 +45,6 @@ pub async fn get_history( let mut origin_hashes: Vec> = Vec::with_capacity(git_commits.len()); for (i, git_commit) in git_commits.iter().enumerate() { - let db_commit = - crate::db::commits::get_commit_by_hash(&pool, &git_commit.hash).unwrap_or(None); - - let parent_db = git_commits.get(i + 1).and_then(|parent| { - crate::db::commits::get_commit_by_hash(&pool, &parent.hash) - .ok() - .flatten() - }); - let raw_changes: Vec = git_commits .get(i + 1) .and_then(|parent| { @@ -75,27 +66,23 @@ pub async fn get_history( unique.len() }; - let (change_map, unsummarized_hashes) = if let Some(ref parent) = parent_db { - let diff_hashes: Vec = raw_changes.iter().map(|c| c.hash.clone()).collect(); - match crate::summarize::find_existing::by_base_with_hashes( - &pool, - parent.id, - &diff_hashes, - ) { + // Summaries are content-addressed by change hash, so a commit's change + // map is reconstructed purely from its diff — no commit rows needed. + let (change_map, unsummarized_hashes) = if raw_changes.is_empty() { + (None, vec![]) + } else { + match crate::summarize::find_existing::for_changes(&pool, &raw_changes) { Ok(found) => { - let grouped = crate::summarize::group_existing::from_change_sets(vec![found]); - let unsummarized = grouped.unsummarized_hashes.clone(); - let map = if grouped.groups.is_empty() && grouped.singles.is_empty() { + let unsummarized = found.map.unsummarized_hashes.clone(); + let map = if found.map.groups.is_empty() && found.map.singles.is_empty() { None } else { - Some(grouped) + Some(found.map) }; (map, unsummarized) } Err(_) => (None, vec![]), } - } else { - (None, vec![]) }; let origin_hash = if change_map.is_none() { @@ -126,7 +113,7 @@ pub async fn get_history( is_base, is_external, file_count, - commit: db_commit, + commit: None, change_map, unsummarized_hashes, raw_changes, diff --git a/apps/native/src-tauri/src/managed_edits/managed_edit.rs b/apps/native/src-tauri/src/managed_edits/managed_edit.rs index 540d77d3d..42cbd21be 100644 --- a/apps/native/src-tauri/src/managed_edits/managed_edit.rs +++ b/apps/native/src-tauri/src/managed_edits/managed_edit.rs @@ -20,9 +20,6 @@ pub fn prepare_managed_edit(app: &AppHandle) -> Result { git::status(&dir).context("Failed to get pre-edit working tree status")?; let pool = app.state::(); - let _base_commit_id = - db::commits::store_head_commit(&pool, &dir, None).context("Failed to store HEAD commit")?; - let pre_state = evolve_state::get_session(app); let branch = git::current_branch(&dir).unwrap_or_else(|| "main".to_string()); let evolution_id = db::evolutions::upsert(&pool, pre_state.evolution_id, &branch) @@ -103,13 +100,12 @@ pub async fn finalize_managed_edit( } let pool = app.state::(); - let change_sets = - summarize::find_existing::for_current_state(&pool, &context.dir).unwrap_or_default(); // Record the resulting state in the cells: the change-map write emits // `change_map_changed` and `status_and_cache` emits `git_state_changed` // (evolve state was already set above). The frontend mirrors the events. - let change_map = summarize::group_existing::from_change_sets(change_sets); + let change_map = + summarize::find_existing::for_current_state(&pool, &context.dir).unwrap_or_default(); crate::state::change_map::update(app, change_map); git::query::status_and_cache(&context.dir, app).context("Failed to get git status")?; diff --git a/apps/native/src-tauri/src/shared_types/git.rs b/apps/native/src-tauri/src/shared_types/git.rs index 9df951246..7aa31290d 100644 --- a/apps/native/src-tauri/src/shared_types/git.rs +++ b/apps/native/src-tauri/src/shared_types/git.rs @@ -1,7 +1,7 @@ use serde::{Deserialize, Serialize}; use specta::Type; -use crate::sqlite_types::{Change, ChangeSet, ChangeSummary}; +use crate::sqlite_types::{Change, ChangeSummary}; /// HEAD content vs working-tree content for a file, used by the diff tab Monaco DiffEditor. #[derive(Debug, Clone, Serialize, Deserialize, Type)] @@ -129,28 +129,6 @@ pub struct SemanticChangeMap { pub unsummarized_hashes: Vec, } -#[derive(Debug, Clone, Serialize, Deserialize, Type)] -#[serde(rename_all = "camelCase")] -pub struct SummarizedChange { - /// Raw change row. - pub change: Change, - /// Summary attached directly to this change. - pub own_summary: Option, - /// Summary inherited from this change's group. - pub group_summary: Option, -} - -#[derive(Debug, Clone, Serialize, Deserialize, Type)] -#[serde(rename_all = "camelCase")] -pub struct SummarizedChangeSet { - /// Change set represented by this response. - pub change_set: ChangeSet, - /// Changes in the set with their available summaries. - pub changes: Vec, - /// Change hashes expected in the set but missing from the database. - pub missed_hashes: Vec, -} - /// A commit entry combining git log data, tag-derived flags, optional DB metadata, and raw diff changes. #[derive(Debug, Clone, Serialize, Deserialize, Type)] #[serde(rename_all = "camelCase")] diff --git a/apps/native/src-tauri/src/sqlite_types.rs b/apps/native/src-tauri/src/sqlite_types.rs index 760185ce7..cf2bf7562 100644 --- a/apps/native/src-tauri/src/sqlite_types.rs +++ b/apps/native/src-tauri/src/sqlite_types.rs @@ -24,18 +24,6 @@ pub struct Commit { pub struct Evolution { pub id: i64, pub origin_branch: String, - pub merged: i64, - pub builds: i64, -} - -#[allow(dead_code)] -#[derive(Debug, Clone, Serialize, Deserialize, Type)] -#[serde(rename_all = "camelCase")] -pub struct Prompt { - pub id: i64, - pub text: String, - pub commit_id: Option, - pub created_at: i64, } #[allow(dead_code)] @@ -68,18 +56,3 @@ pub struct ChangeSummary { #[specta(type = f64)] pub created_at: i64, } - -/// Groups Changes for a commit→base_commit pair. `commit_id` is NULL for speculative -/// (uncommitted) changesets. Membership is stored in the `set_changes` join table. -#[allow(dead_code)] -#[derive(Debug, Clone, Serialize, Deserialize, Type, PartialEq)] -#[serde(rename_all = "camelCase")] -pub struct ChangeSet { - pub id: i64, - pub commit_id: Option, - pub base_commit_id: i64, - pub commit_message: Option, - pub generated_commit_message: Option, - pub created_at: i64, - pub evolution_id: Option, -} diff --git a/apps/native/src-tauri/src/state/build_state.rs b/apps/native/src-tauri/src/state/build_state.rs index 0d8907950..05a9e33df 100644 --- a/apps/native/src-tauri/src/state/build_state.rs +++ b/apps/native/src-tauri/src/state/build_state.rs @@ -78,19 +78,13 @@ pub fn current_state_built(app: &AppHandle, current_changes: &[Ch None => current_changes.is_empty(), Some(id) => { let pool = app.state::(); - let Ok(mut conn) = pool.get() else { + // The build snapshot's content key encodes the exact set of change + // hashes that were built; a match means the working tree is unchanged. + let Ok(Some(stored_key)) = crate::db::snapshots::get_key_by_id(&pool, id) else { return false; }; - let Ok(stored_hashes) = - crate::db::changesets::fetch_hashes_for_changeset(&mut conn, id) - else { - return false; - }; - let current_hashes: std::collections::HashSet<&str> = - current_changes.iter().map(|c| c.hash.as_str()).collect(); - let stored_set: std::collections::HashSet<&str> = - stored_hashes.iter().map(|h| h.as_str()).collect(); - current_hashes == stored_set + let hashes: Vec = current_changes.iter().map(|c| c.hash.clone()).collect(); + crate::db::keys::snapshot_key(&hashes) == stored_key } } } @@ -114,18 +108,20 @@ pub fn set_active_build( .map(|_| ()) } -/// Compute build state with a "bare" changeset to verify it +/// Record a bare snapshot (content key only, no message) for the built change +/// set so a later `current_state_built` check can verify it. pub fn record_build(app: &AppHandle, git_status: &GitStatus) -> Result<()> { - let config_dir = crate::storage::store::get_config_dir(app)?; let pool = app.state::(); let build_changeset_id = if !git_status.changes.is_empty() { - let base_id = crate::db::commits::store_head_commit(&pool, &config_dir, None)? - .ok_or_else(|| anyhow::anyhow!("missing HEAD commit while recording build state"))?; - Some(crate::db::store_bare_changeset::store( + let hashes: Vec = git_status.changes.iter().map(|c| c.hash.clone()).collect(); + let now = crate::utils::unix_now(); + Some(crate::db::snapshots::upsert( &pool, - base_id, - &git_status.changes, + &crate::db::keys::snapshot_key(&hashes), + None, + None, + now, )?) } else { None diff --git a/apps/native/src-tauri/src/state/watcher.rs b/apps/native/src-tauri/src/state/watcher.rs index 8539fb654..e271d8fc2 100644 --- a/apps/native/src-tauri/src/state/watcher.rs +++ b/apps/native/src-tauri/src/state/watcher.rs @@ -158,10 +158,7 @@ fn check_git_status( Default::default() } else { let pool = app_handle.state::(); - summarize::find_existing::for_current_state(&pool, dir) - .ok() - .map(summarize::group_existing::from_change_sets) - .unwrap_or_default() + summarize::find_existing::for_current_state(&pool, dir).unwrap_or_default() }; drift_notifications::maybe_notify(Some(&status), external_build_detected); app_handle diff --git a/apps/native/src-tauri/src/summarize/build_prompt.rs b/apps/native/src-tauri/src/summarize/build_prompt.rs index 24a2c7046..5c3e7d291 100644 --- a/apps/native/src-tauri/src/summarize/build_prompt.rs +++ b/apps/native/src-tauri/src/summarize/build_prompt.rs @@ -6,47 +6,41 @@ pub fn join_sections(sections: &[String]) -> String { } pub fn list_changes(changes: &[&crate::sqlite_types::Change]) -> String { - // Use the short_hash format. changes .iter() .map(|c| { format!( "hash: {}\nfile: {}\nlines: {}\ndiff:\n{}\n\n", - crate::utils::short_hash(&c.hash), - c.filename, - c.line_count, - c.diff + c.hash, c.filename, c.line_count, c.diff ) }) .collect() } /// Builds a prompt that summarizes all changes as one or more conventional -/// commit messages, each covering a subset of the changed files. +/// change descriptions, each covering a subset of the individual changes. pub fn whole_diff(changes: &[&crate::sqlite_types::Change]) -> String { join_sections(&[ - "Group the following changes into one or more conventional commit messages. \ + "Group the following changes into one or more semantic changes. \ Each group must share a coherent purpose (a single logical change). \ - Return one object per group.\n\n" + Return the groups in the required JSON response object.\n\n" .to_string(), list_changes(changes), "\nFor each group, return a JSON object with:\n".to_string(), - " - \"summary\": a conventional commit message in the form \ - (): \n".to_string(), - " - \"files\": an array of the file paths included in that group\n".to_string(), - "Allowed types: feat, fix, chore, refactor, docs, style, test, perf\n".to_string(), + " - \"summary\": a concise, factual plain-language summary of the change\n".to_string(), + " - \"changes\": an array of the exact change hashes included in that group\n".to_string(), "Rules:\n".to_string(), "- Base every summary only on the visible changes.\n".to_string(), "- Do not invent intent that is not visible in the diff.\n".to_string(), - "- If the type is unclear, prefer \"chore\".\n".to_string(), - "- Every changed file must appear in exactly one group.\n".to_string(), + "- Do not assign a conventional-commit type, scope, or prefix.\n".to_string(), + "- Every supplied change hash must appear in exactly one group.\n".to_string(), "- Prefer fewer groups; only split when changes are clearly unrelated.\n".to_string(), - "Return ONLY a valid JSON array.\n".to_string(), + "Return ONLY a valid JSON object with a \"groups\" array.\n".to_string(), "Example:\n".to_string(), - "[\n".to_string(), - " {\"summary\":\"feat(darwin): enable dock auto-hide\",\"files\":[\"darwin/dock.nix\"]},\n".to_string(), - " {\"summary\":\"chore: bump flake inputs\",\"files\":[\"flake.lock\"]}\n".to_string(), - "]\n\n".to_string(), + "{\n \"groups\": [\n".to_string(), + " {\"summary\":\"Enable dock auto-hide\",\"changes\":[\"\"]},\n".to_string(), + " {\"summary\":\"Update flake inputs\",\"changes\":[\"\"]}\n".to_string(), + " ]\n}\n\n".to_string(), ]) } @@ -56,7 +50,7 @@ mod tests { use crate::sqlite_types::Change; #[test] - fn whole_diff_requests_multi_item_array() { + fn whole_diff_requests_free_form_semantic_summaries() { let change = Change { id: 1, hash: "deadbeef".into(), @@ -67,13 +61,13 @@ mod tests { own_summary_id: None, }; let out = whole_diff(&[&change]); - assert!(out.contains(&crate::utils::short_hash(&change.hash))); - assert!(out.contains("one or more conventional commit messages")); + assert!(out.contains(&change.hash)); + assert!(out.contains("plain-language summary")); assert!(out.contains("\"summary\"")); - assert!(out.contains("\"files\"")); - assert!(out.contains("Return ONLY a valid JSON array")); - // Legacy single-message contract must be gone. - assert!(!out.contains("\"message\"")); - assert!(!out.contains("single conventional commit message")); + assert!(out.contains("\"changes\"")); + assert!(out.contains("Return ONLY a valid JSON object with a \"groups\" array")); + assert!(!out.contains("conventional commit")); + assert!(!out.contains("Allowed types:")); + assert!(!out.contains("feat, fix, chore")); } } diff --git a/apps/native/src-tauri/src/summarize/find_existing.rs b/apps/native/src-tauri/src/summarize/find_existing.rs index 75ee35a89..b9a8d69ae 100644 --- a/apps/native/src-tauri/src/summarize/find_existing.rs +++ b/apps/native/src-tauri/src/summarize/find_existing.rs @@ -1,97 +1,236 @@ -//! Orchestrators for querying change sets and summarized changes from the DB. +//! Lookup of existing summaries for a live set of changes. +//! +//! Summaries are content-addressed by change hash (no base commit). Given the +//! live changes, this reconstructs a [`SemanticChangeMap`] directly: +//! +//! 1. Load every stored group whose *entire* membership is present in the live +//! set, then greedily select non-overlapping groups preferring larger ones. +//! 2. For changes not covered by a group, look up per-change (single) summaries. +//! 3. Changes with neither a group nor a single summary are unsummarized. +//! 4. Load the cached snapshot (by the content key of all live hashes) for the +//! generated commit message. + +use std::collections::{HashMap, HashSet}; use anyhow::Result; use crate::db::DbPool; -use crate::shared_types::{SummarizedChange, SummarizedChangeSet}; -use crate::sqlite_types::ChangeSet; +use crate::shared_types::{ChangeWithSummary, SemanticChangeGroup, SemanticChangeMap}; +use crate::sqlite_types::{Change, ChangeSummary}; use crate::summarize::sumlog as dbg; -/// Type shared only between `for_current_state` and `group_existing::from_change_sets`. -/// Ensures missing hashes can be passed through when DB had nothing -pub struct FoundSetForCurrent { - pub change_set: Option, - pub changes: Vec, - pub missed_hashes: Vec, +/// Reconstructed summaries for a live set of changes, plus the cached snapshot +/// (id + generated commit message) for that exact set. +pub struct FoundSummaries { + pub map: SemanticChangeMap, + pub snapshot_id: Option, + pub generated_commit_message: Option, } -impl From for FoundSetForCurrent { - fn from(cs: SummarizedChangeSet) -> Self { - Self { - change_set: Some(cs.change_set), - changes: cs.changes, - missed_hashes: cs.missed_hashes, - } +impl FoundSummaries { + /// True when a non-empty commit message is cached for this exact change set. + pub fn has_generated_message(&self) -> bool { + self.generated_commit_message + .as_deref() + .is_some_and(|m| !m.trim().is_empty()) } } -/// Looks up changes for hashes against a known base commit -pub fn by_base_with_hashes( - pool: &DbPool, - base_commit_id: i64, - hashes: &[String], -) -> Result { - let mut conn = pool.get()?; - match crate::db::changesets::query_change_set_for_base_with_hashes( - &mut conn, - base_commit_id, - hashes, - )? { - Some(cs) => Ok(FoundSetForCurrent::from(cs)), - None => Ok(FoundSetForCurrent { - change_set: None, - changes: vec![], - missed_hashes: hashes.to_vec(), - }), +/// Reconstruct summaries for `changes` directly from content-addressed storage. +pub fn for_changes(pool: &DbPool, changes: &[Change]) -> Result { + let hashes: Vec = changes.iter().map(|c| c.hash.clone()).collect(); + let change_by_hash: HashMap<&str, &Change> = + changes.iter().map(|c| (c.hash.as_str(), c)).collect(); + + // 1. Groups whose full membership is live, largest first, non-overlapping. + let mut group_rows = crate::db::summaries::groups_within(pool, &hashes)?; + group_rows.retain(|g| is_valid_status(&g.status)); + group_rows.sort_by(|a, b| b.members.len().cmp(&a.members.len())); + + let mut covered: HashSet = HashSet::new(); + let mut groups: Vec = vec![]; + for g in group_rows { + if g.members.iter().any(|h| covered.contains(h)) { + continue; + } + let mut member_changes = Vec::with_capacity(g.members.len()); + for h in &g.members { + if let Some(change) = change_by_hash.get(h.as_str()) { + member_changes.push(to_change_with_summary(change, &g.title, &g.description)); + covered.insert(h.clone()); + } + } + if member_changes.is_empty() { + continue; + } + groups.push(SemanticChangeGroup { + summary: ChangeSummary { + id: g.id, + title: g.title, + description: g.description, + status: g.status, + created_at: g.created_at, + }, + changes: member_changes, + }); } + + // 2. Uncovered changes fall back to per-change summaries. + let uncovered: Vec = hashes + .iter() + .filter(|h| !covered.contains(*h)) + .cloned() + .collect(); + let patches = crate::db::summaries::patches_for(pool, &uncovered)?; + + let mut singles: Vec = vec![]; + let mut unsummarized_hashes: Vec = vec![]; + for hash in &uncovered { + match patches.get(hash) { + Some(patch) if is_valid_status(&patch.status) => { + if let Some(change) = change_by_hash.get(hash.as_str()) { + singles.push(to_change_with_summary( + change, + &patch.title, + &patch.description, + )); + } + } + // 3. No usable summary → unsummarized. + _ => unsummarized_hashes.push(hash.clone()), + } + } + + // 4. Cached snapshot (commit message) for the exact live set. + let snapshot = crate::db::snapshots::get_by_key(pool, &crate::db::keys::snapshot_key(&hashes))?; + let (snapshot_id, generated_commit_message) = match snapshot { + Some(s) => (Some(s.id), s.generated_commit_message), + None => (None, None), + }; + + let map = SemanticChangeMap { + groups, + singles, + unsummarized_hashes, + }; + dbg::group_log_result(&map); + + Ok(FoundSummaries { + map, + snapshot_id, + generated_commit_message, + }) } -/// Retuns existing summaries for head and missed hashes for unsummarized -pub fn for_current_state(pool: &DbPool, dir: &str) -> Result> { +/// Reconstruct the change map for the working tree's current changes. +pub fn for_current_state(pool: &DbPool, dir: &str) -> Result { let status = crate::git::status(dir)?; + Ok(for_changes(pool, &status.changes)?.map) +} - let Some(head_hash) = status.head_commit_hash.as_deref() else { - return Ok(vec![]); - }; +fn is_valid_status(status: &str) -> bool { + !matches!(status, "FAILED" | "CANCELLED" | "QUEUED") +} - let diff_hashes: Vec = status.changes.iter().map(|c| c.hash.clone()).collect(); +fn to_change_with_summary(change: &Change, title: &str, description: &str) -> ChangeWithSummary { + ChangeWithSummary { + id: change.id, + hash: change.hash.clone(), + filename: change.filename.clone(), + diff: change.diff.clone(), + line_count: change.line_count, + created_at: change.created_at, + own_summary_id: None, + title: title.to_string(), + description: description.to_string(), + } +} - let Some(commit) = crate::db::commits::get_commit_by_hash(pool, head_hash)? else { - // DB has no record for this commit — surface all hashes as missed - return Ok(vec![FoundSetForCurrent { - change_set: None, - changes: vec![], - missed_hashes: diff_hashes, - }]); - }; +#[cfg(test)] +mod tests { + use super::*; + use crate::git::file_diff_to_change; + use crate::sqlite_types::Change; + + fn change(filename: &str, diff: &str) -> Change { + file_diff_to_change( + crate::git::FileDiff { + old_path: None, + new_path: Some(filename.to_string()), + diff: diff.to_string(), + line_count: 1, + }, + 0, + false, + ) + } + + #[tokio::test] + async fn group_covers_members_and_singles_fill_the_rest() { + let temp_dir = tempfile::tempdir().unwrap(); + let db_path = temp_dir.path().join("nixmac.db"); + let pool = crate::db::init_pool_at_path(&db_path).await.unwrap(); + + let a = change("a.nix", "+a"); + let b = change("b.nix", "+b"); + let c = change("c.nix", "+c"); - dbg::find_log_path(&dbg::FindPath { - head_hash, - commit_id: commit.id, - hashes: &diff_hashes, - }); - - let mut conn = pool.get()?; - let result: Vec = - match crate::db::changesets::query_change_set_for_base_with_hashes( - &mut conn, - commit.id, - &diff_hashes, - )? { - Some(cs) => vec![FoundSetForCurrent::from(cs)], - None => vec![FoundSetForCurrent { - change_set: None, - changes: vec![], - missed_hashes: diff_hashes, - }], - }; - - dbg::find_log_result(result.iter().map(|e| { - ( - e.change_set.is_some(), - e.changes.len(), - e.missed_hashes.len(), + crate::db::summaries::store_group( + &pool, + &[a.hash.clone(), b.hash.clone()], + "feat: a and b", + "feat: a and b", + "DONE", + 0, ) - })); - Ok(result) + .unwrap(); + crate::db::summaries::store_patch(&pool, &c.hash, "fix: c", "fix: c", "DONE", 0).unwrap(); + + let found = for_changes(&pool, &[a.clone(), b.clone(), c.clone()]).unwrap(); + assert_eq!(found.map.groups.len(), 1); + assert_eq!(found.map.groups[0].changes.len(), 2); + assert_eq!(found.map.singles.len(), 1); + assert_eq!(found.map.singles[0].hash, c.hash); + assert!(found.map.unsummarized_hashes.is_empty()); + } + + #[tokio::test] + async fn unsummarized_when_no_summary_exists() { + let temp_dir = tempfile::tempdir().unwrap(); + let db_path = temp_dir.path().join("nixmac.db"); + let pool = crate::db::init_pool_at_path(&db_path).await.unwrap(); + + let a = change("a.nix", "+a"); + let found = for_changes(&pool, &[a.clone()]).unwrap(); + assert_eq!(found.map.unsummarized_hashes, vec![a.hash]); + assert!(found.map.groups.is_empty()); + assert!(found.map.singles.is_empty()); + } + + #[tokio::test] + async fn larger_group_is_preferred_over_overlapping_smaller_group() { + let temp_dir = tempfile::tempdir().unwrap(); + let db_path = temp_dir.path().join("nixmac.db"); + let pool = crate::db::init_pool_at_path(&db_path).await.unwrap(); + + let a = change("a.nix", "+a"); + let b = change("b.nix", "+b"); + + crate::db::summaries::store_group( + &pool, + &[a.hash.clone(), b.hash.clone()], + "big", + "big", + "DONE", + 0, + ) + .unwrap(); + crate::db::summaries::store_group(&pool, &[a.hash.clone()], "small", "small", "DONE", 0) + .unwrap(); + + let found = for_changes(&pool, &[a.clone(), b.clone()]).unwrap(); + assert_eq!(found.map.groups.len(), 1); + assert_eq!(found.map.groups[0].summary.title, "big"); + assert_eq!(found.map.groups[0].changes.len(), 2); + } } diff --git a/apps/native/src-tauri/src/summarize/group_existing.rs b/apps/native/src-tauri/src/summarize/group_existing.rs deleted file mode 100644 index 3facc0568..000000000 --- a/apps/native/src-tauri/src/summarize/group_existing.rs +++ /dev/null @@ -1,211 +0,0 @@ -//! Orchestrates grouping of summarized changes into a SemanticChangeMap. - -use std::collections::HashMap; - -use crate::shared_types::{ChangeWithSummary, SemanticChangeGroup, SemanticChangeMap}; -use crate::sqlite_types::ChangeSummary; -use crate::summarize::find_existing::FoundSetForCurrent; -use crate::summarize::sumlog as dbg; - -pub fn from_change_sets(change_sets: Vec) -> SemanticChangeMap { - let mut groups: HashMap)> = HashMap::new(); - let mut singles: Vec = vec![]; - let mut unsummarized_hashes: Vec = vec![]; - // true = placed in a group, false = placed in singles - let mut seen: HashMap = HashMap::new(); - - for cs in change_sets { - unsummarized_hashes.extend(cs.missed_hashes); - for sc in cs.changes { - let change_id = sc.change.id; - if sc.own_summary.as_ref().is_none_or(is_invalid) { - if let std::collections::hash_map::Entry::Vacant(e) = seen.entry(change_id) { - unsummarized_hashes.push(sc.change.hash.clone()); - e.insert(false); - } - continue; - } - match seen.get(&change_id).copied() { - Some(true) => continue, // already in a group, nothing better to do - Some(false) => { - // check for valid group before deduping change currently single - if let Some(gs) = sc.group_summary - && !is_invalid(&gs) - { - singles.retain(|c| c.id != change_id); - let cws = to_change_with_summary(&sc.change, sc.own_summary.as_ref()); - groups - .entry(gs.id) - .or_insert_with(|| (gs, vec![])) - .1 - .push(cws); - seen.insert(change_id, true); - } - } - None => { - let cws = to_change_with_summary(&sc.change, sc.own_summary.as_ref()); - match sc.group_summary { - Some(gs) if !is_invalid(&gs) => { - groups - .entry(gs.id) - .or_insert_with(|| (gs, vec![])) - .1 - .push(cws); - seen.insert(change_id, true); - } - _ => { - singles.push(cws); - seen.insert(change_id, false); - } - } - } - } - } - } - - let map = SemanticChangeMap { - groups: groups - .into_values() - .map(|(summary, changes)| SemanticChangeGroup { summary, changes }) - .collect(), - singles, - unsummarized_hashes, - }; - dbg::group_log_result(&map); - map -} - -// ── Lookup helpers ───────────────────────────────────────────────────────────── - -pub fn hash_matches(stored: &str, query: &str) -> bool { - if query.len() < stored.len() { - stored.starts_with(query) - } else { - stored == query - } -} - -#[allow(dead_code)] -pub fn find_group_by_id(map: &SemanticChangeMap, id: i64) -> Option<&SemanticChangeGroup> { - map.groups.iter().find(|g| g.summary.id == id) -} - -#[allow(dead_code)] -pub fn find_in_group_by_hash<'a>( - map: &'a SemanticChangeMap, - hash: &str, -) -> Option<&'a SemanticChangeGroup> { - map.groups - .iter() - .find(|g| g.changes.iter().any(|c| hash_matches(&c.hash, hash))) -} - -#[allow(dead_code)] -pub fn find_in_singles_by_hash<'a>( - map: &'a SemanticChangeMap, - hash: &str, -) -> Option<&'a ChangeWithSummary> { - map.singles.iter().find(|c| hash_matches(&c.hash, hash)) -} - -fn is_invalid(summary: &ChangeSummary) -> bool { - matches!(summary.status.as_str(), "FAILED" | "CANCELLED" | "QUEUED") -} - -fn to_change_with_summary( - change: &crate::sqlite_types::Change, - own_summary: Option<&ChangeSummary>, -) -> ChangeWithSummary { - let (title, description) = own_summary - .map(|s| (s.title.clone(), s.description.clone())) - .unwrap_or_default(); - ChangeWithSummary { - id: change.id, - hash: change.hash.clone(), - filename: change.filename.clone(), - diff: change.diff.clone(), - line_count: change.line_count, - created_at: change.created_at, - own_summary_id: change.own_summary_id, - title, - description, - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::shared_types::SummarizedChange; - use crate::sqlite_types::{Change, ChangeSummary}; - use crate::summarize::find_existing::FoundSetForCurrent; - - fn make_change(hash: &str) -> Change { - Change { - id: 1, - hash: hash.to_string(), - filename: "file.nix".to_string(), - diff: "diff".to_string(), - line_count: 1, - created_at: 0, - own_summary_id: Some(1), - } - } - - fn make_summary(status: &str) -> ChangeSummary { - ChangeSummary { - id: 1, - title: "t".to_string(), - description: "d".to_string(), - status: status.to_string(), - created_at: 0, - } - } - - fn found(change: Change, own_summary: Option) -> FoundSetForCurrent { - found_grouped(change, own_summary, None) - } - - fn found_grouped( - change: Change, - own_summary: Option, - group_summary: Option, - ) -> FoundSetForCurrent { - FoundSetForCurrent { - change_set: None, - changes: vec![SummarizedChange { - change, - own_summary, - group_summary, - }], - missed_hashes: vec![], - } - } - - #[test] - fn failed_own_summary_is_treated_as_unsummarized() { - let change = make_change("abc123"); - let map = from_change_sets(vec![found(change, Some(make_summary("FAILED")))]); - assert_eq!(map.unsummarized_hashes, vec!["abc123"]); - assert!(map.singles.is_empty()); - } - - #[test] - fn failed_group_summary_falls_through_to_singles() { - let change = make_change("ghi789"); - let own = make_summary("DONE"); - let group = make_summary("FAILED"); - let map = from_change_sets(vec![found_grouped(change, Some(own), Some(group))]); - assert!(map.unsummarized_hashes.is_empty()); - assert!(map.groups.is_empty()); - assert_eq!(map.singles.len(), 1); - assert_eq!(map.singles[0].hash, "ghi789"); - } - - #[test] - fn queued_own_summary_is_treated_as_unsummarized() { - let change = make_change("def456"); - let map = from_change_sets(vec![found(change, Some(make_summary("QUEUED")))]); - assert_eq!(map.unsummarized_hashes, vec!["def456"]); - assert!(map.singles.is_empty()); - } -} diff --git a/apps/native/src-tauri/src/summarize/mod.rs b/apps/native/src-tauri/src/summarize/mod.rs index 5645f319e..6605f3969 100644 --- a/apps/native/src-tauri/src/summarize/mod.rs +++ b/apps/native/src-tauri/src/summarize/mod.rs @@ -2,7 +2,6 @@ pub mod build_prompt; pub mod find_existing; -pub mod group_existing; pub mod model_calls; pub mod pipelines; pub mod sumlog; @@ -15,8 +14,6 @@ use tauri::{AppHandle, Manager, Runtime}; struct SummaryScope { changes: Vec, - base_commit_id: i64, - hashes: Vec, } /// Wrapper function to summarize changes since HEAD. @@ -39,40 +36,13 @@ pub async fn summarize_since( return Ok(None); }; - let existing = vec![find_existing::by_base_with_hashes( - &pool, - scope.base_commit_id, - &scope.hashes, - )?]; - let existing_id = existing - .iter() - .filter_map(|e| e.change_set.as_ref().map(|cs| cs.id)) - .next(); - - let has_generated_message = existing.iter().any(|entry| { - entry - .change_set - .as_ref() - .and_then(|cs| cs.generated_commit_message.as_deref()) - .is_some_and(|message| !message.trim().is_empty()) - }); - - let semantic_map = group_existing::from_change_sets(existing); - - if semantic_map.unsummarized_hashes.is_empty() && has_generated_message { - return Ok(existing_id); + let found = find_existing::for_changes(&pool, &scope.changes)?; + + if found.map.unsummarized_hashes.is_empty() && found.has_generated_message() { + return Ok(found.snapshot_id); } - pipelines::whole_diff::analyze( - scope.changes, - app, - None, - Some(scope.base_commit_id), - Some(base_ref), - None, - evolution_id, - ) - .await + pipelines::whole_diff::analyze(scope.changes, app, Some(base_ref), evolution_id).await } /// Recompute the change map for the active summary base ref and record it in @@ -93,22 +63,23 @@ pub fn change_map_since( app: &AppHandle, base_ref: &str, ) -> Result { - Ok(group_existing::from_change_sets(found_change_sets_since( - app, base_ref, - )?)) + Ok(found_since(app, base_ref)? + .map(|found| found.map) + .unwrap_or_default()) } -/// Returns all changesets found since `base_ref`, without generating anything new. -pub fn found_change_sets_since( +/// Reconstructs existing summaries (map + cached snapshot) for the changes since +/// `base_ref`, without generating anything new. Returns `None` when there is no +/// summarizable scope (missing ref / no changes). +pub fn found_since( app: &AppHandle, base_ref: &str, -) -> Result> { +) -> Result> { let pool = app.state::(); let Some(scope) = load_summary_scope(app, base_ref)? else { - return Ok(vec![]); + return Ok(None); }; - let found = find_existing::by_base_with_hashes(&pool, scope.base_commit_id, &scope.hashes)?; - Ok(vec![found]) + Ok(Some(find_existing::for_changes(&pool, &scope.changes)?)) } /// Gets the base commit for the current summary or HEAD if no summary exists, so the frontend can use it as a reference point for showing file diffs, etc. @@ -137,32 +108,13 @@ fn existing_summary_base_ref( .map(str::to_string) } -/// Gets the commit for `base_ref` and stores it in the DB if not already present, returning its ID. -fn store_base_ref_commit( - pool: &crate::db::DbPool, - config_dir: &str, - base_ref: &str, -) -> Result> { - let Some(hash) = crate::git::get_ref_sha(config_dir, base_ref) else { - return Ok(None); - }; - let tree_ref = format!("{base_ref}^{{tree}}"); - let Some(tree_hash) = crate::git::get_ref_sha(config_dir, &tree_ref) else { - return Ok(None); - }; - let now = crate::utils::unix_now(); - Ok(Some(crate::db::commits::upsert_commit( - pool, &hash, &tree_hash, None, now, - )?)) -} - -/// Helper method to get the changed files and base commit for use in summarization, returning None if the base_ref doesn't exist or there are no changes. +/// Helper method to get the changed files for use in summarization, returning +/// None if the base_ref doesn't exist or there are no changes. fn load_summary_scope( app: &AppHandle, base_ref: &str, ) -> Result> { let config_dir = crate::storage::store::get_config_dir(app)?; - let pool = app.state::(); if base_ref == "HEAD" && !crate::git::query::has_head_commit(&config_dir) { return Ok(None); @@ -177,16 +129,7 @@ fn load_summary_scope( return Ok(None); } - let Some(base_commit_id) = store_base_ref_commit(&pool, &config_dir, base_ref)? else { - return Ok(None); - }; - let hashes = changes.iter().map(|change| change.hash.clone()).collect(); - - Ok(Some(SummaryScope { - changes, - base_commit_id, - hashes, - })) + Ok(Some(SummaryScope { changes })) } fn changes_since_ref(config_dir: &str, base_ref: &str) -> Result> { diff --git a/apps/native/src-tauri/src/summarize/model_calls.rs b/apps/native/src-tauri/src/summarize/model_calls.rs index e9e95f066..7b8d211b9 100644 --- a/apps/native/src-tauri/src/summarize/model_calls.rs +++ b/apps/native/src-tauri/src/summarize/model_calls.rs @@ -9,22 +9,35 @@ use tauri::{AppHandle, Runtime}; use crate::ai::providers::{TokenUsage, create_provider}; use crate::summarize::token_budgets::changeset_summaries_budget; -/// One item in the model's multi-summary response: a conventional commit -/// message plus the file paths it covers. +/// One item in the model's multi-summary response: a free-form semantic +/// description plus the hashes of the individual changes it covers. #[derive(Debug, Clone, Deserialize)] pub struct ChangesetSummaryItem { pub summary: String, - pub files: Vec, + pub changes: Vec, } -/// The prompt asks for an array, but providers request -/// `response_format: json_object`, which steers models toward a top-level -/// object — especially when there is only one group. Accept both shapes. +/// The canonical response is a `{"groups": [...]}` object because providers +/// request `response_format: json_object`. Continue accepting the bare array, +/// a single item, and the `{"changes": [...]}` envelope already returned by +/// some providers for backwards compatibility. #[derive(Debug, Deserialize)] #[serde(untagged)] enum ChangesetSummariesResponse { Many(Vec), One(ChangesetSummaryItem), + Envelope(ChangesetSummariesEnvelope), +} + +#[derive(Debug, Deserialize)] +struct ChangesetSummariesEnvelope { + #[serde( + rename = "groups", + alias = "changes", + alias = "summaries", + alias = "items" + )] + items: Vec, } impl From for Vec { @@ -32,6 +45,7 @@ impl From for Vec { match response { ChangesetSummariesResponse::Many(items) => items, ChangesetSummariesResponse::One(item) => vec![item], + ChangesetSummariesResponse::Envelope(envelope) => envelope.items, } } } @@ -212,10 +226,10 @@ fn extract_json_object(raw: &str) -> &str { } } -/// Calls the model and parses a `[{ summary, files }]` array response. +/// Calls the model and parses its grouped changes response. /// /// Returns one or more summary items. The caller is responsible for matching -/// `files` back to change rows; any change the model did not assign to a group +/// `changes` back to change rows; any change the model did not assign to a group /// is left for the caller to handle (e.g. as singles). pub async fn generate_changeset_summaries( system_prompt: &str, @@ -287,8 +301,8 @@ mod tests { #[test] fn extract_plain_array() { assert_eq!( - extract_json_object(r#"[{"summary":"a","files":["f.nix"]}]"#), - r#"[{"summary":"a","files":["f.nix"]}]"# + extract_json_object(r#"[{"summary":"a","changes":["hash-a"]}]"#), + r#"[{"summary":"a","changes":["hash-a"]}]"# ); } @@ -300,8 +314,11 @@ mod tests { #[test] fn extract_strips_array_fence() { - let raw = "```json\n[{\"summary\":\"a\",\"files\":[]}]\n```"; - assert_eq!(extract_json_object(raw), r#"[{"summary":"a","files":[]}]"#); + let raw = "```json\n[{\"summary\":\"a\",\"changes\":[]}]\n```"; + assert_eq!( + extract_json_object(raw), + r#"[{"summary":"a","changes":[]}]"# + ); } #[test] @@ -318,8 +335,11 @@ mod tests { #[test] fn extract_narrows_to_balanced_array() { - let raw = "Sure, here you go:\n[{\"summary\":\"a\",\"files\":[]}]\nLet me know."; - assert_eq!(extract_json_object(raw), r#"[{"summary":"a","files":[]}]"#); + let raw = "Sure, here you go:\n[{\"summary\":\"a\",\"changes\":[]}]\nLet me know."; + assert_eq!( + extract_json_object(raw), + r#"[{"summary":"a","changes":[]}]"# + ); } #[test] @@ -333,7 +353,7 @@ mod tests { #[test] fn summaries_response_parses_array() { let parsed: ChangesetSummariesResponse = - serde_json::from_str(r#"[{"summary":"a","files":["f.nix"]}]"#).unwrap(); + serde_json::from_str(r#"[{"summary":"a","changes":["hash-a"]}]"#).unwrap(); let items = Vec::from(parsed); assert_eq!(items.len(), 1); assert_eq!(items[0].summary, "a"); @@ -342,12 +362,24 @@ mod tests { #[test] fn summaries_response_parses_single_object() { let parsed: ChangesetSummariesResponse = serde_json::from_str( - r#"{"summary":"chore(home): rename pi4 host","files":["alex-laptop/home.nix"]}"#, + r#"{"summary":"chore(home): rename pi4 host","changes":["hash-a"]}"#, ) .unwrap(); let items = Vec::from(parsed); assert_eq!(items.len(), 1); - assert_eq!(items[0].files, vec!["alex-laptop/home.nix"]); + assert_eq!(items[0].changes, vec!["hash-a"]); + } + + #[test] + fn summaries_response_parses_object_wrapped_change_list() { + let parsed: ChangesetSummariesResponse = serde_json::from_str( + r#"{"changes":[{"summary":"fix(python): apply compatibility patch","changes":["hash-a","hash-b"]},{"summary":"feat(bb-hook): unload agents","changes":["hash-c"]}]}"#, + ) + .unwrap(); + let items = Vec::from(parsed); + assert_eq!(items.len(), 2); + assert_eq!(items[0].changes, vec!["hash-a", "hash-b"]); + assert_eq!(items[1].changes, vec!["hash-c"]); } #[test] diff --git a/apps/native/src-tauri/src/summarize/pipelines/commit_message.rs b/apps/native/src-tauri/src/summarize/pipelines/commit_message.rs index 4ba82af86..0cf1cc2d9 100644 --- a/apps/native/src-tauri/src/summarize/pipelines/commit_message.rs +++ b/apps/native/src-tauri/src/summarize/pipelines/commit_message.rs @@ -6,35 +6,22 @@ use tauri::{AppHandle, Runtime}; pub async fn generate(app: &AppHandle) -> Result { let base_ref = crate::summarize::active_summary_base_ref(app); - let change_sets = crate::summarize::found_change_sets_since(app, &base_ref)?; - let existing = change_sets.iter().find_map(|entry| { - entry - .change_set - .as_ref() - .and_then(|cs| cs.generated_commit_message.as_deref()) - .filter(|message| !message.trim().is_empty()) - }); + let existing = crate::summarize::found_since(app, &base_ref)? + .and_then(|found| found.generated_commit_message) + .filter(|message| !message.trim().is_empty()); if let Some(message) = existing { - return Ok(message.to_string()); + return Ok(message); } - // No stored message (e.g. summarizeCurrent never ran, or the previous - // model call failed). summarize_since will create or refresh the changeset + // No cached message (e.g. summarizeCurrent never ran, or the previous + // model call failed). summarize_since will create or refresh the snapshot // and retry the commit-message generation. crate::summarize::summarize_since(app, &base_ref, None).await?; - let change_sets = crate::summarize::found_change_sets_since(app, &base_ref)?; - change_sets - .iter() - .find_map(|entry| { - entry - .change_set - .as_ref() - .and_then(|cs| cs.generated_commit_message.as_deref()) - .filter(|message| !message.trim().is_empty()) - }) - .map(str::to_string) + crate::summarize::found_since(app, &base_ref)? + .and_then(|found| found.generated_commit_message) + .filter(|message| !message.trim().is_empty()) .ok_or_else(|| anyhow::anyhow!("no generated commit message found")) } diff --git a/apps/native/src-tauri/src/summarize/pipelines/history.rs b/apps/native/src-tauri/src/summarize/pipelines/history.rs index c54784df7..91648d372 100644 --- a/apps/native/src-tauri/src/summarize/pipelines/history.rs +++ b/apps/native/src-tauri/src/summarize/pipelines/history.rs @@ -26,23 +26,8 @@ pub async fn from_commit_times_number( return Ok(()); } - let mut db_ids: Vec = Vec::with_capacity(commits.len()); - for commit in &commits { - let id = crate::db::commits::upsert_commit( - &pool, - &commit.hash, - &commit.tree_hash, - commit.message.as_deref(), - commit.created_at, - )?; - db_ids.push(id); - } - let limit = commits.len().saturating_sub(1).min(number); for i in 0..limit { - let commit_id = db_ids[i]; - let base_commit_id = db_ids[i + 1]; - let file_diffs = crate::git::query::commit_diff(&config_dir, &commits[i + 1].hash, &commits[i].hash)?; @@ -57,19 +42,16 @@ pub async fn from_commit_times_number( continue; } - let diff_hashes: Vec = all_changes.iter().map(|c| c.hash.clone()).collect(); - let found = crate::summarize::find_existing::by_base_with_hashes( - &pool, - base_commit_id, - &diff_hashes, - )?; - let semantic_map = crate::summarize::group_existing::from_change_sets(vec![found]); + // Summaries are content-addressed by change hash, so a commit's summary + // is reconstructed purely from its changes — no commit rows needed. + let found = crate::summarize::find_existing::for_changes(&pool, &all_changes)?; - if semantic_map.unsummarized_hashes.is_empty() { + if found.map.unsummarized_hashes.is_empty() { continue; } - let unsummarized_set: std::collections::HashSet<&str> = semantic_map + let unsummarized_set: std::collections::HashSet<&str> = found + .map .unsummarized_hashes .iter() .map(String::as_str) @@ -83,17 +65,7 @@ pub async fn from_commit_times_number( continue; } - if let Err(e) = super::whole_diff::analyze( - changes_to_summarize, - app, - Some(commit_id), - Some(base_commit_id), - None, - commits[i].message.as_deref(), - None, - ) - .await - { + if let Err(e) = super::whole_diff::analyze(changes_to_summarize, app, None, None).await { log::error!("[history] pipeline failed for {}: {}", commits[i].hash, e); } } diff --git a/apps/native/src-tauri/src/summarize/pipelines/whole_diff.rs b/apps/native/src-tauri/src/summarize/pipelines/whole_diff.rs index fe4d26617..bb497c7d1 100644 --- a/apps/native/src-tauri/src/summarize/pipelines/whole_diff.rs +++ b/apps/native/src-tauri/src/summarize/pipelines/whole_diff.rs @@ -1,5 +1,6 @@ //! Whole-diff pipeline — one model call on the full diff, producing one or -//! more group summaries (each covering a subset of the changed files). +//! more semantic group descriptions (each covering a subset of the individual +//! changes). use std::collections::HashMap; @@ -11,30 +12,35 @@ use crate::sqlite_types::Change; use crate::summarize::model_calls::ChangesetSummaryItem; use crate::summarize::{build_prompt, sumlog as dbg}; +/// A summary and the hashes of the live changes it covers. +struct SummaryAssignment { + summary: String, + hashes: Vec, +} + const WHOLE_DIFF_SYSTEM_PROMPT: &str = r#" -You are a git commit message generator. +You are a git change reviewer. Rules: -- Group the provided changes into one or more conventional commit messages. +- Group the provided changes into one or more semantic changes. - Each group must share a coherent purpose (a single logical change). +- Give each group a concise, factual, plain-language summary. - Base every summary only on the provided changes. - Do not invent intent that is not visible in the diff. -- If the type is unclear, prefer "chore". -- Every changed file must appear in exactly one group. +- Do not assign a conventional-commit type, scope, or prefix. +- Every supplied change hash must appear in exactly one group. - Prefer fewer groups; only split when changes are clearly unrelated. - Always return valid JSON in this format: -[{"summary":"","files":["", ...]}, ...] +{"groups":[{"summary":"","changes":["", ...]}, ...]} "#; -#[allow(clippy::too_many_arguments)] +/// Run the whole-diff model call for `changes`, persist the resulting group / +/// single summaries plus a cached snapshot, and return the snapshot id. pub async fn analyze( changes: Vec, app: &AppHandle, - commit_id: Option, - base_commit_id: Option, base_ref: Option<&str>, - commit_message: Option<&str>, evolution_id: Option, ) -> Result> { dbg::new_log_changes(&changes); @@ -43,78 +49,165 @@ pub async fn analyze( return Ok(None); } - let config_dir = crate::storage::store::get_config_dir(app)?; let pool = app.state::(); - let Some(base_commit_id) = - crate::db::commits::store_head_commit(&pool, &config_dir, base_commit_id)? - else { - return Ok(None); - }; let refs: Vec<&Change> = changes.iter().collect(); let user_prompt = build_prompt::whole_diff(&refs); dbg::new_log_prompt(&user_prompt); - let (items, _usage) = crate::summarize::model_calls::generate_changeset_summaries( + let (mut items, _usage) = crate::summarize::model_calls::generate_changeset_summaries( WHOLE_DIFF_SYSTEM_PROMPT, &user_prompt, Some(app), ) .await?; - let groups = partition_changes(changes, &items); + // Keep the full-diff call focused on semantic grouping. Conventional type + // selection happens only after it returns, using each short summary rather + // than the entire diff that prompted the model's analysis. + for item in &mut items { + item.summary = conventionalize_summary(&item.summary); + } - // The model may not reference every file. Build a single display string - // from all returned summaries so `generated_commit_message` (consumed by - // the commit-message pipeline) reflects the full changeset even when the - // model split it into several groups. + // The model may not reference every change. Build a single display string + // from all returned summaries so the snapshot's `generated_commit_message` + // (consumed by the commit-message pipeline) reflects the full changeset even + // when the model split it into several groups. let generated_message = items .iter() .map(|item| item.summary.as_str()) .collect::>() .join("\n\n"); - let change_set_id = crate::db::store_whole_diff_changeset::store( + let assignments = assign_summaries(&changes, &items); + let now = crate::utils::unix_now(); + + // Persist each assignment: multi-member sets become first-class groups, + // single-member sets become per-change (patch) summaries. + for assignment in &assignments { + let description = assignment.summary.trim(); + let title = description.lines().next().unwrap_or(description).trim(); + if assignment.hashes.len() >= 2 { + crate::db::summaries::store_group( + &pool, + &assignment.hashes, + title, + description, + "DONE", + now, + )?; + } else if let Some(hash) = assignment.hashes.first() { + crate::db::summaries::store_patch(&pool, hash, title, description, "DONE", now)?; + } + } + + // Cache the generated commit message for this exact set of change hashes. + let hashes: Vec = changes.iter().map(|c| c.hash.clone()).collect(); + let snapshot_id = crate::db::snapshots::upsert( &pool, - &groups, - &generated_message, - commit_id, - base_commit_id, - commit_message, + &crate::db::keys::snapshot_key(&hashes), + Some(&generated_message), evolution_id, + now, )?; emit_update(app, &pool, base_ref)?; - Ok(Some(change_set_id)) + Ok(Some(snapshot_id)) +} + +const CONVENTIONAL_TYPES: [&str; 8] = [ + "feat", "fix", "chore", "refactor", "docs", "style", "test", "perf", +]; + +/// Adds a conventional-commit type using only the model's already-generated +/// summary. This deliberately does not inspect the diff: the model has already +/// done the semantic work and this final pass should remain cheap and stable. +fn conventionalize_summary(summary: &str) -> String { + let description = strip_conventional_prefix(summary); + let description = description.trim().trim_end_matches('.').trim(); + let description = if description.is_empty() { + summary.trim() + } else { + description + }; + + format!( + "{}: {}", + conventional_type_for_summary(description), + description + ) +} + +fn strip_conventional_prefix(summary: &str) -> &str { + let trimmed = summary.trim(); + let Some((prefix, description)) = trimmed.split_once(':') else { + return trimmed; + }; + let prefix = prefix.trim(); + let is_conventional_type = CONVENTIONAL_TYPES.iter().any(|kind| { + prefix == *kind + || prefix + .strip_prefix(kind) + .is_some_and(|scope| scope.starts_with('(') && scope.ends_with(')')) + }); + + if is_conventional_type { + description.trim() + } else { + trimmed + } } -/// A change assigned to a group summary. -pub struct GroupedChange { - pub change: Change, - pub summary: String, +fn conventional_type_for_summary(summary: &str) -> &'static str { + let summary = summary.to_ascii_lowercase(); + let contains_any = |keywords: &[&str]| keywords.iter().any(|keyword| summary.contains(keyword)); + + if contains_any(&[ + "fix", "repair", "resolve", "correct", "prevent", "restore", "compatib", "patch", + ]) { + "fix" + } else if contains_any(&["optimiz", "performance", "faster", "speed up"]) { + "perf" + } else if contains_any(&["test", "coverage", "fixture", "assertion"]) { + "test" + } else if contains_any(&["document", "documentation", "readme", "guide"]) { + "docs" + } else if contains_any(&["refactor", "restructur", "reorganiz", "simplif", "extract"]) { + "refactor" + } else if contains_any(&["format", "styling", "style "]) { + "style" + } else if contains_any(&[ + "add", + "enable", + "support", + "introduce", + "implement", + "create", + "allow", + ]) { + "feat" + } else { + "chore" + } } -/// Matches model-returned file paths to change rows and groups them. +/// Assigns each live change hash to the model summary that covers it, grouping +/// hashes by summary text. /// -/// Files are matched by repository-relative basename (the model frequently -/// returns short paths or basenames). Any change the model did not assign to -/// a group is collected into a fallback group using the first summary, so no -/// file is left unsummarized (which would otherwise trigger re-summarization -/// loops in `group_existing`). -fn partition_changes(changes: Vec, items: &[ChangesetSummaryItem]) -> Vec { - // Index model file paths by basename for tolerant matching. The model - // often emits `"foo.nix"` even when the stored path is `"dir/foo.nix"`. - let mut path_to_item: HashMap = HashMap::new(); +/// Hashes identify individual hunks, so independent changes in one file can +/// receive distinct summaries. Any change the model did not assign is folded +/// into the first summary's bucket, so no change is left unsummarized (which +/// would otherwise trigger re-summarization loops in `find_existing`). Buckets +/// that share summary text are merged, matching the content-addressed group +/// identity used when reading summaries back. +fn assign_summaries(changes: &[Change], items: &[ChangesetSummaryItem]) -> Vec { + let mut hash_to_item: HashMap<&str, usize> = HashMap::new(); for (i, item) in items.iter().enumerate() { - for file in &item.files { - path_to_item.insert(file.clone(), i); - if let Some(base) = std::path::Path::new(file) - .file_name() - .and_then(|n| n.to_str()) - { - path_to_item.insert(base.to_string(), i); - } + for hash in &item.changes { + // Keep the first assignment if a model repeats a hash. One hunk + // must have only one semantic owner. + hash_to_item.entry(hash.as_str()).or_insert(i); } } @@ -123,35 +216,29 @@ fn partition_changes(changes: Vec, items: &[ChangesetSummaryItem]) -> Ve .map(|i| i.summary.clone()) .unwrap_or_else(|| "chore: summarize changes".to_string()); - let mut assigned = vec![false; changes.len()]; - let mut groups: Vec = Vec::new(); + // Preserve first-seen order of summaries while merging by text. + let mut order: Vec = Vec::new(); + let mut by_summary: HashMap> = HashMap::new(); - for (idx, change) in changes.iter().enumerate() { - let matched = path_to_item.get(&change.filename).copied().or_else(|| { - std::path::Path::new(&change.filename) - .file_name() - .and_then(|n| n.to_str()) - .and_then(|base| path_to_item.get(base).copied()) + for change in changes { + let summary = match hash_to_item.get(change.hash.as_str()) { + Some(&i) => items[i].summary.clone(), + None => fallback_summary.clone(), + }; + let bucket = by_summary.entry(summary.clone()).or_insert_with(|| { + order.push(summary.clone()); + Vec::new() }); - if let Some(i) = matched { - assigned[idx] = true; - groups.push(GroupedChange { - change: change.clone(), - summary: items[i].summary.clone(), - }); - } - } - - for (idx, change) in changes.into_iter().enumerate() { - if !assigned[idx] { - groups.push(GroupedChange { - change, - summary: fallback_summary.clone(), - }); - } + bucket.push(change.hash.clone()); } - groups + order + .into_iter() + .map(|summary| { + let hashes = by_summary.remove(&summary).unwrap_or_default(); + SummaryAssignment { summary, hashes } + }) + .collect() } fn emit_update( @@ -163,8 +250,7 @@ fn emit_update( crate::summarize::change_map_since(app, base_ref)? } else { let config_dir = crate::storage::store::get_config_dir(app)?; - let change_sets = crate::summarize::find_existing::for_current_state(pool, &config_dir)?; - crate::summarize::group_existing::from_change_sets(change_sets) + crate::summarize::find_existing::for_current_state(pool, &config_dir)? }; // The cell write emits `change_map_changed`. crate::state::change_map::update(app, semantic_map); @@ -187,50 +273,103 @@ mod tests { } } + fn find<'a>(assignments: &'a [SummaryAssignment], summary: &str) -> &'a SummaryAssignment { + assignments + .iter() + .find(|a| a.summary == summary) + .expect("assignment for summary") + } + #[test] - fn matched_files_are_grouped_by_summary() { + fn matched_changes_are_grouped_by_summary() { let changes = vec![change("a.nix"), change("b.nix"), change("c.nix")]; let items = vec![ ChangesetSummaryItem { summary: "feat: a and b".into(), - files: vec!["a.nix".into(), "b.nix".into()], + changes: vec!["h-a.nix".into(), "h-b.nix".into()], }, ChangesetSummaryItem { summary: "fix: c".into(), - files: vec!["c.nix".into()], + changes: vec!["h-c.nix".into()], + }, + ]; + let assignments = assign_summaries(&changes, &items); + assert_eq!(assignments.len(), 2); + assert_eq!(find(&assignments, "feat: a and b").hashes.len(), 2); + assert_eq!(find(&assignments, "fix: c").hashes, vec!["h-c.nix"]); + } + + #[test] + fn same_file_changes_can_belong_to_distinct_semantic_groups() { + let mut first = change("configuration.nix"); + first.hash = "first-change".into(); + let mut second = change("configuration.nix"); + second.hash = "second-change".into(); + let items = vec![ + ChangesetSummaryItem { + summary: "feat: add a service".into(), + changes: vec!["first-change".into()], + }, + ChangesetSummaryItem { + summary: "fix: update a package".into(), + changes: vec!["second-change".into()], }, ]; - let groups = partition_changes(changes, &items); - assert_eq!(groups.len(), 3); - assert_eq!(groups[0].summary, "feat: a and b"); - assert_eq!(groups[1].summary, "feat: a and b"); - assert_eq!(groups[2].summary, "fix: c"); + + let assignments = assign_summaries(&[first, second], &items); + + assert_eq!(assignments.len(), 2); + assert_eq!(find(&assignments, "feat: add a service").hashes, vec!["first-change"]); + assert_eq!(find(&assignments, "fix: update a package").hashes, vec!["second-change"]); } #[test] - fn unmatched_files_fall_back_to_first_summary() { + fn unmatched_changes_fall_back_to_first_summary() { let changes = vec![change("a.nix"), change("orphan.nix")]; let items = vec![ChangesetSummaryItem { summary: "feat: a".into(), - files: vec!["a.nix".into()], + changes: vec!["h-a.nix".into()], }]; - let groups = partition_changes(changes, &items); - assert_eq!(groups.len(), 2); - assert_eq!(groups[0].summary, "feat: a"); - // Orphan keeps the fallback so it isn't flagged unsummarized. - assert_eq!(groups[1].summary, "feat: a"); - assert_eq!(groups[1].change.filename, "orphan.nix"); + let assignments = assign_summaries(&changes, &items); + // Orphan is folded into the first summary's bucket so it isn't unsummarized. + assert_eq!(assignments.len(), 1); + let bucket = find(&assignments, "feat: a"); + assert!(bucket.hashes.contains(&"h-a.nix".to_string())); + assert!(bucket.hashes.contains(&"h-orphan.nix".to_string())); } #[test] - fn basename_matching_resolves_nested_paths() { + fn complete_change_hashes_match_nested_paths() { let changes = vec![change("modules/darwin/dock.nix")]; let items = vec![ChangesetSummaryItem { summary: "feat: dock".into(), - files: vec!["dock.nix".into()], + changes: vec!["h-modules/darwin/dock.nix".into()], }]; - let groups = partition_changes(changes, &items); - assert_eq!(groups.len(), 1); - assert_eq!(groups[0].summary, "feat: dock"); + let assignments = assign_summaries(&changes, &items); + assert_eq!(assignments.len(), 1); + assert_eq!(find(&assignments, "feat: dock").hashes.len(), 1); + } + + #[test] + fn conventional_type_is_determined_from_the_completed_summary() { + assert_eq!( + conventionalize_summary("Apply daemon_pool compatibility patch."), + "fix: Apply daemon_pool compatibility patch" + ); + assert_eq!( + conventionalize_summary("Enable Prelude theme support"), + "feat: Enable Prelude theme support" + ); + assert_eq!( + conventionalize_summary("refactor(helix): reorganize config declarations"), + "refactor: reorganize config declarations" + ); + } + + #[test] + fn whole_diff_system_prompt_requests_only_free_form_descriptions() { + assert!(WHOLE_DIFF_SYSTEM_PROMPT.contains("plain-language summary")); + assert!(!WHOLE_DIFF_SYSTEM_PROMPT.contains("conventional commit messages")); + assert!(!WHOLE_DIFF_SYSTEM_PROMPT.contains("prefer \"chore\"")); } } diff --git a/apps/native/src-tauri/src/summarize/sumlog.rs b/apps/native/src-tauri/src/summarize/sumlog.rs index d48eb00fa..79ce0fee6 100644 --- a/apps/native/src-tauri/src/summarize/sumlog.rs +++ b/apps/native/src-tauri/src/summarize/sumlog.rs @@ -1,6 +1,5 @@ //! Centralized debug logging for summarize pipelines. -pub const FIND_EXISTING: bool = false; pub const GROUP_EXISTING: bool = false; pub const WHOLE_DIFF: bool = false; @@ -24,48 +23,6 @@ fn emit_text(pipeline: &str, step: &str, text: &str) { log::warn!("╚══ {} ══╝", label); } -pub struct FindPath<'a> { - pub head_hash: &'a str, - pub commit_id: i64, - pub hashes: &'a [String], -} - -pub fn find_log_path(path: &FindPath) { - if !FIND_EXISTING { - return; - } - emit_json( - "FIND_EXISTING", - "path", - &serde_json::json!({ - "head_hash": path.head_hash, - "commit_id": path.commit_id, - "hashes": path.hashes, - }), - ); -} - -pub fn find_log_result(entries: impl Iterator + Clone) { - if !FIND_EXISTING { - return; - } - let rows: Vec<_> = entries - .clone() - .map(|(has_cs, changes, missed)| { - serde_json::json!({ - "has_change_set": has_cs, - "changes": changes, - "missed_hashes": missed, - }) - }) - .collect(); - emit_json( - "FIND_EXISTING", - "result", - &serde_json::json!({ "count": rows.len(), "entries": rows }), - ); -} - pub fn group_log_result(map: &SemanticChangeMap) { if !GROUP_EXISTING { return; diff --git a/apps/native/src/ipc/sqlite.ts b/apps/native/src/ipc/sqlite.ts index 9610cac58..b8a0d2e17 100644 --- a/apps/native/src/ipc/sqlite.ts +++ b/apps/native/src/ipc/sqlite.ts @@ -4,12 +4,6 @@ export type Change = { id: number; hash: string; filename: string; diff: string; lineCount: number; createdAt: number; ownSummaryId: number } -/** - * Groups Changes for a commit→base_commit pair. `commit_id` is NULL for speculative - * (uncommitted) changesets. Membership is stored in the `set_changes` join table. - */ -export type ChangeSet = { id: number; commitId: number | null; baseCommitId: number; commitMessage: string | null; generatedCommitMessage: string | null; createdAt: number; evolutionId: number | null } - export type ChangeSummary = { id: number; title: string; description: string; /** * One of `"QUEUED"`, `"DONE"`, `"FAILED"`, `"CANCELLED"`. @@ -18,7 +12,5 @@ status: string; createdAt: number } export type Commit = { id: number; hash: string; treeHash: string; message: string | null; createdAt: number } -export type Evolution = { id: number; originBranch: string; merged: number; builds: number } - -export type Prompt = { id: number; text: string; commitId: number | null; createdAt: number } +export type Evolution = { id: number; originBranch: string } diff --git a/apps/native/src/ipc/types.ts b/apps/native/src/ipc/types.ts index 92f2d3025..18cd07d1b 100644 --- a/apps/native/src/ipc/types.ts +++ b/apps/native/src/ipc/types.ts @@ -131,12 +131,6 @@ output: string } export type Change = { id: number; hash: string; filename: string; diff: string; lineCount: number; createdAt: number; ownSummaryId: number } -/** - * Groups Changes for a commit→base_commit pair. `commit_id` is NULL for speculative - * (uncommitted) changesets. Membership is stored in the `set_changes` join table. - */ -export type ChangeSet = { id: number; commitId: number | null; baseCommitId: number; commitMessage: string | null; generatedCommitMessage: string | null; createdAt: number; evolutionId: number | null } - export type ChangeSummary = { id: number; title: string; description: string; /** * One of `"QUEUED"`, `"DONE"`, `"FAILED"`, `"CANCELLED"`. @@ -2069,34 +2063,6 @@ dir: string; */ changed: boolean } -export type SummarizedChange = { -/** - * Raw change row. - */ -change: Change; -/** - * Summary attached directly to this change. - */ -ownSummary: ChangeSummary | null; -/** - * Summary inherited from this change's group. - */ -groupSummary: ChangeSummary | null } - -export type SummarizedChangeSet = { -/** - * Change set represented by this response. - */ -changeSet: ChangeSet; -/** - * Changes in the set with their available summaries. - */ -changes: SummarizedChange[]; -/** - * Change hashes expected in the set but missing from the database. - */ -missedHashes: string[] } - /** * Remote sync state for the current account, returned by `sync_status`. */