From 5f47dd13db9cbc4f9b05f4998e7f995761f4fec7 Mon Sep 17 00:00:00 2001 From: tinkle-community Date: Sun, 8 Mar 2026 16:44:45 +0800 Subject: [PATCH] fix(telegram): eliminate narration, add full-setup workflow and tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rewrite NO NARRATION rule: response is EITHER api_call tag alone OR final text reply — no text before api_call under any circumstances - Ban all narration patterns: 现在我将/好的/正在/I will/Let me etc. - Add 'create strategy + create trader + start' full setup workflow - Add 12 automated tests covering: - No narration leaking to user (5 narration variants tested) - api_call tag never leaks to user - Full setup workflow: POST strategy → verify → POST trader → start - Start existing trader workflow - Max iterations safety, tag stripping, parser edge cases --- telegram/agent/agent_test.go | 305 +++++++++++++++++++++++++++++++---- telegram/agent/prompt.go | 46 +++--- 2 files changed, 299 insertions(+), 52 deletions(-) diff --git a/telegram/agent/agent_test.go b/telegram/agent/agent_test.go index 809d84b0..aa736aa1 100644 --- a/telegram/agent/agent_test.go +++ b/telegram/agent/agent_test.go @@ -47,6 +47,29 @@ func mockGetLLM(llm *mockLLM) func() mcp.AIClient { const testPrompt = "You are a test assistant." +// mockAPIServer creates a test HTTP server with configurable route handlers. +func mockAPIServer(handlers map[string]string) (*httptest.Server, int) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + key := r.Method + " " + r.URL.Path + if body, ok := handlers[key]; ok { + w.Write([]byte(body)) //nolint:errcheck + return + } + // Also try path-only match (for GET) + if body, ok := handlers[r.URL.Path]; ok { + w.Write([]byte(body)) //nolint:errcheck + return + } + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"error":"not found"}`)) //nolint:errcheck + })) + var port int + fmt.Sscanf(srv.Listener.Addr().String(), "127.0.0.1:%d", &port) + return srv, port +} + +// ── Basic agent behaviour ────────────────────────────────────────────────── + // TestAgentDirectReply: LLM replies without api_call — one call, direct reply. func TestAgentDirectReply(t *testing.T) { llm := &mockLLM{responses: []string{"Hello! How can I help you?"}} @@ -64,27 +87,20 @@ func TestAgentDirectReply(t *testing.T) { // TestAgentAPICall: LLM calls API, gets result, gives final reply — two LLM calls. func TestAgentAPICall(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/api/my-traders" { - w.Write([]byte(`[{"id":"t1","name":"BTC Strategy"}]`)) //nolint:errcheck - return - } - w.WriteHeader(404) - })) + srv, port := mockAPIServer(map[string]string{ + "/api/my-traders": `[{"trader_id":"t1","trader_name":"BTC Trader","is_running":false}]`, + }) defer srv.Close() - var port int - fmt.Sscanf(srv.Listener.Addr().String(), "127.0.0.1:%d", &port) - llm := &mockLLM{responses: []string{ - `Let me check.{"method":"GET","path":"/api/my-traders","body":{}}`, - "You have one trader: BTC Strategy.", + `{"method":"GET","path":"/api/my-traders","body":{}}`, + "You have one trader: BTC Trader.", }} a := New(port, "tok", "test-user", mockGetLLM(llm), testPrompt) reply := a.Run("list my traders", nil) - if reply != "You have one trader: BTC Strategy." { + if reply != "You have one trader: BTC Trader." { t.Fatalf("unexpected reply: %q", reply) } if llm.calls != 2 { @@ -94,17 +110,15 @@ func TestAgentAPICall(t *testing.T) { // TestAgentMultiStep: LLM chains two API calls before final reply — three LLM calls. func TestAgentMultiStep(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte(`{"ok":true}`)) //nolint:errcheck - })) + srv, port := mockAPIServer(map[string]string{ + "/api/account": `{"total_equity":1000}`, + "/api/positions": `[]`, + }) defer srv.Close() - var port int - fmt.Sscanf(srv.Listener.Addr().String(), "127.0.0.1:%d", &port) - llm := &mockLLM{responses: []string{ - `Checking account.{"method":"GET","path":"/api/account","body":{}}`, - `Now checking positions.{"method":"GET","path":"/api/positions","body":{}}`, + `{"method":"GET","path":"/api/account","body":{}}`, + `{"method":"GET","path":"/api/positions","body":{}}`, "Account looks healthy and no open positions.", }} a := New(port, "tok", "test-user", mockGetLLM(llm), testPrompt) @@ -121,14 +135,11 @@ func TestAgentMultiStep(t *testing.T) { // TestAgentAPIResultInContext: API result must appear in next LLM message. func TestAgentAPIResultInContext(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte(`{"balance":1234.56}`)) //nolint:errcheck - })) + srv, port := mockAPIServer(map[string]string{ + "/api/account": `{"balance":1234.56}`, + }) defer srv.Close() - var port int - fmt.Sscanf(srv.Listener.Addr().String(), "127.0.0.1:%d", &port) - llm := &mockLLM{responses: []string{ `{"method":"GET","path":"/api/account","body":{}}`, "Balance is 1234.56 USDT.", @@ -148,10 +159,193 @@ func TestAgentAPIResultInContext(t *testing.T) { } } +// ── NO NARRATION tests ───────────────────────────────────────────────────── + +// TestNoNarrationBeforeAPICall: any text before must NOT reach the user. +// The agent strips text-before-tag and only forwards it as assistant context. +func TestNoNarrationBeforeAPICall(t *testing.T) { + srv, port := mockAPIServer(map[string]string{ + "/api/strategies": `[{"id":"s1","name":"BTC Trend"}]`, + }) + defer srv.Close() + + narrations := []string{ + "现在我将为您创建策略。\n", + "好的，我来帮你查询。", + "Let me check this for you. ", + "正在处理...", + "I will call the API now. ", + } + + for _, narration := range narrations { + llm := &mockLLM{responses: []string{ + // LLM outputs narration before the api_call tag (bad behaviour we must handle) + narration + `{"method":"GET","path":"/api/strategies","body":{}}`, + "你有1个策略：BTC Trend。", + }} + a := New(port, "tok", "test-user", mockGetLLM(llm), testPrompt) + reply := a.Run("查询我的策略", nil) + + // Final reply must not contain narration fragments + if strings.Contains(reply, "现在我将") || strings.Contains(reply, "Let me") || + strings.Contains(reply, "正在处理") || strings.Contains(reply, "好的，我来") || + strings.Contains(reply, "I will call") { + t.Fatalf("narration leaked into reply for input %q: got %q", narration, reply) + } + // api_call tag must not appear in reply + if strings.Contains(reply, "") { + t.Fatalf("api_call tag leaked into reply: %q", reply) + } + } +} + +// TestAPICallTagNotLeakedToUser: tag must never appear in returned reply. +func TestAPICallTagNotLeakedToUser(t *testing.T) { + srv, port := mockAPIServer(map[string]string{ + "/api/account": `{"total_equity":500}`, + }) + defer srv.Close() + + llm := &mockLLM{responses: []string{ + `{"method":"GET","path":"/api/account","body":{}}`, + `账户余额 500 USDT。{"method":"GET","path":"/api/account","body":{}}`, + }} + a := New(port, "tok", "test-user", mockGetLLM(llm), testPrompt) + reply := a.Run("show balance", nil) + + if strings.Contains(reply, "") { + t.Fatalf("api_call tag leaked to user: %q", reply) + } +} + +// ── Workflow tests ───────────────────────────────────────────────────────── + +// TestCreateStrategyWorkflow: simulates creating a BTC trend strategy. +// Verifies: POST strategy → GET verify → final reply shows strategy info. +func TestCreateStrategyWorkflow(t *testing.T) { + srv, port := mockAPIServer(map[string]string{ + "POST /api/strategies": `{"id":"s1","name":"BTC趋势"}`, + "GET /api/strategies/s1": `{"id":"s1","name":"BTC趋势","config":{"coin_source":{"source_type":"static","static_coins":["BTC/USDT"]},"leverage":5}}`, + }) + defer srv.Close() + + llm := &mockLLM{responses: []string{ + // Step 1: create strategy + `{"method":"POST","path":"/api/strategies","body":{"name":"BTC趋势","config":{}}}`, + // Step 2: verify strategy + `{"method":"GET","path":"/api/strategies/s1","body":{}}`, + // Step 3: final reply + "策略已创建：BTC趋势，币种 BTC/USDT，杠杆 5x。", + }} + a := New(port, "tok", "test-user", mockGetLLM(llm), testPrompt) + reply := a.Run("帮我配置个btc趋势交易的策略", nil) + + if llm.calls != 3 { + t.Fatalf("expected 3 LLM calls, got %d", llm.calls) + } + if reply == "" || strings.Contains(reply, "") { + t.Fatalf("bad final reply: %q", reply) + } +} + +// TestFullSetupWorkflow: create strategy → create trader → start trader. +// This is the "帮我配置策略并跑起来" workflow. +func TestFullSetupWorkflow(t *testing.T) { + calls := map[string]int{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + key := r.Method + " " + r.URL.Path + calls[key]++ + switch key { + case "POST /api/strategies": + w.Write([]byte(`{"id":"s1","name":"BTC趋势"}`)) //nolint:errcheck + case "GET /api/strategies/s1": + w.Write([]byte(`{"id":"s1","name":"BTC趋势","config":{}}`)) //nolint:errcheck + case "POST /api/traders": + w.Write([]byte(`{"id":"tr1","name":"BTC趋势交易员"}`)) //nolint:errcheck + case "POST /api/traders/tr1/start": + w.Write([]byte(`{"ok":true}`)) //nolint:errcheck + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + var port int + fmt.Sscanf(srv.Listener.Addr().String(), "127.0.0.1:%d", &port) + + llm := &mockLLM{responses: []string{ + // 1. create strategy + `{"method":"POST","path":"/api/strategies","body":{"name":"BTC趋势"}}`, + // 2. verify strategy + `{"method":"GET","path":"/api/strategies/s1","body":{}}`, + // 3. create trader + `{"method":"POST","path":"/api/traders","body":{"name":"BTC趋势交易员","strategy_id":"s1"}}`, + // 4. start trader + `{"method":"POST","path":"/api/traders/tr1/start","body":{}}`, + // 5. final reply + "策略和交易员已创建并启动！BTC趋势交易员正在运行。", + }} + a := New(port, "tok", "test-user", mockGetLLM(llm), testPrompt) + reply := a.Run("帮我配置个btc趋势交易的策略交易 跑起来", nil) + + if llm.calls != 5 { + t.Fatalf("expected 5 LLM calls, got %d", llm.calls) + } + // Verify each API was called + if calls["POST /api/strategies"] != 1 { + t.Errorf("expected 1 POST /api/strategies, got %d", calls["POST /api/strategies"]) + } + if calls["POST /api/traders"] != 1 { + t.Errorf("expected 1 POST /api/traders, got %d", calls["POST /api/traders"]) + } + if calls["POST /api/traders/tr1/start"] != 1 { + t.Errorf("expected 1 POST /api/traders/tr1/start, got %d", calls["POST /api/traders/tr1/start"]) + } + if strings.Contains(reply, "") { + t.Fatalf("api_call tag in final reply: %q", reply) + } +} + +// TestStartExistingTrader: when trader already exists, just start it. +func TestStartExistingTrader(t *testing.T) { + calls := map[string]int{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + key := r.Method + " " + r.URL.Path + calls[key]++ + switch key { + case "GET /api/my-traders": + w.Write([]byte(`[{"trader_id":"tr1","trader_name":"BTC Trader","is_running":false}]`)) //nolint:errcheck + case "POST /api/traders/tr1/start": + w.Write([]byte(`{"ok":true}`)) //nolint:errcheck + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + var port int + fmt.Sscanf(srv.Listener.Addr().String(), "127.0.0.1:%d", &port) + + llm := &mockLLM{responses: []string{ + `{"method":"GET","path":"/api/my-traders","body":{}}`, + `{"method":"POST","path":"/api/traders/tr1/start","body":{}}`, + "交易员 BTC Trader 已启动。", + }} + a := New(port, "tok", "test-user", mockGetLLM(llm), testPrompt) + reply := a.Run("启动交易员", nil) + + if calls["POST /api/traders/tr1/start"] != 1 { + t.Errorf("expected trader to be started, got %d start calls", calls["POST /api/traders/tr1/start"]) + } + if strings.Contains(reply, "") { + t.Fatalf("api_call tag in reply: %q", reply) + } +} + +// ── Parser tests ─────────────────────────────────────────────────────────── + // TestParseAPICall: unit tests for the XML tag parser. func TestParseAPICall(t *testing.T) { - t.Run("valid call", func(t *testing.T) { - resp := `Stopping trader.{"method":"POST","path":"/api/traders/t1/stop","body":{}}` + t.Run("valid call no text before", func(t *testing.T) { + resp := `{"method":"POST","path":"/api/traders/t1/stop","body":{}}` req, text := parseAPICall(resp) if req == nil { t.Fatal("expected api_call, got nil") @@ -159,6 +353,17 @@ func TestParseAPICall(t *testing.T) { if req.Method != "POST" || req.Path != "/api/traders/t1/stop" { t.Fatalf("unexpected req: %+v", req) } + if text != "" { + t.Fatalf("expected empty text before tag, got: %q", text) + } + }) + + t.Run("text before tag is captured", func(t *testing.T) { + resp := `Stopping trader.{"method":"POST","path":"/api/traders/t1/stop","body":{}}` + req, text := parseAPICall(resp) + if req == nil { + t.Fatal("expected api_call, got nil") + } if text != "Stopping trader." { t.Fatalf("unexpected text before tag: %q", text) } @@ -181,3 +386,45 @@ func TestParseAPICall(t *testing.T) { } }) } + +// TestStripAPICallTag: defensive cleanup of stray tags in final reply. +func TestStripAPICallTag(t *testing.T) { + cases := []struct { + input string + want string + }{ + {`正常回复`, `正常回复`}, + {`回复{"method":"GET","path":"/x"}`, `回复`}, + {`{"method":"GET","path":"/x"}`, ``}, + } + for _, c := range cases { + got := stripAPICallTag(c.input) + if strings.TrimSpace(got) != c.want { + t.Errorf("stripAPICallTag(%q) = %q, want %q", c.input, got, c.want) + } + } +} + +// TestMaxIterations: agent stops after maxIterations and returns a summary. +func TestMaxIterations(t *testing.T) { + srv, port := mockAPIServer(map[string]string{ + "/api/account": `{"ok":true}`, + }) + defer srv.Close() + + // Always returns another api_call — should hit max iterations + responses := make([]string, maxIterations+2) + for i := range responses { + responses[i] = `{"method":"GET","path":"/api/account","body":{}}` + } + responses[maxIterations] = "Final summary after max iterations." + + llm := &mockLLM{responses: responses} + a := New(port, "tok", "test-user", mockGetLLM(llm), testPrompt) + reply := a.Run("loop forever", nil) + + if strings.Contains(reply, "") { + t.Fatalf("api_call tag in reply after max iterations: %q", reply) + } + _ = reply // just confirm it terminates +} diff --git a/telegram/agent/prompt.go b/telegram/agent/prompt.go index c13160aa..bc0d8d01 100644 --- a/telegram/agent/prompt.go +++ b/telegram/agent/prompt.go @@ -15,31 +15,31 @@ func BuildAgentPrompt(apiDocs, userID string) string { ## Tool: api_call -Append EXACTLY ONE tag at the very end of your reply when you need to call the API: +When you need to call the API, your ENTIRE response must be ONLY the tag — nothing else: {"method":"GET","path":"/api/xxx","body":{}} -Rules: -- The tag must be the LAST thing in your message — nothing after it +When you have a final answer (no more API calls needed), reply with plain text — NO tag at all. + +ABSOLUTE RULES — violation = broken product: +- 【ZERO NARRATION】Your response is EITHER the api_call tag alone OR a final text reply. NEVER both except api_call at the very end. +- NEVER output ANY text before an api_call tag. No "好的", no "现在", no "我将", no "Let me", no "I will", no "正在", no "Creating...", no ellipsis, NOTHING. - NEVER more than one tag per response -- 【CRITICAL】NEVER say "让我查询..."、"现在获取..."、"I will call..."、"Let me check..." — just ACT silently, no narration at all - method: "GET" | "POST" | "PUT" | "DELETE" - body: JSON object (use {} for GET requests) - query parameters go in the path: /api/positions?trader_id=xxx ## NOFX API Documentation -The following API documentation includes full parameter schemas. Use these to understand exactly what each field means and construct correct requests. - %s ## Behavior Rules -1. 【NO NARRATION】Never tell the user what API you are calling. Zero narration. Just act. -2. Only ONE tag per response, always at the very end -3. After getting an API result, decide: call another API or give a final reply +1. 【SILENT ACTION】When you need to call an API: output ONLY the tag. Zero words before it. +2. Only ONE tag per response, always alone with nothing else +3. After getting an API result, decide: call another API (output tag only) or give final reply (text only) 4. If the API returns success (2xx), the operation succeeded — do not retry 5. Reply in the same language the user used (中文→中文, English→English) -6. Keep replies concise — show results, not process -7. Ask for ALL required information in ONE message — never ask one field at a time +6. Keep final replies concise — show results, not process +7. Ask for ALL missing required info in ONE message — never ask one field at a time 8. When user provides enough info, act immediately — no confirmation needed 9. Be decisive — infer intent from context, use schema to fill in smart defaults @@ -58,12 +58,6 @@ After ANY PUT or POST that creates or modifies a resource: - 5xx: server error, ask user to try again - stream interrupted / unavailable: apologize briefly and ask user to retry -## How to Use the API Schema -All API knowledge comes from the documentation above. Use field descriptions to: -- Know exactly which fields are required vs optional -- Understand semantics and build correct request bodies from natural language -- For StrategyConfig: intelligently fill all fields based on user's trading style - ## Account State (injected at conversation start) At the start of each new conversation, a [Current Account State] block is provided with: - AI Models: all configured models with their IDs and enabled status @@ -83,10 +77,8 @@ Use this to: **Configure exchange**: Ask for all required fields in ONE message (see schema). Always set enabled:true. -**Create trader**: GET /api/exchanges + GET /api/models to get IDs → confirm with user → POST /api/traders. - -**Create strategy** (most important workflow): -- A strategy is INDEPENDENT of traders. Never GET trader info just to create a strategy. +**Create strategy** (independent from traders): +- Never GET trader info just to create a strategy. - If user specifies style + coins (e.g. "BTC trend"), build and POST immediately — no questions needed. - Build StrategyConfig intelligently from user's description: - "trend" / "趋势" → enable EMA(20,50), MACD, RSI, multi-timeframe (15m,1h,4h), longer primary TF @@ -95,13 +87,21 @@ Use this to: - "BTC/ETH" → set coin_source.source_type="static", static_coins=["BTC/USDT"] or similar - After POST: GET /api/strategies/:id to verify → show user: name, coins, key indicators, leverage +**"帮我配置策略并跑起来" / "create strategy and start" (full setup workflow)**: +Execute these steps IN ORDER with NO user confirmation between them: +1. POST /api/strategies — create strategy with config built from user's description +2. GET /api/strategies/:id — verify strategy was saved correctly +3. POST /api/traders — create trader: use exchange_id and model_id from Account State (if only one each, use directly); set strategy_id from step 1; set name like "BTC趋势" or similar +4. POST /api/traders/:id/start — start the trader +5. Final reply: show strategy name, trader name, key config (coins, leverage, indicators), confirm running + **Update strategy config**: 1. GET /api/strategies/:id to read current full config 2. Modify only what user asked (keep all other fields) 3. PUT /api/strategies/:id with complete merged config 4. GET /api/strategies/:id to verify → show user actual saved values for changed fields -**Start/stop trader**: GET /api/my-traders first. If only one trader, act directly. If multiple, list and ask. +**Start/stop existing trader**: From Account State, if only one trader, act directly. If multiple, list and ask. -**Query data**: GET /api/my-traders to get trader_id, then query /api/positions?trader_id=xxx or /api/account?trader_id=xxx etc.`, userID, apiDocs) +**Query data**: Use trader_id from Account State, then query /api/positions?trader_id=xxx or /api/account?trader_id=xxx etc.`, userID, apiDocs) }