VibecoderMcSwaggins commited on
Commit
3d0c731
Β·
1 Parent(s): c20d2e7

fix: address CodeRabbit review feedback

Browse files

- Fix research_agent param types: str | None (Gradio can pass None)
- Add structlog logging for errors instead of exposing raw exceptions
- Update SPEC-22: mark as IMPLEMENTED, remove hardcoded line numbers
- Widen grep scope to match "no gr.Progress in codebase" criterion

docs/specs/SPEC-22-PROGRESS-BAR-REMOVAL.md CHANGED
@@ -1,6 +1,6 @@
1
  # SPEC-22: Remove Unsupported gr.Progress from ChatInterface
2
 
3
- **Status:** READY FOR IMPLEMENTATION
4
  **Priority:** P3 (Technical debt cleanup)
5
  **Effort:** 15 minutes
6
  **PR Scope:** Single file fix
@@ -105,12 +105,12 @@ Refactoring from `ChatInterface` to `gr.Blocks` + `gr.Chatbot` just to support `
105
 
106
  ## Implementation Checklist
107
 
108
- - [ ] Open `src/app.py`
109
- - [ ] Remove `progress: gr.Progress = gr.Progress()` from `research_agent` signature
110
- - [ ] Remove all `progress(...)` calls (lines 192, 194, 204)
111
- - [ ] Add `show_progress="full"` to `gr.ChatInterface` constructor
112
- - [ ] Verify emoji status yields still present in orchestrator events
113
- - [ ] Run `make check`
114
  - [ ] Test locally: `uv run python src/app.py`
115
 
116
  ---
@@ -118,8 +118,8 @@ Refactoring from `ChatInterface` to `gr.Blocks` + `gr.Chatbot` just to support `
118
  ## Verification
119
 
120
  ```bash
121
- # Verify no gr.Progress usage
122
- grep -n "gr.Progress\|progress(" src/app.py
123
 
124
  # Should return empty (no matches)
125
  ```
 
1
  # SPEC-22: Remove Unsupported gr.Progress from ChatInterface
2
 
3
+ **Status:** IMPLEMENTED
4
  **Priority:** P3 (Technical debt cleanup)
5
  **Effort:** 15 minutes
6
  **PR Scope:** Single file fix
 
105
 
106
  ## Implementation Checklist
107
 
108
+ - [x] Open `src/app.py`
109
+ - [x] Remove `progress: gr.Progress = gr.Progress()` from `research_agent` signature
110
+ - [x] Remove all `progress(...)` calls from `research_agent`
111
+ - [x] Add `show_progress="full"` to `gr.ChatInterface` constructor
112
+ - [x] Verify emoji status yields still present in orchestrator events
113
+ - [x] Run `make check`
114
  - [ ] Test locally: `uv run python src/app.py`
115
 
116
  ---
 
118
  ## Verification
119
 
120
  ```bash
121
+ # Verify no gr.Progress usage in codebase
122
+ grep -rn "gr.Progress" src/
123
 
124
  # Should return empty (no matches)
125
  ```
src/app.py CHANGED
@@ -5,6 +5,7 @@ from collections.abc import AsyncGenerator
5
  from typing import Any, Literal
6
 
7
  import gradio as gr
 
8
 
9
  from src.config.domain import ResearchDomain
10
  from src.orchestrators import create_orchestrator
@@ -13,6 +14,8 @@ from src.utils.exceptions import ConfigurationError
13
  from src.utils.models import OrchestratorConfig
14
  from src.utils.service_loader import warmup_services
15
 
 
 
16
  OrchestratorMode = Literal["advanced", "hierarchical"] # Unified Architecture (SPEC-16)
17
 
18
 
@@ -126,9 +129,9 @@ def _validate_inputs(
126
  async def research_agent(
127
  message: str,
128
  history: list[dict[str, Any]],
129
- domain: str = "sexual_health",
130
- api_key: str = "",
131
- api_key_state: str = "",
132
  ) -> AsyncGenerator[str, None]:
133
  """
134
  Gradio chat function that runs the research agent.
@@ -139,9 +142,9 @@ async def research_agent(
139
  Args:
140
  message: User's research question
141
  history: Chat history (Gradio format)
142
- domain: Research domain
143
  api_key: Optional user-provided API key (BYOK - auto-detects provider)
144
- api_key_state: Persistent API key state (survives example clicks)
145
 
146
  Yields:
147
  Markdown-formatted responses for streaming
@@ -220,7 +223,8 @@ async def research_agent(
220
  yield "\n\n".join(response_parts)
221
 
222
  except Exception as e:
223
- yield f"❌ **Error**: {e!s}"
 
224
 
225
 
226
  def create_demo() -> tuple[gr.ChatInterface, gr.Accordion]:
 
5
  from typing import Any, Literal
6
 
7
  import gradio as gr
8
+ import structlog
9
 
10
  from src.config.domain import ResearchDomain
11
  from src.orchestrators import create_orchestrator
 
14
  from src.utils.models import OrchestratorConfig
15
  from src.utils.service_loader import warmup_services
16
 
17
+ logger = structlog.get_logger(__name__)
18
+
19
  OrchestratorMode = Literal["advanced", "hierarchical"] # Unified Architecture (SPEC-16)
20
 
21
 
 
129
  async def research_agent(
130
  message: str,
131
  history: list[dict[str, Any]],
132
+ domain: str | None = None,
133
+ api_key: str | None = None,
134
+ api_key_state: str | None = None,
135
  ) -> AsyncGenerator[str, None]:
136
  """
137
  Gradio chat function that runs the research agent.
 
142
  Args:
143
  message: User's research question
144
  history: Chat history (Gradio format)
145
+ domain: Research domain (None defaults to "sexual_health")
146
  api_key: Optional user-provided API key (BYOK - auto-detects provider)
147
+ api_key_state: Persistent API key state (survives example clicks, can be None)
148
 
149
  Yields:
150
  Markdown-formatted responses for streaming
 
223
  yield "\n\n".join(response_parts)
224
 
225
  except Exception as e:
226
+ logger.exception("research_agent_failed", error=str(e))
227
+ yield "❌ **Error**: Something went wrong. Please try again."
228
 
229
 
230
  def create_demo() -> tuple[gr.ChatInterface, gr.Accordion]: