Azure-Samples / Azure-Samples/python-ai-agent-frameworks-demos

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

Open
#75 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Bicep
Stars
335
Forks
191
Avg merge
3d 13h
Merged PRs (30d)
1

Description

### Minimal steps to reproduce

[`examples/agentframework_workflow.py`](https://github.com/Azure-Samples/python-ai-agent-frameworks-demos/blob/2484d3786778a1767de0fa93c13b34439aceb1f6/examples/agentframework_workflow.py#L45-L65) defines two router conditions that have asymmetric fail-open behavior:

```python
# 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](https://github.com/Azure-Samples/python-ai-agent-frameworks-demos/blob/2484d3786778a1767de0fa93c13b34439aceb1f6/examples/agentframework_workflow.py#L151-L166)):

```python
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 ):

```python
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](https://github.com/Azure-Samples/python-ai-agent-frameworks-demos/blob/2484d3786778a1767de0fa93c13b34439aceb1f6/examples/agentframework_workflow.py#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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.