Skip to content

Add Dexie storage migration foundation#248

Open
maniator wants to merge 27 commits into
masterfrom
storage/dexie-migration-foundation-v2
Open

Add Dexie storage migration foundation#248
maniator wants to merge 27 commits into
masterfrom
storage/dexie-migration-foundation-v2

Conversation

@maniator
Copy link
Copy Markdown
Owner

@maniator maniator commented Apr 24, 2026

Summary

Starts the migration from RxDB-on-Dexie toward direct Dexie/IndexedDB storage without changing the production runtime storage path yet.

This PR establishes the direct Dexie foundation, documents the staged migration plan, and adds a Dexie-backed save/event store slice with parity-oriented tests. RxDB remains the live app storage path for now.

Changes

  • Adds direct dexie and dexie-react-hooks dependencies to package.json.
  • Regenerates yarn.lock for the new direct Dexie dependencies.
  • Adds src/storage/dexieDb.ts with typed Dexie tables for the current storage collections:
    • saves
    • events
    • teams
    • players
    • completedGames
    • batterGameStats
    • pitcherGameStats
  • Adds schema/index tests for the Dexie DB foundation.
  • Adds docs/dexie-migration-plan.md with the staged plan for moving from RxDB to direct Dexie.
  • Adds src/features/saves/storage/dexieSaveStore.ts, a Dexie-backed save/event store that mirrors current SaveStore behavior for:
    • creating/listing/updating/deleting saves
    • max-save eviction with event cleanup
    • per-save queued event appends
    • signed save export/import
    • missing-team rejection during save import
  • Adds Dexie save-store parity tests.

Testing

Not run in this session.

Expected follow-up validation:

yarn typecheck
yarn lint
yarn test:coverage
yarn build

The lockfile now includes the expected dependency resolutions:

  • dexie@npm:^4.2.04.4.2
  • dexie-react-hooks@npm:^4.2.04.4.0

Risks

  • New Dexie files are not wired into production runtime yet; this is intentional, but means the migration is incomplete.
  • The Dexie save-store slice must stay in parity with the existing RxDB SaveStore until the storage abstraction/cutover work is complete.
  • Follow-up work is needed for repository abstraction, custom teams, career stats, React live-query replacement, and the final RxDB-to-Dexie data transition strategy.
  • Validation commands still need to be run after the lockfile update.

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Apr 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
blipit-legends Ready Ready Preview, Comment May 12, 2026 10:24pm

Request Review

Copy link
Copy Markdown
Owner Author

@copilot please continue this PR.

Context:

  • This is the rebuilt Dexie migration foundation PR off current master after PR Allow Copilot setup to repair lockfiles #247 updated Copilot setup to allow non-immutable installs.
  • Please keep commits small and meaningful.
  • Do not wire Dexie into production runtime yet unless required to make this foundation compile/test cleanly.
  • Do not remove RxDB/RxJS in this PR.

Please do the following:

  1. Run yarn install and commit the regenerated yarn.lock for the new dependencies:
    • dexie@^4.2.0
    • dexie-react-hooks@^4.2.0
  2. Run:
    • yarn typecheck
    • yarn lint
    • yarn test:coverage
    • yarn build
  3. Fix only issues caused by this PR.
  4. If coverage thresholds fail because of the new Dexie files, add targeted tests or adjust the new code. Do not lower coverage thresholds.
  5. Report the exact commands run and their pass/fail results in a PR comment.

Expected lockfile resolutions previously seen from Vercel:

  • dexie@npm:^4.2.0 resolves to 4.4.2
  • dexie-react-hooks@npm:^4.2.0 resolves to 4.4.0

@maniator maniator force-pushed the storage/dexie-migration-foundation-v2 branch from f363ad4 to ace5ee5 Compare May 10, 2026 11:32
@maniator
Copy link
Copy Markdown
Owner Author

@copilot run appropriate bmad skills to validate whether this is a worthy effort going forward or not

Copy link
Copy Markdown
Contributor

Copilot AI commented May 10, 2026

@copilot run appropriate bmad skills to validate whether this is a worthy effort going forward or not

Ran BMAD party-mode validation against commit ace5ee5 with John (PM), Winston (Architect), Amelia (Dev), and Mary (Analyst). Verdict: continue the Dexie migration as staged risk-reduction infrastructure, but do not wire runtime Dexie storage until repository parity, realistic migration/rollback tests, reactive UI checks, custom team/career stats coverage, and architect sign-off are complete. No UI changes were made.

@maniator
Copy link
Copy Markdown
Owner Author

@copilot please have bmad create a plan for this effort and document it in the proper place

Copy link
Copy Markdown
Contributor

Copilot AI commented May 10, 2026

@copilot please have bmad create a plan for this effort and document it in the proper place

Done in 2e57b15. Engaged John (bmad-agent-pm) and authored a full bmad-way plan for the entire Dexie migration (v0–v5) at _bmad-output/planning-artifacts/dexie-migration/ — the bmad planning location per _bmad/bmm/config.yaml. Includes README, roadmap, locked-decisions audit trail (D1–D11 + 4 pending), 18-row risk register, CE-menu epics & stories, cutover checklist, and one M1-formatted plan per phase under phase-plans/. The pre-bmad docs/dexie-migration-plan.md is replaced with a pointer note. v4 (runtime flip) and v5 (RxDB removal) both gated on Winston CR APPROVE per D7.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 24feef1c14

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +196 to +200
await db.transaction("rw", db.saves, db.events, async () => {
await db.saves.put(cleanHeader);
if (cleanEvents.length > 0) {
await db.events.bulkPut(cleanEvents);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Remove stale event rows during save import

When importing a save that reuses an existing header.id, this path upserts the header and bulkPuts incoming events but never deletes prior rows for that save. If the existing save has more events than the imported bundle, higher-index stale events remain in events, so later load/replay can read extra history not present in the imported file. The RxDB SaveStore.importRxdbSave path explicitly clears existing events first, so this is a parity regression in the new Dexie store.

Useful? React with 👍 / 👎.


const { matchupMode: _drop, ...headerRest } = header as unknown as Record<string, unknown>;
const cleanHeader = { ...headerRest } as unknown as SaveRecord;
const cleanEvents = Array.isArray(events) ? events : [];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Validate imported event records before persisting

The import flow currently accepts any array as events and writes it directly, without checking saveId/id consistency, index contiguity, or record shape. Because the signature key is static and public, a manually crafted bundle can still pass signature verification and then persist malformed or foreign-save rows, violating assumptions that the existing RxDB import path enforces via event validation. This can leave orphaned/misaligned event logs for the imported save.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR lays groundwork for migrating persistence from RxDB-on-Dexie to direct Dexie/IndexedDB by introducing a typed Dexie schema, a Dexie-backed save/event store slice, and a bmad planning artifact set—while keeping RxDB as the production runtime backend for now.

Changes:

  • Added dexie and dexie-react-hooks dependencies (with lockfile updates).
  • Introduced BallgameDexieDb (typed tables + v1 schema/indexes) and a Dexie-backed DexieSaveStore with parity-oriented unit tests.
  • Added/updated Dexie migration documentation (pointer doc + full bmad artifact set).

Reviewed changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
package.json Adds direct Dexie dependencies.
yarn.lock Locks Dexie dependency resolutions.
src/storage/types.ts Re-exports PortableSaveExport via @storage/types.
src/features/saves/storage/types.ts Renames the portable save envelope type to PortableSaveExport and deprecates RxdbExportedSave.
src/storage/dexieDb.ts Adds typed Dexie DB singleton + v1 schema/index definitions.
src/storage/dexieDb.test.ts Adds schema/index tests for the Dexie DB.
src/features/saves/storage/dexieSaveStore.ts Adds Dexie-backed save/event store implementation (create/list/update/delete, eviction, append queue, export/import).
src/features/saves/storage/dexieSaveStore.test.ts Adds parity-oriented tests for the Dexie save store slice.
docs/dexie-migration-plan.md Converts old plan doc into a pointer to the bmad artifact set.
_bmad-output/planning-artifacts/dexie-migration/README.md Adds planning artifact hub (scope, contracts, validation gates).
_bmad-output/planning-artifacts/dexie-migration/roadmap.md Adds phased v0–v5 migration roadmap.
_bmad-output/planning-artifacts/dexie-migration/decisions.md Captures locked decisions and pending decisions for the migration.
_bmad-output/planning-artifacts/dexie-migration/risks.md Adds risk register with mitigations and owning phases.
_bmad-output/planning-artifacts/dexie-migration/epics-and-stories.md Breaks migration into epics/stories with acceptance criteria.
_bmad-output/planning-artifacts/dexie-migration/cutover-checklist.md Adds pre-merge gates for v4 cutover and v5 RxDB removal.
_bmad-output/planning-artifacts/dexie-migration/phase-plans/v1-foundation.md Adds phase plan for v1 foundation (this PR’s scope).
_bmad-output/planning-artifacts/dexie-migration/phase-plans/v2-repository-abstraction.md Adds phase plan for repository interface seam work.
_bmad-output/planning-artifacts/dexie-migration/phase-plans/v3-dexie-repositories.md Adds phase plan for Dexie repository implementations + parity.
_bmad-output/planning-artifacts/dexie-migration/phase-plans/v4-cutover.md Adds phase plan for runtime flip + bridge migration + kill switch.
_bmad-output/planning-artifacts/dexie-migration/phase-plans/v5-rxdb-removal.md Adds phase plan for RxDB removal once stable.

Comment thread src/features/saves/storage/dexieSaveStore.ts
Comment thread src/features/saves/storage/dexieSaveStore.ts
Comment thread src/features/saves/storage/dexieSaveStore.ts
Comment thread src/features/saves/storage/dexieSaveStore.ts
Comment thread src/features/saves/storage/dexieSaveStore.ts
Comment thread src/storage/dexieDb.ts Outdated
Comment thread src/storage/dexieDb.test.ts
Comment thread src/features/saves/storage/dexieSaveStore.test.ts
Comment thread src/features/saves/storage/dexieSaveStore.test.ts
@maniator
Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request and create a merge commit

Copy link
Copy Markdown
Contributor

Copilot AI commented May 12, 2026

@copilot resolve the merge conflicts in this pull request and create a merge commit

Done in f57b510 — true two-parent merge commit (parents: deaf3330 branch tip, 72cb44eb master). Conflicts resolved in package.json (kept both dexie/dexie-react-hooks and nanoid), src/features/saves/storage/types.ts (kept master's v2 RxdbExportedSave union + V2BundleHeader/V2BundleCollections, with v1 arm folded into the new PortableSaveExport interface), and .github/instructions/bmad-review.instructions.md (front-matter whitespace). yarn.lock regenerated; yarn typecheck passes.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 5 comments.

Comment on lines +67 to +71
if (!nextIdxMap.has(saveId)) {
const existing = await db.events.where("saveId").equals(saveId).toArray();
const highestIdx = existing.reduce((max, event) => Math.max(max, event.idx), -1);
nextIdxMap.set(saveId, highestIdx + 1);
}
Comment on lines +88 to +93
try {
await db.events.bulkAdd(docs);
} catch (err) {
nextIdxMap.set(saveId, startIdx);
throw err;
}
Comment on lines +139 to +140
const events = await db.events.where("saveId").equals(saveId).sortBy("idx");
const sig = fnv1a(PORTABLE_SAVE_EXPORT_KEY + JSON.stringify({ header, events }));
Comment on lines +196 to +200
await db.transaction("rw", db.saves, db.events, async () => {
await db.saves.put(cleanHeader);
if (cleanEvents.length > 0) {
await db.events.bulkPut(cleanEvents);
}
Comment on lines +15 to +57
await db.open();

expect(db.tables.map((table) => table.name).sort()).toEqual([
"batterGameStats",
"completedGames",
"events",
"pitcherGameStats",
"players",
"saves",
"teams",
]);
});

it("creates the indexes needed by current storage queries", async () => {
const db = createDexieDb(TEST_DB_NAME);

await db.open();

expect(db.saves.schema.primKey.keyPath).toBe("id");
expect(db.saves.schema.idxByName.updatedAt).toBeDefined();

expect(db.events.schema.primKey.keyPath).toBe("id");
expect(db.events.schema.idxByName.saveId).toBeDefined();
expect(db.events.schema.idxByName["[saveId+idx]"]).toBeDefined();

expect(db.teams.schema.primKey.keyPath).toBe("id");
expect(db.teams.schema.idxByName.updatedAt).toBeDefined();
expect(db.teams.schema.idxByName.nameLowercase).toBeDefined();

expect(db.players.schema.primKey.keyPath).toBe("id");
expect(db.players.schema.idxByName.teamId).toBeDefined();

expect(db.completedGames.schema.idxByName.playedAt).toBeDefined();
expect(db.completedGames.schema.idxByName["[homeTeamId+playedAt]"]).toBeDefined();
expect(db.completedGames.schema.idxByName["[awayTeamId+playedAt]"]).toBeDefined();

expect(db.batterGameStats.schema.idxByName.gameId).toBeDefined();
expect(db.batterGameStats.schema.idxByName["[playerId+createdAt]"]).toBeDefined();
expect(db.batterGameStats.schema.idxByName["[teamId+createdAt]"]).toBeDefined();

expect(db.pitcherGameStats.schema.idxByName.gameId).toBeDefined();
expect(db.pitcherGameStats.schema.idxByName["[playerId+createdAt]"]).toBeDefined();
expect(db.pitcherGameStats.schema.idxByName["[teamId+createdAt]"]).toBeDefined();
@maniator
Copy link
Copy Markdown
Owner Author

@copilot please fix workflow failures, apply changes based on the comments in this thread, run a bmad code review loop and fix all issues found

Copy link
Copy Markdown
Contributor

Copilot AI commented May 12, 2026

@copilot please fix workflow failures, apply changes based on the comments in this thread, run a bmad code review lo...

Done in a131eff.

Workflow fix: the failing Lint job's Format check step was complaining about .github/instructions/bmad-review.instructions.md; ran yarn format to add the missing blank line after the front-matter delimiter.

Reviewer-thread fixes (against f57b510):

  • appendEvents — seed nextIdxMap via the [saveId+idx] compound .last() instead of scanning the full event log.
  • appendEvents — wrap the bulkAdd in db.transaction("rw", db.events, …) so a partial-failure BulkError rolls back atomically before nextIdxMap is reset.
  • exportSave — read events via the [saveId+idx] compound .between(...).toArray() so rows come back already index-ordered (no sortBy).
  • importSave — added validateImportedEvents (mirrors SaveStore.validateImportedEvents: array shape, primitive types, ${saveId}:${idx} id pattern, contiguous non-negative indices, dedup); validation runs before any DB writes; the transactional write now also delete()s pre-existing events for the imported saveId before bulkPut so re-imports can't leave stale rows behind.
  • dexieDb.test.ts — capture per-test db in a suite-scoped let and db.close() in afterEach before deleteDexieDb.
  • Added 5 new parity tests: replace-on-import, mismatched-saveId, non-contiguous-idx, mismatched-id, non-array-events.

bmad code-review loop: ran parallel_validation. CodeQL: 0 alerts. Code Review surfaced 3 stylistic nits (rename helper, drop double-cast, drop trailing .sort()) that I intentionally did not apply — all three break parity with the RxDB SaveStore (same helper name, same as unknown as EventRecord pattern, same trailing sort because input event order is unspecified). Parity is the stated goal of this slice (D11 in the bmad plan).

Local validation: yarn typecheck, yarn lint, yarn format:check, yarn build, and yarn test --run (2622/2622 pass, 151 files including all 15 dexie tests) all green.

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.

3 participants