Commit 7c46a2f281a9ad93c5abfd4fc56e7473efec00eb
1 parent
bd1404db
Phase 2b: Coder self-plans its own implementation graph via append_to_run_id
Implements org-graph-unification-phase2-universal-self-planning-design.md §2.3, §3, §6 -- Coder's implement_code() no longer runs one hardcoded synthetic "implement" node; it self-plans via plan_work_graph(conn, "coder", ..., append_to_run_id=run_id), appending its own nodes onto Planner's existing work_graph_run instead of creating a separate one. - work_graph_plan.py: per-org_node suggested node-type catalogs (_CODER_NODE_CATALOG/_VALIDATOR_NODE_CATALOG, "starting points, not requirements" same as _PLANNER_NODE_CATALOG), new judgment/on_fail/ promotes_to_artifact fields in the generic planning prompt. New plan_work_graph(..., append_to_run_id=...) parameter: merges newly planned nodes into an existing run (node_id collision -> raise, never silent overwrite), returns new_node_ids so callers can avoid reprocessing nodes an earlier stage already promoted to artifacts. New shared dispatch_build_prompt() for graphs mixing Planner-shaped (artifact_type) and Coder-shaped (task_prompt) nodes side by side. - e2e_pipeline.py: _write_completed_node_artifacts() now also promotes nodes declaring promotes_to_artifact (not just Planner's artifact_type nodes), with an only_node_ids scope so a merged-graph caller doesn't re-touch artifacts an earlier stage already wrote. implement_code() reworked around self-planning; code_diff stays Python-computed (a real git diff, not any node's own content -- no node can know the diff ahead of running it), test_report is promoted from whatever the self-planned graph declares, falling back to a synthesized concatenation if it declares none. needs_worktree=True now passed to run_graph() for Coder (found missing entirely during this rework -- harmless with the old single-node manifest, a real 03-C §3.6 gap once concurrent code-writing nodes are possible). Also fixed a real bug found while wiring this up: _review_planner_output_with_retries()'s "reuse existing artifacts" branch hardcoded run_id=None, even though every real artifact row already has a valid produced_by_run_id -- Coder had nothing to append into on that resume path. - work_graph_fork.py: _reconstruct_build_prompt now dispatches per-node (via the new shared dispatch_build_prompt) instead of requiring the whole graph to be one uniform shape -- a real regression Phase 2b would otherwise have introduced silently, since a forked merged graph can now genuinely mix both node shapes. Sequencing note (unchanged from Phase 2a's own finding): work_graph_run's org_node column removal and the ux_work_graph_run_active index narrowing (design doc §2.1/§2.2) stay deferred to Phase 2c -- Validator still creates its own separate run until Validator also appends into the shared one, so narrowing the index to task_id-only now would collide with it. python3 -m unittest discover -s src (752 tests) + discover -s supervisor -p "test_*.py" (126 tests) both green, zero regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Showing
8 changed files
with
857 additions
and
113 deletions
src/e2e_pipeline.py
| ... | ... | @@ -140,6 +140,12 @@ _DOC_PROMPTS = { |
| 140 | 140 | _CODE_INPUT_SCHEMA = {"type": "object", "required": ["node_id"]} |
| 141 | 141 | _CODE_OUTPUT_SCHEMA = {"type": "object", "required": ["response"]} |
| 142 | 142 | |
| 143 | +# 2026-08-12 (Phase 2 §6): retired as the per-node prompt now that Coder | |
| 144 | +# self-plans (implement_code() no longer runs one synthetic "implement" | |
| 145 | +# node) -- its guidance is folded into _CODER_PLANNING_CONTEXT below, | |
| 146 | +# which feeds plan_work_graph()'s planning call instead of a single | |
| 147 | +# execution call. Kept here, unused, only because deleting it isn't this | |
| 148 | +# change's job and a future revert would want it back verbatim. | |
| 143 | 149 | _CODER_PROMPT = ( |
| 144 | 150 | "Implement this goal per the approved SDD below. Write real code and real tests, run the " |
| 145 | 151 | "tests yourself, and only stop once they pass (or once you've exhausted reasonable attempts " |
| ... | ... | @@ -160,6 +166,32 @@ _CODER_PROMPT = ( |
| 160 | 166 | "Test script:\n\n{test_script_content}" |
| 161 | 167 | ) |
| 162 | 168 | |
| 169 | +# 2026-08-12 (Phase 2 §6): fed as `goal_text` into plan_work_graph(conn, | |
| 170 | +# "coder", ..., append_to_run_id=run_id) -- reuses the existing | |
| 171 | +# {goal_text} substitution point in _generic_prompt_template() rather than | |
| 172 | +# adding a new parameter to plan_work_graph() just for Coder's extra | |
| 173 | +# context (SDD/Benchmark/test dataset/script), which Planner's own | |
| 174 | +# _PROMPT_TEMPLATE call never needed. Same .replace("{goal_text}", ...) | |
| 175 | +# convention _PROMPT_TEMPLATE/_generic_prompt_template() already use (not | |
| 176 | +# .format() -- see work_graph_plan.py's own comment on why), so this | |
| 177 | +# string's remaining braces (none currently, but keep this in mind if | |
| 178 | +# edited) must stay literal, not format fields. | |
| 179 | +_CODER_PLANNING_CONTEXT = ( | |
| 180 | + "{goal_text}\n\n" | |
| 181 | + "Approved SDD:\n\n{sdd_content}\n\n" | |
| 182 | + "Benchmark (test dataset/script may be 'N/A' if this goal has none -- that's expected, not " | |
| 183 | + "an error):\n\n{benchmark_content}\n\n" | |
| 184 | + "Test dataset:\n\n{test_dataset_content}\n\n" | |
| 185 | + "Test script:\n\n{test_script_content}\n\n" | |
| 186 | + "Guidance: write real code and real tests, run the tests yourself as part of your own " | |
| 187 | + "nodes' work, and don't consider the goal done until they pass for real (or you've " | |
| 188 | + "exhausted reasonable attempts and a node explains why not). If the Planner's test script " | |
| 189 | + "above is real (not N/A), materialize it as a real file under this project's own test " | |
| 190 | + "conventions and make it pass for real -- it is not optional, it's part of this goal's " | |
| 191 | + "acceptance bar; the Planner's script is a floor, write additional tests of your own if it " | |
| 192 | + "doesn't cover everything the SDD requires." | |
| 193 | +) | |
| 194 | + | |
| 163 | 195 | _VALIDATOR_PROMPT = ( |
| 164 | 196 | "Independently verify the implementation for this goal against the Benchmark below: run " |
| 165 | 197 | "the tests yourself, don't just trust the Coder's claims. Reply with a verdict line " |
| ... | ... | @@ -486,8 +518,30 @@ def _failed_node_ids(exc): |
| 486 | 518 | return set() |
| 487 | 519 | |
| 488 | 520 | |
| 521 | +def _resolve_node_artifact_type(node, node_artifact_types): | |
| 522 | + """A node promotes to an `artifact` row if EITHER: it's in the caller's | |
| 523 | + precomputed `node_artifact_types` dict (Planner's/Validator's fixed | |
| 524 | + catalogs, where every node is a promotable artifact by construction -- | |
| 525 | + unchanged behavior), OR it declares `artifact_type` directly on | |
| 526 | + itself, OR (2026-08-12, Phase 2 §2.3) it declares `promotes_to_ | |
| 527 | + artifact: {"artifact_type": ...}` (Coder's/Validator's generic | |
| 528 | + self-planned nodes -- most of which should declare NEITHER, meaning | |
| 529 | + "this is an internal graph step, not a final deliverable"). Returns | |
| 530 | + None for a node that isn't promoting anything -- not an error, the | |
| 531 | + expected case for most self-planned nodes.""" | |
| 532 | + if node["node_id"] in node_artifact_types: | |
| 533 | + return node_artifact_types[node["node_id"]] | |
| 534 | + if "artifact_type" in node: | |
| 535 | + return node["artifact_type"] | |
| 536 | + promotes = node.get("promotes_to_artifact") | |
| 537 | + if promotes: | |
| 538 | + return promotes.get("artifact_type") | |
| 539 | + return None | |
| 540 | + | |
| 541 | + | |
| 489 | 542 | def _write_completed_node_artifacts( |
| 490 | - conn, target_folder_path, task_id, run_id, graph_manifest, node_artifact_types, ext_for, run_graph_error=None, | |
| 543 | + conn, target_folder_path, task_id, run_id, graph_manifest, node_artifact_types, ext_for, | |
| 544 | + run_graph_error=None, only_node_ids=None, | |
| 491 | 545 | ): |
| 492 | 546 | """Writes an artifact for every node that's genuinely done AND |
| 493 | 547 | verify_fn-accepted, sourced from `work_node_execution` in the DB -- |
| ... | ... | @@ -511,6 +565,15 @@ def _write_completed_node_artifacts( |
| 511 | 565 | nodes are excluded outright: their row is schema-valid ('done') but |
| 512 | 566 | verify_fn never accepted either attempt, so neither is trustworthy |
| 513 | 567 | content to persist. |
| 568 | + | |
| 569 | + 2026-08-12 (Phase 2 §2.3/§6): a node with no resolvable artifact_type | |
| 570 | + (see `_resolve_node_artifact_type`) is silently skipped, not an | |
| 571 | + error -- most Coder/Validator self-planned nodes are internal graph | |
| 572 | + steps, not final deliverables. `only_node_ids` (default None = every | |
| 573 | + node in `graph_manifest`, unchanged behavior) restricts processing to | |
| 574 | + a subset -- needed once a caller works over a MERGED graph (Coder's | |
| 575 | + `append_to_run_id`) and must avoid re-processing/re-promoting nodes an | |
| 576 | + EARLIER stage already wrote artifacts for. | |
| 514 | 577 | """ |
| 515 | 578 | done_rows = conn.execute( |
| 516 | 579 | "SELECT node_id, output_payload FROM work_node_execution WHERE run_id = ? AND status = 'done'", |
| ... | ... | @@ -529,7 +592,11 @@ def _write_completed_node_artifacts( |
| 529 | 592 | node_id = node["node_id"] |
| 530 | 593 | if node_id not in done_outputs: |
| 531 | 594 | continue |
| 532 | - artifact_type = node_artifact_types[node_id] | |
| 595 | + if only_node_ids is not None and node_id not in only_node_ids: | |
| 596 | + continue | |
| 597 | + artifact_type = _resolve_node_artifact_type(node, node_artifact_types) | |
| 598 | + if artifact_type is None: | |
| 599 | + continue | |
| 533 | 600 | deps = [ |
| 534 | 601 | artifact_ids[node_artifact_types[dep_id]] |
| 535 | 602 | for dep_id in (node.get("depends_on") or []) |
| ... | ... | @@ -743,7 +810,19 @@ def _review_planner_output_with_retries(conn, task_id, goal_text, target_folder_ |
| 743 | 810 | artifact_ids = task_execution._latest_artifact_ids( |
| 744 | 811 | conn, task_id, task_execution._DOC_ARTIFACT_TYPES |
| 745 | 812 | ) |
| 746 | - plan_result = {"run_id": None, "artifact_ids": artifact_ids} | |
| 813 | + # Phase 2 §6.2 (2026-08-12, found while wiring implement_code()'s | |
| 814 | + # new append_to_run_id requirement): this used to hardcode | |
| 815 | + # run_id=None -- harmless for Phase 1 (nothing downstream read | |
| 816 | + # it), but Phase 2's Coder self-planning needs a REAL run_id to | |
| 817 | + # append into. Every real artifact row already has one | |
| 818 | + # (produced_by_run_id is NOT NULL) -- read it off the sdd | |
| 819 | + # artifact (required a few lines below anyway, guaranteed | |
| 820 | + # present whenever `existing` is truthy) rather than inventing | |
| 821 | + # a new lookup path. | |
| 822 | + sdd_run_id = conn.execute( | |
| 823 | + "SELECT produced_by_run_id FROM artifact WHERE artifact_id = ?", (artifact_ids["sdd"],) | |
| 824 | + ).fetchone()[0] | |
| 825 | + plan_result = {"run_id": sdd_run_id, "artifact_ids": artifact_ids} | |
| 747 | 826 | else: |
| 748 | 827 | plan_result = plan_documents(conn, task_id, goal_text + feedback_suffix, target_folder_path) |
| 749 | 828 | artifact_ids = plan_result["artifact_ids"] |
| ... | ... | @@ -792,57 +871,124 @@ def plan_and_review_documents(conn, task_id, goal_text, target_folder_path, time |
| 792 | 871 | |
| 793 | 872 | |
| 794 | 873 | def implement_code( |
| 795 | - conn, task_id, sdd_content, goal_text, target_folder_path, timeout=1200, | |
| 874 | + conn, task_id, run_id, sdd_content, goal_text, target_folder_path, timeout=1200, | |
| 796 | 875 | benchmark_content="N/A: not provided", test_dataset_content="N/A: not provided", |
| 797 | 876 | test_script_content="N/A: not provided", |
| 798 | 877 | ): |
| 799 | - """Architecture §11 criterion 2: a real Coder call via c_work_node_run | |
| 800 | - (work_graph_orchestrate.run_graph with a synthetic single-node | |
| 801 | - manifest -- see module docstring for why there's no real Coder graph | |
| 802 | - planner), then registers whatever the Coder actually committed as | |
| 803 | - code_diff (the diff itself) and test_report (the Coder's own response | |
| 804 | - text, which per its migrated SOUL.md is required to include real test | |
| 805 | - commands + real output) artifacts. | |
| 878 | + """Phase 2 §6 (2026-08-12): Coder now self-plans its own implementation | |
| 879 | + graph via `plan_work_graph(..., append_to_run_id=run_id)` -- `run_id` | |
| 880 | + is Planner's own work_graph_run (created by `plan_and_review_ | |
| 881 | + documents()`), no longer a fresh row this function creates itself. | |
| 882 | + Coder's self-planned nodes get appended into that SAME run, not a | |
| 883 | + separate one -- see work_graph_plan.py's own append_to_run_id | |
| 884 | + docstring for the merge mechanics. Replaces the old synthetic | |
| 885 | + single-node "implement" manifest (architecture §11 criterion 2's | |
| 886 | + original, pre-self-planning shape). | |
| 887 | + | |
| 888 | + `code_diff` stays a Python-computed artifact (a real `git diff` across | |
| 889 | + whatever Coder's whole self-planned graph actually changed on disk, | |
| 890 | + end to end) -- not something any self-planned node's own `content` | |
| 891 | + field could meaningfully hold, since no node knows the diff ahead of | |
| 892 | + running it. `test_report` is promoted from whichever self-planned | |
| 893 | + node(s) declared `promotes_to_artifact: {"artifact_type": | |
| 894 | + "test_report"}` (the catalog's own "integration_check" entry suggests | |
| 895 | + this); if the LLM's plan didn't include one (the catalog is | |
| 896 | + suggestions, not requirements), falls back to concatenating every | |
| 897 | + self-planned node's own content in topological order -- same spirit | |
| 898 | + as the old single-node manifest's `response` field always being | |
| 899 | + Coder's own narrative of what it did, just synthesized instead of | |
| 900 | + directly given by one node. | |
| 901 | + | |
| 902 | + `needs_worktree=True` passed to run_graph() (2026-08-12, found while | |
| 903 | + doing this rework -- NOT previously passed here at all, even though | |
| 904 | + Coder is the one org_node that actually writes real code): with a | |
| 905 | + single synthetic node this was harmless (nothing to collide with | |
| 906 | + inside one run), but a genuinely self-planned graph can have 2+ | |
| 907 | + code-writing nodes in the SAME topological layer running concurrently | |
| 908 | + -- 03-C §3.6's "并行写代码的节点必须先申请 git_worktree_ref" applies for | |
| 909 | + real now, not just in theory. | |
| 806 | 910 | |
| 807 | 911 | 2026-08-06: benchmark_content/test_dataset_content/test_script_content |
| 808 | 912 | (default 'N/A: not provided' for callers that predate this, e.g. a |
| 809 | - resumed task from before this change) are threaded into _CODER_PROMPT | |
| 810 | - so the Coder must make the Planner's real test script pass for real, | |
| 811 | - not just write whatever tests it invents on its own. | |
| 913 | + resumed task from before this change) are folded into | |
| 914 | + _CODER_PLANNING_CONTEXT so Coder's planning call sees the Planner's | |
| 915 | + real test script and must make it pass for real, not just write | |
| 916 | + whatever tests it invents on its own. | |
| 812 | 917 | """ |
| 813 | 918 | before_sha = _current_commit(target_folder_path) |
| 814 | - run_id = str(uuid.uuid4()) | |
| 815 | - manifest = {"nodes": [{"node_id": "implement", "depends_on": []}], "edges": []} | |
| 816 | - conn.execute( | |
| 817 | - "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) " | |
| 818 | - "VALUES (?, ?, 'coder', ?, 'running')", | |
| 819 | - (run_id, task_id, json.dumps(manifest)), | |
| 820 | - ) | |
| 821 | - conn.commit() | |
| 822 | 919 | |
| 823 | - outputs = work_graph_orchestrate.run_graph( | |
| 824 | - conn, run_id, "coder", "coder-implementation", target_folder_path, manifest, | |
| 825 | - lambda node, outputs: _CODER_PROMPT.format( | |
| 826 | - goal_text=goal_text, sdd_content=sdd_content, benchmark_content=benchmark_content, | |
| 827 | - test_dataset_content=test_dataset_content, test_script_content=test_script_content, | |
| 828 | - ), | |
| 829 | - _CODE_INPUT_SCHEMA, _CODE_OUTPUT_SCHEMA, timeout=timeout, | |
| 920 | + planning_context = _CODER_PLANNING_CONTEXT.format( | |
| 921 | + goal_text=goal_text, sdd_content=sdd_content, benchmark_content=benchmark_content, | |
| 922 | + test_dataset_content=test_dataset_content, test_script_content=test_script_content, | |
| 923 | + ) | |
| 924 | + plan_result = work_graph_plan.plan_work_graph( | |
| 925 | + conn, "coder", task_id, planning_context, target_folder_path, | |
| 926 | + timeout=300, append_to_run_id=run_id, | |
| 927 | + ) | |
| 928 | + graph_manifest = plan_result["graph_manifest"] | |
| 929 | + new_node_ids = set(plan_result["new_node_ids"]) | |
| 930 | + | |
| 931 | + # Covers BOTH node shapes that can now coexist in this merged graph: | |
| 932 | + # Planner's own nodes (artifact_type) and Coder's just-appended ones | |
| 933 | + # (promotes_to_artifact) -- used for both _planner_prompt_builder's | |
| 934 | + # prompt construction (only its own artifact_type entries are ever | |
| 935 | + # queried) and _write_completed_node_artifacts' artifact-type/deps | |
| 936 | + # resolution below. | |
| 937 | + node_artifact_types = {} | |
| 938 | + for n in graph_manifest["nodes"]: | |
| 939 | + if "artifact_type" in n: | |
| 940 | + node_artifact_types[n["node_id"]] = n["artifact_type"] | |
| 941 | + elif n.get("promotes_to_artifact"): | |
| 942 | + node_artifact_types[n["node_id"]] = n["promotes_to_artifact"]["artifact_type"] | |
| 943 | + | |
| 944 | + build_prompt = work_graph_plan.dispatch_build_prompt( | |
| 945 | + _planner_prompt_builder(goal_text, node_artifact_types), work_graph_plan.generic_node_prompt_builder, | |
| 830 | 946 | ) |
| 831 | - response = outputs["implement"]["response"] | |
| 947 | + run_graph_error = None | |
| 948 | + outputs = None | |
| 949 | + try: | |
| 950 | + outputs = work_graph_orchestrate.run_graph( | |
| 951 | + conn, run_id, "coder", "coder-implementation", target_folder_path, graph_manifest, | |
| 952 | + build_prompt, _DOC_INPUT_SCHEMA, _DOC_OUTPUT_SCHEMA, timeout=timeout, | |
| 953 | + needs_worktree=True, mechanical_check_registry={"check_script_compiles": check_script_compiles}, | |
| 954 | + ) | |
| 955 | + except Exception as e: | |
| 956 | + run_graph_error = e | |
| 832 | 957 | |
| 833 | 958 | diff = subprocess.run( |
| 834 | 959 | ["git", "diff", before_sha, "HEAD"], cwd=target_folder_path, capture_output=True, text=True |
| 835 | 960 | ).stdout |
| 836 | - | |
| 837 | 961 | diff_result = artifact_write.write_artifact( |
| 838 | 962 | conn, target_folder_path, "code_diff", task_id, run_id, |
| 839 | 963 | ".graphlemon/runs/{}/code_diff.patch".format(task_id), diff or "(no diff -- see test_report)", |
| 840 | 964 | ) |
| 841 | - report_result = artifact_write.write_artifact( | |
| 842 | - conn, target_folder_path, "test_report", task_id, run_id, | |
| 843 | - ".graphlemon/runs/{}/test_report.md".format(task_id), response, | |
| 965 | + | |
| 966 | + artifact_ids = _write_completed_node_artifacts( | |
| 967 | + conn, target_folder_path, task_id, run_id, graph_manifest, node_artifact_types, | |
| 968 | + _planner_ext_for, run_graph_error, only_node_ids=new_node_ids, | |
| 844 | 969 | ) |
| 845 | - return {"code_diff": diff_result["artifact_id"], "test_report": report_result["artifact_id"]} | |
| 970 | + | |
| 971 | + if run_graph_error is not None: | |
| 972 | + raise run_graph_error | |
| 973 | + | |
| 974 | + if "test_report" not in artifact_ids: | |
| 975 | + # The self-planned graph didn't include a node declaring | |
| 976 | + # promotes_to_artifact: {"artifact_type": "test_report"} -- | |
| 977 | + # synthesize one so callers can still rely on this key always | |
| 978 | + # being present (same contract the old single-node manifest gave | |
| 979 | + # unconditionally via its one node's `response` field). | |
| 980 | + report_content = "\n\n---\n\n".join( | |
| 981 | + "### {}\n\n{}".format(node_id, (outputs or {}).get(node_id, {}).get("content", "")) | |
| 982 | + for node_id in new_node_ids | |
| 983 | + if node_id in (outputs or {}) | |
| 984 | + ) or "(Coder's self-planned graph produced no synthesizable report content)" | |
| 985 | + report_result = artifact_write.write_artifact( | |
| 986 | + conn, target_folder_path, "test_report", task_id, run_id, | |
| 987 | + ".graphlemon/runs/{}/test_report.md".format(task_id), report_content, | |
| 988 | + ) | |
| 989 | + artifact_ids["test_report"] = report_result["artifact_id"] | |
| 990 | + | |
| 991 | + return {"code_diff": diff_result["artifact_id"], "test_report": artifact_ids["test_report"]} | |
| 846 | 992 | |
| 847 | 993 | |
| 848 | 994 | def _parse_verdict_line(response): |
| ... | ... | @@ -1093,7 +1239,7 @@ def run_task_lifecycle(conn, task_id, target_folder_path, goal_text): |
| 1093 | 1239 | |
| 1094 | 1240 | org_node_lifecycle.trigger_org_node_full(conn, "coder", task_id, target_folder_path, _READY_PROMPT) |
| 1095 | 1241 | code_artifact_ids = implement_code( |
| 1096 | - conn, task_id, sdd_content, goal_text, target_folder_path, | |
| 1242 | + conn, task_id, plan_result["run_id"], sdd_content, goal_text, target_folder_path, | |
| 1097 | 1243 | benchmark_content=benchmark_content, test_dataset_content=test_dataset_content, |
| 1098 | 1244 | test_script_content=test_script_content, |
| 1099 | 1245 | ) | ... | ... |
src/task_execution.py
| ... | ... | @@ -165,7 +165,7 @@ def run_or_resume_task_lifecycle(conn, task_id, target_folder_path, goal_text): |
| 165 | 165 | _check_not_paused(conn, task_id) |
| 166 | 166 | org_node_lifecycle.trigger_org_node_full(conn, "coder", task_id, target_folder_path, e2e_pipeline._READY_PROMPT) |
| 167 | 167 | code_artifact_ids = e2e_pipeline.implement_code( |
| 168 | - conn, task_id, sdd_content, goal_text, target_folder_path, | |
| 168 | + conn, task_id, plan_result["run_id"], sdd_content, goal_text, target_folder_path, | |
| 169 | 169 | benchmark_content=benchmark_content, test_dataset_content=test_dataset_content, |
| 170 | 170 | test_script_content=test_script_content, |
| 171 | 171 | ) | ... | ... |
src/test_e2e_pipeline.py
| ... | ... | @@ -99,6 +99,56 @@ def _fake_plan_subprocess_run(*args, **kwargs): |
| 99 | 99 | return subprocess.CompletedProcess(args=["claude"], returncode=0, stdout=json.dumps(payload), stderr="") |
| 100 | 100 | |
| 101 | 101 | |
| 102 | +# Phase 2 §6 (2026-08-12): Coder now self-plans too (implement_code() calls | |
| 103 | +# plan_work_graph(conn, "coder", ..., append_to_run_id=...), going through | |
| 104 | +# the SAME mocked subprocess boundary as Planner's own planning call) -- | |
| 105 | +# depends_on Planner's sdd node so it lands in a LATER topological layer | |
| 106 | +# than Planner's already-'done' nodes, not the same one (both being | |
| 107 | +# in-degree-zero would trigger run_graph()'s concurrent ThreadPoolExecutor | |
| 108 | +# layer path purely as a mocking artifact, unrelated to what this test is | |
| 109 | +# actually checking). | |
| 110 | +_FAKE_CODER_GRAPH_MANIFEST = { | |
| 111 | + "nodes": [{ | |
| 112 | + "node_id": "impl", "depends_on": ["sdd"], "task_prompt": "implement it", | |
| 113 | + "promotes_to_artifact": {"artifact_type": "test_report"}, | |
| 114 | + }], | |
| 115 | + "edges": [], | |
| 116 | +} | |
| 117 | + | |
| 118 | + | |
| 119 | +_real_subprocess_run = subprocess.run # captured before any test patches subprocess.run globally | |
| 120 | + | |
| 121 | + | |
| 122 | +def _fake_plan_subprocess_run_dispatch(*args, **kwargs): | |
| 123 | + """Real plan_work_graph() dispatches by the --agent argv value -- this | |
| 124 | + mock does the same, so both Planner's and Coder's planning calls (which | |
| 125 | + now share the same mocked subprocess.run patch target) each get their | |
| 126 | + own realistic fake manifest instead of one hardcoded response serving | |
| 127 | + both callers (which would hand Coder Planner's own brd/prd/... nodes, | |
| 128 | + colliding with the ones already in the run it's appending onto). | |
| 129 | + | |
| 130 | + patch("src.work_graph_plan.subprocess.run", ...) patches subprocess.run | |
| 131 | + GLOBALLY, not just within work_graph_plan.py -- `subprocess` is the same | |
| 132 | + singleton module object everywhere it's imported, so this side_effect | |
| 133 | + also intercepts every OTHER real subprocess.run call this test's own | |
| 134 | + call chain makes (artifact_write.write_artifact's `git add`/`git | |
| 135 | + commit`, e2e_pipeline's `git diff`/`git rev-parse`, etc). Those aren't | |
| 136 | + `claude --agent ... -p ...` calls -- pass them through to the REAL | |
| 137 | + subprocess.run instead of returning a canned fake "claude planning | |
| 138 | + response" CompletedProcess for them too (which happened to not visibly | |
| 139 | + break anything before this dispatch function existed, only because | |
| 140 | + nothing downstream read those specific calls' .stdout in a way that | |
| 141 | + exposed it -- still wrong to keep doing on purpose now that this | |
| 142 | + function actually inspects argv).""" | |
| 143 | + argv = args[0] | |
| 144 | + if not argv or argv[0] != "claude": | |
| 145 | + return _real_subprocess_run(*args, **kwargs) | |
| 146 | + org_node = argv[argv.index("--agent") + 1] | |
| 147 | + manifest = _FAKE_CODER_GRAPH_MANIFEST if org_node == "coder" else _FAKE_GRAPH_MANIFEST | |
| 148 | + payload = {"is_error": False, "result": "```json\n" + json.dumps(manifest) + "\n```"} | |
| 149 | + return subprocess.CompletedProcess(args=["claude"], returncode=0, stdout=json.dumps(payload), stderr="") | |
| 150 | + | |
| 151 | + | |
| 102 | 152 | def _fake_plan_subprocess_run_with_test_script(*args, **kwargs): |
| 103 | 153 | payload = { |
| 104 | 154 | "is_error": False, |
| ... | ... | @@ -117,7 +167,11 @@ class PipelineTestBase(unittest.TestCase): |
| 117 | 167 | self.tmp = tempfile.TemporaryDirectory() |
| 118 | 168 | self.addCleanup(self.tmp.cleanup) |
| 119 | 169 | subprocess.run(["git", "init"], cwd=self.tmp.name, check=True, capture_output=True) |
| 120 | - self.conn = schema.connect(":memory:") | |
| 170 | + # check_same_thread=False (2026-08-12, Phase 2 §6) -- see | |
| 171 | + # test_task_execution.py's TaskExecutionTestBase for why: a merged | |
| 172 | + # graph's concurrent-layer path can now be reached even in tests | |
| 173 | + # that don't otherwise care about G's parallel execution. | |
| 174 | + self.conn = schema.connect(":memory:", check_same_thread=False) | |
| 121 | 175 | schema.create_schema(self.conn) |
| 122 | 176 | project_scaffold.create_project(self.conn, self.tmp.name, init_git_if_missing=False) |
| 123 | 177 | |
| ... | ... | @@ -125,6 +179,94 @@ class PipelineTestBase(unittest.TestCase): |
| 125 | 179 | self.conn.close() |
| 126 | 180 | |
| 127 | 181 | |
| 182 | +class WriteCompletedNodeArtifactsTest(PipelineTestBase): | |
| 183 | + """Phase 2 §2.3/§6 (2026-08-12) direct coverage: promotes_to_artifact | |
| 184 | + resolution and only_node_ids scoping, tested directly rather than only | |
| 185 | + incidentally through plan_documents()/implement_code() integration | |
| 186 | + tests.""" | |
| 187 | + | |
| 188 | + def setUp(self): | |
| 189 | + super().setUp() | |
| 190 | + self.conn.execute( | |
| 191 | + "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " | |
| 192 | + "VALUES ('task-1', (SELECT project_id FROM project), 'goal', 'goal', 'coding', 'coder')" | |
| 193 | + ) | |
| 194 | + self.conn.execute( | |
| 195 | + "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) " | |
| 196 | + "VALUES ('run-1', 'task-1', 'coder', '{}', 'running')" | |
| 197 | + ) | |
| 198 | + self.conn.execute( | |
| 199 | + "INSERT INTO skill (skill_id, owner_org_node, file_path, current_git_commit_sha, source, status) " | |
| 200 | + "VALUES ('coder-implementation', 'coder', 'skills/coder-implementation/', 'unknown', 'new', 'active')" | |
| 201 | + ) | |
| 202 | + self.conn.commit() | |
| 203 | + | |
| 204 | + def _mark_done(self, node_id, content): | |
| 205 | + self.conn.execute( | |
| 206 | + "INSERT INTO work_node_execution (node_exec_id, run_id, node_id, subagent_type, skill_ref, " | |
| 207 | + "input_schema_ref, output_schema_ref, input_payload, output_payload, status) " | |
| 208 | + "VALUES (?, 'run-1', ?, 'coder', 'coder-implementation', 'in.json', 'out.json', '{}', ?, 'done')", | |
| 209 | + ("node-exec-" + node_id, node_id, json.dumps({"content": content})), | |
| 210 | + ) | |
| 211 | + self.conn.commit() | |
| 212 | + | |
| 213 | + def test_node_with_no_artifact_type_or_promotes_to_artifact_is_skipped(self): | |
| 214 | + # most self-planned Coder nodes are internal graph steps, not final | |
| 215 | + # deliverables -- must be silently skipped, not an error. | |
| 216 | + manifest = {"nodes": [{"node_id": "explore", "depends_on": []}], "edges": []} | |
| 217 | + self._mark_done("explore", "some intermediate reasoning") | |
| 218 | + artifact_ids = e2e_pipeline._write_completed_node_artifacts( | |
| 219 | + self.conn, self.tmp.name, "task-1", "run-1", manifest, {}, lambda t: "md", | |
| 220 | + ) | |
| 221 | + self.assertEqual(artifact_ids, {}) | |
| 222 | + count = self.conn.execute("SELECT COUNT(*) FROM artifact WHERE task_id = 'task-1'").fetchone()[0] | |
| 223 | + self.assertEqual(count, 0) | |
| 224 | + | |
| 225 | + def test_promotes_to_artifact_node_is_persisted(self): | |
| 226 | + manifest = { | |
| 227 | + "nodes": [{ | |
| 228 | + "node_id": "final_report", "depends_on": [], | |
| 229 | + "promotes_to_artifact": {"artifact_type": "test_report"}, | |
| 230 | + }], | |
| 231 | + "edges": [], | |
| 232 | + } | |
| 233 | + self._mark_done("final_report", "# Real report content") | |
| 234 | + node_artifact_types = {"final_report": "test_report"} | |
| 235 | + artifact_ids = e2e_pipeline._write_completed_node_artifacts( | |
| 236 | + self.conn, self.tmp.name, "task-1", "run-1", manifest, node_artifact_types, lambda t: "md", | |
| 237 | + ) | |
| 238 | + self.assertEqual(set(artifact_ids), {"test_report"}) | |
| 239 | + content = self.conn.execute( | |
| 240 | + "SELECT file_path FROM artifact WHERE artifact_id = ?", (artifact_ids["test_report"],) | |
| 241 | + ).fetchone()[0] | |
| 242 | + with open(os.path.join(self.tmp.name, content)) as f: | |
| 243 | + self.assertEqual(f.read(), "# Real report content") | |
| 244 | + | |
| 245 | + def test_only_node_ids_scopes_to_new_nodes_skipping_earlier_promotable_ones(self): | |
| 246 | + # a merged graph (Planner's already-artifact'd nodes + Coder's just- | |
| 247 | + # appended ones) must not re-promote/re-process nodes an EARLIER | |
| 248 | + # stage already wrote artifacts for, even though they'd otherwise | |
| 249 | + # be individually promotable. | |
| 250 | + manifest = { | |
| 251 | + "nodes": [ | |
| 252 | + {"node_id": "sdd", "artifact_type": "sdd", "depends_on": []}, | |
| 253 | + { | |
| 254 | + "node_id": "impl", "depends_on": ["sdd"], | |
| 255 | + "promotes_to_artifact": {"artifact_type": "test_report"}, | |
| 256 | + }, | |
| 257 | + ], | |
| 258 | + "edges": [], | |
| 259 | + } | |
| 260 | + self._mark_done("sdd", "# SDD content") | |
| 261 | + self._mark_done("impl", "# Report content") | |
| 262 | + node_artifact_types = {"sdd": "sdd", "impl": "test_report"} | |
| 263 | + artifact_ids = e2e_pipeline._write_completed_node_artifacts( | |
| 264 | + self.conn, self.tmp.name, "task-1", "run-1", manifest, node_artifact_types, lambda t: "md", | |
| 265 | + only_node_ids={"impl"}, | |
| 266 | + ) | |
| 267 | + self.assertEqual(set(artifact_ids), {"test_report"}) # NOT "sdd" -- outside only_node_ids | |
| 268 | + | |
| 269 | + | |
| 128 | 270 | class PlanDocumentsTest(PipelineTestBase): |
| 129 | 271 | def test_produces_five_artifacts_with_correct_lineage(self): |
| 130 | 272 | self.conn.execute( |
| ... | ... | @@ -228,7 +370,7 @@ class PlanDocumentsTest(PipelineTestBase): |
| 228 | 370 | class RunEndToEndTest(PipelineTestBase): |
| 229 | 371 | def test_full_pipeline_wiring(self): |
| 230 | 372 | with patch("src.org_node_lifecycle.trigger_org_node", side_effect=_fake_ready_trigger), \ |
| 231 | - patch("src.work_graph_plan.subprocess.run", side_effect=_fake_plan_subprocess_run), \ | |
| 373 | + patch("src.work_graph_plan.subprocess.run", side_effect=_fake_plan_subprocess_run_dispatch), \ | |
| 232 | 374 | patch.object(work_graph_execute, "write_workflow_script"), \ |
| 233 | 375 | patch.object(work_graph_execute, "execute_work_graph") as mock_execute, \ |
| 234 | 376 | patch.object(org_node_trigger, "trigger_org_node", return_value="VERDICT: PASS\nAll scenarios pass."): |
| ... | ... | @@ -237,7 +379,11 @@ class RunEndToEndTest(PipelineTestBase): |
| 237 | 379 | {"consistent": True, "issues": []}, # PRD consistency check (REQ-02 boundary) |
| 238 | 380 | {"content": _FAKE_DOCS["sdd"]}, {"content": _FAKE_DOCS["benchmark"]}, |
| 239 | 381 | {"content": _FAKE_DOCS["test_dataset"]}, |
| 240 | - {"response": "Implemented. Ran `pytest`: 3 passed."}, | |
| 382 | + # Phase 2 §6 (2026-08-12): Coder self-plans now -- one | |
| 383 | + # self-planned "impl" node (_FAKE_CODER_GRAPH_MANIFEST), | |
| 384 | + # content-keyed like every other self-planned node, not the | |
| 385 | + # old single synthetic node's response-keyed output. | |
| 386 | + {"content": "Implemented. Ran `pytest`: 3 passed."}, | |
| 241 | 387 | # Validator's OWN independent benchmark/dataset (2026-08-06) -- |
| 242 | 388 | # deliberately different content from the Planner's, to also prove |
| 243 | 389 | # nothing here accidentally reuses the Planner's copies. |
| ... | ... | @@ -265,15 +411,31 @@ class RunEndToEndTest(PipelineTestBase): |
| 265 | 411 | # (test_script/validator_test_script disabled, see _VALIDATOR_MANIFEST) |
| 266 | 412 | self.assertEqual(all_artifacts, 9) |
| 267 | 413 | |
| 268 | - # planner, coder, AND validator (2026-08-06: validator now has its own | |
| 269 | - # real work_graph_run too, not a flat direct call) each got their own | |
| 270 | - # work_graph_run (not sharing one) | |
| 271 | - run_org_nodes = { | |
| 414 | + # Phase 2 §6 (2026-08-12): Coder no longer gets its own work_graph_run | |
| 415 | + # -- implement_code() appends its self-planned nodes onto Planner's | |
| 416 | + # SAME run (plan_work_graph(..., append_to_run_id=...)), so only TWO | |
| 417 | + # rows exist now, not three: the shared Planner+Coder run (still | |
| 418 | + # labeled org_node='planner' -- that column reflects who ORIGINATED | |
| 419 | + # the run, not every org_node that has since contributed nodes to it; | |
| 420 | + # Phase 2c is expected to fold Validator into the same run too, at | |
| 421 | + # which point this column stops being a reliable "who's involved" | |
| 422 | + # signal at all, see design doc §2.1) and Validator's own separate run. | |
| 423 | + run_rows = self.conn.execute( | |
| 424 | + "SELECT run_id, org_node FROM work_graph_run WHERE task_id = ?", (result["task_id"],) | |
| 425 | + ).fetchall() | |
| 426 | + self.assertEqual(len(run_rows), 2) | |
| 427 | + self.assertEqual({row[1] for row in run_rows}, {"planner", "validator"}) | |
| 428 | + | |
| 429 | + # Coder's self-planned "impl" node genuinely landed in the SAME run | |
| 430 | + # as Planner's nodes, not a separate one. | |
| 431 | + planner_run_id = next(row[0] for row in run_rows if row[1] == "planner") | |
| 432 | + node_ids = { | |
| 272 | 433 | row[0] for row in self.conn.execute( |
| 273 | - "SELECT org_node FROM work_graph_run WHERE task_id = ?", (result["task_id"],) | |
| 434 | + "SELECT node_id FROM work_node_execution WHERE run_id = ?", (planner_run_id,) | |
| 274 | 435 | ).fetchall() |
| 275 | 436 | } |
| 276 | - self.assertEqual(run_org_nodes, {"planner", "coder", "validator"}) | |
| 437 | + self.assertIn("impl", node_ids) | |
| 438 | + self.assertIn("sdd", node_ids) | |
| 277 | 439 | |
| 278 | 440 | |
| 279 | 441 | class LooksLikeNarrationTest(unittest.TestCase): |
| ... | ... | @@ -437,7 +599,7 @@ class RunTaskLifecycleMissingArtifactTypesTest(PipelineTestBase): |
| 437 | 599 | def test_prd_and_benchmark_missing_does_not_raise_key_error(self): |
| 438 | 600 | captured = {} |
| 439 | 601 | |
| 440 | - def fake_implement_code(conn, task_id, sdd_content, goal_text, target_folder_path, **kwargs): | |
| 602 | + def fake_implement_code(conn, task_id, run_id, sdd_content, goal_text, target_folder_path, **kwargs): | |
| 441 | 603 | captured["benchmark_content"] = kwargs.get("benchmark_content") |
| 442 | 604 | return {"code_diff": "diff-1", "test_report": "report-1"} |
| 443 | 605 | ... | ... |
src/test_task_execution.py
| ... | ... | @@ -14,7 +14,10 @@ import threading |
| 14 | 14 | import unittest |
| 15 | 15 | from unittest.mock import patch |
| 16 | 16 | |
| 17 | -from src import e2e_pipeline, org_node_lifecycle, org_node_trigger, project_scaffold, schema, scheduler, task_execution, work_graph_execute | |
| 17 | +from src import ( | |
| 18 | + e2e_pipeline, org_node_lifecycle, org_node_trigger, project_scaffold, schema, scheduler, | |
| 19 | + task_execution, work_graph_execute, work_graph_plan, | |
| 20 | +) | |
| 18 | 21 | |
| 19 | 22 | |
| 20 | 23 | def _fake_ready_trigger(org_node, prompt, target_folder_path, timeout=300): |
| ... | ... | @@ -22,12 +25,46 @@ def _fake_ready_trigger(org_node, prompt, target_folder_path, timeout=300): |
| 22 | 25 | return "Ready." |
| 23 | 26 | |
| 24 | 27 | |
| 28 | +# Phase 2 §6 (2026-08-12): implement_code() now self-plans via | |
| 29 | +# plan_work_graph(conn, "coder", ..., append_to_run_id=...) instead of | |
| 30 | +# running one hardcoded synthetic node -- these tests aren't about self- | |
| 31 | +# planning mechanics (that's test_e2e_pipeline.py's job), so this fakes | |
| 32 | +# the planning subprocess with the smallest possible one-node manifest, | |
| 33 | +# keeping the rest of each test's mock_execute.side_effect shape close to | |
| 34 | +# what it was before this change (one node execution slot for Coder, | |
| 35 | +# {"content": ...} instead of the old single-node manifest's {"response": ...}). | |
| 36 | +_FAKE_CODER_MANIFEST = { | |
| 37 | + "nodes": [{ | |
| 38 | + "node_id": "impl", "depends_on": [], "task_prompt": "implement it", | |
| 39 | + "promotes_to_artifact": {"artifact_type": "test_report"}, | |
| 40 | + }], | |
| 41 | + "edges": [], | |
| 42 | +} | |
| 43 | + | |
| 44 | + | |
| 45 | +def _fake_coder_plan_subprocess_run(*args, **kwargs): | |
| 46 | + payload = {"is_error": False, "result": "```json\n" + json.dumps(_FAKE_CODER_MANIFEST) + "\n```"} | |
| 47 | + return subprocess.CompletedProcess(args=["claude"], returncode=0, stdout=json.dumps(payload), stderr="") | |
| 48 | + | |
| 49 | + | |
| 25 | 50 | class TaskExecutionTestBase(unittest.TestCase): |
| 26 | 51 | def setUp(self): |
| 27 | 52 | self.tmp = tempfile.TemporaryDirectory() |
| 28 | 53 | self.addCleanup(self.tmp.cleanup) |
| 29 | 54 | subprocess.run(["git", "init"], cwd=self.tmp.name, check=True, capture_output=True) |
| 30 | - self.conn = schema.connect(":memory:") | |
| 55 | + # check_same_thread=False (2026-08-12, Phase 2 §6): a merged graph | |
| 56 | + # can legitimately put an already-'done' node (Planner's, short- | |
| 57 | + # circuiting instantly) in the SAME topological layer as a newly- | |
| 58 | + # appended node with no depends_on between them (e.g. Coder's own | |
| 59 | + # self-planned node happens not to depend on Planner's) -- | |
| 60 | + # work_graph_orchestrate.run_graph()'s multi-node-layer path always | |
| 61 | + # uses a ThreadPoolExecutor once len(pending) > 1, regardless of how | |
| 62 | + # fast one of them finishes. Real production connections already do | |
| 63 | + # this (scheduler.py/task_execution.py's own start_task_execution_ | |
| 64 | + # in_background, same "G" reasoning) -- this test fixture just | |
| 65 | + # hadn't needed it before self-planning could produce concurrent | |
| 66 | + # layers spanning old + new nodes. | |
| 67 | + self.conn = schema.connect(":memory:", check_same_thread=False) | |
| 31 | 68 | schema.create_schema(self.conn) |
| 32 | 69 | project = project_scaffold.create_project(self.conn, self.tmp.name, init_git_if_missing=False) |
| 33 | 70 | self.project_id = project["project_id"] |
| ... | ... | @@ -45,9 +82,18 @@ class TaskExecutionTestBase(unittest.TestCase): |
| 45 | 82 | |
| 46 | 83 | def _write_artifact(self, task_id, artifact_type, content): |
| 47 | 84 | run_id = "run-" + artifact_type |
| 85 | + # Phase 2 §6 (2026-08-12): graph_manifest is no longer a throwaway | |
| 86 | + # '{}' -- implement_code() now reads it back (plan_work_graph's | |
| 87 | + # append_to_run_id mode) whenever it appends onto the run the sdd | |
| 88 | + # artifact points at, so it needs a real "nodes"/"edges" shape, not | |
| 89 | + # an empty object real production data would never actually have. | |
| 90 | + manifest = json.dumps({ | |
| 91 | + "nodes": [{"node_id": artifact_type, "artifact_type": artifact_type, "depends_on": []}], | |
| 92 | + "edges": [], | |
| 93 | + }) | |
| 48 | 94 | self.conn.execute( |
| 49 | 95 | "INSERT OR IGNORE INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) " |
| 50 | - "VALUES (?, ?, 'planner', '{}', 'done')", (run_id, task_id), | |
| 96 | + "VALUES (?, ?, 'planner', ?, 'done')", (run_id, task_id, manifest), | |
| 51 | 97 | ) |
| 52 | 98 | rel_path = ".graphlemon/runs/{}/{}.md".format(task_id, artifact_type) |
| 53 | 99 | abs_path = os.path.join(self.tmp.name, rel_path) |
| ... | ... | @@ -122,12 +168,13 @@ class ResumeFromPlanningTest(TaskExecutionTestBase): |
| 122 | 168 | self._write_artifact("task-1", "benchmark", "# Benchmark content") |
| 123 | 169 | |
| 124 | 170 | with patch.object(org_node_lifecycle, "trigger_org_node", side_effect=_fake_ready_trigger), \ |
| 171 | + patch.object(work_graph_plan.subprocess, "run", side_effect=_fake_coder_plan_subprocess_run), \ | |
| 125 | 172 | patch.object(work_graph_execute, "write_workflow_script"), \ |
| 126 | 173 | patch.object(work_graph_execute, "execute_work_graph") as mock_execute, \ |
| 127 | 174 | patch.object(org_node_trigger, "trigger_org_node", return_value="VERDICT: PASS") as mock_trigger, \ |
| 128 | 175 | patch.object(e2e_pipeline, "plan_documents") as mock_plan: |
| 129 | 176 | mock_execute.side_effect = [ |
| 130 | - {"response": "done, tests pass"}, # coder's implement node | |
| 177 | + {"content": "done, tests pass"}, # coder's self-planned "impl" node | |
| 131 | 178 | # validator's OWN benchmark/test_dataset/test_script graph (2026-08-06) |
| 132 | 179 | {"content": "# Validator's own Benchmark"}, |
| 133 | 180 | {"content": "N/A: no dataset needed"}, |
| ... | ... | @@ -169,12 +216,13 @@ class ResumeFromReviewingTest(TaskExecutionTestBase): |
| 169 | 216 | return _fake_ready_trigger(org_node, prompt, target_folder_path, timeout) |
| 170 | 217 | |
| 171 | 218 | with patch.object(org_node_lifecycle, "trigger_org_node", side_effect=tracking_ready_trigger), \ |
| 219 | + patch.object(work_graph_plan.subprocess, "run", side_effect=_fake_coder_plan_subprocess_run), \ | |
| 172 | 220 | patch.object(work_graph_execute, "write_workflow_script"), \ |
| 173 | 221 | patch.object(work_graph_execute, "execute_work_graph") as mock_execute, \ |
| 174 | 222 | patch.object(org_node_trigger, "trigger_org_node", return_value="VERDICT: PASS") as mock_trigger, \ |
| 175 | 223 | patch.object(e2e_pipeline, "plan_documents") as mock_plan: |
| 176 | 224 | mock_execute.side_effect = [ |
| 177 | - {"response": "done, tests pass"}, # coder's implement node | |
| 225 | + {"content": "done, tests pass"}, # coder's self-planned "impl" node | |
| 178 | 226 | {"content": "# Validator's own Benchmark"}, |
| 179 | 227 | {"content": "N/A: no dataset needed"}, |
| 180 | 228 | {"content": "import sys\nsys.exit(0)\n"}, |
| ... | ... | @@ -209,16 +257,22 @@ class ResumeFromPlanningWithZeroArtifactsTest(TaskExecutionTestBase): |
| 209 | 257 | # writes a real, validation_status='passed' sdd artifact, since |
| 210 | 258 | # org_node_lifecycle._REQUIRED_ARTIFACTS["coder"] checks the real |
| 211 | 259 | # `artifact` table, not just this function's Python return value. |
| 260 | + # run_id matches what _write_artifact's own test fixture convention | |
| 261 | + # actually creates ("run-" + artifact_type) -- a real plan_documents() | |
| 262 | + # call's return value would likewise point at the real work_graph_run | |
| 263 | + # row it just inserted, which Phase 2's implement_code() now needs to | |
| 264 | + # append into. | |
| 212 | 265 | self._write_artifact(task_id, "sdd", "# SDD\nfreshly regenerated") |
| 213 | - return {"run_id": "run-x", "artifact_ids": {"sdd": "art-sdd"}} | |
| 266 | + return {"run_id": "run-sdd", "artifact_ids": {"sdd": "art-sdd"}} | |
| 214 | 267 | |
| 215 | 268 | with patch.object(org_node_lifecycle, "trigger_org_node", side_effect=_fake_ready_trigger), \ |
| 269 | + patch.object(work_graph_plan.subprocess, "run", side_effect=_fake_coder_plan_subprocess_run), \ | |
| 216 | 270 | patch.object(work_graph_execute, "write_workflow_script"), \ |
| 217 | 271 | patch.object(work_graph_execute, "execute_work_graph") as mock_execute, \ |
| 218 | 272 | patch.object(org_node_trigger, "trigger_org_node", return_value="VERDICT: PASS"), \ |
| 219 | 273 | patch.object(e2e_pipeline, "plan_documents", side_effect=fake_plan_documents) as mock_plan: |
| 220 | 274 | mock_execute.side_effect = [ |
| 221 | - {"response": "done, tests pass"}, # coder's implement node | |
| 275 | + {"content": "done, tests pass"}, # coder's self-planned "impl" node | |
| 222 | 276 | {"content": "# Validator's own Benchmark"}, |
| 223 | 277 | {"content": "N/A: no dataset needed"}, |
| 224 | 278 | {"content": "import sys\nsys.exit(0)\n"}, | ... | ... |
src/test_work_graph_fork.py
| ... | ... | @@ -139,6 +139,50 @@ class ForkFromPrdAcceptanceCriterion1Test(ForkTestBase): |
| 139 | 139 | self.assertEqual(run_row, ("done", "run-orig", node_exec_ids["prd"])) |
| 140 | 140 | |
| 141 | 141 | |
| 142 | +class ForkFromMixedShapeGraphTest(ForkTestBase): | |
| 143 | + """Phase 2 §2.1.2 (2026-08-12) regression: a graph_manifest can now | |
| 144 | + genuinely mix Planner-shaped nodes ('artifact_type') and Coder-shaped | |
| 145 | + nodes ('task_prompt') side by side, once plan_work_graph(..., | |
| 146 | + append_to_run_id=...) appends Coder's self-planned nodes onto | |
| 147 | + Planner's original run. Forking from the artifact_type node must | |
| 148 | + still correctly reconstruct a build_prompt for the DOWNSTREAM | |
| 149 | + task_prompt node -- before this fix, _reconstruct_build_prompt | |
| 150 | + required the WHOLE graph to be one uniform shape and would have | |
| 151 | + raised ForkGraphShapeUnsupportedError here.""" | |
| 152 | + | |
| 153 | + def test_downstream_generic_node_reexecutes_after_forking_upstream_planner_node(self): | |
| 154 | + manifest = { | |
| 155 | + "nodes": [ | |
| 156 | + {"node_id": "sdd", "artifact_type": "sdd", "depends_on": []}, | |
| 157 | + { | |
| 158 | + "node_id": "impl", "depends_on": ["sdd"], "task_prompt": "implement per the sdd", | |
| 159 | + "promotes_to_artifact": {"artifact_type": "test_report"}, | |
| 160 | + }, | |
| 161 | + ], | |
| 162 | + "edges": [], | |
| 163 | + } | |
| 164 | + contents = {"sdd": {"content": "original sdd"}, "impl": {"content": "original impl"}} | |
| 165 | + node_exec_ids = self._seed_completed_run("run-orig", "planner", manifest, contents) | |
| 166 | + | |
| 167 | + execute_calls = [] | |
| 168 | + | |
| 169 | + def fake_execute(cwd, name, args, output_schema, timeout=600): | |
| 170 | + execute_calls.append(name) | |
| 171 | + return {"content": "regenerated for " + name} | |
| 172 | + | |
| 173 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | |
| 174 | + patch.object(work_graph_execute, "execute_work_graph", side_effect=fake_execute): | |
| 175 | + result = work_graph_fork.fork_work_graph_run( | |
| 176 | + self.conn, node_exec_ids["sdd"], {"content": "a completely different sdd"}, "/tmp/proj-1", | |
| 177 | + ) | |
| 178 | + | |
| 179 | + # only "impl" (downstream of the fork point) genuinely re-ran. | |
| 180 | + self.assertEqual(len(execute_calls), 1) | |
| 181 | + outputs = result["outputs"] | |
| 182 | + self.assertEqual(outputs["sdd"], {"content": "a completely different sdd"}) | |
| 183 | + self.assertNotEqual(outputs["impl"], {"content": "original impl"}) | |
| 184 | + | |
| 185 | + | |
| 142 | 186 | class OriginalRunUnaffectedTest(ForkTestBase): |
| 143 | 187 | """Acceptance standard 2: the original run's records are completely |
| 144 | 188 | untouched by a fork.""" | ... | ... |
src/test_work_graph_plan.py
| ... | ... | @@ -399,6 +399,129 @@ class GenericPlanningPromptTest(PlanWorkGraphTestBase): |
| 399 | 399 | self.assertNotIn("BRD, PRD, SDD, Benchmark", prompt) |
| 400 | 400 | self.assertIn("task_prompt", prompt) |
| 401 | 401 | |
| 402 | + def test_coder_prompt_includes_its_own_suggested_catalog_not_validators(self): | |
| 403 | + # Phase 2 §3 (2026-08-12): each org_node gets its OWN suggested | |
| 404 | + # node-type catalog, not a shared one -- "starting points, not | |
| 405 | + # requirements", same contract _PLANNER_NODE_CATALOG already uses. | |
| 406 | + with patch("src.work_graph_plan.subprocess.run") as run: | |
| 407 | + run.return_value = _completed_process( | |
| 408 | + stdout_obj={"is_error": False, "result": _fenced_result(_CODER_THREE_MODULE_MANIFEST)} | |
| 409 | + ) | |
| 410 | + work_graph_plan.plan_work_graph(self.conn, "coder", "task-1", "goal", "/tmp/proj-1") | |
| 411 | + prompt = run.call_args.args[0][run.call_args.args[0].index("-p") + 1] | |
| 412 | + self.assertIn("Suggested node types for this goal", prompt) | |
| 413 | + self.assertIn('"node_type": "unit_test"', prompt) | |
| 414 | + self.assertIn('"node_type": "integration_check"', prompt) | |
| 415 | + self.assertNotIn('"node_type": "independent_benchmark"', prompt) # validator-only | |
| 416 | + self.assertIn("promotes_to_artifact", prompt) | |
| 417 | + self.assertIn("dynamic_loop_back", prompt) # explicitly told NOT to use it | |
| 418 | + | |
| 419 | + def test_validator_prompt_includes_its_own_catalog_not_coders(self): | |
| 420 | + with patch("src.work_graph_plan.subprocess.run") as run: | |
| 421 | + run.return_value = _completed_process( | |
| 422 | + stdout_obj={"is_error": False, "result": _fenced_result(_CODER_THREE_MODULE_MANIFEST)} | |
| 423 | + ) | |
| 424 | + work_graph_plan.plan_work_graph(self.conn, "validator", "task-1", "goal", "/tmp/proj-1") | |
| 425 | + prompt = run.call_args.args[0][run.call_args.args[0].index("-p") + 1] | |
| 426 | + self.assertIn('"node_type": "independent_benchmark"', prompt) | |
| 427 | + self.assertIn('"node_type": "test_script"', prompt) | |
| 428 | + self.assertNotIn('"node_type": "unit_test"', prompt) # coder-only | |
| 429 | + | |
| 430 | + def test_unknown_org_node_gets_no_catalog_section_at_all(self): | |
| 431 | + # An org_node this hasn't been extended for yet (Phase 1's | |
| 432 | + # planner_reviewer, for instance) still gets the generic path -- | |
| 433 | + # just with no suggested catalog, same as before catalogs existed. | |
| 434 | + with patch("src.work_graph_plan.subprocess.run") as run: | |
| 435 | + run.return_value = _completed_process( | |
| 436 | + stdout_obj={"is_error": False, "result": _fenced_result(_CODER_THREE_MODULE_MANIFEST)} | |
| 437 | + ) | |
| 438 | + work_graph_plan.plan_work_graph(self.conn, "planner_reviewer", "task-1", "goal", "/tmp/proj-1") | |
| 439 | + prompt = run.call_args.args[0][run.call_args.args[0].index("-p") + 1] | |
| 440 | + self.assertNotIn("Suggested node types for this goal", prompt) | |
| 441 | + self.assertIn("task_prompt", prompt) # still the generic template itself | |
| 442 | + | |
| 443 | + | |
| 444 | +class AppendToRunIdTest(PlanWorkGraphTestBase): | |
| 445 | + """Phase 2 §6.1 (2026-08-12): plan_work_graph(..., append_to_run_id=...) | |
| 446 | + merges new nodes into an EXISTING work_graph_run instead of INSERTing a | |
| 447 | + fresh row.""" | |
| 448 | + | |
| 449 | + def _seed_run(self, run_id, manifest): | |
| 450 | + self.conn.execute( | |
| 451 | + "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) " | |
| 452 | + "VALUES (?, 'task-1', 'planner', ?, 'running')", | |
| 453 | + (run_id, json.dumps(manifest)), | |
| 454 | + ) | |
| 455 | + self.conn.commit() | |
| 456 | + | |
| 457 | + def test_merges_new_nodes_into_existing_run_not_a_new_row(self): | |
| 458 | + self._seed_run("run-1", {"nodes": [{"node_id": "sdd", "artifact_type": "sdd", "depends_on": []}], "edges": []}) | |
| 459 | + new_manifest = {"nodes": [{"node_id": "impl", "depends_on": ["sdd"], "task_prompt": "do it"}], "edges": []} | |
| 460 | + with patch("src.work_graph_plan.subprocess.run") as run: | |
| 461 | + run.return_value = _completed_process( | |
| 462 | + stdout_obj={"is_error": False, "result": _fenced_result(new_manifest)} | |
| 463 | + ) | |
| 464 | + result = work_graph_plan.plan_work_graph( | |
| 465 | + self.conn, "coder", "task-1", "goal", "/tmp/proj-1", append_to_run_id="run-1", | |
| 466 | + ) | |
| 467 | + | |
| 468 | + self.assertEqual(result["run_id"], "run-1") | |
| 469 | + self.assertEqual(result["new_node_ids"], ["impl"]) | |
| 470 | + merged_node_ids = {n["node_id"] for n in result["graph_manifest"]["nodes"]} | |
| 471 | + self.assertEqual(merged_node_ids, {"sdd", "impl"}) | |
| 472 | + | |
| 473 | + # exactly one work_graph_run row for this task -- no second row created. | |
| 474 | + row_count = self.conn.execute("SELECT COUNT(*) FROM work_graph_run WHERE task_id = 'task-1'").fetchone()[0] | |
| 475 | + self.assertEqual(row_count, 1) | |
| 476 | + persisted = json.loads( | |
| 477 | + self.conn.execute("SELECT graph_manifest FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] | |
| 478 | + ) | |
| 479 | + self.assertEqual({n["node_id"] for n in persisted["nodes"]}, {"sdd", "impl"}) | |
| 480 | + | |
| 481 | + def test_existing_node_ids_are_injected_into_the_prompt(self): | |
| 482 | + self._seed_run("run-1", {"nodes": [{"node_id": "sdd", "artifact_type": "sdd", "depends_on": []}], "edges": []}) | |
| 483 | + new_manifest = {"nodes": [{"node_id": "impl", "depends_on": ["sdd"], "task_prompt": "do it"}], "edges": []} | |
| 484 | + with patch("src.work_graph_plan.subprocess.run") as run: | |
| 485 | + run.return_value = _completed_process( | |
| 486 | + stdout_obj={"is_error": False, "result": _fenced_result(new_manifest)} | |
| 487 | + ) | |
| 488 | + work_graph_plan.plan_work_graph( | |
| 489 | + self.conn, "coder", "task-1", "goal", "/tmp/proj-1", append_to_run_id="run-1", | |
| 490 | + ) | |
| 491 | + prompt = run.call_args.args[0][run.call_args.args[0].index("-p") + 1] | |
| 492 | + self.assertIn("already has these nodes", prompt) | |
| 493 | + self.assertIn("sdd", prompt) | |
| 494 | + self.assertIn("ADDITIONAL nodes only", prompt) | |
| 495 | + | |
| 496 | + def test_node_id_collision_with_existing_node_raises(self): | |
| 497 | + self._seed_run("run-1", {"nodes": [{"node_id": "sdd", "artifact_type": "sdd", "depends_on": []}], "edges": []}) | |
| 498 | + # the "new" plan reuses "sdd" as its own node_id -- a real collision. | |
| 499 | + colliding_manifest = {"nodes": [{"node_id": "sdd", "depends_on": [], "task_prompt": "oops"}], "edges": []} | |
| 500 | + with patch("src.work_graph_plan.subprocess.run") as run: | |
| 501 | + run.return_value = _completed_process( | |
| 502 | + stdout_obj={"is_error": False, "result": _fenced_result(colliding_manifest)} | |
| 503 | + ) | |
| 504 | + with self.assertRaises(work_graph_plan.PlanningFailedError): | |
| 505 | + work_graph_plan.plan_work_graph( | |
| 506 | + self.conn, "coder", "task-1", "goal", "/tmp/proj-1", append_to_run_id="run-1", | |
| 507 | + ) | |
| 508 | + # the original run's manifest must be untouched by the failed append. | |
| 509 | + persisted = json.loads( | |
| 510 | + self.conn.execute("SELECT graph_manifest FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] | |
| 511 | + ) | |
| 512 | + self.assertEqual([n["node_id"] for n in persisted["nodes"]], ["sdd"]) | |
| 513 | + | |
| 514 | + def test_append_to_nonexistent_run_id_raises(self): | |
| 515 | + new_manifest = {"nodes": [{"node_id": "impl", "depends_on": [], "task_prompt": "do it"}], "edges": []} | |
| 516 | + with patch("src.work_graph_plan.subprocess.run") as run: | |
| 517 | + run.return_value = _completed_process( | |
| 518 | + stdout_obj={"is_error": False, "result": _fenced_result(new_manifest)} | |
| 519 | + ) | |
| 520 | + with self.assertRaises(ValueError): | |
| 521 | + work_graph_plan.plan_work_graph( | |
| 522 | + self.conn, "coder", "task-1", "goal", "/tmp/proj-1", append_to_run_id="no-such-run", | |
| 523 | + ) | |
| 524 | + | |
| 402 | 525 | |
| 403 | 526 | class CoderThreeIndependentModulesScenarioTest(PlanWorkGraphTestBase): |
| 404 | 527 | """E acceptance criterion 1: a Coder scenario that naturally splits into | ... | ... |
src/work_graph_fork.py
| ... | ... | @@ -94,23 +94,46 @@ def _downstream_node_ids(graph_manifest, start_node_id): |
| 94 | 94 | def _reconstruct_build_prompt(conn, task_id, org_node, graph_manifest): |
| 95 | 95 | """Rebuilds a build_prompt(node, outputs) usable for the re-executed |
| 96 | 96 | downstream subgraph, purely from persisted state -- no access to the |
| 97 | - original call site's Python closures. Two supported shapes (design doc | |
| 98 | - §2): Planner's canonical doc chain (every node has 'artifact_type', | |
| 97 | + original call site's Python closures. Two supported node shapes | |
| 98 | + (design doc §2): Planner's canonical doc chain ('artifact_type', | |
| 99 | 99 | prompt reconstructed via e2e_pipeline's own builder + task.goal_text) |
| 100 | - and E's generic dynamic path (every node carries its own | |
| 101 | - self-contained 'task_prompt'). Anything else raises rather than | |
| 102 | - silently building broken prompts.""" | |
| 100 | + and E's generic dynamic path ('task_prompt'). A node with neither | |
| 101 | + raises rather than silently building a broken prompt. | |
| 102 | + | |
| 103 | + 2026-08-12 (Phase 2 §2.1.2, found while designing append_to_run_id): | |
| 104 | + this used to require the WHOLE graph to be one uniform shape (all | |
| 105 | + nodes 'artifact_type', or all 'task_prompt') -- correct when every | |
| 106 | + graph_manifest genuinely was one shape throughout, which stopped being | |
| 107 | + true the moment Coder's self-planned nodes (task_prompt) can get | |
| 108 | + appended onto Planner's original run (artifact_type) via | |
| 109 | + plan_work_graph(..., append_to_run_id=...). Dispatches per-NODE now | |
| 110 | + (work_graph_plan.dispatch_build_prompt) instead of once for the whole | |
| 111 | + graph -- every pre-Phase-2 graph_manifest still has just one shape | |
| 112 | + throughout, so this is a strict generalization, not a behavior change | |
| 113 | + for anything that isn't a merged graph. `org_node` is unused by this | |
| 114 | + per-node dispatch itself (kept as a parameter -- callers still pass it | |
| 115 | + for other purposes, e.g. run_graph()'s own org_node argument; | |
| 116 | + removing it from work_graph_run/this signature is Phase 2c's job | |
| 117 | + alongside the actual org_node column removal, not this fix's).""" | |
| 103 | 118 | nodes = graph_manifest["nodes"] |
| 104 | - if org_node == "planner" and nodes and all("artifact_type" in n for n in nodes): | |
| 105 | - goal_text = conn.execute("SELECT goal_text FROM task WHERE task_id = ?", (task_id,)).fetchone()[0] | |
| 106 | - node_artifact_types = {n["node_id"]: n["artifact_type"] for n in nodes} | |
| 107 | - return e2e_pipeline._planner_prompt_builder(goal_text, node_artifact_types) | |
| 108 | - if nodes and all("task_prompt" in n for n in nodes): | |
| 109 | - return work_graph_plan.generic_node_prompt_builder | |
| 110 | - raise ForkGraphShapeUnsupportedError( | |
| 111 | - "graph_manifest nodes must either all have 'artifact_type' (Planner's canonical chain) " | |
| 112 | - "or all have 'task_prompt' (the generic dynamic-planning path) to reconstruct prompts for a fork" | |
| 113 | - ) | |
| 119 | + if not nodes: | |
| 120 | + raise ForkGraphShapeUnsupportedError("graph_manifest has no nodes to reconstruct a build_prompt for") | |
| 121 | + goal_text = conn.execute("SELECT goal_text FROM task WHERE task_id = ?", (task_id,)).fetchone()[0] | |
| 122 | + node_artifact_types = {n["node_id"]: n["artifact_type"] for n in nodes if "artifact_type" in n} | |
| 123 | + planner_builder = e2e_pipeline._planner_prompt_builder(goal_text, node_artifact_types) | |
| 124 | + dispatch = work_graph_plan.dispatch_build_prompt(planner_builder, work_graph_plan.generic_node_prompt_builder) | |
| 125 | + | |
| 126 | + def build_prompt(node, outputs): | |
| 127 | + try: | |
| 128 | + return dispatch(node, outputs) | |
| 129 | + except ValueError: | |
| 130 | + raise ForkGraphShapeUnsupportedError( | |
| 131 | + "node {} has neither 'artifact_type' (Planner's canonical chain) nor 'task_prompt' " | |
| 132 | + "(the generic dynamic-planning path) -- can't reconstruct a prompt for a fork".format( | |
| 133 | + node.get("node_id") | |
| 134 | + ) | |
| 135 | + ) | |
| 136 | + return build_prompt | |
| 114 | 137 | |
| 115 | 138 | |
| 116 | 139 | def create_fork(conn, source_node_exec_id, new_output_payload): | ... | ... |
src/work_graph_plan.py
| ... | ... | @@ -41,10 +41,17 @@ GRAPH_MANIFEST_SCHEMA = { |
| 41 | 41 | "required": ["node_id", "artifact_type", "depends_on"], |
| 42 | 42 | "properties": { |
| 43 | 43 | "node_id": {"type": "string"}, |
| 44 | + # "artifact_type" (Planner's fixed-catalog nodes) and "task_prompt" | |
| 45 | + # (Coder/Validator's generic self-planned nodes, _generic_prompt_ | |
| 46 | + # template()) are mutually exclusive node shapes in practice -- | |
| 47 | + # "required" above only reflects Planner's shape (this schema | |
| 48 | + # predates the generic path); see work_graph_plan.py's own dispatch | |
| 49 | + # helpers for how callers tell the two apart per-node. | |
| 44 | 50 | "artifact_type": { |
| 45 | 51 | "type": "string", |
| 46 | 52 | "enum": ["brd", "prd", "sdd", "benchmark", "test_dataset", "test_script"], |
| 47 | 53 | }, |
| 54 | + "task_prompt": {"type": "string"}, | |
| 48 | 55 | "depends_on": {"type": "array", "items": {"type": "string"}}, |
| 49 | 56 | # 2026-08-08 (Work Graph self-planning + loop-back) -- both optional, |
| 50 | 57 | # documentation only (this schema isn't jsonschema-enforced, see |
| ... | ... | @@ -60,10 +67,20 @@ GRAPH_MANIFEST_SCHEMA = { |
| 60 | 67 | "on_fail": { |
| 61 | 68 | "type": "object", |
| 62 | 69 | "properties": { |
| 63 | - "action": {"type": "string", "enum": ["retry_self", "loop_back", "hard_fail"]}, | |
| 70 | + "action": { | |
| 71 | + "type": "string", | |
| 72 | + "enum": ["retry_self", "loop_back", "hard_fail", "dynamic_loop_back"], | |
| 73 | + }, | |
| 64 | 74 | "max_attempts": {"type": "integer"}, |
| 65 | 75 | }, |
| 66 | 76 | }, |
| 77 | + # 2026-08-12 (Phase 2 §2.3/§4): also optional/documentation-only. | |
| 78 | + "promotes_to_artifact": { | |
| 79 | + "type": "object", | |
| 80 | + "properties": {"artifact_type": {"type": "string"}}, | |
| 81 | + }, | |
| 82 | + "subagent_type": {"type": "string"}, | |
| 83 | + "skill_ref": {"type": "string"}, | |
| 67 | 84 | }, |
| 68 | 85 | }, |
| 69 | 86 | }, |
| ... | ... | @@ -126,13 +143,19 @@ _PLANNER_NODE_CATALOG = [ |
| 126 | 143 | }, |
| 127 | 144 | ] |
| 128 | 145 | |
| 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 | |
| 146 | +# NOTE: no `str.format()` involved in building or filling this template -- | |
| 147 | +# the embedded catalog JSON and the instructional JSON examples below | |
| 148 | +# (e.g. {"kind": "none"}) are full of their own literal braces, which | |
| 149 | +# .format() would try to parse as format fields and crash on. | |
| 150 | +# plan_work_graph() fills in goal_text with a plain | |
| 134 | 151 | # `.replace("{goal_text}", goal_text)` instead, which treats every other |
| 135 | 152 | # brace in this string as inert literal text -- no escaping needed at all. |
| 153 | +# 2026-08-12: _generic_prompt_template() (below) now follows this SAME | |
| 154 | +# convention for the same reason -- its own catalog JSON has the same | |
| 155 | +# literal-brace problem .format() would choke on, which the original | |
| 156 | +# _GENERIC_PROMPT_TEMPLATE constant used to dodge by having no embedded | |
| 157 | +# JSON at all; now that it does (Phase 2 §3's node catalogs), it needed | |
| 158 | +# the same fix this template already had. | |
| 136 | 159 | _PROMPT_TEMPLATE = ( |
| 137 | 160 | "Plan (do not execute) the Work Graph for this goal: {goal_text}\n" |
| 138 | 161 | "Available node types (use any subset, in any order that makes sense -- this goal may not " |
| ... | ... | @@ -163,29 +186,107 @@ _PROMPT_TEMPLATE = ( |
| 163 | 186 | # _PROMPT_TEMPLATE above unchanged (zero regression risk, see |
| 164 | 187 | # docs/done/dynamic-work-graph-design-skill-design.md §3): that chain is a |
| 165 | 188 | # fixed architectural constant (brd->prd->sdd->benchmark->...), not |
| 166 | -# something that should vary call to call, whereas Coder/Validator's | |
| 167 | -# sub-tasks have no such fixed small vocabulary of node types. Each node | |
| 168 | -# here carries its own self-contained `task_prompt` (see the | |
| 169 | -# design-dynamic-work-graph Skill this prompt points at) instead of an | |
| 170 | -# `artifact_type` that a fixed lookup table like _DOC_PROMPTS could resolve. | |
| 171 | -_GENERIC_PROMPT_TEMPLATE = ( | |
| 172 | - "Plan (do not execute) a Work Graph for this goal: {goal_text}\n" | |
| 173 | - "Follow the design-dynamic-work-graph Skill's methodology: only add a depends_on edge " | |
| 174 | - "when a node's task_prompt genuinely needs another node's output content -- nodes with " | |
| 175 | - "no real data dependency between them will be scheduled to run CONCURRENTLY, so adding " | |
| 176 | - "an edge just to make the graph look ordered actively slows execution down. If this goal " | |
| 177 | - "is small enough to finish in one call, output a single-node graph_manifest rather than " | |
| 178 | - "forcing a split.\n" | |
| 179 | - "Output a graph_manifest whose nodes each have: node_id (a short readable identifier), " | |
| 180 | - "depends_on (list of other node_ids), and task_prompt (the FULL, self-contained prompt " | |
| 181 | - "for that node's own execution -- assume the agent running it sees nothing except this " | |
| 182 | - "task_prompt and its dependencies' outputs, not this planning conversation). Every edge " | |
| 183 | - "needs edge_type: 'zero_token' for pure aggregation/reduce steps, 'needs_judgment' for " | |
| 184 | - "anything requiring the agent to actually reason about content.\n" | |
| 185 | - "End your reply with the graph_manifest as a single fenced ```json code block " | |
| 186 | - "shaped exactly like {{\"nodes\": [...], \"edges\": [...]}} -- no extra wrapper key." | |
| 189 | +# something that should vary call to call. Each node here carries its own | |
| 190 | +# self-contained `task_prompt` (see the design-dynamic-work-graph Skill | |
| 191 | +# this prompt points at) instead of an `artifact_type` that a fixed lookup | |
| 192 | +# table like _DOC_PROMPTS could resolve. | |
| 193 | +# | |
| 194 | +# 2026-08-12 (Phase 2 §3, user-confirmed 2026-08-08 -- reverses this | |
| 195 | +# comment's own prior "no fixed small vocabulary of node types" framing): | |
| 196 | +# Coder/Validator's sub-tasks DO get a suggested (not required) catalog | |
| 197 | +# now, same "starting points, not requirements" contract _PLANNER_NODE_ | |
| 198 | +# CATALOG already uses -- the LLM can ignore every entry and invent its | |
| 199 | +# own node_ids/task_prompts freely, same as before this change. | |
| 200 | +_NODE_REVIEW_JUDGE_PROMPT = ( | |
| 201 | + "Judge whether this node's output is genuinely what it claims to be, not a narrative " | |
| 202 | + "description of an action taken (e.g. 'I ran/produced/completed X') standing in for the real " | |
| 203 | + "content itself -- this is a known recurring failure mode in nodes that explore with tools " | |
| 204 | + "before producing a final answer. ok=false if the content is narration/progress-reporting " | |
| 205 | + "prose rather than the real deliverable (code, test output, structured analysis, etc.), or if " | |
| 206 | + "it's otherwise clearly incomplete/wrong for what was asked. List each concrete problem in " | |
| 207 | + "issues.\n\nContent:\n\n{content}" | |
| 187 | 208 | ) |
| 188 | 209 | |
| 210 | +_CODER_NODE_CATALOG = [ | |
| 211 | + {"node_type": "impl", "description": "Actual code changes implementing the SDD."}, | |
| 212 | + { | |
| 213 | + "node_type": "unit_test", | |
| 214 | + "description": "New/updated unit tests covering the change -- same repo convention as existing tests.", | |
| 215 | + "suggested_judgment": {"kind": "llm", "judge_prompt": _NODE_REVIEW_JUDGE_PROMPT}, | |
| 216 | + }, | |
| 217 | + { | |
| 218 | + "node_type": "integration_check", | |
| 219 | + "description": "Actually runs the real test suite/benchmark script against the change and reports " | |
| 220 | + "pass/fail -- not a description of doing so.", | |
| 221 | + "suggested_judgment": {"kind": "llm", "judge_prompt": _NODE_REVIEW_JUDGE_PROMPT}, | |
| 222 | + "suggested_promotes_to_artifact": {"artifact_type": "test_report"}, | |
| 223 | + }, | |
| 224 | +] | |
| 225 | + | |
| 226 | +_VALIDATOR_NODE_CATALOG = [ | |
| 227 | + { | |
| 228 | + "node_type": "independent_benchmark", | |
| 229 | + "description": "Validator's own held-out acceptance scenarios -- NOT reused from Planner's/Coder's " | |
| 230 | + "benchmark, a genuine second opinion.", | |
| 231 | + }, | |
| 232 | + {"node_type": "test_dataset", "description": "Concrete test data, if this goal has a natural one."}, | |
| 233 | + { | |
| 234 | + "node_type": "test_script", | |
| 235 | + "description": "A real, runnable script mechanically checking the independent benchmark's scenarios.", | |
| 236 | + "suggested_judgment": {"kind": "mechanical", "check_ref": "check_script_compiles"}, | |
| 237 | + }, | |
| 238 | +] | |
| 239 | + | |
| 240 | +_GENERIC_NODE_CATALOGS = {"coder": _CODER_NODE_CATALOG, "validator": _VALIDATOR_NODE_CATALOG} | |
| 241 | + | |
| 242 | + | |
| 243 | +def _generic_prompt_template(org_node): | |
| 244 | + """Builds _GENERIC_PROMPT_TEMPLATE's replacement, parameterized by | |
| 245 | + org_node so each gets its own suggested catalog (or none, for any | |
| 246 | + future org_node this hasn't been extended for yet -- catalog_section | |
| 247 | + is simply empty then, same as before this change existed). | |
| 248 | + node_type is a label in the SUGGESTION data only, not a required | |
| 249 | + graph_manifest node field -- node_id/task_prompt stay exactly as | |
| 250 | + freeform as before.""" | |
| 251 | + catalog = _GENERIC_NODE_CATALOGS.get(org_node, []) | |
| 252 | + catalog_section = ( | |
| 253 | + "Suggested node types for this goal (use any subset, in any order, invent your own if " | |
| 254 | + "none fit -- these are starting points, not requirements):\n" + json.dumps(catalog) + "\n" | |
| 255 | + if catalog else "" | |
| 256 | + ) | |
| 257 | + return ( | |
| 258 | + "Plan (do not execute) a Work Graph for this goal: {goal_text}\n" | |
| 259 | + + catalog_section + | |
| 260 | + "Follow the design-dynamic-work-graph Skill's methodology: only add a depends_on edge " | |
| 261 | + "when a node's task_prompt genuinely needs another node's output content -- nodes with " | |
| 262 | + "no real data dependency between them will be scheduled to run CONCURRENTLY, so adding " | |
| 263 | + "an edge just to make the graph look ordered actively slows execution down. If this goal " | |
| 264 | + "is small enough to finish in one call, output a single-node graph_manifest rather than " | |
| 265 | + "forcing a split.\n" | |
| 266 | + "Output a graph_manifest whose nodes each have: node_id (a short readable identifier), " | |
| 267 | + "depends_on (list of other node_ids), and task_prompt (the FULL, self-contained prompt " | |
| 268 | + "for that node's own execution -- assume the agent running it sees nothing except this " | |
| 269 | + "task_prompt and its dependencies' outputs, not this planning conversation), plus:\n" | |
| 270 | + " - judgment (optional): how to tell if this node's output is acceptable -- " | |
| 271 | + "{\"kind\": \"none\"} if nothing needs checking (the default if omitted), {\"kind\": " | |
| 272 | + "\"llm\", \"judge_prompt\": \"...{content}...\"} for your own judgment prompt with a " | |
| 273 | + "{content} placeholder.\n" | |
| 274 | + " - on_fail (only meaningful if judgment.kind != \"none\"): {\"action\": \"retry_self\"}, " | |
| 275 | + "{\"action\": \"hard_fail\"}, or {\"action\": \"loop_back\"} with a matching on_failure " | |
| 276 | + "edge to a genuine upstream dependency (same rule _PLANNER_NODE_CATALOG's own nodes " | |
| 277 | + "follow) -- do NOT use \"dynamic_loop_back\", that's reserved for the terminal review " | |
| 278 | + "stage outside this graph, not for nodes you plan yourself.\n" | |
| 279 | + " - promotes_to_artifact (optional): {\"artifact_type\": \"test_report\"} ONLY for a " | |
| 280 | + "node producing this task's actual final deliverable text -- most nodes should omit " | |
| 281 | + "this; it marks a node's content for persistence beyond this graph's own execution. " | |
| 282 | + "(code_diff, if relevant, is derived separately from real file changes, not from any " | |
| 283 | + "node's own content -- never declare artifact_type: \"code_diff\" here.)\n" | |
| 284 | + "Every edge needs edge_type: 'zero_token' for pure aggregation/reduce steps, " | |
| 285 | + "'needs_judgment' for anything requiring the agent to actually reason about content.\n" | |
| 286 | + "End your reply with the graph_manifest as a single fenced ```json code block " | |
| 287 | + "shaped exactly like {{\"nodes\": [...], \"edges\": [...]}} -- no extra wrapper key." | |
| 288 | + ) | |
| 289 | + | |
| 189 | 290 | _JSON_FENCE_RE = re.compile(r"```(?:json)?\s*(\{.*?\})\s*```", re.DOTALL) |
| 190 | 291 | |
| 191 | 292 | |
| ... | ... | @@ -333,22 +434,61 @@ def _normalize_graph_manifest(manifest): |
| 333 | 434 | return manifest |
| 334 | 435 | |
| 335 | 436 | |
| 336 | -def plan_work_graph(conn, org_node, task_id, goal_text, target_folder_path, timeout=300): | |
| 337 | - """Returns {"run_id", "graph_manifest"}. Creates one `work_graph_run` row | |
| 338 | - (status='planning') -- moving it to 'running'/executing it is out of scope. | |
| 437 | +def plan_work_graph(conn, org_node, task_id, goal_text, target_folder_path, timeout=300, append_to_run_id=None): | |
| 438 | + """Returns {"run_id", "graph_manifest", "new_node_ids"}. Creates one | |
| 439 | + `work_graph_run` row (status='planning') -- moving it to 'running'/ | |
| 440 | + executing it is out of scope. | |
| 339 | 441 | |
| 340 | 442 | 2026-08-08: `org_node == 'planner'` gets `_PROMPT_TEMPLATE` (now a |
| 341 | 443 | genuine self-planning prompt over `_PLANNER_NODE_CATALOG`, see that |
| 342 | 444 | template's own comment for why goal_text is filled in via `.replace()` |
| 343 | 445 | 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.""" | |
| 446 | + `_generic_prompt_template(org_node)` -- the cross-Org-Agent dynamic | |
| 447 | + planning path described in the design-dynamic-work-graph Skill, whose | |
| 448 | + nodes carry their own task_prompt instead of a fixed artifact_type. | |
| 449 | + | |
| 450 | + 2026-08-12 (Phase 2 §6.1, user-confirmed 2026-08-08: a parameter on | |
| 451 | + this function, not a separate plan_work_graph_append()): with | |
| 452 | + `append_to_run_id=None` (default), behavior is unchanged from before | |
| 453 | + this parameter existed -- a fresh INSERT. With `append_to_run_id= | |
| 454 | + <existing run_id>`, this call plans ADDITIONAL nodes for an | |
| 455 | + already-existing work_graph_run (e.g. Coder appending its own | |
| 456 | + implementation nodes onto Planner's run) instead of creating a new | |
| 457 | + row: the existing manifest's node_ids are injected into the prompt so | |
| 458 | + new nodes can `depends_on` them, a `node_id` collision between the | |
| 459 | + newly-planned nodes and the existing ones raises PlanningFailedError | |
| 460 | + (never silently renamed or overwritten), and the row is UPDATEd with | |
| 461 | + the merged manifest instead of a new row being INSERTed. | |
| 462 | + `new_node_ids` (only meaningfully non-empty in append mode, though | |
| 463 | + always present for a uniform return shape) is the set of node_ids this | |
| 464 | + specific call added -- callers that promote node output to `artifact` | |
| 465 | + rows (e2e_pipeline.py's `_write_completed_node_artifacts`) need this | |
| 466 | + to avoid re-processing nodes an EARLIER stage already promoted; the | |
| 467 | + merged manifest itself doesn't distinguish "planned this call" from | |
| 468 | + "already existed" once merged.""" | |
| 469 | + existing_manifest = None | |
| 470 | + if append_to_run_id: | |
| 471 | + row = conn.execute( | |
| 472 | + "SELECT graph_manifest FROM work_graph_run WHERE run_id = ?", (append_to_run_id,) | |
| 473 | + ).fetchone() | |
| 474 | + if row is None: | |
| 475 | + raise ValueError("no such work_graph_run to append to: " + append_to_run_id) | |
| 476 | + existing_manifest = json.loads(row[0]) | |
| 477 | + | |
| 348 | 478 | if org_node == "planner": |
| 349 | 479 | prompt = _PROMPT_TEMPLATE.replace("{goal_text}", goal_text) |
| 350 | 480 | else: |
| 351 | - prompt = _GENERIC_PROMPT_TEMPLATE.format(goal_text=goal_text) | |
| 481 | + prompt = _generic_prompt_template(org_node).replace("{goal_text}", goal_text) | |
| 482 | + | |
| 483 | + if existing_manifest is not None: | |
| 484 | + existing_ids = [n["node_id"] for n in existing_manifest["nodes"]] | |
| 485 | + prompt = prompt.replace( | |
| 486 | + "Plan (do not execute)", | |
| 487 | + "This graph already has these nodes from earlier stages (you may depends_on them, but do " | |
| 488 | + "NOT redeclare or duplicate them -- pick different node_ids for your own new nodes): " | |
| 489 | + "{}. Plan (do not execute) ADDITIONAL nodes only".format(existing_ids), | |
| 490 | + 1, | |
| 491 | + ) | |
| 352 | 492 | |
| 353 | 493 | result = subprocess.run( |
| 354 | 494 | [ |
| ... | ... | @@ -374,24 +514,48 @@ def plan_work_graph(conn, org_node, task_id, goal_text, target_folder_path, time |
| 374 | 514 | payload = json.loads(result.stdout) |
| 375 | 515 | if payload.get("is_error"): |
| 376 | 516 | raise PlanningFailedError("plan failed for task {}: {}".format(task_id, payload)) |
| 377 | - graph_manifest = _extract_graph_manifest(payload.get("result", "")) | |
| 378 | - if not graph_manifest: | |
| 517 | + new_manifest = _extract_graph_manifest(payload.get("result", "")) | |
| 518 | + if not new_manifest: | |
| 379 | 519 | raise PlanningFailedError("could not extract a graph_manifest for task {}: {}".format(task_id, payload)) |
| 380 | - graph_manifest = _normalize_graph_manifest(graph_manifest) | |
| 520 | + new_manifest = _normalize_graph_manifest(new_manifest) | |
| 521 | + | |
| 522 | + if existing_manifest is None: | |
| 523 | + run_id = str(uuid.uuid4()) | |
| 524 | + conn.execute( | |
| 525 | + "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) " | |
| 526 | + "VALUES (?, ?, ?, ?, 'planning')", | |
| 527 | + (run_id, task_id, org_node, json.dumps(new_manifest)), | |
| 528 | + ) | |
| 529 | + conn.commit() | |
| 530 | + return {"run_id": run_id, "graph_manifest": new_manifest, "new_node_ids": []} | |
| 381 | 531 | |
| 382 | - run_id = str(uuid.uuid4()) | |
| 532 | + new_node_ids = [n["node_id"] for n in new_manifest["nodes"]] | |
| 533 | + existing_ids_set = {n["node_id"] for n in existing_manifest["nodes"]} | |
| 534 | + collision = existing_ids_set & set(new_node_ids) | |
| 535 | + if collision: | |
| 536 | + raise PlanningFailedError( | |
| 537 | + "append_to_run_id={} node_id collision with existing nodes: {}".format(append_to_run_id, collision) | |
| 538 | + ) | |
| 539 | + merged = { | |
| 540 | + "nodes": existing_manifest["nodes"] + new_manifest["nodes"], | |
| 541 | + "edges": existing_manifest.get("edges", []) + new_manifest.get("edges", []), | |
| 542 | + } | |
| 543 | + # _validate_loop_back_targets over the MERGED graph, not just the new | |
| 544 | + # nodes -- a new node's on_fail.loop_back could (in principle) target | |
| 545 | + # an existing node, and _normalize_graph_manifest already validated | |
| 546 | + # new_manifest in isolation above, not against the merged shape. | |
| 547 | + _validate_loop_back_targets(merged) | |
| 383 | 548 | conn.execute( |
| 384 | - "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) " | |
| 385 | - "VALUES (?, ?, ?, ?, 'planning')", | |
| 386 | - (run_id, task_id, org_node, json.dumps(graph_manifest)), | |
| 549 | + "UPDATE work_graph_run SET graph_manifest = ? WHERE run_id = ?", | |
| 550 | + (json.dumps(merged), append_to_run_id), | |
| 387 | 551 | ) |
| 388 | 552 | conn.commit() |
| 389 | - return {"run_id": run_id, "graph_manifest": graph_manifest} | |
| 553 | + return {"run_id": append_to_run_id, "graph_manifest": merged, "new_node_ids": new_node_ids} | |
| 390 | 554 | |
| 391 | 555 | |
| 392 | 556 | def generic_node_prompt_builder(node, outputs): |
| 393 | 557 | """E (2026-08-06): the `build_prompt(node, outputs)` run_graph() needs |
| 394 | - for a graph_manifest produced via _GENERIC_PROMPT_TEMPLATE -- lives | |
| 558 | + for a graph_manifest produced via `_generic_prompt_template()` -- lives | |
| 395 | 559 | here, not in e2e_pipeline.py, because it's tightly coupled to the |
| 396 | 560 | `task_prompt` field convention that prompt asks for (keeping both in |
| 397 | 561 | the same module means a field rename can't silently desync producer |
| ... | ... | @@ -399,3 +563,31 @@ def generic_node_prompt_builder(node, outputs): |
| 399 | 563 | per the design-dynamic-work-graph Skill's contract, so `outputs` |
| 400 | 564 | (already-completed dependency content) isn't needed here.""" |
| 401 | 565 | return node["task_prompt"] |
| 566 | + | |
| 567 | + | |
| 568 | +def dispatch_build_prompt(planner_builder, generic_builder): | |
| 569 | + """2026-08-12 (Phase 2 §6.1): a build_prompt(node, outputs) that picks | |
| 570 | + per-NODE between planner_builder (Planner's artifact_type-keyed nodes, | |
| 571 | + e.g. e2e_pipeline._planner_prompt_builder's return value) and | |
| 572 | + generic_builder (task_prompt-keyed nodes, e.g. generic_node_prompt_ | |
| 573 | + builder above) -- needed once a single graph_manifest can genuinely | |
| 574 | + contain BOTH shapes side by side, which append_to_run_id makes real | |
| 575 | + for the first time (Planner's original nodes + Coder's/Validator's | |
| 576 | + later self-planned ones, merged into one run). Every pre-Phase-2 | |
| 577 | + caller only ever had ONE shape for a whole graph and dispatched once | |
| 578 | + up front; this replaces that with a per-node check so a caller doesn't | |
| 579 | + need to know in advance whether ANY given node in a merged graph is | |
| 580 | + Planner-shaped or generic-shaped. Shared here (not duplicated in | |
| 581 | + e2e_pipeline.py/work_graph_fork.py, its two real callers) so the two | |
| 582 | + don't drift on what "neither shape" should do.""" | |
| 583 | + def build_prompt(node, outputs): | |
| 584 | + if "artifact_type" in node: | |
| 585 | + return planner_builder(node, outputs) | |
| 586 | + if "task_prompt" in node: | |
| 587 | + return generic_builder(node, outputs) | |
| 588 | + raise ValueError( | |
| 589 | + "node {} has neither 'artifact_type' nor 'task_prompt' -- can't dispatch a build_prompt".format( | |
| 590 | + node.get("node_id") | |
| 591 | + ) | |
| 592 | + ) | |
| 593 | + return build_prompt | ... | ... |
-
mentioned in commit 82368fc6d3a38e988a10509bc8526465276529a8