feat(runtime): materialize binding and flow usages as connector objects - #474
Conversation
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| // one of them failing is reported as its own, with the connector kept. | ||
| func (ctx *Context) materializeConnectorAs(owner *Instance, connSym, base *symbols.Symbol, id int64, keep func(*Instance)) error { | ||
| ends := ctx.model.semantics.ConnectorEndAttachments(connSym) | ||
| ends := ctx.model.semantics.ConnectorObjectEnds(connSym) |
There was a problem hiding this comment.
🟡 Connector ends retain stale values
After bnd or f materializes, changing an endpoint leaves its ConnectorEnd.Value stale. The connector stores a value without retaining the endpoint feature.
Learn more
Connector materialization evaluates each attachment once and stores the resulting Value in both Instance.Ends and the synthesized end feature value. SetFeatureValue can later replace the connected feature's value, but the stored connector value has no dependency or reference back to that FeatureValue. This especially affects bindings between scalar attributes, where reassignment is common, and flows whose ends are value features.
Example: Materialize binding bnd bind x = y while both attributes equal 3, then write x = 5. Reading bnd.source still returns 3, although the connected feature now holds 5.
Recommended fix: Represent each materialized end as a reference to its owning FeatureValue, or add invalidation/rebinding that refreshes every attached connector end when that feature changes. Apply the same mechanism to Instance.Ends and named end feature values so both views stay consistent.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is the behavior of the shared materialization path: connect connector objects on develop store each end's value once at materialization in exactly the same way, and this PR deliberately routes bindings and flows through that path rather than a parallel one. Making a materialized end a live reference to the endpoint's FeatureValue (for all connector kinds, including connect) is a separate change to materializeConnectorAs/Instance.Ends; left out of this PR to keep it to the object-model gap, and noted as a limitation.
Co-Authored-By: jason.han <hanhuijun@gmail.com>
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| case ast.UsageBinding: | ||
| return len(usage.ConnectorEnds) == 2 |
There was a problem hiding this comment.
🟡 Effective binding ends stay invisible
When a binding inherits or body-declares both ends, IsConnectorObjectUsage rejects it because ConnectorEnds contains only clause ends. Reading the valid binding then yields no connector object.
Learn more
A binding's effective ends include body-declared and inherited ends. ConnectorEndCount already recognizes these forms, including binding b2 :> b1, but this predicate only counts the current declaration's clause syntax. The runtime therefore takes the ordinary feature path and leaves the binding without an instance value.
Example: binding b1 of x = y; binding b2 :> b1; gives b2 two inherited ends. Reading b2 must return a connector object attached to x and y, but the predicate returns false because b2.ConnectorEnds is empty.
Recommended fix: Decide connector-object eligibility from the effective two-ended binding shape, then make ConnectorObjectEnds derive attachments from the effective ends' original declarations. Cover inherited and body-only binary bindings in semantic and runtime tests.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This matches the eligibility rule the shared path already applies to connect: IsConnectorUsage requires the usage's own connect clause, so connection c2 :> c1; with inherited ends is likewise not materialized as a connector object today. Bindings and flows follow the same rule on purpose so that both kinds have one materialization path. Deriving eligibility and attachments from the effective (inherited/body-declared) ends is a change to that shared path for all connector kinds, and is left out of this PR; noted as a limitation alongside the flow payload.
| case ast.UsageBinding: | ||
| kind = symbols.SymbolBindingUsage |
There was a problem hiding this comment.
🟡 Anonymous binding end names disappear
For an anonymous binding with named ends, anonymousConnectorSymbol discards the scope containing those end symbols. The object exposes fallback source and target features instead of the declared names.
Learn more
ownedEnds can recover a declared end name only through the connector symbol's child scope. The synthetic anonymous symbol carries the usage and owner scope but no child scope, although the builder registered the real anonymous symbol and its end symbols. ConnectorObjectEnds therefore falls back to binary source and target names.
Example: For binding of left ::> a = right ::> b;, OwnedConnectors returns an object with source and target. Calls for left and right fail even though those are the declared end features.
Recommended fix: Reuse the builder's anonymous symbol, or preserve its child scope when constructing the runtime symbol. Add a runtime test that reads both named ends of an anonymous binding.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Same behavior as an anonymous connect left ::> a to right ::> b; on develop: anonymousConnectorSymbol has always built a scope-less stand-in, so declared end names of any anonymous connector fall back to source/target. This PR only adds the flow/binding kinds to the existing stand-in. Reusing the builder's registered anonymous symbol (with its child scope) fixes it for every connector kind at once and belongs in a follow-up on that helper rather than in the binding/flow change.
What and why
A two-ended binding (
bind a.p = b.q,binding bnd bind a.p = b.q,binding of x = y) and a non-message flow (flow f of Item from a.p to b.q,flow a.p to b.q) are connectors of the kernel layer, but until now onlyconnectconnectors were materialized as connector objects; a binding or flow reached routing and value propagation through lowering and was otherwise invisible in the instance model. This PR materializes both through the same connector pathconnectuses — no parallel structure — sobnd.source,f.target, feature chains through an end,OwnedConnectors, REPL completion and queries see them like any connector object.semantics/connector.go: newModel.IsConnectorObjectUsageandModel.ConnectorObjectEndsbeside the existingIsConnectorUsage/ConnectorEndAttachments, which stayconnect-only for views, document queries and passes. Both share one end-naming helper (connectorEndAttachments), soconnectends are named exactly as before; a binding's ends are its twoConnectorEnds, a flow's areFlowEnds.From/Toin declaration order. A one-ended binding (bind x;,binding b of x;) and amessageare not connector objects.runtime: every object-model consumer (connector.go,instance.go,adopt.go,classifier_behavior.go,subsetting.go,variation.go,repl/complete.go) switches to the wider predicate;anonymousConnectorsnow asks the semantics for ends instead of countingConnectorEnds, andanonymousConnectorSymbolgives flows and bindings their own symbol kinds.runtime/binding.goandrouting.goare untouched.symbols: a binding usage had no symbol kind (SymbolUnknown), which two passes were compensating for. It is nowSymbolBindingUsage(BindingConnectorAsUsagein the semantic annotations and query metamodel), included in the feature kinds, typecheck table and shape features. Because the kind is persisted, the library index record version (26 → 27) and stdlib snapshot version (17 → 18) are bumped andstdlib.snapshotregenerated; the two kind-mapping digests move accordingly.Known limitation (documented): a flow object does not yet hold its
payload.Specification basis
KerML 1.0 §7.4.6 (connectors;⚠️ → ✅) and removes "Binding and flow connector objects" from "Implementable But Not Yet Done".
BindingConnector :> Connector,Links::selfLinks), SysML v2 §7.13.2 / §8.3.13 (BindingConnectorAsUsage,FlowConnectionUsage;Connections.sysml),Transfers::flowTransfers. Moves the connector rows of the Structural map indocs/project/spec-compliance.md(the flow/binding row goesHow it was verified
semantics/connector_test.goTestConnectorObjectEnds(named/anonymous binding and flow,binding of, one-ended binding,message, succession;ConnectorEndAttachmentsstill nil for binding/flow).runtime/connector_test.go:TestBindingConnectorIsAnObjectOfItsEnds,TestFlowConnectorIsAnObjectOfItsEnds,TestAnonymousBindingAndFlowAreOwnedConnectors,TestBindingConnectorEndFollowsAFeatureChain,TestBindingConnectorEndsHoldBoundValues.runtime/robustness_connector_objects_test.goTestRuntimeRobustnessConnectorObjects(unattachable binding/flow ends are typedErrConnectorEndwith a location and leave no object; unvalued ends; one-ended binding is no connector object).connector_object_binding_flow.sysml(identity assertions onbnd.source/f.target).runtime/routing_differential_test.goTestRoutingUnchangedByConnectorObjectsruns everysend_*,port_*,*_routing*,action_port_communication*andbinding_*fixture normally and again with every owned connector forced (recursively over held objects) before execution, and requires identical outcomes; thesend_bind_relay_*relays are asserted to have run in forced mode.go build ./...,go vet ./...,gofmt -l .empty,go test -count=1 ./..., training + pilot corpus gates with the require variables set (no ratchet movement), corpus round-trip gate, pilot-library XMI identity gate,scripts/check-doc-ids.py,scripts/changelog.py check,make lint.Checklist
make testandmake lintpass locallychanges/unreleased/<slug>.<section>.md, not as an edit toCHANGELOG.mdmake docs-countsrun if a gate count moved (compliance rows need nothing: the census is counted at docs build)F4,K5) in the body, docs, or changelog