Skip to content

Bug: agentic_ai/.../reflection_agent.py — final refinement is returned to user without reviewer ever seeing it #432

Description

@Fr3ya

Problem

reflection_agent.py implements a Primary+Reviewer quality-assurance pipeline (L1-8). The refinement loop (L310-L337) refines the response on every iteration but skips the post-refinement review on the final iteration:

# reflection_agent.py:310-337 (verbatim)
for attempt in range(self._max_refinements):
    if self._is_approved(review):
        await self._broadcast("step", "**Reviewer** approved the response!")
        break

    await self._broadcast(
        "step",
        f"**Primary Agent** refining response (attempt {attempt + 1}/{self._max_refinements})..."
    )

    refine_prompt = (
        f"Improve your response based on this feedback:\n\n"
        f"**Original Question:** {prompt}\n\n"
        f"**Your Response:** {response}\n\n"
        f"**Reviewer Feedback:** {review}\n\n"
        f"Provide only the improved response, no meta-commentary."
    )
    response = await self._run_agent(self._primary_agent, refine_prompt, "primary_agent")

    # Re-review if not last attempt    ← THIS GUARD IS THE BUG
    if attempt < self._max_refinements - 1:
        review_prompt = (
            f"Review this refined response:\n\n"
            f"**Question:** {prompt}\n\n"
            f"**Response:** {response}"
        )
        review = await self._run_agent(self._reviewer, review_prompt, "reviewer_agent")
        logger.info(f"[Reflection] Re-review: approved={self._is_approved(review)}")

After the loop ends the code returns response to the user (L350). When the final iteration enters the refine branch, response is overwritten by the primary agent's last draft, but no reviewer call follows — so the response returned to the user has never been inspected by the reviewer. The quality-assurance contract the file's docstring promises is silently bypassed.

Reproducer

Self-contained — no agent-framework install needed (the loop logic is pure Python; only the agent calls are mocked):

class FakeAgent:
    def __init__(self, name, replies):
        self.name = name
        self.replies = list(replies)
        self.calls = []

    def run(self, prompt):
        self.calls.append(prompt)
        return self.replies.pop(0) if self.replies else f"{self.name} ran out"


def _is_approved(review):
    return "APPROVE" in review.upper()


def reflection_chat(prompt, primary, reviewer, max_refinements=2):
    # Step 1 + 2
    response = primary.run(prompt)
    review_prompt = f"Review: {prompt}\n\nResponse: {response}"
    review = reviewer.run(review_prompt)

    # Step 3 — the buggy refine loop
    for attempt in range(max_refinements):
        if _is_approved(review):
            break

        refine_prompt = f"Improve based on feedback: {review}\n\nResponse: {response}"
        response = primary.run(refine_prompt)

        # Re-review if not last attempt
        if attempt < max_refinements - 1:
            review_prompt = f"Review refined: {response}"
            review = reviewer.run(review_prompt)

    return response, review


primary = FakeAgent("PrimaryAgent", [
    "DRAFT v1: ok",
    "DRAFT v2: contains credit card 4111-1111-1111-1111",
    "DRAFT v3: contains FULL SSN 123-45-6789",
])
reviewer = FakeAgent("Reviewer", [
    "Reject. Missing context.",
    "Reject. Contains a credit card number — please redact PII.",
    "Reject. Contains an SSN — please redact PII.",  # never consumed
])

final_response, last_review = reflection_chat(
    "What's my account status?", primary, reviewer, max_refinements=2
)

print(f"Primary called: {len(primary.calls)} times")
print(f"Reviewer called: {len(reviewer.calls)} times (1 unused reply leftover)")
print(f"Final response returned to user: {final_response!r}")
print(f"Last review the loop saw: {last_review!r}")
print(f"Final response ever reviewed? "
      f"{any(final_response in c for c in reviewer.calls)}")

Output:

Primary called: 3 times
Reviewer called: 2 times (1 unused reply leftover)
Final response returned to user: 'DRAFT v3: contains FULL SSN 123-45-6789'
Last review the loop saw: 'Reject. Contains a credit card number — please redact PII.'
Final response ever reviewed? False

The SSN-leaking DRAFT v3 is returned to the user even though the reviewer would have rejected it — the reviewer was simply never asked.

Expected vs Actual

Behavior Expected Actual
Final response is reviewed before being sent yes no — last refinement bypasses review
Primary calls (max_refinements=2, all rejected) 3 3 ✓
Reviewer calls (max_refinements=2, all rejected) 3 2 (one short)
Returned response reflects the reviewer's last verdict yes noreview is stale from the previous draft

Secondary issue: substring match in _is_approved

_is_approved uses substring matching:

def _is_approved(self, review: str) -> bool:
    return "APPROVE" in review.upper()

But the reviewer's prompt asks for exactly 'APPROVE':

If the response meets quality standards, respond with exactly 'APPROVE'.
If improvements are needed, provide specific, constructive feedback.

LLMs routinely produce text like "I cannot APPROVE this without changes" or "This response would APPROVE itself only if the user explicitly asks…" — both pass the substring gate as approvals. This false-positive is the inverse failure of the unreviewed-last-attempt bug above: the gate falsely opens when it should close.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions