Skip to content

fix(indexeddb): patch 5 bugs in PR #678 IndexedDB implementation + add tests#686

Closed
Copilot wants to merge 3 commits intomainfrom
copilot/review-indexeddb-implementation
Closed

fix(indexeddb): patch 5 bugs in PR #678 IndexedDB implementation + add tests#686
Copilot wants to merge 3 commits intomainfrom
copilot/review-indexeddb-implementation

Conversation

Copy link

Copilot AI commented Feb 24, 2026

Review and follow-up fixes for the IndexedDB tracking-data storage feature. Five correctness bugs, two leftover debug statements, and missing test coverage addressed.

Bug Fixes

IDBTimelineDatabase.batchSave — two critical bugs

IDBIndex.getAll() called with an array (src/database/timeline-database/idb.ts)
IDBIndex.getAll(query) accepts a single key or IDBKeyRange, not an array. Passing hosts[] always returned zero results, silently defeating all conflict-detection and merge logic — every tick was unconditionally inserted.

// Before (broken — getAll ignores the array, returns [])
const exist = await req2Promise<timer.timeline.Tick[]>(index.getAll(hosts)) ?? []

// After — fetch per-host
for (const host of hosts) {
    const hostTicks = await req2Promise<timer.timeline.Tick[]>(index.getAll(IDBKeyRange.only(host))) ?? []
    if (hostTicks.length) existByHost.set(host, hostTicks)
}

Incorrect merged-interval endstart was mutated before being used to compute duration:

// Before (wrong end: uses modified start + original duration)
start = Math.min(mergeTarget.start, start)
duration = Math.max(mergeTarget.start + mergeTarget.duration, start + duration) - start

// After (capture tickEnd first)
const tickEnd = tick.start + tick.duration
const start = Math.min(mergeTarget.start, tick.start)
const duration = Math.max(mergeTarget.start + mergeTarget.duration, tickEnd) - start

BaseIDBStorage.initDb — missing onblocked handler

Without a handler, a blocked DB upgrade (another tab holds the old version open) hangs indefinitely. Now rejects with an actionable error message.

DB_NAME evaluated at module load time

const DB_NAME = \tt4b_${chrome.runtime.id}`threw in environments wherechrome.runtimeis not yet initialised. Changed to a lazygetDbName()called only insideinitDb`.

IndexedMigrator.onUpdate — migration ran on every update

Classic→IndexedDB migration fired on every extension update regardless of the previous version. Added a major < 4 version guard and a NaN safety check for malformed version strings.

Debug Code Removed

  • console.log(host, start) + // debugger in IDBTimelineDatabase.judgeIndex
  • console.log('Found non-VNode child …') in OptionLines.tsx

Tests Added

  • test/database/timeline-database/idb.test.ts (7 tests) — batchSave conflict skip, correct merge bounds, cross-host independence, empty-list safety; select filters by start/host. Uses fake-indexeddb.
  • test/background/migrator/indexed-migrator.test.ts (6 tests) — migration triggered only for major < 4, skipped for ≥ 4, safe on empty/malformed version strings.
Original prompt

Review PR #678 in sheepzh/time-tracker-4-browser (branch indexeddb -> main) titled "Support IndexedDB to store tracking data".

Tasks:

  1. Pull the PR diff and list of changed files.
  2. Perform a thorough code review focusing on:
    • IndexedDB schema design, versioning, and upgrade/migration logic from previous storage.
    • Data integrity, transaction usage, concurrency/race conditions, batching/debouncing writes.
    • Error handling and fallbacks when IndexedDB is unavailable/quota exceeded/upgrade blocked.
    • Performance considerations (read/write patterns, indexes, large objects).
    • Security/privacy considerations for stored tracking data.
    • Code organization, naming, typing, and maintainability.
    • Tests: coverage, correctness, and missing cases.
  3. Add review comments to the PR:
    • A top-level review summary.
    • Inline comments on specific lines/files where changes are suggested.
  4. If necessary, push small follow-up commits to improve the implementation (tests, bug fixes, refactors) as part of the PR.

Context:

Acceptance criteria:

  • PR has actionable review comments posted.
  • Any proposed code changes compile and tests pass (or add/adjust tests accordingly).

This pull request was created from Copilot chat.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

sheepzh and others added 2 commits February 24, 2026 15:33
- Remove debug console.log and // debugger from IDBTimelineDatabase.judgeIndex
- Fix IDBIndex.getAll(hosts-array) bug: iterate per-host with IDBKeyRange.only()
- Fix batchSave merge-duration calculation (use tickEnd before start is mutated)
- Add onblocked handler in BaseIDBStorage.initDb upgrade path
- Make DB_NAME lazy (getDbName()) so chrome.runtime.id is safe at module load
- Guard IndexedMigrator.onUpdate to only migrate from versions < 4.x
- Fix NaN guard for malformed version strings in IndexedMigrator
- Remove leftover debug console.log from OptionLines.tsx
- Add 7 tests for IDBTimelineDatabase (batchSave & select) using fake-indexeddb
- Add 6 tests for IndexedMigrator version-gated migration logic

Co-authored-by: sheepzh <19428473+sheepzh@users.noreply.github.com>
Copilot AI changed the title [WIP] Review support for IndexedDB to store tracking data fix(indexeddb): patch 5 bugs in PR #678 IndexedDB implementation + add tests Feb 24, 2026
Copilot AI requested a review from sheepzh February 24, 2026 15:56
@sheepzh sheepzh closed this Feb 25, 2026
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.

2 participants