Unify offline rebase and rebaseline pull semantics - #61
Conversation
✅ Deploy Preview for rdlabo-ionic-angular-library ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (page.rebaselineRequired) { | ||
| removeRows.push(...(await this.#confirmedRowsForRebaseline(scope, scopeCommands))); | ||
| } |
There was a problem hiding this comment.
🔴 再ベースライン時に取得したばかりの最新データが同じ処理内で消えてしまう
サーバーからの作り直し指示を受けたとき、削除対象に選んだ古いデータの一覧に、同じ取り込みで書き直された最新データも含まれたまま渡される（removeRows.push(...) at projects/kit/offline/src/lib/offline-replica-pull.service.ts:85）ため、書き込みの後に削除が行われて最新データが消えます。
Impact: 再取得直後に本来残るはずの既存データや派生表示用データが端末から消え、しおり位置（カーソル）は進むため再取得もされません。
書き込み順序（putRows→removeRows）と鍵の衝突
#confirmedRowsForRebaseline は syncState === 'confirmed' の全行を削除対象として収集します（projects/kit/offline/src/lib/offline-replica-pull.service.ts:228-243）。その後の変更適用ループでは、同じ行が更新対象になると putRows に {...existing, identity: existing.identity} として積まれます。最終的に両者が同一トランザクションへ渡されますが（projects/kit/offline/src/lib/offline-replica-pull.service.ts:210-216）、Web 実装は putRows を先に適用してから removeRows を削除し（projects/kit/offline/src/lib/offline-repository.ts:678-689）、SQLite 実装も同様の順序です（projects/kit/offline/src/lib/sqlite-offline-repository.ts:411-420）。そのため、スナップショットページで再確認された既存行、および projector が再構築した既存の localOnly 派生行（これらも confirmed なら削除対象に入る）が削除されます。テストでは削除対象 remoteId=10 と新規 remoteId=42、新規 view-42 を使っているため衝突が起きず検出されません。
修正案としては、putRows（projector 分を含む）に含まれる行キーを removeRows から除外する、あるいはリポジトリ側で remove を put より前に適用することが考えられます。
Prompt for agents
再ベースライン（rebaselineRequired）で収集した confirmed 行の削除リストと、同じページで書き直される putRows（projector 由来の localOnly 行を含む）が同一の行キーで衝突する。リポジトリ実装（IonicOfflineRepository / SqliteOfflineRepository）はいずれも transactReplica 内で putRows を先に適用してから removeRows を削除するため、最新化されたばかりの行が削除されてしまう。OfflineReplicaPullService.pull で最終的な putRows を組み立てた後に、その行キー集合に含まれるものを removeRows から除外する（キー比較は canonicalOfflineReplicaIdentity など既存の正規化と同じ方式を使う）か、リポジトリ側で remove→put の順に変更することを検討する。既存テストは衝突しないデータを使っているため、同一 identity が snapshot ページで更新されるケースの回帰テストも追加すること。
Was this helpful? React with 👍 or 👎 to provide feedback.
| #rowKey(row: OfflineReplicaRowKey): string { | ||
| return `${row.userId}:${row.scopeId}:${row.sourceKey}:${JSON.stringify(row.identity)}`; | ||
| } |
There was a problem hiding this comment.
🔴 未送信の変更に紐づく派生データが再ベースラインで誤って削除される
未送信の変更が守るべき派生データかどうかを判定する鍵（JSON.stringify(row.identity) at projects/kit/offline/src/lib/offline-replica-pull.service.ts:246）が、既存実装の正規化方式と異なり付随情報まで含めて比較するため、同じデータでも別物とみなされて削除されます。
Impact: 送信待ちの操作に伴って表示していた関連データが再取得時に消え、画面表示が壊れます。
identity 正規化の不一致
コードベース全体では行 identity の同一性は canonicalOfflineReplicaIdentity（projects/kit/offline/src/lib/offline-identity.ts:73-83）で判定され、generated identity は localId のみで比較され remoteId は無視されます。しかし新規の #rowKey（projects/kit/offline/src/lib/offline-replica-pull.service.ts:245-247）は JSON.stringify(row.identity) を使うため、{kind:'generated', localId:'x', remoteId:null} として保存されたコマンドの companion キーと、その後 remoteId が割り当てられたリポジトリ上の行が一致せず、保全集合から漏れて削除されます。さらに JSON 文字列化はプロパティ順やキー順の差にも依存します。offline-sync.service.ts:680-682 の #replicaRowKey と同じ正規化方式に揃えるべきです。
| #rowKey(row: OfflineReplicaRowKey): string { | |
| return `${row.userId}:${row.scopeId}:${row.sourceKey}:${JSON.stringify(row.identity)}`; | |
| } | |
| #rowKey(row: OfflineReplicaRowKey): string { | |
| return `${canonicalOfflinePrincipalId(row.userId)}:${row.scopeId}:${row.sourceKey}:${canonicalOfflineReplicaIdentity(row.identity)}`; | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (page.rebaselineRequired) { | ||
| removeRows.push(...(await this.#confirmedRowsForRebaseline(scope, scopeCommands))); | ||
| } |
There was a problem hiding this comment.
🟡 別パーティションの未送信操作に紐づく共有データが再ベースラインで消える
再ベースラインで守るべきデータを決めるとき、現在の区画の未送信操作だけを参照している（#confirmedRowsForRebaseline(scope, scopeCommands) at projects/kit/offline/src/lib/offline-replica-pull.service.ts:85）ため、利用者単位で共有されるデータに紐づく他区画の未送信操作分が守られず削除されます。
Impact: 別の区画で行った未送信の操作に伴う表示データが、再取得時に消えます。
scopeCommands と userCommands の使い分け
同じ pull 処理内では、schema.scope === 'user' のエンティティに対しては userCommands（getCommandsForUser）を使ってコマンドを突き合わせています（projects/kit/offline/src/lib/offline-replica-pull.service.ts:71-81）。一方 #confirmedRowsForRebaseline に渡されるのは scopeCommands のみです。また削除候補は getReplicaRows(scope, entity.sourceKey) により user スコープのエンティティでは全区画の行が返るため、他区画のコマンドが持つ optimisticCompanions 行も削除対象に入り得ます。user スコープのエンティティについては userCommands を保全集合に含めるべきです。
Was this helpful? React with 👍 or 👎 to provide feedback.
| /** The server can no longer continue this cursor and requires a confirmed-state snapshot rebuild. */ | ||
| rebaselineRequired?: boolean; |
There was a problem hiding this comment.
🟡 サーバー応答の変換関数を通すと作り直し指示を型として渡せない
作り直し指示の項目が取り込みページの型にだけ追加され、バックエンド応答用の型には追加されていない（rebaselineRequired?: boolean at projects/kit/offline/src/lib/offline-replica-puller.ts:57）ため、共有の変換関数を使う製品はこの指示を型安全に伝えられません。
Impact: 共通変換を利用しているアプリでは、サーバーからの作り直し指示を渡すのに型回避が必要になります。
OfflineReplicaWirePullPage との不整合
normalizeOfflineReplicaPullPage（projects/kit/offline/src/lib/offline-replica-puller.ts:89-104）は OfflineReplicaWirePullPage を受け取り OfflineReplicaPullPage を返しますが、入力側インターフェース（projects/kit/offline/src/lib/offline-replica-puller.ts:77-83）には rebaselineRequired がありません。実行時はスプレッドで通過しますが、型上は宣言できないため、as unknown などのキャストが必要になります。入力型にも省略可能な rebaselineRequired を追加するのが自然です。
Was this helpful? React with 👍 or 👎 to provide feedback.
rdlabo
changed the title
1 participant
Summary
Why
Conflict rebase and cursor rebaseline both modify the same pull transaction. Shipping them independently would leave an untested merge boundary. This stacked PR includes #59 and proves both invariants together: revision-sensitive writes still conflict, commutative intent may be safely replayed, and an expired cursor cannot split confirmed cleanup from the first snapshot commit.
Verification