Skip to content

Sprint 01: SurrealDB spike — parity adapters, migrations, bench (ADR-01) - #1

Merged
Thib2108 merged 13 commits into
mainfrom
sprint-01-surreal-spike
Jul 3, 2026
Merged

Thib2108 merged 13 commits into
mainfrom
sprint-01-surreal-spike

Conversation

@Thib2108

@Thib2108 Thib2108 commented Jul 3, 2026

Copy link
Copy Markdown
Owner

What

Sprint-01 SurrealDB spike (SDAI-CHAT-003 ADR-01 verdict machinery):

  • db/surreal.py — env-driven connect(), idempotent apply_migrations() (tracked in a migration table), all SurrealQL constants live here (Principle 4).
  • repos/surreal_impl.py — SurrealMessageStore + SurrealIntentLog satisfying the ports Protocols structurally. Append is one BEGIN/COMMIT transaction: ensure conversation → conv_counter UPSERT n += 1 RETURN AFTER → CREATE message with the allocated turn. Server stamps id (rand::ulid()), turn, created_at (schema VALUE). Python byte-check is the authoritative 256 KB cap. No update/delete methods — append-only by construction.
  • 0001_core.surql — sanctioned additions (base stays verbatim from spec Block 5): conversation table, conv_counter (n DEFAULT 0), and intent_log field definitions (bug: SCHEMAFULL with no fields strips every write — the outbox would have silently stored empty records).
  • tests/conftest.py — surreal params now run when SURREAL_URL is set; each test gets a fresh isolated database with migrations applied. Skips cleanly otherwise.
  • scripts/bench_snapshot.py — ADR-01 tripwire: 1,000 appends, 200 snapshots, p50/p95/p99/max; exit 1 if p95 ≥ 100 ms.
  • Removed 4 exploration scratch files from the WIP handoff.

Contract-test bodies: untouched (standing rule)

Verdict process

  1. CI must be green: memory params + surreal params against a fresh in-memory SurrealDB.
  2. Bench p95 < 100 ms measured locally on real hardware decides ADR-01 (CI bench is informational — runners are noisy).
  3. Do not merge until the architect signs off and the spec gets its 0.1.2 sync (fold the sanctioned .surql additions into SDAI-CHAT-003 Block 5).

Known risks for review

  • SurrealQL result shapes vary by SDK version; _normalise_rows is deliberately tolerant. If CI shows shape errors, fix parsing — not the tests.
  • string::len ASSERT counts chars, not bytes — adapter byte-check is authoritative (belt-and-braces by design).

Knowledge Processor Dev and others added 13 commits July 3, 2026 15:34
…res, bench

- append is now ONE BEGIN/COMMIT transaction (counter + create); no crash gaps
- 0001_core.surql sanctioned additions: conversation, conv_counter (DEFAULT 0),
  intent_log fields (SCHEMAFULL was stripping undefined fields — outbox breaker)
- type::record -> type::thing (documented API); created_at left to schema VALUE
- conftest: surreal params run when SURREAL_URL set, isolated db per test
- scripts/bench_snapshot.py: ADR-01 tripwire (1k msgs, 200 snapshots, p50/95/99)
- spec sync note: fold .surql additions into SDAI-CHAT-003 Block 5 at 0.1.2 bump
CI surreal params all errored in fixture setup: apply_migrations hit
`Parse error: Unexpected token 'an identifier', expected Eof --> [12:56]`
— column 56 of line 12 is MTREE. The surrealdb/surrealdb:latest image is
v3, which dropped MTREE; HNSW is the supported (and better) vector index.
Embeddings are L2-normalized, so default Euclidean distance ranks
identically to cosine. Sanctioned minimal fix; fold into SDAI-CHAT-003
Block 5 at the 0.1.2 spec sync.
The 2.x-to-3.x migration guide lists type::record as the new name for
type::thing ('consistent terminology', severity: will break). Restores
the function name Gemini originally used — the architect's swap to
type::thing was correct for v2 but wrong for the v3 image CI runs.
CI: 19 passed, 4 failed — all four failures were append getting None
back from the BEGIN/COMMIT block. The SDK returns the last statement's
result, and COMMIT yields nothing. Fix per the original architect ruling:
LET $msg = (CREATE ...); RETURN $msg; inside the transaction — the
transaction's return value is then the created record.

Also: bounded retry (25 attempts, linear backoff) when SurrealDB's
optimistic transactions report a conflict/retry error — relevant for the
50-concurrent-appends contract test hitting one counter record. Safe
because the transaction is all-or-nothing: a conflicted attempt creates
nothing, so a retry can never double-append.
…txns)

The Python SDK yields None for any BEGIN/COMMIT batch, even with an
explicit RETURN — confirmed across two CI runs. Restructure per the
original ruling #5: the adapter generates the ULID (dependency-free
Crockford base32 generator, no lockfile churn), the transaction is
write-only, and the message is read back by known id with a plain
single-statement SELECT — the shape that demonstrably works (the whole
intent-log suite passes on single statements).

turn and created_at remain server-stamped; atomicity is unchanged
(counter + create still commit together, retry-on-conflict retained).
…ently

CI: 22/23. Last failure showed conflicted transactions abort WITHOUT
raising — the SDK returns None for txn batches either way, so the
exception-based retry never fired. The read-back by known ULID is now
the success check: message present -> committed; absent -> conflict
abort -> retry the whole transaction.

Safety: an aborted txn creates nothing and rolls back the counter (no
turn gaps); a duplicate commit would fail loudly on the existing ULID
rather than double-append.

Thib2108 commented Jul 3, 2026

Copy link
Copy Markdown
Owner Author

✅ CI green — 23/23

Full contract suite passes identically against memory and surreal params. Sprint 01 is code-complete. Remaining before merge: local bench run for the ADR-01 p95 verdict + spec sync (below) + architect sign-off.

SurrealQL / SDK learnings to fold into SDAI-CHAT-003 @ 0.1.2

All four CI failures were adapter/migration-side; the contract tests were never modified.

  1. MTREE removed in SurrealDB 3.x → vector index is now HNSW DIMENSION 768 (embeddings are L2-normalized, so default Euclidean ≡ cosine ranking).
  2. type::thing() renamed to type::record() in 3.x — used throughout db/surreal.py.
  3. SDK returns None for any BEGIN/COMMIT batch, even with an explicit RETURN. Consequence: transactions must be write-only; results are read back with single-statement queries afterwards.
  4. Conflicted transactions abort silently (no exception, same None). Consequence: the adapter generates the message ULID itself (per ruling #5), and the read-back by known id doubles as the commit/abort check — absent record = conflict = retry the whole transaction (50 attempts, linear backoff). Safe: aborts roll back the counter atomically (no turn gaps), and a duplicate commit would fail loudly on the existing ULID.

Item 4 was a real production bug, not a test artifact — without the read-back check, concurrent appends would silently drop messages in the live app. Exactly what 003 Block 7 test #6 exists to catch.

Bench (local, authoritative)

cd services/api
uv run python scripts/bench_snapshot.py

with SurrealDB running. CI bench is informational only (noisy runners). Tripwire: p95 < 100ms or ADR-01 gets revisited.

@Thib2108
Thib2108 marked this pull request as ready for review July 3, 2026 14:13
@Thib2108
Thib2108 merged commit 9cce27e into main Jul 3, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant