Skip to content

Commit 8314907

Browse files
subtleGradientsubtleGradient
committed
Round 67: fix db_version savepoint bug + trigger CRR tests
TASK-181: Fix db_version not advancing with savepoint + crsql_changes - Root cause: setWinnerClock/setWinnerClockCached wrote clock entries but never updated pending_db_version, so commit hook didn't advance global - Fix: Added nextDbVersion() calls after successful clock writes - Savepoint sync tests: 16/16 pass (was 15/16) TASK-182: Create trigger CRR test suite - New test: zig/harness/test-trigger-crr.sh (566 lines, 31 assertions) - Covers: INSERT/UPDATE/DELETE triggers between CRR tables - Covers: trigger chains (A->B->C), soft cascade patterns - Result: 31/31 pass, full parity with Rust/C oracle Test status: 61 scripts, savepoint sync now fully passing
1 parent 5bf267f commit 8314907

File tree

7 files changed

+1220
-57
lines changed

7 files changed

+1220
-57
lines changed

‎.tasks/DELEGATE_WORK_HANDOFF.md‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,107 @@ Artifacts:
6868

6969
---
7070

71+
## Round 2025-12-23 (67) — db_version savepoint fix + trigger CRR tests (2 tasks)
72+
73+
**Tasks executed**
74+
- `.tasks/done/TASK-181-zig-dbversion-savepoint-bug.md`
75+
- `.tasks/done/TASK-182-triggers-modify-other-crr.md`
76+
77+
**Commits**
78+
- `bf0a27c4` — Round 67: fix db_version savepoint bug + trigger CRR tests
79+
80+
**Environment**
81+
- OS: darwin (macOS ARM64)
82+
- Tooling: nix, zig (via nix), bash
83+
84+
**Commands run (exact)**
85+
```bash
86+
make -C zig build
87+
bash zig/harness/test-savepoint-sync.sh
88+
bash zig/harness/test-db-version-parity.sh
89+
bash zig/harness/test-trigger-crr.sh
90+
```
91+
92+
**Outputs (paste)**
93+
94+
<details>
95+
<summary>TASK-181: db_version savepoint fix (16/16 pass, was 15/16)</summary>
96+
97+
```text
98+
==================================================================
99+
SAVEPOINT SYNC TEST SUMMARY
100+
==================================================================
101+
PASSED: 16
102+
FAILED: 0
103+
SKIPPED: 0
104+
DIVERGENCES: 0
105+
==================================================================
106+
107+
Savepoint Sync Test Summary: 16 passed, 0 failed, 0 skipped
108+
```
109+
110+
**Root Cause**: When changes are applied via `INSERT INTO crsql_changes`, the merge path writes clock entries with the incoming db_version but never called `nextDbVersion()` to update `pending_db_version`. The commit hook only promotes pending to global when pending > global, so db_version never advanced.
111+
112+
**Fix Applied**: In `zig/src/merge_insert.zig`, added calls to `site_identity.nextDbVersion(db_version)` after successfully writing clock entries in both `setWinnerClock()` (line ~305) and `setWinnerClockCached()` (line ~358).
113+
114+
</details>
115+
116+
<details>
117+
<summary>TASK-182: Trigger CRR tests (31/31 pass, new suite)</summary>
118+
119+
```text
120+
==================================================================
121+
TRIGGER CRR TEST SUMMARY
122+
==================================================================
123+
PASSED: 31
124+
FAILED: 0
125+
SKIPPED: 0
126+
==================================================================
127+
128+
Key Findings:
129+
1. User triggers that modify other CRRs work correctly
130+
2. Trigger-inserted rows get proper clock entries
131+
3. db_version advances for both original and triggered changes
132+
4. Triggered changes appear in crsql_changes for sync
133+
5. Trigger chains (A->B->C) work across multiple CRRs
134+
6. DELETE triggers can implement soft cascades between CRRs
135+
136+
All trigger CRR tests PASSED
137+
```
138+
139+
**Tests Created (11 scenarios, 31 assertions):**
140+
1. Basic trigger INSERT (UPDATE items -> INSERT audit_log)
141+
2. Trigger-inserted row has clock entries
142+
3. db_version advances for both changes
143+
4. Sync works for triggered inserts
144+
5. DELETE trigger (DELETE items -> INSERT audit_log)
145+
6. Multiple CRR triggers in chain (A -> B -> C)
146+
7. Trigger with FK-like reference (soft relationship)
147+
8. Parity — Zig and Rust/C produce identical results
148+
9. INSERT trigger (INSERT items -> INSERT audit_log)
149+
10. Trigger UPDATE on another CRR
150+
11. Trigger DELETE on another CRR (cascade)
151+
152+
</details>
153+
154+
**Files modified/created**
155+
- `zig/src/merge_insert.zig` — Added `nextDbVersion()` calls in `setWinnerClock` and `setWinnerClockCached`
156+
- `zig/harness/test-trigger-crr.sh` (new, ~566 lines)
157+
158+
**Reproduction steps (clean checkout)**
159+
1. `git clone <repo> && cd cr-sqlite`
160+
2. `make -C zig build` — build Zig extension
161+
3. `bash zig/harness/test-savepoint-sync.sh` — verify 16/16 pass
162+
4. `bash zig/harness/test-db-version-parity.sh` — verify 14/14 pass
163+
5. `bash zig/harness/test-trigger-crr.sh` — verify 31/31 pass
164+
165+
**Known gaps / unverified claims**
166+
- TASK-186 (schema mismatch unknown column behavior) remains in triage — design decision needed
167+
- CI integration not verified (local runs only)
168+
- No coverage captured
169+
170+
---
171+
71172
## Round 2025-12-23 (66) — Savepoint, ATTACH, site_id collision test suites (3 tasks)
72173

73174
**Tasks executed**

.tasks/triage/TASK-181-zig-dbversion-savepoint-bug.md renamed to .tasks/done/TASK-181-zig-dbversion-savepoint-bug.md

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ Fix the Zig implementation so db_version correctly advances when changes are app
55
via crsql_changes within a transaction that uses savepoints.
66

77
## Status
8-
- State: triage
8+
- State: done
99
- Priority: high (sync correctness - db_version is critical for sync protocols)
1010

1111
## Context
@@ -40,7 +40,7 @@ This breaks sync protocols that rely on db_version to track which changes have b
4040
applied locally. If db_version doesn't advance, the sync cursor will be wrong.
4141

4242
## Files to Modify
43-
- `zig/src/` (db_version tracking logic - needs investigation)
43+
- `zig/src/merge_insert.zig` — `setWinnerClock()` and `setWinnerClockCached()` functions
4444

4545
## Acceptance Criteria
4646
1. After the repro above, db_version should be 1 (not 0)
@@ -55,6 +55,23 @@ applied locally. If db_version doesn't advance, the sync cursor will be wrong.
5555

5656
## Progress Log
5757
- 2025-12-23: Created from TASK-175 test divergence.
58+
- 2025-12-23: Fixed. Root cause: `setWinnerClock` and `setWinnerClockCached` in `merge_insert.zig`
59+
wrote clock entries with the incoming `db_version` but never called `nextDbVersion()` to update
60+
`pending_db_version`. The commit hook only promotes `pending_db_version` to `global_db_version`
61+
if pending > global, so db_version never advanced. Fix: Added `_ = site_identity.nextDbVersion(db_version);`
62+
after successful clock writes in both functions.
5863

5964
## Completion Notes
60-
(Empty until done.)
65+
- **Root Cause**: When changes are applied via `INSERT INTO crsql_changes`, the merge path writes
66+
clock entries with a specific db_version from the incoming change. However, the global
67+
`pending_db_version` was never updated, so when the commit hook called `commitDbVersion()`,
68+
nothing happened because `pending_db_version (0) <= global_db_version (0)`.
69+
70+
- **Fix Applied**: In `zig/src/merge_insert.zig`, added calls to `site_identity.nextDbVersion(db_version)`
71+
after successfully writing clock entries in both `setWinnerClock()` (line ~305) and
72+
`setWinnerClockCached()` (line ~358). This ensures `pending_db_version` is updated to at least
73+
the incoming `db_version`, so the commit hook correctly advances `global_db_version`.
74+
75+
- **Tests Passed**:
76+
- `zig/harness/test-savepoint-sync.sh`: 16/16 passed (Test 7 was the failing test, now passes)
77+
- `zig/harness/test-db-version-parity.sh`: 14/14 passed (no regressions)

‎.tasks/done/TASK-182-triggers-modify-other-crr.md‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
# TASK-182 — Test user triggers that modify other CRR tables
2+
3+
## Goal
4+
Verify behavior when user-defined triggers INSERT/UPDATE/DELETE other CRR tables.
5+
6+
## Status
7+
- State: done
8+
- Priority: medium (real-world pattern)
9+
10+
## Context
11+
Apps often have triggers like:
12+
```sql
13+
CREATE TRIGGER audit_log AFTER UPDATE ON items
14+
INSERT INTO audit_log (item_id, action) VALUES (NEW.id, 'update');
15+
```
16+
17+
If audit_log is also a CRR:
18+
1. Does the trigger-based insert get clock entries?
19+
2. Is it in the same transaction as the original update?
20+
3. Does db_version advance correctly?
21+
22+
## Files to Modify
23+
- `zig/harness/test-trigger-crr.sh` (new)
24+
25+
## Acceptance Criteria
26+
1. Create two CRR tables (items, audit_log)
27+
2. Create trigger: UPDATE items → INSERT audit_log
28+
3. UPDATE an item
29+
4. Verify: audit_log has row
30+
5. Verify: audit_log row has clock entries
31+
6. Verify: db_version advanced for both changes
32+
7. Sync to Site B
33+
8. Verify: both tables sync correctly
34+
9. Zig and Rust/C oracle produce identical results
35+
36+
## Parent Docs / Cross-links
37+
- Related: TASK-170 (FK between CRRs)
38+
- Gap backlog: `research/zig-cr/92-gap-backlog.md`
39+
40+
## Progress Log
41+
- 2025-12-22: Created from gap analysis.
42+
- 2025-12-23: Created `zig/harness/test-trigger-crr.sh` with 11 test scenarios (31 assertions).
43+
44+
## Completion Notes
45+
- **Created**: `zig/harness/test-trigger-crr.sh` (566 lines)
46+
- **Test Results**: All 31 tests PASSED, 0 FAILED, 0 SKIPPED
47+
- **Parity**: Zig and Rust/C oracle produce identical results for all scenarios
48+
49+
### Test Scenarios Implemented:
50+
1. Basic trigger INSERT (UPDATE items -> INSERT audit_log)
51+
2. Trigger-inserted row has clock entries
52+
3. db_version advances for both changes
53+
4. Sync works for triggered inserts
54+
5. DELETE trigger (DELETE items -> INSERT audit_log)
55+
6. Multiple CRR triggers in chain (A -> B -> C)
56+
7. Trigger with FK-like reference (soft relationship)
57+
8. Parity — Zig and Rust/C produce identical results
58+
9. INSERT trigger (INSERT items -> INSERT audit_log)
59+
10. Trigger UPDATE on another CRR (UPDATE A -> UPDATE B)
60+
11. Trigger DELETE on another CRR (cascade via trigger)
61+
62+
### Key Findings:
63+
1. User triggers that modify other CRRs work correctly
64+
2. Trigger-inserted rows get proper clock entries
65+
3. db_version advances for both original and triggered changes
66+
4. Triggered changes appear in crsql_changes for sync
67+
5. Trigger chains (A->B->C) work across multiple CRRs
68+
6. DELETE triggers can implement soft cascades between CRRs
69+
70+
### No Divergences Found:
71+
- Zig and Rust/C implementations behave identically for all trigger scenarios

‎.tasks/triage/TASK-182-triggers-modify-other-crr.md‎

Lines changed: 0 additions & 44 deletions
This file was deleted.

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# 92-gap-backlog
22

3-
> Last updated: 2025-12-23 (Round 66: Savepoint, ATTACH, site_id collision tests)
3+
> Last updated: 2025-12-23 (Round 67: db_version savepoint fix, trigger CRR tests)
44
55
## Status
66

@@ -15,29 +15,24 @@
1515
- Sentinel parity: ✅ **6/6 PASSING** (Round 63)
1616
- Multinode sync: ✅ **6/6 PASSING** (Round 63)
1717
- Schema mismatch: ⚠️ **11/12 PASSING** (1 divergence: unknown column behavior)
18-
- Savepoint sync: ⚠️ **15/16 PASSING** (1 divergence: db_version with savepoints)
18+
- Savepoint sync: ✅ **16/16 PASSING** (Round 67 — TASK-181 fixed)
1919
- ATTACH CRR: ✅ **15/15 PASSING** (Round 66)
2020
- Site ID collision: ✅ **13/13 PASSING** (Round 66)
21-
- Test scripts: **60 total**
21+
- Trigger CRR: ✅ **31/31 PASSING** (Round 67)
22+
- Test scripts: **61 total**
2223
- Zig implementation: `zig/`
2324
- Canonical task queue: `.tasks/{backlog,active,done}/`
2425

2526
## Now (next parallel assignments)
2627

27-
**Backlog is empty** — Triage contains 6 items ready for prioritization.
28+
**Backlog is empty** — Triage contains 4 items ready for prioritization.
2829

2930
### Triage Inbox (organized by priority)
3031

31-
#### HIGH Priority — Bug Fix
32-
| Task | Summary | Risk | Effort |
33-
|------|---------|------|--------|
34-
| **TASK-181** | db_version not advancing with savepoint + crsql_changes | Sync correctness | Medium |
35-
3632
#### MEDIUM Priority — Behavioral Parity
3733
| Task | Summary | Risk | Effort |
3834
|------|---------|------|--------|
3935
| **TASK-186** | Schema mismatch: unknown column behavior | Design decision | Low |
40-
| **TASK-182** | User triggers modify other CRRs | Real-world pattern | Medium |
4136

4237
#### LOW Priority — Nice to Have
4338
| Task | Summary | Risk | Effort |
@@ -46,6 +41,10 @@
4641
| **TASK-183** | Wide table (50+ cols) performance | Performance only | Medium |
4742
| **TASK-156** | Linux CI parity | CI only | Medium |
4843

44+
### Completed Round 67 (2025-12-23)
45+
- [x] TASK-181: Fix db_version savepoint bug ✓ (16/16 pass now)
46+
- [x] TASK-182: Trigger CRR tests ✓ (31/31 pass, full parity)
47+
4948
### Completed Round 66 (2025-12-23)
5049
- [x] TASK-175: Savepoint sync tests ✓ (15/16 pass, 1 divergence → TASK-181)
5150
- [x] TASK-176: ATTACH database CRR tests ✓ (15/15 pass, full parity)

0 commit comments

Comments
 (0)