Skip to content

Bug: examples/agentframework_workflow.py — Reviewer fail-open routes unparseable reviews directly to Publisher, skipping Editor #75

Description

@Fr3ya

Minimal steps to reproduce

examples/agentframework_workflow.py defines two router conditions that have asymmetric fail-open behavior:

# examples/agentframework_workflow.py:45-65 (verbatim)
def needs_editing(message: Any) -> bool:
    """Check if content needs editing based on review score."""
    if not isinstance(message, AgentExecutorResponse):
        return False
    try:
        review = ReviewResult.model_validate_json(message.agent_response.text)
        return review.score < 80
    except Exception:
        return False        # ← fail-CLOSED: don't route to editor


def is_approved(message: Any) -> bool:
    """Check if content is approved (high quality)."""
    if not isinstance(message, AgentExecutorResponse):
        return True
    try:
        review = ReviewResult.model_validate_json(message.agent_response.text)
        return review.score >= 80
    except Exception:
        return True         # ← fail-OPEN: route to publisher anyway

These are then wired in the WorkflowBuilder (L151-166):

workflow = (
    WorkflowBuilder(start_executor=writer, ...)
    .add_edge(writer, reviewer)
    .add_edge(reviewer, publisher, condition=is_approved)   # ← fires on parse failure
    .add_edge(reviewer, editor, condition=needs_editing)    # ← does NOT fire on parse failure
    .add_edge(editor, publisher)
    .add_edge(publisher, summarizer)
    .build()
)

When the reviewer agent's response can't be parsed as ReviewResult (refusal text, markdown-wrapped JSON, commentary-prefixed JSON, empty stream, missing required fields), the publisher edge fires and the editor edge does not — so unreviewed content goes straight to publishing.

Reproducer (self-contained ):

from typing import Any
from pydantic import BaseModel


class ReviewResult(BaseModel):
    score: int
    feedback: str
    clarity: int
    completeness: int
    accuracy: int
    structure: int


# Stand-in for agent_framework.AgentExecutorResponse (only `.agent_response.text`
# is touched by the conditions)
class _StubResp:
    def __init__(self, text):
        self.text = text


class AgentExecutorResponse:
    def __init__(self, text):
        self.agent_response = _StubResp(text)


def needs_editing(message: Any) -> bool:
    if not isinstance(message, AgentExecutorResponse):
        return False
    try:
        review = ReviewResult.model_validate_json(message.agent_response.text)
        return review.score < 80
    except Exception:
        return False


def is_approved(message: Any) -> bool:
    if not isinstance(message, AgentExecutorResponse):
        return True
    try:
        review = ReviewResult.model_validate_json(message.agent_response.text)
        return review.score >= 80
    except Exception:
        return True


def simulate_routing(msg):
    targets = []
    if is_approved(msg):
        targets.append("publisher")
    if needs_editing(msg):
        targets.append("editor")
    return targets


# Inputs real LLMs produce
cases = [
    ("well-formed score=85",
     '{"score": 85, "feedback": "ok", "clarity": 90, "completeness": 80, "accuracy": 90, "structure": 80}'),
    ("well-formed score=60",
     '{"score": 60, "feedback": "bad", "clarity": 50, "completeness": 70, "accuracy": 60, "structure": 60}'),
    ("markdown-wrapped JSON",
     '```json\n{"score": 40, "feedback": "low", "clarity": 50, "completeness": 30, "accuracy": 20, "structure": 50}\n```'),
    ("model refusal",
     "I cannot evaluate this content as it appears to violate content policies."),
    ("commentary + JSON",
     'Based on my analysis: {"score": 30, "feedback": "low", "clarity": 30, "completeness": 20, "accuracy": 30, "structure": 30}'),
    ("empty/truncated stream",
     ""),
    ("missing required fields",
     '{"score": 30}'),
]
for label, text in cases:
    print(f"{label!r:50s}{simulate_routing(AgentExecutorResponse(text))}")

Output:

'well-formed score=85'                              → ['publisher']
'well-formed score=60'                              → ['editor']
'markdown-wrapped JSON'                             → ['publisher']
'model refusal'                                     → ['publisher']
'commentary + JSON'                                 → ['publisher']
'empty/truncated stream'                            → ['publisher']
'missing required fields'                           → ['publisher']

5/7 reviewer outputs that should route to the editor (because the reviewer either failed or implicitly indicated rejection) are silently routed to the publisher.

Any log messages given by the failure

None — the silent failure is the bug. The workflow runs to completion, agent_framework.devui surfaces the publisher+summarizer trail as a successful run, and there's no log entry distinguishing "reviewer said 85" from "reviewer's output couldn't be parsed and we defaulted to publishing".

Expected/desired behavior

When the reviewer's response can't be parsed as ReviewResult, the graph should either:

  • Fail closed: route to the editor (most defensive — at least the content gets one more pass before publication), OR
  • Route to a dedicated recovery executor: re-prompt the reviewer, or escalate to a human-review node, OR
  • Halt the workflow: raise an explicit error so the operator sees the parse failure rather than discovering unreviewed content in production.

The current "publisher fires, editor doesn't" branch is the worst of the three: the unsafe target is always reachable on parse failure, the safe target is never reachable.

Note on response_format=ReviewResult

L96 sets default_options={"response_format": ReviewResult} on the reviewer. This reduces but does not eliminate the parse-failure surface:

  • Azure OpenAI content-filter trips return a refusal field, not the schema'd JSON.
  • Model-side refusals (the schema may be honored, but score/feedback may be a refusal string typed as int — pydantic will raise ValidationError).
  • Stream truncation under timeout / token-limit produces partial JSON.
  • Provider 5xx / network errors can be wrapped into an AgentExecutorResponse with an error text rather than re-raised.

So the except Exception: block is reached in practice, and the asymmetric fail-open is reachable in practice.

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