poimen/tasks/T3.4-stepid-stability-checks.md

T3.4 — StepId stability checks

Field	Value
Phase	P3 — Workflow as data
Size	S — under 1 day
Status	Not started
Flags	—
Spec	inlined below
Blocks	—

Goal

Report added, removed and retained StepIds across a version bump, and reject a bump that retains nothing unless it is explicitly marked a rewrite.

Facts (inlined — no spec read needed)

• Credit assignment attributes outcomes to steps across workflow versions. That requires a step identity surviving edits: insert a step at position 2 and every positional index shifts, but StepId does not.
• The framework cannot infer this. It is a documented obligation on the workflow author, enforced by three load-time checks:
  1. StepId unique within a version.
  2. On a version bump, report added, removed and retained ids. A version retaining no ids from its parent is almost certainly a renumbering accident and is rejected unless explicitly marked as a rewrite.
  3. StepId is opaque to the framework — never parsed, never ordered, never assumed numeric.
• This is why the stability contract is load-bearing rather than cosmetic: per-step credit (T5.10) and the whole attribution path depend on it.

Steps

1. Take parent and child WorkflowDefs; compute the three sets: added, removed, retained.
2. Emit the report as structured data, not a log line — it belongs on the version record so it is auditable later.
3. Reject when retained.is_empty() and the child is not marked as a rewrite. Name the count of added and removed ids in the error.
4. Add the rewrite marker as an explicit field on the workflow definition, so marking it is a deliberate edit in the user's own file.
5. Audit for any code that parses, sorts numerically, or ranges over StepId. Delete it — ordering comes from transitions, not from the id.
6. Test both directions: renumbering every id rejected by default, accepted with the marker.

Acceptance

• Renumbering every step id is rejected by default and accepted with the explicit marker.

Verify

Harness: version pairs committed as fixtures — parent and child — covering each diff shape.

Integration test — tests/it_stepid_stability.rs:

1. Renumber everything, no marker: assert rejected, and that the error names the added/removed counts.
2. Renumber everything, marker set: assert accepted.
3. Insert a step at position 2, ids unchanged: assert accepted, and that the report shows 1 added, 0 removed, N retained. This is the case the whole contract exists for.
4. Rename one of five ids: assert accepted with 1 added, 1 removed, 4 retained — a partial rename must not trip the all-or-nothing rule.
5. Duplicate id within one version: rejected.
6. Assert the report is persisted on the version record, readable after a restart — not emitted only as a log line.
7. Opacity audit: grep the codebase for numeric parsing, sorting or ranging over StepId; assert none. Add a StepId in a fixture that is non-numeric and contains a ., and assert nothing breaks.

Command: cargo test -p workflow stepid_stability

False pass:

• Only testing steps 1 and 2. The rule "reject when retained is empty" is trivially satisfiable by an implementation that computes nothing else — steps 3 and 4 are what prove the diff is real.
• Asserting the report exists in stdout. Credit assignment reads it later from storage; step 6 is the check that matters.
• Fixtures whose ids are step1, step2, … so accidental numeric ordering is indistinguishable from correct behaviour. Step 7's odd id is the guard.

Traps

• Sorting steps by StepId for display and then depending on that order.
• Auto-generating StepId from position when the author omits one. That guarantees the renumbering accident the check exists to catch.

---

Background (not required to do this task): rust-agentic-sys.md §4.3, §11.7 · rust-agentic-task.md