fix(agent-harness): enforce required structured-output block every turn (#4117)#4900
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3a2c28a7c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "[agent_loop] required output block `{}` recovered via re-prompt (append)", | ||
| contract.block_key | ||
| ); | ||
| repair_text |
There was a problem hiding this comment.
When a streamed reply omits the block, the original answer has already been sent to the client, but this branch appends the entire corrective re-prompt output. The re-prompt explicitly asks the model to reply again and continue with the answer, so a compliant model will return the JSON block plus the full answer, causing the user-visible and persisted reply to duplicate the answer after the appended block. For streamed repairs, append only the extracted required JSON block or use a block-only repair prompt.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 4539106. Confirmed legit: repair_instruction asks the model to lead with the block and continue with its answer, so the streamed append path was concatenating the whole re-prompt reply (block + restated answer) after the already-streamed prose, duplicating the answer in both the live stream and the persisted reply.
Fix: in the streamed path we now append only the extracted required block (find_required_block on the re-prompt reply, serialized), falling back to a synthesized block when the re-prompt doesn't recover one. The original prose is preserved and precedes the block; nothing re-streams the answer. Added a regression test (turn_appends_only_block_not_restated_answer_when_streamed) that returns block + restated answer from the re-prompt and asserts the answer appears exactly once.
Redo of the closed tinyhumansai#4387 against the current harness. Add an opt-in RequiredOutputContract (block_key + required_keys) on AgentConfig; when active, the turn engine validates the final reply and repairs an omitted/invalid block before the turn is accepted. - Validate: reply must carry a leading JSON object with every required key non-null (reuses the harness JSON extractor; blank block_key is inert). - Repair: one bounded corrective re-prompt, reconciled with streaming — when the invalid reply was already streamed, the repaired block is append-synthed so the client's final reply stays consistent with what was streamed (fixes the streaming/return divergence that blocked tinyhumansai#4387). - Tests: 6 turn tests (streamed append-synth, reprompt-failure->synth, preview- is-prefix) + 12 unit tests over validate/repair/reconcile. Closes tinyhumansai#4117
f3a2c28 to
9c2c4e9
Compare
…tinyhumansai#4900) Codex review: in the streamed (append) repair path, the whole corrective re-prompt reply was appended after the already-streamed prose. Since repair_instruction asks the model to lead with the block *and continue with its answer*, a compliant re-prompt returns block + restated answer — so the user-visible and persisted reply duplicated the answer after the block. Append only the extracted required block (via find_required_block), falling back to a synthesized block when the re-prompt doesn't recover one. The original streamed prose is still preserved and precedes the block; no restated answer is re-streamed or persisted. Add a regression test where the re-prompt returns block + restated answer and assert the answer appears exactly once (and the streamed continuation carries only the block).
W5 sweep — pushed 4539106Codex review (1 item, P2): Append only the repaired block for streamed replies — fixed. The streamed repair path appended the entire corrective re-prompt reply after the already-streamed prose; since CI: Comment-only per role — not approving/merging. |
…rn (tinyhumansai#4117) (tinyhumansai#4900) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
2 participants
Closes #4117
Redo of the closed #4387, rebuilt against the current harness (post-#4399) and resolving the streaming blocker that stopped the original.
Approach
Opt-in
RequiredOutputContract { block_key, required_keys }onAgentConfig. When active, the turn engine validates the final reply and repairs an omitted/invalid block before accepting the turn.block_keyis inert.Tests
6 turn tests (streamed append-synth, reprompt-failure→synth, preview-is-a-prefix) + 12 unit tests over validate/repair/reconcile — targeting the changed lines for the ≥80% diff-coverage gate.
Notes
Built on current
main; allAgentConfigliterals repo-wide use..Default::default(), so the new field is additive. No AI attribution.