feat: add langchain decorator #356
Conversation
|
Warning Rate limit exceeded@akitaSummer has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 13 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds three new packages (core/langchain-decorator, plugin/langchain, plugin/mcp-client) and extensive LangChain/LangGraph and MCP integrations: decorators, metadata models/builders, qualifiers, lifecycle hooks, MCP HTTP clients/factories, graph compilation protos/objects, tests, fixtures, typings and package manifests. No public APIs removed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Loader as LoadUnit discovery
participant ProtoHook as GraphPrototypeHook
participant CompiledProto as CompiledStateGraphProto
participant Factory as EggPrototypeFactory
participant ObjFactory as EggObjectFactory
participant ObjHook as GraphObjectHook / BoundModelObjectHook
participant Node as Graph Node Instance
participant MCP as MCP Client
participant Chat as Chat Model
Note over Loader,ProtoHook: discover decorators → register graph/tool/node/bound-model metadata
Loader->>ProtoHook: register proto entries
ProtoHook->>CompiledProto: create compiled graph protos (deferred wiring)
Factory->>ObjFactory: construct objects from protos
ObjFactory->>ObjHook: postCreate hooks (node/tool/boundModel)
ObjHook->>MCP: load MCP-provided tools (optional)
ObjHook->>Chat: bind found tools to ChatModel or create ToolNode
Node->>CompiledProto: compile/bind nodes & edges → CompiledStateGraphObject
CompiledProto->>Node: invoke graph (runtime)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas needing extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Summary of ChangesHello @akitaSummer, 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 the "@eggjs/tegg-langchain-decorator" package, which aims to streamline the development of AI-powered applications by integrating LangChain's powerful capabilities directly into the Egg.js ecosystem. By leveraging a new set of TypeScript decorators, developers can now define complex AI workflows, including state graphs, processing nodes, conditional edges, and AI tools, in a declarative and maintainable manner. This enhancement fosters a more idiomatic approach to building intelligent services, allowing for easier configuration and dependency management of LangChain components within Egg.js applications. Highlights
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new package, @eggjs/tegg-langchain-decorator, to integrate LangChain with the Tegg framework. It provides a set of decorators (@Graph, @GraphNode, @GraphEdge, @GraphTool, @BoundModel) to declaratively build and configure LangChain graphs. The overall approach is solid and aligns well with the Tegg ecosystem.
However, I've found a few issues that need to be addressed:
- The
CHANGELOG.mdfor the new package seems to be incorrectly copied from another package. - The
package.jsonuses"next"for LangChain dependencies, which can cause build instability. - There are several instances of commented-out code and deep imports that should be cleaned up for better maintainability.
- One of the utility classes introduces a global state, which could be problematic.
- A test fixture is missing a required configuration for a graph.
Details and suggestions are provided in the specific comments.
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (18)
core/langchain-decorator/test/index.test.ts (2)
6-6: Use strict assertion API.Prefer importing the strict variant to avoid coercion.
-import assert from 'node:assert'; +import { strict as assert } from 'node:assert';
8-12: Make test intent explicit and assert strictly.Rename the test and use strictEqual for clarity.
-describe('index.test.ts', () => { - it('should success', () => { +describe('ChatModel qualifier', () => { + it('returns "chat" for Foo.chatModel', () => { const chatModelQualifier = QualifierUtil.getProperQualifier(Foo, 'chatModel', ChatModelQualifierAttribute); - assert.equal(chatModelQualifier, 'chat'); + assert.strictEqual(chatModelQualifier, 'chat'); }); });core/langchain-decorator/src/model/GraphEdgeMetadata.ts (1)
8-16: Implement interface fields, ensure immutability, and align with other metadata classes.Currently only two fields are assigned; params extend SingletonProtoParams and future fields could be lost. Also clone arrays to prevent external mutation.
-export class GraphEdgeMetadata { - fromNodeName: string; - toNodeNames: string[]; - - constructor(params: IGraphEdgeMetadata) { - this.fromNodeName = params.fromNodeName; - this.toNodeNames = params.toNodeNames; - } -} +export class GraphEdgeMetadata implements IGraphEdgeMetadata { + readonly fromNodeName: string; + readonly toNodeNames: string[]; + // Optional: include any fields from SingletonProtoParams in the future + // readonly name?: string; + // readonly accessLevel?: string; + + constructor(params: IGraphEdgeMetadata) { + this.fromNodeName = params.fromNodeName; + this.toNodeNames = [ ...params.toNodeNames ]; + // Optional validation: + // if (!this.toNodeNames.length) throw new Error('GraphEdgeMetadata.toNodeNames must be non-empty'); + } +}core/langchain-decorator/CHANGELOG.md (1)
871-876: Fix heading levels to satisfy markdownlint (MD001).“### Features/Bug Fixes” should not jump from H1; change to H2 (and similarly elsewhere).
Example fix:
-### Features +## FeaturesApply the same adjustment to all similar sections. Based on static analysis hints.
core/langchain-decorator/src/builder/GraphToolMetaBuilder.ts (1)
12-17: Return explicitly when metadata is absent.Add an explicit
return undefined;to satisfy noImplicitReturns and improve clarity.build(): GraphToolMetadata | undefined { const metadata = GraphToolInfoUtil.getGraphToolMetadata(this.clazz); if (metadata) { return new GraphToolMetadata(metadata); } + return undefined; }core/langchain-decorator/src/builder/GraphNodeMetaBuilder.ts (2)
12-17: Return explicitly when metadata is absent.Mirror GraphToolMetaBuilder and return
undefinedexplicitly for consistency and noImplicitReturns.build(): GraphNodeMetadata | undefined { const metadata = GraphNodeInfoUtil.getGraphNodeMetadata(this.clazz); if (metadata) { return new GraphNodeMetadata(metadata); } + return undefined; }
5-22: Reduce duplication across MetaBuilders.GraphNodeMetaBuilder and GraphToolMetaBuilder are structurally identical. Consider a small generic/base builder to DRY.
Sketch:
abstract class BaseMetaBuilder<TMeta> { constructor(protected readonly clazz: EggProtoImplClass) {} protected abstract fetch(): TMeta | undefined; protected abstract wrap(meta: TMeta): any; build() { const m = this.fetch(); return m ? this.wrap(m) : undefined; } }Then extend for node/tool with tiny overrides.
core/langchain-decorator/src/model/BoundModelMetadata.ts (1)
14-18: Consider using Object.assign for consistency.The constructor manually assigns each property, while
GraphToolMetadatausesObject.assign(this, params). Consider adopting a consistent approach across all metadata model classes.Apply this diff for consistency:
constructor(params: IBoundModelMetadata) { - this.modelName = params.modelName; - this.tools = params.tools; - this.mcpServers = params.mcpServers; + Object.assign(this, params); }core/langchain-decorator/src/model/GraphToolMetadata.ts (2)
6-6: Remove or address the commented code.The commented
schemaproperty suggests incomplete work or a placeholder. Please either implement the property if needed, document why it's commented, or remove it.
10-11: Remove redundant default values.The default values
toolName = ''anddescription = ''are immediately overwritten byObject.assign(this, params)in the constructor, making them redundant.Apply this diff:
export class GraphToolMetadata implements IGraphToolMetadata { - toolName = ''; - description = ''; + toolName: string; + description: string; constructor(params: IGraphToolMetadata) { Object.assign(this, params); } }core/langchain-decorator/src/decorator/GraphTool.ts (1)
1-1: Remove commented-out import.The commented import is dead code that should be removed for cleaner codebase maintenance.
Apply this diff:
-// import type { ToolSchemaBase, DynamicStructuredTool } from '../../core/tools.js'; -core/langchain-decorator/src/model/GraphNodeMetadata.ts (1)
14-18: Consider using Object.assign for consistency.The constructor manually assigns each property, while GraphToolMetadata uses
Object.assign(this, params). For consistency across metadata models, consider using Object.assign here as well.Apply this diff:
constructor(params: IGraphNodeMetadata) { - this.nodeName = params.nodeName; - this.tools = params.tools; - this.mcpServers = params.mcpServers; + Object.assign(this, params); }core/langchain-decorator/src/decorator/GraphNode.ts (2)
1-1: Remove commented-out import.This dead code should be removed for cleaner maintenance.
Apply this diff:
-// import type { EggProtoImplClass } from '@alipay/tegg-types'; -
30-36: Remove commented-out type signature.The commented code at line 32 is dead code that should be removed.
Apply this diff:
export interface IGraphNode<S extends StateDefinition = StateDefinition, T = any> { - // execute(state: T extends AbstractStateGraph<infer S> ? GraphStateType<S> : never): Promise<GraphUpdateType<T> extends object ? GraphUpdateType<T> & Record<string, any> : GraphUpdateType<T>>; execute(state: AnnotationRoot<S>['State']): Promise<UpdateType<S> & Record<string, any>> | Promise<ToolNode<T>>;core/langchain-decorator/src/decorator/GraphEdge.ts (1)
25-35: Enforce execute when multiple toNodeNames.Today it’s a doc comment only. Consider a runtime assert in the builder or a conditional type to require execute when params.toNodeNames has length > 1.
Example runtime guard (in GraphEdgeMetaBuilder):
if (params.toNodeNames.length > 1 && typeof clazz.prototype.execute !== 'function') { throw new Error(`GraphEdge ${clazz.name} must implement execute() when multiple toNodeNames are provided`); }Also applies to: 42-44
core/langchain-decorator/src/decorator/Graph.ts (1)
31-38: Remove ts-ignore; use declare fields to anchor generics.Avoids disabling checks while keeping the intent.
Apply:
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - private _names: N; + protected declare _names: N; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - private _state: S; + protected declare _state: S;core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (2)
6-10: Avoid importing from internal dist paths; use public module paths.Swap to public exports to prevent breakage on package rearranges.
Apply:
-import { BaseMessage } from '@langchain/core/dist/messages/base'; +import { BaseMessage } from '@langchain/core/messages'; ... -import { AIMessage } from '@langchain/core/dist/messages/ai'; +import { AIMessage } from '@langchain/core/messages';
7-7: Keep AccessLevel import consistent with production code.Elsewhere it’s imported from @eggjs/tegg; consider aligning.
Example:
-import { AccessLevel, ToolArgs } from '@eggjs/tegg-types'; +import { AccessLevel } from '@eggjs/tegg'; +import { ToolArgs } from '@eggjs/tegg-types';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
core/langchain-decorator/CHANGELOG.md(1 hunks)core/langchain-decorator/README.md(1 hunks)core/langchain-decorator/package.json(1 hunks)core/langchain-decorator/src/builder/BoundModelMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphEdgeMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphNodeMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphToolMetaBuilder.ts(1 hunks)core/langchain-decorator/src/decorator/BoundModel.ts(1 hunks)core/langchain-decorator/src/decorator/Graph.ts(1 hunks)core/langchain-decorator/src/decorator/GraphEdge.ts(1 hunks)core/langchain-decorator/src/decorator/GraphNode.ts(1 hunks)core/langchain-decorator/src/decorator/GraphTool.ts(1 hunks)core/langchain-decorator/src/index.ts(1 hunks)core/langchain-decorator/src/model/BoundModelMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphEdgeMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphNodeMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphToolMetadata.ts(1 hunks)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts(1 hunks)core/langchain-decorator/src/type/metadataKey.ts(1 hunks)core/langchain-decorator/src/util/BoundModelInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphToolInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/index.ts(1 hunks)core/langchain-decorator/test/fixtures/modules/langchain/index.ts(1 hunks)core/langchain-decorator/test/fixtures/modules/langchain/package.json(1 hunks)core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts(1 hunks)core/langchain-decorator/test/graph.test.ts(1 hunks)core/langchain-decorator/test/index.test.ts(1 hunks)core/langchain-decorator/test/package.json(1 hunks)core/langchain-decorator/tsconfig.json(1 hunks)core/langchain-decorator/tsconfig.pub.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (23)
core/langchain-decorator/src/builder/BoundModelMetaBuilder.ts (3)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/BoundModelMetadata.ts (1)
BoundModelMetadata(9-19)core/langchain-decorator/src/util/BoundModelInfoUtil.ts (1)
BoundModelInfoUtil(6-14)
core/langchain-decorator/src/model/BoundModelMetadata.ts (1)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)
core/langchain-decorator/src/util/GraphInfoUtil.ts (4)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/GraphMetadata.ts (1)
IGraphMetadata(4-8)core/core-decorator/src/util/MetadataUtil.ts (1)
MetadataUtil(5-97)core/langchain-decorator/src/type/metadataKey.ts (1)
GRAPH_GRAPH_METADATA(4-4)
core/langchain-decorator/test/index.test.ts (2)
core/core-decorator/src/util/QualifierUtil.ts (1)
QualifierUtil(6-100)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifierAttribute(3-3)
core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (4)
core/langchain-decorator/src/model/GraphNodeMetadata.ts (1)
IGraphNodeMetadata(3-7)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/MetadataUtil.ts (1)
MetadataUtil(5-97)core/langchain-decorator/src/type/metadataKey.ts (1)
GRAPH_NODE_METADATA(3-3)
core/langchain-decorator/src/builder/GraphToolMetaBuilder.ts (3)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/GraphToolMetadata.ts (1)
GraphToolMetadata(9-16)core/langchain-decorator/src/util/GraphToolInfoUtil.ts (1)
GraphToolInfoUtil(6-14)
core/langchain-decorator/src/util/GraphToolInfoUtil.ts (4)
core/langchain-decorator/src/model/GraphToolMetadata.ts (1)
IGraphToolMetadata(3-7)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/MetadataUtil.ts (1)
MetadataUtil(5-97)core/langchain-decorator/src/type/metadataKey.ts (1)
GRAPH_TOOL_METADATA(1-1)
core/langchain-decorator/src/builder/GraphNodeMetaBuilder.ts (3)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/GraphNodeMetadata.ts (1)
GraphNodeMetadata(9-19)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
GraphNodeInfoUtil(6-14)
core/langchain-decorator/src/builder/GraphEdgeMetaBuilder.ts (3)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/GraphEdgeMetadata.ts (1)
GraphEdgeMetadata(8-16)core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts (1)
GraphEdgeInfoUtil(6-14)
core/langchain-decorator/test/graph.test.ts (10)
core/langchain-decorator/src/builder/GraphMetaBuilder.ts (1)
GraphMetaBuilder(5-22)core/langchain-decorator/src/builder/GraphEdgeMetaBuilder.ts (1)
GraphEdgeMetaBuilder(5-22)core/langchain-decorator/src/builder/GraphNodeMetaBuilder.ts (1)
GraphNodeMetaBuilder(5-22)core/langchain-decorator/src/builder/BoundModelMetaBuilder.ts (1)
BoundModelMetaBuilder(5-22)core/langchain-decorator/src/builder/GraphToolMetaBuilder.ts (1)
GraphToolMetaBuilder(5-22)core/langchain-decorator/src/model/GraphToolMetadata.ts (1)
GraphToolMetadata(9-16)core/controller-decorator/src/util/MCPInfoUtil.ts (1)
MCPInfoUtil(42-166)core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (1)
ToolType(38-42)core/langchain-decorator/src/model/GraphMetadata.ts (1)
GraphMetadata(10-20)core/langchain-decorator/src/decorator/GraphNode.ts (1)
TeggToolNode(38-44)
core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts (4)
core/langchain-decorator/src/model/GraphEdgeMetadata.ts (1)
IGraphEdgeMetadata(3-6)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/MetadataUtil.ts (1)
MetadataUtil(5-97)core/langchain-decorator/src/type/metadataKey.ts (1)
GRAPH_EDGE_METADATA(2-2)
core/langchain-decorator/src/decorator/GraphEdge.ts (5)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (5)
GraphEdge(92-107)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/model/GraphEdgeMetadata.ts (2)
IGraphEdgeMetadata(3-6)constructor(12-15)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts (1)
GraphEdgeInfoUtil(6-14)
core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
core/core-decorator/src/util/QualifierUtil.ts (1)
QualifierUtil(6-100)
core/langchain-decorator/src/util/BoundModelInfoUtil.ts (4)
core/langchain-decorator/src/model/BoundModelMetadata.ts (1)
IBoundModelMetadata(3-7)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/MetadataUtil.ts (1)
MetadataUtil(5-97)core/langchain-decorator/src/type/metadataKey.ts (1)
BOUND_MODEL_METADATA(6-6)
core/langchain-decorator/src/decorator/GraphTool.ts (4)
core/langchain-decorator/src/model/GraphToolMetadata.ts (2)
IGraphToolMetadata(3-7)constructor(13-15)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphToolInfoUtil.ts (1)
GraphToolInfoUtil(6-14)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (9)
core/langchain-decorator/test/fixtures/modules/langchain/index.ts (1)
SingletonProto(7-14)core/langchain-decorator/src/decorator/GraphTool.ts (2)
GraphTool(15-26)IGraphTool(28-31)core/controller-decorator/src/decorator/mcp/MCPTool.ts (1)
ToolArgsSchema(35-52)core/types/controller-decorator/MCPToolParams.ts (1)
ToolArgs(5-5)core/langchain-decorator/src/decorator/BoundModel.ts (2)
BoundModel(14-25)TeggBoundModel(30-30)core/langchain-decorator/src/decorator/GraphNode.ts (3)
GraphNode(17-28)TeggToolNode(38-44)IGraphNode(30-36)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifier(6-10)core/langchain-decorator/src/decorator/GraphEdge.ts (3)
GraphStateType(38-38)GraphEdge(25-36)IGraphEdge(42-44)core/langchain-decorator/src/decorator/Graph.ts (2)
Graph(12-23)TeggCompiledStateGraph(40-40)
core/langchain-decorator/src/model/GraphMetadata.ts (1)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)
core/langchain-decorator/src/decorator/GraphNode.ts (5)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (7)
GraphNode(63-68)GraphNode(71-90)GraphNode(123-147)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/model/GraphNodeMetadata.ts (2)
IGraphNodeMetadata(3-7)constructor(14-18)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
GraphNodeInfoUtil(6-14)
core/langchain-decorator/src/model/GraphNodeMetadata.ts (1)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)
core/langchain-decorator/src/decorator/BoundModel.ts (4)
core/langchain-decorator/src/model/BoundModelMetadata.ts (2)
IBoundModelMetadata(3-7)constructor(14-18)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/BoundModelInfoUtil.ts (1)
BoundModelInfoUtil(6-14)
core/langchain-decorator/src/builder/GraphMetaBuilder.ts (3)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/GraphMetadata.ts (1)
GraphMetadata(10-20)core/langchain-decorator/src/util/GraphInfoUtil.ts (1)
GraphInfoUtil(6-21)
core/langchain-decorator/test/fixtures/modules/langchain/index.ts (1)
core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifier(6-10)
core/langchain-decorator/src/decorator/Graph.ts (4)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (6)
Graph(110-120)Graph(150-175)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/model/GraphMetadata.ts (2)
IGraphMetadata(4-8)constructor(15-19)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphInfoUtil.ts (1)
GraphInfoUtil(6-21)
🪛 Biome (2.1.2)
core/langchain-decorator/src/decorator/GraphEdge.ts
[error] 27-28: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/GraphTool.ts
[error] 16-16: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/GraphNode.ts
[error] 18-18: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/BoundModel.ts
[error] 15-15: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/Graph.ts
[error] 13-13: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 markdownlint-cli2 (0.18.1)
core/langchain-decorator/CHANGELOG.md
873-873: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
900-900: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
911-911: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
938-938: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
963-963: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
991-991: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (24)
core/langchain-decorator/test/package.json (1)
1-3: LGTM!The test package configuration correctly sets the module type to CommonJS, which is a standard pattern for test directories.
core/langchain-decorator/test/fixtures/modules/langchain/package.json (1)
1-6: LGTM!The test fixture package correctly defines an Egg module for testing purposes.
core/langchain-decorator/tsconfig.json (1)
1-12: LGTM!The TypeScript configuration is correct. Note the duplication concern raised in
tsconfig.pub.json.core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
1-10: LGTM!The qualifier implementation correctly uses
Symbol.forfor a globally shared symbol and properly delegates toQualifierUtil.addInjectQualifier. The decorator signature matches the standard pattern for supporting both property and parameter injection.core/langchain-decorator/src/model/GraphMetadata.ts (1)
1-20: LGTM!The GraphMetadata model is well-structured with optional properties for flexible graph configuration. The union type for
checkpoint(class or string) provides good flexibility for different use cases.core/langchain-decorator/tsconfig.pub.json (1)
1-12: No changes needed—the separate tsconfig files are intentionally part of the monorepo design.The
tsconfig.jsonandtsconfig.pub.jsonfiles are separate by design, not redundant duplication. Each package uses them for distinct purposes:
tsconfig.json: used for development builds (npm run tsc)tsconfig.pub.json: used for publishing builds (npm run tsc:pub, invoked byprepublishOnly)This pattern is consistent across all 20+ packages in the monorepo and is coordinated through root-level lerna scripts (
tscvstsc:pub). While they currently have identical content, the separation provides architectural flexibility—configs can diverge for publishing workflows without affecting development builds, and the distinction is semantically clear in the build pipeline.Likely an incorrect or invalid review comment.
core/langchain-decorator/src/util/index.ts (1)
1-5: LGTM!The barrel export pattern is clean and follows ES module conventions with
.jsextensions.core/langchain-decorator/test/fixtures/modules/langchain/index.ts (1)
1-14: LGTM!The test fixture correctly demonstrates the usage of
@ChatModelQualifierwith dependency injection for testing purposes.core/langchain-decorator/src/builder/BoundModelMetaBuilder.ts (1)
1-22: LGTM!The builder pattern is clean and consistent. The
build()method correctly returnsundefinedwhen metadata is not found, matching the declared return type.core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
1-14: LGTM!The utility class provides a clean abstraction over
MetadataUtilfor graph node metadata management.core/langchain-decorator/src/util/BoundModelInfoUtil.ts (1)
1-14: LGTM!The utility class follows the same clean pattern as other metadata utilities in the package.
core/langchain-decorator/src/builder/GraphMetaBuilder.ts (1)
1-22: LGTM!The builder pattern is consistent with other builders in the package and correctly handles the optional metadata case.
core/langchain-decorator/src/decorator/GraphTool.ts (2)
15-26: LGTM!The decorator factory correctly applies SingletonProto, records file path, and stores graph tool metadata. The parameter name
constructorat line 16 is idiomatic for TypeScript class decorators and the static analysis warning is a false positive in this context.
28-31: LGTM!The interface cleanly defines the GraphTool contract with proper typing using LangChain's DynamicStructuredTool.
core/langchain-decorator/src/builder/GraphEdgeMetaBuilder.ts (1)
1-22: LGTM!The builder follows a clean, consistent pattern for extracting and constructing GraphEdgeMetadata from decorated classes.
core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts (1)
1-14: LGTM!The utility provides clean getter/setter methods for graph edge metadata, following the established pattern across the codebase.
core/langchain-decorator/src/util/GraphInfoUtil.ts (1)
7-21: Verify necessity of dual storage pattern.The class maintains metadata in both
MetadataUtil(line 10) and the staticgraphMap(line 11), which creates redundancy and potential for inconsistency. If metadata is set via MetadataUtil directly elsewhere, graphMap won't reflect it.Consider:
- Is the separate graphMap necessary for performance or iteration purposes?
- If so, document why dual storage is required.
- If not, remove graphMap and rely solely on MetadataUtil.
For now, ensure all code paths use
GraphInfoUtil.setGraphMetadataexclusively to maintain consistency.core/langchain-decorator/src/util/GraphToolInfoUtil.ts (1)
1-14: LGTM!The utility provides clean metadata management for graph tools, following the established pattern.
core/langchain-decorator/test/graph.test.ts (1)
1-54: LGTM!The test suite provides solid coverage of the graph metadata system, including metadata construction, parameter validation, and prototype inheritance. The assertions are clear and comprehensive.
core/langchain-decorator/src/decorator/GraphNode.ts (1)
17-28: LGTM!The decorator factory correctly applies SingletonProto, records file path, and stores graph node metadata. The parameter name
constructorat line 18 is idiomatic for TypeScript class decorators and the static analysis warning is a false positive.core/langchain-decorator/src/type/metadataKey.ts (1)
1-6: LGTM: clear, distinct metadata keys.core/langchain-decorator/src/index.ts (1)
1-25: Index export surface looks coherent.Re-exports are consistent and help DX.
core/langchain-decorator/src/decorator/BoundModel.ts (1)
11-11: BaseChatOpenAI is stable and does not require refactoring.BaseChatOpenAI is not marked as deprecated or unstable in LangChain changelogs or API docs, and the class remains in the public API with active maintenance. The premise of the review comment is incorrect. The original import and type definition using BaseChatOpenAI do not need to be changed.
Likely an incorrect or invalid review comment.
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (1)
144-146: The review comment conflates unrelated code locations and mischaracterizes the type safety.The
bindTools()method on ChatOpenAI has the declared signature:bindTools(tools: ChatOpenAIToolType[], kwargs?: Partial<CallOptions>) => Runnable<BaseLanguageModelInput, AIMessageChunk, CallOptions>, which is a concrete type—notany.Lines 144-146 (BarNode.build()) and lines 164-165 (BarGraph.fooChatModel property) are in separate classes and are unrelated. BarNode.build() uses ChatOpenAI.bindTools() directly, and TypeScript will correctly infer the return type as
Runnable<...>without widening toany. TeggBoundModel is used only for the fooChatModel property type in BarGraph and does not affect BarNode.build()'s type inference. The usage ofbindTools([ this.fooTool ])is correct and type-safe per the @langchain/openai API.Likely an incorrect or invalid review comment.
akitaSummer
force-pushed
the
feat/langchain
branch
from
29a8e22 to
46fbbf2
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (11)
core/langchain-decorator/README.md (1)
1-7: Expand README with overview, API, and examples.Add purpose, decorator list, quick-start graph/node/tool examples, DI integration, and links to LangChain/LangGraph docs. Happy to draft.
Apply (append sections):
+# Overview +Short description of what the package does and when to use it. + +# Decorators +- @Graph: ... +- @GraphNode: ... +- @GraphEdge: ... +- @GraphTool: ... +- @BoundModel: ... +- @ChatModelQualifier: ... + +# Examples +## Graph definition +// minimal graph + node/edge example + +## Tools & DI +// binding a tool/model and injecting via Tegg + +## Chat integration +// invoking a compiled graph + +# References +- LangChain docs +- LangGraph docscore/langchain-decorator/src/model/GraphToolMetadata.ts (1)
6-6: Remove commented-out property.Delete the stale comment to keep the model clean.
- // schema: Parameters<McpServer['tool']>['2'];core/langchain-decorator/src/util/GraphInfoUtil.ts (1)
6-20: Avoid leaking mutable global state; add reset and return ReadonlyMap.Global cache can flake tests and enable unintended mutation. Provide a reset for tests and expose a readonly view.
export class GraphInfoUtil { static graphMap = new Map<EggProtoImplClass, IGraphMetadata>(); static setGraphMetadata(metadata: IGraphMetadata, clazz: EggProtoImplClass) { MetadataUtil.defineMetaData(GRAPH_GRAPH_METADATA, metadata, clazz); GraphInfoUtil.graphMap.set(clazz, metadata); } static getGraphMetadata(clazz: EggProtoImplClass): IGraphMetadata | undefined { return MetadataUtil.getMetaData(GRAPH_GRAPH_METADATA, clazz); } - static getAllGraphMetadata(): Map<EggProtoImplClass, IGraphMetadata> { - return GraphInfoUtil.graphMap; - } + static getAllGraphMetadata(): ReadonlyMap<EggProtoImplClass, IGraphMetadata> { + return GraphInfoUtil.graphMap; + } + + /** For tests/framework reset */ + static reset() { + GraphInfoUtil.graphMap.clear(); + } }core/langchain-decorator/src/decorator/GraphNode.ts (2)
1-1: Remove commented-out import.-// import type { EggProtoImplClass } from '@alipay/tegg-types';
38-44: Implement interface contract: add state parameter and initialize field.execute must accept state; also ensure toolNode is initialized or marked definite.
-export class TeggToolNode implements IGraphNode { - toolNode: ToolNode; - - async execute() { +export class TeggToolNode implements IGraphNode { + toolNode!: ToolNode; + + async execute(_state?: AnnotationRoot<StateDefinition>['State']) { return this.toolNode; } }core/langchain-decorator/src/decorator/Graph.ts (2)
12-22: Rename callback param to avoid Biome “constructor” shadow.-export function Graph<N extends string = '', S extends StateDefinition = StateDefinition>(params: IGraphMetadata) { - return (constructor: EggProtoImplClass<AbstractStateGraph<N, S>>) => { +export function Graph<N extends string = '', S extends StateDefinition = StateDefinition>(params: IGraphMetadata) { + return (clazz: EggProtoImplClass<AbstractStateGraph<N, S>>) => { const func = SingletonProto({ accessLevel: params?.accessLevel ?? AccessLevel.PUBLIC, name: params?.name, }); - func(constructor); - PrototypeUtil.setFilePath(constructor, StackUtil.getCalleeFromStack(false, 5)); + func(clazz); + PrototypeUtil.setFilePath(clazz, StackUtil.getCalleeFromStack(false, 5)); - GraphInfoUtil.setGraphMetadata(params, constructor); + GraphInfoUtil.setGraphMetadata(params, clazz); }; }
25-27: Apply Option A: replace build() return type with TeggCompiledStateGraph helper.Line 26 currently erases N and S generics by using hardcoded
StateDefinition. The TeggCompiledStateGraph helper at line 40 already exists in the file and correctly preserves generics. Apply the change:export interface IGraph<N extends string = '', S extends StateDefinition = StateDefinition> extends StateGraph<S, AnnotationRoot<S>['State'], UpdateType<S>, N> { - build?(): Promise<CompiledStateGraph<StateType<StateDefinition>, UpdateType<StateDefinition>, string, StateDefinition, StateDefinition, StateDefinition> | undefined>; + build?(): Promise<TeggCompiledStateGraph<AbstractStateGraph<N, S>> | undefined>; }core/langchain-decorator/src/decorator/GraphTool.ts (2)
1-1: Remove commented-out importLeftover comment; drop it to keep the file clean. (Same point as earlier review.)
-// import type { ToolSchemaBase, DynamicStructuredTool } from '../../core/tools.js';
13-13: Avoid deep import from '@langchain/core/dist/tools'Deep paths are brittle. Prefer the public entry and make it type-only.
-import { DynamicStructuredTool, ToolSchemaBase } from '@langchain/core/dist/tools'; +import type { DynamicStructuredTool, ToolSchemaBase } from '@langchain/core/tools';To verify which public path your installed version exposes, run:
#!/bin/bash set -euo pipefail echo "Resolve public tool entry:" node -p "require.resolve('@langchain/core/tools')" echo "Resolve deep path (should be avoided):" node -p "require.resolve('@langchain/core/dist/tools')" || echo "deep path not resolvable (good)"core/langchain-decorator/src/decorator/BoundModel.ts (1)
14-25: Renameconstructorparameter to avoid shadowing.The decorator callback parameter named
constructorshadows the global property and triggers Biome linting errors. This issue was previously flagged and should be addressed by renaming toclazzortarget.core/langchain-decorator/src/decorator/GraphEdge.ts (1)
25-36: Renameconstructorparameter to avoid shadowing.The decorator callback parameter
constructorshadows the global property and triggers Biome linting errors. This was flagged in previous review comments and should be renamed toclazzortarget.
🧹 Nitpick comments (11)
core/langchain-decorator/CHANGELOG.md (1)
870-916: Normalize heading levels to satisfy MD001.Headings jump levels; adjust to increment by one (e.g., use “##” for versions after the first “#”).
Also applies to: 937-971, 991-991
core/langchain-decorator/test/index.test.ts (2)
8-9: Name the test for intentRename to describe behavior, e.g., “should read ChatModel qualifier from property injection”.
10-11: Prefer explicit strict assertionUse strictEqual for clarity.
- assert.equal(chatModelQualifier, 'chat'); + assert.strictEqual(chatModelQualifier, 'chat');core/langchain-decorator/test/fixtures/modules/langchain/index.ts (1)
1-1: Avoid runtime dependency in testsOnly the type is needed. Use a type-only import to prevent requiring @langchain/openai at runtime.
-import { ChatOpenAI } from '@langchain/openai'; +import type { ChatOpenAI } from '@langchain/openai';core/langchain-decorator/src/decorator/GraphTool.ts (1)
15-26: Rename parameter to avoid shadowing “constructor”Prevents confusion and satisfies the linter. Update references accordingly.
-export function GraphTool<ToolSchema = ToolSchemaBase>(params: IGraphToolMetadata) { - return (constructor: EggProtoImplClass<IGraphTool<ToolSchema>>) => { +export function GraphTool<ToolSchema = ToolSchemaBase>(params: IGraphToolMetadata) { + return (Ctor: EggProtoImplClass<IGraphTool<ToolSchema>>) => { const func = SingletonProto({ accessLevel: params?.accessLevel ?? AccessLevel.PUBLIC, name: params?.name, }); - func(constructor); - PrototypeUtil.setFilePath(constructor, StackUtil.getCalleeFromStack(false, 5)); + func(Ctor); + PrototypeUtil.setFilePath(Ctor, StackUtil.getCalleeFromStack(false, 5)); - GraphToolInfoUtil.setGraphToolMetadata(params, constructor); + GraphToolInfoUtil.setGraphToolMetadata(params, Ctor); }; }core/langchain-decorator/test/graph.test.ts (3)
35-37: Use assert.ok for boolean predicateImproves readability for boolean checks.
- assert.deepEqual(meta instanceof GraphToolMetadata, true); + assert.ok(meta instanceof GraphToolMetadata);
47-49: Use assert.ok for boolean predicateSame here.
- assert.deepEqual(meta instanceof GraphMetadata, true); + assert.ok(meta instanceof GraphMetadata);
51-53: Use assert.ok for prototype checkCleaner than comparing to true.
- assert.equal(TeggToolNode.prototype.isPrototypeOf(ToolNode.prototype), true); + assert.ok(TeggToolNode.prototype.isPrototypeOf(ToolNode.prototype));core/langchain-decorator/src/decorator/BoundModel.ts (2)
27-28: Simplify the BaseChatModel type alias.The conditional type
extends infer C ? C : neveris redundant.InstanceType<typeof BaseChatOpenAI<T>>already resolves to a concrete type without needing the conditional.Apply this diff to simplify:
-type BaseChatModel<T extends ChatOpenAICallOptions = ChatOpenAICallOptions> = InstanceType<typeof BaseChatOpenAI<T>> extends infer C - ? C : never; +type BaseChatModel<T extends ChatOpenAICallOptions = ChatOpenAICallOptions> = InstanceType<typeof BaseChatOpenAI<T>>;
30-30: Remove unnecessaryNonNullablewrapper for consistency.The
bindToolsmethod is always defined onBaseChatOpenAIand its derived types; it's not optional. In TypeScript,BaseChatOpenAI/ChatOpenAIhasbindTools(tools, kwargs?)as a standard method signature. Line 35 ofGraphNode.tsaccessesConfigurableModel['bindTools']without wrapping it, showing the codebase treatsbindToolsas non-optional elsewhere. RemoveNonNullablefor consistency:export type TeggBoundModel<S, CallOptions extends ChatOpenAICallOptions = ChatOpenAICallOptions> = S & ReturnType<BaseChatModel<CallOptions>['bindTools']>;core/langchain-decorator/src/decorator/GraphEdge.ts (1)
13-24: Consider using English for inline documentation.The JSDoc contains Chinese comments on lines 19-20, which may reduce accessibility for international contributors. For consistency and broader maintainability, consider translating these to English.
Example translation:
- Line 19:
// Marks the start point. If only fromNodeName and toNodeNames are specified, this creates a unidirectional edge- Line 20:
// Marks the end point(s). Can be multiple. When multiple, execute method must be implemented
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
core/langchain-decorator/CHANGELOG.md(1 hunks)core/langchain-decorator/README.md(1 hunks)core/langchain-decorator/package.json(1 hunks)core/langchain-decorator/src/builder/BoundModelMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphEdgeMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphNodeMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphToolMetaBuilder.ts(1 hunks)core/langchain-decorator/src/decorator/BoundModel.ts(1 hunks)core/langchain-decorator/src/decorator/Graph.ts(1 hunks)core/langchain-decorator/src/decorator/GraphEdge.ts(1 hunks)core/langchain-decorator/src/decorator/GraphNode.ts(1 hunks)core/langchain-decorator/src/decorator/GraphTool.ts(1 hunks)core/langchain-decorator/src/index.ts(1 hunks)core/langchain-decorator/src/model/BoundModelMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphEdgeMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphNodeMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphToolMetadata.ts(1 hunks)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts(1 hunks)core/langchain-decorator/src/type/metadataKey.ts(1 hunks)core/langchain-decorator/src/util/BoundModelInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphToolInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/index.ts(1 hunks)core/langchain-decorator/test/fixtures/modules/langchain/index.ts(1 hunks)core/langchain-decorator/test/fixtures/modules/langchain/package.json(1 hunks)core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts(1 hunks)core/langchain-decorator/test/graph.test.ts(1 hunks)core/langchain-decorator/test/index.test.ts(1 hunks)core/langchain-decorator/test/package.json(1 hunks)core/langchain-decorator/tsconfig.json(1 hunks)core/langchain-decorator/tsconfig.pub.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- core/langchain-decorator/test/fixtures/modules/langchain/package.json
- core/langchain-decorator/test/package.json
🚧 Files skipped from review as they are similar to previous changes (16)
- core/langchain-decorator/src/util/index.ts
- core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts
- core/langchain-decorator/src/builder/GraphToolMetaBuilder.ts
- core/langchain-decorator/src/builder/GraphMetaBuilder.ts
- core/langchain-decorator/src/type/metadataKey.ts
- core/langchain-decorator/src/builder/GraphNodeMetaBuilder.ts
- core/langchain-decorator/src/qualifier/ChatModelQualifier.ts
- core/langchain-decorator/src/util/BoundModelInfoUtil.ts
- core/langchain-decorator/tsconfig.pub.json
- core/langchain-decorator/package.json
- core/langchain-decorator/src/model/BoundModelMetadata.ts
- core/langchain-decorator/src/model/GraphNodeMetadata.ts
- core/langchain-decorator/src/model/GraphEdgeMetadata.ts
- core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts
- core/langchain-decorator/src/builder/GraphEdgeMetaBuilder.ts
- core/langchain-decorator/src/util/GraphToolInfoUtil.ts
🧰 Additional context used
🧬 Code graph analysis (12)
core/langchain-decorator/test/graph.test.ts (10)
core/langchain-decorator/src/builder/GraphMetaBuilder.ts (1)
GraphMetaBuilder(5-22)core/langchain-decorator/src/builder/GraphEdgeMetaBuilder.ts (1)
GraphEdgeMetaBuilder(5-22)core/langchain-decorator/src/builder/GraphNodeMetaBuilder.ts (1)
GraphNodeMetaBuilder(5-22)core/langchain-decorator/src/builder/BoundModelMetaBuilder.ts (1)
BoundModelMetaBuilder(5-22)core/langchain-decorator/src/builder/GraphToolMetaBuilder.ts (1)
GraphToolMetaBuilder(5-22)core/langchain-decorator/src/model/GraphToolMetadata.ts (1)
GraphToolMetadata(9-16)core/controller-decorator/src/util/MCPInfoUtil.ts (1)
MCPInfoUtil(42-166)core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (1)
ToolType(38-42)core/langchain-decorator/src/model/GraphMetadata.ts (1)
GraphMetadata(10-20)core/langchain-decorator/src/decorator/GraphNode.ts (1)
TeggToolNode(38-44)
core/langchain-decorator/src/builder/BoundModelMetaBuilder.ts (3)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/BoundModelMetadata.ts (1)
BoundModelMetadata(9-19)core/langchain-decorator/src/util/BoundModelInfoUtil.ts (1)
BoundModelInfoUtil(6-14)
core/langchain-decorator/src/util/GraphInfoUtil.ts (4)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/GraphMetadata.ts (1)
IGraphMetadata(4-8)core/core-decorator/src/util/MetadataUtil.ts (1)
MetadataUtil(5-97)core/langchain-decorator/src/type/metadataKey.ts (1)
GRAPH_GRAPH_METADATA(4-4)
core/langchain-decorator/test/index.test.ts (2)
core/core-decorator/src/util/QualifierUtil.ts (1)
QualifierUtil(6-100)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifierAttribute(3-3)
core/langchain-decorator/test/fixtures/modules/langchain/index.ts (2)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (2)
SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifier(6-10)
core/langchain-decorator/src/model/GraphMetadata.ts (1)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)
core/langchain-decorator/src/decorator/GraphEdge.ts (5)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (5)
GraphEdge(92-107)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/model/GraphEdgeMetadata.ts (2)
IGraphEdgeMetadata(3-6)constructor(12-15)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts (1)
GraphEdgeInfoUtil(6-14)
core/langchain-decorator/src/decorator/Graph.ts (4)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (6)
Graph(110-120)Graph(150-175)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/model/GraphMetadata.ts (2)
IGraphMetadata(4-8)constructor(15-19)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphInfoUtil.ts (1)
GraphInfoUtil(6-21)
core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (4)
core/langchain-decorator/src/model/GraphNodeMetadata.ts (1)
IGraphNodeMetadata(3-7)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/MetadataUtil.ts (1)
MetadataUtil(5-97)core/langchain-decorator/src/type/metadataKey.ts (1)
GRAPH_NODE_METADATA(3-3)
core/langchain-decorator/src/decorator/BoundModel.ts (4)
core/langchain-decorator/src/model/BoundModelMetadata.ts (2)
IBoundModelMetadata(3-7)constructor(14-18)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/BoundModelInfoUtil.ts (1)
BoundModelInfoUtil(6-14)
core/langchain-decorator/src/decorator/GraphNode.ts (4)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (7)
GraphNode(63-68)GraphNode(71-90)GraphNode(123-147)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/model/GraphNodeMetadata.ts (2)
IGraphNodeMetadata(3-7)constructor(14-18)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
GraphNodeInfoUtil(6-14)
core/langchain-decorator/src/decorator/GraphTool.ts (3)
core/langchain-decorator/src/model/GraphToolMetadata.ts (2)
IGraphToolMetadata(3-7)constructor(13-15)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphToolInfoUtil.ts (1)
GraphToolInfoUtil(6-14)
🪛 Biome (2.1.2)
core/langchain-decorator/src/decorator/GraphEdge.ts
[error] 27-28: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/Graph.ts
[error] 13-13: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/BoundModel.ts
[error] 15-15: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/GraphNode.ts
[error] 18-18: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/GraphTool.ts
[error] 16-16: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 markdownlint-cli2 (0.18.1)
core/langchain-decorator/CHANGELOG.md
873-873: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
900-900: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
911-911: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
938-938: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
963-963: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
991-991: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-macos (18)
🔇 Additional comments (7)
core/langchain-decorator/tsconfig.json (1)
1-12: Configuration is consistent with all sibling packages—no action required.The tsconfig.json follows the established pattern used across all core packages in the monorepo (verified against 5 sibling packages). The configuration properly extends the root tsconfig and uses consistent compiler options and exclusions.
core/langchain-decorator/src/model/GraphMetadata.ts (1)
4-19: LGTM.Types align with usage; optional fields make sense.
core/langchain-decorator/src/decorator/Graph.ts (1)
12-22: Clarify the specific Biome shadowing issue being checked.The executed script confirms the code matches the provided snippet (line 13 shows
return (constructor: ...)). However, the review comment lacks an explicit problem statement. The parameterconstructorin the arrow function doesn't shadow any outer scope variable, andconst funcis a standard local variable.Please clarify which Biome rule or specific shadowing concern should be verified, or confirm if this review comment can be dismissed.
core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
6-13: Verify whether graph node metadata should inherit from parent classes or remain class-specific.The codebase pattern shows class-level metadata (e.g., ControllerMetadata, DaoInfoUtil) uses
getOwnMetaData()to retrieve only the class's own metadata. GraphNodeInfoUtil currently usesgetMetaData(), which traverses the prototype chain and inherits from parent classes. Confirm whether this inheritance behavior is intentional or if it should usegetOwnMetaData()to be consistent with similar class-level metadata patterns.core/langchain-decorator/src/builder/BoundModelMetaBuilder.ts (1)
5-21: Builder looks good and consistent with peersAPI and behavior align with other MetaBuilders.
core/langchain-decorator/src/decorator/GraphEdge.ts (1)
38-44: LGTM!The type aliases and interface definition properly integrate LangGraph's state types with Tegg's decorator pattern. The optional
executemethod appropriately supports conditional edge routing.core/langchain-decorator/src/index.ts (1)
1-25: LGTM!The barrel export structure is clean and well-organized. Exports are logically grouped by category (builders, decorators, models, utils, qualifier) with consistent naming conventions and visual separation, making the public API easy to navigate.
akitaSummer
force-pushed
the
feat/langchain
branch
from
46fbbf2 to
84604d2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (17)
core/langchain-decorator/CHANGELOG.md (1)
1-1002: The package name mismatch has already been flagged in previous reviews.Multiple past reviews have identified that this changelog incorrectly references "@eggjs/tegg-orm-decorator" throughout instead of "@eggjs/tegg-langchain-decorator". Please address the existing comments.
core/langchain-decorator/src/decorator/GraphNode.ts (5)
1-1: Remove commented-out import.This appears to be leftover development code.
14-15: Replace deep imports with stable public API paths.Past reviews have identified that these deep
/distpaths are brittle. Use public API exports instead.
17-27: Rename 'constructor' parameter to avoid shadowing the global property.Biome flags this as shadowing the built-in "constructor" property. Past reviews have suggested renaming to 'clazz' or 'target'.
32-32: Remove commented-out code.This dead code should be cleaned up for maintainability.
38-44: Add missing state parameter to match IGraphNode interface.Past reviews have confirmed that TeggToolNode.execute() is missing the required state parameter from the IGraphNode interface contract.
core/langchain-decorator/src/decorator/BoundModel.ts (1)
14-25: Rename 'constructor' parameter to avoid shadowing the global property.This has been flagged in past reviews. The parameter should be renamed to avoid the Biome linting error.
core/langchain-decorator/src/util/index.ts (1)
1-5: Remove .js extensions from export paths for consistency.Past reviews have noted that these extensions are inconsistent with other files in the package.
core/langchain-decorator/src/model/GraphToolMetadata.ts (1)
6-6: Remove the commented-out schema property.The commented property should be removed to keep the code clean.
core/langchain-decorator/src/util/GraphInfoUtil.ts (1)
7-20: Global state and duplicate storage concerns.The static
graphMapintroduces global mutable state that persists across contexts and can cause issues in testing. Additionally, metadata is stored in bothMetadataUtil(via Reflect) andgraphMap, creating redundancy. Note thatgetGraphMetadataonly retrieves fromMetadataUtil, not fromgraphMap, which could lead to inconsistencies if metadata is set directly viaMetadataUtil.Consider whether
graphMapis necessary, or implement a reset mechanism for testing.core/langchain-decorator/src/decorator/GraphTool.ts (2)
1-1: Remove the commented-out import.This commented import should be removed as it's a leftover that references a potentially incorrect path.
13-13: Avoid deep imports from @langchain/core.The deep import
@langchain/core/dist/toolsis fragile and may break with package updates. Check ifDynamicStructuredToolandToolSchemaBasecan be imported from the public API, such as@langchain/core/tools.What is the correct public API import path for DynamicStructuredTool and ToolSchemaBase in @langchain/core?core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (2)
6-6: Avoid deep imports from @langchain/core.The deep imports from
@langchain/core/dist/messages/baseand@langchain/core/dist/messages/aiare fragile and may break with package updates. Import from the public API instead.Apply this diff:
-import { BaseMessage } from '@langchain/core/dist/messages/base'; -import { AIMessage } from '@langchain/core/dist/messages/ai'; +import { BaseMessage, AIMessage } from '@langchain/core/messages';Also applies to: 9-9
170-173: Consider adding setEntryPoint for graph clarity.The
build()method adds a node and conditional edges but doesn't explicitly set an entry point. For graphs with cycles or non-standard flows, callingthis.setEntryPoint(FooGraphNodeName.ACTION)before adding nodes would make the graph structure more explicit.async build() { + this.setEntryPoint(FooGraphNodeName.ACTION); this.addNode(FooGraphNodeName.ACTION, this.barNode.execute); this.addConditionalEdges(FooGraphNodeName.ACTION, this.fooContinueEdge.execute); return this.compile({ checkpointer: this.fooSaver }); }core/langchain-decorator/src/decorator/GraphEdge.ts (1)
26-26: Rename parameter from "constructor" to avoid shadowing.The parameter name
constructorshadows the globalFunction.prototype.constructorproperty and triggers a lint error. Rename it totargetorclazz.Apply this diff:
- return (constructor: EggProtoImplClass<IGraphEdge<S, N>>) => { + return (target: EggProtoImplClass<IGraphEdge<S, N>>) => { const func = SingletonProto({ accessLevel: params?.accessLevel ?? AccessLevel.PUBLIC, name: params?.name, }); - func(constructor); - PrototypeUtil.setFilePath(constructor, StackUtil.getCalleeFromStack(false, 5)); + func(target); + PrototypeUtil.setFilePath(target, StackUtil.getCalleeFromStack(false, 5)); - GraphEdgeInfoUtil.setGraphEdgeMetadata(params, constructor); + GraphEdgeInfoUtil.setGraphEdgeMetadata(params, target); };core/langchain-decorator/src/decorator/Graph.ts (2)
13-13: Rename parameter from "constructor" to avoid shadowing.The parameter name
constructorshadows the globalFunction.prototype.constructorproperty and triggers a lint error. Rename it totargetorclazz.Apply this diff:
- return (constructor: EggProtoImplClass<AbstractStateGraph<N, S>>) => { + return (target: EggProtoImplClass<AbstractStateGraph<N, S>>) => { const func = SingletonProto({ accessLevel: params?.accessLevel ?? AccessLevel.PUBLIC, name: params?.name, }); - func(constructor); - PrototypeUtil.setFilePath(constructor, StackUtil.getCalleeFromStack(false, 5)); + func(target); + PrototypeUtil.setFilePath(target, StackUtil.getCalleeFromStack(false, 5)); - GraphInfoUtil.setGraphMetadata(params, constructor); + GraphInfoUtil.setGraphMetadata(params, target); };
25-27: Fix build() return type to preserve generics S and N.The current
build()signature erases the generic parametersNandS, replacing them with concrete typesstringandStateDefinition. This loses type information for callers.Apply this diff to preserve the generics:
export interface IGraph<N extends string = '', S extends StateDefinition = StateDefinition> extends StateGraph<S, AnnotationRoot<S>['State'], UpdateType<S>, N> { - build?(): Promise<CompiledStateGraph<StateType<StateDefinition>, UpdateType<StateDefinition>, string, StateDefinition, StateDefinition, StateDefinition> | undefined>; + build?(): Promise<CompiledStateGraph<StateType<S>, UpdateType<S>, N, S, S> | undefined>; }
🧹 Nitpick comments (2)
core/langchain-decorator/src/model/GraphToolMetadata.ts (1)
13-15: Consider consistent constructor patterns across metadata models.This constructor uses
Object.assign(this, params)whileGraphMetadata.tsuses explicit property assignment. For consistency and maintainability, consider aligning the constructor pattern across all metadata model classes.core/langchain-decorator/src/decorator/Graph.ts (1)
31-37: Remove unused private fields with ts-ignore directives.The private fields
_namesand_stateare not used and require@ts-ignorecomments to suppress errors. If these fields serve no purpose, they should be removed to reduce code clutter.If they are placeholders for future use or serve a type-level purpose, please add a comment explaining their intent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
core/langchain-decorator/CHANGELOG.md(1 hunks)core/langchain-decorator/README.md(1 hunks)core/langchain-decorator/package.json(1 hunks)core/langchain-decorator/src/builder/BoundModelMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphEdgeMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphNodeMetaBuilder.ts(1 hunks)core/langchain-decorator/src/builder/GraphToolMetaBuilder.ts(1 hunks)core/langchain-decorator/src/decorator/BoundModel.ts(1 hunks)core/langchain-decorator/src/decorator/Graph.ts(1 hunks)core/langchain-decorator/src/decorator/GraphEdge.ts(1 hunks)core/langchain-decorator/src/decorator/GraphNode.ts(1 hunks)core/langchain-decorator/src/decorator/GraphTool.ts(1 hunks)core/langchain-decorator/src/index.ts(1 hunks)core/langchain-decorator/src/model/BoundModelMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphEdgeMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphNodeMetadata.ts(1 hunks)core/langchain-decorator/src/model/GraphToolMetadata.ts(1 hunks)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts(1 hunks)core/langchain-decorator/src/type/metadataKey.ts(1 hunks)core/langchain-decorator/src/util/BoundModelInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/GraphToolInfoUtil.ts(1 hunks)core/langchain-decorator/src/util/index.ts(1 hunks)core/langchain-decorator/test/fixtures/modules/langchain/index.ts(1 hunks)core/langchain-decorator/test/fixtures/modules/langchain/package.json(1 hunks)core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts(1 hunks)core/langchain-decorator/test/graph.test.ts(1 hunks)core/langchain-decorator/test/index.test.ts(1 hunks)core/langchain-decorator/test/package.json(1 hunks)core/langchain-decorator/tsconfig.json(1 hunks)core/langchain-decorator/tsconfig.pub.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- core/langchain-decorator/test/fixtures/modules/langchain/package.json
- core/langchain-decorator/test/package.json
🚧 Files skipped from review as they are similar to previous changes (17)
- core/langchain-decorator/README.md
- core/langchain-decorator/src/builder/BoundModelMetaBuilder.ts
- core/langchain-decorator/src/util/GraphToolInfoUtil.ts
- core/langchain-decorator/tsconfig.pub.json
- core/langchain-decorator/tsconfig.json
- core/langchain-decorator/src/model/BoundModelMetadata.ts
- core/langchain-decorator/src/builder/GraphEdgeMetaBuilder.ts
- core/langchain-decorator/src/qualifier/ChatModelQualifier.ts
- core/langchain-decorator/src/model/GraphNodeMetadata.ts
- core/langchain-decorator/test/graph.test.ts
- core/langchain-decorator/package.json
- core/langchain-decorator/test/fixtures/modules/langchain/index.ts
- core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts
- core/langchain-decorator/src/model/GraphEdgeMetadata.ts
- core/langchain-decorator/src/builder/GraphNodeMetaBuilder.ts
- core/langchain-decorator/src/util/BoundModelInfoUtil.ts
- core/langchain-decorator/src/index.ts
🧰 Additional context used
🧬 Code graph analysis (12)
core/langchain-decorator/src/util/GraphInfoUtil.ts (4)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/GraphMetadata.ts (1)
IGraphMetadata(4-8)core/core-decorator/src/util/MetadataUtil.ts (1)
MetadataUtil(5-97)core/langchain-decorator/src/type/metadataKey.ts (1)
GRAPH_GRAPH_METADATA(4-4)
core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (4)
core/langchain-decorator/src/model/GraphNodeMetadata.ts (1)
IGraphNodeMetadata(3-7)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/MetadataUtil.ts (1)
MetadataUtil(5-97)core/langchain-decorator/src/type/metadataKey.ts (1)
GRAPH_NODE_METADATA(3-3)
core/langchain-decorator/src/builder/GraphMetaBuilder.ts (3)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/GraphMetadata.ts (1)
GraphMetadata(10-20)core/langchain-decorator/src/util/GraphInfoUtil.ts (1)
GraphInfoUtil(6-21)
core/langchain-decorator/src/builder/GraphToolMetaBuilder.ts (3)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/GraphToolMetadata.ts (1)
GraphToolMetadata(9-16)core/langchain-decorator/src/util/GraphToolInfoUtil.ts (1)
GraphToolInfoUtil(6-14)
core/langchain-decorator/test/index.test.ts (2)
core/core-decorator/src/util/QualifierUtil.ts (1)
QualifierUtil(6-100)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifierAttribute(3-3)
core/langchain-decorator/src/model/GraphMetadata.ts (1)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (8)
core/langchain-decorator/src/decorator/GraphTool.ts (2)
GraphTool(15-26)IGraphTool(28-31)core/controller-decorator/src/decorator/mcp/MCPTool.ts (1)
ToolArgsSchema(35-52)core/types/controller-decorator/MCPToolParams.ts (1)
ToolArgs(5-5)core/langchain-decorator/src/decorator/BoundModel.ts (2)
BoundModel(14-25)TeggBoundModel(30-30)core/langchain-decorator/src/decorator/GraphNode.ts (3)
GraphNode(17-28)TeggToolNode(38-44)IGraphNode(30-36)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifier(6-10)core/langchain-decorator/src/decorator/GraphEdge.ts (3)
GraphStateType(38-38)GraphEdge(25-36)IGraphEdge(42-44)core/langchain-decorator/src/decorator/Graph.ts (2)
Graph(12-23)TeggCompiledStateGraph(40-40)
core/langchain-decorator/src/decorator/BoundModel.ts (4)
core/langchain-decorator/src/model/BoundModelMetadata.ts (2)
IBoundModelMetadata(3-7)constructor(14-18)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/BoundModelInfoUtil.ts (1)
BoundModelInfoUtil(6-14)
core/langchain-decorator/src/decorator/Graph.ts (5)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (6)
Graph(110-120)Graph(150-175)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/model/GraphMetadata.ts (2)
IGraphMetadata(4-8)constructor(15-19)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphInfoUtil.ts (1)
GraphInfoUtil(6-21)
core/langchain-decorator/src/decorator/GraphEdge.ts (5)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (5)
GraphEdge(92-107)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/model/GraphEdgeMetadata.ts (2)
IGraphEdgeMetadata(3-6)constructor(12-15)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts (1)
GraphEdgeInfoUtil(6-14)
core/langchain-decorator/src/decorator/GraphNode.ts (5)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (7)
GraphNode(63-68)GraphNode(71-90)GraphNode(123-147)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/model/GraphNodeMetadata.ts (2)
IGraphNodeMetadata(3-7)constructor(14-18)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
GraphNodeInfoUtil(6-14)
core/langchain-decorator/src/decorator/GraphTool.ts (4)
core/langchain-decorator/src/model/GraphToolMetadata.ts (2)
IGraphToolMetadata(3-7)constructor(13-15)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphToolInfoUtil.ts (1)
GraphToolInfoUtil(6-14)
🪛 Biome (2.1.2)
core/langchain-decorator/src/decorator/BoundModel.ts
[error] 15-15: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/Graph.ts
[error] 13-13: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/GraphEdge.ts
[error] 27-28: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/GraphNode.ts
[error] 18-18: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/GraphTool.ts
[error] 16-16: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 markdownlint-cli2 (0.18.1)
core/langchain-decorator/CHANGELOG.md
873-873: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
900-900: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
911-911: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
938-938: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
963-963: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
991-991: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-macos (16)
🔇 Additional comments (6)
core/langchain-decorator/test/index.test.ts (1)
1-15: LGTM!The test correctly validates the ChatModelQualifier mechanism using QualifierUtil. The Node version check and dynamic imports are appropriate for this test context.
core/langchain-decorator/src/builder/GraphToolMetaBuilder.ts (1)
5-22: LGTM!The builder follows a clean, consistent pattern for retrieving and wrapping GraphTool metadata. The implementation aligns well with the other metadata builders in this PR.
core/langchain-decorator/src/builder/GraphMetaBuilder.ts (1)
5-22: LGTM!The builder follows the same clean pattern as the other metadata builders in this PR. The implementation is consistent and correct.
core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
6-14: LGTM!The utility provides clean metadata storage and retrieval for GraphNode decorators. The implementation is consistent with the other InfoUtil classes in this package.
core/langchain-decorator/src/model/GraphMetadata.ts (1)
1-20: LGTM!The model definition is clean and the explicit property assignment in the constructor is clear and type-safe.
core/langchain-decorator/src/type/metadataKey.ts (1)
1-6: LGTM!The metadata keys are well-defined using
Symbol.for()with unique, descriptive identifiers.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (2)
6-9: Use public message exports.Please switch to the stable
@langchain/core/messagesentry point instead of deepdistpaths to avoid breakage on package updates.-import { BaseMessage } from '@langchain/core/dist/messages/base'; -import { AIMessage } from '@langchain/core/dist/messages/ai'; +import { BaseMessage, AIMessage } from '@langchain/core/messages';
170-174: Set the graph entry point.
compile()will still throw because no entry node is defined. Callthis.setEntryPoint(FooGraphNodeName.ACTION);before wiring nodes/edges.async build() { + this.setEntryPoint(FooGraphNodeName.ACTION); this.addNode(FooGraphNodeName.ACTION, this.barNode.execute); this.addConditionalEdges(FooGraphNodeName.ACTION, this.fooContinueEdge.execute); return this.compile({ checkpointer: this.fooSaver }); }
🧹 Nitpick comments (3)
plugin/langchain/lib/config/QualifierUtil.ts (1)
5-12: Consider adding return type annotation and documentation.The method lacks an explicit return type, which could affect type safety and IDE autocompletion. Additionally, JSDoc would help clarify the purpose of module config qualifiers and their usage in the qualifier system.
Apply this diff to add type safety:
- static getModuleConfigQualifier(moduleName: string) { + static getModuleConfigQualifier(moduleName: string): { moduleConfig: Array<{ attribute: symbol; value: string }> } { return {Optionally, add JSDoc for better documentation:
/** * Creates a module configuration qualifier for dependency injection. * @param moduleName - The name of the module to qualify * @returns A qualifier object with module config attribute */ static getModuleConfigQualifier(moduleName: string): { moduleConfig: Array<{ attribute: symbol; value: string }> } {plugin/langchain/lib/ChatModelHelper.ts (1)
6-15: Consider adding explicit return type and documentation.The implementation is correct and follows the framework's qualifier pattern. To improve type safety and maintainability, consider:
- Adding an explicit return type annotation to
getChatModelQualifier- Adding JSDoc comments to document the method's purpose and parameters
Example with return type:
- static getChatModelQualifier(clientName: string) { + static getChatModelQualifier(clientName: string): { [key: string]: Array<{ attribute: symbol; value: string }> } { return {change_tag.sh (1)
3-4: Verify the hardcoded version and consider parameterization.The script hardcodes version
3.56.2, but the package.json in this PR specifies3.62.0. This mismatch suggests either the script is outdated or targets a specific older release.Consider making the version a command-line parameter to avoid hardcoding:
#!/bin/bash # 目标版本和标签 -TARGET_VERSION="3.56.2" -TAG="latest" # 如果需自定义标签名,修改这里 +TARGET_VERSION="${1:-3.62.0}" +TAG="${2:-latest}" # 如果需自定义标签名,修改这里 + +if [ $# -eq 0 ]; then + echo "Usage: $0 <version> [tag]" + echo "Example: $0 3.62.0 latest" + exit 1 +fiThis makes the script more maintainable and less error-prone.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
change_tag.sh(1 hunks)core/langchain-decorator/index.ts(1 hunks)core/langchain-decorator/test/fixtures/modules/langchain/index.ts(1 hunks)core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts(1 hunks)plugin/controller/test/http/request.test.ts(1 hunks)plugin/langchain/CHANGELOG.md(1 hunks)plugin/langchain/README.md(1 hunks)plugin/langchain/index.ts(1 hunks)plugin/langchain/lib/ChatModelHelper.ts(1 hunks)plugin/langchain/lib/ChatOpenAI.ts(1 hunks)plugin/langchain/lib/config/QualifierUtil.ts(1 hunks)plugin/langchain/lib/util.ts(1 hunks)plugin/langchain/package.json(1 hunks)plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts(1 hunks)plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/module.yml(1 hunks)plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/package.json(1 hunks)plugin/langchain/test/fixtures/apps/langchain/config/config.default.js(1 hunks)plugin/langchain/test/fixtures/apps/langchain/config/module.json(1 hunks)plugin/langchain/test/fixtures/apps/langchain/config/plugin.js(1 hunks)plugin/langchain/test/fixtures/apps/langchain/package.json(1 hunks)plugin/langchain/test/llm.test.ts(1 hunks)plugin/langchain/tsconfig.json(1 hunks)plugin/langchain/tsconfig.pub.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- plugin/langchain/tsconfig.pub.json
- plugin/langchain/test/fixtures/apps/langchain/package.json
- plugin/langchain/tsconfig.json
- plugin/langchain/test/fixtures/apps/langchain/config/module.json
🚧 Files skipped from review as they are similar to previous changes (1)
- core/langchain-decorator/test/fixtures/modules/langchain/index.ts
🧰 Additional context used
🧬 Code graph analysis (6)
plugin/langchain/lib/config/QualifierUtil.ts (1)
core/types/core-decorator/enum/Qualifier.ts (1)
ConfigSourceQualifierAttribute(1-1)
plugin/langchain/lib/util.ts (2)
core/types/core-decorator/model/EggMultiInstancePrototypeInfo.ts (1)
ObjectInfo(6-10)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifierAttribute(3-3)
plugin/langchain/lib/ChatOpenAI.ts (7)
core/types/core-decorator/model/EggMultiInstancePrototypeInfo.ts (2)
MultiInstancePrototypeGetObjectsContext(12-16)ObjectInfo(6-10)core/common-util/src/ModuleConfig.ts (1)
ModuleConfigUtil(31-380)plugin/langchain/lib/util.ts (2)
getClientNames(10-14)getChatModelConfig(16-24)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (2)
ChatModelInjectName(4-4)ChatModelQualifierAttribute(3-3)plugin/langchain/lib/ChatModelHelper.ts (1)
ChatModelHelper(6-15)plugin/langchain/lib/config/QualifierUtil.ts (1)
QualifierUtil(4-13)core/core-decorator/src/decorator/MultiInstanceInfo.ts (1)
MultiInstanceInfo(4-9)
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts (2)
core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifier(6-10)core/controller-decorator/src/decorator/http/HTTPMethod.ts (1)
HTTPMethod(7-23)
plugin/langchain/lib/ChatModelHelper.ts (1)
core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (2)
ChatModelInjectName(4-4)ChatModelQualifierAttribute(3-3)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (9)
core/langchain-decorator/test/fixtures/modules/langchain/index.ts (1)
SingletonProto(7-14)core/langchain-decorator/src/decorator/GraphTool.ts (2)
GraphTool(15-26)IGraphTool(28-31)core/controller-decorator/src/decorator/mcp/MCPTool.ts (1)
ToolArgsSchema(35-52)core/types/controller-decorator/MCPToolParams.ts (1)
ToolArgs(5-5)core/langchain-decorator/src/decorator/BoundModel.ts (2)
BoundModel(14-25)TeggBoundModel(30-30)core/langchain-decorator/src/decorator/GraphNode.ts (3)
GraphNode(17-28)TeggToolNode(38-44)IGraphNode(30-36)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifier(6-10)core/langchain-decorator/src/decorator/GraphEdge.ts (3)
GraphStateType(38-38)GraphEdge(25-36)IGraphEdge(42-44)core/langchain-decorator/src/decorator/Graph.ts (2)
Graph(12-23)TeggCompiledStateGraph(40-40)
🪛 Biome (2.1.2)
plugin/langchain/test/llm.test.ts
[error] 32-34: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🪛 markdownlint-cli2 (0.18.1)
plugin/langchain/CHANGELOG.md
596-596: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
802-802: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
901-901: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
984-984: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
1027-1027: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
1062-1062: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
1073-1073: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
1094-1094: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
1115-1115: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
1150-1150: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
1161-1161: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (16)
🔇 Additional comments (10)
plugin/langchain/test/fixtures/apps/langchain/config/config.default.js (1)
3-16: LGTM! Test configuration is appropriate.The configuration correctly disables CSRF and body parser for the test fixture, and uses a hardcoded key suitable for testing. This follows standard Egg.js conventions for test configurations.
plugin/langchain/lib/config/QualifierUtil.ts (1)
3-3: Clarify the TODO and "mist impl" reference.The TODO suggests this implementation needs refactoring, and "mist impl" is unclear—possibly a typo for "missed impl" or "must impl"? Please clarify whether this work is blocking for this PR or can be tracked separately.
plugin/langchain/lib/ChatModelHelper.ts (1)
1-4: LGTM!The imports are correctly sourced from the decorator package and both are used in the method implementation.
plugin/controller/test/http/request.test.ts (1)
33-33: Remove.onlyfrom line 33 — this prevents other tests from running.The
.onlymodifier causes Mocha to execute only that single test, skipping "stream should work" (lines 54–61) and "error stream should work" (lines 63–69). This modifier is a debugging tool that must be removed before committing.- it.only('Request should work', async () => { + it('Request should work', async () => {plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/package.json (1)
1-6: LGTM!The test fixture module metadata is properly configured with the module name and eggModule settings.
plugin/langchain/package.json (1)
1-92: LGTM!The package manifest is properly configured with appropriate dependencies, scripts, and metadata for the LangChain plugin.
plugin/langchain/test/fixtures/apps/langchain/config/plugin.js (1)
1-26: LGTM!The plugin configuration correctly enables all required Tegg plugins and properly computes the path for the LangChain plugin under test.
plugin/langchain/index.ts (1)
1-1: LGTM!Clean barrel export for the ChatOpenAI module.
plugin/langchain/test/llm.test.ts (1)
36-48: LGTM!The test properly mocks the ChatOpenAIModel and validates the HTTP endpoint integration.
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts (1)
1-26: LGTM!The controller properly demonstrates the use of
ChatModelQualifierfor injecting the ChatOpenAI model and exposes a clean HTTP endpoint for testing.
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/module.yml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugin/langchain/lib/ChatOpenAI.ts (1)
46-47: Consider verifying type compatibility instead of usingas any.Multiple
as anycasts bypass type checking. While sometimes necessary for library interop, verify that:
- The
fetchsignature is compatible with whatchatConfig.configurationexpects- The
chatConfigstructure matches theChatOpenAIconstructor parameterIf the types are genuinely incompatible and these casts are required, consider adding a comment explaining why they're necessary (similar to the existing comment on line 44).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugin/langchain/lib/ChatOpenAI.ts(1 hunks)plugin/langchain/package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugin/langchain/lib/ChatOpenAI.ts (6)
core/types/core-decorator/model/EggMultiInstancePrototypeInfo.ts (2)
MultiInstancePrototypeGetObjectsContext(12-16)ObjectInfo(6-10)plugin/langchain/lib/util.ts (2)
getClientNames(10-14)getChatModelConfig(16-24)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (2)
ChatModelInjectName(4-4)ChatModelQualifierAttribute(3-3)plugin/langchain/lib/ChatModelHelper.ts (1)
ChatModelHelper(6-15)plugin/langchain/lib/config/QualifierUtil.ts (1)
QualifierUtil(4-13)core/core-decorator/src/decorator/MultiInstanceInfo.ts (1)
MultiInstanceInfo(4-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (16)
🔇 Additional comments (2)
plugin/langchain/package.json (1)
1-91: ✓ Well-structured plugin manifest with proper Egg.js conventions.The
package.jsonis properly configured as an Egg.js tegg plugin with correct metadata, consistent versioning across the @eggjs ecosystem packages (all ^3.62.0), and appropriate npm scripts for TypeScript compilation and testing.A minor note for verification: koa-router appears only in
devDependencies(line 82) while@types/koa-routeris in maindependencies(line 66). This is likely intentional if koa-router is only needed for test fixtures or examples. If the plugin runtime doesn't reference it, this is fine; if it does, koa-router should move to main dependencies.plugin/langchain/lib/ChatOpenAI.ts (1)
18-36: Dynamic multi-instance initialization looks well-structured.The
getObjectsmethod correctly:
- Loads module configuration to determine required instances
- Maps each ChatModel client to an
ObjectInfowith proper qualifiers for dependency injection- Wires module config qualifiers to support per-module configuration
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugin/langchain/types.ts (2)
48-50: Remove redundant JSDoc comments.The JSDoc comments on lines 48-50 and 60-62 duplicate the descriptions already provided within the
Type.Optional()definitions. This creates unnecessary maintenance burden and inconsistency with other fields.Apply this diff to remove the redundant comments:
- /** - * List of stop words to use when generating. Alias for `stopSequences` - */ stop: Type.Optional(Type.Array(Type.String(), { description: 'List of stop words to use when generating. Alias for `stopSequences`', })),- /** - * API key to use when making requests to OpenAI. Defaults to the value of `OPENAI_API_KEY` environment variable. Alias for `apiKey` - */ openAIApiKey: Type.Optional(Type.String({ description: 'API key to use when making requests to OpenAI. Defaults to the value of `OPENAI_API_KEY` environment variable. Alias for `apiKey`', })),Also applies to: 60-62
128-130: Consider documenting the empty interface extension.The
Applicationinterface extension (lines 128-130) is syntactically correct but has an empty body. The actual properties are inherited fromModuleConfigApplication. For clarity, consider adding a comment explaining this or using a more explicit pattern.Optional: Add a clarifying comment:
export interface Application extends ModuleConfigApplication { - + // Inherits moduleReferences, moduleConfigs, and eggContainerFactory from ModuleConfigApplication }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugin/langchain/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugin/langchain/types.ts (2)
core/types/common/ModuleConfig.ts (1)
ModuleReference(1-6)core/runtime/src/factory/EggContainerFactory.ts (1)
EggContainerFactory(18-88)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-ubuntu (16)
🔇 Additional comments (2)
plugin/langchain/types.ts (2)
39-44: Clarify precedence for alias field pairs.The schema includes several alias pairs without clear precedence rules:
modelName(line 39) vsmodel(line 42)stop(line 51) vsstopSequences(line 54)openAIApiKey(line 63) vsapiKey(line 66)If both fields in a pair are provided, the behavior is ambiguous. Consider documenting which field takes precedence or adding runtime validation to prevent both from being set simultaneously.
Also applies to: 51-56, 63-68
110-110: LGTM!The type derivation using
Static<typeof ...>correctly extracts the TypeScript type from the runtime schema.
akitaSummer
force-pushed
the
feat/langchain
branch
from
fed2430 to
e6263f6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugin/langchain/lib/ChatOpenAI.ts (1)
45-45: Global singleton modified in instance constructor.This issue was previously flagged:
FetchFactory.setDispatchersets a global singleton but is called in the constructor of a multi-instance class. Each instance creation will reset the global dispatcher, potentially causing race conditions and interference with other code usingFetchFactory.
🧹 Nitpick comments (1)
plugin/langchain/lib/ChatOpenAI.ts (1)
46-47: Consider investigating proper types instead ofas anycasts.Multiple
as anytype assertions bypass TypeScript's type checking. While this may be a workaround for type definition mismatches between@langchain/openaiandurllib, it could hide type safety issues. Consider whether proper types can be used or if type definitions need adjustment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugin/langchain/lib/ChatOpenAI.ts(1 hunks)plugin/langchain/package.json(1 hunks)plugin/langchain/typings/index.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- plugin/langchain/typings/index.d.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- plugin/langchain/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
plugin/langchain/lib/ChatOpenAI.ts (7)
core/types/core-decorator/model/EggMultiInstancePrototypeInfo.ts (2)
MultiInstancePrototypeGetObjectsContext(12-16)ObjectInfo(6-10)core/common-util/src/ModuleConfig.ts (1)
ModuleConfigUtil(31-380)plugin/langchain/lib/util.ts (2)
getClientNames(10-14)getChatModelConfig(16-24)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (2)
ChatModelInjectName(4-4)ChatModelQualifierAttribute(3-3)plugin/langchain/lib/ChatModelHelper.ts (1)
ChatModelHelper(6-15)plugin/langchain/lib/config/QualifierUtil.ts (1)
QualifierUtil(4-13)core/core-decorator/src/decorator/MultiInstanceInfo.ts (1)
MultiInstanceInfo(4-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Analyze (javascript)
akitaSummer
force-pushed
the
feat/langchain
branch
2 times, most recently
from
6dc01b3 to
1f98b69
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/langchain-decorator/test/graph.test.ts(1 hunks)plugin/langchain/lib/ChatOpenAI.ts(1 hunks)plugin/langchain/package.json(1 hunks)plugin/langchain/typings/index.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- core/langchain-decorator/test/graph.test.ts
- plugin/langchain/typings/index.d.ts
- plugin/langchain/lib/ChatOpenAI.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
plugin/langchain/package.json (2)
57-70: Verify LangChain dependency status and update dependencies configuration.The plugin relies directly on @langchain/openai and indirectly on other LangChain packages through lib/util.ts imports, yet none of these dependencies are declared in package.json. The build may fail due to missing runtime dependencies or imports.
Additionally, the package is missing TypeScript type definitions for langchain packages and the schema validation library zod, which is commonly used with LangChain.
To resolve:
- Add langchain direct dependencies to package.json
- Add @types/langchain as devDependency
- Determine if zod should be added as a dependency
- Update peer dependencies as needed
35-40: No issues found—mocha is properly configured.A
.mocharc.ymlfile exists at the root level with complete TypeScript support configuration, including the test glob patterntest/**/*.test.ts, TypeScript extension, andts-node/registerfor runtime support. The npm script correctly inherits this configuration. Tests following the pattern exist atplugin/langchain/test/llm.test.ts.Likely an incorrect or invalid review comment.
akitaSummer
force-pushed
the
feat/langchain
branch
from
1f98b69 to
23d88e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
plugin/langchain/package.json (1)
23-33: Remove non-existent app outputs from the files array—issue remains unresolved.As flagged in the prior review, the
filesarray includesapp.js,app.d.ts,app/**/*.js, andapp/**/*.d.tswhich do not exist in this package. These entries will never be generated during the build process, resulting in an incomplete npm package declaration. The investigation confirmed that other plugins in the repo defineapp.tssource files, but this package lacks both source and output.Either remove these entries or add the corresponding
app.tsandapp/directory structure if app functionality is intended."files": [ "app.js", "app.d.ts", "index.js", "index.d.ts", "lib/**/*.js", "lib/**/*.d.ts", - "app/**/*.js", - "app/**/*.d.ts", "typings/*.d.ts" ],
🧹 Nitpick comments (3)
core/langchain-decorator/test/index.test.ts (1)
6-6: Consider fixing the test name grammar.The test name 'should success' should be 'should succeed' for correct grammar.
Apply this diff:
- it('should success', () => { + it('should succeed', () => {plugin/langchain/typings/index.d.ts (2)
48-52: Minor: Redundant comment above field definition.The comment block on lines 48-50 duplicates the description already provided in the Type.Optional schema on line 52. Consider removing the comment to reduce redundancy.
Apply this diff to remove the redundant comment:
modelKwargs: Type.Optional(Type.Record(Type.String(), Type.Any(), { description: 'Holds any additional parameters that are valid to pass to {@link https://platform.openai.com/docs/api-reference/completions/create | `openai.createCompletion`} that are not explicitly specified on this class.', })), - /** - * List of stop words to use when generating. Alias for `stopSequences` - */ stop: Type.Optional(Type.Array(Type.String(), { description: 'List of stop words to use when generating. Alias for `stopSequences`', })),
60-65: Minor: Redundant comment above field definition.The comment block on lines 60-62 duplicates the description already provided in the Type.Optional schema on line 64. Consider removing the comment to reduce redundancy.
Apply this diff to remove the redundant comment:
timeout: Type.Optional(Type.Number({ description: 'Timeout to use when making requests to OpenAI.', })), - /** - * API key to use when making requests to OpenAI. Defaults to the value of `OPENAI_API_KEY` environment variable. Alias for `apiKey` - */ openAIApiKey: Type.Optional(Type.String({ description: 'API key to use when making requests to OpenAI. Defaults to the value of `OPENAI_API_KEY` environment variable. Alias for `apiKey`', })),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
core/langchain-decorator/test/graph.test.ts(1 hunks)core/langchain-decorator/test/index.test.ts(1 hunks)plugin/langchain/lib/ChatOpenAI.ts(1 hunks)plugin/langchain/package.json(1 hunks)plugin/langchain/typings/index.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- plugin/langchain/lib/ChatOpenAI.ts
- core/langchain-decorator/test/graph.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
core/langchain-decorator/test/index.test.ts (1)
core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifierAttribute(3-3)
plugin/langchain/typings/index.d.ts (2)
core/types/common/ModuleConfig.ts (1)
ModuleReference(1-6)core/runtime/src/factory/EggContainerFactory.ts (1)
EggContainerFactory(18-88)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-macos (16)
🔇 Additional comments (5)
plugin/langchain/package.json (1)
66-66: Verify @types/koa-router placement in dependencies.Type definition packages (
@types/*) are typically dev-only dependencies since they're not needed at runtime.@types/koa-routeris listed in runtime dependencies (line 66) whilekoa-routeritself is in devDependencies (line 83). Confirm whether type definitions are actually required at runtime, or move@types/koa-routerto devDependencies for consistency with standard package practices.Also applies to: 83-83
core/langchain-decorator/test/index.test.ts (2)
1-3: LGTM!The imports are appropriate for testing the qualifier utility.
8-13: LGTM!The dynamic requires are appropriate here since the test is gated by Node version, and the test logic correctly validates the qualifier functionality.
plugin/langchain/typings/index.d.ts (2)
110-119: LGTM!The type export and module augmentation for
@eggjs/teggfollow standard TypeScript patterns. The optionalChatModelconfiguration is correctly integrated into the module config system.
121-131: Structure looks correct, but depends on missing imports.The module augmentation for
eggcorrectly extends theApplicationinterface with module configuration properties. However, this depends on the missing type imports (ModuleReferenceandModuleConfigHolder) flagged in the earlier comment.Note: Once the import issue is resolved, verify that the
ModuleConfigApplicationinterface properties align with the runtime implementation.
akitaSummer
force-pushed
the
feat/langchain
branch
from
23d88e0 to
927a5ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
plugin/langchain/package.json (1)
23-33: Remove non-existentappoutputs from the files array (unresolved critical issue).The
filesarray declaresapp.js,app.d.ts,app/**/*.js, andapp/**/*.d.ts(lines 24–25, 30–31), but this package has no correspondingapp.tssource file orapp/directory. These outputs will never be generated during the build, resulting in an incomplete npm package.Remove the four problematic entries:
"files": [ - "app.js", - "app.d.ts", "index.js", "index.d.ts", "lib/**/*.js", "lib/**/*.d.ts", - "app/**/*.js", - "app/**/*.d.ts", "typings/*.d.ts" ],Alternatively, if app functionality is needed, add
app.tsandapp/directory structure matching other Egg plugins in this repository.core/langchain-decorator/test/index.test.ts (1)
5-5: Version parsing is fragile for Node v100+.This issue has already been flagged in a previous review.
plugin/langchain/test/llm.test.ts (2)
7-7: Version parsing is fragile for Node v100+.This issue has already been flagged in a previous review.
12-14: Remove duplicateafterhook.This issue has already been flagged in a previous review.
Also applies to: 32-34
plugin/langchain/lib/ChatOpenAI.ts (1)
45-45: Global singleton modified in instance constructor.This issue has already been flagged in a previous review.
🧹 Nitpick comments (1)
plugin/langchain/lib/ChatOpenAI.ts (1)
46-47: Consider reducing reliance on type assertions.Multiple
as anyassertions bypass type safety and could hide type incompatibilities between the config structure and ChatOpenAI's expected types.If the types genuinely differ, consider creating an explicit type adapter or using more specific type assertions that document the known safe conversions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
core/langchain-decorator/test/graph.test.ts(1 hunks)core/langchain-decorator/test/index.test.ts(1 hunks)plugin/langchain/lib/ChatOpenAI.ts(1 hunks)plugin/langchain/package.json(1 hunks)plugin/langchain/test/llm.test.ts(1 hunks)plugin/langchain/typings/index.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- core/langchain-decorator/test/graph.test.ts
- plugin/langchain/typings/index.d.ts
🧰 Additional context used
🧬 Code graph analysis (2)
core/langchain-decorator/test/index.test.ts (1)
core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifierAttribute(3-3)
plugin/langchain/lib/ChatOpenAI.ts (6)
core/types/core-decorator/model/EggMultiInstancePrototypeInfo.ts (2)
MultiInstancePrototypeGetObjectsContext(12-16)ObjectInfo(6-10)core/common-util/src/ModuleConfig.ts (1)
ModuleConfigUtil(31-380)plugin/langchain/lib/util.ts (2)
getClientNames(10-14)getChatModelConfig(16-24)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (2)
ChatModelInjectName(4-4)ChatModelQualifierAttribute(3-3)plugin/langchain/lib/ChatModelHelper.ts (1)
ChatModelHelper(6-15)plugin/langchain/lib/config/QualifierUtil.ts (1)
QualifierUtil(4-13)
🪛 Biome (2.1.2)
plugin/langchain/test/llm.test.ts
[error] 32-34: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-macos (16)
akitaSummer
force-pushed
the
feat/langchain
branch
from
9ef0fba to
765d63a
Compare
There was a problem hiding this comment.
Actionable comments posted: 17
♻️ Duplicate comments (10)
core/langchain-decorator/src/decorator/GraphTool.ts (3)
1-1: Remove commented import.This commented-out import should be removed as it appears to be leftover code.
13-13: Verify if DynamicStructuredTool and ToolSchemaBase can be imported from the public API.Deep imports from
@langchain/core/toolscan be brittle and may break with future package updates.What is the public API for importing DynamicStructuredTool and ToolSchemaBase from @langchain/core package?
16-16: Rename parameter from "constructor" to avoid shadowing.The parameter name
constructorshadows the globalFunction.prototype.constructorproperty and triggers a lint error.Apply this diff:
- return (constructor: EggProtoImplClass<IGraphTool<ToolSchema>>) => { + return (target: EggProtoImplClass<IGraphTool<ToolSchema>>) => { const func = SingletonProto({ accessLevel: params?.accessLevel ?? AccessLevel.PUBLIC, name: params?.name, }); - func(constructor); - PrototypeUtil.setFilePath(constructor, StackUtil.getCalleeFromStack(false, 5)); + func(target); + PrototypeUtil.setFilePath(target, StackUtil.getCalleeFromStack(false, 5)); - GraphToolInfoUtil.setGraphToolMetadata(params, constructor); + GraphToolInfoUtil.setGraphToolMetadata(params, target); };core/langchain-decorator/src/decorator/Graph.ts (2)
13-13: Rename parameter from "constructor" to avoid shadowing.The parameter name
constructorshadows the globalFunction.prototype.constructorproperty and triggers a lint error.Apply this diff:
- return (constructor: EggProtoImplClass<AbstractStateGraph<N, S>>) => { + return (target: EggProtoImplClass<AbstractStateGraph<N, S>>) => { const func = SingletonProto({ accessLevel: params?.accessLevel ?? AccessLevel.PUBLIC, name: params?.name, }); - func(constructor); - PrototypeUtil.setFilePath(constructor, StackUtil.getCalleeFromStack(false, 5)); + func(target); + PrototypeUtil.setFilePath(target, StackUtil.getCalleeFromStack(false, 5)); - GraphInfoUtil.setGraphMetadata(params, constructor); + GraphInfoUtil.setGraphMetadata(params, target); };
26-26: Fix build() return type to preserve generics S and N.The current signature erases the generic parameters, which loses type information for consumers of this interface.
Apply this diff:
- build?(): Promise<CompiledStateGraph<StateType<StateDefinition>, UpdateType<StateDefinition>, string, StateDefinition, StateDefinition, StateDefinition> | undefined>; + build?(): Promise<CompiledStateGraph<StateType<S>, UpdateType<S>, N, S, S> | undefined>;plugin/langchain/typings/index.d.ts (1)
1-6: Critical: Missing type imports persist despite previous reviews.Lines 124-125 reference
ModuleReferenceandModuleConfigHolder, but these types are still not imported. This issue was flagged in previous reviews and marked as addressed in commit e6263f6, yet the imports remain missing. TypeScript compilation will fail without these imports.Apply this fix to add the required imports:
import { EggContainerFactory } from '@eggjs/tegg-runtime'; import { Type, Static } from '@eggjs/tegg/ajv'; +import type { ModuleReference, ModuleConfigHolder } from '@eggjs/tegg-types'; import 'egg'; import '@eggjs/tegg-plugin/typings'; import '@eggjs/tegg-controller-plugin/typings'; import '@eggjs/tegg-mcp-client/typings';If
@eggjs/tegg-typesis not the correct package path, verify the actual export location for these types and adjust the import accordingly.#!/bin/bash # Verify the correct import path for ModuleReference and ModuleConfigHolder # Find where these types are exported echo "=== Searching for ModuleReference export ===" rg -nP --type=ts 'export\s+(interface|type)\s+ModuleReference\b' -A2 echo "" echo "=== Searching for ModuleConfigHolder export ===" rg -nP --type=ts 'export\s+(interface|type)\s+ModuleConfigHolder\b' -A2 echo "" echo "=== Checking package.json exports in core/types ===" fd -e json package.json core/types -x cat {}core/langchain-decorator/src/decorator/GraphNode.ts (4)
1-1: Remove the commented-out import.This commented-out import should be removed for code clarity.
32-32: Remove commented-out code.This commented-out code should be removed to improve maintainability.
15-15: Update ToolNode import to stable public API.The import path
@langchain/langgraph/prebuiltis deprecated. Import ToolNode from the stable public API:-import { ToolNode } from '@langchain/langgraph/prebuilt'; +import { ToolNode } from 'langchain';Based on learnings from previous reviews and current LangChain documentation.
17-28: Rename parameter to avoid shadowing the global "constructor" property.The parameter name
constructoron line 18 shadows the built-in global property. Rename to avoid confusion:export function GraphNode<S extends StateDefinition = StateDefinition>(params: IGraphNodeMetadata) { - return (constructor: EggProtoImplClass<IGraphNode<S> | TeggToolNode>) => { + return (clazz: EggProtoImplClass<IGraphNode<S> | TeggToolNode>) => { const func = SingletonProto({ accessLevel: params?.accessLevel ?? AccessLevel.PUBLIC, name: params?.name, }); - func(constructor); - PrototypeUtil.setFilePath(constructor, StackUtil.getCalleeFromStack(false, 5)); + func(clazz); + PrototypeUtil.setFilePath(clazz, StackUtil.getCalleeFromStack(false, 5)); - GraphNodeInfoUtil.setGraphNodeMetadata(params, constructor); + GraphNodeInfoUtil.setGraphNodeMetadata(params, clazz); }; }🧰 Tools
🪛 Biome (2.1.2)
[error] 18-18: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🧹 Nitpick comments (7)
core/langchain-decorator/src/qualifier/ChatCheckpointSaverQualifier.ts (1)
6-10: Solid implementation following established patterns.The decorator factory correctly implements the qualifier pattern used throughout the codebase. Consider adding JSDoc to improve discoverability:
+/** + * Decorator to qualify a ChatCheckpointSaver injection point with a specific name. + * @param chatCheckpointSaverName - The name of the checkpoint saver instance to inject + * @returns A parameter/property decorator + * @example + * ```ts + * class MyGraph { + * constructor(@ChatCheckpointSaverQualifier('my-saver') saver: ChatCheckpointSaver) {} + * } + * ``` + */ export function ChatCheckpointSaverQualifier(chatCheckpointSaverName: string) {Optionally, consider validating the input parameter for defensive coding:
export function ChatCheckpointSaverQualifier(chatCheckpointSaverName: string) { if (!chatCheckpointSaverName || typeof chatCheckpointSaverName !== 'string') { throw new Error('chatCheckpointSaverName must be a non-empty string'); } return function(target: any, propertyKey?: PropertyKey, parameterIndex?: number) { QualifierUtil.addInjectQualifier(target, propertyKey, parameterIndex, ChatCheckpointSaverQualifierAttribute, chatCheckpointSaverName); }; }core/langchain-decorator/src/decorator/Graph.ts (1)
31-37: Consider removing unused phantom fields.The private fields
_namesand_stateare never used and require@ts-ignoredirectives. TypeScript doesn't require phantom fields to preserve generic type parameters at the instance level—the generic parameters on the class itself are sufficient.Apply this diff to remove the phantom fields:
export abstract class AbstractStateGraph<N extends string = '', S extends StateDefinition = StateDefinition> extends StateGraph<S, AnnotationRoot<S>['State'], UpdateType<S>, N> implements IGraph<N, S> { - - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - private _names: N; - - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - private _state: S; }core/mcp-client/src/MCPClientQualifier.ts (1)
27-28: Fix misleading error message text.This branch is resolving MCP client configuration, but the thrown message still says “ChatModel”. Please update it so on-call engineers aren’t debugging the wrong subsystem.
- throw new Error(`not found ChatModel config for ${mcpClientName}`); + throw new Error(`not found MCP client config for ${mcpClientName}`);plugin/langchain/lib/graph/CompiledStateGraphProto.ts (1)
46-49: Fix verifyQualifier logic.The current logic has a subtle bug: when no matching qualifier is found,
selfQualifiersisundefined, andundefined?.valueevaluates toundefined, which will never equalqualifier.value(returningfalsecorrectly). However, this comparison is fragile and unclear.Apply this diff for clearer logic:
verifyQualifier(qualifier: QualifierInfo): boolean { const selfQualifiers = this.qualifiers.find(t => t.attribute === qualifier.attribute); - return selfQualifiers?.value === qualifier.value; + return selfQualifiers !== undefined && selfQualifiers.value === qualifier.value; }plugin/langchain/lib/boundModel/BoundModelObjectHook.ts (2)
30-49: Add error handling for tool loading failures.The
findGraphToolsmethod loads tools but doesn't handle potential errors. IfEggContainerFactory.getOrCreateEggObjectFromClazzfails or metadata is missing, the error will propagate uncaught.Consider adding try-catch blocks or validation:
async findGraphTools() { const tools = this.boundModelMetadata.tools ?? []; let dTools: Parameters<ConfigurableModel['bindTools']>['0'] = []; for (let i = 0; i < tools.length; i++) { + try { const toolsObj = await EggContainerFactory.getOrCreateEggObjectFromClazz(tools[i]); const toolMetadata = GraphToolInfoUtil.getGraphToolMetadata((toolsObj.proto as unknown as EggPrototypeWithClazz).clazz!); const ToolDetail = MCPInfoUtil.getMCPToolArgsIndex((toolsObj.proto as unknown as EggPrototypeWithClazz).clazz!, 'execute'); if (toolMetadata && ToolDetail) { const tool = new DynamicStructuredTool({ description: toolMetadata.description, name: toolMetadata.toolName, func: (toolsObj.obj as unknown as IGraphTool<any>).execute.bind(toolsObj.obj), schema: z.object(ToolDetail.argsSchema) as any, }); dTools = dTools.concat(tool); } + } catch (error) { + throw new Error(`Failed to load graph tool ${tools[i].name}: ${error.message}`); + } }
30-67: Consider extracting shared tool-loading logic.The methods
findGraphToolsandfindMcpServerToolsin this file are nearly identical to those inplugin/langchain/lib/graph/GraphObjectHook.ts. This code duplication makes maintenance harder and increases the risk of divergence.Consider extracting the shared tool-loading logic into a utility class that both
BoundModelObjectHookandGraphObjectHookcan use.plugin/langchain/lib/graph/GraphObjectHook.ts (1)
86-109: Document the property hijacking pattern.Lines 103-105 use
Object.definePropertyto hijack the injectedchatModelproperty with a getter that returns the bound model. While this approach is less destructive thansetPrototypeOf, it's still a form of runtime mutation that could be surprising to developers.Consider adding a comment explaining why this pattern is used and its implications:
+ // Hijack the chatModel property to return the bound version with tools + // This allows the node to use this[injectObject.objName] and get the bound model Object.defineProperty(nodeObj, injectObject.objName, { get: () => boundChatModel, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (66)
.gitignore(1 hunks)core/langchain-decorator/index.ts(1 hunks)core/langchain-decorator/src/decorator/Graph.ts(1 hunks)core/langchain-decorator/src/decorator/GraphNode.ts(1 hunks)core/langchain-decorator/src/decorator/GraphTool.ts(1 hunks)core/langchain-decorator/src/qualifier/ChatCheckpointSaverQualifier.ts(1 hunks)core/langchain-decorator/tsconfig.json(1 hunks)core/langchain-decorator/tsconfig.pub.json(1 hunks)core/mcp-client/CHANGELOG.md(1 hunks)core/mcp-client/README.md(1 hunks)core/mcp-client/index.ts(1 hunks)core/mcp-client/package.json(1 hunks)core/mcp-client/src/HeaderUtil.ts(1 hunks)core/mcp-client/src/HttpMCPClient.ts(1 hunks)core/mcp-client/src/MCPClientQualifier.ts(1 hunks)core/mcp-client/test/HttpMCPClient.test.ts(1 hunks)core/mcp-client/test/fixtures/sse-mcp-server/http.ts(1 hunks)core/mcp-client/test/fixtures/streamable-mcp-server/http.ts(1 hunks)core/mcp-client/tsconfig.json(1 hunks)core/mcp-client/tsconfig.pub.json(1 hunks)core/mcp-client/typings/index.d.ts(1 hunks)plugin/langchain/README.md(1 hunks)plugin/langchain/app.ts(1 hunks)plugin/langchain/lib/ChatOpenAI.ts(1 hunks)plugin/langchain/lib/boundModel/BoundModelObjectHook.ts(1 hunks)plugin/langchain/lib/graph/CompiledStateGraphObject.ts(1 hunks)plugin/langchain/lib/graph/CompiledStateGraphProto.ts(1 hunks)plugin/langchain/lib/graph/GraphBuildHook.ts(1 hunks)plugin/langchain/lib/graph/GraphLoadUnitHook.ts(1 hunks)plugin/langchain/lib/graph/GraphObjectHook.ts(1 hunks)plugin/langchain/lib/graph/GraphPrototypeHook.ts(1 hunks)plugin/langchain/lib/tracing/LangGraphTracer.ts(1 hunks)plugin/langchain/package.json(1 hunks)plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts(1 hunks)plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/module.yml(1 hunks)plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/service/BoundChatModel.ts(1 hunks)plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/service/Graph.ts(1 hunks)plugin/langchain/test/fixtures/apps/langchain/config/module.json(1 hunks)plugin/langchain/test/fixtures/apps/langchain/config/plugin.js(1 hunks)plugin/langchain/test/fixtures/sse-mcp-server/http.ts(1 hunks)plugin/langchain/test/llm.test.ts(1 hunks)plugin/langchain/tsconfig.json(1 hunks)plugin/langchain/tsconfig.pub.json(1 hunks)plugin/langchain/typings/index.d.ts(1 hunks)plugin/mcp-client/CHANGELOG.md(1 hunks)plugin/mcp-client/README.md(1 hunks)plugin/mcp-client/index.ts(1 hunks)plugin/mcp-client/lib/EggHttpMCPClient.ts(1 hunks)plugin/mcp-client/lib/EggHttpStaticMCPClient.ts(1 hunks)plugin/mcp-client/lib/HttpMCPClientFactory.ts(1 hunks)plugin/mcp-client/lib/QualifierUtil.ts(1 hunks)plugin/mcp-client/lib/constants.ts(1 hunks)plugin/mcp-client/package.json(1 hunks)plugin/mcp-client/test/fixtures/apps/mcpclient/app/modules/bar/controller/AppController.ts(1 hunks)plugin/mcp-client/test/fixtures/apps/mcpclient/app/modules/bar/module.yml(1 hunks)plugin/mcp-client/test/fixtures/apps/mcpclient/app/modules/bar/package.json(1 hunks)plugin/mcp-client/test/fixtures/apps/mcpclient/config/config.default.js(1 hunks)plugin/mcp-client/test/fixtures/apps/mcpclient/config/module.json(1 hunks)plugin/mcp-client/test/fixtures/apps/mcpclient/config/plugin.js(1 hunks)plugin/mcp-client/test/fixtures/apps/mcpclient/package.json(1 hunks)plugin/mcp-client/test/fixtures/sse-mcp-server/http.ts(1 hunks)plugin/mcp-client/test/fixtures/streamable-mcp-server/http.ts(1 hunks)plugin/mcp-client/test/mcpclient.test.ts(1 hunks)plugin/mcp-client/tsconfig.json(1 hunks)plugin/mcp-client/tsconfig.pub.json(1 hunks)plugin/mcp-client/typings/index.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- core/mcp-client/README.md
- plugin/mcp-client/CHANGELOG.md
- core/mcp-client/tsconfig.json
- plugin/mcp-client/tsconfig.pub.json
- plugin/mcp-client/test/fixtures/apps/mcpclient/package.json
- core/langchain-decorator/tsconfig.json
- plugin/mcp-client/test/fixtures/apps/mcpclient/app/modules/bar/package.json
- plugin/mcp-client/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- core/langchain-decorator/tsconfig.pub.json
- plugin/langchain/test/fixtures/apps/langchain/config/module.json
- plugin/langchain/README.md
- plugin/langchain/test/fixtures/apps/langchain/config/plugin.js
- core/langchain-decorator/index.ts
🧰 Additional context used
🧬 Code graph analysis (34)
core/mcp-client/typings/index.d.ts (1)
plugin/langchain/typings/index.d.ts (1)
ModuleConfig(118-119)
core/langchain-decorator/src/decorator/GraphNode.ts (5)
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/service/Graph.ts (3)
GraphNode(53-88)GraphNode(90-94)SingletonProto(20-21)core/langchain-decorator/src/model/GraphNodeMetadata.ts (2)
IGraphNodeMetadata(3-7)constructor(14-18)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
GraphNodeInfoUtil(6-14)
plugin/langchain/typings/index.d.ts (2)
core/types/common/ModuleConfig.ts (1)
ModuleReference(1-6)core/runtime/src/factory/EggContainerFactory.ts (1)
EggContainerFactory(18-88)
core/mcp-client/src/HeaderUtil.ts (5)
plugin/langchain/test/fixtures/sse-mcp-server/http.ts (1)
headers(36-36)plugin/mcp-client/test/fixtures/sse-mcp-server/http.ts (1)
headers(36-36)plugin/mcp-client/test/fixtures/streamable-mcp-server/http.ts (1)
headers(35-35)core/mcp-client/test/fixtures/sse-mcp-server/http.ts (1)
headers(35-35)core/mcp-client/test/fixtures/streamable-mcp-server/http.ts (1)
headers(34-34)
core/mcp-client/src/HttpMCPClient.ts (1)
core/mcp-client/src/HeaderUtil.ts (1)
mergeHeaders(3-13)
plugin/langchain/lib/graph/GraphLoadUnitHook.ts (7)
core/types/lifecycle/LifecycleHook.ts (1)
LifecycleHook(12-19)core/metadata/src/factory/EggPrototypeFactory.ts (1)
EggPrototypeFactory(7-60)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/model/GraphMetadata.ts (1)
IGraphMetadata(4-8)plugin/langchain/lib/graph/CompiledStateGraphProto.ts (1)
CompiledStateGraphProto(17-64)plugin/tegg/app/extend/application.ts (1)
eggPrototypeFactory(29-31)core/langchain-decorator/src/util/GraphInfoUtil.ts (1)
GraphInfoUtil(6-21)
plugin/mcp-client/lib/EggHttpMCPClient.ts (3)
core/mcp-client/src/HttpMCPClient.ts (2)
HttpClientOptions(25-25)HttpMCPClient(27-116)core/mcp-client/src/HeaderUtil.ts (1)
mergeHeaders(3-13)core/runtime/src/model/ContextHandler.ts (1)
ContextHandler(6-19)
plugin/langchain/lib/boundModel/BoundModelObjectHook.ts (10)
core/langchain-decorator/src/model/BoundModelMetadata.ts (2)
IBoundModelMetadata(3-7)BoundModelMetadata(9-19)core/runtime/src/factory/EggContainerFactory.ts (1)
EggContainerFactory(18-88)core/langchain-decorator/src/util/GraphToolInfoUtil.ts (1)
GraphToolInfoUtil(6-14)core/controller-decorator/src/util/MCPInfoUtil.ts (1)
MCPInfoUtil(42-166)core/langchain-decorator/src/decorator/GraphTool.ts (1)
IGraphTool(28-31)core/mcp-client/src/MCPClientQualifier.ts (2)
MCPClientInjectName(5-5)MCPClientQualifierAttribute(4-4)core/langchain-decorator/src/decorator/GraphNode.ts (1)
IGraphNode(30-36)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (2)
ChatModelInjectName(4-4)ChatModelQualifierAttribute(3-3)core/types/lifecycle/LifecycleHook.ts (1)
LifecycleHook(12-19)core/langchain-decorator/src/type/metadataKey.ts (1)
BOUND_MODEL_METADATA(6-6)
plugin/langchain/lib/graph/CompiledStateGraphProto.ts (4)
core/types/metadata/model/EggPrototype.ts (2)
EggPrototype(117-154)InjectObjectProto(15-36)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggPrototypeName(6-6)core/langchain-decorator/src/model/GraphMetadata.ts (1)
GraphMetadata(10-20)core/lifecycle/src/IdenticalObject.ts (1)
IdenticalUtil(3-39)
plugin/mcp-client/test/fixtures/apps/mcpclient/app/modules/bar/controller/AppController.ts (3)
core/mcp-client/src/MCPClientQualifier.ts (2)
MCPClientQualifier(7-12)MCPClientInjectName(5-5)core/mcp-client/src/HttpMCPClient.ts (1)
HttpMCPClient(27-116)core/controller-decorator/src/decorator/http/HTTPMethod.ts (1)
HTTPMethod(7-23)
plugin/langchain/lib/graph/GraphObjectHook.ts (10)
core/langchain-decorator/src/model/GraphNodeMetadata.ts (1)
GraphNodeMetadata(9-19)core/runtime/src/factory/EggContainerFactory.ts (1)
EggContainerFactory(18-88)core/langchain-decorator/src/util/GraphToolInfoUtil.ts (1)
GraphToolInfoUtil(6-14)core/controller-decorator/src/util/MCPInfoUtil.ts (1)
MCPInfoUtil(42-166)core/langchain-decorator/src/decorator/GraphTool.ts (1)
IGraphTool(28-31)core/mcp-client/src/MCPClientQualifier.ts (2)
MCPClientInjectName(5-5)MCPClientQualifierAttribute(4-4)core/langchain-decorator/src/decorator/GraphNode.ts (2)
IGraphNode(30-36)TeggToolNode(38-44)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (2)
ChatModelQualifierAttribute(3-3)ChatModelInjectName(4-4)core/types/lifecycle/LifecycleHook.ts (1)
LifecycleHook(12-19)core/langchain-decorator/src/type/metadataKey.ts (1)
GRAPH_NODE_METADATA(3-3)
plugin/mcp-client/lib/HttpMCPClientFactory.ts (2)
core/mcp-client/src/HttpMCPClient.ts (1)
HttpClientOptions(25-25)plugin/mcp-client/lib/EggHttpMCPClient.ts (1)
EggHttpMCPClient(24-118)
plugin/mcp-client/lib/EggHttpStaticMCPClient.ts (7)
plugin/langchain/lib/ChatOpenAI.ts (1)
MultiInstanceProto(18-52)core/types/core-decorator/model/EggMultiInstancePrototypeInfo.ts (2)
MultiInstancePrototypeGetObjectsContext(12-16)ObjectInfo(6-10)core/common-util/src/ModuleConfig.ts (1)
ModuleConfigUtil(31-380)plugin/mcp-client/lib/QualifierUtil.ts (1)
QualifierUtil(4-13)core/mcp-client/src/MCPClientQualifier.ts (4)
MCPClientInjectName(5-5)MCPClientQualifierAttribute(4-4)getMCPClientName(17-21)getMCPClientConfig(23-30)plugin/mcp-client/lib/EggHttpMCPClient.ts (1)
EggHttpMCPClient(24-118)core/lifecycle/src/decorator/index.ts (1)
LifecycleInit(28-28)
plugin/mcp-client/lib/QualifierUtil.ts (1)
core/types/core-decorator/enum/Qualifier.ts (1)
ConfigSourceQualifierAttribute(1-1)
plugin/mcp-client/test/fixtures/apps/mcpclient/config/plugin.js (1)
plugin/langchain/test/fixtures/apps/langchain/config/plugin.js (1)
path(4-4)
plugin/mcp-client/test/mcpclient.test.ts (2)
plugin/mcp-client/test/fixtures/streamable-mcp-server/http.ts (2)
startStreamableServer(38-89)stopStreamableServer(91-93)plugin/mcp-client/test/fixtures/sse-mcp-server/http.ts (2)
startSSEServer(39-68)stopSSEServer(70-72)
plugin/langchain/test/fixtures/sse-mcp-server/http.ts (2)
plugin/mcp-client/test/fixtures/sse-mcp-server/http.ts (4)
headers(36-36)httpServer(38-38)startSSEServer(39-68)stopSSEServer(70-72)core/mcp-client/test/fixtures/sse-mcp-server/http.ts (4)
headers(35-35)httpServer(37-37)startSSEServer(38-67)stopSSEServer(69-71)
plugin/langchain/lib/graph/GraphPrototypeHook.ts (9)
core/types/lifecycle/LifecycleHook.ts (1)
LifecycleHook(12-19)core/types/metadata/model/EggPrototype.ts (2)
EggPrototypeLifecycleContext(111-115)EggPrototype(117-154)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
GraphNodeInfoUtil(6-14)core/mcp-client/src/MCPClientQualifier.ts (2)
MCPClientInjectName(5-5)MCPClientQualifierAttribute(4-4)core/langchain-decorator/src/util/GraphToolInfoUtil.ts (1)
GraphToolInfoUtil(6-14)core/langchain-decorator/src/util/GraphInfoUtil.ts (1)
GraphInfoUtil(6-21)core/langchain-decorator/src/qualifier/ChatCheckpointSaverQualifier.ts (2)
ChatCheckpointSaverInjectName(4-4)ChatCheckpointSaverQualifierAttribute(3-3)core/langchain-decorator/src/util/GraphEdgeInfoUtil.ts (1)
GraphEdgeInfoUtil(6-14)
core/mcp-client/test/HttpMCPClient.test.ts (3)
core/mcp-client/test/fixtures/streamable-mcp-server/http.ts (2)
startStreamableServer(37-88)stopStreamableServer(90-92)core/mcp-client/src/HttpMCPClient.ts (1)
HttpMCPClient(27-116)core/mcp-client/test/fixtures/sse-mcp-server/http.ts (2)
startSSEServer(38-67)stopSSEServer(69-71)
core/mcp-client/src/MCPClientQualifier.ts (4)
plugin/mcp-client/lib/QualifierUtil.ts (1)
QualifierUtil(4-13)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/mcp-client/typings/index.d.ts (1)
ModuleConfig(37-38)core/types/core-decorator/model/EggMultiInstancePrototypeInfo.ts (1)
ObjectInfo(6-10)
plugin/langchain/lib/tracing/LangGraphTracer.ts (3)
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/service/Graph.ts (1)
SingletonProto(20-21)plugin/mcp-client/lib/HttpMCPClientFactory.ts (1)
SingletonProto(14-36)core/tegg/index.ts (1)
Logger(11-11)
plugin/mcp-client/test/fixtures/streamable-mcp-server/http.ts (3)
plugin/langchain/test/fixtures/sse-mcp-server/http.ts (2)
headers(36-36)httpServer(38-38)plugin/mcp-client/test/fixtures/sse-mcp-server/http.ts (2)
headers(36-36)httpServer(38-38)core/mcp-client/test/fixtures/streamable-mcp-server/http.ts (4)
headers(34-34)httpServer(36-36)startStreamableServer(37-88)stopStreamableServer(90-92)
core/mcp-client/test/fixtures/streamable-mcp-server/http.ts (3)
plugin/langchain/test/fixtures/sse-mcp-server/http.ts (2)
headers(36-36)httpServer(38-38)plugin/mcp-client/test/fixtures/sse-mcp-server/http.ts (2)
headers(36-36)httpServer(38-38)plugin/mcp-client/test/fixtures/streamable-mcp-server/http.ts (4)
headers(35-35)httpServer(37-37)startStreamableServer(38-89)stopStreamableServer(91-93)
plugin/langchain/lib/ChatOpenAI.ts (7)
core/types/core-decorator/model/EggMultiInstancePrototypeInfo.ts (2)
MultiInstancePrototypeGetObjectsContext(12-16)ObjectInfo(6-10)core/common-util/src/ModuleConfig.ts (1)
ModuleConfigUtil(31-380)plugin/langchain/lib/util.ts (2)
getClientNames(10-14)getChatModelConfig(16-24)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (2)
ChatModelInjectName(4-4)ChatModelQualifierAttribute(3-3)plugin/langchain/lib/ChatModelHelper.ts (1)
ChatModelHelper(6-15)plugin/langchain/lib/config/QualifierUtil.ts (1)
QualifierUtil(4-13)core/core-decorator/src/decorator/MultiInstanceInfo.ts (1)
MultiInstanceInfo(4-9)
plugin/langchain/lib/graph/CompiledStateGraphObject.ts (13)
core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggPrototypeName(6-6)plugin/langchain/lib/graph/CompiledStateGraphProto.ts (1)
CompiledStateGraphProto(17-64)core/types/controller-decorator/model/types.ts (1)
EggContext(3-3)core/langchain-decorator/src/model/GraphMetadata.ts (1)
GraphMetadata(10-20)core/types/core-decorator/model/InjectObjectInfo.ts (1)
EggObjectName(1-1)core/runtime/src/model/ContextHandler.ts (1)
ContextHandler(6-19)core/lifecycle/src/IdenticalObject.ts (1)
IdenticalUtil(3-39)core/runtime/src/factory/EggContainerFactory.ts (1)
EggContainerFactory(18-88)core/langchain-decorator/src/qualifier/ChatCheckpointSaverQualifier.ts (2)
ChatCheckpointSaverInjectName(4-4)ChatCheckpointSaverQualifierAttribute(3-3)core/langchain-decorator/src/decorator/GraphNode.ts (2)
IGraphNode(30-36)TeggToolNode(38-44)core/langchain-decorator/src/model/GraphNodeMetadata.ts (1)
GraphNodeMetadata(9-19)core/langchain-decorator/src/type/metadataKey.ts (2)
GRAPH_NODE_METADATA(3-3)GRAPH_EDGE_METADATA(2-2)core/langchain-decorator/src/decorator/GraphEdge.ts (1)
IGraphEdge(42-44)
plugin/langchain/lib/graph/GraphBuildHook.ts (2)
core/metadata/src/model/graph/GlobalGraph.ts (1)
GlobalGraph(38-272)core/metadata/src/model/ProtoDescriptor/ClassProtoDescriptor.ts (1)
ClassProtoDescriptor(10-38)
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/controller/AppController.ts (5)
plugin/mcp-client/test/fixtures/apps/mcpclient/app/modules/bar/controller/AppController.ts (1)
HTTPController(15-66)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifier(6-10)core/langchain-decorator/src/decorator/BoundModel.ts (1)
TeggBoundModel(30-30)core/langchain-decorator/src/decorator/Graph.ts (1)
TeggCompiledStateGraph(40-40)core/controller-decorator/src/decorator/http/HTTPMethod.ts (1)
HTTPMethod(7-23)
plugin/mcp-client/test/fixtures/sse-mcp-server/http.ts (2)
plugin/langchain/test/fixtures/sse-mcp-server/http.ts (4)
headers(36-36)httpServer(38-38)startSSEServer(39-68)stopSSEServer(70-72)core/mcp-client/test/fixtures/sse-mcp-server/http.ts (4)
headers(35-35)httpServer(37-37)startSSEServer(38-67)stopSSEServer(69-71)
plugin/langchain/app.ts (9)
plugin/langchain/typings/index.d.ts (1)
Application(129-131)plugin/langchain/lib/graph/GraphObjectHook.ts (1)
GraphObjectHook(113-124)plugin/langchain/lib/graph/GraphLoadUnitHook.ts (1)
GraphLoadUnitHook(11-53)plugin/langchain/lib/boundModel/BoundModelObjectHook.ts (1)
BoundModelObjectHook(86-97)plugin/langchain/lib/graph/GraphPrototypeHook.ts (1)
GraphPrototypeHook(7-266)plugin/langchain/lib/graph/CompiledStateGraphProto.ts (1)
CompiledStateGraphProto(17-64)plugin/langchain/lib/graph/CompiledStateGraphObject.ts (1)
CompiledStateGraphObject(15-130)core/metadata/src/model/graph/GlobalGraph.ts (1)
GlobalGraph(38-272)plugin/langchain/lib/graph/GraphBuildHook.ts (1)
GraphBuildHook(5-29)
core/mcp-client/test/fixtures/sse-mcp-server/http.ts (1)
plugin/langchain/test/fixtures/sse-mcp-server/http.ts (4)
headers(36-36)httpServer(38-38)startSSEServer(39-68)stopSSEServer(70-72)
core/langchain-decorator/src/decorator/Graph.ts (4)
core/langchain-decorator/src/model/GraphMetadata.ts (2)
IGraphMetadata(4-8)constructor(15-19)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphInfoUtil.ts (1)
GraphInfoUtil(6-21)
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/service/Graph.ts (6)
core/langchain-decorator/src/decorator/GraphTool.ts (2)
GraphTool(15-26)IGraphTool(28-31)core/controller-decorator/src/decorator/mcp/MCPTool.ts (1)
ToolArgsSchema(35-52)core/types/controller-decorator/MCPToolParams.ts (1)
ToolArgs(5-5)core/langchain-decorator/src/decorator/GraphNode.ts (3)
GraphNode(17-28)IGraphNode(30-36)TeggToolNode(38-44)core/langchain-decorator/src/decorator/GraphEdge.ts (2)
GraphStateType(38-38)IGraphEdge(42-44)core/langchain-decorator/src/decorator/Graph.ts (1)
Graph(12-23)
core/langchain-decorator/src/decorator/GraphTool.ts (5)
plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/service/Graph.ts (2)
GraphTool(41-51)SingletonProto(20-21)core/langchain-decorator/src/model/GraphToolMetadata.ts (2)
IGraphToolMetadata(3-7)constructor(13-15)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphToolInfoUtil.ts (1)
GraphToolInfoUtil(6-14)
plugin/langchain/test/llm.test.ts (1)
plugin/langchain/test/fixtures/sse-mcp-server/http.ts (2)
startSSEServer(39-68)stopSSEServer(70-72)
🪛 Biome (2.1.2)
core/langchain-decorator/src/decorator/GraphNode.ts
[error] 18-18: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugin/mcp-client/test/mcpclient.test.ts
[error] 26-36: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
[error] 38-40: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
core/langchain-decorator/src/decorator/Graph.ts
[error] 13-13: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
core/langchain-decorator/src/decorator/GraphTool.ts
[error] 16-16: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugin/langchain/test/llm.test.ts
[error] 28-38: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
[error] 40-42: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🪛 markdownlint-cli2 (0.18.1)
core/mcp-client/CHANGELOG.md
873-873: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
900-900: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
911-911: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
938-938: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
963-963: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
991-991: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Runner-macos (20)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (typescript)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (26)
core/mcp-client/tsconfig.pub.json (1)
1-12: Configuration looks good.Standard TypeScript publish config with appropriate settings: extends root config, reasonable compiler options, and proper exclusions for build artifacts, dependencies, and tests.
.gitignore (1)
38-45: Pattern additions look appropriate for new packages.The negated patterns correctly create exceptions to the global JS and
.d.tsignore rules on lines 23–26, selectively un-ignoring generated TypeScript definitions and config files in the relevant directories. The patterns follow existing conventions in the file and are appropriately scoped tocore/*/typings/*.d.ts,plugin/*/test/fixtures/apps/*/config/*.js,plugin/*/test/fixtures/apps/**/typings/*.d.ts, andbenchmark/**/*.js.plugin/langchain/tsconfig.json (1)
1-12: Configuration looks good.The TypeScript config follows the established monorepo patterns for development builds, with appropriate module resolution settings (
nodenextfor modern Node.js).plugin/langchain/tsconfig.pub.json (1)
1-12: Configuration is well structured for publishing.The publish-oriented config appropriately excludes the output directory (
dist) and specifiesoutDir, preventing unnecessary recompilation of artifacts. This follows the established publish-build pattern across the monorepo.core/langchain-decorator/src/qualifier/ChatCheckpointSaverQualifier.ts (2)
1-1: LGTM!Clean import of the DI framework utility.
3-4: LGTM!The exported constants follow a clean, consistent naming convention and properly define the qualifier's identity in the DI system.
plugin/langchain/typings/index.d.ts (3)
8-109: Comprehensive ChatModel configuration schema looks good.The schema covers all essential ChatModel parameters including temperature, token limits, penalties, streaming options, API configuration, and retry logic. The structure is well-organized and descriptions are clear.
111-111: LGTM!Standard type extraction from the ajv schema.
113-132: Module augmentation structure is appropriate.The augmentation pattern properly extends both
@eggjs/teggandeggmodules to add LangChain-specific configuration and application properties. The approach of definingLangChainModuleConfigseparately and then extendingModuleConfigis clean and maintainable.Note: The missing imports flagged separately must be resolved for these augmentations to compile.
plugin/mcp-client/package.json (4)
1-92: Overall package structure looks well-aligned with monorepo conventions.The manifest follows standard Egg.js/Tegg plugin patterns: consistent version pinning across dependencies (3.62.0), proper Egg plugin/module configuration, TypeScript support enabled, and standard build/publish workflows. The Node >=18.0.0 requirement is appropriate for modern TypeScript.
23-33: TypeScript configuration files verified and properly configured.Both
tsconfig.jsonandtsconfig.pub.jsonexist inplugin/mcp-client/and are correctly configured. Each file extends the parenttsconfig.json, setsbaseUrlto./, and appropriately excludesnode_modulesandtestdirectories.
57-70: @eggjs/mcp-client dependency verified and properly configured.The core package
@eggjs/[email protected]exists incore/mcp-clientwith correct versioning. The files array comprehensively covers all compiled TypeScript outputs (lib, app, typings directories). Langchain is appropriately excluded—plugin/langchainexists as a separate plugin (@eggjs/tegg-langchain), maintaining correct architectural separation. No additional dependencies are needed.
2-22: No changes required — the package design is correct.The review comment's premise is based on an incorrect assumption. The actual dependency flow is the inverse of what was suggested:
plugin/langchaindepends on@eggjs/tegg-mcp-client(as confirmed in its package.json), not the other way around.
plugin/mcp-clientis intentionally standalone (confirmed by zero langchain references in its source), providing a generic egg plugin wrapper for the core MCP client library.plugin/langchainis then a consumer of that plugin, using MCP client capabilities for LangGraph integration. This is the correct separation of concerns—mcp-client should remain independent to avoid circular dependencies and allow use in non-langchain contexts.plugin/mcp-client/test/fixtures/apps/mcpclient/config/module.json (1)
1-7: LGTM!The module configuration is straightforward and correctly structured for a test fixture.
plugin/mcp-client/index.ts (1)
1-3: LGTM!Clean barrel export pattern establishing the public API surface for the MCP client plugin.
plugin/mcp-client/test/fixtures/apps/mcpclient/app/modules/bar/module.yml (1)
1-14: LGTM!The MCP client configurations for both SSE and STREAMABLE_HTTP transports are correctly defined and align with the test suite expectations.
plugin/mcp-client/test/fixtures/apps/mcpclient/config/config.default.js (1)
1-16: LGTM!Standard test configuration appropriately disabling security features for the test environment.
core/mcp-client/index.ts (1)
1-3: LGTM!Clean barrel export pattern for the core MCP client public API.
plugin/mcp-client/test/mcpclient.test.ts (1)
42-130: LGTM!The three test cases comprehensively validate SSE, Streamable HTTP, and factory-based MCP client functionality with proper assertion of the expected tool schema.
core/mcp-client/test/HttpMCPClient.test.ts (1)
1-50: LGTM!Well-structured tests that properly validate both STREAMABLE_HTTP and SSE transport mechanisms with appropriate server lifecycle management and custom header injection for SSE.
plugin/mcp-client/test/fixtures/apps/mcpclient/config/plugin.js (1)
1-26: LGTM!Standard Egg.js plugin configuration correctly enabling the necessary tegg plugins and appropriately disabling the watcher for the test environment.
plugin/langchain/app.ts (1)
11-44: LGTM!The lifecycle hook management is correctly implemented with symmetric registration and cleanup. The hooks are properly initialized in the constructor, registered in
configWillLoad/configDidLoad, and cleaned up inbeforeClose.plugin/langchain/lib/ChatOpenAI.ts (1)
45-50: The review comment is incorrect and should be disregarded.The code at lines 45-50 is intentionally designed with proper safeguards:
Line 46 properly initializes the configuration object:
chatConfig.configuration = chatConfig.configuration || {};ensures a valid object exists before property assignment.The
anycast on line 49 is necessary, not a flaw: The configuration schema (defined intypings/index.d.tslines 81-104) does not include afetchproperty. Theas anycast is required to add this external property, and this is intentional per the inline comment explaining the urllib compatibility workaround.No runtime error risk: Since line 46 guarantees a non-null object exists, assigning the
fetchproperty cannot cause a runtime error. The concern about an incompatible interface is unfounded.This pattern is consistent elsewhere in the codebase: Similar patterns of explicitly passing
fetchto client implementations are used inEggHttpMCPClient.tsandHttpMCPClient.ts.Likely an incorrect or invalid review comment.
plugin/langchain/lib/graph/CompiledStateGraphProto.ts (2)
28-35: No issues found - empty qualifiers initialization is intentional design.The empty qualifiers array in
CompiledStateGraphProtois not a bug but a deliberate architectural choice. Evidence shows:
Always retrieved with empty qualifiers: GraphLoadUnitHook.ts line 38 calls
obj.getEggPrototype(graphName, [])— consistently passing an empty qualifiers array.Correct behavior with empty input: The
verifyQualifiersmethod returnstruefor empty input arrays (the loop never executes), which is the intended behavior for resources that don't use qualifier-based filtering.Design rationale:
CompiledStateGraphProtowraps LangGraph graph resources, not user-defined classes. UnlikeSingletonModelProto(which extracts qualifiers from class metadata viaQualifierUtil.getProtoQualifiers(model)), compiled graphs have no class-level qualifier metadata to extract.The proto selection mechanism for graphs uses name-based matching, not qualifier-based matching. Empty qualifiers serve as a sentinel value indicating "no qualifier filtering needed."
41-44: Disregard this review comment.The review incorrectly assumes that nodes and edges retrieved via
EggContainerFactory.getOrCreateEggObjectFromClazz()haveCompiledStateGraphProtoproto instances. In reality, these EggObject instances have proto of typeEggPrototype(interface), which is implemented by other classes likeEggPrototypeImpl. Those implementations properly implementgetMetaDataby delegating toMetadataUtil.getMetaData().
CompiledStateGraphProto.getMetaData()returningundefinedis intentional—the proto stores metadata via thegraphMetadataproperty, not through the metadata system. All call sites (lines 85, 102 in CompiledStateGraphObject.ts; line 116 in GraphObjectHook.ts, etc.) safely handleundefinedreturns with conditional checks.Likely an incorrect or invalid review comment.
plugin/langchain/lib/graph/GraphObjectHook.ts (1)
69-110: Verify whether missing ChatModel handling represents a real issue or is by design.The concern about silent failure when tools are declared without ChatModel injection is theoretically valid; however, all @graphnode usages found are test fixtures, and each properly injects ChatModel, uses the build() method, or extends TeggToolNode. The framework appears to provide multiple binding paths intentionally (auto-binding, manual build(), ToolNode), suggesting the current behavior may be by design rather than an oversight.
No production GraphNode implementations exist in the codebase to confirm whether this scenario occurs in practice or whether users are expected to follow the test patterns shown.
plugin/mcp-client/test/fixtures/apps/mcpclient/app/modules/bar/controller/AppController.ts
Show resolved
Hide resolved
akitaSummer
force-pushed
the
feat/langchain
branch
2 times, most recently
from
52d23f4 to
c5db249
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
plugin/mcp-client/lib/EggHttpStaticMCPClient.ts (1)
70-77: Forward transport & request options to the base client.We still drop
transportOptions/requestOptionswhen callingsuper(...), so any MCP client that needs custom headers, auth providers, or timeouts silently breaks. Please pass through the full option set pulled frommcpServerSubConfig.super({ clientName, clientVersion: sseClientConfig.version ?? '1.0.0', transportType: mcpServerSubConfig.transportType as any, url: mcpServerSubConfig.url, logger, fetch: FetchFactory?.fetch, + transportOptions: mcpServerSubConfig.transportOptions, + requestOptions: mcpServerSubConfig.requestOptions, });plugin/mcp-client/test/mcpclient.test.ts (1)
13-42: Consolidate duplicate lifecycle hooks.The duplicate
before()andafter()hooks create confusion and incomplete cleanup. This issue was previously flagged but remains unresolved.Apply this diff to consolidate:
before(async () => { await startStreamableServer(17263); await startSSEServer(17253); - }); - - after(async () => { - await app.close(); - await stopSSEServer(); - await stopStreamableServer(); - }); - - afterEach(() => { - mm.restore(); - }); - - before(async () => { mm(process.env, 'EGG_TYPESCRIPT', true); mm(process, 'cwd', () => { return path.join(__dirname, '..'); }); app = mm.app({ baseDir: path.join(__dirname, 'fixtures/apps/mcpclient'), framework: path.dirname(require.resolve('egg')), }); await app.ready(); }); + afterEach(() => { + mm.restore(); + }); + after(() => { - return app.close(); + await app.close(); + await stopSSEServer(); + await stopStreamableServer(); });plugin/langchain/test/llm.test.ts (1)
16-43: Consolidate duplicate test hooks.The test suite has duplicate
beforeandafterhooks that will cause failures:
- Two
afterhooks (lines 20-23 and 41-43) both callapp.close(), closing the app twice- Two
beforehooks (lines 16-18 and 29-39) are unnecessarily split- Line 21 references
appbefore it's initialized on line 34Apply this diff to consolidate the hooks:
before(async () => { await startSSEServer(17283); - }); - - after(async () => { - await app.close(); - await stopSSEServer(); - }); - - afterEach(() => { - mm.restore(); - }); - - before(async () => { mm(process.env, 'EGG_TYPESCRIPT', true); mm(process, 'cwd', () => { return path.join(__dirname, '..'); }); app = mm.app({ baseDir: path.join(__dirname, 'fixtures/apps/langchain'), framework: path.dirname(require.resolve('egg')), }); await app.ready(); }); + afterEach(() => { + mm.restore(); + }); + after(() => { - return app.close(); + await stopSSEServer(); + return app.close(); });core/langchain-decorator/src/decorator/GraphNode.ts (4)
1-1: Remove the commented-out import.This leftover from development should be cleaned up.
-// import type { EggProtoImplClass } from '@alipay/tegg-types'; - import { StackUtil } from '@eggjs/tegg-common-util';
33-33: Remove the commented-out code.The commented execute signature should be removed to improve clarity.
export interface IGraphNode<S extends StateDefinition = StateDefinition, T = any> { - // execute(state: T extends AbstractStateGraph<infer S> ? GraphStateType<S> : never): Promise<GraphUpdateType<T> extends object ? GraphUpdateType<T> & Record<string, any> : GraphUpdateType<T>>; execute(state: AnnotationRoot<S>['State']): Promise<UpdateType<S> & Record<string, any>> | Promise<ToolNode<T>>;
18-29: Rename the callback parameter to avoid shadowing the global "constructor" property.The parameter name
constructortriggers a Biome lint error because it shadows the built-in global property. Use a safe identifier likeclazzortarget.export function GraphNode<S extends StateDefinition = StateDefinition>(params: IGraphNodeMetadata) { - return (constructor: EggProtoImplClass<IGraphNode<S> | TeggToolNode>) => { + return (clazz: EggProtoImplClass<IGraphNode<S> | TeggToolNode>) => { const func = SingletonProto({ accessLevel: params?.accessLevel ?? AccessLevel.PUBLIC, name: params?.name, }); - func(constructor); - PrototypeUtil.setFilePath(constructor, StackUtil.getCalleeFromStack(false, 5)); + func(clazz); + PrototypeUtil.setFilePath(clazz, StackUtil.getCalleeFromStack(false, 5)); - GraphNodeInfoUtil.setGraphNodeMetadata(params, constructor); + GraphNodeInfoUtil.setGraphNodeMetadata(params, clazz); }; }
39-45: Add the missing state parameter to satisfy the IGraphNode interface contract.The
execute()method is missing the requiredstateparameter defined in theIGraphNodeinterface at line 34. This will cause a type mismatch whenTeggToolNodeis used as anIGraphNode.export class TeggToolNode implements IGraphNode { toolNode: ToolNode; - async execute() { + async execute(_state?: any) { return this.toolNode; } }core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (1)
170-174: Define an entry point for the graph.The
BarGraph.build()method adds nodes and edges but doesn't specify where graph execution should begin. Without callingthis.setEntryPoint(FooGraphNodeName.ACTION)or adding an edge fromSTART, the graph won't know its entry point, which will likely cause a runtime error during compilation or invocation.async build() { + this.setEntryPoint(FooGraphNodeName.ACTION); this.addNode(FooGraphNodeName.ACTION, this.barNode.execute); this.addConditionalEdges(FooGraphNodeName.ACTION, this.fooContinueEdge.execute); return this.compile({ checkpointer: this.fooSaver }); }
🧹 Nitpick comments (3)
plugin/langchain/test/fixtures/apps/langchain/tsconfig.json (1)
1-13: Consider extending the root tsconfig for consistency and inherited settings.This test fixture's tsconfig doesn't extend the root configuration (unlike the core/langchain-decorator/tsconfig.json in this PR), which means it won't inherit type checking rules, strict compiler settings, or other project-wide standards. If fixture isolation is intentional, this is fine, but confirm this is the desired pattern.
Additionally, Line 11 excludes the
"test"directory. For a fixture app at this location, this exclusion may be unnecessary or could unintentionally exclude source code. Consider whether this is correct.plugin/mcp-client/test/mcpclient.test.ts (1)
48-131: Consider extracting the expected response to reduce duplication.All three tests expect an identical response structure. Extract this to a shared constant to improve maintainability.
+ const expectedToolsResponse = { + tools: [ + { + name: 'add', + inputSchema: { + type: 'object', + properties: { + a: { type: 'number' }, + b: { type: 'number' }, + }, + required: ['a', 'b'], + additionalProperties: false, + $schema: 'http://json-schema.org/draft-07/schema#', + }, + }, + ], + }; + it('should sse work', async () => { const res = await app.httpRequest() .get('/mcpclient/hello-sse') .expect(200); - assert.deepStrictEqual(res.body, { - tools: [ - { - name: 'add', - inputSchema: { - type: 'object', - properties: { - a: { - type: 'number', - }, - b: { - type: 'number', - }, - }, - required: [ - 'a', - 'b', - ], - additionalProperties: false, - $schema: 'http://json-schema.org/draft-07/schema#', - }, - }, - ], - }); + assert.deepStrictEqual(res.body, expectedToolsResponse); });Apply similar changes to the other two tests.
core/langchain-decorator/src/decorator/GraphNode.ts (1)
14-15: Consider updating ToolNode import to the stable public API.Based on past review research, as of October 2025,
ToolNodeshould be imported from the'langchain'package rather than'@langchain/langgraph/prebuilt', as the latter path has been marked deprecated.-import { ConfigurableModel } from 'langchain/chat_models/universal'; -import { ToolNode } from '@langchain/langgraph/prebuilt'; +import { ConfigurableModel } from 'langchain/chat_models/universal'; +import { ToolNode } from 'langchain'; import { BaseChatOpenAI } from '@langchain/openai';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
core/langchain-decorator/src/decorator/GraphNode.ts(1 hunks)core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts(1 hunks)core/langchain-decorator/test/tsconfig.json(1 hunks)core/langchain-decorator/tsconfig.json(1 hunks)core/mcp-client/test/HttpMCPClient.test.ts(1 hunks)plugin/langchain/test/fixtures/apps/langchain/tsconfig.json(1 hunks)plugin/langchain/test/llm.test.ts(1 hunks)plugin/mcp-client/lib/EggHttpStaticMCPClient.ts(1 hunks)plugin/mcp-client/test/mcpclient.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- core/langchain-decorator/test/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (1)
- core/mcp-client/test/HttpMCPClient.test.ts
🧰 Additional context used
🧬 Code graph analysis (5)
plugin/langchain/test/llm.test.ts (1)
plugin/langchain/test/fixtures/sse-mcp-server/http.ts (2)
startSSEServer(39-68)stopSSEServer(70-72)
plugin/mcp-client/lib/EggHttpStaticMCPClient.ts (6)
core/types/core-decorator/model/EggMultiInstancePrototypeInfo.ts (2)
MultiInstancePrototypeGetObjectsContext(12-16)ObjectInfo(6-10)plugin/mcp-client/lib/QualifierUtil.ts (1)
QualifierUtil(4-13)core/mcp-client/src/MCPClientQualifier.ts (4)
MCPClientInjectName(5-5)MCPClientQualifierAttribute(4-4)getMCPClientName(17-21)getMCPClientConfig(23-30)plugin/mcp-client/lib/EggHttpMCPClient.ts (1)
EggHttpMCPClient(24-118)core/core-decorator/src/decorator/MultiInstanceInfo.ts (1)
MultiInstanceInfo(4-9)core/lifecycle/src/decorator/index.ts (1)
LifecycleInit(28-28)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (9)
core/langchain-decorator/test/fixtures/modules/langchain/index.ts (1)
SingletonProto(7-14)core/langchain-decorator/src/decorator/GraphTool.ts (2)
GraphTool(15-26)IGraphTool(28-31)core/controller-decorator/src/decorator/mcp/MCPTool.ts (1)
ToolArgsSchema(35-52)core/types/controller-decorator/MCPToolParams.ts (1)
ToolArgs(5-5)core/langchain-decorator/src/decorator/BoundModel.ts (2)
BoundModel(14-25)TeggBoundModel(30-30)core/langchain-decorator/src/decorator/GraphNode.ts (3)
GraphNode(18-29)TeggToolNode(39-45)IGraphNode(31-37)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifier(6-10)core/langchain-decorator/src/decorator/GraphEdge.ts (3)
GraphStateType(38-38)GraphEdge(25-36)IGraphEdge(42-44)core/langchain-decorator/src/decorator/Graph.ts (2)
Graph(12-23)TeggCompiledStateGraph(40-40)
core/langchain-decorator/src/decorator/GraphNode.ts (5)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (7)
GraphNode(63-68)GraphNode(71-90)GraphNode(123-147)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)plugin/langchain/test/fixtures/apps/langchain/app/modules/bar/service/Graph.ts (3)
GraphNode(53-88)GraphNode(90-94)SingletonProto(20-21)core/langchain-decorator/src/model/GraphNodeMetadata.ts (2)
IGraphNodeMetadata(3-7)constructor(14-18)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
GraphNodeInfoUtil(6-14)
plugin/mcp-client/test/mcpclient.test.ts (2)
plugin/mcp-client/test/fixtures/streamable-mcp-server/http.ts (2)
startStreamableServer(38-89)stopStreamableServer(91-93)plugin/mcp-client/test/fixtures/sse-mcp-server/http.ts (2)
startSSEServer(39-68)stopSSEServer(70-72)
🪛 Biome (2.1.2)
plugin/langchain/test/llm.test.ts
[error] 29-39: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
[error] 41-43: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
core/langchain-decorator/src/decorator/GraphNode.ts
[error] 19-19: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugin/mcp-client/test/mcpclient.test.ts
[error] 28-38: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
[error] 40-42: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-macos (18)
🔇 Additional comments (5)
core/langchain-decorator/tsconfig.json (1)
1-13: LGTM!This tsconfig properly extends the root configuration, ensuring inheritance of project-wide TypeScript settings and compiler rules. The overrides for module/moduleResolution and the minimal excludes list are appropriate for a core package.
plugin/mcp-client/test/mcpclient.test.ts (1)
6-6: Verify the Node version requirement.The tests only run on Node > 17. Ensure this constraint is documented and aligns with MCP client requirements or dependencies.
plugin/langchain/test/llm.test.ts (3)
1-14: LGTM!The conditional test execution based on Node.js version and lazy require pattern are appropriate for this integration test setup.
45-57: LGTM!The test correctly mocks the ChatOpenAI model and validates the integration endpoint response.
59-77: LGTM!Both test cases appropriately validate the bound model integration and graph functionality. The different response formats (
textvsvalue) correctly reflect the distinct endpoint behaviors.
akitaSummer
force-pushed
the
feat/langchain
branch
2 times, most recently
from
ebd5d41 to
63d2142
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
plugin/mcp-client/test/mcpclient.test.ts (1)
13-42: Duplicate lifecycle hooks must be addressed before merge.This issue was already flagged in a previous review but remains unresolved. The test suite has duplicate
before()andafter()hooks causing execution order confusion and incomplete cleanup:
- Two
before()hooks (lines 13-16 and 28-38)- Two
after()hooks (lines 18-22 and 40-42)The second
after()hook (lines 40-42) is particularly problematic as it attempts to close the app redundantly and omits server cleanup.Please apply the consolidation fix from the previous review comment to resolve this before merging.
core/langchain-decorator/src/decorator/GraphNode.ts (5)
1-1: Remove commented-out import.This commented import is leftover code that should be removed.
33-33: Remove commented-out code.This commented line is dead code that should be removed for clarity.
14-15: Update ToolNode import to use stable public API.Line 14 has been correctly updated to use the public API path. However, line 15 should import ToolNode from
'langchain'instead of'@langchain/langgraph/prebuilt', as the prebuilt path is deprecated and less stable.Apply this diff:
-import { ToolNode } from '@langchain/langgraph/prebuilt'; +import { ToolNode } from 'langchain';
18-29: Rename parameter to avoid shadowing global 'constructor' property.The parameter name
constructorshadows the global property and triggers a Biome linting error. Rename it to avoid confusion and fix the lint issue.Apply this diff:
export function GraphNode<S extends StateDefinition = StateDefinition>(params: IGraphNodeMetadata) { - return (constructor: EggProtoImplClass<IGraphNode<S> | TeggToolNode>) => { + return (clazz: EggProtoImplClass<IGraphNode<S> | TeggToolNode>) => { const func = SingletonProto({ accessLevel: params?.accessLevel ?? AccessLevel.PUBLIC, name: params?.name, }); - func(constructor); - PrototypeUtil.setFilePath(constructor, StackUtil.getCalleeFromStack(false, 5)); + func(clazz); + PrototypeUtil.setFilePath(clazz, StackUtil.getCalleeFromStack(false, 5)); - GraphNodeInfoUtil.setGraphNodeMetadata(params, constructor); + GraphNodeInfoUtil.setGraphNodeMetadata(params, clazz); }; }
39-45: Add missing state parameter to match IGraphNode interface.The
execute()method is missing the requiredstateparameter from theIGraphNodeinterface. This violates the interface contract.Apply this diff:
export class TeggToolNode implements IGraphNode { toolNode: ToolNode; - async execute() { + async execute(_state?: any) { return this.toolNode; } }plugin/mcp-client/lib/EggHttpStaticMCPClient.ts (1)
70-77: Still missing transport/request option passthroughThis was already flagged earlier: without forwarding
transportOptions/requestOptionsfrommcpServerSubConfig, any per-client headers, auth, or timeout settings are silently lost, so HTTP clients that rely on them will fail to connect. Please keep passing the full option set into the parent.super({ clientName, clientVersion: sseClientConfig.version ?? '1.0.0', transportType: mcpServerSubConfig.transportType as any, url: mcpServerSubConfig.url, logger, fetch, + transportOptions: mcpServerSubConfig.transportOptions, + requestOptions: mcpServerSubConfig.requestOptions, });
🧹 Nitpick comments (1)
plugin/mcp-client/lib/HttpMCPClientFactory.ts (1)
23-33: Remove the unnecessary type cast for clearer type safety.The type cast
as HttpClientOptionson line 28 is misleading and unnecessary. Theoptionsparameter is explicitly typed asOmit<HttpClientOptions, 'logger'>, so casting it toHttpClientOptions(which includes the logger property) contradicts the method signature. Sinceloggeris provided separately on line 29, TypeScript can infer the correct type without the cast.Apply this diff to remove the unnecessary cast:
async build(clientInfo: Implementation, options: Omit<HttpClientOptions, 'logger'>): Promise<EggHttpMCPClient> { const httpMCPClient = new EggHttpMCPClient({ clientName: clientInfo.name, clientVersion: clientInfo.version, fetch, - ...options as HttpClientOptions, + ...options, logger: this.logger, }); await httpMCPClient.init(); return httpMCPClient; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
core/langchain-decorator/src/decorator/GraphNode.ts(1 hunks)core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts(1 hunks)core/langchain-decorator/test/tsconfig.json(1 hunks)core/langchain-decorator/tsconfig.json(1 hunks)core/mcp-client/test/HttpMCPClient.test.ts(1 hunks)plugin/langchain/test/fixtures/apps/langchain/tsconfig.json(1 hunks)plugin/langchain/test/llm.test.ts(1 hunks)plugin/mcp-client/lib/EggHttpStaticMCPClient.ts(1 hunks)plugin/mcp-client/lib/HttpMCPClientFactory.ts(1 hunks)plugin/mcp-client/test/mcpclient.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- core/mcp-client/test/HttpMCPClient.test.ts
- core/langchain-decorator/tsconfig.json
- core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts
- core/langchain-decorator/test/tsconfig.json
🧰 Additional context used
🧬 Code graph analysis (5)
plugin/langchain/test/llm.test.ts (1)
plugin/langchain/test/fixtures/sse-mcp-server/http.ts (2)
startSSEServer(39-68)stopSSEServer(70-72)
plugin/mcp-client/lib/HttpMCPClientFactory.ts (2)
core/mcp-client/src/HttpMCPClient.ts (1)
HttpClientOptions(25-25)plugin/mcp-client/lib/EggHttpMCPClient.ts (1)
EggHttpMCPClient(24-118)
plugin/mcp-client/test/mcpclient.test.ts (2)
plugin/mcp-client/test/fixtures/streamable-mcp-server/http.ts (2)
startStreamableServer(38-89)stopStreamableServer(91-93)plugin/mcp-client/test/fixtures/sse-mcp-server/http.ts (2)
startSSEServer(39-68)stopSSEServer(70-72)
plugin/mcp-client/lib/EggHttpStaticMCPClient.ts (6)
core/types/core-decorator/model/EggMultiInstancePrototypeInfo.ts (2)
MultiInstancePrototypeGetObjectsContext(12-16)ObjectInfo(6-10)plugin/mcp-client/lib/QualifierUtil.ts (1)
QualifierUtil(4-13)core/mcp-client/src/MCPClientQualifier.ts (4)
MCPClientInjectName(5-5)MCPClientQualifierAttribute(4-4)getMCPClientName(17-21)getMCPClientConfig(23-30)plugin/mcp-client/lib/EggHttpMCPClient.ts (1)
EggHttpMCPClient(24-118)core/core-decorator/src/decorator/MultiInstanceInfo.ts (1)
MultiInstanceInfo(4-9)core/lifecycle/src/decorator/index.ts (1)
LifecycleInit(28-28)
core/langchain-decorator/src/decorator/GraphNode.ts (5)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (7)
GraphNode(63-68)GraphNode(71-90)GraphNode(123-147)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/model/GraphNodeMetadata.ts (2)
IGraphNodeMetadata(3-7)constructor(14-18)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
GraphNodeInfoUtil(6-14)
🪛 Biome (2.1.2)
plugin/langchain/test/llm.test.ts
[error] 29-39: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
[error] 41-43: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
plugin/mcp-client/test/mcpclient.test.ts
[error] 28-38: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
[error] 40-42: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
core/langchain-decorator/src/decorator/GraphNode.ts
[error] 19-19: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Analyze (javascript)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-ubuntu (20)
🔇 Additional comments (5)
plugin/mcp-client/test/mcpclient.test.ts (1)
44-132: Test cases are well-structured.The three test cases provide good coverage of the MCP client functionality across different transport mechanisms (SSE, Streamable, Factory). Each test properly verifies both the HTTP response status and the complete tool schema structure.
plugin/langchain/test/llm.test.ts (3)
45-57: Test logic looks good.The mock setup and assertions are appropriate for testing the LLM hello endpoint.
59-71: Test logic looks good.The mock setup and assertions are appropriate for testing the bound chat endpoint.
73-77: Integration test logic looks good.This test appropriately validates the graph endpoint without mocking, providing valuable integration coverage.
core/langchain-decorator/src/decorator/GraphNode.ts (1)
31-32: LGTM: Interface contract is well-defined.The
IGraphNodeinterface properly defines the contract for graph nodes with correct generic typing for state and execution. The optionalbuildmethod signature aligns with LangChain'sbindToolspattern as seen in the test fixtures.Also applies to: 34-37
akitaSummer
force-pushed
the
feat/langchain
branch
from
63d2142 to
380490c
Compare
| "tsc:pub": "npm run clean && tsc -p ./tsconfig.pub.json", | ||
| "prepublishOnly": "npm run tsc:pub" | ||
| }, | ||
| "author": "killagu <[email protected]>", |
plugin/langchain/app.ts
Outdated
| import { GraphPrototypeHook } from './lib/graph/GraphPrototypeHook'; | ||
| import { GraphBuildHook } from './lib/graph/GraphBuildHook'; | ||
|
|
||
| export default class ModuleDRMHook implements IBoot { |
| import { BaseTracer, Run } from '@langchain/core/tracers/base'; | ||
|
|
||
| @SingletonProto() | ||
| export class LangGraphTracer extends BaseTracer { |
There was a problem hiding this comment.
内网的需要extends这个tracer
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
plugin/mcp-client/test/mcpclient.test.ts (1)
13-22: Consolidate duplicate Mocha hooks and avoid doubleapp.close()There are two
before()and twoafter()hooks inside the samedescribe, which triggers thenoDuplicateTestHookslint error and results inapp.close()being called twice while server shutdown is split across hooks. This is exactly what the previous review/lint warning was pointing out.You can simplify to a single
before()that both starts the servers and boots the app, and a singleafter()that closes the app and stops both servers:- let app; - - before(async () => { - await startStreamableServer(17263); - await startSSEServer(17253); - }); - - after(async () => { - await app.close(); - await stopSSEServer(); - await stopStreamableServer(); - }); - - afterEach(() => { - mm.restore(); - }); - - before(async () => { - mm(process.env, 'EGG_TYPESCRIPT', true); - mm(process, 'cwd', () => { - return path.join(__dirname, '..'); - }); - app = mm.app({ - baseDir: path.join(__dirname, 'fixtures/apps/mcpclient'), - framework: path.dirname(require.resolve('egg')), - }); - await app.ready(); - }); - - after(() => { - return app.close(); - }); + let app; + + before(async () => { + await startStreamableServer(17263); + await startSSEServer(17253); + + mm(process.env, 'EGG_TYPESCRIPT', true); + mm(process, 'cwd', () => { + return path.join(__dirname, '..'); + }); + app = mm.app({ + baseDir: path.join(__dirname, 'fixtures/apps/mcpclient'), + framework: path.dirname(require.resolve('egg')), + }); + await app.ready(); + }); + + afterEach(() => { + mm.restore(); + }); + + after(async () => { + if (app) { + await app.close(); + } + await stopSSEServer(); + await stopStreamableServer(); + });This removes the duplicate hooks, ensures full cleanup happens in one place, and avoids closing the app twice.
Also applies to: 28-42
plugin/mcp-client/lib/EggHttpStaticMCPClient.ts (1)
60-76: Forward transport/request options from MCP client config intoEggHttpMCPClientThe constructor currently ignores any
transportOptions/requestOptions(and similar HTTP-level config) defined in the MCP client config:super({ clientName, clientVersion: sseClientConfig.version ?? '1.0.0', transportType: mcpServerSubConfig.transportType as any, url: mcpServerSubConfig.url, logger, fetch, });This means per-client headers, auth, timeouts, etc., configured under
mcp.clients[<name>]never reachEggHttpMCPClient/HttpMCPClient, which can cause real clients to fail to connect.You can fix this by forwarding the relevant fields from
mcpServerSubConfig:- super({ - clientName, - clientVersion: sseClientConfig.version ?? '1.0.0', - transportType: mcpServerSubConfig.transportType as any, - url: mcpServerSubConfig.url, - logger, - fetch, - }); + super({ + clientName, + clientVersion: sseClientConfig.version ?? '1.0.0', + transportType: mcpServerSubConfig.transportType as any, + url: mcpServerSubConfig.url, + logger, + fetch, + transportOptions: mcpServerSubConfig.transportOptions, + requestOptions: mcpServerSubConfig.requestOptions, + });If additional HTTP-related per-client fields exist in the config and are supported by the underlying client, they should be threaded through here as well.
core/langchain-decorator/src/decorator/GraphNode.ts (2)
1-1: Remove stale commented-out import.The old
@alipay/tegg-typesimport comment is now obsolete sinceEggProtoImplClasscomes from@eggjs/tegg; it just adds noise and should be deleted.-// import type { EggProtoImplClass } from '@alipay/tegg-types';
18-28: Renameconstructorcallback param to avoid Biome lint error.Biome flags
constructorhere as shadowing a restricted global name. Renaming keeps lint clean with no behavior change.-export function GraphNode<S extends StateDefinition = StateDefinition>(params: IGraphNodeMetadata) { - return (constructor: EggProtoImplClass<IGraphNode<S> | TeggToolNode>) => { +export function GraphNode<S extends StateDefinition = StateDefinition>(params: IGraphNodeMetadata) { + return (clazz: EggProtoImplClass<IGraphNode<S> | TeggToolNode>) => { const func = SingletonProto({ accessLevel: params?.accessLevel ?? AccessLevel.PUBLIC, name: params?.name, }); - func(constructor); - PrototypeUtil.setFilePath(constructor, StackUtil.getCalleeFromStack(false, 5)); - - GraphNodeInfoUtil.setGraphNodeMetadata(params, constructor); + func(clazz); + PrototypeUtil.setFilePath(clazz, StackUtil.getCalleeFromStack(false, 5)); + + GraphNodeInfoUtil.setGraphNodeMetadata(params, clazz); }; }core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (1)
170-174: BindBarNode.executeand define an entry point before compilingBarGraph.Two issues in
BarGraph.build:
this.barNode.executeis passed unbound, so when the graph invokes the node,thisinsideBarNode.executewill beundefinedandthis.chatModel/this.fooToolwill fail at runtime.- For a manually wired graph with cycles, you typically need to set an explicit entry point before calling
compile(). A previous review already called this out.A minimal, safer version:
async build() { - this.addNode(FooGraphNodeName.ACTION, this.barNode.execute); - this.addConditionalEdges(FooGraphNodeName.ACTION, this.fooContinueEdge.execute); - return this.compile({ checkpointer: this.fooSaver }); + this.setEntryPoint(FooGraphNodeName.ACTION); + this.addNode( + FooGraphNodeName.ACTION, + this.barNode.execute.bind(this.barNode), + ); + this.addConditionalEdges( + FooGraphNodeName.ACTION, + this.fooContinueEdge.execute.bind(this.fooContinueEdge), + ); + return this.compile({ checkpointer: this.fooSaver }); }#!/bin/bash # Quick check: find other manual Graph.build implementations to mirror their pattern. rg -n "class .*Graph extends AbstractStateGraph" -S rg -n "async build\(" -S
🧹 Nitpick comments (3)
plugin/langchain/test/llm.test.ts (1)
16-18: Consider consolidating duplicatebeforehooks.While duplicate
beforehooks execute in registration order (so this works functionally), having two violates Mocha best practices and triggers static analysis warnings. Consider merging into a singlebeforehook for cleaner test setup.before(async () => { await startSSEServer(17283); - }); - - after(async () => { - await app.close(); - await stopSSEServer(); - }); - - afterEach(() => { - mm.restore(); - }); - - before(async () => { mm(process.env, 'EGG_TYPESCRIPT', true); mm(process, 'cwd', () => { return path.join(__dirname, '..'); }); app = mm.app({ baseDir: path.join(__dirname, 'fixtures/apps/langchain'), framework: path.dirname(require.resolve('egg')), }); await app.ready(); });Based on static analysis hints.
Also applies to: 29-39
plugin/mcp-client/lib/HttpMCPClientFactory.ts (1)
23-30: Tighten options typing to avoidas HttpClientOptionscastThe factory works functionally, but the combination of
options: Omit<HttpClientOptions, 'logger'>and...options as HttpClientOptionsweakens type safety and couples this layer directly to the lower-levelHttpClientOptionsunion.Consider introducing a factory-specific options type (e.g. “whatever EggHttpMCPClient needs minus
clientName/clientVersion/logger”) and using it directly, so you can drop the cast and keep the factory decoupled from core HTTP client internals. That also makes it clearer which fields callers are expected to provide.core/langchain-decorator/src/decorator/GraphNode.ts (1)
31-37: AlignIGraphNode.buildsignature with typical implementations (optional).
IGraphNode.buildis typed as taking atoolsargument, but concrete implementations likeBarNode.build(incore/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts) don’t accept any parameters and instead use injected tools. This likely still type-checks, but the current interface can be misleading for consumers.Consider one of:
- Making the parameter optional in the interface:
- build?: (tools: Parameters<ConfigurableModel['bindTools']>['0']) => Promise<ReturnType<ConfigurableModel['bindTools']> | ReturnType<BaseChatOpenAI<any>['bindTools']>>; + build?: (tools?: Parameters<ConfigurableModel['bindTools']>['0']) => Promise<ReturnType<ConfigurableModel['bindTools']> | ReturnType<BaseChatOpenAI<any>['bindTools']>>;
- Or documenting that
toolsis framework-supplied and may be ignored so future node implementations don’t get confused.Also applies to: 39-45
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
core/langchain-decorator/package.json(1 hunks)core/langchain-decorator/src/decorator/GraphNode.ts(1 hunks)core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts(1 hunks)core/langchain-decorator/test/tsconfig.json(1 hunks)core/langchain-decorator/tsconfig.json(1 hunks)core/mcp-client/package.json(1 hunks)core/mcp-client/test/HttpMCPClient.test.ts(1 hunks)plugin/langchain/app.ts(1 hunks)plugin/langchain/test/fixtures/apps/langchain/tsconfig.json(1 hunks)plugin/langchain/test/llm.test.ts(1 hunks)plugin/mcp-client/lib/EggHttpStaticMCPClient.ts(1 hunks)plugin/mcp-client/lib/HttpMCPClientFactory.ts(1 hunks)plugin/mcp-client/test/mcpclient.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- core/mcp-client/test/HttpMCPClient.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- plugin/langchain/test/fixtures/apps/langchain/tsconfig.json
- core/langchain-decorator/test/tsconfig.json
- core/langchain-decorator/tsconfig.json
- core/mcp-client/package.json
- core/langchain-decorator/package.json
- plugin/langchain/app.ts
🧰 Additional context used
🧬 Code graph analysis (6)
plugin/mcp-client/lib/EggHttpStaticMCPClient.ts (7)
core/types/core-decorator/model/EggMultiInstancePrototypeInfo.ts (2)
MultiInstancePrototypeGetObjectsContext(12-16)ObjectInfo(6-10)core/common-util/src/ModuleConfig.ts (1)
ModuleConfigUtil(31-380)plugin/mcp-client/lib/QualifierUtil.ts (1)
QualifierUtil(4-13)core/mcp-client/src/MCPClientQualifier.ts (4)
MCPClientInjectName(5-5)MCPClientQualifierAttribute(4-4)getMCPClientName(17-21)getMCPClientConfig(23-30)plugin/mcp-client/lib/EggHttpMCPClient.ts (1)
EggHttpMCPClient(24-118)core/core-decorator/src/decorator/MultiInstanceInfo.ts (1)
MultiInstanceInfo(4-9)core/lifecycle/src/decorator/index.ts (1)
LifecycleInit(28-28)
plugin/mcp-client/lib/HttpMCPClientFactory.ts (2)
core/mcp-client/src/HttpMCPClient.ts (1)
HttpClientOptions(25-25)plugin/mcp-client/lib/EggHttpMCPClient.ts (1)
EggHttpMCPClient(24-118)
plugin/langchain/test/llm.test.ts (1)
plugin/langchain/test/fixtures/sse-mcp-server/http.ts (2)
startSSEServer(39-68)stopSSEServer(70-72)
core/langchain-decorator/src/decorator/GraphNode.ts (5)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (7)
GraphNode(63-68)GraphNode(71-90)GraphNode(123-147)constructor(117-119)constructor(166-168)SingletonProto(23-24)SingletonProto(177-190)core/langchain-decorator/src/model/GraphNodeMetadata.ts (2)
IGraphNodeMetadata(3-7)constructor(14-18)core/types/core-decorator/model/EggPrototypeInfo.ts (1)
EggProtoImplClass(5-5)core/core-decorator/src/util/PrototypeUtil.ts (1)
PrototypeUtil(16-291)core/langchain-decorator/src/util/GraphNodeInfoUtil.ts (1)
GraphNodeInfoUtil(6-14)
core/langchain-decorator/test/fixtures/modules/langgraph/Graph.ts (9)
core/langchain-decorator/test/fixtures/modules/langchain/index.ts (1)
SingletonProto(7-14)core/langchain-decorator/src/decorator/GraphTool.ts (2)
GraphTool(15-26)IGraphTool(28-31)core/controller-decorator/src/decorator/mcp/MCPTool.ts (1)
ToolArgsSchema(35-52)core/types/controller-decorator/MCPToolParams.ts (1)
ToolArgs(5-5)core/langchain-decorator/src/decorator/BoundModel.ts (2)
BoundModel(14-25)TeggBoundModel(30-30)core/langchain-decorator/src/decorator/GraphNode.ts (3)
GraphNode(18-29)TeggToolNode(39-45)IGraphNode(31-37)core/langchain-decorator/src/qualifier/ChatModelQualifier.ts (1)
ChatModelQualifier(6-10)core/langchain-decorator/src/decorator/GraphEdge.ts (3)
GraphStateType(38-38)GraphEdge(25-36)IGraphEdge(42-44)core/langchain-decorator/src/decorator/Graph.ts (2)
Graph(12-23)TeggCompiledStateGraph(40-40)
plugin/mcp-client/test/mcpclient.test.ts (2)
plugin/mcp-client/test/fixtures/streamable-mcp-server/http.ts (2)
startStreamableServer(38-89)stopStreamableServer(91-93)plugin/mcp-client/test/fixtures/sse-mcp-server/http.ts (2)
startSSEServer(39-68)stopSSEServer(70-72)
🪛 Biome (2.1.2)
plugin/langchain/test/llm.test.ts
[error] 29-39: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
[error] 41-43: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
core/langchain-decorator/src/decorator/GraphNode.ts
[error] 19-19: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
plugin/mcp-client/test/mcpclient.test.ts
[error] 28-38: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
[error] 40-42: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Analyze (typescript)
- GitHub Check: Analyze (javascript)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugin/mcp-proxy/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-ubuntu (18)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugin/mcp-client/test/mcpclient.test.ts (1)
13-42: Remove duplicate lifecycle hooks.This issue was flagged in a previous review but remains unresolved. The duplicate
before()andafter()hooks create confusion and potential runtime errors:
- First
after()(lines 18-22) closes the app and stops servers- Second
after()(lines 40-42) attempts to close the already-closed appConsolidate into single
before()andafter()hooks as suggested in the previous review.
🧹 Nitpick comments (1)
plugin/mcp-client/test/mcpclient.test.ts (1)
44-126: Consider reducing test duplication.All three test cases use identical assertion logic (lines 48-69, 76-97, 104-125), differing only in the endpoint path. Consider extracting a helper or using parameterized tests:
Example with a helper:
const expectedToolsResponse = { tools: [ { name: 'add', inputSchema: { type: 'object', properties: { a: { type: 'number' }, b: { type: 'number' }, }, required: ['a', 'b'], }, }, ], }; async function testEndpoint(endpoint: string) { const res = await app.httpRequest() .get(endpoint) .expect(200); assert.deepStrictEqual(res.body, expectedToolsResponse); } it('should sse work', async () => { await testEndpoint('/mcpclient/hello-sse'); }); it('should streamable work', async () => { await testEndpoint('/mcpclient/hello-streamable'); }); it('should factory work', async () => { await testEndpoint('/mcpclient/hello-factory'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugin/mcp-client/test/mcpclient.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugin/mcp-client/test/mcpclient.test.ts (2)
plugin/mcp-client/test/fixtures/streamable-mcp-server/http.ts (2)
startStreamableServer(38-89)stopStreamableServer(91-93)plugin/mcp-client/test/fixtures/sse-mcp-server/http.ts (2)
startSSEServer(39-68)stopSSEServer(70-72)
🪛 Biome (2.1.2)
plugin/mcp-client/test/mcpclient.test.ts
[error] 28-38: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
[error] 40-42: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Runner-macos (18)
- GitHub Check: Analyze (javascript)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-ubuntu (16)
Checklist
npm testpassesAffected core subsystem(s)
Description of change
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.