Merge pull request #138 from The-Obstacle-Is-The-Way/claude/gradio-specs-alignment-014wuSArAiv3SWQMnfUoCSBM

docs/specs/SPEC-22-PROGRESS-BAR-REMOVAL.md

@@ -1,127 +1,151 @@
-# SPEC-22: Progress Bar Removal
+# SPEC-22: Remove Unsupported gr.Progress from ChatInterface
 
-**Status:** READY FOR IMPLEMENTATION
-**Priority:** P3 (Cosmetic UX fix)
+**Status:** IMPLEMENTED
+**Priority:** P3 (Technical debt cleanup)
 **Effort:** 15 minutes
 **PR Scope:** Single file fix
 
 ---
 
-## Problem Statement
+## Executive Summary
 
-The `gr.Progress()` bar conflicts with Gradio's `ChatInterface`, causing visual glitches:
-- Progress bar "floats" in the middle of chat output
-- Text overlaps with progress bar
-- Looks unprofessional
-
-**Root Cause:** `gr.Progress()` is designed for `gr.Interface`, not `ChatInterface`. It's a known Gradio limitation.
+We are using `gr.Progress()` with `gr.ChatInterface`, but **Gradio does not support this combination**. This is not a workaround - this is the correct fix to align with Gradio's architecture.
 
 ---
 
-## Current Code (BROKEN)
+## Technical Background
+
+### Gradio's Progress Mechanisms
+
+| Mechanism | Designed For | Works With ChatInterface |
+|-----------|--------------|-------------------------|
+| `gr.Progress()` | `gr.Interface` | ❌ NO - causes visual glitches |
+| `show_progress` param | `ChatInterface` | ✅ YES - built-in spinner/timer |
+| Streaming yields | `ChatInterface` | ✅ YES - native support |
+| `ChatMessage.metadata.status` | `ChatInterface` | ✅ YES - per-message indicators |
+
+### Why gr.Progress Fails with ChatInterface
+
+1. **GitHub Issue [#5967](https://github.com/gradio-app/gradio/issues/5967)**: "gr.Progress is not integrated with ChatInterface or Chatbots" - closed without resolution (Jan 2024)
+2. **Visual symptoms**: Progress bar floats in middle of chat output, overlaps text
+3. **Root cause**: `gr.Progress` injects UI into the output component area, but `ChatInterface` manages its own output rendering
+
+### What We Already Have (Working)
+
+Our `research_agent` function already yields semantic status messages:
 
 ```python
-# src/app.py - research_agent function
-async def research_agent(
-    message: str,
-    history: list[dict[str, Any]],
-    domain: str = "sexual_health",
-    api_key: str = "",
-    api_key_state: str = "",
-    progress: gr.Progress = gr.Progress(),  # ← PROBLEM: Causes visual glitches
-) -> AsyncGenerator[str, None]:
-    ...
-    if event.type == "started":
-        progress(0, desc="Starting research...")  # ← These cause overlap
-    elif event.type == "progress":
-        progress(p, desc=event.message)
+yield "🧠 **Backend**: Paid API (OpenAI) | **Domain**: Sexual Health"
+yield "⏳ **Processing...** Searching PubMed, ClinicalTrials.gov..."
+# During orchestration:
+yield "⏱️ **PROGRESS**: Round 1/5 (~3m 0s remaining)"
+yield "🔬 **Step 2: SearchAgent** - Searching for evidence..."
+yield "✅ **COMPLETE**: Research finished"
 ```
 
+These work perfectly with ChatInterface streaming.
+
 ---
 
-## Required Fix
+## The Fix
 
-Remove `gr.Progress()` entirely. We already have emoji status messages in chat output.
+### What to Remove
 
 ```python
-# src/app.py - Fixed version
-async def research_agent(
-    message: str,
-    history: list[dict[str, Any]],
-    domain: str = "sexual_health",
-    api_key: str = "",
-    api_key_state: str = "",
-    # REMOVED: progress: gr.Progress = gr.Progress(),
-) -> AsyncGenerator[str, None]:
-    ...
-    # REMOVED: All progress(...) calls
+# src/app.py - REMOVE from research_agent signature:
+progress: gr.Progress = gr.Progress(),  # noqa: B008
 
-    # KEEP: Emoji status messages are already being yielded
-    # These work great with ChatInterface:
-    # yield "⏱️ **PROGRESS**: Round 1/5 (~3m 0s remaining)"
+# src/app.py - REMOVE all progress() calls:
+progress(0, desc="Starting research...")
+progress(0.1, desc="Multi-agent reasoning...")
+progress(p, desc=event.message)
 ```
 
----
+### What to Add (Optional Enhancement)
 
-## Implementation Checklist
-
-- [ ] Open `src/app.py`
-- [ ] Remove `progress: gr.Progress = gr.Progress()` from `research_agent` signature
-- [ ] Remove all `progress(...)` calls inside `research_agent`
-- [ ] Verify emoji status yields are still present (they should be)
-- [ ] Run `uv run python -c "from src.app import create_demo; print('OK')"`
-- [ ] Run `make check`
-- [ ] Test locally: `uv run python src/app.py` and verify no floating progress bar
+```python
+# src/app.py - In create_demo(), add show_progress for built-in spinner:
+demo = gr.ChatInterface(
+    fn=research_agent,
+    show_progress="full",  # Shows spinner + runtime timer (Gradio native)
+    ...
+)
+```
 
 ---
 
-## What We Keep
+## Why This Is The Correct Approach (Not A Workaround)
 
-The emoji status messages in chat output:
+### ❌ What Would Be Over-Engineering
 
-```
-⏱️ **PROGRESS**: Round 1/5 (~3m 0s remaining)
-🔬 **Step 2: SearchAgent** - Searching for evidence...
-✅ **COMPLETE**: Research finished in 45 seconds
-```
+Refactoring from `ChatInterface` to `gr.Blocks` + `gr.Chatbot` just to support `gr.Progress`:
 
-These are yielded directly to chat and work perfectly with `ChatInterface`.
+| ChatInterface provides FREE | gr.Blocks would require manual |
+|---------------------------|-------------------------------|
+| MCP server support | Unknown if compatible |
+| Chat history state | Manual `gr.State` management |
+| Submit/Stop buttons | Manual button wiring |
+| Example handling | Manual click handlers |
+| Streaming support | Manual async iteration |
+| Accordion for inputs | Manual accordion component |
 
----
+**Effort**: Days of refactoring + testing
+**Benefit**: A progress bar (which we already have via emoji status)
+**Verdict**: Not justified
 
-## Acceptance Criteria
+### ✅ What Is Professional Engineering
 
-1. No `gr.Progress()` in `research_agent` function signature
-2. No `progress(...)` calls in `research_agent` function body
-3. Emoji status messages still appear in chat output
-4. No floating/overlapping progress bar in UI
-5. `make check` passes
+1. Use `ChatInterface` as designed (high-level, batteries-included)
+2. Remove unsupported feature (`gr.Progress`)
+3. Rely on supported mechanisms:
+   - Streaming status yields (already implemented)
+   - `show_progress="full"` (Gradio native)
 
 ---
 
-## Dependencies
+## Implementation Checklist
 
-None. This is a standalone cosmetic fix.
+- [x] Open `src/app.py`
+- [x] Remove `progress: gr.Progress = gr.Progress()` from `research_agent` signature
+- [x] Remove all `progress(...)` calls from `research_agent`
+- [x] Add `show_progress="full"` to `gr.ChatInterface` constructor
+- [x] Verify emoji status yields still present in orchestrator events
+- [x] Run `make check`
+- [ ] Test locally: `uv run python src/app.py`
 
 ---
 
-## Testing
+## Verification
 
 ```bash
-# Start local server
-uv run python src/app.py
-
-# In browser:
-# 1. Submit a research query
-# 2. Verify NO floating progress bar appears
-# 3. Verify emoji status messages DO appear in chat
-# 4. Verify chat messages don't have visual glitches
+# Verify no gr.Progress usage in codebase
+grep -rn "gr.Progress" src/
+
+# Should return empty (no matches)
 ```
 
+### Manual Test
+1. Start app: `uv run python src/app.py`
+2. Submit a research query
+3. Verify:
+   - ✅ Gradio spinner appears (top-right timer)
+   - ✅ Emoji status messages stream in chat
+   - ❌ No floating/overlapping progress bar
+
+---
+
+## Acceptance Criteria
+
+1. No `gr.Progress` in codebase
+2. `show_progress="full"` added to ChatInterface
+3. Emoji status messages continue working
+4. No visual glitches in UI
+5. `make check` passes
+
 ---
 
-## Notes
+## References
 
-- This is the recommended fix from Gradio's own documentation
-- `ChatInterface.show_progress="minimal"` (default) still shows a spinner, which is fine
-- If we need a real progress bar later, we'd need to refactor to `gr.Blocks` wrapper
+- [Gradio ChatInterface Docs](https://www.gradio.app/docs/gradio/chatinterface)
+- [Gradio Progress Bars Guide](https://www.gradio.app/guides/progress-bars)
+- [GitHub #5967: Progress bar for ChatInterface](https://github.com/gradio-app/gradio/issues/5967)

docs/specs/SPEC-23-GRADIO-MODERNIZATION.md

@@ -0,0 +1,187 @@
+# SPEC-23: Gradio 6.0 Modernization Audit
+
+**Status:** READY FOR IMPLEMENTATION
+**Priority:** P3 (Technical alignment)
+**Effort:** 30 minutes
+**Dependencies:** SPEC-22 (Progress Bar Removal)
+**Gradio Version:** 6.0.1 (December 2025)
+**Last Verified:** 2025-12-07
+
+---
+
+## Executive Summary
+
+Audit of `src/app.py` against Gradio 6.0.1 best practices. Identifies parameters we should add or update for full alignment with modern Gradio.
+
+---
+
+## Gradio Architecture Clarification
+
+### The Three High-Level Classes
+
+| Class | Purpose | Flexibility | Use Case |
+|-------|---------|-------------|----------|
+| `gr.Blocks` | Low-level building blocks | Most flexible | Custom layouts, multiple components |
+| `gr.Interface` | Input → Output wrapper | Medium | ML models, transformations |
+| `gr.ChatInterface` | Chat app wrapper | Opinionated | **Chatbots (what we use)** |
+
+### Inheritance Hierarchy
+
+```
+gr.Blocks (base)
+    ↑
+gr.ChatInterface (inherits from Blocks, adds chat-specific features)
+```
+
+**Key insight:** `ChatInterface` IS a `Blocks` context internally. We're not mixing different things - we're using the right abstraction for a chat app.
+
+### What ChatInterface Gives Us (For Free)
+
+- Chat history state management
+- Submit/Stop buttons
+- Streaming support
+- Example handling
+- `additional_inputs` accordion
+- MCP server support (`mcp_server=True`)
+
+### When Would We Need Pure `gr.Blocks`?
+
+Only if we needed:
+- Multiple independent chat windows
+- Complex multi-panel layouts
+- Non-chat components as primary UI
+
+**Verdict:** `ChatInterface` is the correct choice for DeepBoner.
+
+---
+
+## Current State vs Best Practices
+
+| Feature | Current | Best Practice | Action |
+|---------|---------|---------------|--------|
+| `type` param | Not set (legacy) | `type="messages"` | **ADD** - Uses OpenAI-style message format |
+| `fill_height` | Not set | `fill_height=True` | **ADD** - Chat fills vertical space |
+| `autoscroll` | Not set | `autoscroll=True` | **ADD** - Auto-scroll to latest message |
+| `show_progress` | Not set (minimal) | `show_progress="full"` | **ADD** - Per SPEC-22 |
+| `gr.Progress` | Used (broken) | Remove | **REMOVE** - Per SPEC-22 |
+
+---
+
+## Detailed Findings
+
+### 1. Message Format (`type="messages"`)
+
+**Current:** Not specified (uses legacy tuple format)
+**Recommended:** `type="messages"`
+
+The `type="messages"` format uses OpenAI-style dictionaries:
+```python
+{"role": "user" | "assistant", "content": str}
+```
+
+This is the modern standard and aligns with our LLM backends.
+
+**Note:** This may require updating how we handle `history` parameter.
+
+### 2. Fill Height (`fill_height=True`)
+
+**Current:** Not set
+**Recommended:** `fill_height=True`
+
+Makes the chat interface fill available vertical space. Better UX on larger screens.
+
+**Known Issue:** [GitHub #10407](https://github.com/gradio-app/gradio/issues/10407) - May conflict with `save_history=True`. We don't use `save_history`, so should be fine.
+
+### 3. Autoscroll (`autoscroll=True`)
+
+**Current:** Not set
+**Recommended:** `autoscroll=True`
+
+Ensures chat auto-scrolls to the latest message during streaming. Critical for long research outputs.
+
+---
+
+## Implementation Checklist
+
+### SPEC-22 Items (Do First)
+- [ ] Remove `progress: gr.Progress = gr.Progress()` from `research_agent` signature
+- [ ] Remove all `progress(...)` calls in `research_agent`
+
+### SPEC-23 Items
+- [ ] Add `show_progress="full"` to `gr.ChatInterface`
+- [ ] Add `fill_height=True` to `gr.ChatInterface`
+- [ ] Add `autoscroll=True` to `gr.ChatInterface`
+- [ ] Evaluate `type="messages"` migration (may require history format changes)
+
+---
+
+## Code Changes
+
+### Before (Current)
+```python
+demo = gr.ChatInterface(
+    fn=research_agent,
+    title="🍆 DeepBoner",
+    description=description,
+    examples=[...],
+    cache_examples=False,
+    run_examples_on_click=False,
+    additional_inputs_accordion=additional_inputs_accordion,
+    additional_inputs=[...],
+)
+```
+
+### After (Modernized)
+```python
+demo = gr.ChatInterface(
+    fn=research_agent,
+    title="🍆 DeepBoner",
+    description=description,
+    examples=[...],
+    cache_examples=False,
+    run_examples_on_click=False,
+    additional_inputs_accordion=additional_inputs_accordion,
+    additional_inputs=[...],
+    # SPEC-22: Use native progress instead of gr.Progress
+    show_progress="full",
+    # SPEC-23: Modern Gradio 6.0 settings
+    fill_height=True,
+    autoscroll=True,
+    # NOTE: type="messages" requires history format migration - evaluate separately
+)
+```
+
+---
+
+## Risk Assessment
+
+| Change | Risk | Mitigation |
+|--------|------|------------|
+| `show_progress="full"` | Low | Native Gradio feature |
+| `fill_height=True` | Low | May affect layout, test visually |
+| `autoscroll=True` | Low | Native feature, improves UX |
+| `type="messages"` | Medium | Requires history format changes - defer if needed |
+
+---
+
+## Verification
+
+```bash
+# After changes
+make check
+
+# Manual test
+uv run python src/app.py
+# Verify:
+# 1. Chat fills vertical space
+# 2. Auto-scrolls during streaming
+# 3. Spinner appears (not floating progress bar)
+```
+
+---
+
+## References
+
+- [Gradio ChatInterface Docs](https://www.gradio.app/docs/gradio/chatinterface)
+- [GitHub #10407: fill_height with save_history](https://github.com/gradio-app/gradio/issues/10407)
+- [GitHub #11109: Autoscroll issue](https://github.com/gradio-app/gradio/issues/11109)

src/app.py

@@ -5,6 +5,7 @@ from collections.abc import AsyncGenerator
 from typing import Any, Literal
 
 import gradio as gr
+import structlog
 
 from src.config.domain import ResearchDomain
 from src.orchestrators import create_orchestrator
@@ -13,6 +14,8 @@ from src.utils.exceptions import ConfigurationError
 from src.utils.models import OrchestratorConfig
 from src.utils.service_loader import warmup_services
 
+logger = structlog.get_logger(__name__)
+
 OrchestratorMode = Literal["advanced", "hierarchical"]  # Unified Architecture (SPEC-16)
 
 
@@ -126,10 +129,9 @@ def _validate_inputs(
 async def research_agent(
     message: str,
     history: list[dict[str, Any]],
-    domain: str = "sexual_health",
-    api_key: str = "",
-    api_key_state: str = "",
-    progress: gr.Progress = gr.Progress(),  # noqa: B008
+    domain: str | None = None,
+    api_key: str | None = None,
+    api_key_state: str | None = None,
 ) -> AsyncGenerator[str, None]:
     """
     Gradio chat function that runs the research agent.
@@ -140,10 +142,9 @@ async def research_agent(
     Args:
         message: User's research question
         history: Chat history (Gradio format)
-        domain: Research domain
+        domain: Research domain (None defaults to "sexual_health")
         api_key: Optional user-provided API key (BYOK - auto-detects provider)
-        api_key_state: Persistent API key state (survives example clicks)
-        progress: Gradio progress tracker
+        api_key_state: Persistent API key state (survives example clicks, can be None)
 
     Yields:
         Markdown-formatted responses for streaming
@@ -187,21 +188,8 @@ async def research_agent(
         )
 
         async for event in orchestrator.run(message):
-            # Update progress bar
-            if event.type == "started":
-                progress(0, desc="Starting research...")
-            elif event.type == "thinking":
-                progress(0.1, desc="Multi-agent reasoning...")
-            elif event.type == "progress":
-                # 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)
+            # SPEC-22: Progress bar removed - gr.Progress() causes visual glitches
+            # with ChatInterface. Status feedback via AgentEvent.to_markdown()
 
             # BUG FIX: Handle streaming events separately to avoid token-by-token spam
             if event.type == "streaming":
@@ -235,7 +223,8 @@ async def research_agent(
             yield "\n\n".join(response_parts)
 
     except Exception as e:
-        yield f"❌ **Error**: {e!s}"
+        logger.exception("research_agent_failed", error=str(e))
+        yield "❌ **Error**: Something went wrong. Please try again."
 
 
 def create_demo() -> tuple[gr.ChatInterface, gr.Accordion]:
@@ -272,6 +261,11 @@ def create_demo() -> tuple[gr.ChatInterface, gr.Accordion]:
         fn=research_agent,
         title="🍆 DeepBoner",
         description=description,
+        # SPEC-22: Use native progress instead of gr.Progress (which breaks ChatInterface)
+        show_progress="full",
+        # SPEC-23: Gradio 6.0 best practices
+        fill_height=True,
+        autoscroll=True,
         examples=[
             # SPEC-16: Mode is always "advanced" (unified architecture)
             # Examples now only need: [question, domain, api_key, api_key_state]