Let a compute step's parameters carry a capture or steering reference - #790
Merged
Conversation
RecipeComputeStep.parameters copied verbatim through the Recipe
aggregate's own event-payload wire, unlike its sibling RecipeActionStep
which already resolves BindingRef per-value. Left as-is, a CaptureRef or
SteeringRef written into a compute parameter would round-trip back out
of Postgres as a bare {"__capture__": ...} dict instead of the sentinel
object: silent corruption, and upstream of anything Operation BC does.
Also fixes a latent bug in validate_capture_refs: folding a compute
step's own capture_name declaration and its parameters' reference check
into one if/elif let a step that both declares and forward-references
skip its own consume check. Declare and consume now run independently,
consume first, mirroring validate_output_refs's existing ordering.
First slice of letting a compute step (e.g. a tomography reconstruction)
read a measured value or a steering brain's advice into its own
parameters, the way a setpoint already can.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s literal steps_to_wire's ComputeStep arm copied parameters verbatim, which would crash canonical_json_bytes the moment one carried a CaptureRef or SteeringRef (no default= to fall back on). Adds a per-value wire encoder mirroring the existing input_uris/OutputRef element encoder and the SetpointStep.value sentinel shape, so a ref-bearing parameter hashes deterministically instead of crashing register_procedure_from_recipe. expand() itself needed no functional change: a shallow dict copy already preserves a CaptureRef/SteeringRef object unchanged, since resolve_value (which only ever substitutes a BindingRef) was never called on parameters. Only the stale comment claiming parameters were purely literal needed correcting. Second slice of letting a compute step read a measured value or a steering brain's advice into its own parameters. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d too step_to_payload/_step_from_payload is a second, independent encode/decode site from _recipe_expansion._expand's determinism hash: it is what ResolvedStepsRecorded persists for resume replay. Left unfixed after the prior two slices, a pinned compute step carrying a CaptureRef/SteeringRef parameter would crash canonical_json_bytes at conduct time (this path runs on every conduct, not just recipe-driven ones) even though the hash serializer already handled it. Adds the same per-value encode/decode pair here, kept as an independent copy rather than importing _expand's, matching this file's own existing precedent for _input_uri_to_wire/_input_uri_from_wire (duplicated between the two modules already, not shared, despite both living under the single cora.operation tach boundary). Extends the architecture fitness test that exists specifically to catch a new field with no serializer arm, so the feature ships without a hole in its own safety net. Third slice; ComputeStep.parameters can now carry a ref through every encoding path. Runtime resolution against the per-conduct captures dict is the next slice. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…eters The final slice: _run_compute now resolves every CaptureRef/SteeringRef value in ComputeStep.parameters against the per-conduct captures dict, immediately after the existing input_uris/OutputRef resolve and before building the JobSpec. An unresolved capture or unseeded steering axis loud-fails with a recorded entry, no in-flight marker, nothing submitted, parity with the OutputRef case and with _run_setpoint's own CaptureRef/SteeringRef handling. Reuses _ERROR_UNRESOLVED_CAPTURE for both ref kinds rather than adding a new error class, matching _run_setpoint's own precedent (it already uses this label for its SteeringRef arm, not a separate one). Provenance rides a new parameter_refs payload key, emitted only when a parameter actually carries a ref, so a literal-only step's recorded entries stay byte-identical to before this feature. Also fixes the pre-existing OutputRef-unresolved failure body, which copied parameters via a raw dict() even though parameters may still hold an unresolved ref at that point (that failure returns before the new parameter resolution runs); it now goes through the same wire encoder. This closes the loop the first three slices set up: a tomography reconstruction (or any compute job) can now read a value an earlier step measured, or a coordinate a steering brain just advised, the same way a setpoint already could. Compute stops being read-only in the conduct loop. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Operation docstring still enumerated the runtime Step union as Setpoint | Action | Check in four places, two step kinds out of date since capture and compute landed. Replaced the enumerations with a pointer to STEP_KIND_VALUES and the fitness test that pins the arm set, rather than a fresh hand-list that goes stale the next time an arm lands. The module doc gains the split the five kinds already have but that no single page stated: capture and check observe, setpoint and action act, compute does both, and deciding is not a step kind at all because the brain runs between passes rather than inside one. That last part is the load-bearing half. Reading the kinds as though they ought to partition into observe, think and act compresses two altitudes into one, which is the reading the prose now heads off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The paragraph shipped a day ago said deciding is deliberately not a step kind. A CheckStep evaluates a criterion and gates the walk on the result, and BrainKind names RULE as a real brain, so a deterministic brain already runs inside the step list. The accurate statement is that deciding happens at two altitudes: the cheap deterministic kind fits in a step, and the loop brain does not, because its verdict can complete the whole Procedure and it reads across every pass rather than one. The reasons the loop brain sits outside the list were written down nowhere, so a reader had to reconstruct them from the DecidePort docstring and the conductor's decide pseudo-kind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||
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
1 participant
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.
What
A steering brain could already whisper a new value to a motor: a
SetpointStep's value accepts aCaptureRefor aSteeringRef, and the per-conduct captures bus resolves it at execute time. It could not whisper one to a reconstruction. AComputeStep'sparameterstook literals only, so an autonomous loop could move the stage but never retune the job it fed.Nothing new was needed to close that. The captures bus already carries both kinds of reference and the seed-the-captures keystone already deposits a brain's advice into it. The value simply had to survive every place a step is written down and read back.
The five places a reference has to survive
recipe/aggregates/recipe/body.py{"__capture__": "x"}dict: silent corruption, no error.operation/_recipe_expansion/_expand.pycanonical_json_bytestakes nodefault=, so an unserialized ref object crashes the hash rather than quietly differing.conductor.py(pinned-step payload)ResolvedStepsRecordedis what a resume replays against, so a ref that cannot round-trip here breaks resume, not execution.conductor.py(_run_compute)SetpointStepreads. Loud failure on an unseeded name, matching the setpoint path.conductor.py(provenance)parameter_refswhen any parameter was a reference, so the record says what was asked for as well as what it resolved to.A latent bug fixed on the way
validate_capture_refsusedif/elifover the step kinds, so aRecipeComputeStepthat both declared acapture_nameand consumed a forward reference had its consume check swallowed by the declare branch. Declaring and consuming are now checked independently. This was reachable before this PR; the widened type just makes it easy to hit.Two documentation commits
The Operation module docstring enumerated the runtime
Stepunion as three arms in four places, having gone stale twice as arms landed. It now points atSTEP_KIND_VALUESand the fitness test that pins them, so it cannot go stale a third time.The module doc now also states where deciding happens, which was written down nowhere. Deciding sits at two altitudes: a
CheckStepevaluates a criterion and gates the walk, which is the deterministic rule brainBrainKind.RULEnames, while the brain behindDecidePortstays outside the step list because its verdict can complete the whole Procedure and it reads the full cross-pass history that no step sees.Verification
tests/architecture,tests/unit/operation,tests/unit/recipemake docs-buildstrict, clean🤖 Generated with Claude Code