fix(core): improve typing/docs for on_chat_model_start to clarify required positional args#35324
Merged
ccurme (ccurme) merged 3 commits intolangchain-ai:masterfrom Feb 22, 2026
Conversation
…t fallback lacks args When a callback handler raises NotImplementedError for on_chat_model_start, the fallback to on_llm_start accesses args[1] without checking if sufficient args were provided, causing an IndexError that masks the original error. Add a len(args) guard before the fallback in both sync handle_event() and async _ahandle_event_for_handler(). When args are insufficient, log a warning instead of crashing. Closes langchain-ai#31576 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ired positional args Per maintainer feedback from @ccurme on issue langchain-ai#31576: 'The on_chat_model_start has 2 required arguments, this should just be typed / documented better.' Changes: - Revert runtime len(args) guard from handle_event() and _ahandle_event_for_handler() — unnecessary because the callback manager always calls on_chat_model_start with both required positional args (serialized, messages) - Add a clear note docblock to both sync and async on_chat_model_start in base.py, warning implementors to declare serialized and messages as named positional params (not *args) to avoid IndexError in the on_llm_start fallback path - Update test_handle_event.py to cover only valid usage (correct signature + NotImplementedError fallback); remove invalid-usage tests that only exercised the reverted guard Closes langchain-ai#31576
Balaji Seshadri (gitbalaji)
requested a review
from Eugene Yurtsev (eyurtsev)
as a code owner
github-actions
Bot
added
core
ccurme (ccurme)
approved these changes
Feb 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rework of #35323 based on maintainer feedback from ccurme (@ccurme).
Instead of a runtime
len(args) >= 2guard, this PR improves the documentation onon_chat_model_startto make the required signature contract explicit.What changed
_MIN_ARGS_FOR_CHAT_MODEL_FALLBACKconstant andlen(args)checks) fromhandle_event()and_ahandle_event_for_handler()inmanager.pyon_chat_model_startin bothCallbackManagerMixin(sync) andAsyncCallbackHandler(async) inbase.py, with an explicitcd /Users/balajiseshadri/workspace/langchain && git push origin fix/core-handle-event-index-error --force-with-lease! noteblock:serializedandmessagesare required positional arguments*argsin an override suppresses these names, causing anIndexErrorin the fallback path toon_llm_starttest_handle_event.py— removed invalid-usage tests (no-args, one-arg) that only tested the reverted guard; kept tests for the valid fallback path (correct signature +NotImplementedError)Why
The 2 required positional args (
serialized,messages) are always provided by the internal call sites inmanager.py. TheIndexErroronly occurs when user-written handlers wrongly use*argsin their override, violating the signature contract. The right fix is to make this contract clear in the documentation.Closes #31576