[myzie]

Depends on #91 (the guard PRD, which creates the decisions/ module) and on a new expr API (Program.Calls, Program.Literal, deepnoodle-ai/expr#32).

Proposal

A developer who writes agent workflows often branches on a judgment: is this urgent, is the draft done, which team owns this. Today that takes a step whose only job is to feed the next edge. This PRD lets the author ask the question in the edge itself. decide(question, subject) returns the probability of yes. choose(question, subject, options) returns one option, typed as an enum, so Load checks the switch cases. Apply stays pure. The question goes out as an ordinary dispatch of kind decision. The answer is saved in state, and a replay never asks again.

- name: intake
  action: refunds.fetch
  match: first
  next:
    - step: approve
      when: decide("Is this refund legitimate?", intake.body) > 0.9
    - step: deny
      when: decide("Is this refund legitimate?", intake.body) < 0.2
    - step: human_review

Issue

#86. It asks for typed decision calls in when and switch, with no I/O in Apply.

Decisions to check

• A host turns on decisions with the decision kind (workflow.KindDecision) and one handler. Hosts already declare kinds this way. Rejected: a new LoadOption.
• A decision is a dispatch with the StepID <node>.next#N. Retry, timeout and idempotency come for free. Rejected: new command and event types.
• Every call in a next block is asked, in one job, with no short-circuit. Equal questions are asked once. The questions then never depend on answers. Rejected: lazy evaluation.
• A failed decision fails the step it leaves. Its value becomes {error: ...}, and its catch applies. Load already types catch routes this way. Rejected: keep the output readable on the catch route.
• Every decision uses one fixed policy: 3 attempts, 1 minute each. The step's retry and timeout are for its own work, which can take 30 minutes. Rejected: inherit the step's policy.
• A host must not wrap the decision handler in a Guard (docs(prd): check each job with a decision model before it runs #91). A flagged or failed check would fail the source step. Rejected: let the Guard check decision jobs too.
• Load finds the calls with two new read-only expr methods. Rejected: parse expressions in the root, which would copy expr's rewrite rules.

Not in scope

• score, plain-English conditions and a decide step kind. Two functions cover the use cases.
• catch on control steps such as join. Issue Route a failed decision on a join, set or sleep step #93.
• Model, provider or threshold settings in the definition. The host owns the model.
• The real handler. It goes in decisions/ after docs(prd): check each job with a decision model before it runs #91.

PRD review

Verdict: revise. All seven findings were fixed in 4bc3b33.

1. Catch-route typing: fixed. A failed decision makes the step's value the error value.
2. Control steps: fixed. They use the default policy, and a failed decision fails the run. New use case UC-6. Route a failed decision on a join, set or sleep step #93 covers catch on control steps.
3. A bare decide(...) in when: fixed. It gets its own Load error.
4. Subject format: fixed. A string goes as is, other values as canonical JSON.
5. Subject copies in state: fixed. State keeps a hash.
6. Kind name clash: fixed. The name stays decision, and a clash is a Load error. hosts.md notes Guard.
7. Visibility: fixed. Each saved answer records its question.

PRD: planning/prds/013-decision-edges.md · Spec: planning/specs/013-decision-edges.md · Implementation: #103

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 3 billable files and costs up to $0.75.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 36 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 73 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Essentials
• Run ID: e25b9bb6-cba7-4386-bb14-2829041fc07e

Commits

Reviewing files that changed from the base of the PR and between 7b7b09d and 3067445.

Files selected for processing: 3

• planning/README.md
• planning/prds/013-decision-edges.md
• planning/specs/013-decision-edges.md

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[mobiusops]

Comment: Two fixes must land before merge: the Guard claim in the PRD (S1) and the "we confirmed" sentence in the spec (S2).

Docs only: the PRD and tech spec for decide and choose edges (#86). The engine, the expr v1.4.0 release (go.mod is still on v1.3.2), and the real handler come later. I checked the design text against the current code: route, failNode, canStillArrive, the Retry defaults, the control-step rules, the hostcore timer check, and the example names all match.

Should fix

S1. A Guard would fail decision jobs, not just add an ask (planning/prds/010-decision-edges.md:481)
The PRD says a Guard on decision jobs is "harmless but costs an ask." It isn't. The Guard in #88 and PRD #91 blocks by default and fails closed. Its default text is the JSON of the job's params, which holds the decision's questions and subjects. So a flagged or unavailable check on a .next#N job fails the source step: catch routes it, or the run fails. In hold mode, the decision waits until a person approves it. Fix: change the risk to say a Guard must skip decision jobs, and put that exclusion in the Guard PRD (#91), which doesn't mention kinds yet.

S2. The spec says ask was checked, but ask isn't built (planning/specs/010-decision-edges.md:305)
"We confirmed both paths against ask" refers to a function that isn't in the repo. The claim can't be true yet. Fix: say the replay path is a test to write, and name it, for example as a case in DecisionNotAskedTwice.

Should fix, follow-up is fine

S3. The guard PR and module name disagree (planning/prds/010-decision-edges.md:5)
The PRD header says it depends on #88, and the PR body says #91. #88 is the Guard issue, and it names the module decide/. PRD #91 names decisions/ and gives the reason. Fix: cite #91 for the module and #88 for the issue, in the PRD header, the spec header (:8), and the spec's "after #88" lines (:36, :426).

Questions

Q1. A decision uses the source step's retry and timeout (planning/specs/010-decision-edges.md:311). An agent step with a 30-minute timeout would let one hung judgment hold the run for 30 minutes per attempt. Is that intended?

Q2. Is one minute per attempt enough for a yes-or-no judgment over a large subject, such as UC-6's full diff? A slow model could time out on every attempt (planning/specs/010-decision-edges.md:316).

Nits

• N1. The "Decision handler" bullet runs past 80 columns; rewrap it. (planning/prds/010-decision-edges.md:113)

Prompt for your AI tool: fix S1, S2, S3

Fix these review findings in deepnoodle-ai/workflow, PR #95 (docs only).

1. S1, planning/prds/010-decision-edges.md, Risks section, bullet "The
   decision kind can reach a Guard": it says a Guard on decision jobs is
   "harmless but costs an ask." Wanted: say that a Guard blocks by default
   and fails closed, so a guarded decision job can fail its source step or
   hold the run. A Guard must skip jobs of kind "decision". Update the same
   point in documentation text (hosts.md line in the PRD's R-17). Also add
   that exclusion to the Guard PRD (#91). Test: none (docs).
2. S2, planning/specs/010-decision-edges.md, "State shape" note for #87:
   "We confirmed both paths against `ask`" refers to code that does not
   exist. Wanted: state that the replay path is a test to write, and name
   it (for example, a case in `DecisionNotAskedTwice`). Test: none (docs).
3. S3, planning/prds/010-decision-edges.md line 5 and the spec header and
   "after #88" lines: #88 is the Guard issue and names the module decide/;
   PRD #91 names decisions/. Wanted: cite #91 for the module name and #88
   for the issue, consistently in both files. Test: none (docs).

Keep the PRD and spec consistent with each other after the edits.

[mobiusops]

Reviewed head e967586. Verdict: Comment. Docs-only PR, so no shipped code is wrong. I did not run tests, builds, or linters, and did not use an explorer.

Since 23ab191: the PRD now links PR #95 and #93. The spec drops the Piped field and says expr desugars pipes, so x | decide("q") reports decide(x, "q"). That matches the open expr#32 (not yet released). I did not verify it against code here, since expr is not in this repo and v1.4.0 is not in the module cache. The pipe error row is removed, which is fine.

Still open from the last review:

• S1, fix before merge: a Guard would fail decision jobs, not just add an ask. planning/prds/010-decision-edges.md:480-482 says a Guard on decision jobs is "harmless but costs an ask." Under #88 and Guard PRD #91, a flagged job does not run, and a Guard that cannot decide fails closed. So a Guard on a decision job can fail the source step, and in hold mode it can stall the run. Say that in the PRD, and make the "leave it out of the Guard" rule a requirement, not a note in hosts.md.
• S2, fix before merge: "We confirmed both paths against ask" is unsupported. planning/specs/010-decision-edges.md:304-305. There is no ask in this repo, and this PR is docs-only. Reword it as a planned test and name the test in the test plan.
• S3, follow-up: name mismatch. The PRD uses decisions/ for the module. #88 says decide/, and Guard PRD #91 says decisions/. Pick one before implementation.
• Q1: Should decision jobs inherit the step's timeout and retry? An agent step can have a long attempt (30 minutes was cited). Or should decisions always use the 1-minute default?
• Q2: Is 1 minute per attempt enough for a large subject, such as a full diff?
• N1, nit: The Decision handler bullet (planning/prds/010-decision-edges.md:113) is 99 columns. The files already have several long lines, so this is minor.

No blocking findings. S1 and S2 need fixing before merge.

[mobiusops]

Comment: Updated for 129b088. The new commit adds a resume claim that the code does not support yet. S1, S2 and S4 need fixing before merge.

Still open

• S1. A Guard would fail decision jobs, not just add an ask. (needed before merge)
• S2. "We confirmed both paths against ask" describes code that is not built. (needed before merge)
• S3. Module name: the PRD says decisions/, #88 says decide/. (fine as a follow-up)
• Q1, Q2, N1 from the last review are unchanged.

New

S4. Resume skips a running decision (outstanding.go:30)
The spec says Outstanding rebuilds a running decision's dispatch and timer, so local can resume a run that waits on a decision (planning/specs/010-decision-edges.md:363-364). The loop reads only n.Units. A decision's unit lives in n.Decision.Unit (planning/specs/010-decision-edges.md:159-162, :259-264), so resume never re-asks it, and the run waits forever. The implementation plan's hostcore list does not name this change. Fix: name the outstanding.go change in the spec's Hosts section and implementation plan, and plan a resume test for a run waiting on a decision (local/resume_test.go).

The Unit.Output and State.Unit wording in this commit matches the code: apply.go:529 sets output on completion, and AttemptOutcome reads it.

Prompt for your AI tool: fix S1, S2, S3, S4

Fix these review findings in deepnoodle-ai/workflow, PR #95. All are in
planning/prds/010-decision-edges.md or planning/specs/010-decision-edges.md.

1. S1, planning/prds/010-decision-edges.md, "The decision kind can reach a
   Guard" (line ~481): The PRD says a Guard on decision jobs is "harmless
   but costs an ask." Under #88 and Guard PRD #91, a flagged job does not
   run, and a Guard that cannot decide fails closed. So a Guard on a
   decision job can fail the source step, and in hold mode it can stall
   the run. Wanted: say that, and make "leave decision jobs out of the
   Guard" a requirement, not a note in hosts.md. Test: none (docs).

2. S2, planning/specs/010-decision-edges.md, "We confirmed both paths
   against `ask`" (lines ~306-307): There is no `ask` in this repo, and
   this PR is docs-only. Wanted: reword as a planned test and name it in
   the test plan, such as the DecisionNotAskedTwice test. Test: none
   (docs).

3. S3, planning/prds/010-decision-edges.md and
   planning/specs/010-decision-edges.md: The module is named `decisions/`,
   but #88 names it `decide/`. Wanted: pick one name and use it in both
   files, and note the choice against #88 and #91. Test: none (docs).

4. S4, planning/specs/010-decision-edges.md, Hosts section (lines
   ~363-364) and the implementation plan: The spec says `Outstanding`
   rebuilds a running decision's dispatch and timer. outstanding.go:30
   loops only over n.Units, and a decision's unit is in n.Decision.Unit.
   Wanted: list the outstanding.go change in the Hosts section and the
   implementation plan, and add a resume test for a run waiting on a
   decision (local/resume_test.go). Test: resume a run that waits on a
   decision, and check that the decision is asked again.

[myzie]

Replies to the review of e967586. All fixes are in 847684a. That commit also renumbers the PRD and spec to 013, because 009 to 012 are taken by other PRDs.

S1. A Guard would fail decision jobs, not just add an ask

Fixed in 847684a. New requirement R-15a: a host must not wrap the decision handler in a Guard. A flagged or failed check fails the source step. The Risks bullet now says this, hosts.md will state the rule, and there is no code coupling between the two. The Guard (#91) now only blocks and has no hold mode.

S2. The spec says ask was checked, but ask isn't built

Fixed in 847684a. The #87 note now names a planned test, TestDecisionReplayAsksTheSameQuestion, and the test plan lists it. The test re-applies a run's events to a fresh state. It requires the same decision job, subject text and ID.

S3. The guard PR and module name disagree

Fixed in 847684a. The module is decisions/. The PRD and spec headers cite PRD PR #91 for the module and #88 for the issue. The "after #88" lines now say #91.

Q1. A decision uses the source step's retry and timeout. ... Is that intended?

Fixed in 847684a. No. A decision never inherits the step's retry or timeout. Every decision uses one fixed policy: 3 attempts, 1 minute each. The step's policy is for its own work, which can take 30 minutes. A decision is a short model call, and one rule is easier to explain. A failed decision still fails the source step, and its catch applies. UC-5, UC-6, R-10, Decisions and the spec are updated.

Q2. Is one minute per attempt enough for a yes-or-no judgment over a large subject?

Fixed in 847684a. For decision models, yes: they answer in seconds. The PRD records the risk. A timeout is retried twice, then fails the step, and catch can route that failure. If users hit the limit, we change the constant.

N1. The "Decision handler" bullet runs past 80 columns

Fixed in 847684a. The bullet is rewrapped.

[mobiusops]

Reviewed 847684a (one commit since 129b088: renumber to 013 and address the review). Docs only. I read the delta and the touched files in full. I did not run tests, builds or linters.

Verdict: Comment. 0 blocking findings. One fix before merge (S4), plus one PR-body fix (N2).

Resolved since 129b088

• S1 (Guard on decision jobs called "harmless"): fixed. R-15a and the hosts.md rule now say a Guard fails the source step. Good.
• S2 (claim of confirmation against ask): fixed. The spec now names TestDecisionReplayAsksTheSameQuestion as the check. Good.
• S3 (decide/ vs decisions/): fixed. Only decisions/ appears now.
• Q1 (timeout/retry inheritance): resolved by the fixed one-policy design (R-10), which states the rejected alternative.
• Q2 (one minute for large subjects): accepted in the PRD risks section with a stated reason. Fine as a known risk.
• N1 (long line): no added line over 80 columns.

Open

• S4 (must fix before merge). The spec's Hosts section says Outstanding "rebuilds a running decision's dispatch and timer from state, so local resumes it." outstanding.go (line ~30) loops only over n.Units. The spec puts decision state in n.Decision.Unit, so that loop never reaches it. The Rollout step 2 list also leaves out outstanding.go. Fix: add an outstanding.go change (iterate the node's decision unit too) to the Rollout list and the test plan, or drop the claim. The resume path is what the replay guarantee depends on, so it should be named.
• N2 (PR body, not the docs). The PR body still describes the old design: "Retry and timeout come from the step, else a fixed default" (now R-10 says the fixed policy only). It also links planning/prds/010-decision-edges.md and planning/specs/010-decision-edges.md, which no longer exist after the rename to 013. Update the body to match.

Not blocking

• Renumbering to 013 leaves a gap at 010–012. Fine if intended; confirm that 010 is not reserved elsewhere.

[mobiusops]

Thanks, I checked the replies against 847684a. S1, S2, S3, Q1, Q2 and N1 are fixed in the diff, as you describe.

N2 is also fixed: the PR body now uses the fixed decision policy and the 013 paths.

One item is still open before merge: S4. The spec says Outstanding rebuilds a running decision's dispatch and timer, so local can resume it. outstanding.go loops only over n.Units, and decision state lives in n.Decision.Unit. The rollout list also doesn't name outstanding.go. Either add that change to the rollout and test plan, or drop the claim.

[mobiusops]

Reviewed f13f58b, the one-commit delta on 847684a. It adds seven lines to planning/specs/013-decision-edges.md on typing each decision subject alone. I checked it against the spec's ask flow and the typer (scopeWith, the switch optional check, and the existing "may be nil" errors). No tests, builds, or linters were run.

Verdict: Comment. 0 blocking.

Non-blocking

• N3 (wording, and a missing error row). "Load rejects a subject that can fail" claims more than Load can check. The typer has no general failure analysis. It rejects only an unguarded read of a possibly-nil value, the same check that gives the existing "may be nil" errors. It cannot see index errors, divide by zero, or failing functions. Those fail the run at runtime with ErrorTypeExpression, as the spec already says. Suggest: "Load rejects a subject that reads a possibly-nil value without ?.", and add a row for it to the Load errors table. That table does not list this error yet.

Still open from earlier reviews

• S4. The spec (line ~372) says Outstanding rebuilds a running decision's dispatch and timer. outstanding.go iterates only n.Units, and the spec's state design keeps the decision unit in n.Decision.Unit. The rollout's step 2 file list does not include outstanding.go. Either list it as a change and say how Outstanding finds the decision unit, or narrow the claim.

Checked and sound

• The reasoning holds. ask evaluates every subject before routing, with r.env(p, nil), so the condition's nil guard never applies to a subject. Typing the subject alone in the edge scope is the right place to catch that.
• Rejecting lazy asking is consistent with one job per block and with questions that do not depend on answers.

[myzie]

Replies to the review of f13f58b. Both findings are fixed in 1805137.

N3 (wording, and a missing error row).

Fixed in 1805137. The spec now says Load rejects a subject that reads a possibly-nil value without ?.. Other run-time failures stay outside the promise and fail with ErrorTypeExpression. The Load errors table has a row for this error.

S4. The spec says Outstanding rebuilds a running decision's dispatch and timer.

Fixed in 1805137. Rollout step 2 lists outstanding.go, and the Hosts section says how it finds the unit: it checks n.Decision.Unit on each node and builds the commands with the first dispatch's code. #103 does this, and TestDecisionOutstanding covers it.

[mobiusops]

Approve: Updated for f13787e. The only change is a one-line link from the PRD to the implementation PR, #103.

Fixed: S4 is resolved in #103. outstanding.go at fb5a6d5 rebuilds a running decision from n.Decision.Unit (line 28). The spec's claim now matches the code.

Still open

• N3. Spec line 151 says Load "rejects a subject that can fail". The typer only catches unguarded reads of possibly-nil values. Runtime failures are not caught. Reword it and add a Load error row. (fine as a follow-up)

Earlier comment on this PR still stands for the rest of the spec.

[mobiusops]

Reviewed 1805137 (diff from f13787e, 1 file: planning/specs/013-decision-edges.md). Docs only; no tests, builds or linters run.

Verdict: Approve. 0 blocking.

Status of earlier findings

• N3 (subject-failure wording, missing Load error row): fixed. The spec now says Load rejects a subject that reads a possibly-nil value without ?., which matches what the typer checks. Other run-time failures are stated as outside the promise, failing with ErrorTypeExpression. The Load error table has the matching row.
• S4 (Outstanding and n.Decision.Unit): fixed. Rollout step 2 lists outstanding.go, and the Hosts section says it checks n.Decision.Unit. I confirmed this against the implementation in #103 earlier.

Non-blocking

• N4 (nit): the new paragraph line "outside Load's promise and fail the run with ErrorTypeExpression. Rejected: ask lazily, which would make" runs about 100 columns, while the rest of the paragraph wraps near 78. Rewrap it when convenient. This is the same kind of issue as N1 in the first review.
• N5 (nit, for #103 and the PRD's example): the example intake.body?.text is a valid fix, but the spec does not say whether decide accepts an optional subject after ?.. A short note would help implementers.

[mobiusops]

Comment: Updated for 64fbcb3. S1 must land before merge, because the new retry rule does not match how the engine retries today.

The change makes every decision failure, including a malformed answer, use one of the 3 attempts. The rest of the diff is consistent. The PRD and spec now agree, and the tightened invalid_output text matches.

Should fix

S1. Malformed decision answers are not retried by the existing path (planning/specs/013-decision-edges.md:257, :333)
The spec says a malformed answer uses one of the 3 attempts. The existing retry path does not do that. unitFailedAttempt retries only when err.Retryable is set (apply.go:538). The invalid_output errors from unitCompleted are built without that flag (apply.go:503, apply.go:509). The Retry doc also says nothing retries without a retryable failure (definition.go:202). So if a decision returns a string where a number belongs, or drops one question ID, it fails on attempt 1. That contradicts R-10 and the PR body's "Retry … come for free." Fix: say in the spec that a decision retries any failure, and add that change to rollout step 2 with apply.go. Add a test where a malformed answer is retried and then succeeds.

Nits

• N4. Still open: the line that starts "outside Load's promise" is about 100 columns, while the paragraph wraps near 78 (planning/specs/013-decision-edges.md:155).
• N5. Still open: the spec does not say whether decide accepts an optional subject after ?..
• N6. The new text at planning/specs/013-decision-edges.md:259 runs about 110 columns. The sentence "The policy does not depend on the step's / kind" also breaks mid-phrase around line 336.

[mobiusops]

Reviewed 79880af, the only commit since 64fbcb3. It changes only planning/specs/013-decision-edges.md.

Verdict: Comment. 0 blocking. S1 is still needed before merge, but only the test-plan half.

S1 (partly resolved). The spec now says decisions get their own retry path in decision.go and not unitFailedAttempt, and rollout step 2 names apply.go. That answers the engine half. The test plan still has no case for the behavior the spec now promises. The invalid_output bullet says each bad answer "uses one of the 3 attempts," but it never shows a retry that succeeds. Add a case: a malformed answer on attempt 1, a valid answer on attempt 2. Check that the step routes and that the step record shows two attempts.

N4 (non-blocking). Several spec lines are over 100 characters, up to about 310 at line 127. If those lines are code or a diagram, leave them. If they are prose, wrap them.

N5 (resolved). The optional-subject wording now matches the possibly-nil rule. intake.body?.text is allowed and asks about null.

N6 (non-blocking). The rollout step 2 list wraps unevenly, with the event.go line running long.

N7 (non-blocking, unverified). The spec says invalid_output is marked Retryable "so a host's step record says the attempt was retried." I did not check whether step records read Retryable. If they do not, drop that sentence, or add the code path that does.

Not reviewed here: PR #103. Checks run: none. No tests, builds, or linters.

[myzie]

Replies to the review of 64fbcb3. The spec fixes are in 79880af, and the code is in #103.

S1. Malformed decision answers are not retried by the existing path

Fixed in 79880af, with the code in #103 (6f2bc3e, de3beff). Decisions do not use unitFailedAttempt. They have their own path in decision.go: every failure goes to decisionFailedAttempt, which dispatches the next attempt until the third. That includes a handler error, a timeout, a bad answer and the state limit. Host steps keep today's rule. A decision's invalid_output is also marked Retryable, so a step record shows the retry. The spec now says this. Rollout step 2 lists decision.go and apply.go with their roles. TestDecisionBadAnswerThenAGoodOne returns a malformed answer on attempt 1, checks that attempt 2 is dispatched with a retryable invalid_output, and requires the run to route on attempt 2's answer.

N4. the line that starts "outside Load's promise" is about 100 columns

Fixed in 79880af. The paragraph is rewrapped.

N5. the spec does not say whether decide accepts an optional subject after ?.

Fixed in 79880af. It does. decide("q", intake.body?.text) is allowed, and when the value is nil it asks about null. The subject rule in the spec now says so.

N6. The new text ... runs about 110 columns ... breaks mid-phrase

Fixed in 79880af. Both places are rewrapped.

[mobiusops]

Thanks. I checked each reply against the code.

• S1, closed in code. TestDecisionBadAnswerThenAGoodOne in feat: branch on a model's judgment with decide and choose in edges #103 (decision_test.go:522) exists. It returns a malformed answer on attempt 1, requires main/intake.next#2 with a retryable invalid_output, and requires the run to complete on main/approve. The spec's Retry section now matches the code.
• S1, one line still open in this PR. The spec's test plan (013-decision-edges.md, around line 485) still has only "each using one of the 3 attempts." Add a bullet that names the case: a malformed answer on attempt 1, a valid one on attempt 2, and the route is taken. Once that is in, I can approve.
• N4, resolved. The "outside Load's promise" paragraph is rewrapped. The remaining long lines are Load-error table rows, which is normal in markdown.
• N5, resolved. Optional subjects are allowed and asking about null is described.
• N6, resolved. Both rewraps are in 79880af.

Verdict unchanged: Comment. The one item needed before merge is the spec test-plan bullet for S1. Nothing else is open on this PR. I did not review #103 in this pass.

[myzie]

Replies to the review of 79880af. The spec fixes are in 044e77a.

S1 (partly resolved). ... Add a case: a malformed answer on attempt 1, a valid answer on attempt 2.

Fixed in 044e77a. The test plan has a bullet for TestDecisionBadAnswerThenAGoodOne: attempt 1 returns a malformed answer, attempt 2 a valid one, the step routes on attempt 2's answer, and two decision attempts are dispatched. The test is in #103.

N4 (non-blocking). Several spec lines are over 100 characters

Fixed in 044e77a. The remaining long lines are in the overview diagram, a Go code block and the Load errors table. The prose lines wrap near 78.

N6 (non-blocking). The rollout step 2 list wraps unevenly

Fixed in 044e77a. Rollout step 2 is rewrapped.

N7 (non-blocking, unverified). The spec says invalid_output is marked Retryable "so a host's step record says the attempt was retried."

Fixed in 044e77a. No step record reads Retryable; hostcore, postgres and sqlite use it only to decide whether an interrupted claim is given back. The spec no longer makes the claim.

[mobiusops]

Reviewed the delta from 79880af to 044e77a (planning/specs/013-decision-edges.md only). Verdict: approve. 0 blocking.

• S1 (closed): The test plan now has a TestDecisionBadAnswerThenAGoodOne bullet: attempt 1 malformed, attempt 2 valid, routes on attempt 2, two decision attempts dispatched. PR #103 contains this test, and it matches the test plan.
• N7 (closed): The unverified claim that the step record says "retried" is removed. The spec no longer makes that claim.
• N6 (non-blocking, cosmetic): The rollout step 2 rewrap is fine as is.

Doc-only change. I did not run tests, builds, or linters.

[mobiusops]

Thanks, @myzie. Closing the review items from 79880afa:

• S1: closed. The test plan line for TestDecisionBadAnswerThenAGoodOne covers the malformed-then-valid case.
• N4: closed. Limited to the table rows, as I asked. The rest is fine as is.
• N6: closed.
• N7: closed. The spec no longer makes the step-record claim. I didn't trace the hostcore, postgres, or sqlite uses of Retryable, so I'm not confirming that part.

Still approved at 044e77a4. Nothing else needed from this PR.

[mobiusops]

Approve: Updated for 3067445. The only change is a merge of main, and the PR's three files are the same as the last reviewed head.

Merging main adds the PRDs and specs for #89 to #92 to planning/README.md and README.md. The 013 PRD and spec are byte-identical to 044e77a. The earlier S1 and N-items are still closed. No open items remain.