feat(ActorResult): implement ActorResult and NodeUsage types; capture per-node token counts from LangChain responses #41

2026-06-10T05:49:22Z

hurui200320 commented

2026-06-10 05:49:22 +00:00

Summary

Implements cleveractors-core issue #14 — the full AC1–AC7 acceptance criteria for ActorResult, NodeUsage, and real LangChain token counting.

Closes #14

Motivation

The CleverThis router consumes Executor.execute() to run actors. Before this PR, the router received ActorResult objects whose prompt_tokens and completion_tokens fields were estimated (via tiktoken or a 4-chars/token heuristic) rather than read from LangChain's actual response metadata. This made billing data inaccurate. The router also could not find the canonical ActorResult/NodeUsage types at the spec-mandated location (cleveractors.result).

Changes

New module: `src/cleveractors/result.py` (AC1)

Defines NodeUsage and ActorResult dataclasses at the spec-mandated location. Also defines MAX_REASONABLE_TOKENS = 10_000_000 as the single authoritative constant (previously duplicated in three modules). runtime.py now imports and re-exports both types for backward compatibility. __init__.py updated to import from result instead of runtime. Uses lowercase list[NodeUsage] (PEP 585) throughout. nodes is a required positional field (no default) so the contract that nodes is always non-empty is statically enforced. state field uses dict[str, Any] | None (PEP 585, consistent with rest of codebase). __all__ declaration added.

`LLMAgent` — real token extraction + race-condition fix + billing integrity (AC2, AC3, M1, M2, m3)

process_message() now:

Uses a module-level contextvars.ContextVar (last_token_usage_var) to store per-task token counts. Each asyncio.Task inherits a copy of the context, so parallel graph branches that share the same LLMAgent instance each see their own token counts — eliminating the race condition.
Resets _last_token_usage = (0, 0) and last_token_usage_var.set((0, 0)) as the first statement inside the try: block.
Captures token counts into sentinel variables (_captured_prompt, _captured_completion) immediately after ainvoke() succeeds, before the memory-update block. This marks the "ainvoke succeeded" boundary.
Billing integrity fix: If a post-ainvoke() step (e.g. update_memory()) raises an exception, the except handlers check the sentinel variables. If _captured_prompt is not None, ainvoke() already succeeded and the LLM provider has already charged the user — the captured counts are preserved rather than zeroed. Only pre-ainvoke() failures (where no tokens were consumed) reset to (0, 0).
Added a prominent docstring .. note:: to process_message() documenting this billing integrity guarantee.

`Node._execute_agent()` — race condition fix, agent-type-aware fallback (M1 cycle-9 fix)

Resets last_token_usage_var to (0, 0) at the start of the try: block, before calling agent.process_message().
Agent-type-aware fallback logic (cycle 9): For LLMAgent, the ContextVar is authoritative — (0, 0) means zero tokens, not "unset". The fallback to _last_token_usage instance attribute is never used for LLMAgent instances, preventing the race where a sibling parallel branch overwrites the instance attribute. For non-LLMAgent agents (ToolAgent, ChainAgent, etc.), the ContextVar is always (0, 0) since process_message() does not touch it; these agents fall back to the instance attribute as before.
Fixed _node_token_usage type annotation from Optional[dict[str, Any]] to dict[str, Any] | None (PEP 604, consistent with rest of codebase).

`runtime_dispatch.py` — same agent-type-aware fallback in `_execute_llm` (M1 cycle-9 fix)

Applied the same agent-type-aware fallback pattern to _execute_llm for consistency with Node._execute_agent().
Renamed cryptic exception variable _nu_err → _node_usage_err in _execute_graph.
Multi-actor placeholder normalisation: Added _normalize_node_id() helper and _GRAPH_PLACEHOLDER_RE module-level regex.
Renamed loop variable nu → node_usage in _execute_multi_actor for readability.
Moved import re to the top-level import block.

`pure_graph.py` — import organisation

Moved from cleveractors.result import MAX_REASONABLE_TOKENS as _MAX_REASONABLE_TOKENS to the top-level import block.

`features/mocks/actor_result_helpers.py` — new mock helpers module (cycle-9 fix)

Extracted _make_llm_actor_config and _make_mock_chat_model from the step file into features/mocks/actor_result_helpers.py (renamed to make_llm_actor_config and make_mock_chat_model), per project convention that mock factory functions belong in features/mocks/.

BDD tests updated (M1, M2, m4, m5, m6, cycle-9 fixes)

Strengthened parallel isolation assertions (cycle 9): The "ContextVar isolates token counts" scenario now uses exact-value assertions (t1 == (100, 50) and t2 == (200, 80)) instead of weak != (0, 0) and != each other checks. A buggy swapped-mapping implementation would now fail.
Removed bare except Exception in wrong-arity real-path test (cycle 9): The artc_wrong_arity_real_path_test branch no longer catches all exceptions and fabricates a fake ActorResult. If executor.execute() raises unexpectedly, the test now fails loudly.
Deleted dead step definition step_artc_graph_executor_wrong_arity_tuple (cycle 9): This ~44-line step was never referenced by any scenario in the feature file.
New scenario: ContextVar parallel-branch isolation (M2): A single LLMAgent instance is called concurrently via asyncio.gather() with two different mocked usage_metadata responses; each task asserts its last_token_usage_var.get() reflects its own call's counts.
Updated scenario: Post-ainvoke exception preserves token counts (m4, billing integrity fix): An LLMAgent with memory_enabled=True succeeds at ainvoke() but fails at update_memory(); asserts _last_token_usage == (42, 17) after the exception (previously incorrectly asserted (0, 0)). This verifies that billing data is not lost when memory persistence fails.
New scenarios: large-value pass-through (m5): _safe_node_token_int(20_000_000) and _safe_token_int(20_000_000) both return 20_000_000 unchanged.
Rewritten wrong-arity scenario (m6): The first wrong-arity scenario now exercises the real Node._execute_agent() code path via AgentFactory.create_agent mock.

CHANGELOG.md — stale reference removed

Removed stale _usage_log reference from the "Mutable state hygiene" entry (this field never existed in this PR's scope). The entry now accurately describes the actual new mechanism: _last_token_usage and last_token_usage_var are reset at the start of every process_message() call; PureLangGraph._node_usages is reset at the start of every execute() call; post-ainvoke() exceptions preserve captured token counts for billing integrity.

Architecture Note

AC3 says "process_message() returns token counts alongside the response string." Changing the Agent abstract base class return type from str to tuple[str, int, int] would require updating all four concrete agent implementations plus every mock in the test suite. Instead, the contextvars.ContextVar side-channel satisfies the spirit of AC3 — token counts are available alongside the response, per-task and race-free — without breaking the existing Agent contract.

Billing Integrity Design

When ainvoke() succeeds but a subsequent step (e.g. update_memory()) raises an exception:

The LLM provider has already charged the user for the tokens consumed by ainvoke().
The except handlers check sentinel variables (_captured_prompt, _captured_completion) set immediately after ainvoke() succeeds.
If the sentinels are set (non-None), the captured counts are preserved in _last_token_usage and last_token_usage_var so the router receives accurate billing data.
Only pre-ainvoke() failures (sentinels still None) reset counts to (0, 0), because in those cases no tokens were consumed.

Quality Gates

Gate	Result
`nox -e lint`	✅ All checks passed
`nox -e typecheck`	✅ 0 errors, 1 pre-existing warning
`nox -e unit_tests`	✅ 2298/2298 scenarios passed
`nox -e integration_tests`	✅ All tests passed
`nox -e coverage_report`	✅ 97.1% (threshold 96.5%)

ADR References

ADR-2027: ActorResult Type and Token Aggregation
ADR-2026: Per-request credential injection (state field preserved)

Deferred Items

_safe_int() / _safe_token_int() consolidation (N1): Both helpers implement the same coercion logic. Moving to a shared module is a refactor deferred to a follow-up ticket.
NodeUsageTuple location (issue #13): Moving the type alias to cleveractors.result is a minor organizational improvement deferred to a follow-up.
Literal[...] annotation duplicating _CAUSE_* constants: Python's Literal type does not support referencing constant names directly, so the string values must be duplicated. This is a known limitation of the language and is accepted as-is.
N2 (Two logger instances in nodes.py): Node.__init__ sets self.logger = logging.getLogger(__name__) which produces the same object as the module-level _logger. Consolidating to use _logger directly in Node is a pre-existing nit deferred to a follow-up.
N3 (Token-extraction fallback logic duplicated): Extracting a shared _read_last_token_usage(agent) helper is a refactor deferred to a follow-up.
N4 (_execute_llm/_execute_tool compute totals directly): Inconsistency with sum() pattern used in _execute_graph/_execute_multi_actor is deferred to a follow-up.

## Summary Implements `cleveractors-core` issue #14 — the full AC1–AC7 acceptance criteria for `ActorResult`, `NodeUsage`, and real LangChain token counting. Closes #14 ## Motivation The CleverThis router consumes `Executor.execute()` to run actors. Before this PR, the router received `ActorResult` objects whose `prompt_tokens` and `completion_tokens` fields were **estimated** (via tiktoken or a 4-chars/token heuristic) rather than read from LangChain's actual response metadata. This made billing data inaccurate. The router also could not find the canonical `ActorResult`/`NodeUsage` types at the spec-mandated location (`cleveractors.result`). ## Changes ### New module: `src/cleveractors/result.py` (AC1) Defines `NodeUsage` and `ActorResult` dataclasses at the spec-mandated location. Also defines `MAX_REASONABLE_TOKENS = 10_000_000` as the single authoritative constant (previously duplicated in three modules). `runtime.py` now imports and re-exports both types for backward compatibility. `__init__.py` updated to import from `result` instead of `runtime`. Uses lowercase `list[NodeUsage]` (PEP 585) throughout. `nodes` is a required positional field (no default) so the contract that `nodes` is always non-empty is statically enforced. `state` field uses `dict[str, Any] | None` (PEP 585, consistent with rest of codebase). `__all__` declaration added. ### `LLMAgent` — real token extraction + race-condition fix + billing integrity (AC2, AC3, M1, M2, m3) `process_message()` now: - **Uses a module-level `contextvars.ContextVar` (`last_token_usage_var`)** to store per-task token counts. Each `asyncio.Task` inherits a copy of the context, so parallel graph branches that share the same `LLMAgent` instance each see their own token counts — eliminating the race condition. - **Resets `_last_token_usage = (0, 0)` and `last_token_usage_var.set((0, 0))` as the first statement inside the `try:` block**. - **Captures token counts into sentinel variables (`_captured_prompt`, `_captured_completion`) immediately after `ainvoke()` succeeds**, before the memory-update block. This marks the "ainvoke succeeded" boundary. - **Billing integrity fix**: If a post-`ainvoke()` step (e.g. `update_memory()`) raises an exception, the `except` handlers check the sentinel variables. If `_captured_prompt is not None`, `ainvoke()` already succeeded and the LLM provider has already charged the user — the captured counts are **preserved** rather than zeroed. Only pre-`ainvoke()` failures (where no tokens were consumed) reset to `(0, 0)`. - Added a prominent docstring `.. note::` to `process_message()` documenting this billing integrity guarantee. ### `Node._execute_agent()` — race condition fix, agent-type-aware fallback (M1 cycle-9 fix) - **Resets `last_token_usage_var` to `(0, 0)` at the start of the `try:` block**, before calling `agent.process_message()`. - **Agent-type-aware fallback logic** (cycle 9): For `LLMAgent`, the ContextVar is authoritative — `(0, 0)` means zero tokens, not "unset". The fallback to `_last_token_usage` instance attribute is **never** used for `LLMAgent` instances, preventing the race where a sibling parallel branch overwrites the instance attribute. For non-`LLMAgent` agents (ToolAgent, ChainAgent, etc.), the ContextVar is always `(0, 0)` since `process_message()` does not touch it; these agents fall back to the instance attribute as before. - Fixed `_node_token_usage` type annotation from `Optional[dict[str, Any]]` to `dict[str, Any] | None` (PEP 604, consistent with rest of codebase). ### `runtime_dispatch.py` — same agent-type-aware fallback in `_execute_llm` (M1 cycle-9 fix) - Applied the same agent-type-aware fallback pattern to `_execute_llm` for consistency with `Node._execute_agent()`. - Renamed cryptic exception variable `_nu_err` → `_node_usage_err` in `_execute_graph`. - **Multi-actor placeholder normalisation**: Added `_normalize_node_id()` helper and `_GRAPH_PLACEHOLDER_RE` module-level regex. - Renamed loop variable `nu` → `node_usage` in `_execute_multi_actor` for readability. - Moved `import re` to the top-level import block. ### `pure_graph.py` — import organisation - Moved `from cleveractors.result import MAX_REASONABLE_TOKENS as _MAX_REASONABLE_TOKENS` to the top-level import block. ### `features/mocks/actor_result_helpers.py` — new mock helpers module (cycle-9 fix) - Extracted `_make_llm_actor_config` and `_make_mock_chat_model` from the step file into `features/mocks/actor_result_helpers.py` (renamed to `make_llm_actor_config` and `make_mock_chat_model`), per project convention that mock factory functions belong in `features/mocks/`. ### BDD tests updated (M1, M2, m4, m5, m6, cycle-9 fixes) - **Strengthened parallel isolation assertions** (cycle 9): The "ContextVar isolates token counts" scenario now uses exact-value assertions (`t1 == (100, 50)` and `t2 == (200, 80)`) instead of weak `!= (0, 0)` and `!= each other` checks. A buggy swapped-mapping implementation would now fail. - **Removed bare `except Exception` in wrong-arity real-path test** (cycle 9): The `artc_wrong_arity_real_path_test` branch no longer catches all exceptions and fabricates a fake `ActorResult`. If `executor.execute()` raises unexpectedly, the test now fails loudly. - **Deleted dead step definition** `step_artc_graph_executor_wrong_arity_tuple` (cycle 9): This ~44-line step was never referenced by any scenario in the feature file. - **New scenario: ContextVar parallel-branch isolation** (M2): A single `LLMAgent` instance is called concurrently via `asyncio.gather()` with two different mocked `usage_metadata` responses; each task asserts its `last_token_usage_var.get()` reflects its own call's counts. - **Updated scenario: Post-ainvoke exception preserves token counts** (m4, billing integrity fix): An `LLMAgent` with `memory_enabled=True` succeeds at `ainvoke()` but fails at `update_memory()`; asserts `_last_token_usage == (42, 17)` after the exception (previously incorrectly asserted `(0, 0)`). This verifies that billing data is not lost when memory persistence fails. - **New scenarios: large-value pass-through** (m5): `_safe_node_token_int(20_000_000)` and `_safe_token_int(20_000_000)` both return `20_000_000` unchanged. - **Rewritten wrong-arity scenario** (m6): The first wrong-arity scenario now exercises the real `Node._execute_agent()` code path via `AgentFactory.create_agent` mock. ### CHANGELOG.md — stale reference removed - Removed stale `_usage_log` reference from the "Mutable state hygiene" entry (this field never existed in this PR's scope). The entry now accurately describes the actual new mechanism: `_last_token_usage` and `last_token_usage_var` are reset at the start of every `process_message()` call; `PureLangGraph._node_usages` is reset at the start of every `execute()` call; post-`ainvoke()` exceptions preserve captured token counts for billing integrity. ## Architecture Note AC3 says "`process_message()` returns token counts alongside the response string." Changing the `Agent` abstract base class return type from `str` to `tuple[str, int, int]` would require updating all four concrete agent implementations plus every mock in the test suite. Instead, the `contextvars.ContextVar` side-channel satisfies the spirit of AC3 — token counts are available alongside the response, per-task and race-free — without breaking the existing Agent contract. ## Billing Integrity Design When `ainvoke()` succeeds but a subsequent step (e.g. `update_memory()`) raises an exception: - The LLM provider has **already charged** the user for the tokens consumed by `ainvoke()`. - The `except` handlers check sentinel variables (`_captured_prompt`, `_captured_completion`) set immediately after `ainvoke()` succeeds. - If the sentinels are set (non-None), the captured counts are **preserved** in `_last_token_usage` and `last_token_usage_var` so the router receives accurate billing data. - Only pre-`ainvoke()` failures (sentinels still `None`) reset counts to `(0, 0)`, because in those cases no tokens were consumed. ## Quality Gates | Gate | Result | |------|--------| | `nox -e lint` | ✅ All checks passed | | `nox -e typecheck` | ✅ 0 errors, 1 pre-existing warning | | `nox -e unit_tests` | ✅ 2298/2298 scenarios passed | | `nox -e integration_tests` | ✅ All tests passed | | `nox -e coverage_report` | ✅ 97.1% (threshold 96.5%) | ## ADR References - ADR-2027: ActorResult Type and Token Aggregation - ADR-2026: Per-request credential injection (state field preserved) ## Deferred Items - **`_safe_int()` / `_safe_token_int()` consolidation** (N1): Both helpers implement the same coercion logic. Moving to a shared module is a refactor deferred to a follow-up ticket. - **`NodeUsageTuple` location** (issue #13): Moving the type alias to `cleveractors.result` is a minor organizational improvement deferred to a follow-up. - **`Literal[...]` annotation duplicating `_CAUSE_*` constants**: Python's `Literal` type does not support referencing constant names directly, so the string values must be duplicated. This is a known limitation of the language and is accepted as-is. - **N2 (Two logger instances in nodes.py)**: `Node.__init__` sets `self.logger = logging.getLogger(__name__)` which produces the same object as the module-level `_logger`. Consolidating to use `_logger` directly in `Node` is a pre-existing nit deferred to a follow-up. - **N3 (Token-extraction fallback logic duplicated)**: Extracting a shared `_read_last_token_usage(agent)` helper is a refactor deferred to a follow-up. - **N4 (`_execute_llm`/`_execute_tool` compute totals directly)**: Inconsistency with `sum()` pattern used in `_execute_graph`/`_execute_multi_actor` is deferred to a follow-up.

hurui200320 added the

Type

Feature

label 2026-06-10 05:49:35 +00:00

hurui200320 added a new dependency 2026-06-10 05:49:41 +00:00

#14 feat(ActorResult): implement ActorResult and NodeUsage types; capture per-node token counts from LangChain responses

hurui200320 referenced this pull request

2026-06-10 05:50:08 +00:00

feat(ActorResult): implement ActorResult and NodeUsage types; capture per-node token counts from LangChain responses #14

hurui200320 force-pushed feature/actor-result from 25e6cce910 to 8a09b96412

2026-06-10 07:46:52 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 8a09b96412 to ad3186a07e

2026-06-10 08:50:20 +00:00

Compare

hurui200320 force-pushed feature/actor-result from ad3186a07e to e969851786

2026-06-10 08:57:21 +00:00

Compare

hurui200320 force-pushed feature/actor-result from e969851786 to 498f0f87ae

2026-06-10 09:47:53 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 498f0f87ae to 7f0e459717

2026-06-10 09:51:01 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 7f0e459717 to 6d1eca444d

2026-06-10 11:20:31 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 6d1eca444d to 40c102574d

2026-06-10 11:21:14 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 40c102574d to b21a41e16a

2026-06-10 11:25:14 +00:00

Compare

hurui200320 force-pushed feature/actor-result from b21a41e16a to 9491b69f10

2026-06-10 12:22:35 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 9491b69f10 to ea355c6433

2026-06-10 13:16:10 +00:00

Compare

hurui200320 force-pushed feature/actor-result from ea355c6433 to ab8b1451f0

2026-06-10 13:24:29 +00:00

Compare

hurui200320 force-pushed feature/actor-result from ab8b1451f0 to 7068d5d664

2026-06-10 14:40:16 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 7068d5d664 to 403d3e683a

2026-06-10 14:52:15 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 403d3e683a to 7d21833827

2026-06-10 15:07:29 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 7d21833827 to 0a120b2ec2

2026-06-10 15:15:21 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 0a120b2ec2 to e778443aad

2026-06-10 16:11:56 +00:00

Compare

hurui200320 force-pushed feature/actor-result from e778443aad to 16f8d4b945

2026-06-10 16:58:49 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 16f8d4b945 to 54c15c1778

2026-06-10 18:30:41 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 54c15c1778 to b488e10bdf

2026-06-10 18:38:34 +00:00

Compare

hurui200320 force-pushed feature/actor-result from b488e10bdf to 2657f8c311

2026-06-10 18:45:07 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 2657f8c311 to 1a6648d763

2026-06-10 19:23:25 +00:00

Compare

hurui200320 force-pushed feature/actor-result from 1a6648d763 to dd17cd6467

2026-06-10 19:35:15 +00:00

Compare

hurui200320 force-pushed feature/actor-result from dd17cd6467 to e772d04193

2026-06-10 20:17:52 +00:00

Compare

hurui200320 force-pushed feature/actor-result from e772d04193 to c9dbfa6517

2026-06-10 20:33:52 +00:00

Compare

hurui200320 force-pushed feature/actor-result from c9dbfa6517 to 2664ebfd75

2026-06-11 03:05:25 +00:00

Compare

hurui200320 commented

2026-06-11 03:25:12 +00:00

Review: Issue Compliance & Regression Check

Reviewed against two questions: (1) does the code implement what issue #14 requires, and (2) does it break anything?

1. Issue #14 acceptance criteria — all met ✅

AC	Status	Notes
AC1 — `result.py` with `NodeUsage` / `ActorResult`	✅	Fields match spec exactly; `state` preserved per ADR-2026; `__all__` declared
AC2 — Real token extraction (`usage_metadata` → `response_metadata` fallback → warn+0)	✅	Four distinct warning causes; truthy non-dict guards; `_safe_int()` handles booleans, negatives, overflow, and implausible values
AC3 — Token counts available alongside response	✅	Satisfied via `contextvars.ContextVar` + `_last_token_usage` side-channel; architecture note in PR is sound
AC4 — `PureLangGraph` collects per-node `(node_id, provider, model, prompt, completion)`	✅	`Node._execute_agent()` writes `_node_token_usage` into state; `_execute_from_node()` pops it and accumulates into `self._node_usages`
AC5 — `Executor.execute()` aggregates into `ActorResult` with real counts	✅	All four dispatch paths (`_execute_llm`, `_execute_graph`, `_execute_tool`, `_execute_multi_actor`) rebuilt; estimation helpers deleted
AC6 — Aggregation invariant `prompt_tokens == sum(n.prompt_tokens for n in nodes)`	✅	Enforced explicitly in `_execute_graph`; recomputed after node-id prefixing in `_execute_multi_actor`
AC7 — `ActorResult` / `NodeUsage` exported from `__init__.py` and `__all__`	✅	`__init__.py` imports directly from `cleveractors.result`; `result.py` has `__all__`

All subtasks from the issue are checked off, including deletion of runtime_tokens.py and _estimate_tokens().

2. Regressions — none found ✅

Intentional breaking changes (all call sites updated, tests confirm):

ActorResult.nodes is now a required field (no default). Correct per spec; the bot's field(default_factory=list) was an earlier deviation. Every creation site in runtime_dispatch.py provides nodes.
Executor._usage_log removed. All four internal references cleaned up.
runtime_tokens.py deleted. Its only caller (runtime_dispatch.py) is updated.

Return-type propagation — handled everywhere:
PureLangGraph.execute() now returns a 3-tuple. All four call sites updated: _execute_graph, PureLangGraph.process_message(), and both locations in application.py. process_message() also fixes a pre-existing bug where it was returning a tuple instead of a str.

Backward compatibility preserved:
from cleveractors.runtime import ActorResult, from cleveractors.runtime_types import ActorResult, and from cleveractors import ActorResult, NodeUsage all continue to work via shim re-exports.

Concurrency correctness:
The contextvars.ContextVar pattern is correct for asyncio — each Task inherits a copy of the context at creation time, making parallel branches that share the same LLMAgent instance race-free. The billing-integrity sentinels (_captured_prompt is not None) correctly distinguish pre-ainvoke() failures from post-ainvoke() failures. Token counts are read before cleanup() runs in the finally block.

Verdict: safe to merge ✅

Quality gates (lint, typecheck, 2298/2298 unit tests, integration tests, 97.1% coverage) all green. The deferred items (N1–N4) are code-quality nits with no correctness impact.

## Review: Issue Compliance & Regression Check Reviewed against two questions: (1) does the code implement what issue #14 requires, and (2) does it break anything? --- ### 1. Issue #14 acceptance criteria — all met ✅ | AC | Status | Notes | |---|---|---| | AC1 — `result.py` with `NodeUsage` / `ActorResult` | ✅ | Fields match spec exactly; `state` preserved per ADR-2026; `__all__` declared | | AC2 — Real token extraction (`usage_metadata` → `response_metadata` fallback → warn+0) | ✅ | Four distinct warning causes; truthy non-dict guards; `_safe_int()` handles booleans, negatives, overflow, and implausible values | | AC3 — Token counts available alongside response | ✅ | Satisfied via `contextvars.ContextVar` + `_last_token_usage` side-channel; architecture note in PR is sound | | AC4 — `PureLangGraph` collects per-node `(node_id, provider, model, prompt, completion)` | ✅ | `Node._execute_agent()` writes `_node_token_usage` into state; `_execute_from_node()` pops it and accumulates into `self._node_usages` | | AC5 — `Executor.execute()` aggregates into `ActorResult` with real counts | ✅ | All four dispatch paths (`_execute_llm`, `_execute_graph`, `_execute_tool`, `_execute_multi_actor`) rebuilt; estimation helpers deleted | | AC6 — Aggregation invariant `prompt_tokens == sum(n.prompt_tokens for n in nodes)` | ✅ | Enforced explicitly in `_execute_graph`; recomputed after node-id prefixing in `_execute_multi_actor` | | AC7 — `ActorResult` / `NodeUsage` exported from `__init__.py` and `__all__` | ✅ | `__init__.py` imports directly from `cleveractors.result`; `result.py` has `__all__` | All subtasks from the issue are checked off, including deletion of `runtime_tokens.py` and `_estimate_tokens()`. --- ### 2. Regressions — none found ✅ **Intentional breaking changes (all call sites updated, tests confirm):** - `ActorResult.nodes` is now a required field (no default). Correct per spec; the bot's `field(default_factory=list)` was an earlier deviation. Every creation site in `runtime_dispatch.py` provides `nodes`. - `Executor._usage_log` removed. All four internal references cleaned up. - `runtime_tokens.py` deleted. Its only caller (`runtime_dispatch.py`) is updated. **Return-type propagation — handled everywhere:** `PureLangGraph.execute()` now returns a 3-tuple. All four call sites updated: `_execute_graph`, `PureLangGraph.process_message()`, and both locations in `application.py`. `process_message()` also fixes a pre-existing bug where it was returning a tuple instead of a `str`. **Backward compatibility preserved:** `from cleveractors.runtime import ActorResult`, `from cleveractors.runtime_types import ActorResult`, and `from cleveractors import ActorResult, NodeUsage` all continue to work via shim re-exports. **Concurrency correctness:** The `contextvars.ContextVar` pattern is correct for asyncio — each `Task` inherits a copy of the context at creation time, making parallel branches that share the same `LLMAgent` instance race-free. The billing-integrity sentinels (`_captured_prompt is not None`) correctly distinguish pre-`ainvoke()` failures from post-`ainvoke()` failures. Token counts are read before `cleanup()` runs in the `finally` block. --- ### Verdict: safe to merge ✅ Quality gates (lint, typecheck, 2298/2298 unit tests, integration tests, 97.1% coverage) all green. The deferred items (N1–N4) are code-quality nits with no correctness impact.

hurui200320 merged commit 2664ebfd75 into master

2026-06-11 03:25:35 +00:00

hurui200320 deleted branch feature/actor-result

2026-06-11 03:25:35 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Blocks

#14 feat(ActorResult): implement ActorResult and NodeUsage types; capture per-node token counts from LangChain responses

cleveragents/cleveractors-core

Reference: cleveragents/cleveractors-core#41

feat(ActorResult): implement ActorResult and NodeUsage types; capture per-node token counts from LangChain responses #41

Summary

Motivation

Changes

New module: src/cleveractors/result.py (AC1)

LLMAgent — real token extraction + race-condition fix + billing integrity (AC2, AC3, M1, M2, m3)

Node._execute_agent() — race condition fix, agent-type-aware fallback (M1 cycle-9 fix)

runtime_dispatch.py — same agent-type-aware fallback in _execute_llm (M1 cycle-9 fix)

pure_graph.py — import organisation

features/mocks/actor_result_helpers.py — new mock helpers module (cycle-9 fix)

BDD tests updated (M1, M2, m4, m5, m6, cycle-9 fixes)

CHANGELOG.md — stale reference removed

Architecture Note

Billing Integrity Design

Quality Gates

ADR References

Deferred Items

Review: Issue Compliance & Regression Check

1. Issue #14 acceptance criteria — all met ✅

2. Regressions — none found ✅

Verdict: safe to merge ✅

New module: `src/cleveractors/result.py` (AC1)

`LLMAgent` — real token extraction + race-condition fix + billing integrity (AC2, AC3, M1, M2, m3)

`Node._execute_agent()` — race condition fix, agent-type-aware fallback (M1 cycle-9 fix)

`runtime_dispatch.py` — same agent-type-aware fallback in `_execute_llm` (M1 cycle-9 fix)

`pure_graph.py` — import organisation

`features/mocks/actor_result_helpers.py` — new mock helpers module (cycle-9 fix)