From 6c4cb4a59f14374696e607514a40224859f612a6 Mon Sep 17 00:00:00 2001 From: Rajeev Jain Date: Fri, 7 Aug 2026 04:05:16 -0500 Subject: [PATCH 1/2] Preserve domain scientific status through the analysis front door The front door assigned a fresh scientific_status dict when finalizing every analysis result, overwriting the block the domain layer had already attached via attach_scientific_status(). Codes the front door cannot rederive were silently dropped: SPHERE_RADIUS_UNAVAILABLE is raised by UXarray inside the computation, so only the domain sees it. The effect was that gradient, curl, and divergence results computed on a grid with no sphere_radius came back as an unqualified status 'complete' with physically_interpretable null and no warning codes, when the domain had correctly marked them 'warning' with physically_interpretable false. A result known to be uninterpretable was presented as a clean one -- the exact silent laundering the result contract exists to prevent. Merge the two blocks instead, taking the stricter judgment on every field so neither layer can upgrade the other's negative verdict, and keeping domain-only detail keys such as physical_scaling_applied. The override path still forces 'unverified' with interpretability false. --- src/uxarray_mcp/tools/frontdoor.py | 33 +++++++-- tests/test_frontdoor_semantics.py | 109 +++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 5 deletions(-) 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" From 8264f0c9dd1cff0d774894640802beb5712b6c29 Mon Sep 17 00:00:00 2001 From: Rajeev Jain Date: Fri, 7 Aug 2026 04:33:06 -0500 Subject: [PATCH 2/2] Derive the worker Python version in the provenance drift test The test hardcoded 3.12.13 as the worker's version. The CI image now ships exactly that patch release, so worker and submitter matched, the drift branch never fired, and submitter_python_version was never written. Derive a version that differs from the submitter's instead so the test exercises drift on any runner. --- tests/test_hpc_safety.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) 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