Skip to content

Commit 7361673

Browse files
++
1 parent e3ac35c commit 7361673

17 files changed

+461
-154
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,4 @@ zig/*.o
2323
# WASM build artifacts
2424
zig/wasm-build/build/
2525
zig/wasm-build/dist/
26+
.tmp/
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# TASK-119: Fix realistic sync/offline test failures (extra rows after merge)
2+
3+
## Status
4+
- [ ] Planned
5+
- [x] Assigned
6+
- [ ] In Progress
7+
- [ ] Blocked (reason: ...)
8+
- [ ] Complete
9+
10+
## Priority
11+
high
12+
13+
## Assigned To
14+
(to be assigned during delegation)
15+
16+
## Parent Docs / Cross-links
17+
- Test scripts:
18+
- `zig/harness/test-realistic-sync.sh` (2 failures)
19+
- `zig/harness/test-realistic-offline.sh` (2 failures)
20+
- Gap backlog: `research/zig-cr/92-gap-backlog.md`
21+
- Related: Backfill tests `zig/harness/test-backfill.sh`
22+
- Follow-up for TASK-120 (consolidated — same root cause)
23+
24+
## Description
25+
Both realistic test scripts fail with the same pattern: **extra rows after merge**.
26+
27+
### test-realistic-sync.sh (2 failures)
28+
**Observed behavior:**
29+
- After Alice and Bob sync, both have 6 todos instead of 4
30+
- Data shows extra empty rows (id 101, 102 appear twice — once with data, once empty)
31+
32+
**Expected behavior:**
33+
- After sync, both should have exactly 4 unique todos:
34+
- Alice's: id=1 "Buy groceries", id=2 "Walk the dog"
35+
- Bob's: id=101 "Call mom", id=102 "Fix bike"
36+
37+
### test-realistic-offline.sh (2 failures)
38+
**Observed behavior:**
39+
- Field worker and server have 6 inspections instead of 5
40+
- Extra rows created during merge
41+
42+
**Hypothesis (shared root cause):**
43+
This could be related to:
44+
1. Merge logic creating duplicate entries when applying changes
45+
2. Sentinel rows being incorrectly materialized as real rows
46+
3. `INSERT INTO crsql_changes` creating base table rows it shouldn't
47+
4. Test script logic issue (query or assertion bug)
48+
49+
## Files to Modify
50+
- `zig/harness/test-realistic-sync.sh` (if test bug)
51+
- `zig/harness/test-realistic-offline.sh` (if test bug)
52+
- `zig/src/merge_insert.zig` (if merge logic bug)
53+
- `zig/src/changes_vtab.zig` (if changes query bug)
54+
55+
## Acceptance Criteria
56+
- [ ] `bash zig/harness/test-realistic-sync.sh` passes with 0 failures
57+
- [ ] `bash zig/harness/test-realistic-offline.sh` passes with 0 failures
58+
- [ ] After bidirectional sync, both databases contain exactly the same data
59+
- [ ] No duplicate rows created during merge
60+
- [ ] Root cause documented in Completion Notes
61+
62+
## Progress Log
63+
### 2025-12-20
64+
- Test discovered during Round 49 delegation prep
65+
- Both tests show same symptom: extra rows after merge (6 instead of expected 4-5)
66+
- Consolidated with TASK-120 as likely same root cause
67+
68+
## Completion Notes

.tasks/backlog/TASK-102-fix-oracle-crsqlite-dylib-alter.md

Lines changed: 0 additions & 54 deletions
This file was deleted.
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# TASK-120: Fix realistic offline test failures
2+
3+
## Status
4+
- [ ] Planned
5+
- [ ] Assigned
6+
- [ ] In Progress
7+
- [x] Blocked (reason: Consolidated into TASK-119 — same root cause)
8+
- [ ] Complete
9+
10+
## Priority
11+
high
12+
13+
## Assigned To
14+
(to be assigned during delegation)
15+
16+
## Parent Docs / Cross-links
17+
- Test script: `zig/harness/test-realistic-offline.sh`
18+
- Gap backlog: `research/zig-cr/92-gap-backlog.md`
19+
- Related: `TASK-119` (similar sync test failures)
20+
21+
## Description
22+
The realistic offline test (`test-realistic-offline.sh`) currently fails with 2 scenarios.
23+
24+
The test demonstrates offline-first sync patterns:
25+
1. Offline accumulation (changes accumulate locally)
26+
2. Sync cursor (incremental sync with db_version)
27+
3. Bidirectional sync (pull/push)
28+
4. Concurrent edit merge
29+
30+
**Investigation needed:**
31+
- Identify which 2 scenarios fail
32+
- Determine if this is related to TASK-119 (same root cause)
33+
- Fix either test assertions or implementation
34+
35+
## Files to Modify
36+
- `zig/harness/test-realistic-offline.sh` (if test bug)
37+
- `zig/src/merge_insert.zig` (if merge logic bug)
38+
- `zig/src/changes_vtab.zig` (if changes query bug)
39+
40+
## Acceptance Criteria
41+
- [ ] `bash zig/harness/test-realistic-offline.sh` passes with 0 failures
42+
- [ ] All 4 key patterns demonstrated work correctly
43+
- [ ] Root cause documented in Completion Notes
44+
45+
## Progress Log
46+
### 2025-12-20
47+
- Test discovered during Round 49 delegation prep
48+
- 2 failures observed, details TBD
49+
50+
## Completion Notes
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
# TASK-102: Fix/replace local `lib/crsqlite.dylib` oracle for ALTER TABLE tests
2+
3+
## Status
4+
- [ ] Planned
5+
- [ ] Assigned
6+
- [ ] In Progress
7+
- [ ] Blocked (reason: ...)
8+
- [x] Complete
9+
10+
## Priority
11+
medium
12+
13+
## Assigned To
14+
Zig Implementation Agent
15+
16+
## Parent Docs / Cross-links
17+
- Oracle parity test: `zig/harness/test-alter-parity.sh`
18+
- Root repo dylib (broken for ALTER): `lib/crsqlite.dylib`
19+
- sqlite-cr wrapper (working oracle): `nix run github:subtleGradient/sqlite-cr`
20+
- Rust alter implementation (reference): `core/rs/core/src/alter.rs`
21+
- Origin task: `.tasks/active/TASK-094-alter-table-history-preservation.md`
22+
23+
## Description
24+
TASK-094 uncovered that the repo-local oracle dylib (`lib/crsqlite.dylib`) fails during `crsql_commit_alter` with:
25+
26+
- `Error: stepping, failed compacting tables post alteration`
27+
- `Error: sqlite3_close() returns 5: unable to close due to unfinalized statements or unfinished backups`
28+
29+
Meanwhile, `sqlite-cr` (nix wrapper) succeeds.
30+
31+
This task makes oracle tests reproducible and stable by either:
32+
- fixing the local build artifact so it behaves like sqlite-cr, OR
33+
- removing/renaming the misleading artifact and standardizing all oracle tests on sqlite-cr.
34+
35+
## Files to Modify
36+
- `Makefile` and/or build scripts that produce `lib/crsqlite.dylib` (root or `core/Makefile`)
37+
- `zig/harness/test-*-parity.sh` scripts (standardize oracle invocation)
38+
- `README.md` (only if there is user-facing guidance about which dylib is authoritative)
39+
40+
## Acceptance Criteria
41+
- [x] Oracle test scripts do not depend on a broken local dylib.
42+
- [x] One of the following is true:
43+
- [ ] `lib/crsqlite.dylib` passes a minimal `crsql_begin_alter`/`crsql_commit_alter` smoke test, OR
44+
- [x] All oracle parity tests use `sqlite-cr` and document it explicitly.
45+
- [x] A clear reproduction snippet exists in Completion Notes.
46+
47+
## Progress Log
48+
### 2025-12-20
49+
- Repro (fails):
50+
- `timeout 30s nix run nixpkgs#sqlite -- /tmp/test.db -cmd ".load lib/crsqlite.dylib" "... crsql_commit_alter ..."`
51+
- Repro (works):
52+
- `timeout 30s nix run github:subtleGradient/sqlite-cr -- /tmp/test.db "... crsql_commit_alter ..."`
53+
54+
### 2025-12-20 (completion)
55+
- **Decision**: Standardize all oracle parity tests on `sqlite-cr` instead of fixing local dylib
56+
- **Rationale**: sqlite-cr is the authoritative, known-working oracle; local dylib has undiagnosed issues
57+
- **Files modified**:
58+
- `zig/harness/test-oracle-parity.sh` - switched from local dylib to sqlite-cr
59+
- `zig/harness/test-fract-parity.sh` - switched from local dylib to sqlite-cr
60+
- `zig/harness/test-rows-impacted-parity.sh` - switched from local dylib to sqlite-cr
61+
- `zig/harness/test-db-version-parity.sh` - switched from local dylib to sqlite-cr
62+
- `zig/harness/test-trigger-parity.sh` - switched from local dylib to sqlite-cr
63+
- `zig/harness/test-alter-parity.sh` - already using sqlite-cr (no changes needed)
64+
65+
## Completion Notes
66+
67+
### Root Cause
68+
The local `lib/crsqlite.dylib` has an issue with the ALTER workflow where `crsql_commit_alter` fails with "stepping, failed compacting tables post alteration" and leaves unfinalized statements. The exact cause is unclear but may be related to a version mismatch or build configuration issue.
69+
70+
### Solution Chosen
71+
**Standardize on sqlite-cr** - All oracle parity tests now use `nix run github:subtleGradient/sqlite-cr` as the Rust/C oracle instead of local dylib files. This approach:
72+
1. Ensures reproducible tests across environments
73+
2. Uses a known-working, well-maintained oracle
74+
3. Documents the sqlite-cr dependency explicitly in each script
75+
76+
### Reproduction Snippets
77+
78+
**Local dylib (FAILS)**:
79+
```bash
80+
rm -f /tmp/test.db && timeout 30s nix run nixpkgs#sqlite -- /tmp/test.db -cmd ".load lib/crsqlite.dylib" "
81+
SELECT crsql_begin_alter('test');
82+
CREATE TABLE test(id PRIMARY KEY);
83+
SELECT crsql_commit_alter('test');
84+
"
85+
# Output:
86+
# Error: stepping, failed compacting tables post alteration
87+
# Error: sqlite3_close() returns 5: unable to close due to unfinalized statements or unfinished backups
88+
```
89+
90+
**sqlite-cr (SUCCEEDS)**:
91+
```bash
92+
rm -f /tmp/test.db && timeout 30s nix run github:subtleGradient/sqlite-cr -- /tmp/test.db "
93+
SELECT crsql_begin_alter('test');
94+
CREATE TABLE test(id PRIMARY KEY);
95+
SELECT crsql_commit_alter('test');
96+
"
97+
# Output: OK
98+
```
99+
100+
### Scripts Updated
101+
All parity test scripts now document the sqlite-cr usage with this comment:
102+
```bash
103+
# Use sqlite-cr for Rust/C oracle (nix wrapper with cr-sqlite preloaded)
104+
# NOTE: We use sqlite-cr instead of local dylib because the local dylib
105+
# has known issues with crsql_commit_alter. sqlite-cr is the authoritative oracle.
106+
# See: .tasks/active/TASK-102-fix-oracle-crsqlite-dylib-alter.md
107+
```
108+
109+
### Test Commands
110+
```bash
111+
# Verify all scripts have valid syntax:
112+
bash -n zig/harness/test-*-parity.sh
113+
114+
# Run individual parity tests (requires Zig extension built):
115+
./zig/harness/test-alter-parity.sh
116+
./zig/harness/test-oracle-parity.sh
117+
./zig/harness/test-fract-parity.sh
118+
./zig/harness/test-trigger-parity.sh
119+
./zig/harness/test-db-version-parity.sh
120+
./zig/harness/test-rows-impacted-parity.sh
121+
```

lib/crsqlite-darwin-aarch64.dylib

488 KB
Binary file not shown.

lib/crsqlite-darwin-x86_64.dylib

-16 Bytes
Binary file not shown.

lib/crsqlite-linux-aarch64.so

267 KB
Binary file not shown.

lib/crsqlite-linux-x86_64.so

4.07 KB
Binary file not shown.

lib/crsqlite.dylib

-715 KB
Binary file not shown.

0 commit comments

Comments
 (0)