Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions crates/ironclaw_reborn_composition/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3315,13 +3315,23 @@ pub async fn build_reborn_runtime(
Some(resolved) => build_skill_learning_provider(&resolved.config).await,
None => None,
};
// Caller instrumentation seam (e.g. a benchmark harness layering
// token/reasoning capture): carry the resolved LLM's provider factory into
// the cold-boot gateway so the wrapper wraps the swappable and stays in the
// call path across the boot-time reload. `llm` is held by shared reference
// here (already read above for the NEAR AI MCP bootstrap), so clone the
// cheap Arc handle rather than move the factory out of the borrow.
let boot_provider_factory = llm
.as_ref()
.and_then(|resolved| resolved.provider_factory.clone());
#[cfg(any(test, feature = "test-support"))]
let (model_gateway, llm_cost_table, llm_reload) = match model_gateway_override {
Some(override_gateway) => (override_gateway, None, None),
None => build_production_model_gateway().await?,
None => build_production_model_gateway(boot_provider_factory).await?,
};
#[cfg(not(any(test, feature = "test-support")))]
let (model_gateway, llm_cost_table, llm_reload) = build_production_model_gateway().await?;
let (model_gateway, llm_cost_table, llm_reload) =
build_production_model_gateway(boot_provider_factory).await?;

// Resolved cost table is either: the LLM-policy-derived table (real
// LLM wired), a test override (so tests can drive deterministic
Expand Down Expand Up @@ -4490,7 +4500,17 @@ impl CapabilitySurfaceProfileResolver for AllowAllCapabilitySurfaceResolver {
/// through the same live-reload path the settings UI uses
/// (`RebornLlmReloadAdapter::reload`). No cost table is derived here: there's
/// no real model to cost until that reload swaps in a real provider.
async fn build_production_model_gateway() -> Result<
///
/// `provider_factory` is the caller's optional instrumentation decorator
/// (e.g. a benchmark harness layering token/reasoning capture) carried on the
/// resolved LLM. It wraps the *swappable* provider, so the wrapper stays in the
/// call path across the boot-time reload that swaps a real provider into the
/// placeholder (see [`wrap_swappable_gateway`]). Without threading it here the
/// `ResolvedRebornLlm::with_provider_factory` seam would be silently dropped on
/// the cold-boot path.
async fn build_production_model_gateway(
provider_factory: Option<crate::runtime_input::RebornProviderFactory>,
) -> Result<
(
Arc<dyn ironclaw_loop_host::HostManagedModelGateway>,
Option<ironclaw_loop_host::StaticModelCostTable>,
Expand All @@ -4500,7 +4520,7 @@ async fn build_production_model_gateway() -> Result<
> {
let LlmGatewayBundle {
gateway, reload, ..
} = build_placeholder_llm_gateway().await?;
} = build_placeholder_llm_gateway(provider_factory).await?;
Ok((gateway, None, Some(reload)))
}

Expand Down Expand Up @@ -4564,11 +4584,18 @@ pub(crate) struct RebornLlmReloadParts {
/// errors until swapped) so the model-gateway + reload seam exist from the
/// start; the first configuration applied through the settings UI swaps the
/// placeholder for a real provider chain with no restart.
async fn build_placeholder_llm_gateway() -> Result<LlmGatewayBundle, RebornRuntimeError> {
///
/// `provider_factory` is the caller's optional instrumentation decorator. It is
/// applied over the *swappable* wrapper (not the placeholder), so it survives
/// the boot-time reload that swaps in the real provider — the reload-stable
/// contract documented on [`wrap_swappable_gateway`].
async fn build_placeholder_llm_gateway(
provider_factory: Option<crate::runtime_input::RebornProviderFactory>,
) -> Result<LlmGatewayBundle, RebornRuntimeError> {
let session =
ironclaw_llm::create_session_manager(ironclaw_llm::SessionConfig::default()).await;
let raw: Arc<dyn ironclaw_llm::LlmProvider> = Arc::new(PlaceholderLlmProvider);
wrap_swappable_gateway(raw, session, None)
wrap_swappable_gateway(raw, session, provider_factory)
}

/// Wrap a raw provider in a [`SwappableLlmProvider`] + reload handle and build
Expand Down
59 changes: 58 additions & 1 deletion crates/ironclaw_reborn_composition/src/runtime/tests/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2258,7 +2258,7 @@ fn dead_endpoint_nearai_config(session_path: std::path::PathBuf) -> ironclaw_llm
}

/// Regression guard for Firat's review: the provider factory (caller
/// instrumentation) must survive a live config reload. `build_llm_gateway`
/// instrumentation) must survive a live config reload. `wrap_swappable_gateway`
/// wraps the factory over the `SwappableLlmProvider`, so reloading — which
/// swaps the swappable's *inner* — keeps the wrapper in the call path. If the
/// factory were applied to the bare provider instead, the first reload would
Expand Down Expand Up @@ -2321,6 +2321,63 @@ async fn provider_factory_survives_live_reload() {
);
}

/// Regression guard for the benchmark instrumentation seam: a
/// `ResolvedRebornLlm` carrying a `provider_factory` must have that factory
/// invoked during `build_reborn_runtime`, i.e. the caller's instrumentation
/// wrapper is threaded into the cold-boot gateway.
///
/// PR #6174 collapsed the boot path to `build_placeholder_llm_gateway()`, which
/// hardcoded `None` for the factory, so `ResolvedRebornLlm::with_provider_factory`
/// silently never ran on the production path — the benchmark harness saw every
/// task fail with zero model calls (no instrumented provider). The
/// `provider_factory_survives_live_reload` test above exercises the
/// `wrap_swappable_gateway` helper directly with `Some(..)`, so it cannot catch
/// a boot path that never calls the helper with a factory at all. This drives
/// the real caller (`build_reborn_runtime`) instead.
#[tokio::test]
async fn provider_factory_runs_during_production_boot() {
let root = tempfile::tempdir().expect("tempdir");
let session_dir = tempfile::tempdir().expect("session tempdir");
let local_dev_root = root.path().join("local-dev");

let factory_ran = Arc::new(std::sync::atomic::AtomicUsize::new(0));
let factory_ran_for_closure = Arc::clone(&factory_ran);
// Identity decorator that only records that it was constructed: the factory
// runs once, at gateway construction, to wrap the swappable provider.
let factory: crate::runtime_input::RebornProviderFactory = Arc::new(move |inner| {
factory_ran_for_closure.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
inner
});

let config = dead_endpoint_nearai_config(session_dir.path().join("session.json"));
let llm = crate::runtime_input::ResolvedRebornLlm::from_llm_config(config)
.with_provider_factory(factory);

// No `boot` config is supplied, so the boot-time reload is skipped and the
// dead endpoint is never contacted; the factory still wraps the swappable
// at cold-boot construction.
let input = RebornRuntimeInput::from_services(
RebornBuildInput::local_dev("provider-factory-boot-owner", local_dev_root)
.with_runtime_policy(local_dev_runtime_policy()),
)
.with_resolved_llm(llm)
.with_identity(RebornRuntimeIdentity {
tenant_id: "provider-factory-boot-tenant".to_string(),
agent_id: "provider-factory-boot-agent".to_string(),
source_binding_id: "provider-factory-boot-source".to_string(),
reply_target_binding_id: "provider-factory-boot-reply".to_string(),
});

let _runtime = build_reborn_runtime(input).await.expect("runtime builds");

assert_eq!(
factory_ran.load(std::sync::atomic::Ordering::SeqCst),
1,
"the caller's provider_factory must be invoked once during boot so \
instrumentation wraps the swappable gateway (regression: #6174 dropped it)"
);
}

/// Regression pin for the journey-critical fix (PR #6174): a provider
/// selected purely through `config.toml` + a stored API key (no env var set)
/// must reach the turn-serving provider. This exercises the ONLY mechanism
Expand Down
30 changes: 18 additions & 12 deletions crates/ironclaw_reborn_composition/src/runtime_input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,15 @@ pub struct ResolvedRebornLlm {
provider_id: String,
model: String,
pub(crate) config: ironclaw_llm::LlmConfig,
/// Optional decorator applied to the provider the gateway builds from
/// `config`. `config` is always the construction source (so it stays the
/// single source of truth for `provider_id`/`model` and budget cost-table
/// derivation); the factory only *wraps* the built provider — e.g. a
/// benchmark harness layering token/reasoning instrumentation over it.
/// When `None` the gateway uses the config-built provider as-is.
/// Optional decorator applied over the gateway's *swappable* provider at
/// cold boot — e.g. a benchmark harness layering token/reasoning
/// instrumentation. `config` stays the construction source (single source of
/// truth for `provider_id`/`model` and budget cost-table derivation); the
/// factory only *wraps* the swappable, so it survives the boot-time reload
/// that swaps a real provider into the placeholder. When `None` the gateway
/// drives the swappable directly. Threaded through
/// `build_production_model_gateway` → `build_placeholder_llm_gateway` →
/// `wrap_swappable_gateway`.
pub(crate) provider_factory: Option<RebornProviderFactory>,
}

Expand Down Expand Up @@ -184,12 +187,15 @@ impl ResolvedRebornLlm {
/// it — e.g. to layer token/reasoning/cost instrumentation over the real
/// provider.
///
/// This is the instrumentation seam. The composition still constructs the provider from `config` and hands it
/// to the factory, so `config` remains the single source of truth and the
/// raw `ironclaw_llm::LlmProvider` substrate handle is never accepted
/// wholesale through the facade — the caller only supplies a decorator over
/// a provider the composition built. `build_llm_gateway` applies the factory
/// and never re-exposes the provider.
/// This is the instrumentation seam. The composition still constructs the
/// provider from `config` and hands the
/// factory the *swappable* wrapper over it, so `config` remains the single
/// source of truth and the raw `ironclaw_llm::LlmProvider` substrate handle
/// is never accepted wholesale through the facade — the caller only supplies
/// a decorator over a provider the composition built.
/// `build_placeholder_llm_gateway` applies the factory at cold boot and never
/// re-exposes the provider; because it wraps the swappable, the decorator
/// stays in the call path across the boot-time (and later) reloads.
pub fn with_provider_factory(mut self, factory: RebornProviderFactory) -> Self {
self.provider_factory = Some(factory);
self
Expand Down
Loading