✨ feat(agent-runtime): 新增金融工具动态发现与注入能力#52
Merged
kevinmatthe merged 1 commit intomasterfrom Mar 26, 2026
Merged
Conversation
- 新增resolveDiscoveredFinanceTools逻辑，识别finance_tool_discover调用结果 - 扩展聊天链路支持AdditionalTools参数，动态注入工具到下一轮对话 - 限制单次注入最多3个金融工具，避免schema过大影响模型效果 - 新增单元测试覆盖注入全链路流程 ✨ feat(finance-tools): 新增只读高层金融数据工具集 - 新增finance_market_data_get、finance_news_get、economy_indicator_get三个只读工具 - 底层封装多源fallback逻辑，单源失败自动重试备用数据源 - 工具默认不加入首轮工具集，仅通过discover动态注入，无副作用无需审批 - 配套完整单测覆盖工具定义、provider、handler全链路 ✨ feat(chat-tools): 新增群成员查询工具集 - 新增get_chat_members工具，查询当前群成员open_id与姓名列表 - 新增get_recent_active_members工具，查询最近发言的活跃成员，含发言统计 - 严格限定仅可查询当前chat_id范围数据，禁止跨群越权访问 - 配套完整单测覆盖边界场景 🐛 fix(aktool): 修复股票简称查询接口兼容性 - 切换股票信息查询接口从东方财富到雪球，提升数据稳定性 - 兼容新旧接口返回字段，优先取org_short_name_cn作为股票简称 - 调整symbol前缀拼接逻辑，适配雪球接口要求 📝 docs(plans): 新增功能实现计划文档 - 新增chat被动限流v2实现方案与上线记录文档 - 新增金融工具基于akshare统一实现计划文档 🔧 chore(config): 优化基础配置 - 新增.worktrees目录到.gitignore，适配多工作流开发 - 调高akshareapi默认重试次数从3到5，提升接口调用成功率 💄 style(ui): 调整agent流式卡片标题 - 将流式对话卡片标题修改为"Agentic Chat For You"，优化用户感知
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's Guide在代理聊天运行时中实现对只读金融工具的动态发现和注入；新增金融与聊天成员相关工具，并完成完整的路由与测试；调整 ark/akshare 相关管线（包括重试机制和股票代码查询）；更新文档以及部分 UI/配置细节。 动态金融工具发现与注入的时序图sequenceDiagram
actor User
participant LarkBot
participant ChatflowRuntime
participant ArkResponses
participant FinanceToolDiscoverHandler
participant FinanceToolInjection
participant FinanceToolHandler
participant FinanceProvider
User->>LarkBot: send message asking for finance analysis
LarkBot->>ChatflowRuntime: InitialChatLoopRequest
ChatflowRuntime->>ArkResponses: ResponseTurnRequest (Tools includes finance_tool_discover)
ArkResponses-->>ChatflowRuntime: Model stream with tool call finance_tool_discover
ChatflowRuntime->>FinanceToolDiscoverHandler: Handle finance_tool_discover
FinanceToolDiscoverHandler->>FinanceToolDiscoverHandler: FinanceToolCatalog + filters
FinanceToolDiscoverHandler-->>ChatflowRuntime: JSON tools list (tool_name, schema,...)
ChatflowRuntime->>FinanceToolInjection: resolveDiscoveredFinanceTools(functionName, output, capabilityTools)
FinanceToolInjection->>FinanceToolInjection: validate tool names, limit to 3
FinanceToolInjection-->>ChatflowRuntime: arktools Impl with selected finance tools
ChatflowRuntime->>ArkResponses: Next ResponseTurnRequest
Note right of ChatflowRuntime: PreviousResponseID set
ChatflowRuntime->>ArkResponses: AdditionalTools from injection
ArkResponses->>ArkResponses: mergeResponseTools(base, AdditionalTools)
ArkResponses-->>ChatflowRuntime: Model stream with finance_market_data_get call
ChatflowRuntime->>FinanceToolHandler: Handle finance_market_data_get
FinanceToolHandler->>FinanceProvider: GetMarketData / GetNews / GetEconomyIndicator
FinanceProvider->>FinanceProvider: callInto akshareapi endpoints with fallback
FinanceProvider-->>FinanceToolHandler: FinanceToolResult
FinanceToolHandler-->>ChatflowRuntime: serialized JSON result
ChatflowRuntime->>ArkResponses: ResponseTurnRequest with ToolOutput
ArkResponses-->>ChatflowRuntime: Final assistant reply
ChatflowRuntime-->>LarkBot: reply text
LarkBot-->>User: streaming agentic card with answer
金融目录、提供方与工具处理器的类图classDiagram
class FinanceToolCategory {
<<enum>>
FinanceToolCategoryMarketData
FinanceToolCategoryNews
FinanceToolCategoryEconomy
}
class FinanceSourceSpec {
+string Name
+string EndpointName
}
class FinanceSourceRoute {
+string RequestKind
+[]FinanceSourceSpec Fallbacks
}
class FinanceToolDefinition {
+string Name
+FinanceToolCategory Category
+string Description
+arktools_Param Schema
+[]string Examples
+[]FinanceSourceRoute Routes
+SourceRoute(kind string) FinanceSourceRoute,bool
}
class FinanceToolCatalogAPI {
+FinanceToolCatalog() []FinanceToolDefinition
+LookupFinanceToolDefinition(name string) FinanceToolDefinition,bool
}
class FinanceToolResult {
+string ToolName
+string Category
+string Source
+map[string]any Query
+string Summary
+[]map[string]any Records
}
class FinanceMarketDataRequest {
+string AssetType
+string Symbol
+string Interval
+int Limit
+string StartTime
+string EndTime
}
class FinanceNewsRequest {
+string TopicType
+string Symbol
+int Limit
}
class FinanceEconomyIndicatorRequest {
+string Indicator
+int Limit
}
class FinanceProvider {
-httpProvider http
+NewFinanceProvider(baseURL string) FinanceProvider
+GetMarketData(ctx context.Context, req FinanceMarketDataRequest) FinanceToolResult,error
+GetNews(ctx context.Context, req FinanceNewsRequest) FinanceToolResult,error
+GetEconomyIndicator(ctx context.Context, req FinanceEconomyIndicatorRequest) FinanceToolResult,error
-fetchMarketData(ctx context.Context, def FinanceToolDefinition, source FinanceSourceSpec, req FinanceMarketDataRequest) FinanceToolResult,error
-fetchNews(ctx context.Context, def FinanceToolDefinition, source FinanceSourceSpec, req FinanceNewsRequest) FinanceToolResult,error
-fetchEconomyIndicator(ctx context.Context, def FinanceToolDefinition, source FinanceSourceSpec, req FinanceEconomyIndicatorRequest) FinanceToolResult,error
}
class httpProvider {
+callInto(ctx context.Context, endpoint string, params any, out any) error
+GetStockSymbolInfo(ctx context.Context, symbol string) string,error
}
class financeMarketDataHandler {
-FinanceProvider provider
+ParseTool(raw string) FinanceMarketDataArgs,error
+ToolSpec() xcommand_ToolSpec
+Handle(ctx context.Context, data *larkim_P2MessageReceiveV1, metaData *xhandler_BaseMetaData, arg FinanceMarketDataArgs) error
}
class financeNewsHandler {
-FinanceProvider provider
+ParseTool(raw string) FinanceNewsArgs,error
+ToolSpec() xcommand_ToolSpec
+Handle(ctx context.Context, data *larkim_P2MessageReceiveV1, metaData *xhandler_BaseMetaData, arg FinanceNewsArgs) error
}
class economyIndicatorHandler {
-FinanceProvider provider
+ParseTool(raw string) EconomyIndicatorArgs,error
+ToolSpec() xcommand_ToolSpec
+Handle(ctx context.Context, data *larkim_P2MessageReceiveV1, metaData *xhandler_BaseMetaData, arg EconomyIndicatorArgs) error
}
class financeToolDiscoverHandler {
+ParseTool(raw string) FinanceToolDiscoverArgs,error
+ToolSpec() xcommand_ToolSpec
+Handle(ctx context.Context, data *larkim_P2MessageReceiveV1, metaData *xhandler_BaseMetaData, arg FinanceToolDiscoverArgs) error
}
class FinanceToolDiscoverArgs {
+string Query
+string Category
+[]string ToolNames
+int Limit
}
class FinanceToolDiscoverItem {
+string ToolName
+string Description
+arktools_Param Schema
+[]string Required
+[]string Examples
+[]string Categories
}
class FinanceToolDiscoverResult {
+[]FinanceToolDiscoverItem Tools
}
FinanceToolDefinition "1" o-- "*" FinanceSourceRoute
FinanceSourceRoute "1" o-- "*" FinanceSourceSpec
FinanceToolCatalogAPI <.. FinanceProvider : uses
FinanceToolCatalogAPI <.. financeToolDiscoverHandler : uses
FinanceProvider --> FinanceToolDefinition : consults
financeMarketDataHandler --> FinanceProvider : uses
financeNewsHandler --> FinanceProvider : uses
economyIndicatorHandler --> FinanceProvider : uses
financeToolDiscoverHandler --> FinanceToolDiscoverArgs
financeToolDiscoverHandler --> FinanceToolDiscoverResult
FinanceToolDiscoverResult "1" o-- "*" FinanceToolDiscoverItem
代理运行时与 ark 响应中动态工具注入的类图classDiagram
class arktools_Impl_larkim_P2MessageReceiveV1 {
+map[string]ToolUnit FunctionCallMap
+Add(unit *ToolUnit)
+Get(name string) ToolUnit,bool
+Tools() []*responses_ResponsesTool
}
class InitialChatTurnRequest {
+InitialChatExecutionPlan Plan
+string PreviousResponseID
+InitialChatToolOutput ToolOutput
+arktools_Impl_larkim_P2MessageReceiveV1 AdditionalTools
}
class ReplyTurnLoopRequest {
+InitialChatTurnRequest TurnRequest
+int ToolTurns
+func TurnExecutor
+CapabilityRequest BaseRequest
+CapabilityRegistry Registry
+arktools_Impl_larkim_P2MessageReceiveV1 CapabilityTools
+CapabilityReplyPlan FallbackPlan
+InitialTraceRecorder Recorder
}
class ReplyTurnLoopResult {
+CapabilityReplyPlan Plan
+ToolExecutionSnapshot LastSnapshot
}
class InitialChatLoopRequest {
+InitialChatExecutionPlan Plan
+int ToolTurns
+func TurnExecutor
+larkim_P2MessageReceiveV1 Event
+CapabilityRegistry Registry
+arktools_Impl_larkim_P2MessageReceiveV1 CapabilityTools
+func Finalizer
}
class CapabilityReplyTurnRequest {
+AgentSession Session
+AgentRun Run
+AgentStep Step
+CapabilityCallInput Input
+CapabilityResult Result
+InitialTraceRecorder Recorder
+arktools_Impl_larkim_P2MessageReceiveV1 AdditionalTools
}
class ContinuationReplyTurnRequest {
+AgentSession Session
+AgentRun Run
+ResumeSource Source
+WaitingReason WaitingReason
+StepKind PreviousStepKind
+string PreviousStepTitle
+string PreviousStepExternalRef
+string ResumeSummary
+[]byte ResumePayloadJSON
+string ThoughtFallback
+string ReplyFallback
+InitialTraceRecorder Recorder
+arktools_Impl_larkim_P2MessageReceiveV1 AdditionalTools
}
class replyTurnRuntime {
+string chatID
+string openID
+string modelID
+arktools_Impl_larkim_P2MessageReceiveV1 tools
+arktools_Impl_larkim_P2MessageReceiveV1 capabilityTools
+CapabilityRegistry registry
}
class CapabilityRegistryTools {
+arktools_Impl_larkim_P2MessageReceiveV1 Tools
+arktools_Impl_larkim_P2MessageReceiveV1 CapabilityTools
+func Builder
+func TurnExecutor
+int DefaultToolTurns
}
class RuntimeInitialChatLoopOptions {
+ChatGenerationPlan Plan
+arktools_Impl_larkim_P2MessageReceiveV1 Tools
+arktools_Impl_larkim_P2MessageReceiveV1 CapabilityTools
+func Builder
+func TurnExecutor
+int DefaultToolTurns
+func Finalizer
}
class ResponseTurnRequest {
+string ModelID
+string SystemPrompt
+string UserInput
+[]string Files
+string PreviousResponseID
+ToolOutputInput ToolOutput
+[]responses_ResponsesTool AdditionalTools
}
class ResponsesImpl_T {
+[]*responses_ResponsesTool tools
+StreamTurn(ctx context.Context, req ResponseTurnRequest) (iter_Seq,func() ResponseTurnSnapshot,error)
-buildTurnRequest(req ResponseTurnRequest) *responses_CreateResponseTurnReq,error
}
class FinanceToolInjection {
+int maxInjectedFinanceTools
+resolveDiscoveredFinanceTools(functionName string, output string, capabilityTools *arktools_Impl_larkim_P2MessageReceiveV1) *arktools_Impl_larkim_P2MessageReceiveV1,bool
+financeToolNames(toolset *arktools_Impl_larkim_P2MessageReceiveV1) []string
+coalesceToolset(primary *arktools_Impl_larkim_P2MessageReceiveV1, fallback *arktools_Impl_larkim_P2MessageReceiveV1) *arktools_Impl_larkim_P2MessageReceiveV1
}
class mergeResponseToolsAPI {
+mergeResponseTools(base []*responses_ResponsesTool, extra []*responses_ResponsesTool) []*responses_ResponsesTool
+responseToolName(tool *responses_ResponsesTool) string
+responseToolNames(tools []*responses_ResponsesTool) []string
}
InitialChatLoopRequest --> InitialChatTurnRequest : builds
ReplyTurnLoopRequest --> InitialChatTurnRequest : owns
CapabilityReplyTurnRequest --> ReplyTurnLoopRequest : used to build
ContinuationReplyTurnRequest --> ReplyTurnLoopRequest : used to build
RuntimeInitialChatLoopOptions --> InitialChatLoopRequest : configures
CapabilityRegistryTools --> InitialChatLoopRequest : passed as tools
replyTurnRuntime --> CapabilityRegistryTools : constructed from
InitialChatTurnRequest --> ResponseTurnRequest : mapped by ExecuteInitialChatTurn
ResponsesImpl_T --> ResponseTurnRequest : consumes
ResponsesImpl_T --> mergeResponseToolsAPI : uses
FinanceToolInjection <.. ReplyTurnLoopRequest : used in ExecuteReplyTurnLoop
FinanceToolInjection <.. InitialChatLoopRequest : used in StreamInitialChatLoop
arktools_Impl_larkim_P2MessageReceiveV1 <.. CapabilityRegistryTools
arktools_Impl_larkim_P2MessageReceiveV1 <.. RuntimeInitialChatLoopOptions
arktools_Impl_larkim_P2MessageReceiveV1 <.. replyTurnRuntime
arktools_Impl_larkim_P2MessageReceiveV1 <.. InitialChatTurnRequest
从聊天到 akshareapi 的金融数据工具流转图flowchart LR
A[Lark chat message
user asks for finance data] --> B[Chatflow runtime
builds execution plan]
B --> C[Ark Responses
model turn]
C --> D{Model decides
next action}
D -->|Call finance_tool_discover| E[finance_tool_discover_handler
filters FinanceToolCatalog]
E --> F[Discover JSON
list of tools]
F --> G[FinanceToolInjection
resolveDiscoveredFinanceTools]
G --> H[InitialChatTurnRequest
with AdditionalTools]
H --> I[ResponsesImpl
buildTurnRequest
mergeResponseTools]
I --> J{Model function call
finance_market_data_get /
finance_news_get /
economy_indicator_get}
J --> K[Finance tool handler
finance_market_data_handler /
finance_news_handler /
economy_indicator_handler]
K --> L[FinanceProvider
GetMarketData/GetNews/
GetEconomyIndicator]
L --> M[akshareapi endpoints
with retry and fallback]
M --> L
L --> K
K --> N[ToolOutput JSON
FinanceToolResult]
N --> O[Ark Responses
next model turn
with ToolOutput]
O --> P[Chatflow runtime
compose final reply]
P --> Q[LarkBot sends
Agentic Chat For You card]
文件级变更
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 可以：
Getting HelpOriginal review guide in EnglishReviewer's GuideImplements dynamic discovery and injection of read-only finance tools into the agent chat runtime, adds new finance and chat-member tools with full routing and tests, adjusts ark/akshare plumbing (including retries and stock symbol lookup), and updates docs and minor UI/config details. Sequence diagram for dynamic finance tool discovery and injectionsequenceDiagram
actor User
participant LarkBot
participant ChatflowRuntime
participant ArkResponses
participant FinanceToolDiscoverHandler
participant FinanceToolInjection
participant FinanceToolHandler
participant FinanceProvider
User->>LarkBot: send message asking for finance analysis
LarkBot->>ChatflowRuntime: InitialChatLoopRequest
ChatflowRuntime->>ArkResponses: ResponseTurnRequest (Tools includes finance_tool_discover)
ArkResponses-->>ChatflowRuntime: Model stream with tool call finance_tool_discover
ChatflowRuntime->>FinanceToolDiscoverHandler: Handle finance_tool_discover
FinanceToolDiscoverHandler->>FinanceToolDiscoverHandler: FinanceToolCatalog + filters
FinanceToolDiscoverHandler-->>ChatflowRuntime: JSON tools list (tool_name, schema,...)
ChatflowRuntime->>FinanceToolInjection: resolveDiscoveredFinanceTools(functionName, output, capabilityTools)
FinanceToolInjection->>FinanceToolInjection: validate tool names, limit to 3
FinanceToolInjection-->>ChatflowRuntime: arktools Impl with selected finance tools
ChatflowRuntime->>ArkResponses: Next ResponseTurnRequest
Note right of ChatflowRuntime: PreviousResponseID set
ChatflowRuntime->>ArkResponses: AdditionalTools from injection
ArkResponses->>ArkResponses: mergeResponseTools(base, AdditionalTools)
ArkResponses-->>ChatflowRuntime: Model stream with finance_market_data_get call
ChatflowRuntime->>FinanceToolHandler: Handle finance_market_data_get
FinanceToolHandler->>FinanceProvider: GetMarketData / GetNews / GetEconomyIndicator
FinanceProvider->>FinanceProvider: callInto akshareapi endpoints with fallback
FinanceProvider-->>FinanceToolHandler: FinanceToolResult
FinanceToolHandler-->>ChatflowRuntime: serialized JSON result
ChatflowRuntime->>ArkResponses: ResponseTurnRequest with ToolOutput
ArkResponses-->>ChatflowRuntime: Final assistant reply
ChatflowRuntime-->>LarkBot: reply text
LarkBot-->>User: streaming agentic card with answer
Class diagram for finance catalog, provider, and tool handlersclassDiagram
class FinanceToolCategory {
<<enum>>
FinanceToolCategoryMarketData
FinanceToolCategoryNews
FinanceToolCategoryEconomy
}
class FinanceSourceSpec {
+string Name
+string EndpointName
}
class FinanceSourceRoute {
+string RequestKind
+[]FinanceSourceSpec Fallbacks
}
class FinanceToolDefinition {
+string Name
+FinanceToolCategory Category
+string Description
+arktools_Param Schema
+[]string Examples
+[]FinanceSourceRoute Routes
+SourceRoute(kind string) FinanceSourceRoute,bool
}
class FinanceToolCatalogAPI {
+FinanceToolCatalog() []FinanceToolDefinition
+LookupFinanceToolDefinition(name string) FinanceToolDefinition,bool
}
class FinanceToolResult {
+string ToolName
+string Category
+string Source
+map[string]any Query
+string Summary
+[]map[string]any Records
}
class FinanceMarketDataRequest {
+string AssetType
+string Symbol
+string Interval
+int Limit
+string StartTime
+string EndTime
}
class FinanceNewsRequest {
+string TopicType
+string Symbol
+int Limit
}
class FinanceEconomyIndicatorRequest {
+string Indicator
+int Limit
}
class FinanceProvider {
-httpProvider http
+NewFinanceProvider(baseURL string) FinanceProvider
+GetMarketData(ctx context.Context, req FinanceMarketDataRequest) FinanceToolResult,error
+GetNews(ctx context.Context, req FinanceNewsRequest) FinanceToolResult,error
+GetEconomyIndicator(ctx context.Context, req FinanceEconomyIndicatorRequest) FinanceToolResult,error
-fetchMarketData(ctx context.Context, def FinanceToolDefinition, source FinanceSourceSpec, req FinanceMarketDataRequest) FinanceToolResult,error
-fetchNews(ctx context.Context, def FinanceToolDefinition, source FinanceSourceSpec, req FinanceNewsRequest) FinanceToolResult,error
-fetchEconomyIndicator(ctx context.Context, def FinanceToolDefinition, source FinanceSourceSpec, req FinanceEconomyIndicatorRequest) FinanceToolResult,error
}
class httpProvider {
+callInto(ctx context.Context, endpoint string, params any, out any) error
+GetStockSymbolInfo(ctx context.Context, symbol string) string,error
}
class financeMarketDataHandler {
-FinanceProvider provider
+ParseTool(raw string) FinanceMarketDataArgs,error
+ToolSpec() xcommand_ToolSpec
+Handle(ctx context.Context, data *larkim_P2MessageReceiveV1, metaData *xhandler_BaseMetaData, arg FinanceMarketDataArgs) error
}
class financeNewsHandler {
-FinanceProvider provider
+ParseTool(raw string) FinanceNewsArgs,error
+ToolSpec() xcommand_ToolSpec
+Handle(ctx context.Context, data *larkim_P2MessageReceiveV1, metaData *xhandler_BaseMetaData, arg FinanceNewsArgs) error
}
class economyIndicatorHandler {
-FinanceProvider provider
+ParseTool(raw string) EconomyIndicatorArgs,error
+ToolSpec() xcommand_ToolSpec
+Handle(ctx context.Context, data *larkim_P2MessageReceiveV1, metaData *xhandler_BaseMetaData, arg EconomyIndicatorArgs) error
}
class financeToolDiscoverHandler {
+ParseTool(raw string) FinanceToolDiscoverArgs,error
+ToolSpec() xcommand_ToolSpec
+Handle(ctx context.Context, data *larkim_P2MessageReceiveV1, metaData *xhandler_BaseMetaData, arg FinanceToolDiscoverArgs) error
}
class FinanceToolDiscoverArgs {
+string Query
+string Category
+[]string ToolNames
+int Limit
}
class FinanceToolDiscoverItem {
+string ToolName
+string Description
+arktools_Param Schema
+[]string Required
+[]string Examples
+[]string Categories
}
class FinanceToolDiscoverResult {
+[]FinanceToolDiscoverItem Tools
}
FinanceToolDefinition "1" o-- "*" FinanceSourceRoute
FinanceSourceRoute "1" o-- "*" FinanceSourceSpec
FinanceToolCatalogAPI <.. FinanceProvider : uses
FinanceToolCatalogAPI <.. financeToolDiscoverHandler : uses
FinanceProvider --> FinanceToolDefinition : consults
financeMarketDataHandler --> FinanceProvider : uses
financeNewsHandler --> FinanceProvider : uses
economyIndicatorHandler --> FinanceProvider : uses
financeToolDiscoverHandler --> FinanceToolDiscoverArgs
financeToolDiscoverHandler --> FinanceToolDiscoverResult
FinanceToolDiscoverResult "1" o-- "*" FinanceToolDiscoverItem
Class diagram for agent runtime and ark responses dynamic tool injectionclassDiagram
class arktools_Impl_larkim_P2MessageReceiveV1 {
+map[string]ToolUnit FunctionCallMap
+Add(unit *ToolUnit)
+Get(name string) ToolUnit,bool
+Tools() []*responses_ResponsesTool
}
class InitialChatTurnRequest {
+InitialChatExecutionPlan Plan
+string PreviousResponseID
+InitialChatToolOutput ToolOutput
+arktools_Impl_larkim_P2MessageReceiveV1 AdditionalTools
}
class ReplyTurnLoopRequest {
+InitialChatTurnRequest TurnRequest
+int ToolTurns
+func TurnExecutor
+CapabilityRequest BaseRequest
+CapabilityRegistry Registry
+arktools_Impl_larkim_P2MessageReceiveV1 CapabilityTools
+CapabilityReplyPlan FallbackPlan
+InitialTraceRecorder Recorder
}
class ReplyTurnLoopResult {
+CapabilityReplyPlan Plan
+ToolExecutionSnapshot LastSnapshot
}
class InitialChatLoopRequest {
+InitialChatExecutionPlan Plan
+int ToolTurns
+func TurnExecutor
+larkim_P2MessageReceiveV1 Event
+CapabilityRegistry Registry
+arktools_Impl_larkim_P2MessageReceiveV1 CapabilityTools
+func Finalizer
}
class CapabilityReplyTurnRequest {
+AgentSession Session
+AgentRun Run
+AgentStep Step
+CapabilityCallInput Input
+CapabilityResult Result
+InitialTraceRecorder Recorder
+arktools_Impl_larkim_P2MessageReceiveV1 AdditionalTools
}
class ContinuationReplyTurnRequest {
+AgentSession Session
+AgentRun Run
+ResumeSource Source
+WaitingReason WaitingReason
+StepKind PreviousStepKind
+string PreviousStepTitle
+string PreviousStepExternalRef
+string ResumeSummary
+[]byte ResumePayloadJSON
+string ThoughtFallback
+string ReplyFallback
+InitialTraceRecorder Recorder
+arktools_Impl_larkim_P2MessageReceiveV1 AdditionalTools
}
class replyTurnRuntime {
+string chatID
+string openID
+string modelID
+arktools_Impl_larkim_P2MessageReceiveV1 tools
+arktools_Impl_larkim_P2MessageReceiveV1 capabilityTools
+CapabilityRegistry registry
}
class CapabilityRegistryTools {
+arktools_Impl_larkim_P2MessageReceiveV1 Tools
+arktools_Impl_larkim_P2MessageReceiveV1 CapabilityTools
+func Builder
+func TurnExecutor
+int DefaultToolTurns
}
class RuntimeInitialChatLoopOptions {
+ChatGenerationPlan Plan
+arktools_Impl_larkim_P2MessageReceiveV1 Tools
+arktools_Impl_larkim_P2MessageReceiveV1 CapabilityTools
+func Builder
+func TurnExecutor
+int DefaultToolTurns
+func Finalizer
}
class ResponseTurnRequest {
+string ModelID
+string SystemPrompt
+string UserInput
+[]string Files
+string PreviousResponseID
+ToolOutputInput ToolOutput
+[]responses_ResponsesTool AdditionalTools
}
class ResponsesImpl_T {
+[]*responses_ResponsesTool tools
+StreamTurn(ctx context.Context, req ResponseTurnRequest) (iter_Seq,func() ResponseTurnSnapshot,error)
-buildTurnRequest(req ResponseTurnRequest) *responses_CreateResponseTurnReq,error
}
class FinanceToolInjection {
+int maxInjectedFinanceTools
+resolveDiscoveredFinanceTools(functionName string, output string, capabilityTools *arktools_Impl_larkim_P2MessageReceiveV1) *arktools_Impl_larkim_P2MessageReceiveV1,bool
+financeToolNames(toolset *arktools_Impl_larkim_P2MessageReceiveV1) []string
+coalesceToolset(primary *arktools_Impl_larkim_P2MessageReceiveV1, fallback *arktools_Impl_larkim_P2MessageReceiveV1) *arktools_Impl_larkim_P2MessageReceiveV1
}
class mergeResponseToolsAPI {
+mergeResponseTools(base []*responses_ResponsesTool, extra []*responses_ResponsesTool) []*responses_ResponsesTool
+responseToolName(tool *responses_ResponsesTool) string
+responseToolNames(tools []*responses_ResponsesTool) []string
}
InitialChatLoopRequest --> InitialChatTurnRequest : builds
ReplyTurnLoopRequest --> InitialChatTurnRequest : owns
CapabilityReplyTurnRequest --> ReplyTurnLoopRequest : used to build
ContinuationReplyTurnRequest --> ReplyTurnLoopRequest : used to build
RuntimeInitialChatLoopOptions --> InitialChatLoopRequest : configures
CapabilityRegistryTools --> InitialChatLoopRequest : passed as tools
replyTurnRuntime --> CapabilityRegistryTools : constructed from
InitialChatTurnRequest --> ResponseTurnRequest : mapped by ExecuteInitialChatTurn
ResponsesImpl_T --> ResponseTurnRequest : consumes
ResponsesImpl_T --> mergeResponseToolsAPI : uses
FinanceToolInjection <.. ReplyTurnLoopRequest : used in ExecuteReplyTurnLoop
FinanceToolInjection <.. InitialChatLoopRequest : used in StreamInitialChatLoop
arktools_Impl_larkim_P2MessageReceiveV1 <.. CapabilityRegistryTools
arktools_Impl_larkim_P2MessageReceiveV1 <.. RuntimeInitialChatLoopOptions
arktools_Impl_larkim_P2MessageReceiveV1 <.. replyTurnRuntime
arktools_Impl_larkim_P2MessageReceiveV1 <.. InitialChatTurnRequest
Flow diagram for finance data tools from chat to akshareapiflowchart LR
A[Lark chat message
user asks for finance data] --> B[Chatflow runtime
builds execution plan]
B --> C[Ark Responses
model turn]
C --> D{Model decides
next action}
D -->|Call finance_tool_discover| E[finance_tool_discover_handler
filters FinanceToolCatalog]
E --> F[Discover JSON
list of tools]
F --> G[FinanceToolInjection
resolveDiscoveredFinanceTools]
G --> H[InitialChatTurnRequest
with AdditionalTools]
H --> I[ResponsesImpl
buildTurnRequest
mergeResponseTools]
I --> J{Model function call
finance_market_data_get /
finance_news_get /
economy_indicator_get}
J --> K[Finance tool handler
finance_market_data_handler /
finance_news_handler /
economy_indicator_handler]
K --> L[FinanceProvider
GetMarketData/GetNews/
GetEconomyIndicator]
L --> M[akshareapi endpoints
with retry and fallback]
M --> L
L --> K
K --> N[ToolOutput JSON
FinanceToolResult]
N --> O[Ark Responses
next model turn
with ToolOutput]
O --> P[Chatflow runtime
compose final reply]
P --> Q[LarkBot sends
Agentic Chat For You card]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 4 个问题，并给出了一些整体性的反馈：
- 只读金融处理器（
FinanceMarketData、FinanceNews、EconomyIndicator）在构造FinanceProvider时使用了空的 baseURL，但FinanceProvider最终依赖的httpProvider会使用这个 base URL。为了更安全，建议像现有 aktool 接线一样，把配置好的 akshare base URL 贯穿传入这些 handler，而不是硬编码为""。 - 在
finance_tool_discover中，tool_names的输入 schema 被声明为一个泛型数组，但没有指定items的类型约束，而代码假设这里是字符串数组；建议在参数 schema 中显式将items声明为string，以便让契约更清晰，并减少模型误用的风险。
给 AI Agent 的提示词
请根据下面的代码评审意见进行修改：
## 整体评论
- 只读金融处理器（`FinanceMarketData`、`FinanceNews`、`EconomyIndicator`）在构造 `FinanceProvider` 时使用了空的 baseURL，但 `FinanceProvider` 最终依赖的 `httpProvider` 会使用这个 base URL。为了更安全，建议像现有 aktool 接线一样，把配置好的 akshare base URL 贯穿传入这些 handler，而不是硬编码为 `""`。
- 在 `finance_tool_discover` 中，`tool_names` 的输入 schema 被声明为一个泛型数组，但没有指定 `items` 的类型约束，而代码假设这里是字符串数组；建议在参数 schema 中显式将 `items` 声明为 `string`，以便让契约更清晰，并减少模型误用的风险。
## 单独评论
### 评论 1
<location path="internal/application/lark/handlers/finance_tools_test.go" line_range="89-44" />
<code_context>
+func TestFinanceMarketDataHandlerReturnsStructuredJSON(t *testing.T) {
</code_context>
<issue_to_address>
**suggestion (testing):** 为 FinanceNews 和 EconomyIndicator 处理器补充覆盖测试，包括错误路径
为了保持整个金融能力面的一致性，请同时：
- 为 `FinanceNews` 和 `EconomyIndicator` 增加成功路径测试，使用 `httptest.Server` 模拟 `stock_news_em` / `stock_news_main_cx` 以及相关宏观接口。
- 为每个处理器增加一个错误路径测试（例如上游返回 502），并断言该处理器会向上透传错误，同时不会设置 result extra。
这样可以让三个金融处理器在行为和测试覆盖上保持一致。
建议实现：
```golang
func TestFinanceMarketDataHandlerReturnsStructuredJSON(t *testing.T) {
t.Helper()
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/public/spot_quotations_sge" {
t.Fatalf("unexpected path: %s", r.URL.Path)
}
_ = json.NewEncoder(w).Encode([]map[string]any{
{"品种": "Au99.99", "时间": "09:31:00", "现价": 578.32, "更新时间": "2026-03-20 09:31:00"},
})
}))
```
你需要在 `internal/application/lark/handlers/finance_tools_test.go` 中新增四个测试，遵循 `TestFinanceMarketDataHandlerReturnsStructuredJSON` 已有的风格和辅助方法。
下面的示例假设：
- 你已有与行情处理器类似的构造函数/辅助函数（例如 `newFinanceNewsHandler(baseURL string)` / `newEconomyIndicatorHandler(baseURL string)`，或者一个通用的金融工具工厂）。
- 每个处理器都会往 Lark 响应的 `Result.Extra` map 中写入数据（例如 `resp.Result.Extra["finance_news"]`、`resp.Result.Extra["economy_indicator"]`），并且错误透传要么通过 HTTP 状态码可见，要么通过结果中的 error 可见。
根据你实际的代码来调整名称和接线（处理器名称、extra key、辅助函数等）：
```go
func TestFinanceNewsHandler_Success(t *testing.T) {
t.Helper()
// 模拟上游 stock_news_em / stock_news_main_cx API。
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/stock_news_em":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"title": "EM headline 1", "time": "2026-03-20 09:30:00"},
})
case "/api/stock_news_main_cx":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"title": "Main CX headline 1", "time": "2026-03-20 09:31:00"},
})
default:
t.Fatalf("unexpected path: %s", r.URL.Path)
}
}))
defer srv.Close()
// TODO: 用你真实的 FinanceNews handler 构造方式替换这里。
// 比如如果有接受 base URL 的构造函数：
// handler := newFinanceNewsHandler(srv.URL)
handler := newFinanceNewsHandlerForTest(t, srv.URL) // 你定义的辅助函数，与行情 handler 的测试辅助函数保持一致
req := httptest.NewRequest(http.MethodPost, "/finance/news", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusOK)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
newsAny, ok := resp.Result.Extra["finance_news"]
if !ok {
t.Fatalf("expected finance_news extra to be set")
}
news, ok := newsAny.([]any)
if !ok {
t.Fatalf("finance_news extra has unexpected type %T", newsAny)
}
if len(news) == 0 {
t.Fatalf("finance_news extra should not be empty")
}
}
func TestFinanceNewsHandler_UpstreamError(t *testing.T) {
t.Helper()
// 上游返回 502 来模拟 provider 失败。
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "bad gateway", http.StatusBadGateway)
}))
defer srv.Close()
handler := newFinanceNewsHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/news", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
// 断言 handler 会透传上游错误。
if rr.Code != http.StatusBadGateway {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusBadGateway)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err == nil {
if resp.Result.Extra != nil {
if _, ok := resp.Result.Extra["finance_news"]; ok {
t.Fatalf("finance_news extra should not be set on upstream error")
}
}
}
}
func TestEconomyIndicatorHandler_Success(t *testing.T) {
t.Helper()
// 模拟上游宏观接口。
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/macro_gdp":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"year": 2025, "gdp": 123.45},
})
case "/api/macro_cpi":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"month": "2025-12", "cpi": 2.3},
})
default:
t.Fatalf("unexpected path: %s", r.URL.Path)
}
}))
defer srv.Close()
handler := newEconomyIndicatorHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/economy_indicator", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusOK)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
indicatorAny, ok := resp.Result.Extra["economy_indicator"]
if !ok {
t.Fatalf("expected economy_indicator extra to be set")
}
indicators, ok := indicatorAny.([]any)
if !ok {
t.Fatalf("economy_indicator extra has unexpected type %T", indicatorAny)
}
if len(indicators) == 0 {
t.Fatalf("economy_indicator extra should not be empty")
}
}
func TestEconomyIndicatorHandler_UpstreamError(t *testing.T) {
t.Helper()
// 上游宏观接口失败。
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "bad gateway", http.StatusBadGateway)
}))
defer srv.Close()
handler := newEconomyIndicatorHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/economy_indicator", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusBadGateway {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusBadGateway)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err == nil {
if resp.Result.Extra != nil {
if _, ok := resp.Result.Extra["economy_indicator"]; ok {
t.Fatalf("economy_indicator extra should not be set on upstream error")
}
}
}
}
```
你还需要一些小的辅助函数，使之与当前行情处理器测试保持一致，例如：
```go
func newFinanceNewsHandlerForTest(t *testing.T, baseURL string) http.Handler {
t.Helper()
// 依照行情处理器在测试中的依赖注入方式来接线，
// 但将 baseURL 用作上游接口的根路径。
return NewFinanceNewsHandler(
http.DefaultClient,
baseURL,
// ... 其他与生产构造保持一致的依赖 ...
)
}
func newEconomyIndicatorHandlerForTest(t *testing.T, baseURL string) http.Handler {
t.Helper()
return NewEconomyIndicatorHandler(
http.DefaultClient,
baseURL,
// ... 其他依赖 ...
)
}
```
请根据你实际的 handler 和测试约定调整：
- `NewFinanceNewsHandler`、`NewEconomyIndicatorHandler`,
- 请求路径（`/finance/news`、`/finance/economy_indicator`），
- extra key（`"finance_news"`、`"economy_indicator"`），
- 以及上游路径（`/api/stock_news_em` 等）。
关键的行为要求是：
- 成功用例：确保 handler 能正确地将 JSON 结构化写入对应的 `Result.Extra` 条目中。
- 错误用例：确保 handler 会返回上游的错误状态码，并且不会设置对应的 `Result.Extra` 条目。
</issue_to_address>
### 评论 2
<location path="internal/application/lark/handlers/member_tools_test.go" line_range="61-39" />
<code_context>
+func TestRecentActiveMembersHandlerUsesCurrentChatScopeAndDedupsByRecency(t *testing.T) {
</code_context>
<issue_to_address>
**suggestion (testing):** 建议在 member 工具中增加关于 limit 归一化和大 lookback 值的测试
为了完整覆盖 `normalizeMembersLimit`、`normalizeRecentActiveTopK` 和 `normalizeLookbackMessages` 的边界逻辑，请增加以下测试：
- 使用非常大的 `TopK` 和 `LookbackMessages` 值（例如 1000）调用 `RecentActiveMembers.Handle`，并断言传入 `recentActiveMembersHistoryLoader` 的 `size` 被限制在 200 之内，同时返回的成员切片长度被限制在 50 之内。
- 使用 `Limit` > 200 调用 `ChatMembers.Handle`，并断言序列化结果中只包含 200 个成员。
这样可以锁定针对无界查询和 payload 的保护逻辑。
建议实现：
```golang
func TestRecentActiveMembersHandlerNormalizesLargeTopKAndLookback(t *testing.T) {
old := recentActiveMembersHistoryLoader
defer func() {
recentActiveMembersHistoryLoader = old
}()
var (
capturedChatID string
capturedSize int
)
recentActiveMembersHistoryLoader = func(ctx context.Context, chatID string, size int) (history.OpensearchMsgLogList, error) {
capturedChatID = chatID
capturedSize = size
var empty history.OpensearchMsgLogList
return empty, nil
}
args := RecentActiveMembersArgs{
ChatID: "chat_id",
TopK: 1000,
LookbackMessages: 1000,
}
err := RecentActiveMembers.Handle(context.Background(), nil, &xhandler.BaseMetaData{}, args)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if capturedChatID != "chat_id" {
t.Fatalf("expected chat_id %q, got %q", "chat_id", capturedChatID)
}
if capturedSize != 200 {
t.Fatalf("expected normalized history size 200, got %d", capturedSize)
}
}
func TestRecentActiveMembersHandlerUsesCurrentChatScopeAndDedupsByRecency(t *testing.T) {
```
要完全落实这条评审意见，你还需要：
1. 为 `RecentActiveMembers.Handle` 编写一个对应测试，断言返回的成员切片长度被限制在 50：
- 仍然复用 `recentActiveMembersHistoryLoader` 覆盖，但让它返回超过 50 条的模拟消息；
- 按照本测试文件已有的响应写入模式来截获并解析 `RecentActiveMembers.Handle` 的响应（例如使用 `httptest.NewRecorder` + `json.Unmarshal` 到已有的响应结构体中）；
- 即便 loader 返回了超过 50 条消息并且 `TopK` 设置为一个很大的值（例如 1000），也要断言解析出的成员切片长度是 50。
2. 为 `ChatMembers.Handle` 新增一个测试来断言 limit 归一化：
- 覆盖 `ChatMembers` 内部使用的 loader（例如本文件中的 `chatMembersLoader` 或等价变量），记录传入的 limit 值，并/或返回可控的成员集合；
- 使用 `ChatMembersArgs{ChatID: "chat_id", Limit: 1000}` 调用 `ChatMembers.Handle`；
- 以与本文件其他 `ChatMembers` 测试相同的方式解析序列化响应；
- 断言序列化结果中最多只有 200 个成员，并（可选地）断言传递给底层 loader 的 limit 也是 200。
如果你项目中 `RecentActiveMembersArgs` 的字段名以及 `ChatMembers` 所用 loader 的变量名不完全一致，请相应地调整。
</issue_to_address>
### 评论 3
<location path="internal/infrastructure/ark_dal/responses_manual_test.go" line_range="76-42" />
<code_context>
+ }
+}
+
+func TestBuildTurnRequestDedupesAdditionalToolsByName(t *testing.T) {
+ turn := New[struct{}]("oc_chat", "ou_actor", nil).WithTools(
+ arktools.New[struct{}]().Add(
+ arktools.NewUnit[struct{}]().
+ Name("finance_market_data_get").
+ Desc("默认工具").
+ Params(arktools.NewParams("object")),
+ ),
+ )
+
+ req, err := turn.buildTurnRequest(ResponseTurnRequest{
+ ModelID: "ep-test",
+ SystemPrompt: "system",
+ UserPrompt: "user",
+ AdditionalTools: []*responses.ResponsesTool{
+ testResponseFunctionTool("finance_market_data_get"),
+ },
+ })
+ if err != nil {
+ t.Fatalf("buildTurnRequest() error = %v", err)
+ }
+ if len(req.Tools) != 1 {
+ t.Fatalf("tool count = %d, want 1", len(req.Tools))
+ }
+}
+
+func testResponseFunctionTool(name string) *responses.ResponsesTool {
</code_context>
<issue_to_address>
**suggestion (testing):** 为非 function 工具（例如 web_search）以及 mergeResponseTools 中的 nil AdditionalTools 增加测试
当前测试覆盖了函数工具去重，但还没有覆盖 `mergeResponseTools` 中对 web search 工具和 nil 条目的特殊处理。
请再增加以下测试：
- 使用一个基础切片，其中包含一个函数工具和一个 web search 工具；再传入 `AdditionalTools`，其中重复 web search 工具并包含 `nil`；断言合并后的工具集合中只保留一个 web search 工具，且不包含 nil。
- 使用 `AdditionalTools` 为 `nil` 调用 `buildTurnRequest`，以确认行为与之前一致且不会 panic。
这样可以在未来增加更多工具类型时，更好地防止工具合并逻辑回归。
建议实现：
```golang
func TestBuildTurnRequestDedupesAdditionalToolsByName(t *testing.T) {
turn := New[struct{}]("oc_chat", "ou_actor", nil).WithTools(
arktools.New[struct{}]().Add(
arktools.NewUnit[struct{}]().
Name("finance_market_data_get").
Desc("默认工具").
Params(arktools.NewParams("object")),
),
)
req, err := turn.buildTurnRequest(ResponseTurnRequest{
ModelID: "ep-test",
SystemPrompt: "system",
UserPrompt: "user",
AdditionalTools: []*responses.ResponsesTool{
testResponseFunctionTool("finance_market_data_get"),
},
})
if err != nil {
t.Fatalf("buildTurnRequest() error = %v", err)
}
if len(req.Tools) != 1 {
t.Fatalf("tool count = %d, want 1", len(req.Tools))
}
}
func TestMergeResponseTools_WebSearchAndNil(t *testing.T) {
t.Helper()
base := []*responses.ResponsesTool{
testResponseFunctionTool("fn_base"),
testResponseWebSearchTool(),
}
additional := []*responses.ResponsesTool{
testResponseWebSearchTool(), // duplicate web search tool
nil, // explicit nil entry should be ignored
}
merged := mergeResponseTools(base, additional)
if merged == nil {
t.Fatalf("mergeResponseTools() returned nil slice")
}
if len(merged) != 2 {
t.Fatalf("merged tool count = %d, want %d", len(merged), 2)
}
var (
functionCount int
webSearchCount int
nilCount int
)
for i, tool := range merged {
if tool == nil {
nilCount++
t.Fatalf("merged tool at index %d is nil; nil tools should be filtered out", i)
}
switch {
case tool.GetToolFunction() != nil:
functionCount++
case tool.GetToolWebSearch() != nil:
webSearchCount++
}
}
if functionCount != 1 {
t.Fatalf("function tool count = %d, want %d", functionCount, 1)
}
if webSearchCount != 1 {
t.Fatalf("web search tool count = %d, want %d", webSearchCount, 1)
}
if nilCount != 0 {
t.Fatalf("nil tool count = %d, want %d", nilCount, 0)
}
}
func TestBuildTurnRequestHandlesNilAdditionalTools(t *testing.T) {
turn := New[struct{}]("oc_chat", "ou_actor", nil).WithTools(
arktools.New[struct{}]().Add(
arktools.NewUnit[struct{}]().
Name("finance_market_data_get").
Desc("默认工具").
Params(arktools.NewParams("object")),
),
)
req, err := turn.buildTurnRequest(ResponseTurnRequest{
ModelID: "ep-test",
SystemPrompt: "system",
UserPrompt: "user",
AdditionalTools: nil, // explicitly nil, should not panic and should preserve base tools
})
if err != nil {
t.Fatalf("buildTurnRequest() with nil AdditionalTools error = %v", err)
}
if req == nil {
t.Fatalf("buildTurnRequest() returned nil request")
}
if len(req.Tools) != 1 {
t.Fatalf("tool count with nil AdditionalTools = %d, want %d", len(req.Tools), 1)
}
}
func testResponseFunctionTool(name string) *responses.ResponsesTool {
return &responses.ResponsesTool{
Union: &responses.ResponsesTool_ToolFunction{
ToolFunction: &responses.ToolFunction{
Name: name,
Type: responses.ToolType_function,
Description: gptr.Of("dynamic tool"),
Parameters: &responses.Bytes{Value: []byte(`{"type":"object","properties":{}}`)},
},
},
}
}
func testResponseWebSearchTool() *responses.ResponsesTool {
return &responses.ResponsesTool{
Union: &responses.ResponsesTool_ToolWebSearch{
ToolWebSearch: &responses.ToolWebSearch{
// Type: responses.ToolType_web_search,
// Populate additional fields as required by the WebSearch tool definition.
},
},
}
}
```
补充说明：
1. `testResponseWebSearchTool` 帮助函数假设在 `responses` 包中存在 `ResponsesTool_ToolWebSearch` 这个 oneof 包装类型以及 `ToolWebSearch` 消息，并且 `ResponsesTool` 上存在 `GetToolWebSearch()` 访问方法。请根据你实际生成的 protobuf API 调整类型和字段名（例如 `ResponsesTool_WebSearch`、`WebSearch` 等），并添加所有必需字段（比如 `Type: responses.ToolType_web_search` 或配置字段），以保证该工具在你的服务中是合法的。
2. 如果 `mergeResponseTools` 位于其他包中或者在另一个文件中是未导出的，请确保该测试文件和它在同一个包中（通常是 `ark_dal`），这样才能直接调用 `mergeResponseTools`。如果它在其他包中，要么将其导出，要么新增一个供测试使用的小的导出封装函数。
3. 如果 `buildTurnRequest` 在 `AdditionalTools == nil` 时以往的行为期望不同的工具数量（例如，当没有基础工具配置时期望工具数量为 0），请根据你代码库中的实际基线行为调整 `TestBuildTurnRequestHandlesNilAdditionalTools` 中对 `len(req.Tools)` 的断言。
</issue_to_address>
### 评论 4
<location path="docs/superpowers/plans/2026-03-26-finance-tools-akshare-plan.md" line_range="53" />
<code_context>
+
+### Important Behavioral Change Since Last Review
+- `toolmeta/runtime_behavior.go` 现在把 `gold_price_get` 和 `stock_zh_a_get` 标记为需要审批的 chat-write 工具。
+- 因为它们会发送卡片，当前 runtime prompt 语义上也会把它们当“执行动作”而不是“只读取数”。
+- 所以新的金融 discover / API 工具层应该和现有展示型工具分层，而不是直接把旧工具重命名。
+
</code_context>
<issue_to_address>
**nitpick (typo):** 建议将“只读取数”改为更常见的“只读取数据”
“只读取数”读起来像是被截断的表达，在这份文档中略显不自然。建议改为“只读取数据”，可以更准确地表达只读能力，并与整体术语风格保持一致。
```suggestion
- 因为它们会发送卡片，当前 runtime prompt 语义上也会把它们当“执行动作”而不是“只读取数据”。
```
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会根据你的反馈改进后续评审质量。
Original comment in English
Hey - I've found 4 issues, and left some high level feedback:
- The read-only finance handlers (
FinanceMarketData,FinanceNews,EconomyIndicator) construct theirFinanceProviderwith an empty baseURL, butFinanceProviderultimately relies onhttpProviderusing that base URL, so it would be safer to thread the configured akshare base URL (similar to existing aktool wiring) into these handlers instead of hardcoding"". - In
finance_tool_discover, the input schema fortool_namesis declared as a generic array without anitemstype constraint, while the code assumes an array of strings; consider specifyingitemsasstringin the param schema to make the contract clearer and reduce model misuse.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The read-only finance handlers (`FinanceMarketData`, `FinanceNews`, `EconomyIndicator`) construct their `FinanceProvider` with an empty baseURL, but `FinanceProvider` ultimately relies on `httpProvider` using that base URL, so it would be safer to thread the configured akshare base URL (similar to existing aktool wiring) into these handlers instead of hardcoding `""`.
- In `finance_tool_discover`, the input schema for `tool_names` is declared as a generic array without an `items` type constraint, while the code assumes an array of strings; consider specifying `items` as `string` in the param schema to make the contract clearer and reduce model misuse.
## Individual Comments
### Comment 1
<location path="internal/application/lark/handlers/finance_tools_test.go" line_range="89-44" />
<code_context>
+func TestFinanceMarketDataHandlerReturnsStructuredJSON(t *testing.T) {
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for FinanceNews and EconomyIndicator handlers, including error paths
To keep the finance surface consistent, please also:
- Add success-path tests for `FinanceNews` and `EconomyIndicator` using `httptest.Server` to simulate `stock_news_em` / `stock_news_main_cx` and the macro endpoints.
- Add one error-path test per handler (e.g., upstream 502) asserting the handler propagates the error and does not set the result extra.
This will align behavior and coverage across all three finance handlers.
Suggested implementation:
```golang
func TestFinanceMarketDataHandlerReturnsStructuredJSON(t *testing.T) {
t.Helper()
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/public/spot_quotations_sge" {
t.Fatalf("unexpected path: %s", r.URL.Path)
}
_ = json.NewEncoder(w).Encode([]map[string]any{
{"品种": "Au99.99", "时间": "09:31:00", "现价": 578.32, "更新时间": "2026-03-20 09:31:00"},
})
}))
```
You’ll need to add four new tests to `internal/application/lark/handlers/finance_tools_test.go` following the same style and helpers used for `TestFinanceMarketDataHandlerReturnsStructuredJSON`.
The examples below assume:
- You have constructor/helpers similar to those used for the market data handler (e.g., something like `newFinanceNewsHandler(baseURL string)` / `newEconomyIndicatorHandler(baseURL string)` or a generic factory for finance tools).
- Each handler writes into some `Result.Extra` map in the Lark response (e.g. `resp.Result.Extra["finance_news"]`, `resp.Result.Extra["economy_indicator"]`), and error propagation is visible either as an HTTP status code or an error in the result.
Adapt the names/wiring to match your actual code (handlers, extra keys, helper functions):
```go
func TestFinanceNewsHandler_Success(t *testing.T) {
t.Helper()
// Simulate upstream stock_news_em / stock_news_main_cx API.
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/stock_news_em":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"title": "EM headline 1", "time": "2026-03-20 09:30:00"},
})
case "/api/stock_news_main_cx":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"title": "Main CX headline 1", "time": "2026-03-20 09:31:00"},
})
default:
t.Fatalf("unexpected path: %s", r.URL.Path)
}
}))
defer srv.Close()
// TODO: Replace this with the real way you construct the FinanceNews handler.
// For example, if there's a constructor taking the base URL:
// handler := newFinanceNewsHandler(srv.URL)
handler := newFinanceNewsHandlerForTest(t, srv.URL) // helper you define mirroring the market-data one
req := httptest.NewRequest(http.MethodPost, "/finance/news", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusOK)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
newsAny, ok := resp.Result.Extra["finance_news"]
if !ok {
t.Fatalf("expected finance_news extra to be set")
}
news, ok := newsAny.([]any)
if !ok {
t.Fatalf("finance_news extra has unexpected type %T", newsAny)
}
if len(news) == 0 {
t.Fatalf("finance_news extra should not be empty")
}
}
func TestFinanceNewsHandler_UpstreamError(t *testing.T) {
t.Helper()
// Upstream returns 502 to simulate a failing provider.
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "bad gateway", http.StatusBadGateway)
}))
defer srv.Close()
handler := newFinanceNewsHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/news", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
// Assert that the handler propagates the upstream error.
if rr.Code != http.StatusBadGateway {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusBadGateway)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err == nil {
if resp.Result.Extra != nil {
if _, ok := resp.Result.Extra["finance_news"]; ok {
t.Fatalf("finance_news extra should not be set on upstream error")
}
}
}
}
func TestEconomyIndicatorHandler_Success(t *testing.T) {
t.Helper()
// Simulate upstream macro endpoints.
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/macro_gdp":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"year": 2025, "gdp": 123.45},
})
case "/api/macro_cpi":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"month": "2025-12", "cpi": 2.3},
})
default:
t.Fatalf("unexpected path: %s", r.URL.Path)
}
}))
defer srv.Close()
handler := newEconomyIndicatorHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/economy_indicator", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusOK)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
indicatorAny, ok := resp.Result.Extra["economy_indicator"]
if !ok {
t.Fatalf("expected economy_indicator extra to be set")
}
indicators, ok := indicatorAny.([]any)
if !ok {
t.Fatalf("economy_indicator extra has unexpected type %T", indicatorAny)
}
if len(indicators) == 0 {
t.Fatalf("economy_indicator extra should not be empty")
}
}
func TestEconomyIndicatorHandler_UpstreamError(t *testing.T) {
t.Helper()
// Upstream macro endpoint failure.
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "bad gateway", http.StatusBadGateway)
}))
defer srv.Close()
handler := newEconomyIndicatorHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/economy_indicator", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusBadGateway {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusBadGateway)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err == nil {
if resp.Result.Extra != nil {
if _, ok := resp.Result.Extra["economy_indicator"]; ok {
t.Fatalf("economy_indicator extra should not be set on upstream error")
}
}
}
}
```
You will also need small helpers that parallel whatever you already do for the market data handler, for example:
```go
func newFinanceNewsHandlerForTest(t *testing.T, baseURL string) http.Handler {
t.Helper()
// Wire dependencies the same way as for the market data handler under test,
// but using baseURL as the upstream endpoint root.
return NewFinanceNewsHandler(
http.DefaultClient,
baseURL,
// ... any other dependencies mirrored from production construction ...
)
}
func newEconomyIndicatorHandlerForTest(t *testing.T, baseURL string) http.Handler {
t.Helper()
return NewEconomyIndicatorHandler(
http.DefaultClient,
baseURL,
// ... any other dependencies ...
)
}
```
Adjust:
- `NewFinanceNewsHandler`, `NewEconomyIndicatorHandler`,
- request paths (`/finance/news`, `/finance/economy_indicator`),
- extra keys (`"finance_news"`, `"economy_indicator"`),
- and upstream paths (`/api/stock_news_em`, etc.)
to match your actual handlers and existing test conventions. The key behavioral requirements are:
- Success tests: ensure the handler correctly structures JSON into the `Result.Extra` entry.
- Error tests: ensure the handler returns the upstream error status and does not set the corresponding `Result.Extra` entry.
</issue_to_address>
### Comment 2
<location path="internal/application/lark/handlers/member_tools_test.go" line_range="61-39" />
<code_context>
+func TestRecentActiveMembersHandlerUsesCurrentChatScopeAndDedupsByRecency(t *testing.T) {
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for limit normalization and large lookback values in member tools
To fully exercise the boundary logic in `normalizeMembersLimit`, `normalizeRecentActiveTopK`, and `normalizeLookbackMessages`, please add tests that:
- Call `RecentActiveMembers.Handle` with very large `TopK` and `LookbackMessages` values (e.g., 1000) and assert that the `size` passed to `recentActiveMembersHistoryLoader` is capped at 200 and the returned slice length is capped at 50.
- Call `ChatMembers.Handle` with a `Limit` > 200 and assert that only 200 members are present in the serialized result.
This will lock in the safeguards against unbounded queries and payloads.
Suggested implementation:
```golang
func TestRecentActiveMembersHandlerNormalizesLargeTopKAndLookback(t *testing.T) {
old := recentActiveMembersHistoryLoader
defer func() {
recentActiveMembersHistoryLoader = old
}()
var (
capturedChatID string
capturedSize int
)
recentActiveMembersHistoryLoader = func(ctx context.Context, chatID string, size int) (history.OpensearchMsgLogList, error) {
capturedChatID = chatID
capturedSize = size
var empty history.OpensearchMsgLogList
return empty, nil
}
args := RecentActiveMembersArgs{
ChatID: "chat_id",
TopK: 1000,
LookbackMessages: 1000,
}
err := RecentActiveMembers.Handle(context.Background(), nil, &xhandler.BaseMetaData{}, args)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if capturedChatID != "chat_id" {
t.Fatalf("expected chat_id %q, got %q", "chat_id", capturedChatID)
}
if capturedSize != 200 {
t.Fatalf("expected normalized history size 200, got %d", capturedSize)
}
}
func TestRecentActiveMembersHandlerUsesCurrentChatScopeAndDedupsByRecency(t *testing.T) {
```
To fully implement your review comment, you will also need:
1. A corresponding test for `RecentActiveMembers.Handle` that asserts the returned member slice is capped at 50. This will require:
- Using the same `recentActiveMembersHistoryLoader` override but returning more than 50 synthetic messages.
- Capturing and decoding the response from `RecentActiveMembers.Handle` using whatever response-writing pattern is already present in this test file (e.g., `httptest.NewRecorder` + `json.Unmarshal` into the existing response struct).
- Asserting that the decoded members slice length is 50 even when more than 50 messages are returned from the loader and `TopK` is set to a large value (e.g., 1000).
2. A new test for `ChatMembers.Handle` that asserts limit normalization:
- Override the loader used inside `ChatMembers` (e.g., `chatMembersLoader` or equivalent in this file) to record the requested limit and/or to return a controllable set of members.
- Call `ChatMembers.Handle` with `ChatMembersArgs{ChatID: "chat_id", Limit: 1000}`.
- Decode the serialized response using the same approach as other `ChatMembers` tests in this file.
- Assert that at most 200 members are present in the serialized result, and (optionally) that the limit passed to the underlying loader is also 200.
Adjust the field names in `RecentActiveMembersArgs` and the loader variable name for `ChatMembers` to match the actual definitions in your codebase if they differ.
</issue_to_address>
### Comment 3
<location path="internal/infrastructure/ark_dal/responses_manual_test.go" line_range="76-42" />
<code_context>
+ }
+}
+
+func TestBuildTurnRequestDedupesAdditionalToolsByName(t *testing.T) {
+ turn := New[struct{}]("oc_chat", "ou_actor", nil).WithTools(
+ arktools.New[struct{}]().Add(
+ arktools.NewUnit[struct{}]().
+ Name("finance_market_data_get").
+ Desc("默认工具").
+ Params(arktools.NewParams("object")),
+ ),
+ )
+
+ req, err := turn.buildTurnRequest(ResponseTurnRequest{
+ ModelID: "ep-test",
+ SystemPrompt: "system",
+ UserPrompt: "user",
+ AdditionalTools: []*responses.ResponsesTool{
+ testResponseFunctionTool("finance_market_data_get"),
+ },
+ })
+ if err != nil {
+ t.Fatalf("buildTurnRequest() error = %v", err)
+ }
+ if len(req.Tools) != 1 {
+ t.Fatalf("tool count = %d, want 1", len(req.Tools))
+ }
+}
+
+func testResponseFunctionTool(name string) *responses.ResponsesTool {
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for non-function tools (e.g., web_search) and nil additional tools in mergeResponseTools
The current tests cover function tool deduplication but not the special handling for web search tools and nil entries in `mergeResponseTools`.
Please also add tests that:
- Use a base slice with a function tool and a web search tool plus `AdditionalTools` that repeats the web search tool and includes `nil`, and assert the merged tools contain a single web search tool and no nils.
- Call `buildTurnRequest` with `AdditionalTools` set to `nil` to confirm it matches previous behavior and does not panic.
This will better protect the tool-merging behavior from regressions as more tool types are added.
Suggested implementation:
```golang
func TestBuildTurnRequestDedupesAdditionalToolsByName(t *testing.T) {
turn := New[struct{}]("oc_chat", "ou_actor", nil).WithTools(
arktools.New[struct{}]().Add(
arktools.NewUnit[struct{}]().
Name("finance_market_data_get").
Desc("默认工具").
Params(arktools.NewParams("object")),
),
)
req, err := turn.buildTurnRequest(ResponseTurnRequest{
ModelID: "ep-test",
SystemPrompt: "system",
UserPrompt: "user",
AdditionalTools: []*responses.ResponsesTool{
testResponseFunctionTool("finance_market_data_get"),
},
})
if err != nil {
t.Fatalf("buildTurnRequest() error = %v", err)
}
if len(req.Tools) != 1 {
t.Fatalf("tool count = %d, want 1", len(req.Tools))
}
}
func TestMergeResponseTools_WebSearchAndNil(t *testing.T) {
t.Helper()
base := []*responses.ResponsesTool{
testResponseFunctionTool("fn_base"),
testResponseWebSearchTool(),
}
additional := []*responses.ResponsesTool{
testResponseWebSearchTool(), // duplicate web search tool
nil, // explicit nil entry should be ignored
}
merged := mergeResponseTools(base, additional)
if merged == nil {
t.Fatalf("mergeResponseTools() returned nil slice")
}
if len(merged) != 2 {
t.Fatalf("merged tool count = %d, want %d", len(merged), 2)
}
var (
functionCount int
webSearchCount int
nilCount int
)
for i, tool := range merged {
if tool == nil {
nilCount++
t.Fatalf("merged tool at index %d is nil; nil tools should be filtered out", i)
}
switch {
case tool.GetToolFunction() != nil:
functionCount++
case tool.GetToolWebSearch() != nil:
webSearchCount++
}
}
if functionCount != 1 {
t.Fatalf("function tool count = %d, want %d", functionCount, 1)
}
if webSearchCount != 1 {
t.Fatalf("web search tool count = %d, want %d", webSearchCount, 1)
}
if nilCount != 0 {
t.Fatalf("nil tool count = %d, want %d", nilCount, 0)
}
}
func TestBuildTurnRequestHandlesNilAdditionalTools(t *testing.T) {
turn := New[struct{}]("oc_chat", "ou_actor", nil).WithTools(
arktools.New[struct{}]().Add(
arktools.NewUnit[struct{}]().
Name("finance_market_data_get").
Desc("默认工具").
Params(arktools.NewParams("object")),
),
)
req, err := turn.buildTurnRequest(ResponseTurnRequest{
ModelID: "ep-test",
SystemPrompt: "system",
UserPrompt: "user",
AdditionalTools: nil, // explicitly nil, should not panic and should preserve base tools
})
if err != nil {
t.Fatalf("buildTurnRequest() with nil AdditionalTools error = %v", err)
}
if req == nil {
t.Fatalf("buildTurnRequest() returned nil request")
}
if len(req.Tools) != 1 {
t.Fatalf("tool count with nil AdditionalTools = %d, want %d", len(req.Tools), 1)
}
}
func testResponseFunctionTool(name string) *responses.ResponsesTool {
return &responses.ResponsesTool{
Union: &responses.ResponsesTool_ToolFunction{
ToolFunction: &responses.ToolFunction{
Name: name,
Type: responses.ToolType_function,
Description: gptr.Of("dynamic tool"),
Parameters: &responses.Bytes{Value: []byte(`{"type":"object","properties":{}}`)},
},
},
}
}
func testResponseWebSearchTool() *responses.ResponsesTool {
return &responses.ResponsesTool{
Union: &responses.ResponsesTool_ToolWebSearch{
ToolWebSearch: &responses.ToolWebSearch{
// Type: responses.ToolType_web_search,
// Populate additional fields as required by the WebSearch tool definition.
},
},
}
}
```
1. The helper `testResponseWebSearchTool` assumes the existence of the `ResponsesTool_ToolWebSearch` oneof wrapper and a `ToolWebSearch` message on the `responses` package, plus a `GetToolWebSearch()` accessor on `ResponsesTool`. Adjust the type and field names to match your actual generated protobuf API (e.g., `ResponsesTool_WebSearch`, `WebSearch`, etc.), and add any required fields (such as `Type: responses.ToolType_web_search` or configuration fields) so the tool is valid for your service.
2. If `mergeResponseTools` lives in a different package or is unexported in another file, ensure this test file is in the same package (likely `ark_dal`) so it can call `mergeResponseTools` directly. If it is in another package, either export it or add a small exported wrapper for testing.
3. If the previous behavior for `buildTurnRequest` with `AdditionalTools == nil` expects a different tool count (e.g., zero tools when no base tools are configured), adjust the `len(req.Tools)` assertion in `TestBuildTurnRequestHandlesNilAdditionalTools` to match the expected baseline behavior in your codebase.
</issue_to_address>
### Comment 4
<location path="docs/superpowers/plans/2026-03-26-finance-tools-akshare-plan.md" line_range="53" />
<code_context>
+
+### Important Behavioral Change Since Last Review
+- `toolmeta/runtime_behavior.go` 现在把 `gold_price_get` 和 `stock_zh_a_get` 标记为需要审批的 chat-write 工具。
+- 因为它们会发送卡片，当前 runtime prompt 语义上也会把它们当“执行动作”而不是“只读取数”。
+- 所以新的金融 discover / API 工具层应该和现有展示型工具分层，而不是直接把旧工具重命名。
+
</code_context>
<issue_to_address>
**nitpick (typo):** Consider changing “只读取数” to the more standard “只读取数据”.
“只读取数”像是被截断的表述，在这份文档里读起来有些不自然。建议改为“只读取数据”，以更准确表达只读能力并与整体术语风格保持一致。
```suggestion
- 因为它们会发送卡片，当前 runtime prompt 语义上也会把它们当“执行动作”而不是“只读取数据”。
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if len(result.Tools[0].Categories) == 0 || result.Tools[0].Categories[0] != "economy" { | ||
| t.Fatalf("categories = %+v, want include economy", result.Tools[0].Categories) | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion (testing): 为 FinanceNews 和 EconomyIndicator 处理器补充覆盖测试，包括错误路径
为了保持整个金融能力面的一致性，请同时：
- 为
FinanceNews和EconomyIndicator增加成功路径测试，使用httptest.Server模拟stock_news_em/stock_news_main_cx以及相关宏观接口。 - 为每个处理器增加一个错误路径测试（例如上游返回 502），并断言该处理器会向上透传错误，同时不会设置 result extra。
这样可以让三个金融处理器在行为和测试覆盖上保持一致。
建议实现：
func TestFinanceMarketDataHandlerReturnsStructuredJSON(t *testing.T) {
t.Helper()
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/public/spot_quotations_sge" {
t.Fatalf("unexpected path: %s", r.URL.Path)
}
_ = json.NewEncoder(w).Encode([]map[string]any{
{"品种": "Au99.99", "时间": "09:31:00", "现价": 578.32, "更新时间": "2026-03-20 09:31:00"},
})
}))你需要在 internal/application/lark/handlers/finance_tools_test.go 中新增四个测试，遵循 TestFinanceMarketDataHandlerReturnsStructuredJSON 已有的风格和辅助方法。
下面的示例假设：
- 你已有与行情处理器类似的构造函数/辅助函数（例如
newFinanceNewsHandler(baseURL string)/newEconomyIndicatorHandler(baseURL string)，或者一个通用的金融工具工厂）。 - 每个处理器都会往 Lark 响应的
Result.Extramap 中写入数据（例如resp.Result.Extra["finance_news"]、resp.Result.Extra["economy_indicator"]），并且错误透传要么通过 HTTP 状态码可见，要么通过结果中的 error 可见。
根据你实际的代码来调整名称和接线（处理器名称、extra key、辅助函数等）：
func TestFinanceNewsHandler_Success(t *testing.T) {
t.Helper()
// 模拟上游 stock_news_em / stock_news_main_cx API。
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/stock_news_em":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"title": "EM headline 1", "time": "2026-03-20 09:30:00"},
})
case "/api/stock_news_main_cx":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"title": "Main CX headline 1", "time": "2026-03-20 09:31:00"},
})
default:
t.Fatalf("unexpected path: %s", r.URL.Path)
}
}))
defer srv.Close()
// TODO: 用你真实的 FinanceNews handler 构造方式替换这里。
// 比如如果有接受 base URL 的构造函数：
// handler := newFinanceNewsHandler(srv.URL)
handler := newFinanceNewsHandlerForTest(t, srv.URL) // 你定义的辅助函数，与行情 handler 的测试辅助函数保持一致
req := httptest.NewRequest(http.MethodPost, "/finance/news", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusOK)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
newsAny, ok := resp.Result.Extra["finance_news"]
if !ok {
t.Fatalf("expected finance_news extra to be set")
}
news, ok := newsAny.([]any)
if !ok {
t.Fatalf("finance_news extra has unexpected type %T", newsAny)
}
if len(news) == 0 {
t.Fatalf("finance_news extra should not be empty")
}
}
func TestFinanceNewsHandler_UpstreamError(t *testing.T) {
t.Helper()
// 上游返回 502 来模拟 provider 失败。
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "bad gateway", http.StatusBadGateway)
}))
defer srv.Close()
handler := newFinanceNewsHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/news", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
// 断言 handler 会透传上游错误。
if rr.Code != http.StatusBadGateway {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusBadGateway)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err == nil {
if resp.Result.Extra != nil {
if _, ok := resp.Result.Extra["finance_news"]; ok {
t.Fatalf("finance_news extra should not be set on upstream error")
}
}
}
}
func TestEconomyIndicatorHandler_Success(t *testing.T) {
t.Helper()
// 模拟上游宏观接口。
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/macro_gdp":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"year": 2025, "gdp": 123.45},
})
case "/api/macro_cpi":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"month": "2025-12", "cpi": 2.3},
})
default:
t.Fatalf("unexpected path: %s", r.URL.Path)
}
}))
defer srv.Close()
handler := newEconomyIndicatorHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/economy_indicator", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusOK)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
indicatorAny, ok := resp.Result.Extra["economy_indicator"]
if !ok {
t.Fatalf("expected economy_indicator extra to be set")
}
indicators, ok := indicatorAny.([]any)
if !ok {
t.Fatalf("economy_indicator extra has unexpected type %T", indicatorAny)
}
if len(indicators) == 0 {
t.Fatalf("economy_indicator extra should not be empty")
}
}
func TestEconomyIndicatorHandler_UpstreamError(t *testing.T) {
t.Helper()
// 上游宏观接口失败。
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "bad gateway", http.StatusBadGateway)
}))
defer srv.Close()
handler := newEconomyIndicatorHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/economy_indicator", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusBadGateway {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusBadGateway)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err == nil {
if resp.Result.Extra != nil {
if _, ok := resp.Result.Extra["economy_indicator"]; ok {
t.Fatalf("economy_indicator extra should not be set on upstream error")
}
}
}
}你还需要一些小的辅助函数，使之与当前行情处理器测试保持一致，例如：
func newFinanceNewsHandlerForTest(t *testing.T, baseURL string) http.Handler {
t.Helper()
// 依照行情处理器在测试中的依赖注入方式来接线，
// 但将 baseURL 用作上游接口的根路径。
return NewFinanceNewsHandler(
http.DefaultClient,
baseURL,
// ... 其他与生产构造保持一致的依赖 ...
)
}
func newEconomyIndicatorHandlerForTest(t *testing.T, baseURL string) http.Handler {
t.Helper()
return NewEconomyIndicatorHandler(
http.DefaultClient,
baseURL,
// ... 其他依赖 ...
)
}请根据你实际的 handler 和测试约定调整：
NewFinanceNewsHandler、NewEconomyIndicatorHandler,- 请求路径（
/finance/news、/finance/economy_indicator）， - extra key（
"finance_news"、"economy_indicator"）， - 以及上游路径（
/api/stock_news_em等）。
关键的行为要求是：
- 成功用例：确保 handler 能正确地将 JSON 结构化写入对应的
Result.Extra条目中。 - 错误用例：确保 handler 会返回上游的错误状态码，并且不会设置对应的
Result.Extra条目。
Original comment in English
suggestion (testing): Add coverage for FinanceNews and EconomyIndicator handlers, including error paths
To keep the finance surface consistent, please also:
- Add success-path tests for
FinanceNewsandEconomyIndicatorusinghttptest.Serverto simulatestock_news_em/stock_news_main_cxand the macro endpoints. - Add one error-path test per handler (e.g., upstream 502) asserting the handler propagates the error and does not set the result extra.
This will align behavior and coverage across all three finance handlers.
Suggested implementation:
func TestFinanceMarketDataHandlerReturnsStructuredJSON(t *testing.T) {
t.Helper()
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/public/spot_quotations_sge" {
t.Fatalf("unexpected path: %s", r.URL.Path)
}
_ = json.NewEncoder(w).Encode([]map[string]any{
{"品种": "Au99.99", "时间": "09:31:00", "现价": 578.32, "更新时间": "2026-03-20 09:31:00"},
})
}))You’ll need to add four new tests to internal/application/lark/handlers/finance_tools_test.go following the same style and helpers used for TestFinanceMarketDataHandlerReturnsStructuredJSON.
The examples below assume:
- You have constructor/helpers similar to those used for the market data handler (e.g., something like
newFinanceNewsHandler(baseURL string)/newEconomyIndicatorHandler(baseURL string)or a generic factory for finance tools). - Each handler writes into some
Result.Extramap in the Lark response (e.g.resp.Result.Extra["finance_news"],resp.Result.Extra["economy_indicator"]), and error propagation is visible either as an HTTP status code or an error in the result.
Adapt the names/wiring to match your actual code (handlers, extra keys, helper functions):
func TestFinanceNewsHandler_Success(t *testing.T) {
t.Helper()
// Simulate upstream stock_news_em / stock_news_main_cx API.
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/stock_news_em":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"title": "EM headline 1", "time": "2026-03-20 09:30:00"},
})
case "/api/stock_news_main_cx":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"title": "Main CX headline 1", "time": "2026-03-20 09:31:00"},
})
default:
t.Fatalf("unexpected path: %s", r.URL.Path)
}
}))
defer srv.Close()
// TODO: Replace this with the real way you construct the FinanceNews handler.
// For example, if there's a constructor taking the base URL:
// handler := newFinanceNewsHandler(srv.URL)
handler := newFinanceNewsHandlerForTest(t, srv.URL) // helper you define mirroring the market-data one
req := httptest.NewRequest(http.MethodPost, "/finance/news", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusOK)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
newsAny, ok := resp.Result.Extra["finance_news"]
if !ok {
t.Fatalf("expected finance_news extra to be set")
}
news, ok := newsAny.([]any)
if !ok {
t.Fatalf("finance_news extra has unexpected type %T", newsAny)
}
if len(news) == 0 {
t.Fatalf("finance_news extra should not be empty")
}
}
func TestFinanceNewsHandler_UpstreamError(t *testing.T) {
t.Helper()
// Upstream returns 502 to simulate a failing provider.
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "bad gateway", http.StatusBadGateway)
}))
defer srv.Close()
handler := newFinanceNewsHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/news", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
// Assert that the handler propagates the upstream error.
if rr.Code != http.StatusBadGateway {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusBadGateway)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err == nil {
if resp.Result.Extra != nil {
if _, ok := resp.Result.Extra["finance_news"]; ok {
t.Fatalf("finance_news extra should not be set on upstream error")
}
}
}
}
func TestEconomyIndicatorHandler_Success(t *testing.T) {
t.Helper()
// Simulate upstream macro endpoints.
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/macro_gdp":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"year": 2025, "gdp": 123.45},
})
case "/api/macro_cpi":
_ = json.NewEncoder(w).Encode([]map[string]any{
{"month": "2025-12", "cpi": 2.3},
})
default:
t.Fatalf("unexpected path: %s", r.URL.Path)
}
}))
defer srv.Close()
handler := newEconomyIndicatorHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/economy_indicator", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusOK)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
indicatorAny, ok := resp.Result.Extra["economy_indicator"]
if !ok {
t.Fatalf("expected economy_indicator extra to be set")
}
indicators, ok := indicatorAny.([]any)
if !ok {
t.Fatalf("economy_indicator extra has unexpected type %T", indicatorAny)
}
if len(indicators) == 0 {
t.Fatalf("economy_indicator extra should not be empty")
}
}
func TestEconomyIndicatorHandler_UpstreamError(t *testing.T) {
t.Helper()
// Upstream macro endpoint failure.
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "bad gateway", http.StatusBadGateway)
}))
defer srv.Close()
handler := newEconomyIndicatorHandlerForTest(t, srv.URL)
req := httptest.NewRequest(http.MethodPost, "/finance/economy_indicator", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusBadGateway {
t.Fatalf("unexpected status code: got %d, want %d", rr.Code, http.StatusBadGateway)
}
var resp struct {
Result struct {
Extra map[string]any `json:"extra"`
} `json:"result"`
}
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err == nil {
if resp.Result.Extra != nil {
if _, ok := resp.Result.Extra["economy_indicator"]; ok {
t.Fatalf("economy_indicator extra should not be set on upstream error")
}
}
}
}You will also need small helpers that parallel whatever you already do for the market data handler, for example:
func newFinanceNewsHandlerForTest(t *testing.T, baseURL string) http.Handler {
t.Helper()
// Wire dependencies the same way as for the market data handler under test,
// but using baseURL as the upstream endpoint root.
return NewFinanceNewsHandler(
http.DefaultClient,
baseURL,
// ... any other dependencies mirrored from production construction ...
)
}
func newEconomyIndicatorHandlerForTest(t *testing.T, baseURL string) http.Handler {
t.Helper()
return NewEconomyIndicatorHandler(
http.DefaultClient,
baseURL,
// ... any other dependencies ...
)
}Adjust:
NewFinanceNewsHandler,NewEconomyIndicatorHandler,- request paths (
/finance/news,/finance/economy_indicator), - extra keys (
"finance_news","economy_indicator"), - and upstream paths (
/api/stock_news_em, etc.)
to match your actual handlers and existing test conventions. The key behavioral requirements are:
- Success tests: ensure the handler correctly structures JSON into the
Result.Extraentry. - Error tests: ensure the handler returns the upstream error status and does not set the corresponding
Result.Extraentry.
| if result, ok := meta.GetExtra("chat_members_result"); !ok || !strings.Contains(result, `"open_id":"ou_a"`) { | ||
| t.Fatalf("chat_members_result extra missing expected payload: %q", result) | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion (testing): 建议在 member 工具中增加关于 limit 归一化和大 lookback 值的测试
为了完整覆盖 normalizeMembersLimit、normalizeRecentActiveTopK 和 normalizeLookbackMessages 的边界逻辑，请增加以下测试：
- 使用非常大的
TopK和LookbackMessages值（例如 1000）调用RecentActiveMembers.Handle，并断言传入recentActiveMembersHistoryLoader的size被限制在 200 之内，同时返回的成员切片长度被限制在 50 之内。 - 使用
Limit> 200 调用ChatMembers.Handle，并断言序列化结果中只包含 200 个成员。
这样可以锁定针对无界查询和 payload 的保护逻辑。
建议实现：
func TestRecentActiveMembersHandlerNormalizesLargeTopKAndLookback(t *testing.T) {
old := recentActiveMembersHistoryLoader
defer func() {
recentActiveMembersHistoryLoader = old
}()
var (
capturedChatID string
capturedSize int
)
recentActiveMembersHistoryLoader = func(ctx context.Context, chatID string, size int) (history.OpensearchMsgLogList, error) {
capturedChatID = chatID
capturedSize = size
var empty history.OpensearchMsgLogList
return empty, nil
}
args := RecentActiveMembersArgs{
ChatID: "chat_id",
TopK: 1000,
LookbackMessages: 1000,
}
err := RecentActiveMembers.Handle(context.Background(), nil, &xhandler.BaseMetaData{}, args)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if capturedChatID != "chat_id" {
t.Fatalf("expected chat_id %q, got %q", "chat_id", capturedChatID)
}
if capturedSize != 200 {
t.Fatalf("expected normalized history size 200, got %d", capturedSize)
}
}
func TestRecentActiveMembersHandlerUsesCurrentChatScopeAndDedupsByRecency(t *testing.T) {要完全落实这条评审意见，你还需要：
- 为
RecentActiveMembers.Handle编写一个对应测试，断言返回的成员切片长度被限制在 50：- 仍然复用
recentActiveMembersHistoryLoader覆盖，但让它返回超过 50 条的模拟消息； - 按照本测试文件已有的响应写入模式来截获并解析
RecentActiveMembers.Handle的响应（例如使用httptest.NewRecorder+json.Unmarshal到已有的响应结构体中）； - 即便 loader 返回了超过 50 条消息并且
TopK设置为一个很大的值（例如 1000），也要断言解析出的成员切片长度是 50。
- 仍然复用
- 为
ChatMembers.Handle新增一个测试来断言 limit 归一化：- 覆盖
ChatMembers内部使用的 loader（例如本文件中的chatMembersLoader或等价变量），记录传入的 limit 值，并/或返回可控的成员集合； - 使用
ChatMembersArgs{ChatID: "chat_id", Limit: 1000}调用ChatMembers.Handle； - 以与本文件其他
ChatMembers测试相同的方式解析序列化响应； - 断言序列化结果中最多只有 200 个成员，并（可选地）断言传递给底层 loader 的 limit 也是 200。
如果你项目中RecentActiveMembersArgs的字段名以及ChatMembers所用 loader 的变量名不完全一致，请相应地调整。
- 覆盖
Original comment in English
suggestion (testing): Consider adding tests for limit normalization and large lookback values in member tools
To fully exercise the boundary logic in normalizeMembersLimit, normalizeRecentActiveTopK, and normalizeLookbackMessages, please add tests that:
- Call
RecentActiveMembers.Handlewith very largeTopKandLookbackMessagesvalues (e.g., 1000) and assert that thesizepassed torecentActiveMembersHistoryLoaderis capped at 200 and the returned slice length is capped at 50. - Call
ChatMembers.Handlewith aLimit> 200 and assert that only 200 members are present in the serialized result.
This will lock in the safeguards against unbounded queries and payloads.
Suggested implementation:
func TestRecentActiveMembersHandlerNormalizesLargeTopKAndLookback(t *testing.T) {
old := recentActiveMembersHistoryLoader
defer func() {
recentActiveMembersHistoryLoader = old
}()
var (
capturedChatID string
capturedSize int
)
recentActiveMembersHistoryLoader = func(ctx context.Context, chatID string, size int) (history.OpensearchMsgLogList, error) {
capturedChatID = chatID
capturedSize = size
var empty history.OpensearchMsgLogList
return empty, nil
}
args := RecentActiveMembersArgs{
ChatID: "chat_id",
TopK: 1000,
LookbackMessages: 1000,
}
err := RecentActiveMembers.Handle(context.Background(), nil, &xhandler.BaseMetaData{}, args)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if capturedChatID != "chat_id" {
t.Fatalf("expected chat_id %q, got %q", "chat_id", capturedChatID)
}
if capturedSize != 200 {
t.Fatalf("expected normalized history size 200, got %d", capturedSize)
}
}
func TestRecentActiveMembersHandlerUsesCurrentChatScopeAndDedupsByRecency(t *testing.T) {To fully implement your review comment, you will also need:
- A corresponding test for
RecentActiveMembers.Handlethat asserts the returned member slice is capped at 50. This will require:- Using the same
recentActiveMembersHistoryLoaderoverride but returning more than 50 synthetic messages. - Capturing and decoding the response from
RecentActiveMembers.Handleusing whatever response-writing pattern is already present in this test file (e.g.,httptest.NewRecorder+json.Unmarshalinto the existing response struct). - Asserting that the decoded members slice length is 50 even when more than 50 messages are returned from the loader and
TopKis set to a large value (e.g., 1000).
- Using the same
- A new test for
ChatMembers.Handlethat asserts limit normalization:- Override the loader used inside
ChatMembers(e.g.,chatMembersLoaderor equivalent in this file) to record the requested limit and/or to return a controllable set of members. - Call
ChatMembers.HandlewithChatMembersArgs{ChatID: "chat_id", Limit: 1000}. - Decode the serialized response using the same approach as other
ChatMemberstests in this file. - Assert that at most 200 members are present in the serialized result, and (optionally) that the limit passed to the underlying loader is also 200.
Adjust the field names inRecentActiveMembersArgsand the loader variable name forChatMembersto match the actual definitions in your codebase if they differ.
- Override the loader used inside
| @@ -38,3 +40,74 @@ func TestBuildTurnRequestDefaultsReasoningEffortToMedium(t *testing.T) { | |||
| t.Fatalf("reasoning effort = %+v, want %v", req.GetReasoning(), responses.ReasoningEffort_medium) | |||
| } | |||
| } | |||
There was a problem hiding this comment.
suggestion (testing): 为非 function 工具（例如 web_search）以及 mergeResponseTools 中的 nil AdditionalTools 增加测试
当前测试覆盖了函数工具去重，但还没有覆盖 mergeResponseTools 中对 web search 工具和 nil 条目的特殊处理。
请再增加以下测试：
- 使用一个基础切片，其中包含一个函数工具和一个 web search 工具；再传入
AdditionalTools，其中重复 web search 工具并包含nil；断言合并后的工具集合中只保留一个 web search 工具，且不包含 nil。 - 使用
AdditionalTools为nil调用buildTurnRequest，以确认行为与之前一致且不会 panic。
这样可以在未来增加更多工具类型时，更好地防止工具合并逻辑回归。
建议实现：
func TestBuildTurnRequestDedupesAdditionalToolsByName(t *testing.T) {
turn := New[struct{}]("oc_chat", "ou_actor", nil).WithTools(
arktools.New[struct{}]().Add(
arktools.NewUnit[struct{}]().
Name("finance_market_data_get").
Desc("默认工具").
Params(arktools.NewParams("object")),
),
)
req, err := turn.buildTurnRequest(ResponseTurnRequest{
ModelID: "ep-test",
SystemPrompt: "system",
UserPrompt: "user",
AdditionalTools: []*responses.ResponsesTool{
testResponseFunctionTool("finance_market_data_get"),
},
})
if err != nil {
t.Fatalf("buildTurnRequest() error = %v", err)
}
if len(req.Tools) != 1 {
t.Fatalf("tool count = %d, want 1", len(req.Tools))
}
}
func TestMergeResponseTools_WebSearchAndNil(t *testing.T) {
t.Helper()
base := []*responses.ResponsesTool{
testResponseFunctionTool("fn_base"),
testResponseWebSearchTool(),
}
additional := []*responses.ResponsesTool{
testResponseWebSearchTool(), // duplicate web search tool
nil, // explicit nil entry should be ignored
}
merged := mergeResponseTools(base, additional)
if merged == nil {
t.Fatalf("mergeResponseTools() returned nil slice")
}
if len(merged) != 2 {
t.Fatalf("merged tool count = %d, want %d", len(merged), 2)
}
var (
functionCount int
webSearchCount int
nilCount int
)
for i, tool := range merged {
if tool == nil {
nilCount++
t.Fatalf("merged tool at index %d is nil; nil tools should be filtered out", i)
}
switch {
case tool.GetToolFunction() != nil:
functionCount++
case tool.GetToolWebSearch() != nil:
webSearchCount++
}
}
if functionCount != 1 {
t.Fatalf("function tool count = %d, want %d", functionCount, 1)
}
if webSearchCount != 1 {
t.Fatalf("web search tool count = %d, want %d", webSearchCount, 1)
}
if nilCount != 0 {
t.Fatalf("nil tool count = %d, want %d", nilCount, 0)
}
}
func TestBuildTurnRequestHandlesNilAdditionalTools(t *testing.T) {
turn := New[struct{}]("oc_chat", "ou_actor", nil).WithTools(
arktools.New[struct{}]().Add(
arktools.NewUnit[struct{}]().
Name("finance_market_data_get").
Desc("默认工具").
Params(arktools.NewParams("object")),
),
)
req, err := turn.buildTurnRequest(ResponseTurnRequest{
ModelID: "ep-test",
SystemPrompt: "system",
UserPrompt: "user",
AdditionalTools: nil, // explicitly nil, should not panic and should preserve base tools
})
if err != nil {
t.Fatalf("buildTurnRequest() with nil AdditionalTools error = %v", err)
}
if req == nil {
t.Fatalf("buildTurnRequest() returned nil request")
}
if len(req.Tools) != 1 {
t.Fatalf("tool count with nil AdditionalTools = %d, want %d", len(req.Tools), 1)
}
}
func testResponseFunctionTool(name string) *responses.ResponsesTool {
return &responses.ResponsesTool{
Union: &responses.ResponsesTool_ToolFunction{
ToolFunction: &responses.ToolFunction{
Name: name,
Type: responses.ToolType_function,
Description: gptr.Of("dynamic tool"),
Parameters: &responses.Bytes{Value: []byte(`{"type":"object","properties":{}}`)},
},
},
}
}
func testResponseWebSearchTool() *responses.ResponsesTool {
return &responses.ResponsesTool{
Union: &responses.ResponsesTool_ToolWebSearch{
ToolWebSearch: &responses.ToolWebSearch{
// Type: responses.ToolType_web_search,
// Populate additional fields as required by the WebSearch tool definition.
},
},
}
}补充说明：
testResponseWebSearchTool帮助函数假设在responses包中存在ResponsesTool_ToolWebSearch这个 oneof 包装类型以及ToolWebSearch消息，并且ResponsesTool上存在GetToolWebSearch()访问方法。请根据你实际生成的 protobuf API 调整类型和字段名（例如ResponsesTool_WebSearch、WebSearch等），并添加所有必需字段（比如Type: responses.ToolType_web_search或配置字段），以保证该工具在你的服务中是合法的。- 如果
mergeResponseTools位于其他包中或者在另一个文件中是未导出的，请确保该测试文件和它在同一个包中（通常是ark_dal），这样才能直接调用mergeResponseTools。如果它在其他包中，要么将其导出，要么新增一个供测试使用的小的导出封装函数。 - 如果
buildTurnRequest在AdditionalTools == nil时以往的行为期望不同的工具数量（例如，当没有基础工具配置时期望工具数量为 0），请根据你代码库中的实际基线行为调整TestBuildTurnRequestHandlesNilAdditionalTools中对len(req.Tools)的断言。
Original comment in English
suggestion (testing): Add a test for non-function tools (e.g., web_search) and nil additional tools in mergeResponseTools
The current tests cover function tool deduplication but not the special handling for web search tools and nil entries in mergeResponseTools.
Please also add tests that:
- Use a base slice with a function tool and a web search tool plus
AdditionalToolsthat repeats the web search tool and includesnil, and assert the merged tools contain a single web search tool and no nils. - Call
buildTurnRequestwithAdditionalToolsset tonilto confirm it matches previous behavior and does not panic.
This will better protect the tool-merging behavior from regressions as more tool types are added.
Suggested implementation:
func TestBuildTurnRequestDedupesAdditionalToolsByName(t *testing.T) {
turn := New[struct{}]("oc_chat", "ou_actor", nil).WithTools(
arktools.New[struct{}]().Add(
arktools.NewUnit[struct{}]().
Name("finance_market_data_get").
Desc("默认工具").
Params(arktools.NewParams("object")),
),
)
req, err := turn.buildTurnRequest(ResponseTurnRequest{
ModelID: "ep-test",
SystemPrompt: "system",
UserPrompt: "user",
AdditionalTools: []*responses.ResponsesTool{
testResponseFunctionTool("finance_market_data_get"),
},
})
if err != nil {
t.Fatalf("buildTurnRequest() error = %v", err)
}
if len(req.Tools) != 1 {
t.Fatalf("tool count = %d, want 1", len(req.Tools))
}
}
func TestMergeResponseTools_WebSearchAndNil(t *testing.T) {
t.Helper()
base := []*responses.ResponsesTool{
testResponseFunctionTool("fn_base"),
testResponseWebSearchTool(),
}
additional := []*responses.ResponsesTool{
testResponseWebSearchTool(), // duplicate web search tool
nil, // explicit nil entry should be ignored
}
merged := mergeResponseTools(base, additional)
if merged == nil {
t.Fatalf("mergeResponseTools() returned nil slice")
}
if len(merged) != 2 {
t.Fatalf("merged tool count = %d, want %d", len(merged), 2)
}
var (
functionCount int
webSearchCount int
nilCount int
)
for i, tool := range merged {
if tool == nil {
nilCount++
t.Fatalf("merged tool at index %d is nil; nil tools should be filtered out", i)
}
switch {
case tool.GetToolFunction() != nil:
functionCount++
case tool.GetToolWebSearch() != nil:
webSearchCount++
}
}
if functionCount != 1 {
t.Fatalf("function tool count = %d, want %d", functionCount, 1)
}
if webSearchCount != 1 {
t.Fatalf("web search tool count = %d, want %d", webSearchCount, 1)
}
if nilCount != 0 {
t.Fatalf("nil tool count = %d, want %d", nilCount, 0)
}
}
func TestBuildTurnRequestHandlesNilAdditionalTools(t *testing.T) {
turn := New[struct{}]("oc_chat", "ou_actor", nil).WithTools(
arktools.New[struct{}]().Add(
arktools.NewUnit[struct{}]().
Name("finance_market_data_get").
Desc("默认工具").
Params(arktools.NewParams("object")),
),
)
req, err := turn.buildTurnRequest(ResponseTurnRequest{
ModelID: "ep-test",
SystemPrompt: "system",
UserPrompt: "user",
AdditionalTools: nil, // explicitly nil, should not panic and should preserve base tools
})
if err != nil {
t.Fatalf("buildTurnRequest() with nil AdditionalTools error = %v", err)
}
if req == nil {
t.Fatalf("buildTurnRequest() returned nil request")
}
if len(req.Tools) != 1 {
t.Fatalf("tool count with nil AdditionalTools = %d, want %d", len(req.Tools), 1)
}
}
func testResponseFunctionTool(name string) *responses.ResponsesTool {
return &responses.ResponsesTool{
Union: &responses.ResponsesTool_ToolFunction{
ToolFunction: &responses.ToolFunction{
Name: name,
Type: responses.ToolType_function,
Description: gptr.Of("dynamic tool"),
Parameters: &responses.Bytes{Value: []byte(`{"type":"object","properties":{}}`)},
},
},
}
}
func testResponseWebSearchTool() *responses.ResponsesTool {
return &responses.ResponsesTool{
Union: &responses.ResponsesTool_ToolWebSearch{
ToolWebSearch: &responses.ToolWebSearch{
// Type: responses.ToolType_web_search,
// Populate additional fields as required by the WebSearch tool definition.
},
},
}
}- The helper
testResponseWebSearchToolassumes the existence of theResponsesTool_ToolWebSearchoneof wrapper and aToolWebSearchmessage on theresponsespackage, plus aGetToolWebSearch()accessor onResponsesTool. Adjust the type and field names to match your actual generated protobuf API (e.g.,ResponsesTool_WebSearch,WebSearch, etc.), and add any required fields (such asType: responses.ToolType_web_searchor configuration fields) so the tool is valid for your service. - If
mergeResponseToolslives in a different package or is unexported in another file, ensure this test file is in the same package (likelyark_dal) so it can callmergeResponseToolsdirectly. If it is in another package, either export it or add a small exported wrapper for testing. - If the previous behavior for
buildTurnRequestwithAdditionalTools == nilexpects a different tool count (e.g., zero tools when no base tools are configured), adjust thelen(req.Tools)assertion inTestBuildTurnRequestHandlesNilAdditionalToolsto match the expected baseline behavior in your codebase.
|
|
||
| ### Important Behavioral Change Since Last Review | ||
| - `toolmeta/runtime_behavior.go` 现在把 `gold_price_get` 和 `stock_zh_a_get` 标记为需要审批的 chat-write 工具。 | ||
| - 因为它们会发送卡片，当前 runtime prompt 语义上也会把它们当“执行动作”而不是“只读取数”。 |
There was a problem hiding this comment.
nitpick (typo): 建议将“只读取数”改为更常见的“只读取数据”
“只读取数”读起来像是被截断的表达，在这份文档中略显不自然。建议改为“只读取数据”，可以更准确地表达只读能力，并与整体术语风格保持一致。
Suggested change
| - 因为它们会发送卡片，当前 runtime prompt 语义上也会把它们当“执行动作”而不是“只读取数”。 | |
| - 因为它们会发送卡片，当前 runtime prompt 语义上也会把它们当“执行动作”而不是“只读取数据”。 |
Original comment in English
nitpick (typo): Consider changing “只读取数” to the more standard “只读取数据”.
“只读取数”像是被截断的表述，在这份文档里读起来有些不自然。建议改为“只读取数据”，以更准确表达只读能力并与整体术语风格保持一致。
Suggested change
| - 因为它们会发送卡片，当前 runtime prompt 语义上也会把它们当“执行动作”而不是“只读取数”。 | |
| - 因为它们会发送卡片，当前 runtime prompt 语义上也会把它们当“执行动作”而不是“只读取数据”。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ feat(finance-tools): 新增只读高层金融数据工具集
✨ feat(chat-tools): 新增群成员查询工具集
🐛 fix(aktool): 修复股票简称查询接口兼容性
📝 docs(plans): 新增功能实现计划文档
🔧 chore(config): 优化基础配置
💄 style(ui): 调整agent流式卡片标题
Summary by Sourcery
在代理运行时中引入动态金融工具发现与注入机制，新增只读金融工具和聊天成员工具，并优化基础设施、配置和界面以支持这些能力。
New Features:
finance_tool_discover动态发现金融工具的支持，并将其中的受限子集注入到后续模型轮次中。finance_market_data_get、finance_news_get、economy_indicator_get），由统一的基于 akshare 的提供方驱动，并带有内部数据源回退机制。get_chat_members、get_recent_active_members），严格在当前聊天范围内运行，并提供结构化的成员和活动数据。Bug Fixes:
org_short_name_cn（同时保持对旧字段的兼容），修复股票简称查询问题。Enhancements:
AdditionalTools，在去重的前提下将其与默认工具合并，并更新能力工具的接线方式，以区分默认工具与感知能力的工具集。Build:
Documentation:
Tests:
Chores:
.worktrees目录，以更好地支持多 worktree 的开发流程。Original summary in English
Summary by Sourcery
Introduce dynamic finance tool discovery and injection into the agent runtime, add read‑only finance and chat member tooling, and refine infrastructure, configuration, and UI to support these capabilities.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: