chore: refine builders

CHANGELOG.md

@@ -60,7 +60,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/2.0.0.
   single parameter for `ApiClient` methods. Replaces the positional
   `(Vec<Message>, Option<String>, Option<Vec<ToolSchema>>)` parameter lists on
   `stream_messages`, `create_message`, and their `_with_options` variants.
-  Builders: `new`, `system`, `system_opt`, `tools`, `tools_opt`.
+  Builders: `new`, `with_system`, `with_tools` (both take `Option`).
 - `GeminiClientBuilder::include_thoughts(bool)` builder: opt into Gemini's
   `thinkingConfig.includeThoughts` for reasoning-capable models (2.5 Pro/Flash,
   Gemini 3). Defaults to `false` — the Gemini API rejects `thinkingConfig`
@@ -169,9 +169,54 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/2.0.0.
   Path-aware invalidation via the caller-supplied `PathExtractor` trait;
   TTL-based expiry per `ttl_turns`. Default-off; only successful results
   are cached.
+- Fluent `with_*()` builder methods on `LoopConfig` (one per public field,
+  e.g. `with_model`, `with_max_turns`, `with_session_id`,
+  `with_parallel_tool_dispatch`). Each is `#[must_use]`, consuming, and
+  returns `Self` for chaining. Purely additive — `Default`-based and direct
+  struct-literal construction are unchanged and remain first-class.
+- Consuming `with_*()` fluent builders on `BareLoop`, mirroring the existing
+  `&mut self` setters so a loop can be assembled as a chain off `BareLoop::new`:
+  `with_reflector`, `with_recovery_strategy`, `with_context_manager`,
+  `with_stream_handler`, `with_hook_executor` (`hooks` feature),
+  `with_health_registry` (`tool_health` feature), `with_pipeline` (returns
+  `Result<Self, LoopError>` — chain with `?`), `with_observer`,
+  `with_text_streamer`, `with_contributor`, `with_request_options`. The
+  original `set_*`/`register_*` setters are unchanged and remain available.
 
 ### Changed
 
+- **Breaking (renames):** consuming builder methods that return `Self` are now
+  uniformly prefixed `with_`, matching the crate-wide convention. The old
+  no-prefix names are removed. Affected types and methods (old → new):
+  `SessionResultBuilder` (`session_id`/`total_turns`/`input_tokens`/
+  `output_tokens`/`total_duration`/`tool_calls`/`success`/`final_output`/
+  `error` → `with_*`); `ModelSwitch` (`context_window`, `max_tokens` →
+  `with_*`); `StreamRequest` (`system`, `system_opt`, `tools`, `tools_opt` →
+  `with_*`); `RequestOptions` (`response_format`, `tool_constraint` → `with_*`);
+  `UnixShieldBuilder` (`warn_threshold`, `block_threshold`, `pattern`,
+  `combination_rule` → `with_*`); `AutoCommitConfigBuilder` (`enabled`,
+  `message_template`, `auto_push`, `files` → `with_*`); `ToolPipelineBuilder`
+  (`core` → `with_core`; the middleware accumulators `with` and `with_arc` are
+  renamed to `with_middleware` and `with_middleware_arc` so they no longer read
+  ambiguously next to `with_core`). Migration: add the `with_` prefix at each
+  call site.
+- `ToolPipelineBuilder::build` now distinguishes its two failure cases:
+  `PipelineError::Empty` when neither middleware nor a core was added (the
+  builder is untouched), and `PipelineError::MissingCore` when at least one
+  middleware was added but no core registry was set. Previously both cases
+  returned `MissingCore`, and `Empty` was unreachable. The `Empty` and
+  `MissingCore` variants now carry multiline rustdoc explaining each condition.
+- **Breaking (optional-field builders unified):** builder methods that set an
+  `Option<T>` field now uniformly take `Option<T>` and drop the `_opt` suffix —
+  the parameter type already conveys "optional," so the name should not.
+  `StreamRequest` loses `with_system_opt`/`with_tools_opt`; `with_system` and
+  `with_tools` now take `Option<String>`/`Option<Vec<ToolSchema>>` (pass
+  `Some(…)` to set, `None` to clear). `LoopConfig::with_system_prompt` changes
+  from `impl Into<String>` to `Option<String>`, so it can now clear the
+  override with `None` (previously it could only set). `AttemptRecord::with_reason`
+  and `Operation::with_result_hash` already followed this shape and are
+  unchanged. Migration: wrap existing literal/value arguments in `Some(…)` (or
+  rename `with_*_opt` → `with_*`).
 - **Breaking:** `stream::DeltaPart` is now `#[non_exhaustive]`. Every
   exhaustive `match` on `DeltaPart` in downstream code must add a `_ =>` arm
   (or a `Thinking =>` arm for the new variant). Same-crate matches are

src/api.rs

@@ -21,28 +21,48 @@ use std::sync::Arc;
 /// so method signatures stay short (`&self, request, ...`) instead of
 /// repeating `messages, system, tools` at every call site.
 ///
-/// Construct via `StreamRequest::new(messages)` and chain `.system(...)`
-/// / `.tools(...)` as needed, or build with struct-literal syntax.
+/// Construct via `StreamRequest::new(messages)` and chain
+/// `.with_system(...)` / `.with_tools(...)` as needed, or build with
+/// struct-literal syntax.
 ///
 /// # Example
 ///
 /// ```rust,ignore
 /// use loopctl::api::StreamRequest;
 ///
 /// let req = StreamRequest::new(vec![Message::user("hi")])
-///     .system("be brief")
-///     .tools(vec![search_tool]);
+///     .with_system(Some("be brief".to_string()))
+///     .with_tools(Some(vec![search_tool]));
 /// let stream = client.stream_messages(req);
 /// ```
 #[derive(Debug, Clone)]
 pub struct StreamRequest {
-    /// The conversation history. Owned because the request body must be built
-    /// from owned data — callers clone the full history each turn
-    /// (O(n) in messages).
+    /// The conversation history sent to the model.
+    ///
+    /// Owned because the request body is built from owned data — callers clone
+    /// the full history each turn (O(n) in messages). This is the only required
+    /// field; an empty `Vec` is permitted (a cold-start prompt with no prior
+    /// turns).
     pub messages: Vec<Message>,
-    /// An optional system prompt to prepend.
+
+    /// An optional system prompt prepended above `messages`.
+    ///
+    /// When `Some`, providers emit it in their native system-prompt slot
+    /// (top-level `system` on Anthropic/Gemini, a leading `system` role message
+    /// on OpenAI). When `None`, the provider receives no system prompt for the
+    /// turn. The value is forwarded verbatim from
+    /// [`LoopConfig::system_prompt`](crate::config::LoopConfig::system_prompt);
+    /// set it there to drive this field.
     pub system: Option<String>,
-    /// Optional tool definitions the model may invoke.
+
+    /// Optional tool definitions the model may invoke this turn.
+    ///
+    /// When `Some`, providers advertise these schemas to the model (OpenAI
+    /// `tools`, Anthropic `tools`, Gemini `functionDeclarations`). When `None`,
+    /// no tools are advertised and the model cannot issue tool calls. Note the
+    /// framework silently suppresses `tools` when a
+    /// [`response_format`](crate::structured::RequestOptions) constraint is set
+    /// — see the structured-output docs.
     pub tools: Option<Vec<ToolSchema>>,
 }
 
@@ -58,35 +78,24 @@ impl StreamRequest {
     }
 
     /// Set the system prompt.
-    #[must_use]
-    pub fn system(mut self, system: impl Into<String>) -> Self {
-        self.system = Some(system.into());
-        self
-    }
-
-    /// Set the system prompt from an existing [`Option<String>`].
     ///
-    /// Convenience for call sites that already hold the prompt as an `Option`
-    /// (e.g. forwarded from another field). `None` leaves it unset.
+    /// The field is optional: pass `Some(prompt)` to set it or `None` to leave
+    /// it unset (the default). This mirrors the field type directly, so a
+    /// caller that already holds an `Option<String>` (e.g. forwarded from
+    /// another config field) needs no conversion.
     #[must_use]
-    pub fn system_opt(mut self, system: Option<String>) -> Self {
+    pub fn with_system(mut self, system: Option<String>) -> Self {
         self.system = system;
         self
     }
 
     /// Set the tool definitions.
-    #[must_use]
-    pub fn tools(mut self, tools: Vec<ToolSchema>) -> Self {
-        self.tools = Some(tools);
-        self
-    }
-
-    /// Set the tool definitions from an existing [`Option<Vec<ToolSchema>>`].
     ///
-    /// Convenience for call sites that already hold tools as an `Option`.
-    /// `None` leaves them unset.
+    /// The field is optional: pass `Some(tools)` to attach tool schemas or
+    /// `None` to send no tools (the default). Mirrors the field type so an
+    /// existing `Option<Vec<ToolSchema>>` maps without conversion.
     #[must_use]
-    pub fn tools_opt(mut self, tools: Option<Vec<ToolSchema>>) -> Self {
+    pub fn with_tools(mut self, tools: Option<Vec<ToolSchema>>) -> Self {
         self.tools = tools;
         self
     }
@@ -462,15 +471,9 @@ mod tests {
 
     #[test]
     fn stream_request_system_builder() {
-        let req = StreamRequest::new(vec![]).system("be brief");
+        let req = StreamRequest::new(vec![]).with_system(Some("be brief".to_string()));
         assert_eq!(req.system.as_deref(), Some("be brief"));
-    }
-
-    #[test]
-    fn stream_request_system_opt_builder() {
-        let req = StreamRequest::new(vec![]).system_opt(Some("x".into()));
-        assert_eq!(req.system.as_deref(), Some("x"));
-        let req = StreamRequest::new(vec![]).system_opt(None);
+        let req = StreamRequest::new(vec![]).with_system(None);
         assert!(req.system.is_none());
     }
 
@@ -481,20 +484,9 @@ mod tests {
             description: "Search".into(),
             input_schema: serde_json::json!({"type": "object"}),
         }];
-        let req = StreamRequest::new(vec![]).tools(tools);
-        assert_eq!(req.tools.as_ref().unwrap().len(), 1);
-    }
-
-    #[test]
-    fn stream_request_tools_opt_builder() {
-        let tools = vec![crate::tool::ToolSchema {
-            tool: "search".into(),
-            description: "Search".into(),
-            input_schema: serde_json::json!({"type": "object"}),
-        }];
-        let req = StreamRequest::new(vec![]).tools_opt(Some(tools));
+        let req = StreamRequest::new(vec![]).with_tools(Some(tools));
         assert_eq!(req.tools.as_ref().unwrap().len(), 1);
-        let req = StreamRequest::new(vec![]).tools_opt(None);
+        let req = StreamRequest::new(vec![]).with_tools(None);
         assert!(req.tools.is_none());
     }