refactor(db): remove redundant data, significantly simplify - #629
refactor(db): remove redundant data, significantly simplify#629cooper (czxtm) wants to merge 1 commit into
Conversation
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.
🎨 Storybook previewUpdated for fcf2f6c
|
|
| Option | What it gives users | What it costs | Effort to change later |
|---|---|---|---|
| Current (content key) | Summaries survive rewrites; cache always additive | Two different evolutions touching the same hunks share a summary, even if intent differed | Medium — would require re-keying every stored summary |
| Old (base commit id) | Each evolution is independently summarized | Summaries orphan after rebase; DB must mirror every commit; re-evolutions miss cache | Already done — would be a regression |
2. Two edits in the same file that fall in different git hunks are now treated as separate independently-summarizable units, rather than being grouped into one entry per file.
This is the right granularity in principle. A commit that enables dock auto-hide on line 3 and bumps a package version on line 200 of the same .nix file is genuinely two unrelated changes; giving them the same summary loses information. Per-hunk granularity lets the model (and the history view) surface this correctly.
The concern: the per-hunk split feeds directly into the decision that asks the model to reproduce hash strings (see next decision). If hunk-hashes are the reference mechanism, and models are unreliable at reproducing long hashes, splitting into more hunks increases the probability of grouping failures. The granularity decision is sound but its value depends on whether the referencing mechanism works. If short or sequential IDs are substituted in the prompt, the concern dissolves.
3. The AI model is no longer asked to assign a conventional commit type; instead, a keyword-matching function reads the model's plain-language description and adds the appropriate prefix (feat/fix/chore/etc.).
Separating semantic description (model's strength) from categorical classification (deterministic rules) is a reasonable division of labour. The model can focus on describing what changed without worrying about the fix(scope): format, and the keyword matcher applies consistent rules. In practice, conventional-commit type classifiers built on keyword rules are common (commitizen, semantic-release) and work acceptably for most changes.
The concern is the false-match surface. The word fix appearing in a description like "Remove the legacy fix for import ordering" or "Add a fix for X then revert it" will be classified as fix even though the intent is chore or revert. Similarly, patch maps to fix, which is reasonable for security patches but potentially wrong for configuration patches. For a developer-tooling product whose output becomes git commit history, commit message quality is visible and matters to users. The heuristic will be wrong on a meaningful fraction of summaries. It should be treated as a good default that power users can override, not a reliable classifier.
4. The AI model must reproduce exact 64-character SHA-256 hex strings to assign its summaries to specific changes; any hash it fails to reproduce exactly causes the affected change to fall into a silent fallback group using the first available summary.
This is the decision most likely to silently degrade user-facing quality. Large language models are unreliable at reproducing arbitrary long strings verbatim — they transpose characters, truncate, or paraphrase. The old approach asked the model to return file paths, which are human-readable strings it reliably produces, with a basename fuzzy-match fallback to handle dock.nix vs modules/darwin/dock.nix. The new approach removes all tolerance: if the model returns a1b2c3... with one character wrong, the change is unmatched.
The failure mode is silent and compounding. When a hash is unmatched, the change is folded into the first summary's bucket — it is counted as summarized with no unsummarized_hashes entry. The system does not retry; the user sees the history view with unrelated changes arbitrarily grouped, with no indication of the error. Per-hunk granularity amplifies this because there are now more hashes to reproduce correctly per file.
| Option | What users see | Reliability | Effort to change |
|---|---|---|---|
| Current (full 64-char hash) | Correct groupings when model succeeds; silent mis-groupings when it fails | Low — models make copying errors on long hex strings | Medium — requires prompt and parser changes |
| Sequential IDs (1, 2, 3...) mapped server-side to hashes | Same user-visible groupings, model references short integers | High — models reproduce small integers reliably | Low — mapping table at call site |
| Short hash (first 8 chars) | Same as sequential IDs | High — 8 chars is the git norm models have seen extensively | Low — truncate before prompt, expand after parse |
| Revert to file paths (old) | One summary per file | High — model produces paths it was given | Already done — regression |
The recommended fix before shipping: substitute short sequential IDs (1, 2, 3) or 8-char prefix hashes in the prompt; map back to full hashes on the server after parsing. This preserves the content-addressed grouping architecture while making the model's task tractable.
5. History entries no longer carry database-stored commit details — the `commit` field in every history entry is now always absent.
The commit field mirrored data (hash, tree hash, message) that git already tracks. Git is the canonical source for commit metadata; the database was maintaining a redundant copy. Removing the copy eliminates a sync burden and closes a class of bugs where the two sources diverge after rebases. Any frontend component that read commit identity or message from history should read it from the git-sourced fields that remain present. If any component was already reading from the git data rather than the DB field, there is no user-facing change at all.
6. The record of which text prompt was used in each evolution is no longer stored.
The prompts table tracked (text, commit_id, created_at). The struct was already annotated #[allow(dead_code)] and removed from the specta type generation, indicating it was not surfaced in the UI. Removing it eliminates a table with a foreign key to commits — which is itself being removed. No evidence that prompt history was exposed to users, so no user-facing functionality is lost.
7. Evolution tracking no longer records how many successful builds or merges an evolution produced.
The merged and builds counters on the evolutions table were already never read by any observed code path. The Evolution TypeScript type losing these fields is not a breaking change for any visible UI behaviour. Keeping unreachable metrics creates a maintenance burden (code to update them, tests to verify them) for data nobody reads. Removing them is correct housekeeping.
8. When stored summary groups overlap on the same live changes, the group covering more changes wins — a two-member group beats a single-member group that shares one member.
This is the correct semantic for content-addressed groups whose identity is their exact membership. A group of (A, B) is a different entity than a group of (A), and if both exist in the store, preferring the larger means the history view shows the most semantically complete grouping available. The alternative — last-written wins — would mean adding a new single-change summary for A could break apart an existing (A, B) group, which would confuse users who expect stable history groupings.
9. A 'snapshot' — a content-addressed record of the generated commit message for an exact set of change hashes — replaces the old 'changeset' as the mechanism for checking whether the current working tree matches the last successful build.
The old mechanism stored a changeset with member links (join table), then compared the stored member set to current changes on every build-state check. The new mechanism stores a single key (sha256(sorted hashes)) and compares that key to a freshly computed key from current changes. These are algebraically equivalent — same set implies same key — but the new version requires no join query and no set-comparison loop. The invariant 'a bare (build-check) snapshot must not overwrite a real commit message' is correctly enforced by the upsert logic that skips the message update when the incoming message is null or empty.
10. The TypeScript types `ChangeSet`, `SummarizedChange`, `SummarizedChangeSet`, and `Prompt` are removed from the frontend API contract.
Removing types that no longer have a server-side backing is correct — stale types in the contract create confusion and false safety for future frontend code. The concern is whether any current component or Storybook story references these types. The Rust structs were annotated #[allow(dead_code)] and excluded from specta generation before this PR, which is a signal they were already unused. But if any frontend file imports ChangeSet or SummarizedChangeSet from @/ipc/types or @/ipc/sqlite, it will fail silently (TypeScript undefined) rather than at compile time — the migration plan in docs specifically warns about this class of import-trail issue (F1a/F1b). A grep for these type names in apps/native/src/ should gate the merge.
Open questions
-
Can the AI model reliably reproduce 64-character SHA-256 hex strings in its JSON response? Have any prompt-evaluation runs been done on this specific task with the models the product uses (OpenAI, Ollama, Claude CLI)?
-
What is the upgrade path for users who have existing summarization data in the old schema? The initial migration's SQL is rewritten, but users who have already run migration 01 will not re-run it — their databases retain
commits,change_sets,changes, etc. while the application expectspatch_summaries,snapshots, etc. Is there a migration 04 (or equivalent) that drops old tables and creates new ones? -
Were
mergedandbuildson theevolutionstable displayed anywhere — in analytics, a developer tab, or an internal dashboard — even if not in the main UI? -
Does any currently-shipped frontend component reference
ChangeSet,SummarizedChange,SummarizedChangeSet, orPromptfrom@/ipc/typesor@/ipc/sqlite? A failing import would be silent at runtime. -
The
commit: Nonechange inget_history.rsmeansHistoryEntry.commitis always null. Is the history view's rendering path conditioned on this being non-null for any visible element (e.g. a commit detail pane)?
Recommendation
Ship with changes
The schema simplification and content-addressing architecture are sound and should ship. The one decision that warrants a concrete fix before merging is the requirement for the AI model to reproduce full 64-character SHA-256 hex strings: this will silently produce mis-grouped summaries with no retry signal, degrading a core user-visible feature. Substituting short sequential IDs or 8-character hash prefixes in the prompt — mapped back to full hashes server-side — is a low-effort change that preserves the entire content-addressing design while making the model's task tractable. A database upgrade migration (for existing installations) and a grep for removed TypeScript types in the frontend are the other items that should be confirmed before merging.
![prelint[bot]](https://avatars.githubusercontent.com/in/2822344?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
The entire 01-initial migration is rewritten in place, and migrations 02-restore-commits and 03-drop-queued-summaries are deleted — but no new migration (e.g.
apps/native/src-tauri/migrations/01-initial/up.sql:61
Warning
commit: db_commit is replaced with commit: None, meaning the Commit field in every HistoryEntry is now permanently null.
apps/native/src-tauri/src/history/get_history.rs:113
2 finding(s) posted as inline comments.
| -- 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); |
There was a problem hiding this comment.
Caution
The entire 01-initial migration is rewritten in place, and migrations 02-restore-commits and 03-drop-queued-summaries are deleted — but no new migration (e.g. 04-new-schema) is added to carry existing databases forward. Diesel migrations are identified by directory name and tracked in __diesel_schema_migrations; any database that already applied 01-initial will not re-run it, so patch_summaries, summary_groups, summary_group_members, and snapshots are never created, causing every call to db::summaries and db::snapshots to fail with "no such table." The state management migration plan locks "Preserve compatibility during frontend migration" and requires migrations to be "tested against temporary database," with no provision for rewriting baseline migrations in place.
| @@ -126,7 +113,7 @@ pub async fn get_history<R: Runtime>( | |||
| is_base, | |||
There was a problem hiding this comment.
Warning
commit: db_commit is replaced with commit: None, meaning the Commit field in every HistoryEntry is now permanently null. The Commit struct is still registered in specta_gen_ts.rs and exported to apps/native/src/ipc/sqlite.ts, suggesting the frontend type contract still expects this field to be populated for some entries; callers that read historyEntry.commit.hash (or similar) will now silently receive null where they previously received data.
📋 PR Overview
🔬 Coverage
|
1 participant
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. There's no need to store information
which is already available via a
gitcommand, it just creates a burdenhaving to keep it updated.
summarize::find_existing now returns a SemanticChangeMap directly which
summarize multiple files. falls back to single file summary