fix: advance the state file's Phase Progress rows at phase boundaries (2.3.6) - #562
Conversation
leandrodamascena
left a comment
There was a problem hiding this comment.
APPROVE
I dug into this one. The Phase Progress section it fixes is display-only — routing reads Lifecycle Phase and the stage checkboxes, and --status recomputes live — so a wrong row here mis-displays but can't misroute. Nothing I found blocks the merge.
Here's what I actually checked. First I worried the setField matcher might rewrite the wrong line, so I read the state template: the five phase labels only show up as - **<Label>**: bullets under ## Phase Progress, once each, and the regex is anchored with the : right after ** — so - **Construction**: can't match - **Construction Autonomy Mode**:. No collision. Then I grepped all of core/**/*.ts for anything that reads those row values back, to make sure the "display-only" claim holds — zero readers for routing; the statusline computes from the checkbox blocks, not these rows. And I confirmed t232 isn't just asserting literals: no-op'ing setPhaseProgress flips exactly the row assertions to failing.
Two display inconsistencies I did reproduce, both worth a follow-up but neither blocking:
- A double-boundary advance leaves the middle phase stale (
aidlc-state.ts:1204). advance flips only the phase you left and the one you entered. If a single transition steps over a whole phase, that middle phase keeps its birthPendingabove its own all-[S]checkboxes — which is the exact row-vs-checkbox contradiction #556 is trying to kill, just relocated. The jump path handles this (it marks stepped-over phasesSkipped), so the two paths disagree. No stock scope sandwiches an all-SKIP phase between two executing ones, so you won't hit it on a normal advance, but I reproduced it withskip: skip all of Inception, advance out of Ideation, and you land on Construction withInception: Pending. I'd have advance/finalize mark fully-skipped middle phases the way jump already does, and add a double-boundary case to t232. - A forward jump marks the source phase
Verifiedeven when none of its stages ran (aidlc-jump.ts:382). Jump forward from a phase's first stage and its row goesVerifiedabove all-[S]checkboxes, while the phase you jumped clean over readsSkipped— same zero-completed state, different row, purely by where the cursor was. It's tracking the existing PHASE_VERIFIED audit event so I'm not calling it a bug, butSkippedwould read truer.
Non-blocking housekeeping: version 2.3.7 collides with #560 (same version, same ## [2.3.7] heading, both add a t232-* file). Different bases today, so no live conflict — whichever shares a lineage second re-bumps and renumbers. t232 also has no double-boundary case and I didn't trace the per-unit Construction swarm re-entry, both lower-risk and still display-only.
The mid-flow staleness the code comments already call out is accurate for the cases I traced. Good to merge.
275faca to
39f570a
Compare
The four-PR train (#550 2.3.5, #562 2.3.6, #563 2.3.7) took this branch's old version and test slots, so versions shift to 2.3.8 (workspace verbs) and 2.3.9 (config/plugin verbs), t230-handler-additions moves to t231, and t231-build-binaries moves to t238 (t232-t237 are claimed by merged and in-flight work). Coverage registry regenerated; dist regen in sync.
Summary
Fixes #556: the state file's
## Phase Progresssection was seeded once at intent birth and never advanced, so a few stages in it still showedIdeation: Pendingabove a fully-checked Ideation checkbox block. ThephaseStatuscomment promised an Active/Verified flip that no code performed. The section is display-only (routing readsLifecycle Phase+ the Stage Progress checkboxes;/aidlc --statusrecomputes its phase block live), so this is a correctness fix for the human-facing state file, not a routing change.This takes the issue's option 1 (implement the documented flip) and closes two additional drift surfaces the issue did not cover: jumps and plan re-shaping.
Changes
setPhaseProgresshelper (core/tools/aidlc-lib.ts): flips one- **<Phase>**:row via the existingsetFieldmatcher. Silent no-op when the row is absent - an older or hand-edited state file must never fail a transition over a display row.advance/finalize(core/tools/aidlc-state.ts): at a phase boundary, completed phase ->Verified, entered phase ->Active, in the same atomic state write as thePHASE_COMPLETED/PHASE_VERIFIED/PHASE_STARTEDtrio. Non-boundary transitions leave the section byte-identical.complete-workflow(and a final-stagefinalize): last phase ->Verified.jump execute(core/tools/aidlc-jump.ts): forward marks the source phaseVerified, wholly jumped-over phasesSkipped, targetActive; backward returns reset phases (those with EXECUTE stages) toPending. Jump already emits the phase-boundary audit trio, so the rows now track it.scope-change/recompose(core/tools/aidlc-utility.ts): re-derive only not-yet-reached rows (Pending/Skipped) against the new plan;Verified/Activerows record history and are never rewritten.Initialization: Verified+ first post-init phaseActive(previouslyActive/Pending, contradicting the audit events birth emits).docs/reference/12-state-machine.md: phase-machine prose + transition table updated to describe what the code now does (the table previously described this exact behaviour aspirationally).dist/<harness>/trees regenerated (package.ts --checkclean).Note for a run already mid-flow: rows stay stale for boundaries already passed and correct from the next boundary onward (as the issue predicted); a fresh run is correct from birth.
Tests
tests/unit/t232-phase-progress-flip.test.ts(12 cases, spawns the real shipped CLIs): birth seeds for feature/bugfix scopes, byte-identical non-boundary advance, boundary flips for advance/finalize/complete-workflow, both jump directions, scope-change/recompose re-derivation, and missing-section tolerance.t39pin updated:Initializationat birth is now assertedVerified(wasActive).EXPECTED_NONE_TO_CLIratchet; registry regenerated.Verification
t92linter/type-check sensor assertions that fail identically on the untouched v2 tip (verified via a detached baseline worktree).t68version/changelog sync,t39,t232,package.ts --check, coverage--check, andbun run typecheckall green at the new base.dist/claudetools in a scratch project (birth -> 7 advances -> forward jump -> backward jump -> complete-workflow -> scope-change -> recompose).