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
- 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
Assessment
This issue has not been assessed yet.