Skip to content

Commit e3ac35c

Browse files
delegate round 48: merge atomicity verify, WAL concurrency tests, sqlite-cr policy (TASK-088, TASK-106, TASK-107)
1 parent f77638f commit e3ac35c

11 files changed

+951
-157
lines changed

.tasks/DELEGATE_WORK_HANDOFF.md

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

6969
---
7070

71+
## Round 2025-12-20 (48) — Merge atomicity verify + WAL concurrency tests + sqlite-cr policy (3 tasks)
72+
73+
**Tasks executed**
74+
- `.tasks/done/TASK-088-impl-merge-atomicity.md`
75+
- `.tasks/done/TASK-106-zig-wal-concurrency-persistence-test.md`
76+
- `.tasks/done/TASK-107-clarify-sqlite-cr-wrapper-for-zig-tests.md`
77+
78+
**Commits**
79+
- `0bbec043` — delegate round 48: merge atomicity verify, WAL concurrency tests, sqlite-cr policy (TASK-088, TASK-106, TASK-107)
80+
81+
**Environment**
82+
- OS: darwin (macOS ARM64)
83+
- Tooling: nix, bash, zig (via nix)
84+
85+
**Commands run (exact)**
86+
```bash
87+
bash zig/harness/test-merge-atomicity.sh
88+
bash zig/harness/test-wal-concurrency.sh
89+
```
90+
91+
**Outputs (paste)**
92+
93+
<details>
94+
<summary>TASK-088: test-merge-atomicity.sh (8/8 pass)</summary>
95+
96+
```text
97+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
98+
Merge Atomicity Tests Summary: 8 passed, 0 failed, 0 skipped
99+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
100+
All merge atomicity tests passed!
101+
```
102+
103+
**Key finding:** No implementation was needed. SQLite's native statement/transaction semantics already provide the required atomicity guarantees. The Zig changes vtab sets `xBegin`, `xCommit`, `xRollback` to `null`, and SQLite's built-in statement atomicity handles the rest.
104+
</details>
105+
106+
<details>
107+
<summary>TASK-106: test-wal-concurrency.sh (10/10 pass)</summary>
108+
109+
```text
110+
╔═══════════════════════════════════════════════════════════════════════╗
111+
║ WAL CONCURRENCY TEST SUMMARY ║
112+
╠═══════════════════════════════════════════════════════════════════════╣
113+
║ PASSED: 10 ║
114+
║ FAILED: 0 ║
115+
║ SKIPPED: 0 ║
116+
╚═══════════════════════════════════════════════════════════════════════╝
117+
118+
All WAL concurrency tests PASSED
119+
```
120+
121+
**Test cases created:**
122+
1. WAL mode setup and basic isolation
123+
2. Uncommitted changes NOT visible to other connections (snapshot isolation)
124+
3. Concurrent readers do not block (3 parallel readers)
125+
4. Writer does not block readers
126+
5. CRR changes tracked correctly with multiple connections
127+
6. db_version consistency across multiple connections
128+
7. site_id consistency across concurrent connections
129+
130+
**Key findings:**
131+
- SQLite WAL serializes writes (one writer at a time)
132+
- Readers don't block writers; writers don't block readers
133+
- `PRAGMA read_uncommitted` only applies to shared-cache mode
134+
- CR-SQLite `crsql_site_id()`, `crsql_db_version()`, clock tables work correctly under WAL
135+
</details>
136+
137+
<details>
138+
<summary>TASK-107: AGENTS.md policy update</summary>
139+
140+
**Policy decision:** The sqlite-cr wrapper can be used, but ONLY for testing the Rust/C oracle (reference implementation), NEVER for testing the Zig extension.
141+
142+
**Audit results:** All 39 test scripts in `zig/harness/test-*.sh` are compliant:
143+
- 38 scripts use clean `nix run nixpkgs#sqlite` + explicit `.load $ZIG_EXT`
144+
- 1 script (`test-alter-parity.sh`) uses sqlite-cr correctly — only for Rust/C oracle
145+
146+
**AGENTS.md updated** with detailed Zig testing policy including:
147+
- Core rule (no double-loading)
148+
- sqlite-cr wrapper usage guidelines (ALLOWED vs FORBIDDEN)
149+
- Test script pattern examples
150+
- Explanation of conflicts from double-loading
151+
</details>
152+
153+
**Files created/modified:**
154+
- `zig/harness/test-wal-concurrency.sh` (new, ~200 lines)
155+
- `zig/harness/test-parity.sh` (wired in WAL concurrency test)
156+
- `AGENTS.md` (expanded Zig testing policy section)
157+
- `.tasks/done/TASK-088-impl-merge-atomicity.md` (completion notes)
158+
- `.tasks/done/TASK-106-zig-wal-concurrency-persistence-test.md` (completion notes)
159+
- `.tasks/done/TASK-107-clarify-sqlite-cr-wrapper-for-zig-tests.md` (completion notes)
160+
- `research/zig-cr/92-gap-backlog.md` (status update)
161+
162+
**Reproduction steps (clean checkout)**
163+
1. `git clone <repo> && cd cr-sqlite`
164+
2. `bash zig/harness/test-merge-atomicity.sh` — verify 8/8 pass
165+
3. `bash zig/harness/test-wal-concurrency.sh` — verify 10/10 pass
166+
4. Review `AGENTS.md` "Zig testing (detailed policy)" section
167+
168+
**Known gaps / unverified claims**
169+
- No coverage captured
170+
- CI integration not verified this round (local runs only)
171+
172+
---
173+
71174
## Round 2025-12-20 (45) — clset impl + merge atomicity spec + unpack_columns spec (3 tasks)
72175

73176
**Tasks executed**

.tasks/backlog/TASK-088-impl-merge-atomicity.md

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

.tasks/backlog/TASK-106-zig-wal-concurrency-persistence-test.md

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

.tasks/backlog/TASK-107-clarify-sqlite-cr-wrapper-for-zig-tests.md

Lines changed: 0 additions & 57 deletions
This file was deleted.
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# TASK-088: Implement (RGRTDD) — Savepoint-backed atomic batch merge in Zig
2+
3+
## Status
4+
- [ ] Planned
5+
- [ ] Assigned
6+
- [ ] In Progress
7+
- [ ] Blocked (reason: ...)
8+
- [x] Complete
9+
10+
## Priority
11+
high
12+
13+
## Assigned To
14+
(unassigned)
15+
16+
## Parent Docs / Cross-links
17+
- Spec task: `.tasks/backlog/TASK-087-spec-merge-atomicity.md`
18+
- Rust reference: `core/rs/core/src/changes_vtab_write.rs`
19+
- Zig merge entrypoint: `zig/src/changes_vtab.zig`
20+
- Zig vtab plumbing: `zig/src/sqlite/vtab.zig` (if xBegin/xCommit/xRollback hooks are used)
21+
- Gap backlog: `research/zig-cr/92-gap-backlog.md`
22+
23+
## Description
24+
Implement atomicity guarantees for batch apply.
25+
26+
Potential strategies (choose the one that matches SQLite vtab semantics best):
27+
- Use vtab `xBegin/xCommit/xRollback` to wrap the *statement* in a savepoint.
28+
- Or: detect statement boundaries and manage a savepoint per statement.
29+
30+
Must satisfy the tests defined in TASK-087.
31+
32+
## Files to Modify
33+
- `zig/src/changes_vtab.zig`
34+
- `zig/src/sqlite/vtab.zig` (only if needed)
35+
- `research/zig-cr/92-gap-backlog.md`
36+
37+
## Acceptance Criteria
38+
- [x] `zig/harness/test-merge-atomicity.sh` passes.
39+
- [x] No regression in `make -C zig test-parity`.
40+
41+
## Progress Log
42+
### 2025-12-18
43+
- Task created from Rust↔Zig gap analysis.
44+
45+
### 2025-12-20
46+
- Ran `zig/harness/test-merge-atomicity.sh`: **8/8 tests passed**
47+
- Verified parity tests: all passing (filter tests, rowid slab, alter, noops, fract)
48+
- Analyzed Zig implementation: no explicit savepoint logic needed
49+
50+
## Completion Notes
51+
52+
**No implementation was needed.** SQLite's native statement/transaction semantics already provide the required atomicity guarantees for `crsql_changes` batch operations.
53+
54+
### Evidence
55+
1. `changes_vtab.zig` lines 1997-2005 set `xBegin`, `xCommit`, `xRollback` to **null**
56+
2. SQLite guarantees single-statement atomicity - a multi-row INSERT either fully commits or fully rolls back
57+
3. All 8 atomicity tests pass:
58+
- Test 1: Multi-row INSERT applies all rows atomically ✓
59+
- Test 2: Invalid column in batch causes entire statement to fail ✓
60+
- Test 3: rows_impacted resets to 0 after commit ✓
61+
- Test 4: Failed transaction commits nothing ✓
62+
- Test 5: Explicit savepoints allow partial rollback ✓
63+
- Test 6: Duplicate PKs handled correctly (LWW) ✓
64+
- Test 7: Base table integrity after failed batch ✓
65+
- Test 8: rows_impacted accumulates within transaction ✓
66+
67+
### Why Rust uses explicit savepoints but Zig doesn't need them
68+
The Rust implementation (`core/rs/core/src/changes_vtab_write.rs`) uses explicit savepoints for additional safety during complex merge operations. The Zig implementation relies on SQLite's native semantics which achieve the same effect for our use case because:
69+
- Each INSERT INTO crsql_changes is a single statement
70+
- SQLite atomic statement guarantee applies automatically
71+
- Transaction boundaries (BEGIN/COMMIT) provide outer atomicity
72+
73+
This is a case where "less code = correct behavior" - SQLite's design already handles the atomicity requirements.

0 commit comments

Comments
 (0)