refactor(THU-739): remove modes table and legacy chat-modes code - #1181
refactor(THU-739): remove modes table and legacy chat-modes code#1181arienemaiara wants to merge 4 commits into
Conversation
- Drop the modes table from the synced schema, DAL exports, and PowerSync sync rules - The chat-modes feature was removed; chat is the default and Search/Research ship as skills - Update architecture docs to reflect the removed table
Semgrep Security ScanNo security issues found. |
There was a problem hiding this comment.
🟡 Not ready to approve
Dropping modes from the shared PowerSync table allowlist can cause the backend upload endpoint to hard-fail (400) any legacy modes CRUD operations, potentially wedging sync for older clients during rollout.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Removes the legacy modes synced table and related “chat-modes” references across the frontend schema/DAL, backend PowerSync mappings, sync-rule configs, and architecture docs, reflecting that Chat is now the default and Search/Research ship as skills.
Changes:
- Dropped
modesfrom the PowerSync table list, frontend synced schema, encryption map, and DAL export expectations. - Updated PowerSync sync rules (local + deploy + k8s) to stop selecting
powersync.modes. - Updated docs and inline comments to remove/replace
modesreferences and explain the legacy table status on the backend.
File summaries
| File | Description |
|---|---|
| src/lib/reconcile-defaults.ts | Updates reconcile gating comments to remove modes from the reconciled-table narrative. |
| src/lib/reconcile-defaults.test.ts | Updates test comments/docs to align version-gate terminology after removing modes. |
| src/db/tables.ts | Removes the legacy modesTable from the frontend Drizzle/SQLite schema. |
| src/db/powersync/schema.ts | Removes modes from the frontend syncedTables map. |
| src/db/powersync/database.ts | Updates initial-sync priority documentation to remove modes. |
| src/db/encryption/config.ts | Removes modes encrypted columns configuration. |
| src/dal/settings.ts | Updates DAL docstring references from modes/tasks/skills/settings → tasks/skills/settings. |
| src/dal/index.ts | Removes the “Modes” section marker now that no modes DAL is exported. |
| src/dal/export.test.ts | Updates the expected exported table keys to no longer include modes. |
| shared/powersync-tables.ts | Removes modes from the canonical PowerSync table name list and invalidation map. |
| powersync-service/config/config.yaml | Removes powersync.modes from the user_essentials sync bucket. |
| docs/multi-device-sync.md | Removes modes from the documented synced table list. |
| docs/customize.md | Removes “Modes” from feature/customization documentation. |
| docs/architecture/powersync-account-devices.md | Removes modes from the architecture’s synced table enumeration. |
| docs/architecture/multi-device-sync.md | Removes modes from the architecture’s synced table table. |
| docs/architecture/export-format.md | Updates export format docs/sample to remove the modes bucket. |
| docs/architecture/composite-primary-keys-and-default-data.md | Removes modes from the composite-PK/default-data documentation. |
| deploy/README.md | Updates PowerSync bucket table list to remove modes. |
| deploy/k8s/templates/configmaps.yaml | Removes powersync.modes from the Helm-rendered sync rules. |
| deploy/config/powersync-config.yaml | Removes powersync.modes from the baked PowerSync config. |
| backend/src/db/powersync-schema.ts | Keeps modesTable as legacy, but removes it from PowerSync table maps / conflict targets and updates comments. |
| backend/README.md | Removes modes from backend docs describing composite PK default-data tables. |
Review details
Suppressed comments (1)
shared/powersync-tables.ts:27
- Removing
modesfrompowersyncTableNamesmakes the backend treat any legacy PowerSync upload operations withtype: "modes"as invalid (validTablesis built from this list), causing/api/powersyncto return 400 and the client to retry the batch indefinitely. If any older clients still have queuedmodesCRUD (even from historical data), this can wedge sync for that account/device.
Consider adding an explicit backward-compat path on the backend (e.g. treat type === "modes" as a no-op success, or allow-and-ignore legacy tables) so schema removals don’t hard-break older clients during rollout.
export const powersyncTableNames = [
'settings',
'chat_threads',
'chat_messages',
'tasks',
'models',
'prompts',
'skills',
'triggers',
'model_profiles',
'devices',
'agents',
] as const
- Files reviewed: 22/22 changed files
- Comments generated: 0
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Preview environment deployed 🚀
Stack: Auto-destroys on PR close/merge. Login via the bundled Keycloak realm — |
PR Metrics
Updated Tue, 04 Aug 2026 17:14:31 GMT · run #2577 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
- Backend accepts and ignores PowerSync uploads for dropped tables (legacyPowerSyncTableNames) so a device with a queued write from before the table was removed drains its CRUD queue instead of looping on a 400 retry.
There was a problem hiding this comment.
🟡 Not ready to approve
The PR description claims legacy modes uploads are accepted-and-ignored to prevent old clients from wedging uploads, but the current code shown does not implement that compatibility path, which would cause infinite upload retries for affected devices.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
docs/architecture/composite-primary-keys-and-default-data.md:17
- This doc removes
modesfrom the composite-PK table list, but later sections still usedefault-modeandmodesTable/chatThreadsTable.modeIdas examples of composite-key references. Withmodesno longer being a synced table, those examples are now misleading—please update them to reference a still-existing composite-PK table (e.g.modelsviachatMessagesTable.modelId) or explicitly markmodeId/modes as legacy-only.
| settings| `(key, user_id)` | Setting keys like `theme` or `model_id` are shared; each user has their own value. |
| models | `(id, user_id)` | Default models (e.g. `openai-gpt-4o`) are seeded per user with same ID. |
| tasks | `(id, user_id)` | Default tasks (e.g. `inbox`) are seeded per user with same ID. |
| prompts | `(id, user_id)` | Default automations/prompts are seeded per user with same ID. |
| model_profiles | `(id, user_id)` | Per-model inference tuning (temperature, nudges, prompt overrides) seeded per user. |
- Files reviewed: 24/24 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Human review recommended
It changes production sync rules and sync-ingest behavior (backward-compat upload handling), which has significant operational impact and should receive final human verification.
Review details
- Files reviewed: 24/24 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
- Remove mode_id from chatThreadsTable and updateChatThread pick - Update composite-key docs to reference modelId instead of modeId
- Correct paths to resolve from docs/architecture, not docs root
There was a problem hiding this comment.
🟡 Human review recommended
It changes production sync rules/schema behavior and adds a backward-compat upload gate, which warrants careful human verification of rollout sequencing and edge cases.
Review details
Suppressed comments (1)
src/lib/reconcile-defaults.test.ts:1049
- The comment says “the four new markers (models/tasks/skills/settings)”, but THU-677 is described in this file as extending the existing models gate to the other tables; “models” is likely not a new marker here. Rewording avoids an inaccurate historical claim and keeps the intent (missing per-table markers) clear.
// upgrading to this build: the four new markers (models/tasks/skills/
// settings) don't exist yet, but the rows themselves were already
- Files reviewed: 25/25 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Not ready to approve
The frontend schema drops chat_threads.mode_id while the backend schema and sync rules still replicate it via SELECT *, risking PowerSync schema mismatch during sync.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
src/db/tables.ts:31
chat_threads.mode_idwas removed from the frontend SQLite/PowerSync schema here, but the backend PowerSync schema still defineschat_threads.mode_id(backend/src/db/powersync-schema.ts:52) and the sync rules still doSELECT * FROM powersync.chat_threads .... That leaves a frontend/backend schema mismatch during replication and can break initial sync (or silently drop the column) depending on PowerSync’s schema enforcement. Consider keepingmode_idin the client schema until the backend column + sync rule projection are updated together.
id: text('id').primaryKey(),
title: text('title'),
isEncrypted: integer('is_encrypted').default(0),
triggeredBy: text('triggered_by'),
wasTriggeredByAutomation: integer('was_triggered_by_automation').default(0),
contextSize: integer('context_size'),
acpSessionId: text('acp_session_id'),
agentId: text('agent_id'),
deletedAt: text('deleted_at'),
- Files reviewed: 25/25 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
The
modestable backed the removed chat-modes feature and no code reads or writes it. Dropping a synced table splits into two PRs so the running PowerSync service stops replicatingpowersync.modesbefore the table is dropped:modes— remove it from the sync rules and client schema. Postgres table stays.DROP TABLEmigration.Changes
modesrule from all three configs (powersync-service,deploy/config,deploy/k8s).modesfrompowersyncTableNames, the clientsyncedTables, encryption map, and export allowlist; cleaned up stale mentions in DAL/reconcile comments and docs.modesmap entries but kept themodesTabledef so drizzle-kit generates no DROP (that's PR 2).modeswrite would 400 on upload and wedge sync.applyOperationnow accepts-and-ignores ops forlegacyPowerSyncTableNames(drains the queue), mirroring the existing empty-patch no-op. Unknown tables still 400.Deploy
Merge → wait for the new
thunderbolt-powersyncimage → roll the Renderpowersyncservice. After that, merge #1186Testing
export.test.ts+reconcile-defaults.test.ts(82) andpowersync.test.ts(25, incl. the legacy accept-and-ignore case) pass.bun db generate→ "No schema changes" (no accidental DROP); type-check clean.