nordisk/nornir

Warehouse-trait detach — design & phase log

Goal. Lift nornir's warehouse fully behind its Warehouse trait so alternate backends (a thin RemoteWarehouse gRPC client, DuckDB, …) plug in without touching the ~150 sync block_on call sites. The trait is the durable seam; IcebergWarehouse is the default (and, until this pass, only) impl.

This file is the running phase log for that detach. It is additive & phased — nornir is large and central, so no phase is allowed to regress the working warehouse (L2: the ~150 call sites keep passing).

The seam as it actually stands (audit)

The trait exists (src/warehouse/mod.rs), and open() / open_read_only() already return Box<dyn Warehouse>. But the audit found the seam is nominal, not yet load-bearing, for three concrete reasons:

1. The warehouse's OWN per-table modules are Iceberg-intrinsic, not accidentally coupled. ~40 free functions in src/warehouse/*.rs (release_events, saga_events, coverage, funnel, yggdrasil, test_results, …) take wh: &IcebergWarehouse and reach for wh.catalog() / wh.block_on() / wh.table_ident() to build Iceberg RecordBatches against the redb catalog. These ARE the backend's body — they are not the coupling to break; they are what a different backend would re-implement.

2. ~50 named methods live INHERENT on IcebergWarehouse, off the trait. The trait deliberately kept only the generic Arrow core + two named wrappers (append_bench_run/query_bench_runs). Everything else consumers call — append_advisories, query_sbom_components, ingest_symbol_scans, root(), block_on(), append_mcp_calls, query_vuln_findings, … — is inherent. So a consumer holding &IcebergWarehouse cannot be flipped to &dyn Warehouse until the method it calls is first lifted onto the trait.

3. Consumers construct the concrete type directly. Almost every reader (the viz/* tabs, funnel/store, docs, security) calls IcebergWarehouse::open_read_only(root) itself rather than receiving an injected Box<dyn Warehouse>. The open() box is barely used as the injection point.

Audit tally (grep over src/, excluding the impl file itself):

• &IcebergWarehouse reference params/fields: ~170 sites.
• IcebergWarehouse::open[_read_only] constructions: ~250 (the vast majority are test helpers — fine to stay concrete; tests need a real backend).
• Files touching the concrete type: 72.
• Consumers that call only the generic trait surface today: 0 — every one either calls an inherent named method or forwards wh to an Iceberg-intrinsic free function. This is the finding that scopes the routing work: routing is gated on the inherent-method lift (§ Deferred).

Phase 3 landed this pass — RemoteWarehouse (the seam proof)

src/warehouse/remote.rs — a SECOND concrete Warehouse impl, a thin gRPC client that owns no catalog and no Parquet, yet satisfies the same trait the call sites drive. This is what proves the trait is a real, detachable seam. Compiled behind the tonic stack (#[cfg(any(feature = "server", "viz", "mcp"))], all default) — no new dependency (reuses the already-generated nornir.v1 warehouse_client).

Wired to the existing generic-browser gRPC (proto/nornir.proto, service Warehouse):

The unwired methods return a typed RemoteWarehouseError::Unsupported { method, todo } — never a silent no-op (L2). The Warehouse.Scan RPC returns stringified cells (the display preview), so the reconstructed batch is honestly all-Utf8 — types are lost on the wire; a typed remote read (Arrow Flight / per-table RPC) is the documented follow-up.

Phase 2 (additive) landed this pass — scan_preview on the trait

Added fn scan_preview(&self, table, limit) -> Result<TablePreview> to the trait (re-exporting TablePreview from mod). This is the read seam that makes the generic warehouse browser source-agnostic: the Iceberg backend reads Parquet + stringifies; RemoteWarehouse maps the Warehouse.Scan response 1:1. The Iceberg trait impl delegates to the existing inherent scan_preview (inherent methods win method resolution, so it is NOT a recursion) — zero behavior change.

Tests (L4, RED-when-broken)

• warehouse::remote::tests::remote_scan_response_maps_to_preview — Warehouse.Scan → TablePreview 1:1 (drops a col/row ⇒ RED).
• warehouse::remote::tests::remote_preview_reconstructs_utf8_batch — preview → all-Utf8 batch, right shape/values (RED on lost column/cell).
• warehouse::remote::tests::remote_tables_response_sorts_names — sorted contract.
• warehouse::remote::tests::remote_warehouse_is_a_boxed_warehouse_with_typed_write_errors — seam proof: Box<dyn Warehouse> = Box::new(RemoteWarehouse); writes return a typed Unsupported (a silent Ok ⇒ RED).
• warehouse::iceberg::tests::boxed_warehouse_drives_append_and_scan_preview_through_the_trait — owned Box<dyn Warehouse> drives append + table_names + the new scan_preview (won't compile without scan_preview on the trait; boxed dispatch dropping the row ⇒ RED).

All prior warehouse tests stay green (cargo test --lib warehouse:: — see commit gate).

Deferred (the large sweep — logged with file:line in workspace_nordisk/nornir_agents_problems.md)

• Lift the ~50 inherent named methods onto the trait, then route the ~170 &IcebergWarehouse consumer params to &dyn Warehouse. Order of value: the read/query methods first (query_sbom_components, query_vuln_findings, list_arch_wiring, warehouse_stats, load_preferred) — a thin client can serve those over new RPCs.
• Inject Box<dyn Warehouse> into the viz tabs / funnel store instead of each constructing IcebergWarehouse::open_read_only(root) — so nornir viz can point at a RemoteWarehouse.
• Typed remote read (Arrow Flight or per-table RPCs) to retire the all-Utf8 preview reconstruction and wire scan_filtered / query_bench_runs on RemoteWarehouse.
• open() wiring: a storage.kind = "grpc" (endpoint + token + workspace) that constructs RemoteWarehouse — needs new Storage fields; deliberately not done this pass to avoid config churn. RemoteWarehouse::connect(endpoint, token, workspace) is the explicit constructor until then.