Просмотр исходного кода

docs: design for the Tier 1 nodes, and the two port changes they need

fszontagh 1 месяц назад
Родитель
Сommit
fd006b5bea
1 измененных файлов с 178 добавлено и 0 удалено
  1. 178 0
      docs/superpowers/specs/2026-08-04-tier-1-nodes-design.md

+ 178 - 0
docs/superpowers/specs/2026-08-04-tier-1-nodes-design.md

@@ -0,0 +1,178 @@
+# Tier 1 Nodes - Design
+
+Date: 2026-08-04
+Source: [docs/node-roadmap.md](../../node-roadmap.md), Tier 1
+
+## Goal
+
+Ship the eleven Tier 1 data-shaping and flow-control nodes, so that ordinary
+workflows stop reaching for a Code node to rename a field, branch three ways or
+sort a list. Two small pieces of platform work come first, because two of the
+nodes cannot exist without them.
+
+## Background: how ports work today
+
+A node's ports travel a fixed path:
+
+```
+nodes/<cat>/<node>.js
+  -> regex parse in src/webserver/nodes/node_store.cpp (parseNodeCode)
+  -> NodeIO structs in the node store
+  -> database
+  -> gRPC, proto/runner.proto NodeDefinition / NodeDefinitionWithCode
+  -> REST, src/webserver/api/node_controller.cpp
+  -> data.outputs in webui/src/components/workflow/WorkflowNode.tsx
+```
+
+Three properties of that path drive this design:
+
+1. **Ports belong to the node type, not the node instance.** Nothing on the path
+   reads a placed node's config, so a node cannot vary its ports per instance.
+2. **`const outputs = [...]` is parsed; `const inputs = [...]` is not.**
+   `node_store.cpp` parses the outputs array and defaults inputs to a single
+   `data` handle. `WorkflowNode.tsx` draws exactly one unnamed target handle.
+3. **Execution does not care about port count.** `workflow_engine.cpp` matches
+   `conn.source_output` against the `_activeBranch` string as opaque text. Its
+   only reference to the port list is a `outputs.size() > 1` test used to decide
+   whether a *disabled* node decides a branch.
+
+A fourth property shapes several nodes: a connection from a named port on a node
+that emits **no** `_activeBranch` reads `output[source_output]` directly. So
+named ports without a branch marker are all live at once, where a node that sets
+`_activeBranch` activates exactly one.
+
+## Part 1 - Dynamic output ports
+
+A node file may declare an optional `dynamicOutputs` literal beside its static
+`outputs`:
+
+```javascript
+const outputs = [
+    { name: 'fallback', displayName: 'Fallback', type: 'any', color: '#6b7280' }
+];
+
+const dynamicOutputs = {
+    from: 'rules',
+    namePrefix: 'case',
+    labelFrom: 'label',
+    color: '#3b82f6'
+};
+```
+
+Read as: for each entry in `config.rules`, render a port named `case0`, `case1`,
+... labelled by that entry's `label` field, then render the static `outputs`
+after them.
+
+It is a data literal rather than a function because the WebUI cannot execute
+node JavaScript - a declaration is the only form that crosses both the C++ and
+the TypeScript boundary.
+
+Port names are **positional** (`case0`), not value-derived (`case_paid`), so an
+edge survives editing a rule's label or match value. Deleting a rule shifts the
+names after it; the editor drops edges whose `sourceHandle` no longer exists.
+
+### Changes
+
+| File | Change |
+| --- | --- |
+| `src/webserver/nodes/node_store.cpp` | Parse `const dynamicOutputs = {...}` via the existing `jsLiteralToJson` helper, alongside the three schema parsers |
+| `src/webserver/nodes/node_store.hpp` | `nlohmann::json dynamic_outputs` member, carried through `toJson`/`fromJson` |
+| `proto/runner.proto` | `string dynamic_outputs` on `NodeDefinition` (field 13) and `NodeDefinitionWithCode` (field 17), a JSON string like `config_schema` |
+| `src/webserver/grpc/node_sync_service.cpp` | Pass through on gRPC |
+| `src/webserver/api/node_controller.cpp` | Pass through on REST, both list and single |
+| `src/runner/node_registry.cpp` | Parse `dynamic_outputs` from proto |
+| `src/runner/workflow_engine.cpp` | The two `outputs.size() > 1` disabled-node checks become "more than one output **or** has dynamicOutputs" |
+| `webui/src/api/workflows.ts` | `dynamicOutputs?` on `NodeDefinition` |
+| `webui/src/components/workflow/WorkflowNode.tsx` | Derive the handle list from `dynamicOutputs` plus the instance config |
+| `webui/src/pages/WorkflowEditorPage.tsx` | Same derivation where node data is built; drop edges pointing at ports that no longer exist |
+
+The execution path needs no change beyond the disabled-node check.
+
+## Part 2 - Named inputs
+
+`merge` needs two input handles. Today two edges into one node both land on
+`input.data` and the second silently overwrites the first.
+
+Mirror the outputs pipeline for inputs: parse `const inputs = [...]` in
+`node_store.cpp`, carry it through proto and REST as the outputs already are,
+and render one target handle per entry in `WorkflowNode.tsx`, positioned with
+the existing `calculateHandlePosition` helper.
+
+This was chosen over making the engine collect same-target edges into an array,
+which would need no UI work but would change behaviour for existing workflows
+that already fan two edges into one node.
+
+Nodes that declare no `inputs` keep the single default `data` handle, so no
+existing node changes.
+
+## Part 3 - The nodes
+
+All eleven live in `nodes/core/`. Node code is stored as one blob per node with
+no imports between files, so the field-path helper is copied into each node that
+needs one. `if-condition.js` and `loop.js` each carry a different variant today;
+the new nodes use one agreed version: dot paths, `[n]` and bare numeric array
+indices, `.length` on arrays, `undefined` for anything missing.
+
+Config values arrive pre-evaluated. `WorkflowEngine::evaluateExpressions` walks
+the config recursively before `execute()`, and a config string that is exactly
+one `{{...}}` keeps its native type instead of being stringified. Nodes must not
+re-interpolate.
+
+| File | Node | Ports | Config |
+| --- | --- | --- | --- |
+| `set-fields.js` | Set / Edit Fields | main | `fields: [{name, value}]`, `mode: keep-all \| only-set`. Dot paths in `name` build nested objects |
+| `switch.js` | Switch | dynamic `case0..n` + `fallback` | `field`, `rules: [{label, operator, value}]`. First match wins, sets `_activeBranch` |
+| `filter.js` | Filter | `kept`, `discarded`, both live | Condition list and operators matching `if-condition`, applied per array item |
+| `merge.js` | Merge | main, two named inputs | `mode: append \| combine-by-key \| choose-first`, `key` for combine |
+| `split-out.js` | Split Out | main | `field`, `include: all-fields \| selected` for parent scalars |
+| `aggregate.js` | Aggregate | main | `field` for the collected array, optional `groupBy` |
+| `sort-limit-dedupe.js` | Sort / Limit / Dedupe | main | `sort: [{field, direction}]`, `limit`, `dedupeBy`. One node because the three are always used together and each alone is ten lines |
+| `template.js` | Template | main | `template` multi-line text, rendered with `smartbotic.utils.interpolate` |
+| `json.js` | JSON | main | `operation: parse \| stringify \| extract`, `path`, `onError: throw \| null` |
+| `datetime.js` | Date & Time | main | `operation: now \| parse \| format \| add \| subtract \| diff`, `format`, `amount`, `unit`, `offset` |
+| `stop-and-error.js` | Stop and Error | none | `message`, thrown so `error-trigger` picks it up |
+
+### Timezones
+
+QuickJS here is Bellard's build with no ICU (`cmake/Dependencies.cmake`), so
+`Intl` does not exist and named zones like `Europe/Budapest` cannot be resolved.
+`datetime.js` supports UTC, host local time, and explicit `+02:00` style offsets,
+and says so in its description. Shipping a DST rule table inside the node was
+rejected: it goes stale silently when a country changes policy.
+
+### Conventions
+
+Per `docs/nodes.md`: four-space indentation, single quotes, no inline comments
+inside schema literals, and throw on failure rather than returning
+`success: false`. The schema parser is regex-based, so a literal it cannot parse
+fails quietly - which is why registration is verified explicitly below.
+
+## Part 4 - Verification
+
+Services run as bare processes, not through the user units, so the C++ work
+means stopping and restarting the webserver and runner by hand. Build and
+restart happen once, before any node is migrated, because `switch` and `merge`
+cannot register correctly until the parser understands `dynamicOutputs` and
+`const inputs`.
+
+Per node:
+
+1. `POST /api/v1/nodes/migrate`, then `GET /api/v1/nodes` to confirm the parser
+   accepted the file, its config schema and its ports.
+2. `POST /api/v1/workflows` with a click-trigger feeding the node under test,
+   with fixed input data.
+3. `POST /api/v1/workflows/{id}/execute`, then assert on the node's recorded
+   output.
+4. For `switch`, `filter` and `merge`, downstream marker nodes on each port,
+   asserting exactly the expected ones ran. For `switch` this is the proof that
+   a dynamically-named port such as `case2` actually routes.
+
+Test workflows are deleted after each node, leaving the database as it started.
+
+## Out of scope
+
+Tier 2 through 5. The two roadmap entries that need engine work -
+**Respond to Webhook** and **Execute Sub-workflow** - stay unbuilt; this spec
+covers only the smaller engine changes the Tier 1 nodes themselves require.
+
+Rebuilding QuickJS against ICU for real timezone support is separate work.