test(graph): capture unvalidated edge target regression (#91) #102

Merged
CoreRasurae merged 1 commit from tdd/m1-graph-edge-target-validation into master 2026-08-04 12:58:30 +00:00
Member

Summary

Adds a failing-first Behave regression test proving issue #91: a graph route whose edge target (or source) references a non-existent node is accepted at load time and only fails by silently truncating execution — violating §11 (load-time validation, ConfigurationError), §11.3.4 (edge source/target must reference existing nodes), and §6.11.5 (edge validation) of the Actor Configuration Standard.

The scenario drives a graph actor whose only edge targets an undefined node through the public create_executor()/execute() API and asserts a ConfigurationError naming the missing node is raised before any node runs. Today it isn't: the actor executes every node up to the dangling edge and returns partial output silently instead.

The scenario is tagged @tdd_issue, @tdd_issue_91, and @tdd_expected_fail per the TDD issue-capture workflow.

The actual fix lands separately on bugfix/m1-graph-edge-target-validation per issue #91.

Test plan

  • nox -s lint green
  • nox -s format -- --check green
  • nox -s typecheck green
  • nox -s security_scan green
  • nox -s dead_code green
  • nox -s unit_tests green (2900 scenarios, including the new tagged scenario) — confirms the assertion fails via AssertionError without the tag, and the hook inverts it to a pass with the tag present
  • nox -s coverage_report — 96.6% (>= 96.5% threshold)
  • nox -s integration_tests — 329 Robot tests passed
  • nox -s complexity, nox -s benchmark, nox -s benchmark_regression, nox -s docs, nox -s build all green

Closes #92

## Summary Adds a failing-first Behave regression test proving issue #91: a graph route whose edge `target` (or `source`) references a non-existent node is accepted at load time and only fails by silently truncating execution — violating §11 (load-time validation, `ConfigurationError`), §11.3.4 (edge source/target must reference existing nodes), and §6.11.5 (edge validation) of the Actor Configuration Standard. The scenario drives a graph actor whose only edge targets an undefined node through the public `create_executor()`/`execute()` API and asserts a `ConfigurationError` naming the missing node is raised before any node runs. Today it isn't: the actor executes every node up to the dangling edge and returns partial output silently instead. The scenario is tagged `@tdd_issue`, `@tdd_issue_91`, and `@tdd_expected_fail` per the TDD issue-capture workflow. The actual fix lands separately on `bugfix/m1-graph-edge-target-validation` per issue #91. ## Test plan - [x] `nox -s lint` green - [x] `nox -s format -- --check` green - [x] `nox -s typecheck` green - [x] `nox -s security_scan` green - [x] `nox -s dead_code` green - [x] `nox -s unit_tests` green (2900 scenarios, including the new tagged scenario) — confirms the assertion fails via `AssertionError` without the tag, and the hook inverts it to a pass with the tag present - [x] `nox -s coverage_report` — 96.6% (>= 96.5% threshold) - [x] `nox -s integration_tests` — 329 Robot tests passed - [x] `nox -s complexity`, `nox -s benchmark`, `nox -s benchmark_regression`, `nox -s docs`, `nox -s build` all green Closes #92
CoreRasurae added this to the v2.1.0 milestone 2026-08-04 11:40:33 +00:00
hurui200320 left a comment

PR Review: !102 (Ticket #92)

Verdict: Approve

The PR delivers a well-structured TDD regression test that captures the runtime gap described in issue #91/#92: graph configs with dangling edge targets are accepted by create_executor()/execute() and execute partially instead of raising ConfigurationError before any node runs. The scenario uses the mandatory @tdd_issue @tdd_issue_91 @tdd_expected_fail tag set, signals the bug via AssertionError, and the step definitions are fully implemented. No critical or major issues were found.

Critical Issues

None

Major Issues

None

Minor Issues

None

Nits

  1. Test coverage completenessfeatures/pure_graph_edge_target_validation.feature:24: The scenario title and feature mention validating both edge target and source, but only the dangling-target case is exercised. Consider adding a second @tdd_issue_91 scenario for an undefined source node, or ensure the companion bugfix PR covers it (issue #91 AC explicitly requires both).
  2. Assertion vs. step wordingfeatures/steps/pure_graph_edge_target_validation_steps.py:67-78: The Then step text promises the ConfigurationError was raised "before any node ran", but the assertions only check the error type and message content. A stronger assertion that context.executor_result is None would make the "before any node ran" guarantee explicit and guard against a future fix that raises the error after partial execution.

Summary

A clean, focused TDD issue-capture PR. It correctly exercises the public runtime API, follows the project's @tdd_expected_fail workflow, and leaves the codebase in a mergeable state with the bug behavior documented and guarded. The two nits above are optional improvements and do not block approval.

## PR Review: !102 (Ticket #92) ### Verdict: Approve The PR delivers a well-structured TDD regression test that captures the runtime gap described in issue #91/#92: graph configs with dangling edge targets are accepted by `create_executor()`/`execute()` and execute partially instead of raising `ConfigurationError` before any node runs. The scenario uses the mandatory `@tdd_issue @tdd_issue_91 @tdd_expected_fail` tag set, signals the bug via `AssertionError`, and the step definitions are fully implemented. No critical or major issues were found. ### Critical Issues None ### Major Issues None ### Minor Issues None ### Nits 1. **Test coverage completeness** — `features/pure_graph_edge_target_validation.feature:24`: The scenario title and feature mention validating both edge `target` and `source`, but only the dangling-target case is exercised. Consider adding a second `@tdd_issue_91` scenario for an undefined `source` node, or ensure the companion bugfix PR covers it (issue #91 AC explicitly requires both). 2. **Assertion vs. step wording** — `features/steps/pure_graph_edge_target_validation_steps.py:67-78`: The Then step text promises the `ConfigurationError` was raised "before any node ran", but the assertions only check the error type and message content. A stronger assertion that `context.executor_result is None` would make the "before any node ran" guarantee explicit and guard against a future fix that raises the error after partial execution. ### Summary A clean, focused TDD issue-capture PR. It correctly exercises the public runtime API, follows the project's `@tdd_expected_fail` workflow, and leaves the codebase in a mergeable state with the bug behavior documented and guarded. The two nits above are optional improvements and do not block approval.
CoreRasurae force-pushed tdd/m1-graph-edge-target-validation from ba1a747936
Some checks failed
CI / lint (pull_request) Successful in 52s
CI / typecheck (pull_request) Successful in 1m36s
CI / security (pull_request) Successful in 1m17s
CI / build (pull_request) Successful in 58s
CI / quality (pull_request) Successful in 1m48s
CI / integration_tests (pull_request) Successful in 4m20s
CI / unit_tests (pull_request) Successful in 6m12s
CI / coverage (pull_request) Failing after 14m27s
CI / status-check (pull_request) Failing after 18s
CI / benchmark (pull_request) Failing after 23m57s
to 25ab52b204
Some checks failed
CI / lint (pull_request) Successful in 1m4s
CI / typecheck (pull_request) Successful in 1m39s
CI / security (pull_request) Successful in 1m32s
CI / quality (pull_request) Successful in 59s
CI / build (pull_request) Successful in 1m43s
CI / integration_tests (pull_request) Successful in 3m58s
CI / unit_tests (pull_request) Successful in 6m8s
CI / status-check (pull_request) Has been cancelled
CI / coverage (pull_request) Has been cancelled
CI / benchmark (pull_request) Has been cancelled
2026-08-04 12:32:41 +00:00
Compare
Author
Member

Thanks for the review, @hurui200320! Addressing both nits from your review (approved, no blockers):

Nit 1 — Test coverage completeness (undefined source case): not applied here, intentionally

Checked this against the ticket metadata before deciding:

  • Issue #92 (the TDD-capture issue this PR closes) scopes its Background, Current/Expected behavior, Acceptance criteria, and Subtasks exclusively to the dangling target case ("Loads a graph config whose edge target names an undefined node"). It never mentions source.
  • Issue #91 (referenced in the tags) was closed as a duplicate of #89, which is the actual bug ticket. #89's own "Tests (Behave)" subtask already commits to "scenarios for invalid target, invalid source, start/end normalization ... and a valid graph loading cleanly" on the companion bugfix/m1-graph-edge-target-validation branch — i.e. the source case is already owned and tracked there, not here.

I agree that §11.3.4/§6.11.5 state the source/target rule as one combined normative requirement, so testing both together isn't wrong in the abstract. But extending this scenario here would push issue #92 past its own declared single-responsibility scope and duplicate work #89 already commits to delivering. So I left this PR as-is — the source scenario will land with the fix on bugfix/m1-graph-edge-target-validation, not deferred vaguely but explicitly tracked in #89's subtasks.

Nit 2 — Assertion vs. step wording (executor_result is None): applied

features/steps/pure_graph_edge_target_validation_steps.py's Then step now also asserts context.executor_result is None, in addition to the existing ConfigurationError type/message checks. This makes the "before any node ran" guarantee explicit and would catch a future (incorrect) fix that raises the error only after partial execution instead of before it, exactly as you flagged.

Verified via nox -s unit_tests (targeted at this feature file): the scenario still correctly fails via AssertionError on the new assertion when @tdd_expected_fail is removed, and passes with the tag present (hook inversion) — so the regression-capture behavior is unchanged, just strengthened.

Also re-ran nox -s lint, nox -s typecheck, and nox -s format -- --check — all green. No coverage_report re-run was needed: the change only touches features/steps/..., which is fully excluded from Slipcover's --omit patterns, so there's no coverage delta from this change.

Since this branch only had a single commit, I amended it in place (commit 25ab52b) rather than adding a follow-up commit, per project commit-hygiene rules, and force-pushed.

Thanks for the review, @hurui200320! Addressing both nits from your review (approved, no blockers): **Nit 1 — Test coverage completeness (undefined `source` case): not applied here, intentionally** Checked this against the ticket metadata before deciding: - Issue #92 (the TDD-capture issue this PR closes) scopes its Background, Current/Expected behavior, Acceptance criteria, and Subtasks *exclusively* to the dangling **target** case ("Loads a graph config whose edge `target` names an undefined node"). It never mentions `source`. - Issue #91 (referenced in the tags) was closed as a duplicate of #89, which is the actual bug ticket. #89's own "Tests (Behave)" subtask already commits to "scenarios for invalid `target`, invalid `source`, `start`/`end` normalization ... and a valid graph loading cleanly" on the companion `bugfix/m1-graph-edge-target-validation` branch — i.e. the `source` case is already owned and tracked there, not here. I agree that §11.3.4/§6.11.5 state the source/target rule as one combined normative requirement, so testing both together isn't wrong in the abstract. But extending this scenario here would push issue #92 past its own declared single-responsibility scope and duplicate work #89 already commits to delivering. So I left this PR as-is — the `source` scenario will land with the fix on `bugfix/m1-graph-edge-target-validation`, not deferred vaguely but explicitly tracked in #89's subtasks. **Nit 2 — Assertion vs. step wording (`executor_result is None`): applied** `features/steps/pure_graph_edge_target_validation_steps.py`'s `Then` step now also asserts `context.executor_result is None`, in addition to the existing `ConfigurationError` type/message checks. This makes the "before any node ran" guarantee explicit and would catch a future (incorrect) fix that raises the error only after partial execution instead of before it, exactly as you flagged. Verified via `nox -s unit_tests` (targeted at this feature file): the scenario still correctly fails via `AssertionError` on the new assertion when `@tdd_expected_fail` is removed, and passes with the tag present (hook inversion) — so the regression-capture behavior is unchanged, just strengthened. Also re-ran `nox -s lint`, `nox -s typecheck`, and `nox -s format -- --check` — all green. No coverage_report re-run was needed: the change only touches `features/steps/...`, which is fully excluded from Slipcover's `--omit` patterns, so there's no coverage delta from this change. Since this branch only had a single commit, I amended it in place (commit `25ab52b`) rather than adding a follow-up commit, per project commit-hygiene rules, and force-pushed.
CoreRasurae force-pushed tdd/m1-graph-edge-target-validation from 25ab52b204
Some checks failed
CI / lint (pull_request) Successful in 1m4s
CI / typecheck (pull_request) Successful in 1m39s
CI / security (pull_request) Successful in 1m32s
CI / quality (pull_request) Successful in 59s
CI / build (pull_request) Successful in 1m43s
CI / integration_tests (pull_request) Successful in 3m58s
CI / unit_tests (pull_request) Successful in 6m8s
CI / status-check (pull_request) Has been cancelled
CI / coverage (pull_request) Has been cancelled
CI / benchmark (pull_request) Has been cancelled
to 7d0491976d
Some checks failed
CI / lint (pull_request) Successful in 54s
CI / quality (pull_request) Successful in 50s
CI / typecheck (pull_request) Successful in 1m34s
CI / security (pull_request) Successful in 1m41s
CI / build (pull_request) Successful in 54s
CI / unit_tests (pull_request) Has been cancelled
CI / integration_tests (pull_request) Has been cancelled
CI / benchmark (pull_request) Has been cancelled
CI / coverage (pull_request) Has been cancelled
CI / status-check (pull_request) Has been cancelled
2026-08-04 12:41:26 +00:00
Compare
Author
Member

Update on nit 1 (undefined source case): on reconsideration, added it to this PR after all. Since this is still the same TDD-capture PR/branch and the addition is purely additive test coverage for the same underlying regression (issue #91), it doesn't push the fix itself out of scope — only strengthens the capture test, so the earlier scope objection doesn't hold up as a reason to withhold it here.

Added a second scenario + Given/When/Then step set in pure_graph_edge_target_validation.feature / pure_graph_edge_target_validation_steps.py, tagged with the same @tdd_issue @tdd_issue_91 @tdd_expected_fail set.

Note the runtime symptom differs from the target case, so the wording/assertions differ slightly: a dangling edge source can never actually be traversed (nothing ever "is" the missing node to transition from), so instead of the graph running partially and exiting silently (the target case), it executes to completion normally, silently ignoring the structurally invalid edge. Both cases still violate §11.3.4/§6.11.5's requirement to reject such edges at load time, and both are asserted via ConfigurationError + executor_result is None (per your nit 2, applied to both scenarios now).

Verified via targeted nox -s unit_tests (2 scenarios / 6 steps passing, correctly reproducing the bug with @tdd_expected_fail present), and re-ran nox -s lint, nox -s typecheck, nox -s format -- --check — all green. Commit amended again (7d04919) and force-pushed.

Update on nit 1 (undefined `source` case): on reconsideration, added it to this PR after all. Since this is still the same TDD-capture PR/branch and the addition is purely additive test coverage for the same underlying regression (issue #91), it doesn't push the fix itself out of scope — only strengthens the capture test, so the earlier scope objection doesn't hold up as a reason to withhold it here. Added a second scenario + Given/When/Then step set in `pure_graph_edge_target_validation.feature` / `pure_graph_edge_target_validation_steps.py`, tagged with the same `@tdd_issue @tdd_issue_91 @tdd_expected_fail` set. Note the runtime symptom differs from the target case, so the wording/assertions differ slightly: a dangling edge **source** can never actually be traversed (nothing ever "is" the missing node to transition from), so instead of the graph running partially and exiting silently (the target case), it executes to completion normally, silently ignoring the structurally invalid edge. Both cases still violate §11.3.4/§6.11.5's requirement to reject such edges at load time, and both are asserted via `ConfigurationError` + `executor_result is None` (per your nit 2, applied to both scenarios now). Verified via targeted `nox -s unit_tests` (2 scenarios / 6 steps passing, correctly reproducing the bug with `@tdd_expected_fail` present), and re-ran `nox -s lint`, `nox -s typecheck`, `nox -s format -- --check` — all green. Commit amended again (`7d04919`) and force-pushed.
test(graph): capture unvalidated edge target regression (#91)
Some checks failed
CI / lint (pull_request) Successful in 1m3s
CI / quality (pull_request) Successful in 51s
CI / typecheck (pull_request) Successful in 1m48s
CI / security (pull_request) Successful in 1m55s
CI / build (pull_request) Successful in 1m30s
CI / integration_tests (pull_request) Successful in 3m3s
CI / unit_tests (pull_request) Successful in 5m5s
CI / coverage (pull_request) Successful in 5m13s
CI / benchmark (pull_request) Has been cancelled
CI / status-check (pull_request) Successful in 7s
CI / lint (push) Successful in 47s
CI / quality (push) Successful in 59s
CI / typecheck (push) Successful in 1m49s
CI / security (push) Successful in 1m57s
CI / build (push) Successful in 1m58s
CI / integration_tests (push) Successful in 3m30s
CI / unit_tests (push) Successful in 4m8s
CI / coverage (push) Successful in 4m56s
CI / status-check (push) Successful in 8s
CI / benchmark (push) Failing after 17m45s
da37a3db3a
Adds a failing-first Behave regression test proving issue #91:
PureLangGraph._analyze_graph() builds its adjacency lists straight
from the configured edges without checking that each edge's
source/target names a node that actually exists. When execution
later reaches a node name that pure_graph.py's _execute_from_node()
cannot find in self.nodes, it logs an error and returns the
accumulated output as-is instead of raising. The net effect: a graph
with a dangling edge target loads without error, runs every node up
to the dangling edge, and then exits silently with partial output —
violating the load-time validation required by the Actor
Configuration Standard §11 preamble, §11.3.4 (graph edge
source/target must reference existing nodes), and §6.11.5 (edge
validation).

The scenario is tagged @tdd_issue, @tdd_issue_91, and
@tdd_expected_fail per the TDD issue-capture workflow: it drives a
graph actor whose only edge targets an undefined node through the
public create_executor()/execute() API and asserts a
ConfigurationError naming the missing node was raised before any
node ran. Confirmed the assertion fails via AssertionError when the
tag is removed, and passes (via TddExpectedFailPolicy's inversion)
with the tag present.

Per review feedback from Rui Hu on !102, the Then step now also
asserts executor_result is None, making the "before any node ran"
guarantee explicit instead of relying only on the error type/message
checks. A second scenario/step set covers the reviewer's other nit,
an edge whose source (rather than target) names an undefined node —
this manifests differently at runtime (the dangling edge is simply
unreachable and never traversed, so the graph completes normally
instead of exiting partially), so it gets its own Given/When/Then
with wording matching that symptom, tagged with the same
@tdd_issue/@tdd_issue_91/@tdd_expected_fail set.

The actual fix lands separately on
bugfix/m1-graph-edge-target-validation per issue #91.

Refs: #91, #92
CoreRasurae force-pushed tdd/m1-graph-edge-target-validation from 7d0491976d
Some checks failed
CI / lint (pull_request) Successful in 54s
CI / quality (pull_request) Successful in 50s
CI / typecheck (pull_request) Successful in 1m34s
CI / security (pull_request) Successful in 1m41s
CI / build (pull_request) Successful in 54s
CI / unit_tests (pull_request) Has been cancelled
CI / integration_tests (pull_request) Has been cancelled
CI / benchmark (pull_request) Has been cancelled
CI / coverage (pull_request) Has been cancelled
CI / status-check (pull_request) Has been cancelled
to da37a3db3a
Some checks failed
CI / lint (pull_request) Successful in 1m3s
CI / quality (pull_request) Successful in 51s
CI / typecheck (pull_request) Successful in 1m48s
CI / security (pull_request) Successful in 1m55s
CI / build (pull_request) Successful in 1m30s
CI / integration_tests (pull_request) Successful in 3m3s
CI / unit_tests (pull_request) Successful in 5m5s
CI / coverage (pull_request) Successful in 5m13s
CI / benchmark (pull_request) Has been cancelled
CI / status-check (pull_request) Successful in 7s
CI / lint (push) Successful in 47s
CI / quality (push) Successful in 59s
CI / typecheck (push) Successful in 1m49s
CI / security (push) Successful in 1m57s
CI / build (push) Successful in 1m58s
CI / integration_tests (push) Successful in 3m30s
CI / unit_tests (push) Successful in 4m8s
CI / coverage (push) Successful in 4m56s
CI / status-check (push) Successful in 8s
CI / benchmark (push) Failing after 17m45s
2026-08-04 12:43:53 +00:00
Compare
CoreRasurae deleted branch tdd/m1-graph-edge-target-validation 2026-08-04 12:58:47 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveractors-core!102
No description provided.