Skip to content

Phase-6 cross-story lessons

Short, reusable takeaways harvested from each story attempt. Append-only.

From S1-02 (2026-05-25)

  • Story prose may name types that don't exist in the codebase yet. S1-02 named GateOutcome in AC-4 (the actual type is TrustOutcome) and ChainHead("blake3:<64hex>") in the TDD-Green prose (the actual newtype is bare 64-hex without prefix). Default: Rule 11 (match existing convention). Surface every such inconsistency in the attempt log so it can be patched at the story or validation-report level.
  • Two definitions of "terminal" coexist in Phase 6. Class-level terminal (S1-01 TerminalState Literal — three values) is a contract declaration. Operationally terminal (zero outgoing edges — two values: completed, failed_unrecoverable) is a graph-level property. awaiting_human_review is the third class-level terminal but is operationally resumable. Tests that conflate the two trip on awaiting_human_review → plan_ready legal edges.
  • triggering_outcome as JsonValue, not as a discriminated union, defers the codegenie.plugins.subgraph coupling. A union including NodeTransition would force Advance.model_rebuild() to be called in vuln_ledger.py, dragging in SubgraphState (out of Phase-6 scope and a kernel cycle). The substrate only needs deterministic bytes for the chain head; the producer side keeps the typed shape.
  • The __all__ allowlist sentinel landed in S1-01 must be amended additively by each subsequent story. S1-02 grew the public surface from 4 → 15 names; the fence at tests/fence/test_workflows_public_surface.py and the pin at tests/unit/workflows/test_vuln_sut_shape.py::test_ac1_all_is_exact_set both need to widen.
  • ChainHead already exists from Phase-4 S4-04 — reuse, never redefine. The same applies to BlobDigest, SignalKind, AttemptNumber, HumanReviewReason, RemediationError, WorkflowId. Greenfield-feeling Phase-6 stories that look like they want a fresh identifier almost always need to reuse one from codegenie.types.identifiers (drift test enforces).
  • The Phase-6 contract snapshot meta-test is the right home for "additive vs breaking" rule extensions. When S1-02 added legal_transitions to the snapshot, four new meta-test cases (additive edge add, breaking edge removal, breaking variant removal, breaking required-field removal) kept the classifier's mutation-resistance pinned.

From S2-01 (2026-05-25)

  • Port + two adapters: composition via free functions, not inheritance. Both SqliteCheckpointStore and InMemoryCheckpointStore call free helpers (_assert_boundary, _canonical_event_bytes) from checkpoints.py rather than inheriting from a shared base. Mirrors the Phase-3 EventStreamSink + ZstdAppendingFileSink + InMemorySink precedent. Resist the urge to add BaseCheckpointStore even when the adapters look similar; the inheritance coupling would block the Phase-9 Postgres adapter swap.
  • Hypothesis property tests need per-example fresh sub-directories when the substrate is stateful. A single tmp_path shared across @given examples lets an SQLite store accumulate transition_ids; shrunk re-tries fail with UNIQUE constraint failed masking the real assertion failure. Use a module-level counter + _fresh_subdir(tmp_path) helper to give each example a clean root.
  • unique_by=lambda e: e.transition_id on st.lists(...) is essential when the substrate has UNIQUE constraints. Without it, Hypothesis draws duplicate ULIDs and your write fails on the database constraint rather than the property you're trying to check.
  • Detection-substrate-only vs. integrity-policy is a load-bearing separation. tail_chain_head returns whatever is persisted; it never recomputes. The replay verifier (S2-02) owns recomputation. Test this explicitly: tamper a row directly via SQLite and assert the tampered value comes back from tail_chain_head. Without that test, an executor "helpfully" recomputing inside the store would silently collapse policy / detection.
  • Sanitization is a write-time defense; chain head is computed over the live event. This preserves byte-equality between adapters even when on-disk bytes are redacted. On replay (S2-02), the recomputation either mirrors the write-path sanitization or surfaces redaction as a benign divergence — an S2-02 design call, not an S2-01 concern. Surfaced in the attempt log for the S2-02 executor.
  • Store types do NOT enter codegenie.workflows.__all__. Phase-6.5's bench harness must not depend on checkpoint backend internals (final-design.md §"Relationship to Phase 6.5"). The 14-name allowlist is the public surface; adding CheckpointStore "for API convenience" is the exact mutation the AC-2 byte-equality test catches.
  • The contract snapshot extension always pays for itself. S1-01 → S1-02 → S2-01. Each story that ships a new structural surface (Protocol shape, closed-set constant, schema string) gets a snapshot row + a corresponding meta-test pair (one additive case, one breaking case) so the classifier mutation-resistance compounds.

From S2-02 (2026-05-25)

  • Sanitization-aware fold = fold over the sanitized-reconstructed event, not the live event. S2-01 left the chain head computed over the LIVE event while the on-disk row was SANITIZED bytes — meaning the verifier could not reproduce the head from persisted bytes when sanitization triggered. The fix lands in both adapters: reparse sanitize_for_persistence(canonical_bytes) into a TransitionEvent and fold over that reconstructed event. For non-secret events the round-trip is byte-equal to the live event (sanitize is a no-op), so existing chain heads + goldens are unchanged. The chain protects BYTES ON DISK — the substrate-replayable invariant.
  • Additive Protocol extension over substrate-coupling. When the verifier needed per-row persisted next_head to compute divergence_index, three options surfaced: (a) widen the Protocol additively, (b) drop the field, (c) substrate-specific reads. (a) is the cleanest: the Protocol stays the kernel, the substrate stays agnostic, the contract-snapshot meta-test classifies the delta as additive. Resist (c) — a SQLite-specific shortcut would break the in-memory adapter parity.
  • Tagged-union return + AST exhaustiveness > bool/exception return. Single verify() -> ReplayVerdict discriminated union (Verified | ChainMismatch | TornWrite | EmptyWorkflow) over four _FROZEN_FORBID Pydantic variants, plus a _dispatch_verdict match with four case arms + case _: drift guard. AST test counts arms; future variant additions surface a missing-arm CI failure (mypy-version-independent).
  • while True: next(iterator) instead of enumerate(...) when ValidationError can fire INSIDE the iterator's next(). With enumerate, sequence_count advances only on successful next; a parse failure leaves the counter at the previous row. The while loop advances the counter AFTER each successful iteration, so TornWrite.offending_sequence reports the actual failing row.
  • Per-mapping routing tests, not parametrize. When the four arms of a discriminated-union dispatch have different setup costs (empty / clean / tamper / torn-write), per-mapping tests are clearer and break with more specificity than a single parametrized harness that has to over-build for the simple cases.
  • Hydrated.kind MUST be a NEW closed tag, not a reused LedgerStateKind. Two unions answering different questions ("what state is the workflow IN?" vs "what HAPPENED during hydration?"). Reuse would let Hydrated(kind="needs_plan") slip through as a category error. Sanity-test: assert the new kind is NOT a member of the other Literal.
  • AST fence on the "fail-closed before hydrate" path is a load-bearing safety invariant. The verifier module MUST NOT construct any non-terminal VulnLedgerState variant. AST-walk for NeedsPlan() / PlanReady() / etc. calls; only FailedUnrecoverable construction is allowed on the failure arm. Catches the silent-resume-after-tamper failure mode at the source level.