Repository navigation
fix(core): add iteration limit wrap-up nudge and reasoning length err… - #1029
Premvkmishra wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: 8567f66 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Preview approval requiredThis pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit |
![qodex-ai[bot]](https://avatars.githubusercontent.com/u/8568200?s=60&v=4){kind=small}
|
|
||
| if (finishError !== undefined) { | ||
| const errorContent = completion?.type === 'error' ? completion.error_message : finishError; | ||
| if (finishReason === 'length') { |
There was a problem hiding this comment.
🟠 Major Content-filter failures reported as success · bug
The length-only checks ignore content_filter errors returned by finishReasonErrorMessage. Root threads report filtered responses as successful turns. Child threads return filtered output as successful completions. Check finishError in both branches and preserve its content-filter message alongside the specialized length message.
Qodex reviewReviewed units u1. Adds a core patch changeset. Effort: ●●●○○ (3/5) · 1 finding: 1 major Scope: the whole pull request (no previous review: the whole change is reviewed). Qodex review threads: 1 open, 0 resolved by Qodex. 1 inline comment attached to specific lines below. WalkthroughNotes
What Qodex checkedLenses (openqodex review with checked answers, all dimensions)
Scanners
Quality filters
Verification (M1b probes against PR preview)
Cross-review dedup
Output
Reviewer: shown 12 of 12 changed hunks, opened 6 files (2 changed files never opened), 18 tool calls served, 14 model calls, 0 correction rounds, 52.2s. The library does not report tool calls asked apart from those it served, or a step cap. Every dimension is reviewed. Bugs, security, and performance post inline; low-severity style and convention notes are collected in the group above rather than dropped. Mark a finding resolved or flag a false positive in the Qodex Pull Requests view. |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
|
|
||
| if (finishError !== undefined) { | ||
| const errorContent = completion?.type === 'error' ? completion.error_message : finishError; | ||
| if (finishReason === 'length') { |
There was a problem hiding this comment.
Medium: Preserve the content-filter stop before tool execution
Replacing finishError !== undefined with a length-only check lets content_filter fall through to stepToolResponse() when accumulated tool calls are present; VercelAILLM retains those calls on a filtered finish, although I couldn't confirm a live provider emits valid calls before that finish. Preserve both finishError guards, including the parent-completion branch at line 1295, and customize only the length-error wording so the provider's filtering signal remains a terminal stop.
PR overviewThe PR adds an iteration-limit wrap-up nudge and adjusts reasoning-length error handling in the core agent runtime. One open issue remains: the revised finish-error checks can allow accumulated tool calls to execute after a provider reports a content-filter stop. This weakens the filtering signal as a terminal boundary, though a live provider returning valid tool calls with that finish condition has not been confirmed. Preserving the general finish-error guards while customizing only length-error wording would address the concern. Open issues (1)
Fixed/addressed: 0 · PR risk: 4/10 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8567f66. Configure here.
| if (finishError !== undefined) { | ||
| const errorContent = completion?.type === 'error' ? completion.error_message : finishError; | ||
| if (finishReason === 'length') { | ||
| const errorContent = completion?.type === 'error' ? completion.error_message : baseLengthErrorMessage; |
There was a problem hiding this comment.
Content filter no longer fails turns
High Severity
The finish-reason guard now checks finishReason === 'length' instead of finishError !== undefined. A content_filter stop no longer errors the turn: finishReasonErrorMessage still returns the blocked-output copy, but that value is ignored. Root turns complete as done with partial content, and sub-agents send that content to the parent as success.
Reviewed by Cursor Bugbot for commit 8567f66. Configure here.
Summary
Turns that reach
iteration_limitnow get a chance to finish cleanly. A short wrap-up warning goes to the agent a few calls before the limit, so it can write its pending files and summarize what's left. Output-cap errors caused by reasoning now say so and point to the setting that fixes it.Closes #950
Changes
min(3, iteration_limit - 1)LLM calls remain, each request carries a short note asking the agent to write pending outputs and summarize anything incomplete. It is sent with the request only, so it never enters thread history and works with compaction. It is skipped wheniteration_limitis 1. The limit and its error message are unchanged.lengtherror: when a response stops atmax_tokensand reasoning tokens make up at least half of the output, the error now says the budget was used by reasoning. It points tomodel.params.max_tokensandreasoning_effort. Without reasoning usage, the message staysmax_tokens breached.max_tokensandreasoning_effortas the per-agent output and reasoning controls.{ "type": "input_tokens", "value": 40000 }.@truefoundry/trueforge-core.No schema, default or compaction behavior changes.
How was this tested?
New
tests/orchestration/orchestrationIterationLimit.test.tscovers:lengtherror with and without reasoning usageThe full
trueforge-coresuite passes on Node 22, alongsidepnpm lint:ci,pnpm typecheckandpnpm format:check.Checklist
pnpm build,pnpm test,pnpm typecheck,pnpm lint:ci, andpnpm format:checkpass locallypackages/trueforge-sdk,python/trueforge_sdk,.github/fern/openapi/openapi.json,docs/openapi.json).env.exampleupdated if configuration or behavior changedNote
Low Risk
Runtime behavior changes are scoped to end-of-turn warnings and clearer error text; iteration limit enforcement and defaults in code are unchanged aside from request-only messaging.
Overview
Adds an iteration-limit wrap-up nudge: on the last up-to-three LLM calls in a turn (
min(3, iteration_limit - 1)),AgentThreadinjects an ephemeral internal system message asking the model to finish files and summarize before the harness halts. It is attached only to the outgoing request (not persisted in thread context), including when compaction rewrites context on those steps. The nudge is skipped wheniteration_limitis 1 or the turn ends before the reserve window.finish_reason: lengthhandling now surfaces a richer error when reasoning tokens consume most of the output budget, pointing users atmodel.params.max_tokensandreasoning_effort; otherwise the message staysmax_tokens breached.Docs align the documented default iteration limit with 512, describe the wrap-up behavior, expand
max_tokens/reasoning_effortguidance, and add a short-session compaction threshold example. New orchestration tests cover nudge timing, non-leakage, compaction interaction, and length errors with/without reasoning usage.Reviewed by Cursor Bugbot for commit 8567f66. Bugbot is set up for automated code reviews on this repo. Configure here.