From 4622297543b460773241e19be679811436dd5bf2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 22:09:45 +0000 Subject: [PATCH] feat(manifest): drop unimplemented json sidecar store (#112) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit V-L2-F2, split from #45. The `[sidecar].storage` enum advertised `sqlite | json`, but `json` never had an implementation and #45 left it rejected with a "tracked by #112" pointer. Decision: drop `json` rather than implement it. The octad data layer is intrinsically relational — provenance hash-chains rely on `BEGIN IMMEDIATE` serialisation, temporal "exactly one current version" is a partial UNIQUE index, enum columns are `CHECK`-constrained, and lineage acyclicity is a recursive-CTE concern. A JSON document store "with the same octad-dimension coverage" would mean reimplementing that entire layer (plus gc/drift) against a non-relational model. The ROADMAP lists only the SQLite sidecar (Phase 1) and never references a JSON store; its "Multi-Backend" phase is about interception sources, not sidecar formats. Changes (the drop path from the issue's acceptance): - `SqlDialect::from_storage` no longer special-cases `json`; it falls into the generic unsupported-value error, which now advertises only the supported stores (no "#112"/"coming soon" pointer). This stays the single source of truth for accepted `[sidecar].storage` values. - `validate` gains a `sidecar-storage-supported` check that defers to `from_storage`, so `validate`/`doctor` surface a dropped/typo'd backend before it reaches codegen. - `SidecarConfig` docs (manifest + abi), the manifest template comment, and the `generate` call-site comment no longer present `json` as an option. - Tests: `unsupported_storage_fails` (validate) and an updated `test_storage_to_dialect_mapping` assert `json` is rejected plainly and never advertised as planned. https://claude.ai/code/session_01S2xDQQU5o85N3xTpeFUSfN --- src/abi/mod.rs | 2 +- src/codegen/overlay.rs | 41 +++++++++++++++--------- src/main.rs | 6 ++-- src/manifest/mod.rs | 71 +++++++++++++++++++++++++++++++++++++++--- 4 files changed, 97 insertions(+), 23 deletions(-) diff --git a/src/abi/mod.rs b/src/abi/mod.rs index be2f1cc..6b94921 100644 --- a/src/abi/mod.rs +++ b/src/abi/mod.rs @@ -460,7 +460,7 @@ impl AccessPolicy { /// with validation and path resolution. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SidecarConfig { - /// Storage backend: "sqlite" or "json". + /// Storage backend: "sqlite" (default) or "postgres"/"postgresql". pub storage: String, /// File path for the sidecar database. pub path: String, diff --git a/src/codegen/overlay.rs b/src/codegen/overlay.rs index 893231e..d7650d7 100644 --- a/src/codegen/overlay.rs +++ b/src/codegen/overlay.rs @@ -74,6 +74,10 @@ fn must_validate_identifier(name: &str) -> &str { /// genuinely dialect-divergent fragment is the metadata upsert /// (`INSERT OR IGNORE` vs `INSERT … ON CONFLICT DO NOTHING`), which lives /// in the [`sqlite`] / [`postgres`] modules. +/// +/// [`from_storage`](SqlDialect::from_storage) is the single source of +/// truth for which `[sidecar].storage` values are accepted; `generate`, +/// `validate`, and `doctor` all defer to it. #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum SqlDialect { Sqlite, @@ -81,23 +85,24 @@ pub enum SqlDialect { } impl SqlDialect { - /// Map a `[sidecar].storage` value to a dialect. `sqlite` → - /// [`SqlDialect::Sqlite`]; `postgres`/`postgresql` → - /// [`SqlDialect::Postgres`]. `json` and unknown values are rejected - /// (the previous behaviour silently emitted SQLite DDL regardless, - /// V-L2-F1). The JSON store is tracked separately by #112. + /// Map a `[sidecar].storage` value to a dialect (case-insensitive): + /// `sqlite` → [`SqlDialect::Sqlite`]; `postgres`/`postgresql` → + /// [`SqlDialect::Postgres`]. Every other value is rejected rather than + /// silently emitting SQLite DDL regardless of the backend (V-L2-F1). + /// + /// The octad data layer is intrinsically relational (hash-chains under + /// `BEGIN IMMEDIATE`, partial-unique temporal indexes, `CHECK` + /// constraints, recursive-CTE lineage acyclicity), so the + /// never-implemented `json` document store was dropped rather than + /// built (V-L2-F2, #112). It is now an unsupported value like any + /// other — no special-casing, no "coming soon" pointer. pub fn from_storage(storage: &str) -> anyhow::Result { match storage.to_lowercase().as_str() { "sqlite" => Ok(SqlDialect::Sqlite), "postgres" | "postgresql" => Ok(SqlDialect::Postgres), - "json" => anyhow::bail!( - "[sidecar].storage = \"json\" is not implemented (it previously \ - emitted SQLite DDL silently). Use \"sqlite\". The JSON sidecar \ - store is tracked by hyperpolymath/verisimiser#112." - ), other => anyhow::bail!( - "unknown [sidecar].storage {other:?}; supported: \"sqlite\" \ - (\"postgres\" for a PostgreSQL sidecar; \"json\" is #112)." + "unsupported [sidecar].storage {other:?}; supported values are \ + \"sqlite\" (default) and \"postgres\"/\"postgresql\"." ), } } @@ -879,10 +884,18 @@ mod tests { SqlDialect::from_storage("PostgreSQL").unwrap(), SqlDialect::Postgres ); + // V-L2-F2 (#112): the json store was dropped, never implemented. It + // is now rejected like any other unsupported value, and the error + // advertises only the supported stores — it must NOT imply json is + // planned (no "#112" / "not implemented" pointer). let json_err = SqlDialect::from_storage("json").unwrap_err().to_string(); assert!( - json_err.contains("not implemented") && json_err.contains("#112"), - "json must be rejected with the #112 pointer, got: {json_err}" + json_err.contains("unsupported") && json_err.contains("sqlite"), + "json must be rejected as an unsupported store, got: {json_err}" + ); + assert!( + !json_err.contains("#112") && !json_err.to_lowercase().contains("not implemented"), + "the dropped json store must not be advertised as planned, got: {json_err}" ); assert!(SqlDialect::from_storage("mariadb").is_err()); } diff --git a/src/main.rs b/src/main.rs index c5df959..0e7de2e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -209,9 +209,9 @@ fn main() -> Result<()> { // Create output directory. std::fs::create_dir_all(&output)?; - // The sidecar DDL dialect follows [sidecar].storage. This - // rejects `json` (tracked by #112) instead of silently - // emitting SQLite DDL for a non-SQLite store (V-L2-F1). + // The sidecar DDL dialect follows [sidecar].storage. Any value + // other than sqlite/postgres is rejected here (V-L2-F1) instead + // of silently emitting SQLite DDL for a non-SQLite store. let dialect = codegen::overlay::SqlDialect::from_storage(&m.sidecar.storage)?; // Generate sidecar overlay schema. Errors here surface invalid diff --git a/src/manifest/mod.rs b/src/manifest/mod.rs index 3659e5b..4bf4379 100644 --- a/src/manifest/mod.rs +++ b/src/manifest/mod.rs @@ -279,11 +279,11 @@ mod octad_tests { /// temporal versions, and access policies. It never writes to your target database. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SidecarConfig { - /// Storage backend for the sidecar. `"sqlite"` (default) is the only - /// implemented store; `"postgres"`/`"postgresql"` selects the - /// PostgreSQL DDL dialect. `"json"` is **not implemented** and is - /// rejected at `generate` time — tracked by #112 (V-L2-F2). The - /// dialect mapping lives in `codegen::overlay::SqlDialect`. + /// Storage backend for the sidecar. `"sqlite"` (default) is the + /// reference store; `"postgres"`/`"postgresql"` selects the PostgreSQL + /// DDL dialect. Any other value is rejected at `validate` and + /// `generate` time by `codegen::overlay::SqlDialect::from_storage`, + /// the single source of truth for supported stores. #[serde(default = "default_sidecar_storage")] pub storage: String, @@ -394,6 +394,41 @@ mod validate_manifest_tests { assert_eq!(failed, vec!["schema-source-exists"]); } + /// An unsupported `[sidecar].storage` (here the dropped `json` store, + /// V-L2-F2 / #112) must fail `sidecar-storage-supported`, and the + /// failure detail must not advertise json as a planned option. + #[test] + fn unsupported_storage_fails() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("verisimiser.toml"); + let sidecar_path = dir.path().join("sidecar.db"); + let body = format!( + "[project]\n\ + name = \"test\"\n\ + [database]\n\ + backend = \"sqlite\"\n\ + [sidecar]\n\ + storage = \"json\"\n\ + path = \"{}\"\n", + sidecar_path.display().to_string().replace('\\', "/") + ); + std::fs::write(&path, body).expect("write"); + + let report = validate_manifest(path.to_str().unwrap()); + assert!(!report.passed); + let storage = report + .checks + .iter() + .find(|c| c.name == "sidecar-storage-supported") + .expect("storage check must run"); + assert!(!storage.passed, "json storage must fail the check"); + let detail = storage.detail.as_deref().unwrap_or_default(); + assert!( + detail.contains("unsupported") && !detail.contains("#112"), + "detail must reject json plainly without a 'coming soon' pointer; got: {detail}" + ); + } + /// A malformed manifest must fail `manifest-loads` and stop further /// checks (because the rest depend on having a parsed manifest). #[test] @@ -621,6 +656,7 @@ enable-constraints = {enable_constraints} enable-simulation = {enable_simulation} [sidecar] +# storage backend: "sqlite" (default) or "postgres"/"postgresql" storage = "{sidecar_storage}" path = "{sidecar_path}" @@ -734,6 +770,9 @@ impl ValidationReport { /// set, the file at that path is readable. /// 3. **`sidecar-path-writable`** — the parent directory of /// `[sidecar].path` is writable (or createable). +/// 4. **`sidecar-storage-supported`** — `[sidecar].storage` names a +/// backend `codegen` can emit (`sqlite`/`postgres`). Catches typos and +/// the dropped `json` store (V-L2-F2 / #112). /// /// Out of scope here: V-L2-E1 backend/target_db conflict (own issue), /// target-DB reachability (needs live connection). @@ -820,6 +859,28 @@ pub fn validate_manifest(path: &str) -> ValidationReport { )), }); } + + // 4. Sidecar storage backend is supported. Delegates to the one + // validator (`SqlDialect::from_storage`) so `validate`/`doctor` + // and `generate` agree on the accepted set. This is where a typo'd + // or dropped backend (e.g. the never-implemented `json` store, + // V-L2-F2 / #112) is surfaced before it reaches codegen. + let storage_check = ValidationCheck { + name: "sidecar-storage-supported".to_string(), + description: "[sidecar].storage names a supported backend".to_string(), + passed: true, + detail: None, + }; + checks.push( + match crate::codegen::overlay::SqlDialect::from_storage(&m.sidecar.storage) { + Ok(_) => storage_check, + Err(e) => ValidationCheck { + passed: false, + detail: Some(e.to_string()), + ..storage_check + }, + }, + ); } let passed = checks.iter().all(|c| c.passed);