Repository navigation
feat: let operators inside a loop use its variables ($i) - #8645
Closed
aglinxinyuan wants to merge 21 commits into
Closed
aglinxinyuan wants to merge 21 commits into
aglinxinyuan wants to merge 21 commits into
Conversation
Inside a control block the iteration's loop variables reach every operator as a state message. The Python worker now stores that message on `Operator.loop_state` right before `process_state` runs -- in DataProcessor.process_state and in the LoopEnd's deferred consume, which calls the operator directly -- so the callback and every call after it can consult the current iteration's variables. The default callback is unchanged and still forwards the state.
A property of an operator inside a loop block (LoopStart ... LoopEnd) may name a loop variable as `$K`. The frontend keeps sending the literal string whatever the property's type; the backend binds it in three steps. Parse. LogicalOp is now a StateReferencing descriptor, with a `stateReferences` sidecar (JSON pointer -> variable name) that the property panel never shows. StateReferenceModule, registered on JSONUtils.objectMapper and on the workflow-compiling-service mapper, adds a Jackson problem handler: when a whole-string `$name` cannot be converted to an integral, floating-point or boolean property, it returns the placeholder 0 / 0.0 / false and records the value's pointer, relative to the operator, in the operator's sidecar. A thin wrapper around each concrete StateReferencing deserializer opens that recording frame. An object that a polymorphic type deserializer hands over mid-way is first copied into a buffer of its own, because Jackson replays the fields before a late type id from a buffer whose parse context copies the first field's value, not the object. String properties keep the literal, and any other target (an enum, ...) keeps Jackson's ordinary error. Compile. WorkflowCompiler works out which operators lie inside a loop block (LoopBlockMembership, the frontend's block walk) and makes the sidecar final before the physical plan serializes it into the executor's descString. Inside a block it adds every whole-string `$name` of the operator's JSON. Outside every block a string such as "$AAPL" stays the literal it is on main, and a typed placeholder is a compile error naming the property and the variable. Inside a block, an operator whose executor runs generated code, or with an input fed from outside the block, is a compile error too. Worker. ExecFactory hands out a LateBoundExecutor when the descString's sidecar is not empty. It keeps and forwards each state message, and binds when the real executor is first needed, at the first tuple or finishing port: the channel has delivered its state messages by then, so their values are merged with the last one winning, and an inner loop's variable shadows an outer one of the same name. It patches the placeholders, coerced to their JSON type (a string property takes only a string, a number or a boolean), builds the real executor through the normal factory path, opens it and replays the kept state messages into it in order, so an executor that acts on them (If) behaves as without references. Every later call goes to that executor, which is never rebound. A variable still missing then is an error naming it. Closes apache#8635
A property of an operator inside a loop block (LoopStart ... LoopEnd) may name a loop variable as `$K`. The frontend keeps sending the literal string whatever the property's type; the backend binds it in three steps. Parse. LogicalOp is now a StateReferencing descriptor, with a `stateReferences` sidecar (JSON pointer -> variable name) that the property panel never shows. StateReferenceModule, registered on JSONUtils.objectMapper and on the workflow-compiling-service mapper, adds a Jackson problem handler: when a whole-string `$name` cannot be converted to an integral, floating-point or boolean property, it returns the placeholder 0 / 0.0 / false and records the value's pointer, relative to the operator, in the operator's sidecar. A thin wrapper around each concrete StateReferencing deserializer opens that recording frame. An object that a polymorphic type deserializer hands over mid-way is first copied into a buffer of its own, because Jackson replays the fields before a late type id from a buffer whose parse context copies the first field's value, not the object. String properties keep the literal, and any other target (an enum, ...) keeps Jackson's ordinary error. Compile. WorkflowCompiler works out which operators lie inside a loop block (LoopBlockMembership, the frontend's block walk) and makes the sidecar final before the physical plan serializes it into the executor's descString. Inside a block it adds every whole-string `$name` of the operator's JSON. Outside every block a string such as "$AAPL" stays the literal it is on main, and a typed placeholder is a compile error naming the property and the variable. Inside a block, an operator whose executor runs generated code, or with an input fed from outside the block, is a compile error too. Worker. ExecFactory hands out a LateBoundExecutor when the descString's sidecar is not empty. It keeps and forwards each state message, and binds when the real executor is first needed, at the first tuple or finishing port: the channel has delivered its state messages by then, so their values are merged with the last one winning, and an inner loop's variable shadows an outer one of the same name. It patches the placeholders, coerced to their JSON type (a string property takes only a string, a number or a boolean), builds the real executor through the normal factory path, opens it and replays the kept state messages into it in order, so an executor that acts on them (If) behaves as without references. Every later call goes to that executor, which is never rebound. A variable still missing then is an error naming it. Closes apache#8635
Inside a control block the iteration's loop variables reach every operator as a state message. The Python worker now stores that message on `Operator.loop_state` right before `process_state` runs -- in DataProcessor.process_state and in the LoopEnd's deferred consume, which calls the operator directly -- so the callback and every call after it can consult the current iteration's variables. The default callback is unchanged and still forwards the state.
Inside a control block (Loop Start ... Loop End) any primitive property of an operator may hold a loop-variable reference such as "$K", which the backend binds at run time (apache#8635). The property panel renders such fields as a text input with an autocomplete of the variables the enclosing Loop Starts declare, stores a complete typed numeral or true/false as the property's primitive and a reference as the literal string, and flags a name no enclosing block declares. The canvas validation waives the schema type check exactly where a reference sits in a primitive property, so the run button stays enabled, and still rejects undeclared names. Operators outside every block are unchanged. Both follow edits elsewhere in the graph: after a link is added or removed, or a Loop Start's properties change, every operator holding a reference is re-validated and the open form is rebuilt when its scope changed. Block membership (getEnclosingLoopStarts) and loop-variable extraction from the Loop Start's initialization code are pure functions with their own specs. The block walk caps its open-block count, so it ends on a cycle through a LoopStart or LoopEnd. Extraction follows Python's module-level binding: assignments under if/else, try/except, while and for, and for-loop targets count; def and class bodies do not.
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8645 +/- ##
============================================
- Coverage 92.74% 92.71% -0.04%
- Complexity 4925 4959 +34
============================================
Files 1241 1250 +9
Lines 52454 53218 +764
Branches 6480 6671 +191
============================================
+ Hits 48648 49339 +691
- Misses 2204 2227 +23
- Partials 1602 1652 +50
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 408 | 0.249 | 23,531/30,576/30,576 us | 🟢 -17.9% / 🔴 +80.9% |
| 🔴 | bs=100 sw=10 sl=64 | 822 | 0.502 | 119,853/146,314/146,314 us | 🔴 +5.9% / 🔴 +30.5% |
| 🔴 | bs=1000 sw=10 sl=64 | 944 | 0.576 | 1,055,823/1,180,155/1,180,155 us | 🔴 +5.2% / 🔴 +10.0% |
Baseline details
Latest main 9ead3ac from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 408 tuples/sec | 420 tuples/sec | 736.99 tuples/sec | -2.9% | -44.6% |
| bs=10 sw=10 sl=64 | MB/s | 0.249 MB/s | 0.256 MB/s | 0.45 MB/s | -2.7% | -44.6% |
| bs=10 sw=10 sl=64 | p50 | 23,531 us | 21,994 us | 13,174 us | +7.0% | +78.6% |
| bs=10 sw=10 sl=64 | p95 | 30,576 us | 37,225 us | 16,900 us | -17.9% | +80.9% |
| bs=10 sw=10 sl=64 | p99 | 30,576 us | 37,225 us | 19,889 us | -17.9% | +53.7% |
| bs=100 sw=10 sl=64 | throughput | 822 tuples/sec | 870 tuples/sec | 946.47 tuples/sec | -5.5% | -13.2% |
| bs=100 sw=10 sl=64 | MB/s | 0.502 MB/s | 0.531 MB/s | 0.578 MB/s | -5.5% | -13.1% |
| bs=100 sw=10 sl=64 | p50 | 119,853 us | 113,178 us | 105,257 us | +5.9% | +13.9% |
| bs=100 sw=10 sl=64 | p95 | 146,314 us | 138,851 us | 112,150 us | +5.4% | +30.5% |
| bs=100 sw=10 sl=64 | p99 | 146,314 us | 138,851 us | 125,962 us | +5.4% | +16.2% |
| bs=1000 sw=10 sl=64 | throughput | 944 tuples/sec | 951 tuples/sec | 973.64 tuples/sec | -0.7% | -3.0% |
| bs=1000 sw=10 sl=64 | MB/s | 0.576 MB/s | 0.58 MB/s | 0.594 MB/s | -0.7% | -3.1% |
| bs=1000 sw=10 sl=64 | p50 | 1,055,823 us | 1,052,750 us | 1,032,217 us | +0.3% | +2.3% |
| bs=1000 sw=10 sl=64 | p95 | 1,180,155 us | 1,121,831 us | 1,072,497 us | +5.2% | +10.0% |
| bs=1000 sw=10 sl=64 | p99 | 1,180,155 us | 1,121,831 us | 1,102,623 us | +5.2% | +7.0% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,490.26,200,128000,408,0.249,23531.30,30575.55,30575.55
1,100,10,64,20,2433.05,2000,1280000,822,0.502,119853.46,146313.77,146313.77
2,1000,10,64,20,21187.69,20000,12800000,944,0.576,1055823.01,1180154.92,1180154.92
This was referenced Sep 24, 2026
The attribute the runtime registers on every operator right before process_state is now `state` instead of `loop_state`. LoopStartOperator and LoopEndOperator already used `self.state` for their own loop variables, so registering the incoming message there would have merged it into itself (LoopStart) and made the back-edge write the message instead of the updated variables (LoopEnd). Their own field is renamed to `variables`, and the back-edge in MainLoop._jump_to_loop_start now writes `executor.variables`. Loop operator behavior is unchanged. Two new tests pin the split: a LoopStart run through DataProcessor.process_state keeps its variables apart from the registered state, and the LoopEnd back-edge writes the updated variables rather than the registered state.
The attribute the runtime registers on every operator right before process_state is now `state` instead of `loop_state`. LoopStartOperator and LoopEndOperator already used `self.state` for their own loop variables, so registering the incoming message there would have merged it into itself (LoopStart) and made the back-edge write the message instead of the updated variables (LoopEnd). Their own field is renamed to `variables`, and the back-edge in MainLoop._jump_to_loop_start now writes `executor.variables`. Loop operator behavior is unchanged. Two new tests pin the split: a LoopStart run through DataProcessor.process_state keeps its variables apart from the registered state, and the LoopEnd back-edge writes the updated variables rather than the registered state.
The attribute the runtime registers before process_state is now `state` (apache#8642), so the method that registers it follows: Operator.register_loop_state is now Operator.register_state. DataProcessor.process_state and the deferred consume in MainLoop call it under the new name, and its docstring and the tests say `state` / `register_state` throughout. The loop-variable reads, loop_variable_text and loop_variable_value, and their _loop_variables are unchanged.
renovate-bot
pushed a commit
to renovate-bot/apache-_-texera
that referenced
this pull request
Oct 3, 2026
### What changes were proposed in this PR? Inside a loop block (LoopStart ... LoopEnd), a property of a JVM operator may now hold `$i`, a reference to the loop variable `i`. This works for string and numeric/boolean properties alike; the frontend (apache#8640) sends the literal `"$i"`, and the operator's own code does not change. The reference is handled in three places: - **Parse.** When `"$i"` cannot be converted to a numeric or boolean property's type, a Jackson problem handler (`StateReferenceModule`) puts in a typed placeholder (0 / 0.0 / false) and records the property's JSON pointer in a hidden `stateReferences` sidecar on `LogicalOp`. Strings are left alone. - **Compile.** The compiler works out which operators sit inside a loop block (`LogicalPlan.enclosingLoopStarts`: each operator's Loop Starts, from folds over the plan's topological order). Inside a block it also records every whole-string `"$i"` literal. Outside every block, a typed reference is a compile error. - **Worker.** The operator's own executor is built and opened as usual, with the placeholders in its setting: the descriptor its constructor parses. The worker registers every state message on the executor right before `processState` (`OperatorExecutor.registerState` and `state`, the JVM counterpart of Python's `self.state` in apache#8642). Until the operator first sees data, each referenced variable that a message carries is written into that setting, in place. Right before the first tuple, or the end of input, the worker checks that every reference has a value, and the setting is not written after that. Copies of one loop's state that disagree are an error; an inner loop's value shadows an outer one's (the worker passes each message's nesting depth). **Which settings can hold `$i`** (inside a loop block) | Setting | Holds `$i`? | What the user sees | |---|---|---| | Text read while the operator processes rows (a Filter's value or column, ...) | Yes | The iteration's value, written into the setting before the operator's first row | | Number, integer or decimal (Limit's `limit`, Interval Join's constant, ...) | Yes | The iteration's value, converted to the setting's type. Before the run, the compiler sees the placeholder `0` / `0.0` | | True/false | Yes | Same as a number, with the placeholder `false` | | A setting the compiled plan is built from: an output column's name or type, or what an input is partitioned on (Projection's columns and Drop Option, Unnest String's and Dictionary Matcher's result column, Aggregate, join keys, Type Casting, Sort Partitions) | No | With apache#8750, a compile error: `Projection cannot refer to loop variables (/isDrop -> $drop): its output schema or partitioning is built from that property when the workflow is compiled, before the loop runs`. With this PR alone, a wrong result or the operator's own error (Projection's missing-column message) | | Any setting of an operator that runs generated or user code: Python UDFs (code editor included), sklearn, Python charts, Sort | No | Compile error: `Sort cannot refer to loop variables yet (/attributes/0/attribute -> $col): its code is generated before the loop runs` | | A dropdown of fixed choices (an enum, e.g. a Filter's condition) | No; the panel (apache#8640) doesn't offer `$i` there | If it is sent anyway (e.g. through `SyncExecutionResource`), the workflow fails to parse with Jackson's `not one of the values accepted for Enum class` | **Other cases** (replaces the current Case / Behaviour table) | Case | Behaviour | |---|---| | Outside every loop block | A text `"$AAPL"` stays a literal, as on main. A number or true/false `"$n"` is a compile error: `Limit refers to loop variables (/limit -> $n), but is not inside a loop block` | | An input the operator reads first is fed, even by a single link, from outside one of the operator's loop blocks: from outside the loop, from an outer loop's body, or from a sibling block | Compile error: `Interval Join refers to loop variables (/constant -> $i), but its input 'left table' is fed from outside the loop block, so tuples from there can arrive before the loop state`. An input read only after one that carries the state (a join's probe side) may come from anywhere | | A name no enclosing Loop Start assigns (the panel flags it; `SyncExecutionResource` doesn't) | Fails at the operator's first row, or at its finish if it gets no rows: `property /limit refers to loop variable n, but no state message carried it` | | A value that doesn't fit the setting | Fails at the same point: `property /limit refers to loop variable n, but its value abc does not fit it: ...` | | Nested loops that name the same variable | The inner loop's value wins | | Copies of one loop's state that reach an operator disagree (e.g. a UDF changed `i` on one branch, and the branches meet) | Fails: `loop variable i got two different values in one iteration, 1 and 2: a loop's variables must not change inside its body`. On a single path there is only one copy, so the operators after the UDF get its changed value | | Editing the operator while the workflow runs | Refused, with apache#8750 | Which operators are inside a block is decided on the plan the compiler receives. That plan has no disabled operators, and for run-to-operator it keeps only the target's upstream. The panel decides on the whole canvas. **The operator's lifecycle is unchanged.** The value is set in the operator's setting when the state arrives, so the operator runs exactly as it does outside a loop: ``` worker starts state messages arrive first tuple executor built and opened, registered on the executor (state), and every reference has a value, or the operator placeholders in its setting each variable written into the setting, fails naming the missing ones; in place (a later message wins) the setting is not written after this ``` `open()` runs at worker start, and `produceStateOnStart` and `processState` run on the operator itself, with what they return used. Three executors used to read their setting before a state can arrive, and now read it at first use: Reservoir Sampling in its constructor, and Split and Dictionary Matcher in `open()`. Every other JVM executor already reads its setting while it processes data. ### Any related issues, documentation, discussions? Closes apache#8635 Lands together with apache#8750, which makes `$i` in a setting the compiled plan is built from a compile error. Frontend: apache#8640 (merges after this PR). Python operators: apache#8642 (independent). User-facing overview: apache#8643 ### How was this PR tested? | Suite | Result | |---|---| | WorkflowCore: `org.apache.texera.amber.core.state.*` and `core.executor.*`, incl. `StateReferenceModuleSpec`, `StateReferenceBindingSpec`, `CoreExecutorReflectionSpec` | 71/71 | | WorkflowCompiler full `test`, incl. `LogicalPlanSpec` (loop-block membership and enclosing Loop Starts, checked against a port of the panel's walk on every case and 400 random plans) and the loop-variable cases of `WorkflowCompilerSpec` (lenient and strict) | 80/80 | | WorkflowOperator: Reservoir Sampling, Split, Dictionary Matcher and metadata specs | 260/260 | | workflow-compiling-service `test` | 55/55 | | amber: `Test/compile`, `DataProcessorSpec`, `DPThreadSpec`, the worker's promise handlers incl. `EndChannelHandlerSpec`, `ComputingUnitMasterSpec` | 116/116 | | Lint | `scalafixAll --check` and `scalafmtCheckAll` on all five touched sbt projects | ``` sbt "project WorkflowCore" "testOnly *StateReferenceModuleSpec *StateReferenceBindingSpec *CoreExecutorReflectionSpec" sbt "project WorkflowCompiler" test ``` What the tests pin: - **End to end in unit form.** The frontend's `{"limit":"$n","operatorType":"Limit"}` is compiled inside `TextInput -> LoopStart -> Limit -> LoopEnd` and built through `ExecFactory`. After the state `{n: 2}`, exactly 2 of 5 tuples pass. - **Parse handovers.** Typed placeholders get the right pointers whether the object is parsed directly, with the type id first, in the middle or last, or nested in another document. - **Compile rules.** Outside every block, Filter `"$AAPL"` stays a literal and Limit `"$n"` is a compile error. Inside a block, a Scala-built Filter `"$i"` gets its sidecar, while Sort `"$col"` is a compile error, and so is an operator with a reference whose read-first input has a link from outside one of its loop blocks (outside the loop, an outer loop's body, a sibling block, or one outside link next to an inside one). - **Worker.** The executor is built and opened at worker start. `produceStateOnStart` and `processState` run on the operator itself, and what `processState` returns is emitted. Each state message writes the variables into the setting in place, into every property that names them, top-level and nested pointers alike, keeping the object the executor holds; an outer `{i: 1}` followed by an inner `{i: 5}` leaves 5. A tuple before any state fails, naming only the missing variables, and the setting is not written after the first tuple; an operator with no rows fails the same way at its finish. Copies of one loop's state with different values fail, equal ones pass, and a deeper loop's value replaces an outer one's in either arrival order. Values are coerced to the property's type, and a null, a list or a map for a string property is refused. - **Executors.** Reservoir Sampling, Split and Dictionary Matcher use the value written after construction and `open()`. - **Mappers.** Every Dropwizard application that parses workflows registers the module through `JSONUtils.registerWorkflowModules`; `WorkflowCompilingServiceRunSpec` and `ComputingUnitMasterSpec` parse a typed `"$n"` through each bootstrap mapper. `LoopIntegrationSpec` gains three `@IntegrationTest` runs, which CI's `amber-integration` job runs: - a number setting: `Limit($n)` with n = 1, 2, 3 gives 1 + 2 + 3 = 6 rows; - nested loops that both name `i`: `Filter(line = $i)` in the inner body passes all 2 x 2 rows only with the inner `i`; - a text setting on two branches meeting at a Union before `Filter(line = $i)`: 2 copies x 3 iterations = 6 rows (a constant or a `$i` that never rebinds gives 2). **Run by hand.** On a local deployment of this PR together with apache#8642 and apache#8640 (the test branch in apache#8645), a workflow `Text Input (1..6) -> Loop Start (i = 0, output table) -> Limit ($i) -> Loop End (i += 1 while i < len(table))` runs to completion. Each iteration binds `$i` to its own `i`, so Loop End collects 0 + 1 + 2 + 3 + 4 + 5 = 15 rows: `1`, then `1, 2`, then `1, 2, 3`, and so on. The Limit fed straight from Text Input, outside the loop, keeps its constant 3. | The Limit's property inside the loop | Loop End's result after the run | |---|---| |  |  |  ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Fable 5.1, Claude Opus 5.5) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
renovate-bot
pushed a commit
to renovate-bot/apache-_-texera
that referenced
this pull request
Oct 24, 2026
### What changes were proposed in this PR? Inside a control block (`Loop Start ... Loop End`), any primitive property of an operator (string, integer, number, boolean) can now hold a loop-variable reference such as `$i`. The frontend sends the literal string `"$i"` unchanged. The backend marks it as a placeholder when it compiles the plan and binds it when the iteration's state message reaches the worker (apache#8639). Operators outside every block behave as before. ``` Loop Start (i = 0) -> Limit(limit: "$i") -> Loop End ^ panel offers $i; canvas valid; sent as "$i" rename i to j in the Loop Start -> Limit warns "$i is not a variable of an enclosing block; ..."; Run stays enabled delete a link upstream -> Limit outside the block -> "must be integer" disable the Loop End -> the same: the backend compiles and runs only enabled operators ``` | Piece | What it does | |---|---| | Block membership (`enclosingLoopStarts`, `loop-block.util.ts`) | Worked out in the frontend from the graph as it stands: every enabled operator, valid or not, and the links between them, which is the plan the backend compiles and runs (`WorkflowGraph.getSubDAG`). It is a port of the backend's `LogicalPlan.enclosingLoopStarts` (apache#8639), with the same treatment of a Loop Start that no Loop End closes, nested and sequential blocks, branches, and outermost-first order. `LogicalPlanSpec` and `loop-block.util.spec.ts` both run one shared fixture (`common/workflow-compiler/src/test/resources/loop-blocks.json`: named cases plus random graphs, with the backend's answers), so the two cannot drift apart, and a change to the fixture runs the frontend stack too (`labeler.yml`). An unfinished operator elsewhere on the canvas no longer keeps an operator in a block it has left. | | Loop variables (`extractLoopVariables`, `loopVariablesInScope`) | Reads the variables from each enclosing Loop Start's `initialization` code, following Python's module-level binding: assignments at any depth outside a `def`/`class` body, for-loop targets, one-line compound bodies such as `try: k = 1`, and names an import binds (`from math import pi`, `import numpy as np`, relative modules, parenthesized lists). | | Property panel | Inside a block, each plain primitive field becomes a text box that offers `$name` for every variable in scope. So does a field whose accepted values depend on another field of its row (value rules, as on the sklearn trainers' hyperparameter values): its rules still judge every value that is not a reference, and the text box suggests the values they accept after the variables. Enums, arrays and custom widgets keep their own controls, and so does a property the backend marks `"noLoopVariable": true` (apache#8750: settings the plan is built from, such as Projection's columns), together with everything nested under it. A `$name` that no enclosing block declares gets a warning under the field; the operator stays valid, and the run itself reports the name if no Loop Start sets it. | | Canvas validation | Inside a block, the schema `type` error is waived exactly where a reference sits in a primitive-typed property. `loopScopeChangeStream` emits on every edit that can change an operator's scope: an operator or link added or deleted, an operator disabled or enabled, or a Loop Start's properties changed. On each one, validation re-validates every operator that holds a reference, so the Run button follows edits two hops away. | | Open panel | Rebuilt on `loopScopeChangeStream` when the operator's scope changed. | | Shared editing (`updateYTypeFromObject`) | A string now replaces a number or boolean instead of throwing, which storing `"$i"` over a number needs. | Panel behavior for a field inside a block: | Field type | Typed text | Stored value | |---|---|---| | integer | `7` | `7` (number) | | integer / number / boolean | `$i` | `"$i"` | | number | `0.5`, typed one key at a time | `0.5`; partial text such as `7.` stays text until complete | | boolean | `true` / `FALSE` | `true` / `false`; other text is flagged | | string | `hello` / `$i` | `"hello"` / `"$i"` | | value rules (e.g. `C`, which must be > 0) | `$i` / `1.0` / `-1` | `"$i"` / `"1.0"` / `"-1"`, flagged by the rules | | any | `$foo` (undeclared) | `"$foo"`, with a warning; the operator stays valid | **Related.** apache#8639 (on `main`) binds the references and computes the blocks. apache#8750 (on `main`) adds the `noLoopVariable` marks. **Screenshots.** The Limit outside the loop keeps the number field it has on `main`. The Limit inside the loop gets a text box that suggests the loop's variables and flags a name the loop does not declare. The third shot predates the change of that flag to a warning, which leaves Run enabled. | Before: outside a loop (same as `main`) | After: inside a loop | After: an undeclared name | |---|---|---| |  |  |  | The workflow in the shots: `Text Input -> Loop Start (i = 0) -> Limit ($i) -> Loop End`, plus a second Limit fed straight from Text Input, outside the loop.  ### Any related issues, documentation, discussions? Closes apache#8636 ### How was this PR tested? New and updated Vitest specs, run from `frontend/`: | Spec | Covers | |---|---| | `LogicalPlanSpec` (backend) | `enclosingLoopStarts` on every case and random graph of the shared fixture, outermost first. The fixture's Loop Start and Loop End are read by the backend's JSON type names, so a renamed type fails here. The walk-based check on named cases and 400 random DAGs stays. | | `loop-block.util.spec.ts` (new) | The frontend's port on every case and random graph of the same fixture, and on every operator type it names. A graph with a cycle, which the backend rejects but the canvas allows, ends and counts as outside every block. A link to an operator it was not given is ignored. | | `loop-variable.util.spec.ts` | Assignment forms; if/else, try/except, while and for bodies; for targets; one-line compound bodies; import forms (`from ... import`, aliases, relative modules, parenthesized lists, `import a.b`, several on one line; `*` binds nothing); def/class bodies and with/except-as names excluded; Python keywords never taken for names. `loopVariablesInScope`: outermost first without duplicates, undefined outside every block, a disabled operator and its links left out. `loopScopeChangeStream` emits on adds, deletes, disables, enables and Loop Start property changes, and on nothing else. | | `loop-variable-field.util.spec.ts` | Complete vs partial numerals (`7.`, `-0.`, `1e-` kept as text); the parser sets the control without redrawing the box; the string `""` default; the boolean validator rejects `maybe`. On a field with value rules: a reference passes the rules, every other value is still judged by them, the re-validation hook survives, and `props` stays the one object `templateOptions` names. An unknown name gives a warning, not a validation error. `inheritNoLoopVariable` carries a mark to every schema under it, through each keyword that holds a schema and through `$ref`s, without marking a definition an unmarked property shares. | | `loop-variable-input.component.spec.ts` (new) | Rendered through a real formly form: typing `1.05`, `-0.5` and `7.5` one key at a time stores the right number and keeps the box as typed; `$i` and integers; formly attributes reach the input. An unknown name shows the warning. With value rules: the suggestions add the accepted values of the row's parameter (empty box, typed prefix in either case, `$` prefix), and a changed parameter re-judges the value. | | `validation-workflow.service.spec.ts` | The waiver applies only to primitive types. An unknown name leaves the operator valid. Canvas state follows each edit at once: a block opened or closed, and an operator moved out of its block and back while another operator is invalid. Edits two hops away are covered too: an upstream link deleted, a block closed by a link between two other operators, an operator inserted into the body, and a Loop End disabled and enabled again. Each of these four fails if validation stops listening to `loopScopeChangeStream`. | | `operator-property-edit-frame.component.spec.ts` | Primitive fields converted inside a block; file picker, enum and array keep their controls; the form rebuilds when a link, a disabled or enabled operator, or the Loop Start's variables change its scope, and only then. The open operator leaves its block and comes back while another operator on the canvas is invalid. A value-rules field takes `$i` inside a block and keeps its own control outside; with a schema enum or a custom widget it keeps its control inside too. A `noLoopVariable` property, and a field nested under one, keeps its control inside a block. Typing `$` into a number field inside a block keeps the form as it is. | | `shared-editing.interface.spec.ts` | A string replaces a number in a Y.Map instead of throwing. | Each new behavior spec was checked red against the pre-fix code before the final run. ``` yarn ng test --watch=false --include=<the 7 loop and shared-editing specs> # 488 passed, 1 skipped (existing it.skip) yarn ng test --watch=false # 240 files, 6815 passed, 1 skipped (existing it.skip) yarn exec tsc --noEmit -p src/tsconfig.app.json (and src/tsconfig.spec.json) # clean yarn exec ngc -p src/tsconfig.app.json --noEmit # AOT + strictTemplates clean yarn prettier-eslint --list-different <changed files> && yarn eslint <changed files> # clean sbt "WorkflowCompiler/testOnly *LogicalPlanSpec" # 27 passed sbt scalafmtCheckAll "WorkflowCompiler/scalafixAll --check" # clean ``` `yarn ng build` did not complete locally, because the worktree's `node_modules` is a symlink; CI's frontend build passes on macOS, Ubuntu and Windows. An earlier revision was checked by hand on a local deployment with apache#8639 and apache#8642 (the test branch in apache#8645): the loop workflow runs, and the panel behaves as in the screenshots. The changes since (membership from the whole graph, imports, the warning, `noLoopVariable`) are covered by the specs above. Manual steps for the reviewer (Chrome, desktop): link `Loop Start -> Limit -> Loop End` and set the Loop Start initialization to `i = 0`. Open `Limit`: its integer field is a text box, typing `$` offers `$i`, and choosing it keeps the operator valid. Typing `$foo` shows a warning, "$foo is not a variable of an enclosing block; ...", and Run stays enabled. Delete a link so `Limit` leaves the block, or disable the Loop End: the field is a number box again and `$i` is a type error. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Fable 5.1, Claude Opus 5.5) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
What changes were proposed in this PR?
This draft puts the whole loop-variable feature in one branch so it can be tried and tested end to end: inside a Loop Start ... Loop End block, any operator's property can hold
$i, and every iteration runs the operator with the current value of the loop variablei.It combines four PRs, with their commits unchanged (#8671 arrives as a merge of its branch):
feat(amber): bind $i loop-variable references in operators,feat(amber): bind $i in the operator's setting when the state arrives$iin string and numeric/boolean properties, keeps it only for operators inside a loop block, and writes the variable's value into the operator's setting when the iteration's state arrives.feat(pyamber): register loop_state before process_state,refactor(pyamber): rename loop_state to stateself.state.feat(frontend): accept $variable references in loop blocks$iin any primitive field, suggests the enclosing loop's variables when you type$, and flags a name no enclosing loop declares.feat(frontend): let value-rule fields take loop-variable references$i, keeps its rules for every other value, and still suggests the values they accept.feat: bind $i in generated Python operators' text properties,feat: bind numeric and boolean $i in generated operators,refactor: rename register_loop_state to register_state$iinside a loop: text properties, and numeric and boolean ones wherever the code holds the value as it is.Outside every loop nothing changes: a text such as
$AAPLstays plain text.Any related issues, documentation, discussions?
Combines #8639, #8642, #8640 and #8671. Related to #8635, #8641, #8636 and #8669. User-facing overview: #8643
How was this PR tested?
On this branch, after merging #8671:
mainequals its PR's version (#8639, #8642, #8640, #8671)testPythonOperatorDescriptorSpecand the loop descriptor specsTest/compile(incl.LoopIntegrationSpec)Each part's own tests are described in #8639, #8642, #8640 and #8671.
LoopIntegrationSpec's new case (TextInput -> Loop Start -> Filter(line = $i) -> Loop End, 3 rows) runs in theamber-integrationCI job.Manual steps (Chrome, desktop): link
Loop Start -> Limit -> Loop Endand set the Loop Start initialization toi = 0. OpenLimit: typing$in its integer field offers$i, and choosing it keeps the operator valid. Run the workflow, and each iteration applies the currenti.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Fable 5.1, Claude Opus 5.5)
🤖 Generated with Claude Code