feat(instrumentation-langchain): Migrate langchain semantic conventions to 1.40#920
Conversation
dvirski
requested review from
OzBenSimhonTraceloop,
galkleinman,
lenatraceloop and
nina-kollman
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR upgrades LangChain integrations to v1.x, moves instrumentation to incubating OpenTelemetry GenAI semantic attributes, adds ESM support for patching LangChain's CallbackManager, revises span semantics for chains/tools/agents, and updates tests and sample imports to new package paths. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant LangChain
participant CallbackHandler as TraceloopCallbackHandler
participant Span as OpenTelemetrySpan
participant Backend as TracingBackend
Client->>LangChain: invoke chain/tool/LLM
LangChain->>CallbackHandler: onStart(inputs, meta)
CallbackHandler->>Span: start span (name = "{operation} {model/actor}")
CallbackHandler->>Span: set gen_ai.provider.name, gen_ai.operation.name
CallbackHandler->>Span: set gen_ai.input.messages (serialized)
Span->>Backend: record start
LangChain->>CallbackHandler: onEnd(outputs, llmOutput, usage)
CallbackHandler->>Span: set gen_ai.output.messages, gen_ai.response.id, model info
CallbackHandler->>Span: set gen_ai.response.finish_reasons
CallbackHandler->>Span: set gen_ai.usage.input_tokens / output_tokens / total
Span->>Backend: end span with attributes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/instrumentation-langchain/src/callback_handler.ts (1)
239-243: Consider omittingfinish_reasonfrom output messages when null.The output message always includes
finish_reasoneven when it resolves tonull. Per the OTel spec and theopenai-semconv-review.mdin this PR,finish_reasonshould be omitted when not available.🔧 Suggested fix
- return { - role: "assistant", - parts: [{ type: "text", content: text }], - finish_reason: genFinishReason, - }; + const outputMsg: { role: string; parts: object[]; finish_reason?: string } = { + role: "assistant", + parts: [{ type: "text", content: text }], + }; + if (genFinishReason) { + outputMsg.finish_reason = genFinishReason; + } + return outputMsg;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/instrumentation-langchain/src/callback_handler.ts` around lines 239 - 243, The returned assistant message object always includes finish_reason even when genFinishReason is null; change the code that constructs the returned object (the block producing role: "assistant", parts: [...], finish_reason: genFinishReason) to only add the finish_reason property when genFinishReason is not null/undefined (e.g., build a result object and conditionally assign result.finish_reason = genFinishReason if genFinishReason != null) so that finish_reason is omitted when unavailable.packages/instrumentation-langchain/src/instrumentation.ts (1)
66-76: Potential race condition with async ESM patching.The async
import()for ESM patching is fire-and-forget. If user code imports and invokes LangChain ESM modules before this promise resolves, those calls won't be instrumented. This is a known limitation of dynamic patching in ESM environments.Consider documenting this behavior or adding a
manuallyInstrumentcall guidance for ESM-first codebases where early instrumentation is critical.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/instrumentation-langchain/src/instrumentation.ts` around lines 66 - 76, The dynamic ESM import in instrumentation.ts that calls this.patchCallbackManager on esmModule.CallbackManager is fire-and-forget and can miss early ESM-first usage; update documentation and the public API to provide explicit guidance and a helper: add a documented manuallyInstrument (or ensureInstrument) export on the Instrumentation class that synchronously accepts the CallbackManager constructor (or waits for the import) so callers can blockingly instrument ESM-first code paths, and update the import("@langchain/core/callbacks/manager") usage comment to reference the new manuallyInstrument/ensureInstrument helper and the patchCallbackManager method for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/instrumentation-langchain/test/instrumentation.test.ts`:
- Around line 531-553: The fallback branch that selects completionSpan by name
(variable completionSpan) incorrectly asserts the provider attribute: remove or
adjust the assertion checking completionAttributes[ATTR_GEN_AI_PROVIDER_NAME]
=== GEN_AI_PROVIDER_NAME_VALUE_AWS_BEDROCK because this branch is entered when
no span with that provider exists; instead only assert the operation name
(completionAttributes[ATTR_GEN_AI_OPERATION_NAME] ===
GEN_AI_OPERATION_NAME_VALUE_CHAT) or change the lookup logic so the provider
assertion is valid (e.g., only enter this fallback when provider matches).
---
Nitpick comments:
In `@packages/instrumentation-langchain/src/callback_handler.ts`:
- Around line 239-243: The returned assistant message object always includes
finish_reason even when genFinishReason is null; change the code that constructs
the returned object (the block producing role: "assistant", parts: [...],
finish_reason: genFinishReason) to only add the finish_reason property when
genFinishReason is not null/undefined (e.g., build a result object and
conditionally assign result.finish_reason = genFinishReason if genFinishReason
!= null) so that finish_reason is omitted when unavailable.
In `@packages/instrumentation-langchain/src/instrumentation.ts`:
- Around line 66-76: The dynamic ESM import in instrumentation.ts that calls
this.patchCallbackManager on esmModule.CallbackManager is fire-and-forget and
can miss early ESM-first usage; update documentation and the public API to
provide explicit guidance and a helper: add a documented manuallyInstrument (or
ensureInstrument) export on the Instrumentation class that synchronously accepts
the CallbackManager constructor (or waits for the import) so callers can
blockingly instrument ESM-first code paths, and update the
import("@langchain/core/callbacks/manager") usage comment to reference the new
manuallyInstrument/ensureInstrument helper and the patchCallbackManager method
for clarity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc8213b1-3c41-4ff2-bfe0-ae77b3d00881
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
openai-semconv-review.mdpackages/instrumentation-langchain/package.jsonpackages/instrumentation-langchain/src/callback_handler.tspackages/instrumentation-langchain/src/instrumentation.tspackages/instrumentation-langchain/test/instrumentation.test.tspackages/sample-app/package.jsonpackages/sample-app/src/sample_langchain.tspackages/traceloop-sdk/package.json
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/instrumentation-langchain/src/callback_handler.ts (1)
343-372:⚠️ Potential issue | 🟠 Major
handleChainStart()now marks every chain asinvoke_agent.This callback also covers ordinary chains, so
RetrievalQAChaincalls like the ones at Lines 47 and 110 inpackages/sample-app/src/sample_langchain.tswill now emit agent semantics (gen_ai.operation.name = invoke_agent,gen_ai.agent.name,traceloop.span.kind = workflow). Please gate the agent attributes to actual agent runs and keep non-agent chains on a generic chain path.As per coding guidelines: Instrumentations must create spans with appropriate AI/LLM semantic attributes for calls they wrap.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/instrumentation-langchain/src/callback_handler.ts` around lines 343 - 372, The current handleChainStart() unconditionally marks every chain as an agent run; change it to detect real agent runs and only set agent-specific semantics for those. Inside handleChainStart(), before using GEN_AI_OPERATION_NAME_VALUE_INVOKE_AGENT and setting agent attributes (the tracer.startSpan call, span.setAttributes, and this.spans.set entries), add a guard that checks whether the chain is an agent (e.g., inspect chain properties such as chain.agent, chain.isAgent, chain.tags or chain.metadata that indicate an agent) and only then use GEN_AI_OPERATION_NAME_VALUE_INVOKE_AGENT, ATTR_GEN_AI_AGENT_NAME, and traceloop.workflow/span.kind="workflow"; for non-agent chains create the span with a generic chain operation name (e.g., GEN_AI_OPERATION_NAME_VALUE_INVOKE_CHAIN or a neutral name), set non-agent attributes (e.g., traceloop.span.kind="chain" and traceloop.chain.name) and record the span in this.spans with operationType reflecting a chain, so ordinary RetrievalQAChain runs keep generic chain semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/instrumentation-langchain/src/callback_handler.ts`:
- Around line 326-341: The handleChainStart override currently assumes the
`@langchain/core` >=1.0.0 signature (method handleChainStart) but package
peerDependencies still allow 0.3.x which leads to misordered params and wrong
span names; either tighten peerDependencies to ">=1.0.0 <2.0.0" in package.json
to accurately reflect supported versions, or add runtime detection in the
handleChainStart implementation to support both signatures (detect 0.3.x vs
1.0.0+ by inspecting argument types/positions or reading `@langchain/core` version
from its package metadata) and normalize parameters (re-map
runType/runName/parentRunId/etc. into the expected variables) before using them;
update the method (handleChainStart) and related tests and comments accordingly.
In `@packages/instrumentation-langchain/test/instrumentation.test.ts`:
- Around line 525-533: The tests use truthiness checks for token counts which
fail when counts are 0; update the assertions in instrumentation.test.ts to
check definedness instead of truthiness: replace
assert.ok(attributes[ATTR_GEN_AI_USAGE_INPUT_TOKENS], ...) and
assert.ok(attributes[ATTR_GEN_AI_USAGE_OUTPUT_TOKENS], ...) with assertions that
the values are not undefined (e.g.,
assert.notStrictEqual(attributes[ATTR_GEN_AI_USAGE_INPUT_TOKENS], undefined) and
assert.notStrictEqual(attributes[ATTR_GEN_AI_USAGE_OUTPUT_TOKENS], undefined))
so handleLLMEnd()’s preserved 0 counts pass the test.
- Around line 28-35: The tests reference langchainChainsModule but the top
import block removed its declaration, causing TS errors; re-add
langchainChainsModule as a named import alongside createOpenAIToolsAgent and
AgentExecutor in the file's import section so the symbol is available to the
code that uses it (the references around langchainChainsModule in the test).
Ensure you import the exact exported name (langchainChainsModule) from the
module that originally exported it so TypeScript can resolve the identifier.
In `@packages/sample-app/src/sample_langchain.ts`:
- Around line 11-22: The LangChain symbols (HNSWLib, OpenAIEmbeddings, OpenAI,
RecursiveCharacterTextSplitter, RetrievalQAChain, loadQAStuffChain,
createOpenAIToolsAgent, AgentExecutor, Calculator, ChatPromptTemplate,
PromptTemplate, ChatOpenAI) are currently statically imported so they load
before traceloop.initialize(); change these to dynamic imports using await
import(...) after you call traceloop.initialize() (or your initialize() wrapper)
so LangChain’s runtime patches apply before the modules load; locate where those
static imports are declared in sample_langchain.ts, remove them, and replace
usages by awaiting the corresponding modules (e.g., const { OpenAI } = await
import("@langchain/openai")) immediately after initialization.
---
Outside diff comments:
In `@packages/instrumentation-langchain/src/callback_handler.ts`:
- Around line 343-372: The current handleChainStart() unconditionally marks
every chain as an agent run; change it to detect real agent runs and only set
agent-specific semantics for those. Inside handleChainStart(), before using
GEN_AI_OPERATION_NAME_VALUE_INVOKE_AGENT and setting agent attributes (the
tracer.startSpan call, span.setAttributes, and this.spans.set entries), add a
guard that checks whether the chain is an agent (e.g., inspect chain properties
such as chain.agent, chain.isAgent, chain.tags or chain.metadata that indicate
an agent) and only then use GEN_AI_OPERATION_NAME_VALUE_INVOKE_AGENT,
ATTR_GEN_AI_AGENT_NAME, and traceloop.workflow/span.kind="workflow"; for
non-agent chains create the span with a generic chain operation name (e.g.,
GEN_AI_OPERATION_NAME_VALUE_INVOKE_CHAIN or a neutral name), set non-agent
attributes (e.g., traceloop.span.kind="chain" and traceloop.chain.name) and
record the span in this.spans with operationType reflecting a chain, so ordinary
RetrievalQAChain runs keep generic chain semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 973a0fe2-5d9c-46bd-ab1f-2ddc34e80bb3
📒 Files selected for processing (4)
packages/instrumentation-langchain/src/callback_handler.tspackages/instrumentation-langchain/src/instrumentation.tspackages/instrumentation-langchain/test/instrumentation.test.tspackages/sample-app/src/sample_langchain.ts
✅ Files skipped from review due to trivial changes (1)
- packages/instrumentation-langchain/src/instrumentation.ts
packages/instrumentation-langchain/test/instrumentation.test.ts
Outdated
Show resolved
Hide resolved
| import { HNSWLib } from "@langchain/community/vectorstores/hnswlib"; | ||
| import { OpenAIEmbeddings, OpenAI } from "@langchain/openai"; | ||
| import { RecursiveCharacterTextSplitter } from "@langchain/textsplitters"; | ||
| import { RetrievalQAChain, loadQAStuffChain } from "@langchain/classic/chains"; | ||
| import { | ||
| createOpenAIToolsAgent, | ||
| AgentExecutor, | ||
| } from "@langchain/classic/agents"; | ||
| import { Calculator } from "@langchain/community/tools/calculator"; | ||
| import { ChatPromptTemplate, PromptTemplate } from "@langchain/core/prompts"; | ||
| import { ChatOpenAI } from "@langchain/openai"; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for file in package.json packages/sample-app/package.json; do
[ -f "$file" ] || continue
echo "FILE: $file"
python - <<'PY' "$file"
import json, sys
with open(sys.argv[1]) as f:
data = json.load(f)
print({
"type": data.get("type"),
"main": data.get("main"),
"module": data.get("module"),
"exports": data.get("exports"),
})
PY
done
fd -p 'tsconfig.*json$' packages/sample-app . | while read -r file; do
echo "FILE: $file"
python - <<'PY' "$file"
import json, sys
with open(sys.argv[1]) as f:
data = json.load(f)
opts = data.get("compilerOptions") or {}
print({
"module": opts.get("module"),
"moduleResolution": opts.get("moduleResolution"),
"verbatimModuleSyntax": opts.get("verbatimModuleSyntax"),
"target": opts.get("target"),
})
PY
doneRepository: traceloop/openllmetry-js
Length of output: 4963
Convert LangChain imports to dynamic imports after initialization.
In this ESM module ("type": "module" + "module": "esnext" in tsconfig), static imports are evaluated before the module body executes. Reordering the imports below initialize() does not change this—LangChain still patches after the imports are already loaded. Use await import(...) calls after the traceloop.initialize(...) block to ensure patches apply before modules load.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sample-app/src/sample_langchain.ts` around lines 11 - 22, The
LangChain symbols (HNSWLib, OpenAIEmbeddings, OpenAI,
RecursiveCharacterTextSplitter, RetrievalQAChain, loadQAStuffChain,
createOpenAIToolsAgent, AgentExecutor, Calculator, ChatPromptTemplate,
PromptTemplate, ChatOpenAI) are currently statically imported so they load
before traceloop.initialize(); change these to dynamic imports using await
import(...) after you call traceloop.initialize() (or your initialize() wrapper)
so LangChain’s runtime patches apply before the modules load; locate where those
static imports are declared in sample_langchain.ts, remove them, and replace
usages by awaiting the corresponding modules (e.g., const { OpenAI } = await
import("@langchain/openai")) immediately after initialization.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/instrumentation-langchain/package.json (1)
57-71:⚠️ Potential issue | 🟠 MajorUpdate Node engine requirement to
>=20to match LangChain v1 compatibility.Line 25 declares
"node": ">=14", but LangChain v1.0.0 (as shown in the dependencies) requires Node.js 20+. The package should not advertise support for Node 14 when its dependencies enforce Node 20 as the minimum, which will cause runtime failures for users on older Node versions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/instrumentation-langchain/package.json` around lines 57 - 71, Update the package.json "engines" node requirement from ">=14" to ">=20" so it matches LangChain v1 dependency compatibility; edit the "engines" entry in packages/instrumentation-langchain/package.json (the "node" field) to ">=20" and ensure any README or documentation in this package that references the supported Node version is updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/instrumentation-langchain/package.json`:
- Around line 40-47: The SDK currently causes a hard dependency on
`@langchain/core` because LangChainInstrumentation is imported eagerly from
packages/traceloop-sdk/src/lib/tracing/index.ts and that module
(packages/instrumentation-langchain/src/callback_handler.ts) imports
`@langchain/core` at module load; fix by either (A) moving "@langchain/core" from
peerDependencies into dependencies in
packages/instrumentation-langchain/package.json so installs of the SDK bring it
along, or (B) lazy-loading the LangChain integration: remove the top-level
import of LangChainInstrumentation in traceloop-sdk/src/lib/tracing/index.ts and
instead dynamically import/create it only when used (e.g., inside the function
that registers/instantiates LangChainInstrumentation), or modify
callback_handler.ts to perform a runtime/dynamic import of "@langchain/core"
within the LangChainInstrumentation constructor or initialization method so the
module is only loaded when the instrumentation is explicitly activated.
In `@packages/instrumentation-langchain/src/callback_handler.ts`:
- Around line 259-273: The code currently skips setting
SpanAttributes.GEN_AI_USAGE_TOTAL_TOKENS when totalTokens is 0; update the logic
in callback_handler.ts where you compute totalTokens from output.llmOutput.usage
so that the span attribute is set whenever usage exists (or whenever totalTokens
is a defined number), not only when totalTokens > 0 — i.e., after computing
totalTokens = (usage.input_tokens || 0) + (usage.output_tokens || 0), set
SpanAttributes.GEN_AI_USAGE_TOTAL_TOKENS on span even if totalTokens === 0 (use
a defined-check on usage or totalTokens rather than > 0) to preserve zero
totals.
- Around line 216-226: The code currently only takes the first generation from
each LLMResult (e.g., using generation[0]) which drops secondary generations and
their finish reasons; update the serialization logic that builds output messages
and attributes (references: LLMResult, generation, extractFinishReason,
langchainFinishReasonMap, ATTR_GEN_AI_RESPONSE_FINISH_REASONS) to iterate all
inner-generation entries, include every generation's content and finish_reason,
and map each finish_reason via langchainFinishReasonMap (not just the first) so
the span attribute and output messages contain arrays of all generations and
their corresponding mapped finish reasons.
---
Outside diff comments:
In `@packages/instrumentation-langchain/package.json`:
- Around line 57-71: Update the package.json "engines" node requirement from
">=14" to ">=20" so it matches LangChain v1 dependency compatibility; edit the
"engines" entry in packages/instrumentation-langchain/package.json (the "node"
field) to ">=20" and ensure any README or documentation in this package that
references the supported Node version is updated accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: df073dd0-d847-43df-b56a-5bcc099a3fa4
📒 Files selected for processing (3)
packages/instrumentation-langchain/package.jsonpackages/instrumentation-langchain/src/callback_handler.tspackages/instrumentation-langchain/src/instrumentation.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/instrumentation-langchain/package.json (1)
24-26: Breaking change: Node.js minimum version bumped to 20.This is a significant change from
>=14. Ensure this is documented in the changelog/release notes, as it may affect users on Node 16 or 18 LTS.Consider whether Node 18 LTS (supported until April 2025) should still be included:
"engines": { - "node": ">=20" + "node": ">=18" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/instrumentation-langchain/package.json` around lines 24 - 26, The package.json change bumps the Node.js minimum to "node": ">=20" which is a breaking change for users on Node 16/18; add a clear changelog/release-notes entry describing this breaking change, listing affected Node LTS versions (e.g., Node 16 and 18), migration guidance, and the rationale for moving to Node 20, and either revert or broaden the engines field (the "engines" -> "node" entry) to include ">=18" if you want to keep Node 18 supported until April 2025; update release documentation and any contributing/developer docs that reference supported Node versions and CI configurations to match the final decision.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/instrumentation-langchain/src/callback_handler.ts`:
- Around line 386-390: The span entry currently hardcodes operationType to
GEN_AI_OPERATION_NAME_VALUE_INVOKE_AGENT causing wrong metadata for non-agent
chains; update the spans.set call (the object stored for runId in the spans Map
inside the callback handler) to use the previously computed operationName (or a
conditional based on isAgentChain()) instead of the hardcoded
GEN_AI_OPERATION_NAME_VALUE_INVOKE_AGENT so operationType reflects the actual
chain type (refer to spans.set, runId, operationType, operationName, and
isAgentChain()).
---
Nitpick comments:
In `@packages/instrumentation-langchain/package.json`:
- Around line 24-26: The package.json change bumps the Node.js minimum to
"node": ">=20" which is a breaking change for users on Node 16/18; add a clear
changelog/release-notes entry describing this breaking change, listing affected
Node LTS versions (e.g., Node 16 and 18), migration guidance, and the rationale
for moving to Node 20, and either revert or broaden the engines field (the
"engines" -> "node" entry) to include ">=18" if you want to keep Node 18
supported until April 2025; update release documentation and any
contributing/developer docs that reference supported Node versions and CI
configurations to match the final decision.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ed93a07-be31-4696-9e82-0036773e2c3e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/instrumentation-langchain/package.jsonpackages/instrumentation-langchain/src/callback_handler.tspackages/instrumentation-langchain/test/instrumentation.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/instrumentation-langchain/test/instrumentation.test.ts
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/instrumentation-langchain/src/callback_handler.ts`:
- Around line 265-271: The code is incorrectly falling back to
allFinishReasons[0] and can assign another candidate's finish reason to a
generation; change the logic in the genFinishReason computation so you only set
a finish_reason when the specific generation exposes one (e.g., check
gen.finish_reason or the local raw value and map via langchainFinishReasonMap),
and otherwise do not include or set finish_reason (leave it unset) on the
returned message object instead of using allFinishReasons[0]; update the code
around genFinishReason, raw, langchainFinishReasonMap, allFinishReasons and the
returned object so finish_reason is only present when that generation provides
it.
- Around line 132-146: The current input serialization in mapMessageTypeToRole /
inputMessages collapses non-string BaseMessage.content into a single text part
(using JSON.stringify) losing structured ContentBlock data; change inputMessages
construction to detect array/object content and preserve its structured blocks
as parts (e.g., map ContentBlock items to parts) and set
ATTR_GEN_AI_INPUT_MESSAGES to that structured representation instead of
stringified text. For generation outputs, use gen?.message (the full structured
message) rather than gen?.text when building candidate content so tool-call
blocks and rich types are retained. Finally, when populating candidate finish
reasons (where allFinishReasons and per-candidate finish_reason are used), stop
using allFinishReasons[0] as a fallback from other candidates; instead prefer
the candidate's own extracted finish_reason and if missing assign a neutral
fallback like "unknown" (or empty string) so reasons are not misattributed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5acaa99c-9d8c-4a20-8af2-2de2649e8843
📒 Files selected for processing (1)
packages/instrumentation-langchain/src/callback_handler.ts
| const inputMessages = flatMessages.map((message) => { | ||
| const role = this.mapMessageTypeToRole(message.type); | ||
| const content = | ||
| typeof message.content === "string" | ||
| ? message.content | ||
| : JSON.stringify(message.content); | ||
| return { | ||
| role, | ||
| parts: [{ type: "text", content }], | ||
| }; | ||
| }); | ||
| span.setAttribute( | ||
| ATTR_GEN_AI_INPUT_MESSAGES, | ||
| JSON.stringify(inputMessages), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@langchain/core JavaScript v1 BaseMessage content array blocks tool_calls ChatGeneration message vs text
💡 Result:
In @langchain/core JavaScript v1, BaseMessage has a content property that is an array of content blocks (Array), supporting structured content like text, images, tool calls, etc. This is the modern v1 standard for multimodal and provider-agnostic message content. There's also a deprecated contentBlocks property and a text property for extracting plain text. Tool calls in AIMessage are represented as ContentBlock.Tools.ToolCall blocks within the content array (e.g., { type: "tool_call", name: "...", args: {...}, id: "..." }), or via tool_calls/tool_call_chunks arrays on AIMessage/AIMessageChunk for legacy/compatibility. The additional_kwargs may contain deprecated function_call or tool_calls fields. ChatGeneration is an output interface from chat model .generate with properties: text: string (generated text), message: BaseMessage (structured message with content array), and optional generationInfo. The text is derived from the message's text content (via message.text), while the message provides the full structured content array, tool calls, etc. Key differences: - BaseMessage content: Structured array blocks (v1 standard, supports tool calls as blocks). - ChatGeneration text: Plain string (legacy/simple access). - ChatGeneration message: Full BaseMessage with content array. This design allows backward compatibility while enabling rich content like tool calls in content blocks.
Citations:
- 1: https://reference.langchain.com/javascript/langchain-core/messages/BaseMessage
- 2: https://reference.langchain.com/javascript/langchain-core/messages/BaseMessage/content
- 3: https://github.com/langchain-ai/langchainjs/blob/main/libs/langchain-core/src/messages/base.ts
- 4: https://docs.langchain.com/oss/javascript/langchain/messages
- 5: https://docs.langchain.com/oss/javascript/releases/langchain-v1
- 6: https://v03.api.js.langchain.com/interfaces/_langchain_core.outputs.ChatGeneration.html
- 7: https://reference.langchain.com/javascript/langchain-core/outputs/ChatGeneration
🏁 Script executed:
fd -t f "callback_handler.ts" packages/instrumentation-langchain/Repository: traceloop/openllmetry-js
Length of output: 126
🏁 Script executed:
cat -n packages/instrumentation-langchain/src/callback_handler.ts | sed -n '120,280p'Repository: traceloop/openllmetry-js
Length of output: 6441
Preserve structured LangChain message content instead of collapsing to plain text.
Lines 132–146 serialize non-string BaseMessage.content (which is an array of ContentBlock objects in LangChain v1) by stringifying it into a single { type: "text", content } part. This loses structured blocks such as tool calls. Similarly, line 270 uses gen?.text instead of gen?.message, dropping the full structured message content that includes tool-call blocks and other rich content types.
Also, line 267 assigns allFinishReasons[0] as a fallback finish reason to any candidate lacking its own finish_reason. Since allFinishReasons is collected from all candidates across all generation groups, this misattributes one candidate's finish reason to another. Each candidate should either have its own extracted finish reason or be assigned a consistent, candidate-agnostic fallback (e.g., an empty string or "unknown"), not a value from a different candidate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/instrumentation-langchain/src/callback_handler.ts` around lines 132
- 146, The current input serialization in mapMessageTypeToRole / inputMessages
collapses non-string BaseMessage.content into a single text part (using
JSON.stringify) losing structured ContentBlock data; change inputMessages
construction to detect array/object content and preserve its structured blocks
as parts (e.g., map ContentBlock items to parts) and set
ATTR_GEN_AI_INPUT_MESSAGES to that structured representation instead of
stringified text. For generation outputs, use gen?.message (the full structured
message) rather than gen?.text when building candidate content so tool-call
blocks and rich types are retained. Finally, when populating candidate finish
reasons (where allFinishReasons and per-candidate finish_reason are used), stop
using allFinishReasons[0] as a fallback from other candidates; instead prefer
the candidate's own extracted finish_reason and if missing assign a neutral
fallback like "unknown" (or empty string) so reasons are not misattributed.
dvirski
changed the title
2 participants
Summary by CodeRabbit
New Features
Chores
Style
Tests