cleveragents/cleveractors-core

Fork 0

fix(langgraph): implement content_not_contains condition in pure graph edge evaluator #100

Merged

CoreRasurae merged 1 commit from bugfix/m1-pure-graph-content-not-contains into master

2026-08-04 12:39:02 +00:00

CoreRasurae commented

2026-08-04 07:29:34 +00:00

Member

Summary

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.
Removes @tdd_expected_fail from the issue #95 regression scenario (kept @tdd_issue/@tdd_issue_95 forever as a regression guard), 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 both directions.
CHANGELOG entry added under ### Fixed.

Branch note: this branch is stacked on tdd/m1-pure-graph-content-not-contains (PR #99, not yet merged) since the fix builds on that regression test. Once #99 merges to master, this PR's diff will automatically shrink to just the bugfix commit.

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 — 2795/2795 scenarios, including the now-unconditionally-passing @tdd_issue_95 scenario and 5 new direct coverage scenarios
nox -s integration_tests green — 321/321 tests, including the 2 new Robot tests
nox -s coverage_report — 96.8% (>= 96.5% threshold)
nox -s benchmark_regression -- --quick — flags 1.1-1.56x noise in unrelated registry/cache/local_store/token_budget benchmarks (none touch pure_graph.py; no benchmark exists for this code path); consistent with known --quick-mode measurement noise on a loaded runner, not a real regression from this change

Note: Forgejo CI is currently down (confirmed by maintainer) — all checks above were run and verified locally.

Closes #95

## Summary `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`. - Removes `@tdd_expected_fail` from the issue #95 regression scenario (kept `@tdd_issue`/`@tdd_issue_95` forever as a regression guard), 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 both directions. - CHANGELOG entry added under `### Fixed`. **Branch note:** this branch is stacked on `tdd/m1-pure-graph-content-not-contains` (PR #99, not yet merged) since the fix builds on that regression test. Once #99 merges to `master`, this PR's diff will automatically shrink to just the bugfix commit. ## 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 — 2795/2795 scenarios, including the now-unconditionally-passing `@tdd_issue_95` scenario and 5 new direct coverage scenarios - [x] `nox -s integration_tests` green — 321/321 tests, including the 2 new Robot tests - [x] `nox -s coverage_report` — 96.8% (>= 96.5% threshold) - [x] `nox -s benchmark_regression -- --quick` — flags 1.1-1.56x noise in unrelated registry/cache/local_store/token_budget benchmarks (none touch `pure_graph.py`; no benchmark exists for this code path); consistent with known `--quick`-mode measurement noise on a loaded runner, not a real regression from this change Note: Forgejo CI is currently down (confirmed by maintainer) — all checks above were run and verified locally. Closes #95

CoreRasurae added this to the v2.1.0 milestone

2026-08-04 07:29:34 +00:00

CoreRasurae added a new dependency

2026-08-04 07:31:05 +00:00

#95 Pure-graph edge evaluator does not implement content_not_contains, causing mutually-exclusive branch conditions to both fire

CoreRasurae added the

Type

Bug

label

2026-08-04 07:31:07 +00:00

CoreRasurae commented

2026-08-04 07:31:32 +00:00

Author

Member

LGTM ✅ — self-approving per project policy (1 approval required, self-approval permitted). All quality gates verified locally: nox -s lint, nox -s format -- --check, nox -s typecheck, nox -s security_scan, nox -s dead_code, nox -s unit_tests (2795/2795 scenarios), nox -s integration_tests (321/321 tests), and nox -s coverage_report (96.8%) all pass. nox -s benchmark_regression -- --quick flags noise in unrelated registry/cache/local_store/token_budget benchmarks (not touched by this change, and no benchmark exists for pure_graph.py) — consistent with known --quick-mode measurement noise. Forgejo CI is currently down per maintainer confirmation; not blocking on it this session. Leaving merge to a maintainer/human.

LGTM ✅ — self-approving per project policy (1 approval required, self-approval permitted). All quality gates verified locally: `nox -s lint`, `nox -s format -- --check`, `nox -s typecheck`, `nox -s security_scan`, `nox -s dead_code`, `nox -s unit_tests` (2795/2795 scenarios), `nox -s integration_tests` (321/321 tests), and `nox -s coverage_report` (96.8%) all pass. `nox -s benchmark_regression -- --quick` flags noise in unrelated registry/cache/local_store/token_budget benchmarks (not touched by this change, and no benchmark exists for pure_graph.py) — consistent with known --quick-mode measurement noise. Forgejo CI is currently down per maintainer confirmation; not blocking on it this session. Leaving merge to a maintainer/human.

CoreRasurae force-pushed bugfix/m1-pure-graph-content-not-contains from 3f57ecf9fe

CI / lint (pull_request) Successful in 1m1s

Details

CI / security (pull_request) Successful in 1m20s

Details

CI / typecheck (pull_request) Successful in 1m20s

Details

CI / quality (pull_request) Successful in 1m31s

Details

CI / build (pull_request) Successful in 41s

Details

CI / integration_tests (pull_request) Successful in 2m50s

Details

CI / unit_tests (pull_request) Successful in 4m36s

Details

CI / coverage (pull_request) Successful in 5m23s

Details

CI / status-check (pull_request) Successful in 9s

Details

CI / benchmark (pull_request) Failing after 19m3s

Details

to 7511c62c8c

CI / quality (pull_request) Successful in 1m5s

Details

CI / security (pull_request) Successful in 1m31s

Details

CI / typecheck (pull_request) Successful in 1m36s

Details

CI / lint (pull_request) Successful in 1m35s

Details

CI / build (pull_request) Successful in 1m30s

Details

CI / integration_tests (pull_request) Successful in 4m38s

Details

CI / unit_tests (pull_request) Successful in 5m10s

Details

CI / benchmark (pull_request) Failing after 20m40s

Details

CI / coverage (pull_request) Successful in 4m47s

Details

CI / status-check (pull_request) Successful in 17s

Details

2026-08-04 10:20:35 +00:00

Compare

hurui200320 approved these changes

2026-08-04 10:53:37 +00:00

hurui200320 left a comment

PR Review: !100 (Ticket #95)

Verdict: Approve

The fix correctly implements content_not_contains in PureLangGraph._evaluate_edge_condition() per Actor Configuration Standard §5.4, and the test matrix (direct evaluator unit-style Behave scenarios plus an end-to-end Robot integration test) provides strong regression coverage for the mutually-exclusive sibling-edge idiom described in #95. I reviewed the changed code, related implementations (nodes.py, bridge.py, reactive/stream_router.py), the spec, and project quality gates. No critical or major correctness issues were found. The remaining items are minor process/documentation gaps.

Critical Issues

None

Major Issues

None

Minor Issues

Coverage falls below the documented 97% threshold.
- The PR reports 96.8% coverage. CONTRIBUTING.md (and the coverage_report docstring) state that the enforced merge gate is 97%, while noxfile.py currently sets COVERAGE_THRESHOLD = 96.5. Because the actual tooling passes, this is not a blocker for this PR, but the project should reconcile the documented threshold with the configured threshold, or add coverage to reach 97%.
The non-string/non-dict fallback in content_not_contains is not exercised by tests.
- File: src/cleveractors/langgraph/pure_graph.py, line 2054 (return True).
- The implementation mirrors content_contains's existing fallback pattern, but the True branch for unknown message shapes has no direct test. Adding a Behave scenario (e.g., message is a list/object that does not contain the text) would improve confidence that the fallback behaves consistently with the semantics documented in the code comment.

Nits

Step-definition docstring is stale.
- File: features/steps/pure_graph_content_not_contains_steps.py, lines 1–6.
- The module docstring says it exercises PureLangGraph._get_next_nodes(), but the new scenarios also call _evaluate_edge_condition() directly. Update the docstring to mention both entry points.

Summary

This is a focused, well-tested bugfix. The core change is minimal and symmetric with the existing content_contains branch, and the added Behave/Robot scenarios directly cover the regression from #95. The only items worth addressing are minor: aligning the documented vs. configured coverage threshold and adding coverage for the final fallback branch. I am approving so the fix can land; the minor items can be handled in a fast-follow or as part of the ongoing coverage work.

## PR Review: !100 (Ticket #95) ### Verdict: Approve The fix correctly implements `content_not_contains` in `PureLangGraph._evaluate_edge_condition()` per Actor Configuration Standard §5.4, and the test matrix (direct evaluator unit-style Behave scenarios plus an end-to-end Robot integration test) provides strong regression coverage for the mutually-exclusive sibling-edge idiom described in #95. I reviewed the changed code, related implementations (`nodes.py`, `bridge.py`, `reactive/stream_router.py`), the spec, and project quality gates. No critical or major correctness issues were found. The remaining items are minor process/documentation gaps. ### Critical Issues None ### Major Issues None ### Minor Issues 1. **Coverage falls below the documented 97% threshold.** - The PR reports 96.8% coverage. `CONTRIBUTING.md` (and the `coverage_report` docstring) state that the enforced merge gate is **97%**, while `noxfile.py` currently sets `COVERAGE_THRESHOLD = 96.5`. Because the actual tooling passes, this is not a blocker for this PR, but the project should reconcile the documented threshold with the configured threshold, or add coverage to reach 97%. 2. **The non-string/non-dict fallback in `content_not_contains` is not exercised by tests.** - File: `src/cleveractors/langgraph/pure_graph.py`, line 2054 (`return True`). - The implementation mirrors `content_contains`'s existing fallback pattern, but the `True` branch for unknown message shapes has no direct test. Adding a Behave scenario (e.g., message is a list/object that does not contain the text) would improve confidence that the fallback behaves consistently with the semantics documented in the code comment. ### Nits 1. **Step-definition docstring is stale.** - File: `features/steps/pure_graph_content_not_contains_steps.py`, lines 1–6. - The module docstring says it exercises `PureLangGraph._get_next_nodes()`, but the new scenarios also call `_evaluate_edge_condition()` directly. Update the docstring to mention both entry points. ### Summary This is a focused, well-tested bugfix. The core change is minimal and symmetric with the existing `content_contains` branch, and the added Behave/Robot scenarios directly cover the regression from #95. The only items worth addressing are minor: aligning the documented vs. configured coverage threshold and adding coverage for the final fallback branch. I am approving so the fix can land; the minor items can be handled in a fast-follow or as part of the ongoing coverage work.

CoreRasurae commented

2026-08-04 11:31:49 +00:00

Author

Member

Minor issues number 1, is not relevant, we have a noxfile threshold at 96.5% while having a 97% in the specification, but the 97% is the rounded value, which is also obtainable from a 96.5% real coverage.

CoreRasurae added 1 commit

2026-08-04 12:15:48 +00:00

fix(langgraph): implement content_not_contains condition in pure graph edge evaluator

CI / lint (pull_request) Successful in 53s

Details

CI / typecheck (pull_request) Successful in 1m38s

Details

CI / security (pull_request) Successful in 1m22s

Details

CI / quality (pull_request) Successful in 1m57s

Details

CI / build (pull_request) Successful in 58s

Details

CI / integration_tests (pull_request) Successful in 3m31s

Details

CI / unit_tests (pull_request) Successful in 6m3s

Details

CI / coverage (pull_request) Successful in 6m5s

Details

CI / status-check (pull_request) Successful in 16s

Details

CI / benchmark (pull_request) Failing after 16m56s

Details

CI / lint (push) Successful in 1m5s

Details

CI / typecheck (push) Successful in 1m45s

Details

CI / security (push) Successful in 2m11s

Details

CI / quality (push) Successful in 2m18s

Details

CI / build (push) Successful in 2m29s

Details

CI / integration_tests (push) Successful in 3m5s

Details

CI / unit_tests (push) Successful in 4m30s

Details

CI / coverage (push) Successful in 5m7s

Details

CI / status-check (push) Successful in 14s

Details

CI / benchmark (push) Successful in 15m51s

Details

a0c2119cc3

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