Skip to content

Commit 38e4572

Browse files
feat(orchestration): run verify_orchestration_prompt on what Step 1 emits (#299)
Merge PR #299
1 parent f1bff7c commit 38e4572

5 files changed

Lines changed: 136 additions & 7 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
99

1010
## [Unreleased]
1111

12+
### Added
13+
14+
- **Step 1 now verifies the orchestration prompt it emits.** `verify_orchestration_prompt()` existed, was exported and had four unit tests, and was called by nothing; it is now wired in at the point the prompt is stored, and its findings are recorded on `orchestrator_result["prompt_warnings"]` and logged. The gap it closes is narrow and specific: both degraded paths in the node already log (an `ERROR` from prompt-gen, and an empty response), so this is not for those. It is for a response that is **non-empty and therefore silent** but not a usable prompt — a truncated template, the wrong file, a stub — which until now was indistinguishable from a good one. Warnings are never fatal: `STEP1_CONTRACT` documents the short raw-task fallback as a legitimate degraded path, and it trips both checks by design, so failing here would convert a recoverable run into a dead one.
15+
1216
### Removed
1317

1418
- **`langgraph_engine/helper_nodes/` is deleted** (5 modules, 538 lines). All nine of its public node functions were also defined in `orchestrator.py`, which is the module the graph actually wires, and nothing imported the package -- it was an extraction that was started and never completed. Two documentation corrections existed **only** in the orphaned copy and were ported to `orchestrator.py` before the deletion rather than discarded with it: `save_workflow_memory`'s comment named `~/.claude/memory/sessions/` while its code writes to `memory/logs/sessions/`, and `optimize_context_after_level1` still said "Level 3" in three places, a level that stopped existing in the `06b0463` rename. The live module had the stale text in both cases.

‎CLAUDE.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ Claude Workflow Engine is a 3-level LangGraph-based orchestration pipeline for a
2020
| **Status** | Active Development |
2121
| **Primary Location** | langgraph_engine/ |
2222
| **MCP Servers** | 13 servers -- all in separate repos under [techdeveloper-org](https://github.com/orgs/techdeveloper-org/repositories); 1 also keeps an in-engine copy in `src/mcp/` |
23-
| **Total Python Files** | 248 (langgraph_engine/); 488 repo-wide |
24-
| **Test Files** | 100 (91 unit, 5 integration, 3 e2e, 1 load) |
23+
| **Total Python Files** | 248 (langgraph_engine/); 489 repo-wide |
24+
| **Test Files** | 101 (92 unit, 5 integration, 3 e2e, 1 load) |
2525
| **Call Graph** | 578 classes, 3,985 methods, 4 languages (Python/Java/TS/Kotlin) |
2626

2727
---

‎README.md‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
[![PyPI](https://img.shields.io/badge/PyPI-claude--workflow--engine-orange)](https://pypi.org/project/claude-workflow-engine/)
88
[![CI](https://github.com/techdeveloper-org/claude-workflow-engine/actions/workflows/ci.yml/badge.svg)](https://github.com/techdeveloper-org/claude-workflow-engine/actions/workflows/ci.yml)
99
[![License](https://img.shields.io/badge/License-MIT-yellow)](LICENSE)
10-
[![Tests](https://img.shields.io/badge/Tests-100%20files%20·%205%20integration-brightgreen)](tests/)
10+
[![Tests](https://img.shields.io/badge/Tests-101%20files%20·%205%20integration-brightgreen)](tests/)
1111
[![Discussions](https://img.shields.io/badge/Discussions-GitHub-blueviolet)](../../discussions)
1212

1313
---
@@ -461,8 +461,8 @@ claude-workflow-engine/ # 369 Python files total
461461
├── policies/ # non-.md policy state only (failure-kb.json) — see note below
462462
│ └── 03-execution-system/failure-prevention/failure-kb.json
463463
│
464-
├── tests/ # 100 test_*.py files (121 total Python files)
465-
│ ├── test_*.py # 91 unit tests
464+
├── tests/ # 101 test_*.py files (122 total Python files)
465+
│ ├── test_*.py # 92 unit tests
466466
│ ├── integration/ # 5 integration tests (GitHub, MCP, runtime verification)
467467
│ ├── e2e/ # 3 end-to-end scenario tests
468468
│ └── load/ # 1 concurrency / load test
@@ -620,11 +620,11 @@ ENABLE_HEALTH_SERVER=1 ENABLE_METRICS=1 LOG_FORMAT=json \
620620

621621
## Testing
622622

623-
100 test_*.py files (121 total Python files in `tests/` including conftest and `__init__` files):
623+
101 test_*.py files (122 total Python files in `tests/` including conftest and `__init__` files):
624624

625625
| Category | Files | Notes |
626626
|---|:---:|---|
627-
| Unit tests | 91 | In `tests/test_*.py` — no external dependencies required |
627+
| Unit tests | 92 | In `tests/test_*.py` — no external dependencies required |
628628
| Integration tests | 5 | `tests/integration/` — require live GitHub token or MCP servers |
629629
| E2E tests | 3 | `tests/e2e/` — require full pipeline environment |
630630
| Load tests | 1 | `tests/load/` — enabled with `RUN_LOAD_TESTS=1` |

‎langgraph_engine/sdlc_pipeline/nodes/task_orchestration.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,13 +236,34 @@ def step1_task_analysis_node(state: FlowState) -> Dict[str, Any]:
236236
"--- BEGIN ORCHESTRATION TEMPLATE ---\n\n" + orchestration_prompt + "\n\n--- END ORCHESTRATION TEMPLATE ---"
237237
)
238238

239+
# Structural check on what is about to be emitted. Both degraded paths above
240+
# already log -- an ERROR from prompt_gen, or an empty response -- so this is
241+
# not here to catch those. It is here for the case neither covers: a response
242+
# that is non-empty and therefore silent, but not a usable orchestration
243+
# prompt (a truncated template, the wrong file, a stub). Until now that was
244+
# indistinguishable from a good prompt.
245+
#
246+
# Warnings are recorded and logged, never fatal. The raw-task fallback is a
247+
# legitimate degraded path -- STEP1_CONTRACT documents it as such -- and it
248+
# trips both checks by design, so failing here would turn a recoverable run
249+
# into a dead one.
250+
from ...runtime_verification.schema_verifier import verify_orchestration_prompt # noqa: PLC0415
251+
252+
prompt_warnings = verify_orchestration_prompt(orchestration_prompt)
253+
239254
orch_result: Dict[str, Any] = {
240255
"mode": "emitted",
241256
"success": True,
242257
"prompt_chars": len(_full_prompt),
243258
"template_source": "claude-global-library/ORCHESTRATION_TEMPLATE.md",
244259
"library_version": _library_version(),
260+
"prompt_warnings": prompt_warnings,
245261
}
262+
if prompt_warnings:
263+
logger.warning(
264+
"[v2] Step 1 emitted a structurally questionable orchestration prompt: {}",
265+
"; ".join(prompt_warnings),
266+
)
246267
logger.info(
247268
"[v2] Step 1 emitted orchestration prompt: {} chars, library v{}",
248269
orch_result["prompt_chars"],
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
"""Step 1 must actually run verify_orchestration_prompt on what it emits.
2+
3+
The function was written, exported and unit-tested, and then called by nothing.
4+
Its sibling verify_orchestrator_result was deleted for that reason plus a false
5+
premise; this one encodes a true contract, so it was wired in instead.
6+
7+
A wire-in needs a test that would notice the wire coming loose, otherwise it
8+
recreates the original problem in a new place -- a validator that exists, passes
9+
its own unit tests, and never sees production data. So every assertion here goes
10+
through step1_task_analysis_node rather than calling the verifier directly:
11+
tests/test_schema_verifier.py already covers the function in isolation, and that
12+
coverage is exactly what failed to notice it had no callers.
13+
14+
The two cases are a pair on purpose. A test that only asserts "a good prompt
15+
produces no warnings" passes just as well when the verifier is never invoked,
16+
because an unwired check and a satisfied check both produce an empty list.
17+
"""
18+
19+
import sys
20+
from pathlib import Path
21+
from unittest.mock import patch
22+
23+
_PROJECT_ROOT = Path(__file__).resolve().parent.parent
24+
if str(_PROJECT_ROOT) not in sys.path:
25+
sys.path.insert(0, str(_PROJECT_ROOT))
26+
27+
# Long enough to clear the 200-char floor and carrying the "Phase" keyword the
28+
# verifier looks for, so it stands in for a real master template.
29+
WELL_FORMED_PROMPT = "Phase A -- foundation. " + ("orchestration detail. " * 40)
30+
31+
# Non-empty, so neither degraded path in the node fires and nothing else logs:
32+
# this is the case the wire-in exists for.
33+
MALFORMED_PROMPT = "do the thing"
34+
35+
36+
def _script_mock(prompt_text):
37+
"""Return a call_execution_script stand-in yielding the given prompt."""
38+
39+
def _side_effect(script_name, args=None, model_tier=None, silence_interval=None):
40+
if script_name == "prompt_gen_expert_caller":
41+
return {"status": "SUCCESS", "llm_response": prompt_text, "prompt": "raw"}
42+
return {"status": "SUCCESS"}
43+
44+
return _side_effect
45+
46+
47+
def _run_node(prompt_text):
48+
"""Drive step1_task_analysis_node with a mocked prompt-gen response."""
49+
from langgraph_engine.sdlc_pipeline.nodes.task_orchestration import step1_task_analysis_node
50+
51+
state = {"user_message": "Add a new endpoint", "project_root": str(_PROJECT_ROOT)}
52+
with patch(
53+
"langgraph_engine.sdlc_pipeline.helpers.call_execution_script",
54+
side_effect=_script_mock(prompt_text),
55+
):
56+
return step1_task_analysis_node(state)
57+
58+
59+
class TestTheVerifierRunsOnWhatIsEmitted:
60+
"""The check must reach the emitted prompt, not merely exist."""
61+
62+
def test_a_well_formed_prompt_records_no_warnings(self):
63+
result = _run_node(WELL_FORMED_PROMPT)
64+
orch = result["orchestrator_result"]
65+
assert "prompt_warnings" in orch, "the verifier result must reach orchestrator_result"
66+
assert orch["prompt_warnings"] == []
67+
68+
def test_a_malformed_prompt_records_warnings(self):
69+
"""The half that fails if the verifier is never called.
70+
71+
A non-empty but unusable prompt is the case neither degraded path in the
72+
node covers -- prompt_gen returned something, so nothing else logs. If
73+
this list came back empty the check would be unwired, and the pass case
74+
above could not tell the difference.
75+
"""
76+
result = _run_node(MALFORMED_PROMPT)
77+
warnings = result["orchestrator_result"]["prompt_warnings"]
78+
assert warnings, "a 12-character prompt with no 'Phase' keyword must be reported"
79+
joined = " ".join(warnings).lower()
80+
assert "short" in joined or "200" in joined
81+
assert "phase" in joined
82+
83+
84+
class TestWarningsAreNotFatal:
85+
"""A questionable prompt is reported, not turned into a failed run."""
86+
87+
def test_the_step_still_emits_and_reports_success(self):
88+
"""The raw-task fallback trips both checks by design.
89+
90+
STEP1_CONTRACT documents a short raw task as a legitimate degraded path,
91+
so a verifier that failed the step would convert every recoverable run
92+
into a dead one.
93+
"""
94+
result = _run_node(MALFORMED_PROMPT)
95+
orch = result["orchestrator_result"]
96+
assert orch["prompt_warnings"]
97+
assert orch["success"] is True
98+
assert orch["mode"] == "emitted"
99+
assert result["orchestration_prompt"] == MALFORMED_PROMPT
100+
101+
def test_prompt_chars_still_counts_the_full_emitted_prompt(self):
102+
"""Guards against the wire-in being placed where it truncates the result."""
103+
result = _run_node(WELL_FORMED_PROMPT)
104+
assert result["orchestrator_result"]["prompt_chars"] > len(WELL_FORMED_PROMPT)

0 commit comments

Comments
 (0)