Skip to content

Commit 58308a3

Browse files
author
David Noble
committed
feat(13.0(n) Phase 4.5 B-class.3): Subgraph.Parameters() filters local FrameBindings
Closes the B-class triplet. Subgraph.Parameters() now subtracts variable names this subgraph binds locally via FrameBindings before returning the bubble-up surface, completing the one-level encapsulation rule the user settled in the spec session. Behavior change Before: Parameters() returned every VariableValue reference walked across immediate children's slots and the recursively-computed surface of each child Subgraph, deduplicated and sorted by Name. No consideration of whether this subgraph bound any of those names locally. After: same walk, then a delete pass that removes any seen[name] entry whose Name is a key in s.FrameBindings. The exposed surface is what a parent caller must supply — bindings already covered locally by this subgraph's own kwargs are resolved within its frame at dispatch time and do not propagate up. An empty or nil FrameBindings produces no filtering — the change is fully backward-compatible with subgraphs built without kwarg classification. Encapsulation under β semantics The β reading the user confirmed during the Phase 5 spec discussion ("when an outer subgraph's bubble-up walks an immediate child Subgraph, it takes the child Subgraph's exposed parameter surface") is now structurally realized: - Inner subgraph binds variable `secret` locally (inner.FrameBindings["secret"] = ...). Inner.Parameters() filters out "secret". - Outer subgraph contains inner as a child. The outer's bubble-up calls inner.Parameters() which already returns the locally-filtered surface — "secret" never appears in outer's surface. This is the encapsulation contract: each subgraph's surface is a stable interface its parent can see; deeply-nested variables are the deep subgraph's concern, not the parent's. Implementation pkg/op/subgraph.go: Three-line filter loop appended after the bubble-up walk and before the sort+materialize step: for name := range s.FrameBindings { delete(seen, name) } Subgraph.Parameters() doc comment rewritten to describe the filter contract: bubble-up MINUS local FrameBindings; what callers can expect; what an empty FrameBindings does (no-op). pkg/op/subgraph_test.go (4 new cases): - TestSubgraph_Parameters_FrameBindings_FullyBoundReturnsEmpty: child references "dest_dir"; subgraph binds it; surface = []. - TestSubgraph_Parameters_FrameBindings_PartialBindingFiltersOnlyMatching: children reference "alpha" and "beta"; subgraph binds "alpha"; surface = ["beta"]. - TestSubgraph_Parameters_FrameBindings_NestedHidesFromOuter: inner binds "secret" locally; outer contains inner + a child referencing "shared"; outer.Parameters() = ["shared"] (not ["secret", "shared"]). - TestSubgraph_Parameters_FrameBindings_EmptyMapIsNoOp: empty FrameBindings does not filter; surface unchanged from the pre-B-class.3 behavior. Existing Phase 3 Parameters tests are untouched — they construct subgraphs without setting FrameBindings, so the filter is a no-op for them and they continue to pass. What's deferred Bridge-time validation that kwargs names MUST match a variable in the subgraph's surface (extra kwargs are bridge-time errors per the user's spec). That validation lives in dispatchContainer (B-class.2's territory) and would compare classified kwargs against Parameters() at the moment of subgraph construction. Not yet wired — unknown kwargs currently flow into FrameBindings without error. A follow-up commit can add the validation once we settle the desired error shape. Executor-side frame resolution that actually USES FrameBindings to mint the dispatch-time Frame and resolve VariableValue references — that's the Phase 5 executeSubgraph rewrite. The data shape is correct after B-class.2; the consumer is pending. Build state make build / make vet clean. pkg/op + all provider packages green (including the 4 new tests). cmd/devlore-test / cmd/star/star failures unchanged in scope; the error message now points at the Phase 5 executeSubgraph stub since graph.Root itself dispatches through executeSubgraph, but the underlying materialization gap is the same one Phase 5 closes. Issue: #283
1 parent c09587c commit 58308a3

2 files changed

Lines changed: 119 additions & 23 deletions

File tree

‎pkg/op/subgraph.go‎

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,21 @@ import (
1111

1212
// Subgraph is a subsystem of the graph — a functional, structural, and transactional boundary.
1313
//
14-
// Subgraphs are recursive: a subgraph contains nodes and child subgraphs, forming a tree. The graph is
15-
// the root of the tree. All subgraphs participate in the saga pattern: retry, compensation, status
16-
// tracking. Nodes and subgraphs are peers at any level — both are vertices in the same topological sort.
14+
// Subgraphs are recursive: a subgraph contains nodes and child subgraphs, forming a tree. The graph is the root of the
15+
// tree. All subgraphs participate in the saga pattern: retry, compensation, status tracking. Nodes and subgraphs are
16+
// peers at any level — both are vertices in the same topological sort.
1717
type Subgraph struct {
1818
executableUnit
1919

20-
// Name is the subgraph name (e.g., "install").
20+
// Name is the name of the subgraph (e.g., "install").
2121
Name string
2222

23-
// Children are the nodes and child subgraphs in declaration order. Execution proceeds through this
24-
// list after topological sorting by edges at this level.
23+
// Children are the nodes and child subgraphs in declaration order. Execution proceeds through this list after
24+
// topological sorting by edges at this level.
2525
Children []SubgraphChild
2626

27-
// Edges are ordering constraints between children at this level. Each edge references children by
28-
// ID (both node IDs and subgraph IDs).
27+
// Edges are ordering constraints between children at this level. Each edge references children by ID (both node IDs
28+
// and subgraph IDs).
2929
Edges []Edge
3030

3131
// Status of this subgraph: pending, completed, failed, rolled_back, skipped.
@@ -79,8 +79,7 @@ func NewSubgraph(id string) *Subgraph {
7979

8080
// region State management
8181

82-
// AddChild appends a child (Node or Subgraph) to this subgraph's Children and stamps the child's parentID to this
83-
// subgraph's ID.
82+
// AddChild appends an [ExecutableUnit] to this subgraph's Children and stamps its parentID to this subgraph's ID.
8483
//
8584
// Centralizing wiring through this method keeps ownership accurate (plan-doc D11) without callers having to remember to
8685
// maintain the back-reference themselves.
@@ -91,8 +90,8 @@ func NewSubgraph(id string) *Subgraph {
9190
// context).
9291
//
9392
// Parameters:
94-
// - `child`: the [SubgraphChild] variant to attach. Exactly one of Node or Subgraph must be set;
95-
// AddChild stamps parentID on whichever is present.
93+
// - `child`: the [SubgraphChild] variant to attach. Exactly one of Node or Subgraph must be set; AddChild stamps
94+
// parentID on whichever is present.
9695
func (s *Subgraph) AddChild(child SubgraphChild) {
9796

9897
s.Children = append(s.Children, child)
@@ -110,24 +109,30 @@ func (s *Subgraph) AddChild(child SubgraphChild) {
110109

111110
// region Behaviors
112111

113-
// Parameters returns the bubble-up variable surface of this subgraph.
112+
// Parameters are the exposed bubble-up variable surface of this subgraph.
114113
//
115114
// The deduplicated set of [VariableValue] references walked across every child's slots, recursing into nested subgraphs
116-
// (plan-doc D3). This shadows the embedded [executableUnit.Parameters] for *Subgraph callers and for interface dispatch
117-
// through [ExecutableUnit] on *Subgraph.
115+
// (plan-doc D3), MINUS the variables this subgraph binds locally via [Subgraph.FrameBindings]. The exposed surface is
116+
// what a parent caller must supply when invoking this subgraph: variables already bound locally are resolved within
117+
// this subgraph's frame at dispatch time and do not propagate up. This shadows the embedded [executableUnit.Parameters]
118+
// for *Subgraph callers and for interface dispatch through [ExecutableUnit] on *Subgraph.
118119
//
119120
// Discovery is a graph-walk: for each child node, iterate its slots; for each slot whose Value is a [VariableValue],
120121
// contribute a [Parameter] under the variable's Name, carrying the slot's declared Type and Default. For each child
121-
// subgraph, recurse — its [Subgraph.Parameters] already returns deduped, type-checked entries; merge them into the
122-
// parent's working set. [ImmediateValue] and [PromiseValue] slot fills do not contribute (they are intrinsically
123-
// resolved at execution time).
122+
// subgraph, recurse — its [Subgraph.Parameters] already returns its own deduped, locally-filtered exposed surface;
123+
// merge those entries into the parent's working set. [ImmediateValue] and [PromiseValue] slot fills do not contribute
124+
// (they are intrinsically resolved at execution time).
124125
//
125126
// Parameters with the same name and same type collapse to one entry. Parameters with the same name and different types
126127
// are caught as plan-time errors (panic via [assert.Failf]) because the variable map at runtime is keyed by name and
127128
// carries one value.
128129
//
130+
// After the bubble-up walk completes, any entry whose Name is a key in [Subgraph.FrameBindings] is removed from the
131+
// result — those variables are bound locally and not part of the caller's interface. An empty or nil FrameBindings
132+
// map produces no filtering.
133+
//
129134
// Returns:
130-
// - []Parameter: the deduplicated bubble-up surface, in stable order by Name.
135+
// - []Parameter: the exposed, deduplicated bubble-up surface, in stable order by Name.
131136
func (s *Subgraph) Parameters() []Parameter {
132137

133138
seen := make(map[string]Parameter)
@@ -160,6 +165,10 @@ func (s *Subgraph) Parameters() []Parameter {
160165
}
161166
}
162167

168+
for name := range s.FrameBindings {
169+
delete(seen, name)
170+
}
171+
163172
out := make([]Parameter, 0, len(seen))
164173
names := make([]string, 0, len(seen))
165174

@@ -199,9 +208,11 @@ func (s *Subgraph) mergeBubbled(seen map[string]Parameter, bubbled Parameter) {
199208
}
200209

201210
if existing.Type != bubbled.Type {
202-
assert.Failf(
203-
"subgraph %q: variable %q declared with incompatible types %s and %s across slots",
204-
s.ID(), bubbled.Name, existing.Type, bubbled.Type)
211+
assert.Failf("subgraph %q: variable %q declared with incompatible types %s and %s across slots",
212+
s.ID(),
213+
bubbled.Name,
214+
existing.Type,
215+
bubbled.Type)
205216
}
206217
}
207218

@@ -243,7 +254,7 @@ type RollbackEntry struct {
243254

244255
// SubgraphChild is a child of a subgraph or graph — either a node or a nested subgraph.
245256
//
246-
// Exactly one field is set.
257+
// It is eitherExactly one field is set.
247258
type SubgraphChild struct {
248259

249260
// Node is the [Node] this child wraps; nil when this child wraps a Subgraph.
@@ -285,12 +296,16 @@ type SubgraphStatus string
285296
const (
286297
// SubgraphPending indicates the subgraph has not yet been executed.
287298
SubgraphPending SubgraphStatus = "pending"
299+
288300
// SubgraphCompleted indicates the subgraph executed successfully.
289301
SubgraphCompleted SubgraphStatus = "completed"
302+
290303
// SubgraphFailed indicates the subgraph failed during execution.
291304
SubgraphFailed SubgraphStatus = "failed"
305+
292306
// SubgraphRolledBack indicates the subgraph was rolled back after failure.
293307
SubgraphRolledBack SubgraphStatus = "rolled_back"
308+
294309
// SubgraphSkipped indicates the subgraph was skipped.
295310
SubgraphSkipped SubgraphStatus = "skipped"
296311
)

‎pkg/op/subgraph_test.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,87 @@ func TestSubgraph_Parameters_SortedByName(t *testing.T) {
203203
}
204204
}
205205

206+
func TestSubgraph_Parameters_FrameBindings_FullyBoundReturnsEmpty(t *testing.T) {
207+
208+
sg := NewSubgraph("sg")
209+
sg.AddChild(SubgraphChild{Node: nodeWithSlots("n",
210+
stringSlot("path", VariableValue{Name: "dest_dir"}),
211+
)})
212+
sg.FrameBindings = map[string]SlotValue{
213+
"dest_dir": ImmediateValue{Value: "/tmp/x"},
214+
}
215+
216+
params := sg.Parameters()
217+
if len(params) != 0 {
218+
t.Errorf("len = %d, want 0 (dest_dir is bound locally)", len(params))
219+
}
220+
}
221+
222+
func TestSubgraph_Parameters_FrameBindings_PartialBindingFiltersOnlyMatching(t *testing.T) {
223+
224+
sg := NewSubgraph("sg")
225+
sg.AddChild(SubgraphChild{Node: nodeWithSlots("n",
226+
stringSlot("p1", VariableValue{Name: "alpha"}),
227+
stringSlot("p2", VariableValue{Name: "beta"}),
228+
)})
229+
sg.FrameBindings = map[string]SlotValue{
230+
"alpha": ImmediateValue{Value: "hello"},
231+
}
232+
233+
params := sg.Parameters()
234+
if len(params) != 1 {
235+
t.Fatalf("len = %d, want 1 (alpha bound; beta exposed)", len(params))
236+
}
237+
if params[0].Name != "beta" {
238+
t.Errorf("Name = %q, want beta (alpha was filtered out)", params[0].Name)
239+
}
240+
}
241+
242+
func TestSubgraph_Parameters_FrameBindings_NestedHidesFromOuter(t *testing.T) {
243+
244+
// Inner subgraph references "secret" and binds it locally via FrameBindings; "secret" must NOT bubble up to
245+
// outer. Outer references "shared" only via its own child node; that variable stays exposed.
246+
247+
inner := NewSubgraph("inner")
248+
inner.AddChild(SubgraphChild{Node: nodeWithSlots("inner_node",
249+
stringSlot("p", VariableValue{Name: "secret"}),
250+
)})
251+
inner.FrameBindings = map[string]SlotValue{
252+
"secret": ImmediateValue{Value: "hidden"},
253+
}
254+
255+
outer := NewSubgraph("outer")
256+
outer.AddChild(SubgraphChild{Subgraph: inner})
257+
outer.AddChild(SubgraphChild{Node: nodeWithSlots("outer_node",
258+
stringSlot("q", VariableValue{Name: "shared"}),
259+
)})
260+
261+
params := outer.Parameters()
262+
if len(params) != 1 {
263+
t.Fatalf("len = %d, want 1 (inner's secret is filtered at its own level; only shared bubbles up)", len(params))
264+
}
265+
if params[0].Name != "shared" {
266+
t.Errorf("Name = %q, want shared", params[0].Name)
267+
}
268+
}
269+
270+
func TestSubgraph_Parameters_FrameBindings_EmptyMapIsNoOp(t *testing.T) {
271+
272+
sg := NewSubgraph("sg")
273+
sg.AddChild(SubgraphChild{Node: nodeWithSlots("n",
274+
stringSlot("path", VariableValue{Name: "dest_dir"}),
275+
)})
276+
sg.FrameBindings = map[string]SlotValue{}
277+
278+
params := sg.Parameters()
279+
if len(params) != 1 {
280+
t.Fatalf("len = %d, want 1 (empty FrameBindings does not filter)", len(params))
281+
}
282+
if params[0].Name != "dest_dir" {
283+
t.Errorf("Name = %q, want dest_dir", params[0].Name)
284+
}
285+
}
286+
206287
func TestSubgraph_AddChild_StampsParent_Node(t *testing.T) {
207288

208289
sg := NewSubgraph("sg")

0 commit comments

Comments
 (0)