Skip to content

fix(langchain): clean up streaming run memo on error - #1978

Open
dhionson2 wants to merge 1 commit into
langfuse:mainfrom
dhionson2:fix/langchain-streaming-error-memo-1977
Open

dhionson2 wants to merge 1 commit into
langfuse:mainfrom
dhionson2:fix/langchain-streaming-error-memo-1977

Conversation

@dhionson2

@dhionson2 dhionson2 commented Oct 11, 2026 •

Copy link
Copy Markdown

Summary

Fixes #1977.

This PR fixes stale run ID retention in LangchainCallbackHandler when a streamed LLM execution terminates with an error.

Root cause

on_llm_new_token() adds the run ID to _updated_completion_start_time_memo after recording the first-token timestamp.

While on_llm_end() removes the run ID in its finally block, on_llm_error() did not perform the same cleanup.

This could cause incremental UUID retention when long-lived callback handlers are reused across failed streaming executions.

Changes

  • Add per-run memo cleanup to the finally block of on_llm_error().
  • Add a regression test covering a failed streamed run and another active run.
  • Verify that cleanup removes only the failed run ID.
  • Ensure the remaining active run is properly finalized during test teardown.

The change preserves existing public SDK behavior and LangGraph control-flow handling.

Verification

Regression test

  • RED: confirmed the failed run ID remained in the memo.
  • GREEN: confirmed the ID is removed after the fix.

Targeted validation

  • tests/unit/test_langchain.py: 24 passed.
  • Ruff lint and formatting: passed.
  • git diff --check: passed.

Full unit suite on Windows

  • 883 passed
  • 2 skipped
  • 3 failed outside the modified test module: two subprocess import failures involving pydantic and one Windows/POSIX path serialization mismatch.

The full suite has not yet been confirmed green on Linux. CI validation is pending.

Scope

This is a minimal, backward-compatible fix limited to the LangChain callback implementation and its regression test.

No dependency, generated API, or public interface changes.

RetriggerConfidence Score: 5/5

This PR appears safe to merge; no actionable issues were found.

Summary

This PR removes a streamed run’s first-token memo entry when on_llm_error finishes.

  • Errored LLM streams clear their own first-token memo.

Reviews (1) · Last reviewed commit: "fix(langchain): clean up streaming run m..." · Reviewed by Greptile

@claude claude 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@CLAassistant

CLAassistant commented Oct 11, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-11T01:55:45.069099Z 11ac8a0 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chrikrah chrikrah 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.

@dhionson2 approving at 11ac8a0. This fixes #1977, and the new test fails when I put back the old on_llm_error.

$ python -m pytest tests/unit/test_langchain.py -q -p no:cacheprovider    # Python 3.14, uv sync --frozen
# head 11ac8a0
24 passed
# head tests, CallbackHandler.py from base bf11ec1
1 failed, 23 passed
FAILED tests/unit/test_langchain.py::test_streamed_llm_error_cleans_only_its_completion_start_time_memo

@hassiebp could you review this one?

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(langchain): failed streaming LLM runs retain IDs in callback memo

3 participants