triage: add 7 follow-up tasks for Rust/C pks schema migration

Created comprehensive task cards for remaining work after TASK-147 partial
completion (compound PK bug fix + findPkFromBlob refactor).

NEW TASKS:
- TASK-149: Refactor insertIntoPksTableAndGetPk for new schema
  Priority: HIGH - blocks sync INSERT operations

- TASK-150: Eliminate base_rowid dependency from base table operations
  Priority: HIGH - blocks sync UPDATE/DELETE operations
  Functions: updateBaseTableColumn, deleteFromBaseTable, rowExistsInBaseTable

- TASK-151: Update TableMergeStmts cached statement variants
  Priority: MEDIUM - performance optimization after main refactoring

- TASK-152: Update tombstone handling to use clock table sentinels
  Priority: MEDIUM - correctness for DELETE sync
  Remove base_rowid=NULL logic, use col_name='-1' sentinel markers

- TASK-153: Sweep codebase for remaining old schema references
  Priority: MEDIUM - cleanup task, grep for base_rowid/pks blob

- TASK-154: Fix sync parity test failures
  Priority: HIGH - validation of schema migration work
  Current: 9 rows_impacted tests failing, sync operations broken

- TASK-155: Review insertIntoBaseTable for new schema compatibility
  Priority: HIGH - part of sync INSERT path, may cascade from other tasks

All tasks link back to TASK-147 parent and include:
- Files to Modify (tight scope)
- Acceptance Criteria (testable)
- Cross-links to related work
- Clear dependencies (TASK-149→150→152→154)

NEXT STEP: Execute TASK-149 (insertIntoPksTableAndGetPk) to unblock sync.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>

Author: subtleGradient

.tasks/triage/TASK-149-refactor-insertIntoPksTableAndGetPk.md

@@ -0,0 +1,68 @@
+# TASK-149 — Refactor insertIntoPksTableAndGetPk for new pks schema
+
+## Goal
+Adapt `insertIntoPksTableAndGetPk` and related pks table insert functions to work with the new Rust/C-compatible schema where PK column values are stored directly instead of as a packed blob.
+
+## Status
+- State: triage
+- Priority: high (blocks sync operations)
+
+## Context
+The sync INSERT path currently fails because `insertIntoPksTableAndGetPk()` tries to insert into the old schema:
+
+OLD SCHEMA:
+```sql
+INSERT INTO table__crsql_pks (base_rowid, pks) VALUES (?, ?)
+```
+
+NEW SCHEMA (no base_rowid, no pks blob):
+```sql
+INSERT INTO table__crsql_pks (__crsql_key, pk_col1, pk_col2, ...) VALUES (NULL, ?, ?)
+-- __crsql_key is auto-increment, pk_col values come from unpacked blob
+```
+
+Triggering issue:
+- `findPkFromBlob` was refactored (commit 3b9a984d) and now correctly queries new schema
+- When it returns `NoRows` for non-existent entry, changes_vtab calls INSERT path
+- INSERT path still uses old functions that reference `base_rowid` and `pks` columns
+
+## Files to Modify
+- `zig/src/merge_insert.zig`:
+  - `insertIntoPksTable()` (line ~658)
+  - `insertIntoPksTableAndGetPk()` (line ~671)
+  - `TableMergeStmts.sql_insert_pks` buffer and statement (line ~135)
+
+## Acceptance Criteria
+1. `insertIntoPksTableAndGetPk()` must:
+   - Unpack pk_blob into individual PK column values
+   - Build dynamic INSERT with column names: `INSERT INTO pks (__crsql_key, "col1", "col2") VALUES (NULL, ?, ?)`
+   - Bind unpacked values in PK order
+   - Return the auto-generated `__crsql_key` via `last_insert_rowid()`
+   - NOT reference `base_rowid` or `pks` columns
+
+2. Remove `base_rowid` parameter from function signatures:
+   - OLD: `insertIntoPksTableAndGetPk(db, table, base_rowid, pks_blob, len)`
+   - NEW: `insertIntoPksTableAndGetPk(db, table, pks_blob, len)`
+
+3. Update `TableMergeStmts.sql_insert_pks` to be dynamic per-table (or mark deprecated)
+
+4. Test case passes:
+   ```sql
+   CREATE TABLE foo (a INT NOT NULL PRIMARY KEY, b TEXT);
+   SELECT crsql_as_crr('foo');
+   INSERT INTO crsql_changes VALUES ('foo', X'0901', 'b', 'hello', 1, 1, X'01...10', 1, 0);
+   SELECT * FROM foo; -- row with a=1, b='hello'
+   SELECT * FROM foo__crsql_pks; -- __crsql_key=1, a=1
+   ```
+
+## Parent Docs / Cross-links
+- Parent: TASK-147 (Rust/C pks schema migration - done)
+- Related: commit 3b9a984d (findPkFromBlob refactor)
+- Upstream: `research/zig-cr/92-gap-backlog.md` (schema compatibility)
+- Blocks: TASK-154 (sync parity test fixes)
+
+## Progress Log
+- 2025-12-21: Created from TASK-147 refactoring work. Compound PK bug fixed, findPkFromBlob refactored, this is next critical blocker.
+
+## Completion Notes
+(Empty until done.)

.tasks/triage/TASK-150-eliminate-base-rowid-from-base-table-ops.md

@@ -0,0 +1,85 @@
+# TASK-150 — Eliminate base_rowid dependency from base table operations
+
+## Goal
+Refactor all merge_insert.zig functions that operate on the base table to use PK column values from the pks table instead of relying on a stored `base_rowid` column (which no longer exists in the new schema).
+
+## Status
+- State: triage
+- Priority: high (blocks sync UPDATE/DELETE operations)
+
+## Context
+Multiple functions in merge_insert.zig follow this pattern:
+1. Given `pk` (__crsql_key from pks table)
+2. Look up `base_rowid` from pks table: `SELECT base_rowid FROM pks WHERE pk = ?`
+3. Operate on base table using rowid: `UPDATE table SET col = ? WHERE rowid = base_rowid`
+
+This breaks with new schema because:
+- The pks table has no `base_rowid` column
+- Instead, it has PK column values directly: `(__crsql_key, pk_col1, pk_col2, ...)`
+- Base table operations must use PK columns: `UPDATE table SET col = ? WHERE pk_col1 = ? AND pk_col2 = ?`
+
+Affected functions discovered in Round 49 (TASK-119) and Round 58:
+- `getBaseRowidFromPk()` - entire function obsolete
+- `updateBaseTableColumn()` - uses getBaseRowidFromPk
+- `deleteFromBaseTable()` - uses getBaseRowidFromPk
+- `rowExistsInBaseTable()` - uses getBaseRowidFromPk
+- Cached variants: `*Cached()` versions of above
+
+## Files to Modify
+- `zig/src/merge_insert.zig`:
+  - `getBaseRowidFromPk()` (line ~385) - DELETE or convert to `getPkValuesFromKey()`
+  - `updateBaseTableColumn()` (line ~250)
+  - `deleteFromBaseTable()` (line ~415)
+  - `rowExistsInBaseTable()` (line ~457)
+  - `updateBaseTableColumnCached()` (line ~TBD)
+  - `deleteFromBaseTableCached()` (line ~985)
+  - `rowExistsInBaseTableCached()` (line ~968)
+  - `TableMergeStmts` cached statement buffers (line ~74-78)
+
+## Acceptance Criteria
+1. NEW helper function `getPkValuesFromKey()`:
+   ```zig
+   fn getPkValuesFromKey(db, table_name, __crsql_key, allocator) ![]codec.Value
+   // SELECT pk_col1, pk_col2, ... FROM table__crsql_pks WHERE __crsql_key = ?
+   // Returns unpacked PK column values in order
+   ```
+
+2. `updateBaseTableColumn()` refactored:
+   - Call `getPkValuesFromKey(__crsql_key)` to get PK values
+   - Build SQL: `UPDATE table SET col = ? WHERE "pk1" = ? AND "pk2" = ?`
+   - Bind column value + all PK values
+   - Remove all references to `base_rowid`
+
+3. `deleteFromBaseTable()` refactored:
+   - Get PK values from pks table
+   - Build SQL: `DELETE FROM table WHERE "pk1" = ? AND "pk2" = ?`
+   - Remove pks tombstoning logic (handled by clock table in new schema)
+
+4. `rowExistsInBaseTable()` refactored:
+   - Get PK values from pks table
+   - Build SQL: `SELECT 1 FROM table WHERE "pk1" = ? AND "pk2" = ? LIMIT 1`
+
+5. Test compound PK update:
+   ```sql
+   CREATE TABLE foo(a INT NOT NULL, b INT NOT NULL, c TEXT, PRIMARY KEY(a,b));
+   SELECT crsql_as_crr('foo');
+   -- Sync insert row
+   INSERT INTO crsql_changes VALUES ('foo', X'090109 02', 'c', 'hello', 1, 1, X'01...', 1, 0);
+   -- Sync update column
+   INSERT INTO crsql_changes VALUES ('foo', X'0901
+0902', 'c', 'world', 2, 2, X'01...', 2, 0);
+   SELECT c FROM foo WHERE a=1 AND b=2; -- 'world'
+   ```
+
+## Parent Docs / Cross-links
+- Parent: TASK-147 (Rust/C pks schema migration)
+- Related: TASK-119 (Round 49 - similar bug with pk vs base_rowid confusion)
+- Depends on: TASK-149 (insertIntoPksTableAndGetPk must work first for INSERT path)
+- Upstream: `research/zig-cr/92-gap-backlog.md`
+- Blocks: TASK-154 (sync parity tests)
+
+## Progress Log
+- 2025-12-21: Created from TASK-147 work. Root cause identified: base_rowid column no longer exists, need to query PK values from pks and use in WHERE clauses.
+
+## Completion Notes
+(Empty until done.)

.tasks/triage/TASK-151-update-merge-stmts-cache-for-new-schema.md

@@ -0,0 +1,68 @@
+# TASK-151 — Update TableMergeStmts cached statement variants for new schema
+
+## Goal
+Refactor the `TableMergeStmts` statement cache and all `*Cached()` function variants to work with the new pks schema (no base_rowid, no pks blob, PK columns stored directly).
+
+## Status
+- State: triage
+- Priority: medium (performance optimization; uncached variants work first)
+
+## Context
+`TableMergeStmts` provides per-table statement caching to avoid thousands of prepare/finalize cycles during sync. For a 1000-change sync, this reduces ~4000+ prepares to ~4 per table (significant perf win).
+
+Current cached statements affected by schema change:
+```zig
+struct TableMergeStmts {
+    find_pk_stmt: ?*sqlite3_stmt = null,        // SELECT pk FROM pks WHERE pks = ?
+    row_exists_base_stmt: ?*sqlite3_stmt = null, // SELECT 1 FROM table WHERE rowid = ?
+    delete_base_stmt: ?*sqlite3_stmt = null,     // DELETE FROM table WHERE rowid = ?
+    // ... others
+    sql_find_pk: [512]u8,     // SQL buffer for find_pk
+    sql_insert_pks: [1024]u8, // SQL buffer for insert_pks
+}
+```
+
+Problem: These SQL buffers and statements are built once per table and expect old schema (pks blob, base_rowid). They need dynamic construction per PK column count.
+
+## Files to Modify
+- `zig/src/merge_insert.zig`:
+  - `TableMergeStmts` struct definition (line ~51)
+  - `TableMergeStmts.init()` (line ~100)
+  - `findPkFromBlobCached()` (line ~947)
+  - `rowExistsInBaseTableCached()` (line ~968)
+  - `deleteFromBaseTableCached()` (line ~985)
+  - `updateBaseTableColumnCached()` (if exists)
+
+## Acceptance Criteria
+1. Option A (simple): Mark cached variants as TODO and use uncached:
+   - Document that caching is temporarily disabled pending schema stabilization
+   - All `*Cached()` functions call uncached variants
+   - Remove stale SQL buffers from `TableMergeStmts`
+
+2. Option B (optimal): Implement full caching for new schema:
+   - `TableMergeStmts.init()` takes `TableInfo` parameter
+   - Dynamically build SQL with PK column count:
+     - `find_pk`: `SELECT __crsql_key FROM pks WHERE col1=? AND col2=?`
+     - `row_exists`: `SELECT 1 FROM table WHERE col1=? AND col2=?`
+     - `delete`: `DELETE FROM table WHERE col1=? AND col2=?`
+   - Store prepared statements (still cached, but schema-aware)
+
+3. Performance regression test:
+   - Sync 1000 changes to same table
+   - Verify statement reuse (check prepare count in profiling)
+   - Compare to baseline (should be ~same if caching works)
+
+4. Choose Option A for MVP (unblock sync), Option B for optimization pass
+
+## Parent Docs / Cross-links
+- Parent: TASK-147 (Rust/C pks schema migration)
+- Depends on: TASK-149 (insertIntoPksTableAndGetPk)
+- Depends on: TASK-150 (base table ops refactor)
+- Related: `zig/src/merge_insert.zig:7-17` (statement caching design doc)
+- Upstream: `research/zig-cr/92-gap-backlog.md`
+
+## Progress Log
+- 2025-12-21: Created from TASK-147 work. Statement caching is perf-critical but can be temporarily disabled to unblock schema migration.
+
+## Completion Notes
+(Empty until done.)

.tasks/triage/TASK-152-update-tombstone-handling-clock-sentinels.md

@@ -0,0 +1,91 @@
+# TASK-152 — Update tombstone handling to use clock table sentinels (no base_rowid = NULL)
+
+## Goal
+Remove `base_rowid = NULL` tombstone logic from merge_insert.zig and update to use clock table sentinel markers for deletion tracking, matching Rust/C implementation.
+
+## Status
+- State: triage
+- Priority: medium (correctness for DELETE sync)
+
+## Context
+OLD TOMBSTONE MECHANISM (Zig with base_rowid):
+```sql
+-- Mark row as deleted (tombstone)
+UPDATE table__crsql_pks SET base_rowid = NULL WHERE pk = ?;
+
+-- Check if tombstoned
+SELECT base_rowid FROM table__crsql_pks WHERE pk = ?;
+-- base_rowid IS NULL → tombstoned
+```
+
+NEW TOMBSTONE MECHANISM (Rust/C via clock sentinels):
+- Pks table row is NEVER removed or marked NULL
+- Instead, clock table sentinel with `col_name = '-1'` tracks deletion:
+  ```sql
+  -- Sentinel with even col_version = tombstone (deleted)
+  INSERT INTO clock (key, col_name, col_version, ...) VALUES (pk, '-1', 2, ...);
+
+  -- Sentinel with odd col_version = creation marker (exists)
+  INSERT INTO clock (key, col_name, col_version, ...) VALUES (pk, '-1', 1, ...);
+  ```
+
+Verified in testing:
+```
+Before DELETE: clock has (key=1, col_name='c', col_version=1)
+After DELETE:  clock has (key=1, col_name='-1', col_version=2)
+PKS table still has (key=1, a=1, b=2) unchanged
+```
+
+Current code locations with old tombstone logic:
+- `deleteFromBaseTable()` - sets `base_rowid = NULL` after delete
+- `getBaseRowidFromPk()` - checks `base_rowid IS NULL`
+- `rowExistsInBaseTable()` - relies on getBaseRowidFromPk tombstone check
+
+## Files to Modify
+- `zig/src/merge_insert.zig`:
+  - `deleteFromBaseTable()` (line ~415) - remove pks UPDATE
+  - `deleteFromBaseTableCached()` (line ~985) - same
+  - `getBaseRowidFromPk()` (line ~385) - remove NULL check (or delete function entirely)
+  - `rowExistsInBaseTable()` (line ~457) - check clock sentinel instead
+
+## Acceptance Criteria
+1. `deleteFromBaseTable()` refactored:
+   - Delete from base table using PK WHERE (from TASK-150)
+   - Insert clock sentinel: `INSERT INTO clock (key, col_name, col_version, ...) VALUES (?, '-1', ?, ...)`
+   - Do NOT update pks table (it remains unchanged)
+
+2. `rowExistsInBaseTable()` checks two sources:
+   - Base table query: `SELECT 1 FROM table WHERE pk1=? AND pk2=?`
+   - If no row, check clock sentinel:
+     - `SELECT col_version FROM clock WHERE key=? AND col_name='-1'`
+     - Even col_version → tombstoned (return false)
+     - Odd col_version or no sentinel → exists or never created
+
+3. Remove all `base_rowid = NULL` and `base_rowid IS NULL` references
+
+4. Test tombstone sync:
+   ```sql
+   CREATE TABLE foo(a INT NOT NULL PRIMARY KEY, b TEXT);
+   SELECT crsql_as_crr('foo');
+   -- Insert via sync
+   INSERT INTO crsql_changes VALUES ('foo', X'0901', 'b', 'data', 1, 1, X'01...', 1, 0);
+   SELECT * FROM foo; -- row exists
+   -- Delete via sync (negative cl)
+   INSERT INTO crsql_changes VALUES ('foo', X'0901', '-1', NULL, 2, 2, X'01...', -2, 0);
+   SELECT * FROM foo; -- empty
+   SELECT col_version FROM foo__crsql_clock WHERE col_name='-1'; -- 2 (even = tombstone)
+   SELECT * FROM foo__crsql_pks; -- still has __crsql_key=1, a=1
+   ```
+
+## Parent Docs / Cross-links
+- Parent: TASK-147 (Rust/C pks schema migration)
+- Related: Explore agent findings on tombstone handling (session above)
+- Related: `zig/src/as_crr.zig:472-483` (tombstone design comments)
+- Depends on: TASK-150 (base table ops must work first)
+- Upstream: `research/zig-cr/92-gap-backlog.md`
+
+## Progress Log
+- 2025-12-21: Created from TASK-147 work. Rust/C testing revealed clock sentinel mechanism (col_name='-1', even/odd col_version).
+
+## Completion Notes
+(Empty until done.)

.tasks/triage/TASK-153-sweep-codebase-old-schema-references.md

@@ -0,0 +1,74 @@
+# TASK-153 — Sweep codebase for remaining base_rowid / pks blob references
+
+## Goal
+Systematically search and eliminate all remaining references to the old pks schema (`base_rowid` column, `pks BLOB` column) throughout the Zig codebase.
+
+## Status
+- State: triage
+- Priority: medium (cleanup after main refactoring)
+
+## Context
+After completing TASK-149, TASK-150, and TASK-152, there may be residual references to the old schema scattered across:
+- Comments and documentation
+- Error messages and debug logging
+- Dead code paths
+- Test fixtures and helper functions
+- SQL string literals in other modules
+
+Known locations already refactored:
+- `zig/src/merge_insert.zig` - refactored in TASK-149, 150, 152
+- `zig/src/local_writes/after_write.zig` - uses new schema (getOrCreatePkKey)
+- `zig/src/as_crr.zig` - creates new schema tables
+
+Potential places with residual references:
+- `zig/src/changes_vtab.zig` - may have comments or old code
+- `zig/test/` - test helpers might reference old schema
+- `zig/harness/` - test scripts might have stale SQL
+- Error messages in merge_insert.zig mentioning "base_rowid" or "pks"
+
+## Files to Modify
+(To be determined by grep sweep; keep tight scope)
+- Candidates: Any `.zig`, `.sh`, `.md` files with schema references
+- Likely: `zig/src/changes_vtab.zig`, test files, harness scripts
+
+## Acceptance Criteria
+1. Code sweep (mandatory):
+   ```bash
+   cd zig
+   grep -r "base_rowid" src/ test/ --include="*.zig" | grep -v ".swp"
+   grep -r "pks BLOB" src/ test/ --include="*.zig" | grep -v ".swp"
+   grep -r 'pks = ?' src/ test/ --include="*.zig" | grep -v ".swp"
+   # All hits must be:
+   # - False positives (e.g., "purpose_rowid" not "base_rowid")
+   # - Historical comments explicitly marked OLD SCHEMA
+   # - Or removed/refactored
+   ```
+
+2. Comment audit:
+   - Any comment referencing old schema must be labeled `OLD SCHEMA (pre-TASK-147):`
+   - Or replaced with accurate new schema description
+
+3. Test fixture update:
+   - If any test helper creates manual pks entries, update to new schema
+   - Test data generators use new column structure
+
+4. Error message clarity:
+   - Any error mentioning "pks" should reference "__crsql_pks table" or "PK columns"
+   - No references to "base_rowid lookup" in production error paths
+
+5. Documentation sweep:
+   - Check `zig/src/merge_insert.zig` top-level comments
+   - Check `research/zig-cr/*.md` files for stale schema diagrams
+   - Update or mark obsolete
+
+## Parent Docs / Cross-links
+- Parent: TASK-147 (Rust/C pks schema migration)
+- Depends on: TASK-149, TASK-150, TASK-152 (main refactoring complete)
+- Related: `.tasks/done/TASK-119` (had similar base_rowid confusion)
+- Upstream: `research/zig-cr/04-schema-and-metadata.md` (may need updates)
+
+## Progress Log
+- 2025-12-21: Created from TASK-147 work. Cleanup task to ensure no stale references remain after major refactoring.
+
+## Completion Notes
+(Empty until done.)