docs(replan-target): the flagged follow-up is done — the note said otherwise (#2665)
Comment only. The block above `resolveReplanTargetColumn` still reads: > **STILL A REAL FOLLOW-UP** … the `return "triage"` fallbacks on the no-match and throw paths name a column the default lineage no longer declares … flagged rather than fixed That directly contradicts the code six lines below it. **#2598 landed the fix:** the no-match path now returns `roles?.hold ?? roles?.intake`, and the throw path returns `undefined`. I wrote that note. A stale *"not fixed yet"* sitting above a fixed implementation is worse than no note — the next reader either distrusts the code or re-does work that is already done. This is the closing-bar item 3 I was assigned, and I nearly re-did it myself: I had the change written and reverted before checking whether main had overtaken me. ## One thing worth recording about #2598's version Its catch-path answer is **stronger than the one I had drafted**. I was going to return `"todo"` — the better guess, since post-U11 the default lineage declares `todo` and not `triage`. #2598 returns `undefined` instead, which forces callers to handle "this workflow could not be resolved" explicitly rather than papering over it with a plausible column id that the move path may then reject. That is the same lesson as the sync-reader audit in #2653: **a defective lookup that returns a valid-looking answer is worse than one that admits it does not know.** Recorded in the comment so the reasoning survives. ## Verification Engine typecheck clean · **51/51** across both replan-target suites · comment-only, no executable change. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -343,10 +343,11 @@ export async function resolveReplanTargetColumn(store: TaskStore, taskId: string
|
||||
planning column and the right replan target. `triage` still matches for the workflows that
|
||||
genuinely declare it (Lead generation, PR review — where it is the intake/planning lane).
|
||||
|
||||
STILL A REAL FOLLOW-UP, unchanged by the correction: the `return "triage"` fallbacks on the
|
||||
no-match and throw paths name a column the default lineage no longer declares, so a workflow
|
||||
with neither `triage` nor `todo` is handed a nonexistent target. Behavior change, so it is
|
||||
flagged rather than fixed in a comment correction.
|
||||
FOLLOW-UP DONE (#2598), and this note is kept only so the history reads straight: the
|
||||
`return "triage"` fallbacks it flagged are gone. The no-match path now resolves the
|
||||
workflow's own planner lane and the throw path returns `undefined` rather than guessing a
|
||||
column, which is the stronger answer — an unresolvable workflow is a case callers must
|
||||
handle, not one to paper over with a plausible id. See the two blocks below.
|
||||
*/
|
||||
if (workflowHasColumn(ir, "triage")) return "triage";
|
||||
if (workflowHasColumn(ir, "todo")) return "todo";
|
||||
|
||||
Reference in New Issue
Block a user