fix: trim_messages breaks when token_counter is a per-message c...#35669
fix: trim_messages breaks when token_counter is a per-message c...#35669Zakir Jiwani (JiwaniZakir) wants to merge 1 commit into
trim_messages breaks when token_counter is a per-message c...#35669Conversation
…unter (langchain-ai#35629) The annotation identity check (`is BaseMessage`) failed for subclass annotations, string annotations, and postponed annotations. Replace it with `_is_per_message_token_counter` that uses `get_type_hints` for proper resolution and `issubclass` for subclass support. Add `token_counter_is_per_message` flag for unannotated callables and lambdas. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Merging this PR will degrade performance by 28.81%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | test_import_time[RunnableLambda] |
439.3 ms | 528.7 ms | -16.91% |
| ❌ | WallTime | test_import_time[PydanticOutputParser] |
475.2 ms | 581.9 ms | -18.34% |
| ❌ | WallTime | test_async_callbacks_in_sync |
18.6 ms | 26.1 ms | -28.81% |
| ❌ | WallTime | test_import_time[InMemoryVectorStore] |
550.3 ms | 651.7 ms | -15.55% |
| ❌ | WallTime | test_import_time[CallbackManager] |
284.9 ms | 337.5 ms | -15.6% |
| ❌ | WallTime | test_import_time[HumanMessage] |
239.9 ms | 280.3 ms | -14.42% |
| ❌ | WallTime | test_import_time[Runnable] |
448.4 ms | 524.4 ms | -14.49% |
| ❌ | WallTime | test_import_time[tool] |
484.6 ms | 589.1 ms | -17.75% |
| ❌ | WallTime | test_import_time[ChatPromptTemplate] |
563.1 ms | 680.2 ms | -17.21% |
| ❌ | WallTime | test_import_time[LangChainTracer] |
409.5 ms | 490.8 ms | -16.55% |
| ❌ | WallTime | test_import_time[BaseChatModel] |
481.4 ms | 567 ms | -15.1% |
| ❌ | WallTime | test_import_time[InMemoryRateLimiter] |
156.3 ms | 183.6 ms | -14.85% |
| ❌ | WallTime | test_import_time[Document] |
170.8 ms | 198.5 ms | -13.95% |
Comparing JiwaniZakir:fix/issue-35629 (ee3d55b) 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. ↩
|
Hi Zakir Jiwani (@JiwaniZakir) ! This is the exact same fix as my original PR #35630. Please stop spamming AI-generated duplicate fixes. And read the thread first on issue please |
ccurme (ccurme)
left a comment
There was a problem hiding this comment.
Duplicated with #35630
Summary
This addresses #35629.
What changed
Implemented a fix based on the issue description. See the diff for specifics.
Testing