diff --git a/CHANGELOG.md b/CHANGELOG.md index 55069cd..d2aa8b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,29 @@ called out explicitly under **Changed** or **Removed**. ## [Unreleased] +### Added + +- **Automation specs can author loops.** A spec names the array variable, + its item variable, a `loop` step with `step_loop: {array_variable, + item_variable}`, the body, a `next_or_finish` closer and any `exit_loop` + steps, and `create`, `update` and `diff` handle them like any other step. + A loop is always sent as a branch group initiator. `diff` no longer + reports the keys Kizen adds to a loop's item variable. The plan preview + no longer warns about a fan-out on every loop. The plan refuses a loop + nested more than three deep, or one over the same array as a loop around + it, as Kizen's builder does. `kizen docs show automation` has a worked + example. + ### Fixed +- **`automations diff` no longer reports Kizen's defaults as changes when a + spec leaves them out.** An omitted `error_notification_email`, + `deduplicate_array`, `array_separator` (on a static list source), + `value_html` (on an `update_variable` source), or `description` on the + steps and trigger Kizen gives a fixed one (`kizen docs show automation` + lists them) is compared as the value Kizen stores for it. So is a null + key in a top-level or `initialize_variable` variable definition. A spec + that sets a different value still shows the change. - **CLI writes no longer fail on automations with loops, and `show` renders loops.** `activate`, `deactivate`, `move`, `steps add|edit|remove` and `roundtrip --execute` failed with `HTTP 400: … step_loop must be provided diff --git a/src/kizen_builder/docs/specs/automation.md b/src/kizen_builder/docs/specs/automation.md index 378f06d..eacd025 100644 --- a/src/kizen_builder/docs/specs/automation.md +++ b/src/kizen_builder/docs/specs/automation.md @@ -112,18 +112,11 @@ branches) and 2026-09-29 (goals). `merge_branches` child to rejoin them. Lanes can be `branch` steps themselves, merged or not, and a `branch` can follow a merge. Put a `branch` step in front of any fan-out: a plain step with two or more children still saves, but the -plan preview warns about it. Kizen's 2026-09-28 migration inserted a `branch` -step ahead of every existing fan-out, and an `update` from a spec without one -removes it again. Confirmed live 2026-09-28. - -**Loops** are initiators too. A `loop` step's body hangs off the loop, its one -`next_or_finish` child closes the body, and the steps after the loop hang off -the `next_or_finish`. `exit_loop` leaves the loop from inside the body. -`next_or_finish` and `exit_loop` take no config block. `step_loop` names an -array variable of records, strings or UUIDs and a separate item variable, each -declared by an `initialize_variable` step. The item variable is a single -variable of the array's type. `--dry-run` checks loops against -the server's rules too. Confirmed live 2026-10-27. +plan preview warns about it. A group's closer (`merge_branches` or +`next_or_finish`) isn't a lane, so it doesn't count toward that warning. +Kizen's 2026-09-28 migration inserted a `branch` step ahead of every existing +fan-out, and an `update` from a spec without one removes it again. Confirmed +live 2026-09-28. **Skipping a condition or goal** (`should_skip_execution: true`) needs `continue_with_branch: "yes"` or `"no"`, the branch the run takes past it. The @@ -141,6 +134,87 @@ sets a step's own severity. Omitted, the step inherits. Confirmed live (`account_region`), *not* object-qualified — it's the filter DSL, same as `records list --filter` and saved views. +### Loops + +A loop runs its body once per item of an array variable. Loops are branch +group initiators, closed by `next_or_finish` instead of `merge_branches`. A +spec names all six parts; the CLI adds none of them: + +1. **The array variable:** an `initialize_variable` step with `is_array: true`. +2. **Its item variable:** an `initialize_variable` step with `sources: []`, + declaring a single variable of the array's type (for records, the same + `data_subtype`). It needs no sources, though the server accepts one. Each + loop needs its own. +3. **The `loop` step.** `step_loop` names both variables by **name**; their ids + are a 400. The CLI always sends a loop as `is_branch_group_initiator: true`, + because the server stores true even when false is sent. +4. **The body**, hanging off the loop. +5. **One `next_or_finish` child of the loop**, which closes the body. The steps + after the loop hang off it. +6. **Optional `exit_loop` steps** inside the body. + +Both variable steps go at the front of the automation, like every +`initialize_variable`. Confirmed live 2026-10-27. + +```json +{ "name": "Visit every account", "api_name": "visit_every_account", + "type": "global", + "steps": [ + { "key": "accounts", "parent_key": null, "step_type": "initialize_variable", + "order": 0, "description": "accounts = []", + "action_initialize_variable": { "sources": [], "array_aggregation_mode": "add_all", + "variable": { "name": "accounts", "data_type": "entity", "is_array": true, + "data_subtype": "" } } }, + { "key": "status", "parent_key": "accounts", "step_type": "initialize_variable", + "order": 1, "action_initialize_variable": { "sources": [], + "variable": { "name": "status", "data_type": "string" } } }, + { "key": "account", "parent_key": "status", "step_type": "initialize_variable", + "order": 2, "description": "Item Reference Variable", + "action_initialize_variable": { "sources": [], + "variable": { "name": "accounts_item", "data_type": "entity", + "data_subtype": "" } } }, + { "key": "find", "parent_key": "account", "step_type": "search_records", + "order": 3, "action_search_records": { "custom_object": "accounts", + "filter_type": "all_records", "destination_variable": "accounts" } }, + { "key": "each", "parent_key": "find", "step_type": "loop", "order": 4, + "description": "For each item in 'accounts'", + "step_loop": { "array_variable": "accounts", "item_variable": "accounts_item" } }, + { "key": "visit", "parent_key": "each", "step_type": "update_variable", "order": 5, + "action_update_variable": { "variable": { "name": "status", "data_type": "string" }, + "sources": [ { "source_type": "static", "source_subtype": "string", + "value": "visited" } ] } }, + { "key": "next", "parent_key": "each", "step_type": "next_or_finish", "order": 6 }, + { "key": "done", "parent_key": "next", "step_type": "update_variable", "order": 7, + "action_update_variable": { "variable": { "name": "status", "data_type": "string" }, + "sources": [ { "source_type": "static", "source_subtype": "string", + "value": "done" } ] } } + ] } +``` + +- **Array types:** strings, UUIDs and records (`data_type` `string`, `uuid` or + `entity`). Kizen rejects an array of any other type at the + `initialize_variable` step. A `search_records` destination can be the loop's + array, as above. Confirmed live 2026-10-27. +- **Descriptions:** a CLI-created loop reads back as "Loop" and its item step as + "Initialize Variable". The UI writes "For each item in ''" and "Item + Reference Variable". Set `description` to match the UI. Confirmed live + 2026-10-27. +- **`diff`** ignores `is_array_item_variable` and `source_array_variable`, which + the server adds to the item variable from the loop. It reads a null key in a + top-level or `initialize_variable` variable definition as absent. Confirmed + live 2026-10-27. +- **`exit_loop`** ends the innermost loop it sits in, and an outer loop carries + on with its next item. It can be the loop's direct child, sit under a + condition in the body, or follow an inner loop inside an outer body. After + the loop's `next_or_finish` it is a 400. Confirmed live 2026-10-27. +- **Body lanes:** a body can be empty or have two lanes. Two lanes get the + fan-out warning, like any plain fan-out. Confirmed live 2026-10-27. +- **Nesting:** a loop can sit in another loop's body, up to three levels, and a + nested loop needs a different array from every loop around it. Kizen's + builder refuses both, so the CLI refuses them at plan time (`--dry-run` shows + it). The API accepts both: six levels, and one array at two levels, each + confirmed live 2026-10-27. + --- ## Wired trigger types (11) @@ -202,8 +276,8 @@ to wait for a reply. Its `wait_type` is `never`, `delay` or | `search_records` | `action_search_records` | search an object, write results into an array variable | | `branch` | none | run its children in parallel — see Branch groups | | `merge_branches` | none | rejoin a branch group — see Branch groups | -| `loop` | `step_loop` | run its body once per item of an array variable — see Branch groups | -| `next_or_finish` | none | close a loop body — see Branch groups | +| `loop` | `step_loop` | run its body once per item of an array variable — see Loops | +| `next_or_finish` | none | close a loop body — see Loops | | `exit_loop` | none | leave the loop from inside its body | `search_records` (the automation-builder equivalent of `records list @@ -896,6 +970,18 @@ changed field is identified by that octet alone; an added or removed step/trigger carries the whole step/trigger, full `id` included, in its `--json` leaf value. +A key the spec leaves out compares as the value Kizen stores when it's +omitted, so a spec that leaves out a default shows no change. A non-default +value on either side still shows. Confirmed live 2026-10-27: + +| Key | Omitted reads back as | +|---|---| +| `error_notification_email` | `''` | +| `deduplicate_array` on `initialize_variable`/`update_variable` | `false` | +| `array_separator` on a static source whose `value` is a list | `'comma'` | +| `value_html` on an `update_variable` source | `''` | +| `description` on a step or trigger | a fixed text per type: `Initialize Variable`, `Update Variable`, `Action: Search Records`, `Loop`, `Next/Finish`, `Exit Loop`, `Condition`, `Branch`, `Merge Branches`, `Delay`, `Stop Execution`, and `Manual` for the manual trigger. It is written on create and on a PUT that omits it, so an update from a spec without one replaces a description set in the UI. Other types compare as sent | + ### Read→write transforms Each of these was discovered via a live 400, 500, or silent data loss: @@ -914,12 +1000,12 @@ Each of these was discovered via a live 400, 500, or silent data loss: | Step/trigger orders with gaps or nulls | sequential 0..N | | `change_field_value.fields_to_clear` (expanded field objects) | bare field UUIDs | | `start_automation.automation_variable_overrides` — flat list, every ref expanded (`target_automation`, `variable_to_override`, plus a `value_source`-specific ref) | grouped by target automation id: `[{automation_id, variable_overrides: [...]}]`, and the unwrapping is non-uniform — `context_entity_field` → bare UUID, `variable_to_override`/`automation_variable` → bare **name**, `specific_value` → passthrough (already a bare scalar on read), `blank` → no key at all | -| `is_branch_group_initiator` on every step | echoed as read, default `false`. Dropped, every merge step 400s: "Merge step's parent (…) must be a branch group initiator step." Confirmed live 2026-09-28 | +| `is_branch_group_initiator` on every step | echoed as read, default `false`, and always `true` on a loop (see Loops). Dropped, every merge step 400s: "Merge step's parent (…) must be a branch group initiator step." Confirmed live 2026-09-28 | | `continue_with_branch`, present only when set | echoed when non-null. A condition or goal with `should_skip_execution: true` 400s without it. Confirmed live 2026-09-28 | | `error_notification_severity_level` on every step | echoed as read. Omitted, the step is silently reset to `inherit`. Confirmed live 2026-09-28 | | `branch` and `merge_branches` steps, read with no config block | no `action_*` block written. Confirmed live 2026-09-28 | | `step_loop` with `array_variable`/`item_variable` as variable definitions | bare variable **names**; their ids are a 400 ("The loop must iterate an array variable…"). `next_or_finish` and `exit_loop` are read with no config block, and none is written. Confirmed live 2026-10-27 | -| A loop's item variable read with `is_array_item_variable: true` and `source_array_variable: ` | echoed as read. The server sets both itself from the loop, so sending them is harmless and omitting them changes nothing. Confirmed live 2026-10-27 | +| A loop's item variable read with `is_array_item_variable: true` and `source_array_variable: ` | echoed as read. The server sets both itself from the loop, so sending them is harmless and omitting them changes nothing. `diff` ignores both. Confirmed live 2026-10-27 | | Extraction/transcription `input_field` plus its mirrored `context_entity_field` `input_value_source` | `input_field` only — see [Extraction input](#extraction-input) | | `modify_related_entities.automation_variables` (expanded variable definitions) and the block's own `id` | not sent; `record_source_variables` (bare names) is the write key | @@ -964,7 +1050,9 @@ inconsistent: variable of the array's type (and, for records, the same object), and no two loops share one. - There is no nesting limit (6 deep saves) and no limit on body lanes. + The server has no nesting limit (6 deep saves) and no limit on body lanes. + The CLI adds two rules of its own, from Kizen's builder: at most three + levels, and no loop over the same array as a loop around it (see Loops). ### Unlisted step types diff --git a/src/kizen_builder/models/spec/automations.py b/src/kizen_builder/models/spec/automations.py index 536633b..212ebc2 100644 --- a/src/kizen_builder/models/spec/automations.py +++ b/src/kizen_builder/models/spec/automations.py @@ -182,7 +182,8 @@ class AutomationStepDef(BaseModel): description=( "Set on a `branch`, `condition` or `goal` step whose lanes rejoin. " "The step then needs exactly one `merge_branches` child, and steps " - "after the merge hang off that merge step." + "after the merge hang off that merge step. A `loop` is always sent " + "as an initiator, whatever this says." ), ) continue_with_branch: Literal["yes", "no"] | None = Field( diff --git a/src/kizen_builder/tools/planners/automations.py b/src/kizen_builder/tools/planners/automations.py index cec6650..a0a4ff4 100644 --- a/src/kizen_builder/tools/planners/automations.py +++ b/src/kizen_builder/tools/planners/automations.py @@ -585,12 +585,20 @@ def _automation_preview( } +# A group's closer is a child of its initiator, not a parallel lane. +_CLOSER_STEP_TYPES = frozenset({"merge_branches", "next_or_finish"}) + + def _fan_out_warnings(auto: AutomationDef) -> dict[str, Any]: """Parallel lanes belong under a `branch` step. A plain fan-out still saves, but Kizen's 2026-09-28 migration put a `branch` step in front of every one, and an update from a spec without it removes it again.""" lanes = Counter( - s.parent_key for s in auto.steps if s.parent_key and s.parent_branch is None + s.parent_key + for s in auto.steps + if s.parent_key + and s.parent_branch is None + and s.step_type not in _CLOSER_STEP_TYPES ) types = {s.key: s.step_type for s in auto.steps} warnings = [ @@ -1220,7 +1228,9 @@ def _build_step_payload( if step.description: p["description"] = step.description # Emitted only when set, so a spec that predates them builds unchanged. - if step.is_branch_group_initiator: + # A loop is always an initiator: the server stores true even when false + # is sent (confirmed live 2026-10-27). + if step.is_branch_group_initiator or step.step_type == "loop": p["is_branch_group_initiator"] = True if step.continue_with_branch is not None: p["continue_with_branch"] = step.continue_with_branch diff --git a/src/kizen_builder/translate.py b/src/kizen_builder/translate.py index 8ea0c75..024d6dc 100644 --- a/src/kizen_builder/translate.py +++ b/src/kizen_builder/translate.py @@ -454,12 +454,18 @@ def _branch_group_problems(steps: list[dict[str, Any]]) -> list[str]: # A loop may iterate an array of these (records, text or UUIDs). _LOOPABLE_DATA_TYPES = frozenset({"entity", "string", "uuid"}) +# Kizen's builder holds loops three deep. The API accepts more (six seen, +# confirmed live 2026-10-27), but the CLI refuses them. +_MAX_LOOP_DEPTH = 3 + def _loop_problems(steps: list[dict[str, Any]]) -> list[str]: - """The server's loop rules, each one a live 400 (confirmed 2026-10-27). - The body hangs off the loop, its one `next_or_finish` child closes it, and - what follows the loop hangs off the `next_or_finish`. A graph with no - loop, `next_or_finish` or `exit_loop` passes untouched.""" + """The server's loop rules, each one a live 400 (confirmed 2026-10-27), + plus two from Kizen's builder that the API doesn't enforce: nesting depth + and one array at two levels. The body hangs off the loop, its one + `next_or_finish` child closes it, and what follows the loop hangs off the + `next_or_finish`. A graph with no loop, `next_or_finish` or `exit_loop` + passes untouched.""" problems: list[str] = [] by_key = {s.get("key"): s for s in steps} declared: dict[str, dict[str, Any]] = {} @@ -495,6 +501,7 @@ def _loop_problems(steps: list[dict[str, Any]]) -> list[str]: ) else: item_owners[item] = key + problems.extend(_builder_loop_problems(s, by_key)) elif step_type == "next_or_finish": pk = s.get("parent_key") if (by_key.get(pk) or {}).get("type") != "loop": @@ -547,20 +554,61 @@ def _loop_variable_problems( return [] +def _builder_loop_problems( + loop: dict[str, Any], by_key: dict[Any, dict[str, Any]] +) -> list[str]: + """Kizen's builder refuses these, though the API accepts them (confirmed + live 2026-10-27), so the CLI refuses them to keep the automation editable.""" + key = loop.get("key") + around = enclosing_loops(loop, by_key) + problems: list[str] = [] + depth = 1 + len(around) + if depth > _MAX_LOOP_DEPTH: + problems.append( + f"loop '{key}' is nested {depth} levels deep; Kizen's builder allows " + "at most three. The API accepts deeper loops, but the CLI refuses them " + "so the automation stays editable in the UI." + ) + array = (loop.get("step_loop") or {}).get("array_variable") + outer = next( + ( + o + for o in around + if (o.get("step_loop") or {}).get("array_variable") == array + ), + None, + ) + if array is not None and outer is not None: + problems.append( + f"loop '{key}' iterates '{array}', which loop '{outer.get('key')}' " + "around it already iterates; Kizen's builder needs a different array " + "variable at each level. The API accepts it, but the CLI refuses it so " + "the automation stays editable in the UI." + ) + return problems + + def _in_loop_body(step: dict[str, Any], by_key: dict[Any, dict[str, Any]]) -> bool: - """Whether walking up from ``step`` reaches a loop through one of its body - steps. Arriving through a loop's `next_or_finish` means the step comes - after that loop, so the walk carries on to any loop around it.""" - cur, seen = step, set() + return bool(enclosing_loops(step, by_key)) + + +def enclosing_loops( + step: dict[str, Any], by_key: dict[Any, dict[str, Any]] +) -> list[dict[str, Any]]: + """The loops ``step`` sits inside, innermost first: walking up, the loops + reached through one of their body steps. Arriving through a loop's + `next_or_finish` means the step comes after that loop, so it isn't + included and the walk carries on to any loop around it.""" + cur, seen, loops = step, set(), [] while cur.get("parent_key") and cur.get("key") not in seen: seen.add(cur.get("key")) parent = by_key.get(cur["parent_key"]) if parent is None: - return False + break if parent.get("type") == "loop" and cur.get("type") != "next_or_finish": - return True + loops.append(parent) cur = parent - return False + return loops # --------------------------------------------------------------------------- @@ -707,6 +755,76 @@ def _diff(a: Any, b: Any, path: str) -> list[tuple[str, Any, Any]]: "error_notification_severity_level": "inherit", } +# Keys a spec may omit that read back as the server's default (confirmed live +# 2026-10-27), so an omitted one compares as that default. A non-default value +# on either side still diffs. +_WIRE_TOP_DEFAULTS = {"error_notification_email": ""} +_VARIABLE_BLOCK_DEFAULTS = {"deduplicate_array": False} + +# What the server writes when a step or trigger is sent without a +# `description`, on create and on a PUT that omits it (confirmed live +# 2026-10-27). A type missing here compares as sent. +_DEFAULT_DESCRIPTIONS = { + "branch": "Branch", + "condition": "Condition", + "delay": "Delay", + "exit_loop": "Exit Loop", + "initialize_variable": "Initialize Variable", + "loop": "Loop", + "manual": "Manual", + "merge_branches": "Merge Branches", + "next_or_finish": "Next/Finish", + "search_records": "Action: Search Records", + "stop_execution": "Stop Execution", + "update_variable": "Update Variable", +} + +# Set by the server on a loop's item variable, from the loop (confirmed live +# 2026-10-27). A spec never names them. +_DERIVED_VARIABLE_KEYS = frozenset({"is_array_item_variable", "source_array_variable"}) + + +def _diffable_variable(var: Any) -> Any: + """A variable definition as `diff` compares it: without the keys the + server derives from a loop, and with a null key the same as an absent + one, since a spec can't send null and an omitted key reads back null.""" + if not isinstance(var, dict): + return var + return { + k: v + for k, v in var.items() + if k not in _DERIVED_VARIABLE_KEYS and v is not None + } + + +def _with_default_description(item: dict[str, Any]) -> dict[str, Any]: + default = _DEFAULT_DESCRIPTIONS.get(str(item.get("type"))) + if default is None or "description" in item: + return item + return {**item, "description": default} + + +def _diffable_variable_block(step_type: str, block: dict[str, Any]) -> dict[str, Any]: + """An initialize_variable/update_variable block with its omitted defaults + filled in: `deduplicate_array`, a static list source's `array_separator`, + and an update_variable source's `value_html`.""" + out = {**_VARIABLE_BLOCK_DEFAULTS, **block} + if step_type == "initialize_variable" and "variable" in out: + out["variable"] = _diffable_variable(out["variable"]) + if isinstance(out.get("sources"), list): + sources = [] + for src in out["sources"]: + if isinstance(src, dict): + if src.get("source_type") == "static" and isinstance( + src.get("value"), list + ): + src = {"array_separator": "comma", **src} + if step_type == "update_variable": + src = {"value_html": "", **src} + sources.append(src) + out["sources"] = sources + return out + def _wire_pairs( live_items: list[dict[str, Any]], spec_items: list[dict[str, Any]] @@ -805,6 +923,7 @@ def _normalize_wire_triggers( if item is None: continue norm = {k: v for k, v in item.items() if k not in _WIRE_DIFF_EXCLUDED | {"id"}} + norm = _with_default_description(norm) # `id` is carried as a normal field, but pinned to the pair's shared # canonical value on both sides so a matched pair never diffs on it # (position-fallback matches can have an id on only one side) while @@ -859,7 +978,11 @@ def _normalize_wire_steps( resolved["trigger_key"], resolved["trigger_key"] ) norm[_GO_TO_BLOCK_FIELD] = resolved - out[_wire_pair_label(pair)] = norm + for step_type in ("initialize_variable", "update_variable"): + field = _block_field_for(step_type) + if isinstance(norm.get(field), dict): + norm[field] = _diffable_variable_block(step_type, norm[field]) + out[_wire_pair_label(pair)] = _with_default_description(norm) return out @@ -897,6 +1020,11 @@ def diff_wire_payloads( norm_live = {k: v for k, v in live.items() if k not in ("triggers", "steps")} norm_spec = {k: v for k, v in spec.items() if k not in ("triggers", "steps")} + for norm in (norm_live, norm_spec): + for k, default in _WIRE_TOP_DEFAULTS.items(): + norm.setdefault(k, default) + if isinstance(norm.get("variables"), list): + norm["variables"] = [_diffable_variable(v) for v in norm["variables"]] norm_live["triggers"] = _normalize_wire_triggers(trigger_pairs, 0) norm_spec["triggers"] = _normalize_wire_triggers(trigger_pairs, 1) norm_live["steps"] = _normalize_wire_steps(step_pairs, trigger_pairs, 0) diff --git a/tests/test_automation_payloads.py b/tests/test_automation_payloads.py index 26c9316..e2b9002 100644 --- a/tests/test_automation_payloads.py +++ b/tests/test_automation_payloads.py @@ -3021,6 +3021,242 @@ def test_broken_loop_spec_fails_plan(patch_live_lookups): assert "next_or_finish step 'next' has parent 'check'" in str(e.value) +def _var(key, parent, name, order, data_type="string", is_array=False, sources=()): + variable = {"name": name, "data_type": data_type, "is_array": is_array} + if data_type == "entity": + variable["data_subtype"] = load_fixture("objects/patients.json")["id"] + block = {"sources": list(sources), "variable": variable} + if is_array: + block["array_aggregation_mode"] = "add_all" + return _s( + key, parent, "initialize_variable", order, action_initialize_variable=block + ) + + +def _loop(key, parent, array, item, order, **kw): + block = {"array_variable": array, "item_variable": item} + return _s(key, parent, "loop", order, step_loop=block, **kw) + + +_STRINGS = [{"source_type": "static", "source_subtype": "string", "value": ["a", "b"]}] + + +def _authored_loop_spec(steps: list[dict], record_based: bool = True) -> dict: + for i, s in enumerate(steps): + s["order"] = i + spec = { + "api_name": "authored_loop", + "name": "Authored Loop", + "type": "record_based" if record_based else "global", + "steps": steps, + } + if record_based: + spec["target_object"] = "patients" + return spec + + +def _string_loop_steps() -> list[dict]: + return [ + _var("arr", None, "arr", 0, is_array=True, sources=_STRINGS), + _var("item", "arr", "arr_item", 0), + _loop("each", "item", "arr", "arr_item", 0), + _s("body", "each", "delay", 0), + _s("next", "each", "next_or_finish", 0), + _s("after", "next", "delay", 0), + ] + + +def _nested_loops( + depth: int, root_parent: str | None = None, arrays: list[str] | None = None +) -> list[dict]: + """`depth` loops, each the body of the one before, and an `exit_loop` in + the innermost body. Each loop iterates its entry in `arrays` (by default + a different array per level) through its own item variable.""" + arrays = arrays or [f"arr{n}" for n in range(depth)] + steps: list[dict] = [] + parent = root_parent + for name in dict.fromkeys(arrays): + steps.append(_var(name, parent, name, 0, is_array=True, sources=_STRINGS)) + parent = name + for n in range(depth): + steps.append(_var(f"item{n}", parent, f"item{n}", 0)) + parent = f"item{n}" + closers = [] + for n, name in enumerate(arrays): + steps.append(_loop(f"l{n}", parent, name, f"item{n}", 0)) + closers.append(_s(f"n{n}", f"l{n}", "next_or_finish", 0)) + parent = f"l{n}" + steps.append(_s("leave", parent, "exit_loop", 0)) + return steps + closers[::-1] + + +def _built_steps(spec: dict) -> dict[str, dict]: + return {s["key"]: s for s in _build(spec)["steps"]} + + +def _preview(spec: dict) -> dict: + return plan_create_automation(spec).operations[0].preview + + +@pytest.mark.parametrize("flag", [None, False, True]) +def test_loop_is_always_sent_as_an_initiator(patch_live_lookups, flag): + """The server stores true on a loop whatever is sent (confirmed live + 2026-10-27), so the builder sends true and `diff` shows no flip.""" + steps = _string_loop_steps() + if flag is not None: + steps[2]["is_branch_group_initiator"] = flag + by_key = _built_steps(_authored_loop_spec(steps, record_based=False)) + assert by_key["each"]["is_branch_group_initiator"] is True + assert [k for k, s in by_key.items() if "is_branch_group_initiator" in s] == [ + "each" + ] + + +def test_string_array_loop_spec(patch_live_lookups): + spec = _authored_loop_spec(_string_loop_steps(), record_based=False) + by_key = _built_steps(spec) + assert by_key["each"]["step_loop"] == { + "loop_type": "for_each_item", + "array_variable": "arr", + "item_variable": "arr_item", + } + assert by_key["item"]["action_initialize_variable"]["sources"] == [] + assert by_key["after"]["parent_key"] == "next" + assert "warnings" not in _preview(spec) + + +def test_record_array_loop_over_search_records(patch_live_lookups): + steps = [ + _var("recs", None, "recs", 0, data_type="entity", is_array=True), + _var("rec", "recs", "recs_item", 0, data_type="entity"), + _s( + "find", + "rec", + "search_records", + 0, + action_search_records={ + "custom_object": "patients", + "filter_type": "all_records", + "destination_variable": "recs", + }, + ), + _loop("each", "find", "recs", "recs_item", 0), + _s("body", "each", "delay", 0), + _s("next", "each", "next_or_finish", 0), + ] + spec = _authored_loop_spec(steps) + by_key = _built_steps(spec) + assert by_key["find"]["action_search_records"]["destination_variable"] == { + "name": "recs" + } + assert by_key["each"]["step_loop"]["array_variable"] == "recs" + assert "warnings" not in _preview(spec) + + +def test_loop_on_a_condition_yes_branch(patch_live_lookups): + steps = _string_loop_steps()[:5] + check = _s( + "check", + "item", + "condition", + 0, + step_condition={"type": "custom_filter", "filter_config": NAME_IS_X}, + ) + steps[2].update(parent_key="check", parent_branch="yes") + other = _s("other", "check", "delay", 0, parent_branch="no") + spec = _authored_loop_spec([*steps[:2], check, *steps[2:], other]) + by_key = _built_steps(spec) + assert by_key["each"]["parent_yes_no"] == "yes" + assert by_key["each"]["is_branch_group_initiator"] is True + assert "warnings" not in _preview(spec) + + +def test_nested_loops_with_an_exit_loop(patch_live_lookups): + spec = _authored_loop_spec(_nested_loops(2), record_based=False) + by_key = _built_steps(spec) + assert by_key["l1"]["parent_key"] == "l0" + assert by_key["leave"]["parent_key"] == "l1" + assert (by_key["n0"]["parent_key"], by_key["n1"]["parent_key"]) == ("l0", "l1") + assert "warnings" not in _preview(spec) + + +def test_loop_with_two_body_lanes_still_warns(patch_live_lookups): + steps = [*_string_loop_steps(), _s("other_lane", "each", "delay", 0)] + assert _preview(_authored_loop_spec(steps))["warnings"] == [ + "step 'each' has 2 parallel children but is a 'loop', not a 'branch'; " + "put a 'branch' step in front of the fan-out" + ] + + +def test_loop_nested_past_three_is_refused(patch_live_lookups): + """Kizen's builder holds three levels. The API accepts more, so this is + the CLI's own rule, checked at plan time.""" + assert "warnings" not in _preview(_authored_loop_spec(_nested_loops(3), False)) + with pytest.raises(PlanError) as e: + _build(_authored_loop_spec(_nested_loops(5), False)) + problems = str(e.value).split("\n - ")[1:] + assert problems == [ + f"loop 'l{n}' is nested {n + 1} levels deep; Kizen's builder allows at " + "most three. The API accepts deeper loops, but the CLI refuses them so " + "the automation stays editable in the UI." + for n in (3, 4) + ] + + +def test_loop_after_a_next_or_finish_is_not_nested_in_that_loop(patch_live_lookups): + """`again` follows the innermost loop of three, so it is three deep, not four.""" + steps = [ + _var("again_arr", None, "again_arr", 0, is_array=True, sources=_STRINGS), + _var("again_item", "again_arr", "again_item", 0), + *_nested_loops(3, root_parent="again_item"), + _loop("again", "n2", "again_arr", "again_item", 0), + _s("again_next", "again", "next_or_finish", 0), + ] + assert "warnings" not in _preview(_authored_loop_spec(steps, False)) + + +def _same_array_problem(key: str, array: str, outer: str) -> str: + return ( + f"loop '{key}' iterates '{array}', which loop '{outer}' around it already " + "iterates; Kizen's builder needs a different array variable at each level. " + "The API accepts it, but the CLI refuses it so the automation stays " + "editable in the UI." + ) + + +@pytest.mark.parametrize( + ("arrays", "problem"), + [ + (["a", "a"], ("l1", "a", "l0")), + (["a", "b", "a"], ("l2", "a", "l0")), + ], + ids=["outer_and_inner", "two_levels_down"], +) +def test_nested_loop_over_an_array_already_iterated_is_refused( + patch_live_lookups, arrays, problem +): + spec = _authored_loop_spec(_nested_loops(len(arrays), arrays=arrays), False) + with pytest.raises(PlanError) as e: + _build(spec) + assert str(e.value).split("\n - ")[1:] == [_same_array_problem(*problem)] + + +def test_loops_over_one_array_in_sequence_build(patch_live_lookups): + """The second loop hangs off the first's next_or_finish: not nested.""" + variables, loop = _string_loop_steps()[:2], _string_loop_steps()[2:] + loop[0]["parent_key"] = "again_item" + steps = [ + *variables, + _var("again_item", "item", "again_item", 0), + *loop, + _loop("again", "after", "arr", "again_item", 0), + _s("again_body", "again", "delay", 0), + _s("again_next", "again", "next_or_finish", 0), + ] + by_key = _built_steps(_authored_loop_spec(steps, False)) + assert by_key["again"]["step_loop"]["array_variable"] == "arr" + + @pytest.mark.parametrize( ("field", "value"), [ @@ -3123,8 +3359,8 @@ def search(obj: str, var: str) -> dict: def test_branching_test_transcription_diffs_clean(patch_live_lookups, monkeypatch): """The spec reproduces the UI-built graph. What's left is noise any spec - shows: server-written descriptions, the plugin app the server fills in - for a `kizen/*` model, and a variable definition's null read defaults.""" + shows: server-written descriptions, and the plugin app the server fills + in for a `kizen/*` model.""" import kizen_builder.tools.planners.automations as ma raw = load_fixture("automations/branching_test.raw.json") @@ -3135,11 +3371,6 @@ def test_branching_test_transcription_diffs_clean(patch_live_lookups, monkeypatc d for d in diff_automation(_branching_test_spec())["diff"] if d["path"].rsplit(".", 1)[-1] not in ("description", "business_plugin_app_id") - and not ( - ".action_initialize_variable.variable." in d["path"] - and d["before"] in (None, False) - and d["after"] == "" - ) ] assert structural == [] diff --git a/tests/test_translate.py b/tests/test_translate.py index 08c00db..882be43 100644 --- a/tests/test_translate.py +++ b/tests/test_translate.py @@ -1328,6 +1328,132 @@ def test_diff_wire_payloads_reads_omitted_step_level_keys_as_defaults( } +def _step_variables(payload: dict) -> list[dict]: + steps = _steps_by_type(payload, "initialize_variable") + return [s["action_initialize_variable"]["variable"] for s in steps] + + +def _variable_definitions(payload: dict) -> list[dict]: + """Top-level `variables` and each initialize_variable step's definition.""" + return [*payload["variables"], *_step_variables(payload)] + + +def test_diff_ignores_what_the_server_derives_on_an_item_variable( + loops: dict, +) -> None: + """A spec never names `is_array_item_variable`/`source_array_variable`: + the server sets both from the loop (confirmed live 2026-10-27).""" + assert any(v["is_array_item_variable"] for v in _variable_definitions(loops)) + spec = copy.deepcopy(loops) + for v in _variable_definitions(spec): + del v["is_array_item_variable"], v["source_array_variable"] + assert diff_wire_payloads(loops, spec) == [] + assert diff_wire_payloads(spec, loops) == [] + + +def test_diff_reads_a_null_variable_key_as_absent(loops: dict) -> None: + """A spec can't send null, and an omitted key reads back null.""" + spec = copy.deepcopy(loops) + for v in _variable_definitions(spec): + for k in [k for k, x in v.items() if x is None]: + del v[k] + assert diff_wire_payloads(loops, spec) == [] + + +def _top(payload: dict) -> dict: + return payload + + +def _init_block(payload: dict) -> dict: + return _steps_by_type(payload, "initialize_variable")[0][ + "action_initialize_variable" + ] + + +def _static_list_source(payload: dict) -> dict: + return _init_block(payload)["sources"][0] + + +def _update_source(payload: dict) -> dict: + (step,) = _steps_by_type(payload, "update_variable") + return step["action_update_variable"]["sources"][0] + + +def _step_zero(payload: dict) -> dict: + return payload["steps"][0] + + +def _trigger_zero(payload: dict) -> dict: + return payload["triggers"][0] + + +# (fixture, where the key lives, key, server default, a non-default value) +_SERVER_DEFAULTS = { + "error_notification_email": ("loops", _top, "", "ops@example.com"), + "deduplicate_array": ("loops", _init_block, False, True), + "array_separator": ("loops", _static_list_source, "comma", "semicolon"), + "value_html": ("branching_test", _update_source, "", "

x

"), + "step description": ( + "loops", + _step_zero, + "Initialize Variable", + "arr0 = [2 items]", + ), + "trigger description": ("loops", _trigger_zero, "Manual", "Run by hand"), +} + + +def _defaults_case(name: str) -> tuple[dict, dict, str, object, object]: + fixture, where, default, other = _SERVER_DEFAULTS[name] + key = name.split()[-1] + live = live_to_payload(load_fixture(f"automations/{fixture}.raw.json")) + where(live)[key] = default + return live, copy.deepcopy(live), key, default, other + + +@pytest.mark.parametrize("name", sorted(_SERVER_DEFAULTS)) +def test_diff_reads_an_omitted_key_as_its_server_default(name: str) -> None: + """Omitted on a PUT, each reads back as its default (confirmed live + 2026-10-27), so a spec that leaves it out changes nothing.""" + live, spec, key, _, _ = _defaults_case(name) + del _SERVER_DEFAULTS[name][1](spec)[key] + assert diff_wire_payloads(live, spec) == [] + assert diff_wire_payloads(spec, live) == [] + + +@pytest.mark.parametrize("name", sorted(_SERVER_DEFAULTS)) +@pytest.mark.parametrize("non_default_side", ["live", "spec"]) +def test_diff_still_shows_a_non_default_value(name: str, non_default_side: str) -> None: + """The key is compared as its default, not dropped: a non-default value on + either side diffs against an omitted one.""" + live, spec, key, default, other = _defaults_case(name) + where = _SERVER_DEFAULTS[name][1] + del where(spec)[key] + if non_default_side == "live": + where(live)[key] = other + expected = [(other, default)] + else: + where(spec)[key] = other + expected = [(default, other)] + assert [(e["before"], e["after"]) for e in diff_wire_payloads(live, spec)] == ( + expected + ) + + +@pytest.mark.parametrize("top_level", [False, True]) +def test_diff_still_shows_an_item_variable_change(loops: dict, top_level: bool) -> None: + spec = copy.deepcopy(loops) + item = next( + v + for v in (spec["variables"] if top_level else _step_variables(spec)) + if v["is_array_item_variable"] + ) + item["data_type"] = "uuid" + assert [(e["before"], e["after"]) for e in diff_wire_payloads(loops, spec)] == [ + ("string", "uuid") + ] + + # --------------------------------------------------------------------------- # Loops: `step_loop` by name, configless closers, and the server's loop rules # (each rule a live 400, each accepted shape a live 200, 2026-10-27) @@ -1522,8 +1648,7 @@ def _accepted_shapes() -> dict[str, list[dict]]: _ws("n", "next_or_finish", "l"), ] shapes["A9-exit-after-body-step"] = _base() + [_ws("x", "exit_loop", "b1")] - shapes["B6-nest-4"] = _nest(4) - shapes["C1-nest-6"] = _nest(6) + shapes["B5-nest-3"] = _nest(3) head, p = _head() shapes["B7-loop-on-yes-branch"] = head + [ _ws("c", "condition", p), @@ -1569,6 +1694,63 @@ def test_loop_shapes_the_server_accepts_validate(name: str) -> None: assert validate_payload(_graph(copy.deepcopy(_ACCEPTED[name]))) == [] +@pytest.mark.parametrize("depth", [4, 6]) +def test_deep_nesting_the_server_accepts_fails_only_the_cli_rule(depth: int) -> None: + """B6 and C1 saved live; Kizen's builder can't hold them, so the CLI refuses.""" + assert _problems(_nest(depth)) == [ + f"loop 'l{d}' is nested {d + 1} levels deep; Kizen's builder allows at " + "most three. The API accepts deeper loops, but the CLI refuses them so " + "the automation stays editable in the UI." + for d in range(3, depth) + ] + + +def _nest_through_conditions(depth: int) -> list[dict]: + """Like `_nest`, but each inner loop sits on the yes lane of a condition + in the outer loop's body.""" + steps, parent = _head(depth) + for d in range(depth): + branch = "" + if d: + steps.append(_ws(f"c{d}", "condition", parent)) + parent, branch = f"c{d}", "yes" + steps.append(_loop(f"l{d}", parent, f"arr{d}", f"arr{d}_item", branch)) + parent = f"l{d}" + steps += [_ws(f"n{d}", "next_or_finish", f"l{d}") for d in reversed(range(depth))] + return steps + + +def test_depth_counts_loops_through_a_condition_lane() -> None: + """A condition between levels is not a level, and doesn't end the walk.""" + assert _problems(_nest_through_conditions(3)) == [] + assert _problems(_nest_through_conditions(4)) == [ + "loop 'l3' is nested 4 levels deep; Kizen's builder allows at most three. " + "The API accepts deeper loops, but the CLI refuses them so the automation " + "stays editable in the UI." + ] + + +def test_one_array_at_two_levels_the_server_accepts_fails_only_the_cli_rule() -> None: + """The API saved one array in an outer and an inner loop, each with its own + item variable (confirmed live 2026-10-27). Kizen's builder refuses it, so + the CLI does too, and with nothing else wrong.""" + steps, p = _head() + steps += [ + _var("vb", "inner_item", p), + _loop("outer", "vb", "arr0", "arr0_item"), + _loop("inner", "outer", "arr0", "inner_item"), + _delay("b1", "inner"), + _ws("n_in", "next_or_finish", "inner"), + _ws("n_out", "next_or_finish", "outer"), + ] + assert _problems(steps) == [ + "loop 'inner' iterates 'arr0', which loop 'outer' around it already " + "iterates; Kizen's builder needs a different array variable at each " + "level. The API accepts it, but the CLI refuses it so the automation " + "stays editable in the UI." + ] + + @pytest.mark.parametrize("fixture", ["loops", "ui_loops"]) def test_captured_loop_graphs_validate(request, fixture: str) -> None: assert validate_payload(request.getfixturevalue(fixture)) == []