diff --git a/src/uxarray_mcp/tools/frontdoor.py b/src/uxarray_mcp/tools/frontdoor.py index 47d02fe..69a1f58 100644 --- a/src/uxarray_mcp/tools/frontdoor.py +++ b/src/uxarray_mcp/tools/frontdoor.py @@ -165,11 +165,34 @@ def _finalize_analysis_result( result["result_type"] = RESULT_TYPE_COMPLETE result["preconditions"] = precondition_block - result["scientific_status"] = { - "status": status, - "physically_interpretable": physically_interpretable, - "warning_codes": warning_codes, - } + # Merge rather than overwrite. The domain layer already attached a status + # via attach_scientific_status() carrying codes the front door cannot + # rederive -- SPHERE_RADIUS_UNAVAILABLE is raised inside UXarray and only + # the computation sees it. Assigning a fresh dict here dropped those codes + # and reset `physically_interpretable` to null, so a result the domain had + # correctly marked uninterpretable was presented as an unqualified + # `complete`. That is exactly the silent laundering the contract exists to + # prevent, so the stricter of the two judgments wins on every field. + existing = result.get("scientific_status") or {} + merged_codes = list(existing.get("warning_codes", [])) + for code in warning_codes: + if code not in merged_codes: + merged_codes.append(code) + if existing.get("status") == "warning" and status == "complete": + status = "warning" + if physically_interpretable is None: + physically_interpretable = existing.get("physically_interpretable") + elif existing.get("physically_interpretable") is False: + physically_interpretable = False + merged_status = dict(existing) + merged_status.update( + { + "status": status, + "physically_interpretable": physically_interpretable, + "warning_codes": merged_codes, + } + ) + result["scientific_status"] = merged_status # #84: an explicit "we did not check" is cheap and stops a caller # implying more confidence than the computation supports. #90: when a # check does run, whether the verdict comes with it is a policy. diff --git a/tests/test_frontdoor_semantics.py b/tests/test_frontdoor_semantics.py index a6badc4..90ec526 100644 --- a/tests/test_frontdoor_semantics.py +++ b/tests/test_frontdoor_semantics.py @@ -254,3 +254,112 @@ def test_remap_partial_coverage_still_only_warns(): "physically_interpretable": False, "warning_codes": ["REMAP_COVERAGE_PARTIAL"], } + + +def test_front_door_preserves_domain_warning_codes(): + """A code only the computation can raise must survive the front door. + + ``SPHERE_RADIUS_UNAVAILABLE`` is emitted by UXarray inside ``curl``/ + ``gradient`` and attached by the domain layer. The front door used to + assign a fresh ``scientific_status`` dict, which dropped the code and + reset the verdict to an unqualified ``complete`` -- presenting a result + the domain had already marked uninterpretable as a clean one. + """ + result = _finalize_analysis_result( + "curl", + { + "component_warnings": [], + "component_evidence": { + "units_supported": True, + "component_identity_supported": True, + }, + "scale_by_radius": True, + "scientific_status": { + "status": "warning", + "physically_interpretable": False, + "warning_codes": ["SPHERE_RADIUS_UNAVAILABLE"], + "warnings": ["grid has no 'sphere_radius' attribute"], + "physical_scaling_requested": True, + "physical_scaling_applied": False, + }, + }, + ) + + status = result["scientific_status"] + assert "SPHERE_RADIUS_UNAVAILABLE" in status["warning_codes"] + assert status["physically_interpretable"] is False + assert status["status"] == "warning" + # Domain-only detail keys survive the merge rather than being dropped. + assert status["physical_scaling_applied"] is False + + +def test_front_door_status_merge_keeps_the_stricter_verdict(): + """Neither layer may upgrade the other's negative judgment. + + Both directions must hold. The front door may downgrade a domain verdict + it knows to be too generous (the override path), and it must not upgrade + a domain verdict that is stricter than its own -- the latter is the case + that regressed when the block was assigned rather than merged. + """ + # Front door is stricter: domain saw nothing wrong, but the caller + # overrode a failed precondition, so the result cannot claim to be clean. + overridden = _finalize_analysis_result( + "curl", + { + "component_warnings": [], + "component_evidence": {}, + "scale_by_radius": True, + "scientific_status": { + "status": "complete", + "physically_interpretable": True, + "warning_codes": [], + }, + }, + OVERRIDE_TOKEN, + ) + assert overridden["scientific_status"]["status"] == "unverified" + assert overridden["scientific_status"]["physically_interpretable"] is False + + # Domain is stricter: the front door's own checks all pass, so on its own + # it would report `complete`. It must not overwrite the domain's warning. + domain_stricter = _finalize_analysis_result( + "curl", + { + "component_warnings": [], + "component_evidence": { + "units_supported": True, + "component_identity_supported": True, + }, + "scale_by_radius": True, + "scientific_status": { + "status": "warning", + "physically_interpretable": False, + "warning_codes": ["SPHERE_RADIUS_UNAVAILABLE"], + }, + }, + ) + assert domain_stricter["scientific_status"]["status"] == "warning" + assert domain_stricter["scientific_status"]["physically_interpretable"] is False + assert domain_stricter["scientific_status"]["warning_codes"] == [ + "SPHERE_RADIUS_UNAVAILABLE" + ] + + # Codes from both layers accumulate instead of one replacing the other. + both = _finalize_analysis_result( + "curl", + { + "component_warnings": [], + "component_evidence": {}, + "scale_by_radius": True, + "scientific_status": { + "status": "warning", + "physically_interpretable": False, + "warning_codes": ["SPHERE_RADIUS_UNAVAILABLE"], + }, + }, + OVERRIDE_TOKEN, + ) + codes = both["scientific_status"]["warning_codes"] + assert "SPHERE_RADIUS_UNAVAILABLE" in codes + assert any(c.startswith("PRECONDITION_FAILED_") for c in codes) + assert both["scientific_status"]["status"] == "unverified" diff --git a/tests/test_hpc_safety.py b/tests/test_hpc_safety.py index 44ef569..ba0a230 100644 --- a/tests/test_hpc_safety.py +++ b/tests/test_hpc_safety.py @@ -529,12 +529,24 @@ def _fake_func(): @requires_globus def test_worker_runtime_overwrites_submitter_python_version(self): + import platform + + # Derive a worker version guaranteed to differ from the submitter's. + # Hardcoding a literal made this test depend on the CI image's patch + # release: when the runner shipped exactly that version the drift + # branch stopped firing and the submitter_* keys were never written, + # failing for a reason that had nothing to do with the code. + local_python = platform.python_version() + major, minor, _ = local_python.split(".") + worker_python = f"{major}.{minor}.{int(_) + 1}" + assert worker_python != local_python + result = self._run( { "n_face": 1, "_worker_runtime": { "hostname": "chr-0123", - "python_version": "3.12.13", + "python_version": worker_python, "uxarray_version": "2026.6.0", "slurm_job_id": "987654", }, @@ -543,11 +555,11 @@ def test_worker_runtime_overwrites_submitter_python_version(self): prov = result["_provenance"] # Top-level runtime fields describe the machine that did the work. - assert prov["python_version"] == "3.12.13" + assert prov["python_version"] == worker_python assert prov["remote_hostname"] == "chr-0123" assert prov["remote_slurm_job_id"] == "987654" # The submitter's own interpreter is preserved, not silently dropped. - assert prov["submitter_python_version"] != "3.12.13" + assert prov["submitter_python_version"] == local_python assert "_worker_runtime" not in result @requires_globus