Skip to content

fix: deleted seed profiles, models, and cleared automation stay deleted#3573

Merged
matthiasn merged 3 commits into
mainfrom
fix/seed-deletion-tombstones
Jul 25, 2026
Merged

fix: deleted seed profiles, models, and cleared automation stay deleted#3573
matthiasn merged 3 commits into
mainfrom
fix/seed-deletion-tombstones

Conversation

@matthiasn

@matthiasn matthiasn commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Summary

Final piece of the inference-profile audit (after #3568 and #3570): seeded data no longer revives after the user deletes it.

  • SeedTombstoneStore — one settings row recording deleted seed identities
  • seedDefaults() and prepopulateModelsForProvider() skip tombstoned identities
  • AiConfigRepository.deleteConfig gains recordTombstone (default true) and writes the record
  • removes the upgradeExisting() skill-assignment backfill

Why

Every seeding pass is idempotent by presence, and deletes are hard deletes with no record — so deletion had no memory:

Path Trigger Effect before
seedDefaults() startup and every provider create/update a deleted default profile returned, often in the same session
backfillNewModels() startup, every provider a deleted known model returned
upgradeExisting() skill backfill startup + provider save a profile whose assignments the user cleared regained them with automate: true

That last one is the worst of the three: the guard was skillAssignments.isEmpty, so clearing every assignment — the obvious way to say "stop doing things automatically" — was exactly what triggered restoring them. It's deleted outright; automation defaults are now written only when a profile is first seeded.

Two details that decide whether this works

Model identity is provider-scoped, not row-scoped. Backfill matches on providerModelId under a provider, and the row id may be deterministic or a UUID depending on whether it came from FTUE, manual setup, or sync. A tombstone keyed by row id would miss the recreated row, so modelKey keys on provider:providerModelId — the identity backfill actually compares. Pinned by a test.

Only user-initiated deletions are recorded. Two paths delete rows the app wants back: removeOrphanedDefaultSeeds() re-seeds when a provider becomes usable again, and a provider cascade must return its models when you re-add that provider. Both pass recordTombstone: false. Getting this backwards would mean re-adding a provider yields no models — also pinned by tests.

Sync

Tombstones are local state rather than a synced entity, but a peer applying a synced delete records its own, because deletes arrive through the same repository path. Residual gap, stated plainly: a device that never observes the delete event at all may re-seed and sync the row back. Closing that fully means a synced tombstone entity — a schema migration plus a new sync message type — which I did not want to bundle into this change. Documented in the store's doc comment and the AI README.

Cost

deleteConfig now reads the row before deleting it, to build the identity (a model's provider-native id only exists on the row). One extra read on an interactive, rare action; skipped entirely when recordTombstone: false.

Verification

  • fvm flutter analyze lib test — no issues found
  • fvm flutter test test/features/ai/ test/features/agents/ — 9410 passed
  • seed_tombstone_store.dart at 100% line coverage (verified against coverage/lcov.info)
  • new tests: 9 for the store, 2 for profile revival, 2 for model revival (including the getIt default-constructor path), 4 for repository tombstone recording

Three existing tests in profile_seeding_service_slots_test.dart asserted the removed backfill. Two were rewritten to assert the opposite contract (cleared assignments stay cleared; ambiguous slots are left alone and nothing is written), and the third — which only covered sanitising of backfilled assignments — was deleted along with the code path. hasSlotForSkillType keeps its own matrix test and is still used by the oMLX transcription upgrade.

Audit complete

This closes lotti3-8f7. All four findings are now shipped or tracked: the consent gate and auto-wake (#3568), the in-use badge (#3570), and seed revival (here).

Summary by CodeRabbit

  • Bug Fixes

    • Deleted bundled AI profiles and models now stay deleted across app launches and provider updates.
    • Clearing a profile’s automated skills no longer re-enables default automation on the next launch.
    • Default upgrading no longer backfills missing skill assignments for existing default profiles.
  • Documentation

    • Updated AI “Seeded Defaults” documentation to clarify deletion persistence and seeding/upgrading behavior.
  • Release & Packaging

    • Refreshed Flatpak release metadata and updated the app version to 0.9.1067.

Every seeding pass is idempotent by presence: seedDefaults writes a gated-in
template whenever its row is missing, and backfillNewModels recreates any known
model a configured provider lacks. Both run at startup and again after a
provider is created or updated, and deletes are hard deletes with no record —
so deleting a bundled default undid itself, often within the same session.

SeedTombstoneStore records what the user deleted in one settings row, and both
passes skip those identities. Model identity is provider-scoped
(provider:providerModelId) rather than row-scoped, because backfill matches on
providerModelId under a provider and the row id may be deterministic or a UUID
depending on whether it came from FTUE, manual setup, or sync.

Only user-initiated deletions record a tombstone. deleteConfig takes
recordTombstone (default true); removeOrphanedDefaultSeeds and the model rows
removed by a provider cascade both pass false, because both want their rows
back — the orphan pass re-seeds when the provider becomes usable again, and
re-adding a provider must bring its models with it.

Also removes the skill-assignment backfill in upgradeExisting. Its guard was
skillAssignments.isEmpty, so clearing every assignment — the obvious way to say
'stop doing things automatically' — was exactly what restored them with
automate: true on the next launch. Automation defaults are now written only
when a profile is first seeded.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5f1559fb-b6ce-4a31-8eaa-270444f09db4

📥 Commits

Reviewing files that changed from the base of the PR and between 10d4094 and 648bbca.

📒 Files selected for processing (3)
  • test/features/ai/test_utils.dart
  • test/features/ai/util/model_prepopulation_service_test.dart
  • test/features/ai/util/profile_seeding_service_test.dart
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/features/ai/util/profile_seeding_service_test.dart

📝 Walkthrough

Walkthrough

This change adds persistent tombstones for deleted bundled AI profiles and models, prevents default skill assignments from being restored after clearing, wires tombstone storage through initialization paths, updates tests and documentation, and releases version 0.9.1067.

Changes

AI seed persistence

Layer / File(s) Summary
Persistent tombstone storage
lib/features/ai/util/seed_tombstone_store.dart, test/features/ai/util/seed_tombstone_store_test.dart
Adds settings-backed profile and provider-scoped model identities with tolerant reads, serialized updates, removal, and sorted JSON persistence.
Deletion tombstone recording
lib/features/ai/repository/ai_config_repository.dart, test/features/ai/repository/ai_config_repository_test.dart
User deletions record seed identities, while provider cascades and cleanup deletions disable tombstone recording.
Tombstone-aware seeding and upgrades
lib/features/ai/util/*, test/features/ai/util/*, lib/features/ai/README.md
Profile and model seeding skips deleted identities, removes rows tombstoned during a pass, and no longer restores cleared automated skill assignments.
Initialization and release validation
lib/features/agents/state/*, lib/features/ai/state/*, lib/features/ai/ui/settings/*, test/features/ai/state/*, test/features/ai/test_utils.dart, CHANGELOG.md, flatpak/com.matthiasn.lotti.metainfo.xml, pubspec.yaml
Injects tombstone storage into startup and provider setup, updates test fixtures, and records version 0.9.1067 release notes.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant AiConfigRepository
  participant SeedTombstoneStore
  participant ProfileSeedingService
  User->>AiConfigRepository: delete bundled profile or model
  AiConfigRepository->>SeedTombstoneStore: remember seed identity
  ProfileSeedingService->>SeedTombstoneStore: deletedIdentities()
  SeedTombstoneStore-->>ProfileSeedingService: tombstoned identities
  ProfileSeedingService->>ProfileSeedingService: skip deleted seeds
Loading

Possibly related PRs

  • matthiasn/lotti#3230: Both changes modify profile seeding and upgrade behavior around default profiles and skill assignments.
  • matthiasn/lotti#3392: Both changes modify ProfileSeedingService upgrade and migration logic.
  • matthiasn/lotti#3440: Both changes modify default profile seeding and orphan cleanup paths.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: deleted seeded profiles/models stay deleted and cleared automation remains cleared.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/seed-deletion-tombstones

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
lib/features/ai/util/model_prepopulation_service.dart (1)

69-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Document the tombstone skip in the method docstring.

The docstring above (steps 1–4) still describes the pre-tombstone behavior; a reader can't tell that user-deleted models are permanently skipped here.

♻️ Suggested docstring addition
   /// This method:
   /// 1. Looks up known models for the provider type
+  /// 2. Skips models the user deleted (recorded in [SeedTombstoneStore])
-  /// 2. Generates unique IDs for each model
-  /// 3. Checks if usable rows with those provider-native model IDs exist
-  /// 4. Creates only the models that don't exist yet
+  /// 3. Generates unique IDs for each remaining model
+  /// 4. Checks if usable rows with those provider-native model IDs exist
+  /// 5. Creates only the models that don't exist yet

As per coding guidelines: "Whenever touching a function, consider whether its docstring needs updating."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/features/ai/util/model_prepopulation_service.dart` around lines 69 - 84,
Update the method docstring governing the model prepopulation flow to document
that models marked as deleted in the tombstone store are skipped and will not be
recreated on later launches. Keep the existing steps and behavior unchanged, and
reference the tombstone check used by the prepopulation method.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/features/ai/repository/ai_config_repository.dart`:
- Around line 83-90: Make deletion, tombstone updates, and profile seeding
mutually atomic for each identity. In
lib/features/ai/repository/ai_config_repository.dart:83-90, coordinate the
delete, _rememberSeedTombstone, and seed operation so a concurrent seed cannot
recreate a deleted config. In
lib/features/ai/util/seed_tombstone_store.dart:61-72, make remember and forget
atomic across store instances using a transaction or shared keyed coordinator,
preserving concurrent identities. In
lib/features/ai/util/profile_seeding_service.dart:115-141, perform the tombstone
check and seed write under the same coordination, and add a regression test
covering concurrent deletion and seeding.

In `@test/features/ai/test_utils.dart`:
- Around line 442-448: Update the helper that creates the SeedTombstoneStore to
retain the in-memory SettingsDb instance and register a test teardown that
closes it before returning the store. Preserve the existing identity seeding
behavior while ensuring each setUp-created database is released after the test.

---

Nitpick comments:
In `@lib/features/ai/util/model_prepopulation_service.dart`:
- Around line 69-84: Update the method docstring governing the model
prepopulation flow to document that models marked as deleted in the tombstone
store are skipped and will not be recreated on later launches. Keep the existing
steps and behavior unchanged, and reference the tombstone check used by the
prepopulation method.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a2d9c187-8b25-41c5-b64a-e854e83824c7

📥 Commits

Reviewing files that changed from the base of the PR and between 0798849 and cf72409.

📒 Files selected for processing (21)
  • CHANGELOG.md
  • flatpak/com.matthiasn.lotti.metainfo.xml
  • lib/features/agents/state/agent_providers.dart
  • lib/features/ai/README.md
  • lib/features/ai/repository/ai_config_repository.dart
  • lib/features/ai/state/ai_config_initialization.dart
  • lib/features/ai/state/settings/inference_provider_form_controller.dart
  • lib/features/ai/ui/settings/ai_settings_page.dart
  • lib/features/ai/ui/settings/inference_provider_edit_page.dart
  • lib/features/ai/util/model_prepopulation_service.dart
  • lib/features/ai/util/profile_seeding_service.dart
  • lib/features/ai/util/seed_tombstone_store.dart
  • pubspec.yaml
  • test/features/ai/repository/ai_config_repository_test.dart
  • test/features/ai/state/ai_config_initialization_test.dart
  • test/features/ai/state/settings/inference_provider_form_controller_test.dart
  • test/features/ai/test_utils.dart
  • test/features/ai/util/model_prepopulation_service_test.dart
  • test/features/ai/util/profile_seeding_service_slots_test.dart
  • test/features/ai/util/profile_seeding_service_test.dart
  • test/features/ai/util/seed_tombstone_store_test.dart
💤 Files with no reviewable changes (1)
  • lib/features/ai/ui/settings/ai_settings_page.dart

Comment thread lib/features/ai/repository/ai_config_repository.dart
Comment thread test/features/ai/test_utils.dart Outdated
@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.14%. Comparing base (069edde) to head (648bbca).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3573      +/-   ##
==========================================
- Coverage   99.22%   99.14%   -0.08%     
==========================================
  Files        1781     1782       +1     
  Lines      130336   130408      +72     
==========================================
- Hits       129322   129296      -26     
- Misses       1014     1112      +98     
Flag Coverage Δ
glados 14.12% <25.00%> (+<0.01%) ⬆️
standard 98.89% <100.00%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Review follow-up on two ordering hazards.

Concurrent ledger mutations could lose an identity: the repository builds a
fresh store per deletion and a synced delete can land while a user delete is in
flight, so both read the same prior set and the second write drops the first's
identity — silently reviving that seed. All mutations now run through one
static queue shared by every store instance in the isolate, and a failed write
no longer wedges the writes queued behind it.

A deletion landing after a seeding pass read the ledger but before it wrote the
row left the row resurrected with a tombstone recorded, and nothing would clean
it up because seeding only ever creates. The row and the tombstone live in
different databases, so no transaction can span them; instead seedDefaults and
prepopulateModelsForProvider re-read the ledger after writing and drop anything
tombstoned while they ran, restoring the invariant at the end of each pass. The
re-read is skipped when the pass wrote nothing, which is the common case.

Also closes the in-memory settings database the test helper opened per setUp.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/features/ai/util/profile_seeding_service.dart (1)

251-266: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

upgradeExisting docstring still describes the removed skill-assignment backfill.

The doc comment says the method "then backfills default skill assignments only for default profiles whose assignments are empty," but lines 313-318 now explicitly remove that behavior — the PR's whole point is that clearing assignments must not be restored. Leaving the stale docstring risks a future contributor "fixing" the missing backfill and reintroducing this exact bug.

📝 Proposed docstring fix
   /// old Local Power seed from Ollama to oMLX, migrates untouched Melious image
   /// generation and transcription to the Flux 2 Klein 9B and Whisper Large v3
-  /// defaults, moves untouched Melious profiles to Qwen thinking, GLM 5.2
-  /// high-end, and Voxtral transcription defaults, then backfills default
-  /// skill assignments only for default profiles whose assignments are empty.
+  /// defaults, and moves untouched Melious profiles to Qwen thinking, GLM 5.2
+  /// high-end, and Voxtral transcription defaults. Default skill assignments
+  /// are deliberately never backfilled here — only seeding a profile at
+  /// creation writes automation defaults.

As per path instructions: "Whenever touching a function, consider whether its docstring needs updating."

Also applies to: 313-318

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/features/ai/util/profile_seeding_service.dart` around lines 251 - 266,
Update the doc comment for upgradeExisting to remove the statement that it
backfills default skill assignments for profiles with empty assignments. Keep
the remaining descriptions of profile healing and migration behavior accurate,
without reintroducing or documenting skill-assignment restoration.

Source: Path instructions

🧹 Nitpick comments (1)
test/features/ai/util/profile_seeding_service_test.dart (1)

129-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the repeated "no deletion happened" mock verification.

The same verifyNever(() => mockRepo.deleteConfig(any(), fromSync: any(named: 'fromSync'), recordTombstone: any(named: 'recordTombstone'))) block is copy-pasted across six tests. A shared helper would reduce duplication and centralize the matcher if the deleteConfig signature changes again.

♻️ Proposed helper extraction
+    void verifyNoDeletion() {
+      verifyNever(
+        () => mockRepo.deleteConfig(
+          any(),
+          fromSync: any(named: 'fromSync'),
+          recordTombstone: any(named: 'recordTombstone'),
+        ),
+      );
+    }
+
     test(
       'leaves profiles alone when nothing was tombstoned mid-pass',
       () async {
         await service.seedDefaults();

-        verifyNever(
-          () => mockRepo.deleteConfig(
-            any(),
-            fromSync: any(named: 'fromSync'),
-            recordTombstone: any(named: 'recordTombstone'),
-          ),
-        );
+        verifyNoDeletion();
       },
     );

Repeat the verifyNoDeletion() call at the other five sites (713-719, 738-744, 780-786, 836-842, 864-870).

As per path instructions: "Extract shared widget pump/setup helpers, repeated mock stubs, and complex state into helpers or test-bench classes; pass varying values as named parameters."

Also applies to: 713-719, 738-744, 780-786, 836-842, 864-870

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/features/ai/util/profile_seeding_service_test.dart` around lines 129 -
142, Extract the repeated verifyNever deleteConfig matcher into a shared
verifyNoDeletion() helper near the test setup, then replace the inline
verification blocks in all six affected tests with calls to that helper.
Preserve the existing any(), fromSync, and recordTombstone matching behavior.

Source: Path instructions

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@lib/features/ai/util/profile_seeding_service.dart`:
- Around line 251-266: Update the doc comment for upgradeExisting to remove the
statement that it backfills default skill assignments for profiles with empty
assignments. Keep the remaining descriptions of profile healing and migration
behavior accurate, without reintroducing or documenting skill-assignment
restoration.

---

Nitpick comments:
In `@test/features/ai/util/profile_seeding_service_test.dart`:
- Around line 129-142: Extract the repeated verifyNever deleteConfig matcher
into a shared verifyNoDeletion() helper near the test setup, then replace the
inline verification blocks in all six affected tests with calls to that helper.
Preserve the existing any(), fromSync, and recordTombstone matching behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4c26c615-ee10-4345-a0ca-f351c01007eb

📥 Commits

Reviewing files that changed from the base of the PR and between cf72409 and 10d4094.

📒 Files selected for processing (7)
  • lib/features/ai/README.md
  • lib/features/ai/util/model_prepopulation_service.dart
  • lib/features/ai/util/profile_seeding_service.dart
  • lib/features/ai/util/seed_tombstone_store.dart
  • test/features/ai/test_utils.dart
  • test/features/ai/util/profile_seeding_service_test.dart
  • test/features/ai/util/seed_tombstone_store_test.dart
🚧 Files skipped from review as they are similar to previous changes (3)
  • test/features/ai/test_utils.dart
  • lib/features/ai/util/model_prepopulation_service.dart
  • lib/features/ai/README.md

The profile-seeding race had a regression test; its model-backfill mirror did
not, which Codecov caught as three uncovered lines. Adds the equivalent test
and lifts the late-tombstone store double into the shared AI test utils so
both suites use one implementation.
@matthiasn
matthiasn merged commit 9e51d70 into main Jul 25, 2026
32 checks passed
@matthiasn
matthiasn deleted the fix/seed-deletion-tombstones branch July 25, 2026 11:08
matthiasn added a commit that referenced this pull request Jul 25, 2026
Follow-up to the seed tombstones merged in #3573, which introduced this gap.

Onboarding binds each area to a well-known seeded profile id unconditionally.
Once deletions are remembered, a user who had deleted that bundled profile and
later re-ran onboarding got a category pointing at a row seeding now refuses to
recreate: _resolveModel returns noModel and the first task degrades to
title-only. Onboarding is re-runnable from the maintenance page and the
onboarding settings panel, so this is reachable rather than theoretical.

The key step now clears that provider's tombstone before running the FTUE
setup, so the seeding it triggers can recreate the profile. Deliberately
setting the provider up again is the 'bring it back' signal the store's forget
was built for; every other path still honors the deletion.

Exposes the store as seedTombstoneStoreProvider rather than reaching into
getIt from a widget, which also lets tests substitute one.

Also fixes a defect in the tombstone serialization from that same PR: the
mutation queue was a single static, so it was shared across every database in
the isolate. In tests one stalled mutation against a discarded database wedged
every later write, and the onboarding flow hung mid-connect. The queue is now
keyed by settings database, which is both what the serialization actually needs
and test-safe.
matthiasn added a commit that referenced this pull request Jul 25, 2026
Supersedes the SeedTombstoneStore from #3573. Deleting an AI config now stamps
deletedAt on the row and re-saves it, so the row itself is the tombstone.

The ledger was the wrong shape. It lived in settings.sqlite while the config
lives in ai_config.sqlite, so no transaction could span a deletion and a
seeding check — the race CodeRabbit raised, which I could only mitigate with a
post-pass reconciliation. It was local state, so a device that never observed
the delete could re-seed and sync the row back. And two stores writing the same
row could lose an identity, which needed a serialization queue that itself
shipped with a cross-database bug.

The field costs neither of the things I assumed ruled it out. ai_configs stores
the whole config in a serialized TEXT column, so a new freezed field lands in
that JSON and schemaVersion stays 1. SyncMessage.aiConfig already carries the
whole config, so the deletion replicates on the existing path. It also matches
how the journal domain deletes synced entities (CategoryRepository).

Reads split by intent: getConfigById/getConfigsByType hide deleted rows by
default, the seeding passes ask for them so a deleted row reads as present and
blocks re-creation, and watchConfigsByType always filters. Two paths keep
hard-deleting through hardDeleteConfig — removeOrphanedDefaultSeeds and the
provider cascade — because both want their rows back later.

Deletes SeedTombstoneStore, its test, seedTombstoneStoreProvider, the settings
key, recordTombstone, the mutation queue, and the mid-pass reconciliation in
both seeding passes.

Also makes the delete service's undo explicit: it re-saved a pre-delete
snapshot, which happened to clear the stamp but would silently revert any
change made during the undo window. It now calls restoreConfig.
matthiasn added a commit that referenced this pull request Jul 25, 2026
Supersedes the SeedTombstoneStore from #3573. Deleting an AI config now stamps
deletedAt on the row and re-saves it, so the row itself is the tombstone.

The ledger was the wrong shape. It lived in settings.sqlite while the config
lives in ai_config.sqlite, so no transaction could span a deletion and a
seeding check — the race CodeRabbit raised, which I could only mitigate with a
post-pass reconciliation. It was local state, so a device that never observed
the delete could re-seed and sync the row back. And two stores writing the same
row could lose an identity, which needed a serialization queue that itself
shipped with a cross-database bug.

The field costs neither of the things I assumed ruled it out. ai_configs stores
the whole config in a serialized TEXT column, so a new freezed field lands in
that JSON and schemaVersion stays 1. SyncMessage.aiConfig already carries the
whole config, so the deletion replicates on the existing path. It also matches
how the journal domain deletes synced entities (CategoryRepository).

Reads split by intent: getConfigById/getConfigsByType hide deleted rows by
default, the seeding passes ask for them so a deleted row reads as present and
blocks re-creation, and watchConfigsByType always filters. Two paths keep
hard-deleting through hardDeleteConfig — removeOrphanedDefaultSeeds and the
provider cascade — because both want their rows back later.

Deletes SeedTombstoneStore, its test, seedTombstoneStoreProvider, the settings
key, recordTombstone, the mutation queue, and the mid-pass reconciliation in
both seeding passes.

Also makes the delete service's undo explicit: it re-saved a pre-delete
snapshot, which happened to clear the stamp but would silently revert any
change made during the undo window. It now calls restoreConfig.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant