aimdb-dev · lxsaah · Jun 10, 2026 · Jun 10, 2026
@@ -44,6 +44,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed (breaking)
 
+- **Design 034 Phase 2 — panic-free builder validation: `build()` reports every configuration mistake at once (Issue #133, [review doc §3.4](docs/design/034-technical-debt-review.md)).** Builder methods never panic on user mistakes anymore. Conflicting `.source()`/`.transform()`/`.link_from()` registrations, missing serializers/deserializers, invalid connector URLs, unregistered schemes, and key-reused-with-different-type are *recorded* (the conflicting registration is skipped) and `build()` returns one `DbError::InvalidConfiguration { errors: Vec<ConfigError> }` carrying **all** findings — each with the record key and, where applicable, the connector URL. The worst panic — "requires a buffer" firing at spawn time inside a connector factory closure — is now a build()-time check, which also makes `.buffer()` after `.link_to()`/`.link_from()` legal (order-independent). Duplicate keys and dependency-graph cycles fold into the same collected report (previously distinct `DuplicateRecordKey`/`CyclicDependency` returns from `build()`). Remaining `panic!`/`expect`s in the builder path are internal invariants and say "this is a bug in aimdb-core". ([aimdb-core](aimdb-core/CHANGELOG.md))
+
 - **Design 034 Phase 2 — registrar lifetime fix + de-erased builder internals (Issue #130, [review doc](docs/design/034-technical-debt-review.md)).** `RecordRegistrar`'s fluent methods now take fresh borrows (`&mut self -> &mut Self`) instead of borrowing the registrar for its entire lifetime — a `configure` closure can finally use separate statements (`reg.source_raw(…); reg.tap_raw(…);`) and reuse the registrar after a chain. `configure`'s closure bound drops its HRTB; `OutboundConnectorBuilder`/`InboundConnectorBuilder` gain a second lifetime parameter (`<'r, 'a, T, R>`); `RecordT::register` and the adapter/persistence extension traits follow. Internally, `AimDbBuilder` stores its spawn/start functions typed (`SpawnFnType<R>`/`StartFnType<R>`) instead of `Box<dyn Any>` — the panicking downcasts in `build()` are gone, and `AimDb<R>`'s struct-level bound moved to its impls. Closure-based user code compiles unchanged. ([aimdb-core](aimdb-core/CHANGELOG.md), [aimdb-embassy-adapter](aimdb-embassy-adapter/CHANGELOG.md), [aimdb-persistence](aimdb-persistence/CHANGELOG.md))
 
 - **Design 034 Phase 1 — mechanical debt cleanup (Issues #129, #132, [review doc](docs/design/034-technical-debt-review.md)).** `DbError` is unified on `alloc::string::String`: every variant has one shape on all targets, `thiserror` derives `Display`/`Error` unconditionally (now a mandatory no_std dependency of `aimdb-core`), and no_std builds produce the same error messages as std builds instead of `Error 0xNNNN` codes — on no_std the `_field: ()` placeholders become the real `String` fields. The dead `Database<A>` wrapper, the `TokioDatabase`/`EmbassyDatabase` aliases, and the deprecated `RecordRegistrar::link()` are removed. `ConsumerTrait::subscribe_any` is infallible (returns `Box<dyn AnyReader>`), `OutboundRoute` is a struct with named fields, and `aimdb-core`'s `std` feature no longer pulls a `tokio` dependency (tests cover it via dev-deps). Internally: all dual std/alloc import pairs and the duplicated `RuntimeContext` impl blocks are collapsed, and crate-private `log_*!` macros replace the 62 per-call-site `#[cfg(feature = "tracing")]` gates. The `aimdb-wasm-adapter` ring-lag error now carries a `buffer_name` like the other adapters. ([aimdb-core](aimdb-core/CHANGELOG.md), [aimdb-tokio-adapter](aimdb-tokio-adapter/CHANGELOG.md), [aimdb-embassy-adapter](aimdb-embassy-adapter/CHANGELOG.md), [aimdb-mqtt-connector](aimdb-mqtt-connector/CHANGELOG.md), [aimdb-knx-connector](aimdb-knx-connector/CHANGELOG.md))

@@ -43,6 +43,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed (breaking)
 
+- **Phase 2 — panic-free builder validation; `build()` collects every configuration mistake (Issue #133, [design doc §3.4](../docs/design/034-technical-debt-review.md)).** One failure model instead of two: builder methods stay infallible and never panic on user mistakes; `build()` performs all validation and returns one `DbError::InvalidConfiguration { errors: Vec<ConfigError> }` (new variant + new public `ConfigError { record_key, url, message }` type, error code `0x4003`) carrying **every** finding from the run. Specifics:
+  - `TypedRecord::set_producer`/`set_transform`/`add_inbound_connector` mutual-exclusion violations and duplicate producers/transforms: recorded, conflicting registration skipped (observable: `has_producer()` stays `false` after a conflicting `.source()`).
+  - `OutboundConnectorBuilder::finish()`/`InboundConnectorBuilder::finish()`: invalid URL, missing serializer/deserializer, unregistered scheme, and the transform/source conflicts are recorded with record key + URL; the link is not registered and the registrar is returned as usual.
+  - The spawn-time factory panics (missing buffer, record lookup) are now build()-time checks: any record with an outbound or inbound link and no buffer fails `build()` with key + link URL. Consequence: `.buffer()` may be called after `.link_to()`/`.link_from()` — the finish()-time buffer check is gone.
+  - `configure()` with a key already registered under a different type: recorded, the closure is skipped (previously `assert!`).
+  - Duplicate record keys and `DependencyGraph` findings (cycles, unregistered transform inputs) fold into the same `InvalidConfiguration` report — callers matching on `DuplicateRecordKey`/`CyclicDependency` from `build()` must update.
+  - Surviving `panic!`/`expect`s on the builder path are internal invariants only, worded "this is a bug in aimdb-core". `AnyRecord` gains `has_buffer()` and `drain_config_errors()` (breaking for external implementors).
+
 - **Phase 2 — `RecordRegistrar` fluent methods take fresh borrows (Issue #130, [design doc §3.5](../docs/design/034-technical-debt-review.md)).** The methods previously took `&'a mut self` and returned `&'a mut Self` with `'a` = the struct's own lifetime parameter, so the first call borrowed the registrar for its entire remaining lifetime and only one unbroken chain per `configure` closure was possible. Now:
   - All fluent methods are `&mut self -> &mut Self`; separate statements (`reg.source_raw(…); reg.tap_raw(…);`) and registrar reuse after a chain work.
   - `configure`'s closure bound is `FnOnce(&mut RecordRegistrar<'_, T, R>)` (HRTB dropped) — existing closures compile unchanged.

@@ -301,6 +301,12 @@ pub struct AimDbBuilder<R = NoRuntime> {
     /// Generic extension storage for external crates (e.g., persistence, metrics).
     /// Moved into AimDbInner during build() so it can be read on the live AimDb handle.
     extensions: Extensions,
+
+    /// Builder-level configuration mistakes (e.g. re-registering a key with a
+    /// different type), recorded instead of panicking and reported — together
+    /// with the per-record errors — by `build()` via
+    /// [`DbError::InvalidConfiguration`].
+    config_errors: Vec<crate::error::ConfigError>,
 }
 
 impl AimDbBuilder<NoRuntime> {
@@ -315,6 +321,7 @@ impl AimDbBuilder<NoRuntime> {
             spawn_fns: Vec::new(),
             start_fns: Vec::new(),
             extensions: Extensions::new(),
+            config_errors: Vec::new(),
         }
     }
 
@@ -353,6 +360,7 @@ impl AimDbBuilder<NoRuntime> {
             spawn_fns: Vec::new(),
             start_fns: Vec::new(),
             extensions: self.extensions,
+            config_errors: self.config_errors,
         }
     }
 }
@@ -495,17 +503,24 @@ where
 
         let (rec, is_new_record) = match record_index {
             Some(idx) => {
-                // Use existing record
+                // Use existing record. A key re-registered with a different
+                // type is a user mistake: record it (reported from build())
+                // and skip the closure — a registrar for the wrong `T`
+                // cannot even be constructed.
                 let (_, existing_type, record) = &mut self.records[idx];
-                assert!(
-                    *existing_type == type_id,
-                    "StringKey '{}' already registered with different type",
-                    record_key.as_str()
-                );
+                if *existing_type != type_id {
+                    self.config_errors.push(crate::error::ConfigError::new(
+                        record_key.as_str(),
+                        None,
+                        "key already registered with a different type",
+                    ));
+                    return self;
+                }
                 (
-                    record
-                        .as_typed_mut::<T, R>()
-                        .expect("type mismatch in record registry"),
+                    record.as_typed_mut::<T, R>().expect(
+                        "record registry type mismatch despite TypeId check — \
+                         this is a bug in aimdb-core",
+                    ),
                     false,
                 )
             }
@@ -515,9 +530,10 @@ where
                     .push((record_key, type_id, Box::new(TypedRecord::<T, R>::new())));
                 let (_, _, record) = self.records.last_mut().unwrap();
                 (
-                    record
-                        .as_typed_mut::<T, R>()
-                        .expect("type mismatch in record registry"),
+                    record.as_typed_mut::<T, R>().expect(
+                        "record registry type mismatch despite TypeId check — \
+                         this is a bug in aimdb-core",
+                    ),
                     true,
                 )
             }
@@ -645,6 +661,16 @@ where
     /// `DbResult<(AimDb<R>, AimDbRunner)>` — the handle (cloneable) and the
     /// non-`Clone` runner that owns the collected futures.
     ///
+    /// # Errors
+    ///
+    /// `build()` is the single surface for configuration mistakes: everything
+    /// recorded during configuration (conflicting `.source()`/`.transform()`/
+    /// `.link_from()`, missing serializers, unregistered schemes, …) plus the
+    /// build-time checks here (record validation, linked records without a
+    /// buffer, duplicate keys, dependency-graph cycles) is collected and
+    /// returned as one [`DbError::InvalidConfiguration`] carrying **all**
+    /// findings — one run surfaces every mistake.
+    ///
     /// # Example
     ///
     /// ```rust,ignore
@@ -657,22 +683,56 @@ where
     /// let handle = db.clone();              // clone freely before runner.run()
     /// runner.run().await;                   // drives everything to completion
     /// ```
-    pub async fn build(self) -> DbResult<(AimDb<R>, AimDbRunner)>
+    pub async fn build(mut self) -> DbResult<(AimDb<R>, AimDbRunner)>
     where
         R: crate::RuntimeForProfiling,
     {
-        // Validate all records
-        for (key, _, record) in &self.records {
-            record.validate().map_err(|msg| {
-                DbError::runtime_error(alloc::format!(
-                    "Record '{}' validation failed: {}",
+        use crate::error::ConfigError;
+
+        // ── Validation pass: collect every configuration mistake before any
+        // effectful phase runs (issue #133). Nothing returns early here.
+        let mut errors = core::mem::take(&mut self.config_errors);
+
+        for (key, _, record) in self.records.iter_mut() {
+            // Mistakes recorded during configuration — fill in the record key
+            // the setters didn't have.
+            for mut e in record.drain_config_errors() {
+                if e.record_key.is_empty() {
+                    e.record_key = key.as_str().to_string();
+                }
+                errors.push(e);
+            }
+
+            // Record-level validation (e.g. remote access without a buffer).
+            if let Err(msg) = record.validate() {
+                errors.push(ConfigError::new(key.as_str(), None, msg));
+            }
+
+            // Connector links subscribe to / produce into the record's buffer;
+            // a linked record without one only surfaced at spawn time before.
+            let has_links =
+                record.outbound_connector_count() > 0 || !record.inbound_connectors().is_empty();
+            if has_links && !record.has_buffer() {
+                let url = record
+                    .outbound_connectors()
+                    .first()
+                    .map(|l| l.url.to_string())
+                    .or_else(|| {
+                        record
+                            .inbound_connectors()
+                            .first()
+                            .map(|l| l.url.to_string())
+                    });
+                errors.push(ConfigError::new(
                     key.as_str(),
-                    msg
-                ))
-            })?;
+                    url,
+                    "linked record requires a buffer (call .buffer(...))",
+                ));
+            }
         }
 
-        // Ensure runtime is set
+        // Ensure runtime is set. Unreachable through the public API — the
+        // typed builder only exists once `.runtime()` was called.
         let runtime = self
             .runtime
             .ok_or_else(|| DbError::runtime_error("runtime not set (use .runtime())"))?;
@@ -685,17 +745,16 @@ where
         let mut types: Vec<TypeId> = Vec::with_capacity(record_count);
         let mut keys: Vec<StringKey> = Vec::with_capacity(record_count);
 
-        for (i, (key, type_id, record)) in self.records.into_iter().enumerate() {
-            let id = RecordId::new(i as u32);
-
-            // Check for duplicate keys (should not happen if configure() is used correctly)
+        for (key, type_id, record) in self.records.into_iter() {
+            // Duplicate keys (should not happen if configure() is used
+            // correctly): collect and skip so every mistake is reported.
             if by_key.contains_key(&key) {
-                return Err(DbError::DuplicateRecordKey {
-                    key: key.as_str().to_string(),
-                });
+                errors.push(ConfigError::new(key.as_str(), None, "duplicate record key"));
+                continue;
             }
 
             // Build index structures
+            let id = RecordId::new(storages.len() as u32);
             storages.push(record);
             by_key.insert(key, id);
             by_type.entry(type_id).or_default().push(id);
@@ -726,8 +785,20 @@ where
             })
             .collect();
 
-        // Build and validate the dependency graph
-        let dependency_graph = DependencyGraph::build_and_validate(&record_infos)?;
+        // Build and validate the dependency graph; fold its findings (cycles,
+        // unregistered transform inputs) into the collected errors.
+        let dependency_graph = match DependencyGraph::build_and_validate(&record_infos) {
+            Ok(graph) => graph,
+            Err(e) => {
+                errors.push(ConfigError::new("", None, alloc::format!("{e}")));
+                return Err(DbError::InvalidConfiguration { errors });
+            }
+        };
+
+        // All validation done — report every collected mistake at once.
+        if !errors.is_empty() {
+            return Err(DbError::InvalidConfiguration { errors });
+        }
 
         log_debug!(
             "Dependency graph built successfully ({} nodes, {} edges, topo order: {:?})",