Baradb/docs/superpowers/specs/2026-07-30-stability-hardening-design.md

BaraDB Stability Hardening — Design & Findings

Date: 2026-07-30 Status: Implemented (phase A). Phases B/C proposed, awaiting decision.

Goal

"Make the database better" — chosen direction: stability first. Establish a verified baseline, fix what is actually broken, and make the whole test suite run with one command before attempting any large refactoring or new features.

Baseline established

• Debug build passes cleanly on Nim 2.2.10 (nimble build_debug).
• The hunos build failure from HUNOS_ISSUE.md is no longer an issue: hunos 1.3.3 is installed and contains the urandom fix. The nimble file already allows >= 1.3.0.
• Of the 13 test files in tests/, only test_all (+ stress_test in CI) ran automatically. Baseline run of the other 11: 10 pass, nimforum_smoke_test fails.

Changes implemented

1. Parser: header usable as a column name (bug fix)

Root cause. header is a keyword token (tkHeader) used by IMPORT/EXPORT ... HEADER. The parser never accepted it as an identifier, so any table with a column named header (e.g. the nimforum schema) failed with Expected identifier but got tkHeader.

Fix (src/barabadb/query/parser.nim):

• Added tkHeader to identLikeKinds (soft-keyword set used by expectIdent, line 38).
• Added tkHeader to the identifier branch of parsePrimary (line 84) so SELECT header ... and WHERE header = ... work.
• Dotted-path parsing (a.b.c) now uses expectIdent instead of expect(tkIdent) so post.header works too.

IMPORT/EXPORT parsing is unaffected: statement dispatch keys on the leading tkImport/tkExport and the clause parser peeks for tkHeader explicitly.

Regression tests (tests/bugfix_test.nim, new suite):

• CREATE TABLE / INSERT / SELECT / WHERE with a column named header.
• IMPORT FROM ... HEADER no and EXPORT TO ... HEADER yes still parse.

Note: IMPORT ... FORMAT csv currently fails because csv is also a keyword (tkCsv) and parseImportFrom expects tkIdent after FORMAT. Pre-existing limitation, not addressed here (out of scope; recorded for phase B).

2. All 13 test files wired into nimble test and CI

• baradadb.nimble test task now builds build/baradadb (the smoke test talks to it over TCP) and runs all 13 suites: quick embedded suites first, fuzz/property/stress last.
• .github/workflows/ci.yml runs nimble test (was: only test_all); the now-redundant separate stress-test steps were removed.
• Verified locally: full nimble test exits 0 with 648 passing checks.

2b. Soft-keyword cleanup (phase B3, implemented)

Extended the header approach to all clause-only keywords, so they work as table/column names everywhere (DDL, DML, aliases, dotted paths, CTEs, JOINs, MERGE, GRANT/REVOKE, SET): format, delimiter, batch, csv, ndjson, status, migration, apply, up, down, dryrun, user, policy, enable, disable, recover, before, after, instead, of.

• identLikeKinds and the parsePrimary identifier branch now include them.
• All 69 expect(tkIdent) call sites now use expectIdent — a strict superset, so previously valid SQL is unaffected (verified: full suite green).
• IMPORT/EXPORT: FORMAT csv/ndjson/json and HEADER true/false now parse (previously csv/true/false lexed as keywords and were rejected despite the grammar clearly intending them). Clause table names use expectIdent.

Structural keywords (where, group, order, join, on, for, using, view, trigger, import, export, grant, ...) remain reserved.

Regression tests (tests/bugfix_test.nim): a table with 21 keyword-named columns through CREATE/INSERT/UPDATE/SELECT/qualified refs, plus IMPORT/EXPORT keyword-value parsing.

3. ORC crash — reproduced, bisected, three root-cause attempts failed

nim.cfg forces --mm:arc because ORC's cycle collector crashed ("markGray/trace SIGSEGV after ~20 sequential INSERTs"). The ARC cycle-breaking in commit ed5a719 did not fix the ORC path.

Reproduction (tests/orc_repro.py): build the server with --mm:orc, drive 1000 sequential TCP INSERTs plus 10 concurrent connections. The server dies with the exact documented signature (handleClient → nimDecRefIsLastCyclicStatic → collectCyclesBacon → markGray → trace → SIGSEGV).

Bisect: 200 pings + 200 SELECTs over TCP are fine; the crash lands between 20 and 500 sequential INSERTs — INSERT path only.

Failed root-cause attempts:

1. ed5a719 — callback cycle breaks (shard/gossip).
2. {.cursor.} on ExecutionContext.registry — breaks the real DatabaseRegistry ↔ ExecutionContext cycle (kept: it is the correct ownership annotation regardless), but the crash persists unchanged.
3. Guarding ctx.onChange against zero WS subscribers (reverted: fixed nothing).

Conclusion: per the 3-strikes rule this is a deep ORC+async issue — possibly an upstream Nim 2.2.x ORC bug with async closure environments and/or complex generic types — not a single app-level cycle. ARC remains the supported memory manager (full suite green under it). The findings are recorded in nim.cfg and tests/orc_repro.py for a future attempt (e.g. re-test with a newer Nim runtime, or a minimal repro filed upstream).

Proposed next phases (not started)

• B2. Split query/executor.nim (5,398 lines) — DONE: split into 15 modules under query/exec/, executor.nim down to 1,578 lines, full suite green (650 checks).
• C. Features — real Raft network transport, persistence for graph/FTS/columnar engines, benchmark validation.
• ORC (blocked): re-test tests/orc_repro.py against a newer Nim runtime; if it persists, distill a minimal repro and file upstream. Not app-actionable today (see "ORC crash" section).

Verification evidence

• nimble test (all 13 suites): exit 0, 650 [OK], 0 failed — final run after all changes (phase A + B3 + cursor).
• nimforum_smoke_test (rebuilt server): all suites [OK], including NimForum schema creation (previously [FAILED]).
• B3 TDD: new keyword tests failed first with the expected tkFormat/tkCsv errors, then passed; test_all stayed green (461 [OK]).
• ORC investigation: embedded test_wire_insert_stress passes even when compiled with --mm:orc; the TCP server compiled with --mm:orc crashes as documented above. All shipped artifacts use ARC and are green.