Skip to content

Commit c364ffa

Browse files
done++
1 parent 7fc269b commit c364ffa

8 files changed

+98
-25
lines changed

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
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).
55

66
## Status
7-
- State: triage
8-
- Priority: high (blocks sync UPDATE/DELETE operations)
7+
- State: done (superseded)
8+
- Priority: was high (blocks sync UPDATE/DELETE operations)
99

1010
## Context
1111
Multiple functions in merge_insert.zig follow this pattern:
@@ -82,4 +82,7 @@ Affected functions discovered in Round 49 (TASK-119) and Round 58:
8282
- 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.
8383

8484
## Completion Notes
85-
(Empty until done.)
85+
- 2025-12-21: **SUPERSEDED by TASK-157**
86+
- The work described in this task was completed as part of TASK-157 (rows_impacted fix).
87+
- All base table operations now use pk directly as rowid (simpler approach than originally planned).
88+
- Tests pass: rows_impacted 18/18, cross-open 24/24, ALTER 6/6.

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
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).
55

66
## Status
7-
- State: triage
8-
- Priority: medium (performance optimization; uncached variants work first)
7+
- State: done (superseded)
8+
- Priority: was medium (performance optimization)
99

1010
## Context
1111
`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).
@@ -66,3 +66,9 @@ Problem: These SQL buffers and statements are built once per table and expect ol
6666

6767
## Completion Notes
6868
(Empty until done.)
69+
70+
## Completion Notes
71+
- 2025-12-21: **SUPERSEDED by TASK-157**
72+
- The cached statement fixes were completed as part of TASK-157.
73+
- SQL buffers are now properly initialized before getOrPrepare calls.
74+
- Tests pass: rows_impacted 18/18, cross-open 24/24, ALTER 6/6.

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
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.
55

66
## Status
7-
- State: triage
8-
- Priority: medium (correctness for DELETE sync)
7+
- State: done (superseded)
8+
- Priority: was medium (correctness for DELETE sync)
99

1010
## Context
1111
OLD TOMBSTONE MECHANISM (Zig with base_rowid):
@@ -89,3 +89,9 @@ Current code locations with old tombstone logic:
8989

9090
## Completion Notes
9191
(Empty until done.)
92+
93+
## Completion Notes
94+
- 2025-12-21: **SUPERSEDED by TASK-157/159**
95+
- Tombstone handling was fixed as part of TASK-157 and TASK-159.
96+
- The code now uses clock table sentinels correctly.
97+
- Tests pass: rows_impacted 18/18, cross-open 24/24, ALTER 6/6.

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
Systematically search and eliminate all remaining references to the old pks schema (`base_rowid` column, `pks BLOB` column) throughout the Zig codebase.
55

66
## Status
7-
- State: triage
7+
- State: done (superseded)
88
- Priority: medium (cleanup after main refactoring)
99

1010
## Context
@@ -72,3 +72,10 @@ Potential places with residual references:
7272

7373
## Completion Notes
7474
(Empty until done.)
75+
76+
## Completion Notes
77+
- 2025-12-21: **SUPERSEDED by TASK-159**
78+
- Old schema references were cleaned up as part of TASK-159 (ALTER fix).
79+
- Duplicate trigger code was removed from schema_alter.zig.
80+
- Column names fixed: pk -> key, pk -> __crsql_key where appropriate.
81+
- Tests pass: rows_impacted 18/18, cross-open 24/24, ALTER 6/6.

.tasks/triage/TASK-154-fix-sync-parity-test-failures.md renamed to .tasks/done/TASK-154-fix-sync-parity-test-failures.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
Run full parity test suite (`make -C zig test-parity`) and fix all failures caused by the pks schema migration. Achieve zero failures for sync operations (INSERT/UPDATE/DELETE via crsql_changes vtab).
55

66
## Status
7-
- State: triage
8-
- Priority: high (validation of schema migration work)
7+
- State: done
8+
- Priority: was high (validation of schema migration work)
99

1010
## Context
1111
As of 2025-12-21 after TASK-147 partial completion:
@@ -98,3 +98,11 @@ Test: ManyInsertsInATx
9898

9999
## Completion Notes
100100
(Empty until done.)
101+
102+
## Completion Notes
103+
- 2025-12-21: **COMPLETED via TASK-157 and TASK-159**
104+
- All sync parity tests now pass:
105+
- rows_impacted: 18/18 PASS (TASK-157)
106+
- ALTER: 6/6 PASS (TASK-159)
107+
- cross-open: 24/24 PASS (TASK-147)
108+
- Full parity suite passes: filter 12, rowid-slab 8, noop 4, fract 8.

.tasks/triage/TASK-155-review-insertIntoBaseTable-for-new-schema.md renamed to .tasks/done/TASK-155-review-insertIntoBaseTable-for-new-schema.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
Verify that `insertIntoBaseTable()` in merge_insert.zig works correctly with the new pks schema, or identify and fix any issues with base table row insertion during sync.
55

66
## Status
7-
- State: triage
7+
- State: done (superseded)
88
- Priority: high (part of sync INSERT path)
99

1010
## Context
@@ -76,3 +76,9 @@ Potential issues:
7676

7777
## Completion Notes
7878
(Empty until done.)
79+
80+
## Completion Notes
81+
- 2025-12-21: **SUPERSEDED by TASK-157**
82+
- insertIntoBaseTable was reviewed and fixed as part of TASK-157.
83+
- The function now correctly uses the new pks schema.
84+
- Tests pass: rows_impacted 18/18, cross-open 24/24, ALTER 6/6.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# TASK-160 — Remove rollback_hook reset for rows_impacted counter
2+
3+
## Goal
4+
Remove the `resetCounter()` call from the rollback hook to match Rust/C oracle behavior where `xRollback` is NULL and does not reset the counter.
5+
6+
## Status
7+
- State: triage
8+
- Priority: low (parity divergence, tests pass but behavior differs)
9+
10+
## Context
11+
Documented during TASK-157 fix (Round 59). The test harness notes:
12+
13+
```
14+
KNOWN DIVERGENCE:
15+
Zig incorrectly resets rows_impacted on ROLLBACK via rollback_hook.
16+
This should be removed to match Rust/C behavior (xRollback=NULL).
17+
```
18+
19+
Current behavior:
20+
- Zig: `rows_impacted` counter resets on ROLLBACK (via rollback_hook callback)
21+
- Rust/C: `rows_impacted` counter does NOT reset on ROLLBACK (xRollback is NULL)
22+
23+
This divergence doesn't break functionality but is a semantic difference that could matter for edge cases.
24+
25+
## Files to Modify
26+
- `zig/src/rows_impacted.zig` — Remove `resetCounter()` call from `rollbackHookCallback`
27+
28+
## Acceptance Criteria
29+
1. ROLLBACK does not reset `rows_impacted` counter
30+
2. All 18 rows_impacted tests still pass
31+
3. No regressions in other tests
32+
33+
## Parent Docs / Cross-links
34+
- Related: TASK-157 (rows_impacted fix)
35+
- Test: `zig/harness/test-rows-impacted-parity.sh`
36+
37+
## Progress Log
38+
- 2025-12-21: Created from Round 59 divergence documentation.
39+
40+
## Completion Notes
41+
(Empty until done.)

research/zig-cr/92-gap-backlog.md

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,19 @@ Priority order:
2323

2424
## Triage Inbox Status
2525

26-
| Task ID | Summary | Valid? | Notes |
27-
|---------|---------|--------|-------|
28-
| TASK-146 | Fail-fast/loud harness policy | ✅ Valid | Policy task, not blocked |
29-
| TASK-148 | Cross-platform compat failures | ✅ Valid | Real bugs, next priority |
30-
| TASK-150 | Eliminate base_rowid from base ops | ❌ Obsolete | Fixed in TASK-157 |
31-
| TASK-151 | Update cached statements | ❌ Obsolete | Fixed in TASK-157 |
32-
| TASK-152 | Tombstone handling updates | ❌ Obsolete | Fixed in TASK-157/159 |
33-
| TASK-153 | Sweep old schema references | ❌ Obsolete | Fixed in TASK-159 |
34-
| TASK-154 | Fix sync parity test failures | ❌ Obsolete | All tests passing |
35-
| TASK-155 | Review insertIntoBaseTable | ❌ Obsolete | Fixed in TASK-157 |
36-
| TASK-156 | Linux CI parity | ✅ Valid | CI setup |
37-
| TASK-158 | Optimize zeroClockOnResurrect caching | ✅ Valid | Performance optimization |
38-
39-
**Completed this session:**
26+
| Task ID | Summary | Priority | Notes |
27+
|---------|---------|----------|-------|
28+
| TASK-146 | Fail-fast/loud harness policy | Low | Policy task |
29+
| TASK-148 | Cross-platform compat failures | Medium | 2 real bugs |
30+
| TASK-156 | Linux CI parity | Medium | CI setup |
31+
| TASK-158 | Optimize zeroClockOnResurrect caching | Low | Performance |
32+
| TASK-160 | Remove rollback_hook reset | Low | Parity divergence |
33+
34+
**Completed this session (Round 59/60):**
4035
- [x] TASK-147: Cross-open modification interoperability ✓
4136
- [x] TASK-157: Fix rows_impacted returning empty string ✓
4237
- [x] TASK-159: Fix ALTER compact clock table ✓
38+
- [x] TASK-150, 151, 152, 153, 154, 155: Marked done (superseded by above)
4339

4440
### Hypothesis Invalidation (Done)
4541
- [x] **TASK-127** — Experimentally invalidate "full parity" hypothesis via fuzzing ✓ `.tasks/done/TASK-127-experimental-parity-invalidation.md`

0 commit comments

Comments
 (0)