Compare a source against another source (2.12.0) - #48
Merged
Conversation
Every comparison weighed a source against a constant. Comparing two live
readings was not expressible, and the workaround cost correctness: the live
"Alla laddar" gate compares a group's countTrue against flow.phones_charging, a
count some other node stored earlier. The two sides are sampled at different
moments, so someone coming home makes the equality fail until the variable
catches up — self-correcting, and therefore the kind of wrong nobody notices.
A new value type, `state`, resolves the right-hand side in the same pass as the
left. The window cannot exist.
One spec describes it everywhere — { src, thing, item, group, groupFunction },
the shape hal2Bayes already used for its steps — so both sides of a rule and all
three rule nodes talk about a source the same way. Groups on the right are the
point rather than a bonus: the case that prompted this compares two group
counts, and readGroup already computes any function on demand.
Gate's sourceOf turned out to be the resolver already, minus a name. Extracted
as readSource(spec) and now used for both sides, which also removed the
duplicate thing-lookup it had grown.
Bayes was the sensitive one. conditionMatches was pure, and the resolver had to
be threaded through it and conditionHolds — five call sites. Two of them can be
reached without a resolver, where a state comparison cannot be evaluated: it
returns false there, joining the existing rule that a condition which cannot be
evaluated has not matched. A test drives that path, because one unevaluable step
taking a whole evaluation with it is exactly the failure this could have added.
The picker is a source selector of its own rather than an entry in the type
dropdown: this location alone has 281 readable items and about a hundred
group/function combinations.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ayes Three faults, all in the editors — the runtime was fine. Event built its typedInput with the state type and then threw it away: the operator change handler rewrites the whole type list, and it ran on every open. Adding state to the construction call was necessary and not sufficient. Event also restored the saved type only when there was a saved value. state has no value (hasValue:false), so every saved state comparison loaded back as num. The type is now restored on its own. Bayes set the types inside syncOperatorOptions, which returns early unless the step's time-ness changed. It happened to run at construction, but the value field is hidden for the default operator, so nothing was visible until an operator was picked — and the type list was one refactor away from not being set at all. Set at construction, where the other types are.
…list Same fault in three places, found one node at a time. Bayes: opHandler rewrites the type list on every operator change and at construction, without state. Setting the types anywhere else was decoration. Fixed there, where it is decided. Gate: the value field is constructed with no types list, so it starts with Node-RED's built-in set, which has no state. The saved type was restored before the operator handler had installed the real list, and typedInput drops a type it does not offer — so every saved state comparison came back as something else. Restored after the operator now. Also removed three reads of rule.ct, which is assigned nowhere; with an undefined argument typedInput's type() is a getter, so they did nothing. Event: the Compare row sat above Trigger though it belongs to the value below it, and stayed visible when the operator stopped taking a value. One function decides the row now, called from both the operator and the value. The numeric operators keep a chosen state comparison instead of forcing num back over it, in Event and Bayes both.
…ary one Three complaints with one cause: the compare picker was built as a compact add-on rather than as another source picker, so it laid out and explained itself differently from the picker directly above it. Thing and item are on separate rows now, in Event and Gate at the same widths as everything else in those dialogs. Bayes cannot use those rows in a multi-step editor, so the source row and the item/value row each span flex:1 between the 50px lead and the remove-button pad — which is exactly what line 1 gives its thing and item selects together, so the two sides of a comparison line up down the dialog. The tip line moved into halFillSourcePicker as a setTip callback rather than being written three times. It says what a group's function means, or the notes on the thing and item — the same things the primary picker says. It follows the item and the function too, not only the source: which item is picked decides which notes apply. The item label reads Value for a group and Item for a thing, as above. Three rows now move together, so each node has one showCmp that decides them. refreshRuleView in Bayes was toggling only the first of the three, which would have left the other two behind on the next change.
flex:5 for the source and flex:3 for what is read from it — the same split line 1 gives its thing and item, so the compare pair sits directly under the pair it is compared against. The tip row stays on its own line.
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.
Every rule in hal2 compared one source against a constant: a number, a string, or a variable read at evaluation time. Comparing two live readings was not expressible, and the workaround cost correctness.
The case that exposed it: a Gate answering "is everyone who is home asleep?" by comparing
countTrueof the people-home group againstflow.phones_charging, a count a change node had stored earlier. The two sides are then sampled at different moments — someone comes home, the variable has not caught up, and the equality silently fails. It is self-correcting, and therefore the kind of wrong that never gets reported.Reading both sides at evaluation time makes the staleness impossible, and removes the change node, the variable, and the discipline of remembering to recompute it from both directions.
What it looks like
A new value type,
state, inhal2Gate,hal2Eventandhal2Bayes. Choosing it reveals a second source picker beside the value:Groups on the right-hand side are the point rather than a bonus — the case that prompted this compares two group counts, and
readGroup()already computes any function on demand.A flat list was not an option: this location has 281 readable items and roughly a hundred group/function combinations. The picker is a source selector of the same shape as the one the rule already uses.
Design
One spec shape everywhere —
{ src, thing, item, group, groupFunction }, which is whathal2Bayesalready used for its steps. Both sides of a rule and all three nodes describe a source the same way, and in Bayes the comparison side gets thing, group, flow, global and env for free, becauseresolveStatealready handles them.Gate's
sourceOfturned out to be the resolver already, minus a name. Extracted asreadSource(spec)and used for both sides.Bayes was the sensitive one.
conditionMatcheswas pure, and the resolver had to be threaded through it andconditionHolds— five call sites. Two of them are reachable without a resolver, where a state comparison cannot be evaluated; it returnsfalsethere, joining the existing rule that a condition which cannot be evaluated has not matched. There is a test for exactly that path, because one unevaluable step taking an entire Bayes evaluation with it is the failure this change could have introduced.stateis offered only where comparing values makes sense — not fortrue/false,regex, or Gate'slast_*operators, which compare against a duration.Editor fixes found while testing this
state, so offering the type anywhere else was decoration.state. The saved type was restored before the operator handler installed the real list, and typedInput silently drops a type it does not offer — every saved state comparison came back as something else.rule.ctin Gate, which is assigned nowhere; with an undefined argument typedInput'stype()is a getter, so they did nothing while looking like they preserved the type.halFillSourcePickerrather than being written three times.Out of scope
a > b + 2). The comparison is against the other reading as it is; a derived value still belongs in a flow.last_*against another source. Those compare against a duration, and "changed longer ago than that one" is a different feature with its own questions.Verification
329 tests passing; clean restart with no
[error]in the log. Verified live in the editor across all three nodes.🤖 Generated with Claude Code