From a0c2119cc30ef8c5353aeba0f7f6b2e79a1856e4 Mon Sep 17 00:00:00 2001 From: "Luis Mendes (CoreRasurae)" Date: Tue, 4 Aug 2026 06:18:41 +0000 Subject: [PATCH] fix(langgraph): implement content_not_contains condition in pure graph edge evaluator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PureLangGraph._evaluate_edge_condition() had no branch for content_not_contains (Actor Configuration Standard §5.4), so it fell into the "unknown condition type" catch-all and always returned True. A classifier node with sibling content_contains/content_not_contains edges (the classic if/else idiom) would traverse both edges whenever the configured text was present, instead of the single edge intended. Adds a content_not_contains branch mirroring the existing correct implementations in nodes.py, bridge.py, and reactive/stream_router.py: true when the text is absent from the (stringified) message content, for both string and dict messages; falls back to True (not found, so not contained) for any other message shape, matching the sibling content_not_contains implementations' "absent" fallback rather than content_contains's False fallback. Removes @tdd_expected_fail from the issue #95 regression scenario now that the fix makes it pass unconditionally, and adds direct content_not_contains true/false coverage plus a sibling-edge scenario for the text-absent direction. Adds a Robot integration test (robot/pure_graph_sibling_edges.robot, robot/PureGraphLib.py) that executes a real two-branch pure graph end-to-end and asserts exactly one branch node runs for each direction. Per rui.hu's PR #100 review: adds direct coverage of the non-string/ non-dict message fallback branch (previously untested) and updates the step-definitions module docstring, which was stale about which method the scenarios exercise. The reviewer's coverage-threshold-reconciliation note is a pre-existing, project-wide config/doc mismatch out of scope for this issue and was already addressed in review comments. Verified locally (Forgejo CI is currently down): nox -s lint, format --check, typecheck, unit_tests (2905/2905 scenarios) are green, and the new fallback scenario covers the previously-uncovered line in pure_graph.py. ISSUES CLOSED: #95 Refs: #96 --- CHANGELOG.md | 4 + .../pure_graph_content_not_contains.feature | 33 ++++++- .../pure_graph_content_not_contains_steps.py | 60 ++++++++++++- robot/PureGraphLib.py | 89 +++++++++++++++++++ robot/pure_graph_sibling_edges.robot | 21 +++++ src/cleveractors/langgraph/pure_graph.py | 13 +++ 6 files changed, 217 insertions(+), 3 deletions(-) create mode 100644 robot/PureGraphLib.py create mode 100644 robot/pure_graph_sibling_edges.robot diff --git a/CHANGELOG.md b/CHANGELOG.md index e3836f7..df10576 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,10 @@ and this project adheres to [Clever Semantic Versioning](https://www.w3.org/subm ### Fixed +- **Pure-Graph `content_not_contains` Edge Condition Not Implemented (issue #95)** (`langgraph/pure_graph.py`): `PureLangGraph._evaluate_edge_condition()` had no branch for the `content_not_contains` condition type (Actor Configuration Standard §5.4), so it fell into the "unknown condition type" catch-all and always evaluated to `True`, logging `Unknown condition type: content_not_contains`. Graphs using the common classifier if/else idiom — a pair of sibling edges from the same source node, one `content_contains`, one `content_not_contains` — would traverse both edges whenever the configured text was present, instead of the single edge the graph author intended. Fixed by adding a `content_not_contains` branch that evaluates against the actual message content, mirroring the existing correct implementations in `nodes.py`, `bridge.py`, and `reactive/stream_router.py`. Regression scenarios (tagged `@tdd_issue_95`) and a new Robot integration test (`robot/pure_graph_sibling_edges.robot`) verify sibling edges now select exactly one branch, including direct coverage of the non-string/non-dict message fallback. + + **Module:** `src/cleveractors/langgraph/pure_graph.py` (`PureLangGraph._evaluate_edge_condition`). BDD: scenarios in `features/pure_graph_content_not_contains.feature`. Robot: `robot/pure_graph_sibling_edges.robot`. + - **ConnectionError Retry Exhaustion Silent Swallow (issue #71)** (`nodes.py`): When all LLM communication retries are exhausted and `call_with_retry()` raises `ExecutionError(kind="timeout")`, the exception was systematically caught by broad `except Exception` handlers in `Node._execute_agent()` and `Node.execute()`, converting it into an error string in graph state. The retry mechanism's work (backoff, budget tracking, error reporting) was wasted and the graph silently returned error strings or empty responses instead of propagating the timeout. Fixed by adding targeted `except ExecutionError as e` handlers that check `e.kind == "timeout"` and re-raise, while non-timeout `ExecutionError` instances (tool errors, etc.) continue to be caught and handled gracefully. Two new BDD scenarios verify both propagation and graceful handling paths. **Module:** `src/cleveractors/langgraph/nodes.py`. BDD: scenarios in `features/nodes_coverage_gaps.feature`. diff --git a/features/pure_graph_content_not_contains.feature b/features/pure_graph_content_not_contains.feature index 076ab8b..4ea281f 100644 --- a/features/pure_graph_content_not_contains.feature +++ b/features/pure_graph_content_not_contains.feature @@ -15,9 +15,40 @@ Feature: Pure-graph edge evaluator honors content_not_contains conditions Background: Given a fresh pure graph test context (cnc) - @tdd_issue @tdd_issue_95 @tdd_expected_fail + @tdd_issue @tdd_issue_95 Scenario: Sibling content_contains/content_not_contains edges select exactly one branch Given a classifier node with sibling content_contains and content_not_contains edges for "NO_FIXES" (cnc) When the classifier output contains "NO_FIXES" (cnc) Then exactly one next node should be selected (cnc) And the selected next node should be the content_contains target (cnc) + + Scenario: content_not_contains evaluates true with a string message lacking the text + Given an edge with a content_not_contains condition for "needle" (cnc) + When the edge is evaluated against the string message "just haystack" (cnc) + Then the edge condition should evaluate to true (cnc) + + Scenario: content_not_contains evaluates false with a string message containing the text + Given an edge with a content_not_contains condition for "needle" (cnc) + When the edge is evaluated against the string message "haystack needle here" (cnc) + Then the edge condition should evaluate to false (cnc) + + Scenario: content_not_contains evaluates true with a dict message lacking the text + Given an edge with a content_not_contains condition for "found_me" (cnc) + When the edge is evaluated against the dict message content "nothing here" (cnc) + Then the edge condition should evaluate to true (cnc) + + Scenario: content_not_contains evaluates false with a dict message containing the text + Given an edge with a content_not_contains condition for "found_me" (cnc) + When the edge is evaluated against the dict message content "prefix found_me suffix" (cnc) + Then the edge condition should evaluate to false (cnc) + + Scenario: content_not_contains evaluates true with a non-string non-dict message + Given an edge with a content_not_contains condition for "needle" (cnc) + When the edge is evaluated against the integer message 42 (cnc) + Then the edge condition should evaluate to true (cnc) + + Scenario: Sibling edges select exactly one branch when the text is absent + Given a classifier node with sibling content_contains and content_not_contains edges for "NO_FIXES" (cnc) + When the classifier output does not contain "NO_FIXES" (cnc) + Then exactly one next node should be selected (cnc) + And the selected next node should be the content_not_contains target (cnc) diff --git a/features/steps/pure_graph_content_not_contains_steps.py b/features/steps/pure_graph_content_not_contains_steps.py index 6265260..f66cef9 100644 --- a/features/steps/pure_graph_content_not_contains_steps.py +++ b/features/steps/pure_graph_content_not_contains_steps.py @@ -2,8 +2,10 @@ Exercises ``PureLangGraph._get_next_nodes()`` with the classic classifier if/else idiom: sibling edges from the same source node, one -``content_contains``, one ``content_not_contains``. See -``pure_graph_content_not_contains.feature`` for the scenario this backs. +``content_contains``, one ``content_not_contains``. Also exercises +``PureLangGraph._evaluate_edge_condition()`` directly against string, dict, +and non-string/non-dict messages. See +``pure_graph_content_not_contains.feature`` for the scenarios this backs. """ from __future__ import annotations @@ -66,3 +68,57 @@ def step_selected_is_contains_target(context): assert context.next_nodes == [CONTAINS_TARGET], ( f"Expected [{CONTAINS_TARGET!r}], got {context.next_nodes!r}" ) + + +@when('the classifier output does not contain "{text}" (cnc)') +def step_classifier_output_does_not_contain(context, text): + context.next_nodes = context.graph._get_next_nodes( + "classifier", "Everything looks fine, no issues were found in this batch." + ) + + +@then("the selected next node should be the content_not_contains target (cnc)") +def step_selected_is_not_contains_target(context): + assert context.next_nodes == [NOT_CONTAINS_TARGET], ( + f"Expected [{NOT_CONTAINS_TARGET!r}], got {context.next_nodes!r}" + ) + + +@given('an edge with a content_not_contains condition for "{text}" (cnc)') +def step_edge_content_not_contains(context, text): + config = PureGraphConfig( + name="test_cnc", + nodes={"w": NodeConfig(name="w", type=NodeType.FUNCTION)}, + edges=[], + ) + context.graph = PureLangGraph(config) + context.edge = Edge( + source="w", target="t", condition={"type": "content_not_contains", "text": text} + ) + + +@when('the edge is evaluated against the string message "{message}" (cnc)') +def step_evaluate_against_string_message(context, message): + context.edge_result = context.graph._evaluate_edge_condition(context.edge, message) + + +@when('the edge is evaluated against the dict message content "{content}" (cnc)') +def step_evaluate_against_dict_message(context, content): + context.edge_result = context.graph._evaluate_edge_condition( + context.edge, {"content": content} + ) + + +@when("the edge is evaluated against the integer message {value:d} (cnc)") +def step_evaluate_against_integer_message(context, value): + context.edge_result = context.graph._evaluate_edge_condition(context.edge, value) + + +@then("the edge condition should evaluate to true (cnc)") +def step_edge_result_true(context): + assert context.edge_result is True, f"Expected True, got {context.edge_result!r}" + + +@then("the edge condition should evaluate to false (cnc)") +def step_edge_result_false(context): + assert context.edge_result is False, f"Expected False, got {context.edge_result!r}" diff --git a/robot/PureGraphLib.py b/robot/PureGraphLib.py new file mode 100644 index 0000000..7ff0e62 --- /dev/null +++ b/robot/PureGraphLib.py @@ -0,0 +1,89 @@ +"""Robot Framework keyword library for PureLangGraph edge-condition integration tests. + +Exercises the real PureLangGraph execution path (no mocks, no external +services) end-to-end to prove that a classifier node with sibling +content_contains / content_not_contains edges (issue #95) selects exactly +one branch, per the Actor Configuration Standard §5.4. +""" + +from __future__ import annotations + +import asyncio +from typing import Any + +from cleveractors.langgraph.nodes import Edge, NodeConfig, NodeType +from cleveractors.langgraph.pure_graph import PureGraphConfig, PureLangGraph + +CONTAINS_BRANCH = "ui_branch" +NOT_CONTAINS_BRANCH = "fixes_branch" + + +class PureGraphLib: # pragma: no cover - integration test library + """Keyword library for PureLangGraph sibling-edge integration tests.""" + + ROBOT_LIBRARY_SCOPE = "TEST SUITE" + + def create_sibling_branch_pure_graph(self) -> None: + """Build a small pure graph mirroring the classifier if/else idiom. + + classifier -> ui_branch (content_contains: NO_FIXES) + classifier -> fixes_branch (content_not_contains: NO_FIXES) + """ + config = PureGraphConfig( + name="sibling_branch_graph", + entry_point="start", + nodes={ + "classifier": NodeConfig( + name="classifier", type=NodeType.FUNCTION, function="validate" + ), + CONTAINS_BRANCH: NodeConfig( + name=CONTAINS_BRANCH, type=NodeType.FUNCTION, function="validate" + ), + NOT_CONTAINS_BRANCH: NodeConfig( + name=NOT_CONTAINS_BRANCH, + type=NodeType.FUNCTION, + function="validate", + ), + }, + edges=[ + Edge(source="start", target="classifier"), + Edge( + source="classifier", + target=CONTAINS_BRANCH, + condition={"type": "content_contains", "text": "NO_FIXES"}, + ), + Edge( + source="classifier", + target=NOT_CONTAINS_BRANCH, + condition={"type": "content_not_contains", "text": "NO_FIXES"}, + ), + Edge(source=CONTAINS_BRANCH, target="end"), + Edge(source=NOT_CONTAINS_BRANCH, target="end"), + ], + ) + self._graph: PureLangGraph = PureLangGraph(config) + + def execute_pure_graph_with_message(self, message: str) -> None: + self._output: Any = asyncio.run(self._graph.execute(message)) + + def exactly_one_branch_node_should_have_executed( + self, node_a: str, node_b: str + ) -> None: + visited = [n for n in self._graph.execution_history if n in (node_a, node_b)] + if len(visited) != 1: + raise AssertionError( + f"Expected exactly one of {node_a!r}/{node_b!r} to execute, " + f"got {visited!r} (full history: {self._graph.execution_history!r})" + ) + + def executed_branch_node_should_be(self, expected_node: str) -> None: + visited = [ + n + for n in self._graph.execution_history + if n in (CONTAINS_BRANCH, NOT_CONTAINS_BRANCH) + ] + if visited != [expected_node]: + raise AssertionError( + f"Expected branch node {expected_node!r} to have executed, " + f"got {visited!r} (full history: {self._graph.execution_history!r})" + ) diff --git a/robot/pure_graph_sibling_edges.robot b/robot/pure_graph_sibling_edges.robot new file mode 100644 index 0000000..6a32a3a --- /dev/null +++ b/robot/pure_graph_sibling_edges.robot @@ -0,0 +1,21 @@ +*** Settings *** +Documentation Integration tests for PureLangGraph sibling content_contains / +... content_not_contains edge conditions (issue #95). Exercises +... the classic classifier if/else idiom end-to-end via +... PureLangGraph.execute() with no mocks. +Library PureGraphLib.py + +*** Test Cases *** +Sibling Edges Select Only The Content Contains Branch When Text Present + [Documentation] Message containing NO_FIXES must route to ui_branch only. + Create Sibling Branch Pure Graph + Execute Pure Graph With Message The review found NO_FIXES needed for this batch. + Exactly One Branch Node Should Have Executed ui_branch fixes_branch + Executed Branch Node Should Be ui_branch + +Sibling Edges Select Only The Content Not Contains Branch When Text Absent + [Documentation] Message without NO_FIXES must route to fixes_branch only. + Create Sibling Branch Pure Graph + Execute Pure Graph With Message Everything looks fine, no issues found. + Exactly One Branch Node Should Have Executed ui_branch fixes_branch + Executed Branch Node Should Be fixes_branch diff --git a/src/cleveractors/langgraph/pure_graph.py b/src/cleveractors/langgraph/pure_graph.py index b08fe6c..8b002ea 100644 --- a/src/cleveractors/langgraph/pure_graph.py +++ b/src/cleveractors/langgraph/pure_graph.py @@ -2040,6 +2040,19 @@ class PureLangGraph: return text in str(content) return False + elif condition_type == "content_not_contains": + # True if the substring is NOT in the message content (§5.4). + # Mirrors content_contains above but inverted, including the + # fallback for message types without determinable content: if + # the text cannot be found, it is, by definition, not contained. + text = edge.condition.get("text", "") + if isinstance(message, str): + return text not in message + elif isinstance(message, dict): + content = message.get("content", "") + return text not in str(content) + return True + elif condition_type == "context_value": # Check context value key = edge.condition.get("key") -- 2.52.0