Commit
·
fd7948d
1
Parent(s):
67bdc5a
chore: Address CodeRabbit review feedback
Browse files1. Add
@pytest
.mark.unit to test class for proper categorization
2. Handle user_task before text extraction (edge case fix)
3. Improve truncation tests to actually detect regression:
- Test sentence boundary truncation with proper position
- Test word boundary fallback separately
4. Remove unused imports
Co-authored-by: CodeRabbit AI <coderabbit@users.noreply.github.com>
src/orchestrators/advanced.py
CHANGED
|
@@ -377,11 +377,8 @@ The final output should be a structured research report."""
|
|
| 377 |
if event.kind in ("task_ledger", "instruction"):
|
| 378 |
return None
|
| 379 |
|
| 380 |
-
text
|
| 381 |
-
|
| 382 |
-
return None
|
| 383 |
-
|
| 384 |
-
# TRANSFORMATION: Make manager events user-friendly
|
| 385 |
if event.kind == "user_task":
|
| 386 |
return AgentEvent(
|
| 387 |
type="progress",
|
|
@@ -389,6 +386,11 @@ The final output should be a structured research report."""
|
|
| 389 |
iteration=iteration,
|
| 390 |
)
|
| 391 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 392 |
# Default fallback for other manager events
|
| 393 |
return AgentEvent(
|
| 394 |
type="judging",
|
|
|
|
| 377 |
if event.kind in ("task_ledger", "instruction"):
|
| 378 |
return None
|
| 379 |
|
| 380 |
+
# TRANSFORMATION: Handle user_task BEFORE text extraction
|
| 381 |
+
# (user_task uses static message, doesn't need text content)
|
|
|
|
|
|
|
|
|
|
| 382 |
if event.kind == "user_task":
|
| 383 |
return AgentEvent(
|
| 384 |
type="progress",
|
|
|
|
| 386 |
iteration=iteration,
|
| 387 |
)
|
| 388 |
|
| 389 |
+
# For other manager events, extract and validate text
|
| 390 |
+
text = self._extract_text(event.message)
|
| 391 |
+
if not text:
|
| 392 |
+
return None
|
| 393 |
+
|
| 394 |
# Default fallback for other manager events
|
| 395 |
return AgentEvent(
|
| 396 |
type="judging",
|
tests/unit/orchestrators/test_advanced_events.py
CHANGED
|
@@ -1,13 +1,12 @@
|
|
| 1 |
"""Test for AdvancedOrchestrator event processing (P1 Bug)."""
|
| 2 |
|
| 3 |
-
from unittest.mock import MagicMock
|
| 4 |
-
|
| 5 |
import pytest
|
| 6 |
-
from agent_framework import
|
| 7 |
|
| 8 |
from src.orchestrators.advanced import AdvancedOrchestrator
|
| 9 |
|
| 10 |
|
|
|
|
| 11 |
class TestAdvancedEventProcessing:
|
| 12 |
"""Test event processing logic in AdvancedOrchestrator."""
|
| 13 |
|
|
@@ -77,21 +76,34 @@ class TestAdvancedEventProcessing:
|
|
| 77 |
|
| 78 |
def test_prevents_mid_sentence_truncation(self, orchestrator: AdvancedOrchestrator) -> None:
|
| 79 |
"""
|
| 80 |
-
Bug P1: Long messages should be smart-truncated
|
|
|
|
|
|
|
|
|
|
| 81 |
"""
|
| 82 |
-
#
|
| 83 |
-
long_text =
|
|
|
|
|
|
|
|
|
|
| 84 |
|
| 85 |
-
#
|
| 86 |
-
|
| 87 |
-
mock_message.content = long_text
|
| 88 |
-
mock_message.text = long_text
|
| 89 |
|
| 90 |
-
|
|
|
|
|
|
|
|
|
|
| 91 |
|
| 92 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 93 |
|
| 94 |
-
|
| 95 |
-
|
| 96 |
-
#
|
| 97 |
-
assert
|
|
|
|
|
|
| 1 |
"""Test for AdvancedOrchestrator event processing (P1 Bug)."""
|
| 2 |
|
|
|
|
|
|
|
| 3 |
import pytest
|
| 4 |
+
from agent_framework import MagenticOrchestratorMessageEvent
|
| 5 |
|
| 6 |
from src.orchestrators.advanced import AdvancedOrchestrator
|
| 7 |
|
| 8 |
|
| 9 |
+
@pytest.mark.unit
|
| 10 |
class TestAdvancedEventProcessing:
|
| 11 |
"""Test event processing logic in AdvancedOrchestrator."""
|
| 12 |
|
|
|
|
| 76 |
|
| 77 |
def test_prevents_mid_sentence_truncation(self, orchestrator: AdvancedOrchestrator) -> None:
|
| 78 |
"""
|
| 79 |
+
Bug P1: Long messages should be smart-truncated at sentence boundaries.
|
| 80 |
+
|
| 81 |
+
Tests _smart_truncate directly to ensure regression protection.
|
| 82 |
+
The function truncates at sentence boundary if period is after halfway point.
|
| 83 |
"""
|
| 84 |
+
# First sentence ends at position ~55, which is > 50 (100//2)
|
| 85 |
+
long_text = (
|
| 86 |
+
"This is a longer first sentence that ends past the midpoint. "
|
| 87 |
+
"Second sentence continues with more text that would be cut."
|
| 88 |
+
)
|
| 89 |
|
| 90 |
+
# Call the helper directly to test its behavior explicitly
|
| 91 |
+
truncated = orchestrator._smart_truncate(long_text, max_len=100)
|
|
|
|
|
|
|
| 92 |
|
| 93 |
+
# Should truncate at the end of the first sentence (period > max_len//2)
|
| 94 |
+
assert truncated.endswith("midpoint.")
|
| 95 |
+
assert "Second sentence" not in truncated
|
| 96 |
+
assert len(truncated) <= 100
|
| 97 |
|
| 98 |
+
def test_smart_truncate_word_boundary_fallback(
|
| 99 |
+
self, orchestrator: AdvancedOrchestrator
|
| 100 |
+
) -> None:
|
| 101 |
+
"""Test that truncation falls back to word boundary when no sentence end."""
|
| 102 |
+
# No sentence ending in the first 80 chars
|
| 103 |
+
long_text = "This is a very long text without any sentence ending in the limit"
|
| 104 |
|
| 105 |
+
truncated = orchestrator._smart_truncate(long_text, max_len=50)
|
| 106 |
+
|
| 107 |
+
# Should end with "..." and not cut mid-word
|
| 108 |
+
assert truncated.endswith("...")
|
| 109 |
+
assert len(truncated) <= 53 # 50 + "..."
|