fix(reborn): re-thread provider_factory through the cold-boot gateway (#6174 regression) - #6300
Conversation
…#6174 regression) #6174 collapsed the reborn boot path to always build the placeholder LLM gateway and swap the real provider in via a post-construction reload. That dropped the `ResolvedRebornLlm::provider_factory` threading: `build_production_model_gateway` took no factory and `build_placeholder_llm_gateway` hardcoded `None` into `wrap_swappable_gateway`, so `ResolvedRebornLlm::with_provider_factory` became dead — the field was set but never read on any production path (only a unit test passed `Some`). That factory is the instrumentation seam the benchmark harness (nearai-bench, `--framework ironclaw-reborn`) uses to wrap the provider in `InstrumentedLlm` for token/reasoning/cost capture. With it silently dropped, every claw-swe-bench-lite task failed instantly with 0 model calls ("reborn provider factory never ran (no instrumented provider)"), tanking the score to noise. Thread the resolved LLM's `provider_factory` through build_production_model_gateway -> build_placeholder_llm_gateway -> wrap_swappable_gateway. It wraps the *swappable* provider, so it stays in the call path across the boot-time reload — the reload-stable contract already documented on `wrap_swappable_gateway` and covered by `provider_factory_survives_live_reload`. Regression test drives the real caller (`build_reborn_runtime`), not just the helper: `provider_factory_runs_during_production_boot` asserts the factory is invoked once during boot. It fails on the pre-fix boot path (helper called with `None`) and passes with the fix. Also refreshes stale docs that referenced the removed `build_llm_gateway`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
github-actions
Bot
added
size: M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Reborn runtime now preserves an optional provider factory through production cold boot, placeholder gateway creation, and swappable gateway setup. A regression test verifies invocation during boot, and documentation describes the retained wrapper behavior. ChangesProvider factory boot preservation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request ensures that the caller's optional instrumentation decorator (provider_factory) is correctly threaded into the cold-boot gateway during build_reborn_runtime instead of being silently dropped. It updates build_production_model_gateway and build_placeholder_llm_gateway to accept and propagate this factory, updates relevant documentation, and adds a regression test (provider_factory_runs_during_production_boot) to verify this behavior. There are no review comments to address, and I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| 0 | 0 | 0 | fd7614ff2350 |
Head: fd7614ff23506a8a6bd75a81d1d063396617bad7
Next: Human review or validation is required before merging.
Run details
Status: Current
Needs human: no
Needs validation: yes
Summary
No concrete correctness or security issue found in the focused cold-boot factory threading change. Runtime validation could not run because this environment has no Rust toolchain.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
|
🚅 Deployed to the ironclaw-pr-6300 environment in ironclaw-ci-preview
|
|
✅ Ready for merge. Reviewed the cold-boot factory re-threading and ran the validation IronLoop's toolchain-less env couldn't: Review — correct and well-scoped. #6174 collapsed the boot path to Test is exactly right. Validation (IronLoop couldn't — no Rust toolchain in its env):
IronLoop: 0 blocking, 0 notes, 0 findings. CI green (52 pass, 0 fail). |
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 86.2% — 319514 / 370673 lines Per-crate breakdown (65 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
Reconcile the cold-boot provider_factory threading with main's deletion of the `root-llm-provider` feature (landed via the §5.3 collapse, #6299): drop the now- dead `#[cfg(feature = "root-llm-provider")]` gates while keeping the factory threaded unconditionally through build_production_model_gateway → build_placeholder_llm_gateway → wrap_swappable_gateway. Un-gate the provider_factory_runs_during_production_boot regression test (its feature is gone) and refresh the runtime_input doc. Verified: composition compiles (test-support,libsql and libsql-only/production); provider_factory_runs_during_production_boot runs + passes; clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Merge with main resolved (reconciled the provider_factory threading against main's |
There was a problem hiding this comment.
⚠️ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| 0 | 0 | 0 | 090572c224f3 |
Head: 090572c224f326128ff6419887ae5d1e332e6416
Next: Human review or validation is required before merging.
Run details
Status: Current
Needs human: no
Needs validation: yes
Summary
Static review found the provider factory is threaded through every production gateway call and remains outside the swappable provider across boot-time reloads. The new caller-level regression covers the previously missing forwarding branch; no actionable code findings found.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
|
✅ Ready for merge. Merge-with-main conflicts resolved (reconciled the cold-boot |
This branch was successfully deployed
2 participants
What
Re-thread the caller-supplied
provider_factory(the LLM instrumentation seam)through the reborn cold-boot gateway so
ResolvedRebornLlm::with_provider_factoryis honored again.
Why
#6174 turned
with_provider_factoryinto dead code. That PR moved rebornboot to always build the placeholder LLM gateway and swap the real provider in
via a post-construction
RebornLlmReloadAdapter::reload. In the process:build_production_model_gateway()stopped taking the resolved LLM, andbuild_placeholder_llm_gateway()hardcodedNoneintowrap_swappable_gateway.So the factory carried on
ResolvedRebornLlmwas set but never read on anyproduction path (only a unit test passed
Some). The helper worked; nothingcalled it with a factory.
That factory is how nearai-bench (
--framework ironclaw-reborn) wraps theprovider in
InstrumentedLlmto capture tokens / reasoning / cost. With itdropped, every
claw-swe-bench-litetask fails immediately with 0 modelcalls:
i.e. the benchmark cannot measure latest
mainat all.Fix
Thread the resolved LLM's
provider_factorythroughbuild_production_model_gateway→build_placeholder_llm_gateway→wrap_swappable_gateway. It wraps the swappable provider, so it stays inthe call path across the boot-time reload that swaps a real provider into the
placeholder — the reload-stable contract already documented on
wrap_swappable_gatewayand covered byprovider_factory_survives_live_reload.No new types, traits, or dependency edges: this threads one existing
pub(crate)field through two private fns.Tests
New regression test drives the real caller (
build_reborn_runtime), whichis exactly the gap that let the regression through — the existing
provider_factory_survives_live_reloadonly exercised thewrap_swappable_gatewayhelper directly, so it could not catch a boot path thatnever calls the helper with
Some:provider_factory_runs_during_production_boot— asserts the factory isinvoked once during boot. Fails on the pre-fix path (helper called with
None), passes with the fix. Verified red→green locally.Also refreshes stale docs referencing the removed
build_llm_gateway.Validation
cargo clippy -p ironclaw_reborn_composition --all-targetsacrossall-features / default / libsql-only,
-D warnings— clean.cargo test -p ironclaw_reborn_composition --all-features— green.cargo test -p ironclaw_architecture— green (no boundary changes).🤖 Generated with Claude Code