Commit 04aa7971d1cab1256b97756859f3f556082231d5
1 parent
43c3f050
graphlemon: auto-commit uncommitted changes for 18a9005c-aba8-4e35-88a6-184de4b9…
…4843-main-checkout-drift
Showing
15 changed files
with
2633 additions
and
203 deletions
docs/todo/task-execution-resume-assumes-planner-artifacts-exist.md
0 → 100644
| 1 | +# `task_execution.py` 恢复逻辑假设 Planner 一定已产出 sdd/prd/benchmark artifact——问题记录 | |
| 2 | + | |
| 3 | +**日期**: 2026-08-08 | |
| 4 | +**性质**: 问题记录(真实复现,根因已定位,未修复)。发现于 `docs/todo/work-graph-self-planning-and-loop-back-requirements.md`(Planner 自规划)上线后的第一次真实端到端验证。 | |
| 5 | +**状态**: 待修复,未立项。 | |
| 6 | + | |
| 7 | +## 1. 现象(真实复现) | |
| 8 | + | |
| 9 | +任务 `2084590a`(目标:修复 `work_node_run.py` 的 `execution_log_file` UNIQUE 约束 bug): | |
| 10 | + | |
| 11 | +1. 第一次真实调用:Planner 自规划出 `sdd → benchmark → test_script` 三节点图(没有 brd/prd——goal 是个小 bug 修复,Planner 判断不需要),`sdd` 节点的 `build_prompt()` 因为 `_DOC_PROMPTS["sdd"]` 硬编码的 `{prd_content}` 占位符找不到值而抛 `KeyError('prd_content')`(已修复,见 `e2e_pipeline.py::_MissingContentDefault`)。此时 `sdd` 节点本身从未成功过,`artifact` 表里这个 task 一行都没有。 | |
| 12 | +2. 通过 `/route` 的 `manual_resume` 恢复:`task.status` 被设置回 `planning_brd_prd_sdd`。 | |
| 13 | +3. 重新执行时走的是 `task_execution.py::run_or_resume_task_lifecycle()` 的"从 `planning_brd_prd_sdd` 恢复"分支,而不是从头重跑 `plan_documents()`——这个分支无条件调用 `_artifact_content(conn, ..., "sdd")`/`"prd"`/`"benchmark"`,读不到就直接 `TaskExecutionError`。因为这个 task 的 Planner 图从未成功过一个节点,三个 artifact 全部不存在,报错为 `"task ... has no sdd artifact to resume from"`,任务再次卡住。 | |
| 14 | + | |
| 15 | +## 2. 根因 | |
| 16 | + | |
| 17 | +`org_node_lifecycle.trigger_org_node_full(conn, "planner", ...)` 在 `run_task_lifecycle()` 里**先于** `plan_documents()` 执行,并把 `task.status` 设成 `_POST_STATE["planner"] = "planning_brd_prd_sdd"`(`org_node_lifecycle.py:32`)——这个状态转移只代表"Planner 的 ready 确认调用成功了",不代表"Planner 的真实规划(`plan_documents()`)已经产出了任何东西"。 | |
| 18 | + | |
| 19 | +`task_execution.py::run_or_resume_task_lifecycle()` 里 `planning_brd_prd_sdd` 分支(`task_execution.py:126-129`)的注释写的是"Planner done, Coder not yet triggered"——这个假设在 Work Graph 自规划(`docs/todo/work-graph-self-planning-and-loop-back-requirements.md`)上线前基本成立:固定链的第一个节点是 brd,几乎不可能在第一个节点就失败到零产出。自规划之后这个假设不再可靠——自规划出的图可能把一个更容易失败的节点(比如需要更复杂 judgment 的 sdd)排在第一位,如果它在第一次尝试就彻底失败(耗尽 loop_back/retry 上限,或者像这次一样是一个跟自规划本身相关的新 bug),Planner 的 `work_graph_run` 会整个失败,不会有任何 artifact 落盘,但 `task.status` 已经先一步被置成了 `planning_brd_prd_sdd`。 | |
| 20 | + | |
| 21 | +`test_dataset_content`/`test_script_content` 已经用 `_artifact_content_or_default()`(读不到就给默认值,不硬失败)处理过同一类问题(2026-08-06,那次是"这两个 artifact 类型本来就允许不存在"),但 `sdd`/`prd`/`benchmark` 三个仍然用会硬失败的 `_artifact_content()`——这个区分在固定链时代是对的(sdd/prd/benchmark 曾经是保证存在的),自规划之后不再成立。 | |
| 22 | + | |
| 23 | +## 3. 跟已实现的两个相关修复的区别 | |
| 24 | + | |
| 25 | +- 不是 `work-graph-partial-artifact-persistence-requirements.md` 那次修复能覆盖的场景——那次解决的是"图跑到一半、后面节点失败,前面成功节点的产出要保留",这次是"图的第一个节点就彻底失败,压根没有'前面成功节点'可保留",`_write_completed_node_artifacts` 在这种情况下正确地什么都不落盘(因为真的什么都没成功),不是它的 bug。 | |
| 26 | +- 不是这次 session 里刚修的 `_MissingContentDefault`(`e2e_pipeline.py::_planner_prompt_builder`)能覆盖的——那个修的是"build_prompt 拼提示词时崩溃",这次是崩溃之后**恢复逻辑**本身的假设过时了,是下游的另一层问题。 | |
| 27 | + | |
| 28 | +## 4. 可能的修复方向(未验证,仅供后续参考) | |
| 29 | + | |
| 30 | +在 `run_or_resume_task_lifecycle()` 的 `planning_brd_prd_sdd` 分支里,先检查这个 task 是否**真的有任何 doc 类 artifact**(`_latest_artifact_ids(conn, task_id, _DOC_ARTIFACT_TYPES)` 是否为空): | |
| 31 | + | |
| 32 | +- 如果有(哪怕只有部分,比如这次自规划场景下可能只有 sdd 没有 prd/benchmark),按现有逻辑继续,但 `prd`/`benchmark` 也应该跟 `test_dataset`/`test_script` 一样改用 `_artifact_content_or_default`,不再假设它们一定存在(自规划场景下 prd/benchmark 本来就可能被 Planner 判断为不需要,不是"缺失的异常情况")。 | |
| 33 | +- 如果一个都没有(这次真实复现的情况——Planner 的图从第一个节点就彻底失败,什么都没留下),说明"从这个状态恢复"这个假设本身不成立,应该等价于从 `queued` 重新完整跑一次 `run_task_lifecycle()`(即重新调用 `plan_documents()`),而不是尝试读取不存在的 artifact。 | |
| 34 | + | |
| 35 | +## 5. 临时规避(已用于解决这次真实卡住的任务) | |
| 36 | + | |
| 37 | +直接把 `task.status` 手动改回 `queued`(`UPDATE task SET status='queued', blocked_reason=NULL WHERE task_id=...`),让下一次执行走 `run_task_lifecycle()` 的"queued"分支、重新完整调用一次 `plan_documents()`——不是通用修复,只是这一次真实卡住的任务的人工解法。 | ... | ... |
docs/todo/validator-test-script-narrative-output-issue.md
| ... | ... | @@ -2,7 +2,7 @@ |
| 2 | 2 | |
| 3 | 3 | ****: 2026-08-07 |
| 4 | 4 | **扯捶**: 憸扇敶 + 敺,銝撌脩&霈斗瘙﹝嚗鈭敶隞遢遢撌脩雿誨 bug 撟嗡耨憭,餈遢"閫鞊",蝖桀 |
| 5 | -****: 歇摰洵 6 蝻圾撌脣嚗洵 7 _NARRATION_RETRY_HINT`嚗活銝曆遙 test_script 餈葵蝻圾鈭颲蝏找耨憭洵銝甈∠膩嚗憸**2026-08-07 嚗洵鈭蔭嚗 Validator 憸911861fe` 餈葵蝥臬垢隞餃 Planner 撌梁 `n_test_script` 銝鈭璅∪敞霈 3 銝芯遙6 甈∪颲 test_script 蝐餉嚗6 甈∪憭嚗100%嚗赤頝 Planner Validator 銝斤遣霈格漣嚗** **2026-08-07 嚗洵銝蔭嚗嚗 08-06 "Benchmark 膩鈭"嚗ommit `07a0a27`嚗掩憭est_script 蝐餉敶撌脩內霂璇唳蝵嚗 6/6 憭"self-hosting 霂情"餈葵瘜 test_script 內霂頨怨"極蝝W漣捆"嚗芋蝝W撠曇憭拍"瘙/鈭銋"滲璅∪elf-hosting 憭批嚗悟膩霂餉絲蝎曄&嚗蛹蝝W笆鞊∪停 graphlemon 撌梧”蝎曄&撖孵敹隞嗚RD/PRD/SDD/Benchmark/test_dataset 餈葵銝閬蝝Y滲蝏嚗 08-06 憭嚗迨** | |
| 5 | +****: 歇摰洵 6 蝑2026-08-07嚗誧蝏剖內霂/‘銝**蝳 test_script/validator_test_script **嚗lanner `work_graph_plan._PROMPT_TEMPLATE` 銝葵嚗alidator `e2e_pipeline._VALIDATOR_MANIFEST` 隞 3 蝻拇 2 嚗賒憒閬霈曇恣嚗洵 8 找耨憭"蝝""蝏"舅甈∠蝡嚗_NARRATION_RETRY_HINT`嚗洵 7 誨 `_test_script_verify_fn` ,敶頝臬◤閫血誑餈葵,餈葵蝻圾靘**2026-08-08 嚗洵蔭嚗閫 Work Graph 瘙漱蝑**嚗docs/todo/work-graph-self-planning-and-loop-back-requirements.md`嚗lanner 閫 + 憭梯揖楝瘙瑪est_script 隡雿蛹撘蝏 Planner 閫&摰**銝﹝找耨憭**嚗arrative-output 憸迨嚗撌脩銝◤﹝霈啣洵 6 迤蝏找耨憭**銝隞賢鈭"蝏 Coder agent test_script 璅∪"﹝**韏瑁圾嚗﹝蝏抒賒頝**2026-08-07 嚗洵鈭蔭嚗 Validator 憸911861fe` 餈葵蝥臬垢隞餃 Planner 撌梁 `n_test_script` 銝鈭璅∪敞霈 3 銝芯遙6 甈∪颲 test_script 蝐餉嚗6 甈∪憭嚗100%嚗赤頝 Planner Validator 銝斤遣霈格漣嚗** **2026-08-07 嚗洵銝蔭嚗嚗 08-06 "Benchmark 膩鈭"嚗ommit `07a0a27`嚗掩憭est_script 蝐餉敶撌脩內霂璇唳蝵嚗 6/6 憭"self-hosting 霂情"餈葵瘜 test_script 內霂頨怨"極蝝W漣捆"嚗芋蝝W撠曇憭拍"瘙/鈭銋"滲璅∪elf-hosting 憭批嚗悟膩霂餉絲蝎曄&嚗蛹蝝W笆鞊∪停 graphlemon 撌梧”蝎曄&撖孵敹隞嗚RD/PRD/SDD/Benchmark/test_dataset 餈葵銝閬蝝Y滲蝏嚗 08-06 憭嚗迨** | |
| 6 | 6 | |
| 7 | 7 | ## 1. 鞊 |
| 8 | 8 | |
| ... | ... | @@ -79,3 +79,17 @@ self-hosting 嚗target_folder_path` graphlemon 撌梧餈葵 |
| 79 | 79 | 霈曇恣銝隞嗅嚗"If your previous output was..."嚗閮,餈撖寧迤祗瘜秤嚗膩,誨 typo 撖潸蝻仃韐伐漣秤撖潭扳,銝銝芣拿內_test_script_verify_fn` Planner/Validator 銝芸嚗script_artifact_type` 舅颲),銝憭舅颲寥 reflect prompt feedback 捆,銝蜓內霂 `check_script_compiles()` 璇唳餉頨怒 |
| 80 | 80 | |
| 81 | 81 | 賒憒葵蝻圾隞活銝曆遙活憭),銝甇亥砲洵 6.2 "蝝W撠曆"敶餃獢閬 test_script 內霂頨恍,"蝝U漣""蝏◆蝥臭誨"餈舅銝芷畾菜&,撠曇蕭蝥 |
| 82 | + | |
| 83 | +## 8. 撌脣嚗2026-08-07嚗洵鈭活蝑蝳 test_script/validator_test_script | |
| 84 | + | |
| 85 | +蝑蝏抒賒內霂‘銝洵 7 圾撌脩扳銝活&霈歹,銝舅銝芾隞,賒瘙霈曇恣," goal 隞餃摰葵"銝銝芣憭抒憸lanner 撌梁﹝嚗work_graph_plan._PROMPT_TEMPLATE`嚗餈撅 LLM 閫,雿活靚內霂′蝻"敹◆銝 brd/prd/sdd/benchmark/test_dataset/test_script 銝銝芾"銝銝芸摰虜2026-08-06 挽霈∪摰 `docs/done/dynamic-work-graph-design-skill-design.md 禮3`嚗,銝 LLM goal 摰alidator 撌梁嚗_VALIDATOR_MANIFEST`嚗敶餃,餈"餈撅 LLM 閫"瘝⊥,蝖祉 Python 摮迤"LLM 撌勗摰葵"歇蝏鈭 `_GENERIC_PROMPT_TEMPLATE`嚗 Coder/Validator 隞遙),雿瓷餈舅﹝銝閬 Planner/Validator 撌梁﹝銝,銝銝芣憭抒閬蝑賒,銝甈⊥ | |
| 86 | + | |
| 87 | +**雿**嚗 | |
| 88 | +- `src/work_graph_plan.py::_PROMPT_TEMPLATE`嚗 test_script 嚗閬 brd/prd/sdd/benchmark/test_dataset 5 銝迎 | |
| 89 | +- `src/work_graph_plan.py::_CANONICAL_DOC_CHAIN`嚗 6 憬 5 ,郊銝蝏 test_script 瘜典"憸 benchmark"畾萎誨_normalize_graph_manifest` | |
| 90 | +- `src/e2e_pipeline.py::_VALIDATOR_MANIFEST`嚗 3 嚗alidator_benchmarkalidator_test_datasetalidator_test_script嚗憬 2 | |
| 91 | +- `validate_implementation_with_own_graph()`嚗outputs["validator_test_script"]` 挪 `in` ,銝 `script_execution` 韏"N/A: 撌脩",verdict 摰 LLM 撌梁摰蝖祇蝳 | |
| 92 | +- 銝虜瘨晶run_task_lifecycle`/`task_execution.py` 粉 test_script artifact 捆 Coder 餉撠勗歇蝏 `"test_script" in artifact_ids`/`_artifact_content_or_default` 捆 2026-08-06 test_script 瑪,銝箔摰"餈葵銋隞餃"敺⊥找誨,餈活銝閬1餈舅憭 | |
| 93 | +- `_DOC_PROMPTS["test_script"]`/`_VALIDATOR_DOC_PROMPTS["validator_test_script"]`/`check_script_compiles()`/`_test_script_verify_fn`/`_NARRATION_RETRY_HINT` 蝑虜隞靽,銝◤閫血誑,閬舅憭摰 | |
| 94 | + | |
| 95 | +**瘚**嚗src/test_work_graph_plan.py`/`src/test_e2e_pipeline.py` 靘 6/3 摰靘摨嚗銝"仃韐亥銝敞鈭批"嚗artial-artifact-persistence 靘"仃韐亙撩 FAIL"嚗ard-gate 靘撘銝銝芯葩銵乩 test_script/validator_test_script anifest 瘚蛹餈舅頨思誨,暺恕銝◤閫,銝砲蛹暺恕蝳撠勗仃python3 -m unittest discover -s src`嚗643 tests嚗+ `discover -s supervisor -p "test_*.py"`嚗126 tests嚗蝏踴 | ... | ... |
docs/todo/work-graph-self-planning-and-loop-back-design.md
0 → 100644
| 1 | +# Planner 动态自规划 Work Graph(含节点判断与失败回路)——详细设计(SDD) | |
| 2 | + | |
| 3 | +**日期**: 2026-08-08 | |
| 4 | +**性质**: 详细设计,对应需求文档 `docs/todo/work-graph-self-planning-and-loop-back-requirements.md`(已确认)。本设计给出可直接实施的 schema/接口/算法/测试方案,逐条落实需求文档「8. 已确认的决策」。 | |
| 5 | +**依赖代码**:`src/work_graph_orchestrate.py`(`run_graph`/`topological_layers`/`_run_one_node`/`_verify_and_maybe_regenerate`/`_ensure_node_row`/`_LockedConn`)、`src/work_graph_plan.py`(`_PROMPT_TEMPLATE`/`_normalize_graph_manifest`/`GRAPH_MANIFEST_SCHEMA`)、`src/e2e_pipeline.py`(`plan_documents`/`check_script_compiles`/`_check_prd_consistency`)、`src/settings_config.py`(`get_/upsert_concurrency_config` 先例)、`src/schema.py`(`migrate_expand_artifact_types` 先例)。 | |
| 6 | +**状态**: ✅ 已实现(2026-08-08)。实施顺序按第 11 节执行,`python3 -m unittest discover -s src`(692 tests)+ `discover -s supervisor -p "test_*.py"`(126 tests)全绿。实现过程中用真实测试发现并修正了本设计原文的几处问题(设计文档本身未回填,实现是最终准确来源): | |
| 7 | +- §2 最初的 `on_fail.target_node_id` 字段设计,在实现前按用户反馈改成了从 `on_failure` 边读取(已反映在正文),但 §4.0(`topological_layers()` 必须过滤 `on_failure` 边)和 §8.3(`_validate_loop_back_targets` 改成校验边而不是字段)是**开始写代码之后**才发现遗漏并现场修的两处,不是设计阶段就想全的。 | |
| 8 | +- `_next_exec_node_id`(原设计称 `_count_prior_attempts`,按行数计数)在真实测试中暴露两个问题:① 按 DB 行数计数会把"resume 一个从未失败过的节点"误判成需要新开 attempt 后缀;② 只统计"自己是不是直接 loop_back 目标"会漏掉"作为下游节点被牵连重跑"的情况。最终实现改成按 `execution_event` 里真实的 loop_back 触发次数、且用 `_transitive_dependents` 判断是否被牵连来计数,两个问题都已修复并有专门测试覆盖。 | |
| 9 | +- `_run_one_node` 最初设计里"先统一调用 verify_fn 判断、再决定走哪条分支"的写法,会导致legacy(无 judgment 声明)节点的 verify_fn 被调用两次(`_verify_and_maybe_regenerate` 内部自己还会再调一次)——测试跑出来直接暴露,改成先判断 `has_own_judgment` 决定路径,只有新路径才提前调用 verify_fn。 | |
| 10 | +- `work_graph_plan.py::_PROMPT_TEMPLATE` 最初设计的"先 `.format()` 填 catalog_json、留 `{goal_text}` 给以后再填一次"写法有真实的转义 bug(catalog JSON 自带的花括号会被第二次 `.format()` 误当成占位符),实现时改成用字符串拼接嵌入 catalog、最终用 `.replace("{goal_text}", goal_text)` 而不是 `.format()` 填充目标文本,彻底绕开转义问题。 | |
| 11 | +- 顺带在真实测试中发现一个**不在本设计范围内、`work_node_run.py` 自己的既有缺口**:`run_work_node()` 每次调用都无条件往 `execution_log_file` 插入一行新记录,没有检查该 `node_exec_id` 是否已有记录——如果一个节点在 `execute_work_graph()` 真正返回之前就进程崩溃(不是"干净的 done",也不是全新节点),下次对同一个 `node_exec_id` 调用 `run_work_node()` 会撞 `execution_log_file.node_exec_id` 的 UNIQUE 约束报错。这个缺口在 loop_back 机制之前就存在(任何节点崩溃在这个时间窗口都会触发,不是 loop_back 特有的),只是这次因为测试真实构造了"进程崩溃后重启"的场景才被发现——不在本次范围内修复,记录在案供后续单独立项。 | |
| 12 | + | |
| 13 | +--- | |
| 14 | + | |
| 15 | +## 1. 设计概览 | |
| 16 | + | |
| 17 | +现状(`work_graph_orchestrate.py`,见其自身 docstring 与代码):`run_graph()` 把 `graph_manifest` 用 `topological_layers()` 分层,层间屏障、层内(G,2026-08-06)用 `ThreadPoolExecutor` 并发;每个节点的判断逻辑完全靠调用方传入的单一 `verify_fn(node, output) -> (ok, feedback)` 回调,失败处理固定死在 `_verify_and_maybe_regenerate()` 里——原地重新生成同一节点恰好一次,还失败就 `ConsistencyCheckFailedError`,没有"跳回更早节点"这条路径。 | |
| 18 | + | |
| 19 | +改造后新增两条能力,**都以 `graph_manifest` 节点自身携带的声明式字段驱动,不再要求调用方为每种 artifact_type 手写 Python 分支**: | |
| 20 | + | |
| 21 | +1. **`judgment`**:节点自己说"怎么判断我的输出对不对"(机械检查 or 触发一次额外 LLM 判断)。 | |
| 22 | +2. **`on_fail`**:节点自己说"判断失败了怎么办"(原地重试 / 跳回更早节点重新执行 / 直接判失败)。 | |
| 23 | + | |
| 24 | +**不改变的部分**(刻意保持稳定): | |
| 25 | +- 每个节点仍然是"渲染成一个单节点 workflow 脚本 + 独立 `claude -p` 子进程调用"(`_make_run_fn`/`_render_node_script` 不变)。 | |
| 26 | +- G(并行执行)的层内并发机制、`_LockedConn`、`WorkGraphLayerError` 完全不变——回路是"层与层之间的控制流",跟"层内节点并发执行"是正交的两层机制,互不冲突(详见第 4 节)。 | |
| 27 | +- 不声明 `judgment`/`on_fail` 的现有 `graph_manifest`(Coder 的合成单节点图)逐字节行为不变(FR-A8,第 8 节有完整核对表)。 | |
| 28 | +- `run_work_node()`/`work_node_run.py` 的断点续跑状态机完全不变,回路复用它现成的"新 node_id → 新行 → 真实重跑"机制(第 5 节),不需要改这一层。 | |
| 29 | + | |
| 30 | +## 2. `graph_manifest` 节点 schema 扩展 | |
| 31 | + | |
| 32 | +```jsonc | |
| 33 | +{ | |
| 34 | + "node_id": "benchmark", | |
| 35 | + "artifact_type": "benchmark", // 沿用现状;_GENERIC_PROMPT_TEMPLATE 路径用 task_prompt 代替,同样适用 | |
| 36 | + "depends_on": ["sdd"], | |
| 37 | + | |
| 38 | + // 新增,可选。缺省等价于 {"kind": "none"} —— 不做任何自动判断, | |
| 39 | + // 节点输出直接被接受(如果调用方仍然传了外部 verify_fn,外部的继续生效,见第 3 节)。 | |
| 40 | + "judgment": { | |
| 41 | + "kind": "mechanical", // "mechanical" | "llm" | "none" | |
| 42 | + "check_ref": "check_script_compiles" // 仅 kind="mechanical" 时必填,见第 3.1 节的白名单机制 | |
| 43 | + // kind="llm" 时改用: | |
| 44 | + // "judge_prompt": "Review the following content for ...: {content}" | |
| 45 | + // —— 由规划者(Planner 自己,或人工在固定链里手写)自己撰写, | |
| 46 | + // self-contained,不依赖调用方额外拼接上下文(同 _GENERIC_PROMPT_TEMPLATE | |
| 47 | + // 的 task_prompt 设计原则)。 | |
| 48 | + }, | |
| 49 | + | |
| 50 | + // 新增,可选。缺省等价于 {"action": "hard_fail"} 当没有 judgment 时; | |
| 51 | + // 当声明了 judgment 但没声明 on_fail 时,缺省等价于 {"action": "retry_self", "max_attempts": 1} | |
| 52 | + // ——即完全复刻现状 _verify_and_maybe_regenerate() 的行为(原地重试恰好一次)。 | |
| 53 | + "on_fail": { | |
| 54 | + "action": "loop_back", // "retry_self" | "loop_back" | "hard_fail" | |
| 55 | + "max_attempts": 3 // 可选,覆盖 FR-A5 配置表给出的默认值 | |
| 56 | + // 注意:没有 target_node_id 字段——action="loop_back" 的目标节点不是 | |
| 57 | + // node 自己声明的一个独立字段,而是从 graph_manifest 的 edges 里读出来 | |
| 58 | + // 的(见下)。retry_self/hard_fail 不需要任何 edge,target 就是自己/无。 | |
| 59 | + } | |
| 60 | +} | |
| 61 | +``` | |
| 62 | + | |
| 63 | +**2026-08-08 修订(用户反馈)**:失败路径的目标节点不应该是节点自己声明的、跟 `edges` 脱节的一个独立字段——"下一个节点是谁(成功路径)、失败了回到哪个节点(失败路径),都应该在规划这个节点的时候,用边(edge)定义好",跟成功路径的 `depends_on`/`edges` 是同一套结构,不是节点身上另开的一个影子字段。所以 `on_fail.loop_back` 的目标改成**从一条新的 `edge_type: "on_failure"` 边读出来**,不再单独存 `target_node_id`: | |
| 64 | + | |
| 65 | +```jsonc | |
| 66 | +// graph_manifest 顶层 edges 数组: | |
| 67 | +{"from": "sdd", "to": "benchmark", "edge_type": "needs_judgment"}, // 成功路径:sdd 产出喂给 benchmark(既有语义,depends_on 的另一种表达) | |
| 68 | +{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"} // 失败路径:benchmark 判断失败时回到 sdd 重新执行 | |
| 69 | +``` | |
| 70 | + | |
| 71 | +即:一个节点的"下一步去哪"完全由它参与的 `edges` 决定——`needs_judgment`/`zero_token` 两种既有类型继续表达"成功之后数据往哪流",新增的 `on_failure` 表达"判断失败之后控制流跳去哪"。节点自己的 `on_fail.action` 只声明"失败了要不要 loop_back"这个决策本身,不重复存目标是谁,避免两处数据不一致的风险(比如 `on_fail.target_node_id` 和实际 `edges` 没对上)。`GRAPH_MANIFEST_SCHEMA` 的 `edge_type` 枚举(`work_graph_plan.py:60`)加上 `"on_failure"`。 | |
| 72 | + | |
| 73 | +`GRAPH_MANIFEST_SCHEMA`(`work_graph_plan.py:33`)同步补上 `judgment`/`on_fail` 的形状 + `on_failure` edge_type(文档性质,跟现状一样不接入真正的 jsonschema 校验——`--json-schema` 和 `--agent` 不能同时用,见该文件已有说明)。 | |
| 74 | + | |
| 75 | +## 3. `run_graph()` 改造:judgment 解析 | |
| 76 | + | |
| 77 | +### 3.1 机械检查白名单——注入而不是硬编码 | |
| 78 | + | |
| 79 | +`work_graph_orchestrate.py` 现在对 `e2e_pipeline.py` **零依赖**(依赖方向是反过来的),不能直接 `import e2e_pipeline` 去拿 `check_script_compiles`(会成环)。机械检查改成调用方注入的白名单字典: | |
| 80 | + | |
| 81 | +```python | |
| 82 | +def run_graph( | |
| 83 | + conn, run_id, org_node, skill_id, target_folder_path, graph_manifest, | |
| 84 | + build_prompt, input_schema, output_schema, timeout=600, needs_worktree=False, | |
| 85 | + verify_fn=None, sleep=time.sleep, now=time.time, max_workers=_DEFAULT_MAX_WORKERS, | |
| 86 | + mechanical_check_registry=None, # 新增:{check_ref: callable(content) -> (ok, detail)} | |
| 87 | +): | |
| 88 | +``` | |
| 89 | + | |
| 90 | +`e2e_pipeline.py` 的调用点传 `mechanical_check_registry={"check_script_compiles": check_script_compiles}`。`judgment.kind="mechanical"` 但 `check_ref` 不在注册表里,视为配置错误,直接在 `run_graph()` 一开始(还没跑任何节点前)校验并抛 `ValueError`——不要等跑到那个节点才发现拼错了名字,浪费一整条链的 token。**LLM 只能从白名单里选,不能发明新的机械检查**——这是有意的限制,不做"LLM 生成检查代码后拿去执行"这种沙箱执行的大工程。 | |
| 91 | + | |
| 92 | +### 3.2 LLM 判断——复用 `_check_prd_consistency` 的隔离机制,泛化提示词 | |
| 93 | + | |
| 94 | +```python | |
| 95 | +def _llm_judgment_verify_fn(judgment, org_node, target_folder_path, timeout=180): | |
| 96 | + """通用化的 _check_prd_consistency(e2e_pipeline.py):同样的 self-target | |
| 97 | + worktree 隔离 + 独立 agent() 调用机制,但判断提示词来自 judgment['judge_prompt'] | |
| 98 | + (节点自己声明的,不是硬编码的 PRD 专属提示词),返回 schema 也通用化成 | |
| 99 | + {"ok": bool, "issues": [str, ...]}(PRD 检查原来的 "consistent"/"issues" 改名 | |
| 100 | + 成更通用的 "ok",语义不变)。""" | |
| 101 | + is_self = paths.is_self_target(target_folder_path) | |
| 102 | + | |
| 103 | + def verify_fn(node, output): | |
| 104 | + node_exec_id = "judgment-" + uuid.uuid4().hex[:12] | |
| 105 | + worktree_path = work_node_run.create_worktree(target_folder_path, node_exec_id) if is_self else None | |
| 106 | + cwd = worktree_path or target_folder_path | |
| 107 | + try: | |
| 108 | + prompt = judgment["judge_prompt"].format(content=output["content"]) | |
| 109 | + script_name = "graphlemon-judgment-" + uuid.uuid4().hex | |
| 110 | + script_content = ( | |
| 111 | + "export const meta = {{ name: {name}, description: 'graphlemon node judgment' }}\n\n" | |
| 112 | + "const result = await agent({prompt}, {{ agentType: {org_node_json}, schema: {schema} }})\n\n" | |
| 113 | + "return result\n" | |
| 114 | + ).format( | |
| 115 | + name=json.dumps(script_name), prompt=json.dumps(prompt), | |
| 116 | + org_node_json=json.dumps(org_node), schema=json.dumps(_JUDGMENT_SCHEMA), | |
| 117 | + ) | |
| 118 | + work_graph_execute.write_workflow_script(cwd, script_name, script_content) | |
| 119 | + result = work_graph_execute.execute_work_graph(cwd, script_name, {}, _JUDGMENT_SCHEMA, timeout=timeout) | |
| 120 | + if worktree_path: | |
| 121 | + work_node_run.ensure_worktree_committed(worktree_path, node_exec_id) | |
| 122 | + work_node_run.ensure_worktree_committed(target_folder_path, node_exec_id + "-main-checkout-drift") | |
| 123 | + work_node_run.merge_worktree(target_folder_path, node_exec_id) | |
| 124 | + ok = _coerce_bool(result["ok"]) | |
| 125 | + return ok, (result.get("issues") or []) | |
| 126 | + finally: | |
| 127 | + if worktree_path: | |
| 128 | + work_node_run.remove_worktree(target_folder_path, worktree_path) | |
| 129 | + | |
| 130 | + return verify_fn | |
| 131 | + | |
| 132 | + | |
| 133 | +_JUDGMENT_SCHEMA = { | |
| 134 | + "type": "object", "required": ["ok", "issues"], | |
| 135 | + "properties": {"ok": {"type": "boolean"}, "issues": {"type": "array", "items": {"type": "string"}}}, | |
| 136 | +} | |
| 137 | +``` | |
| 138 | + | |
| 139 | +`_coerce_bool`(原属 `e2e_pipeline.py`,见其 2026-08-02 的"LLM 把 boolean 回成字符串"教训)需要下沉成 `work_graph_orchestrate.py` 自己的小工具函数(或 `paths.py` 这类共享工具模块),两边都用同一份,不重复实现。`e2e_pipeline.py::_check_prd_consistency`/`_prd_consistency_verify_fn` 保留不动(FR-A8 向后兼容,现有固定链路径不强制迁移),新代码只是提供一个可选的、更通用的平行实现。 | |
| 140 | + | |
| 141 | +### 3.3 节点级 verify_fn 解析 | |
| 142 | + | |
| 143 | +```python | |
| 144 | +def _resolve_node_verify_fn(node, external_verify_fn, org_node, target_folder_path, mechanical_check_registry): | |
| 145 | + judgment = node.get("judgment") | |
| 146 | + if not judgment or judgment.get("kind", "none") == "none": | |
| 147 | + return external_verify_fn # 完全现状:没声明 judgment,谁传了外部 verify_fn 就用谁的(可能是 None) | |
| 148 | + if judgment["kind"] == "mechanical": | |
| 149 | + check_fn = (mechanical_check_registry or {}).get(judgment["check_ref"]) | |
| 150 | + if check_fn is None: | |
| 151 | + raise ValueError("unknown mechanical check_ref: {!r}".format(judgment["check_ref"])) | |
| 152 | + def verify_fn(node, output, _check_fn=check_fn): | |
| 153 | + ok, detail = _check_fn(output["content"]) | |
| 154 | + return ok, ([detail] if detail else None) | |
| 155 | + return verify_fn | |
| 156 | + if judgment["kind"] == "llm": | |
| 157 | + return _llm_judgment_verify_fn(judgment, org_node, target_folder_path) | |
| 158 | + raise ValueError("unknown judgment.kind: {!r}".format(judgment["kind"])) | |
| 159 | +``` | |
| 160 | + | |
| 161 | +`_run_one_node()` 改成对每个节点各自调用 `_resolve_node_verify_fn(...)` 拿到"这个节点该用哪个 verify_fn",而不是像现在这样整张图共用调用方传入的同一个 `verify_fn`。**向后兼容关键点**:现有调用方(`plan_documents`/`validate_implementation_with_own_graph`)继续传它们现在的 `_combined_planner_verify_fn(...)`/`_test_script_verify_fn(...)` 作为 `external_verify_fn`,只要节点没有 `judgment` 字段(现状 100% 如此),`_resolve_node_verify_fn` 原样返回这个外部函数,行为逐字节不变(FR-A8 验收标准 5)。 | |
| 162 | + | |
| 163 | +## 4. `on_fail` 执行:回路引擎 | |
| 164 | + | |
| 165 | +### 4.0 前提修复:`topological_layers()` 必须过滤 `on_failure` 边(FR-A7) | |
| 166 | + | |
| 167 | +**这是把 loop_back 目标从节点字段改成 `edges` 里的一条边(第 2 节 2026-08-08 修订)之后,必须同步修的一处,否则会引入真环**:现有 `topological_layers()`(`work_graph_orchestrate.py:196-200`)把 `graph_manifest["edges"]` 里**每一条边、不分 `edge_type`**都当成数据依赖合并进 `deps`: | |
| 168 | + | |
| 169 | +```python | |
| 170 | +for edge in graph_manifest.get("edges", []): | |
| 171 | + to_id, from_id = edge.get("to"), edge.get("from") | |
| 172 | + if to_id in deps and from_id in nodes: | |
| 173 | + deps[to_id].add(from_id) | |
| 174 | +``` | |
| 175 | + | |
| 176 | +如果不改,一条 `{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}` 会被这段代码当成"`sdd` 依赖 `benchmark`"塞进 `deps["sdd"]`——而 `benchmark` 本来就依赖 `sdd`(正常数据依赖),两条方向相反的依赖凑在一起就是一个真环,`topological_layers()` 会在图规划阶段还没触发任何回路之前,就先被静态校验直接拒绝,FR-A7 的验收标准 4 会失败。 | |
| 177 | + | |
| 178 | +修复:合并 `deps` 时跳过 `edge_type == "on_failure"` 的边——**只有 `on_failure` 需要特殊处理**,`zero_token`/`needs_judgment` 两种既有类型继续按现状合并(它们表达的就是真实数据依赖): | |
| 179 | + | |
| 180 | +```python | |
| 181 | +def topological_layers(graph_manifest): | |
| 182 | + nodes = {n["node_id"]: n for n in graph_manifest["nodes"]} | |
| 183 | + deps = {node_id: set(n.get("depends_on") or []) for node_id, n in nodes.items()} | |
| 184 | + for edge in graph_manifest.get("edges", []): | |
| 185 | + if edge.get("edge_type") == "on_failure": | |
| 186 | + continue # 新增:控制流边不参与静态拓扑排序 | |
| 187 | + to_id, from_id = edge.get("to"), edge.get("from") | |
| 188 | + if to_id in deps and from_id in nodes: | |
| 189 | + deps[to_id].add(from_id) | |
| 190 | + ... # 其余逻辑不变 | |
| 191 | +``` | |
| 192 | + | |
| 193 | +`topological_order()`(薄封装)不用改。这条修复是本设计里**唯一一处直接改动 G(并行执行)已经上线的现有函数**的地方,改动是纯粹的"多一个 `continue` 条件",不影响任何现有(不含 `on_failure` 边的)`graph_manifest` 的分层结果——`src/test_work_graph_orchestrate.py` 里 G 自己的既有测试用例不需要改一行断言即可继续通过。 | |
| 194 | + | |
| 195 | +### 4.1 关键简化:`retry_self` 是 `loop_back` 的特例 | |
| 196 | + | |
| 197 | +`retry_self` ≡ `loop_back` 且 `target_node_id` = 自己。原因:`retry_self` 触发时这个节点本身还没成功过,不可能已经有下游节点跑完并读取过它的(不存在的)输出,所以"重跑 target 到失败节点之间的下游链"这条规则应用到"target=自己"时,受影响集合就是 `{自己}`,跟现状的"原地重跑"完全等价。**一套机制两种声明方式**,不用维护两条独立代码路径。 | |
| 198 | + | |
| 199 | +### 4.2 受影响子图:只重跑真正的下游依赖,不是整层 | |
| 200 | + | |
| 201 | +需求文档非目标已声明"不追求完全通用的图执行框架",但"回路触发时把同层不相关的兄弟节点也无谓重跑一遍"没必要,也和已完成节点白白重花 token 相悖,成本可控且实现不复杂: | |
| 202 | + | |
| 203 | +```python | |
| 204 | +def _transitive_dependents(graph_manifest, node_id): | |
| 205 | + """node_id 自身 + 所有直接/间接依赖它的下游节点 id 集合(沿 depends_on/edges | |
| 206 | + 正向遍历)。用于 loop_back 时判断哪些节点的输出已经"过期"、必须重跑。""" | |
| 207 | + dependents = {} # node_id -> [直接依赖它的节点 id, ...] | |
| 208 | + for node in graph_manifest["nodes"]: | |
| 209 | + dependents.setdefault(node["node_id"], []) | |
| 210 | + deps = {n["node_id"]: set(n.get("depends_on") or []) for n in graph_manifest["nodes"]} | |
| 211 | + for edge in graph_manifest.get("edges", []): | |
| 212 | + to_id, from_id = edge.get("to"), edge.get("from") | |
| 213 | + if to_id in deps and from_id in deps: | |
| 214 | + deps[to_id].add(from_id) | |
| 215 | + for node_id_, dep_set in deps.items(): | |
| 216 | + for dep in dep_set: | |
| 217 | + dependents.setdefault(dep, []).append(node_id_) | |
| 218 | + | |
| 219 | + affected, stack = set(), [node_id] | |
| 220 | + while stack: | |
| 221 | + current = stack.pop() | |
| 222 | + if current in affected: | |
| 223 | + continue | |
| 224 | + affected.add(current) | |
| 225 | + stack.extend(dependents.get(current, [])) | |
| 226 | + return affected | |
| 227 | + | |
| 228 | + | |
| 229 | +def _on_failure_target(graph_manifest, node_id): | |
| 230 | + """节点 node_id 声明了 on_fail.action='loop_back' 时,目标节点不是节点 | |
| 231 | + 自己存的字段,而是 graph_manifest['edges'] 里唯一一条 | |
| 232 | + {"from": node_id, "to": ..., "edge_type": "on_failure"} 边的 "to"—— | |
| 233 | + 2026-08-08 修订:失败路径跟成功路径一样,都是"边",不是节点身上另开 | |
| 234 | + 的一个跟 edges 脱节的影子字段。恰好 0 条或 2+ 条这样的边是规划错误, | |
| 235 | + 在 _validate_loop_back_targets(8.3 节)里提前拦截,这里假设已经过 | |
| 236 | + 校验,直接找第一条匹配的。""" | |
| 237 | + for edge in graph_manifest.get("edges", []): | |
| 238 | + if edge.get("from") == node_id and edge.get("edge_type") == "on_failure": | |
| 239 | + return edge["to"] | |
| 240 | + raise ValueError("node {} declares on_fail.action='loop_back' but has no on_failure edge".format(node_id)) | |
| 241 | +``` | |
| 242 | + | |
| 243 | +### 4.3 层级回退循环——`run_graph()` 主循环改造 | |
| 244 | + | |
| 245 | +```python | |
| 246 | +class _LoopBackRequested(Exception): | |
| 247 | + """节点内部信号:这个节点的 on_fail.action=loop_back 触发了,需要外层 | |
| 248 | + run_graph() 主循环回退重跑,不是真正的失败——不通过正常返回值传递是 | |
| 249 | + 因为它可能发生在 ThreadPoolExecutor 的 worker 线程里(层内并发场景), | |
| 250 | + 异常是唯一能干净地从 future.result() 里带出这个信号的方式。""" | |
| 251 | + def __init__(self, source_node_id, target_node_id, feedback): | |
| 252 | + self.source_node_id = source_node_id | |
| 253 | + self.target_node_id = target_node_id | |
| 254 | + self.feedback = feedback | |
| 255 | + super().__init__("node {} requests loop_back to {}".format(source_node_id, target_node_id)) | |
| 256 | + | |
| 257 | + | |
| 258 | +def run_graph(..., mechanical_check_registry=None): | |
| 259 | + ensure_placeholder_skill(conn, skill_id, org_node) | |
| 260 | + effective_needs_worktree = needs_worktree or paths.is_self_target(target_folder_path) | |
| 261 | + layers = topological_layers(graph_manifest) | |
| 262 | + node_layer_index = {node["node_id"]: i for i, layer in enumerate(layers) for node in layer} | |
| 263 | + outputs = {} | |
| 264 | + locked_conn = _LockedConn(conn) | |
| 265 | + attempt_counts = {} # (target_node_id, source_node_id) -> 已尝试次数,本次 run_graph() 调用内的内存计数 | |
| 266 | + # 断点续跑时从 execution_event 重建,见第 6.3 节 | |
| 267 | + | |
| 268 | + layer_index = 0 | |
| 269 | + while layer_index < len(layers): | |
| 270 | + layer = layers[layer_index] | |
| 271 | + try: | |
| 272 | + _run_layer( | |
| 273 | + conn, locked_conn, run_id, org_node, skill_id, target_folder_path, layer, outputs, | |
| 274 | + build_prompt, input_schema, output_schema, timeout, effective_needs_worktree, | |
| 275 | + verify_fn, mechanical_check_registry, max_workers, sleep, now, | |
| 276 | + ) | |
| 277 | + except _LoopBackRequested as sig: | |
| 278 | + key = (sig.target_node_id, sig.source_node_id) | |
| 279 | + attempt_counts[key] = attempt_counts.get(key, 0) + _loop_attempt_count_from_events(conn, run_id, key) | |
| 280 | + limit = _resolve_loop_limit(conn, org_node, graph_manifest, sig.target_node_id) | |
| 281 | + if attempt_counts[key] > limit: | |
| 282 | + conn.execute("UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ?", (run_id,)) | |
| 283 | + conn.commit() | |
| 284 | + raise ConsistencyCheckFailedError( | |
| 285 | + "loop_back from {} to {} exceeded max_attempts={}".format( | |
| 286 | + sig.source_node_id, sig.target_node_id, limit | |
| 287 | + ), | |
| 288 | + node_id=sig.source_node_id, | |
| 289 | + ) | |
| 290 | + _record_loop_back_event(conn, run_id, sig, attempt_counts[key]) | |
| 291 | + affected = _transitive_dependents(graph_manifest, sig.target_node_id) | |
| 292 | + for affected_id in affected: | |
| 293 | + outputs.pop(affected_id, None) # 强制这些节点在重跑时被当成"未完成" | |
| 294 | + layer_index = node_layer_index[sig.target_node_id] | |
| 295 | + continue | |
| 296 | + layer_index += 1 | |
| 297 | + | |
| 298 | + return outputs | |
| 299 | +``` | |
| 300 | + | |
| 301 | +`_run_layer(...)` 是现有 `run_graph()` 主循环体(单节点同步 / 多节点 `ThreadPoolExecutor`)原样搬过来的辅助函数,唯一的改动是:**只重跑本轮 `affected` 集合里的节点**(第一次进入某层时 `affected` 概念不适用,正常跑全部;loop_back 回退重入某层时,只有 `outputs` 里被 `.pop()` 掉的节点需要重新执行,其余节点直接复用已有的 `outputs[node_id]`,不重新调用 `_run_one_node`)——`outputs.pop()` 天然标记了"这个需要重跑",`_run_layer` 判断 `node["node_id"] not in outputs` 决定要不要真的执行这个节点,跳过的节点保留原值。 | |
| 302 | + | |
| 303 | +`_run_one_node()` 里判断 `verify_fn` 失败后该走哪条路径(原地重试 vs 抛 `_LoopBackRequested`): | |
| 304 | + | |
| 305 | +```python | |
| 306 | +def _run_one_node(..., node, outputs, ...): | |
| 307 | + node_id = node["node_id"] | |
| 308 | + # 每次真正执行都用一个新的 attempt 后缀 node_id,绝不复用同一个已经 | |
| 309 | + # 'done' 的 node_id——_ensure_node_row 按 (run_id, node_id) 去重,复用 | |
| 310 | + # 同一个 node_id 会被 run_work_node 的断点续跑短路成"已完成,直接返回 | |
| 311 | + # 旧输出",这正是必须避免的(详见 5 节)。 | |
| 312 | + attempt = _count_prior_attempts(conn, run_id, node_id) + 1 | |
| 313 | + exec_node_id = node_id if attempt == 1 else "{}-attempt{}".format(node_id, attempt) | |
| 314 | + | |
| 315 | + node_exec_id = _ensure_node_row(conn, run_id, exec_node_id, org_node, skill_id, ..., {"node_id": node_id}) | |
| 316 | + prompt = build_prompt(node, outputs) + _loop_history_prompt_suffix(conn, run_id, node_id) | |
| 317 | + run_fn = _make_run_fn(conn, target_folder_path, node_exec_id, org_node, prompt, output_schema, timeout) | |
| 318 | + output = work_node_run.run_work_node(conn, node_exec_id, {"node_id": node_id}, input_schema, output_schema, run_fn, ...) | |
| 319 | + | |
| 320 | + resolved_verify_fn = _resolve_node_verify_fn(node, verify_fn, org_node, target_folder_path, mechanical_check_registry) | |
| 321 | + if resolved_verify_fn is None: | |
| 322 | + return output | |
| 323 | + ok, feedback = resolved_verify_fn(node, output) | |
| 324 | + if ok: | |
| 325 | + return output | |
| 326 | + | |
| 327 | + on_fail = node.get("on_fail") or {"action": "retry_self" if node.get("judgment") else "hard_fail"} | |
| 328 | + action = on_fail.get("action", "retry_self") | |
| 329 | + if action == "hard_fail" or (action == "retry_self" and not node.get("judgment")): | |
| 330 | + # 完全现状路径:没声明 judgment 的老节点,外部 verify_fn 失败时走 | |
| 331 | + # 原有的 _verify_and_maybe_regenerate()(恰好一次原地重试),逐字节不变。 | |
| 332 | + return _verify_and_maybe_regenerate(conn, run_id, org_node, skill_id, target_folder_path, | |
| 333 | + node, node_id, prompt, output, input_schema, output_schema, | |
| 334 | + timeout, needs_worktree, resolved_verify_fn, sleep, now) | |
| 335 | + target = node_id if action == "retry_self" else _on_failure_target(graph_manifest, node_id) | |
| 336 | + raise _LoopBackRequested(source_node_id=node_id, target_node_id=target, feedback=feedback) | |
| 337 | +``` | |
| 338 | + | |
| 339 | +(`_run_one_node` 需要拿到 `graph_manifest` 才能查 `on_failure` 边——现有签名没有这个参数,改造时顺带传进去;`retry_self` 完全不查边,`target_node_id` 直接等于自己,第 4.1 节已经说明原因。) | |
| 340 | + | |
| 341 | +**关键取舍**:`hard_fail`、以及"没声明 `judgment` 的老节点走 `retry_self`"这两种情况,继续复用现有的 `_verify_and_maybe_regenerate()` 一字不改——不是所有失败路径都要改造成新机制,只有节点**主动声明了 `judgment`**(即 Part B 自规划出的新节点,或 Part A 上线后手工升级的固定链节点)才会走新的 `_LoopBackRequested` 回路路径。这保证 FR-A8 的向后兼容不是"理论上兼容"而是"代码路径都没变过"。 | |
| 342 | + | |
| 343 | +## 5. 断点续跑兼容性:新 attempt 后缀就是全部答案 | |
| 344 | + | |
| 345 | +不需要专门为回路设计新的续跑逻辑。原理:`_run_one_node()` 每次真正执行前,都先查询"这个 node_id 已经有几次真正的执行尝试"(`_count_prior_attempts`,扫 `work_node_execution` 里 `node_id` 匹配 `original_id` 或 `original_id-attemptN` 模式的行数),用下一个序号构造 `exec_node_id`。进程崩溃重启后重新调用 `run_graph()`: | |
| 346 | + | |
| 347 | +- 还没跑到的层:`outputs` 里没有对应 key,`_run_layer` 判断需要执行,正常走一遍——`_count_prior_attempts` 会看到之前留下的部分尝试记录(如果崩溃发生在某次 attempt 执行中途),用下一个序号继续,不会跟已存在的行冲突(`_ensure_node_row` 按 `exec_node_id` 去重,`exec_node_id` 本身按 attempt 序号递增,天然不冲突)。 | |
| 348 | +- 已经 `done` 的 attempt:`run_work_node()` 现有的"已完成短路"逻辑对每个 `exec_node_id` 各自生效,不需要改。 | |
| 349 | +- `attempt_counts`(内存字典,第 4.3 节):崩溃后是空的,但 `run_graph()` 一旦真的触发 `_LoopBackRequested` 时会先调用 `_loop_attempt_count_from_events(conn, run_id, key)`(第 6.3 节,从 `execution_event` 里查询真实发生过的次数),不依赖内存状态——重启后第一次遇到某个 loop_back key 时,从 DB 查出来的次数就是崩溃前的真实累计值,不会被内存清零重置为 0。 | |
| 350 | + | |
| 351 | +## 6. 循环上限配置(FR-A5) | |
| 352 | + | |
| 353 | +### 6.1 新表(`settings_config.py` 风格) | |
| 354 | + | |
| 355 | +```sql | |
| 356 | +-- schema.py 新增 | |
| 357 | +CREATE TABLE work_graph_loop_limit_config ( | |
| 358 | + config_id TEXT PRIMARY KEY, -- "{org_node}:{node_type}" / "{org_node}:*" / "default" | |
| 359 | + max_loop_attempts INTEGER NOT NULL, | |
| 360 | + updated_by TEXT, | |
| 361 | + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP | |
| 362 | +); | |
| 363 | +``` | |
| 364 | + | |
| 365 | +```python | |
| 366 | +# settings_config.py 新增,跟 get_/upsert_concurrency_config 完全同构 | |
| 367 | +def get_loop_limit_config(conn, config_id): | |
| 368 | + row = conn.execute( | |
| 369 | + "SELECT config_id, max_loop_attempts, updated_at FROM work_graph_loop_limit_config WHERE config_id = ?", | |
| 370 | + (config_id,), | |
| 371 | + ).fetchone() | |
| 372 | + return None if row is None else {"config_id": row[0], "max_loop_attempts": row[1], "updated_at": row[2]} | |
| 373 | + | |
| 374 | + | |
| 375 | +def upsert_loop_limit_config(conn, config_id, max_loop_attempts, updated_by): | |
| 376 | + if max_loop_attempts < 1: | |
| 377 | + raise InvalidParamError("max_loop_attempts must be >= 1, got {!r}".format(max_loop_attempts)) | |
| 378 | + exists = conn.execute( | |
| 379 | + "SELECT 1 FROM work_graph_loop_limit_config WHERE config_id = ?", (config_id,) | |
| 380 | + ).fetchone() is not None | |
| 381 | + if exists: | |
| 382 | + conn.execute( | |
| 383 | + "UPDATE work_graph_loop_limit_config SET max_loop_attempts = ?, updated_by = ?, " | |
| 384 | + "updated_at = CURRENT_TIMESTAMP WHERE config_id = ?", | |
| 385 | + (max_loop_attempts, updated_by, config_id), | |
| 386 | + ) | |
| 387 | + else: | |
| 388 | + conn.execute( | |
| 389 | + "INSERT INTO work_graph_loop_limit_config (config_id, max_loop_attempts, updated_by) VALUES (?, ?, ?)", | |
| 390 | + (config_id, max_loop_attempts, updated_by), | |
| 391 | + ) | |
| 392 | + conn.commit() | |
| 393 | + return get_loop_limit_config(conn, config_id) | |
| 394 | +``` | |
| 395 | + | |
| 396 | +### 6.2 查找顺序 + 硬编码兜底 | |
| 397 | + | |
| 398 | +```python | |
| 399 | +# work_graph_orchestrate.py | |
| 400 | +_DEFAULT_LOOP_LIMIT = 3 # 只在配置表连 "default" 行都没有时才会用到(比如全新数据库还没跑过 seed) | |
| 401 | + | |
| 402 | +def _resolve_loop_limit(conn, org_node, graph_manifest, target_node_id): | |
| 403 | + node = next(n for n in graph_manifest["nodes"] if n["node_id"] == target_node_id) | |
| 404 | + node_level = (node.get("on_fail") or {}).get("max_attempts") | |
| 405 | + if node_level is not None: | |
| 406 | + return node_level # 节点自己声明的优先级最高(FR-A5) | |
| 407 | + node_type = node.get("artifact_type", target_node_id) | |
| 408 | + for config_id in ("{}:{}".format(org_node, node_type), "{}:*".format(org_node), "default"): | |
| 409 | + config = settings_config.get_loop_limit_config(conn, config_id) | |
| 410 | + if config: | |
| 411 | + return config["max_loop_attempts"] | |
| 412 | + return _DEFAULT_LOOP_LIMIT | |
| 413 | +``` | |
| 414 | + | |
| 415 | +初始 seed:`schema.create_schema()`/迁移里插入一条 `config_id="default", max_loop_attempts=3` 的行(跟 `org_loop_route._LOOP_LIMIT=3` 取值一致,作为一个"暂时先抄现有跨 Org loop 的经验值,后续按真实调用数据调整"的起点——呼应需求文档「7. 风险」第二条,这不是拍脑袋,是有意选择一个已经在生产验证过的数量级作为起点,且明确设计成之后可以不改代码调整)。 | |
| 416 | + | |
| 417 | +## 7. loop 历史持久化(FR-A4):`execution_event` 迁移 | |
| 418 | + | |
| 419 | +### 7.1 新增 `event_type`:`loop_back_triggered` | |
| 420 | + | |
| 421 | +`execution_event.event_type` 是 CHECK 约束枚举(`schema.py:262-266`),SQLite 无法直接 `ALTER`,沿用 `migrate_expand_artifact_types` 的 rebuild-copy-swap 手法: | |
| 422 | + | |
| 423 | +```python | |
| 424 | +# schema.py 新增 | |
| 425 | +def migrate_add_loop_back_event_type(conn): | |
| 426 | + """2026-08-08: 给 execution_event.event_type 的 CHECK 枚举加 | |
| 427 | + 'loop_back_triggered',用于记录 Work Graph 内部回路——见 | |
| 428 | + docs/todo/work-graph-self-planning-and-loop-back-design.md §7。 | |
| 429 | + 幂等,安全对已有数据库无条件调用(同 migrate_expand_artifact_types 的写法)。 | |
| 430 | + """ | |
| 431 | + row = conn.execute("SELECT sql FROM sqlite_master WHERE type='table' AND name='execution_event'").fetchone() | |
| 432 | + if row is None or "'loop_back_triggered'" in row[0]: | |
| 433 | + return | |
| 434 | + conn.executescript( | |
| 435 | + """ | |
| 436 | + CREATE TABLE execution_event_new ( | |
| 437 | + event_id TEXT PRIMARY KEY, | |
| 438 | + project_id TEXT NOT NULL REFERENCES project(project_id), | |
| 439 | + task_id TEXT REFERENCES task(task_id), | |
| 440 | + run_id TEXT REFERENCES work_graph_run(run_id), | |
| 441 | + node_exec_id TEXT REFERENCES work_node_execution(node_exec_id), | |
| 442 | + event_type TEXT NOT NULL CHECK (event_type IN ( | |
| 443 | + 'task_status_changed', 'node_status_changed', 'artifact_created', | |
| 444 | + 'bug_recorded', 'loop_routed', 'budget_updated', 'skill_iterated', | |
| 445 | + 'log_appended', 'quota_wait_started', 'loop_back_triggered' | |
| 446 | + )), | |
| 447 | + payload TEXT NOT NULL, | |
| 448 | + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP | |
| 449 | + ); | |
| 450 | + INSERT INTO execution_event_new SELECT * FROM execution_event; | |
| 451 | + DROP TABLE execution_event; | |
| 452 | + ALTER TABLE execution_event_new RENAME TO execution_event; | |
| 453 | + CREATE INDEX idx_execution_event_project_created ON execution_event(project_id, created_at); | |
| 454 | + CREATE INDEX idx_execution_event_task_created ON execution_event(task_id, created_at); | |
| 455 | + """ | |
| 456 | + ) | |
| 457 | + conn.commit() | |
| 458 | +``` | |
| 459 | + | |
| 460 | +`_ensure_schema()`(`backend_server.py:1020`)加一行无条件调用,跟 `migrate_expand_artifact_types`/`migrate_relax_org_node_constraints` 并列——这正是 2026-08-06 那次"忘记接进 `_ensure_schema`导致生产 500"的教训要求的(该函数自己的注释已经把这条约定写死)。 | |
| 461 | + | |
| 462 | +### 7.2 写入 + 查询 | |
| 463 | + | |
| 464 | +```python | |
| 465 | +def _record_loop_back_event(conn, run_id, sig, attempt_number): | |
| 466 | + task_id, project_id = conn.execute( | |
| 467 | + "SELECT task_id, project_id FROM work_graph_run wgr JOIN task t ON wgr.task_id = t.task_id " | |
| 468 | + "WHERE wgr.run_id = ?", (run_id,) | |
| 469 | + ).fetchone() | |
| 470 | + conn.execute( | |
| 471 | + "INSERT INTO execution_event (event_id, project_id, task_id, run_id, event_type, payload) " | |
| 472 | + "VALUES (?, ?, ?, ?, 'loop_back_triggered', ?)", | |
| 473 | + (str(uuid.uuid4()), project_id, task_id, run_id, json.dumps({ | |
| 474 | + "source_node_id": sig.source_node_id, "target_node_id": sig.target_node_id, | |
| 475 | + "attempt_number": attempt_number, "feedback": sig.feedback, | |
| 476 | + })), | |
| 477 | + ) | |
| 478 | + conn.commit() | |
| 479 | + | |
| 480 | + | |
| 481 | +def _loop_attempt_count_from_events(conn, run_id, key): | |
| 482 | + target_node_id, source_node_id = key | |
| 483 | + rows = conn.execute( | |
| 484 | + "SELECT payload FROM execution_event WHERE run_id = ? AND event_type = 'loop_back_triggered'", (run_id,) | |
| 485 | + ).fetchall() | |
| 486 | + return sum( | |
| 487 | + 1 for (payload,) in rows | |
| 488 | + if json.loads(payload)["target_node_id"] == target_node_id | |
| 489 | + and json.loads(payload)["source_node_id"] == source_node_id | |
| 490 | + ) | |
| 491 | + | |
| 492 | + | |
| 493 | +def _loop_history_prompt_suffix(conn, run_id, node_id): | |
| 494 | + """target_node_id 重新执行时,把它过去每一轮被打回的完整历史(不只 | |
| 495 | + 最近一轮)拼进提示词——FR-A4。""" | |
| 496 | + rows = conn.execute( | |
| 497 | + "SELECT payload FROM execution_event WHERE run_id = ? AND event_type = 'loop_back_triggered' " | |
| 498 | + "ORDER BY created_at", (run_id,) | |
| 499 | + ).fetchall() | |
| 500 | + history = [json.loads(p) for (p,) in rows if json.loads(p)["target_node_id"] == node_id] | |
| 501 | + if not history: | |
| 502 | + return "" | |
| 503 | + lines = [ | |
| 504 | + "Attempt {}: rejected because: {}".format(h["attempt_number"], "; ".join(h["feedback"] or [])) | |
| 505 | + for h in history | |
| 506 | + ] | |
| 507 | + return "\n\nThis node was sent back for revision before, with this history:\n" + "\n".join(lines) | |
| 508 | +``` | |
| 509 | + | |
| 510 | +不新增表,`execution_event` 一张表回答"这个节点被打回几次、每次谁触发的、为什么、按什么顺序"——满足决策 #4,且天然带 `run_id`/`node_exec_id`/`created_at`,现有图可视化机制(`v_graph_visualizer`,若按 `run_id` 拉取 `execution_event` 渲染)不需要新的数据源就能把 `source_node_id → target_node_id` 画成一条回路边。 | |
| 511 | + | |
| 512 | +## 8. Part B:Planner 自规划节点集合 | |
| 513 | + | |
| 514 | +### 8.1 节点类型清单——数据,不是硬编码提示词字符串(FR-B2) | |
| 515 | + | |
| 516 | +```python | |
| 517 | +# work_graph_plan.py 新增 | |
| 518 | +_PLANNER_NODE_CATALOG = [ | |
| 519 | + {"artifact_type": "brd", "description": "商业需求文档...", "judgment": {"kind": "none"}}, | |
| 520 | + {"artifact_type": "prd", "description": "产品需求文档...", | |
| 521 | + "judgment": {"kind": "llm", "judge_prompt": _PRD_CONSISTENCY_JUDGE_PROMPT}}, | |
| 522 | + {"artifact_type": "sdd", "description": "详细设计文档...", "judgment": {"kind": "none"}}, | |
| 523 | + {"artifact_type": "benchmark", "description": "验收基准...", "judgment": {"kind": "none"}}, | |
| 524 | + {"artifact_type": "test_dataset", "description": "测试数据集(可选,goal 不需要时回复 N/A)...", | |
| 525 | + "judgment": {"kind": "none"}}, | |
| 526 | + {"artifact_type": "test_script", "description": "可执行验收脚本(可选,goal 不需要时回复 N/A)...", | |
| 527 | + "judgment": {"kind": "mechanical", "check_ref": "check_script_compiles"}}, | |
| 528 | +] | |
| 529 | +``` | |
| 530 | + | |
| 531 | +`_PRD_CONSISTENCY_JUDGE_PROMPT` 是现有 `_PRD_CONSISTENCY_PROMPT`(`e2e_pipeline.py:72`)改写成 `judge_prompt` 的通用形状(`{content}` 占位符替代原来的 `{prd_content}`),供 Planner 在自规划时"抄"进它给 prd 节点声明的 `judgment` 里——这份清单只是给 LLM 的**参考建议**,不是强制:Planner 完全可以给某个节点声明跟清单建议不同的 `judgment`/`on_fail`(FR-B4 已确认"全部自由规划"),清单只是降低它每次都要从零发明判断方式的成本。 | |
| 532 | + | |
| 533 | +### 8.2 新 `_PROMPT_TEMPLATE` | |
| 534 | + | |
| 535 | +```python | |
| 536 | +_PROMPT_TEMPLATE = ( | |
| 537 | + "Plan (do not execute) the Work Graph for this goal: {goal_text}\n" | |
| 538 | + "Available node types (use any subset, in any order that makes sense -- " | |
| 539 | + "this goal may not need all of them):\n{catalog_json}\n" | |
| 540 | + "For each node you include, decide: node_id, artifact_type, depends_on, " | |
| 541 | + "a judgment config (how to tell if this node's output is acceptable -- you " | |
| 542 | + "may reuse a catalog entry's suggested judgment, or write your own 'llm' " | |
| 543 | + "judge_prompt), and an on_fail config (what happens if judgment fails: " | |
| 544 | + "retry_self, loop_back, or hard_fail). If a node's on_fail.action is " | |
| 545 | + "'loop_back', you MUST also add a corresponding edge in the top-level " | |
| 546 | + "'edges' array with edge_type 'on_failure', from that node to the exact " | |
| 547 | + "earlier node_id it should return to -- the target must be a real " | |
| 548 | + "upstream dependency of this node (something that genuinely produced " | |
| 549 | + "input this node used), not an arbitrary node. This mirrors how the " | |
| 550 | + "success path already works: every node's 'yes, this succeeded, what's " | |
| 551 | + "next' is expressed as a normal dependency edge, and 'no, this failed, " | |
| 552 | + "go back to X' is expressed the same way, just with edge_type " | |
| 553 | + "'on_failure' instead. Every other edge needs edge_type: 'zero_token' " | |
| 554 | + "for pure aggregation/reduce steps, 'needs_judgment' for anything " | |
| 555 | + "requiring the agent to actually reason about content.\n" | |
| 556 | + "End your reply with the graph_manifest as a single fenced ```json code " | |
| 557 | + "block shaped exactly like {{\"nodes\": [...], \"edges\": [...]}} -- no " | |
| 558 | + "extra wrapper key." | |
| 559 | +).format(catalog_json=json.dumps(_PLANNER_NODE_CATALOG), goal_text="{goal_text}") | |
| 560 | +``` | |
| 561 | + | |
| 562 | +(`.format()` 两次是因为 `catalog_json` 在模块加载时就能确定、`goal_text` 要留到调用时才填——用 `{{`/`}}` 转义加上先格式化一次 `catalog_json`,再对外暴露带 `{goal_text}` 占位符的最终模板字符串,写法上等价于现有 `_GENERIC_PROMPT_TEMPLATE` 的处理方式,不是新模式。) | |
| 563 | + | |
| 564 | +### 8.3 `_normalize_graph_manifest` 的变化:删除,不是加固 | |
| 565 | + | |
| 566 | +`_CANONICAL_DOC_CHAIN` 常量、以及"当六个/五个规范类型全部出现时强制线性依赖顺序"那段逻辑(`work_graph_plan.py`,2026-08-07 已经因为禁用 test_script 缩成 5 元组)**整体删除**——FR-B4 已确认"全部自由规划,不保留任何代码级固定顺序兜底"。`_normalize_graph_manifest` 保留的部分: | |
| 567 | +- `id`/`node_id` 键名兼容(2026-08-01 真实教训)。 | |
| 568 | +- 从 `edges` 反推 `depends_on`(同上)。 | |
| 569 | + | |
| 570 | +新增的部分: | |
| 571 | +- **2026-08-08 修订(用户反馈)**:失败路径的目标节点不是节点身上一个跟 `edges` 脱节的独立字段,而是跟成功路径一样,由规划这个节点的时候一并定好的边决定——"yes 走哪个节点、no 走哪个节点,都是边定义好的"。所以校验对象变成:每个声明了 `on_fail.action="loop_back"` 的节点,必须**恰好有一条**从它出发、`edge_type="on_failure"` 的边(见第 4.2 节 `_on_failure_target`),且这条边指向的节点必须是该节点的(直接或间接)上游依赖——LLM 规划时可能"跳"到一个跟自己毫无依赖关系的节点(逻辑上没有意义:loop_back 的语义是"回到更早、真正影响了我的节点"),这种情况在 `_normalize_graph_manifest` 里直接拒绝并抛 `PlanningFailedError`(复用现有异常类),比让它在 `run_graph()` 执行到一半才报错更早暴露问题。 | |
| 572 | + | |
| 573 | +```python | |
| 574 | +def _validate_loop_back_targets(graph_manifest): | |
| 575 | + node_ids = {n["node_id"] for n in graph_manifest["nodes"]} | |
| 576 | + for node in graph_manifest["nodes"]: | |
| 577 | + on_fail = node.get("on_fail") or {} | |
| 578 | + if on_fail.get("action") != "loop_back": | |
| 579 | + continue | |
| 580 | + node_id = node["node_id"] | |
| 581 | + on_failure_edges = [ | |
| 582 | + e for e in graph_manifest.get("edges", []) | |
| 583 | + if e.get("from") == node_id and e.get("edge_type") == "on_failure" | |
| 584 | + ] | |
| 585 | + if len(on_failure_edges) != 1: | |
| 586 | + raise PlanningFailedError( | |
| 587 | + "node {} declares on_fail.action='loop_back' but has {} on_failure edge(s), expected exactly 1".format( | |
| 588 | + node_id, len(on_failure_edges) | |
| 589 | + ) | |
| 590 | + ) | |
| 591 | + target = on_failure_edges[0]["to"] | |
| 592 | + if target not in node_ids: | |
| 593 | + raise PlanningFailedError( | |
| 594 | + "node {}'s on_failure edge targets {!r}, which does not exist in this graph_manifest".format( | |
| 595 | + node_id, target | |
| 596 | + ) | |
| 597 | + ) | |
| 598 | + if target not in _transitive_ancestors(graph_manifest, node_id): | |
| 599 | + raise PlanningFailedError( | |
| 600 | + "node {}'s on_failure edge targets {!r}, which is not an upstream dependency -- " | |
| 601 | + "loop_back must target a node that genuinely influenced this one".format(node_id, target) | |
| 602 | + ) | |
| 603 | +``` | |
| 604 | + | |
| 605 | +(`_transitive_ancestors` 是 `_transitive_dependents`——第 4.2 节,`work_graph_orchestrate.py`——的反向版本;两边各自独立实现一份即可,`work_graph_plan.py` 不需要因此新引入对 `work_graph_orchestrate.py` 的依赖,两个模块现在也没有互相 import,保持这个边界。`_on_failure_target`——第 4.2 节——运行时可以放心假设"恰好一条"已经在这里校验过,不用再处理 0 条/多条的情况。) | |
| 606 | + | |
| 607 | +### 8.4 `plan_documents()` 调用点改动 | |
| 608 | + | |
| 609 | +`e2e_pipeline.py::plan_documents()` 不再需要 `_combined_planner_verify_fn`(PRD 一致性检查 + test_script 编译检查两个手写分支)——改成 `verify_fn=None`,因为自规划出的每个节点都自带 `judgment`,`run_graph()` 内部会自动解析。`mechanical_check_registry={"check_script_compiles": check_script_compiles}` 照常传入。旧的 `_combined_planner_verify_fn`/`_prd_consistency_verify_fn`/`_test_script_verify_fn` 函数**不删除**——`validate_implementation_with_own_graph()`(Validator 自己的硬编码 3/2 节点图,本轮不改,见需求文档非目标)继续用它们,且它们仍然是"外部 verify_fn 向后兼容路径"的真实调用方,删除会破坏 FR-A8。 | |
| 610 | + | |
| 611 | +## 9. 向后兼容性核对表(FR-A8) | |
| 612 | + | |
| 613 | +| 现有调用点 | 是否声明 `judgment`/`on_fail` | 改造后走的路径 | | |
| 614 | +|---|---|---| | |
| 615 | +| `implement_code()` 的合成单节点图 | 否 | `_resolve_node_verify_fn` 直接返回 `None`(该调用点本来就不传 `verify_fn`),零改动 | | |
| 616 | +| `validate_implementation_with_own_graph()`(`_VALIDATOR_MANIFEST`) | 否 | `_resolve_node_verify_fn` 返回外部 `_test_script_verify_fn(...)`,失败时走 `hard_fail`/现状 `_verify_and_maybe_regenerate`,逐字节不变 | | |
| 617 | +| `work_graph_fork.py`(`_GENERIC_PROMPT_TEMPLATE` 路径) | 否 | 同上,无 `verify_fn` 传入,无判断 | | |
| 618 | +| `plan_documents()`(Part B 改造后) | 是(每个节点自带) | 走新的 `judgment`/`on_fail` 解析路径 | | |
| 619 | + | |
| 620 | +## 10. 测试计划(对应需求文档验收标准 1-10) | |
| 621 | + | |
| 622 | +| 验收标准 | 测试方式 | | |
| 623 | +|---|---| | |
| 624 | +| 1-2. loop_back 累积历史 + 触发上限 | mock `execute_work_graph`,构造 A→B、B 声明 `on_fail=loop_back(target=A)`,B 连续失败 N 次;断言 A 被重跑 N 次、`execution_event` 里有 N 条 `loop_back_triggered`、A 第 N 次收到的 prompt 包含前 N-1 次的 feedback 文本 | | |
| 625 | +| 3. 崩溃重启后正确恢复轮次 | 跑到第 2 次 loop_back 时中断(mock 抛异常模拟进程退出),重新调用 `run_graph()`,断言 `_loop_attempt_count_from_events` 从 DB 读到 2,不是从 0 开始 | | |
| 626 | +| 4. 静态图校验不因 loop_back 报环 | 一个声明了 `on_fail.action="loop_back"` 且带一条指向上游节点的 `on_failure` 边的 `graph_manifest`,`topological_layers()` 正常返回分层,不抛 `ValueError`(因为 `on_failure` 边类型根本不参与 `topological_layers` 的依赖计算,只有 `depends_on`/普通数据依赖 edge 参与) | | |
| 627 | +| 5. 未声明字段的图逐字节不变 | 现有 `test_work_graph_orchestrate.py`/`test_e2e_pipeline.py` 全部既有用例不改断言即通过 | | |
| 628 | +| 6-7. Part B 自由规划 + 健壮性兜底 | 同需求文档,mock LLM 规划响应缺 test_dataset / 缺 depends_on,断言分别正确处理 | | |
| 629 | +| 8. test_script 可选 | mock 规划响应包含 test_script 节点,断言正常规划执行 | | |
| 630 | +| 9. 按 org_node/node_type 配置上限生效 | `settings_config.upsert_loop_limit_config` 写两条不同配置,构造两个 loop_back 场景,断言各自在配置的上限触发 | | |
| 631 | +| 10. loop 历史只在 `execution_event` | 触发若干轮 loop_back 后,只查 `execution_event`(不查任何新表)复原完整历史 | | |
| 632 | +| 新增:`_validate_loop_back_targets` 拒绝非法目标 | 分别构造"`on_failure` 边指向的节点不是上游依赖"、"声明 `loop_back` 但没有 `on_failure` 边"、"同一个节点有 2 条 `on_failure` 边"三种场景,断言 `plan_work_graph()` 都抛 `PlanningFailedError` | | |
| 633 | +| 新增:mechanical_check_registry 未注册报错 | `judgment.check_ref` 指向一个不存在的名字,断言 `run_graph()` 在执行任何节点前就报错,不是跑到那个节点才发现 | | |
| 634 | +| 新增:`topological_layers()` 正确忽略 `on_failure` 边(4.0 节修复) | 构造 A→B(普通依赖)+ B→A(`on_failure` 边)的 `graph_manifest`,断言 `topological_layers()` 返回 `[[A], [B]]`(不因为 B→A 那条边被误判成环);另跑一遍 G 既有的 `test_work_graph_orchestrate.py` 全部用例确认零回归 | | |
| 635 | + | |
| 636 | +## 11. 实施步骤建议 | |
| 637 | + | |
| 638 | +1. `schema.py` 两处迁移(`work_graph_loop_limit_config` 建表 + `migrate_add_loop_back_event_type`)+ `settings_config.py` 两个新函数 + `_ensure_schema()` 接入——纯数据层改动,先落地,不碰任何执行逻辑。 | |
| 639 | +2. `work_graph_orchestrate.py`:`_resolve_node_verify_fn`/机械检查注册表注入/`_llm_judgment_verify_fn`——先只做"judgment 解析",不做 loop_back(这一步验证:没有 `on_fail` 时行为完全不变,第 9 节核对表跑一遍)。 | |
| 640 | +3. `work_graph_orchestrate.py`:`_transitive_dependents`/`_LoopBackRequested`/`run_graph()` 主循环回退逻辑/attempt 后缀机制——loop_back 的核心,单独用 mock 场景验证第 10 节测试计划 1-5、9、10。 | |
| 641 | +4. `work_graph_plan.py`:删除 `_CANONICAL_DOC_CHAIN` 强制顺序逻辑、`_PLANNER_NODE_CATALOG`、新 `_PROMPT_TEMPLATE`、`_validate_loop_back_targets`——Part B,第 10 节测试计划 6-8、新增两项。 | |
| 642 | +5. `e2e_pipeline.py::plan_documents()` 切到 `verify_fn=None` + `mechanical_check_registry`,端到端跑一次 mock 场景确认整条链路。 | |
| 643 | +6. 真实场景验证(`RUN_REAL_AGENT_E2E=1`,小心控制次数/成本):观察一次真实 Planner 自规划调用产出的 `graph_manifest` 长什么样,是否合理、是否触发过 `PlanningFailedError`。 | ... | ... |
docs/todo/work-graph-self-planning-and-loop-back-requirements.md
0 → 100644
| 1 | +# Planner 动态自规划 Work Graph(含节点判断与失败回路)——需求文档 | |
| 2 | + | |
| 3 | +**日期**: 2026-08-08 | |
| 4 | +**性质**: 需求文档(BRD/PRD 层面),不含详细技术设计。回应用户 2026-08-07/08-08 两轮讨论:先是"要不要让 Planner 自己判断要不要有 test_script 节点",深入后明确为一个更完整的诉求——Planner 第一步应该是自己规划 Work Graph(要不要 brd/prd/sdd/benchmark/test 的组合、未来还有更多 tool/skill 可组合调用),并为每个节点规划输入输出边约束、判断成功/失败的方式,失败时把结果送回更早的节点形成回路(loop),并把 loop 过程中的产出记录下来供后续轮次参考改进。详细设计(SDD 层面:graph_manifest 的具体 JSON 形状、`topological_layers()`/`run_graph()` 的具体改造点、循环历史的具体持久化方式)留到本文档确认后单独产出。 | |
| 5 | +**状态**: ✅ 已实现(2026-08-08)。原「8. 开放问题」的 7 个分叉点均已拍板,记录见「8. 已确认的决策」,正文相应章节已按决策更新。详细设计见 `docs/todo/work-graph-self-planning-and-loop-back-design.md`;Part A(判断+回路引擎)与 Part B(Planner 自规划)均已实现,`python3 -m unittest discover -s src`(692 tests)+ `discover -s supervisor -p "test_*.py"`(126 tests)全绿。尚未做真实 `RUN_REAL_AGENT_E2E=1` 端到端验证(需要真实 API 额度,等待用户决定是否现在跑)。 | |
| 6 | + | |
| 7 | +## 1. 背景 | |
| 8 | + | |
| 9 | +### 1.1 现状:三条独立的、能力有限的机制 | |
| 10 | + | |
| 11 | +- **Planner 自己的文档链**(`work_graph_plan.py::_PROMPT_TEMPLATE`):过一层真实的 LLM 规划调用,但提示词硬编码要求"必须为 brd/prd/sdd/benchmark/test_dataset 各出一个节点"(test_script 已于 2026-08-07 从这个固定清单里移除,见 `docs/todo/validator-test-script-narrative-output-issue.md`)——节点集合本质是固定的,规划这一步只是在确认依赖关系,不是真的自由决策"要不要有某个节点"。这是 2026-08-06 的一次明确设计决定(`docs/done/dynamic-work-graph-design-skill-design.md` §3 原话:"不是让 Claude 完全自由决定 Planner 要生成哪些节点——Planner 那条链是'架构常量'")。 | |
| 12 | +- **`_GENERIC_PROMPT_TEMPLATE`**(同文件,服务 Coder/Validator 的子任务图):真正让 LLM 自由规划 `{node_id, depends_on, task_prompt}`,但目前只有 `work_graph_fork.py`(人工发起的 time-travel 调试功能)在用,没有接入任何生产路径的自动执行;且和 Planner 的固定链一样,只是一张纯 DAG,没有判断/回路概念。 | |
| 13 | +- **`work_graph_orchestrate.py::run_graph()` 的验证-重试机制**:`_verify_and_maybe_regenerate()` 只支持"某节点验证失败 → 原地重新生成同一个节点一次 → 还失败就整体 `ConsistencyCheckFailedError`"。这不是回路——失败结果不会被送回更早的节点,历史反馈也只保留最近一轮(`"; ".join(feedback)` 直接拼进 reflect prompt,不会累积多轮)。 | |
| 14 | + | |
| 15 | +### 1.2 两个此前未被充分利用的既有事实 | |
| 16 | + | |
| 17 | +- **`edge_type`(`zero_token`/`needs_judgment`)字段已经存在于 `GRAPH_MANIFEST_SCHEMA`,但从未被任何执行逻辑读取**——`topological_layers()` 只看 `depends_on`/`from-to` 决定分层,完全不管 `edge_type` 的值,是纯文档字段。 | |
| 18 | +- **`topological_layers()` 对任何环直接 `raise ValueError`**——现在的执行引擎从数据结构到算法都是严格 DAG,没有"跳回更早节点"这种控制流的容身之处。`docs/sdd/01-M-模型设计.md:60` 里 `work_graph_run.graph_manifest` 字段的文档说明其实从一开始就写了"概念上应涵盖 fan-out/fan-in 分组、loop 规则",只是从未真正实现过。 | |
| 19 | + | |
| 20 | +### 1.3 一个必须正面回应的架构决定 | |
| 21 | + | |
| 22 | +`docs/graphlemon_architecture.md`"方案 A"明确设想过:Work Graph 内部的 fan-out/fan-in/loop/verifier 应该直接用 Claude Code 原生 Dynamic Workflow 原语(`agent()`/`parallel()`/`pipeline()`,类似 loop-until-dry)编排,而不是手撸 Python 引擎。但 `work_graph_orchestrate.py` 的实际实现故意背离了这个设想(一个节点一个独立 `claude -p` 子进程 + Python 拓扑引擎逐节点持久化状态),原因写在该模块自己的 docstring 里:REQ-06 断点续跑——原生原语跑在一次不透明的 `claude -p` 调用里,中途崩溃就会丢失全部循环历史和已完成节点状态;现在的设计换来的是"进程重启后能从任意已完成节点继续,不重跑"。 | |
| 23 | + | |
| 24 | +本需求延续这个既有背离,在 Python 引擎里新增回路能力,而不是回退去用原生原语——因为 REQ-06 是已经在生产里被真实依赖的能力(self-hosting 崩溃恢复、worktree resume),原生原语给不了这个保证。 | |
| 25 | + | |
| 26 | +## 2. 目标 | |
| 27 | + | |
| 28 | +1. `graph_manifest` 的节点能声明式地表达"怎么判断这个节点成功/失败"(机械检查引用,或另一次 LLM 判断),不再要求调用方在 `e2e_pipeline.py` 里为每种 artifact_type 手写专门的 `verify_fn` 分支。 | |
| 29 | +2. `graph_manifest` 的节点能声明式地表达"失败之后怎么办":原地重试(现状)、跳回更早的某个指定节点重新执行(新增)、或直接判定整图失败(现状的显式化)。 | |
| 30 | +3. 触发"跳回更早节点"时,被跳回节点到失败节点之间的整条下游链要被重新执行(下游数据是基于坏输入产出的,不能继续用);同时,这个节点每一轮被拒绝的历史(不只是最近一轮)要被完整地喂给它自己的下一次重新生成,让它能看到"过去几次分别为什么被拒绝",而不是每次都从零开始猜。 | |
| 31 | +4. 这套回路能力必须有可配置的上限,且在进程中途崩溃重启后能正确恢复"当前是第几轮",不能死循环也不能重置归零。 | |
| 32 | +5. 在回路执行引擎具备之后,让 Planner 自己的规划调用能真正决定"这个 goal 需要哪些节点",而不是像现在这样被提示词硬编码死一份固定清单——这份清单本身也应该是可扩展的数据,为未来接入更多 tool/skill 节点类型留出空间(这轮不要求真的新增新类型,只要求机制本身可扩展)。 | |
| 33 | +6. **长期方向(本轮不实施,仅作为设计约束)**:用户明确的最终目标是"Planner 改造成功后,Coder、Validator、甚至未来新增的 org graph node,都需要具备同样的能力——先 plan 自己的 work graph,再按照 work graph 执行"。本轮的 Part A(judgment/on_fail 机制)设计必须是 org-agent 无关的通用能力,不能是 Planner 专属的特殊逻辑,即使这轮只把它接到 Planner 一个 org node 上(见「3. 非目标」第 1 条、「8. 已确认的决策」第 6 条)。 | |
| 34 | + | |
| 35 | +## 3. 非目标(本轮明确不做) | |
| 36 | + | |
| 37 | +1. **本轮实施范围仍只覆盖 Planner,不把 Coder/Validator 也接上自规划**。`_VALIDATOR_MANIFEST` 现在完全是硬编码的 Python 字面量,连"过一层 LLM 规划"都没有;Coder 现在是合成单节点图。用户已确认长期方向是所有 org agent(含未来新增的)都要具备同样能力(见「2. 目标」第 6 条),但落地顺序是"Planner 改造验证成功后,再依次推广"——不是本轮的实施范围,本轮只是设计时不能把机制设计成 Planner 专属。 | |
| 38 | +2. **不回退到 Claude Code 原生 Dynamic Workflow 原语**(见 1.3)。继续在 Python 层的 `work_graph_orchestrate.py` 里扩展执行引擎,保住 REQ-06 断点续跑。 | |
| 39 | +3. **不追求完全通用的图执行框架**。回路的触发方式限定为"某节点判断失败 → 跳回自己声明的 `on_fail.target_node_id`",不支持任意节点间的条件分支、多目标跳转、或图结构在运行时被重新规划——这些是更大的能力,不在这轮范围内。 | |
| 40 | +4. **不重新设计跨 Org 的既有 loop 机制**(`org_level_loop.py`/`org_loop_route.py`:Validator FAIL/INCONCLUSIVE → 判根因 → 路由回 Planner/Coder,`loop_round` 封顶 3 轮)。本需求的回路是"同一个 Org Agent 自己的 Work Graph 内部"这一层,比跨 Org 的 loop 粒度更细,两者独立计数、不共享额度、不互相替代——内层耗尽自己的上限后照旧抛 `ConsistencyCheckFailedError`,外层机制不变地接管。 | |
| 41 | +5. **不在本需求里解决 test_script/validator_test_script 节点"叙述文字而非代码"的结构性根因**。这个节点已于 2026-08-07 因该问题禁用(`docs/todo/validator-test-script-narrative-output-issue.md`),根因是"探索后收尾习惯",跟本需求要解决的问题(节点集合是否可以自由规划、失败能否形成回路)完全正交。用户已确认(见「8. 已确认的决策」第 7 条):Part B 上线后,test_script 可以直接作为可选节点提供给 Planner 自由规划选择,不需要等这个结构性问题修复;该结构性问题的真正修复安排在另一份文档里,跟"给 Coder agent 加入 test_script 模块"一起解决——本需求只负责不阻塞它,不负责修它。 | |
| 42 | +6. **不修改现有 `_GENERIC_PROMPT_TEMPLATE` 路径本身**(Coder/Validator 子任务图、`work_graph_fork.py` 用的那条)。如果这轮的 judgment/on_fail 字段设计得足够通用,未来可以顺带让这条路径也受益,但不是本轮的验收范围。 | |
| 43 | + | |
| 44 | +## 4. 功能需求 | |
| 45 | + | |
| 46 | +分两部分:Part A 是执行引擎扩展(判断 + 回路能力),Part B 是 Planner 自规划节点集合。**Part B 依赖 Part A 先完成**——自由规划出的图如果没有真正的失败回路可用,只是重新引入"LLM 自由吐图结构"的脆弱性(2026-08-03 那次真实故障:LLM 漏给 `depends_on`/`edges`,`benchmark` 排到 `sdd` 前面执行,直接 `KeyError` 崩溃),却拿不到新能力。 | |
| 47 | + | |
| 48 | +### Part A:节点判断 + 失败回路执行引擎(`work_graph_orchestrate.py`) | |
| 49 | + | |
| 50 | +| # | 需求描述 | 优先级 | | |
| 51 | +|---|---|---| | |
| 52 | +| FR-A1 | `graph_manifest` 的节点应支持声明式的 `judgment` 配置(机械检查引用,或触发一次额外 LLM 判断调用),替代现在"调用方在 Python 里为每种 artifact_type 手写 `verify_fn` 分支"的模式。 | 必须 | | |
| 53 | +| FR-A2 | `graph_manifest` 的节点应支持声明式的 `on_fail` 配置,至少支持三种行为:`retry_self`(现状:原地重新生成同一节点一次)、`loop_back`(跳转到指定的更早节点重新执行)、`hard_fail`(不重试,直接判定整图失败——现状默认行为的显式化)。 | 必须 | | |
| 54 | +| FR-A3 | 触发 `loop_back` 时,不只重跑 `target_node_id` 本身,还要重跑它到触发失败的节点之间的整条下游链,因为这条链上的产出都是基于(即将被修正的)过期输入生成的。 | 必须 | | |
| 55 | +| FR-A4 | `loop_back` 的每一轮反馈(这一轮产出了什么、为什么被判定失败)必须可完整还原,且 `target_node_id` 重新执行时,累积的完整历史(不只是最近一轮)要被喂进它的提示词。**不新增专门的历史表**——复用现有 `execution_event` 日志机制(`node_start`/`node_completed` 等既有事件),每次 `loop_back` 触发/重新执行时在事件里记录 predecessor/successor 节点关联,让"这个节点被打回过几次、每次因为什么"可以直接从现有事件流重建,同时让现有的图可视化机制(`v_graph_visualizer`)能把回路自然地呈现成图上的一条边,不需要另外维护一份历史、也不需要事后做双份数据的一致性对账。 | 必须 | | |
| 56 | +| FR-A5 | 每一对 `(loop_back 目标节点, 触发回路的失败节点)` 需要独立计数,并有上限;超限时整个 `run_graph()` 调用判定失败(复用/扩展现有 `ConsistencyCheckFailedError` 语义),不允许无限循环。**上限不是写死的全局常量**——复用 `settings_config.py`(现有 `org_concurrency_config` 的同类模式:DB 存储、`get_/upsert_` 读写接口)新增一张按 `(org_node, node 类型或 node_id 模式)` 维度可配置的上限表,允许后续根据真实调用经验(哪些节点类型容易被反复打回、上限设多少更合理)动态调整,不需要改代码重新部署;节点自身也可以在 `on_fail.max_attempts` 里覆盖这个默认值。 | 必须 | | |
| 57 | +| FR-A6 | 循环计数与历史必须能扛得住 REQ-06 式的断点续跑:进程在循环中途崩溃重启后,应正确恢复"当前是第几轮、历史反馈有哪些",不能从零重置,也不能因为状态丢失而死循环。 | 必须 | | |
| 58 | +| FR-A7 | `topological_layers()` 的静态分层只应基于"数据依赖边"(`depends_on`/`consumes`);节点声明的 `on_fail.target_node_id`(运行时才可能触发的控制流跳转)不参与静态拓扑排序,不应被当成图里的环而报错。 | 必须 | | |
| 59 | +| FR-A8 | 不声明 `judgment`/`on_fail` 字段的既有 `graph_manifest`(例如 Coder 现在的合成单节点图)行为必须完全不变——这两个字段可选,缺省等价于现状(`judgment=none` 直接通过、`on_fail=hard_fail` 即现在的"验证失败重试一次后硬失败")。 | 必须 | | |
| 60 | +| FR-A9 | `judgment`/`on_fail` 机制的实现(`graph_manifest` schema、`run_graph()` 的执行逻辑、FR-A5 的上限配置表)必须是 org-agent 无关的通用能力——不能依赖任何 `org_node == "planner"` 的特判。这轮只有 Planner 会真正产出带这些字段的 `graph_manifest`,但 Coder/Validator 未来接入时,不应该需要改 `work_graph_orchestrate.py` 本身,只需要它们自己的规划提示词开始产出这些字段(呼应「2. 目标」第 6 条的长期方向)。 | 必须 | | |
| 61 | + | |
| 62 | +### Part B:Planner 自规划节点集合(依赖 Part A) | |
| 63 | + | |
| 64 | +| # | 需求描述 | 优先级 | | |
| 65 | +|---|---|---| | |
| 66 | +| FR-B1 | Planner 的规划提示词从"必须产出固定清单里的每一个节点"改为:给定一份可用节点类型清单,由 LLM 自己判断这个 goal 需要哪些节点、它们的依赖关系,以及(复用 Part A)每个节点的 `judgment`/`on_fail` 配置。 | 必须 | | |
| 67 | +| FR-B2 | 可用节点类型清单本身应是可扩展的数据(而不是硬编码在提示词字符串里的固定枚举),为未来接入更多 tool/skill 节点类型留出空间;本轮不要求真的新增除现有文档类型之外的新类型。 | 应该 | | |
| 68 | +| FR-B3 | 自由规划出的 `graph_manifest` 必须经过与现有 `_normalize_graph_manifest` 同等严格的健壮性校验/兜底,防止 2026-08-03 那次真实故障重演(LLM 输出遗漏 `depends_on`/`edges`,导致执行顺序错误甚至崩溃)——节点集合变自由不代表可以降低这层鲁棒性要求。 | 必须 | | |
| 69 | +| FR-B4 | **brd/prd/sdd/benchmark/test_dataset(以及 test_script,一旦被规划出来)全部由 LLM 自由规划,不保留任何代码级的固定顺序/固定节点集合兜底**——已确认(「8. 已确认的决策」第 1 条):完全推翻 `docs/done/dynamic-work-graph-design-skill-design.md` §3"Planner 链是架构常量"的决定。FR-B3 的健壮性校验兜底(防止 LLM 漏给 `depends_on`/`edges`)因此变得更重要:没有任何硬编码的顺序可以兜底,健壮性完全依赖 FR-B3。 | 必须 | | |
| 70 | + | |
| 71 | +## 5. 验收标准 | |
| 72 | + | |
| 73 | +1. 构造一个 `graph_manifest`:节点 A 产出,节点 B 依赖 A、声明 `judgment` 机械检查、`on_fail=loop_back` 指向 A;用 mock 让 B 连续两轮判断失败、第三轮通过。断言:A 被重新执行了两次(不是原地重试 B),且 A 第二次、第三次重新执行时收到的提示词里包含前面所有轮次的失败反馈(不只是最近一轮)。 | |
| 74 | +2. 同上场景,把 B 的失败改成"每一轮都失败",断言在达到可配置上限后,`run_graph()` 判定整图失败并明确抛出(而不是无限循环挂起),且失败前 A 已经产出的内容仍按现有"部分成功节点落盘"机制(`_write_completed_node_artifacts`)被正确保留。 | |
| 75 | +3. 模拟"回路进行到第 2 轮时进程中断重启",断言恢复执行后能正确识别"已经是第 2 轮"并继续,而不是把计数器归零重新从第 1 轮开始,也不是把已经持久化的历史反馈丢掉。 | |
| 76 | +4. 一个声明了 `on_fail.target_node_id` 指向更早节点的 `graph_manifest`,`topological_layers()`/图校验逻辑不应把它当作静态环拒绝(不抛 `ValueError`)。 | |
| 77 | +5. 一个完全不使用 `judgment`/`on_fail` 字段的既有 `graph_manifest`(例如 Coder 当前用的合成单节点图),跑一遍 `run_graph()`,行为与改造前逐字节一致——现有 `src/test_work_graph_orchestrate.py`/`src/test_e2e_pipeline.py` 里未显式测试新字段的用例,不修改断言即可继续通过。 | |
| 78 | +6. (Part B)给 Planner 一个明显不需要 test_dataset 的 goal(例如纯配置项调整),断言自规划产出的 `graph_manifest` 里确实没有 test_dataset 节点,且整条链路(Coder 拿到的 `test_dataset_content` 走 `"N/A: not provided"` 兜底)不因为节点缺失而报错——复用现有的 `"test_dataset" in artifact_ids` 容错逻辑,不需要新代码。 | |
| 79 | +7. (Part B)人为构造一个自由规划回复"遗漏 `depends_on`"的场景(模拟 2026-08-03 那次真实故障的输入),断言健壮性兜底仍能纠正执行顺序,不崩溃。 | |
| 80 | +8. (Part B)给 Planner 一个明显需要机械化验收脚本的 goal,可用节点类型清单里包含 test_script,断言 Planner 能够(不是必须,是"能够")自由规划出一个 test_script 节点——确认 test_script 已按「8. 已确认的决策」第 7 条重新开放为可选节点,不再被清单排除。 | |
| 81 | +9. (FR-A5)验证上限配置确实按 `(org_node, node 类型)` 生效:给同一个 org_node 下两种不同 node 类型分别配置不同上限,构造两个各自持续失败的 loop_back 场景,断言各自在自己配置的上限触发失败,不是共用同一个全局值;再验证节点自身 `on_fail.max_attempts` 能覆盖配置表里的默认值。 | |
| 82 | +10. (FR-A4)验证 loop_back 历史确实通过 `execution_event` 而非新表重建:触发一次多轮 loop_back 后,仅查询 `execution_event` 表(不查询任何新增表)即可还原"这个节点被打回几次、每次的 predecessor/successor 是谁、每次的反馈内容"。 | |
| 83 | + | |
| 84 | +## 6. 约束与假设 | |
| 85 | + | |
| 86 | +- 复用 REQ-06 已经建立的"`work_node_execution.output_payload` 是节点成功后的权威持久化数据"这一既有约定;回路历史**不新增专门的表**,完整复用 `execution_event` 现有机制(见 FR-A4),必须遵循"重启后可完整恢复"这一约束,不引入新的、易丢失的临时状态。 | |
| 87 | +- 复用 `org_loop_route.py::_LOOP_LIMIT` 的既有模式("计数 + 达到上限即转 `blocked_pending_human`")作为 Part A 回路上限触发后果的参照,但两者是独立的计数器,不共享同一个 `task.loop_round` 字段;上限的具体数值来源改为复用 `settings_config.py` 的 DB 配置模式(见 FR-A5),不是抄一个固定常量。 | |
| 88 | +- `judgment.kind="llm"` 复用 `_check_prd_consistency` 现有模式:额外发起一次 `agent()` 调用(`agentType='planner'`),self-target 场景下走独立 worktree 隔离,避免重新设计一套判断机制。 | |
| 89 | +- Part A 完成后,Coder 现在的合成单节点图、`_GENERIC_PROMPT_TEMPLATE` 路径都不强制迁移使用新字段——新字段是可选能力,不是所有 `graph_manifest` 生产者都必须立刻采用;但机制本身必须 org-agent 无关(见 FR-A9),为后续推广铺路。 | |
| 90 | + | |
| 91 | +## 7. 风险与已知权衡 | |
| 92 | + | |
| 93 | +- **用户已知情并接受:Part B 上线后 test_script 类节点可能被自由规划回来,narrative-output 问题可能重现**。`docs/todo/validator-test-script-narrative-output-issue.md` 记录的根因是"探索后收尾习惯",跟本需求解决的问题(节点集合能不能自由规划、失败能否形成回路)完全正交,本需求不修它。用户决策(「8. 已确认的决策」第 7 条):不等结构性修复,真正的修复安排在"给 Coder agent 加入 test_script 模块"那份文档里一起解决——本需求上线后,如果自举/真实任务里又复现了叙述文字问题,这是已知的、被接受的风险,不是本需求的回归。 | |
| 94 | +- **回路能力本身可能被滥用成隐藏的高成本重试**:一个设计不当的 `judgment`(例如过于严格的 LLM 判断标准)配合 `loop_back`,可能导致某个节点反复被打回、消耗大量 token 才勉强通过上限。FR-A5 的按 `(org_node, node 类型)` 可配置上限缓解了"一刀切"的问题,但初始默认值仍需要设计阶段给出具体理由,不能只是"抄一个数字";上线后应该根据真实调用数据回头调整(这正是选择"DB 配置表"而不是"硬编码常量"的原因)。 | |
| 95 | +- **Part B 是对已有架构决定的正式推翻**,不是渐进式增强。`docs/done/dynamic-work-graph-design-skill-design.md` §3 需要在 Part B 实施后同步更新,避免文档和代码再次出现本 session 里多次遇到的"文档记录的意图与实际代码不一致"的问题。 | |
| 96 | +- **Part A + Part B 一并实施(已确认,见「8. 已确认的决策」第 5 条),没有中间的"仅 Part A 先跑一段时间验证稳定性"的缓冲阶段**——意味着 Part A 的回路引擎第一次被真实使用,就是在"节点集合本身也是自由规划、不受固定链条约束"这个更不可预测的场景下,两个新变量同时引入,出问题时定位"是回路引擎的 bug 还是自由规划出了一个奇怪的图结构"会比分阶段更难。这是用户已知情的权衡,不是本需求遗漏。 | |
| 97 | + | |
| 98 | +## 8. 已确认的决策(2026-08-08) | |
| 99 | + | |
| 100 | +1. **Part B 的推翻力度**:全部自由规划。brd/prd/sdd/benchmark/test_dataset(以及重新开放的 test_script)不保留任何代码级固定顺序/固定节点集合兜底,完全推翻 `docs/done/dynamic-work-graph-design-skill-design.md` §3 的既有决定(对应 FR-B4)。健壮性完全依赖 FR-B3 的校验兜底。 | |
| 101 | +2. **回路上限**:不设单一全局默认值,按 `(org_node, node 类型)` 可配置,复用 `settings_config.py` 的 DB 配置模式,根据实际调用经验动态调整(对应 FR-A5)。 | |
| 102 | +3. **`judgment.kind="llm"` 的实现**:复用现有 `_check_prd_consistency` 模式(额外发起一次 `agent()` 调用,`agentType='planner'`,self-target 场景走独立 worktree)。 | |
| 103 | +4. **回路历史持久化**:不新增专门的表,完整复用 `execution_event` 现有日志机制,记录 predecessor/successor 节点关联,天然可被现有图可视化机制呈现为有向图上的边(对应 FR-A4)。 | |
| 104 | +5. **落地顺序**:Part A、Part B 一起做,不分阶段(见「7. 风险」最后一条,这是已知权衡,不是遗漏)。 | |
| 105 | +6. **Validator/Coder 是否一起做**:本轮仍只落地 Planner,但 Part A 的机制设计必须 org-agent 无关(FR-A9)——用户明确的长期方向是 Planner 验证成功后,Coder、Validator、乃至未来新增的 org graph node 都要具备"先 plan 自己的 work graph,再按图执行"的同一能力(见「2. 目标」第 6 条)。 | |
| 106 | +7. **test_script 是否要等结构性修复**:不需要等。Part B 上线后,test_script 直接作为可选节点开放给 Planner 自由规划;narrative-output 问题的真正结构性修复安排在"给 Coder agent 加入 test_script 模块"的另一份文档里一起解决,不是本需求的阻塞项(见「7. 风险」第一条)。 | ... | ... |
src/backend_server.py
| ... | ... | @@ -1036,6 +1036,8 @@ def _ensure_schema(conn): |
| 1036 | 1036 | # startup). Run all three every time, not just on first-ever creation. |
| 1037 | 1037 | schema.migrate_expand_artifact_types(conn) |
| 1038 | 1038 | schema.migrate_relax_org_node_constraints(conn) # also runs migrate_add_work_graph_fork_columns internally |
| 1039 | + schema.migrate_add_work_graph_loop_limit_config(conn) | |
| 1040 | + schema.migrate_add_loop_back_event_type(conn) | |
| 1039 | 1041 | |
| 1040 | 1042 | |
| 1041 | 1043 | class _Server(ThreadingHTTPServer): | ... | ... |
src/e2e_pipeline.py
| ... | ... | @@ -208,6 +208,10 @@ _VALIDATOR_DOC_PROMPTS = { |
| 208 | 208 | ), |
| 209 | 209 | } |
| 210 | 210 | |
| 211 | +# validator_test_script node temporarily disabled (2026-08-07, same | |
| 212 | +# stopgap as work_graph_plan._PROMPT_TEMPLATE -- see its comment for the | |
| 213 | +# full rationale/upgrade path): 6/6 real repro rate for narrative-instead- | |
| 214 | +# of-code output, docs/todo/validator-test-script-narrative-output-issue.md. | |
| 211 | 215 | _VALIDATOR_MANIFEST = { |
| 212 | 216 | "nodes": [ |
| 213 | 217 | {"node_id": "validator_benchmark", "artifact_type": "validator_benchmark", "depends_on": []}, |
| ... | ... | @@ -215,15 +219,6 @@ _VALIDATOR_MANIFEST = { |
| 215 | 219 | "node_id": "validator_test_dataset", "artifact_type": "validator_test_dataset", |
| 216 | 220 | "depends_on": ["validator_benchmark"], |
| 217 | 221 | }, |
| 218 | - { | |
| 219 | - # depends on BOTH -- its own prompt needs | |
| 220 | - # {validator_benchmark_content} directly, not just what | |
| 221 | - # survives into validator_test_dataset_content (same reasoning | |
| 222 | - # as the Planner's own test_script node, see | |
| 223 | - # work_graph_plan._normalize_graph_manifest). | |
| 224 | - "node_id": "validator_test_script", "artifact_type": "validator_test_script", | |
| 225 | - "depends_on": ["validator_test_dataset", "validator_benchmark"], | |
| 226 | - }, | |
| 227 | 222 | ], |
| 228 | 223 | "edges": [], |
| 229 | 224 | } |
| ... | ... | @@ -240,6 +235,24 @@ _VALIDATOR_VERDICT_PROMPT = ( |
| 240 | 235 | ) |
| 241 | 236 | |
| 242 | 237 | |
| 238 | +class _MissingContentDefault(dict): | |
| 239 | + """2026-08-08 (found via a real self-planned run, task involving a | |
| 240 | + src/work_node_run.py bug fix): `_DOC_PROMPTS` entries were all written | |
| 241 | + for the old fixed brd->prd->sdd->benchmark->... chain and hardcode | |
| 242 | + placeholders like {prd_content} assuming that upstream node always | |
| 243 | + exists. Work Graph self-planning (FR-B4 "全部自由规划") legitimately | |
| 244 | + lets the Planner skip nodes that don't apply -- this real run's SDD | |
| 245 | + node correctly had no PRD node at all (a small bug-fix goal doesn't | |
| 246 | + need one), so `_DOC_PROMPTS["sdd"]`'s `{prd_content}` placeholder had | |
| 247 | + nothing to fill it and `.format()` raised a bare KeyError, landing the | |
| 248 | + task on `blocked_pending_human` with the unhelpful generic | |
| 249 | + 'execution_error' reason. `str.format_map` with this as the mapping | |
| 250 | + substitutes a clear placeholder for any content key the self-planned | |
| 251 | + graph didn't actually produce, instead of crashing.""" | |
| 252 | + def __missing__(self, key): | |
| 253 | + return "N/A (no corresponding upstream node was included in this Work Graph)" | |
| 254 | + | |
| 255 | + | |
| 243 | 256 | def _planner_prompt_builder(goal_text, node_artifact_types): |
| 244 | 257 | """node_artifact_types: dict[node_id] -> artifact_type, read off the |
| 245 | 258 | real graph_manifest before running it (build_prompt only ever sees one |
| ... | ... | @@ -247,13 +260,13 @@ def _planner_prompt_builder(goal_text, node_artifact_types): |
| 247 | 260 | def build_prompt(node, outputs): |
| 248 | 261 | node_id = node["node_id"] |
| 249 | 262 | artifact_type = node_artifact_types.get(node_id, node_id) |
| 250 | - content_kwargs = {} | |
| 263 | + content_kwargs = {"goal_text": goal_text} | |
| 251 | 264 | for dep_id in (node.get("depends_on") or []): |
| 252 | 265 | dep_output = outputs.get(dep_id) |
| 253 | 266 | if dep_output is not None: |
| 254 | 267 | dep_type = node_artifact_types.get(dep_id, dep_id) |
| 255 | 268 | content_kwargs[dep_type + "_content"] = dep_output["content"] |
| 256 | - return _DOC_PROMPTS[artifact_type].format(goal_text=goal_text, **content_kwargs) | |
| 269 | + return _DOC_PROMPTS[artifact_type].format_map(_MissingContentDefault(content_kwargs)) | |
| 257 | 270 | return build_prompt |
| 258 | 271 | |
| 259 | 272 | |
| ... | ... | @@ -558,6 +571,20 @@ def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): |
| 558 | 571 | re-raised unchanged after salvaging -- this does not change whether or |
| 559 | 572 | when a task ends up blocked_pending_human, only what's left behind for |
| 560 | 573 | a human's Resume to build on when it does. |
| 574 | + | |
| 575 | + 2026-08-08 (Work Graph self-planning + loop-back, FR-B1): `verify_fn` | |
| 576 | + is no longer `_combined_planner_verify_fn` -- each self-planned node | |
| 577 | + now carries its own `judgment` field (work_graph_plan.py's | |
| 578 | + `_PLANNER_NODE_CATALOG` suggests one per node type, the Planner can | |
| 579 | + reuse or override it), which `run_graph()` resolves itself per node. | |
| 580 | + `_combined_planner_verify_fn`/`_prd_consistency_verify_fn`/ | |
| 581 | + `_test_script_verify_fn` are NOT deleted -- `validate_implementation_ | |
| 582 | + with_own_graph()` (Validator's still-hardcoded manifest, unaffected by | |
| 583 | + this round) continues to use them (FR-A8). `mechanical_check_registry` | |
| 584 | + is still passed unconditionally: a self-planned node declaring | |
| 585 | + `judgment.kind == "mechanical"` (e.g. reusing the catalog's suggested | |
| 586 | + `check_script_compiles` for a test_script node) needs it registered, | |
| 587 | + same as before. | |
| 561 | 588 | """ |
| 562 | 589 | plan_result = work_graph_plan.plan_work_graph( |
| 563 | 590 | conn, "planner", task_id, goal_text, target_folder_path, timeout=timeout |
| ... | ... | @@ -571,7 +598,7 @@ def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): |
| 571 | 598 | work_graph_orchestrate.run_graph( |
| 572 | 599 | conn, run_id, "planner", "planner-doc-writing", target_folder_path, graph_manifest, |
| 573 | 600 | _planner_prompt_builder(goal_text, node_artifact_types), _DOC_INPUT_SCHEMA, _DOC_OUTPUT_SCHEMA, |
| 574 | - verify_fn=_combined_planner_verify_fn(node_artifact_types, target_folder_path), | |
| 601 | + mechanical_check_registry={"check_script_compiles": check_script_compiles}, | |
| 575 | 602 | ) |
| 576 | 603 | except Exception as e: |
| 577 | 604 | run_graph_error = e |
| ... | ... | @@ -792,10 +819,16 @@ def validate_implementation_with_own_graph( |
| 792 | 819 | if run_graph_error is not None: |
| 793 | 820 | raise run_graph_error |
| 794 | 821 | |
| 795 | - script_content = outputs["validator_test_script"]["content"] | |
| 796 | - script_execution = run_test_script_for_real( | |
| 797 | - script_content, target_folder_path, task_id, "validator_test_script", timeout=timeout, | |
| 798 | - ) | |
| 822 | + if "validator_test_script" in outputs: | |
| 823 | + script_content = outputs["validator_test_script"]["content"] | |
| 824 | + script_execution = run_test_script_for_real( | |
| 825 | + script_content, target_folder_path, task_id, "validator_test_script", timeout=timeout, | |
| 826 | + ) | |
| 827 | + else: | |
| 828 | + script_execution = { | |
| 829 | + "ran": False, "exit_code": None, "stdout": "", "stderr": "", | |
| 830 | + "note": "N/A: validator_test_script node is currently disabled (see _VALIDATOR_MANIFEST)", | |
| 831 | + } | |
| 799 | 832 | |
| 800 | 833 | response = org_node_trigger.trigger_org_node( |
| 801 | 834 | "validator", | ... | ... |
src/schema.py
| ... | ... | @@ -2,7 +2,8 @@ |
| 2 | 2 | |
| 3 | 3 | U-001 scope: the 15 platform entities from docs/sdd/01-M-模型设计.md §1.0-1.14 |
| 4 | 4 | (PH0-PH4). `deployment`/`deploy_health_signal` (§1.15/§1.16, added here in U-034, |
| 5 | -2026-08-02) are PH5-only -- 17 tables total as of PH5. | |
| 5 | +2026-08-02) are PH5-only. `work_graph_loop_limit_config` (§1.11.1, 2026-08-08, | |
| 6 | +Work Graph self-planning + loop-back) added post-PH5 -- 18 tables total. | |
| 6 | 7 | |
| 7 | 8 | Interpretive conventions used below, where 01-M leaves the physical type implicit: |
| 8 | 9 | - Booleans are stored as INTEGER CHECK(x IN (0, 1)) -- sqlite3 has no native bool type. |
| ... | ... | @@ -46,6 +47,7 @@ TABLE_NAMES = ( |
| 46 | 47 | "agent_profile", |
| 47 | 48 | "budget_guardrail_config", |
| 48 | 49 | "org_concurrency_config", |
| 50 | + "work_graph_loop_limit_config", | |
| 49 | 51 | "execution_event", |
| 50 | 52 | "execution_log_file", |
| 51 | 53 | "log_retention_config", |
| ... | ... | @@ -262,7 +264,7 @@ CREATE TABLE execution_event ( |
| 262 | 264 | event_type TEXT NOT NULL CHECK (event_type IN ( |
| 263 | 265 | 'task_status_changed', 'node_status_changed', 'artifact_created', |
| 264 | 266 | 'bug_recorded', 'loop_routed', 'budget_updated', 'skill_iterated', |
| 265 | - 'log_appended', 'quota_wait_started' | |
| 267 | + 'log_appended', 'quota_wait_started', 'loop_back_triggered' | |
| 266 | 268 | )), |
| 267 | 269 | payload TEXT NOT NULL, |
| 268 | 270 | created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP |
| ... | ... | @@ -271,6 +273,15 @@ CREATE TABLE execution_event ( |
| 271 | 273 | CREATE INDEX idx_execution_event_project_created ON execution_event(project_id, created_at); |
| 272 | 274 | CREATE INDEX idx_execution_event_task_created ON execution_event(task_id, created_at); |
| 273 | 275 | |
| 276 | +-- 1.11.1 work_graph_loop_limit_config (Work Graph self-planning + loop-back, | |
| 277 | +-- docs/todo/work-graph-self-planning-and-loop-back-design.md §6) | |
| 278 | +CREATE TABLE work_graph_loop_limit_config ( | |
| 279 | + config_id TEXT PRIMARY KEY, | |
| 280 | + max_loop_attempts INTEGER NOT NULL, | |
| 281 | + updated_by TEXT, | |
| 282 | + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP | |
| 283 | +); | |
| 284 | + | |
| 274 | 285 | -- 1.13 execution_log_file |
| 275 | 286 | CREATE TABLE execution_log_file ( |
| 276 | 287 | log_file_id TEXT PRIMARY KEY, |
| ... | ... | @@ -581,3 +592,57 @@ def migrate_relax_org_node_constraints(conn): |
| 581 | 592 | ) |
| 582 | 593 | finally: |
| 583 | 594 | conn.execute("PRAGMA foreign_keys = ON") |
| 595 | + | |
| 596 | + | |
| 597 | +def migrate_add_work_graph_loop_limit_config(conn): | |
| 598 | + """2026-08-08 (Work Graph self-planning + loop-back, | |
| 599 | + docs/todo/work-graph-self-planning-and-loop-back-design.md §6): adds | |
| 600 | + work_graph_loop_limit_config on an ALREADY-POPULATED database. Plain | |
| 601 | + `CREATE TABLE IF NOT EXISTS` -- a brand-new table, no CHECK constraint | |
| 602 | + to rebuild around, unlike migrate_expand_artifact_types. Idempotent and | |
| 603 | + safe to call unconditionally, including against a freshly | |
| 604 | + create_schema()'d database (which already has it).""" | |
| 605 | + conn.execute( | |
| 606 | + "CREATE TABLE IF NOT EXISTS work_graph_loop_limit_config (" | |
| 607 | + "config_id TEXT PRIMARY KEY, max_loop_attempts INTEGER NOT NULL, " | |
| 608 | + "updated_by TEXT, updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP)" | |
| 609 | + ) | |
| 610 | + conn.commit() | |
| 611 | + | |
| 612 | + | |
| 613 | +def migrate_add_loop_back_event_type(conn): | |
| 614 | + """2026-08-08 (Work Graph self-planning + loop-back, | |
| 615 | + docs/todo/work-graph-self-planning-and-loop-back-design.md §7): adds | |
| 616 | + 'loop_back_triggered' to execution_event.event_type's CHECK enum on an | |
| 617 | + ALREADY-POPULATED database -- same create-copy-drop-rename dance as | |
| 618 | + migrate_expand_artifact_types (SQLite has no ALTER-based constraint | |
| 619 | + migration). Idempotent and safe to call unconditionally, including | |
| 620 | + against a freshly create_schema()'d database (which already has the | |
| 621 | + expanded constraint).""" | |
| 622 | + row = conn.execute("SELECT sql FROM sqlite_master WHERE type='table' AND name='execution_event'").fetchone() | |
| 623 | + if row is None or "'loop_back_triggered'" in row[0]: | |
| 624 | + return | |
| 625 | + conn.executescript( | |
| 626 | + """ | |
| 627 | + CREATE TABLE execution_event_new ( | |
| 628 | + event_id TEXT PRIMARY KEY, | |
| 629 | + project_id TEXT NOT NULL REFERENCES project(project_id), | |
| 630 | + task_id TEXT REFERENCES task(task_id), | |
| 631 | + run_id TEXT REFERENCES work_graph_run(run_id), | |
| 632 | + node_exec_id TEXT REFERENCES work_node_execution(node_exec_id), | |
| 633 | + event_type TEXT NOT NULL CHECK (event_type IN ( | |
| 634 | + 'task_status_changed', 'node_status_changed', 'artifact_created', | |
| 635 | + 'bug_recorded', 'loop_routed', 'budget_updated', 'skill_iterated', | |
| 636 | + 'log_appended', 'quota_wait_started', 'loop_back_triggered' | |
| 637 | + )), | |
| 638 | + payload TEXT NOT NULL, | |
| 639 | + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP | |
| 640 | + ); | |
| 641 | + INSERT INTO execution_event_new SELECT * FROM execution_event; | |
| 642 | + DROP TABLE execution_event; | |
| 643 | + ALTER TABLE execution_event_new RENAME TO execution_event; | |
| 644 | + CREATE INDEX idx_execution_event_project_created ON execution_event(project_id, created_at); | |
| 645 | + CREATE INDEX idx_execution_event_task_created ON execution_event(task_id, created_at); | |
| 646 | + """ | |
| 647 | + ) | |
| 648 | + conn.commit() | ... | ... |
src/settings_config.py
| ... | ... | @@ -45,6 +45,44 @@ def upsert_concurrency_config(conn, max_concurrent_goals, updated_by, config_id= |
| 45 | 45 | return get_concurrency_config(conn, config_id) |
| 46 | 46 | |
| 47 | 47 | |
| 48 | +def get_loop_limit_config(conn, config_id): | |
| 49 | + """Work Graph self-planning + loop-back (docs/todo/work-graph-self- | |
| 50 | + planning-and-loop-back-design.md §6): no single 'default' config_id -- | |
| 51 | + callers look up "{org_node}:{node_type}" then "{org_node}:*" then | |
| 52 | + "default", falling back to a hardcoded constant if even "default" is | |
| 53 | + unseeded (same pattern scheduler.py already uses for | |
| 54 | + get_concurrency_config: `config["x"] if config else <constant>`).""" | |
| 55 | + row = conn.execute( | |
| 56 | + "SELECT config_id, max_loop_attempts, updated_at FROM work_graph_loop_limit_config WHERE config_id = ?", | |
| 57 | + (config_id,), | |
| 58 | + ).fetchone() | |
| 59 | + if row is None: | |
| 60 | + return None | |
| 61 | + return {"config_id": row[0], "max_loop_attempts": row[1], "updated_at": row[2]} | |
| 62 | + | |
| 63 | + | |
| 64 | +def upsert_loop_limit_config(conn, config_id, max_loop_attempts, updated_by): | |
| 65 | + if max_loop_attempts < 1: | |
| 66 | + raise InvalidParamError("max_loop_attempts must be >= 1, got {!r}".format(max_loop_attempts)) | |
| 67 | + | |
| 68 | + exists = conn.execute( | |
| 69 | + "SELECT 1 FROM work_graph_loop_limit_config WHERE config_id = ?", (config_id,) | |
| 70 | + ).fetchone() is not None | |
| 71 | + if exists: | |
| 72 | + conn.execute( | |
| 73 | + "UPDATE work_graph_loop_limit_config SET max_loop_attempts = ?, updated_by = ?, " | |
| 74 | + "updated_at = CURRENT_TIMESTAMP WHERE config_id = ?", | |
| 75 | + (max_loop_attempts, updated_by, config_id), | |
| 76 | + ) | |
| 77 | + else: | |
| 78 | + conn.execute( | |
| 79 | + "INSERT INTO work_graph_loop_limit_config (config_id, max_loop_attempts, updated_by) VALUES (?, ?, ?)", | |
| 80 | + (config_id, max_loop_attempts, updated_by), | |
| 81 | + ) | |
| 82 | + conn.commit() | |
| 83 | + return get_loop_limit_config(conn, config_id) | |
| 84 | + | |
| 85 | + | |
| 48 | 86 | def get_log_retention_config(conn, config_id="default"): |
| 49 | 87 | row = conn.execute( |
| 50 | 88 | "SELECT config_id, retention_days, max_total_size_mb, updated_at FROM log_retention_config " | ... | ... |
src/test_e2e_pipeline.py
| ... | ... | @@ -22,6 +22,7 @@ or work_graph_orchestrate's own mechanics (test_work_graph_orchestrate.py). |
| 22 | 22 | Run: python3 -m unittest src.test_e2e_pipeline -v |
| 23 | 23 | """ |
| 24 | 24 | |
| 25 | +import contextlib | |
| 25 | 26 | import json |
| 26 | 27 | import os |
| 27 | 28 | import subprocess |
| ... | ... | @@ -37,6 +38,13 @@ _FAKE_DOCS = { |
| 37 | 38 | "test_script": "assert True # fake but real, compilable script", |
| 38 | 39 | } |
| 39 | 40 | |
| 41 | +# test_script disabled 2026-08-07 (docs/todo/validator-test-script- | |
| 42 | +# narrative-output-issue.md) -- _PROMPT_TEMPLATE no longer asks for it, so | |
| 43 | +# the realistic fake planning response matches that. Tests that still need | |
| 44 | +# to exercise a test_script node (e.g. the partial-failure salvage | |
| 45 | +# mechanism, which is generic and doesn't care whether test_script is | |
| 46 | +# currently enabled by default) build their own local manifest instead of | |
| 47 | +# using this shared fixture. | |
| 40 | 48 | _FAKE_GRAPH_MANIFEST = { |
| 41 | 49 | "nodes": [ |
| 42 | 50 | {"node_id": "brd", "artifact_type": "brd", "depends_on": []}, |
| ... | ... | @@ -44,14 +52,44 @@ _FAKE_GRAPH_MANIFEST = { |
| 44 | 52 | {"node_id": "sdd", "artifact_type": "sdd", "depends_on": ["prd"]}, |
| 45 | 53 | {"node_id": "benchmark", "artifact_type": "benchmark", "depends_on": ["sdd"]}, |
| 46 | 54 | {"node_id": "test_dataset", "artifact_type": "test_dataset", "depends_on": ["benchmark"]}, |
| 47 | - {"node_id": "test_script", "artifact_type": "test_script", "depends_on": ["test_dataset"]}, | |
| 48 | 55 | ], |
| 49 | 56 | "edges": [ |
| 50 | 57 | {"from": "brd", "to": "prd", "edge_type": "needs_judgment"}, |
| 51 | 58 | {"from": "prd", "to": "sdd", "edge_type": "needs_judgment"}, |
| 52 | 59 | {"from": "sdd", "to": "benchmark", "edge_type": "needs_judgment"}, |
| 53 | 60 | {"from": "benchmark", "to": "test_dataset", "edge_type": "needs_judgment"}, |
| 61 | + ], | |
| 62 | +} | |
| 63 | + | |
| 64 | +# Only used by tests that need a test_script node present despite it being | |
| 65 | +# disabled by default (see _FAKE_GRAPH_MANIFEST's comment above). | |
| 66 | +_FAKE_GRAPH_MANIFEST_WITH_TEST_SCRIPT = { | |
| 67 | + "nodes": _FAKE_GRAPH_MANIFEST["nodes"] + [ | |
| 68 | + # depends_on both -- _DOC_PROMPTS["test_script"] needs | |
| 69 | + # {benchmark_content} directly, not just what survives into | |
| 70 | + # test_dataset_content. work_graph_plan._normalize_graph_manifest | |
| 71 | + # used to auto-inject this second dependency for the canonical | |
| 72 | + # chain's test_script node; that logic was removed along with | |
| 73 | + # test_script from _CANONICAL_DOC_CHAIN (2026-08-07), so this fixture | |
| 74 | + # sets it explicitly instead of relying on any auto-injection. | |
| 75 | + # 2026-08-08: `judgment` is required now -- plan_documents() no | |
| 76 | + # longer passes an external verify_fn at all (Work Graph | |
| 77 | + # self-planning + loop-back, FR-B1); a node only gets checked if it | |
| 78 | + # declares its own judgment, same as a real self-planned node would | |
| 79 | + # by reusing work_graph_plan._PLANNER_NODE_CATALOG's suggestion. | |
| 80 | + { | |
| 81 | + "node_id": "test_script", "artifact_type": "test_script", | |
| 82 | + "depends_on": ["test_dataset", "benchmark"], | |
| 83 | + "judgment": {"kind": "mechanical", "check_ref": "check_script_compiles"}, | |
| 84 | + # max_attempts=1 -- "1 retry allowed" (matches the legacy exactly- | |
| 85 | + # one-retry default): exactly 2 real executions before exceeding, | |
| 86 | + # matching this fixture's tests providing exactly 2 mock responses. | |
| 87 | + "on_fail": {"action": "retry_self", "max_attempts": 1}, | |
| 88 | + }, | |
| 89 | + ], | |
| 90 | + "edges": _FAKE_GRAPH_MANIFEST["edges"] + [ | |
| 54 | 91 | {"from": "test_dataset", "to": "test_script", "edge_type": "needs_judgment"}, |
| 92 | + {"from": "benchmark", "to": "test_script", "edge_type": "needs_judgment"}, | |
| 55 | 93 | ], |
| 56 | 94 | } |
| 57 | 95 | |
| ... | ... | @@ -61,6 +99,14 @@ def _fake_plan_subprocess_run(*args, **kwargs): |
| 61 | 99 | return subprocess.CompletedProcess(args=["claude"], returncode=0, stdout=json.dumps(payload), stderr="") |
| 62 | 100 | |
| 63 | 101 | |
| 102 | +def _fake_plan_subprocess_run_with_test_script(*args, **kwargs): | |
| 103 | + payload = { | |
| 104 | + "is_error": False, | |
| 105 | + "result": "```json\n" + json.dumps(_FAKE_GRAPH_MANIFEST_WITH_TEST_SCRIPT) + "\n```", | |
| 106 | + } | |
| 107 | + return subprocess.CompletedProcess(args=["claude"], returncode=0, stdout=json.dumps(payload), stderr="") | |
| 108 | + | |
| 109 | + | |
| 64 | 110 | def _fake_ready_trigger(org_node, prompt, target_folder_path, timeout=300): |
| 65 | 111 | assert prompt == e2e_pipeline._READY_PROMPT |
| 66 | 112 | return "Ready." |
| ... | ... | @@ -80,7 +126,7 @@ class PipelineTestBase(unittest.TestCase): |
| 80 | 126 | |
| 81 | 127 | |
| 82 | 128 | class PlanDocumentsTest(PipelineTestBase): |
| 83 | - def test_produces_six_artifacts_with_correct_lineage(self): | |
| 129 | + def test_produces_five_artifacts_with_correct_lineage(self): | |
| 84 | 130 | self.conn.execute( |
| 85 | 131 | "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " |
| 86 | 132 | "VALUES ('task-1', (SELECT project_id FROM project), '实现贪吃蛇游戏', 'goal', " |
| ... | ... | @@ -95,12 +141,12 @@ class PlanDocumentsTest(PipelineTestBase): |
| 95 | 141 | {"content": _FAKE_DOCS["brd"]}, {"content": _FAKE_DOCS["prd"]}, |
| 96 | 142 | {"consistent": True, "issues": []}, # PRD consistency check (REQ-02 boundary) |
| 97 | 143 | {"content": _FAKE_DOCS["sdd"]}, {"content": _FAKE_DOCS["benchmark"]}, |
| 98 | - {"content": _FAKE_DOCS["test_dataset"]}, {"content": _FAKE_DOCS["test_script"]}, | |
| 144 | + {"content": _FAKE_DOCS["test_dataset"]}, | |
| 99 | 145 | ] |
| 100 | 146 | plan_result = e2e_pipeline.plan_documents(self.conn, "task-1", "实现贪吃蛇游戏", self.tmp.name) |
| 101 | 147 | |
| 102 | 148 | artifact_ids = plan_result["artifact_ids"] |
| 103 | - self.assertEqual(set(artifact_ids), {"brd", "prd", "sdd", "benchmark", "test_dataset", "test_script"}) | |
| 149 | + self.assertEqual(set(artifact_ids), {"brd", "prd", "sdd", "benchmark", "test_dataset"}) | |
| 104 | 150 | prd_deps = json.loads( |
| 105 | 151 | self.conn.execute( |
| 106 | 152 | "SELECT depends_on_artifact_ids FROM artifact WHERE artifact_id = ?", (artifact_ids["prd"],) |
| ... | ... | @@ -127,6 +173,12 @@ class PlanDocumentsTest(PipelineTestBase): |
| 127 | 173 | # still propagate (failure semantics unchanged), but BRD/PRD/SDD/ |
| 128 | 174 | # Benchmark/TestDataset -- all genuinely produced and accepted -- |
| 129 | 175 | # must be persisted as artifacts, not silently discarded. |
| 176 | + # | |
| 177 | + # test_script itself is disabled by default as of the same date | |
| 178 | + # (docs/todo/validator-test-script-narrative-output-issue.md) -- this | |
| 179 | + # test uses _fake_plan_subprocess_run_with_test_script to keep | |
| 180 | + # exercising the salvage mechanism itself (generic, not specific to | |
| 181 | + # test_script) rather than losing this coverage entirely. | |
| 130 | 182 | self.conn.execute( |
| 131 | 183 | "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " |
| 132 | 184 | "VALUES ('task-1', (SELECT project_id FROM project), 'goal', 'goal', " |
| ... | ... | @@ -134,23 +186,23 @@ class PlanDocumentsTest(PipelineTestBase): |
| 134 | 186 | ) |
| 135 | 187 | self.conn.commit() |
| 136 | 188 | |
| 137 | - # _fake_plan_subprocess_run ignores its args and always returns a | |
| 138 | - # canned success payload -- fine for the "claude" planning call it's | |
| 139 | - # meant for, but `patch("src.work_graph_plan.subprocess.run", ...)` | |
| 140 | - # patches the ONE global `subprocess` module every other importer | |
| 141 | - # shares too (work_graph_plan does `import subprocess`, not | |
| 142 | - # `from subprocess import run`), so it would ALSO silently hijack | |
| 143 | - # check_script_compiles's real `py_compile`/`node --check` calls, | |
| 144 | - # making them always report success regardless of actual content -- | |
| 145 | - # exactly defeating the failure this test needs to trigger. Only | |
| 146 | - # intercept the "claude" planning call; let everything else | |
| 147 | - # (including this test's real compile checks) run for real. | |
| 189 | + # _fake_plan_subprocess_run_with_test_script ignores its args and | |
| 190 | + # always returns a canned success payload -- fine for the "claude" | |
| 191 | + # planning call it's meant for, but `patch("src.work_graph_plan. | |
| 192 | + # subprocess.run", ...)` patches the ONE global `subprocess` module | |
| 193 | + # every other importer shares too (work_graph_plan does `import | |
| 194 | + # subprocess`, not `from subprocess import run`), so it would ALSO | |
| 195 | + # silently hijack check_script_compiles's real `py_compile`/`node | |
| 196 | + # --check` calls, making them always report success regardless of | |
| 197 | + # actual content -- exactly defeating the failure this test needs to | |
| 198 | + # trigger. Only intercept the "claude" planning call; let everything | |
| 199 | + # else (including this test's real compile checks) run for real. | |
| 148 | 200 | real_subprocess_run = subprocess.run |
| 149 | 201 | |
| 150 | 202 | def _discriminating_fake(*args, **kwargs): |
| 151 | 203 | cmd = args[0] if args else kwargs.get("args") |
| 152 | 204 | if cmd and cmd[0] == "claude": |
| 153 | - return _fake_plan_subprocess_run(*args, **kwargs) | |
| 205 | + return _fake_plan_subprocess_run_with_test_script(*args, **kwargs) | |
| 154 | 206 | return real_subprocess_run(*args, **kwargs) |
| 155 | 207 | |
| 156 | 208 | with patch("src.work_graph_plan.subprocess.run", side_effect=_discriminating_fake), \ |
| ... | ... | @@ -184,20 +236,19 @@ class RunEndToEndTest(PipelineTestBase): |
| 184 | 236 | {"content": _FAKE_DOCS["brd"]}, {"content": _FAKE_DOCS["prd"]}, |
| 185 | 237 | {"consistent": True, "issues": []}, # PRD consistency check (REQ-02 boundary) |
| 186 | 238 | {"content": _FAKE_DOCS["sdd"]}, {"content": _FAKE_DOCS["benchmark"]}, |
| 187 | - {"content": _FAKE_DOCS["test_dataset"]}, {"content": _FAKE_DOCS["test_script"]}, | |
| 239 | + {"content": _FAKE_DOCS["test_dataset"]}, | |
| 188 | 240 | {"response": "Implemented. Ran `pytest`: 3 passed."}, |
| 189 | - # Validator's OWN independent benchmark/dataset/script (2026-08-06) -- | |
| 241 | + # Validator's OWN independent benchmark/dataset (2026-08-06) -- | |
| 190 | 242 | # deliberately different content from the Planner's, to also prove |
| 191 | 243 | # nothing here accidentally reuses the Planner's copies. |
| 192 | 244 | {"content": "# Validator's own Benchmark\nfake"}, |
| 193 | 245 | {"content": "N/A: validator judged no dataset needed either"}, |
| 194 | - {"content": "assert True # validator's own real, compilable script"}, | |
| 195 | 246 | ] |
| 196 | 247 | result = e2e_pipeline.run_end_to_end(self.conn, self.tmp.name, "实现贪吃蛇游戏") |
| 197 | 248 | |
| 198 | 249 | self.assertEqual(result["verdict"]["verdict"], "PASS") |
| 199 | 250 | self.assertEqual( |
| 200 | - set(result["doc_artifact_ids"]), {"brd", "prd", "sdd", "benchmark", "test_dataset", "test_script"} | |
| 251 | + set(result["doc_artifact_ids"]), {"brd", "prd", "sdd", "benchmark", "test_dataset"} | |
| 201 | 252 | ) |
| 202 | 253 | self.assertEqual(set(result["code_artifact_ids"]), {"code_diff", "test_report"}) |
| 203 | 254 | |
| ... | ... | @@ -209,9 +260,10 @@ class RunEndToEndTest(PipelineTestBase): |
| 209 | 260 | all_artifacts = self.conn.execute( |
| 210 | 261 | "SELECT COUNT(*) FROM artifact WHERE task_id = ?", (result["task_id"],) |
| 211 | 262 | ).fetchone()[0] |
| 212 | - # brd, prd, sdd, benchmark, test_dataset, test_script, code_diff, test_report, | |
| 213 | - # validator_benchmark, validator_test_dataset, validator_test_script | |
| 214 | - self.assertEqual(all_artifacts, 11) | |
| 263 | + # brd, prd, sdd, benchmark, test_dataset, code_diff, test_report, | |
| 264 | + # validator_benchmark, validator_test_dataset | |
| 265 | + # (test_script/validator_test_script disabled, see _VALIDATOR_MANIFEST) | |
| 266 | + self.assertEqual(all_artifacts, 9) | |
| 215 | 267 | |
| 216 | 268 | # planner, coder, AND validator (2026-08-06: validator now has its own |
| 217 | 269 | # real work_graph_run too, not a flat direct call) each got their own |
| ... | ... | @@ -338,6 +390,33 @@ class TestScriptVerifyFnTest(unittest.TestCase): |
| 338 | 390 | self.assertTrue(ok) |
| 339 | 391 | |
| 340 | 392 | |
| 393 | +class PlannerPromptBuilderTest(unittest.TestCase): | |
| 394 | + """2026-08-08: found via a real self-planned run whose SDD node had no | |
| 395 | + PRD node at all (a legitimate Work Graph self-planning choice, FR-B4 | |
| 396 | + "全部自由规划") -- _DOC_PROMPTS["sdd"] hardcodes a {prd_content} | |
| 397 | + placeholder left over from the old fixed chain, and plain .format() | |
| 398 | + raised a bare KeyError, landing the task on blocked_pending_human with | |
| 399 | + an unhelpful generic 'execution_error' reason instead of a real result.""" | |
| 400 | + | |
| 401 | + def test_missing_upstream_dependency_content_does_not_raise(self): | |
| 402 | + node_artifact_types = {"sdd": "sdd"} | |
| 403 | + build_prompt = e2e_pipeline._planner_prompt_builder("fix a bug", node_artifact_types) | |
| 404 | + # sdd's depends_on is empty -- no prd node was ever planned, so | |
| 405 | + # {prd_content} has nothing to fill it. Must not raise KeyError. | |
| 406 | + prompt = build_prompt({"node_id": "sdd", "depends_on": []}, {}) | |
| 407 | + self.assertIn("N/A (no corresponding upstream node was included in this Work Graph)", prompt) | |
| 408 | + self.assertIn("fix a bug", prompt) | |
| 409 | + | |
| 410 | + def test_present_upstream_dependency_content_is_used_normally(self): | |
| 411 | + node_artifact_types = {"prd": "prd", "sdd": "sdd"} | |
| 412 | + build_prompt = e2e_pipeline._planner_prompt_builder("goal", node_artifact_types) | |
| 413 | + prompt = build_prompt( | |
| 414 | + {"node_id": "sdd", "depends_on": ["prd"]}, {"prd": {"content": "real prd text"}} | |
| 415 | + ) | |
| 416 | + self.assertIn("real prd text", prompt) | |
| 417 | + self.assertNotIn("N/A (no corresponding", prompt) | |
| 418 | + | |
| 419 | + | |
| 341 | 420 | class CombinedPlannerVerifyFnTest(unittest.TestCase): |
| 342 | 421 | """Dispatches to the right per-node-type check -- both real checks |
| 343 | 422 | (PRD consistency and script-compiles) are exercised elsewhere in |
| ... | ... | @@ -494,6 +573,28 @@ class RunTestScriptForRealTest(unittest.TestCase): |
| 494 | 573 | self.assertTrue(os.path.exists(expected)) |
| 495 | 574 | |
| 496 | 575 | |
| 576 | +# validator_test_script disabled by default 2026-08-07 (see | |
| 577 | +# e2e_pipeline._VALIDATOR_MANIFEST's own comment) -- tests below that need | |
| 578 | +# to exercise the hard-gate/salvage mechanisms involving that node patch | |
| 579 | +# _VALIDATOR_MANIFEST back to this 3-node shape for their own duration, so | |
| 580 | +# that real code (still in the codebase, just not planned by default) stays | |
| 581 | +# covered instead of silently rotting. | |
| 582 | +_VALIDATOR_MANIFEST_WITH_TEST_SCRIPT = { | |
| 583 | + "nodes": [ | |
| 584 | + {"node_id": "validator_benchmark", "artifact_type": "validator_benchmark", "depends_on": []}, | |
| 585 | + { | |
| 586 | + "node_id": "validator_test_dataset", "artifact_type": "validator_test_dataset", | |
| 587 | + "depends_on": ["validator_benchmark"], | |
| 588 | + }, | |
| 589 | + { | |
| 590 | + "node_id": "validator_test_script", "artifact_type": "validator_test_script", | |
| 591 | + "depends_on": ["validator_test_dataset", "validator_benchmark"], | |
| 592 | + }, | |
| 593 | + ], | |
| 594 | + "edges": [], | |
| 595 | +} | |
| 596 | + | |
| 597 | + | |
| 497 | 598 | class ValidateImplementationWithOwnGraphTest(unittest.TestCase): |
| 498 | 599 | """2026-08-06: the safety-critical piece -- a real non-zero exit from |
| 499 | 600 | Validator's own script must force FAIL no matter what the LLM's verdict |
| ... | ... | @@ -515,21 +616,28 @@ class ValidateImplementationWithOwnGraphTest(unittest.TestCase): |
| 515 | 616 | def tearDown(self): |
| 516 | 617 | self.conn.close() |
| 517 | 618 | |
| 518 | - def _run(self, script_content, verdict_line): | |
| 519 | - with patch.object(work_graph_execute, "write_workflow_script"), \ | |
| 520 | - patch.object(work_graph_execute, "execute_work_graph") as mock_execute, \ | |
| 521 | - patch.object(org_node_trigger, "trigger_org_node", return_value=verdict_line): | |
| 522 | - mock_execute.side_effect = [ | |
| 523 | - {"content": "# Validator's own Benchmark"}, | |
| 524 | - {"content": "N/A: no dataset needed"}, | |
| 525 | - {"content": script_content}, | |
| 526 | - ] | |
| 619 | + def _run(self, script_content, verdict_line, with_test_script=False): | |
| 620 | + mock_responses = [ | |
| 621 | + {"content": "# Validator's own Benchmark"}, | |
| 622 | + {"content": "N/A: no dataset needed"}, | |
| 623 | + ] | |
| 624 | + if with_test_script: | |
| 625 | + mock_responses.append({"content": script_content}) | |
| 626 | + with contextlib.ExitStack() as stack: | |
| 627 | + stack.enter_context(patch.object(work_graph_execute, "write_workflow_script")) | |
| 628 | + mock_execute = stack.enter_context(patch.object(work_graph_execute, "execute_work_graph")) | |
| 629 | + stack.enter_context(patch.object(org_node_trigger, "trigger_org_node", return_value=verdict_line)) | |
| 630 | + if with_test_script: | |
| 631 | + stack.enter_context( | |
| 632 | + patch.object(e2e_pipeline, "_VALIDATOR_MANIFEST", _VALIDATOR_MANIFEST_WITH_TEST_SCRIPT) | |
| 633 | + ) | |
| 634 | + mock_execute.side_effect = mock_responses | |
| 527 | 635 | return e2e_pipeline.validate_implementation_with_own_graph( |
| 528 | 636 | self.conn, "task-1", "sdd content", "prd content", "goal text", self.tmp.name, |
| 529 | 637 | ) |
| 530 | 638 | |
| 531 | 639 | def test_llm_says_pass_and_script_really_passes_stays_pass(self): |
| 532 | - result = self._run("import sys\nsys.exit(0)\n", "VERDICT: PASS\nlooks good") | |
| 640 | + result = self._run("import sys\nsys.exit(0)\n", "VERDICT: PASS\nlooks good", with_test_script=True) | |
| 533 | 641 | self.assertEqual(result["verdict"], "PASS") |
| 534 | 642 | self.assertTrue(result["script_execution"]["ran"]) |
| 535 | 643 | self.assertEqual(result["script_execution"]["exit_code"], 0) |
| ... | ... | @@ -538,7 +646,9 @@ class ValidateImplementationWithOwnGraphTest(unittest.TestCase): |
| 538 | 646 | # the exact case the user's hard-gate rule exists for: a script |
| 539 | 647 | # that objectively fails must not be argued away by the LLM's own |
| 540 | 648 | # verdict line. |
| 541 | - result = self._run("import sys\nsys.exit(1)\n", "VERDICT: PASS\nlooks good to me") | |
| 649 | + result = self._run( | |
| 650 | + "import sys\nsys.exit(1)\n", "VERDICT: PASS\nlooks good to me", with_test_script=True | |
| 651 | + ) | |
| 542 | 652 | self.assertEqual(result["verdict"], "FAIL") |
| 543 | 653 | |
| 544 | 654 | def test_llm_says_fail_and_script_fails_stays_fail(self): |
| ... | ... | @@ -550,8 +660,21 @@ class ValidateImplementationWithOwnGraphTest(unittest.TestCase): |
| 550 | 660 | self.assertEqual(result["verdict"], "INCONCLUSIVE") |
| 551 | 661 | self.assertFalse(result["script_execution"]["ran"]) |
| 552 | 662 | |
| 553 | - def test_writes_three_validator_artifacts_with_lineage(self): | |
| 663 | + def test_writes_two_validator_artifacts_with_lineage(self): | |
| 664 | + # test_script disabled by default -- see _VALIDATOR_MANIFEST_WITH_ | |
| 665 | + # TEST_SCRIPT variant below for the 3-artifact/script-lineage case. | |
| 554 | 666 | result = self._run("import sys\nsys.exit(0)\n", "VERDICT: PASS\nlooks good") |
| 667 | + self.assertEqual(set(result["artifact_ids"]), {"validator_benchmark", "validator_test_dataset"}) | |
| 668 | + dataset_deps = json.loads( | |
| 669 | + self.conn.execute( | |
| 670 | + "SELECT depends_on_artifact_ids FROM artifact WHERE artifact_id = ?", | |
| 671 | + (result["artifact_ids"]["validator_test_dataset"],), | |
| 672 | + ).fetchone()[0] | |
| 673 | + ) | |
| 674 | + self.assertIn(result["artifact_ids"]["validator_benchmark"], dataset_deps) | |
| 675 | + | |
| 676 | + def test_writes_three_validator_artifacts_with_lineage_when_test_script_re_enabled(self): | |
| 677 | + result = self._run("import sys\nsys.exit(0)\n", "VERDICT: PASS\nlooks good", with_test_script=True) | |
| 555 | 678 | self.assertEqual( |
| 556 | 679 | set(result["artifact_ids"]), |
| 557 | 680 | {"validator_benchmark", "validator_test_dataset", "validator_test_script"}, |
| ... | ... | @@ -619,8 +742,12 @@ class ValidateImplementationWithOwnGraphTest(unittest.TestCase): |
| 619 | 742 | # -- ConsistencyCheckFailedError must still propagate, but |
| 620 | 743 | # validator_benchmark/validator_test_dataset (genuinely produced |
| 621 | 744 | # and accepted) must be persisted, not discarded along with it. |
| 745 | + # validator_test_script itself is disabled by default (see | |
| 746 | + # _VALIDATOR_MANIFEST_WITH_TEST_SCRIPT above) -- re-enabled here to | |
| 747 | + # keep the salvage mechanism (generic, not test_script-specific) covered. | |
| 622 | 748 | with patch.object(work_graph_execute, "write_workflow_script"), \ |
| 623 | - patch.object(work_graph_execute, "execute_work_graph") as mock_execute: | |
| 749 | + patch.object(work_graph_execute, "execute_work_graph") as mock_execute, \ | |
| 750 | + patch.object(e2e_pipeline, "_VALIDATOR_MANIFEST", _VALIDATOR_MANIFEST_WITH_TEST_SCRIPT): | |
| 624 | 751 | mock_execute.side_effect = [ |
| 625 | 752 | {"content": "# Validator's own Benchmark"}, |
| 626 | 753 | {"content": "N/A: no dataset needed"}, | ... | ... |
src/test_schema.py
| ... | ... | @@ -2,6 +2,8 @@ |
| 2 | 2 | Done criterion (unique / foreign key / enum) are actually enforced. |
| 3 | 3 | U-034 (PH5, 2026-08-02) added `deployment`/`deploy_health_signal` (17 tables |
| 4 | 4 | total) and widened `bug_record.root_cause_category` to accept `core_engine`. |
| 5 | +2026-08-08 added `work_graph_loop_limit_config` (18 tables total, Work | |
| 6 | +Graph self-planning + loop-back). | |
| 5 | 7 | |
| 6 | 8 | Run: python3 -m unittest src.test_schema -v |
| 7 | 9 | """ |
| ... | ... | @@ -21,13 +23,13 @@ class SchemaCreationTest(unittest.TestCase): |
| 21 | 23 | def tearDown(self): |
| 22 | 24 | self.conn.close() |
| 23 | 25 | |
| 24 | - def test_all_17_tables_created(self): | |
| 26 | + def test_all_18_tables_created(self): | |
| 25 | 27 | rows = self.conn.execute( |
| 26 | 28 | "SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT LIKE 'sqlite_%'" |
| 27 | 29 | ).fetchall() |
| 28 | 30 | actual = {row[0] for row in rows} |
| 29 | 31 | self.assertEqual(actual, set(schema.TABLE_NAMES)) |
| 30 | - self.assertEqual(len(schema.TABLE_NAMES), 17) | |
| 32 | + self.assertEqual(len(schema.TABLE_NAMES), 18) | |
| 31 | 33 | |
| 32 | 34 | def test_schema_is_idempotent_to_reopen_on_disk(self): |
| 33 | 35 | # not required by the Done criterion, but a schema that only works once |
| ... | ... | @@ -737,5 +739,115 @@ class MigrateRelaxOrgNodeConstraintsTest(unittest.TestCase): |
| 737 | 739 | fresh.close() |
| 738 | 740 | |
| 739 | 741 | |
| 742 | +class MigrateAddWorkGraphLoopLimitConfigTest(unittest.TestCase): | |
| 743 | + """2026-08-08: real upgrade path for an already-populated database that | |
| 744 | + predates work_graph_loop_limit_config (plain CREATE TABLE IF NOT | |
| 745 | + EXISTS, no CHECK constraint to rebuild around).""" | |
| 746 | + | |
| 747 | + def setUp(self): | |
| 748 | + self.conn = schema.connect(":memory:") | |
| 749 | + old_sql = schema.SCHEMA_SQL.replace( | |
| 750 | + "-- 1.11.1 work_graph_loop_limit_config (Work Graph self-planning + loop-back,\n" | |
| 751 | + "-- docs/todo/work-graph-self-planning-and-loop-back-design.md §6)\n" | |
| 752 | + "CREATE TABLE work_graph_loop_limit_config (\n" | |
| 753 | + " config_id TEXT PRIMARY KEY,\n" | |
| 754 | + " max_loop_attempts INTEGER NOT NULL,\n" | |
| 755 | + " updated_by TEXT,\n" | |
| 756 | + " updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP\n" | |
| 757 | + ");\n\n", | |
| 758 | + "", | |
| 759 | + ) | |
| 760 | + self.assertNotIn("work_graph_loop_limit_config", old_sql) # sanity: substitution matched | |
| 761 | + self.conn.executescript(old_sql) | |
| 762 | + self.conn.commit() | |
| 763 | + | |
| 764 | + def tearDown(self): | |
| 765 | + self.conn.close() | |
| 766 | + | |
| 767 | + def test_migration_creates_the_table(self): | |
| 768 | + schema.migrate_add_work_graph_loop_limit_config(self.conn) | |
| 769 | + self.conn.execute( | |
| 770 | + "INSERT INTO work_graph_loop_limit_config (config_id, max_loop_attempts, updated_by) " | |
| 771 | + "VALUES ('default', 3, 'test')" | |
| 772 | + ) | |
| 773 | + self.conn.commit() | |
| 774 | + row = self.conn.execute( | |
| 775 | + "SELECT max_loop_attempts FROM work_graph_loop_limit_config WHERE config_id = 'default'" | |
| 776 | + ).fetchone() | |
| 777 | + self.assertEqual(row[0], 3) | |
| 778 | + | |
| 779 | + def test_idempotent_on_a_database_already_migrated(self): | |
| 780 | + schema.migrate_add_work_graph_loop_limit_config(self.conn) | |
| 781 | + schema.migrate_add_work_graph_loop_limit_config(self.conn) # must not raise | |
| 782 | + | |
| 783 | + def test_noop_on_a_freshly_created_database(self): | |
| 784 | + fresh = schema.connect(":memory:") | |
| 785 | + try: | |
| 786 | + schema.create_schema(fresh) | |
| 787 | + schema.migrate_add_work_graph_loop_limit_config(fresh) # must not raise | |
| 788 | + finally: | |
| 789 | + fresh.close() | |
| 790 | + | |
| 791 | + | |
| 792 | +class MigrateAddLoopBackEventTypeTest(unittest.TestCase): | |
| 793 | + """2026-08-08: real upgrade path for an already-populated database whose | |
| 794 | + execution_event table still has the pre-expansion CHECK constraint.""" | |
| 795 | + | |
| 796 | + def setUp(self): | |
| 797 | + self.conn = schema.connect(":memory:") | |
| 798 | + old_sql = schema.SCHEMA_SQL.replace( | |
| 799 | + "'log_appended', 'quota_wait_started', 'loop_back_triggered'", | |
| 800 | + "'log_appended', 'quota_wait_started'", | |
| 801 | + ) | |
| 802 | + self.assertNotIn("loop_back_triggered", old_sql) # sanity: substitution matched | |
| 803 | + self.conn.executescript(old_sql) | |
| 804 | + self.conn.commit() | |
| 805 | + self.conn.execute( | |
| 806 | + "INSERT INTO project (project_id, name, target_folder_path, has_git_repo, status) " | |
| 807 | + "VALUES ('proj-1', 'P', '/tmp/proj-1', 1, 'active')" | |
| 808 | + ) | |
| 809 | + self.conn.execute( | |
| 810 | + "INSERT INTO execution_event (event_id, project_id, event_type, payload) " | |
| 811 | + "VALUES ('evt-1', 'proj-1', 'log_appended', '{}')" | |
| 812 | + ) | |
| 813 | + self.conn.commit() | |
| 814 | + | |
| 815 | + def tearDown(self): | |
| 816 | + self.conn.close() | |
| 817 | + | |
| 818 | + def test_old_check_constraint_rejects_new_type_before_migration(self): | |
| 819 | + with self.assertRaises(sqlite3.IntegrityError): | |
| 820 | + self.conn.execute( | |
| 821 | + "INSERT INTO execution_event (event_id, project_id, event_type, payload) " | |
| 822 | + "VALUES ('evt-2', 'proj-1', 'loop_back_triggered', '{}')" | |
| 823 | + ) | |
| 824 | + | |
| 825 | + def test_migration_preserves_existing_rows_and_accepts_new_type(self): | |
| 826 | + schema.migrate_add_loop_back_event_type(self.conn) | |
| 827 | + | |
| 828 | + row = self.conn.execute("SELECT event_type FROM execution_event WHERE event_id = 'evt-1'").fetchone() | |
| 829 | + self.assertEqual(row[0], "log_appended") | |
| 830 | + | |
| 831 | + self.conn.execute( | |
| 832 | + "INSERT INTO execution_event (event_id, project_id, event_type, payload) " | |
| 833 | + "VALUES ('evt-2', 'proj-1', 'loop_back_triggered', '{}')" | |
| 834 | + ) | |
| 835 | + self.conn.commit() | |
| 836 | + count = self.conn.execute("SELECT COUNT(*) FROM execution_event").fetchone()[0] | |
| 837 | + self.assertEqual(count, 2) | |
| 838 | + | |
| 839 | + def test_idempotent_on_a_database_already_migrated(self): | |
| 840 | + schema.migrate_add_loop_back_event_type(self.conn) | |
| 841 | + schema.migrate_add_loop_back_event_type(self.conn) # must not raise | |
| 842 | + | |
| 843 | + def test_noop_on_a_freshly_created_already_expanded_database(self): | |
| 844 | + fresh = schema.connect(":memory:") | |
| 845 | + try: | |
| 846 | + schema.create_schema(fresh) | |
| 847 | + schema.migrate_add_loop_back_event_type(fresh) # must not raise | |
| 848 | + finally: | |
| 849 | + fresh.close() | |
| 850 | + | |
| 851 | + | |
| 740 | 852 | if __name__ == "__main__": |
| 741 | 853 | unittest.main() | ... | ... |
src/test_settings_config.py
| ... | ... | @@ -49,6 +49,43 @@ class ConcurrencyConfigTest(SettingsConfigTestBase): |
| 49 | 49 | settings_config.upsert_concurrency_config(self.conn, 50, "admin") |
| 50 | 50 | |
| 51 | 51 | |
| 52 | +class LoopLimitConfigTest(SettingsConfigTestBase): | |
| 53 | + """2026-08-08: Work Graph self-planning + loop-back (docs/todo/ | |
| 54 | + work-graph-self-planning-and-loop-back-design.md §6). No implicit | |
| 55 | + 'default' config_id -- callers always pass one explicitly (they look up | |
| 56 | + "{org_node}:{node_type}", "{org_node}:*", "default" in turn).""" | |
| 57 | + | |
| 58 | + def test_get_returns_none_when_unconfigured(self): | |
| 59 | + self.assertIsNone(settings_config.get_loop_limit_config(self.conn, "planner:benchmark")) | |
| 60 | + | |
| 61 | + def test_creates_config_on_first_use(self): | |
| 62 | + result = settings_config.upsert_loop_limit_config(self.conn, "planner:benchmark", 5, "admin") | |
| 63 | + self.assertEqual(result["max_loop_attempts"], 5) | |
| 64 | + self.assertEqual( | |
| 65 | + settings_config.get_loop_limit_config(self.conn, "planner:benchmark")["max_loop_attempts"], 5 | |
| 66 | + ) | |
| 67 | + | |
| 68 | + def test_updates_existing_config(self): | |
| 69 | + settings_config.upsert_loop_limit_config(self.conn, "planner:benchmark", 5, "admin") | |
| 70 | + result = settings_config.upsert_loop_limit_config(self.conn, "planner:benchmark", 2, "admin2") | |
| 71 | + self.assertEqual(result["max_loop_attempts"], 2) | |
| 72 | + self.assertEqual( | |
| 73 | + self.conn.execute("SELECT COUNT(*) FROM work_graph_loop_limit_config").fetchone()[0], 1 | |
| 74 | + ) | |
| 75 | + | |
| 76 | + def test_different_config_ids_are_independent(self): | |
| 77 | + settings_config.upsert_loop_limit_config(self.conn, "planner:benchmark", 5, "admin") | |
| 78 | + settings_config.upsert_loop_limit_config(self.conn, "planner:*", 2, "admin") | |
| 79 | + self.assertEqual( | |
| 80 | + settings_config.get_loop_limit_config(self.conn, "planner:benchmark")["max_loop_attempts"], 5 | |
| 81 | + ) | |
| 82 | + self.assertEqual(settings_config.get_loop_limit_config(self.conn, "planner:*")["max_loop_attempts"], 2) | |
| 83 | + | |
| 84 | + def test_below_one_rejected(self): | |
| 85 | + with self.assertRaises(settings_config.InvalidParamError): | |
| 86 | + settings_config.upsert_loop_limit_config(self.conn, "default", 0, "admin") | |
| 87 | + | |
| 88 | + | |
| 52 | 89 | class LogRetentionConfigTest(SettingsConfigTestBase): |
| 53 | 90 | def test_get_returns_none_when_unconfigured(self): |
| 54 | 91 | self.assertIsNone(settings_config.get_log_retention_config(self.conn)) | ... | ... |
src/test_work_graph_orchestrate.py
| ... | ... | @@ -130,6 +130,78 @@ class TopologicalLayersTest(unittest.TestCase): |
| 130 | 130 | flat = [n["node_id"] for layer in layers for n in layer] |
| 131 | 131 | self.assertEqual([n["node_id"] for n in wgo.topological_order(manifest)], flat) |
| 132 | 132 | |
| 133 | + def test_on_failure_edge_does_not_create_a_spurious_cycle(self): | |
| 134 | + # 2026-08-08: an on_failure edge commonly points BACKWARD (from a | |
| 135 | + # node to one of its own upstream dependencies) -- the old | |
| 136 | + # unfiltered edge merge would have folded this in as a (reversed, | |
| 137 | + # incorrect) data dependency, creating a real cycle with the | |
| 138 | + # existing forward depends_on and making this raise ValueError. | |
| 139 | + manifest = { | |
| 140 | + "nodes": [ | |
| 141 | + {"node_id": "sdd", "depends_on": []}, | |
| 142 | + {"node_id": "benchmark", "depends_on": ["sdd"]}, | |
| 143 | + ], | |
| 144 | + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], | |
| 145 | + } | |
| 146 | + layers = wgo.topological_layers(manifest) | |
| 147 | + self.assertEqual([[n["node_id"] for n in layer] for layer in layers], [["sdd"], ["benchmark"]]) | |
| 148 | + | |
| 149 | + | |
| 150 | +class TransitiveDependentsTest(unittest.TestCase): | |
| 151 | + def test_includes_self_and_all_downstream(self): | |
| 152 | + manifest = { | |
| 153 | + "nodes": [ | |
| 154 | + {"node_id": "a", "depends_on": []}, | |
| 155 | + {"node_id": "b", "depends_on": ["a"]}, | |
| 156 | + {"node_id": "c", "depends_on": ["b"]}, | |
| 157 | + {"node_id": "d", "depends_on": []}, # unrelated branch | |
| 158 | + ], | |
| 159 | + "edges": [], | |
| 160 | + } | |
| 161 | + self.assertEqual(wgo._transitive_dependents(manifest, "a"), {"a", "b", "c"}) | |
| 162 | + self.assertEqual(wgo._transitive_dependents(manifest, "c"), {"c"}) | |
| 163 | + self.assertEqual(wgo._transitive_dependents(manifest, "d"), {"d"}) | |
| 164 | + | |
| 165 | + def test_ignores_on_failure_edges(self): | |
| 166 | + manifest = { | |
| 167 | + "nodes": [ | |
| 168 | + {"node_id": "sdd", "depends_on": []}, | |
| 169 | + {"node_id": "benchmark", "depends_on": ["sdd"]}, | |
| 170 | + ], | |
| 171 | + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], | |
| 172 | + } | |
| 173 | + # sdd's dependents must be {sdd, benchmark} via the real depends_on | |
| 174 | + # edge, not affected by the on_failure edge going the other way. | |
| 175 | + self.assertEqual(wgo._transitive_dependents(manifest, "sdd"), {"sdd", "benchmark"}) | |
| 176 | + | |
| 177 | + | |
| 178 | +class TransitiveAncestorsTest(unittest.TestCase): | |
| 179 | + def test_includes_self_and_all_upstream(self): | |
| 180 | + manifest = { | |
| 181 | + "nodes": [ | |
| 182 | + {"node_id": "a", "depends_on": []}, | |
| 183 | + {"node_id": "b", "depends_on": ["a"]}, | |
| 184 | + {"node_id": "c", "depends_on": ["b"]}, | |
| 185 | + ], | |
| 186 | + "edges": [], | |
| 187 | + } | |
| 188 | + self.assertEqual(wgo._transitive_ancestors(manifest, "c"), {"a", "b", "c"}) | |
| 189 | + self.assertEqual(wgo._transitive_ancestors(manifest, "a"), {"a"}) | |
| 190 | + | |
| 191 | + | |
| 192 | +class OnFailureTargetTest(unittest.TestCase): | |
| 193 | + def test_finds_the_on_failure_edge_target(self): | |
| 194 | + manifest = { | |
| 195 | + "nodes": [{"node_id": "sdd"}, {"node_id": "benchmark"}], | |
| 196 | + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], | |
| 197 | + } | |
| 198 | + self.assertEqual(wgo._on_failure_target(manifest, "benchmark"), "sdd") | |
| 199 | + | |
| 200 | + def test_missing_edge_raises(self): | |
| 201 | + manifest = {"nodes": [{"node_id": "benchmark"}], "edges": []} | |
| 202 | + with self.assertRaises(ValueError): | |
| 203 | + wgo._on_failure_target(manifest, "benchmark") | |
| 204 | + | |
| 133 | 205 | |
| 134 | 206 | class RunGraphTestBase(unittest.TestCase): |
| 135 | 207 | def setUp(self): |
| ... | ... | @@ -297,6 +369,54 @@ class RunGraphClassifierBlockedTest(RunGraphTestBase): |
| 297 | 369 | self.assertEqual(status, "blocked") |
| 298 | 370 | |
| 299 | 371 | |
| 372 | +class RunGraphUnanticipatedExceptionMarksRunBlockedTest(RunGraphTestBase): | |
| 373 | + """2026-08-08: found via a real self-planned run -- a bare exception | |
| 374 | + from build_prompt() (a caller bug, not one of the three specifically- | |
| 375 | + handled failure paths: loop_back limit exceeded, on_fail='hard_fail', | |
| 376 | + legacy _verify_and_maybe_regenerate double-failure) used to propagate | |
| 377 | + straight out of run_graph() leaving work_graph_run stuck at its | |
| 378 | + non-terminal status forever, permanently blocking every future retry | |
| 379 | + for this (task_id, org_node) via ux_work_graph_run_active's UNIQUE | |
| 380 | + index -- the real incident this test reproduces.""" | |
| 381 | + | |
| 382 | + def test_build_prompt_keyerror_still_marks_run_blocked(self): | |
| 383 | + manifest = {"nodes": [{"node_id": "sdd", "depends_on": []}], "edges": []} | |
| 384 | + | |
| 385 | + def bad_build_prompt(node, outputs): | |
| 386 | + raise KeyError("prd_content") | |
| 387 | + | |
| 388 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | |
| 389 | + patch.object(work_graph_execute, "execute_work_graph"): | |
| 390 | + with self.assertRaises(KeyError): | |
| 391 | + wgo.run_graph( | |
| 392 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | |
| 393 | + bad_build_prompt, _IN_SCHEMA, _OUT_SCHEMA, | |
| 394 | + ) | |
| 395 | + status = self.conn.execute("SELECT status FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] | |
| 396 | + self.assertEqual(status, "blocked") | |
| 397 | + | |
| 398 | + def test_already_terminal_status_is_left_alone(self): | |
| 399 | + # if some inner handler already set a more specific terminal | |
| 400 | + # status before the exception reaches run_graph()'s own catch-all, | |
| 401 | + # the catch-all's WHERE clause must be a no-op, not overwrite it. | |
| 402 | + self.conn.execute("UPDATE work_graph_run SET status = 'done' WHERE run_id = 'run-1'") | |
| 403 | + self.conn.commit() | |
| 404 | + manifest = {"nodes": [{"node_id": "sdd", "depends_on": []}], "edges": []} | |
| 405 | + | |
| 406 | + def bad_build_prompt(node, outputs): | |
| 407 | + raise RuntimeError("boom") | |
| 408 | + | |
| 409 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | |
| 410 | + patch.object(work_graph_execute, "execute_work_graph"): | |
| 411 | + with self.assertRaises(RuntimeError): | |
| 412 | + wgo.run_graph( | |
| 413 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | |
| 414 | + bad_build_prompt, _IN_SCHEMA, _OUT_SCHEMA, | |
| 415 | + ) | |
| 416 | + status = self.conn.execute("SELECT status FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] | |
| 417 | + self.assertEqual(status, "done") | |
| 418 | + | |
| 419 | + | |
| 300 | 420 | class VerifyFnTest(RunGraphTestBase): |
| 301 | 421 | """REQ-02 boundary: verify_fn intercepts a bad node output before it |
| 302 | 422 | propagates downstream, via one regeneration attempt.""" |
| ... | ... | @@ -722,5 +842,420 @@ class RunGraphRealSqliteConcurrencyTest(unittest.TestCase): |
| 722 | 842 | conn.close() |
| 723 | 843 | |
| 724 | 844 | |
| 845 | +class CoerceBoolTest(unittest.TestCase): | |
| 846 | + def test_real_bool_passes_through(self): | |
| 847 | + self.assertTrue(wgo._coerce_bool(True)) | |
| 848 | + self.assertFalse(wgo._coerce_bool(False)) | |
| 849 | + | |
| 850 | + def test_string_false_is_not_truthy(self): | |
| 851 | + # regression: a real call has returned "false" as a JSON STRING | |
| 852 | + # despite "type": "boolean" in the schema -- `if "false":` is | |
| 853 | + # truthy in Python (same class of bug e2e_pipeline._coerce_bool | |
| 854 | + # was written for). | |
| 855 | + self.assertFalse(wgo._coerce_bool("false")) | |
| 856 | + | |
| 857 | + def test_string_true_is_treated_as_true(self): | |
| 858 | + self.assertTrue(wgo._coerce_bool("true")) | |
| 859 | + | |
| 860 | + | |
| 861 | +class ResolveNodeVerifyFnTest(unittest.TestCase): | |
| 862 | + """2026-08-08: node-declared `judgment` takes over verify_fn | |
| 863 | + resolution; a node with no `judgment` falls through to whatever the | |
| 864 | + caller passed as `verify_fn` (FR-A8 -- every pre-existing graph_manifest | |
| 865 | + has no `judgment` field at all).""" | |
| 866 | + | |
| 867 | + def test_no_judgment_falls_through_to_external_verify_fn(self): | |
| 868 | + external = lambda node, output: (True, None) | |
| 869 | + resolved = wgo._resolve_node_verify_fn({"node_id": "x"}, external, "planner", "/repo", None) | |
| 870 | + self.assertIs(resolved, external) | |
| 871 | + | |
| 872 | + def test_judgment_kind_none_falls_through_to_external_verify_fn(self): | |
| 873 | + external = lambda node, output: (True, None) | |
| 874 | + node = {"node_id": "x", "judgment": {"kind": "none"}} | |
| 875 | + resolved = wgo._resolve_node_verify_fn(node, external, "planner", "/repo", None) | |
| 876 | + self.assertIs(resolved, external) | |
| 877 | + | |
| 878 | + def test_no_judgment_and_no_external_verify_fn_returns_none(self): | |
| 879 | + resolved = wgo._resolve_node_verify_fn({"node_id": "x"}, None, "planner", "/repo", None) | |
| 880 | + self.assertIsNone(resolved) | |
| 881 | + | |
| 882 | + def test_mechanical_judgment_dispatches_to_registered_check(self): | |
| 883 | + node = {"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "my_check"}} | |
| 884 | + registry = {"my_check": lambda content: (content == "good", None if content == "good" else "bad content")} | |
| 885 | + resolved = wgo._resolve_node_verify_fn(node, None, "planner", "/repo", registry) | |
| 886 | + ok, feedback = resolved(node, {"content": "good"}) | |
| 887 | + self.assertTrue(ok) | |
| 888 | + ok, feedback = resolved(node, {"content": "bad"}) | |
| 889 | + self.assertFalse(ok) | |
| 890 | + self.assertEqual(feedback, ["bad content"]) | |
| 891 | + | |
| 892 | + def test_mechanical_judgment_unknown_check_ref_raises(self): | |
| 893 | + node = {"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "typo"}} | |
| 894 | + with self.assertRaises(ValueError): | |
| 895 | + wgo._resolve_node_verify_fn(node, None, "planner", "/repo", {"real_check": lambda c: (True, None)}) | |
| 896 | + | |
| 897 | + def test_unknown_judgment_kind_raises(self): | |
| 898 | + node = {"node_id": "x", "judgment": {"kind": "astrology"}} | |
| 899 | + with self.assertRaises(ValueError): | |
| 900 | + wgo._resolve_node_verify_fn(node, None, "planner", "/repo", None) | |
| 901 | + | |
| 902 | + | |
| 903 | +class ValidateMechanicalCheckRefsTest(unittest.TestCase): | |
| 904 | + """Fail before running any node, not when the offending node is reached.""" | |
| 905 | + | |
| 906 | + def test_unregistered_check_ref_raises(self): | |
| 907 | + manifest = { | |
| 908 | + "nodes": [{"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "typo"}}], | |
| 909 | + "edges": [], | |
| 910 | + } | |
| 911 | + with self.assertRaises(ValueError): | |
| 912 | + wgo._validate_mechanical_check_refs(manifest, {"real_check": lambda c: (True, None)}) | |
| 913 | + | |
| 914 | + def test_registered_check_ref_passes(self): | |
| 915 | + manifest = { | |
| 916 | + "nodes": [{"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "real_check"}}], | |
| 917 | + "edges": [], | |
| 918 | + } | |
| 919 | + wgo._validate_mechanical_check_refs(manifest, {"real_check": lambda c: (True, None)}) # must not raise | |
| 920 | + | |
| 921 | + def test_no_judgment_nodes_pass_with_no_registry(self): | |
| 922 | + manifest = {"nodes": [{"node_id": "brd"}], "edges": []} | |
| 923 | + wgo._validate_mechanical_check_refs(manifest, None) # must not raise | |
| 924 | + | |
| 925 | + | |
| 926 | +class LlmJudgmentVerifyFnTest(unittest.TestCase): | |
| 927 | + def test_non_self_target_calls_directly_without_worktree(self): | |
| 928 | + judgment = {"judge_prompt": "Review: {content}"} | |
| 929 | + verify_fn = wgo._llm_judgment_verify_fn(judgment, "planner", "/repo") | |
| 930 | + with patch.object(wgo.paths, "is_self_target", return_value=False), \ | |
| 931 | + patch.object(work_graph_execute, "write_workflow_script") as mock_write, \ | |
| 932 | + patch.object( | |
| 933 | + work_graph_execute, "execute_work_graph", | |
| 934 | + return_value={"ok": True, "issues": []}, | |
| 935 | + ) as mock_execute: | |
| 936 | + ok, issues = verify_fn({"node_id": "x"}, {"content": "some content"}) | |
| 937 | + self.assertTrue(ok) | |
| 938 | + self.assertEqual(issues, []) | |
| 939 | + self.assertEqual(mock_write.call_args[0][0], "/repo") | |
| 940 | + self.assertEqual(mock_execute.call_args[0][0], "/repo") | |
| 941 | + | |
| 942 | + def test_string_ok_is_coerced(self): | |
| 943 | + judgment = {"judge_prompt": "Review: {content}"} | |
| 944 | + verify_fn = wgo._llm_judgment_verify_fn(judgment, "planner", "/repo") | |
| 945 | + with patch.object(wgo.paths, "is_self_target", return_value=False), \ | |
| 946 | + patch.object(work_graph_execute, "write_workflow_script"), \ | |
| 947 | + patch.object( | |
| 948 | + work_graph_execute, "execute_work_graph", | |
| 949 | + return_value={"ok": "false", "issues": ["bad"]}, | |
| 950 | + ): | |
| 951 | + ok, issues = verify_fn({"node_id": "x"}, {"content": "some content"}) | |
| 952 | + self.assertFalse(ok) | |
| 953 | + self.assertEqual(issues, ["bad"]) | |
| 954 | + | |
| 955 | + def test_self_target_isolates_in_its_own_worktree(self): | |
| 956 | + # same pattern as e2e_pipeline.py's PrdConsistencySelfTargetWorktreeTest | |
| 957 | + judgment = {"judge_prompt": "Review: {content}"} | |
| 958 | + verify_fn = wgo._llm_judgment_verify_fn(judgment, "planner", "/repo") | |
| 959 | + with patch.object(wgo.paths, "is_self_target", return_value=True), \ | |
| 960 | + patch.object(wgo.work_node_run, "create_worktree", return_value="/repo/.graphlemon-worktrees/x") as mock_create, \ | |
| 961 | + patch.object(wgo.work_node_run, "ensure_worktree_committed") as mock_ensure, \ | |
| 962 | + patch.object(wgo.work_node_run, "merge_worktree") as mock_merge, \ | |
| 963 | + patch.object(wgo.work_node_run, "remove_worktree") as mock_remove, \ | |
| 964 | + patch.object(work_graph_execute, "write_workflow_script") as mock_write, \ | |
| 965 | + patch.object( | |
| 966 | + work_graph_execute, "execute_work_graph", | |
| 967 | + return_value={"ok": True, "issues": []}, | |
| 968 | + ) as mock_execute: | |
| 969 | + ok, issues = verify_fn({"node_id": "x"}, {"content": "some content"}) | |
| 970 | + self.assertTrue(ok) | |
| 971 | + mock_create.assert_called_once_with("/repo", unittest.mock.ANY) | |
| 972 | + self.assertEqual(mock_write.call_args[0][0], "/repo/.graphlemon-worktrees/x") | |
| 973 | + self.assertEqual(mock_execute.call_args[0][0], "/repo/.graphlemon-worktrees/x") | |
| 974 | + self.assertEqual(mock_ensure.call_count, 2) | |
| 975 | + mock_merge.assert_called_once() | |
| 976 | + mock_remove.assert_called_once() | |
| 977 | + | |
| 978 | + | |
| 979 | +class RunGraphMechanicalJudgmentTest(RunGraphTestBase): | |
| 980 | + """End-to-end: a node's own judgment field (not an externally-passed | |
| 981 | + verify_fn) drives verification through run_graph().""" | |
| 982 | + | |
| 983 | + def test_node_declared_mechanical_judgment_regenerates_on_failure(self): | |
| 984 | + manifest = { | |
| 985 | + "nodes": [{"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "compiles"}}], | |
| 986 | + "edges": [], | |
| 987 | + } | |
| 988 | + registry = {"compiles": lambda content: (content == "real code", None if content == "real code" else "not code")} | |
| 989 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | |
| 990 | + patch.object(work_graph_execute, "execute_work_graph") as mock_execute: | |
| 991 | + mock_execute.side_effect = [{"content": "narrative text"}, {"content": "real code"}] | |
| 992 | + outputs = wgo.run_graph( | |
| 993 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | |
| 994 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, | |
| 995 | + mechanical_check_registry=registry, | |
| 996 | + ) | |
| 997 | + self.assertEqual(outputs["script"], {"content": "real code"}) | |
| 998 | + | |
| 999 | + def test_unregistered_check_ref_fails_before_any_node_runs(self): | |
| 1000 | + manifest = { | |
| 1001 | + "nodes": [{"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "typo"}}], | |
| 1002 | + "edges": [], | |
| 1003 | + } | |
| 1004 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | |
| 1005 | + patch.object(work_graph_execute, "execute_work_graph") as mock_execute: | |
| 1006 | + with self.assertRaises(ValueError): | |
| 1007 | + wgo.run_graph( | |
| 1008 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | |
| 1009 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, | |
| 1010 | + mechanical_check_registry={"compiles": lambda c: (True, None)}, | |
| 1011 | + ) | |
| 1012 | + mock_execute.assert_not_called() | |
| 1013 | + | |
| 1014 | + | |
| 1015 | +class RunGraphLoopBackTest(RunGraphTestBase): | |
| 1016 | + """2026-08-08, Work Graph self-planning + loop-back design §4.""" | |
| 1017 | + | |
| 1018 | + def test_retry_self_regenerates_with_accumulating_history(self): | |
| 1019 | + # A node with on_fail.action='retry_self' loops back to itself -- | |
| 1020 | + # fails twice, succeeds the third time. Each retry's prompt must | |
| 1021 | + # include the FULL history of prior rejections, not just the last one. | |
| 1022 | + manifest = { | |
| 1023 | + "nodes": [{ | |
| 1024 | + "node_id": "benchmark", | |
| 1025 | + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, | |
| 1026 | + "on_fail": {"action": "retry_self", "max_attempts": 5}, | |
| 1027 | + }], | |
| 1028 | + "edges": [], | |
| 1029 | + } | |
| 1030 | + contents = ["bad v1", "bad v2", "good"] | |
| 1031 | + prompts_seen = [] | |
| 1032 | + | |
| 1033 | + def build_prompt(node, outputs): | |
| 1034 | + return "base prompt" | |
| 1035 | + | |
| 1036 | + def fake_execute(cwd, name, args, output_schema, timeout=600): | |
| 1037 | + return {"content": contents.pop(0)} | |
| 1038 | + | |
| 1039 | + registry = {"compiles": lambda content: (content == "good", "not real code: " + content)} | |
| 1040 | + | |
| 1041 | + with patch.object(work_graph_execute, "write_workflow_script") as mock_write, \ | |
| 1042 | + patch.object(work_graph_execute, "execute_work_graph", side_effect=fake_execute): | |
| 1043 | + outputs = wgo.run_graph( | |
| 1044 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | |
| 1045 | + build_prompt, _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, | |
| 1046 | + ) | |
| 1047 | + self.assertEqual(outputs["benchmark"], {"content": "good"}) | |
| 1048 | + | |
| 1049 | + # 3 real executions happened (2 failures + 1 success) -- 3 distinct | |
| 1050 | + # work_node_execution rows, not 1 (which would mean a stale 'done' | |
| 1051 | + # row got silently reused instead of genuinely re-running). | |
| 1052 | + rows = self.conn.execute( | |
| 1053 | + "SELECT node_id FROM work_node_execution WHERE run_id = 'run-1' ORDER BY node_id" | |
| 1054 | + ).fetchall() | |
| 1055 | + self.assertEqual([r[0] for r in rows], ["benchmark", "benchmark-attempt2", "benchmark-attempt3"]) | |
| 1056 | + | |
| 1057 | + events = self.conn.execute( | |
| 1058 | + "SELECT payload FROM execution_event WHERE event_type = 'loop_back_triggered' ORDER BY created_at" | |
| 1059 | + ).fetchall() | |
| 1060 | + self.assertEqual(len(events), 2) | |
| 1061 | + payloads = [json.loads(p) for (p,) in events] | |
| 1062 | + self.assertEqual(payloads[0]["attempt_number"], 1) | |
| 1063 | + self.assertEqual(payloads[1]["attempt_number"], 2) | |
| 1064 | + | |
| 1065 | + # the THIRD (successful) execution's prompt must carry both prior | |
| 1066 | + # rejections' feedback, not just the most recent one. | |
| 1067 | + third_script_prompt = json.loads( | |
| 1068 | + self.conn.execute( | |
| 1069 | + "SELECT input_payload FROM work_node_execution WHERE node_id = 'benchmark-attempt3'" | |
| 1070 | + ).fetchone()[0] | |
| 1071 | + ) | |
| 1072 | + # the real prompt text lives in the rendered workflow script, not | |
| 1073 | + # input_payload (that's just {"node_id": ...}) -- inspect what got | |
| 1074 | + # passed to write_workflow_script for the third attempt instead. | |
| 1075 | + third_call_script = mock_write.call_args_list[2].args[2] | |
| 1076 | + self.assertIn("not real code: bad v1", third_call_script) | |
| 1077 | + self.assertIn("not real code: bad v2", third_call_script) | |
| 1078 | + | |
| 1079 | + def test_loop_back_to_earlier_node_reruns_target_and_downstream(self): | |
| 1080 | + # benchmark declares on_fail.action='loop_back' with an on_failure | |
| 1081 | + # edge back to sdd -- sdd must be genuinely re-executed (not | |
| 1082 | + # short-circuited to its stale 'done' output), and benchmark's | |
| 1083 | + # second attempt must see the NEW sdd output. | |
| 1084 | + manifest = { | |
| 1085 | + "nodes": [ | |
| 1086 | + {"node_id": "sdd", "depends_on": []}, | |
| 1087 | + { | |
| 1088 | + "node_id": "benchmark", "depends_on": ["sdd"], | |
| 1089 | + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, | |
| 1090 | + "on_fail": {"action": "loop_back", "max_attempts": 3}, | |
| 1091 | + }, | |
| 1092 | + ], | |
| 1093 | + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], | |
| 1094 | + } | |
| 1095 | + sdd_outputs = ["sdd v1", "sdd v2 (fixed)"] | |
| 1096 | + benchmark_seen_sdd = [] | |
| 1097 | + | |
| 1098 | + def build_prompt(node, outputs): | |
| 1099 | + if node["node_id"] == "benchmark": | |
| 1100 | + benchmark_seen_sdd.append(outputs["sdd"]["content"]) | |
| 1101 | + return "benchmark prompt using " + outputs["sdd"]["content"] | |
| 1102 | + return "sdd prompt" | |
| 1103 | + | |
| 1104 | + # sdd runs before benchmark in each pass (topological order), so | |
| 1105 | + # call order alone deterministically identifies which node this is: | |
| 1106 | + # sdd v1, bad benchmark, sdd v2 (re-run via loop_back), good benchmark. | |
| 1107 | + call_order = [] | |
| 1108 | + | |
| 1109 | + def side_effect(cwd, name, args, output_schema, timeout=600): | |
| 1110 | + call_order.append(name) | |
| 1111 | + call_index = len(call_order) | |
| 1112 | + if call_index == 1: | |
| 1113 | + return {"content": sdd_outputs[0]} | |
| 1114 | + if call_index == 2: | |
| 1115 | + return {"content": "bad benchmark"} | |
| 1116 | + if call_index == 3: | |
| 1117 | + return {"content": sdd_outputs[1]} | |
| 1118 | + return {"content": "good benchmark"} | |
| 1119 | + | |
| 1120 | + registry = {"compiles": lambda content: (content == "good benchmark", "rejected: " + content)} | |
| 1121 | + | |
| 1122 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | |
| 1123 | + patch.object(work_graph_execute, "execute_work_graph", side_effect=side_effect): | |
| 1124 | + outputs = wgo.run_graph( | |
| 1125 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | |
| 1126 | + build_prompt, _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, | |
| 1127 | + ) | |
| 1128 | + | |
| 1129 | + self.assertEqual(outputs["sdd"], {"content": sdd_outputs[1]}) | |
| 1130 | + self.assertEqual(outputs["benchmark"], {"content": "good benchmark"}) | |
| 1131 | + self.assertEqual(benchmark_seen_sdd, [sdd_outputs[0], sdd_outputs[1]]) | |
| 1132 | + | |
| 1133 | + sdd_rows = self.conn.execute( | |
| 1134 | + "SELECT node_id FROM work_node_execution WHERE run_id = 'run-1' AND " | |
| 1135 | + "(node_id = 'sdd' OR node_id LIKE 'sdd-attempt%') ORDER BY node_id" | |
| 1136 | + ).fetchall() | |
| 1137 | + self.assertEqual([r[0] for r in sdd_rows], ["sdd", "sdd-attempt2"]) | |
| 1138 | + | |
| 1139 | + def test_exceeding_limit_raises_and_blocks_run(self): | |
| 1140 | + manifest = { | |
| 1141 | + "nodes": [{ | |
| 1142 | + "node_id": "benchmark", | |
| 1143 | + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, | |
| 1144 | + "on_fail": {"action": "retry_self", "max_attempts": 2}, | |
| 1145 | + }], | |
| 1146 | + "edges": [], | |
| 1147 | + } | |
| 1148 | + registry = {"compiles": lambda content: (False, "always rejected")} | |
| 1149 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | |
| 1150 | + patch.object(work_graph_execute, "execute_work_graph", return_value={"content": "always bad"}): | |
| 1151 | + with self.assertRaises(wgo.ConsistencyCheckFailedError): | |
| 1152 | + wgo.run_graph( | |
| 1153 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | |
| 1154 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, | |
| 1155 | + ) | |
| 1156 | + status = self.conn.execute("SELECT status FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] | |
| 1157 | + self.assertEqual(status, "blocked") | |
| 1158 | + | |
| 1159 | + def test_hard_fail_raises_immediately_with_no_retry(self): | |
| 1160 | + manifest = { | |
| 1161 | + "nodes": [{ | |
| 1162 | + "node_id": "benchmark", | |
| 1163 | + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, | |
| 1164 | + "on_fail": {"action": "hard_fail"}, | |
| 1165 | + }], | |
| 1166 | + "edges": [], | |
| 1167 | + } | |
| 1168 | + registry = {"compiles": lambda content: (False, "bad")} | |
| 1169 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | |
| 1170 | + patch.object(work_graph_execute, "execute_work_graph", return_value={"content": "x"}) as mock_execute: | |
| 1171 | + with self.assertRaises(wgo.ConsistencyCheckFailedError): | |
| 1172 | + wgo.run_graph( | |
| 1173 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | |
| 1174 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, | |
| 1175 | + ) | |
| 1176 | + self.assertEqual(mock_execute.call_count, 1) # no retry at all | |
| 1177 | + | |
| 1178 | + def test_resume_after_restart_continues_from_the_real_attempt_count(self): | |
| 1179 | + # Simulates a PRIOR process that ran attempt 1 (rejected), recorded | |
| 1180 | + # the loop_back event, then died before attempt 2 ever began -- | |
| 1181 | + # seeded directly via SQL rather than literally raising mid-flight | |
| 1182 | + # inside execute_work_graph, which would instead exercise a | |
| 1183 | + # separate, pre-existing gap in work_node_run.py's own resume | |
| 1184 | + # logic (it unconditionally INSERTs a new execution_log_file row | |
| 1185 | + # every call, with no existing-row check, so resuming a node_exec_id | |
| 1186 | + # that already has one hits a real UNIQUE constraint violation -- | |
| 1187 | + # a genuine bug, but in a different module and out of scope here). | |
| 1188 | + manifest = { | |
| 1189 | + "nodes": [{ | |
| 1190 | + "node_id": "benchmark", | |
| 1191 | + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, | |
| 1192 | + "on_fail": {"action": "retry_self", "max_attempts": 5}, | |
| 1193 | + }], | |
| 1194 | + "edges": [], | |
| 1195 | + } | |
| 1196 | + registry = {"compiles": lambda content: (content == "good", "bad: " + content)} | |
| 1197 | + | |
| 1198 | + wgo.ensure_placeholder_skill(self.conn, "planner-doc-writing", "planner") | |
| 1199 | + self.conn.execute( | |
| 1200 | + "INSERT INTO work_node_execution (node_exec_id, run_id, node_id, subagent_type, skill_ref, " | |
| 1201 | + "input_schema_ref, output_schema_ref, input_payload, output_payload, status) " | |
| 1202 | + "VALUES ('exec-1', 'run-1', 'benchmark', 'planner', 'planner-doc-writing', " | |
| 1203 | + "'work_graph_node_input.schema.json', 'work_graph_node_output.schema.json', " | |
| 1204 | + "'{}', '{\"content\": \"bad v1\"}', 'done')" | |
| 1205 | + ) | |
| 1206 | + self.conn.execute( | |
| 1207 | + "INSERT INTO execution_event (event_id, project_id, task_id, run_id, event_type, payload) " | |
| 1208 | + "VALUES ('evt-1', 'proj-1', 'task-1', 'run-1', 'loop_back_triggered', ?)", | |
| 1209 | + (json.dumps({ | |
| 1210 | + "source_node_id": "benchmark", "target_node_id": "benchmark", | |
| 1211 | + "attempt_number": 1, "feedback": ["bad: bad v1"], | |
| 1212 | + }),), | |
| 1213 | + ) | |
| 1214 | + self.conn.commit() | |
| 1215 | + | |
| 1216 | + # "restart": a fresh run_graph() call for the same run_id, this | |
| 1217 | + # time succeeding -- must resume as attempt 2 (reading the true | |
| 1218 | + # count from execution_event), not restart from attempt 1 or lose | |
| 1219 | + # the earlier rejection's history. | |
| 1220 | + with patch.object(work_graph_execute, "write_workflow_script") as mock_write, \ | |
| 1221 | + patch.object(work_graph_execute, "execute_work_graph", return_value={"content": "good"}): | |
| 1222 | + outputs = wgo.run_graph( | |
| 1223 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | |
| 1224 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, | |
| 1225 | + ) | |
| 1226 | + self.assertEqual(outputs["benchmark"], {"content": "good"}) | |
| 1227 | + rows = self.conn.execute( | |
| 1228 | + "SELECT node_id FROM work_node_execution WHERE run_id = 'run-1' ORDER BY node_id" | |
| 1229 | + ).fetchall() | |
| 1230 | + self.assertEqual([r[0] for r in rows], ["benchmark", "benchmark-attempt2"]) | |
| 1231 | + self.assertIn("bad: bad v1", mock_write.call_args[0][2]) | |
| 1232 | + | |
| 1233 | + def test_org_node_type_scoped_config_limit_used_when_no_node_level_override(self): | |
| 1234 | + from src import settings_config | |
| 1235 | + settings_config.upsert_loop_limit_config(self.conn, "planner:benchmark", 1, "test") | |
| 1236 | + manifest = { | |
| 1237 | + "nodes": [{ | |
| 1238 | + "node_id": "benchmark", "artifact_type": "benchmark", | |
| 1239 | + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, | |
| 1240 | + "on_fail": {"action": "retry_self"}, # no max_attempts -- falls back to config | |
| 1241 | + }], | |
| 1242 | + "edges": [], | |
| 1243 | + } | |
| 1244 | + registry = {"compiles": lambda content: (False, "bad")} | |
| 1245 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | |
| 1246 | + patch.object(work_graph_execute, "execute_work_graph", return_value={"content": "x"}) as mock_execute: | |
| 1247 | + with self.assertRaises(wgo.ConsistencyCheckFailedError): | |
| 1248 | + wgo.run_graph( | |
| 1249 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | |
| 1250 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, | |
| 1251 | + ) | |
| 1252 | + # max_attempts=1 means "1 retry allowed" (matches the legacy | |
| 1253 | + # exactly-one-retry default, design doc §2's on_fail comment) -- | |
| 1254 | + # attempt_number=1 on the first failure is not > 1, so one retry | |
| 1255 | + # happens; attempt_number=2 on THAT failure is > 1 and exceeds. | |
| 1256 | + # Two real executions total, not one. | |
| 1257 | + self.assertEqual(mock_execute.call_count, 2) | |
| 1258 | + | |
| 1259 | + | |
| 725 | 1260 | if __name__ == "__main__": |
| 726 | 1261 | unittest.main() | ... | ... |
src/test_work_graph_plan.py
| ... | ... | @@ -133,14 +133,15 @@ class NormalizeGraphManifestTest(unittest.TestCase): |
| 133 | 133 | result = work_graph_plan._normalize_graph_manifest(manifest) |
| 134 | 134 | self.assertEqual(result["nodes"][0]["depends_on"], []) |
| 135 | 135 | |
| 136 | - def test_canonical_doc_chain_enforced_even_with_zero_dependency_info(self): | |
| 137 | - # 2026-08-03 regression: a real call omitted depends_on AND edges | |
| 138 | - # entirely -- worse than test_depends_on_derived_from_edges_when_absent, | |
| 139 | - # which at least had edges to derive from. With nothing to | |
| 140 | - # normalize, topological_order() scheduled 'benchmark' before 'sdd' | |
| 141 | - # (alphabetically first among zero-in-degree nodes), and | |
| 142 | - # e2e_pipeline's build_prompt() crashed formatting a template that | |
| 143 | - # needed sdd's not-yet-produced output. | |
| 136 | + def test_zero_dependency_info_leaves_nodes_independent(self): | |
| 137 | + # 2026-08-08 (FR-B4 "全部自由规划"): the canonical-doc-chain forced- | |
| 138 | + # order enforcement that used to backfill this exact case (2026-08-03 | |
| 139 | + # incident) is deliberately removed, not extended -- Planner's node | |
| 140 | + # set is no longer a fixed architectural constant. A real response | |
| 141 | + # with zero dependency info now genuinely produces zero depends_on, | |
| 142 | + # same as any other node set; robustness rests entirely on the | |
| 143 | + # edge-derived backfill (test_depends_on_derived_from_edges_when_absent) | |
| 144 | + # plus _validate_loop_back_targets, not a hardcoded chain to guess from. | |
| 144 | 145 | manifest = { |
| 145 | 146 | "nodes": [ |
| 146 | 147 | {"node_id": "brd", "artifact_type": "brd"}, |
| ... | ... | @@ -148,59 +149,142 @@ class NormalizeGraphManifestTest(unittest.TestCase): |
| 148 | 149 | {"node_id": "sdd", "artifact_type": "sdd"}, |
| 149 | 150 | {"node_id": "benchmark", "artifact_type": "benchmark"}, |
| 150 | 151 | {"node_id": "test_dataset", "artifact_type": "test_dataset"}, |
| 151 | - {"node_id": "test_script", "artifact_type": "test_script"}, | |
| 152 | 152 | ], |
| 153 | 153 | "edges": [], |
| 154 | 154 | } |
| 155 | 155 | result = work_graph_plan._normalize_graph_manifest(manifest) |
| 156 | - by_type = {n["artifact_type"]: n for n in result["nodes"]} | |
| 157 | - self.assertEqual(by_type["brd"]["depends_on"], []) | |
| 158 | - self.assertEqual(by_type["prd"]["depends_on"], ["brd"]) | |
| 159 | - self.assertEqual(by_type["sdd"]["depends_on"], ["prd"]) | |
| 160 | - self.assertEqual(by_type["benchmark"]["depends_on"], ["sdd"]) | |
| 161 | - self.assertEqual(by_type["test_dataset"]["depends_on"], ["benchmark"]) | |
| 162 | - # test_script needs both -- its own prompt references | |
| 163 | - # {benchmark_content} directly, not just what survives into | |
| 164 | - # test_dataset_content (see work_graph_plan._normalize_graph_manifest). | |
| 165 | - self.assertEqual(by_type["test_script"]["depends_on"], ["test_dataset", "benchmark"]) | |
| 166 | - | |
| 167 | - def test_canonical_chain_enforcement_uses_real_node_ids_not_artifact_types(self): | |
| 168 | - # node_id and artifact_type can differ (e.g. real LLM output naming | |
| 169 | - # nodes "n1"/"n2"/... while still setting artifact_type correctly) -- | |
| 170 | - # the enforced dependency must reference the real node_id. | |
| 156 | + for node in result["nodes"]: | |
| 157 | + self.assertEqual(node["depends_on"], []) | |
| 158 | + | |
| 159 | + def test_on_failure_edge_does_not_get_folded_into_depends_on(self): | |
| 160 | + # 2026-08-08: an on_failure edge is control-flow, not a data | |
| 161 | + # dependency -- the edge-derived depends_on backfill above must | |
| 162 | + # skip it (same reasoning as work_graph_orchestrate.py's | |
| 163 | + # topological_layers()/_transitive_dependents). | |
| 171 | 164 | manifest = { |
| 172 | 165 | "nodes": [ |
| 173 | - {"node_id": "step-1", "artifact_type": "brd"}, | |
| 174 | - {"node_id": "step-2", "artifact_type": "prd"}, | |
| 175 | - {"node_id": "step-3", "artifact_type": "sdd"}, | |
| 176 | - {"node_id": "step-4", "artifact_type": "benchmark"}, | |
| 177 | - {"node_id": "step-5", "artifact_type": "test_dataset"}, | |
| 178 | - {"node_id": "step-6", "artifact_type": "test_script"}, | |
| 166 | + {"node_id": "sdd", "artifact_type": "sdd"}, | |
| 167 | + { | |
| 168 | + "node_id": "benchmark", "artifact_type": "benchmark", "depends_on": ["sdd"], | |
| 169 | + "on_fail": {"action": "loop_back"}, | |
| 170 | + }, | |
| 179 | 171 | ], |
| 180 | - "edges": [], | |
| 172 | + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], | |
| 181 | 173 | } |
| 182 | 174 | result = work_graph_plan._normalize_graph_manifest(manifest) |
| 183 | - by_type = {n["artifact_type"]: n for n in result["nodes"]} | |
| 184 | - self.assertEqual(by_type["prd"]["depends_on"], ["step-1"]) | |
| 185 | - self.assertEqual(by_type["sdd"]["depends_on"], ["step-2"]) | |
| 186 | - self.assertEqual(by_type["benchmark"]["depends_on"], ["step-3"]) | |
| 187 | - self.assertEqual(by_type["test_dataset"]["depends_on"], ["step-4"]) | |
| 188 | - self.assertEqual(by_type["test_script"]["depends_on"], ["step-5", "step-4"]) | |
| 189 | - | |
| 190 | - def test_partial_canonical_set_does_not_trigger_enforcement(self): | |
| 191 | - # only brd/prd/sdd present, no benchmark -- not a recognized | |
| 192 | - # canonical 4-node doc chain, must not guess at a shape. | |
| 175 | + by_id = {n["node_id"]: n for n in result["nodes"]} | |
| 176 | + self.assertEqual(by_id["sdd"]["depends_on"], []) | |
| 177 | + | |
| 178 | + | |
| 179 | +class TransitiveAncestorsTest(unittest.TestCase): | |
| 180 | + def test_includes_self_and_all_upstream(self): | |
| 193 | 181 | manifest = { |
| 194 | 182 | "nodes": [ |
| 195 | - {"node_id": "brd", "artifact_type": "brd"}, | |
| 196 | - {"node_id": "prd", "artifact_type": "prd"}, | |
| 197 | - {"node_id": "sdd", "artifact_type": "sdd"}, | |
| 183 | + {"node_id": "a", "depends_on": []}, | |
| 184 | + {"node_id": "b", "depends_on": ["a"]}, | |
| 185 | + {"node_id": "c", "depends_on": ["b"]}, | |
| 198 | 186 | ], |
| 199 | 187 | "edges": [], |
| 200 | 188 | } |
| 201 | - result = work_graph_plan._normalize_graph_manifest(manifest) | |
| 202 | - for node in result["nodes"]: | |
| 203 | - self.assertEqual(node["depends_on"], []) | |
| 189 | + self.assertEqual(work_graph_plan._transitive_ancestors(manifest, "c"), {"a", "b", "c"}) | |
| 190 | + self.assertEqual(work_graph_plan._transitive_ancestors(manifest, "a"), {"a"}) | |
| 191 | + | |
| 192 | + def test_ignores_on_failure_edges(self): | |
| 193 | + manifest = { | |
| 194 | + "nodes": [{"node_id": "sdd"}, {"node_id": "benchmark"}], | |
| 195 | + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], | |
| 196 | + } | |
| 197 | + # an on_failure edge alone (no real depends_on/data edge) must NOT | |
| 198 | + # make benchmark an ancestor of sdd. | |
| 199 | + self.assertEqual(work_graph_plan._transitive_ancestors(manifest, "sdd"), {"sdd"}) | |
| 200 | + | |
| 201 | + | |
| 202 | +class ValidateLoopBackTargetsTest(unittest.TestCase): | |
| 203 | + def test_valid_loop_back_passes(self): | |
| 204 | + manifest = { | |
| 205 | + "nodes": [ | |
| 206 | + {"node_id": "sdd", "depends_on": []}, | |
| 207 | + { | |
| 208 | + "node_id": "benchmark", "depends_on": ["sdd"], | |
| 209 | + "on_fail": {"action": "loop_back"}, | |
| 210 | + }, | |
| 211 | + ], | |
| 212 | + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], | |
| 213 | + } | |
| 214 | + work_graph_plan._validate_loop_back_targets(manifest) # must not raise | |
| 215 | + | |
| 216 | + def test_no_on_failure_edge_raises(self): | |
| 217 | + manifest = { | |
| 218 | + "nodes": [ | |
| 219 | + {"node_id": "sdd", "depends_on": []}, | |
| 220 | + {"node_id": "benchmark", "depends_on": ["sdd"], "on_fail": {"action": "loop_back"}}, | |
| 221 | + ], | |
| 222 | + "edges": [], | |
| 223 | + } | |
| 224 | + with self.assertRaises(work_graph_plan.PlanningFailedError): | |
| 225 | + work_graph_plan._validate_loop_back_targets(manifest) | |
| 226 | + | |
| 227 | + def test_two_on_failure_edges_from_same_node_raises(self): | |
| 228 | + manifest = { | |
| 229 | + "nodes": [ | |
| 230 | + {"node_id": "sdd", "depends_on": []}, | |
| 231 | + {"node_id": "prd", "depends_on": []}, | |
| 232 | + { | |
| 233 | + "node_id": "benchmark", "depends_on": ["sdd", "prd"], | |
| 234 | + "on_fail": {"action": "loop_back"}, | |
| 235 | + }, | |
| 236 | + ], | |
| 237 | + "edges": [ | |
| 238 | + {"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}, | |
| 239 | + {"from": "benchmark", "to": "prd", "edge_type": "on_failure"}, | |
| 240 | + ], | |
| 241 | + } | |
| 242 | + with self.assertRaises(work_graph_plan.PlanningFailedError): | |
| 243 | + work_graph_plan._validate_loop_back_targets(manifest) | |
| 244 | + | |
| 245 | + def test_target_not_an_upstream_dependency_raises(self): | |
| 246 | + # sdd and unrelated are siblings with no dependency relationship -- | |
| 247 | + # benchmark depends on sdd but its on_failure edge points at the | |
| 248 | + # unrelated node instead, which never influenced benchmark at all. | |
| 249 | + manifest = { | |
| 250 | + "nodes": [ | |
| 251 | + {"node_id": "sdd", "depends_on": []}, | |
| 252 | + {"node_id": "unrelated", "depends_on": []}, | |
| 253 | + { | |
| 254 | + "node_id": "benchmark", "depends_on": ["sdd"], | |
| 255 | + "on_fail": {"action": "loop_back"}, | |
| 256 | + }, | |
| 257 | + ], | |
| 258 | + "edges": [{"from": "benchmark", "to": "unrelated", "edge_type": "on_failure"}], | |
| 259 | + } | |
| 260 | + with self.assertRaises(work_graph_plan.PlanningFailedError): | |
| 261 | + work_graph_plan._validate_loop_back_targets(manifest) | |
| 262 | + | |
| 263 | + def test_target_not_in_manifest_at_all_raises(self): | |
| 264 | + manifest = { | |
| 265 | + "nodes": [{"node_id": "benchmark", "depends_on": [], "on_fail": {"action": "loop_back"}}], | |
| 266 | + "edges": [{"from": "benchmark", "to": "nonexistent", "edge_type": "on_failure"}], | |
| 267 | + } | |
| 268 | + with self.assertRaises(work_graph_plan.PlanningFailedError): | |
| 269 | + work_graph_plan._validate_loop_back_targets(manifest) | |
| 270 | + | |
| 271 | + def test_nodes_without_loop_back_are_unaffected(self): | |
| 272 | + manifest = { | |
| 273 | + "nodes": [ | |
| 274 | + {"node_id": "brd", "depends_on": []}, | |
| 275 | + {"node_id": "prd", "depends_on": ["brd"]}, | |
| 276 | + ], | |
| 277 | + "edges": [], | |
| 278 | + } | |
| 279 | + work_graph_plan._validate_loop_back_targets(manifest) # must not raise | |
| 280 | + | |
| 281 | + def test_normalize_graph_manifest_calls_validation_and_raises_on_bad_loop_back(self): | |
| 282 | + manifest = { | |
| 283 | + "nodes": [{"node_id": "benchmark", "on_fail": {"action": "loop_back"}}], | |
| 284 | + "edges": [], | |
| 285 | + } | |
| 286 | + with self.assertRaises(work_graph_plan.PlanningFailedError): | |
| 287 | + work_graph_plan._normalize_graph_manifest(manifest) | |
| 204 | 288 | |
| 205 | 289 | |
| 206 | 290 | class HappyPathTest(PlanWorkGraphTestBase): |
| ... | ... | @@ -288,15 +372,21 @@ _CODER_THREE_MODULE_MANIFEST = { |
| 288 | 372 | |
| 289 | 373 | |
| 290 | 374 | class GenericPlanningPromptTest(PlanWorkGraphTestBase): |
| 291 | - def test_planner_still_gets_fixed_doc_chain_template(self): | |
| 375 | + def test_planner_gets_the_self_planning_catalog_template(self): | |
| 376 | + # 2026-08-08 (FR-B4 "全部自由规划"): Planner no longer gets a fixed | |
| 377 | + # "produce exactly BRD/PRD/SDD/Benchmark" instruction -- it gets the | |
| 378 | + # node type catalog and decides itself which nodes this goal needs. | |
| 292 | 379 | with patch("src.work_graph_plan.subprocess.run") as run: |
| 293 | 380 | run.return_value = _completed_process( |
| 294 | 381 | stdout_obj={"is_error": False, "result": _fenced_result(_MANIFEST)} |
| 295 | 382 | ) |
| 296 | 383 | work_graph_plan.plan_work_graph(self.conn, "planner", "task-1", "goal", "/tmp/proj-1") |
| 297 | 384 | prompt = run.call_args.args[0][run.call_args.args[0].index("-p") + 1] |
| 298 | - self.assertIn("BRD, PRD, SDD, Benchmark", prompt) | |
| 385 | + self.assertIn("Available node types", prompt) | |
| 386 | + self.assertIn("\"artifact_type\": \"benchmark\"", prompt) | |
| 387 | + self.assertIn("on_failure", prompt) | |
| 299 | 388 | self.assertNotIn("design-dynamic-work-graph", prompt) |
| 389 | + self.assertNotIn("{goal_text}", prompt) # the placeholder itself must have been substituted | |
| 300 | 390 | |
| 301 | 391 | def test_coder_gets_generic_template_referencing_the_skill(self): |
| 302 | 392 | with patch("src.work_graph_plan.subprocess.run") as run: | ... | ... |
src/work_graph_orchestrate.py
| ... | ... | @@ -70,7 +70,7 @@ import threading |
| 70 | 70 | import time |
| 71 | 71 | import uuid |
| 72 | 72 | |
| 73 | -from src import execution_log, paths, work_graph_execute, work_node_run | |
| 73 | +from src import execution_log, paths, settings_config, work_graph_execute, work_node_run | |
| 74 | 74 | |
| 75 | 75 | |
| 76 | 76 | def _log_event(conn, node_exec_id, event_type, **payload): |
| ... | ... | @@ -111,6 +111,22 @@ class WorkGraphLayerError(Exception): |
| 111 | 111 | super().__init__("{} node(s) failed in one layer: {}".format(len(errors), summary)) |
| 112 | 112 | |
| 113 | 113 | |
| 114 | +class _LoopBackRequested(Exception): | |
| 115 | + """Internal signal (2026-08-08, Work Graph self-planning + loop-back): | |
| 116 | + a node's on_fail.action='loop_back' fired -- run_graph()'s main loop | |
| 117 | + catches this and rewinds to source_node_id's target layer, it's not a | |
| 118 | + real failure. Raised as an exception (not returned) because it can | |
| 119 | + originate inside a ThreadPoolExecutor worker thread (a multi-node | |
| 120 | + layer) -- an exception is the only clean way to carry this out through | |
| 121 | + future.result().""" | |
| 122 | + | |
| 123 | + def __init__(self, source_node_id, target_node_id, feedback): | |
| 124 | + self.source_node_id = source_node_id | |
| 125 | + self.target_node_id = target_node_id | |
| 126 | + self.feedback = feedback | |
| 127 | + super().__init__("node {} requests loop_back to {}".format(source_node_id, target_node_id)) | |
| 128 | + | |
| 129 | + | |
| 114 | 130 | class _FetchedRows: |
| 115 | 131 | """A pre-materialized stand-in for a sqlite3.Cursor, holding rows |
| 116 | 132 | already fetched while _LockedConn's lock was held (see _LockedConn's |
| ... | ... | @@ -191,10 +207,21 @@ def topological_layers(graph_manifest): |
| 191 | 207 | might populate one and not the other; trusting only one would silently |
| 192 | 208 | mis-order nodes rather than raise. Raises ValueError on a cycle or a |
| 193 | 209 | dependency referencing an unknown node_id. |
| 210 | + | |
| 211 | + 2026-08-08 (Work Graph self-planning + loop-back): edges with | |
| 212 | + `edge_type == "on_failure"` are control-flow (a node's failure path, | |
| 213 | + see `_on_failure_target`), not data dependencies -- they're excluded | |
| 214 | + here. An `on_failure` edge commonly points from a node back to one of | |
| 215 | + its own upstream dependencies, which the OLD unfiltered merge would | |
| 216 | + have folded in as an (incorrect, reversed) data dependency, creating a | |
| 217 | + spurious cycle that would have made this function reject the graph | |
| 218 | + before any node ever ran. | |
| 194 | 219 | """ |
| 195 | 220 | nodes = {n["node_id"]: n for n in graph_manifest["nodes"]} |
| 196 | 221 | deps = {node_id: set(n.get("depends_on") or []) for node_id, n in nodes.items()} |
| 197 | 222 | for edge in graph_manifest.get("edges", []): |
| 223 | + if edge.get("edge_type") == "on_failure": | |
| 224 | + continue | |
| 198 | 225 | to_id, from_id = edge.get("to"), edge.get("from") |
| 199 | 226 | if to_id in deps and from_id in nodes: |
| 200 | 227 | deps[to_id].add(from_id) |
| ... | ... | @@ -232,6 +259,73 @@ def topological_order(graph_manifest): |
| 232 | 259 | return [node for layer in topological_layers(graph_manifest) for node in layer] |
| 233 | 260 | |
| 234 | 261 | |
| 262 | +def _transitive_dependents(graph_manifest, node_id): | |
| 263 | + """node_id itself + every node that (directly or transitively) depends | |
| 264 | + on it, via `depends_on`/normal data-dependency edges (`on_failure` | |
| 265 | + edges excluded, same reasoning as topological_layers()). Used when a | |
| 266 | + loop_back fires: node_id's output is about to be regenerated, so | |
| 267 | + everything downstream of it was computed from what's now stale input | |
| 268 | + and must be re-run too (2026-08-08, Work Graph self-planning + | |
| 269 | + loop-back design §4.2).""" | |
| 270 | + dependents = {n["node_id"]: [] for n in graph_manifest["nodes"]} | |
| 271 | + deps = {n["node_id"]: set(n.get("depends_on") or []) for n in graph_manifest["nodes"]} | |
| 272 | + for edge in graph_manifest.get("edges", []): | |
| 273 | + if edge.get("edge_type") == "on_failure": | |
| 274 | + continue | |
| 275 | + to_id, from_id = edge.get("to"), edge.get("from") | |
| 276 | + if to_id in deps and from_id in deps: | |
| 277 | + deps[to_id].add(from_id) | |
| 278 | + for node_id_, dep_set in deps.items(): | |
| 279 | + for dep in dep_set: | |
| 280 | + dependents.setdefault(dep, []).append(node_id_) | |
| 281 | + | |
| 282 | + affected, stack = set(), [node_id] | |
| 283 | + while stack: | |
| 284 | + current = stack.pop() | |
| 285 | + if current in affected: | |
| 286 | + continue | |
| 287 | + affected.add(current) | |
| 288 | + stack.extend(dependents.get(current, [])) | |
| 289 | + return affected | |
| 290 | + | |
| 291 | + | |
| 292 | +def _transitive_ancestors(graph_manifest, node_id): | |
| 293 | + """Reverse of _transitive_dependents: node_id itself + every node it | |
| 294 | + (directly or transitively) depends on. Used to validate that a | |
| 295 | + declared on_failure edge's target is a genuine upstream influence, not | |
| 296 | + an arbitrary node (work_graph_plan.py's _validate_loop_back_targets).""" | |
| 297 | + deps = {n["node_id"]: set(n.get("depends_on") or []) for n in graph_manifest["nodes"]} | |
| 298 | + for edge in graph_manifest.get("edges", []): | |
| 299 | + if edge.get("edge_type") == "on_failure": | |
| 300 | + continue | |
| 301 | + to_id, from_id = edge.get("to"), edge.get("from") | |
| 302 | + if to_id in deps and from_id in deps: | |
| 303 | + deps[to_id].add(from_id) | |
| 304 | + | |
| 305 | + ancestors, stack = set(), [node_id] | |
| 306 | + while stack: | |
| 307 | + current = stack.pop() | |
| 308 | + if current in ancestors: | |
| 309 | + continue | |
| 310 | + ancestors.add(current) | |
| 311 | + stack.extend(deps.get(current, [])) | |
| 312 | + return ancestors | |
| 313 | + | |
| 314 | + | |
| 315 | +def _on_failure_target(graph_manifest, node_id): | |
| 316 | + """The node_id that `node_id`'s on_fail.action='loop_back' should | |
| 317 | + return to -- read from the single `{"from": node_id, "to": ..., | |
| 318 | + "edge_type": "on_failure"}` edge, not a separate node field (2026-08-08 | |
| 319 | + design revision: the failure path is expressed the same way the | |
| 320 | + success path already is, via `edges`). Callers should have already | |
| 321 | + validated (work_graph_plan.py's _validate_loop_back_targets) that | |
| 322 | + exactly one such edge exists before relying on this at runtime.""" | |
| 323 | + for edge in graph_manifest.get("edges", []): | |
| 324 | + if edge.get("from") == node_id and edge.get("edge_type") == "on_failure": | |
| 325 | + return edge["to"] | |
| 326 | + raise ValueError("node {} declares on_fail.action='loop_back' but has no on_failure edge".format(node_id)) | |
| 327 | + | |
| 328 | + | |
| 235 | 329 | def ensure_placeholder_skill(conn, skill_id, owner_org_node): |
| 236 | 330 | """No Skill content has actually been migrated from the Hermes profiles |
| 237 | 331 | yet (skill_update.py's own documented gap) -- work_node_execution. |
| ... | ... | @@ -302,6 +396,221 @@ def _make_run_fn(conn, target_folder_path, node_exec_id, org_node, prompt, outpu |
| 302 | 396 | return run_fn |
| 303 | 397 | |
| 304 | 398 | |
| 399 | +def _coerce_bool(value): | |
| 400 | + """Same fix as e2e_pipeline.py's own _coerce_bool (2026-08-02): a real | |
| 401 | + call has returned e.g. "false" as a JSON STRING despite "type": | |
| 402 | + "boolean" in the schema, and `if "false":` is truthy in Python. Kept as | |
| 403 | + its own small copy here rather than importing e2e_pipeline (which | |
| 404 | + imports this module -- importing back would be circular) or promoting | |
| 405 | + it to a shared utils module for a 3-line helper.""" | |
| 406 | + if isinstance(value, bool): | |
| 407 | + return value | |
| 408 | + if isinstance(value, str): | |
| 409 | + return value.strip().lower() == "true" | |
| 410 | + return bool(value) | |
| 411 | + | |
| 412 | + | |
| 413 | +_JUDGMENT_SCHEMA = { | |
| 414 | + "type": "object", | |
| 415 | + "required": ["ok", "issues"], | |
| 416 | + "properties": {"ok": {"type": "boolean"}, "issues": {"type": "array", "items": {"type": "string"}}}, | |
| 417 | +} | |
| 418 | + | |
| 419 | + | |
| 420 | +def _llm_judgment_verify_fn(judgment, org_node, target_folder_path, timeout=180): | |
| 421 | + """judgment.kind == 'llm': generalizes e2e_pipeline.py's | |
| 422 | + _check_prd_consistency -- same self-target worktree isolation (a real | |
| 423 | + agent() call with memory:project writes an uncommitted change wherever | |
| 424 | + it runs; self-target runs need their own worktree so that doesn't land | |
| 425 | + on the main checkout, see that function's own docstring for the | |
| 426 | + 2026-08-03 incident this guards against), but the judge prompt comes | |
| 427 | + from judgment['judge_prompt'] (node-declared, self-contained -- same | |
| 428 | + design principle as _GENERIC_PROMPT_TEMPLATE's task_prompt) instead of | |
| 429 | + a hardcoded PRD-specific prompt, and the schema is the generic | |
| 430 | + {"ok": bool, "issues": [...]} shape instead of PRD's {"consistent": ...}. | |
| 431 | + e2e_pipeline.py's own _check_prd_consistency/_prd_consistency_verify_fn | |
| 432 | + are untouched and remain the path Validator's fixed manifest uses | |
| 433 | + (FR-A8) -- this is a parallel, more general implementation, not a | |
| 434 | + replacement. `is_self_target` is checked fresh on each call (not cached | |
| 435 | + at closure-creation time) -- matches _check_prd_consistency's own | |
| 436 | + behavior, and means a caller building this verify_fn once and reusing | |
| 437 | + it always reflects the real current state, not a stale snapshot.""" | |
| 438 | + | |
| 439 | + def verify_fn(node, output): | |
| 440 | + is_self = paths.is_self_target(target_folder_path) | |
| 441 | + node_exec_id = "judgment-" + uuid.uuid4().hex[:12] | |
| 442 | + worktree_path = work_node_run.create_worktree(target_folder_path, node_exec_id) if is_self else None | |
| 443 | + cwd = worktree_path or target_folder_path | |
| 444 | + try: | |
| 445 | + prompt = judgment["judge_prompt"].format(content=output["content"]) | |
| 446 | + script_name = "graphlemon-judgment-" + uuid.uuid4().hex | |
| 447 | + script_content = ( | |
| 448 | + "export const meta = {{ name: {name}, description: 'graphlemon node judgment' }}\n\n" | |
| 449 | + "const result = await agent({prompt}, {{ agentType: {org_node_json}, schema: {schema} }})\n\n" | |
| 450 | + "return result\n" | |
| 451 | + ).format( | |
| 452 | + name=json.dumps(script_name), prompt=json.dumps(prompt), | |
| 453 | + org_node_json=json.dumps(org_node), schema=json.dumps(_JUDGMENT_SCHEMA), | |
| 454 | + ) | |
| 455 | + work_graph_execute.write_workflow_script(cwd, script_name, script_content) | |
| 456 | + result = work_graph_execute.execute_work_graph(cwd, script_name, {}, _JUDGMENT_SCHEMA, timeout=timeout) | |
| 457 | + if worktree_path: | |
| 458 | + work_node_run.ensure_worktree_committed(worktree_path, node_exec_id) | |
| 459 | + work_node_run.ensure_worktree_committed(target_folder_path, node_exec_id + "-main-checkout-drift") | |
| 460 | + work_node_run.merge_worktree(target_folder_path, node_exec_id) | |
| 461 | + ok = _coerce_bool(result["ok"]) | |
| 462 | + return ok, (result.get("issues") or []) | |
| 463 | + finally: | |
| 464 | + if worktree_path: | |
| 465 | + work_node_run.remove_worktree(target_folder_path, worktree_path) | |
| 466 | + | |
| 467 | + return verify_fn | |
| 468 | + | |
| 469 | + | |
| 470 | +def _resolve_node_verify_fn(node, external_verify_fn, org_node, target_folder_path, mechanical_check_registry): | |
| 471 | + """A node's own `judgment` field (if declared) takes over verify_fn | |
| 472 | + resolution for that node; a node with no `judgment` (every existing | |
| 473 | + graph_manifest today) falls through to whatever the caller passed as | |
| 474 | + `verify_fn` -- possibly None, meaning no verification at all -- exactly | |
| 475 | + like current behavior (FR-A8).""" | |
| 476 | + judgment = node.get("judgment") | |
| 477 | + if not judgment or judgment.get("kind", "none") == "none": | |
| 478 | + return external_verify_fn | |
| 479 | + if judgment["kind"] == "mechanical": | |
| 480 | + check_fn = (mechanical_check_registry or {}).get(judgment["check_ref"]) | |
| 481 | + if check_fn is None: | |
| 482 | + raise ValueError("unknown mechanical check_ref: {!r}".format(judgment["check_ref"])) | |
| 483 | + | |
| 484 | + def mechanical_verify_fn(node, output, _check_fn=check_fn): | |
| 485 | + ok, detail = _check_fn(output["content"]) | |
| 486 | + return ok, ([detail] if detail else None) | |
| 487 | + | |
| 488 | + return mechanical_verify_fn | |
| 489 | + if judgment["kind"] == "llm": | |
| 490 | + return _llm_judgment_verify_fn(judgment, org_node, target_folder_path) | |
| 491 | + raise ValueError("unknown judgment.kind: {!r}".format(judgment["kind"])) | |
| 492 | + | |
| 493 | + | |
| 494 | +def _validate_mechanical_check_refs(graph_manifest, mechanical_check_registry): | |
| 495 | + """Fail fast before running ANY node -- a typo'd check_ref caught only | |
| 496 | + when that node is finally reached would waste every token spent on the | |
| 497 | + nodes before it.""" | |
| 498 | + registry = mechanical_check_registry or {} | |
| 499 | + for node in graph_manifest["nodes"]: | |
| 500 | + judgment = node.get("judgment") or {} | |
| 501 | + if judgment.get("kind") == "mechanical" and judgment.get("check_ref") not in registry: | |
| 502 | + raise ValueError( | |
| 503 | + "node {}'s judgment.check_ref {!r} is not in mechanical_check_registry".format( | |
| 504 | + node["node_id"], judgment.get("check_ref") | |
| 505 | + ) | |
| 506 | + ) | |
| 507 | + | |
| 508 | + | |
| 509 | +_DEFAULT_LOOP_LIMIT = 3 # only reached if even the "default" config_id row is unseeded | |
| 510 | + | |
| 511 | + | |
| 512 | +def _resolve_loop_limit(conn, org_node, graph_manifest, source_node_id): | |
| 513 | + """How many times `source_node_id` (the node whose judgment failed and | |
| 514 | + declared on_fail.action in {'retry_self', 'loop_back'}) may trigger a | |
| 515 | + loop before run_graph() gives up. Lookup order (2026-08-08 design §6): | |
| 516 | + the failing node's own on_fail.max_attempts, then a settings_config.py | |
| 517 | + row keyed by "{org_node}:{node_type}", then "{org_node}:*", then | |
| 518 | + "default", then a hardcoded fallback.""" | |
| 519 | + source_node = next(n for n in graph_manifest["nodes"] if n["node_id"] == source_node_id) | |
| 520 | + node_level = (source_node.get("on_fail") or {}).get("max_attempts") | |
| 521 | + if node_level is not None: | |
| 522 | + return node_level | |
| 523 | + node_type = source_node.get("artifact_type", source_node_id) | |
| 524 | + for config_id in ("{}:{}".format(org_node, node_type), "{}:*".format(org_node), "default"): | |
| 525 | + config = settings_config.get_loop_limit_config(conn, config_id) | |
| 526 | + if config: | |
| 527 | + return config["max_loop_attempts"] | |
| 528 | + return _DEFAULT_LOOP_LIMIT | |
| 529 | + | |
| 530 | + | |
| 531 | +def _record_loop_back_event(conn, run_id, sig, attempt_number): | |
| 532 | + """Persists one 'loop_back_triggered' execution_event row -- this IS | |
| 533 | + the loop history (2026-08-08 design §7): no separate table, so a | |
| 534 | + node's full "sent back N times, here's why each time" story is | |
| 535 | + reconstructed purely by querying execution_event.""" | |
| 536 | + row = conn.execute( | |
| 537 | + "SELECT wgr.task_id, t.project_id FROM work_graph_run wgr " | |
| 538 | + "JOIN task t ON wgr.task_id = t.task_id WHERE wgr.run_id = ?", (run_id,) | |
| 539 | + ).fetchone() | |
| 540 | + task_id, project_id = row | |
| 541 | + conn.execute( | |
| 542 | + "INSERT INTO execution_event (event_id, project_id, task_id, run_id, event_type, payload) " | |
| 543 | + "VALUES (?, ?, ?, ?, 'loop_back_triggered', ?)", | |
| 544 | + (str(uuid.uuid4()), project_id, task_id, run_id, json.dumps({ | |
| 545 | + "source_node_id": sig.source_node_id, "target_node_id": sig.target_node_id, | |
| 546 | + "attempt_number": attempt_number, "feedback": sig.feedback, | |
| 547 | + })), | |
| 548 | + ) | |
| 549 | + conn.commit() | |
| 550 | + | |
| 551 | + | |
| 552 | +def _loop_back_events(conn, run_id): | |
| 553 | + rows = conn.execute( | |
| 554 | + "SELECT payload FROM execution_event WHERE run_id = ? AND event_type = 'loop_back_triggered' " | |
| 555 | + "ORDER BY created_at", (run_id,) | |
| 556 | + ).fetchall() | |
| 557 | + return [json.loads(p) for (p,) in rows] | |
| 558 | + | |
| 559 | + | |
| 560 | +def _loop_attempt_count_from_events(conn, run_id, target_node_id, source_node_id): | |
| 561 | + """Real count of prior loop_back triggers for this exact (target, | |
| 562 | + source) pair, read from execution_event -- not an in-memory counter, | |
| 563 | + so a process restart mid-loop resumes with the true count (REQ-06).""" | |
| 564 | + return sum( | |
| 565 | + 1 for e in _loop_back_events(conn, run_id) | |
| 566 | + if e["target_node_id"] == target_node_id and e["source_node_id"] == source_node_id | |
| 567 | + ) | |
| 568 | + | |
| 569 | + | |
| 570 | +def _loop_history_prompt_suffix(conn, run_id, node_id): | |
| 571 | + """When node_id is about to be (re-)executed, append the full history | |
| 572 | + of every prior time it was sent back for revision (not just the most | |
| 573 | + recent) -- empty string if node_id was never a loop_back target.""" | |
| 574 | + history = [e for e in _loop_back_events(conn, run_id) if e["target_node_id"] == node_id] | |
| 575 | + if not history: | |
| 576 | + return "" | |
| 577 | + lines = [ | |
| 578 | + "Attempt {}: rejected because: {}".format(h["attempt_number"], "; ".join(h["feedback"] or [])) | |
| 579 | + for h in history | |
| 580 | + ] | |
| 581 | + return "\n\nThis node was sent back for revision before, with this history:\n" + "\n".join(lines) | |
| 582 | + | |
| 583 | + | |
| 584 | +def _next_exec_node_id(conn, run_id, node_id, graph_manifest): | |
| 585 | + """The exec node_id to use for node_id's NEXT real execution. A node | |
| 586 | + that has never had its cached output invalidated by a loop_back (the | |
| 587 | + overwhelming common case, and every node in a graph_manifest with no | |
| 588 | + judgment/on_fail at all) always reuses the plain node_id -- preserving | |
| 589 | + run_work_node's normal breakpoint-resume short-circuit (a 'done' row | |
| 590 | + for node_id returns its persisted output instead of re-running) | |
| 591 | + completely unchanged, including across a process restart on a run | |
| 592 | + that never looped at all. | |
| 593 | + | |
| 594 | + 2026-08-08 (found via a real test failure, not just theory): counting | |
| 595 | + only "was node_id EVER the direct loop_back target" undercounts -- | |
| 596 | + every node in `_transitive_dependents(target)` gets its `outputs` | |
| 597 | + entry popped and genuinely re-executed too (run_graph()'s own | |
| 598 | + control flow), not just the target itself. A downstream node that's | |
| 599 | + never itself a `target_node_id` but IS swept up by some earlier | |
| 600 | + node's loop_back still needs its own fresh '-attemptN' row each time, | |
| 601 | + or it silently short-circuits to its own stale pre-invalidation | |
| 602 | + output on the second pass. So this counts every loop_back event | |
| 603 | + whose target's `_transitive_dependents` set includes node_id, not | |
| 604 | + just events where node_id itself is the literal target.""" | |
| 605 | + prior_invalidations = sum( | |
| 606 | + 1 for e in _loop_back_events(conn, run_id) | |
| 607 | + if node_id in _transitive_dependents(graph_manifest, e["target_node_id"]) | |
| 608 | + ) | |
| 609 | + if prior_invalidations == 0: | |
| 610 | + return node_id | |
| 611 | + return "{}-attempt{}".format(node_id, prior_invalidations + 1) | |
| 612 | + | |
| 613 | + | |
| 305 | 614 | _DEFAULT_MAX_WORKERS = 4 # see this module's own docstring section on G for rationale |
| 306 | 615 | |
| 307 | 616 | |
| ... | ... | @@ -309,6 +618,7 @@ def run_graph( |
| 309 | 618 | conn, run_id, org_node, skill_id, target_folder_path, graph_manifest, |
| 310 | 619 | build_prompt, input_schema, output_schema, timeout=600, needs_worktree=False, |
| 311 | 620 | verify_fn=None, sleep=time.sleep, now=time.time, max_workers=_DEFAULT_MAX_WORKERS, |
| 621 | + mechanical_check_registry=None, | |
| 312 | 622 | ): |
| 313 | 623 | """Runs every node in `graph_manifest` through `run_work_node()`, each |
| 314 | 624 | backed by a real one-node dynamic-workflow `agent()` call using |
| ... | ... | @@ -372,75 +682,225 @@ def run_graph( |
| 372 | 682 | even after every sibling has finished -- Python threads have no clean |
| 373 | 683 | preemptible cancel, and building one is out of scope for this pass. |
| 374 | 684 | |
| 685 | + `mechanical_check_registry` (2026-08-08, Work Graph self-planning + | |
| 686 | + loop-back): {check_ref: callable(content) -> (ok, detail)}, consulted | |
| 687 | + by any node declaring `judgment: {"kind": "mechanical", "check_ref": | |
| 688 | + ...}` -- see `_resolve_node_verify_fn`. A node with no `judgment` field | |
| 689 | + (every graph_manifest that predates this) ignores this entirely and | |
| 690 | + keeps using whatever `verify_fn` the caller passed, unchanged (FR-A8). | |
| 691 | + | |
| 692 | + **loop_back (2026-08-08, Work Graph self-planning + loop-back)**: a | |
| 693 | + node declaring its own `judgment` can also declare `on_fail.action`: | |
| 694 | + `hard_fail` (fail the whole run immediately, no retry), `retry_self` | |
| 695 | + (loop back to itself), or `loop_back` (jump to an earlier node named | |
| 696 | + by that node's own `edge_type: "on_failure"` edge -- see | |
| 697 | + `_on_failure_target`). `retry_self`/`loop_back` share one mechanism: | |
| 698 | + the target node (and everything transitively depending on it, since | |
| 699 | + their input is now stale) is re-executed with the full accumulated | |
| 700 | + history of every prior rejection appended to its prompt | |
| 701 | + (`_loop_history_prompt_suffix`), bounded per (target, source) pair by | |
| 702 | + `_resolve_loop_limit` -- exceeding it raises `ConsistencyCheckFailedError` | |
| 703 | + same as the legacy single-retry path. A node with NO `judgment` field | |
| 704 | + (every graph_manifest that predates this) is entirely unaffected -- | |
| 705 | + it keeps going through `_verify_and_maybe_regenerate` exactly as | |
| 706 | + before, regardless of any of this (FR-A8). | |
| 707 | + | |
| 375 | 708 | Returns {node_id: output_payload}. |
| 376 | 709 | """ |
| 710 | + _validate_mechanical_check_refs(graph_manifest, mechanical_check_registry) | |
| 377 | 711 | ensure_placeholder_skill(conn, skill_id, org_node) |
| 378 | 712 | effective_needs_worktree = needs_worktree or paths.is_self_target(target_folder_path) |
| 379 | 713 | layers = topological_layers(graph_manifest) |
| 714 | + node_layer_index = {node["node_id"]: i for i, layer in enumerate(layers) for node in layer} | |
| 380 | 715 | outputs = {} |
| 381 | 716 | locked_conn = _LockedConn(conn) |
| 382 | 717 | |
| 383 | - for layer in layers: | |
| 384 | - if len(layer) == 1: | |
| 385 | - node = layer[0] | |
| 386 | - outputs[node["node_id"]] = _run_one_node( | |
| 387 | - conn, run_id, org_node, skill_id, target_folder_path, node, outputs, | |
| 388 | - build_prompt, input_schema, output_schema, timeout, | |
| 389 | - effective_needs_worktree, verify_fn, sleep, now, | |
| 390 | - ) | |
| 718 | + try: | |
| 719 | + return _run_layers( | |
| 720 | + conn, run_id, org_node, skill_id, target_folder_path, layers, node_layer_index, outputs, | |
| 721 | + graph_manifest, build_prompt, input_schema, output_schema, timeout, effective_needs_worktree, | |
| 722 | + verify_fn, sleep, now, max_workers, mechanical_check_registry, locked_conn, | |
| 723 | + ) | |
| 724 | + except Exception: | |
| 725 | + # 2026-08-08 (found via a real self-planned run): only three specific, | |
| 726 | + # anticipated failure paths (loop_back exceeding its limit, | |
| 727 | + # on_fail.action='hard_fail', the legacy _verify_and_maybe_regenerate | |
| 728 | + # double-failure) ever mark work_graph_run terminal. Any OTHER | |
| 729 | + # exception -- e.g. a bare KeyError from a caller's build_prompt(), | |
| 730 | + # which is exactly what a real run hit -- used to propagate straight | |
| 731 | + # out of run_graph() leaving the row stuck at its non-terminal status | |
| 732 | + # forever, permanently blocking every future attempt for this | |
| 733 | + # (task_id, org_node) via ux_work_graph_run_active's UNIQUE index | |
| 734 | + # (schema.py) -- the retry itself would fail immediately on the | |
| 735 | + # INSERT, with no clean error explaining why. This is a catch-all | |
| 736 | + # safety net, not a substitute for the three specific handlers above | |
| 737 | + # (which run first and set more specific end-states along the way); | |
| 738 | + # the WHERE clause is a no-op if one of them (or a nested call) had | |
| 739 | + # already marked this run terminal. | |
| 740 | + conn.execute( | |
| 741 | + "UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ? " | |
| 742 | + "AND status IN ('planning', 'running', 'merging', 'verifying', 'reflecting')", | |
| 743 | + (run_id,), | |
| 744 | + ) | |
| 745 | + conn.commit() | |
| 746 | + raise | |
| 747 | + | |
| 748 | + | |
| 749 | +def _run_layers( | |
| 750 | + conn, run_id, org_node, skill_id, target_folder_path, layers, node_layer_index, outputs, | |
| 751 | + graph_manifest, build_prompt, input_schema, output_schema, timeout, effective_needs_worktree, | |
| 752 | + verify_fn, sleep, now, max_workers, mechanical_check_registry, locked_conn, | |
| 753 | +): | |
| 754 | + """run_graph()'s actual node-by-node/layer-by-layer execution loop -- | |
| 755 | + split out so run_graph() itself can wrap the whole thing in one | |
| 756 | + try/except (see its own comment) without deeply nesting this loop.""" | |
| 757 | + layer_index = 0 | |
| 758 | + while layer_index < len(layers): | |
| 759 | + pending = [node for node in layers[layer_index] if node["node_id"] not in outputs] | |
| 760 | + if not pending: | |
| 761 | + layer_index += 1 | |
| 391 | 762 | continue |
| 392 | 763 | |
| 393 | - worker_count = min(max_workers, len(layer)) | |
| 394 | - errors = [] | |
| 395 | - with concurrent.futures.ThreadPoolExecutor(max_workers=worker_count) as pool: | |
| 396 | - futures = { | |
| 397 | - pool.submit( | |
| 398 | - _run_one_node, locked_conn, run_id, org_node, skill_id, target_folder_path, | |
| 399 | - node, outputs, build_prompt, input_schema, output_schema, timeout, | |
| 400 | - effective_needs_worktree, verify_fn, sleep, now, | |
| 401 | - ): node | |
| 402 | - for node in layer | |
| 403 | - } | |
| 404 | - done, _ = concurrent.futures.wait(futures, return_when=concurrent.futures.ALL_COMPLETED) | |
| 405 | - for future in done: | |
| 406 | - node = futures[future] | |
| 407 | - try: | |
| 408 | - outputs[node["node_id"]] = future.result() | |
| 409 | - except Exception as e: | |
| 410 | - errors.append((node["node_id"], e)) | |
| 411 | - | |
| 412 | - if errors: | |
| 413 | - raise WorkGraphLayerError(errors) | |
| 764 | + loop_back_signal = None | |
| 765 | + if len(pending) == 1: | |
| 766 | + node = pending[0] | |
| 767 | + try: | |
| 768 | + outputs[node["node_id"]] = _run_one_node( | |
| 769 | + conn, run_id, org_node, skill_id, target_folder_path, node, outputs, graph_manifest, | |
| 770 | + build_prompt, input_schema, output_schema, timeout, | |
| 771 | + effective_needs_worktree, verify_fn, sleep, now, mechanical_check_registry, | |
| 772 | + ) | |
| 773 | + except _LoopBackRequested as sig: | |
| 774 | + loop_back_signal = sig | |
| 775 | + else: | |
| 776 | + worker_count = min(max_workers, len(pending)) | |
| 777 | + errors = [] | |
| 778 | + loop_back_signals = [] | |
| 779 | + with concurrent.futures.ThreadPoolExecutor(max_workers=worker_count) as pool: | |
| 780 | + futures = { | |
| 781 | + pool.submit( | |
| 782 | + _run_one_node, locked_conn, run_id, org_node, skill_id, target_folder_path, | |
| 783 | + node, outputs, graph_manifest, build_prompt, input_schema, output_schema, timeout, | |
| 784 | + effective_needs_worktree, verify_fn, sleep, now, mechanical_check_registry, | |
| 785 | + ): node | |
| 786 | + for node in pending | |
| 787 | + } | |
| 788 | + done, _ = concurrent.futures.wait(futures, return_when=concurrent.futures.ALL_COMPLETED) | |
| 789 | + for future in done: | |
| 790 | + node = futures[future] | |
| 791 | + try: | |
| 792 | + outputs[node["node_id"]] = future.result() | |
| 793 | + except _LoopBackRequested as sig: | |
| 794 | + loop_back_signals.append(sig) | |
| 795 | + except Exception as e: | |
| 796 | + errors.append((node["node_id"], e)) | |
| 797 | + | |
| 798 | + if loop_back_signals: | |
| 799 | + # A layer can have at most one *winning* rewind per pass -- | |
| 800 | + # pick deterministically (lowest source_node_id). Any plain | |
| 801 | + # `errors` from OTHER siblings in this same pass are | |
| 802 | + # dropped, not raised: they'll surface again naturally on | |
| 803 | + # the next pass if still genuinely wrong after the rewind | |
| 804 | + # (same "accept a known, documented limitation rather than | |
| 805 | + # build a bigger coordinator" philosophy as the quota-wait | |
| 806 | + # limitation already documented above for G). | |
| 807 | + loop_back_signal = sorted(loop_back_signals, key=lambda s: s.source_node_id)[0] | |
| 808 | + elif errors: | |
| 809 | + raise WorkGraphLayerError(errors) | |
| 810 | + | |
| 811 | + if loop_back_signal is not None: | |
| 812 | + sig = loop_back_signal | |
| 813 | + attempt_number = _loop_attempt_count_from_events( | |
| 814 | + conn, run_id, sig.target_node_id, sig.source_node_id | |
| 815 | + ) + 1 | |
| 816 | + limit = _resolve_loop_limit(conn, org_node, graph_manifest, sig.source_node_id) | |
| 817 | + if attempt_number > limit: | |
| 818 | + conn.execute("UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ?", (run_id,)) | |
| 819 | + conn.commit() | |
| 820 | + raise ConsistencyCheckFailedError( | |
| 821 | + "loop_back from {} to {} exceeded max_attempts={}".format( | |
| 822 | + sig.source_node_id, sig.target_node_id, limit | |
| 823 | + ), | |
| 824 | + node_id=sig.source_node_id, | |
| 825 | + ) | |
| 826 | + _record_loop_back_event(conn, run_id, sig, attempt_number) | |
| 827 | + for affected_id in _transitive_dependents(graph_manifest, sig.target_node_id): | |
| 828 | + outputs.pop(affected_id, None) | |
| 829 | + layer_index = node_layer_index[sig.target_node_id] | |
| 830 | + continue | |
| 831 | + | |
| 832 | + layer_index += 1 | |
| 414 | 833 | |
| 415 | 834 | return outputs |
| 416 | 835 | |
| 417 | 836 | |
| 418 | 837 | def _run_one_node( |
| 419 | - conn, run_id, org_node, skill_id, target_folder_path, node, outputs, | |
| 838 | + conn, run_id, org_node, skill_id, target_folder_path, node, outputs, graph_manifest, | |
| 420 | 839 | build_prompt, input_schema, output_schema, timeout, needs_worktree, verify_fn, sleep, now, |
| 840 | + mechanical_check_registry=None, | |
| 421 | 841 | ): |
| 422 | 842 | """One node's full lifecycle (row creation, prompt, run, optional |
| 423 | - verify+regenerate) -- shared by both the synchronous single-node-layer | |
| 424 | - path and the ThreadPoolExecutor multi-node-layer path in run_graph(), | |
| 425 | - so there's exactly one implementation of "run a node" to maintain.""" | |
| 843 | + verify+regenerate/loop_back) -- shared by both the synchronous | |
| 844 | + single-node-layer path and the ThreadPoolExecutor multi-node-layer | |
| 845 | + path in run_graph(), so there's exactly one implementation of "run a | |
| 846 | + node" to maintain. | |
| 847 | + | |
| 848 | + 2026-08-08: uses `_next_exec_node_id` rather than always the plain | |
| 849 | + node_id -- see that function's own docstring for why (a loop_back | |
| 850 | + target must never collide with, and short-circuit against, its own | |
| 851 | + stale pre-loop_back 'done' row; every other node keeps resuming | |
| 852 | + exactly like before).""" | |
| 426 | 853 | node_id = node["node_id"] |
| 854 | + exec_node_id_str = _next_exec_node_id(conn, run_id, node_id, graph_manifest) | |
| 855 | + | |
| 427 | 856 | node_exec_id = _ensure_node_row( |
| 428 | - conn, run_id, node_id, org_node, skill_id, | |
| 857 | + conn, run_id, exec_node_id_str, org_node, skill_id, | |
| 429 | 858 | "work_graph_node_input.schema.json", "work_graph_node_output.schema.json", |
| 430 | 859 | {"node_id": node_id}, |
| 431 | 860 | ) |
| 432 | - prompt = build_prompt(node, outputs) | |
| 861 | + prompt = build_prompt(node, outputs) + _loop_history_prompt_suffix(conn, run_id, node_id) | |
| 433 | 862 | run_fn = _make_run_fn(conn, target_folder_path, node_exec_id, org_node, prompt, output_schema, timeout) |
| 434 | 863 | output = work_node_run.run_work_node( |
| 435 | 864 | conn, node_exec_id, {"node_id": node_id}, input_schema, output_schema, run_fn, |
| 436 | 865 | needs_worktree=needs_worktree, repo_path=target_folder_path, sleep=sleep, now=now, |
| 437 | 866 | ) |
| 438 | - if verify_fn is not None: | |
| 439 | - output = _verify_and_maybe_regenerate( | |
| 867 | + | |
| 868 | + judgment = node.get("judgment") | |
| 869 | + has_own_judgment = bool(judgment) and judgment.get("kind", "none") != "none" | |
| 870 | + resolved_verify_fn = _resolve_node_verify_fn( | |
| 871 | + node, verify_fn, org_node, target_folder_path, mechanical_check_registry | |
| 872 | + ) | |
| 873 | + if resolved_verify_fn is None: | |
| 874 | + return output | |
| 875 | + | |
| 876 | + if not has_own_judgment: | |
| 877 | + # Legacy path (FR-A8): this node has no judgment of its own -- | |
| 878 | + # whatever external verify_fn the caller passed behaves exactly as | |
| 879 | + # before (_verify_and_maybe_regenerate does its OWN verify_fn | |
| 880 | + # call internally -- checking here too would call it twice for | |
| 881 | + # every node, once redundantly), regardless of any on_fail field | |
| 882 | + # (legacy nodes don't have one). | |
| 883 | + return _verify_and_maybe_regenerate( | |
| 440 | 884 | conn, run_id, org_node, skill_id, target_folder_path, node, node_id, prompt, |
| 441 | - output, input_schema, output_schema, timeout, needs_worktree, verify_fn, sleep, now, | |
| 885 | + output, input_schema, output_schema, timeout, needs_worktree, resolved_verify_fn, sleep, now, | |
| 886 | + ) | |
| 887 | + | |
| 888 | + ok, feedback = resolved_verify_fn(node, output) | |
| 889 | + if ok: | |
| 890 | + return output | |
| 891 | + | |
| 892 | + action = (node.get("on_fail") or {}).get("action", "retry_self") | |
| 893 | + if action == "hard_fail": | |
| 894 | + conn.execute("UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ?", (run_id,)) | |
| 895 | + conn.commit() | |
| 896 | + raise ConsistencyCheckFailedError( | |
| 897 | + "node {} failed judgment with on_fail.action='hard_fail': {}".format( | |
| 898 | + node_id, "; ".join(feedback or []) | |
| 899 | + ), | |
| 900 | + node_id=node_id, | |
| 442 | 901 | ) |
| 443 | - return output | |
| 902 | + target = node_id if action == "retry_self" else _on_failure_target(graph_manifest, node_id) | |
| 903 | + raise _LoopBackRequested(source_node_id=node_id, target_node_id=target, feedback=feedback) | |
| 444 | 904 | |
| 445 | 905 | |
| 446 | 906 | def _verify_and_maybe_regenerate( | ... | ... |
src/work_graph_plan.py
| ... | ... | @@ -46,6 +46,24 @@ GRAPH_MANIFEST_SCHEMA = { |
| 46 | 46 | "enum": ["brd", "prd", "sdd", "benchmark", "test_dataset", "test_script"], |
| 47 | 47 | }, |
| 48 | 48 | "depends_on": {"type": "array", "items": {"type": "string"}}, |
| 49 | + # 2026-08-08 (Work Graph self-planning + loop-back) -- both optional, | |
| 50 | + # documentation only (this schema isn't jsonschema-enforced, see | |
| 51 | + # module docstring's --json-schema/--agent note). | |
| 52 | + "judgment": { | |
| 53 | + "type": "object", | |
| 54 | + "properties": { | |
| 55 | + "kind": {"type": "string", "enum": ["none", "mechanical", "llm"]}, | |
| 56 | + "check_ref": {"type": "string"}, | |
| 57 | + "judge_prompt": {"type": "string"}, | |
| 58 | + }, | |
| 59 | + }, | |
| 60 | + "on_fail": { | |
| 61 | + "type": "object", | |
| 62 | + "properties": { | |
| 63 | + "action": {"type": "string", "enum": ["retry_self", "loop_back", "hard_fail"]}, | |
| 64 | + "max_attempts": {"type": "integer"}, | |
| 65 | + }, | |
| 66 | + }, | |
| 49 | 67 | }, |
| 50 | 68 | }, |
| 51 | 69 | }, |
| ... | ... | @@ -57,24 +75,87 @@ GRAPH_MANIFEST_SCHEMA = { |
| 57 | 75 | "properties": { |
| 58 | 76 | "from": {"type": "string"}, |
| 59 | 77 | "to": {"type": "string"}, |
| 60 | - "edge_type": {"type": "string", "enum": ["zero_token", "needs_judgment"]}, | |
| 78 | + # "on_failure" (2026-08-08): a node's failure path, expressed the | |
| 79 | + # same way its success path already is -- see _on_failure_target | |
| 80 | + # in work_graph_orchestrate.py. | |
| 81 | + "edge_type": {"type": "string", "enum": ["zero_token", "needs_judgment", "on_failure"]}, | |
| 61 | 82 | }, |
| 62 | 83 | }, |
| 63 | 84 | }, |
| 64 | 85 | }, |
| 65 | 86 | } |
| 66 | 87 | |
| 88 | +# 2026-08-08 (Work Graph self-planning + loop-back, docs/todo/work-graph- | |
| 89 | +# self-planning-and-loop-back-requirements.md, FR-B4 "全部自由规划"): | |
| 90 | +# Planner's doc-writing chain is no longer a fixed architectural constant | |
| 91 | +# (formally reverses docs/done/dynamic-work-graph-design-skill-design.md | |
| 92 | +# §3's decision) -- the LLM decides which node types this goal needs, in | |
| 93 | +# what order, and how each one's judgment/on_fail should work. This | |
| 94 | +# catalog is reference DATA the prompt hands the LLM, not a requirement | |
| 95 | +# that every entry appears -- FR-B2. test_script (disabled 2026-08-07, | |
| 96 | +# docs/todo/validator-test-script-narrative-output-issue.md) is back in | |
| 97 | +# this catalog per that decision's §8 item 7: Part B doesn't wait for the | |
| 98 | +# narrative-output structural fix, that fix is tracked separately | |
| 99 | +# alongside a future Coder-side test_script module. | |
| 100 | +_PRD_JUDGE_PROMPT = ( | |
| 101 | + "Review the following PRD for internally CONTRADICTORY scenario assumptions -- e.g. two " | |
| 102 | + "requirements that cannot both hold at once, or the same scenario described two incompatible " | |
| 103 | + "ways. Do NOT flag mere incompleteness, ambiguity, or style issues, only genuine " | |
| 104 | + "self-contradiction. Reply via the required schema: ok=false only if you found a real " | |
| 105 | + "contradiction, and list each one in issues.\n\nPRD:\n\n{content}" | |
| 106 | +) | |
| 107 | + | |
| 108 | +_PLANNER_NODE_CATALOG = [ | |
| 109 | + {"artifact_type": "brd", "description": "Business Requirements Document -- why this goal matters, who it's for."}, | |
| 110 | + { | |
| 111 | + "artifact_type": "prd", "description": "Product Requirements Document -- concrete scenarios/behavior.", | |
| 112 | + "suggested_judgment": {"kind": "llm", "judge_prompt": _PRD_JUDGE_PROMPT}, | |
| 113 | + }, | |
| 114 | + {"artifact_type": "sdd", "description": "Solution Design Document -- how it will be implemented."}, | |
| 115 | + {"artifact_type": "benchmark", "description": "Concrete acceptance scenarios with observable pass criteria."}, | |
| 116 | + { | |
| 117 | + "artifact_type": "test_dataset", | |
| 118 | + "description": "Concrete test data (inputs + expected outputs) if this goal has a natural one -- " | |
| 119 | + "many goals (UI wiring, config changes) don't, that's fine, omit this node then.", | |
| 120 | + }, | |
| 121 | + { | |
| 122 | + "artifact_type": "test_script", | |
| 123 | + "description": "A real, runnable script that mechanically checks the Benchmark's scenarios -- " | |
| 124 | + "only include this if a genuine mechanical check is possible for this goal.", | |
| 125 | + "suggested_judgment": {"kind": "mechanical", "check_ref": "check_script_compiles"}, | |
| 126 | + }, | |
| 127 | +] | |
| 128 | + | |
| 129 | +# NOTE: no `str.format()` involved in building or filling this template | |
| 130 | +# (unlike _GENERIC_PROMPT_TEMPLATE) -- the embedded catalog JSON and the | |
| 131 | +# instructional JSON examples below (e.g. {"kind": "none"}) are full of | |
| 132 | +# their own literal braces, which .format() would try to parse as format | |
| 133 | +# fields and crash on. plan_work_graph() fills in goal_text with a plain | |
| 134 | +# `.replace("{goal_text}", goal_text)` instead, which treats every other | |
| 135 | +# brace in this string as inert literal text -- no escaping needed at all. | |
| 67 | 136 | _PROMPT_TEMPLATE = ( |
| 68 | - "Plan (do not execute) the Work Graph needed to produce BRD, PRD, SDD, Benchmark, " | |
| 69 | - "Test Dataset, and Test Script artifacts for this goal: {goal_text}\n" | |
| 70 | - "Output a graph_manifest with one node per artifact_type (brd, prd, sdd, benchmark, " | |
| 71 | - "test_dataset, test_script) and edges expressing their dependency chain (sdd depends " | |
| 72 | - "on prd depends on brd, benchmark depends on sdd, test_dataset depends on benchmark, " | |
| 73 | - "test_script depends on test_dataset). Every edge needs edge_type: 'zero_token' for " | |
| 74 | - "pure aggregation/reduce steps, 'needs_judgment' for anything requiring the agent to " | |
| 75 | - "actually reason about content.\n" | |
| 137 | + "Plan (do not execute) the Work Graph for this goal: {goal_text}\n" | |
| 138 | + "Available node types (use any subset, in any order that makes sense -- this goal may not " | |
| 139 | + "need all of them, and 'suggested_judgment' entries are starting points you can reuse or " | |
| 140 | + "override, not requirements):\n" + json.dumps(_PLANNER_NODE_CATALOG) + "\n" | |
| 141 | + "For each node you include, decide: node_id, artifact_type, depends_on (which earlier nodes' " | |
| 142 | + "output it genuinely needs), a judgment config (how to tell if this node's output is " | |
| 143 | + "acceptable -- {\"kind\": \"none\"} if nothing needs checking, {\"kind\": \"mechanical\", " | |
| 144 | + "\"check_ref\": ...} for a named mechanical check, or {\"kind\": \"llm\", \"judge_prompt\": " | |
| 145 | + "\"...{content}...\"} for your own judgment prompt with a {content} placeholder), and an " | |
| 146 | + "on_fail config (what happens if judgment fails: {\"action\": \"retry_self\"}, " | |
| 147 | + "{\"action\": \"loop_back\"}, or {\"action\": \"hard_fail\"}, each optionally with " | |
| 148 | + "\"max_attempts\"). If a node's on_fail.action is 'loop_back', you MUST also add a " | |
| 149 | + "corresponding edge in the top-level 'edges' array with edge_type 'on_failure', from that " | |
| 150 | + "node to the exact earlier node_id it should return to -- the target must be a real upstream " | |
| 151 | + "dependency of this node (something that genuinely produced input this node used), not an " | |
| 152 | + "arbitrary node. This mirrors how the success path already works: every node's 'yes, this " | |
| 153 | + "succeeded, what's next' is expressed as a normal dependency edge, and 'no, this failed, go " | |
| 154 | + "back to X' is expressed the same way, just with edge_type 'on_failure' instead. Every other " | |
| 155 | + "edge needs edge_type: 'zero_token' for pure aggregation/reduce steps, 'needs_judgment' for " | |
| 156 | + "anything requiring the agent to actually reason about content.\n" | |
| 76 | 157 | "End your reply with the graph_manifest as a single fenced ```json code block " |
| 77 | - "shaped exactly like {{\"nodes\": [...], \"edges\": [...]}} -- no extra wrapper key." | |
| 158 | + "shaped exactly like {\"nodes\": [...], \"edges\": [...]} -- no extra wrapper key." | |
| 78 | 159 | ) |
| 79 | 160 | |
| 80 | 161 | # E (2026-08-06): the generic, cross-Org-Agent path -- used for every |
| ... | ... | @@ -134,7 +215,72 @@ def _extract_graph_manifest(text): |
| 134 | 215 | return None |
| 135 | 216 | |
| 136 | 217 | |
| 137 | -_CANONICAL_DOC_CHAIN = ("brd", "prd", "sdd", "benchmark", "test_dataset", "test_script") | |
| 218 | +def _transitive_ancestors(graph_manifest, node_id): | |
| 219 | + """node_id itself + every node it (directly or transitively) depends | |
| 220 | + on, via `depends_on`/normal data-dependency edges (`on_failure` edges | |
| 221 | + excluded -- they're control-flow, not data dependency, same reasoning | |
| 222 | + as work_graph_orchestrate.py's own `_transitive_dependents`/ | |
| 223 | + `topological_layers`). Deliberately reimplemented here rather than | |
| 224 | + imported from work_graph_orchestrate.py -- the two modules don't | |
| 225 | + depend on each other today (work_graph_orchestrate.py is a lower-level | |
| 226 | + execution engine, work_graph_plan.py is planning-only) and this is a | |
| 227 | + small enough function that duplicating it doesn't justify introducing | |
| 228 | + that coupling.""" | |
| 229 | + deps = {n.get("node_id"): set(n.get("depends_on") or []) for n in graph_manifest.get("nodes", [])} | |
| 230 | + for edge in graph_manifest.get("edges", []): | |
| 231 | + if edge.get("edge_type") == "on_failure": | |
| 232 | + continue | |
| 233 | + from_id, to_id = edge.get("from"), edge.get("to") | |
| 234 | + if to_id in deps and from_id in deps: | |
| 235 | + deps[to_id].add(from_id) | |
| 236 | + | |
| 237 | + ancestors, stack = set(), [node_id] | |
| 238 | + while stack: | |
| 239 | + current = stack.pop() | |
| 240 | + if current in ancestors: | |
| 241 | + continue | |
| 242 | + ancestors.add(current) | |
| 243 | + stack.extend(deps.get(current, [])) | |
| 244 | + return ancestors | |
| 245 | + | |
| 246 | + | |
| 247 | +def _validate_loop_back_targets(manifest): | |
| 248 | + """2026-08-08 (Work Graph self-planning + loop-back): every node | |
| 249 | + declaring `on_fail.action == 'loop_back'` must have exactly one | |
| 250 | + `edge_type: 'on_failure'` edge originating from it (see | |
| 251 | + work_graph_orchestrate.py's `_on_failure_target`), and that edge's | |
| 252 | + target must be a genuine upstream dependency -- looping back to a node | |
| 253 | + that never influenced this one has no coherent meaning. Raises | |
| 254 | + PlanningFailedError (not a generic ValueError) so this surfaces the | |
| 255 | + same way every other planning failure does, before any node ever runs | |
| 256 | + -- catching a malformed loop_back here is far cheaper than discovering | |
| 257 | + it mid-execution.""" | |
| 258 | + node_ids = {n.get("node_id") for n in manifest.get("nodes", [])} | |
| 259 | + for node in manifest.get("nodes", []): | |
| 260 | + if (node.get("on_fail") or {}).get("action") != "loop_back": | |
| 261 | + continue | |
| 262 | + node_id = node.get("node_id") | |
| 263 | + on_failure_edges = [ | |
| 264 | + e for e in manifest.get("edges", []) | |
| 265 | + if e.get("from") == node_id and e.get("edge_type") == "on_failure" | |
| 266 | + ] | |
| 267 | + if len(on_failure_edges) != 1: | |
| 268 | + raise PlanningFailedError( | |
| 269 | + "node {} declares on_fail.action='loop_back' but has {} on_failure edge(s), " | |
| 270 | + "expected exactly 1".format(node_id, len(on_failure_edges)) | |
| 271 | + ) | |
| 272 | + target = on_failure_edges[0].get("to") | |
| 273 | + if target not in node_ids: | |
| 274 | + raise PlanningFailedError( | |
| 275 | + "node {}'s on_failure edge targets {!r}, which does not exist in this graph_manifest".format( | |
| 276 | + node_id, target | |
| 277 | + ) | |
| 278 | + ) | |
| 279 | + if target not in _transitive_ancestors(manifest, node_id): | |
| 280 | + raise PlanningFailedError( | |
| 281 | + "node {}'s on_failure edge targets {!r}, which is not an upstream dependency -- " | |
| 282 | + "loop_back must target a node that genuinely influenced this one".format(node_id, target) | |
| 283 | + ) | |
| 138 | 284 | |
| 139 | 285 | |
| 140 | 286 | def _normalize_graph_manifest(manifest): |
| ... | ... | @@ -150,23 +296,22 @@ def _normalize_graph_manifest(manifest): |
| 150 | 296 | depends_on or edges" fallback logic repeated independently. Mutates and |
| 151 | 297 | returns `manifest`. |
| 152 | 298 | |
| 153 | - **Canonical doc-chain enforcement (2026-08-03, real self-hosting run)**: | |
| 154 | - a real call was observed to omit the brd->prd->sdd->benchmark | |
| 155 | - dependency entirely -- no `depends_on`, no `edges` connecting them at | |
| 156 | - all, not just the wrong key names the edge-derived backfill above can | |
| 157 | - already recover from. With no dependency info to normalize from, | |
| 158 | - `topological_order()` scheduled `benchmark` before `sdd` even ran | |
| 159 | - (alphabetically first among nodes with an empty in-degree), and | |
| 160 | - `e2e_pipeline.build_prompt()` crashed with `KeyError: 'sdd_content'` | |
| 161 | - trying to format a template that needed sdd's not-yet-produced output. | |
| 162 | - `_PROMPT_TEMPLATE` always asks for exactly this fixed 4-node linear | |
| 163 | - chain -- it's a constant architectural fact about what the Planner's | |
| 164 | - doc-writing graph looks like, not something that genuinely varies call | |
| 165 | - to call, so don't leave it to chance: whenever all four canonical | |
| 166 | - artifact_types are present, deterministically enforce the known linear | |
| 167 | - order regardless of what edges/depends_on the LLM did or didn't | |
| 168 | - provide. Same "机制用代码" principle already applied elsewhere | |
| 169 | - (`work_node_run.ensure_worktree_committed`, etc.). | |
| 299 | + 2026-08-08 (Work Graph self-planning + loop-back, FR-B4 "全部自由规划"): | |
| 300 | + the canonical-doc-chain forced-order enforcement that used to live here | |
| 301 | + (2026-08-03 incident: a real call omitted brd->prd->sdd->benchmark's | |
| 302 | + dependency entirely, scheduling nodes out of order) is REMOVED, not | |
| 303 | + extended -- Planner's node set/order is no longer a fixed architectural | |
| 304 | + constant (formally reverses docs/done/dynamic-work-graph-design-skill- | |
| 305 | + design.md §3). Robustness against that same failure mode now rests | |
| 306 | + entirely on the edge-derived depends_on backfill below (unchanged) plus | |
| 307 | + `_validate_loop_back_targets` (new, below) -- there is no longer a | |
| 308 | + hardcoded canonical chain to fall back on if an LLM response omits | |
| 309 | + dependency info, so a real reproduction of the 2026-08-03 incident | |
| 310 | + shape is a genuine, currently-unmitigated risk this design accepted | |
| 311 | + (see docs/todo/work-graph-self-planning-and-loop-back-requirements.md | |
| 312 | + §7's risk list) -- FR-B3's own acceptance criterion only covers the | |
| 313 | + "wrong key names" failure mode this backfill already handles, not a | |
| 314 | + complete absence of dependency info with no canonical chain to guess from. | |
| 170 | 315 | """ |
| 171 | 316 | nodes = manifest.get("nodes", []) |
| 172 | 317 | for node in nodes: |
| ... | ... | @@ -176,33 +321,15 @@ def _normalize_graph_manifest(manifest): |
| 176 | 321 | |
| 177 | 322 | node_ids = {n.get("node_id") for n in nodes} |
| 178 | 323 | for edge in manifest.get("edges", []): |
| 324 | + if edge.get("edge_type") == "on_failure": | |
| 325 | + continue # control-flow, not a data dependency -- see _transitive_ancestors | |
| 179 | 326 | from_id, to_id = edge.get("from"), edge.get("to") |
| 180 | 327 | if from_id in node_ids and to_id in node_ids: |
| 181 | 328 | for node in nodes: |
| 182 | 329 | if node.get("node_id") == to_id and from_id not in node["depends_on"]: |
| 183 | 330 | node["depends_on"].append(from_id) |
| 184 | 331 | |
| 185 | - by_artifact_type = {n.get("artifact_type"): n for n in nodes} | |
| 186 | - if all(t in by_artifact_type for t in _CANONICAL_DOC_CHAIN): | |
| 187 | - for prev_type, cur_type in zip(_CANONICAL_DOC_CHAIN, _CANONICAL_DOC_CHAIN[1:]): | |
| 188 | - prev_id = by_artifact_type[prev_type]["node_id"] | |
| 189 | - cur_node = by_artifact_type[cur_type] | |
| 190 | - if prev_id not in cur_node["depends_on"]: | |
| 191 | - cur_node["depends_on"].append(prev_id) | |
| 192 | - | |
| 193 | - # 2026-08-06: test_script's own prompt needs {benchmark_content} | |
| 194 | - # directly (not just what survives into test_dataset_content, which | |
| 195 | - # can be a terse 'N/A: ...' with none of the original scenario | |
| 196 | - # detail) -- the plain linear chain above only gives it | |
| 197 | - # test_dataset as an immediate dependency, so add benchmark as a | |
| 198 | - # second one. e2e_pipeline._planner_prompt_builder already threads | |
| 199 | - # every *immediate* dependency's content through generically, this | |
| 200 | - # is the one canonical node that genuinely needs two. | |
| 201 | - benchmark_id = by_artifact_type["benchmark"]["node_id"] | |
| 202 | - script_node = by_artifact_type["test_script"] | |
| 203 | - if benchmark_id not in script_node["depends_on"]: | |
| 204 | - script_node["depends_on"].append(benchmark_id) | |
| 205 | - | |
| 332 | + _validate_loop_back_targets(manifest) | |
| 206 | 333 | return manifest |
| 207 | 334 | |
| 208 | 335 | |
| ... | ... | @@ -210,14 +337,18 @@ def plan_work_graph(conn, org_node, task_id, goal_text, target_folder_path, time |
| 210 | 337 | """Returns {"run_id", "graph_manifest"}. Creates one `work_graph_run` row |
| 211 | 338 | (status='planning') -- moving it to 'running'/executing it is out of scope. |
| 212 | 339 | |
| 213 | - E (2026-08-06): `org_node == 'planner'` still gets the fixed | |
| 214 | - brd/prd/sdd/benchmark/... chain prompt (_PROMPT_TEMPLATE, unchanged). | |
| 215 | - Every other org_node gets _GENERIC_PROMPT_TEMPLATE -- the cross-Org- | |
| 216 | - Agent dynamic planning path described in the design-dynamic-work-graph | |
| 217 | - Skill, whose nodes carry their own task_prompt instead of a fixed | |
| 218 | - artifact_type. Function signature/return shape are unchanged either way.""" | |
| 219 | - template = _PROMPT_TEMPLATE if org_node == "planner" else _GENERIC_PROMPT_TEMPLATE | |
| 220 | - prompt = template.format(goal_text=goal_text) | |
| 340 | + 2026-08-08: `org_node == 'planner'` gets `_PROMPT_TEMPLATE` (now a | |
| 341 | + genuine self-planning prompt over `_PLANNER_NODE_CATALOG`, see that | |
| 342 | + template's own comment for why goal_text is filled in via `.replace()` | |
| 343 | + rather than `.format()`). Every other org_node gets | |
| 344 | + `_GENERIC_PROMPT_TEMPLATE` -- the cross-Org-Agent dynamic planning path | |
| 345 | + described in the design-dynamic-work-graph Skill, whose nodes carry | |
| 346 | + their own task_prompt instead of a fixed artifact_type. Function | |
| 347 | + signature/return shape are unchanged either way.""" | |
| 348 | + if org_node == "planner": | |
| 349 | + prompt = _PROMPT_TEMPLATE.replace("{goal_text}", goal_text) | |
| 350 | + else: | |
| 351 | + prompt = _GENERIC_PROMPT_TEMPLATE.format(goal_text=goal_text) | |
| 221 | 352 | |
| 222 | 353 | result = subprocess.run( |
| 223 | 354 | [ | ... | ... |