feat(agent-tracing): add @eggjs/agent-tracing package for AI agent tracing#5822
Open
jerryliang64 wants to merge 10 commits intonextfrom
Open
feat(agent-tracing): add @eggjs/agent-tracing package for AI agent tracing#5822jerryliang64 wants to merge 10 commits intonextfrom
jerryliang64 wants to merge 10 commits intonextfrom
Conversation
…acing Add tracing support for AI agents built with LangGraph and Claude Agent SDK. - LangGraphTracer: extends LangChain BaseTracer, hooks into graph lifecycle - ClaudeAgentTracer: converts Claude SDK messages to LangChain Run format - TraceSession: streaming support for real-time message processing - TracingService: shared log formatting, OSS upload, and log service sync - applyTracerConfig: shared configure() logic for both tracers - Comprehensive tests for configure, LangGraph integration, and Claude integration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… code - Rename test files to match PascalCase convention - Remove unused module.yml - Clean up TracingService and ClaudeAgentTracer code - Remove unnecessary dependencies from package.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Bump @langchain/core from ^1.1.1 to ^1.1.29 in langchain-decorator, langchain plugin, and agent-tracing to fix `this._addVersion is not a function` error caused by @langchain/openai@1.2.11 requiring the new _addVersion method introduced in @langchain/core@1.1.29 - Run pnpm dedupe to resolve ERR_PNPM_DEDUPE_CHECK_ISSUES in CI Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lient injection Remove hard dependency on ali-oss. TracingService now receives an optional IOssClient via @InjectOptional(), allowing users to provide any OSS implementation through the Tegg IoC container. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ient IoC injection Remove hardcoded HTTP POST logic in syncLocalToLogService and replace with an abstract ILogServiceClient that users implement and register via Tegg IoC, consistent with the existing IOssClient injection pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a dedicated tracing package for AI agents, enabling detailed monitoring and debugging of agent interactions. It standardizes how agent execution flows, LLM calls, and tool usage are logged, providing structured data for analysis. The architectural improvements, such as dependency injection for external services, ensure the tracing solution is robust, extensible, and easily testable within the Egg.js ecosystem. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Contributor
📝 WalkthroughWalkthroughIntroduces a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as Application
participant CAT as ClaudeAgentTracer
participant TS as TracingService
participant OSS as OSS Client
participant LogSvc as Log Service Client
User->>App: send Claude message sequence
App->>CAT: createSession(sessionId)
App->>CAT: processMessage(sdkMessage)
CAT->>CAT: convertSDKMessage()
alt Init message
CAT->>CAT: handleInit() → create root Run
CAT->>TS: logTrace(rootRun, START)
else Assistant (LLM / tool)
CAT->>CAT: create LLM run (+ tool runs if present)
CAT->>TS: logTrace(llmRun, START)
CAT->>TS: logTrace(toolRun, START/END)
else Tool result
CAT->>CAT: complete tool run
CAT->>TS: logTrace(toolRun, END)
else Final result
CAT->>CAT: finalize root Run (attach outputs/costs)
CAT->>TS: logTrace(rootRun, END)
end
TS->>TS: filterHiddenData(run)
alt Field marked for OSS
TS->>OSS: uploadToOss(key, content)
OSS-->>TS: success
TS->>TS: replace field with IResource ref
end
alt Local environment
TS->>LogSvc: syncLocalToLogService(log)
LogSvc-->>TS: ack
end
TS->>TS: emit final log line
sequenceDiagram
actor User
participant App as Application
participant SG as StateGraph
participant LGT as LangGraphTracer
participant TS as TracingService
User->>App: invoke state graph
App->>SG: execute graph
SG->>LGT: onChainStart(chainRun)
LGT->>TS: logTrace(chainRun, START)
SG->>SG: execute node
alt LLM node
SG->>LGT: onLLMStart(llmRun)
LGT->>TS: logTrace(llmRun, START)
SG->>LGT: onLLMEnd(llmRun)
LGT->>TS: logTrace(llmRun, END)
else Tool node
SG->>LGT: onToolStart(toolRun)
LGT->>TS: logTrace(toolRun, START)
SG->>LGT: onToolEnd(toolRun)
LGT->>TS: logTrace(toolRun, END)
else Error
SG->>LGT: onChainError(chainRun)
LGT->>TS: logTrace(chainRun, ERROR)
end
SG->>LGT: onChainEnd(chainRun)
LGT->>TS: logTrace(chainRun, END)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Update lockfile to reflect the current state of tegg/core/agent-tracing/package.json, fixing CI frozen-lockfile install failure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Deploying egg-v3 with
|
| Latest commit: |
822b9b6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ab1663d5.egg-v3.pages.dev |
| Branch Preview URL: | https://feat-agent-tracing.egg-v3.pages.dev |
Deploying egg with
|
| Latest commit: |
822b9b6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://467d53ec.egg-cci.pages.dev |
| Branch Preview URL: | https://feat-agent-tracing.egg-cci.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new package, @eggjs/agent-tracing, to provide tracing capabilities for AI agents, specifically supporting LangChain and the Claude Agent SDK. The implementation follows good design principles by using Dependency Injection for OSS and logging clients, making the system more modular and testable. The code is well-structured and includes a comprehensive set of tests for the new functionality.
My review identifies a critical issue with incorrect timestamping for child runs in the ClaudeAgentTracer, which would lead to inaccurate trace data. I've also included some suggestions to improve type safety and code clarity in a few areas. Overall, this is a solid contribution that adds valuable tracing features.
Note: Security Review did not run due to the size of the PR.
Comment on lines
+81
to
+88
| const llmRun = this.tracer.createLLMRunInternal( | ||
| message, | ||
| this.rootRunId, | ||
| this.traceId, | ||
| this.executionOrder++, | ||
| this.startTime, | ||
| true, | ||
| ); |
Contributor
There was a problem hiding this comment.
The startTime argument for creating this child run is this.startTime, which corresponds to the start of the entire trace session. This results in incorrect timestamps for all child runs (LLM, tool runs), as they will all share the same initial timestamp. To accurately capture the timing of this event, you should use Date.now() instead. This issue occurs in multiple places within the TraceSession class when creating or completing child runs (e.g., in handleAssistant, handleUser, and handleResult).
Suggested change
| const llmRun = this.tracer.createLLMRunInternal( | |
| message, | |
| this.rootRunId, | |
| this.traceId, | |
| this.executionOrder++, | |
| this.startTime, | |
| true, | |
| ); | |
| const llmRun = this.tracer.createLLMRunInternal( | |
| message, | |
| this.rootRunId, | |
| this.traceId, | |
| this.executionOrder++, | |
| Date.now(), | |
| true, | |
| ); |
Comment on lines
+95
to
+101
| const toolRun = this.tracer.createToolRunStartInternal( | ||
| block, | ||
| this.rootRunId, | ||
| this.traceId, | ||
| this.executionOrder++, | ||
| this.startTime, | ||
| ); |
Contributor
There was a problem hiding this comment.
Similar to the LLM run, the start_time for this tool run is being set to this.startTime. This should be Date.now() to accurately capture when the tool run started.
Suggested change
| const toolRun = this.tracer.createToolRunStartInternal( | |
| block, | |
| this.rootRunId, | |
| this.traceId, | |
| this.executionOrder++, | |
| this.startTime, | |
| ); | |
| const toolRun = this.tracer.createToolRunStartInternal( | |
| block, | |
| this.rootRunId, | |
| this.traceId, | |
| this.executionOrder++, | |
| Date.now(), | |
| ); |
| if (block.type === 'tool_result') { | ||
| const toolRun = this.pendingToolUses.get(block.tool_use_id); | ||
| if (toolRun) { | ||
| this.tracer.completeToolRunInternal(toolRun, block, this.startTime); |
Contributor
There was a problem hiding this comment.
The end_time for the tool run is being set to this.startTime. It should be Date.now() to reflect when the tool result was processed.
Suggested change
| this.tracer.completeToolRunInternal(toolRun, block, this.startTime); | |
| this.tracer.completeToolRunInternal(toolRun, block, Date.now()); |
| // Complete any pending tool runs | ||
| for (const [toolUseId, toolRun] of this.pendingToolUses) { | ||
| this.tracer.logger.warn(`[ClaudeAgentTracer] Tool run ${toolUseId} did not receive result`); | ||
| toolRun.end_time = this.startTime; |
Contributor
There was a problem hiding this comment.
The end_time for this pending tool run is being set to this.startTime. It should be Date.now() to accurately reflect that it's ending now due to not receiving a result.
Suggested change
| toolRun.end_time = this.startTime; | |
| toolRun.end_time = Date.now(); |
| type: 'assistant', | ||
| uuid: msg.uuid, | ||
| session_id: msg.session_id, | ||
| message: msg.message as any, |
Contributor
There was a problem hiding this comment.
The type cast as any for msg.message is unnecessary and weakens type safety. The type of msg.message from SDKAssistantMessage is ClaudeMessageContent, which is compatible with the message property in the ClaudeMessage type. You can remove the cast. This applies to other uses of as any in this function as well, which could be refactored to use type guards for better type safety.
Suggested change
| message: msg.message as any, | |
| message: msg.message, |
Comment on lines
+116
to
+137
| FIELDS_TO_OSS.forEach((field) => { | ||
| if (!runData[field]) { | ||
| return; | ||
| } | ||
| const jsonstr = JSON.stringify(runData[field]); | ||
| if (field === 'outputs') { | ||
| (runData as any).cost = runData?.outputs?.llmOutput; | ||
| } | ||
| delete runData[field]; | ||
| const key = `agents/${name}/${env}/traces/${run.trace_id}/runs/${run.id}/${field}`; | ||
| this.backgroundTaskHelper.run(async () => { | ||
| try { | ||
| await this.uploadToOss(key, jsonstr); | ||
| } catch (e) { | ||
| this.logger.warn( | ||
| `[TraceLogErr] Failed to upload run data to OSS for run_id=${run.id}, field=${field}, error:`, | ||
| e, | ||
| ); | ||
| } | ||
| }); | ||
| (runData as any)[field] = { compress: 'none', key } as IResource; | ||
| }); |
Contributor
There was a problem hiding this comment.
The logic for processing fields to be uploaded to OSS and adding the cost field involves in-place modification of the runData object and uses as any, which can make it difficult to follow and less type-safe. Consider refactoring this to build a new, strongly-typed log object. This would separate the concerns of data transformation and logging, improving readability and maintainability.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #5822 +/- ##
==========================================
+ Coverage 85.31% 85.40% +0.09%
==========================================
Files 655 659 +4
Lines 12591 12781 +190
Branches 1447 1480 +33
==========================================
+ Hits 10742 10916 +174
- Misses 1729 1744 +15
- Partials 120 121 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (7)
tegg/core/agent-tracing/src/types.ts (3)
42-54: Consider replacinganywith more specific types orunknown.
ClaudeMessageContentusesanyin several places (context_management,container, and the index signature). While these may represent dynamic external SDK data, usinganyreduces type safety. Consider usingunknownor more specific types where possible.♻️ Proposed refinement
export interface ClaudeMessageContent { id?: string; type?: string; role?: string; content?: ClaudeContentBlock[]; model?: string; usage?: ClaudeTokenUsage; - context_management?: any; + context_management?: unknown; stop_reason?: string; stop_sequence?: string; - container?: any; - [key: string]: any; + container?: unknown; + [key: string]: unknown; }As per coding guidelines: "Avoid
anytype; useunknownwhen type is truly unknown in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/src/types.ts` around lines 42 - 54, The ClaudeMessageContent interface uses unsafe any types for context_management, container, and the index signature; change those to safer types (e.g., context_management: unknown, container: unknown) and replace the loose index signature [key: string]: any with a more specific form such as [key: string]: unknown or Record<string, unknown> to preserve flexibility while restoring type safety; update any consuming code to properly narrow these fields (type guards/casts) where necessary.
166-175: ThetracingService.configure({})call appears to be a no-op.The
applyTracerConfigfunction always passes an empty object totracingService.configure({}), which doesn't apply any configuration. IfAgentTracingConfigis reserved for future use, consider documenting this or deferring the call until actual config values are available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/src/types.ts` around lines 166 - 175, The call tracingService.configure({}) in applyTracerConfig is a no-op because it always passes an empty object; update applyTracerConfig to build and pass a proper AgentTracingConfig derived from the TracerConfig (e.g., include agentName and any other relevant fields) or only call tracingService.configure when there are actual config values to apply, and if AgentTracingConfig is intentionally empty, add a comment explaining it's reserved for future use; reference the applyTracerConfig function, the tracer.agentName assignment, the tracingService.configure method, TracerConfig param and AgentTracingConfig type when making the change.
69-110: Consider replacinganywithunknowninClaudeMessage.Similar to
ClaudeMessageContent, this interface usesanyforskills,plugins,permission_denials, and the index signature. These weaken type safety for downstream consumers.♻️ Proposed refinement
// System/init message fields cwd?: string; tools?: string[]; mcp_servers?: Array<{ name: string; status: string }>; model?: string; permissionMode?: string; slash_commands?: string[]; apiKeySource?: string; claude_code_version?: string; output_style?: string; agents?: string[]; - skills?: any[]; - plugins?: any[]; + skills?: unknown[]; + plugins?: unknown[]; // ... - permission_denials?: any[]; + permission_denials?: unknown[]; // ... - [key: string]: any; + [key: string]: unknown;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/src/types.ts` around lines 69 - 110, The ClaudeMessage interface uses wide any types (fields skills, plugins, permission_denials and the index signature), reducing type safety; change those to unknown (e.g., replace skills: any[] → skills: unknown[]; plugins: any[] → plugins: unknown[]; permission_denials?: any[] → permission_denials?: unknown[]; and the catch‑all [key: string]: any → [key: string]: unknown) and update any call sites that relied on implicit any to perform explicit type narrowing or casts where necessary (search for usages of ClaudeMessage, skills, plugins, permission_denials, and the index signature to add proper type guards or asserts).tegg/core/agent-tracing/vitest.config.ts (1)
1-9: Use a typed intermediate variable for isolatedDeclarations support.The config should use a typed intermediate variable rather than a direct inline export to support
isolatedDeclarations.♻️ Proposed fix
-import { defineProject } from 'vitest/config'; +import { defineProject, type UserProjectConfigExport } from 'vitest/config'; -export default defineProject({ +const config: UserProjectConfigExport = defineProject({ test: { testTimeout: 15000, include: ['test/**/*.test.ts'], exclude: ['**/test/fixtures/**', '**/node_modules/**', '**/dist/**'], }, }); + +export default config;As per coding guidelines: "Use typed intermediate variables for vitest config exports to support isolatedDeclarations".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/vitest.config.ts` around lines 1 - 9, Create a typed intermediate variable (e.g., const config: UserConfig or ProjectConfig) to hold the result of defineProject(...) instead of exporting the call inline; import the corresponding type via "import type { UserConfig } from 'vitest' (or the appropriate ProjectConfig type from 'vitest/config'), assign the object produced by defineProject(...) to that typed variable (config) and then export default config so isolatedDeclarations is supported; update references to defineProject and the default export accordingly.tegg/core/agent-tracing/test/LangGraphTracer.test.ts (1)
10-10: Replace fixed 500ms sleeps with deterministic async flushing.Repeated hard waits increase suite time and still risk timing flakiness. Prefer deterministic completion signals/microtask flushes from the mocked async path.
Also applies to: 58-58, 86-86, 117-117, 164-164, 195-195, 234-234, 265-265, 297-297
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/test/LangGraphTracer.test.ts` at line 10, The test defines a fixed-delay helper sleep and uses it (sleep(500)) in multiple places which causes slow/flaky tests; replace those hard waits by removing the sleep helper and instead awaiting a deterministic async flush or the mocked async completion signal (e.g., implement and call a flushPromises utility that returns new Promise(r => setImmediate(r)) or await the tracer/mockedEmitter's completion Promise), and update each assertion site that currently calls sleep(...) to await that deterministic flush (reference the sleep constant and the test blocks in LangGraphTracer.test.ts that call sleep).tegg/core/agent-tracing/test/TestUtils.ts (1)
21-24: Avoidanyin shared test helpers to keep type drift visible.The current
Promise<any>and(x as any)casts in utility code reduce the signal of type regressions across all tracer tests.♻️ Suggested typed refactor
+type MockBackgroundTaskHelper = { + run: <T>(fn: () => Promise<T>) => Promise<T>; +}; + -export function createMockBackgroundTaskHelper(): { run: (fn: () => Promise<any>) => Promise<any> } { +export function createMockBackgroundTaskHelper(): MockBackgroundTaskHelper { return { - run: async (fn: () => Promise<any>) => fn(), + run: async <T>(fn: () => Promise<T>) => fn(), }; } export function createMockTracingService(logs?: string[]): TracingService { const tracingService = new TracingService(); - (tracingService as any).logger = createMockLogger(logs); - (tracingService as any).backgroundTaskHelper = createMockBackgroundTaskHelper(); - (tracingService as any).ossClient = createMockOssClient(); - (tracingService as any).logServiceClient = createMockLogServiceClient(logs); + const mutableTracingService = tracingService as unknown as { + logger: Logger; + backgroundTaskHelper: MockBackgroundTaskHelper; + ossClient: IOssClient; + logServiceClient: ILogServiceClient; + }; + mutableTracingService.logger = createMockLogger(logs); + mutableTracingService.backgroundTaskHelper = createMockBackgroundTaskHelper(); + mutableTracingService.ossClient = createMockOssClient(); + mutableTracingService.logServiceClient = createMockLogServiceClient(logs); return tracingService; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/test/TestUtils.ts` around lines 21 - 24, createMockBackgroundTaskHelper currently uses Promise<any> which hides type regressions; make the helper generic by adding a type parameter (e.g., <T>) and change the run signature to accept and return Promise<T> so callers preserve precise types (update the function signature and the returned run method accordingly, referencing createMockBackgroundTaskHelper and its run property).tegg/core/agent-tracing/src/ClaudeAgentTracer.ts (1)
261-307: Reduceanycasts in message conversion/build paths.
convertSDKMessageand run-build helpers rely on multipleas anycasts, which weakens safety exactly where SDK payloads are mapped into tracing schema.As per coding guidelines, "Avoid `any` type; use `unknown` when type is truly unknown in TypeScript".Suggested direction
- message: msg.message as any, + message: msg.message as ClaudeMessage['message'], @@ - if (msg.type === 'user' && 'message' in msg && !('isReplay' in msg && (msg as any).isReplay)) { + if (msg.type === 'user' && 'message' in msg && !(msg as { isReplay?: boolean }).isReplay) { @@ - message: msg.message as any, - parent_tool_use_id: (msg as any).parent_tool_use_id, + message: msg.message as ClaudeMessage['message'], + parent_tool_use_id: (msg as { parent_tool_use_id?: string }).parent_tool_use_id, @@ - const outputs: any = {}; + const outputs: Record<string, unknown> = {}; @@ - const toolUse = toolUseBlock as any; + const toolUse = toolUseBlock as Extract<ClaudeContentBlock, { type: 'tool_use' }>;Also applies to: 366-381, 420-430
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/src/ClaudeAgentTracer.ts` around lines 261 - 307, The convertSDKMessage in ClaudeAgentTracer currently uses multiple "as any" casts (e.g., for message, result, errors, usage, modelUsage); replace these with proper narrowing: add type-guard functions (e.g., isSDKAssistantMessage, isSDKUserMessage, isSDKResultMessage) that validate fields on SDKMessage and narrow message/result/usage/modelUsage to concrete types, use unknown for initially unknown payloads and only cast after successful guards, and in the SDKResultMessage branch safely handle errors with Array.isArray(resultMsg.errors) before joining; update any corresponding run-build helpers to use the same guards and unknown->narrowing pattern instead of "as any".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tegg/core/agent-tracing/src/claude.ts`:
- Line 1: Update the ESM export to use a .js extension: change the export
statement that re-exports ClaudeAgentTracer and TraceSession from
'./ClaudeAgentTracer.ts' to import from './ClaudeAgentTracer.js' so the runtime
uses the correct ESM module filename for ClaudeAgentTracer and TraceSession.
In `@tegg/core/agent-tracing/src/ClaudeAgentTracer.ts`:
- Around line 81-118: Child/tool runs are being created with the session-wide
this.startTime causing zero-duration runs; update the places that call
this.tracer.createLLMRunInternal and this.tracer.createToolRunStartInternal (and
the code that logs their START/END via this.tracer.logTrace) to use a fresh
timestamp for each sub-run (e.g., compute a local start/end time like const now
= Date.now() or similar) instead of this.startTime so each child/tool run gets
correct start and end times; apply the same change in the other occurrences
referenced (the additional
createLLMRunInternal/createToolRunStartInternal/logTrace sites).
- Around line 9-18: The local ESM imports in ClaudeAgentTracer.ts are using .ts
extensions which breaks runtime resolution; update the import specifiers to use
.js instead: change the import for TracingService (import type { TracingService
} from './TracingService.ts') to './TracingService.js' and change the grouped
import that brings in ClaudeMessage, ClaudeContentBlock, ClaudeTokenUsage,
IRunCost, RunStatus, TracerConfig, and applyTracerConfig (currently from
'./types.ts') to './types.js' so all local ESM imports use .js extensions.
In `@tegg/core/agent-tracing/src/langgraph.ts`:
- Line 1: The export uses a TypeScript file extension for an ESM import; update
the export statement that re-exports LangGraphTracer (symbol: LangGraphTracer)
to reference the compiled JS module by changing the path from
'./LangGraphTracer.ts' to './LangGraphTracer.js' so runtime ESM imports resolve
correctly.
In `@tegg/core/agent-tracing/src/LangGraphTracer.ts`:
- Around line 6-7: Change the local ESM import specifiers in LangGraphTracer to
use .js extensions instead of .ts; update the import of TracingService (from
'./TracingService.ts') and the import of RunStatus, TracerConfig,
applyTracerConfig (from './types.ts') to reference './TracingService.js' and
'./types.js' respectively so runtime module resolution follows the project's ESM
rules.
In `@tegg/core/agent-tracing/src/TracingService.ts`:
- Around line 8-10: Update the ESM import specifiers in TracingService.ts to use
.js extensions: change imports that reference './ILogServiceClient.ts',
'./IOssClient.ts', and './types.ts' to './ILogServiceClient.js',
'./IOssClient.js', and './types.js' respectively so symbols like
ILogServiceClient, IOssClient, AgentTracingConfig, FIELDS_TO_OSS, IResource, and
RunStatus are imported with valid ESM file extensions.
- Around line 116-137: The code currently deletes runData[field] and replaces it
with an IResource key before the OSS upload completes, causing loss of payloads
when IOssClient is absent or upload fails; change the flow in the loop over
FIELDS_TO_OSS (where runData[field] is read, jsonstr created and uploadToOss is
invoked inside backgroundTaskHelper.run) to keep the original runData[field]
intact until upload succeeds: check whether an OSS client is configured, perform
uploadToOss first (or await the backgroundTaskHelper-run result) and only on
successful upload set (runData as any)[field] = { compress: 'none', key } as
IResource; on upload failure or missing client, leave runData[field] untouched
and log via this.logger.warn (as in the existing catch) so trace payloads are
preserved.
In `@tegg/core/agent-tracing/test/ClaudeAgentTracer.test.ts`:
- Around line 28-31: The test helper createTestEnv mutates process.env.FAAS_ENV
(and similarly at lines 56-57) without restoring it, risking cross-test
contamination; update createTestEnv to capture the original process.env.FAAS_ENV
(and any other env keys you change), set the env for the test, and return or
register a cleanup callback that restores the original values (or use
beforeEach/afterEach to save and restore) so the global env is not permanently
mutated by ClaudeAgentTracer.test runs.
- Around line 356-359: The test only asserts that rootEnd!.run exists but
doesn't verify the propagated error details; update the error-path test (near
assert(rootEnd!.run, 'Root end run should exist')) to assert that the traced run
contains the expected error payload (e.g., check rootEnd!.run.error or
rootEnd!.run.attributes/errorMessage) and that it matches the simulated error
used in the test (create/use an expectedError or expectedErrorMessage derived
from the test's simulated failure or convertSDKMessage). Ensure you reference
the same conversion path (convertSDKMessage) and assert equality/containment
(strictEqual/deepEqual) so the error details are validated.
In `@tegg/core/agent-tracing/test/LangGraphTracer.test.ts`:
- Line 5: The tests in LangGraphTracer.test.ts modify process.env.FAAS_ENV and
do not restore it; update the test suite to capture the original value (e.g.,
const originalFaasEnv = process.env.FAAS_ENV) before mutating and restore it in
an afterEach hook so each test resets process.env.FAAS_ENV; add an afterEach(()
=> { process.env.FAAS_ENV = originalFaasEnv }) alongside the existing
beforeEach/it blocks to prevent cross-test state leakage.
---
Nitpick comments:
In `@tegg/core/agent-tracing/src/ClaudeAgentTracer.ts`:
- Around line 261-307: The convertSDKMessage in ClaudeAgentTracer currently uses
multiple "as any" casts (e.g., for message, result, errors, usage, modelUsage);
replace these with proper narrowing: add type-guard functions (e.g.,
isSDKAssistantMessage, isSDKUserMessage, isSDKResultMessage) that validate
fields on SDKMessage and narrow message/result/usage/modelUsage to concrete
types, use unknown for initially unknown payloads and only cast after successful
guards, and in the SDKResultMessage branch safely handle errors with
Array.isArray(resultMsg.errors) before joining; update any corresponding
run-build helpers to use the same guards and unknown->narrowing pattern instead
of "as any".
In `@tegg/core/agent-tracing/src/types.ts`:
- Around line 42-54: The ClaudeMessageContent interface uses unsafe any types
for context_management, container, and the index signature; change those to
safer types (e.g., context_management: unknown, container: unknown) and replace
the loose index signature [key: string]: any with a more specific form such as
[key: string]: unknown or Record<string, unknown> to preserve flexibility while
restoring type safety; update any consuming code to properly narrow these fields
(type guards/casts) where necessary.
- Around line 166-175: The call tracingService.configure({}) in
applyTracerConfig is a no-op because it always passes an empty object; update
applyTracerConfig to build and pass a proper AgentTracingConfig derived from the
TracerConfig (e.g., include agentName and any other relevant fields) or only
call tracingService.configure when there are actual config values to apply, and
if AgentTracingConfig is intentionally empty, add a comment explaining it's
reserved for future use; reference the applyTracerConfig function, the
tracer.agentName assignment, the tracingService.configure method, TracerConfig
param and AgentTracingConfig type when making the change.
- Around line 69-110: The ClaudeMessage interface uses wide any types (fields
skills, plugins, permission_denials and the index signature), reducing type
safety; change those to unknown (e.g., replace skills: any[] → skills:
unknown[]; plugins: any[] → plugins: unknown[]; permission_denials?: any[] →
permission_denials?: unknown[]; and the catch‑all [key: string]: any → [key:
string]: unknown) and update any call sites that relied on implicit any to
perform explicit type narrowing or casts where necessary (search for usages of
ClaudeMessage, skills, plugins, permission_denials, and the index signature to
add proper type guards or asserts).
In `@tegg/core/agent-tracing/test/LangGraphTracer.test.ts`:
- Line 10: The test defines a fixed-delay helper sleep and uses it (sleep(500))
in multiple places which causes slow/flaky tests; replace those hard waits by
removing the sleep helper and instead awaiting a deterministic async flush or
the mocked async completion signal (e.g., implement and call a flushPromises
utility that returns new Promise(r => setImmediate(r)) or await the
tracer/mockedEmitter's completion Promise), and update each assertion site that
currently calls sleep(...) to await that deterministic flush (reference the
sleep constant and the test blocks in LangGraphTracer.test.ts that call sleep).
In `@tegg/core/agent-tracing/test/TestUtils.ts`:
- Around line 21-24: createMockBackgroundTaskHelper currently uses Promise<any>
which hides type regressions; make the helper generic by adding a type parameter
(e.g., <T>) and change the run signature to accept and return Promise<T> so
callers preserve precise types (update the function signature and the returned
run method accordingly, referencing createMockBackgroundTaskHelper and its run
property).
In `@tegg/core/agent-tracing/vitest.config.ts`:
- Around line 1-9: Create a typed intermediate variable (e.g., const config:
UserConfig or ProjectConfig) to hold the result of defineProject(...) instead of
exporting the call inline; import the corresponding type via "import type {
UserConfig } from 'vitest' (or the appropriate ProjectConfig type from
'vitest/config'), assign the object produced by defineProject(...) to that typed
variable (config) and then export default config so isolatedDeclarations is
supported; update references to defineProject and the default export
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c9d966a-62a9-4e20-93a2-9f07d03a41c7
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
tegg/core/agent-tracing/package.jsontegg/core/agent-tracing/src/ClaudeAgentTracer.tstegg/core/agent-tracing/src/ILogServiceClient.tstegg/core/agent-tracing/src/IOssClient.tstegg/core/agent-tracing/src/LangGraphTracer.tstegg/core/agent-tracing/src/TracingService.tstegg/core/agent-tracing/src/claude.tstegg/core/agent-tracing/src/index.tstegg/core/agent-tracing/src/langgraph.tstegg/core/agent-tracing/src/types.tstegg/core/agent-tracing/test/ClaudeAgentTracer.test.tstegg/core/agent-tracing/test/Configure.test.tstegg/core/agent-tracing/test/LangGraphTracer.test.tstegg/core/agent-tracing/test/TestUtils.tstegg/core/agent-tracing/tsconfig.jsontegg/core/agent-tracing/vitest.config.tstegg/core/langchain-decorator/package.jsontegg/plugin/langchain/package.jsontsconfig.json
| @@ -0,0 +1 @@ | |||
| export { ClaudeAgentTracer, TraceSession } from './ClaudeAgentTracer.ts'; | |||
Contributor
There was a problem hiding this comment.
Use .js extension instead of .ts for ESM import.
Same as langgraph.ts, the import should use .js extension for ESM runtime compatibility.
🔧 Proposed fix
-export { ClaudeAgentTracer, TraceSession } from './ClaudeAgentTracer.ts';
+export { ClaudeAgentTracer, TraceSession } from './ClaudeAgentTracer.js';As per coding guidelines: "All imports should use .js extensions for ESM files".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export { ClaudeAgentTracer, TraceSession } from './ClaudeAgentTracer.ts'; | |
| export { ClaudeAgentTracer, TraceSession } from './ClaudeAgentTracer.js'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/core/agent-tracing/src/claude.ts` at line 1, Update the ESM export to
use a .js extension: change the export statement that re-exports
ClaudeAgentTracer and TraceSession from './ClaudeAgentTracer.ts' to import from
'./ClaudeAgentTracer.js' so the runtime uses the correct ESM module filename for
ClaudeAgentTracer and TraceSession.
Comment on lines
+9
to
+18
| import type { TracingService } from './TracingService.ts'; | ||
| import { | ||
| type ClaudeMessage, | ||
| type ClaudeContentBlock, | ||
| type ClaudeTokenUsage, | ||
| type IRunCost, | ||
| RunStatus, | ||
| type TracerConfig, | ||
| applyTracerConfig, | ||
| } from './types.ts'; |
Contributor
There was a problem hiding this comment.
Use .js extensions for local ESM imports in this tegg module.
These imports currently use .ts specifiers, which violates the module’s ESM import convention and can break runtime resolution in emitted JS.
Suggested fix
-import type { TracingService } from './TracingService.ts';
+import type { TracingService } from './TracingService.js';
import {
type ClaudeMessage,
type ClaudeContentBlock,
type ClaudeTokenUsage,
type IRunCost,
RunStatus,
type TracerConfig,
applyTracerConfig,
-} from './types.ts';
+} from './types.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import type { TracingService } from './TracingService.ts'; | |
| import { | |
| type ClaudeMessage, | |
| type ClaudeContentBlock, | |
| type ClaudeTokenUsage, | |
| type IRunCost, | |
| RunStatus, | |
| type TracerConfig, | |
| applyTracerConfig, | |
| } from './types.ts'; | |
| import type { TracingService } from './TracingService.js'; | |
| import { | |
| type ClaudeMessage, | |
| type ClaudeContentBlock, | |
| type ClaudeTokenUsage, | |
| type IRunCost, | |
| RunStatus, | |
| type TracerConfig, | |
| applyTracerConfig, | |
| } from './types.js'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/core/agent-tracing/src/ClaudeAgentTracer.ts` around lines 9 - 18, The
local ESM imports in ClaudeAgentTracer.ts are using .ts extensions which breaks
runtime resolution; update the import specifiers to use .js instead: change the
import for TracingService (import type { TracingService } from
'./TracingService.ts') to './TracingService.js' and change the grouped import
that brings in ClaudeMessage, ClaudeContentBlock, ClaudeTokenUsage, IRunCost,
RunStatus, TracerConfig, and applyTracerConfig (currently from './types.ts') to
'./types.js' so all local ESM imports use .js extensions.
| @@ -0,0 +1 @@ | |||
| export { LangGraphTracer } from './LangGraphTracer.ts'; | |||
Contributor
There was a problem hiding this comment.
Use .js extension instead of .ts for ESM import.
The import path uses .ts extension, but ESM imports should use .js extensions for runtime compatibility when TypeScript compiles to JavaScript.
🔧 Proposed fix
-export { LangGraphTracer } from './LangGraphTracer.ts';
+export { LangGraphTracer } from './LangGraphTracer.js';As per coding guidelines: "All imports should use .js extensions for ESM files".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export { LangGraphTracer } from './LangGraphTracer.ts'; | |
| export { LangGraphTracer } from './LangGraphTracer.js'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/core/agent-tracing/src/langgraph.ts` at line 1, The export uses a
TypeScript file extension for an ESM import; update the export statement that
re-exports LangGraphTracer (symbol: LangGraphTracer) to reference the compiled
JS module by changing the path from './LangGraphTracer.ts' to
'./LangGraphTracer.js' so runtime ESM imports resolve correctly.
Comment on lines
+6
to
+7
| import type { TracingService } from './TracingService.ts'; | ||
| import { RunStatus, type TracerConfig, applyTracerConfig } from './types.ts'; |
Contributor
There was a problem hiding this comment.
Use .js specifiers for local ESM imports.
These local imports use .ts extensions, which is inconsistent with tegg ESM import rules and can break runtime module resolution after transpilation.
Suggested fix
-import type { TracingService } from './TracingService.ts';
-import { RunStatus, type TracerConfig, applyTracerConfig } from './types.ts';
+import type { TracingService } from './TracingService.js';
+import { RunStatus, type TracerConfig, applyTracerConfig } from './types.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import type { TracingService } from './TracingService.ts'; | |
| import { RunStatus, type TracerConfig, applyTracerConfig } from './types.ts'; | |
| import type { TracingService } from './TracingService.js'; | |
| import { RunStatus, type TracerConfig, applyTracerConfig } from './types.js'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/core/agent-tracing/src/LangGraphTracer.ts` around lines 6 - 7, Change
the local ESM import specifiers in LangGraphTracer to use .js extensions instead
of .ts; update the import of TracingService (from './TracingService.ts') and the
import of RunStatus, TracerConfig, applyTracerConfig (from './types.ts') to
reference './TracingService.js' and './types.js' respectively so runtime module
resolution follows the project's ESM rules.
Comment on lines
+8
to
+10
| import { ILogServiceClient } from './ILogServiceClient.ts'; | ||
| import { IOssClient } from './IOssClient.ts'; | ||
| import { type AgentTracingConfig, FIELDS_TO_OSS, type IResource, RunStatus } from './types.ts'; |
Contributor
There was a problem hiding this comment.
Switch local ESM imports to .js extensions.
Current .ts specifiers violate tegg ESM import rules and can cause runtime import resolution issues in built output.
Suggested fix
-import { ILogServiceClient } from './ILogServiceClient.ts';
-import { IOssClient } from './IOssClient.ts';
-import { type AgentTracingConfig, FIELDS_TO_OSS, type IResource, RunStatus } from './types.ts';
+import { ILogServiceClient } from './ILogServiceClient.js';
+import { IOssClient } from './IOssClient.js';
+import { type AgentTracingConfig, FIELDS_TO_OSS, type IResource, RunStatus } from './types.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { ILogServiceClient } from './ILogServiceClient.ts'; | |
| import { IOssClient } from './IOssClient.ts'; | |
| import { type AgentTracingConfig, FIELDS_TO_OSS, type IResource, RunStatus } from './types.ts'; | |
| import { ILogServiceClient } from './ILogServiceClient.js'; | |
| import { IOssClient } from './IOssClient.js'; | |
| import { type AgentTracingConfig, FIELDS_TO_OSS, type IResource, RunStatus } from './types.js'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/core/agent-tracing/src/TracingService.ts` around lines 8 - 10, Update
the ESM import specifiers in TracingService.ts to use .js extensions: change
imports that reference './ILogServiceClient.ts', './IOssClient.ts', and
'./types.ts' to './ILogServiceClient.js', './IOssClient.js', and './types.js'
respectively so symbols like ILogServiceClient, IOssClient, AgentTracingConfig,
FIELDS_TO_OSS, IResource, and RunStatus are imported with valid ESM file
extensions.
Comment on lines
+116
to
+137
| FIELDS_TO_OSS.forEach((field) => { | ||
| if (!runData[field]) { | ||
| return; | ||
| } | ||
| const jsonstr = JSON.stringify(runData[field]); | ||
| if (field === 'outputs') { | ||
| (runData as any).cost = runData?.outputs?.llmOutput; | ||
| } | ||
| delete runData[field]; | ||
| const key = `agents/${name}/${env}/traces/${run.trace_id}/runs/${run.id}/${field}`; | ||
| this.backgroundTaskHelper.run(async () => { | ||
| try { | ||
| await this.uploadToOss(key, jsonstr); | ||
| } catch (e) { | ||
| this.logger.warn( | ||
| `[TraceLogErr] Failed to upload run data to OSS for run_id=${run.id}, field=${field}, error:`, | ||
| e, | ||
| ); | ||
| } | ||
| }); | ||
| (runData as any)[field] = { compress: 'none', key } as IResource; | ||
| }); |
Contributor
There was a problem hiding this comment.
Avoid replacing trace fields with OSS keys when OSS is unavailable.
The current flow deletes runData[field] and replaces it with a resource key before upload completion. If IOssClient is not configured (or upload fails), detailed trace payloads are lost from logs.
Suggested fix
FIELDS_TO_OSS.forEach((field) => {
if (!runData[field]) {
return;
}
- const jsonstr = JSON.stringify(runData[field]);
+ if (!this.ossClient) {
+ // Keep inline payload when OSS is unavailable to avoid data loss.
+ return;
+ }
+ const jsonStr = JSON.stringify(runData[field]);
if (field === 'outputs') {
(runData as any).cost = runData?.outputs?.llmOutput;
}
delete runData[field];
const key = `agents/${name}/${env}/traces/${run.trace_id}/runs/${run.id}/${field}`;
this.backgroundTaskHelper.run(async () => {
try {
- await this.uploadToOss(key, jsonstr);
+ await this.uploadToOss(key, jsonStr);
} catch (e) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| FIELDS_TO_OSS.forEach((field) => { | |
| if (!runData[field]) { | |
| return; | |
| } | |
| const jsonstr = JSON.stringify(runData[field]); | |
| if (field === 'outputs') { | |
| (runData as any).cost = runData?.outputs?.llmOutput; | |
| } | |
| delete runData[field]; | |
| const key = `agents/${name}/${env}/traces/${run.trace_id}/runs/${run.id}/${field}`; | |
| this.backgroundTaskHelper.run(async () => { | |
| try { | |
| await this.uploadToOss(key, jsonstr); | |
| } catch (e) { | |
| this.logger.warn( | |
| `[TraceLogErr] Failed to upload run data to OSS for run_id=${run.id}, field=${field}, error:`, | |
| e, | |
| ); | |
| } | |
| }); | |
| (runData as any)[field] = { compress: 'none', key } as IResource; | |
| }); | |
| FIELDS_TO_OSS.forEach((field) => { | |
| if (!runData[field]) { | |
| return; | |
| } | |
| if (!this.ossClient) { | |
| // Keep inline payload when OSS is unavailable to avoid data loss. | |
| return; | |
| } | |
| const jsonStr = JSON.stringify(runData[field]); | |
| if (field === 'outputs') { | |
| (runData as any).cost = runData?.outputs?.llmOutput; | |
| } | |
| delete runData[field]; | |
| const key = `agents/${name}/${env}/traces/${run.trace_id}/runs/${run.id}/${field}`; | |
| this.backgroundTaskHelper.run(async () => { | |
| try { | |
| await this.uploadToOss(key, jsonStr); | |
| } catch (e) { | |
| this.logger.warn( | |
| `[TraceLogErr] Failed to upload run data to OSS for run_id=${run.id}, field=${field}, error:`, | |
| e, | |
| ); | |
| } | |
| }); | |
| (runData as any)[field] = { compress: 'none', key } as IResource; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/core/agent-tracing/src/TracingService.ts` around lines 116 - 137, The
code currently deletes runData[field] and replaces it with an IResource key
before the OSS upload completes, causing loss of payloads when IOssClient is
absent or upload fails; change the flow in the loop over FIELDS_TO_OSS (where
runData[field] is read, jsonstr created and uploadToOss is invoked inside
backgroundTaskHelper.run) to keep the original runData[field] intact until
upload succeeds: check whether an OSS client is configured, perform uploadToOss
first (or await the backgroundTaskHelper-run result) and only on successful
upload set (runData as any)[field] = { compress: 'none', key } as IResource; on
upload failure or missing client, leave runData[field] untouched and log via
this.logger.warn (as in the existing catch) so trace payloads are preserved.
Comment on lines
+28
to
+31
| function createTestEnv() { | ||
| const logs: string[] = []; | ||
| process.env.FAAS_ENV = 'dev'; | ||
|
|
Contributor
There was a problem hiding this comment.
createTestEnv() mutates global env without cleanup.
process.env.FAAS_ENV is set globally and never restored, which can contaminate other tests and produce non-local failures.
Also applies to: 56-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/core/agent-tracing/test/ClaudeAgentTracer.test.ts` around lines 28 - 31,
The test helper createTestEnv mutates process.env.FAAS_ENV (and similarly at
lines 56-57) without restoring it, risking cross-test contamination; update
createTestEnv to capture the original process.env.FAAS_ENV (and any other env
keys you change), set the env for the test, and return or register a cleanup
callback that restores the original values (or use beforeEach/afterEach to save
and restore) so the global env is not permanently mutated by
ClaudeAgentTracer.test runs.
Comment on lines
+356
to
+359
| // The root run should have error field populated | ||
| // (error is extracted from result message via convertSDKMessage) | ||
| assert(rootEnd!.run, 'Root end run should exist'); | ||
| }); |
Contributor
There was a problem hiding this comment.
Assert the actual error payload in the error-path test.
The current check only verifies run existence; it does not validate that error details were propagated into the traced run.
✅ Suggested assertion strengthening
// The root run should have error field populated
// (error is extracted from result message via convertSDKMessage)
assert(rootEnd!.run, 'Root end run should exist');
+ assert(rootEnd!.run.error, 'Root end run should include error details');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // The root run should have error field populated | |
| // (error is extracted from result message via convertSDKMessage) | |
| assert(rootEnd!.run, 'Root end run should exist'); | |
| }); | |
| // The root run should have error field populated | |
| // (error is extracted from result message via convertSDKMessage) | |
| assert(rootEnd!.run, 'Root end run should exist'); | |
| assert(rootEnd!.run.error, 'Root end run should include error details'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/core/agent-tracing/test/ClaudeAgentTracer.test.ts` around lines 356 -
359, The test only asserts that rootEnd!.run exists but doesn't verify the
propagated error details; update the error-path test (near assert(rootEnd!.run,
'Root end run should exist')) to assert that the traced run contains the
expected error payload (e.g., check rootEnd!.run.error or
rootEnd!.run.attributes/errorMessage) and that it matches the simulated error
used in the test (create/use an expectedError or expectedErrorMessage derived
from the test's simulated failure or convertSDKMessage). Ensure you reference
the same conversion path (convertSDKMessage) and assert equality/containment
(strictEqual/deepEqual) so the error details are validated.
|
|
||
| import { FakeLLM } from '@langchain/core/utils/testing'; | ||
| import { StateGraph, Annotation, START, END } from '@langchain/langgraph'; | ||
| import { describe, it, beforeEach } from 'vitest'; |
Contributor
There was a problem hiding this comment.
Restore process.env.FAAS_ENV after each test to avoid cross-test state leakage.
This suite mutates a global process env value and does not reset it, which can affect unrelated tests running in the same worker.
🛠️ Suggested fix
-import { describe, it, beforeEach } from 'vitest';
+import { describe, it, beforeEach, afterEach } from 'vitest';
@@
describe('test/LangGraphTracer.test.ts', () => {
let tracer: LangGraphTracer;
let logs: string[] = [];
+ let previousFaasEnv: string | undefined;
beforeEach(() => {
logs = [];
+ previousFaasEnv = process.env.FAAS_ENV;
process.env.FAAS_ENV = 'dev';
@@
tracer = new LangGraphTracer();
(tracer as any).tracingService = tracingService;
});
+
+ afterEach(() => {
+ if (previousFaasEnv === undefined) {
+ delete process.env.FAAS_ENV;
+ } else {
+ process.env.FAAS_ENV = previousFaasEnv;
+ }
+ });Also applies to: 31-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/core/agent-tracing/test/LangGraphTracer.test.ts` at line 5, The tests in
LangGraphTracer.test.ts modify process.env.FAAS_ENV and do not restore it;
update the test suite to capture the original value (e.g., const originalFaasEnv
= process.env.FAAS_ENV) before mutating and restore it in an afterEach hook so
each test resets process.env.FAAS_ENV; add an afterEach(() => {
process.env.FAAS_ENV = originalFaasEnv }) alongside the existing beforeEach/it
blocks to prevent cross-test state leakage.
- Use ES private fields (#) for TraceSession instead of private keyword - Remove unused OssConfig interface from types.ts - Inline createLLMOutput into extractTokenUsage call site - Switch test assertions to node:assert/strict - Refactor test utils: replace log-string parsing with direct Run capture (createCapturingTracingService replaces createMockTracingService) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hTracer Add comprehensive TracingService tests covering getEnv, isOnlineEnv, getLogInfoPrefix, uploadToOss, syncLocalToLogService, and logTrace. Add direct hook invocation tests in LangGraphTracer for tool, LLM, retriever and agent callbacks to reach ~89% overall coverage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolve conflict in tsconfig.json by including both agent-tracing and agent-runtime package references. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jerryliang64
commented
Mar 6, 2026
| @InjectOptional() | ||
| private readonly logServiceClient: ILogServiceClient; | ||
|
|
||
| configure(_config: AgentTracingConfig): void { |
Contributor
Author
There was a problem hiding this comment.
这里当前如果用不到，为什么要保留
…ession Child runs (LLM, tool) were incorrectly using the session start time (#startTime) instead of the actual event time. This caused all child spans to have zero duration and be anchored to session creation, making trace timeline analysis inaccurate. - handleAssistant: capture eventTime = Date.now() per message - handleUser: use Date.now() when completing tool runs - handleResult: use Date.now() for orphaned pending tool runs - Root run end_time (#startTime + duration_ms) remains unchanged Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
tegg/core/agent-tracing/test/ClaudeAgentTracer.test.ts (2)
12-23:⚠️ Potential issue | 🟠 MajorRestore
FAAS_ENVin this suite.
createTestEnv()mutates global env and never cleans it up, so later tests can inheritdevunexpectedly.Proposed fix
-import { describe, it } from 'vitest'; +import { afterEach, beforeEach, describe, it } from 'vitest'; @@ describe('test/ClaudeAgentTracer.test.ts', () => { + let originalFaasEnv: string | undefined; + + beforeEach(() => { + originalFaasEnv = process.env.FAAS_ENV; + }); + + afterEach(() => { + if (originalFaasEnv === undefined) { + delete process.env.FAAS_ENV; + } else { + process.env.FAAS_ENV = originalFaasEnv; + } + }); + describe('Streaming mode + tool use', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/test/ClaudeAgentTracer.test.ts` around lines 12 - 23, The test helper createTestEnv() changes process.env.FAAS_ENV to 'dev' but never restores it; capture the original value (e.g., const prev = process.env.FAAS_ENV) before setting it, set process.env.FAAS_ENV = 'dev' for the test, and ensure you restore the original value after the test suite (use an afterEach/afterAll hook or return a cleanup function from createTestEnv to reset process.env.FAAS_ENV = prev). Update the test file so any code that calls createTestEnv() also invokes the cleanup or relies on the suite-level teardown to restore FAAS_ENV.
313-317:⚠️ Potential issue | 🟡 MinorAssert the propagated error payload too.
This still passes if the tracer emits
RunStatus.ERRORbut drops the actual error details. Please verify the traced run carries the simulated failure message as well.Suggested assertion strengthening
const rootError = capturedRuns.find((e) => !e.run.parent_run_id && e.status === RunStatus.ERROR); assert(rootError, 'Should have root_run with error status'); assert(rootError.run, 'Root error run should exist'); + assert(rootError.run.error, 'Root error run should include error details'); + assert.match(String(rootError.run.error), /Something went wrong/);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/test/ClaudeAgentTracer.test.ts` around lines 313 - 317, The test currently only checks for a root run with RunStatus.ERROR but not that the actual error details were propagated; update the assertion after finding capturedRuns/rootError to also verify the traced run carries the simulated failure message by asserting the rootError.run error payload contains that message (e.g. check rootError.run.error.message or rootError.run.error_message or the specific error field your tracer uses includes the simulated failure string defined earlier in the test). Ensure you reference capturedRuns, the rootError variable and the same simulated failure message constant/string used in this test when adding the assertion.tegg/core/agent-tracing/test/LangGraphTracer.test.ts (1)
46-54:⚠️ Potential issue | 🟠 MajorRestore
FAAS_ENVafter each test.Line 47 mutates process-wide state, and this suite never puts it back. That makes env-dependent tests order-sensitive across the worker.
Proposed fix
-import { describe, it, beforeEach } from 'vitest'; +import { afterEach, beforeEach, describe, it } from 'vitest'; @@ describe('test/LangGraphTracer.test.ts', () => { let tracer: LangGraphTracer; let capturedRuns: CapturedEntry[]; + let originalFaasEnv: string | undefined; beforeEach(() => { + originalFaasEnv = process.env.FAAS_ENV; process.env.FAAS_ENV = 'dev'; @@ tracer = new LangGraphTracer(); (tracer as any).tracingService = capturing.tracingService; }); + + afterEach(() => { + if (originalFaasEnv === undefined) { + delete process.env.FAAS_ENV; + } else { + process.env.FAAS_ENV = originalFaasEnv; + } + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/test/LangGraphTracer.test.ts` around lines 46 - 54, The beforeEach mutates process.env.FAAS_ENV and never restores it; capture the original value (e.g., const _origFaasEnv = process.env.FAAS_ENV) before setting it to 'dev' in beforeEach and add an afterEach that restores it (if _origFaasEnv is undefined delete process.env.FAAS_ENV else set process.env.FAAS_ENV = _origFaasEnv) so tests using beforeEach in LangGraphTracer.test.ts (and the beforeEach symbol) do not leak env state across tests.
🧹 Nitpick comments (1)
tegg/core/agent-tracing/test/LangGraphTracer.test.ts (1)
34-34: Drop the fixed 500ms waits from these graph tests.With
createCapturingTracingService(),logTrace()appends tocapturedRunssynchronously, so these sleeps do not flush anything real. They just add several seconds to the suite and make failures slower to diagnose.Also applies to: 68-68, 92-92, 120-120, 158-158, 184-184, 212-212, 290-290, 316-316
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/test/LangGraphTracer.test.ts` at line 34, Remove the fixed 500ms sleeps used in LangGraphTracer tests: since createCapturingTracingService()’s logTrace() appends to capturedRuns synchronously, delete the sleep helper (const sleep = ...) and all await sleep(...) calls in the test cases (references around lines where tests call sleep) and instead assert directly against capturedRuns after invoking the traced operation; ensure you still createCapturingTracingService(), call the traced method, then immediately check capturedRuns for expected entries rather than waiting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tegg/core/agent-tracing/src/types.ts`:
- Line 14: Replace all occurrences of broad any in the exported Claude message
type shapes (e.g., the input?: Record<string, any> field and the other
open-ended fields at the referenced ranges 49-53, 87-89, 99-109) with a safer
type such as Record<string, unknown> or a named extension generic (e.g.,
Extension = Record<string, unknown> or TExtension = Record<string, unknown>) so
downstream consumers still get type-checking; update the exported type
declarations (the message interfaces/types in types.ts) to use unknown or a
generic extension parameter and propagate that change to the related fields
(input, metadata/extra/other open fields) so the public API no longer uses any.
- Around line 148-150: Remove the empty interface AgentTracingConfig and replace
it with a clearer placeholder type alias to avoid collapsing to {}: change the
declaration for AgentTracingConfig in types.ts to use a type alias like type
AgentTracingConfig = Record<string, never> so static analysis treats it as an
intentionally empty config until fields are added; update any imports/usages
referencing AgentTracingConfig accordingly to accept the new type alias.
---
Duplicate comments:
In `@tegg/core/agent-tracing/test/ClaudeAgentTracer.test.ts`:
- Around line 12-23: The test helper createTestEnv() changes
process.env.FAAS_ENV to 'dev' but never restores it; capture the original value
(e.g., const prev = process.env.FAAS_ENV) before setting it, set
process.env.FAAS_ENV = 'dev' for the test, and ensure you restore the original
value after the test suite (use an afterEach/afterAll hook or return a cleanup
function from createTestEnv to reset process.env.FAAS_ENV = prev). Update the
test file so any code that calls createTestEnv() also invokes the cleanup or
relies on the suite-level teardown to restore FAAS_ENV.
- Around line 313-317: The test currently only checks for a root run with
RunStatus.ERROR but not that the actual error details were propagated; update
the assertion after finding capturedRuns/rootError to also verify the traced run
carries the simulated failure message by asserting the rootError.run error
payload contains that message (e.g. check rootError.run.error.message or
rootError.run.error_message or the specific error field your tracer uses
includes the simulated failure string defined earlier in the test). Ensure you
reference capturedRuns, the rootError variable and the same simulated failure
message constant/string used in this test when adding the assertion.
In `@tegg/core/agent-tracing/test/LangGraphTracer.test.ts`:
- Around line 46-54: The beforeEach mutates process.env.FAAS_ENV and never
restores it; capture the original value (e.g., const _origFaasEnv =
process.env.FAAS_ENV) before setting it to 'dev' in beforeEach and add an
afterEach that restores it (if _origFaasEnv is undefined delete
process.env.FAAS_ENV else set process.env.FAAS_ENV = _origFaasEnv) so tests
using beforeEach in LangGraphTracer.test.ts (and the beforeEach symbol) do not
leak env state across tests.
---
Nitpick comments:
In `@tegg/core/agent-tracing/test/LangGraphTracer.test.ts`:
- Line 34: Remove the fixed 500ms sleeps used in LangGraphTracer tests: since
createCapturingTracingService()’s logTrace() appends to capturedRuns
synchronously, delete the sleep helper (const sleep = ...) and all await
sleep(...) calls in the test cases (references around lines where tests call
sleep) and instead assert directly against capturedRuns after invoking the
traced operation; ensure you still createCapturingTracingService(), call the
traced method, then immediately check capturedRuns for expected entries rather
than waiting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca9c9dec-2625-4ed7-9637-e74261f83ad1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
tegg/core/agent-tracing/src/ClaudeAgentTracer.tstegg/core/agent-tracing/src/types.tstegg/core/agent-tracing/test/ClaudeAgentTracer.test.tstegg/core/agent-tracing/test/Configure.test.tstegg/core/agent-tracing/test/LangGraphTracer.test.tstegg/core/agent-tracing/test/TestUtils.tstegg/core/agent-tracing/test/TracingService.test.tstsconfig.json
🚧 Files skipped from review as they are similar to previous changes (4)
- tsconfig.json
- tegg/core/agent-tracing/test/TestUtils.ts
- tegg/core/agent-tracing/test/Configure.test.ts
- tegg/core/agent-tracing/src/ClaudeAgentTracer.ts
| type: 'tool_use'; | ||
| id: string; | ||
| name: string; | ||
| input?: Record<string, any>; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any in the exported Claude message types.
These are public package types, so any and [key: string]: any effectively turn off checking for downstream consumers. Use unknown or dedicated extension shapes for the open-ended fields instead.
Possible tightening
export interface ClaudeToolUseContent {
type: 'tool_use';
id: string;
name: string;
- input?: Record<string, any>;
+ input?: Record<string, unknown>;
}
@@
- context_management?: any;
+ context_management?: unknown;
@@
- container?: any;
- [key: string]: any;
+ container?: unknown;
+ [key: string]: unknown;
@@
- skills?: any[];
- plugins?: any[];
+ skills?: unknown[];
+ plugins?: unknown[];
@@
- permission_denials?: any[];
+ permission_denials?: unknown[];
@@
- [key: string]: any;
+ [key: string]: unknown;As per coding guidelines, "Avoid any type; use unknown when type is truly unknown in TypeScript".
Also applies to: 49-53, 87-89, 99-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/core/agent-tracing/src/types.ts` at line 14, Replace all occurrences of
broad any in the exported Claude message type shapes (e.g., the input?:
Record<string, any> field and the other open-ended fields at the referenced
ranges 49-53, 87-89, 99-109) with a safer type such as Record<string, unknown>
or a named extension generic (e.g., Extension = Record<string, unknown> or
TExtension = Record<string, unknown>) so downstream consumers still get
type-checking; update the exported type declarations (the message
interfaces/types in types.ts) to use unknown or a generic extension parameter
and propagate that change to the related fields (input, metadata/extra/other
open fields) so the public API no longer uses any.
Comment on lines
+148
to
+150
| export interface AgentTracingConfig { | ||
| // Reserved for future configuration options | ||
| } |
Contributor
There was a problem hiding this comment.
Remove the empty AgentTracingConfig interface.
This currently collapses to {} and static analysis is already flagging it. If there are no internal options yet, Record<string, never> is a clearer placeholder until real fields land.
Possible fix
-export interface AgentTracingConfig {
- // Reserved for future configuration options
-}
+export type AgentTracingConfig = Record<string, never>;🧰 Tools
🪛 Biome (2.4.4)
[error] 148-150: An empty interface is equivalent to {}.
(lint/suspicious/noEmptyInterface)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/core/agent-tracing/src/types.ts` around lines 148 - 150, Remove the
empty interface AgentTracingConfig and replace it with a clearer placeholder
type alias to avoid collapsing to {}: change the declaration for
AgentTracingConfig in types.ts to use a type alias like type AgentTracingConfig
= Record<string, never> so static analysis treats it as an intentionally empty
config until fields are added; update any imports/usages referencing
AgentTracingConfig accordingly to accept the new type alias.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@eggjs/agent-tracingpackage for AI agent LLM call tracingali-ossdependency withIOssClientIoC injection for better testabilitylogServiceconfig withILogServiceClientIoC injection following DI best practicesChanges
packages/agent-tracing/— New package implementing AI agent tracingIOssClient) for uploading trace dataILogServiceClient) for structured loggingTest plan
pnpm --filter=@eggjs/agent-tracing run test🤖 Generated with Claude Code
Summary by CodeRabbit