Skip to content

fix(agent): tell the model when compaction destroyed the evidence a forced answer needs - #2147

Open
AlexLiu190625 wants to merge 2 commits into
xorbitsai:mainfrom
AlexLiu190625:fix/react-forced-answer-honest-instruction
Open

fix(agent): tell the model when compaction destroyed the evidence a forced answer needs#2147
AlexLiu190625 wants to merge 2 commits into
xorbitsai:mainfrom
AlexLiu190625:fix/react-forced-answer-honest-instruction

Conversation

@AlexLiu190625

@AlexLiu190625 AlexLiu190625 commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

A ReAct turn can be forced back to final_answer alone while that same turn's
compaction removes the tool observations the answer was supposed to rest on.
The turn still asked the model to answer "using the accumulated conversation and
tool results", which on that turn is an instruction to invent them. The forced
instruction now has a second form: when this turn's compaction destroyed tool
evidence, it names the removed observations, forbids reconstructing, estimating
or illustrating a removed value, and withdraws outcome=completed for the turn.
Choosing between the two forms requires knowing why the turn was forced, so every
site that forces one records its reason, and three keys carry that decision
through a pause and resume.

Behavior changes

  • A forced turn whose own compaction dropped tool results receives the honest
    instruction instead of the ordinary one; the ordinary opening is replaced, not
    appended to, so the two cannot contradict each other.
  • On such a turn outcome=completed is withdrawn, and the model is told to use
    outcome=partial or outcome=blocked instead.
  • Every site that sets force_final_answer_next also records why, drawn from a
    fixed set of reasons; an unrecognized reason reloads as none.
  • forced_answer_reason, forced_answer_compaction_recoveries and
    forced_answer_recovery_followup are written to and read back from pattern
    state, so an interrupt between the decision and the answer resumes into a turn
    that still knows to be honest.
  • The three keys default in opposite directions on reload, and each direction is
    deliberate: an unreadable reason or marker does less, while a missing recovery
    counter reads as unspent so runs checkpointed before the counter existed keep
    that path.
  • Whether compaction destroyed evidence is read from the two metadata keys both
    compacting paths write, not from the compacted flag, which is true even when
    the tail window already held every message and nothing was removed.
  • A turn that loses evidence and cannot restore it logs one warning naming the
    unrecoverable tools, bounded by the same name-count and name-length limits the
    compaction notice already applies.

Scope note

force_final_answer_next is set at four existing sites and cleared at five.
This change adds one line at each of the four setting sites so the flag and its
reason are always written together, and introduces a fifth setting site of its
own, the turn that follows a recovery, which writes both in the same statement
pair; the five clearing sites are untouched. Folding
both into a single writer would be the tidier shape, but none of those five sites
has any causal role in this defect, and two of them are existing paths that are
deliberately left alone, so that rework is tracked separately in
#2150 rather than carried here.

Not in this PR

  • Handing the dropped tools back for one re-fetch turn instead of settling for an
    honest answer. That is the follow-up PR, which stacks on this one.
  • Changing when the compaction notice itself is written into the context.
  • Rewriting the guidance the repeated-tool decision writes into the context. It
    still contains the "accumulated tool results" wording, and rewriting it belongs
    to that decision site; a strict xfail test records the gap and fails once it is
    fixed.

Verification

  • tests/core/agent/test_react.py, tests/core/agent/test_grounding.py and the
    new tests/core/agent/test_react_forced_answer_compaction.py: 275 passed,
    1 xfailed. The new file alone: 38 passed, 1 xfailed.
  • Mutation checks on the production file, each reverted immediately: removing the
    two assignments on the declining branch, removing only the loop-local
    assignment, making the missing recovery counter reload as spent, leaving the
    marker uncleared at finalize, and moving the marker write above the pause exit.
    Each turned red, and the tests that caught them were, respectively,
    test_a_declined_turn_still_speaks_honestly_after_a_resume,
    test_a_declined_turn_clears_its_marker_once_it_has_run,
    test_forced_answer_recovery_state_round_trips_through_checkpoint,
    test_a_finished_run_leaves_no_follow_up_marker_behind together with
    test_declining_recovery_records_the_refusal_in_pattern_state, and
    test_a_declined_turn_still_speaks_honestly_after_a_resume.
  • test_real_compaction_reports_the_keys_the_gate_reads runs both real
    compaction paths and asserts the metadata shape this gate reads, so the
    scripted fixture used by the other tests cannot drift from the real one.
  • pre-commit run --files on both changed files: ruff check, ruff format, mypy,
    isort, codespell and the whitespace hooks all pass.

Part of #2146

A ReAct turn forced back to final_answer alone can lose the tool
observations that answer was meant to rest on, because the same turn's
compaction removes them after the decision to answer was already made.
The turn still asked for an answer "using the accumulated conversation
and tool results", which on that turn is a request to invent them.

The forced instruction now has a second form. When this turn's
compaction destroyed tool evidence, it names the removed observations,
forbids reconstructing, estimating or illustrating a removed value, and
withdraws outcome=completed for the turn. The ordinary wording is
replaced rather than appended to, so the two cannot contradict each
other.

Choosing between the two forms needs to know why the turn was forced, so
every site that forces one now records its reason alongside the flag,
and the reason, the recovery budget and the follow-up marker all travel
through checkpoints. Their reload defaults differ on purpose: an
unreadable reason or marker does less, while a missing recovery counter
reads as unspent so runs checkpointed before it existed keep the path.

Refusing to restore tools is recorded in pattern state as well, so an
interrupt between the refusal and the answer resumes into a turn that
still remembers to be honest.
@XprobeBot XprobeBot added the bug Something isn't working label Sep 5, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a mechanism to handle forced-answer turns in the ReAct pattern when compaction has destroyed the tool evidence required for the final answer. It tracks the reasons for forced answers, detects dropped tool evidence, and adjusts the LLM instructions to honestly report missing observations rather than hallucinating them. It also updates state serialization to ensure these variables survive checkpoints, and adds extensive unit tests. The review feedback identifies a potential state-loss bug on resume when recovery is not yet implemented and decline_reason is None, suggesting a temporary fallback to explicitly decline recovery.

Comment on lines +820 to +821
else:
decline_reason = None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In the current implementation, recovery is not yet supported (deferred to a follow-up PR), so the turn still forces final_answer even when decline_reason is None. However, because decline_reason is None, the code fails to set self.forced_answer_recovery_followup = FORCED_ANSWER_FOLLOWUP_NO_EVIDENCE.

If an interrupt occurs before the LLM call on such a turn, the resumed turn will restore the forcing but will have self.forced_answer_recovery_followup = None, causing it to lose the honest instruction and fall back to the ordinary instruction (which incorrectly tells the model to use the destroyed tool results).

To prevent this state-loss bug on resume, we should temporarily decline recovery explicitly (e.g., by setting decline_reason = "recovery_not_implemented") until the actual recovery mechanism is implemented.

Suggested change
else:
decline_reason = None
else:
# Recovery is not implemented in this PR (deferred to a follow-up),
# so we must decline recovery and set a decline reason to ensure
# the honest instruction is preserved and survives a resume.
decline_reason = "recovery_not_implemented"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants