fix(langchain): incorrect role on generations #3465

ianchi · 2025-11-25T03:55:35Z

This fixes an error introduced by #3138 that records the wrong role it uses the generation type instead of the message type.
The generation type is only for serialization and is NOT "ai"/"human".

Important

Fixes incorrect role assignment in set_chat_response by using message type instead of generation type in span_utils.py.

Bug Fix:
- Corrects role assignment in set_chat_response in span_utils.py by using generation.message.type instead of generation.type.
- Ensures role is accurately set to "assistant" when generation.message is not present.

^{This description was created by}^{for 2a6c479. You can customize this summary. It will automatically update as commits are pushed.}

Summary by CodeRabbit

Bug Fixes
- Chat-response span role assignment now derives from the message type when present, falls back to "assistant" for non-message generations, and no longer overwrites role when tool calls exist — improving role accuracy and attribution.
Tests
- Added tests verifying role assignment, default assistant fallback, content storage, and recording of tool calls (including function_call and parsed tool-call arguments) with trace-content enabled.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-11-25T03:55:45Z

Walkthrough

Role assignment in set_chat_response now uses generation.message.type via _message_type_to_role when a message exists; if no message, the role falls back to "assistant". Tool-call processing remains and runs after role determination.

Changes

Cohort / File(s)	Change Summary
Role assignment refactor `packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py`	Removed unconditional `prefix.role` set from `generation.type`. If `generation.message` exists, `prefix.role` is set using `_message_type_to_role(generation.message.type)`; otherwise `prefix.role` is set to `"assistant"`. Tool-call handling (`_set_chat_tool_calls`) is unchanged and runs after role assignment.
Tests for span behavior `packages/opentelemetry-instrumentation-langchain/tests/test_span_utils.py`	Added tests verifying role mapping from `AIMessage` types, defaulting to `"assistant"` for non-message generations, and proper recording of `function_call` in `additional_kwargs` and of `generation.tool_calls` (including JSON args parsing and id/name extraction). Includes fixture enabling trace content and helpers to build spans/results.

Sequence Diagram(s)

sequenceDiagram
  participant Caller as Caller
  participant set_chat_response as set_chat_response
  participant RoleMap as _message_type_to_role
  participant ToolHandler as _set_chat_tool_calls

  Caller->>set_chat_response: call with Generation (may include message, additional_kwargs, tool_calls)
  alt generation.message exists
    set_chat_response->>RoleMap: _message_type_to_role(generation.message.type)
    RoleMap-->>set_chat_response: role
    set_chat_response->>set_chat_response: set prefix.role = role
  else no generation.message
    set_chat_response-->>set_chat_response: set prefix.role = "assistant"
  end
  alt function_call in additional_kwargs or generation.tool_calls present
    set_chat_response->>ToolHandler: _set_chat_tool_calls(...)
    ToolHandler-->>set_chat_response: attach tool_calls attributes
  end
  set_chat_response-->>Caller: return (span attributes updated)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Check _message_type_to_role mapping covers expected message types.
Verify role is set before _set_chat_tool_calls and no regressions in tool-call attribute formatting.
Review new tests for coverage of function_call vs tool_calls cases.

Poem

🐰 I nibble bytes and set the role,

I hop through messages, heart and soul.
Tool-calls lined up, tidy and clear,
The span records what I hold dear.
A tiny hop — the tests cheer near.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: fixing incorrect role assignment on LangChain generations by using message.type instead of generation.type.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b4207ed and dcddd62.

📒 Files selected for processing (2)

packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py (2 hunks)
packages/opentelemetry-instrumentation-langchain/tests/test_span_utils.py (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py
packages/opentelemetry-instrumentation-langchain/tests/test_span_utils.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: Lint
GitHub Check: Test Packages (3.10)
GitHub Check: Test Packages (3.12)
GitHub Check: Build Packages (3.11)
GitHub Check: Test Packages (3.11)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

ellipsis-dev

Important

Looks good to me! 👍

Reviewed everything up to 2a6c479 in 1 minute and 13 seconds. Click for details.

Reviewed 41 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 2 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py:211

Draft comment:
Good removal of the incorrect use of generation.type. The role should be derived from generation.message.type, which avoids misclassification.
Reason this comment was not posted:
Comment looked like it was already resolved.

2. packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py:263

Draft comment:
The new block sets the role attribute using generation.message.type after handling tool calls, with a fallback to 'assistant'. This looks correct and aligns with the intended design.
Reason this comment was not posted:
Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states that the code aligns with the intended design, which is not necessary for the PR author to know.

Workflow ID: wflow_vKaihN1GMn6Uk4xL

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

nirga

Thanks @ianchi - can you add a test for this?

...pentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py

ianchi · 2025-11-25T22:03:12Z

Hi @nirga I added a test and aligned the "completions" caso to OpenAI, (no role at all).

nirga · 2025-11-27T10:56:20Z

@ianchi it should always be with a role. Can you walk me through what are you looking to fix?

ianchi · 2025-11-28T01:39:44Z

I added a detailed explanation of the error, with printscreen and code snippets, in this comment.
Basically, right now any llm response "without" a tool_calls is logged in a span with .role = unknown instead of "assistant".
This is so because current code tries to extract the "role" from the wrong object. (it uses generation.type instead of generation.message.type).

Please let me know if any additional info is required.

Regarding this:

it should always be with a role.

As far as I undertand the openai implementation, in the case of a plain "completion" (not a chat completion) does NOT add a "role" to the span.
Look at opentelemetry.instrumentation.openai.shared.completion_wrappers the contend is added in _handle_response -> _set_completions which only adds attributes gen_ai.completion.{i}.finish_reason and .content it nevers adds a .role attribute.

In the case of "chat completions" the _set_completions does add a .role, extracting it from the message.

From your previous comment:

yes but look at the OpenAI instrumentation - we differentiate between tool call responses and textual ai responses

I understood that you wanted to mirror this behaviour in the Langchain instrumentation also. And thus I also modified the "textual response" case.
But if you prefer it to return role="assistant", I can add it.

ianchi · 2025-12-02T23:54:01Z

Hi @nirga, any comments on this?
Let me know if anything else is needed

ianchi · 2025-12-04T13:33:19Z

@nirga Friendly ping.

We are still getting all responses as role=unkown.
We need a quick fix for this.

Thanks

packages/opentelemetry-instrumentation-langchain/tests/test_span_utils.py

ianchi · 2025-12-12T18:19:47Z

Hi @nirga
as requested I added back the "role=assistant" for plain complations.

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8581e9e and b4207ed.

📒 Files selected for processing (2)

packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py (2 hunks)
packages/opentelemetry-instrumentation-langchain/tests/test_span_utils.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules

Files:

packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py
packages/opentelemetry-instrumentation-langchain/tests/test_span_utils.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: Build Packages (3.11)
GitHub Check: Test Packages (3.12)
GitHub Check: Test Packages (3.10)
GitHub Check: Lint
GitHub Check: Test Packages (3.11)

🔇 Additional comments (2)

packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py (2)

242-246: Correct fix for the role assignment bug.

This properly resolves the issue where generation.type (a serialization field containing "Generation" or "ChatGeneration") was being used instead of generation.message.type (the actual message type like "ai", "human", "system", "tool"). The _message_type_to_role function correctly maps these types to their corresponding roles.

272-275: Reasonable default for legacy non-chat completions.

The fallback to "assistant" for generations without a message object handles legacy non-chat calls appropriately. While this differs from OpenAI instrumentation behavior (which omits role for plain completions), it provides a sensible default for LLM responses and ensures consistent telemetry. Based on the PR discussion, this was a deliberate design decision.

packages/opentelemetry-instrumentation-langchain/tests/test_span_utils.py

ellipsis-dev bot reviewed Nov 25, 2025

View reviewed changes

ianchi force-pushed the langchain branch from 2a6c479 to 5eb7599 Compare November 25, 2025 04:12

nirga reviewed Nov 25, 2025

View reviewed changes

...pentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py Show resolved Hide resolved

ianchi force-pushed the langchain branch 2 times, most recently from b60fc7b to 8581e9e Compare November 25, 2025 22:02

ianchi requested a review from nirga November 25, 2025 23:53

nirga reviewed Dec 9, 2025

View reviewed changes

packages/opentelemetry-instrumentation-langchain/tests/test_span_utils.py Outdated Show resolved Hide resolved

fix(langchain): incorrect role on generations

6f9755a

ianchi force-pushed the langchain branch from 6551d57 to b4207ed Compare December 12, 2025 18:17

coderabbitai bot reviewed Dec 12, 2025

View reviewed changes

packages/opentelemetry-instrumentation-langchain/tests/test_span_utils.py Show resolved Hide resolved

fix(langchain): add role to plain completions

dcddd62

ianchi force-pushed the langchain branch from b4207ed to dcddd62 Compare December 12, 2025 18:26

ianchi requested a review from nirga December 12, 2025 18:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(langchain): incorrect role on generations #3465

fix(langchain): incorrect role on generations #3465

Uh oh!

ianchi commented Nov 25, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 25, 2025 •

edited

Loading

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

nirga left a comment

Uh oh!

Uh oh!

ianchi commented Nov 25, 2025

Uh oh!

nirga commented Nov 27, 2025

Uh oh!

ianchi commented Nov 28, 2025

Uh oh!

ianchi commented Dec 2, 2025

Uh oh!

ianchi commented Dec 4, 2025

Uh oh!

Uh oh!

ianchi commented Dec 12, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix(langchain): incorrect role on generations #3465

Are you sure you want to change the base?

fix(langchain): incorrect role on generations #3465

Uh oh!

Conversation

ianchi commented Nov 25, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

nirga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ianchi commented Nov 25, 2025

Uh oh!

nirga commented Nov 27, 2025

Uh oh!

ianchi commented Nov 28, 2025

Uh oh!

ianchi commented Dec 2, 2025

Uh oh!

ianchi commented Dec 4, 2025

Uh oh!

Uh oh!

ianchi commented Dec 12, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ianchi commented Nov 25, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 25, 2025 •

edited

Loading