Skip to content

Conversation

gjeltep
Copy link

@gjeltep gjeltep commented Oct 20, 2025

Description

Fixed BaseCallbackManager.merge() method to correctly preserve the distinction between handlers and inheritable_handlers during merge operations.

Previously, the merge method was using add_handler() which incorrectly added handlers to both lists when inherit=True, causing cross-contamination between regular and inheritable handlers.

The fix directly passes the combined handler lists to the constructor instead of using add_handler(), ensuring proper separation is maintained.

Issue

Fixes #32028

Dependencies

None

Testing

  • Modified existing test test_merge_preserves_handler_distinction() to verify handlers remain properly separated after merge

Checklist

  • Breaking Changes: No breaking changes - only fixes incorrect behavior
  • Type Hints: All functions have complete type annotations
  • Tests: Fix is fully tested with existing unit test
  • Security: No security implications
  • Documentation: No documentation changes needed - bug fix only
  • Code Quality: Passes lint and format checks
  • Commit Message: Follows Conventional Commits format

@gjeltep gjeltep requested a review from eyurtsev as a code owner October 20, 2025 23:15
@github-actions github-actions bot added fix core Related to the package `langchain-core` and removed fix labels Oct 20, 2025
Copy link

codspeed-hq bot commented Oct 20, 2025

CodSpeed Performance Report

Merging #33617 will improve performances by 11.69%

Comparing gjeltep:fix/callback-manager-merge-bug (2b92cbf) with master (e731ba1)

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

⚡ 1 improvement
✅ 12 untouched
⏩ 21 skipped1

Benchmarks breakdown

Mode Benchmark BASE HEAD Change
WallTime test_import_time[InMemoryRateLimiter] 198.4 ms 177.7 ms +11.69%

Footnotes

  1. 21 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Related to the package `langchain-core`

Projects

None yet

Development

Successfully merging this pull request may close these issues.

core: BaseCallbackManager.merge() mixes handlers and inheritable_handlers in a confusing way

1 participant