Resumable #[StepAI] workflows, and delete the imperative ai() path (#87) - #90
Merged
Conversation
Two sibling subjects from one investigation into the ask/wait flow: a run's pause for a person becomes one typed request (design/human-requests.md), and a step becomes an atomic unit that re-runs whole or replays from a record so there is no mid-step resume to build (design/workflow-resume.md). DECISIONS records the human-request decision; INDEX points at both.
The [question] turn is a no-tool turn the tool-turn checkpoint never reaches, so a crash while parked left nothing recorded and a resume re-asked the model. The loop now checkpoints before it blocks on the answer, and pendingQuestion() reads that tail back to tell a park (continue from the answer) from a settled exchange (replay). First increment of design/workflow-resume.md.
A step is now one of two kinds, told apart by attribute so the resume path decides continue-vs-re-run before running the body. A #[StepAI] method is pure: it returns an AiStep declaration and the base runs the one exchange it names. On resume the base continues the recorded conversation instead of re-running the model, and a crash while PARKED on a person's answer resumes by continuing from that answer rather than asking again. Coexists with the imperative #[Step] path, untouched, so every existing solver and test keeps working. Second increment of design/workflow-resume.md.
The critic and its round cap ride on the attribute, so the resume path reads them without running the body — the same reason the kind is an attribute. The review loop is the imperative path's, reused: the two deterministic guards and the AI critic are pulled into a shared verdictFor() so their wording cannot drift between the two kinds, and the base drives an AI step's re-run by continuing the prior exchange with the supervisor's guidance as its next turn. A reentered guard keeps the critic's own ai() from reloading the step's exchange as its context. Part of design/workflow-resume.md (increment 3).
… later code steps An AiStep may declare ParamRequests: after the work settles, the base continues the step's own conversation to ask the model for exactly that value and pins it with setParam() for the named step, which reads it with param(). It is the handoff mechanism addressed to a step in code rather than the next step's model. The extraction runs under an $extracting guard so its turns are not checkpointed into the step's exchange row — otherwise a crash mid-extraction would leave that row holding the extraction Q&A, which a resume would replay as the step's work. Completes increment 3 of design/workflow-resume.md.
…hannel Reacting to "there are no tokens left" by calling the ask channel spent tokens to ask a model whether to continue — the front tier is the supervisor, a model call — and could not authorise a budget anyway; worse, on a resumed run that call reached the human gate before the parked worker and consumed the human's answer. So enforceBudget now just throws its resumable stop; BudgetPolicy, parseExtraTokens and the CLAW_BUDGET_POLICY knob are gone. Turning a budget stop into a resumable WaitingHuman pause the operator settles out of band is the next step. See DECISIONS 2026-07-25 and design/human-requests.md.
… fail the run A budget stop is marked (WorkflowException::budgetSpent) so the run path tells it from the other deliberate stops. On it the ticket goes to WaitingHuman and the run is left resumable — its status untouched — so raising the limit and running the issue again picks it up where it stopped, rather than failing the run and handing it back to triage, which would throw away work the budget only interrupted. Both the solver and direct paths honour it. Advances design/human-requests.md (budget as a request); the visible typed request + UI is still to come.
From a comment-and-quality review of the branch: the attempt fingerprint is one private helper instead of two copies, so both step kinds decide "the same" the same way; isAiStep() defers to aiAttribute() rather than reflecting a second time; and the stale comments are corrected — the turn loop no longer claims a single checkpoint site, and the class and DEFAULT_MAX_ROUNDS docs name the #[StepAI] kind alongside #[Step].
…(#[StepAI]) understand/assess/draft/save move off the imperative ai() path onto #[StepAI]+#[Step]. State flows on the engine's channels, not fields: the plan and difficulty reach draft as addressed params; draft's model records the solver via the artifact tool (a pure #[StepAI] cannot call artifact()) and hands the source to save as a code param; save runs define_workflow and, on a validator reject, re-drafts once via back() instead of an ai() revise callback. Coexists with the old path — generated solvers and every other workflow are untouched.
SuperviseWorkflow and the ReviewFileWorkflow example move off the imperative ai() path. Supervise's repair declares its exchange and hands the corrected source to save as a code param; save runs define_workflow and re-writes once via back() on a validator reject. The example now shows the two-kind shape (CODE #[Step] + declarative #[StepAI] with an attribute critic) instead of hand-rolled ai() calls and a critic-as-substep.
…ive shape draftPrompt() now teaches #[StepAI] returning an AiStep (not #[Step]+$this->ai()): the two kinds of step, output on channels (artifact via the model's artifact tool, handoff, ParamRequest params), critics on #[StepAI(critic:)], and the model asking/verifying inside the one declared exchange. The granularity philosophy (RECIPE) is style-agnostic and stays. criticRules()'s 'real work' / 'placeholder' definitions move to the declarative shape too.
…example A live generation against a real model adopted the declarative shape (#[StepAI]/AiStep) but garbled the AiStep call syntax; a full copyable method body — one `return new AiStep(...);`, PHP heredocs not Python triple-quotes — anchors it. The critic and validator already reject a malformed draft, so this is a quality nudge, not the gate.
Every test that drove a step with $this->ai() now declares a #[StepAI] exchange (or, where the ai() call was inert filler for a critic/budget probe, a CODE step that records the judged artifact by hand). ProbeWorkflow reaches a model exchange the only way the declarative model allows — a #[StepAI] the base runs, set by runAi(). Dropped one test that exercised TWO ai() calls in one step: a #[StepAI] declares exactly one exchange, so that mechanic no longer exists. No real $this->ai() caller remains in tests (two non-executed solver fixtures aside).
Nothing writes imperative #[Step]+$this->ai() steps any more, so the path goes: - remove protected ai() and the machinery only it used (resumeHistory, stepHistory, the reentered-guarded loadExchange); the declarative runAiExchange resumes from the store directly. - judge() drives the critic exchange through runTurns() (a fresh, review-palette turn) instead of ai(); saveGeneratedWorkflow() is gone, both callers having inlined define_workflow + back(). - WorkflowValidator recognises #[StepAI] as a step: its checks keyed off `#[Step\b`, which does not match StepAI, so a declarative solver was rejected for declaring 'no step' and its critic bindings went unchecked. Now covered, with a test. - FixBugWorkflow (the last library solver) and every remaining teaching prompt (SuperviseWorkflow's repair recipe, DefineWorkflowTool's description) move to the declarative shape, so a generated or repaired solver never reaches for a method that is gone.
… complaint
Adversarial review found a regression: save's back('draft'/'repair') re-enters the step FRESH
(the exchange was cleared when it first finished), so the old critique branch — which handed the
model only the complaint on the assumption it still held its prior attempt — asked it to 'fix'
a class it could no longer see. It near-always failed the one repair pass. The re-draft now
prepends the complaint to the WHOLE brief (task/plan/recipe for the generator, broken source +
error for supervise), so the model rebuilds a correct class. The false 'still holds its previous
attempt' comments are corrected, and every dangling {@see ai()} doc-reference to the deleted
method is repointed to the model exchange / reviewedExchange.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Issue #87: the ask/wait-for-response flow did not survive a restart. A step parked on a human question recorded nothing durable at the park point, so a resumed run asked the model again and the supervisor could answer in the person's place. Fixing that properly meant making a step an atomic unit that either re-runs whole or replays from a record — which is what this branch builds, and then it removes the old imperative path it replaces.
The model
A step is now one of two kinds, told apart by attribute before the body runs:
#[Step]— a CODE step. Pure, deterministic glue; may call$this->tool(...); re-run whole on resume.#[StepAI]— an AI step. A PURE method returning anAiStepdeclaration (prompt, tools, agent, params). The base runs the ONE declared exchange, records it, and on resume CONTINUES the recorded conversation — a crash while PARKED on a human question resumes from the answer, never re-asks.Because a step is atomic there is no mid-step state to serialise: EMPTY → open the prompt; SETTLED → the run carries on; PARKED → the person's answer becomes the next turn.
What's here
[question]park;pendingQuestion()reads it back so resume tells a park from a settled exchange. A parked run's ticket sits inWaitingHumanwith the question in the journal; the answer is matched to the question by id.WaitingHumanfor a person to raise the limit and resume — it no longer calls the ask channel (which would spend tokens to report "no tokens" and, on a resumed run, steal the worker's answer).back()carries its reason. Aback()into a step hands it the reason viacritique(), on both kinds.#[StepAI]hands work on via an artifact (recorded by the model through theartifacttool), the automatic handoff, or aParamRequestaddressed to a later CODE step.GenerateIssueWorkflow, the library workflows (SuperviseWorkflow,FixBugWorkflow, theReviewFileWorkflowexample) and the generator's recipe all move to#[StepAI], so a generated or repaired solver never reaches for imperativeai().ai()path is deleted.protected ai()and the machinery only it used (resumeHistory,stepHistory, thereentered-guarded exchange reload) are gone;judge()drives the critic exchange throughrunTurns()directly.WorkflowValidatornow recognises#[StepAI]as a step (its checks keyed off#[Step\b, which does not matchStepAI, so a declarative solver would have been rejected for "no step" and its critic bindings left unchecked) — with a test.draft/repairfrom a fresh exchange, so the re-draft is handed the whole brief plus the complaint, not a bare one-line "fix it".Testing
composer qagreen: PHPStan level 8 clean, 380 tests, code style clean. The design lives indev/design/workflow-resume.mdanddev/design/human-requests.md; the decision indev/DECISIONS.md(2026-07-25).Closes #87.