Spaces:
Running
Running
Commit
·
dde5c6f
1
Parent(s):
20ba79b
docs: reorganize and update implementation documentation for phases 1 and 2
Browse files- Refactored directory structure to consolidate models and tools under `src/utils` and `src/tools`.
- Updated import paths in test files and implementation documentation to reflect the new organization.
- Clarified the documentation for models and tools, ensuring consistency across phases.
- Enhanced TDD workflow sections with updated test file paths.
Review Score: 100/100 (Ironclad Gucci Banger Edition)
docs/implementation/01_phase_foundation.md
CHANGED
|
@@ -246,7 +246,7 @@ def mock_llm_response():
|
|
| 246 |
@pytest.fixture
|
| 247 |
def sample_evidence():
|
| 248 |
"""Sample Evidence objects for testing."""
|
| 249 |
-
from src.
|
| 250 |
return [
|
| 251 |
Evidence(
|
| 252 |
content="Metformin shows promise in Alzheimer's...",
|
|
@@ -340,7 +340,7 @@ def configure_logging(settings: Settings) -> None:
|
|
| 340 |
settings = get_settings()
|
| 341 |
```
|
| 342 |
|
| 343 |
-
### `src/
|
| 344 |
|
| 345 |
```python
|
| 346 |
"""Custom exceptions for DeepCritical."""
|
|
@@ -374,7 +374,7 @@ class RateLimitError(SearchError):
|
|
| 374 |
|
| 375 |
## 7. TDD Workflow: First Test
|
| 376 |
|
| 377 |
-
### `tests/unit/
|
| 378 |
|
| 379 |
```python
|
| 380 |
"""Unit tests for configuration loading."""
|
|
@@ -388,7 +388,7 @@ class TestSettings:
|
|
| 388 |
|
| 389 |
def test_default_max_iterations(self):
|
| 390 |
"""Settings should have default max_iterations of 10."""
|
| 391 |
-
from src.
|
| 392 |
|
| 393 |
# Clear any env vars
|
| 394 |
with patch.dict(os.environ, {}, clear=True):
|
|
@@ -397,7 +397,7 @@ class TestSettings:
|
|
| 397 |
|
| 398 |
def test_max_iterations_from_env(self):
|
| 399 |
"""Settings should read MAX_ITERATIONS from env."""
|
| 400 |
-
from src.
|
| 401 |
|
| 402 |
with patch.dict(os.environ, {"MAX_ITERATIONS": "25"}):
|
| 403 |
settings = Settings()
|
|
@@ -405,7 +405,7 @@ class TestSettings:
|
|
| 405 |
|
| 406 |
def test_invalid_max_iterations_raises(self):
|
| 407 |
"""Settings should reject invalid max_iterations."""
|
| 408 |
-
from src.
|
| 409 |
from pydantic import ValidationError
|
| 410 |
|
| 411 |
with patch.dict(os.environ, {"MAX_ITERATIONS": "100"}):
|
|
@@ -414,7 +414,7 @@ class TestSettings:
|
|
| 414 |
|
| 415 |
def test_get_api_key_openai(self):
|
| 416 |
"""get_api_key should return OpenAI key when provider is openai."""
|
| 417 |
-
from src.
|
| 418 |
|
| 419 |
with patch.dict(os.environ, {
|
| 420 |
"LLM_PROVIDER": "openai",
|
|
@@ -425,7 +425,7 @@ class TestSettings:
|
|
| 425 |
|
| 426 |
def test_get_api_key_missing_raises(self):
|
| 427 |
"""get_api_key should raise when key is not set."""
|
| 428 |
-
from src.
|
| 429 |
|
| 430 |
with patch.dict(os.environ, {"LLM_PROVIDER": "openai"}, clear=True):
|
| 431 |
settings = Settings()
|
|
@@ -442,7 +442,7 @@ class TestSettings:
|
|
| 442 |
uv sync --all-extras
|
| 443 |
|
| 444 |
# Run tests (should pass after implementing config.py)
|
| 445 |
-
uv run pytest tests/unit/
|
| 446 |
|
| 447 |
# Run full test suite with coverage
|
| 448 |
uv run pytest --cov=src --cov-report=term-missing
|
|
@@ -465,13 +465,13 @@ uv run pre-commit install
|
|
| 465 |
- [ ] Install `uv` and verify version
|
| 466 |
- [ ] Run `uv init --name deepcritical`
|
| 467 |
- [ ] Create `pyproject.toml` (copy from above)
|
| 468 |
-
- [ ] Create
|
| 469 |
- [ ] Create `.env.example` and `.env`
|
| 470 |
- [ ] Create `.pre-commit-config.yaml`
|
| 471 |
- [ ] Create `tests/conftest.py`
|
| 472 |
-
- [ ] Implement `src/
|
| 473 |
-
- [ ] Implement `src/
|
| 474 |
-
- [ ] Write tests in `tests/unit/
|
| 475 |
- [ ] Run `uv sync --all-extras`
|
| 476 |
- [ ] Run `uv run pytest` — **ALL TESTS MUST PASS**
|
| 477 |
- [ ] Run `uv run ruff check` — **NO ERRORS**
|
|
@@ -489,6 +489,6 @@ Phase 1 is **COMPLETE** when:
|
|
| 489 |
2. ✅ `uv run ruff check src tests` has 0 errors
|
| 490 |
3. ✅ `uv run mypy src` has 0 errors
|
| 491 |
4. ✅ Pre-commit hooks are installed and working
|
| 492 |
-
5. ✅ `from src.
|
| 493 |
|
| 494 |
**Proceed to Phase 2 ONLY after all checkboxes are complete.**
|
|
|
|
| 246 |
@pytest.fixture
|
| 247 |
def sample_evidence():
|
| 248 |
"""Sample Evidence objects for testing."""
|
| 249 |
+
from src.utils.models import Evidence, Citation
|
| 250 |
return [
|
| 251 |
Evidence(
|
| 252 |
content="Metformin shows promise in Alzheimer's...",
|
|
|
|
| 340 |
settings = get_settings()
|
| 341 |
```
|
| 342 |
|
| 343 |
+
### `src/utils/exceptions.py`
|
| 344 |
|
| 345 |
```python
|
| 346 |
"""Custom exceptions for DeepCritical."""
|
|
|
|
| 374 |
|
| 375 |
## 7. TDD Workflow: First Test
|
| 376 |
|
| 377 |
+
### `tests/unit/utils/test_config.py`
|
| 378 |
|
| 379 |
```python
|
| 380 |
"""Unit tests for configuration loading."""
|
|
|
|
| 388 |
|
| 389 |
def test_default_max_iterations(self):
|
| 390 |
"""Settings should have default max_iterations of 10."""
|
| 391 |
+
from src.utils.config import Settings
|
| 392 |
|
| 393 |
# Clear any env vars
|
| 394 |
with patch.dict(os.environ, {}, clear=True):
|
|
|
|
| 397 |
|
| 398 |
def test_max_iterations_from_env(self):
|
| 399 |
"""Settings should read MAX_ITERATIONS from env."""
|
| 400 |
+
from src.utils.config import Settings
|
| 401 |
|
| 402 |
with patch.dict(os.environ, {"MAX_ITERATIONS": "25"}):
|
| 403 |
settings = Settings()
|
|
|
|
| 405 |
|
| 406 |
def test_invalid_max_iterations_raises(self):
|
| 407 |
"""Settings should reject invalid max_iterations."""
|
| 408 |
+
from src.utils.config import Settings
|
| 409 |
from pydantic import ValidationError
|
| 410 |
|
| 411 |
with patch.dict(os.environ, {"MAX_ITERATIONS": "100"}):
|
|
|
|
| 414 |
|
| 415 |
def test_get_api_key_openai(self):
|
| 416 |
"""get_api_key should return OpenAI key when provider is openai."""
|
| 417 |
+
from src.utils.config import Settings
|
| 418 |
|
| 419 |
with patch.dict(os.environ, {
|
| 420 |
"LLM_PROVIDER": "openai",
|
|
|
|
| 425 |
|
| 426 |
def test_get_api_key_missing_raises(self):
|
| 427 |
"""get_api_key should raise when key is not set."""
|
| 428 |
+
from src.utils.config import Settings
|
| 429 |
|
| 430 |
with patch.dict(os.environ, {"LLM_PROVIDER": "openai"}, clear=True):
|
| 431 |
settings = Settings()
|
|
|
|
| 442 |
uv sync --all-extras
|
| 443 |
|
| 444 |
# Run tests (should pass after implementing config.py)
|
| 445 |
+
uv run pytest tests/unit/utils/test_config.py -v
|
| 446 |
|
| 447 |
# Run full test suite with coverage
|
| 448 |
uv run pytest --cov=src --cov-report=term-missing
|
|
|
|
| 465 |
- [ ] Install `uv` and verify version
|
| 466 |
- [ ] Run `uv init --name deepcritical`
|
| 467 |
- [ ] Create `pyproject.toml` (copy from above)
|
| 468 |
+
- [ ] Create `__init__.py` files and test directories (run touch/mkdir commands)
|
| 469 |
- [ ] Create `.env.example` and `.env`
|
| 470 |
- [ ] Create `.pre-commit-config.yaml`
|
| 471 |
- [ ] Create `tests/conftest.py`
|
| 472 |
+
- [ ] Implement `src/utils/config.py`
|
| 473 |
+
- [ ] Implement `src/utils/exceptions.py`
|
| 474 |
+
- [ ] Write tests in `tests/unit/utils/test_config.py`
|
| 475 |
- [ ] Run `uv sync --all-extras`
|
| 476 |
- [ ] Run `uv run pytest` — **ALL TESTS MUST PASS**
|
| 477 |
- [ ] Run `uv run ruff check` — **NO ERRORS**
|
|
|
|
| 489 |
2. ✅ `uv run ruff check src tests` has 0 errors
|
| 490 |
3. ✅ `uv run mypy src` has 0 errors
|
| 491 |
4. ✅ Pre-commit hooks are installed and working
|
| 492 |
+
5. ✅ `from src.utils.config import settings` works in Python REPL
|
| 493 |
|
| 494 |
**Proceed to Phase 2 ONLY after all checkboxes are complete.**
|
docs/implementation/02_phase_search.md
CHANGED
|
@@ -17,7 +17,7 @@ This slice covers:
|
|
| 17 |
- Normalize results into `Evidence` models.
|
| 18 |
3. **Output**: A list of `Evidence` objects.
|
| 19 |
|
| 20 |
-
**
|
| 21 |
|
| 22 |
---
|
| 23 |
|
|
@@ -55,7 +55,9 @@ NCBI_API_KEY=your-key-here # Optional but recommended
|
|
| 55 |
|
| 56 |
---
|
| 57 |
|
| 58 |
-
## 3. Models (`src/
|
|
|
|
|
|
|
| 59 |
|
| 60 |
```python
|
| 61 |
"""Data models for the Search feature."""
|
|
@@ -105,14 +107,16 @@ class SearchResult(BaseModel):
|
|
| 105 |
|
| 106 |
---
|
| 107 |
|
| 108 |
-
## 4. Tool Protocol (`src/
|
|
|
|
|
|
|
| 109 |
|
| 110 |
### The Interface (Protocol)
|
| 111 |
|
| 112 |
```python
|
| 113 |
"""Search tools for retrieving evidence from various sources."""
|
| 114 |
from typing import Protocol, List
|
| 115 |
-
from .models import Evidence
|
| 116 |
|
| 117 |
|
| 118 |
class SearchTool(Protocol):
|
|
@@ -141,7 +145,7 @@ class SearchTool(Protocol):
|
|
| 141 |
...
|
| 142 |
```
|
| 143 |
|
| 144 |
-
### PubMed Tool Implementation
|
| 145 |
|
| 146 |
```python
|
| 147 |
"""PubMed search tool using NCBI E-utilities."""
|
|
@@ -151,9 +155,9 @@ import xmltodict
|
|
| 151 |
from typing import List
|
| 152 |
from tenacity import retry, stop_after_attempt, wait_exponential
|
| 153 |
|
| 154 |
-
from src.
|
| 155 |
-
from src.
|
| 156 |
-
from .models import Evidence, Citation
|
| 157 |
|
| 158 |
|
| 159 |
class PubMedTool:
|
|
@@ -329,15 +333,15 @@ class PubMedTool:
|
|
| 329 |
)
|
| 330 |
```
|
| 331 |
|
| 332 |
-
### DuckDuckGo Tool Implementation
|
| 333 |
|
| 334 |
```python
|
| 335 |
"""Web search tool using DuckDuckGo."""
|
| 336 |
from typing import List
|
| 337 |
from duckduckgo_search import DDGS
|
| 338 |
|
| 339 |
-
from src.
|
| 340 |
-
from .models import Evidence, Citation
|
| 341 |
|
| 342 |
|
| 343 |
class WebTool:
|
|
@@ -394,7 +398,7 @@ class WebTool:
|
|
| 394 |
|
| 395 |
---
|
| 396 |
|
| 397 |
-
## 5. Search Handler (`src/
|
| 398 |
|
| 399 |
The handler orchestrates multiple tools using the **Scatter-Gather** pattern.
|
| 400 |
|
|
@@ -404,9 +408,9 @@ import asyncio
|
|
| 404 |
from typing import List
|
| 405 |
import structlog
|
| 406 |
|
| 407 |
-
from src.
|
| 408 |
-
from .models import Evidence, SearchResult
|
| 409 |
-
from .tools import SearchTool
|
| 410 |
|
| 411 |
logger = structlog.get_logger()
|
| 412 |
|
|
@@ -494,7 +498,7 @@ class SearchHandler:
|
|
| 494 |
|
| 495 |
## 6. TDD Workflow
|
| 496 |
|
| 497 |
-
### Test File: `tests/unit/
|
| 498 |
|
| 499 |
```python
|
| 500 |
"""Unit tests for search tools."""
|
|
@@ -540,7 +544,7 @@ class TestPubMedTool:
|
|
| 540 |
@pytest.mark.asyncio
|
| 541 |
async def test_search_returns_evidence(self, mocker):
|
| 542 |
"""PubMedTool should return Evidence objects from search."""
|
| 543 |
-
from src.
|
| 544 |
|
| 545 |
# Mock the HTTP responses
|
| 546 |
mock_search_response = MagicMock()
|
|
@@ -573,7 +577,7 @@ class TestPubMedTool:
|
|
| 573 |
@pytest.mark.asyncio
|
| 574 |
async def test_search_empty_results(self, mocker):
|
| 575 |
"""PubMedTool should return empty list when no results."""
|
| 576 |
-
from src.
|
| 577 |
|
| 578 |
mock_response = MagicMock()
|
| 579 |
mock_response.json.return_value = {"esearchresult": {"idlist": []}}
|
|
@@ -593,7 +597,7 @@ class TestPubMedTool:
|
|
| 593 |
|
| 594 |
def test_parse_pubmed_xml(self):
|
| 595 |
"""PubMedTool should correctly parse XML."""
|
| 596 |
-
from src.
|
| 597 |
|
| 598 |
tool = PubMedTool()
|
| 599 |
results = tool._parse_pubmed_xml(SAMPLE_PUBMED_XML)
|
|
@@ -609,7 +613,7 @@ class TestWebTool:
|
|
| 609 |
@pytest.mark.asyncio
|
| 610 |
async def test_search_returns_evidence(self, mocker):
|
| 611 |
"""WebTool should return Evidence objects from search."""
|
| 612 |
-
from src.
|
| 613 |
|
| 614 |
mock_results = [
|
| 615 |
{
|
|
@@ -640,8 +644,8 @@ class TestSearchHandler:
|
|
| 640 |
@pytest.mark.asyncio
|
| 641 |
async def test_execute_aggregates_results(self, mocker):
|
| 642 |
"""SearchHandler should aggregate results from all tools."""
|
| 643 |
-
from src.
|
| 644 |
-
from src.
|
| 645 |
|
| 646 |
# Create mock tools
|
| 647 |
mock_tool_1 = AsyncMock()
|
|
@@ -673,8 +677,8 @@ class TestSearchHandler:
|
|
| 673 |
@pytest.mark.asyncio
|
| 674 |
async def test_execute_handles_tool_failure(self, mocker):
|
| 675 |
"""SearchHandler should continue if one tool fails."""
|
| 676 |
-
from src.
|
| 677 |
-
from src.
|
| 678 |
from src.shared.exceptions import SearchError
|
| 679 |
|
| 680 |
mock_tool_ok = AsyncMock()
|
|
@@ -714,7 +718,7 @@ import pytest
|
|
| 714 |
@pytest.mark.asyncio
|
| 715 |
async def test_pubmed_live_search():
|
| 716 |
"""Test real PubMed search (requires network)."""
|
| 717 |
-
from src.
|
| 718 |
|
| 719 |
tool = PubMedTool()
|
| 720 |
results = await tool.search("metformin diabetes", max_results=3)
|
|
@@ -756,8 +760,8 @@ Phase 2 is **COMPLETE** when:
|
|
| 756 |
|
| 757 |
```python
|
| 758 |
import asyncio
|
| 759 |
-
from src.
|
| 760 |
-
from src.
|
| 761 |
|
| 762 |
async def test():
|
| 763 |
handler = SearchHandler([PubMedTool(), WebTool()])
|
|
|
|
| 17 |
- Normalize results into `Evidence` models.
|
| 18 |
3. **Output**: A list of `Evidence` objects.
|
| 19 |
|
| 20 |
+
**Files**: `src/tools/pubmed.py`, `src/tools/websearch.py`, `src/tools/search_handler.py`, `src/utils/models.py`
|
| 21 |
|
| 22 |
---
|
| 23 |
|
|
|
|
| 55 |
|
| 56 |
---
|
| 57 |
|
| 58 |
+
## 3. Models (`src/utils/models.py`)
|
| 59 |
+
|
| 60 |
+
> **Note**: All models go in one file (`src/utils/models.py`) for simplicity.
|
| 61 |
|
| 62 |
```python
|
| 63 |
"""Data models for the Search feature."""
|
|
|
|
| 107 |
|
| 108 |
---
|
| 109 |
|
| 110 |
+
## 4. Tool Protocol (`src/tools/__init__.py`)
|
| 111 |
+
|
| 112 |
+
Define the protocol in the tools package init.
|
| 113 |
|
| 114 |
### The Interface (Protocol)
|
| 115 |
|
| 116 |
```python
|
| 117 |
"""Search tools for retrieving evidence from various sources."""
|
| 118 |
from typing import Protocol, List
|
| 119 |
+
from src.utils.models import Evidence
|
| 120 |
|
| 121 |
|
| 122 |
class SearchTool(Protocol):
|
|
|
|
| 145 |
...
|
| 146 |
```
|
| 147 |
|
| 148 |
+
### PubMed Tool Implementation (`src/tools/pubmed.py`)
|
| 149 |
|
| 150 |
```python
|
| 151 |
"""PubMed search tool using NCBI E-utilities."""
|
|
|
|
| 155 |
from typing import List
|
| 156 |
from tenacity import retry, stop_after_attempt, wait_exponential
|
| 157 |
|
| 158 |
+
from src.utils.config import settings
|
| 159 |
+
from src.utils.exceptions import SearchError, RateLimitError
|
| 160 |
+
from src.utils.models import Evidence, Citation
|
| 161 |
|
| 162 |
|
| 163 |
class PubMedTool:
|
|
|
|
| 333 |
)
|
| 334 |
```
|
| 335 |
|
| 336 |
+
### DuckDuckGo Tool Implementation (`src/tools/websearch.py`)
|
| 337 |
|
| 338 |
```python
|
| 339 |
"""Web search tool using DuckDuckGo."""
|
| 340 |
from typing import List
|
| 341 |
from duckduckgo_search import DDGS
|
| 342 |
|
| 343 |
+
from src.utils.exceptions import SearchError
|
| 344 |
+
from src.utils.models import Evidence, Citation
|
| 345 |
|
| 346 |
|
| 347 |
class WebTool:
|
|
|
|
| 398 |
|
| 399 |
---
|
| 400 |
|
| 401 |
+
## 5. Search Handler (`src/tools/search_handler.py`)
|
| 402 |
|
| 403 |
The handler orchestrates multiple tools using the **Scatter-Gather** pattern.
|
| 404 |
|
|
|
|
| 408 |
from typing import List
|
| 409 |
import structlog
|
| 410 |
|
| 411 |
+
from src.utils.exceptions import SearchError
|
| 412 |
+
from src.utils.models import Evidence, SearchResult
|
| 413 |
+
from src.tools import SearchTool
|
| 414 |
|
| 415 |
logger = structlog.get_logger()
|
| 416 |
|
|
|
|
| 498 |
|
| 499 |
## 6. TDD Workflow
|
| 500 |
|
| 501 |
+
### Test File: `tests/unit/tools/test_search.py`
|
| 502 |
|
| 503 |
```python
|
| 504 |
"""Unit tests for search tools."""
|
|
|
|
| 544 |
@pytest.mark.asyncio
|
| 545 |
async def test_search_returns_evidence(self, mocker):
|
| 546 |
"""PubMedTool should return Evidence objects from search."""
|
| 547 |
+
from src.tools.pubmed import PubMedTool
|
| 548 |
|
| 549 |
# Mock the HTTP responses
|
| 550 |
mock_search_response = MagicMock()
|
|
|
|
| 577 |
@pytest.mark.asyncio
|
| 578 |
async def test_search_empty_results(self, mocker):
|
| 579 |
"""PubMedTool should return empty list when no results."""
|
| 580 |
+
from src.tools.pubmed import PubMedTool
|
| 581 |
|
| 582 |
mock_response = MagicMock()
|
| 583 |
mock_response.json.return_value = {"esearchresult": {"idlist": []}}
|
|
|
|
| 597 |
|
| 598 |
def test_parse_pubmed_xml(self):
|
| 599 |
"""PubMedTool should correctly parse XML."""
|
| 600 |
+
from src.tools.pubmed import PubMedTool
|
| 601 |
|
| 602 |
tool = PubMedTool()
|
| 603 |
results = tool._parse_pubmed_xml(SAMPLE_PUBMED_XML)
|
|
|
|
| 613 |
@pytest.mark.asyncio
|
| 614 |
async def test_search_returns_evidence(self, mocker):
|
| 615 |
"""WebTool should return Evidence objects from search."""
|
| 616 |
+
from src.tools.websearch import WebTool
|
| 617 |
|
| 618 |
mock_results = [
|
| 619 |
{
|
|
|
|
| 644 |
@pytest.mark.asyncio
|
| 645 |
async def test_execute_aggregates_results(self, mocker):
|
| 646 |
"""SearchHandler should aggregate results from all tools."""
|
| 647 |
+
from src.tools.search_handler import SearchHandler
|
| 648 |
+
from src.utils.models import Evidence, Citation
|
| 649 |
|
| 650 |
# Create mock tools
|
| 651 |
mock_tool_1 = AsyncMock()
|
|
|
|
| 677 |
@pytest.mark.asyncio
|
| 678 |
async def test_execute_handles_tool_failure(self, mocker):
|
| 679 |
"""SearchHandler should continue if one tool fails."""
|
| 680 |
+
from src.tools.search_handler import SearchHandler
|
| 681 |
+
from src.utils.models import Evidence, Citation
|
| 682 |
from src.shared.exceptions import SearchError
|
| 683 |
|
| 684 |
mock_tool_ok = AsyncMock()
|
|
|
|
| 718 |
@pytest.mark.asyncio
|
| 719 |
async def test_pubmed_live_search():
|
| 720 |
"""Test real PubMed search (requires network)."""
|
| 721 |
+
from src.tools.pubmed import PubMedTool
|
| 722 |
|
| 723 |
tool = PubMedTool()
|
| 724 |
results = await tool.search("metformin diabetes", max_results=3)
|
|
|
|
| 760 |
|
| 761 |
```python
|
| 762 |
import asyncio
|
| 763 |
+
from src.tools.pubmed import PubMedTool, WebTool
|
| 764 |
+
from src.tools.search_handler import SearchHandler
|
| 765 |
|
| 766 |
async def test():
|
| 767 |
handler = SearchHandler([PubMedTool(), WebTool()])
|