Commit
·
c9c58c4
1
Parent(s):
fc78d2d
fix(tests): Address CodeRabbit review feedback
Browse files- Add
@pytest
.mark.unit markers to all test functions
- Implement test_reporter_ran_tracking_still_works test
- Replace `if False: yield` with cleaner _empty_async_generator helper
- Add _create_task_prompt mock to test_internal_messages_are_filtered
- Use ORCH_MSG_KIND_* constants instead of string literals in _process_event
src/orchestrators/advanced.py
CHANGED
|
@@ -539,7 +539,7 @@ The final output should be a structured research report."""
|
|
| 539 |
message = getattr(event, "message", "")
|
| 540 |
|
| 541 |
# FILTERING: Skip internal framework bookkeeping
|
| 542 |
-
if kind in (
|
| 543 |
return None
|
| 544 |
|
| 545 |
# TRANSFORMATION: Handle user_task BEFORE text extraction
|
|
|
|
| 539 |
message = getattr(event, "message", "")
|
| 540 |
|
| 541 |
# FILTERING: Skip internal framework bookkeeping
|
| 542 |
+
if kind in (ORCH_MSG_KIND_TASK_LEDGER, ORCH_MSG_KIND_INSTRUCTION):
|
| 543 |
return None
|
| 544 |
|
| 545 |
# TRANSFORMATION: Handle user_task BEFORE text extraction
|
tests/unit/test_orchestrator_noise.py
CHANGED
|
@@ -12,6 +12,13 @@ from agent_framework import (
|
|
| 12 |
from src.orchestrators.advanced import REPORTER_AGENT_ID, AdvancedOrchestrator
|
| 13 |
|
| 14 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 15 |
@pytest.mark.asyncio
|
| 16 |
async def test_executor_completed_event_is_silenced():
|
| 17 |
"""Verify ExecutorCompletedEvent produces NO UI events."""
|
|
@@ -31,11 +38,7 @@ async def test_executor_completed_event_is_silenced():
|
|
| 31 |
orchestrator._build_workflow = MagicMock(return_value=mock_workflow)
|
| 32 |
|
| 33 |
# Mock init services to avoid side effects
|
| 34 |
-
|
| 35 |
-
if False:
|
| 36 |
-
yield
|
| 37 |
-
|
| 38 |
-
orchestrator._init_workflow_events = mock_init_events
|
| 39 |
orchestrator._init_embedding_service = MagicMock(return_value=None)
|
| 40 |
orchestrator._create_task_prompt = MagicMock(return_value="task")
|
| 41 |
|
|
@@ -53,6 +56,7 @@ async def test_executor_completed_event_is_silenced():
|
|
| 53 |
assert "ManagerAgent" not in event.message
|
| 54 |
|
| 55 |
|
|
|
|
| 56 |
@pytest.mark.asyncio
|
| 57 |
async def test_internal_messages_are_filtered():
|
| 58 |
"""Verify internal task_ledger/instruction messages are filtered."""
|
|
@@ -89,12 +93,9 @@ async def test_internal_messages_are_filtered():
|
|
| 89 |
mock_workflow.run_stream = event_stream
|
| 90 |
orchestrator._build_workflow = MagicMock(return_value=mock_workflow)
|
| 91 |
|
| 92 |
-
|
| 93 |
-
if False:
|
| 94 |
-
yield
|
| 95 |
-
|
| 96 |
-
orchestrator._init_workflow_events = mock_init_events
|
| 97 |
orchestrator._init_embedding_service = MagicMock(return_value=None)
|
|
|
|
| 98 |
|
| 99 |
events = []
|
| 100 |
async for event in orchestrator.run("query"):
|
|
@@ -111,3 +112,36 @@ async def test_internal_messages_are_filtered():
|
|
| 111 |
assert not any('{"some": "json"}' in msg for msg in all_messages)
|
| 112 |
# The instruction text should be filtered
|
| 113 |
assert not any("Internal instruction to agent" in msg for msg in all_messages)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 12 |
from src.orchestrators.advanced import REPORTER_AGENT_ID, AdvancedOrchestrator
|
| 13 |
|
| 14 |
|
| 15 |
+
async def _empty_async_generator(query):
|
| 16 |
+
"""Empty async generator for mocking _init_workflow_events."""
|
| 17 |
+
return
|
| 18 |
+
yield # Makes this an async generator that yields nothing
|
| 19 |
+
|
| 20 |
+
|
| 21 |
+
@pytest.mark.unit
|
| 22 |
@pytest.mark.asyncio
|
| 23 |
async def test_executor_completed_event_is_silenced():
|
| 24 |
"""Verify ExecutorCompletedEvent produces NO UI events."""
|
|
|
|
| 38 |
orchestrator._build_workflow = MagicMock(return_value=mock_workflow)
|
| 39 |
|
| 40 |
# Mock init services to avoid side effects
|
| 41 |
+
orchestrator._init_workflow_events = _empty_async_generator
|
|
|
|
|
|
|
|
|
|
|
|
|
| 42 |
orchestrator._init_embedding_service = MagicMock(return_value=None)
|
| 43 |
orchestrator._create_task_prompt = MagicMock(return_value="task")
|
| 44 |
|
|
|
|
| 56 |
assert "ManagerAgent" not in event.message
|
| 57 |
|
| 58 |
|
| 59 |
+
@pytest.mark.unit
|
| 60 |
@pytest.mark.asyncio
|
| 61 |
async def test_internal_messages_are_filtered():
|
| 62 |
"""Verify internal task_ledger/instruction messages are filtered."""
|
|
|
|
| 93 |
mock_workflow.run_stream = event_stream
|
| 94 |
orchestrator._build_workflow = MagicMock(return_value=mock_workflow)
|
| 95 |
|
| 96 |
+
orchestrator._init_workflow_events = _empty_async_generator
|
|
|
|
|
|
|
|
|
|
|
|
|
| 97 |
orchestrator._init_embedding_service = MagicMock(return_value=None)
|
| 98 |
+
orchestrator._create_task_prompt = MagicMock(return_value="task")
|
| 99 |
|
| 100 |
events = []
|
| 101 |
async for event in orchestrator.run("query"):
|
|
|
|
| 112 |
assert not any('{"some": "json"}' in msg for msg in all_messages)
|
| 113 |
# The instruction text should be filtered
|
| 114 |
assert not any("Internal instruction to agent" in msg for msg in all_messages)
|
| 115 |
+
|
| 116 |
+
|
| 117 |
+
@pytest.mark.unit
|
| 118 |
+
@pytest.mark.asyncio
|
| 119 |
+
async def test_reporter_ran_tracking_still_works():
|
| 120 |
+
"""Verify internal state.reporter_ran is set correctly even though UI events are silenced."""
|
| 121 |
+
orchestrator = AdvancedOrchestrator()
|
| 122 |
+
mock_workflow = MagicMock()
|
| 123 |
+
|
| 124 |
+
async def event_stream(task):
|
| 125 |
+
# Reporter completion event - should set internal flag
|
| 126 |
+
yield ExecutorCompletedEvent(executor_id=REPORTER_AGENT_ID, data=None)
|
| 127 |
+
|
| 128 |
+
mock_workflow.run_stream = event_stream
|
| 129 |
+
orchestrator._build_workflow = MagicMock(return_value=mock_workflow)
|
| 130 |
+
|
| 131 |
+
orchestrator._init_workflow_events = _empty_async_generator
|
| 132 |
+
orchestrator._init_embedding_service = MagicMock(return_value=None)
|
| 133 |
+
orchestrator._create_task_prompt = MagicMock(return_value="task")
|
| 134 |
+
|
| 135 |
+
# Run the workflow
|
| 136 |
+
events = []
|
| 137 |
+
async for event in orchestrator.run("query"):
|
| 138 |
+
events.append(event)
|
| 139 |
+
|
| 140 |
+
# The key assertion: No "synthesis" fallback should have been triggered
|
| 141 |
+
# If reporter_ran was NOT set, we'd see a fallback synthesis event
|
| 142 |
+
fallback_events = [
|
| 143 |
+
e for e in events if e.type == "synthesis" or "fallback" in e.message.lower()
|
| 144 |
+
]
|
| 145 |
+
assert len(fallback_events) == 0, (
|
| 146 |
+
f"Fallback synthesis triggered - reporter_ran tracking broken: {fallback_events}"
|
| 147 |
+
)
|