Skip to content

fix(core): async tracer on_chat_model_start fallback in sync context#7

Open
DhirenMhatre wants to merge 1 commit into
masterfrom
mdrxy/async-tracer
Open

fix(core): async tracer on_chat_model_start fallback in sync context#7
DhirenMhatre wants to merge 1 commit into
masterfrom
mdrxy/async-tracer

Conversation

@DhirenMhatre

@DhirenMhatre DhirenMhatre commented Jun 6, 2026

Copy link
Copy Markdown

User description

Fixes #


Read the full contributing guidelines: https://docs.langchain.com/oss/python/contributing/overview

All contributions must be in English. See the language policy.

If you paste a large clearly AI generated description here your PR may be IGNORED or CLOSED!

Thank you for contributing to LangChain! Follow these steps to have your pull request considered as ready for review.

  1. PR title: Should follow the format: TYPE(SCOPE): DESCRIPTION
  1. PR description:
  • Write 1-2 sentences summarizing the change.
  • The Fixes #xx line at the top is required for external contributions — update the issue number and keep the keyword. This links your PR to the approved issue and auto-closes it on merge.
  • If there are any breaking changes, please clearly describe them.
  • If this PR depends on another PR being merged first, please include "Depends on #PR_NUMBER" in the description.
  1. Run make format, make lint and make test from the root of the package(s) you've modified.
  • We will not consider a PR unless these three are passing in CI.
  1. How did you verify your code works?

Additional guidelines:

  • All external PRs must link to an issue or discussion where a solution has been approved by a maintainer, and you must be assigned to that issue. PRs without prior approval will be closed.
  • PRs should not touch more than one package unless absolutely necessary.
  • Do not update the uv.lock files or add dependencies to pyproject.toml files (even optional ones) unless you have explicit permission to do so by a maintainer.

Social handles (optional)

Twitter: @
LinkedIn: https://linkedin.com/in/


CodeAnt-AI Description

Async chat model callbacks now fall back to LLM start handling in sync code

What Changed

  • When an async tracer does not support chat model start handling and is called from synchronous code, it now falls back to the LLM start path instead of dropping the trace
  • Tracers that do support chat model start handling still use that path normally
  • Added coverage for the fallback path and confirmed it stays quiet without warning logs

Impact

✅ Fewer missing chat traces
✅ More reliable async tracing in sync code
✅ Clearer callback behavior for unsupported chat events

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@codeant-ai

codeant-ai Bot commented Jun 6, 2026

Copy link
Copy Markdown

CodeAnt AI is reviewing your PR.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@codity-chait

codity-chait Bot commented Jun 6, 2026

Copy link
Copy Markdown

PR Summary

What Changed

  • Fixed a bug where async tracers without on_chat_model_start would lose traces when called from synchronous contexts.
  • Added a fallback wrapper that catches NotImplementedError and routes to on_llm_start instead.
  • Added tests to verify the fallback behavior works correctly and silently.

Key Changes by Area

Tracing/Callbacks: Added _achat_model_start_fallback wrapper in manager.py to handle async tracers missing on_chat_model_start.

Testing: Added three test cases in test_handle_event.py covering fallback firing, no false positives, and silent operation.

Files Changed

File Changes Summary
libs/core/langchain_core/callbacks/manager.py Added _achat_model_start_fallback wrapper (lines 256-283) and applied it in handle_event (lines 311-314)
libs/core/tests/unit_tests/callbacks/test_handle_event.py Added three test cases for async tracer fallback behavior

Review Focus Areas

  • Verify the fallback wrapper correctly preserves trace context when routing to on_llm_start.
  • Check that the NotImplementedError catch is narrow enough to avoid masking real errors.
  • Confirm test coverage handles edge cases like multiple async handlers with mixed implementations.

Architecture

Design Decisions: The fallback is intentionally silent (no logs) to avoid noise during normal operation. This trades observability for user experience, assuming missing on_chat_model_start is a common and acceptable pattern for simple tracers.

Risks: The broad except NotImplementedError could mask bugs if handlers raise this error unexpectedly. This is acceptable for now given the callback contract, but future handler refactors should consider more explicit opt-in/opt-out patterns.

Merge Status

MERGEABLE — PR Score 75/100, above threshold (50). All gates passed.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Jun 6, 2026
@codity-chait

codity-chait Bot commented Jun 6, 2026

Copy link
Copy Markdown

Workflow Diagrams

Automatically generated sequence diagrams showing the workflows in this PR

1. Async Callback Handler Fallback Flow

_Medium complexity • Components: handle_event, achat_model_start_fallback, AsyncBaseTracer

sequenceDiagram
    title Async Chat Model Start Fallback in Sync Context

    participant Caller as Sync Caller
    participant HE as handle_event
    participant ACH as AsyncCallbackHandler
    participant ABF as _achat_model_start_fallback
    participant AEH as _ahandle_event_for_handler
    participant RC as _run_coros

    Note over Caller,RC: PR: Fix async tracer fallback when on_chat_model_start not implemented

    Caller->>HE: handle_event(handlers, "on_chat_model_start", ...)
    
    loop For each handler
        HE->>ACH: getattr(handler, "on_chat_model_start")(*args, **kwargs)
        
        alt Handler returns coroutine (async)
            HE->>HE: asyncio.iscoroutine(event) == True
            
            alt event_name is "on_chat_model_start"
                HE->>ABF: _achat_model_start_fallback(coro, handler, *args, **kwargs)
                ABF->>ACH: await coro (original on_chat_model_start)
                
                alt NotImplementedError raised
                    ABF->>ABF: Catch NotImplementedError
                    ABF->>ABF: Convert messages to prompts via get_buffer_string
                    ABF->>AEH: _ahandle_event_for_handler(handler, "on_llm_start", ...)
                    AEH->>ACH: handler.on_llm_start(serialized, prompts, ...)
                    ACH-->>AEH: Return/await
                    AEH-->>ABF: Complete
                    ABF-->>HE: Complete (fallback successful)
                else No error
                    ACH-->>ABF: Return normally
                    ABF-->>HE: Complete (no fallback needed)
                end
                
                HE->>HE: coros.append(wrapped_coro)
            else Other event
                HE->>HE: coros.append(event)
            end
            
        else Handler raises NotImplementedError (sync path)
            HE->>HE: Catch NotImplementedError
            alt event_name is "on_chat_model_start"
                HE->>HE: Convert messages to prompts
                HE->>HE: Call handler.on_llm_start(...) directly
            end
        end
    end
    
    HE->>RC: _run_coros(coros) (execute all collected coroutines)
    RC-->>HE: All coroutines complete
    HE-->>Caller: Return
Loading

Note: Diagrams show detected patterns only. Complex workflows may require manual review.

@codeant-ai

codeant-ai Bot commented Jun 6, 2026

Copy link
Copy Markdown

CodeAnt AI finished reviewing your PR.

SERIALIZED = {"id": ["chat_model"]}


class _NoOpAsyncTracer(AsyncBaseTracer):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional Medium

The _NoOpAsyncTracer class inherits from AsyncBaseTracer, which provides a fully working on_chat_model_start implementation. This prevents the intended fallback mechanism from being tested, as the fallback to on_llm_start only triggers when on_chat_model_start raises NotImplementedError. To properly exercise the fallback code path, _NoOpAsyncTracer should inherit from AsyncCallbackHandler instead, which raises NotImplementedError for on_chat_model_start.

Also reported at: libs/core/tests/unit_tests/callbacks/test_handle_event.py L136–L155

Suggested change
class _NoOpAsyncTracer(AsyncBaseTracer):
class _NoOpAsyncTracer(AsyncCallbackHandler):
Prompt for AI assistance

Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:

You are an expert bash developer with deep knowledge of security, performance, and best practices.

### Context

File: libs/core/tests/unit_tests/callbacks/test_handle_event.py
Lines: 20-20
Issue Type: functional-medium
Severity: medium

Issue Description:
The `_NoOpAsyncTracer` class inherits from `AsyncBaseTracer`, which provides a fully working `on_chat_model_start` implementation. This prevents the intended fallback mechanism from being tested, as the fallback to `on_llm_start` only triggers when `on_chat_model_start` raises `NotImplementedError`. To properly exercise the fallback code path, `_NoOpAsyncTracer` should inherit from `AsyncCallbackHandler` instead, which raises `NotImplementedError` for `on_chat_model_start`.

_Also reported at: `libs/core/tests/unit_tests/callbacks/test_handle_event.py` L136–L155_

Current Code:
class _NoOpAsyncTracer(AsyncBaseTracer):

---

### Instructions

1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed

### Constraints

- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready

---


Like Dislike Create Issue

@codity-chait

codity-chait Bot commented Jun 6, 2026

Copy link
Copy Markdown

Security Scan Summary

Metric Value
Vulnerabilities Critical: 0
Overall Risk Clean
Files Scanned 2

No critical security issues detected

Scan completed in 21.7s

Security scan powered by Codity.ai

@codity-chait

codity-chait Bot commented Jun 6, 2026

Copy link
Copy Markdown

License Compliance Scan

Metric Value
Packages Scanned 0
High Risk (Strong Copyleft) 0
Medium Risk (Weak Copyleft) 0
Low Risk (Permissive) 0
Unknown License 0

All licenses are low-risk and compliant

Powered by Codity.ai · Docs

@codity-chait

codity-chait Bot commented Jun 6, 2026

Copy link
Copy Markdown

Code Quality Report — test-org-codity/langchain · PR #7

Scanned: 2026-06-06 19:01 UTC | Score: 61/100 | Provider: github

Executive Summary

Severity Count
Critical 0
High 1
Medium 2
Low 8
Top Findings

[CQ-LLM-004] libs/core/tests/unit_tests/callbacks/test_handle_event.py:1 (Testability · HIGH)

Issue: The test file lacks a clear structure and organization, making it harder to navigate and understand.
Suggestion: Organize tests into classes or modules and provide clear section headers.

"""Tests for the `on_chat_model_start` fallback in `handle_event`."""

[CQ-LLM-001] libs/core/langchain_core/callbacks/manager.py:254 (Complexity · MEDIUM)

Issue: The function _achat_model_start_fallback has a cyclomatic complexity that may increase with additional exception handling.
Suggestion: Consider simplifying the function or breaking it into smaller functions to reduce complexity.

async def _achat_model_start_fallback(
    coro: Coroutine[Any, Any, Any],
    handler: BaseCallbackHandler,
    *args: Any,
    **kwargs: Any,
) -> None:
    try:
        await coro
    except NotImplementedError:
        message_strings = [get_buffer_string(m) for m in args[1]]
        await _ahandle_event_for_handler(
            handler,
            "on_llm_start",
            "ignore_llm",
            args[0],
            message_strings,
            *args[2:],
            **kwargs,
        )

[CQ-LLM-003] libs/core/langchain_core/callbacks/manager.py:258 (Error_Handling · MEDIUM)

Issue: The exception handling in _achat_model_start_fallback only catches NotImplementedError, potentially missing other exceptions.
Suggestion: Consider handling other exceptions or logging them for better error tracking.

except NotImplementedError:

[CQ-LLM-002] libs/core/langchain_core/callbacks/manager.py:254 (Documentation · LOW)

Issue: The function _achat_model_start_fallback lacks detailed docstring for parameters and return type.
Suggestion: Add detailed docstrings for parameters and return type to improve documentation.

async def _achat_model_start_fallback(
    coro: Coroutine[Any, Any, Any],
    handler: BaseCallbackHandler,
    *args: Any,
    **kwargs: Any,
) -> None:

[CQ-002] libs/core/langchain_core/callbacks/manager.py:314 (Complexity · LOW)

Issue: Deep nesting detected (depth ~6)
Suggestion: Extract nested blocks into helper functions

if event_name == "on_chat_model_start":

[CQ-002] libs/core/langchain_core/callbacks/manager.py:315 (Complexity · LOW)

Issue: Deep nesting detected (depth ~7)
Suggestion: Extract nested blocks into helper functions

event = _achat_model_start_fallback(

[CQ-002] libs/core/langchain_core/callbacks/manager.py:316 (Complexity · LOW)

Issue: Deep nesting detected (depth ~8)
Suggestion: Extract nested blocks into helper functions

event, handler, *args, **kwargs

[CQ-002] libs/core/langchain_core/callbacks/manager.py:317 (Complexity · LOW)

Issue: Deep nesting detected (depth ~7)
Suggestion: Extract nested blocks into helper functions

)

[CQ-007] libs/core/tests/unit_tests/callbacks/test_handle_event.py:34 (Documentation · LOW)

Issue: Public async def 'on_llm_start' missing docstring
Suggestion: Add a docstring describing purpose and parameters

async def on_llm_start(

[CQ-007] libs/core/tests/unit_tests/callbacks/test_handle_event.py:65 (Documentation · LOW)

Issue: Public async def 'on_chat_model_start' missing docstring
Suggestion: Add a docstring describing purpose and parameters

async def on_chat_model_start(

Per-File Breakdown

File Critical High Medium Low Total
libs/core/langchain_core/callbacks/manager.py 0 0 2 5 7
libs/core/tests/unit_tests/callbacks/test_handle_event.py 0 1 0 3 4

Recommendations

  1. Resolve High severity issues, especially error handling gaps and performance bottlenecks.
  • Run automated tests after applying fixes to verify no regressions.

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

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants