feat(agent-tracing): add @eggjs/agent-tracing package for AI agent tracing#5822
Open
jerryliang64 wants to merge 16 commits intonextfrom
Open
feat(agent-tracing): add @eggjs/agent-tracing package for AI agent tracing#5822jerryliang64 wants to merge 16 commits intonextfrom
jerryliang64 wants to merge 16 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
|
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:
📝 WalkthroughWalkthroughIntroduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ClaudeAgentTracer
participant TraceSession
participant Run Tree as Run Tree Building
participant TracingService
participant OSS
participant LogService
Client->>ClaudeAgentTracer: processMessages(sdkMessages[])
ClaudeAgentTracer->>TraceSession: createSession()
activate TraceSession
Client->>TraceSession: processMessage(init)
TraceSession->>Run Tree: createRootRun
Run Tree->>TracingService: logTrace(rootRun, START)
TracingService->>TracingService: filterHiddenFields
activate TracingService
Client->>TraceSession: processMessage(assistant + tool)
TraceSession->>Run Tree: createLLMRun<br/>createToolRun(START)
TracingService->>OSS: uploadToOss(key, largeField)
TracingService->>LogService: syncLocalToLogService(log)
Client->>TraceSession: processMessage(tool_result)
TraceSession->>Run Tree: completeToolRun
TracingService->>TracingService: replaceFieldsWithResources
Client->>TraceSession: processMessage(result)
TraceSession->>Run Tree: finalizeRootRun<br/>attachCosts
TracingService->>TracingService: extractTokenUsage<br/>createRunCost
TracingService->>TracingService: logTrace(rootRun, END)
deactivate TracingService
deactivate TraceSession
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
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: |
9f5ccc8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://51782b11.egg-v3.pages.dev |
| Branch Preview URL: | https://feat-agent-tracing.egg-v3.pages.dev |
Deploying egg with
|
| Latest commit: |
9f5ccc8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1fd38772.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.39% 85.58% +0.19%
==========================================
Files 659 663 +4
Lines 12813 13003 +190
Branches 1474 1508 +34
==========================================
+ Hits 10942 11129 +187
- Misses 1751 1753 +2
- 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
+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.
- 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.
tegg/core/agent-tracing/src/types.ts
Outdated
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.
jerryliang64
commented
Mar 9, 2026
| * If no implementation is registered, log service syncing is silently skipped. | ||
| */ | ||
| export abstract class ILogServiceClient { | ||
| abstract send(log: string): Promise<void>; |
Contributor
Author
There was a problem hiding this comment.
Ixxx 是否为当前项目的 interface 命名风格？
…ngService Add missing test cases to cover uncovered code paths: ClaudeAgentTracer: - Guard clauses: warn when assistant/result received before init message - Pending tool runs cleanup: ERROR status when result arrives before tool_result - processMessages edge cases: empty array and missing init message - Internal error handling: catch blocks in processMessage and processMessages TracingService: - OSS upload failure inside backgroundTask catch block - logTrace outer catch block when backgroundTaskHelper throws Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tegg/core/agent-tracing/test/TracingService.test.ts (1)
6-7: Use.jsextensions for local ESM imports.These imports use
.ts, which diverges from the tegg ESM convention and can cause emitted-path mismatches outside the test pipeline. Switch them to.js.♻️ Proposed fix
-import { TracingService } from '../src/TracingService.ts'; -import { RunStatus } from '../src/types.ts'; +import { TracingService } from '../src/TracingService.js'; +import { RunStatus } from '../src/types.js';As per coding guidelines, "All imports should use
.jsextensions for ESM files".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-tracing/test/TracingService.test.ts` around lines 6 - 7, The test imports use .ts ESM extensions which can cause emitted-path mismatches; update the local imports in TracingService.test.ts to use .js extensions instead (change the import specifiers for TracingService and RunStatus from '../src/TracingService.ts' and '../src/types.ts' to '../src/TracingService.js' and '../src/types.js') so they follow the tegg ESM convention and resolve correctly in emitted files.tegg/core/agent-tracing/test/ClaudeAgentTracer.test.ts (2)
25-43: These mock factories currently bypass SDK type-checking.
Partial<any>plusas unknown as SDKMessagedisables most compile-time validation here, so fixture drift or misspelled Anthropic fields will still compile. Prefer narrowed helper types per message discriminator instead ofany.As per coding guidelines, "Avoid
anytype; useunknownwhen type is truly unknown in TypeScript".Also applies to: 46-65, 68-86, 89-105, 108-130
🤖 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 25 - 43, The test helper createMockInit currently uses Partial<any> and an as unknown as SDKMessage cast which masks type errors; replace this with a narrowed helper type for the "init" discriminator (e.g., a dedicated SDKMessageInit or a Partial<Pick<SDKMessage, ...>> type) so the factory signature is createMockInit(overrides?: Partial<SDKMessageInit>) : SDKMessage and remove the unsafe any/unknown cast; update the other mock factories the same way so each factory (per message discriminator) uses a precise Partial<...> type instead of Partial<any> and returns a correctly-typed SDKMessage without forcing via as unknown as SDKMessage.
6-8: Use.jsextensions for local ESM imports.These test imports use
.ts, which is inconsistent with the tegg ESM rule and can break generated import paths outside Vitest. Switch them to.js.♻️ Proposed fix
-import { ClaudeAgentTracer } from '../src/ClaudeAgentTracer.ts'; -import { RunStatus } from '../src/types.ts'; -import { createMockLogger, createCapturingTracingService } from './TestUtils.ts'; +import { ClaudeAgentTracer } from '../src/ClaudeAgentTracer.js'; +import { RunStatus } from '../src/types.js'; +import { createMockLogger, createCapturingTracingService } from './TestUtils.js';As per coding guidelines, "All imports should use
.jsextensions for ESM files".🤖 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 6 - 8, Change the local ESM test imports to use .js extensions instead of .ts: update the imports that reference ClaudeAgentTracer, RunStatus and the TestUtils helpers (createMockLogger, createCapturingTracingService) so they import from the corresponding .js modules (e.g., '../src/ClaudeAgentTracer.js', '../src/types.js', './TestUtils.js') to satisfy the ESM import rule and avoid broken generated paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tegg/core/agent-tracing/test/ClaudeAgentTracer.test.ts`:
- Around line 25-43: The test helper createMockInit currently uses Partial<any>
and an as unknown as SDKMessage cast which masks type errors; replace this with
a narrowed helper type for the "init" discriminator (e.g., a dedicated
SDKMessageInit or a Partial<Pick<SDKMessage, ...>> type) so the factory
signature is createMockInit(overrides?: Partial<SDKMessageInit>) : SDKMessage
and remove the unsafe any/unknown cast; update the other mock factories the same
way so each factory (per message discriminator) uses a precise Partial<...> type
instead of Partial<any> and returns a correctly-typed SDKMessage without forcing
via as unknown as SDKMessage.
- Around line 6-8: Change the local ESM test imports to use .js extensions
instead of .ts: update the imports that reference ClaudeAgentTracer, RunStatus
and the TestUtils helpers (createMockLogger, createCapturingTracingService) so
they import from the corresponding .js modules (e.g.,
'../src/ClaudeAgentTracer.js', '../src/types.js', './TestUtils.js') to satisfy
the ESM import rule and avoid broken generated paths.
In `@tegg/core/agent-tracing/test/TracingService.test.ts`:
- Around line 6-7: The test imports use .ts ESM extensions which can cause
emitted-path mismatches; update the local imports in TracingService.test.ts to
use .js extensions instead (change the import specifiers for TracingService and
RunStatus from '../src/TracingService.ts' and '../src/types.ts' to
'../src/TracingService.js' and '../src/types.js') so they follow the tegg ESM
convention and resolve correctly in emitted files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7502a00f-d3f0-465b-9a38-3415c7f9dc18
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
tegg/core/agent-tracing/test/ClaudeAgentTracer.test.tstegg/core/agent-tracing/test/TracingService.test.ts
jerryliang64
commented
Mar 10, 2026
Contributor
Author
There was a problem hiding this comment.
文件命名风格与 tegg 项目不符合，参考一下项目已有的风格。
同理，检查并修改当前pr类似的情况
jerryliang64
commented
Mar 10, 2026
| return ( | ||
| `[agent_run][${name}]:` + | ||
| `traceId=${run.trace_id},` + | ||
| `type=${run.parent_run_id ? 'child_run' : 'root_run'},` + |
Contributor
Author
There was a problem hiding this comment.
需要再这里添加一个threadId的信息，参考langchain的threadId可以从此处获取 threadId=${run.extra?.metadata?.thread_id ?? 'unknown'},
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Rename ILogServiceClient/IOssClient to AbstractLogServiceClient/AbstractOssClient to follow tegg naming conventions (use Abstract prefix for abstract classes) - Remove empty AgentTracingConfig interface and unused configure() method - Add threadId to log prefix (from run.extra.metadata.thread_id) per reviewer request Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
tegg/core/agent-tracing/src/TracingService.ts (2)
8-10:⚠️ Potential issue | 🟠 MajorSwitch these local ESM imports to
.js.The current
.tsspecifiers still violate the tegg ESM rule and are easy to carry into emitted JS accidentally.As per coding guidelines, "All imports should use
.jsextensions for ESM files".🤖 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 local ESM import specifiers in TracingService.ts to use .js extensions: change the imports for AbstractLogServiceClient, AbstractOssClient, and the named import FIELDS_TO_OSS (and types IResource, RunStatus) so their source paths end with .js instead of .ts; ensure only the specifiers are changed (not the imported symbol names) so the module resolution follows the project's ESM rule and emitted JS won't carry invalid .ts paths.
114-135:⚠️ Potential issue | 🟠 MajorDon't replace trace payloads with OSS keys before the upload is known to exist.
This path deletes the inline field, schedules a fire-and-forget upload, and immediately logs an
IResourcereference. IfossClientis missing or the upload fails, the trace now points at a key that was never written, andBackgroundTaskHelper.run()will not propagate that failure back to the caller. With the current async flow, keep the original payload inline unless you can confirm a successful upload first.tegg/core/agent-tracing/src/ClaudeAgentTracer.ts (2)
153-155:⚠️ Potential issue | 🟡 MinorDon't fall back to the session start time when
duration_msis missing.If Claude omits
duration_ms, Line 154 makes the root run zero-duration even after a long session. UseDate.now()as the fallback instead.Possible fix
- this.#rootRun.end_time = this.#startTime + (message.duration_ms || 0); + this.#rootRun.end_time = + message.duration_ms !== undefined ? this.#startTime + message.duration_ms : Date.now();🤖 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 153 - 155, The root run end_time calculation in ClaudeAgentTracer incorrectly falls back to this.#startTime when message.duration_ms is missing; change the logic in the block that sets this.#rootRun.end_time so that if message.duration_ms is provided you set end_time = this.#startTime + message.duration_ms, otherwise set end_time = Date.now(); update the code that currently uses (message.duration_ms || 0) to use an explicit presence check so Date.now() is used as the fallback instead of zero-duration.
9-18:⚠️ Potential issue | 🟠 MajorUse
.jsspecifiers for these local ESM imports.The source file is in
tegg/**, so these local imports should follow the package’s.js-specifier rule.As per coding guidelines, "All imports should use
.jsextensions for ESM files".🤖 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, Update the local ESM import specifiers to include the .js extension: change the import of TracingService and the named types/values (ClaudeMessage, ClaudeContentBlock, ClaudeTokenUsage, IRunCost, RunStatus, TracerConfig, applyTracerConfig) so they import from './TracingService.js' and './types.js' respectively; ensure you only modify the specifiers (not the imported symbols) so ESM resolution follows the package rule.tegg/core/agent-tracing/src/types.ts (1)
10-15:⚠️ Potential issue | 🟠 MajorTighten these exported Claude shapes instead of leaking
any.These interfaces are part of the package surface, so the current
anyfields effectively turn off type-checking for downstream users. Useunknown,Record<string, unknown>, or named extension interfaces for the open-ended fields instead.As per coding guidelines, "Avoid
anytype; useunknownwhen type is truly unknown in TypeScript".Also applies to: 42-54, 69-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` around lines 10 - 15, Replace the use of `any` in the exported Claude shapes (starting with ClaudeToolUseContent) with stricter types: use `unknown` when the type is truly unknown, or `Record<string, unknown>` for open-ended object shapes, or introduce named extension types (e.g., ToolInput = Record<string, unknown> or a specific interface) and use those in the exported interfaces; update ClaudeToolUseContent's input to `input?: Record<string, unknown>` (or a named exported type) and apply the same change to all other exported Claude* interfaces in this file so downstream consumers get proper type checking.
🧹 Nitpick comments (1)
tegg/core/agent-tracing/test/LangGraphTracer.test.ts (1)
34-34: Drop the fixed 500ms sleeps from this suite.These tests replace
tracingServicewithcreateCapturingTracingService(), and that helper recordslogTrace()calls synchronously integg/core/agent-tracing/test/TestUtils.ts. Afterawait graph.invoke(...),capturedRunsis already ready for assertions, so the sleeps just add several seconds to the suite and keep the timing brittle.Also applies to: 78-78, 102-102, 130-130, 168-168, 194-194, 222-222, 300-300, 326-326
🤖 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, The fixed 500ms sleep helper (sleep) and all explicit awaits of sleep in LangGraphTracer.test should be removed: tests use createCapturingTracingService() which records logTrace() synchronously, so after await graph.invoke(...) the capturedRuns array is ready for assertions; delete the sleep function and all calls to sleep(...) (referenced around tests invoking graph.invoke and asserting capturedRuns) and rely directly on capturedRuns after graph.invoke without any extra delay.
🤖 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/ClaudeAgentTracer.ts`:
- Line 262: Several helper methods on the exported ClaudeAgentTracer (e.g.,
convertSDKMessage) are annotated `@internal` but lack access modifiers so they
remain public in emitted declarations; change each of these internal helpers to
be marked private (or protected) or move them out as non-exported module-level
functions, specifically update convertSDKMessage and the other `@internal` helper
methods in ClaudeAgentTracer to use the private (or protected) modifier so they
are not included in the public type declarations emitted with
isolatedDeclarations: true.
In `@tegg/core/agent-tracing/test/LangGraphTracer.test.ts`:
- Around line 8-10: The test file imports ESM modules with .ts extensions which
must be .js for runtime; update the three imports for LangGraphTracer,
RunStatus, and the TestUtils utilities by changing '../src/LangGraphTracer.ts' →
'../src/LangGraphTracer.js', '../src/types.ts' → '../src/types.js', and
'./TestUtils.ts' → './TestUtils.js' so symbols LangGraphTracer, RunStatus,
CapturedEntry, and createCapturingTracingService resolve correctly under ESM.
In `@tegg/core/agent-tracing/test/TestUtils.ts`:
- Line 4: Update the local ESM import in TestUtils.ts to use the emitted .js
specifier: replace the import that references '../src/TracingService.ts' with
'../src/TracingService.js' so the TracingService import follows the tegg ESM
convention and matches the built module graph.
---
Duplicate comments:
In `@tegg/core/agent-tracing/src/ClaudeAgentTracer.ts`:
- Around line 153-155: The root run end_time calculation in ClaudeAgentTracer
incorrectly falls back to this.#startTime when message.duration_ms is missing;
change the logic in the block that sets this.#rootRun.end_time so that if
message.duration_ms is provided you set end_time = this.#startTime +
message.duration_ms, otherwise set end_time = Date.now(); update the code that
currently uses (message.duration_ms || 0) to use an explicit presence check so
Date.now() is used as the fallback instead of zero-duration.
- Around line 9-18: Update the local ESM import specifiers to include the .js
extension: change the import of TracingService and the named types/values
(ClaudeMessage, ClaudeContentBlock, ClaudeTokenUsage, IRunCost, RunStatus,
TracerConfig, applyTracerConfig) so they import from './TracingService.js' and
'./types.js' respectively; ensure you only modify the specifiers (not the
imported symbols) so ESM resolution follows the package rule.
In `@tegg/core/agent-tracing/src/TracingService.ts`:
- Around line 8-10: Update the local ESM import specifiers in TracingService.ts
to use .js extensions: change the imports for AbstractLogServiceClient,
AbstractOssClient, and the named import FIELDS_TO_OSS (and types IResource,
RunStatus) so their source paths end with .js instead of .ts; ensure only the
specifiers are changed (not the imported symbol names) so the module resolution
follows the project's ESM rule and emitted JS won't carry invalid .ts paths.
In `@tegg/core/agent-tracing/src/types.ts`:
- Around line 10-15: Replace the use of `any` in the exported Claude shapes
(starting with ClaudeToolUseContent) with stricter types: use `unknown` when the
type is truly unknown, or `Record<string, unknown>` for open-ended object
shapes, or introduce named extension types (e.g., ToolInput = Record<string,
unknown> or a specific interface) and use those in the exported interfaces;
update ClaudeToolUseContent's input to `input?: Record<string, unknown>` (or a
named exported type) and apply the same change to all other exported Claude*
interfaces in this file so downstream consumers get proper type checking.
---
Nitpick comments:
In `@tegg/core/agent-tracing/test/LangGraphTracer.test.ts`:
- Line 34: The fixed 500ms sleep helper (sleep) and all explicit awaits of sleep
in LangGraphTracer.test should be removed: tests use
createCapturingTracingService() which records logTrace() synchronously, so after
await graph.invoke(...) the capturedRuns array is ready for assertions; delete
the sleep function and all calls to sleep(...) (referenced around tests invoking
graph.invoke and asserting capturedRuns) and rely directly on capturedRuns after
graph.invoke without any extra delay.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec67527b-0226-4799-a7c5-08e802577251
📒 Files selected for processing (11)
tegg/core/agent-tracing/src/AbstractLogServiceClient.tstegg/core/agent-tracing/src/AbstractOssClient.tstegg/core/agent-tracing/src/ClaudeAgentTracer.tstegg/core/agent-tracing/src/LangGraphTracer.tstegg/core/agent-tracing/src/TracingService.tstegg/core/agent-tracing/src/index.tstegg/core/agent-tracing/src/types.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.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- tegg/core/agent-tracing/src/index.ts
- tegg/core/agent-tracing/src/LangGraphTracer.ts
- tegg/core/agent-tracing/test/TracingService.test.ts
- tegg/core/agent-tracing/test/Configure.test.ts
| * Convert an SDKMessage to internal ClaudeMessage format. | ||
| * Returns null for message types that are not relevant to tracing. | ||
| */ | ||
| convertSDKMessage(msg: SDKMessage): ClaudeMessage | null { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l tegg/core/agent-tracing/src/ClaudeAgentTracer.tsRepository: eggjs/egg
Length of output: 105
🏁 Script executed:
cat -n tegg/core/agent-tracing/src/ClaudeAgentTracer.ts | head -1 | awk '{print $NF}'Repository: eggjs/egg
Length of output: 67
🏁 Script executed:
cat -n tegg/core/agent-tracing/src/ClaudeAgentTracer.ts | sed -n '1,30p'Repository: eggjs/egg
Length of output: 1225
🏁 Script executed:
cat -n tegg/core/agent-tracing/src/ClaudeAgentTracer.ts | sed -n '255,275p'Repository: eggjs/egg
Length of output: 891
🏁 Script executed:
cat -n tegg/core/agent-tracing/src/ClaudeAgentTracer.ts | sed -n '310,330p'Repository: eggjs/egg
Length of output: 849
🏁 Script executed:
cat -n tegg/core/agent-tracing/src/ClaudeAgentTracer.ts | sed -n '350,365p'Repository: eggjs/egg
Length of output: 524
🏁 Script executed:
cat -n tegg/core/agent-tracing/src/ClaudeAgentTracer.ts | sed -n '405,420p'Repository: eggjs/egg
Length of output: 487
🏁 Script executed:
cat -n tegg/core/agent-tracing/src/ClaudeAgentTracer.ts | sed -n '445,460p'Repository: eggjs/egg
Length of output: 647
🏁 Script executed:
cat -n tegg/core/agent-tracing/src/ClaudeAgentTracer.ts | sed -n '490,515p'Repository: eggjs/egg
Length of output: 810
🏁 Script executed:
find tegg/core/agent-tracing -name "tsconfig*.json" -o -name "package.json" | head -5Repository: eggjs/egg
Length of output: 127
🏁 Script executed:
cat -n tegg/core/agent-tracing/src/ClaudeAgentTracer.ts | sed -n '80,130p'Repository: eggjs/egg
Length of output: 2052
🏁 Script executed:
cat tegg/core/agent-tracing/tsconfig.jsonRepository: eggjs/egg
Length of output: 94
🏁 Script executed:
rg "ClaudeAgentTracer" tegg/core/agent-tracing/src/ -A 2 -B 2Repository: eggjs/egg
Length of output: 5309
🏁 Script executed:
cat tsconfig.json | grep -A 5 -B 5 "declaration\|isolatedDeclarations"Repository: eggjs/egg
Length of output: 461
🏁 Script executed:
rg "@internal" tegg/core/agent-tracing/src/ -B 1 -A 1Repository: eggjs/egg
Length of output: 1907
🏁 Script executed:
rg "convertSDKMessage|createRootRunInternal|createLLMRunInternal|createToolRunStartInternal|completeToolRunInternal|createRunCostInternal|logTrace" --type ts tegg/ --count-matches | head -20Repository: eggjs/egg
Length of output: 358
Mark these helper methods private.
These methods have @internal JSDoc comments but lack access modifiers, so they remain publicly callable on the exported ClaudeAgentTracer class. Since isolatedDeclarations: true is enabled in the project's TypeScript configuration, these public methods will be included in declaration emit and lock this API surface. Either mark them private or protected, or refactor them into non-exported internal helpers if they are only meant to be called by TraceSession.
Also applies to: 318, 356, 414, 452, 495, 509
🤖 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` at line 262, Several helper
methods on the exported ClaudeAgentTracer (e.g., convertSDKMessage) are
annotated `@internal` but lack access modifiers so they remain public in emitted
declarations; change each of these internal helpers to be marked private (or
protected) or move them out as non-exported module-level functions, specifically
update convertSDKMessage and the other `@internal` helper methods in
ClaudeAgentTracer to use the private (or protected) modifier so they are not
included in the public type declarations emitted with isolatedDeclarations:
true.
Comment on lines
+8
to
+10
| import { LangGraphTracer } from '../src/LangGraphTracer.ts'; | ||
| import { RunStatus } from '../src/types.ts'; | ||
| import { type CapturedEntry, createCapturingTracingService } from './TestUtils.ts'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the test file exists and show the imports at lines 8-10
if [ -f "tegg/core/agent-tracing/test/LangGraphTracer.test.ts" ]; then
echo "=== Import statements (lines 8-10) ==="
sed -n '8,10p' "tegg/core/agent-tracing/test/LangGraphTracer.test.ts"
echo ""
echo "=== Full imports section for context ==="
head -15 "tegg/core/agent-tracing/test/LangGraphTracer.test.ts"
else
echo "File not found at tegg/core/agent-tracing/test/LangGraphTracer.test.ts"
fi
# Check if the source files exist
echo ""
echo "=== Checking if target source files exist ==="
ls -la tegg/core/agent-tracing/src/LangGraphTracer.ts 2>&1 | head -1
ls -la tegg/core/agent-tracing/src/types.ts 2>&1 | head -1
ls -la tegg/core/agent-tracing/test/TestUtils.ts 2>&1 | head -1Repository: eggjs/egg
Length of output: 1233
Switch these local imports to .js extensions.
Lines 8-10 use .ts extensions, but all imports in tegg files must use .js for ESM. Change:
'../src/LangGraphTracer.ts'→'../src/LangGraphTracer.js''../src/types.ts'→'../src/types.js''./TestUtils.ts'→'./TestUtils.js'
🤖 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 8 - 10,
The test file imports ESM modules with .ts extensions which must be .js for
runtime; update the three imports for LangGraphTracer, RunStatus, and the
TestUtils utilities by changing '../src/LangGraphTracer.ts' →
'../src/LangGraphTracer.js', '../src/types.ts' → '../src/types.js', and
'./TestUtils.ts' → './TestUtils.js' so symbols LangGraphTracer, RunStatus,
CapturedEntry, and createCapturingTracingService resolve correctly under ESM.
| import type { Logger } from '@eggjs/tegg-types'; | ||
| import type { Run } from '@langchain/core/tracers/base'; | ||
|
|
||
| import { TracingService } from '../src/TracingService.ts'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and view the import statement
cat -n tegg/core/agent-tracing/test/TestUtils.ts | head -10Repository: eggjs/egg
Length of output: 389
🏁 Script executed:
# Check if the target TracingService module exists
ls -la tegg/core/agent-tracing/src/ | grep -i tracingRepository: eggjs/egg
Length of output: 120
🏁 Script executed:
# Check the build configuration to understand how .ts files are processed to .js
cat tegg/core/agent-tracing/tsdown.config.ts 2>/dev/null || echo "No tsdown.config.ts found"Repository: eggjs/egg
Length of output: 78
🏁 Script executed:
# Verify the file structure
find tegg/core/agent-tracing -type f -name "*.ts" | head -20Repository: eggjs/egg
Length of output: 742
🏁 Script executed:
# Check package.json for export configuration
cat tegg/core/agent-tracing/package.jsonRepository: eggjs/egg
Length of output: 2127
🏁 Script executed:
# Check if tsdown.config.ts exists at package root
cat tsdown.config.ts 2>/dev/null || echo "Not found at root"Repository: eggjs/egg
Length of output: 835
🏁 Script executed:
# Search for other local imports in tegg to see what pattern is actually used
rg "from ['\"]\.\..*\.ts['\"]" tegg/core/agent-tracing/test/ -A 0Repository: eggjs/egg
Length of output: 1013
🏁 Script executed:
# Check other tegg packages for their import patterns
rg "from ['\"]\.\..*\.ts['\"]" tegg/core/ --max-count 5 -A 0Repository: eggjs/egg
Length of output: 33560
🏁 Script executed:
# Check if there's a shared build config or tsdown config
find tegg -name "tsdown.config.ts" -o -name "tsup.config.ts" 2>/dev/nullRepository: eggjs/egg
Length of output: 1168
Use a .js specifier for this local ESM import.
The import at line 4 uses ../src/TracingService.ts, which violates the tegg ESM import convention. Change it to ../src/TracingService.js to align with the emitted module graph.
🤖 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` at line 4, Update the local ESM
import in TestUtils.ts to use the emitted .js specifier: replace the import that
references '../src/TracingService.ts' with '../src/TracingService.js' so the
TracingService import follows the tegg ESM convention and matches the built
module graph.
…Tracer Claude SDK runs are manually constructed and don't have LangChain's automatic metadata.thread_id. Use session_id as thread_id in run.extra.metadata so getLogInfoPrefix can extract it consistently across both LangGraph and Claude tracers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
New Features
@eggjs/agent-tracingpackage for comprehensive agent execution tracing across Claude SDK and LangGraph integrations.Dependencies
@langchain/coreto ^1.1.29 across packages.