Repository navigation
Refuse a view binding in a spec with no input closure - #1552
Merged
Merged
Conversation
_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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Contributor
Author
|
Landed by squash as
|
5 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Takes up P3-3 of review 6040 on #1482 (#1482 (comment)).
What it changes
Composition refuses a spec that declares a view binding but carries no
axiom_input_closureresource, whether or not a node selects that binding.compose_transport_graphchecks the spec before it builds any node (_check_view_bindings_have_a_closureintransport/compose.py). The check covers every route to a view binding: a rules node, a solve or counterfactual that selects the binding through anengine_refparam, an extension's node, and a binding no node names._require_relation_inputsrefuses the same case for a rules node, sotransport_rules_noderefuses it when a caller runs that function withoutcompose_transport_graph. Before this PR, that guard returned before its closed-input check when the spec had no closure, so a view node that omitted a defaulted input composed anyway.A binding without views keeps composing with no closure.
docs/transport-declared-units.mdstates the rule.Invariant. If
compose_transport_graphaccepts a spec in which some binding declares views, that spec carriesaxiom_input_closure. A binding without views composes with or without one.What it refuses on main
Nothing tracked [executed]. No JSON or YAML under
packages/microcosm-build/srcdeclaresviews. NZ's packaged spec carries a closure, and its bindings declare no views. Belgium's packaged spec carries neither bindings nor a closure today.Reach
From main
3db65154to5da21904, none of the 45 kernels in the composed toy registry move, nor the threesimulate.*hashes bound to the Axiom adapter, and the US primary-QRF worker identity stays byte-identical [executed].Evidence [executed]
Each mutant ran against the whole of
test_transport_views.py(59 tests):test_compose_refuses_a_view_binding_in_a_spec_without_a_closure,test_a_rules_binding_row_refuses_a_view_binding_without_a_closurerelations-onlyandreal-relationcases oftest_a_binding_without_views_needs_no_closure_at_composereal-relationcase_node's call to the rules-node guard removedtest_a_rules_binding_row_refuses_a_view_binding_without_a_closurecompose_transport_graph's call to the spec check removedtest_composition_refuses_a_view_binding_in_a_spec_without_a_closuretest_composition_needs_no_closure_when_no_binding_declares_viewsformatincludedOne file per process, on
52fe3e0c's code and tests.5da21904changes onlytest_transport_views.py, which passes 59 of 59 at the head:test_transport_views.pytest_transport_registry.pytest_axiom_engine_ref_goldens.pytest_transport_declared_units.pytest_transport_evidence_kernels.pytest_nz_transport_declarations.pytest_nz_transport_toy_graph.pytest_transport_composed_graph.pyThe golden's skip needs a rulespec-nz export, and the two NZ skips need
NZ_CHRONICLE_ARTIFACT.This head merged with main
f023db0bpassestest_transport_views.py(59),test_transport_declared_units.py(18) andtest_nz_transport_toy_graph.py(40). Main's move from1d174fc7changed none of this PR's files, and the diff's patch-id reads the same on either base [executed]. Main then moved to70b9e691(#1217: a changelog fragment, a US test and a US release tool), again none of this PR's files; the patch-id holds there too, andtest_transport_views.pypasses 59 on that merge [executed].Outside this PR. With a closure present, a solve or counterfactual on a view binding still skips the closed-input check that rules nodes get. That gap predates this PR, and a separate follow-up records it.
Checklist
44050d0b: one P2 and four P3s.52fe3e0ctakes up the P2 and the two P3s on the tests. As the review advised, it leaves the presence-only check as it stands, and the gap above goes to the follow-up.5da21904.52fe3e0c(Opus 5.5) approved it, with no P1 or P2 and two optional P3s.5da21904takes up the first: the test helper's rename also rewrote the closure document'sformat, so the renamed document no longer parsed. This body takes up the second.5da21904with no findings.compose.pyhunk, given at52fe3e0c.5da21904leavescompose.pyunchanged, and NZ confirmed that its OK covers5da21904.f023db0b. An earlier run at this head concludedfailurewith four jobs never scheduled, althoughgh pr checksexited 0; closing and reopening this PR started the complete run.🤖 Generated with Claude Code