Fix screening coverage and blank inputs - #8
Conversation
ruthdelalucha
left a comment
There was a problem hiding this comment.
Review: Screening Coverage + Input Validation ✅
APPROVED - Wichtige Sprint-1-Verbesserungen!
Neue Features:
| Feature | Beschreibung |
|---|---|
| Person + Organization | Beide Schema-Typen werden jetzt abgefragt und gemerged |
| Deduplizierung | Matches nach ID+Dataset+Caption dedupliziert |
| Ranking | Nach Score sortiert, Top 10 Ergebnisse |
| Whitespace-Validierung | Leere/Whitespace-only Namen werden abgelehnt (422) |
| UI-Feedback | Klare Fehlermeldung statt grünem "Keine Treffer" |
Code-Qualität:
- ✅
Annotated[StringConstraints]für saubere Pydantic-Validierung - ✅
YENTE_QUERY_SCHEMASDictionary für Erweiterbarkeit - ✅ Deduplizierung mit
seen_matchesSet (O(1) Lookup) - ✅ Strip-Whitespace konsistent in API und UI
Tests:
- ✅ Organisation-Screening Test
- ✅ Whitespace-Query Test
- ✅ Blank-Name API-Validation Test
Produktionsreif:
Jetzt können auch Unternehmen (Sberbank, etc.) geprüft werden – nicht nur Personen.
LGTM 🚀
jsg-claude
left a comment
There was a problem hiding this comment.
PR #8 Review — Fix screening coverage and blank inputs
Kurz: ✅ Solider Feature-PR mit zwei unabhängigen Fixes. Approve & Merge.
Fix 1: Organization-Schema
Bisher wurde nur "Person" gegen yente abgefragt. Sberbank, Gazprom, Wagner Group — alles verpasst. Der PR fragt jetzt beide Schemas parallel ab und merged die Ergebnisse.
Die Deduplizierung über match_key = (id, datasets_tuple, caption) ist korrekt — ein Unternehmen das in mehreren Listen steht, taucht nur einmal auf. Sortierung nach Score absteigend, dann auf YENTE_MATCH_LIMIT gekappt. ✅
Ein Hinweis: YENTE_QUERY_SCHEMAS als Dict macht spätere Erweiterungen (Vessel, LegalEntity) trivial — gute Entscheidung.
Fix 2: Blank-Input-Validierung
ScreenName = Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] — das ist der richtige Ort für diese Regel. Pydantic validiert vor dem Handler, " " → 422, ohne dass Anwendungscode nötig ist. ✅
Im HTML-UI wird Whitespace-Input abgefangen bevor _query_yente aufgerufen wird — assert_not_called() im Test bestätigt das explizit. ✅
Tests
Drei neue Tests, alle sinnvoll:
| Test | Prüft |
|---|---|
| test_search_ui_whitespace_query_shows_validation_error | UI zeigt Fehlermeldung, kein yente-Call |
| test_query_yente_screens_persons_and_organizations | Beide Schemas im Request, Org-Treffer kommt durch |
| test_screen_blank_name_rejected | API gibt 422 für " " |
Einzige Anmerkung
normalized_query im HTML-UI und strip_whitespace=True in Pydantic machen dasselbe Trimmen an zwei verschiedenen Stellen. Das ist redundant aber nicht falsch — der API-Pfad und der UI-Pfad sind getrennt, beide trimmen unabhängig. Kein Handlungsbedarf.
536894c to
4bc50f8
Compare
Summary
PersonandOrganizationschemas and merge the ranked resultsTesting