Implement data portability Stage 2 verification - #46
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
parle | 14cd181 | Commit Preview URL Branch Preview URL |
Jul 27 2026, 09:55 AM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
parle-personal | 14cd181 | Commit Preview URL Branch Preview URL |
Jul 27 2026, 09:56 AM |
WalkthroughStage 2 durable mirror verification now compares localStorage with IndexedDB, repairs mismatches, records integrity and failure metadata, invalidates stale verification through dirty tokens, runs during app startup, and adds comprehensive tests and deployment documentation. ChangesDurable mirror verification
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant verifyDurableDataMirrors
participant localStorage
participant IndexedDB
App->>verifyDurableDataMirrors: verify both durable mirrors
verifyDurableDataMirrors->>localStorage: read authoritative datasets
verifyDurableDataMirrors->>IndexedDB: read mirror datasets
verifyDurableDataMirrors->>IndexedDB: reconcile discrepancies
verifyDurableDataMirrors->>localStorage: persist verification metadata
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
__tests__/tefArchiveStage1Mirror.test.ts (1)
181-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the prototype spy in a teardown hook, not only inline.
vi.spyOn(IDBDatabase.prototype, 'transaction')mutates shared global state. If an awaited assertion orwaitFor*rejects before the inlinemockRestore()(lines 703, 738), the patched prototype leaks into every subsequent test in the file, turning one failure into a cascade. Same concern forvi.stubGlobal('indexedDB', undefined)at line 775.♻️ Suggested teardown
beforeEach(() => { localStorage.clear(); vi.resetModules(); vi.stubGlobal('indexedDB', new IDBFactory()); }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + });🤖 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 `@__tests__/tefArchiveStage1Mirror.test.ts` around lines 181 - 197, Restore the shared `IDBDatabase.prototype.transaction` spy created by `failReadWriteTransactionsForStore` in an `afterEach` teardown so cleanup runs even when awaited assertions fail; also restore `indexedDB` after tests that call `vi.stubGlobal('indexedDB', undefined)`. Remove reliance on the inline `mockRestore()` calls while preserving the existing failure-injection behavior.services/tefArchiveService.ts (2)
503-546: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
writeFailedVerificationMetadatadrops prior diagnostic counts.On failure, the written metadata only carries
verificationError/lastReconciledAt(fromprevious);mismatchCounts,repairCounts, and integrity counts are all omitted rather than carried forward from the last known-goodpreviousrecord. Not a correctness bug, but a reader inspecting a'failed'record loses visibility into the last observed mismatch/integrity state, which is useful for troubleshooting recurring failures.🤖 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 `@services/tefArchiveService.ts` around lines 503 - 546, The metadata constructed in writeFailedVerificationMetadata must preserve prior diagnostic counts from previous. Carry forward mismatchCounts, repairCounts, and integrity counts from the existing previous metadata when creating the failed record, while retaining the current verificationError, timestamps, and record counts.
362-425: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant full-table reads of
SAVED_ADS_STOREon every topic-archive mirror pass.
inspectRecordIntegrity→findArchivesWithMissingAdsperforms a fullreadAllFromStore(SAVED_ADS_STORE)with no memoization. Within a singlemirrorLatestSourceattempt it's invoked up to 3 times for the topic-archive config: once viacreateShadowComparison(Line 570-574), once forpostRepairIntegrity(Line 589), and once forintegrityAfterMetadata(Line 609). SincemirrorLatestSourcenow runs on every topic-archivesave/delete/shadow-verify(not just background verification), this means every single archive mutation triggers up to 3 full ads-table scans, and each retry attempt (up toMAX_MIRROR_STABILITY_ATTEMPTS) multiplies that further. This will scale poorly as the saved-ads collection grows.Consider fetching the ads collection once per attempt and threading it through the three integrity checks instead of re-reading per call.
♻️ Sketch: memoize per-attempt ads read
-async function findArchivesWithMissingAds(archives: TefTopicArchive[]): Promise<string[]> { - const ads = await readAllFromStore<TefSavedAd>(SAVED_ADS_STORE); - const savedAdIds = new Set(ads.map((ad) => ad.id)); - return archives - .filter((archive) => !savedAdIds.has(archive.adId)) - .map((archive) => archive.id); -} +function findArchivesWithMissingAds(archives: TefTopicArchive[], savedAdIds: Set<string>): string[] { + return archives + .filter((archive) => !savedAdIds.has(archive.adId)) + .map((archive) => archive.id); +}Then in
mirrorLatestSource, fetchsavedAdIdsonce per attempt (whenconfig.inspectRelationshipsapplies) and pass it down throughcreateShadowComparison/inspectRecordIntegrityfor all three call sites within that attempt.Also applies to: 567-624
🤖 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 `@services/tefArchiveService.ts` around lines 362 - 425, Memoize the saved-ad lookup once per mirror attempt instead of rereading SAVED_ADS_STORE in findArchivesWithMissingAds. Update mirrorLatestSource to fetch and pass the saved-ad IDs through createShadowComparison and every inspectRecordIntegrity call, and adjust findArchivesWithMissingAds/relationship inspection to reuse that per-attempt data across all three integrity checks and retries.types.ts (1)
164-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider a discriminated union for stronger verification-state typing.
All Stage 2 fields (
lastVerifiedAt,mismatchCounts,repairCounts, integrity counts) are optional regardless ofverificationStatus, so nothing at the type level prevents a consumer from reading a'verified'record with missing counts (or a'failed'record with stale counts left over). Every current write site (writeVerifiedMirrorMetadata/writeFailedVerificationMetadata) already sets these consistently, so this is not an active bug, but a discriminated union keyed onverificationStatuswould make that invariant compiler-enforced for future writers/readers (e.g. App.tsx).♻️ Sketch of a discriminated union
interface DurableDataMigrationMetadataBase { name: DurableDataMigrationName; version: 1; state: 'mirroring'; lastReconciledAt: number; sourceRecordCount: number; destinationRecordCount: number; } export type DurableDataMigrationMetadata = | (DurableDataMigrationMetadataBase & { verificationStatus: 'verified'; lastVerifiedAt: number; mismatchCounts: DurableDataMismatchCounts; repairCounts: DurableDataRepairCounts; preRepairIntegrityCounts: DurableDataIntegrityCounts; postRepairIntegrityCounts: DurableDataIntegrityCounts; relationshipInvalidRecordCount: number; legacyShapeRecordCount: number; }) | (DurableDataMigrationMetadataBase & { verificationStatus: 'failed'; lastVerifiedAt?: number; verificationError: string; });🤖 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 `@types.ts` around lines 164 - 184, Replace DurableDataMigrationMetadata with a discriminated union keyed by verificationStatus, using a shared base for the common migration fields. Require all verification, mismatch, repair, integrity, and flat post-repair count fields on the 'verified' variant, while the 'failed' variant must require verificationError and may only retain lastVerifiedAt as optional. Update dependent type references as needed while preserving the existing writeVerifiedMirrorMetadata and writeFailedVerificationMetadata contracts.
🤖 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.
Nitpick comments:
In `@__tests__/tefArchiveStage1Mirror.test.ts`:
- Around line 181-197: Restore the shared `IDBDatabase.prototype.transaction`
spy created by `failReadWriteTransactionsForStore` in an `afterEach` teardown so
cleanup runs even when awaited assertions fail; also restore `indexedDB` after
tests that call `vi.stubGlobal('indexedDB', undefined)`. Remove reliance on the
inline `mockRestore()` calls while preserving the existing failure-injection
behavior.
In `@services/tefArchiveService.ts`:
- Around line 503-546: The metadata constructed in
writeFailedVerificationMetadata must preserve prior diagnostic counts from
previous. Carry forward mismatchCounts, repairCounts, and integrity counts from
the existing previous metadata when creating the failed record, while retaining
the current verificationError, timestamps, and record counts.
- Around line 362-425: Memoize the saved-ad lookup once per mirror attempt
instead of rereading SAVED_ADS_STORE in findArchivesWithMissingAds. Update
mirrorLatestSource to fetch and pass the saved-ad IDs through
createShadowComparison and every inspectRecordIntegrity call, and adjust
findArchivesWithMissingAds/relationship inspection to reuse that per-attempt
data across all three integrity checks and retries.
In `@types.ts`:
- Around line 164-184: Replace DurableDataMigrationMetadata with a discriminated
union keyed by verificationStatus, using a shared base for the common migration
fields. Require all verification, mismatch, repair, integrity, and flat
post-repair count fields on the 'verified' variant, while the 'failed' variant
must require verificationError and may only retain lastVerifiedAt as optional.
Update dependent type references as needed while preserving the existing
writeVerifiedMirrorMetadata and writeFailedVerificationMetadata contracts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 54a3c2ad-9bd5-4548-88f8-0902026318f0
📒 Files selected for processing (6)
App.tsx__tests__/tefArchiveStage1Mirror.test.tsdocs/data-portability/README.mddocs/data-portability/stages/02-shadow-verification.mdservices/tefArchiveService.tstypes.ts
1 participant
Summary
Why
Stage 2 must prove that both IndexedDB mirrors are complete and equivalent before a later deployment can make IndexedDB primary. Count-only checks cannot detect records with the same IDs but different content, and stale verified metadata after a failed mirror mutation could incorrectly authorize Stage 3.
Review fixes
verifiedtofailedand successful reconciliation restoresverifiedValidation
NODE_OPTIONS=--no-experimental-webstorage npm test -- --run __tests__/tefArchiveStage1Mirror.test.ts __tests__/tefArchiveService.test.ts __tests__/scenarioService.roadmapSteps.test.ts— 32 tests passedNODE_OPTIONS=--no-experimental-webstorage npm test -- --run— 625 tests passednpm run build— passed (existing large-chunk warning only)Deployment note
This PR intentionally does not switch UI reads to IndexedDB and does not authorize Stage 3. Stage 2 still requires deployment observation and recorded per-dataset results before advancing.
Summary by CodeRabbit
New Features
Documentation