[codex] Refactor Reborn composition internals - #5585
Merged
Merged
Conversation
|
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 (13)
💤 Files with no reviewable changes (11)
📝 WalkthroughSummary by CodeRabbit
WalkthroughReorganizes ChangesReborn composition module reorg and related fixes
Benchmark black_box import cleanup
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Runtime
participant local_dev_resource_gate_evidence
participant BudgetGateStore
Runtime->>local_dev_resource_gate_evidence: pending_resource_gate(LoopGateRef)
local_dev_resource_gate_evidence->>local_dev_resource_gate_evidence: budget_gate_id_from_gate_ref(ref)
local_dev_resource_gate_evidence->>BudgetGateStore: lookup gate record
BudgetGateStore-->>local_dev_resource_gate_evidence: BudgetGateRecord or error
local_dev_resource_gate_evidence-->>Runtime: bool (status == Pending)
Possibly related issues
Possibly related PRs
🚥 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 |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5585
July 3, 2026 07:57
Destroyed
github-actions
Bot
added
size: L
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request reorganizes the internal module structure of the ironclaw_reborn_composition crate, grouping related modules into new subdirectories: observability/, outbound/, and support/fs/. It also updates import paths across the codebase, changes the live progress sequence counter to be shared via Arc<AtomicU64>, and updates benchmark files to use std::hint::black_box. Feedback on the changes includes a recommendation to merge redundant imports from the newly created outbound module in webui.rs to improve readability.
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.
Comment on lines
+29
to
+35
| outbound::{ | ||
| OutboundDeliveryTargetProvider, OutboundDeliveryTargetRegistry, | ||
| RebornOutboundPreferencesFacade, | ||
| }, | ||
| outbound::{ | ||
| outbound_delivery_synthetic_provider, outbound_delivery_target_set_operator_tool_info, | ||
| }, |
Contributor
There was a problem hiding this comment.
These imports from the outbound module are redundant and can be merged into a single block to improve readability and maintainability.
outbound::{
OutboundDeliveryTargetProvider, OutboundDeliveryTargetRegistry,
RebornOutboundPreferencesFacade, outbound_delivery_synthetic_provider,
outbound_delivery_target_set_operator_tool_info,
},
Member
There was a problem hiding this comment.
Addressed in 8e5defa — collapsed the duplicated outbound::{...} block in webui.rs into a single import group.
serrrfirat
marked this pull request as ready for review
July 3, 2026 07:58
Contributor
Reborn integration-tier coverageLine coverage (Reborn crates): 16.89% — 10833 / 64125 lines Per-crate breakdown (11 crates, lowest-covered first)
This signal is informational: coverage never gates the PR — not the percentage, not the per-crate holes, not the 0-coverage callout. |
|
🚅 Deployed to the ironclaw-pr-5585 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5585
July 3, 2026 08:32
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5585
July 3, 2026 08:54
Destroyed
Resolve conflicts from the composition module reorg landing alongside main:
- projection.rs: keep the shared `live_sequence` counter (main + PR both
implement it; use the local binding so it isn't double-allocated)
- slack_host_beta.rs / slack_host_beta/runtime_setup.rs: keep the PR's
`crate::outbound::` module rename while preserving main's newly-added
`extension_lifecycle` / `RebornUserIdentityLookup` imports
Also repoint main's new `test_support/outbound_delivery.rs` from the moved
`crate::outbound_delivery_capability_surface::` path to
`crate::outbound::outbound_delivery_capability_surface::`, and address the
Gemini review note by collapsing the duplicated `outbound::{...}` import
block in webui.rs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…publishers The PR's live-projection fix makes multiple `LiveProjectionPublisher` instances built from one `RebornProjectionServices` share a single monotonic `Arc<AtomicU64>` sequence counter, but nothing pinned that behavior — a revert to a per-publisher `AtomicU64::new(0)` passed every existing live-progress test. Add a regression test that drives two independently created publishers (via the milestone-sink path, no feature gate) and asserts their live reasoning items land on distinct projection cursors. Verified red on a reintroduced per-publisher counter (the two items collide on cursor BASE+1 and collapse to one), green with the shared counter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
IronLoop Review StatusHead: Current reviewers:
Recent activity:
Commands:
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5585
July 3, 2026 23:51
Destroyed
ilblackdragon
approved these changes
Jul 3, 2026
This was referenced Jul 9, 2026
25 of 29 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements PR n3 of the Reborn composition dissection by grouping composition internals behind clearer module boundaries:
observability/, including budget events, hooks, operator logs, lifecycle, trace capture, and trajectory observer wiringoutbound/support::fsruntime.rsReview cleanup
After the maintainability pass, this PR intentionally does not include unrelated HTML-to-Markdown golden fixture churn or a post-refactor
composition-pubuse.snapshotbaseline.Validation
cargo fmt --checkgit diff --checkcargo clippy -p ironclaw_reborn_composition --all-targets --features "webui-v2-beta root-llm-provider slack-v2-host-beta openai-compat-beta libsql postgres test-support"env -u NEARAI_API_KEY -u NEARAI_BASE_URL -u NEARAI_API_BASE_URL -u LLM_BACKEND -u LLM_USE_CODEX_AUTH cargo test -p ironclaw_reborn_composition --features "webui-v2-beta root-llm-provider slack-v2-host-beta openai-compat-beta libsql postgres test-support" -- --test-threads=1Note: the unsanitized local shell has
NEARAI_API_KEY,LLM_BACKEND, andLLM_USE_CODEX_AUTHset, which makes existing env-sensitive provider tests fail. The full composition suite passes with those env vars removed.