Skip to content

Commit a205847

Browse files
dylanbstoreyDylan Bobby Storey
andauthored
test: add Clotho bug reproduction tests for Rust bindings (#33)
Add 5 integration tests mirroring exact query patterns reported as broken from Clotho (count aggregates, property-match syntax, OPTIONAL MATCH with nulls, undirected match, pattern predicates). All pass — confirms the issues are Clotho-side, not in graphqlite or its Rust bindings. Also adds Metis tickets GQLITE-T-0139 (EXISTS subquery syntax) and GQLITE-T-0140 (Clotho investigation). Co-authored-by: Dylan Bobby Storey <dstorey@dstorey-personal-m3.local>
1 parent 31c2b3a commit a205847

File tree

4 files changed

+283
-1
lines changed

4 files changed

+283
-1
lines changed
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
---
2+
id: investigate-clotho-integration
3+
level: task
4+
title: "Investigate Clotho integration: count(), OPTIONAL MATCH, property-match reported as broken"
5+
short_code: "GQLITE-T-0140"
6+
created_at: 2026-03-22T00:45:57.695444+00:00
7+
updated_at: 2026-03-22T00:45:57.695444+00:00
8+
parent:
9+
blocked_by: []
10+
archived: false
11+
12+
tags:
13+
- "#task"
14+
- "#phase/backlog"
15+
- "#bug"
16+
17+
18+
exit_criteria_met: false
19+
initiative_id: NULL
20+
---
21+
22+
# Investigate Clotho integration: count(), OPTIONAL MATCH, property-match reported as broken
23+
24+
## Objective
25+
26+
Investigate why Clotho (colliery-io/clotho) reported `count()`, `OPTIONAL MATCH`, and `{key: value}` property-match as broken, when all three work correctly at the C extension level on both v0.3.7 and current main.
27+
28+
## Background
29+
30+
**Reported from Clotho integration sessions (v0.3.7, rusqlite 0.31).** The report claims 8 of 33 queries failed (24% error rate), including:
31+
32+
1. **`count()` aggregate** — "returns column headers but no rows." Cannot reproduce at C level on v0.3.7 or current; returns `[{"total":3}]` correctly.
33+
2. **`{key: 'value'}` property-match** — "syntax error on `:`". Cannot reproduce at C level on v0.3.7 or current; parses and executes correctly.
34+
3. **`OPTIONAL MATCH`** — "returns empty result set." Cannot reproduce at C level; correctly returns rows with `null` for unmatched optionals.
35+
36+
### Reproduction Results
37+
38+
Tested all three on v0.3.7 (via `git checkout v0.3.7 && make extension`) and current main:
39+
40+
| Bug | v0.3.7 | v0.3.10 (current) |
41+
|-----|--------|-------------------|
42+
| count() empty | Works correctly | Works correctly |
43+
| {key: value} syntax error | Works correctly | Works correctly |
44+
| OPTIONAL MATCH empty | Works correctly | Works correctly |
45+
46+
### Suspected Root Cause: Rust Bindings
47+
48+
Since the C extension works, the issue likely lives in the Rust binding layer or Clotho's usage of it. Analysis of `bindings/rust/src/` found several potential data-loss paths:
49+
50+
- **`connection.rs:131`** — If SQLite returns `NULL`, `query_row` gets `None``CypherResult::empty()` silently. Could happen if Clotho uses a different execution path.
51+
- **`result.rs:424-425`** — Empty string from extension → silent empty result.
52+
- **`result.rs:437-444`** — Non-JSON strings (e.g., malformed error messages) are silently wrapped as `{"result": "..."}` instead of raising an error.
53+
- **`result.rs:450-451`** — Empty JSON arrays → silent empty result, no way to distinguish from "query returned no matches."
54+
55+
Clotho may also be using `conn.query_row` or `conn.execute` directly instead of the `cypher()` wrapper, which could bypass result parsing.
56+
57+
## Acceptance Criteria
58+
59+
- [ ] Reproduce the failures in the Rust integration test harness, or confirm they are Clotho-side
60+
- [ ] If Rust binding issue: fix silent empty results — return errors instead of empty `CypherResult`
61+
- [ ] If Clotho-side: document correct usage patterns and close
62+
- [ ] Add Rust integration tests for count(), OPTIONAL MATCH with nulls, and property-match syntax
63+
64+
## Implementation Notes
65+
66+
### Investigation Steps
67+
68+
1. Write Rust integration tests that mirror the exact Clotho queries
69+
2. Check if Clotho pins an older graphqlite-rs version that might have bugs since fixed
70+
3. Check if Clotho uses `execute()` instead of `query_row()` for read queries (would discard results)
71+
4. Review Clotho's `Cargo.lock` for graphqlite version
72+
73+
### Risk Considerations
74+
- May require changes in the Clotho repo rather than graphqlite
75+
- Silent empty results are a usability hazard regardless — worth hardening even if not the root cause
76+
77+
## Status Updates
78+
79+
### 2026-03-21: Rust binding path verified — not the cause
80+
81+
**Investigation complete.** Added 5 Rust integration tests mirroring exact Clotho query patterns (`test_clotho_bug1` through `test_clotho_bug5` + `test_clotho_pattern_predicate_in_where`). All pass through the full Rust binding path (connection → query_row → JSON parsing → CypherResult).
82+
83+
**Key findings:**
84+
- `count()` with WHERE filter: works — returns `{"total":3}` correctly
85+
- `{key: 'value'}` property-match: works — parses and executes correctly
86+
- `OPTIONAL MATCH` with null: works — returns rows with `null` for unmatched optionals, `Option<String>` extracts correctly
87+
- `(a)--(b)` undirected: works on current (was fixed in v0.3.8)
88+
- Pattern predicates: works with updated extension binary
89+
90+
**Stale cache finding:** The bundled extension binary caching in `platform.rs` (versioned filename + size-only check) can serve stale binaries if the binary is rebuilt without a version bump. This caused a false-negative during testing. Not likely the Clotho root cause, but a potential footgun.
91+
92+
**Verdict:** The issue is Clotho-side, not in graphqlite or its Rust bindings. Recommend:
93+
1. Check Clotho's Cargo.lock for pinned graphqlite version
94+
2. Check if Clotho constructs queries with incorrect escaping
95+
3. Check if Clotho uses rusqlite `execute()` for reads (would discard results)
96+
4. Close this ticket once Clotho is investigated
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
---
2+
id: existential-subquery-syntax-exists
3+
level: task
4+
title: "Existential subquery syntax: EXISTS { MATCH ... WHERE ... }"
5+
short_code: "GQLITE-T-0139"
6+
created_at: 2026-03-22T00:45:57.656842+00:00
7+
updated_at: 2026-03-22T00:45:57.656842+00:00
8+
parent:
9+
blocked_by: []
10+
archived: false
11+
12+
tags:
13+
- "#task"
14+
- "#phase/backlog"
15+
- "#feature"
16+
17+
18+
exit_criteria_met: false
19+
initiative_id: NULL
20+
---
21+
22+
# Existential subquery syntax: EXISTS { MATCH ... WHERE ... }
23+
24+
## Objective
25+
26+
Implement full existential subquery syntax `EXISTS { MATCH ... WHERE ... }` per CIP2015-05-13-EXISTS. Currently only `EXISTS((pattern))` and `EXISTS(property)` are supported. The full subquery form allows filtering within the existence check.
27+
28+
## Background
29+
30+
**Reported from Clotho integration (v0.3.7).** Query:
31+
```cypher
32+
MATCH (t) WHERE t.entity_type = 'Transcript'
33+
AND NOT EXISTS { MATCH (e)-[:EXTRACTED_FROM]->(t) }
34+
RETURN t.title
35+
```
36+
Fails with: `syntax error, unexpected '{'`
37+
38+
**Note:** The reporter also used a double-WHERE variant (`WHERE ... WHERE NOT EXISTS ...`) which is invalid Cypher regardless. The single-WHERE + AND form is the correct syntax.
39+
40+
**Workarounds that already work:**
41+
- `EXISTS((t)<-[:EXTRACTED_FROM]-())` — simple pattern form
42+
- `NOT (t)<-[:EXTRACTED_FROM]-()` — pattern predicate (added in v0.3.10)
43+
44+
**Spec basis:** CIP2015-05-13-EXISTS defines three levels:
45+
1. `EXISTS(property)` — property existence (supported)
46+
2. `EXISTS((pattern))` — pattern existence (supported)
47+
3. `EXISTS { MATCH pattern [WHERE filter] }` — full subquery (NOT supported)
48+
49+
## Acceptance Criteria
50+
51+
- [ ] `EXISTS { MATCH (a)-[:REL]->(b) }` parses and evaluates as existence check
52+
- [ ] `NOT EXISTS { MATCH (a)-[:REL]->(b) }` works for non-existence
53+
- [ ] `EXISTS { MATCH (a)-[:REL]->(b) WHERE b.prop = value }` supports filtering within the subquery
54+
- [ ] Variables from outer scope are correctly correlated (e.g., `t` from outer MATCH)
55+
- [ ] Unit tests and functional SQL tests added
56+
- [ ] Existing `EXISTS((pattern))` and `EXISTS(property)` behavior unaffected
57+
58+
## Implementation Notes
59+
60+
### Technical Approach
61+
62+
**Grammar (`cypher_gram.y`):** Add production for `EXISTS '{' clause_list '}'` or a specialized `EXISTS '{' MATCH pattern_list where_opt '}'` rule. The braces `{` `}` already tokenize correctly (used for map literals). The main challenge is disambiguating `EXISTS { ... }` from `EXISTS(...)`.
63+
64+
**AST:** May need a new AST node type (`AST_NODE_EXISTS_SUBQUERY`) or extend `cypher_exists_expr` with a subquery variant that holds a full `MATCH` + optional `WHERE`.
65+
66+
**Transform:** The subquery needs its own variable scope that can reference outer variables. Generate a correlated `EXISTS (SELECT 1 FROM ... WHERE ...)` SQL subquery, similar to the pattern form but with additional WHERE conditions from the subquery body.
67+
68+
### Dependencies
69+
- Pattern predicate support (GQLITE-T-0138) — completed
70+
- CALL subquery (GQLITE-T-0133) may share scoping infrastructure
71+
72+
### Risk Considerations
73+
- Variable scoping: inner variables must not leak to outer scope, but outer variables must be accessible in the subquery
74+
- Grammar conflicts with `EXISTS(...)` form — need careful disambiguation of `{` vs `(`
75+
76+
## Status Updates
77+
78+
*To be added during implementation*

bindings/rust/Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bindings/rust/tests/integration.rs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3268,3 +3268,111 @@ fn test_negative_epoch() {
32683268
assert!(val.contains("1969-12-31"));
32693269
}
32703270

3271+
// =============================================================================
3272+
// Clotho Bug Report Reproduction Tests
3273+
// These mirror the exact query patterns reported as broken from the Clotho
3274+
// integration (v0.3.7, rusqlite 0.31). All should pass — if any fail, the
3275+
// issue is in the Rust binding layer, not the C extension.
3276+
// =============================================================================
3277+
3278+
#[test]
3279+
fn test_clotho_bug1_count_aggregate_with_where_filter() {
3280+
// Reported: count() returns column headers but no rows
3281+
let conn = test_connection();
3282+
conn.cypher("CREATE (:Decision {entity_type: 'Decision', title: 'D1'})").unwrap();
3283+
conn.cypher("CREATE (:Decision {entity_type: 'Decision', title: 'D2'})").unwrap();
3284+
conn.cypher("CREATE (:Decision {entity_type: 'Decision', title: 'D3'})").unwrap();
3285+
conn.cypher("CREATE (:Other {entity_type: 'Other', title: 'O1'})").unwrap();
3286+
3287+
let results = conn
3288+
.cypher("MATCH (n) WHERE n.entity_type = 'Decision' RETURN count(n) AS total")
3289+
.unwrap();
3290+
assert_eq!(results.len(), 1, "count() should return exactly one row");
3291+
assert_eq!(results[0].get::<i64>("total").unwrap(), 3);
3292+
}
3293+
3294+
#[test]
3295+
fn test_clotho_bug2_property_match_syntax() {
3296+
// Reported: {key: 'value'} causes "syntax error, unexpected ':'"
3297+
let conn = test_connection();
3298+
conn.cypher("CREATE (:Decision {entity_type: 'Decision', id: 'd1', title: 'First'})").unwrap();
3299+
conn.cypher("CREATE (:Decision {entity_type: 'Decision', id: 'd2', title: 'Second'})").unwrap();
3300+
3301+
let results = conn
3302+
.cypher("MATCH (n {entity_type: 'Decision'}) RETURN n.id, n.title")
3303+
.unwrap();
3304+
assert_eq!(results.len(), 2, "property-match should find 2 decisions");
3305+
}
3306+
3307+
#[test]
3308+
fn test_clotho_bug3_optional_match_with_where_filter() {
3309+
// Reported: OPTIONAL MATCH produces no results at all
3310+
let conn = test_connection();
3311+
conn.cypher("CREATE (:ClothoDecision {entity_type: 'Decision', title: 'Connected'})").unwrap();
3312+
conn.cypher("CREATE (:ClothoDecision {entity_type: 'Decision', title: 'Orphan'})").unwrap();
3313+
conn.cypher("CREATE (:ClothoProgram {entity_type: 'Program', title: 'Alpha'})").unwrap();
3314+
conn.cypher("MATCH (d:ClothoDecision {title: 'Connected'}), (p:ClothoProgram {title: 'Alpha'}) CREATE (d)-[:BELONGS_TO]->(p)").unwrap();
3315+
3316+
let results = conn
3317+
.cypher("MATCH (d) WHERE d.entity_type = 'Decision' OPTIONAL MATCH (d)-[:BELONGS_TO]->(p) RETURN d.title, p.title AS program")
3318+
.unwrap();
3319+
assert_eq!(results.len(), 2, "OPTIONAL MATCH should return all decisions, including unmatched");
3320+
3321+
// Find the connected and orphan rows
3322+
let mut found_connected = false;
3323+
let mut found_orphan = false;
3324+
for row in results.iter() {
3325+
let title: String = row.get("d.title").unwrap();
3326+
if title == "Connected" {
3327+
assert_eq!(row.get::<String>("program").unwrap(), "Alpha");
3328+
found_connected = true;
3329+
} else if title == "Orphan" {
3330+
assert!(row.get::<Option<String>>("program").unwrap().is_none());
3331+
found_orphan = true;
3332+
}
3333+
}
3334+
assert!(found_connected, "Should find the connected decision");
3335+
assert!(found_orphan, "Should find the orphan decision with null program");
3336+
}
3337+
3338+
#[test]
3339+
fn test_clotho_bug5_undirected_match_bare() {
3340+
// Reported: (a)--(b) not supported
3341+
let conn = test_connection();
3342+
conn.cypher("CREATE (:UndirA {name: 'Alice'})").unwrap();
3343+
conn.cypher("CREATE (:UndirB {name: 'Bob'})").unwrap();
3344+
conn.cypher("MATCH (a:UndirA), (b:UndirB) CREATE (a)-[:KNOWS]->(b)").unwrap();
3345+
3346+
// Bare -- syntax
3347+
let results = conn
3348+
.cypher("MATCH (a)--(b) WHERE a.name = 'Alice' RETURN b.name")
3349+
.unwrap();
3350+
assert_eq!(results.len(), 1);
3351+
assert_eq!(results[0].get::<String>("b.name").unwrap(), "Bob");
3352+
3353+
// -[]- syntax
3354+
let results2 = conn
3355+
.cypher("MATCH (a)-[]-(b) WHERE a.name = 'Alice' RETURN b.name")
3356+
.unwrap();
3357+
assert_eq!(results2.len(), 1);
3358+
assert_eq!(results2[0].get::<String>("b.name").unwrap(), "Bob");
3359+
}
3360+
3361+
#[test]
3362+
fn test_clotho_pattern_predicate_in_where() {
3363+
// The original trigger for this investigation:
3364+
// MATCH (n {entity_type: 'Note'}) WHERE NOT (n)-[:BELONGS_TO]->() RETURN n.id, n.title
3365+
let conn = test_connection();
3366+
conn.cypher("CREATE (:Note {entity_type: 'Note', id: 'n1', title: 'Orphan'})").unwrap();
3367+
conn.cypher("CREATE (:Note {entity_type: 'Note', id: 'n2', title: 'Connected'})").unwrap();
3368+
conn.cypher("CREATE (:Group {id: 'g1'})").unwrap();
3369+
conn.cypher("MATCH (n:Note {id: 'n2'}), (g:Group {id: 'g1'}) CREATE (n)-[:BELONGS_TO]->(g)").unwrap();
3370+
3371+
let results = conn
3372+
.cypher("MATCH (n {entity_type: 'Note'}) WHERE NOT (n)-[:BELONGS_TO]->() AND NOT (n)-[:RELATES_TO]->() RETURN n.id, n.title")
3373+
.unwrap();
3374+
assert_eq!(results.len(), 1);
3375+
assert_eq!(results[0].get::<String>("n.id").unwrap(), "n1");
3376+
assert_eq!(results[0].get::<String>("n.title").unwrap(), "Orphan");
3377+
}
3378+

0 commit comments

Comments
 (0)