Skip to content

Commit a08789f

Browse files
committed
fix(graph): address code review findings on evidence feature
- Add ADD_EVIDENCE audit action instead of reusing CREATE_CONCEPT - Add EvidenceResponse model and response_model on evidence endpoint - Accept creation_method parameter in add_evidence service method - Add tests for evidence endpoint (success, validation, 404, auth) - Add tests for evidence_text requirement on concept creation
1 parent 62f587d commit a08789f

5 files changed

Lines changed: 125 additions & 3 deletions

File tree

api/app/models/concepts.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,15 @@ class EvidenceCreate(BaseModel):
105105
)
106106

107107

108+
class EvidenceResponse(BaseModel):
109+
"""Response from adding evidence to a concept."""
110+
111+
concept_id: str
112+
instance_id: str
113+
source_id: str
114+
evidence_text: str
115+
116+
108117
class ConceptUpdate(BaseModel):
109118
"""Request to update an existing concept (partial update)."""
110119

api/app/routes/concepts.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
ConceptListResponse,
2020
CreationMethod,
2121
EvidenceCreate,
22+
EvidenceResponse,
2223
)
2324
from ..models.auth import UserInDB
2425
from ..dependencies.auth import require_permission
@@ -282,6 +283,7 @@ async def delete_concept(
282283

283284
@router.post(
284285
"/{concept_id}/evidence",
286+
response_model=EvidenceResponse,
285287
status_code=status.HTTP_201_CREATED,
286288
summary="Add evidence to a concept"
287289
)
@@ -311,7 +313,7 @@ async def add_evidence(
311313
log_audit_standalone(
312314
age_client=age_client,
313315
user_id=current_user.id if current_user else None,
314-
action=AuditAction.CREATE_CONCEPT.value,
316+
action=AuditAction.ADD_EVIDENCE.value,
315317
resource_type="evidence",
316318
resource_id=result["instance_id"],
317319
details={

api/app/services/audit_service.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ class AuditAction(str, Enum):
2626
UPDATE_EDGE = "update_edge"
2727
DELETE_EDGE = "delete_edge"
2828

29+
# Evidence operations
30+
ADD_EVIDENCE = "add_evidence"
31+
2932
# Batch operations
3033
BATCH_CREATE = "batch_create"
3134

api/app/services/concept_service.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,8 @@ async def add_evidence(
253253
self,
254254
concept_id: str,
255255
evidence_text: str,
256-
user_id: Optional[str] = None
256+
user_id: Optional[str] = None,
257+
creation_method: CreationMethod = CreationMethod.API
257258
) -> Dict[str, Any]:
258259
"""Add evidence to an existing concept.
259260
@@ -278,7 +279,7 @@ async def add_evidence(
278279
source_id = await self._create_synthetic_source(
279280
ontology=ontology,
280281
concept_label=label,
281-
creation_method=CreationMethod.API,
282+
creation_method=creation_method,
282283
user_id=user_id
283284
)
284285

tests/api/test_concepts.py

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,3 +448,110 @@ def test_get_concept_allowed_with_read_scope(
448448

449449
response = api_client.get("/concepts/c_123", headers=auth_headers_user)
450450
assert response.status_code == 200
451+
452+
453+
class TestAddEvidence:
454+
"""Tests for POST /concepts/{concept_id}/evidence endpoint."""
455+
456+
def test_add_evidence_success(self, api_client: TestClient, auth_headers_user):
457+
"""Add evidence to existing concept returns 201."""
458+
with patch('api.app.routes.concepts.get_age_client') as mock_age, \
459+
patch('api.app.routes.concepts.get_concept_service') as mock_service_factory:
460+
461+
mock_service = AsyncMock()
462+
mock_service.add_evidence.return_value = {
463+
"concept_id": "c_123",
464+
"instance_id": "i_abc",
465+
"source_id": "src_def",
466+
"evidence_text": "This concept is supported by empirical observation."
467+
}
468+
mock_service_factory.return_value = mock_service
469+
mock_age.return_value = MagicMock()
470+
471+
response = api_client.post(
472+
"/concepts/c_123/evidence",
473+
json={"evidence_text": "This concept is supported by empirical observation."},
474+
headers=auth_headers_user
475+
)
476+
477+
assert response.status_code == 201
478+
data = response.json()
479+
assert data["concept_id"] == "c_123"
480+
assert data["instance_id"] == "i_abc"
481+
assert data["evidence_text"] == "This concept is supported by empirical observation."
482+
483+
def test_add_evidence_too_short(self, api_client: TestClient, auth_headers_user):
484+
"""Evidence text shorter than 10 chars is rejected."""
485+
response = api_client.post(
486+
"/concepts/c_123/evidence",
487+
json={"evidence_text": "short"},
488+
headers=auth_headers_user
489+
)
490+
assert response.status_code == 422 # Pydantic validation
491+
492+
def test_add_evidence_concept_not_found(self, api_client: TestClient, auth_headers_user):
493+
"""Add evidence to nonexistent concept returns 404."""
494+
with patch('api.app.routes.concepts.get_age_client'), \
495+
patch('api.app.routes.concepts.get_concept_service') as mock_service_factory:
496+
497+
mock_service = AsyncMock()
498+
mock_service.add_evidence.side_effect = ValueError("Concept c_missing not found")
499+
mock_service_factory.return_value = mock_service
500+
501+
response = api_client.post(
502+
"/concepts/c_missing/evidence",
503+
json={"evidence_text": "Evidence for a concept that does not exist."},
504+
headers=auth_headers_user
505+
)
506+
507+
assert response.status_code == 404
508+
509+
def test_add_evidence_requires_auth(self, api_client: TestClient):
510+
"""Add evidence requires authentication."""
511+
response = api_client.post(
512+
"/concepts/c_123/evidence",
513+
json={"evidence_text": "Evidence without authentication."}
514+
)
515+
assert response.status_code == 401
516+
517+
518+
class TestEvidenceRequiredOnCreate:
519+
"""Tests for evidence_text requirement on manual concept creation."""
520+
521+
def test_create_concept_with_evidence(self, api_client: TestClient, auth_headers_user):
522+
"""Create concept with evidence_text succeeds."""
523+
with patch('api.app.routes.concepts.get_age_client') as mock_age, \
524+
patch('api.app.routes.concepts.get_concept_service') as mock_service_factory:
525+
526+
mock_service = AsyncMock()
527+
mock_service.create_concept.return_value = make_concept_response()
528+
mock_service_factory.return_value = mock_service
529+
mock_age.return_value = MagicMock()
530+
531+
response = api_client.post(
532+
"/concepts",
533+
json={
534+
"label": "Test Concept",
535+
"ontology": "test-ontology",
536+
"evidence_text": "This concept represents a well-established principle."
537+
},
538+
headers=auth_headers_user
539+
)
540+
541+
assert response.status_code == 201
542+
# Verify evidence_text was passed to the service
543+
call_args = mock_service.create_concept.call_args
544+
assert call_args.kwargs["request"].evidence_text == "This concept represents a well-established principle."
545+
546+
def test_create_concept_evidence_too_short(self, api_client: TestClient, auth_headers_user):
547+
"""Evidence text shorter than 10 chars is rejected by Pydantic."""
548+
response = api_client.post(
549+
"/concepts",
550+
json={
551+
"label": "Test Concept",
552+
"ontology": "test-ontology",
553+
"evidence_text": "short"
554+
},
555+
headers=auth_headers_user
556+
)
557+
assert response.status_code == 422

0 commit comments

Comments
 (0)