fix(core): robust per-message token counter detection in trim_messages#35652
fix(core): robust per-message token counter detection in trim_messages#35652Giulio Leone (giulio-leone) wants to merge 3 commits into
Conversation
Merging this PR will degrade performance by 27.3%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | test_import_time[CallbackManager] |
284.9 ms | 329 ms | -13.4% |
| ❌ | WallTime | test_import_time[tool] |
484.6 ms | 566.4 ms | -14.44% |
| ❌ | WallTime | test_import_time[Runnable] |
448.4 ms | 512.2 ms | -12.46% |
| ❌ | WallTime | test_import_time[HumanMessage] |
239.9 ms | 272.1 ms | -11.86% |
| ❌ | WallTime | test_import_time[InMemoryVectorStore] |
550.3 ms | 621.1 ms | -11.39% |
| ❌ | WallTime | test_import_time[PydanticOutputParser] |
475.2 ms | 556.6 ms | -14.64% |
| ❌ | WallTime | test_import_time[Document] |
170.8 ms | 189.9 ms | -10.04% |
| ❌ | WallTime | test_import_time[ChatPromptTemplate] |
563.1 ms | 648.5 ms | -13.16% |
| ❌ | WallTime | test_import_time[RunnableLambda] |
439.3 ms | 503.9 ms | -12.83% |
| ❌ | WallTime | test_import_time[LangChainTracer] |
409.5 ms | 469.8 ms | -12.83% |
| ❌ | WallTime | test_import_time[BaseChatModel] |
481.4 ms | 555.7 ms | -13.37% |
| ❌ | WallTime | test_async_callbacks_in_sync |
18.6 ms | 25.6 ms | -27.3% |
| ❌ | WallTime | test_import_time[InMemoryRateLimiter] |
156.3 ms | 178.7 ms | -12.54% |
Comparing giulio-leone:fix/trim-messages-per-message-counter-detection (6ec1fce) with master (fbfe4b8)
Footnotes
-
23 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
The `trim_messages` function uses `inspect.signature` to detect whether a `token_counter` callable operates per-message or per-list, but the check `annotation is BaseMessage` (identity comparison) fails for: - Subclass annotations (e.g., `msg: HumanMessage`) - String/postponed annotations from `from __future__ import annotations` Replace the strict identity check with `_is_per_message_token_counter()` which: 1. Uses `typing.get_type_hints()` to resolve string/postponed annotations 2. Falls back to `inspect.signature` for raw annotations 3. Checks collection types (list, Sequence, etc.) → per-list 4. Checks BaseMessage subclasses via `issubclass()` → per-message 5. Preserves backward compatibility: unannotated callables → per-list Fixes #35629 Signed-off-by: Giulio Leone <6887247+giulio-leone@users.noreply.github.com>
BLE001 is not enabled in the project's ruff config. Signed-off-by: Giulio Leone <6887247+giulio-leone@users.noreply.github.com>
|
Hi! Creator of the issue here. Thanks for working on this. However, I already have a PR open to fix it from when I originally opened the issue. And please stop spamming on all my git issues lol :) |
The `# type: ignore[type-arg]` on `_is_per_message_token_counter` is flagged as unused by mypy's `unused-ignore` rule, causing CI lint to fail across all Python versions. Signed-off-by: Giulio Leone <6887247+giulio-leone@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ccurme (ccurme)
left a comment
There was a problem hiding this comment.
Duplicated with #35630.
Summary
Fixes #35629 —
trim_messagesmisdetects per-messagetoken_countercallables when using subclass annotations (msg: HumanMessage), string annotations (msg: "BaseMessage"), or postponed annotations (from __future__ import annotations).Root Cause
The detection logic at utils.py#L1420-L1424 uses a strict identity check:
This fails for:
msg: HumanMessageHumanMessage is not BaseMessagemsg: "BaseMessage""BaseMessage" is not BaseMessagefrom __future__ import annotations+msg: BaseMessageIn all failing cases, the callable is misclassified as a per-list counter and called with
list[BaseMessage], causingAttributeError: 'list' object has no attribute 'content'.Fix
Replace the identity check with a new
_is_per_message_token_counter()helper that:typing.get_type_hints()(handles string/postponed annotations)inspect.signaturefor raw annotations whenget_type_hints()failslist[...],Sequence[...], etc.)issubclass(annotation, BaseMessage)(handles all subclasses)def) default to per-list, matching existing behaviorWhat's Fixed vs What's Not
msg: BaseMessagemsg: HumanMessageAttributeErrormsg: "BaseMessage"(string)AttributeErrorfrom __future__ import annotationsAttributeErrorlambda msg: ...(no annotation)def f(msg): ...(no annotation)msgs: list[BaseMessage]msgs: Sequence[BaseMessage]Unannotated callables are intentionally kept as per-list to avoid breaking existing code that relies on the current default.
Tests
All 23 tests pass (21 existing + 2 new):
test_trim_messages_per_message_counter_with_subclass_annotation—msg: HumanMessagetest_trim_messages_per_message_counter_with_postponed_annotations—msg: BaseMessagewith resolved hintsChanges
libs/core/langchain_core/messages/utils.py_is_per_message_token_counter()helper + replaced identity checklibs/core/tests/unit_tests/messages/test_utils.py