feat(agent-loop): tools-capable completion nudge for interactive coding - #6013
Conversation
The agent loop already had a driver-specific completion nudge (`try_final_answer_nudge`), but it was (a) gated off in every production run profile (`SteeringPolicy.allow_driver_specific_nudges = false`) and (b) tool-free — it forces an empty capability view, so the model can only synthesize prose and cannot write a required output artifact before it answers. Turns that trail off mid-task (the model narrates "let me write the file:" but emits no tool call, or a no-progress stop) therefore end without finishing. Make the nudge tools-capable and turn it on for interactive coding: - resolver.rs: `interactive_profile()` now opts into `allow_driver_specific_nudges` (the reborn/LocalDev coding profile). - On a `GracefulStop` whose closing reply trailed off (empty after trim, or ends with ':'), or a `NoProgressDetected` stop, the loop now re-enters for one more ordinary iteration with the FULL tool surface, injecting a "finish the task — write any required output file, then give your final answer" directive. It reuses the existing machinery: the drained-follow-up continue path and the inline control-message injection used by `RepairInvalidModelOutput`. Capped at 2 nudges/run, gated behind the same steering flag; a clean/complete reply is never nudged (no regression on correct answers). This is the in-loop, tools-capable equivalent of nearai-bench's out-of-loop `trailed_off_without_answer` nudge (same trigger heuristic), so ironclaw finishes these tasks itself instead of relying on the harness. New `LoopExecutionState` fields are `#[serde(default)]` (checkpoint compatible) and the default-family fingerprint is unchanged (nudges are not part of loop identity), preserving replay identity — consistent with how the original nudge was added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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 (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe agent loop detects eligible trailing-off or no-progress stops, injects a bounded tool-capable completion prompt, and retries through the prompt pipeline. Persisted state, interactive-profile enablement, and regression tests cover enabled, disabled, and clean-completion behavior. ChangesCompletion nudge flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AssistantReplyStage
participant Executor
participant PromptPipeline
participant Model
AssistantReplyStage->>Executor: Record trailing-off reply status
Executor->>Executor: Evaluate stop kind and nudge budget
Executor->>PromptPipeline: Mark completion nudge pending
PromptPipeline->>Model: Inject completion nudge control message
Model->>Executor: Return completion and tool work
Possibly related PRs
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 |
github-actions
Bot
added
scope: docs
![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 introduces a tools-capable completion nudge mechanism designed to prevent the agent loop from terminating prematurely when an assistant's reply trails off (e.g., ending with a colon or being empty). Instead of stopping, the loop re-enters for another iteration with the full tool surface available, allowing the model to finish its task. This feature is gated by the steering policy and capped at two nudges per run. The review feedback highlights a potential issue where state.last_reply_trailed_off is not reset to false during capability (tool execution) turns, which could incorrectly trigger an unwanted completion nudge on subsequent graceful stops.
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.
| } | ||
| match kind { | ||
| StopKind::NoProgressDetected => true, | ||
| StopKind::GracefulStop => state.last_reply_trailed_off, |
There was a problem hiding this comment.
There is a potential edge case where state.last_reply_trailed_off is never reset to false when a capability (tool execution) turn is executed.
Issue
If a previous turn's reply trailed off (setting last_reply_trailed_off = true), but the model subsequently executes tools successfully in a later turn and the loop eventually stops gracefully (e.g., due to a tool's terminate_hint), state.last_reply_trailed_off will still be true. This will incorrectly trigger an unwanted completion nudge even though the task was completed successfully.
Suggestion
Reset state.last_reply_trailed_off to false whenever a capability turn is executed. For example, in canonical.rs where ParentLoopOutput::CapabilityCalls is matched (around line 228):
ParentLoopOutput::CapabilityCalls(calls) => {
state.last_reply_trailed_off = false;
latency::stage!(
"capabilities",
...
)?
}
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.57% — 302542 / 353574 lines Per-crate breakdown (63 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)
|
|
🚅 Deployed to the ironclaw-pr-6013 environment in ironclaw-ci-preview
|
Add a control/treatment pair through the real CanonicalAgentLoopExecutor:
- WITH the gate on, a trailed-off reply ('...write it to the file:') re-enters
with tools + the directive; the model then EXECUTES its write tool
(batch_invocations == 1) and completes — the artifact it trailed off on gets
produced.
- WITH the gate off (production default), the same trajectory ends right after
the trail-off; the tool never runs (batch_invocations == 0). This is the
failure the nudge fixes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… panic) Match the FINAL_ANSWER_NUDGE construction: return Result and propagate a PlannerContract error instead of .expect(), satisfying the no-panics-in- production-code check. Plus rustfmt. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
/benchmark pinchbench --framework ironclaw-reborn --model deepseek-ai/DeepSeek-V4-Flash |
|
🧪 Started |
serrrfirat
left a comment
serrrfirat
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Enable tools-capable, driver-specific completion nudges for interactive coding when runs trail off or stall, while preserving clean replies and replay compatibility.
Shape: normal; reviewed the direct PR layer against main. No modifiers were selected: the exact comparison is neither mega, mechanical, nor generated/vendor-heavy, and its file list matches the captured GitHub snapshot.
Coverage: complete; exact local-git diff, 10 files (production 8, tests 1, docs 1, generated/vendor 0, CI 0, config 0), 0 packets, no oversized files, no failed reviewers, no limitations.
Stats: 9 findings (from 18 raw, 9 after dedup) across 6 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0.
The body lists every reviewer finding; overlapping findings are merged only for inline comments.
Security
- Medium Stale trail-off state reopens tools after a terminal capability result (
crates/ironclaw_agent_loop/src/executor/canonical.rs:620, confidence 98) — anchor: crates/ironclaw_agent_loop/src/executor/canonical.rs:620
last_reply_trailed_offis set by an admitted assistant reply but is never cleared when a later turn completes via capability calls. A trailing reply followed by drained queued input continues the loop with this flag still true; if the next capability batch setsterminate_hint, the stop strategy returnsGracefulStop, and this branch mistakes the stale flag for a trail-off on the current turn. The executor consequently grants another full-tool iteration after the capability explicitly requested termination, allowing unintended repeated or mutating tool activity.
Bugs / Correctness
- Medium Pending nudge is cleared before it survives the model boundary (
crates/ironclaw_agent_loop/src/executor/prompt.rs:363-368, confidence 100) — anchor: crates/ironclaw_agent_loop/src/executor/prompt.rs:368
The flag is cleared before the BeforeModel checkpoint and model call. If the call is retried, model.rs rebuilds the request from this state and replaces inline_messages with a bundle that no longer contains the completion directive; recovery from the checkpoint likewise resumes with completion_nudges_used incremented but no pending nudge. A transient model failure or lease/process recovery can therefore silently consume the bounded nudge and complete from an ordinary prompt without performing the unfinished tool work. - Medium Completion conversion misses resume and compaction stop paths (
crates/ironclaw_agent_loop/src/executor/canonical.rs:324-341, confidence 100) — anchor: crates/ironclaw_agent_loop/src/executor/canonical.rs:330
The new completion_nudge_should_fire conversion exists only in the PromptStep::Prepared stop branch. The equivalent StopStep::Stop branches after approval/auth/external-tool resume and SkipModel still call ExitStage directly. A resumed capability that supplies the third NoChange result, or a compaction-only turn whose existing output-token window triggers NoProgressDetected, therefore receives only the old terminal/tool-free behavior instead of the promised tools-capable retry. - Medium Stale trailed-off state can nudge an unrelated graceful stop (
crates/ironclaw_agent_loop/src/executor/canonical.rs:618-621, confidence 95) — anchor: crates/ironclaw_agent_loop/src/executor/canonical.rs:620
GracefulStop is produced both for a current ReplyOnly turn and for an all-terminate-hint capability batch, but this predicate consults only the sticky last_reply_trailed_off bit. If a colon-ending reply is preempted by queued follow-up input, the executor continues without clearing that bit; a capability batch for the follow-up that ends via terminate hints is then incorrectly treated as a newly trailed-off reply and gets an extra model/tool iteration.
Performance / Concurrency
No findings.
Tests
- Medium Production-wired interactive nudge lacks an integration test (
crates/ironclaw_turns/src/run_profile/resolver.rs:332, confidence 100) — anchor: crates/ironclaw_turns/src/run_profile/resolver.rs:332
The executor test manually enables nudges on MockHost, while the resolver test only checks the flag value. No tests/integration case drives the default interactive profile through product workflow, scheduler, agent loop, and a real capability seam, so production wiring could drop the flag or tool surface without failing these tests. - Medium No-progress completion-nudge branch is untested (
crates/ironclaw_agent_loop/src/executor/canonical.rs:619, confidence 100) — anchor: crates/ironclaw_agent_loop/src/executor/canonical.rs:619
The new NoProgressDetected arm always requests a tools-capable retry, but existing enabled-gate no-progress tests invoke ExitStage directly and therefore bypass completion_nudge_should_fire and the canonical re-entry path. A regression could leave stalled runs on the old terminal, tool-free behavior while all current tests pass. - Medium Two-nudge hard cap has no boundary coverage (
crates/ironclaw_agent_loop/src/executor/canonical.rs:615-616, confidence 100) — anchor: crates/ironclaw_agent_loop/src/executor/canonical.rs:615
No test reaches completion_nudges_used == COMPLETION_NUDGE_LIMIT. The added trail-off test exercises only the first nudge, so removing or weakening the cap would permit unbounded extra model and tool iterations without failing the suite. - Medium Legacy checkpoint defaults for new nudge state are untested (
crates/ironclaw_agent_loop/src/state.rs:87-103, confidence 100) — anchor: crates/ironclaw_agent_loop/src/state.rs:87
The PR relies on serde(default) for three new LoopExecutionState fields, but the only legacy-checkpoint nudge test removes final_answer_nudges_used. No test strips completion_nudges_used, completion_nudge_pending, and last_reply_trailed_off and verifies that an older checkpoint still decodes with safe defaults. - Low One-shot prompt consumption is not asserted (
crates/ironclaw_agent_loop/src/executor/prompt.rs:368, confidence 100) — anchor: crates/ironclaw_agent_loop/src/executor/prompt.rs:368
The three-call completion test confirms that the second prompt contains the directive but never checks the third prompt. If completion_nudge_pending stopped being cleared, every later prompt in the same tool flow would repeat the control message and the current test would still pass.
Conventions
- Medium Move completion-nudge mechanics out of the lifecycle spine (
crates/ironclaw_agent_loop/src/executor/canonical.rs:329-349, confidence 100) — anchor: crates/ironclaw_agent_loop/CLAUDE.md:42
The new branch directly decides completion-nudge eligibility, mutates nudge state, and reroutes the stop transition insidecanonical.rs. The crate contract requirescanonical.rsto remain the ordered lifecycle spine and explicitly says lifecycle mechanics belong in their owning executor stage, not as branch logic here. - Medium Preserve the control-message validation error (
crates/ironclaw_agent_loop/src/executor/loop_exit.rs:58-62, confidence 100) — anchor: .claude/rules/error-handling.md:16
The addedmap_err(|_| ...)discards theLoopInlineMessageBodyvalidation failure. Repository error-handling rules explicitly reject cause-discardingmap_err(|_| ...)mappings and require carrying or logging the bound source error. - Medium Add required Reborn integration-tier coverage (
crates/ironclaw_agent_loop/src/executor/tests.rs:1954-1955, confidence 100) — anchor: AGENTS.md:100
This production-wired Reborn behavior is covered only by crate-localMockHosttests. The changed-file set contains notests/integration/test, and the PR body does not explain why the integration harness cannot reach the path. The repository rule requires production-wired Reborn behavior to ship with an integration-tier seam assertion, with crate-tier fallback allowed only when the PR states why integration cannot reach it. - Medium Update the lightweight-loop contract for post-reply tool use (
crates/ironclaw_agent_loop/src/executor/canonical.rs:330-340, confidence 100) — anchor: AGENTS.md:96
The new path can persist an assistantReply, then re-enter the loop and execute tools. The authoritative lightweight-loop contract still says everyReplycompletes and stops the run (docs/reborn/contracts/lightweight-agent-loop.md:102) and its contract-test list says a final reply completes without side effects (docs/reborn/contracts/lightweight-agent-loop.md:502). The diff changes that behavior without updating the relevant spec, contrary to the repository documentation rule. The new test comment atexecutor/tests.rs:2022also calls gate-off the production default even though this PR enables it for the interactive production profile.
Local Patterns
- Low Gate comments still describe nudges as disabled in production (
crates/ironclaw_turns/src/run_profile/resolver.rs:326-332, confidence 100) — anchor: Changed line crates/ironclaw_turns/src/run_profile/resolver.rs:332 enables the gate; crates/ironclaw_turns/tests/run_profile_contract.rs:53-54 confirms that interactive coding now opts in.
Enablingallow_driver_specific_nudgesfor the default interactive profile makes several local comments stale:executor/loop_exit.rs:76andexecutor/tests.rs:1766,2022still call the disabled gate the production default, whileexecutor/tests/support.rs:138describes the shared gate as covering only the final-answer nudge. These comments now give maintainers the wrong activation model for both nudge paths. - Nit Comment points to a nonexistent PromptStage::run method (
crates/ironclaw_agent_loop/src/executor/prompt.rs:698-701, confidence 100) — anchor: crates/ironclaw_agent_loop/src/executor/prompt.rs:193-214 showsPromptStage::processdelegating toPromptPlanningPipeline::run, while changed line 701 names the nonexistent method.
The comment says the flag is consumed byPromptStage::run, butPromptStageonly implementsprocess; the clearing happens inPromptPlanningPipeline::run. Searching the named symbol therefore leads nowhere when auditing the one-shot lifecycle.
Maintainability
- Medium Keep the completion-nudge transition inside StopStage (
crates/ironclaw_agent_loop/src/executor/canonical.rs:330-341, confidence 100) — anchor: crates/ironclaw_agent_loop/src/executor/canonical.rs:330; crates/ironclaw_agent_loop/CLAUDE.md:44
The nudge policy and Stop-to-Continue state transition are embedded in only the prepared-model arm's StopStep handler, even though canonical.rs has separate handlers for prepared, resumed-capability, and skip-model turns. This makes identical StopStep outcomes depend on which lifecycle arm produced them and forces future nudge changes to be repeated across the canonical spine. StopStage already owns the stop decision and receives the host context, state, summary, and pending ack needed to perform this transition. - Low Keep reply-shape metadata on the completed-turn summary (
crates/ironclaw_agent_loop/src/state.rs:97-103, confidence 90) — anchor: crates/ironclaw_agent_loop/src/state.rs:97; crates/ironclaw_agent_loop/src/strategies/stop.rs:40
last_reply_trailed_off describes only the reply represented by the current TurnSummary, but it is stored in long-lived checkpoint state, written by AssistantReplyStage, read by stop handling, and manually reset in canonical.rs. That creates a three-module synchronization invariant and checkpoint compatibility surface for transient data that is consumed during the same completed-turn flow.
Approach
- Medium The opt-in also reaches subagent and scheduled-trigger profiles (
crates/ironclaw_turns/src/run_profile/resolver.rs:326-332, confidence 96) — anchor: crates/ironclaw_turns/src/run_profile/resolver.rs:326-332; inherited via crates/ironclaw_turns/src/run_profile/resolver.rs:216-223 and crates/ironclaw_runner/src/planned_driver_factory.rs:224-287
Enabling the flag ininteractive_profile()is broader than the stated interactive-coding goal becauseRunProfileDefinition::interactive_like()clones this definition, and the runner uses that helper for the planned default, subagent, and scheduled-trigger profiles. Consequently background scheduled-trigger runs and subagent runs also gain bounded extra model/tool iterations even though this PR only validates the interactive profile. Make the nudge policy an explicit choice when constructing an interactive-like profile so only the intended interactive/default profile opts in; this preserves the existing behavior of background profiles and keeps the feature's runtime blast radius aligned with its goal.
| )? { | ||
| StopStep::Stop { | ||
| state, | ||
| state: stop_state, |
There was a problem hiding this comment.
Medium — Completion conversion misses resume and compaction stop paths.
The new completion_nudge_should_fire conversion exists only in the PromptStep::Prepared stop branch. The equivalent StopStep::Stop branches after approval/auth/external-tool resume and SkipModel still call ExitStage directly. A resumed capability that supplies the third NoChange result, or a compaction-only turn whose existing output-token window triggers NoProgressDetected, therefore receives only the old terminal/tool-free behavior instead of the promised tools-capable retry.
Fix: Extract shared StopStep::Stop handling and invoke the completion-nudge conversion from Prepared, resume, and SkipModel branches.
Also flagged by: conventions/Medium (Move completion-nudge mechanics out of the lifecycle spine), conventions/Medium (Update the lightweight-loop contract for post-reply tool use), maintainability/Medium (Keep the completion-nudge transition inside StopStage)
| // `build_prompt_bundle_for_surface` (with the full tool surface still | ||
| // available). Clearing here bounds the nudge to exactly this iteration and | ||
| // keeps a later model-error retry from re-injecting it. | ||
| self.state.completion_nudge_pending = false; |
There was a problem hiding this comment.
Medium — Pending nudge is cleared before it survives the model boundary.
The flag is cleared before the BeforeModel checkpoint and model call. If the call is retried, model.rs rebuilds the request from this state and replaces inline_messages with a bundle that no longer contains the completion directive; recovery from the checkpoint likewise resumes with completion_nudges_used incremented but no pending nudge. A transient model failure or lease/process recovery can therefore silently consume the bounded nudge and complete from an ordinary prompt without performing the unfinished tool work.
Fix: Keep completion_nudge_pending set through checkpoints and model retries, and clear it only after a successful model response has consumed the nudged request.
Also flagged by: tests/Low (One-shot prompt consumption is not asserted)
| assert!(matches!(exit, LoopExit::Failed(_))); | ||
| } | ||
|
|
||
| #[tokio::test] |
There was a problem hiding this comment.
Medium — Add required Reborn integration-tier coverage.
This production-wired Reborn behavior is covered only by crate-local MockHost tests. The changed-file set contains no tests/integration/ test, and the PR body does not explain why the integration harness cannot reach the path. The repository rule requires production-wired Reborn behavior to ship with an integration-tier seam assertion, with crate-tier fallback allowed only when the PR states why integration cannot reach it.
Fix: Add a tests/integration/ harness test that observes the nudged model request and capability execution, or document a concrete integration-tier reachability blocker in the PR.
| pub(super) fn completion_nudge_control_message() -> Result<LoopInlineMessage, AgentLoopExecutorError> | ||
| { | ||
| let safe_body = | ||
| LoopInlineMessageBody::new(COMPLETION_NUDGE.trim().to_string()).map_err(|_| { |
There was a problem hiding this comment.
Medium — Preserve the control-message validation error.
The added map_err(|_| ...) discards the LoopInlineMessageBody validation failure. Repository error-handling rules explicitly reject cause-discarding map_err(|_| ...) mappings and require carrying or logging the bound source error.
Fix: Bind the validation error and preserve it in the executor error or log it before returning the sanitized planner-contract error.
| { | ||
| return false; | ||
| } | ||
| if state.completion_nudges_used >= COMPLETION_NUDGE_LIMIT { |
There was a problem hiding this comment.
Medium — Two-nudge hard cap has no boundary coverage.
No test reaches completion_nudges_used == COMPLETION_NUDGE_LIMIT. The added trail-off test exercises only the first nudge, so removing or weakening the cap would permit unbounded extra model and tool iterations without failing the suite.
Fix: tests::executor::completion_nudge_stops_after_two_retries covering three consecutive trailed-off replies and asserting exactly two injected nudges, three model calls, and completion_nudges_used == 2
| /// required output file) before answering. Capped so the loop can't issue | ||
| /// unbounded extra iterations. `#[serde(default)]` keeps older checkpoints | ||
| /// decodable. | ||
| #[serde(default)] |
There was a problem hiding this comment.
Medium — Legacy checkpoint defaults for new nudge state are untested.
The PR relies on serde(default) for three new LoopExecutionState fields, but the only legacy-checkpoint nudge test removes final_answer_nudges_used. No test strips completion_nudges_used, completion_nudge_pending, and last_reply_trailed_off and verifies that an older checkpoint still decodes with safe defaults.
Fix: tests::state::checkpoint_payload_without_completion_nudge_fields_decodes_to_defaults covering omission of all three new fields and asserting 0/false/false after decoding
Also flagged by: maintainability/Low (Keep reply-shape metadata on the completed-turn summary)
| return false; | ||
| } | ||
| match kind { | ||
| StopKind::NoProgressDetected => true, |
There was a problem hiding this comment.
Medium — No-progress completion-nudge branch is untested.
The new NoProgressDetected arm always requests a tools-capable retry, but existing enabled-gate no-progress tests invoke ExitStage directly and therefore bypass completion_nudge_should_fire and the canonical re-entry path. A regression could leave stalled runs on the old terminal, tool-free behavior while all current tests pass.
Fix: tests::executor::completion_nudge_reenters_with_tools_after_no_progress_detected covering repeated no-change outcomes through CanonicalAgentLoopExecutor, followed by a nudged capability call and closing reply
Also flagged by: security/Medium (Stale trail-off state reopens tools after a terminal capability result), bugs/Medium (Stale trailed-off state can nudge an unrelated graceful stop)
| // completion nudge to re-enter with the full tool surface rather than | ||
| // ending mid-task. Best-effort and capped; see the completion-nudge | ||
| // path in `ironclaw_agent_loop`. | ||
| allow_driver_specific_nudges: true, |
There was a problem hiding this comment.
Medium — Production-wired interactive nudge lacks an integration test.
The executor test manually enables nudges on MockHost, while the resolver test only checks the flag value. No tests/integration case drives the default interactive profile through product workflow, scheduler, agent loop, and a real capability seam, so production wiring could drop the flag or tool surface without failing these tests.
Fix: tests::integration::completion_nudge::interactive_profile_executes_tool_after_trailed_off_reply covering a whole Reborn turn that trails off, receives the completion directive, invokes a recorded capability, and persists the closing reply
Also flagged by: local-patterns/Low (Gate comments still describe nudges as disabled in production), approach/Medium (The opt-in also reaches subagent and scheduled-trigger profiles)
| .inline_messages | ||
| .push(invalid_model_output_repair_control_message()); | ||
| } | ||
| // Tools-capable completion nudge scheduled by the stop handling on the prior |
There was a problem hiding this comment.
Nit — Comment points to a nonexistent PromptStage::run method.
The comment says the flag is consumed by PromptStage::run, but PromptStage only implements process; the clearing happens in PromptPlanningPipeline::run. Searching the named symbol therefore leads nowhere when auditing the one-shot lifecycle.
Fix: Replace PromptStage::run with PromptPlanningPipeline::run (or say that the planning pipeline clears the flag after building the final bundle).
🧪 nearai-bench
|
|
/benchmark pinchbench --framework ironclaw-reborn --model deepseek-ai/DeepSeek-V4-Flash |
|
🧪 Started |
🧪 nearai-bench
|
|
/benchmark pinchbench --framework ironclaw-reborn |
|
🧪 Started |
🧪 nearai-bench
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
crates/ironclaw_agent_loop/src/state.rs (2)
88-112: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMedium — Legacy checkpoint defaults for new nudge state are untested.
The PR relies on
serde(default)for three newLoopExecutionStatefields (completion_nudges_used,completion_nudge_pending,last_reply_trailed_off), but the only legacy-checkpoint nudge test removesfinal_answer_nudges_used. No test strips the new completion fields and verifies that an older checkpoint still decodes with safe defaults.Fix: Add
tests::checkpoint_payload_without_completion_nudge_fields_decodes_to_defaultscovering omission of all three new fields and asserting0/false/falseafter decoding.🤖 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 `@crates/ironclaw_agent_loop/src/state.rs` around lines 88 - 112, Add tests::checkpoint_payload_without_completion_nudge_fields_decodes_to_defaults alongside the existing legacy checkpoint tests. Remove completion_nudges_used, completion_nudge_pending, and last_reply_trailed_off from the serialized checkpoint payload, decode it into LoopExecutionState, and assert the fields default to 0, false, and false respectively.
88-112: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMedium — Legacy checkpoint defaults for new nudge state are untested.
The PR relies on
serde(default)for three newLoopExecutionStatefields (completion_nudges_used,completion_nudge_pending,last_reply_trailed_off), but the only legacy-checkpoint nudge test removesfinal_answer_nudges_used. No test strips the new completion fields and verifies that an older checkpoint still decodes with safe defaults.Fix: Add
tests::checkpoint_payload_without_completion_nudge_fields_decodes_to_defaultscovering omission of all three new fields and asserting0/false/falseafter decoding.🤖 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 `@crates/ironclaw_agent_loop/src/state.rs` around lines 88 - 112, Add tests::checkpoint_payload_without_completion_nudge_fields_decodes_to_defaults alongside the existing legacy checkpoint test, removing completion_nudges_used, completion_nudge_pending, and last_reply_trailed_off from the serialized payload before decoding. Assert the decoded LoopExecutionState restores these fields to 0, false, and false respectively.crates/ironclaw_agent_loop/src/executor/tests.rs (6)
2345-2345: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMedium — No-progress completion-nudge branch is untested.
The new
NoProgressDetectedarm always requests a tools-capable retry, but existing enabled-gate no-progress tests invokeExitStagedirectly and therefore bypasscompletion_nudge_should_fireand the canonical re-entry path. A regression could leave stalled runs on the old terminal, tool-free behavior while all current tests pass.Fix: Add
tests::executor::completion_nudge_reenters_with_tools_after_no_progress_detectedcovering repeated no-change outcomes throughCanonicalAgentLoopExecutor, followed by a nudged capability call and closing reply.🤖 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 `@crates/ironclaw_agent_loop/src/executor/tests.rs` at line 2345, Add an async test named tests::executor::completion_nudge_reenters_with_tools_after_no_progress_detected that drives repeated no-change outcomes through CanonicalAgentLoopExecutor, exercises the NoProgressDetected completion-nudge path, verifies re-entry requests tool capabilities, and finishes with a closing reply.
2345-2345: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMedium — No-progress completion-nudge branch is untested.
The new
NoProgressDetectedarm always requests a tools-capable retry, but existing enabled-gate no-progress tests invokeExitStagedirectly and therefore bypasscompletion_nudge_should_fireand the canonical re-entry path. A regression could leave stalled runs on the old terminal, tool-free behavior while all current tests pass.Fix: Add
tests::executor::completion_nudge_reenters_with_tools_after_no_progress_detectedcovering repeated no-change outcomes throughCanonicalAgentLoopExecutor, followed by a nudged capability call and closing reply.🤖 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 `@crates/ironclaw_agent_loop/src/executor/tests.rs` at line 2345, Add the test tests::executor::completion_nudge_reenters_with_tools_after_no_progress_detected in the executor tests, exercising repeated no-change outcomes through CanonicalAgentLoopExecutor, then verifying completion_nudge_should_fire handles NoProgressDetected by re-entering with tools enabled and accepting the closing reply.
2279-2279: 📐 Maintainability & Code Quality | 🟠 Major | ⚖️ Poor tradeoffMedium — Add required Reborn integration-tier coverage.
This production-wired Reborn behavior is covered only by crate-local
MockHosttests. The changed-file set contains notests/integration/test, and the PR body does not explain why the integration harness cannot reach the path. The repository rule requires production-wired Reborn behavior to ship with an integration-tier seam assertion, with crate-tier fallback allowed only when the PR states why integration cannot reach it.Fix: Add a
tests/integration/harness test that observes the nudged model request and capability execution, or document a concrete integration-tier reachability blocker in the PR.🤖 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 `@crates/ironclaw_agent_loop/src/executor/tests.rs` at line 2279, Add integration-tier coverage under tests/integration/ for the production-wired Reborn path exercised by the test near the #[tokio::test] in the executor tests. Assert that the harness observes both the nudged model request and subsequent capability execution; if the path cannot be reached through the integration harness, document the specific reachability blocker in the PR instead.
2377-2377: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMedium — Two-nudge hard cap has no boundary coverage.
No test reaches
completion_nudges_used == COMPLETION_NUDGE_LIMIT. The added trail-off test exercises only the first nudge, so removing or weakening the cap would permit unbounded extra model and tool iterations without failing the suite.Fix: Add
tests::executor::completion_nudge_stops_after_two_retriescovering three consecutive trailed-off replies and asserting exactly two injected nudges, three model calls, andcompletion_nudges_used == 2.🤖 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 `@crates/ironclaw_agent_loop/src/executor/tests.rs` at line 2377, Add the async test completion_nudge_stops_after_two_retries in tests::executor, using three consecutive trailed-off replies to exercise the completion nudge limit. Assert exactly two injected nudges, three model calls, and completion_nudges_used == 2, preserving the existing executor test setup and assertions for the first-nudge behavior.
2279-2279: 📐 Maintainability & Code Quality | 🟠 Major | ⚖️ Poor tradeoffMedium — Add required Reborn integration-tier coverage.
This production-wired Reborn behavior is covered only by crate-local
MockHosttests. The changed-file set contains notests/integration/test, and the PR body does not explain why the integration harness cannot reach the path. The repository rule requires production-wired Reborn behavior to ship with an integration-tier seam assertion, with crate-tier fallback allowed only when the PR states why integration cannot reach it.Fix: Add a
tests/integration/harness test that observes the nudged model request and capability execution, or document a concrete integration-tier reachability blocker in the PR.🤖 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 `@crates/ironclaw_agent_loop/src/executor/tests.rs` at line 2279, Add an integration-tier test under tests/integration/ that exercises the production-wired Reborn flow and asserts both the nudged model request and resulting capability execution, rather than relying only on the crate-local MockHost test around the tokio test. If the integration harness genuinely cannot reach this path, document the specific reachability blocker in the PR instead.
2377-2377: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMedium — Two-nudge hard cap has no boundary coverage.
No test reaches
completion_nudges_used == COMPLETION_NUDGE_LIMIT. The added trail-off test exercises only the first nudge, so removing or weakening the cap would permit unbounded extra model and tool iterations without failing the suite.Fix: Add
tests::executor::completion_nudge_stops_after_two_retriescovering three consecutive trailed-off replies and asserting exactly two injected nudges, three model calls, andcompletion_nudges_used == 2.🤖 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 `@crates/ironclaw_agent_loop/src/executor/tests.rs` at line 2377, Add the async test completion_nudge_stops_after_two_retries under tests::executor, configuring three consecutive trailed-off replies and asserting exactly two injected nudges, three model calls, and completion_nudges_used == 2. Use the existing trail-off test setup and helpers, extending it only to cover the COMPLETION_NUDGE_LIMIT boundary.crates/ironclaw_agent_loop/src/executor/canonical.rs (2)
331-377: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMedium — Completion conversion misses resume and compaction stop paths.
The
completion_nudge_should_firetransition exists only in thePromptStep::Preparedstop branch. The equivalentStopStep::Stopbranches afterResumeApproval/ResumeAuth/ResumeExternalTool(lines 460-478) andSkipModel(lines 548-567) still callExitStagedirectly. A resumed capability that supplies aNoChangeresult, or a compaction-only turn triggeringNoProgressDetected, receives the old terminal, tool-free behavior instead of the tools-capable retry.Fix: Extract the shared
StopStep::Stophandling (including the nudge check) and invoke it from thePrepared, resume, andSkipModelbranches.🤖 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 `@crates/ironclaw_agent_loop/src/executor/canonical.rs` around lines 331 - 377, The completion-nudge transition is implemented only for the Prepared stop path, leaving resume and SkipModel StopStep branches terminal. Extract the shared StopStep::Stop handling, including completion_nudge_should_fire and its tools-capable retry state updates, and reuse it from Prepared, ResumeApproval/ResumeAuth/ResumeExternalTool, and SkipModel branches while preserving normal ExitStage processing.
331-377: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMedium — Completion conversion misses resume and compaction stop paths.
The
completion_nudge_should_firetransition exists only in thePromptStep::Preparedstop branch. The equivalentStopStep::Stopbranches afterResumeApproval/ResumeAuth/ResumeExternalTool(lines 460-478) andSkipModel(lines 548-567) still callExitStagedirectly. A resumed capability that supplies aNoChangeresult, or a compaction-only turn triggeringNoProgressDetected, receives the old terminal, tool-free behavior instead of the tools-capable retry.Fix: Extract the shared
StopStep::Stophandling (including the nudge check) and invoke it from thePrepared, resume, andSkipModelbranches.🤖 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 `@crates/ironclaw_agent_loop/src/executor/canonical.rs` around lines 331 - 377, Extract the shared StopStep::Stop handling into a reusable path that performs completion_nudge_should_fire before invoking ExitStage, preserving ack deferral and stop-state updates. Replace the direct terminal handling in the PromptStep::Prepared, ResumeApproval/ResumeAuth/ResumeExternalTool, and SkipModel branches so resumed NoChange and compaction-only NoProgressDetected stops receive the same tools-capable completion nudge.crates/ironclaw_agent_loop/src/executor/prompt.rs (2)
373-378: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMedium — Pending nudge is cleared before it survives the model boundary.
The
completion_nudge_pendingflag is cleared before theBeforeModelcheckpoint and model call. If the model call fails and is retried,model.rsrebuilds the request from this state and replacesinline_messageswith a bundle that no longer contains the completion directive. Recovery from the checkpoint likewise resumes withcompletion_nudges_usedincremented but no pending nudge. A transient model failure or lease/process recovery can therefore silently consume the bounded nudge and complete from an ordinary prompt without performing the unfinished tool work.Fix: Keep
completion_nudge_pendingset through checkpoints and model retries, and clear it only after a successful model response has consumed the nudged request.🤖 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 `@crates/ironclaw_agent_loop/src/executor/prompt.rs` around lines 373 - 378, Keep completion_nudge_pending set through the BeforeModel checkpoint, model call, retries, and recovery paths; clear it only after a successful model response has consumed the nudged request. Update the completion-nudge handling around build_prompt_bundle_for_surface and the model response flow so failed calls or resumed checkpoints preserve and re-inject the directive.
373-378: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMedium — Pending nudge is cleared before it survives the model boundary.
The
completion_nudge_pendingflag is cleared before theBeforeModelcheckpoint and model call. If the model call fails and is retried,model.rsrebuilds the request from this state and replacesinline_messageswith a bundle that no longer contains the completion directive. Recovery from the checkpoint likewise resumes withcompletion_nudges_usedincremented but no pending nudge. A transient model failure or lease/process recovery can therefore silently consume the bounded nudge and complete from an ordinary prompt without performing the unfinished tool work.Fix: Keep
completion_nudge_pendingset through checkpoints and model retries, and clear it only after a successful model response has consumed the nudged request.🤖 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 `@crates/ironclaw_agent_loop/src/executor/prompt.rs` around lines 373 - 378, Keep state.completion_nudge_pending set through the BeforeModel checkpoint and model retry/recovery paths, rather than clearing it immediately after building the prompt bundle. Clear it only after the model call returns successfully and the nudged request has been consumed, preserving the directive when model.rs rebuilds the request or execution resumes from a checkpoint.
🤖 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.
Outside diff comments:
In `@crates/ironclaw_agent_loop/src/executor/canonical.rs`:
- Around line 331-377: The completion-nudge transition is implemented only for
the Prepared stop path, leaving resume and SkipModel StopStep branches terminal.
Extract the shared StopStep::Stop handling, including
completion_nudge_should_fire and its tools-capable retry state updates, and
reuse it from Prepared, ResumeApproval/ResumeAuth/ResumeExternalTool, and
SkipModel branches while preserving normal ExitStage processing.
- Around line 331-377: Extract the shared StopStep::Stop handling into a
reusable path that performs completion_nudge_should_fire before invoking
ExitStage, preserving ack deferral and stop-state updates. Replace the direct
terminal handling in the PromptStep::Prepared,
ResumeApproval/ResumeAuth/ResumeExternalTool, and SkipModel branches so resumed
NoChange and compaction-only NoProgressDetected stops receive the same
tools-capable completion nudge.
In `@crates/ironclaw_agent_loop/src/executor/prompt.rs`:
- Around line 373-378: Keep completion_nudge_pending set through the BeforeModel
checkpoint, model call, retries, and recovery paths; clear it only after a
successful model response has consumed the nudged request. Update the
completion-nudge handling around build_prompt_bundle_for_surface and the model
response flow so failed calls or resumed checkpoints preserve and re-inject the
directive.
- Around line 373-378: Keep state.completion_nudge_pending set through the
BeforeModel checkpoint and model retry/recovery paths, rather than clearing it
immediately after building the prompt bundle. Clear it only after the model call
returns successfully and the nudged request has been consumed, preserving the
directive when model.rs rebuilds the request or execution resumes from a
checkpoint.
In `@crates/ironclaw_agent_loop/src/executor/tests.rs`:
- Line 2345: Add an async test named
tests::executor::completion_nudge_reenters_with_tools_after_no_progress_detected
that drives repeated no-change outcomes through CanonicalAgentLoopExecutor,
exercises the NoProgressDetected completion-nudge path, verifies re-entry
requests tool capabilities, and finishes with a closing reply.
- Line 2345: Add the test
tests::executor::completion_nudge_reenters_with_tools_after_no_progress_detected
in the executor tests, exercising repeated no-change outcomes through
CanonicalAgentLoopExecutor, then verifying completion_nudge_should_fire handles
NoProgressDetected by re-entering with tools enabled and accepting the closing
reply.
- Line 2279: Add integration-tier coverage under tests/integration/ for the
production-wired Reborn path exercised by the test near the #[tokio::test] in
the executor tests. Assert that the harness observes both the nudged model
request and subsequent capability execution; if the path cannot be reached
through the integration harness, document the specific reachability blocker in
the PR instead.
- Line 2377: Add the async test completion_nudge_stops_after_two_retries in
tests::executor, using three consecutive trailed-off replies to exercise the
completion nudge limit. Assert exactly two injected nudges, three model calls,
and completion_nudges_used == 2, preserving the existing executor test setup and
assertions for the first-nudge behavior.
- Line 2279: Add an integration-tier test under tests/integration/ that
exercises the production-wired Reborn flow and asserts both the nudged model
request and resulting capability execution, rather than relying only on the
crate-local MockHost test around the tokio test. If the integration harness
genuinely cannot reach this path, document the specific reachability blocker in
the PR instead.
- Line 2377: Add the async test completion_nudge_stops_after_two_retries under
tests::executor, configuring three consecutive trailed-off replies and asserting
exactly two injected nudges, three model calls, and completion_nudges_used == 2.
Use the existing trail-off test setup and helpers, extending it only to cover
the COMPLETION_NUDGE_LIMIT boundary.
In `@crates/ironclaw_agent_loop/src/state.rs`:
- Around line 88-112: Add
tests::checkpoint_payload_without_completion_nudge_fields_decodes_to_defaults
alongside the existing legacy checkpoint tests. Remove completion_nudges_used,
completion_nudge_pending, and last_reply_trailed_off from the serialized
checkpoint payload, decode it into LoopExecutionState, and assert the fields
default to 0, false, and false respectively.
- Around line 88-112: Add
tests::checkpoint_payload_without_completion_nudge_fields_decodes_to_defaults
alongside the existing legacy checkpoint test, removing completion_nudges_used,
completion_nudge_pending, and last_reply_trailed_off from the serialized payload
before decoding. Assert the decoded LoopExecutionState restores these fields to
0, false, and false respectively.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 905c4697-7641-4008-8434-a2e21cd38f6f
📒 Files selected for processing (7)
crates/ironclaw_agent_loop/src/executor.rscrates/ironclaw_agent_loop/src/executor/assistant_reply.rscrates/ironclaw_agent_loop/src/executor/canonical.rscrates/ironclaw_agent_loop/src/executor/loop_exit.rscrates/ironclaw_agent_loop/src/executor/prompt.rscrates/ironclaw_agent_loop/src/executor/tests.rscrates/ironclaw_agent_loop/src/state.rs
…rift CI break) main added model_observation to CapabilityResultMessage; the completion-nudge test's batch-outcome literal predates it, so the merge-with-main didn't compile (E0063), failing every Reborn test bucket + clippy. Set None to match the current struct. Nudge logic unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
henrypark133
left a comment
henrypark133
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Make interactive coding completion nudges tools-capable so trailing-off or no-progress turns can finish required tasks before answering.
Stats: 0 net-new findings (4 candidate findings, 3 after overlap deduplication, all covered by existing unresolved review threads) across 0 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach (parent-context fallback because the agent thread limit was reached). Reviewers failed: none. Body-only: 0.
No new actionable findings remain after comparing the forced-pass candidates with the 10 unresolved current-head review threads. The focused completion-nudge tests also pass: 3 passed.
…ntaining "Secretary" aren't scrubbed as "secret" on replay — undoes #5902 tool-result eviction / re-fetch loop (+ 24KB/48KB caps) (#6129) * docs: add design spec for enabling Reborn nudges on 2 profiles 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> * docs: correct nudge-enable spec target from interactive_default to planned_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> * docs: add implementation plan for enabling planned_default/scheduled_trigger nudges Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: extract shared test helpers in nudge-enable plan per thermo-nuclear 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> * feat(ironclaw_turns): add RunProfileDefinition::with_driver_specific_nudges builder * Revert "feat(ironclaw_turns): add RunProfileDefinition::with_driver_specific_nudges builder" This reverts commit 326a16c. * docs: agent-loop canonical.rs slop cleanup design spec Scopes fix for two recurring anti-pattern instances (completion-nudge branching from #6013, latency-span boilerplate from #5487) plus a new guardrail rule file, per Slack discussion on ironclaw_agent_loop growth. * fix(threads): raise result_read preview/chunk cap to undo #5902 benchmark regression PR #5902 (fixing #5838's context-compaction crash) cut the model-visible tool-result preview from 100,000 to 2,048 bytes and capped result_read pagination at the same 2,048-byte chunk with no bulk-fetch option -- recovering a 100KB tool result now takes ~49 manual result_read calls, which most agent policies won't do reliably. This is a likely cause of the reported benchmark score regression. Raises TOOL_RESULT_RECORD_READ_MAX_BYTES to 40KB (still per-call bounded, which is the property #5838 actually needed -- unbounded accumulation in the compacted transcript caused the crash, not single-call size). Derives MAX_MODEL_OBSERVATION_BYTES from that constant (was an independent 4096 literal) so the whole-envelope validation cap can't silently fall behind the preview cap and start dropping large observations to bare summaries. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(tests): size large-echo fixture against raised result_read cap TOOL_RESULT_RECORD_READ_MAX_BYTES was raised 2KiB -> 40KiB earlier in this PR; local_dev_runtime_safe_preview_observer_receives_bounded_payload used a hardcoded ~2.5KB fixture sized for the old cap, so it no longer exceeded the new cap and the truncation path it exercises stopped firing, failing the "raw tail must remain out of the model replay" assertion. Size the fixture relative to the cap instead, matching the fix already applied to the other 3 hardcoded-size fixtures in this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(threads): word-boundary match for sensitive markers (undo "Secretary" scrub) The replay validator matched SENSITIVE/PROMPT_INJECTION markers as raw substrings, so `secret` matched the ordinary word `secretary`. Any tool result containing "Secretary of the Treasury" (i.e. every OfficeQA read, and PinchBench doc tasks) had its model-observation scrubbed on replay and fell back to a stub — evicting the content from the transcript and sending the model into a re-fetch loop (identical read/grep re-issued 16-26x, result_read storms, 3-10x calls/cost, timeouts). This is the residual #5902 regression the preview/observation cap bump alone did not fix. Match markers on word boundaries instead: a marker only trips as a standalone alphanumeric token. Delimiter-bounded markers (`bearer `, `authorization:`) and standalone credentials (`client secret`) still match. Verified on the two hardest-hit OfficeQA tasks (UID0072, UID0034): scrub events 16+/task -> 0, result_read 12-16 -> 0-2, calls 45-173 -> 18-29, score 0.0 -> 1.0, cost at/below pre-5902. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(threads): lower caps to 16KB/32KB + retention regression tests Follow-up on the same PR: - Preview cap 40KB -> 16KB and envelope cap derived as 2x (32KB). 40KB retained per result was too much context; 16KB holds a normal table read while staying bounded, and 32KB envelope comfortably fits a 16KB preview of ordinary text plus schema fields. - With the preview lower, more results overflow onto the result_read path, so the retention of paged content matters more. That path was never a separate bug: a result_read chunk's observation flows through the SAME normalize/validate scrub as the first-look preview, so the word-boundary marker fix already keeps paged chunks intact (confirmed: at 16KB, tasks page result_read and still resolve 1.0). No InlineOnly change needed. - Comprehensive retention tests so this can't regress: * document_content_preview_is_retained_on_replay_not_scrubbed — an 8KB "Secretary of the Treasury" preview stays intact through replay. * observation_envelope_cap_covers_the_preview_cap — structural guard that the envelope cap can never fall below the preview cap (the #5902 drift). * full_cap_preview_survives_replay — a max-size preview still fits. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(threads): pin result_read chunk retention + bump preview cap to 24KB - Add `result_read_chunk_observation_with_document_content_is_retained`: proves a paged result_read CHUNK observation (same shape result_read_observation emits) carrying "Secretary of the Treasury" survives replay intact, not scrubbed to a stub. Chunk observations run through the same normalize/scrub as first-look previews, so this pins that the paged-retrieval path ("get the rest") stays fixed too — the deterministic guarantee that paged content can't silently vanish. - Preview cap 16KB -> 24KB (envelope derives to 48KB). 16KB was tight enough that overflow-heavy reads paged hard; 24KB holds a normal multi-table read while staying bounded. Retention is guaranteed by the tests regardless of the exact cap; 24KB is the tuned middle. Deterministic coverage now spans all three retention failure modes: first-look preview (document_content_preview_is_retained...), paged chunk (this test), and envelope-vs-preview drift (observation_envelope_cap_covers_the_preview_cap). ironclaw_threads 82/82. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(threads): retain result-read previews on replay * fix(threads): satisfy replay CI guards --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Pranav Raja <pranav.raja@near.ai>
What
Makes the agent loop's driver-specific completion nudge tools-capable and turns it on for interactive coding. Two minimal, complementary parts:
run_profile/resolver.rs::interactive_profile()opts intoSteeringPolicy.allow_driver_specific_nudges(the interactive-coding / reborn-LocalDev profile). It wasfalsein every production profile, so the existing nudge never fired.try_final_answer_nudgeis terminal and tool-free (empty capability view → prose only). Instead, when a turn ends by trailing off on aGracefulStop(reply.trim().is_empty() || ends_with(':')) or onNoProgressDetected, the loop now re-enters for one more ordinary iteration with the full tool surface, injecting a "finish the task — write any required output file, then give your final answer" directive.Reuses existing machinery: the drained-follow-up continue and the inline control-message injection used by
RetryAlteration::RepairInvalidModelOutput. Capped at 2 nudges/run (COMPLETION_NUDGE_LIMIT), gated behind the same steering flag. A clean/complete reply is never nudged. The control-message constructor is fallible (no panic), matchingFINAL_ANSWER_NUDGE.Why
Turns that trail off mid-task (model narrates "let me write the file:" but emits no tool call, or a no-progress stop) otherwise end without finishing — the tool-free nudge can't write a required output artifact before answering; this one can. In-loop equivalent of nearai-bench's out-of-loop
trailed_off_without_answernudge (same trigger heuristic), so ironclaw finishes these tasks itself instead of relying on the harness.Compatibility / replay
LoopExecutionStatefields are#[serde(default)](older checkpoints decode).Validation
Deterministic flip proof (control vs treatment), through the real
CanonicalAgentLoopExecutor:completion_nudge_lets_model_use_tools_to_finish_after_trailing_off— a trailed-off reply re-enters with tools + the directive; the model then executes its write tool (batch_invocations == 1) and completes.completion_nudge_disabled_leaves_trailed_off_run_without_tool_use— same trajectory, gate off: the tool never runs (batch_invocations == 0). The failure this fixes.completion_nudge_skipped_on_clean_reply— a complete reply is left untouched (no regression).Green: agent-loop (379) + turns (143) + reborn_composition tests; Formatting / Code Style / No-panics / Clippy (all-features); Reborn E2E + recorded-fixture replay; Live Canary (deterministic-replay).
Live-model note (updated)
The production reborn path (NEAR AI cloud) runs this cleanly: the 2026-07-12 daily PinchBench completed 147/147 at avg 0.935 with 0 errored/hung tasks. An earlier caveat about a "mid-turn hang" was an artifact of local validation over OpenRouter's deepseek endpoint, which intermittently returns undecodable HTTP bodies on tool-heavy requests (
HttpError: error decoding response body); ironclaw retries them as transient, but across a long task they can miss the turn timeout before reaching a stop. That's a provider-transport flake specific to that endpoint — not reborn, this branch, ormain(the NEAR path shows 0 such errors). PinchBench is being run on this PR via the NEAR path to confirm.🤖 Generated with Claude Code