reborn: enable final-answer nudge for planned_default and scheduled_trigger - #5568
Closed
henrypark133 wants to merge 16 commits into
Closed
henrypark133 wants to merge 16 commits into
henrypark133 wants to merge 16 commits into
Conversation
Scopes allow_driver_specific_nudges to interactive_default and scheduled_trigger via a builder method, avoiding a shared-base flip that would leak into planned_default/subagent. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…anned_default Real production interactive/chat/CLI turns request no explicit run profile (submit_user_turn passes requested_run_profile: None) and the production resolver defaults that to planned_default, not the literal interactive_profile() construct. Retargets the design accordingly and simplifies the implementation (no shared-base change needed at all). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…trigger nudges Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lear review Tasks 6/7 previously copy-pasted the same scripted scenario and completion assertion; extract no_progress_script()/ assert_completed_via_nudge() once, following the file's existing run_request/run_context_for_driver helper-extraction pattern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d_default profile
…led_trigger profile
User asked for coverage similar to tests/reborn_group_extensions. The group harness is for cross-thread scenarios; this is single-thread, so the correct analog per tests/support/reborn/CLAUDE.md is a flat reborn_integration_*.rs test using RebornIntegrationHarness. Feasibility checked: submit_turn defaults to planned_default with no override needed, and CapabilityProgress::NoChange is computed generically from real capability output, not test-mock-only. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…comments Fix two wording-only doc-comment inaccuracies from review: the scheduled-trigger profile doc now names both axes that diverge from the shared base (capability surface AND driver-specific-nudges), and the nudge integration test's module doc attributes builtin.echo's exclusion to the test harness's own fixed capability grant list (core_builtin_tools_from_runtime) rather than a nonexistent production run-profile-level exclusion. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The two *_profile_completes_via_final_answer_nudge tests duplicated ~20 lines of driver/host/request construction, differing only in the resolved profile and a context label. Extract assert_nudge_fires_for_resolved_profile to hold the shared run+assert sequence — found during a pre-PR thermo-nuclear pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5568
July 2, 2026 19:40
Destroyed
github-actions
Bot
added
scope: docs
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a ChangesNudge enablement and tests
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Test as E2E/Integration Test
participant Resolver as RunProfileResolver
participant Driver as PlannedDriver
participant Host as ScriptedHost
Test->>Resolver: resolve planned_default / scheduled_trigger profile
Resolver-->>Test: steering_policy.allow_driver_specific_nudges = true
Test->>Driver: run(no-progress script)
Driver->>Host: repeated identical capability calls
Host-->>Driver: identical outcomes (no progress)
Driver->>Driver: detect NoProgressDetected
Driver->>Host: final-answer nudge call
Host-->>Driver: scripted final reply
Driver-->>Test: LoopExit::Completed via nudge
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 |
![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 enables Reborn's final-answer nudge (allow_driver_specific_nudges) for the planned_default and scheduled_trigger run profiles, while keeping it disabled for the subagent profile. To support this, a new builder method with_driver_specific_nudges was added to RunProfileDefinition. The changes are accompanied by comprehensive unit tests, driver-tier end-to-end tests, and a product-level integration test proving that the nudge successfully fires and completes the run when no progress is detected. No review comments were provided, so there is no feedback to address.
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.
Plan/design-spec docs were working artifacts for this session, not meant to ship in the PR.
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5568
July 2, 2026 19:41
Destroyed
Contributor
There was a problem hiding this comment.
Pull request overview
Enables Reborn’s final-answer nudge mechanism by allowing driver-specific nudges for the real interactive planned_default profile and scheduled_trigger, while keeping subagent opted out. This extends run-profile configuration in ironclaw_turns, wires the flag at the intended Reborn profile-definition call sites, and adds end-to-end tests proving the nudge fires both at the driver tier and through the real submit_turn path.
Changes:
- Add
RunProfileDefinition::with_driver_specific_nudges(bool)and unit-test it inironclaw_turns. - Enable the nudge flag for
planned_defaultandscheduled_triggerinironclaw_reborn(with regression assertions thatsubagentremains disabled). - Add driver-tier and product-tier integration tests that exercise no-progress → final-answer-nudge completion.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/reborn_integration_nudge_final_answer.rs | New product-level integration test proving the nudge fires through the real submit_turn pipeline using scripted builtin.http calls. |
| docs/superpowers/specs/2026-07-02-reborn-nudge-profile-enable-design.md | Design/spec write-up explaining scope correction and implementation approach. |
| docs/superpowers/plans/2026-07-02-reborn-nudge-profile-enable.md | Implementation plan and task breakdown for the change (note: contains a few now-stale builtin.echo references). |
| crates/ironclaw_turns/src/run_profile/resolver.rs | Adds the with_driver_specific_nudges builder + unit test verifying it propagates through resolution. |
| crates/ironclaw_reborn/tests/planned_driver_e2e.rs | Adds driver-tier tests that resolve real profiles and prove the nudge causes completion (extra tool-free model call). |
| crates/ironclaw_reborn/src/planned_driver_factory.rs | Opts planned_default and scheduled_trigger into driver-specific nudges; asserts subagent remains off. |
| crates/ironclaw_agent_loop/src/executor/loop_exit.rs | Updates try_final_answer_nudge doc comment to reflect production enablement for select profiles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+32
to
+36
| /// Gated by `SteeringPolicy.allow_driver_specific_nudges` (enabled for select | ||
| /// Reborn run profiles — see `ironclaw_reborn::planned_driver_factory`; off by | ||
| /// default elsewhere) and capped at one nudge per run. Returns `Ok(None)` when | ||
| /// disabled, capped, or the model still declines to answer — callers then keep | ||
| /// their existing behavior. |
Contributor
Reborn integration-tier coverageLine coverage (Reborn crates): 15.06% — 9590 / 63697 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/reborn_integration_nudge_final_answer.rs`:
- Around line 56-87: The test
no_progress_repeated_http_call_completes_via_final_answer_nudge only checks the
synthesized reply text and does not verify that exactly four builtin.http tool
calls happened before finalization. Update this test to assert the recorded
tool-call count on the RebornIntegrationHarness (or its scripted-reply history)
after submit_turn, so it proves the no-progress path triggered on the expected
4th batch rather than incidentally producing the same final text.
- Around line 1-44: Gate the reborn integration test so it does not run under
plain cargo test by default. Update
tests/reborn_integration_nudge_final_answer.rs to use the integration gate
already used elsewhere, either with a cfg attribute or by registering it under
the integration test target in the test manifest. Make sure the test entry point
and its RebornIntegrationHarness-based setup are only enabled when the
integration feature is active, matching the existing integration-only pattern
used by the other product-stack tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dfcdd839-92ef-48fc-a432-758388477102
📒 Files selected for processing (5)
crates/ironclaw_agent_loop/src/executor/loop_exit.rscrates/ironclaw_reborn/src/planned_driver_factory.rscrates/ironclaw_reborn/tests/planned_driver_e2e.rscrates/ironclaw_turns/src/run_profile/resolver.rstests/reborn_integration_nudge_final_answer.rs
Comment on lines
+1
to
+44
| //! Product-level proof: the final-answer nudge fires through the real | ||
| //! `submit_turn` entry point (product workflow → turn coordinator → | ||
| //! scheduler → agent loop → real `LlmProviderModelGateway` decorator chain | ||
| //! → scripted model), one layer up from the executor/driver-tier proof in | ||
| //! `crates/ironclaw_reborn/tests/planned_driver_e2e.rs`. | ||
| //! | ||
| //! `RebornIntegrationHarness::test_default()` resolves `requested_run_profile: | ||
| //! None` to `planned_default` — the profile Task 2 enabled driver-specific | ||
| //! nudges for — with no special wiring. Four identical `builtin.http` calls | ||
| //! (same URL) drive the real no-progress detector: `RecordingRuntimeHttpEgress` | ||
| //! (installed by `.with_builtin_http_tools()`) always returns the same fixed | ||
| //! scripted body, so the first call's output digest is first-seen | ||
| //! (`MadeProgress`) and the next three repeat the same digest (`NoChange`) — | ||
| //! `trailing_no_progress_results` reaches the default | ||
| //! `typed_progress_run_threshold` (3) right after the 4th capability batch — | ||
| //! `DefaultStopConditionStrategy::should_stop_after_observed_turn` in | ||
| //! `crates/ironclaw_agent_loop/src/strategies/stop.rs`. The executor then | ||
| //! resolves that `NoProgressDetected` stop via `try_final_answer_nudge` | ||
| //! (`crates/ironclaw_agent_loop/src/executor/loop_exit.rs`), issuing one | ||
| //! extra tool-free model call that the 5th scripted reply satisfies. | ||
| //! | ||
| //! Deviation from the plan's starting shape: the brief scripted | ||
| //! `builtin.echo`, reasoning that a first-party capability with a stable | ||
| //! digest would drive the detector. `builtin.echo` IS registered as a | ||
| //! first-party handler with `CapabilityVisibility::Model` | ||
| //! (`crates/ironclaw_host_runtime/src/first_party_tools/mod.rs`), but the | ||
| //! resolved capability surface here (as observed via `RUST_LOG=debug` — | ||
| //! `visible_capability_sample`) does not include it — this test harness's | ||
| //! `core_builtin_tools_from_runtime` grants a fixed `capability_ids` list | ||
| //! (`tests/support/reborn/harness.rs`) that omits `builtin.echo`, not a | ||
| //! production run-profile-level exclusion; the model gateway rejects the | ||
| //! scripted call as "outside the visible capability surface" | ||
| //! (`ironclaw_reborn::model_gateway`), which surfaces as a terminal | ||
| //! `model_error`, not a no-progress signal. Swapping to `builtin.http` | ||
| //! (already proven visible + deterministic by | ||
| //! `tests/reborn_integration_tool_call.rs`) exercises the same digest-based | ||
| //! `NoChange` mechanism without depending on a capability outside this | ||
| //! harness's granted surface. | ||
|
|
||
| #[allow(dead_code)] | ||
| #[path = "support/reborn/mod.rs"] | ||
| mod reborn_support; | ||
| #[allow(dead_code)] | ||
| mod support; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether this integration test target is gated via Cargo.toml required-features
rg -n 'reborn_integration' Cargo.toml
rg -n '\[\[test\]\]' -A5 Cargo.tomlRepository: nearai/ironclaw
Length of output: 997
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Cargo test target entries ==\n'
sed -n '340,390p' Cargo.toml
printf '\n== Presence of the reviewed file and neighboring test files ==\n'
git ls-files 'tests/**' | rg 'reborn_integration_nudge_final_answer|reborn_integration|reborn_group|e2e_thread_scheduling'
printf '\n== File header ==\n'
cat -n tests/reborn_integration_nudge_final_answer.rs | sed -n '1,80p'Repository: nearai/ironclaw
Length of output: 8150
Gate tests/reborn_integration_nudge_final_answer.rs behind integration.
This is auto-discovered by plain cargo test, and there is no #[cfg(feature = "integration")] or [[test]] required-features = ["integration"] entry for it, so the full product-stack harness still runs on the default test path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/reborn_integration_nudge_final_answer.rs` around lines 1 - 44, Gate the
reborn integration test so it does not run under plain cargo test by default.
Update tests/reborn_integration_nudge_final_answer.rs to use the integration
gate already used elsewhere, either with a cfg attribute or by registering it
under the integration test target in the test manifest. Make sure the test entry
point and its RebornIntegrationHarness-based setup are only enabled when the
integration feature is active, matching the existing integration-only pattern
used by the other product-stack tests.
Source: Path instructions
Comment on lines
+56
to
+87
| async fn no_progress_repeated_http_call_completes_via_final_answer_nudge() { | ||
| let h = RebornIntegrationHarness::test_default() | ||
| .with_builtin_http_tools() | ||
| .script([ | ||
| RebornScriptedReply::tool_call( | ||
| "builtin.http", | ||
| serde_json::json!({"url": REPEATED_URL}), | ||
| ), | ||
| RebornScriptedReply::tool_call( | ||
| "builtin.http", | ||
| serde_json::json!({"url": REPEATED_URL}), | ||
| ), | ||
| RebornScriptedReply::tool_call( | ||
| "builtin.http", | ||
| serde_json::json!({"url": REPEATED_URL}), | ||
| ), | ||
| RebornScriptedReply::tool_call( | ||
| "builtin.http", | ||
| serde_json::json!({"url": REPEATED_URL}), | ||
| ), | ||
| RebornScriptedReply::text("final answer synthesized via nudge"), | ||
| ]) | ||
| .build() | ||
| .await | ||
| .expect("harness builds"); | ||
| h.submit_turn("fetch the same item four times") | ||
| .await | ||
| .expect("turn completes"); | ||
| h.assert_reply_contains("final answer synthesized via nudge") | ||
| .await | ||
| .expect("reply finalized via the final-answer nudge"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Assertion only checks final text, not that exactly 4 tool calls preceded it.
The test proves the nudge-completion text appears, but doesn't assert the harness actually made 4 builtin.http calls before the synthesized reply — i.e. that no-progress detection fired at the expected point rather than some other path incidentally producing the same final text. Given the doc comment's detailed claim about the exact mechanism (trailing_no_progress_results reaching threshold on the 4th batch), asserting the recorded call count would make this a tighter proof rather than relying solely on the doc comment's narrative.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/reborn_integration_nudge_final_answer.rs` around lines 56 - 87, The
test no_progress_repeated_http_call_completes_via_final_answer_nudge only checks
the synthesized reply text and does not verify that exactly four builtin.http
tool calls happened before finalization. Update this test to assert the recorded
tool-call count on the RebornIntegrationHarness (or its scripted-reply history)
after submit_turn, so it proves the no-progress path triggered on the expected
4th batch rather than incidentally producing the same final text.
Collaborator
Author
|
/benchmark pinchbench --framework ironclaw-reborn |
Contributor
|
🧪 Started |
|
🚅 Deployed to the ironclaw-pr-5568 environment in ironclaw-ci-preview
|
Collaborator
Author
|
Closing as stale — no activity in over three weeks. The branch is untouched; reopen if this is still needed. |
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
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
SteeringPolicy.allow_driver_specific_nudges) forplanned_default(real interactive/chat/CLI traffic) andscheduled_trigger(trigger-fired runs);subagentstays off, guarded by a regression test.RunProfileDefinition::with_driver_specific_nudges(bool)builder mirrors the existingwith_personal_context_policypattern; the sharedplanned_like_profile_definitionhelper is untouched, so each profile opts in individually.crates/ironclaw_reborn/tests/planned_driver_e2e.rs) driving the realPlannedDriver, and a product-level test (tests/reborn_integration_nudge_final_answer.rs) through the realsubmit_turn→ product workflow → agent loop path.Design notes
RunProfileId::interactive_default()construct; corrected mid-design after finding real interactive traffic actually resolves toplanned_default(requested_run_profile: None→ production resolver default). Seedocs/superpowers/specs/2026-07-02-reborn-nudge-profile-enable-design.md.assert_nudge_fires_for_resolved_profile).Test plan
cargo fmtcleancargo clippy --all --benches --tests --examples --all-featuresclean (zero new warnings)cargo test -p ironclaw_turns -p ironclaw_reborn -p ironclaw_agent_loop— all greencargo test --test reborn_integration_nudge_final_answer— greencargo test— green (two environmental SIGABRT flakes under parallel load confirmed unrelated via isolated re-run)🤖 Generated with Claude Code