feat(agent-tracing): rename TraceSession to Trace and support inputs in createTrace#420
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 20 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaced the inline exported Changes
Sequence Diagram(s)sequenceDiagram
participant SDK as SDKMessage
participant Trace as Trace
participant Tracer as ClaudeAgentTracer
participant Store as RunLogger/Store
SDK->>Trace: deliver message (init/assistant/user/result)
Trace->>Tracer: convert/process message, call handlers
Tracer->>Store: log START/END/ERROR runs
Trace->>Store: update root run (inputs/outputs/extra/status)
Trace->>Trace: manage pendingToolUses, executionOrder
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to include user input messages in the root run inputs of a trace session, aligning with the LangGraph chain run format. The changes involve adding a messages property to the TraceSession class and the CreateSessionOptions interface, along with unit tests to verify the behavior. Feedback was provided to improve code maintainability by defining a named type for the message structure instead of using inline definitions.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@core/agent-tracing/src/ClaudeAgentTracer.ts`:
- Around line 74-77: The new assignment of this.messages into
rootRun.inputs.messages in ClaudeAgentTracer.ts should not persist raw user
prompts; before setting rootRun.inputs.messages (the symbol to change) sanitize
and/or truncate the messages: call a new sanitizer (e.g., redactSensitiveFields
or sanitizeInputs) that strips PII, masks tokens, enforces a maximum message
length and optionally applies a policy-gate/config flag, then assign the
sanitized result to rootRun.inputs.messages; also ensure TracingService.logTrace
continues to receive only the sanitized/limited inputs so OSS uploads never
contain raw prompt content.
- Line 40: The constructor currently assigns caller-provided arrays/objects by
reference (this.messages = options?.messages) and later copies them into run
payloads (rootRun.inputs.messages), which allows external mutation to alter
tracer state; fix by deep-cloning the messages and any message-entry objects
when assigning to ClaudeAgentTracer.this.messages and when setting
rootRun.inputs.messages (and the other assignments around the same code path) so
the tracer keeps its own immutable copy—use structuredClone if available
(fallback to a proven deep-clone utility like lodash.cloneDeep) to clone arrays
and nested entries before assignment and ensure all places that copy messages
create clones rather than referencing the original.
🪄 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: 94fc682d-6dcc-4a05-8e87-2a628e7b13df
📒 Files selected for processing (3)
core/agent-tracing/src/ClaudeAgentTracer.tscore/agent-tracing/src/types.tscore/agent-tracing/test/ClaudeAgentTracer.test.ts
jerryliang64
force-pushed
the
feat/claude-tracer-input-messages
branch
from
e56c939 to
f220c18
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
core/agent-tracing/src/ClaudeAgentTracer.ts (3)
245-248: Good deprecation pattern for backward compatibility.The
@deprecatedJSDoc tag provides IDE/documentation warnings. For additional visibility, consider emitting a runtime warning on first invocation (optional, depending on project conventions):/** `@deprecated` Use createTrace() instead. */ public createSession(options?: CreateTraceOptions): TraceSession { this.logger.warn('[ClaudeAgentTracer] createSession() is deprecated, use createTrace() instead'); return this.createTrace(options); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/agent-tracing/src/ClaudeAgentTracer.ts` around lines 245 - 248, The deprecated createSession method should emit a runtime warning the first time it's called to increase visibility; update ClaudeAgentTracer.createSession to log a deprecation message (e.g., via this.logger.warn or similar) and guard it so the warning is only emitted once (use a private flag like _createSessionWarned on the class), then delegate to this.createTrace(options) as before.
36-43: Shallow copy ofinputsallows external mutation to affect tracer state.Line 40 assigns
options?.inputsby reference. If the caller later mutates the original object, the tracer's internal state and logged data will change unexpectedly. Consider creating a shallow clone at construction time:- this.inputs = options?.inputs; + this.inputs = options?.inputs ? { ...options.inputs } : undefined;If
inputsmay contain nested mutable structures (e.g.,messagesarrays), a deeper clone would be safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/agent-tracing/src/ClaudeAgentTracer.ts` around lines 36 - 43, The constructor of ClaudeAgentTracer currently assigns options?.inputs by reference which allows external mutations to alter tracer state; in the constructor of ClaudeAgentTracer, replace the direct assignment to this.inputs with a clone of options.inputs (shallow clone for simple objects, e.g., copy object/array structure) and if inputs can contain nested mutable structures (like messages arrays) perform a deep clone (e.g., structuredClone or a utility like lodash.cloneDeep) so the tracer keeps an immutable snapshot; update the constructor's handling of inputs (and any related CreateTraceOptions usage) to use the cloned value.
74-76:Object.assignperforms a shallow merge—nested objects remain shared.If
this.inputscontains nested structures like{ messages: [...] }, the array reference is copied, not cloned. Mutations to the original array will affectrootRun.inputs.messages.To fully isolate the run payload, deep-clone nested values or ensure immutability at construction (see previous comment). A minimal fix here:
if (this.inputs) { - Object.assign(this.rootRun.inputs, this.inputs); + Object.assign(this.rootRun.inputs, JSON.parse(JSON.stringify(this.inputs))); }Alternatively, use
structuredClone(this.inputs)if available, or a utility likelodash.cloneDeepfor non-JSON-serializable values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/agent-tracing/src/ClaudeAgentTracer.ts` around lines 74 - 76, In ClaudeAgentTracer (where you assign inputs into this.rootRun.inputs), replace the shallow Object.assign(this.rootRun.inputs, this.inputs) with a deep-clone merge to avoid shared nested references (e.g., clone this.inputs via structuredClone(this.inputs) if available, or lodash.cloneDeep(this.inputs), or JSON.parse(JSON.stringify(this.inputs)) for pure-JSON data) and then merge into this.rootRun.inputs so nested arrays/objects (like messages) are not shared between instances; ensure the chosen cloning approach is applied consistently at construct/assignment points to guarantee immutability of rootRun.inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@core/agent-tracing/src/ClaudeAgentTracer.ts`:
- Around line 245-248: The deprecated createSession method should emit a runtime
warning the first time it's called to increase visibility; update
ClaudeAgentTracer.createSession to log a deprecation message (e.g., via
this.logger.warn or similar) and guard it so the warning is only emitted once
(use a private flag like _createSessionWarned on the class), then delegate to
this.createTrace(options) as before.
- Around line 36-43: The constructor of ClaudeAgentTracer currently assigns
options?.inputs by reference which allows external mutations to alter tracer
state; in the constructor of ClaudeAgentTracer, replace the direct assignment to
this.inputs with a clone of options.inputs (shallow clone for simple objects,
e.g., copy object/array structure) and if inputs can contain nested mutable
structures (like messages arrays) perform a deep clone (e.g., structuredClone or
a utility like lodash.cloneDeep) so the tracer keeps an immutable snapshot;
update the constructor's handling of inputs (and any related CreateTraceOptions
usage) to use the cloned value.
- Around line 74-76: In ClaudeAgentTracer (where you assign inputs into
this.rootRun.inputs), replace the shallow Object.assign(this.rootRun.inputs,
this.inputs) with a deep-clone merge to avoid shared nested references (e.g.,
clone this.inputs via structuredClone(this.inputs) if available, or
lodash.cloneDeep(this.inputs), or JSON.parse(JSON.stringify(this.inputs)) for
pure-JSON data) and then merge into this.rootRun.inputs so nested arrays/objects
(like messages) are not shared between instances; ensure the chosen cloning
approach is applied consistently at construct/assignment points to guarantee
immutability of rootRun.inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0709a65d-d564-437b-b192-31f9c34419b7
📒 Files selected for processing (3)
core/agent-tracing/src/ClaudeAgentTracer.tscore/agent-tracing/src/types.tscore/agent-tracing/test/ClaudeAgentTracer.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- core/agent-tracing/src/types.ts
- core/agent-tracing/test/ClaudeAgentTracer.test.ts
…in createTrace - Rename `TraceSession` → `Trace`, `createSession` → `createTrace`, `CreateSessionOptions` → `CreateTraceOptions` - Add `inputs` option to `CreateTraceOptions` for injecting user messages into root run inputs - This aligns Claude tracer with LangGraph tracer which auto-populates `run.inputs.messages` Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jerryliang64
force-pushed
the
feat/claude-tracer-input-messages
branch
from
f220c18 to
df923d8
Compare
jerryliang64
changed the title
…oot run Align with LangGraph convention: root run inputs should only contain user business inputs (e.g. messages), while tools/model/mcp_servers and other config belong in extra. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Split Trace out of ClaudeAgentTracer.ts into its own file for clearer file-level responsibility: Trace.ts owns per-execution state management, ClaudeAgentTracer.ts owns the singleton service and format conversion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
core/agent-tracing/src/Trace.ts (1)
28-35: Consider defensive copying ofinputsto prevent external mutation.The
inputsobject is stored by reference at Line 32, and later shallow-copied viaObject.assignat Line 67. Ifinputscontains nested objects (e.g.,{ messages: [...] }), those nested objects remain as shared references. If the caller mutates the original aftercreateTrace()but beforehandleInit()processes the init message, the trace data could change unexpectedly.🛡️ Proposed fix using structuredClone for defensive copy
constructor(tracer: ClaudeAgentTracer, options?: CreateTraceOptions) { this.tracer = tracer; this.traceId = options?.traceId || randomUUID(); this.threadId = options?.threadId; - this.inputs = options?.inputs; + this.inputs = options?.inputs ? structuredClone(options.inputs) : undefined; this.rootRunId = randomUUID(); this.startTime = Date.now(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/agent-tracing/src/Trace.ts` around lines 28 - 35, The Trace constructor currently assigns options?.inputs by reference which can lead to external mutation; change the constructor in Trace (the constructor of class Trace) to defensively deep-copy inputs when present (so downstream operations in createTrace() and handleInit() work with an immutable snapshot). Use structuredClone(options.inputs) when available, falling back to a safe deep-copy strategy (e.g., JSON.parse(JSON.stringify(options.inputs)) or a utility) to ensure nested objects/arrays are not shared.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/agent-tracing/src/Trace.ts`:
- Around line 171-173: Ensure this.rootRun.error is always a string by
converting message.result before assignment in Trace.ts: when message.is_error
is true, set this.rootRun.error to message.result if it's already a string,
otherwise serialize it (e.g., JSON.stringify with a safe fallback like
String(message.result)) so Run.error remains a string; update the assignment
around message.is_error / this.rootRun.error / message.result accordingly.
- Around line 96-110: The loop creating tool runs must guard against missing
block.id before using it as the Map key; update the tool_use handling in
Trace.ts (the block -> toolRun creation path that calls
createToolRunStartInternal, pushes to this.rootRun.child_runs, sets
this.pendingToolUses, and calls this.tracer.logTrace) to detect when block.id is
undefined and handle it by either (a) generating a unique stable id (e.g.,
crypto.randomUUID() or an internal counter) and assigning it to block.id before
calling this.pendingToolUses.set(...), or (b) skipping creation and logging an
error—ensure whichever approach chosen uses the same id for both
pendingToolUses.set and any later matching logic so tool results will correctly
correlate to the created toolRun.
---
Nitpick comments:
In `@core/agent-tracing/src/Trace.ts`:
- Around line 28-35: The Trace constructor currently assigns options?.inputs by
reference which can lead to external mutation; change the constructor in Trace
(the constructor of class Trace) to defensively deep-copy inputs when present
(so downstream operations in createTrace() and handleInit() work with an
immutable snapshot). Use structuredClone(options.inputs) when available, falling
back to a safe deep-copy strategy (e.g.,
JSON.parse(JSON.stringify(options.inputs)) or a utility) to ensure nested
objects/arrays are not shared.
🪄 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: b8fe64b6-f05f-41a7-8b4d-b04fc8ffbc5b
📒 Files selected for processing (4)
core/agent-tracing/claude.tscore/agent-tracing/src/ClaudeAgentTracer.tscore/agent-tracing/src/Trace.tscore/agent-tracing/test/ClaudeAgentTracer.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- core/agent-tracing/test/ClaudeAgentTracer.test.ts
- core/agent-tracing/claude.ts
… internal class Trace is only used internally by ClaudeAgentTracer and not needed by external callers (they get it via createTrace() return type inference). Keep it as a non-exported class in the same file and remove from exports. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
TraceSession→Trace,createSession()→createTrace(),CreateSessionOptions→CreateTraceOptionsfor clearer semanticsinputsoption toCreateTraceOptions, allowing callers to inject user messages into root run'sinputsfieldinputstoextra, aligning with LangGraph convention whereinputsonly contains user business dataChanges
core/agent-tracing/src/types.ts- Rename type and addinputsfieldcore/agent-tracing/src/ClaudeAgentTracer.ts- Rename class/method, merge inputs inhandleInit, move config fields toextracore/agent-tracing/claude.ts- Update exportcore/agent-tracing/test/ClaudeAgentTracer.test.ts- Update all references + 2 new test cases for inputsRoot Run structure (before → after)
Usage
Test plan
createTraceextra, notinputs🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
API Changes
createSession()method tocreateTrace()for improved semantic clarity.CreateSessionOptionstype toCreateTraceOptions.inputsparameter.