From a69a18643df3c5c4feb0b17310fc4c527157b4ef Mon Sep 17 00:00:00 2001 From: Anthony Miller Date: Thu, 28 May 2026 16:19:17 -0700 Subject: [PATCH 1/2] =?UTF-8?q?docs(cache):=20rewrite=20=C2=A77.1,=20?= =?UTF-8?q?=C2=A77.2,=20=C2=A77.3=20and=20amend=20=C2=A78=20PR-009=20per?= =?UTF-8?q?=20ADR=200006?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0006 (#1002) ratified the in-place row-per-element layout with a `position` column extending the records-table PK. This commit propagates that decision into the engineering plan and execution plan. cache-rewrite-phase1-plan.md changes: - §7.1 DDL: removes `list_value TEXT`; adds `position INTEGER NOT NULL DEFAULT -1`; PK becomes `(cache_key, field_name, position)`. Adds a brief preamble explaining the row-per-element semantics and citing ADR 0006. Updates `child_key_value` comment to acknowledge its dual use for nested-list sub-record indirection. Schema version bumped from 3 to 4. - §7.2 Operations: `selectRecords` now orders by `(cache_key, field_name, position)` and the decoder branches on `position` to dispatch scalar vs list-element rows. `addOrUpdate` produces one row per scalar field and N rows per list-typed field; list-element rows for a field are rewritten atomically. `deleteRecord` handles the depth ≥ 2 cascading reachability walk for nested-list sub-records. - §7.3 Migration: trigger updated from "< 3" to "< 4". Adds a sentence noting that the trigger absorbs both 2.x upgrades and any local-dev databases on v3; v3 never tagged externally. cache-rewrite-phase1-execution.md changes: - §8 PR-009 entry: title updated to "feat(sqlite): row-per-element CRUD with position-keyed schema"; tests-required rewritten to cover position-keyed CRUD, nested-list recursion at depth ≥ 2, atomic list-element rewrite, and the v3 → v4 schema version bump. Adds a note that this PR supersedes the prior "JSON list_value" scope but retains the row-per-field harness landed in the original implementation. LoC estimate bumped from ~600 to ~700, which crosses execution plan §6 trigger 1's 600-LoC threshold. Reviewer awareness is flagged in the PR description; the amended scope is genuinely larger than the original because it covers the schema update, the nested-list recursion, and the atomic-rewrite contract. If the actual diff materially exceeds this estimate, PR-009 should escalate per §6 trigger 1. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Design/cache-rewrite-phase1-execution.md | 2 +- .../Design/cache-rewrite-phase1-plan.md | 24 +++++++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/apollo-ios/Design/cache-rewrite-phase1-execution.md b/apollo-ios/Design/cache-rewrite-phase1-execution.md index aef90d45c..818fa23f7 100644 --- a/apollo-ios/Design/cache-rewrite-phase1-execution.md +++ b/apollo-ios/Design/cache-rewrite-phase1-execution.md @@ -251,7 +251,7 @@ Goal: ship 3.0-alpha at end of this phase. No behavior change for end users. Pub | PR-006 | refactor(cache): change `Record.fields` type to `[CacheKey: CachedField]` | ⬜ | PR-005 | ~400 | Update existing Record/RecordSet tests; verify `record[key]` subscript still returns `Value?` for all existing call sites | | PR-007 | feat(sqlite): add `schema_metadata` table and version detection | ⬜ | PR-006 | ~150 | Unit: schema-version read/write, missing-row defaults to 0, version stamping on init | | PR-008 | feat(sqlite): new schema DDL — records table with composite PK + typed columns | ⬜ | PR-007 | ~200 | Unit: table creation idempotent, `WITHOUT ROWID` preserved, schema_metadata version=3 stamped | -| PR-009 | feat(sqlite): implement insert/select/update/delete on new table (feature-flagged) | ⬜ | PR-008 | ~600 | Unit: each operation against new schema; round-trip Record↔rows; transactional behavior on failure; performance smoke test | +| PR-009 | feat(sqlite): row-per-element CRUD with position-keyed schema | ⬜ | PR-008 | ~700 | Unit: each operation against the position-keyed schema per [ADR 0006](./adr/0006-list-storage-strategy.md); round-trip Record↔rows for scalar and list-typed fields; nested-list synthetic sub-record recursion at depth ≥ 2; atomic list-element rewrite (no partial-list states); schema version bumped from 3 to 4; performance smoke test. Note: this PR supersedes the original "JSON list_value" scope; the row-per-field CRUD harness from the prior implementation is retained, the JSON list-encoding branches in `SQLiteFieldEncoding.swift` are replaced. | | PR-010 | feat(sqlite): switch `SQLiteNormalizedCache` to new schema; drop-and-rebuild migration | ⬜ | PR-009 | ~400 | Unit: migration on detected old schema; integration: existing cache tests pass on new schema; CachePersistenceTests updated | | PR-011 | test(cache): SQLite performance-gate harness on iPhone 16 Pro | ⬜ | PR-010 | ~200 | Performance test asserting all §7.4 gates within 25% margin | | PR-011a | feat(cache): comprehensive performance measurement harness (Tier 1 + Tier 2) | ⬜ | PR-011 | ~700 | Unit: each Tier 1 and Tier 2 scenario runs cleanly; JSON exporter produces well-formed output; harness is re-runnable across versions | diff --git a/apollo-ios/Design/cache-rewrite-phase1-plan.md b/apollo-ios/Design/cache-rewrite-phase1-plan.md index bfac82c64..29c0fc339 100644 --- a/apollo-ios/Design/cache-rewrite-phase1-plan.md +++ b/apollo-ios/Design/cache-rewrite-phase1-plan.md @@ -281,19 +281,21 @@ public enum Source: Sendable { ### 7.1 New schema (DDL) +The schema is row-per-element: each scalar field is one row and each list element is one row, all in the same `records` table. The `position` column distinguishes them — `-1` for scalars, `0..N-1` for list elements — and is part of the primary key. List-element rows live at the same `cache_key` as their parent record's scalar fields and cluster physically next to them on disk via `WITHOUT ROWID`. Per [ADR 0006](./adr/0006-list-storage-strategy.md), nested lists (`[[T]]`) recurse via `child_key_value` indirection to synthetic sub-records that themselves use the same layout. + ```sql CREATE TABLE IF NOT EXISTS records ( cache_key TEXT NOT NULL, field_name TEXT NOT NULL, + position INTEGER NOT NULL DEFAULT -1, -- -1 = scalar; 0..N-1 = list element int_value INTEGER, string_value TEXT, float_value REAL, bool_value INTEGER, - list_value TEXT, -- JSON-encoded list - child_key_value TEXT, -- cache reference + child_key_value TEXT, -- cache reference (or synthetic sub-record key for nested lists; see ADR 0006) custom_scalar_value TEXT, -- JSON-encoded written_at INTEGER NOT NULL, - PRIMARY KEY (cache_key, field_name) + PRIMARY KEY (cache_key, field_name, position) ) WITHOUT ROWID; ``` @@ -304,14 +306,16 @@ CREATE TABLE IF NOT EXISTS schema_metadata ( key TEXT PRIMARY KEY, value TEXT ); --- on init: INSERT OR REPLACE INTO schema_metadata VALUES ('version', '3'); +-- on init: INSERT OR REPLACE INTO schema_metadata VALUES ('version', '4'); ``` +The version bump from 3 to 4 corresponds to ADR 0006's adoption of the row-per-element layout. v3 (the JSON `list_value` shape predecessor) was never tagged externally; the bump exists to give any local dev databases that ran the v3 shape on the plan branch a clean rebuild path on next launch. + ### 7.2 Operations -- `selectRecords(forKeys:)` — single `SELECT … WHERE cache_key IN (?, ?, …) ORDER BY cache_key, field_name`. Reassembles into `Record` instances by grouping by `cache_key` in Swift. Composite-PK clustering ensures rows for one record arrive contiguous in the result set. -- `addOrUpdate(records:)` — shreds each `Record.fields` into N row UPSERTs in one transaction. Each row carries its `written_at`. -- `deleteRecord(for:)` — `DELETE FROM records WHERE cache_key = ?`. +- `selectRecords(forKeys:)` — single `SELECT … WHERE cache_key IN (?, ?, …) ORDER BY cache_key, field_name, position`. Reassembles into `Record` instances by grouping by `cache_key` in Swift; the decoder branches on `position` to dispatch scalar rows (`position = -1`) and list-element rows (`position >= 0`, accumulated in order). Composite-PK clustering ensures rows for one record arrive contiguous in the result set, with scalar and list-element rows for a given field arriving as a contiguous run. +- `addOrUpdate(records:)` — shreds each `Record.fields` into row UPSERTs in one transaction. Scalar fields produce one row at `position = -1`; list-typed fields produce N rows at `position = 0..N-1`. Each row carries its `written_at`. List-element rows for a field are rewritten atomically — an update to a list-typed field deletes the existing element rows and inserts the new ones in the same transaction, so partial-list states are not observable. +- `deleteRecord(for:)` — `DELETE FROM records WHERE cache_key = ?`. Scalar rows and depth-1 list-element rows delete in the same statement (they share the cache_key). Nested-list sub-records at depth ≥ 2 live at synthetic keys (`.[N]` per [ADR 0006](./adr/0006-list-storage-strategy.md)) and require a small cascading reachability walk that follows `child_key_value` columns. - `deleteRecords(matching:)` — unchanged semantics (`WHERE cache_key LIKE ? COLLATE NOCASE`). - `clearDatabase` — unchanged. @@ -320,10 +324,10 @@ CREATE TABLE IF NOT EXISTS schema_metadata ( On `init`, after `createRecordsTableIfNeeded`: 1. Read `schema_metadata` for the version. -2. If version is missing or `< 3`, drop and recreate the records table; insert the new version. -3. If version is `3`, no migration needed. +2. If version is missing or `< 4`, drop and recreate the records table; insert the new version. +3. If version is `4`, no migration needed. -The drop-and-rebuild is silent — no user-visible event other than the network fetches that follow on cache-miss reads. +The drop-and-rebuild is silent — no user-visible event other than the network fetches that follow on cache-miss reads. The migration trigger absorbs both genuine upgrades from 2.x (no `schema_metadata` row at all) and local-dev databases that ran the v3 shape on the plan branch before [ADR 0006](./adr/0006-list-storage-strategy.md) (version row reads `3`). v3 never tagged externally, so no end-user installation is on v3. ### 7.4 Performance gates From 9550491504223bbe00f60b9d74bbf008934fa39d Mon Sep 17 00:00:00 2001 From: Anthony Miller Date: Fri, 29 May 2026 09:26:25 -0700 Subject: [PATCH 2/2] docs(cache): split PR-009 amendment into PR-008b (DDL) + narrowed PR-009 (CRUD) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior commit collapsed both the DDL replacement and the position-aware CRUD work into PR-009's amended scope, bumping its LoC estimate to ~700 — across the §6 trigger 1 (600 LoC) threshold. That also conflated work that conceptually belongs to two different phase slots: PR-008's theme (records-table DDL) and PR-009's theme (CRUD on that DDL). This commit splits the work back into two PRs matching the existing sub-ID convention used by PR-004a, PR-004b, PR-011a, PR-011b, PR-022a (sub-IDs extend a parent PR's theme with additional related work): - New PR-008b: replaces the records DDL with the position-keyed v4 schema per ADR 0006. Drops/recreates the records table; bumps schema_metadata version 3 → 4; updates the migration trigger to `< 4`. ~150 LoC. Base: PR-008. - Narrowed PR-009: position-aware CRUD against PR-008b's DDL. No longer carries the schema version bump. Back to ~600 LoC, under the §6 trigger 1 threshold. Base: PR-008b. Phase 1A header bumped from "(10 PRs)" to "(11 PRs)". The "Phase 1A done" condition on line 331 (PR-005 through PR-012 merged) still covers PR-008b — sub-IDs land within the range. The convention precedent (sub-IDs extend, don't replace) is the deciding factor for naming this PR-008b rather than PR-009b: PR-008b *adds* a DDL replacement to PR-008's theme; PR-009b would *replace* PR-009's planned output, which none of the existing sub-IDs do. Co-Authored-By: Claude Opus 4.7 (1M context) --- apollo-ios/Design/cache-rewrite-phase1-execution.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apollo-ios/Design/cache-rewrite-phase1-execution.md b/apollo-ios/Design/cache-rewrite-phase1-execution.md index 818fa23f7..db8390e6c 100644 --- a/apollo-ios/Design/cache-rewrite-phase1-execution.md +++ b/apollo-ios/Design/cache-rewrite-phase1-execution.md @@ -241,7 +241,7 @@ Phase 0 also produces two spike branches that are **not merged**: Findings from each spike are captured in their respective Phase 0 ADRs (PR-003 references the SQLite spike; the cachecontrol-jsdirective spike findings become a `cache-rewrite/phase-0-adr-cachecontrol-spike` PR if material surprises surface — otherwise findings live as a comment thread on the existing ADR). -### Phase 1A — SQLite schema rewrite + field-aware `Record` (10 PRs) +### Phase 1A — SQLite schema rewrite + field-aware `Record` (11 PRs) Goal: ship 3.0-alpha at end of this phase. No behavior change for end users. Published performance dataset accompanies the alpha tag. @@ -251,7 +251,8 @@ Goal: ship 3.0-alpha at end of this phase. No behavior change for end users. Pub | PR-006 | refactor(cache): change `Record.fields` type to `[CacheKey: CachedField]` | ⬜ | PR-005 | ~400 | Update existing Record/RecordSet tests; verify `record[key]` subscript still returns `Value?` for all existing call sites | | PR-007 | feat(sqlite): add `schema_metadata` table and version detection | ⬜ | PR-006 | ~150 | Unit: schema-version read/write, missing-row defaults to 0, version stamping on init | | PR-008 | feat(sqlite): new schema DDL — records table with composite PK + typed columns | ⬜ | PR-007 | ~200 | Unit: table creation idempotent, `WITHOUT ROWID` preserved, schema_metadata version=3 stamped | -| PR-009 | feat(sqlite): row-per-element CRUD with position-keyed schema | ⬜ | PR-008 | ~700 | Unit: each operation against the position-keyed schema per [ADR 0006](./adr/0006-list-storage-strategy.md); round-trip Record↔rows for scalar and list-typed fields; nested-list synthetic sub-record recursion at depth ≥ 2; atomic list-element rewrite (no partial-list states); schema version bumped from 3 to 4; performance smoke test. Note: this PR supersedes the original "JSON list_value" scope; the row-per-field CRUD harness from the prior implementation is retained, the JSON list-encoding branches in `SQLiteFieldEncoding.swift` are replaced. | +| PR-008b | feat(sqlite): replace records DDL with position-keyed v4 schema per [ADR 0006](./adr/0006-list-storage-strategy.md) | ⬜ | PR-008 | ~150 | Unit: new table creation idempotent with extended PK `(cache_key, field_name, position)` and `position INTEGER NOT NULL DEFAULT -1`; `WITHOUT ROWID` preserved; schema_metadata version=4 stamped; migration trigger updated to drop on `< 4` and absorbs the v3 → v4 rebuild path; PR-007/PR-008 tests updated for the new column shape and PK | +| PR-009 | feat(sqlite): row-per-element CRUD against position-keyed schema | ⬜ | PR-008b | ~600 | Unit: each operation against the position-keyed schema per [ADR 0006](./adr/0006-list-storage-strategy.md); round-trip Record↔rows for scalar and list-typed fields (position-aware encoder + decoder); nested-list synthetic sub-record recursion at depth ≥ 2; atomic list-element rewrite (no partial-list states); performance smoke test. Note: this PR supersedes the original "JSON list_value" scope; the row-per-field CRUD harness from the prior implementation is retained, the JSON list-encoding branches in `SQLiteFieldEncoding.swift` are replaced. | | PR-010 | feat(sqlite): switch `SQLiteNormalizedCache` to new schema; drop-and-rebuild migration | ⬜ | PR-009 | ~400 | Unit: migration on detected old schema; integration: existing cache tests pass on new schema; CachePersistenceTests updated | | PR-011 | test(cache): SQLite performance-gate harness on iPhone 16 Pro | ⬜ | PR-010 | ~200 | Performance test asserting all §7.4 gates within 25% margin | | PR-011a | feat(cache): comprehensive performance measurement harness (Tier 1 + Tier 2) | ⬜ | PR-011 | ~700 | Unit: each Tier 1 and Tier 2 scenario runs cleanly; JSON exporter produces well-formed output; harness is re-runnable across versions |