ADORSYS-GIS / ADORSYS-GIS/lci-mcp
[Ticket]: Evaluate cratestack's embedded API for engine-core's SQLite layer
- Dominant language
- Rust
- Stars
- 0
- Forks
- 0
- Avg merge
- 8h 2m
- Merged PRs (30d)
- 17
Description
### Type
Spike / Investigation
### Summary
We needed to evaluate replacing `engine-core`'s hand-written SQL (`store/schema.rs`, `store/chunks.rs`, `store/graph.rs`, `store/vectors.rs`) with [cratestack](https://github.com/cratestack/cratestack)'s embedded ORM (`include_embedded_schema!` + `cratestack-rusqlite`) because a code review on PR #1 asked us to check its embedded API against our manual SQL.
Expected result:
> A recorded go/no-go decision, backed by an actual attempted integration (not just documentation), so this doesn't get re-litigated from scratch later.
### Intent
Determine whether adopting cratestack's schema-first embedded API would reduce hand-written SQL in `engine-core` without compromising the transactional guarantees the generation-based indexing lifecycle depends on (ADR-0004) — and record the answer with evidence so it doesn't need re-investigating.
### Source of truth (links)
- Code review comment on PR #1 (senior reviewer): "Check cratestack's embedded API and replace this manual SQL" — https://github.com/cratestack/cratestack
- #1
### Current Behavior
`engine-core` hand-writes all SQL directly against `rusqlite`: DDL lives in `packages/engine-core/migrations/*.sql` (embedded via `include_str!`), and every read/write is a manually composed `conn.execute(...)`/`conn.prepare(...)` call across `store/schema.rs`, `store/chunks.rs`, `store/graph.rs`, `store/vectors.rs`, and `store/mod.rs`. For example, the single-row upsert this spike targeted first:
```rust
// packages/engine-core/src/store/mod.rs (current, unchanged)
pub fn set_repository_metadata(&self, repo_key: &str, canonical_root: &str, remote_identity: Option<&str>) -> anyhow::Result<()> {
self.with_conn(|conn| {
conn.execute(
"INSERT INTO repository_metadata (id, repo_key, canonical_root, remote_identity) \
VALUES (1, ?1, ?2, ?3) \
ON CONFLICT(id) DO UPDATE SET repo_key = excluded.repo_key, \
canonical_root = excluded.canonical_root, remote_identity = excluded.remote_identity",
params![repo_key, canonical_root, remote_identity],
)?;
Ok(())
})
}
```
against its schema (`packages/engine-core/migrations/0001_init.sql`):
```sql
CREATE TABLE IF NOT EXISTS repository_metadata (
id INTEGER PRIMARY KEY CHECK (id = 1),
repo_key TEXT NOT NULL,
canonical_root TEXT NOT NULL,
remote_identity TEXT
);
```
### Expected Behavior
N/A — this is a spike. The expected end-state is a documented decision here, plus follow-up tickets only if a genuinely applicable, safe subset of the schema was identified.
### Acceptance Criteria
- [x] `cratestack-sqlite` / `cratestack-macros` / `cratestack-rusqlite` evaluated against `engine-core`'s actual schema and code, not just the project's README.
- [x] A concrete attempt made (dependency added, `.cstack` schema written, real `cargo check` run) rather than a purely theoretical read-through.
- [x] Verdict recorded with the specific evidence that drove it.
- [x] Working tree left clean either way (no half-integrated dependency).
- [ ] Follow-up ticket(s) opened if any partial adoption is still worth pursuing later (none currently planned — see Verdict).
### Out of Scope
- Migrating `chunk_vectors` (a `sqlite-vec` `vec0` virtual table) or `explore_symbol`'s `WITH RECURSIVE` graph traversal to any ORM/schema-generator — both are raw-SQL-only SQLite features regardless of vendor, not specific to cratestack.
- Adopting cratestack for the Postgres/server side of anything (`engine-core` has no server component).
### Technical Context
Relevant files: `packages/engine-core/src/store/{schema,chunks,graph,vectors,mod}.rs`, `packages/engine-core/migrations/0001_init.sql`, root `Cargo.toml` (`[workspace.dependencies]`).
#### What was actually tried
1. Added the real dependency to the workspace:
```toml
# Cargo.toml [workspace.dependencies] — exploratory, reverted
cratestack = { package = "cratestack-sqlite", version = "0.9" }
cratestack-macros = "0.9"
cratestack-rusqlite = "0.9"
```
2. Picked the single safest, most isolated candidate table — `repository_metadata` (one always-`id = 1` row, no cross-table transaction coupling) — and wrote a `.cstack` schema for it:
```
// packages/engine-core/schema.cstack — exploratory, reverted
datasource db {
provider = "sqlite"
url = env("DATABASE_URL")
}
model RepositoryMetadata {
id Int @id
repoKey String
canonicalRoot String
remoteIdentity String?
}
```
3. Ran `cargo check -p lci-mcp-engine-core` to force real dependency resolution (not just read the README).
#### What failed, and why
**First failure — toolchain floor.** The local machine had rustc 1.95.0; cratestack 0.9.4 requires 1.98.0:
```
error: rustc 1.95.0 is not supported by the following packages:
cratestack-client-rust@0.9.4 requires rustc 1.98.0
cratestack-macros@0.9.4 requires rustc 1.98.0
cratestack-rusqlite@0.9.4 requires rustc 1.98.0
cratestack-sqlite@0.9.4 requires rustc 1.98.0
... (13 packages total)
```
Fixed with `rustup update stable` (this repo has no `rust-toolchain.toml` floor, so this isn't itself a blocker — just the first thing that had to happen before evaluation could continue). After that, `cargo check` succeeded and pulled the full dependency tree cleanly.
**Second, decisive finding — connection ownership.** `cratestack-rusqlite`'s `RusqliteRuntime` (`crates/cratestack-rusqlite/src/runtime.rs` in the cratestack repo) owns its own connection outright:
```rust
// cratestack-rusqlite/src/runtime.rs (upstream, read for this spike)
pub struct RusqliteRuntime {
conn: Mutex,
}
impl RusqliteRuntime {
pub fn open_in_memory() -> Result { /* ... */ }
pub fn open(path: impl AsRef) -> Result { /* ... */ }
pub fn with_connection(&self, f: F) -> Result
where F: FnOnce(&mut Connection) -> Result { /* ... */ }
}
```
There is no constructor that wraps an *existing* `rusqlite::Connection`. `SqliteStore` (`packages/engine-core/src/store/mod.rs`) already owns one `Mutex` that registers the `sqlite-vec` extension, sets WAL mode, and runs the migrations system — using `RusqliteRuntime` means a second, independent connection into the same file.
**Third, decisive finding — column typing.** `create_table_sql` (`crates/cratestack-rusqlite/src/ddl.rs`) generates every column as SQLite type `BLOB`, by explicit design:
```rust
// cratestack-rusqlite/src/ddl.rs (upstream, read for this spike)
pub fn create_table_sql(descriptor: &ModelDescriptor) -> String {
let mut sql = format!("CREATE TABLE IF NOT EXISTS {} (\n", descriptor.table_name);
for (idx, column) in descriptor.columns.iter().enumerate() {
if idx > 0 { sql.push_str(",\n"); }
let _ = write!(&mut sql, " {} BLOB", column.sql_name);
if column.sql_name == descriptor.primary_key { sql.push_str(" PRIMARY KEY"); }
}
// ...
}
```
The doc comment on that file explains the rationale (SQLite's TEXT/NUMERIC column affinity otherwise silently mangles bound values on write/read — `BLOB` is the one affinity that preserves the exact storage class of everything `cratestack-rusqlite` binds). Reasonable for cratestack's own design, but it means `repository_metadata.repo_key`/`canonical_root`/`remote_identity` — currently plain `TEXT` — would become `BLOB` if generated this way, sitting next to every other hand-authored table's real `INTEGER`/`TEXT` columns.
#### Trade-off summary
| | Hand-written SQL (current) | cratestack embedded (`include_embedded_schema!`) |
|---|---|---|
| Boilerplate | One `conn.execute(...)` per query, written by hand | Generated `ModelDelegate` (`.create()`, `.find_many().where_(...)`, `.update(id).set(...)`, `.batch_upsert(...)` with per-item `SAVEPOINT`s) |
| Connection | One `Mutex` shared by every table, WAL + `sqlite-vec` + migrations all on it | A second, independent `Mutex` per `RusqliteRuntime` |
| Column types | Real `INTEGER`/`TEXT`/`CHECK` constraints, matches the rest of the schema | Every column `BLOB`, by design — inconsistent with the rest of the DB file |
| Cross-table atomic transactions | `conn.unchecked_transaction()` used directly today for `chunks`+`graph_nodes`+`graph_edges`, and for generation activate/obsolete (ADR-0004) | No demonstrated multi-model transaction API for the embedded/rusqlite path found during this spike (the repo's own transaction-proof example, `examples/db-transaction-verification`, targets the Postgres/server path, not embedded) |
| `sqlite-vec` `vec0` virtual tables | Supported (hand-written) | Not a relational model; out of reach for any schema-first generator, not just cratestack |
| Recursive CTEs (`explore_symbol`) | Supported (hand-written) | Same — out of reach regardless of vendor |
| Maturity | N/A | Pre-1.0, active breaking changes (own `CHANGELOG.md` documents a breaking `0.3.0 → 0.4.0` package split) |
| Toolchain floor | Whatever `dtolnay/rust-toolchain@stable` provides | rustc 1.98 minimum as of 0.9.4 |
**Verdict: does not currently fit.** The two decisive, source-level blockers (separate connection; all-`BLOB` columns) apply even to the single simplest, most isolated table attempted. Combined with the pre-existing known gaps (no multi-model transaction story, no `vec0`/recursive-CTE support), adopting it anywhere in the current schema would be a net regression in consistency, not a cleanup.
### Risks
- cratestack is pre-1.0 and under active breaking change (its own `README.md`/`CHANGELOG.md` document a breaking `0.3.0 → 0.4.0` package split).
- `cratestack-sqlite`/`cratestack-macros`/`cratestack-rusqlite` 0.9.4 require **rustc 1.98** — one full minor ahead of what this machine had installed (1.95) at the time of the attempt. Not a blocker (this repo has no `rust-toolchain.toml` floor, and CI's `dtolnay/rust-toolchain@stable` already tracks current stable), but worth knowing if evaluating a newer cratestack release later.
### Test Plan
Added `cratestack = { package = "cratestack-sqlite", version = "0.9" }`, `cratestack-macros`, `cratestack-rusqlite` to the workspace; wrote a `.cstack` schema for `RepositoryMetadata` (the single safest, most isolated candidate — no cross-table transaction coupling); ran `cargo check -p lci-mcp-engine-core` to pull and resolve the real dependency tree, then read the actual `cratestack-rusqlite` source (`runtime.rs`, `ddl.rs`) rather than inferring from the README.
### Verification evidence
- `cargo check -p lci-mcp-engine-core` failed initially with the rustc-version error quoted above (had 1.95.0). Ran `rustup update stable`, then dependency resolution succeeded (~47s cold, pulling the full cratestack dependency tree — `cratestack-core`, `cratestack-parser`, `cratestack-sql`, `cratestack-policy`, `cratestack-macros`, `cratestack-rusqlite`, `cratestack-sqlite`, plus transitive deps like `reqwest`, `tower`, `rustls-platform-verifier`).
- Read `crates/cratestack-rusqlite/src/runtime.rs` in full: confirmed `RusqliteRuntime { conn: Mutex }`, opened independently via `RusqliteRuntime::open(path)` / `open_in_memory()` — no connection-sharing constructor exists.
- Read `crates/cratestack-rusqlite/src/ddl.rs` in full: confirmed `create_table_sql` emits every column as `BLOB`, with an explicit design-rationale doc comment (quoted above).
- Read `examples/embedded-cli/src/main.rs` (the project's own reference embedded-SQLite example) to confirm the real generated API shape: `ModelDelegate::new(runtime, &MODEL)`, `.create(Input).run()`, `.find_many().order_by(...).where_(...).limit(...).run()`, `.update(id).set(Input).run()`, `.delete(id).run()`, `.batch_update(items).run()`, `.batch_delete(ids).run()`, `.batch_upsert(inputs).run()` (each batch item under its own `SAVEPOINT`, not one shared transaction).
- Reverted the exploratory dependency additions and the `.cstack` file entirely (`git diff` on `Cargo.toml`/`packages/engine-core/Cargo.toml` is clean relative to before the attempt).
- Re-ran the full verification loop after reverting: `cargo build/test/clippy --workspace` (37 unit + 6 integration tests, 0 unexpected warnings), native addon rebuild, and the full `packages/server` suite (typecheck, 34 unit + 9 e2e tests, Biome) — all clean.
### Human accountable owner
@leghadjeu-christian
### AI Usage Declaration
- Understanding code
- Proposing implementation
- Generating code
- Reviewing the diff
### Human verification completed
_(Left unticked — this ticket was drafted and the spike executed by an AI session; the accountable human owner should review the verdict and evidence above before checking these.)_
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.