From 44050d0b7b2df115e6c54d1e18e3edfaa87e7a3d Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 9 Nov 2026 06:58:07 -0500 Subject: [PATCH 1/3] Refuse a view binding in a spec with no input closure _require_relation_inputs returned before its closed-input check when the spec had no axiom_input_closure, so a view node that omitted a defaulted input composed anyway. Composition now refuses the binding and names the missing resource. A binding without views keeps composing without a closure. Review 6040 of #1482 raised this as its P3-3. Co-Authored-By: Claude Opus 5.5 --- docs/transport-declared-units.md | 12 ++++----- .../src/microcosm/build/transport/compose.py | 16 ++++++++---- .../shared/test_transport_views.py | 26 +++++++++++++++++++ 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/docs/transport-declared-units.md b/docs/transport-declared-units.md index 1708affa70..f5d8cc2926 100644 --- a/docs/transport-declared-units.md +++ b/docs/transport-declared-units.md @@ -152,12 +152,12 @@ program, since both would read one person column. Views need A rules node slices each view's selector over the whole person table, as it does a relation's selector columns; composition and every rules kernel apply -the same check. When the spec carries an input closure, composition also -refuses a view node whose person slice omits an input that the closure encodes -(other than through a graph node) or defaults on the logical entity, and a -closure producer whose person slice omits a selector. The closure counts a -view's roots as live on its logical entity and checks them against the person -table; `unreached_roots` still excuses only roots on the binding's engine +the same check. Composition refuses a view binding in a spec with no input +closure. It also refuses a view node whose person slice omits an input that the +closure encodes (other than through a graph node) or defaults on the logical +entity, and a closure producer whose person slice omits a selector. The closure +counts a view's roots as live on its logical entity and checks them against the +person table; `unreached_roots` still excuses only roots on the binding's engine entity. The support roster classes a defaulted root on a view's logical entity as a person column. diff --git a/packages/microcosm-build/src/microcosm/build/transport/compose.py b/packages/microcosm-build/src/microcosm/build/transport/compose.py index d167acb75f..db66e350ca 100644 --- a/packages/microcosm-build/src/microcosm/build/transport/compose.py +++ b/packages/microcosm-build/src/microcosm/build/transport/compose.py @@ -4257,10 +4257,11 @@ def transport_rules_node( def _require_relation_inputs(spec, binding_id, inputs) -> None: """A rules node slices its binding's relation and view dependencies whole. - A view binding's node must also slice, on the person entity, every input - the reviewed closure encodes (other than through a graph node) or - defaults on the view's logical entity: the engine would otherwise run - the view without a column the closure says the frame carries. + A view binding needs the spec's input closure. Its node must also slice, + on the person entity, every input the reviewed closure encodes (other + than through a graph node) or defaults on the view's logical entity: the + engine would otherwise run the view without a column the closure says the + frame carries. """ document = transport_resource(spec, "axiom_rules_bindings") bindings = [ @@ -4290,8 +4291,13 @@ def _require_relation_inputs(spec, binding_id, inputs) -> None: f"Rules binding {binding_id!r} needs whole-table relation inputs {missing}." ) logical = binding_view_entities(bindings[0]) - if not logical or not _has_resource(spec, _CLOSURE_RESOURCE): + if not logical: return + if not _has_resource(spec, _CLOSURE_RESOURCE): + raise ValueError( + f"Rules binding {binding_id!r} runs views on {sorted(logical)}, but " + f"the spec has no {_CLOSURE_RESOURCE!r} resource to close their inputs." + ) path = bindings[0].get("rulespec_path") closed = sorted( { diff --git a/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py b/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py index 6a5ee4c593..39a1a58dbd 100644 --- a/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py +++ b/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py @@ -576,6 +576,32 @@ def test_compose_leaves_graph_node_inputs_to_the_node_that_writes_them( ) +def test_compose_refuses_a_view_binding_in_a_spec_without_a_closure( + tmp_path, monkeypatch +): + registry, _, document, _ = _prepared(tmp_path, monkeypatch) + spec = _spec(document) + del spec["resources"]["axiom_input_closure"] + with pytest.raises(ValueError, match="no 'axiom_input_closure' resource"): + compose_module.transport_rules_node( + spec, _config(registry), "child_benefit", _rules_node() + ) + + +@pytest.mark.parametrize( + "relations", ({}, {"relations": []}), ids=("plain", "relations-only") +) +def test_a_binding_without_views_needs_no_closure_at_compose( + tmp_path, monkeypatch, relations +): + _, _, document, _ = _prepared(tmp_path, monkeypatch) + document["bindings"][0].pop("views") + document["bindings"][0].update(relations) + spec = _spec(document) + del spec["resources"]["axiom_input_closure"] + compose_module._require_relation_inputs(spec, "child_benefit", _rules_node().inputs) + + def _closure_node(person_columns): return Node( "closure", From 52fe3e0cae06dacd5978f86adee0c52f11538d3d Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 9 Nov 2026 12:09:10 -0500 Subject: [PATCH 2/3] Refuse a view binding without a closure on every composition route The first review of this PR found that only rules nodes reached the new guard. A solve or counterfactual that selects a view binding through an engine_ref param, an extension's node, and a binding no node names all composed with no closure, while the docs promised a refusal for any view binding. compose_transport_graph now refuses a spec in which any binding declares views and no axiom_input_closure resource exists, before it builds a node. The node-level guard stays for transport_rules_node, which can run without compose_transport_graph. New tests compose the toy spec, with its closure renamed, for a view binding that no node names and for one a rules node names; they also pass a rules_binding row through _node, and cover a binding with a real relation and no views, which still composes without a closure. The docs now state the rule as the code applies it. Co-Authored-By: Claude Opus 5.5 --- docs/transport-declared-units.md | 17 ++-- .../src/microcosm/build/transport/compose.py | 18 ++++ .../shared/test_transport_views.py | 99 ++++++++++++++++++- 3 files changed, 122 insertions(+), 12 deletions(-) diff --git a/docs/transport-declared-units.md b/docs/transport-declared-units.md index f5d8cc2926..670a3b970e 100644 --- a/docs/transport-declared-units.md +++ b/docs/transport-declared-units.md @@ -152,14 +152,15 @@ program, since both would read one person column. Views need A rules node slices each view's selector over the whole person table, as it does a relation's selector columns; composition and every rules kernel apply -the same check. Composition refuses a view binding in a spec with no input -closure. It also refuses a view node whose person slice omits an input that the -closure encodes (other than through a graph node) or defaults on the logical -entity, and a closure producer whose person slice omits a selector. The closure -counts a view's roots as live on its logical entity and checks them against the -person table; `unreached_roots` still excuses only roots on the binding's engine -entity. The support roster classes a defaulted root on a view's logical entity -as a person column. +the same check. Composition refuses a spec that declares a view binding but +carries no input closure, whether or not a node selects that binding. It also +refuses a rules node on a view binding whose person slice omits an input that +the closure encodes (other than through a graph node) or defaults on the +logical entity, and a closure producer whose person slice omits a selector. The +closure counts a view's roots as live on its logical entity and checks them +against the person table; `unreached_roots` still excuses only roots on the +binding's engine entity. The support roster classes a defaulted root on a +view's logical entity as a person column. A binding without `views` keeps its semantics, engine reference and native execution call. The engine reference carries `views` only when the binding diff --git a/packages/microcosm-build/src/microcosm/build/transport/compose.py b/packages/microcosm-build/src/microcosm/build/transport/compose.py index db66e350ca..2d99de06eb 100644 --- a/packages/microcosm-build/src/microcosm/build/transport/compose.py +++ b/packages/microcosm-build/src/microcosm/build/transport/compose.py @@ -2201,6 +2201,23 @@ def _check_structural_columns(spec: Mapping[str, object]) -> None: ) +def _check_view_bindings_have_a_closure(spec: Mapping[str, object]) -> None: + """Every binding that declares views needs the spec's input closure.""" + + if _has_resource(spec, _CLOSURE_RESOURCE): + return + named = sorted( + binding + for binding, row in _binding_modules(spec).items() + if binding_view_entities(row) + ) + if named: + raise ValueError( + f"Rules bindings {named} declare views, but the spec has no " + f"{_CLOSURE_RESOURCE!r} resource to close their inputs." + ) + + def _check_structural_rows_written(spec: Mapping[str, object], graph: Graph) -> None: """Every declared ``structural_columns`` row names a column the graph writes. @@ -4573,6 +4590,7 @@ def compose_transport_graph( validate_transport_activation(spec) _check_unreached_requests(spec, config.engine_refs) _check_structural_columns(spec) + _check_view_bindings_have_a_closure(spec) from .entitlement import transport_extensions config = replace(config, extensions=transport_extensions(spec, config.extensions)) diff --git a/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py b/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py index 39a1a58dbd..37b2626c90 100644 --- a/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py +++ b/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py @@ -34,6 +34,7 @@ from microcosm.frame.concepts import CONCEPT_ENTITIES, CONCEPTS from microcosm.frame.input_closure import ClosureEntry, InputClosure, UndeterminedPolicy from microcosm.graph import Node, Owned, Slice +from test_support.microcosm_build.transport_composed import make_composed_fixture from test_support.microcosm_build.transport_population import UNIT_RULE PERIOD = "2026-27" @@ -588,18 +589,108 @@ def test_compose_refuses_a_view_binding_in_a_spec_without_a_closure( ) +def test_a_rules_binding_row_refuses_a_view_binding_without_a_closure( + tmp_path, monkeypatch +): + registry, _, document, _ = _prepared(tmp_path, monkeypatch) + spec = _spec(document) + del spec["resources"]["axiom_input_closure"] + row = { + "id": "child_benefit_node", + "kernel": "simulate.rules_by_ref@1", + "rules_binding": "child_benefit", + "population": "create", + "inputs": [ + {"entity": "person", "columns": ["is_child", "age", "child_income"]} + ], + "outputs": [ + {"entity": "person", "column": name, "dtype": "float64"} + for name in CHILD_OUTPUTS + ], + } + with pytest.raises(ValueError, match="no 'axiom_input_closure' resource"): + compose_module._node(spec, row, _config(registry), {}) + + +_RELATION = { + "predicate": "toy_members", + "key": "toy_members:0:1", + "current_slot": 0, + "related_slot": 1, + "arguments": ["Family", "Person"], + "root_entity": "family", + "related_entity": "person", + "membership_column": "person_family_id", + "related_inputs": {"toy_income": "income"}, + "cardinality": "many", + "period": PERIOD, + "role_column": "person_family_role", + "roles": ["head", "partner"], +} + + @pytest.mark.parametrize( - "relations", ({}, {"relations": []}), ids=("plain", "relations-only") + ("fields", "columns"), + ( + ({}, ("is_child", "age", "child_income")), + ({"relations": []}, ("is_child", "age", "child_income")), + ( + {"entity": "family", "engine_entity": "Family", "relations": [_RELATION]}, + ("income", "person_family_role"), + ), + ), + ids=("plain", "relations-only", "real-relation"), ) def test_a_binding_without_views_needs_no_closure_at_compose( - tmp_path, monkeypatch, relations + tmp_path, monkeypatch, fields, columns ): _, _, document, _ = _prepared(tmp_path, monkeypatch) document["bindings"][0].pop("views") - document["bindings"][0].update(relations) + document["bindings"][0].update(fields) spec = _spec(document) del spec["resources"]["axiom_input_closure"] - compose_module._require_relation_inputs(spec, "child_benefit", _rules_node().inputs) + compose_module._require_relation_inputs( + spec, "child_benefit", _rules_node(columns).inputs + ) + + +def _toy_spec_without_a_closure(tmp_path): + """The composed toy spec, with its closure renamed out of ``_has_resource``'s reach. + + A node selects the closure, so renaming keeps activation satisfied where + deleting the resource wouldn't. + """ + + fixture = make_composed_fixture(tmp_path / "fixture") + text = json.dumps(fixture.spec).replace("axiom_input_closure", "other_closure") + return json.loads(text), fixture.config + + +@pytest.mark.parametrize( + "binding", + ("family_housing", "main_benefit"), + ids=("no-node-names-it", "a-rules-node-names-it"), +) +def test_composition_refuses_a_view_binding_in_a_spec_without_a_closure( + tmp_path, binding +): + spec, config = _toy_spec_without_a_closure(tmp_path) + row = next( + row + for row in spec["resources"]["axiom_rules_bindings"]["bindings"] + if row["id"] == binding + ) + row["views"] = [_view()] + with pytest.raises( + ValueError, + match=rf"Rules bindings \['{binding}'\] declare views, but the spec has no", + ): + compose_module.compose_transport_graph(spec, config) + + +def test_composition_needs_no_closure_when_no_binding_declares_views(tmp_path): + spec, config = _toy_spec_without_a_closure(tmp_path) + assert compose_module.compose_transport_graph(spec, config).nodes def _closure_node(person_columns): From 5da219040dbd73a74e643dd91fde3bae0406762c Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 9 Nov 2026 13:16:32 -0500 Subject: [PATCH 3/3] Keep the closure document valid when a test helper renames its resource The delta review found that _toy_spec_without_a_closure renamed every occurrence of axiom_input_closure, the closure document's own format included, so InputClosure.from_dict refused the renamed document. The composition tests never parse it, but a later reuse would fail with a refusal that reads like a closure bug. The helper now replaces the quoted name only, which renames the resource key and the four selections that name it, and parses the renamed document, so a return to the bare replace fails all three tests that use the helper. Co-Authored-By: Claude Opus 5.5 --- .../tests/engine_free/shared/test_transport_views.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py b/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py index 37b2626c90..6b8114d816 100644 --- a/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py +++ b/packages/microcosm-build/tests/engine_free/shared/test_transport_views.py @@ -658,12 +658,16 @@ def _toy_spec_without_a_closure(tmp_path): """The composed toy spec, with its closure renamed out of ``_has_resource``'s reach. A node selects the closure, so renaming keeps activation satisfied where - deleting the resource wouldn't. + deleting the resource wouldn't. The rename matches the quoted name only: it + moves the resource key and the selections that name it, and leaves the + document's own ``format``, so the renamed document still parses. """ fixture = make_composed_fixture(tmp_path / "fixture") - text = json.dumps(fixture.spec).replace("axiom_input_closure", "other_closure") - return json.loads(text), fixture.config + text = json.dumps(fixture.spec).replace('"axiom_input_closure"', '"other_closure"') + spec = json.loads(text) + InputClosure.from_dict(spec["resources"]["other_closure"]) + return spec, fixture.config @pytest.mark.parametrize(