fix: Address all CodeRabbit review nitpicks

- Extract magic number 45 to class constant _EST_SECONDS_PER_ROUND (advanced.py)
- Add

@pytest
.mark.unit to test_advanced_p2_dead_zones.py
- Remove debug print statements from test
- Add pytestmark = pytest.mark.unit to test_magentic_judge_termination.py
- Fix import ordering (stdlib before third-party)
- Add fallback p=0.15 for progress events without iteration (app.py)
- Refactor max_iters lookup with getattr chains to reduce statement count
- Fix SPEC_16 markdown: list spacing (1. vs 1. ) and code block languages

All 327 tests pass.

docs/specs/SPEC_16_UNIFIED_CHAT_CLIENT_ARCHITECTURE.md

@@ -12,15 +12,15 @@ Eliminate the Simple Mode / Advanced Mode parallel universe by implementing a pl
 
 ## Strategic Goals
 
-1.  **Namespace Neutrality**: Decouple the core orchestrator from the `OpenAI` namespace. The system should speak `ChatClient`, not `OpenAIChatClient`.
-2.  **Full-Stack Provider Chain**: Prioritize providers that offer both LLM and Embeddings (OpenAI, Gemini, HuggingFace+Local) to ensure a unified environment.
-3.  **Fragmentation Reduction**: Remove "LLM-only" providers (Anthropic) that force complex hybrid dependency chains (e.g., Anthropic LLM + OpenAI Embeddings).
+1. **Namespace Neutrality**: Decouple the core orchestrator from the `OpenAI` namespace. The system should speak `ChatClient`, not `OpenAIChatClient`.
+2. **Full-Stack Provider Chain**: Prioritize providers that offer both LLM and Embeddings (OpenAI, Gemini, HuggingFace+Local) to ensure a unified environment.
+3. **Fragmentation Reduction**: Remove "LLM-only" providers (Anthropic) that force complex hybrid dependency chains (e.g., Anthropic LLM + OpenAI Embeddings).
 
 ## Problem Statement
 
 ### Current Architecture: Two Parallel Universes
 
-```
+```text
 User Query
     │
     ├── Has API Key? ──Yes──→ Advanced Mode (488 lines)
@@ -42,7 +42,7 @@ User Query
 
 ### Architecture After Implementation
 
-```
+```text
 User Query
     │
     └──→ Advanced Mode (unified)
@@ -55,7 +55,7 @@ User Query
 
 ### New Files
 
-```
+```text
 src/
 ├── clients/
 │   ├── __init__.py
@@ -250,9 +250,9 @@ def has_gemini_key(self) -> bool:
 
 ## Why This is "Elegant"
 
-1.  **One System**: We stop maintaining two parallel universes.
-2.  **Dependency Injection**: The specific LLM provider is injected, not hardcoded.
-3.  **Full Stack Alignment**: We prioritize providers (OpenAI, Gemini) that own the whole vertical (LLM + Embeddings), reducing environment complexity.
+1. **One System**: We stop maintaining two parallel universes.
+2. **Dependency Injection**: The specific LLM provider is injected, not hardcoded.
+3. **Full-Stack Alignment**: We prioritize providers (OpenAI, Gemini) that own the whole vertical (LLM + Embeddings), reducing environment complexity.
 
 ---
 

src/app.py

@@ -245,20 +245,14 @@ async def research_agent(
             elif event.type == "thinking":
                 progress(0.1, desc="Multi-agent reasoning...")
             elif event.type == "progress":
-                # Try to calculate percentage based on max rounds/iterations
-                p = None
-                max_iters = 10  # default
-                if hasattr(orchestrator, "_max_rounds"):
-                    max_iters = orchestrator._max_rounds
-                elif hasattr(orchestrator, "config") and hasattr(
-                    orchestrator.config, "max_iterations"
-                ):
-                    max_iters = orchestrator.config.max_iterations
-
+                # Calculate progress percentage (fallback to 0.15 for events without iteration)
+                p = 0.15
+                max_iters = getattr(orchestrator, "_max_rounds", None) or getattr(
+                    getattr(orchestrator, "config", None), "max_iterations", 10
+                )
                 if event.iteration:
                     # Map 0..max to 0.2..0.9
                     p = 0.2 + (0.7 * (min(event.iteration, max_iters) / max_iters))
-
                 progress(p, desc=event.message)
 
             # BUG FIX: Handle streaming events separately to avoid token-by-token spam

src/orchestrators/advanced.py

@@ -64,6 +64,9 @@ class AdvancedOrchestrator(OrchestratorProtocol):
     - Configurable timeouts and round limits
     """
 
+    # Estimated seconds per coordination round (for progress UI)
+    _EST_SECONDS_PER_ROUND: int = 45
+
     def __init__(
         self,
         max_rounds: int | None = None,
@@ -168,7 +171,7 @@ The final output should be a structured research report."""
     def _get_progress_message(self, iteration: int) -> str:
         """Generate progress message with time estimation."""
         rounds_remaining = max(self._max_rounds - iteration, 0)
-        est_seconds = rounds_remaining * 45
+        est_seconds = rounds_remaining * self._EST_SECONDS_PER_ROUND
         if est_seconds >= 60:
             est_display = f"{est_seconds // 60}m {est_seconds % 60}s"
         else:

tests/unit/agents/test_magentic_judge_termination.py

@@ -2,8 +2,12 @@
 
 from unittest.mock import patch
 
+import pytest
+
 from src.agents.magentic_agents import create_judge_agent
 
+pytestmark = pytest.mark.unit
+
 
 def test_judge_agent_has_termination_instructions() -> None:
     """Judge agent must be created with explicit instructions for early termination."""

tests/unit/orchestrators/test_advanced_p2_dead_zones.py

@@ -6,6 +6,7 @@ from src.orchestrators.advanced import AdvancedOrchestrator
 
 
 @pytest.mark.asyncio
+@pytest.mark.unit
 async def test_advanced_initialization_events():
     """Verify granular progress events are emitted during initialization."""
     # Mock dependencies
@@ -45,10 +46,6 @@ async def test_advanced_initialization_events():
         messages = [e.message for e in events]
         types = [e.type for e in events]
 
-        print("\nEvents received:")
-        for t, m in zip(types, messages, strict=True):
-            print(f"- {t}: {m}")
-
         # Expected sequence:
         # 1. started
         # 2. progress (Loading embedding...)