diff --git a/backend/app/api/routes/quality_checks.py b/backend/app/api/routes/quality_checks.py index 64908d42..7949137b 100644 --- a/backend/app/api/routes/quality_checks.py +++ b/backend/app/api/routes/quality_checks.py @@ -40,9 +40,22 @@ def list_quality_checks( def get_quality_check_evidence_geojson( project_id: UUID, quality_check_id: UUID, + limit: int = Query( + default=QualityEvidenceService.DEFAULT_EVIDENCE_LIMIT, + ge=0, + le=100_000, + description="Maximum evidence features to draw; 0 returns everything. Misses and false positives first.", + ), db: Session = Depends(get_db), ) -> dict: - return envelope(QualityEvidenceService.evidence_geojson(db, project_id=project_id, quality_check_id=quality_check_id)) + return envelope( + QualityEvidenceService.evidence_geojson( + db, + project_id=project_id, + quality_check_id=quality_check_id, + limit=limit, + ) + ) @router.get( diff --git a/backend/app/schemas/qa.py b/backend/app/schemas/qa.py index a59b4f61..cafd4bfb 100644 --- a/backend/app/schemas/qa.py +++ b/backend/app/schemas/qa.py @@ -89,7 +89,13 @@ class QualityEvidenceResponse(BaseModel): candidate_dataset_id: UUID | None = None reference_dataset_id: UUID analysis_run_id: UUID | None = None + # The overlay is capped so a regional check stays reviewable; the counts in + # the quality check itself are always complete. feature_count: int + total_feature_count: int | None = None + role_counts: dict[str, int] = Field(default_factory=dict) + truncated: bool = False + limit: int | None = None warnings: list[str] = Field(default_factory=list) geojson: GeoJsonFeatureCollection diff --git a/backend/app/services/quality_evidence_service.py b/backend/app/services/quality_evidence_service.py index 141ea0b8..38071f06 100644 --- a/backend/app/services/quality_evidence_service.py +++ b/backend/app/services/quality_evidence_service.py @@ -1,5 +1,6 @@ from __future__ import annotations +from dataclasses import dataclass from typing import Any from uuid import UUID @@ -12,84 +13,235 @@ from app.core.errors import AppError from app.models import Detection, DetectionReview, QualityCheck, Segmentation, VectorFeature -class QualityEvidenceService: - @staticmethod - def evidence_geojson(db: Session, *, project_id: UUID, quality_check_id: UUID) -> dict[str, Any]: - quality_check = db.get(QualityCheck, quality_check_id) - if not quality_check or quality_check.project_id != project_id: - raise AppError(code="QUALITY_CHECK_NOT_FOUND", message="Quality check not found", status_code=404) +@dataclass(frozen=True) +class EvidenceItem: + role: str + candidate_feature_id: str | None + reference_feature_id: str | None + evidence: dict[str, Any] - findings = quality_check.findings_json or {} - features: list[dict[str, Any]] = [] - warnings: list[str] = [] - candidate_ids, reference_ids = QualityEvidenceService._evidence_identifiers(findings) - candidate_index = QualityEvidenceService._candidate_feature_index(db, quality_check, candidate_ids) - reference_index = QualityEvidenceService._reference_feature_index(db, quality_check, reference_ids) + +@dataclass(frozen=True) +class EvidencePlan: + """Which evidence to draw, decided before any geometry is fetched.""" + + items: list[EvidenceItem] + candidate_ids: set[str] + reference_ids: set[str] + total_feature_count: int + role_counts: dict[str, int] + truncated: bool + + +class QualityEvidenceService: + DEFAULT_EVIDENCE_LIMIT = 5_000 + + @staticmethod + def plan_evidence(findings: dict[str, Any], *, limit: int) -> EvidencePlan: + """Decide what to draw before resolving a single geometry. + + Building every feature and then discarding most of them meant fetching + 130k geometries to draw 5k, with an ``IN`` clause holding every + identifier in the check. Planning first makes the work proportional to + what is returned. + + A match contributes two features and is kept or dropped as a pair; half + a match is not reviewable evidence. + """ + + matches: list[EvidenceItem] = [] + false_positives: list[EvidenceItem] = [] + false_negatives: list[EvidenceItem] = [] + role_counts: dict[str, int] = {} + + def count(role: str) -> None: + role_counts[role] = role_counts.get(role, 0) + 1 for evidence in QualityEvidenceService._evidence_items(findings.get("match_evidence")): candidate_id = QualityEvidenceService._string_value(evidence.get("candidate_feature_id")) reference_id = QualityEvidenceService._string_value(evidence.get("reference_feature_id")) - iou = evidence.get("iou") + if not candidate_id and not reference_id: + continue if candidate_id: - row = candidate_index.get(candidate_id) - if row is not None: - features.append( - QualityEvidenceService._row_to_feature( - row, - role="match_candidate", - quality_check=quality_check, - evidence=evidence, - ) - ) - else: - warnings.append(f"Candidate evidence feature not found: {candidate_id}") + count("match_candidate") if reference_id: - row = reference_index.get(reference_id) - if row is not None: - features.append( - QualityEvidenceService._row_to_feature( - row, - role="match_reference", - quality_check=quality_check, - evidence={"candidate_feature_id": candidate_id, "reference_feature_id": reference_id, "iou": iou}, - ) - ) - else: - warnings.append(f"Reference evidence feature not found: {reference_id}") + count("match_reference") + matches.append(EvidenceItem("match", candidate_id, reference_id, evidence)) for evidence in QualityEvidenceService._evidence_items(findings.get("false_positive_evidence")): candidate_id = QualityEvidenceService._string_value(evidence.get("candidate_feature_id")) if not candidate_id: continue - row = candidate_index.get(candidate_id) - if row is not None: - features.append( - QualityEvidenceService._row_to_feature( - row, - role="false_positive", - quality_check=quality_check, - evidence=evidence, - ) - ) - else: - warnings.append(f"False-positive evidence feature not found: {candidate_id}") + count("false_positive") + false_positives.append(EvidenceItem("false_positive", candidate_id, None, evidence)) for evidence in QualityEvidenceService._evidence_items(findings.get("false_negative_evidence")): reference_id = QualityEvidenceService._string_value(evidence.get("reference_feature_id")) if not reference_id: continue - row = reference_index.get(reference_id) - if row is not None: - features.append( - QualityEvidenceService._row_to_feature( - row, - role="false_negative", - quality_check=quality_check, - evidence=evidence, - ) - ) + count("false_negative") + false_negatives.append(EvidenceItem("false_negative", None, reference_id, evidence)) + + total_feature_count = sum(role_counts.values()) + + # Both error classes must be visible. Strict priority would mean a check + # with 50.000 misses and three false positives never shows one, so the + # budget is split between them in proportion to their populations, with + # at least one of each. Confirmations fill whatever is left. + selected: list[EvidenceItem] = [] + truncated = False + + if limit <= 0: + selected = false_negatives + false_positives + matches + else: + actionable = len(false_negatives) + len(false_positives) + if actionable <= limit: + miss_budget, false_positive_budget = len(false_negatives), len(false_positives) else: - warnings.append(f"False-negative evidence feature not found: {reference_id}") + miss_budget = round(limit * len(false_negatives) / actionable) + miss_budget = min(len(false_negatives), max(1 if false_negatives else 0, miss_budget)) + false_positive_budget = min(len(false_positives), limit - miss_budget) + if false_positives and false_positive_budget == 0: + false_positive_budget = 1 + miss_budget = min(miss_budget, limit - 1) + + selected = false_negatives[:miss_budget] + false_positives[:false_positive_budget] + truncated = miss_budget < len(false_negatives) or false_positive_budget < len(false_positives) + + drawn = len(selected) + for item in matches: + cost = sum(1 for value in (item.candidate_feature_id, item.reference_feature_id) if value) + if drawn + cost > limit: + truncated = True + continue + selected.append(item) + drawn += cost + + return EvidencePlan( + items=selected, + candidate_ids={item.candidate_feature_id for item in selected if item.candidate_feature_id}, + reference_ids={item.reference_feature_id for item in selected if item.reference_feature_id}, + total_feature_count=total_feature_count, + role_counts=role_counts, + truncated=truncated, + ) + + @staticmethod + def summarize_missing(*, candidate_ids: list[str], reference_ids: list[str]) -> list[str]: + """One statement instead of one warning per unresolvable identifier. + + A run whose detection rows were removed produced tens of thousands of + identically shaped strings, which buries every other warning. + """ + + if not candidate_ids and not reference_ids: + return [] + return [ + f"{len(candidate_ids)} kandidaat- en {len(reference_ids)} referentieobjecten uit dit bewijs zijn niet " + "meer als geometrie terug te vinden; ze staan wel in de bewaarde telling." + ] + + @staticmethod + def evidence_geojson( + db: Session, + *, + project_id: UUID, + quality_check_id: UUID, + limit: int | None = None, + ) -> dict[str, Any]: + quality_check = db.get(QualityCheck, quality_check_id) + if not quality_check or quality_check.project_id != project_id: + raise AppError(code="QUALITY_CHECK_NOT_FOUND", message="Quality check not found", status_code=404) + + findings = quality_check.findings_json or {} + warnings: list[str] = [] + missing_candidates: list[str] = [] + missing_references: list[str] = [] + + resolved_limit = ( + QualityEvidenceService.DEFAULT_EVIDENCE_LIMIT if limit is None else max(0, int(limit)) + ) + # Decide what to draw first, then fetch only those geometries. + plan = QualityEvidenceService.plan_evidence(findings, limit=resolved_limit) + candidate_index = QualityEvidenceService._candidate_feature_index(db, quality_check, plan.candidate_ids) + reference_index = QualityEvidenceService._reference_feature_index(db, quality_check, plan.reference_ids) + + features: list[dict[str, Any]] = [] + for item in plan.items: + if item.role == "match": + if item.candidate_feature_id: + row = candidate_index.get(item.candidate_feature_id) + if row is None: + missing_candidates.append(item.candidate_feature_id) + else: + features.append( + QualityEvidenceService._row_to_feature( + row, + role="match_candidate", + quality_check=quality_check, + evidence=item.evidence, + ) + ) + if item.reference_feature_id: + row = reference_index.get(item.reference_feature_id) + if row is None: + missing_references.append(item.reference_feature_id) + else: + features.append( + QualityEvidenceService._row_to_feature( + row, + role="match_reference", + quality_check=quality_check, + evidence={ + "candidate_feature_id": item.candidate_feature_id, + "reference_feature_id": item.reference_feature_id, + "iou": item.evidence.get("iou"), + }, + ) + ) + continue + + if item.role == "false_positive" and item.candidate_feature_id: + row = candidate_index.get(item.candidate_feature_id) + if row is None: + missing_candidates.append(item.candidate_feature_id) + else: + features.append( + QualityEvidenceService._row_to_feature( + row, + role="false_positive", + quality_check=quality_check, + evidence=item.evidence, + ) + ) + elif item.role == "false_negative" and item.reference_feature_id: + row = reference_index.get(item.reference_feature_id) + if row is None: + missing_references.append(item.reference_feature_id) + else: + features.append( + QualityEvidenceService._row_to_feature( + row, + role="false_negative", + quality_check=quality_check, + evidence=item.evidence, + ) + ) + + warnings.extend( + QualityEvidenceService.summarize_missing( + candidate_ids=missing_candidates, + reference_ids=missing_references, + ) + ) + role_counts = plan.role_counts + total_feature_count = plan.total_feature_count + truncated = plan.truncated + if truncated: + warnings.append( + f"Dit overzicht toont {len(features)} van {total_feature_count} bewijsobjecten: gemiste en " + "onterecht gevonden objecten eerst. De tellingen in de kwaliteitscontrole blijven volledig." + ) QualityEvidenceService._annotate_reviews(db, quality_check, features) @@ -100,6 +252,10 @@ class QualityEvidenceService: "reference_dataset_id": str(quality_check.reference_dataset_id), "analysis_run_id": str(quality_check.analysis_run_id) if quality_check.analysis_run_id else None, "feature_count": len(features), + "total_feature_count": total_feature_count, + "role_counts": role_counts, + "truncated": truncated, + "limit": resolved_limit, "warnings": warnings, "geojson": { "type": "FeatureCollection", diff --git a/backend/tests/test_quality_evidence_bounds.py b/backend/tests/test_quality_evidence_bounds.py new file mode 100644 index 00000000..1599a071 --- /dev/null +++ b/backend/tests/test_quality_evidence_bounds.py @@ -0,0 +1,127 @@ +"""Evidence review must stay usable on a regional run. + +evidence_geojson emitted one feature per false positive, one per false +negative and *two* per match, with no limit. A regional QA run of 40k +detections against 45k reference footprints produced well over a hundred +thousand features in a single response, plus one warning string per +unresolvable identifier. The endpoint the whole review workflow depends on +therefore stopped working exactly where review matters most. + +The budget goes to what a reviewer must act on — misses and false positives — +before confirmations, and the response says what it left out. +""" + +from __future__ import annotations + +from app.services.quality_evidence_service import QualityEvidenceService + + +def _features(role: str, count: int) -> list[dict]: + return [{"properties": {"evidence_role": role}, "id": f"{role}-{index}"} for index in range(count)] + + +def test_missing_identifiers_collapse_into_one_statement() -> None: + warnings = QualityEvidenceService.summarize_missing( + candidate_ids=["a", "b", "c"], + reference_ids=["r1"], + ) + + assert len(warnings) == 1 + assert "3" in warnings[0] + assert "1" in warnings[0] + + +def test_nothing_missing_produces_no_warning() -> None: + assert QualityEvidenceService.summarize_missing(candidate_ids=[], reference_ids=[]) == [] + + +def test_the_plan_is_capped_before_any_geometry_is_fetched() -> None: + """Resolving 130k geometries to draw 5k of them is work for nothing.""" + + findings = { + "match_evidence": [{"candidate_feature_id": f"c{i}", "reference_feature_id": f"r{i}"} for i in range(100)], + "false_positive_evidence": [{"candidate_feature_id": f"fp{i}"} for i in range(10)], + "false_negative_evidence": [{"reference_feature_id": f"fn{i}"} for i in range(10)], + } + + plan = QualityEvidenceService.plan_evidence(findings, limit=8) + + assert plan.truncated is True + assert len(plan.items) == 8 + # Both error classes are represented; confirmations do not get a share + # while errors are still waiting. + assert {item.role for item in plan.items} == {"false_negative", "false_positive"} + # Only the identifiers that will actually be drawn need resolving. + assert len(plan.candidate_ids) + len(plan.reference_ids) == 8 + assert plan.candidate_ids <= {f"fp{i}" for i in range(10)} + assert plan.reference_ids <= {f"fn{i}" for i in range(10)} + + +def test_a_rare_error_class_is_never_crowded_out() -> None: + """50.000 misses must not hide the three false positives.""" + + findings = { + "false_negative_evidence": [{"reference_feature_id": f"fn{i}"} for i in range(5_000)], + "false_positive_evidence": [{"candidate_feature_id": f"fp{i}"} for i in range(3)], + } + + plan = QualityEvidenceService.plan_evidence(findings, limit=100) + + roles = [item.role for item in plan.items] + assert roles.count("false_positive") >= 1 + assert roles.count("false_negative") >= 90 + assert len(plan.items) == 100 + + +def test_the_plan_reports_the_complete_population_not_the_capped_one() -> None: + findings = { + "match_evidence": [{"candidate_feature_id": f"c{i}", "reference_feature_id": f"r{i}"} for i in range(100)], + "false_negative_evidence": [{"reference_feature_id": "fn"}], + } + + plan = QualityEvidenceService.plan_evidence(findings, limit=2) + + assert plan.total_feature_count == 201 + assert plan.role_counts == {"match_candidate": 100, "match_reference": 100, "false_negative": 1} + + +def test_an_uncapped_plan_keeps_everything() -> None: + findings = {"false_negative_evidence": [{"reference_feature_id": f"fn{i}"} for i in range(30)]} + + plan = QualityEvidenceService.plan_evidence(findings, limit=0) + + assert plan.truncated is False + assert len(plan.items) == 30 + assert plan.reference_ids == {f"fn{i}" for i in range(30)} + + +def test_evidence_without_identifiers_is_skipped_not_planned() -> None: + findings = { + "false_positive_evidence": [{"candidate_feature_id": None}, {"candidate_feature_id": "fp"}], + "false_negative_evidence": [{}], + } + + plan = QualityEvidenceService.plan_evidence(findings, limit=0) + + assert [item.role for item in plan.items] == ["false_positive"] + assert plan.candidate_ids == {"fp"} + + + +def test_a_planned_match_keeps_its_candidate_and_reference_together() -> None: + """Half a match is not reviewable evidence.""" + + findings = { + "match_evidence": [ + {"candidate_feature_id": "c1", "reference_feature_id": "r1"}, + {"candidate_feature_id": "c2", "reference_feature_id": "r2"}, + ] + } + + plan = QualityEvidenceService.plan_evidence(findings, limit=3) + + assert plan.truncated is True + # An odd budget drops the second pair rather than showing one side of it. + assert len(plan.items) == 1 + assert plan.candidate_ids == {"c1"} + assert plan.reference_ids == {"r1"} diff --git a/backend/tests/test_sprint112_qa_evidence_overlay.py b/backend/tests/test_sprint112_qa_evidence_overlay.py index a8d4df8b..8361fa2c 100644 --- a/backend/tests/test_sprint112_qa_evidence_overlay.py +++ b/backend/tests/test_sprint112_qa_evidence_overlay.py @@ -95,8 +95,17 @@ def test_quality_check_evidence_geojson_resolves_persisted_vector_features() -> assert result["feature_count"] == 4 assert result["geojson"]["type"] == "FeatureCollection" roles = [feature["properties"]["qa_evidence_role"] for feature in result["geojson"]["features"]] - assert roles == ["match_candidate", "match_reference", "false_positive", "false_negative"] - match_candidate = result["geojson"]["features"][0] + # Every role resolves to persisted geometry. Errors are emitted before + # confirmations, because a capped overlay must spend its budget on the + # objects a reviewer has to act on. + assert sorted(roles) == ["false_negative", "false_positive", "match_candidate", "match_reference"] + assert roles.index("false_negative") < roles.index("match_candidate") + assert roles.index("false_positive") < roles.index("match_candidate") + match_candidate = next( + feature + for feature in result["geojson"]["features"] + if feature["properties"]["qa_evidence_role"] == "match_candidate" + ) assert match_candidate["properties"]["quality_check_id"] == str(quality_check_id) assert match_candidate["properties"]["candidate_feature_id"] == "candidate-match" assert match_candidate["properties"]["reference_feature_id"] == "reference-match" @@ -148,7 +157,11 @@ def test_quality_check_evidence_geojson_api_uses_canonical_envelope(monkeypatch) app.dependency_overrides.pop(get_db, None) assert response.status_code == 200 - assert response.json() == {"data": payload} + # The envelope wraps the service result; asserting the exact field list + # would break every time the response model gains a documented field. + body = response.json() + assert set(body) == {"data"} + assert body["data"].items() >= payload.items() def test_frontend_quality_evidence_overlay_contract_is_wired() -> None: diff --git a/docs/API_CONTRACTS.md b/docs/API_CONTRACTS.md index 4c3cc63d..f5f6e57d 100644 --- a/docs/API_CONTRACTS.md +++ b/docs/API_CONTRACTS.md @@ -1081,6 +1081,30 @@ Return vector stats (feature counts and geometry summary). Return vector bounds and feature count. +### GET `/api/v1/projects/{project_id}/quality-checks/{id}/evidence/geojson` + +Returns the reviewable geometry behind one quality check: the objects the model +missed, the ones it found without a reference, and the confirmed matches. + +The overlay is capped by `limit` (default 5.000, `0` returns everything). A +regional check emitted one feature per false positive, one per false negative +and *two* per match with no bound at all, so a run of 40k detections against +45k footprints produced well over a hundred thousand features in one response — +the endpoint the whole review workflow depends on stopped working exactly where +review matters most. + +What to draw is decided before any geometry is fetched, so the database work is +proportional to what is returned rather than to the size of the check. The +budget is split between misses and false positives in proportion to their +populations, with at least one of each: strict priority would mean a check with +50.000 misses and three false positives never showed one. Confirmations fill +what is left, and a match is kept or dropped as a candidate/reference pair +because half a match is not reviewable evidence. + +`total_feature_count` and `role_counts` describe the complete population, +`truncated` says whether the cap applied, and unresolvable identifiers are +summarised in one warning rather than one per identifier. + ### Export provenance Every exported GeoJSON carries a `geointel_provenance` foreign member on the diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 05385524..d5bea5a7 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -1667,7 +1667,16 @@ export interface QualityEvidenceGeoJsonResponse { candidate_dataset_id?: string | null reference_dataset_id: string analysis_run_id?: string | null + /** + * Features drawn. The overlay is capped so a regional check stays + * reviewable — misses and false positives first — while the counts in the + * quality check itself remain complete. + */ feature_count: number + total_feature_count?: number | null + role_counts?: Record + truncated?: boolean + limit?: number | null warnings: string[] geojson: GeoJSON.FeatureCollection }