# 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 `StepId`s 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 `WorkflowDef`s; 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](../../../rust-agentic-sys.md) §4.3, §11.7 ·
[rust-agentic-task.md](../../../rust-agentic-task.md)