From 00cdda9ba28a097619bed5af2f5c704b663c11e4 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Thu, 22 Oct 2026 09:09:14 -0500 Subject: [PATCH] fix(eval): refuse malformed reports without aborting --- backend/server/eval/suite.py | 51 +++++++++++++++++++ backend/server/eval/tests/test_suite.py | 36 +++++++++++-- backend/server/placement/scene_bridge.py | 8 +-- .../placement/tests/test_scene_bridge.py | 17 +++++++ 4 files changed, 104 insertions(+), 8 deletions(-) diff --git a/backend/server/eval/suite.py b/backend/server/eval/suite.py index 703d17ec..dfd80bcb 100644 --- a/backend/server/eval/suite.py +++ b/backend/server/eval/suite.py @@ -197,6 +197,25 @@ def _pipeline_rows(sites, inputs: Path, repo: Path, work: Path, engine_cfg: Crit return rows +def _report_shape_error(report: object) -> str | None: + """Return a stable refusal for a parsed report with unusable containers.""" + if not isinstance(report, dict): + return f"criteria report must be an object, got {type(report).__name__}" + profiles = report.get("profiles") + if profiles is not None and ( + not isinstance(profiles, list) or not all(isinstance(profile, str) for profile in profiles) + ): + return "criteria report profiles must be a list of strings" + decidability = report.get("decidability") + if decidability is not None and not isinstance(decidability, dict): + return "criteria report decidability must be an object" + if isinstance(decidability, dict): + for profile, value in decidability.items(): + if not isinstance(profile, str) or not isinstance(value, dict): + return "criteria report decidability entries must be objects keyed by profile" + return None + + def _report_diff(produced: dict, committed: dict) -> list[str]: diffs = [] for field in REPORT_FIELDS: @@ -242,6 +261,22 @@ def _criteria_rows(sites, inputs: Path, work: Path): ) ) continue + shape_error = _report_shape_error(produced) + if shape_error: + rows.append( + _row( + f"criteria/{site}/report", + "criteria", + f"{site}/gt-laser", + "criteria_report", + None, + "reproduces committed evidence", + "FAIL", + str(report_path), + shape_error, + ) + ) + continue shares = { p: (produced.get("decidability") or {}).get(p, {}).get("share") for p in produced.get("profiles") or [] } @@ -278,6 +313,22 @@ def _criteria_rows(sites, inputs: Path, work: Path): ) ) continue + committed_shape_error = _report_shape_error(committed) + if committed_shape_error: + rows.append( + _row( + f"criteria/{site}/report", + "criteria", + f"{site}/gt-laser", + "criteria_report", + shares, + "reproduces committed evidence", + "FAIL", + str(committed_path), + committed_shape_error, + ) + ) + continue diffs = _report_diff(produced, committed) rows.append( _row( diff --git a/backend/server/eval/tests/test_suite.py b/backend/server/eval/tests/test_suite.py index cec48bb7..122ddca4 100644 --- a/backend/server/eval/tests/test_suite.py +++ b/backend/server/eval/tests/test_suite.py @@ -11,7 +11,7 @@ from server.eval.adapters.criteria import CriteriaConfig, evaluate_scene from server.eval.coverage import MissingOccluderError -from server.eval.suite import _engine_config, _matches_reference, _reference, run_suite +from server.eval.suite import _criteria_rows, _engine_config, _matches_reference, _reference, run_suite from server.eval.tests.flat_wall import write as write_flat_wall ROOT = Path(__file__).resolve().parents[3] @@ -200,9 +200,7 @@ def test_committed_fixtures_suite(tmp_path: Path): # Checking per recon row keeps a partial restore (one cloud present, one # absent) a pass instead of a broken test. assert not other, json.dumps(other, indent=1, default=str) - expected_missing = sum( - 1 for d in recon_dirs if (d / "scene.json").is_file() and not (d / "points.ply").is_file() - ) + expected_missing = sum(1 for d in recon_dirs if (d / "scene.json").is_file() and not (d / "points.ply").is_file()) if expected_missing == 0: assert not produce_hint, json.dumps(produce_hint, indent=1, default=str) else: @@ -370,6 +368,36 @@ def test_a_broken_labels_file_fails_its_row_and_the_run_still_reports(tmp_path: assert (tmp_path / "out" / "SUMMARY.md").is_file() +def test_malformed_produced_criteria_report_is_a_fail_row(tmp_path: Path): + """A parsed JSON scalar or list must fail its row instead of crashing the suite.""" + inputs = tmp_path / "inputs" + work = tmp_path / "work" + report_path = work / "flat-wall" / "criteria" / "criteria_report.json" + report_path.parent.mkdir(parents=True) + report_path.write_text("null\n") + + rows = _criteria_rows(["flat-wall"], inputs, work) + + assert rows[0]["status"] == "FAIL" + assert "must be an object" in rows[0]["note"] + + +def test_malformed_committed_criteria_report_is_a_fail_row(tmp_path: Path): + inputs = tmp_path / "inputs" + work = tmp_path / "work" + report_path = work / "flat-wall" / "criteria" / "criteria_report.json" + report_path.parent.mkdir(parents=True) + report_path.write_text(json.dumps({"profiles": ["C"], "decidability": {"C": {"share": 1.0}}})) + committed = inputs / "criteria" / "flat-wall" / "gt-laser" + committed.mkdir(parents=True) + (committed / "criteria_report.json").write_text("[]\n") + + rows = _criteria_rows(["flat-wall"], inputs, work) + + assert rows[0]["status"] == "FAIL" + assert "must be an object" in rows[0]["note"] + + def test_a_broken_committed_criteria_report_fails_its_row(tmp_path: Path): """The committed criteria report is read per row: a malformed one is a FAIL row naming the unreadable file, not a crashed run.""" diff --git a/backend/server/placement/scene_bridge.py b/backend/server/placement/scene_bridge.py index 55feea7d..70acebe3 100644 --- a/backend/server/placement/scene_bridge.py +++ b/backend/server/placement/scene_bridge.py @@ -588,10 +588,10 @@ def end_out(end, side: str) -> dict[str, Any]: observed: list[dict[str, Any]] = [] - def band_out(band, pointer: str) -> dict[str, Any]: + def band_out(band, pointer: str, *, require_ground_depth: bool) -> dict[str, Any]: if band.wall_id is not None and band.wall_id != wall_id: _invalid(f"the band names unknown wall {band.wall_id!r}", f"{pointer}/wall_id") - if band.band == "ground" and band.out_m is None: + if require_ground_depth and band.band == "ground" and band.out_m is None: # scene.schema.json requires out_ft on every ground band ("the ground in # front of the wall out to out_ft"), while the source contract leaves # out_m optional — this band cannot be represented, so refuse rather @@ -610,14 +610,14 @@ def band_out(band, pointer: str) -> dict[str, Any]: return record for index, band in enumerate(scene.coverage.bands or []): - observed.append(band_out(band, f"/coverage/bands/{index}")) + observed.append(band_out(band, f"/coverage/bands/{index}", require_ground_depth=True)) for sensitivity_index, sensitivity in enumerate(scene.coverage.sensitivity or []): for band_index, band in enumerate(sensitivity.bands): # Sensitivity variants are validated but are not observations: the wire's # observed list is the ground-truth coverage the device saw, and a what-if # sensitivity band is not that. A sensitivity-only scene still converts to # ends only — no observation is invented. - band_out(band, f"/coverage/sensitivity/{sensitivity_index}/bands/{band_index}") + band_out(band, f"/coverage/sensitivity/{sensitivity_index}/bands/{band_index}", require_ground_depth=False) if len(observed) > _WIRE_MAX_OBSERVED_BANDS: _unsupported( diff --git a/backend/server/placement/tests/test_scene_bridge.py b/backend/server/placement/tests/test_scene_bridge.py index 28c5be4a..2a517acd 100644 --- a/backend/server/placement/tests/test_scene_bridge.py +++ b/backend/server/placement/tests/test_scene_bridge.py @@ -339,6 +339,23 @@ def test_meter_wall_rect_geometry_places_the_meter_at_its_span_centre_mid_height assert parsed.point_at(0.0)[0] == pytest.approx(0.0, abs=1e-6) +def test_ground_sensitivity_without_depth_is_discarded() -> None: + """Sensitivity bands are validated for span but never emitted to the wire.""" + document = base_scene() + document["coverage"]["sensitivity"] = [ + { + "surface": "ground", + "max_incidence_deg": 75, + "bands": [{"band": "ground", "s_m": [-2.0, 4.0]}], + } + ] + + converted = convert_scene(document) + + assert "observed" in converted["coverage"] + assert all(band["band"] != "ground" for band in converted["coverage"]["observed"]) + + def test_sensitivity_only_coverage_converts_to_ends_only() -> None: """A sensitivity band is a what-if variant, not a device observation: a scene whose coverage carries only sensitivity bands must convert to ends only, with