Skip to content

refactor(store): numbered migration files instead of a hand-typed SCHEMA_VERSION - #427

Merged
itsskofficial merged 1 commit into
mainfrom
refactor/db-migration-files
Oct 17, 2026
Merged

itsskofficial merged 1 commit into
mainfrom
refactor/db-migration-files

Conversation

@itsskofficial

@itsskofficial itsskofficial commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Closes #424

What changed

  • One file per migration step in sentient/store/migrations/ (NNNN_<what>.py with async def migrate(store), or NNNN_<what>.sql for statements that are safe to run twice). SCHEMA_VERSION in sentient/store/db.py is now MIGRATIONS[-1].number, found from the files, never typed by hand.

  • Store.open: an existing database below the latest version gets the snapshot (Backup, restore and snapshots before migrations #151, unchanged), then runs the steps above its version in order and stamps PRAGMA user_version after each one. After that the schema.sql files run and add any missing tables and indexes. A new database runs no steps: the schema.sql files now hold the whole current shape.

  • Steps 1-12 port the old behaviour. I traced every ensure_column back through git history to the commit that added it and the SCHEMA_VERSION at that point:

    • Each documented step keeps its columns (2 sessions.incognito, 3 messages.sources, 4 tasks.notepad, 5 sessions.model, 6 approval_grants, 7 usage.cost_usd, 8 evolution_reviews.activity_mark, 9 messages.approval, 10 try runs and file snapshots, 11 memory scopes plus checks, 12 folders.scope).
    • A column that arrived without a bump goes in the step after the version it arrived at. For example, channel_chats.is_group arrived at 11, so it is in 12.
    • Step 1 holds everything from before feat(backup): backup, restore and snapshots before migrations #287. Databases from those builds say version 0, so they run every step.
    • The old "summaries take their chat's untrusted mark" backfill is now part of step 1. It only ever changed rows written before the mark existed.
  • Step 13 (new): skill_stats.check_report and pending_check_report (Outcome checks, part 2: skill validation before proposing, and read-back for Home Assistant #386) arrived at version 12 without a bump, so they get their own step. The version goes from 12 to 13.

  • Packages no longer call store.ensure_column on start (tasks, memory, skills, channels, integrations, evolution, reminders, snapshots). The columns that only those calls used to add are now in their schema.sql:

    • tasks.keep_running, task_runs.last_activity_at and idx_tasks_next_exec
    • channel_chats.is_group and owner_id
    • facts.recall_count, last_recalled_at and merged_*
    • integrations.status and error
    • the nine extra skill_stats columns
    • idx_facts_scope and idx_usage_paid
  • tests/test_migrations.py fails when:

    • two step files share a number;
    • the numbers have a gap;
    • a file is misnamed, or a .py step has no migrate;
    • a database migrated from version 1 or version 0 differs from a new one in its tables, columns (ignoring order), indexes or triggers. The old schemas in tests/schemas/ were dumped by starting the 716658a and 4a5cb2d builds' apps.

    It also checks that a step that fails part way leaves the version at the last finished step, and that a new database runs no steps. On its first run it caught a real slip: I had put task_runs.sources in step 1, but it arrived at version 7, so it belongs in step 8.

  • Docs:

    • docs/DEVELOPING.md has a new "Changing the database" section: how to add a table, a column or a step, and what to do when two PRs pick the same number.
    • ADR 0030 records the change, and ADR 0003 is marked as amended.
    • sentient/AGENTS.md and the snapshot notes in docs/API.md are updated.
  • Packaging: the frozen engine already collects every sentient.* submodule, and PyInstaller's pkgutil hook lists modules from the PYZ archive, so the .py steps are found (verified below). The spec now also ships migrations/*.sql as data.

  • Changelog: changes/424.changed.md (the towncrier layout from ci: changelog entries as one file per change in changes/ (towncrier) #418).

Real test

No model calls. Every home's model addresses pointed at a closed port, and the engines ran on port 8786 with a random token and their own SENTIENT_HOME on D:.

  1. A real version-8 database. I checked out 810d00a (SCHEMA_VERSION = 8) in a separate worktree and ran its own desktop/scripts/seed-*.py in the documented order (Maya Rao persona). That gave user_version 8 with 13 chats, 86 messages, 20 tasks, 12 task runs, 60 memories, 5 summaries, 9 skill stats rows and 22 notifications.
  2. New engine on a copy of that home (python -m sentient serve --port 8786):
    • Log line: snapshot sentient-snapshot-20261017-150042.zip taken before updating the database (version 8 to 13). The zip holds sentient.db at user_version 8 with 13 chats.
    • The migrated sentient.db is at user_version 13 and has skill_stats.check_report, task_runs.sources and folders.scope.
    • Over REST:
      • GET /api/sessions: 13 chats ("Bills and a dinner booking"...).
      • GET /api/sessions/demo-chat-readme/messages: 10 messages.
      • GET /api/tasks: 20 tasks ("Evening Brief", "Daily Brief"...).
      • GET /api/memories: 60 memories ("Maya prefers dark mode in every app"..., scope personal).
      • GET /api/memories/summaries: 5.
      • GET /api/skills: 7 active and 3 pending.
      • GET /api/notifications: 22.
      • GET /api/backups: the snapshot, reason "before updating the database (version 8 to 13)", schema_version 8.
  3. New engine on a brand-new home.
    • No snapshot is taken and user_version is 13.
    • Compared with the migrated database: 62 tables, 30 indexes and 5 triggers are identical. The only extra tables in the migrated one are the sqlite-vec tables (facts_vec, summaries_vec, user_insights_vec and their shadow tables), which are made at run time once the seeded home has embeddings.
  4. Same as the old code. I opened another copy of the version-8 home with origin/main (b213dbe) and compared it with the new engine's result: 77 tables, 30 indexes and 5 triggers each, 0 differences (columns compared without order). The only difference is version 12 against 13.
  5. Frozen engine. I froze the engine with packaging/sentient-engine.spec (PyInstaller 6.22) and started sentient-engine.exe serve on another copy of the version-8 home. It logged snapshot ... taken before updating the database (version 8 to 13) and served 13 chats, 20 tasks and 60 memories over REST, so the steps are found inside the frozen build too.
  6. Full suite: pytest -q: 2477 passed, 2 skipped (13 min). ruff check sentient tests packaging is clean.

For the owner

  • Version 13 means every existing install takes one snapshot on its first start after this lands, as with any schema update.
  • When two open PRs both add 0014_..., git won't show a conflict because the file names differ. The test fails on main (or on the second PR after a rebase) with "two migrations share a number", and that PR renames its file. This is documented in DEVELOPING.md.

Summary by CodeRabbit

  • Improvements
    • Existing databases are updated automatically when you start the app after upgrading. A snapshot is taken before the database changes are applied, and existing data is retained.
    • Database updates now run in sequence, helping older installations move to the current database structure reliably.

…EMA_VERSION

One file per step in sentient/store/migrations (NNNN_<what>.py or .sql);
SCHEMA_VERSION is the highest number. Steps 1-12 port what Store.open and
the services' ensure_column calls did, columns that arrived without a bump
go in the step after the version they arrived at (task_runs.sources in 8),
and the skill check columns (#386) get step 13. The schema.sql files now
hold the whole current shape; a new database runs no steps. ADR 0030.

Closes #424
@github-actions github-actions Bot added documentation Improvements or additions to documentation area: engine Agent loop, models, approvals, storage, gateway area: tasks Long-running, scheduled, triggered and watcher tasks area: memory Facts, user model, nightly consolidation area: proactivity Suggestions, heartbeat, self-improving skills area: integrations App integrations, change feeds, webhooks area: channels Telegram, Discord area: packaging Installers and releases labels Oct 17, 2026
@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 59dfe447-724e-4a80-a839-d046a65dbf3c
📥 Commits

Reviewing files that changed from the base of the PR and between b213dbe and 56d7719.

📒 Files selected for processing: 38
  • changes/424.changed.md
  • docs/API.md
  • docs/DEVELOPING.md
  • docs/adr/0003-single-sqlite-database.md
  • docs/adr/0030-numbered-migration-files.md
  • docs/adr/README.md
  • packaging/sentient-engine.spec
  • sentient/AGENTS.md
  • sentient/channels/schema.sql
  • sentient/channels/store.py
  • sentient/evolution/service.py
  • sentient/integrations/service.py
  • sentient/memory/schema.py
  • sentient/reminders/service.py
  • sentient/skills/loader.py
  • sentient/snapshots/service.py
  • sentient/store/db.py
  • sentient/store/migrations/0001_before_numbered_steps.py
  • sentient/store/migrations/0002_incognito_chats.py
  • sentient/store/migrations/0003_answer_sources.py
  • sentient/store/migrations/0004_task_notepad.py
  • sentient/store/migrations/0005_chat_model.py
  • sentient/store/migrations/0006_approval_grants.py
  • sentient/store/migrations/0007_usage_cost.py
  • sentient/store/migrations/0008_evolution_activity_mark.py
  • sentient/store/migrations/0009_activity_timeline.py
  • sentient/store/migrations/0010_try_runs_and_file_snapshots.py
  • sentient/store/migrations/0011_memory_scopes.py
  • sentient/store/migrations/0012_folder_scopes.py
  • sentient/store/migrations/0013_skill_checks.py
  • sentient/store/migrations/__init__.py
  • sentient/store/schema.sql
  • sentient/tasks/repo.py
  • sentient/tasks/schema.sql
  • sentient/tasks/service.py
  • tests/schemas/schema_v0.sql
  • tests/schemas/schema_v1.sql
  • tests/test_migrations.py
💤 Files with no reviewable changes: 8
  • sentient/channels/store.py
  • sentient/snapshots/service.py
  • sentient/evolution/service.py
  • sentient/integrations/service.py
  • sentient/reminders/service.py
  • sentient/tasks/service.py
  • sentient/tasks/repo.py
  • sentient/skills/loader.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

📝 Walkthrough

The store now discovers numbered migration steps and derives SCHEMA_VERSION from the highest number. It applies pending steps to older databases before applying current schema files. The change also adds schema-equivalence tests and updates migration guidance and packaging.

Changes: Numbered Database Migrations

Layer / File(s) Summary
Migration discovery and startup execution
sentient/store/migrations/__init__.py, sentient/store/db.py, tests/test_migrations.py
The store discovers and validates numbered Python and SQL steps, derives the schema version from the highest step, and runs pending steps for older databases. Tests cover numbering, execution order, version stamping, and reopening.
Historical upgrades and current schemas
sentient/store/migrations/000*.py, sentient/store/schema.sql, sentient/channels/schema.sql, sentient/tasks/schema.sql, sentient/*/service.py, sentient/*/schema.py, sentient/skills/loader.py, sentient/tasks/repo.py
Steps 1–13 contain existing column changes and a summary data backfill. Current schema files include the full current shape. Package startup and repository code no longer add columns through ensure_column calls.
Migration equivalence tests
tests/schemas/schema_v*.sql, tests/test_migrations.py
Tests compare databases upgraded from versions 0 and 1 with newly created databases, including schema objects and selected retained values.
Migration documentation and packaging
docs/API.md, docs/DEVELOPING.md, docs/adr/*, docs/adr/README.md, sentient/AGENTS.md, packaging/sentient-engine.spec, changes/424.changed.md
The documentation describes numbered migration steps and schema responsibilities. The packaging spec includes SQL migration files. The change note records schema version 13 and a snapshot on first start after updating.

Priority: ➖ Normal

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

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant Store.open
  participant migrations.migrate
  participant MigrationStep
  participant Store
  participant SQLite
  Store.open->>migrations.migrate: Run steps above the database version
  migrations.migrate->>MigrationStep: Execute each pending step in order
  MigrationStep->>Store: Call Store.ensure_column when needed
  migrations.migrate->>SQLite: Stamp user_version and commit each successful step
  Store.open->>SQLite: Apply core and package schemas
Loading
Merge Risk: ⚪ Minimal · up to 56d77

Existing databases now upgrade through ordered, numbered migration steps instead of column setup scattered across package startup, and new databases are built from the full schema files. The columns, indexes and tables that the old startup code added are carried into steps or schema files that run when the database opens. Untrusted-chat summaries keep their marking. No merge-blocking risk was identified.

Security architecture risk: 🟡 Moderate · up to 56d77

Numbered upgrades improve consistency, but moving a recurring trust-label repair into a one-time step can leave some older conversation summaries unprotected after their chat is deleted. This is a conditional risk affecting one installation; deletion remains authenticated.

Retained concerns

  • Medium · security · inferred: Restricting the recurring summary-mark repair to migration 0001 weakens provenance preservation for legacy NULL-marked summaries. Existing session classification can leave the summary dependent on its session’s mark; deleting the session then removes that guard while retaining the summary, making it eligible for clean-memory search. Base startup repaired this state whenever the marked session still existed. Exposure requires an affected legacy row and subsequent deletion; production prevalence and attacker control of that sequence remain uncertain.
Security review details

Security Blast Radius

  • inferred — The identified path affects retained summaries and their downstream use within one installation’s database. It can cross conversation boundaries through clean-memory search and profile inputs; the evidence does not establish cross-tenant compromise, credential access, or direct exfiltration.

Security Findings and Attack Paths

  • inferred — No canonical Security finding is retained. Source inspection supports a conditional path: a legacy NULL-marked summary survives upgrade, its session acquires an outside-content mark without updating the summary, and deletion removes the session reference. The summary then satisfies the clean-search predicate. The PR removes a recurring startup repair for this state; deletion before such repair was already possible at the base. Attacker-supplied summary content and attacker ability to induce deletion are not established.

Trust Boundaries and Controls

  • observed — The inspected deletion route requires the gateway token. Migration discovery loads executable modules and SQL from the installed migration package; Store.open supplies that package’s discovered steps rather than a request-controlled source.

Hardening Proposals

  • proposed — Preserve inherited trust metadata durably before a summary loses its session reference, including legacy NULL-marked rows. Make this invariant hold at the lifecycle transition rather than depending only on startup repair.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 35 functions across 17 files. (13 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: replacing a manually maintained schema version with numbered migration files.
Linked Issues check Passed Issue #424 coding requirements are implemented. sentient/store/migrations/ contains numbered steps 1–13, MIGRATIONS is discovered from the files, and SCHEMA_VERSION uses the highest step number.…
Out of Scope Changes check Passed The changes stay within issue #424. Schema-file updates, removal of startup ensure_column calls, packaging of SQL migration data, migration tests, documentation, ADR updates, and the changelog all s…
Full details: Linked Issues check

Explanation

Issue #424 coding requirements are implemented. sentient/store/migrations/ contains numbered steps 1–13, MIGRATIONS is discovered from the files, and SCHEMA_VERSION uses the highest step number. Store.open snapshots existing databases before upgrades, runs steps in order, stamps user_version after each step, and skips migration steps for new databases. The migration tests cover duplicate numbers, gaps, invalid files, missing Python entry points, partial failure stamping, new-database behavior, and schema parity for tables, columns, indexes, and triggers. docs/DEVELOPING.md and ADR 0030 document the procedure, and ADR 0003 records the amendment.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #424. Schema-file updates, removal of startup ensure_column calls, packaging of SQL migration data, migration tests, documentation, ADR updates, and the changelog all support numbered migrations, schema parity, or deployment of the migration files. No unrelated change is demonstrated.

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 35 functions across 17 files. (13 skipped: 13 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

I’m a rabbit with a numbered trail,
I hop through steps from start to end.
Fresh schemas wait in tidy rows,
Old tables gain the fields they need.
I thump once when versions align.

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

@itsskofficial
itsskofficial merged commit 980a59c into main Oct 17, 2026
12 of 13 checks passed
@itsskofficial
itsskofficial deleted the refactor/db-migration-files branch October 17, 2026 11:33
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: channels Telegram, Discord area: engine Agent loop, models, approvals, storage, gateway area: integrations App integrations, change feeds, webhooks area: memory Facts, user model, nightly consolidation area: packaging Installers and releases area: proactivity Suggestions, heartbeat, self-improving skills area: tasks Long-running, scheduled, triggered and watcher tasks documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Numbered migration files instead of one hand-typed SCHEMA_VERSION

1 participant