Skip to content

Commit fdeae71

Browse files
chore: add follow-up tasks from Round 73
TASK-203: Empty blob PK encoding divergence (LOW) TASK-204: Fix PK UPDATE test schema mismatch (LOW) TASK-205: Fix inventory app test (LOW)
1 parent 2436dd6 commit fdeae71

File tree

3 files changed

+169
-0
lines changed

3 files changed

+169
-0
lines changed
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# TASK-203 — Empty Blob PK Encoding Divergence
2+
3+
## Goal
4+
Fix the PK blob encoding for empty blob primary keys to match Rust/C oracle.
5+
6+
## Status
7+
- State: triage
8+
- Priority: LOW (edge case, not blocking sync)
9+
- Discovered: 2025-12-20 (TASK-133)
10+
11+
## Problem
12+
13+
When a table has an empty blob (`X''`) as its primary key value, the encoded PK in `crsql_changes` differs:
14+
15+
```
16+
Zig: 0105
17+
Rust/C: 0104
18+
```
19+
20+
## Reproduction
21+
22+
```bash
23+
cd /Users/tom/Developer/effect-native/cr-sqlite
24+
bash zig/harness/test-pk-blob-parity.sh
25+
```
26+
27+
Look for test WF-028.
28+
29+
## Impact
30+
31+
This only affects tables with:
32+
1. BLOB primary keys
33+
2. That contain empty blob values
34+
35+
This is a rare edge case and doesn't affect sync correctness (the blob value itself syncs correctly, just the encoding differs).
36+
37+
## Files to Investigate
38+
39+
- `zig/src/pack_columns.zig` — PK blob encoding logic
40+
- `core/rs/core/src/pack_columns.rs` — Rust reference
41+
42+
## Acceptance Criteria
43+
44+
1. [ ] Empty blob PK encoded as `0104` (matching Rust/C)
45+
2. [ ] `bash zig/harness/test-pk-blob-parity.sh` passes 9/9
46+
47+
## Parent Docs / Cross-links
48+
49+
- Test: `zig/harness/test-pk-blob-parity.sh` (WF-028)
50+
- Related: `.tasks/done/TASK-133-pk-blob-format-edge-case-parity.md`
51+
52+
## Progress Log
53+
- 2025-12-25: Created from Round 73 test results.
54+
55+
## Completion Notes
56+
(Empty until done.)
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# TASK-204 — Fix PK UPDATE Test Schema Mismatch
2+
3+
## Goal
4+
Fix `test-pk-update.sh` Test 1d to use correct pks table schema.
5+
6+
## Status
7+
- State: triage
8+
- Priority: LOW (test bug, not implementation bug)
9+
- Discovered: 2025-12-25 (Round 73)
10+
11+
## Problem
12+
13+
Test 1d in `zig/harness/test-pk-update.sh` uses outdated column names:
14+
15+
```sql
16+
-- Test currently uses:
17+
SELECT COUNT(*) FROM foo__crsql_clock c JOIN foo__crsql_pks p ON c.pk = p.pk WHERE p.pks = X'010901';
18+
19+
-- But the actual schema is:
20+
CREATE TABLE "foo__crsql_pks" (__crsql_key INTEGER PRIMARY KEY, "id")
21+
```
22+
23+
The columns `pk` and `pks` don't exist. The actual columns are:
24+
- `__crsql_key` (the key)
25+
- `id` (the actual PK column value)
26+
27+
## Fix
28+
29+
Update the test to use:
30+
```sql
31+
SELECT COUNT(*) FROM foo__crsql_clock c WHERE c.key = (SELECT __crsql_key FROM foo__crsql_pks WHERE id = 1);
32+
```
33+
34+
Or use the clock table's `key` column directly:
35+
```sql
36+
SELECT COUNT(*) FROM foo__crsql_clock WHERE key = <expected_key>;
37+
```
38+
39+
## Files to Modify
40+
41+
- `zig/harness/test-pk-update.sh` — Fix Test 1d SQL queries
42+
43+
## Acceptance Criteria
44+
45+
1. [ ] Test 1d passes
46+
2. [ ] All 16 PK UPDATE tests pass
47+
48+
## Parent Docs / Cross-links
49+
50+
- Related: `.tasks/done/TASK-110-zig-pk-update-compound-text-pk.md`
51+
52+
## Progress Log
53+
- 2025-12-25: Created from Round 73 findings.
54+
55+
## Completion Notes
56+
(Empty until done.)
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# TASK-205 — Fix Inventory App Simulation Test
2+
3+
## Goal
4+
Fix `test-app-inventory.sh` which fails for BOTH Zig AND Rust/C implementations.
5+
6+
## Status
7+
- State: triage
8+
- Priority: LOW (test bug, not implementation bug)
9+
- Discovered: 2025-12-25 (Round 73)
10+
11+
## Problem
12+
13+
The inventory app test fails with both implementations showing sites don't converge:
14+
15+
```
16+
Step 3: Verify convergence
17+
FAIL: Sites did not converge
18+
A: PROD-A|site-a|150
19+
PROD-B|site-a|75
20+
B: PROD-A|site-b|200
21+
PROD-C|site-b|50
22+
C: PROD-B|site-c|100
23+
PROD-C|site-c|125
24+
```
25+
26+
Since both implementations fail identically, this is a test design issue, not an implementation bug.
27+
28+
## Root Cause (Suspected)
29+
30+
The test creates inventory at different sites but:
31+
1. Each site creates different products (PROD-A at site-a, PROD-B at site-a, etc.)
32+
2. The sync only transfers data, not schema
33+
3. Sites end up with their local data only
34+
35+
The test should either:
36+
1. Have all sites share the same products first
37+
2. Or verify that each site has ALL products after sync (not just their local ones)
38+
39+
## Files to Modify
40+
41+
- `zig/harness/test-app-inventory.sh` — Fix test logic
42+
43+
## Acceptance Criteria
44+
45+
1. [ ] Test passes for both Zig and Rust/C
46+
2. [ ] Or document as intentional behavior and mark test as xfail
47+
48+
## Parent Docs / Cross-links
49+
50+
- Test script: `zig/harness/test-app-inventory.sh`
51+
- Related: `.tasks/done/TASK-194-real-world-app-simulation.md`
52+
53+
## Progress Log
54+
- 2025-12-25: Created from Round 73 findings.
55+
56+
## Completion Notes
57+
(Empty until done.)

0 commit comments

Comments
 (0)