From 864ae2b7fe04d9b20e90751780cf673b9f11f579 Mon Sep 17 00:00:00 2001 From: sinohqb Date: Sun, 9 Aug 2026 03:20:40 +0800 Subject: [PATCH] fix(review): address release correctness findings --- backend/agenteval/channels/tutu.py | 36 ++++++------ .../agenteval/evaluation/campaign_runner.py | 5 +- backend/agenteval/web/routers/campaigns.py | 5 +- frontend/web/src/read/intelligentEval.test.ts | 40 ++++++++++++- frontend/web/src/read/intelligentEval.ts | 44 ++++++++++++--- .../src/read/useIntelligentEvalRead.test.tsx | 56 +++++++++++++++++++ .../web/src/read/useIntelligentEvalRead.ts | 17 ++++-- scripts/deploy-volcengine-102.sh | 22 +++++--- scripts/production-entrypoint.sh | 5 +- .../integration/test_campaign_analysis_api.py | 4 +- .../test_campaign_analysis_auto_trigger.py | 2 +- tests/unit/test_http_channel_and_rules.py | 27 ++++++++- tests/unit/test_production_entrypoint.py | 41 ++++++++++++++ 13 files changed, 249 insertions(+), 55 deletions(-) create mode 100644 tests/unit/test_production_entrypoint.py diff --git a/backend/agenteval/channels/tutu.py b/backend/agenteval/channels/tutu.py index 001df08..d57db36 100644 --- a/backend/agenteval/channels/tutu.py +++ b/backend/agenteval/channels/tutu.py @@ -111,24 +111,26 @@ class TutuApiChannel(EvalChannel): "size": 20, } resp = await client.get(url, headers=self._build_headers(), params=params) - if resp.status_code == 200: - data = resp.json() - records = data.get("data", []) if isinstance(data, dict) else [] - for msg in records: - msg_id = msg.get("id") or msg.get("msgId") - if not msg_id or msg_id in seen_ids: - continue - seen_ids.add(msg_id) + if resp.status_code != 200: + raise ChannelTransportError(f"轮询失败: HTTP {resp.status_code}: {resp.text[:500]}") - meta = msg.get("metadata", {}) - if meta.get("questionMsgId") == question_msg_id: - return Reply( - question_msg_id=question_msg_id, - content=msg.get("msgBody"), - sender_name=msg.get("senderName") or msg.get("actualSenderName"), - msg_time=msg.get("msgTime"), - raw_message=msg, - ) + data = resp.json() + records = data.get("data", []) if isinstance(data, dict) else [] + for msg in records: + msg_id = msg.get("id") or msg.get("msgId") + if not msg_id or msg_id in seen_ids: + continue + seen_ids.add(msg_id) + + meta = msg.get("metadata", {}) + if meta.get("questionMsgId") == question_msg_id: + return Reply( + question_msg_id=question_msg_id, + content=msg.get("msgBody"), + sender_name=msg.get("senderName") or msg.get("actualSenderName"), + msg_time=msg.get("msgTime"), + raw_message=msg, + ) except (httpx.HTTPError, json.JSONDecodeError) as exc: raise ChannelTransportError(f"轮询异常: {exc}") from exc diff --git a/backend/agenteval/evaluation/campaign_runner.py b/backend/agenteval/evaluation/campaign_runner.py index 60b985e..d94f55c 100644 --- a/backend/agenteval/evaluation/campaign_runner.py +++ b/backend/agenteval/evaluation/campaign_runner.py @@ -21,8 +21,7 @@ from typing import Optional from sqlmodel import Session -from agenteval.evaluation.analysis import enqueue_campaign_analysis as start_campaign_analysis -from agenteval.evaluation.analysis import resolve_analysis_model +from agenteval.evaluation.analysis import enqueue_campaign_analysis, resolve_analysis_model from agenteval.evaluation.campaign_lifecycle import complete_campaign from agenteval.evaluation.campaign_lifecycle import start_campaign as start_campaign_lifecycle from agenteval.evaluation.campaign_scheduler import ( @@ -255,7 +254,7 @@ def _auto_start_analysis(campaign: Campaign, session: Session) -> None: try: if resolve_analysis_model(campaign, session) is None: return - start_campaign_analysis(campaign.id, triggered_by="auto") + enqueue_campaign_analysis(campaign.id, triggered_by="auto") except Exception as exc: _logger.warning("活动 %s 自动分析触发失败（已跳过）: %s", campaign.id, exc) diff --git a/backend/agenteval/web/routers/campaigns.py b/backend/agenteval/web/routers/campaigns.py index 5c11adc..c458bb0 100644 --- a/backend/agenteval/web/routers/campaigns.py +++ b/backend/agenteval/web/routers/campaigns.py @@ -11,8 +11,7 @@ from fastapi import APIRouter, Body, Depends, HTTPException, Response from pydantic import BaseModel, Field from sqlmodel import Session -from agenteval.evaluation.analysis import enqueue_campaign_analysis as start_campaign_analysis -from agenteval.evaluation.analysis import resolve_analysis_model +from agenteval.evaluation.analysis import enqueue_campaign_analysis, resolve_analysis_model from agenteval.evaluation.campaign_lifecycle import CampaignCreateError, CampaignLifecycleError from agenteval.evaluation.campaign_lifecycle import cancel_campaign as cancel_campaign_lifecycle from agenteval.evaluation.campaign_lifecycle import create_campaign as create_campaign_lifecycle @@ -191,7 +190,7 @@ async def trigger_campaign_analysis(campaign_id: str, session: Session = Depends status_code=400, detail="未配置分析模型：请在模型配置中心将某个 chat 配置设为「分析默认」，或为该活动指定分析模型", ) - start_campaign_analysis(campaign_id, triggered_by="manual") + enqueue_campaign_analysis(campaign_id, triggered_by="manual") return {"status": "generating"} diff --git a/frontend/web/src/read/intelligentEval.test.ts b/frontend/web/src/read/intelligentEval.test.ts index 17f5279..b354cb7 100644 --- a/frontend/web/src/read/intelligentEval.test.ts +++ b/frontend/web/src/read/intelligentEval.test.ts @@ -29,10 +29,46 @@ describe('intelligent evaluation read state', () => { }) it('surfaces an initial detail failure without an existing snapshot', () => { - const failed = intelligentEvalReadReducer(initialIntelligentEvalReadState, { + const loading = intelligentEvalReadReducer(initialIntelligentEvalReadState, { + type: 'detail_requested', + id: 'eval-1', + requestId: 1, + }) + const failed = intelligentEvalReadReducer(loading, { type: 'detail_failed', + id: 'eval-1', + requestId: 1, error: '不存在', }) - expect(failed.detail).toEqual({ phase: 'error', value: null, error: '不存在' }) + expect(failed.detail).toEqual({ + phase: 'error', + value: null, + error: '不存在', + selectedId: 'eval-1', + requestId: 1, + }) + }) + + it('ignores a stale detail response after the selection changes', () => { + const first = intelligentEvalReadReducer(initialIntelligentEvalReadState, { + type: 'detail_requested', + id: 'eval-1', + requestId: 1, + }) + const second = intelligentEvalReadReducer(first, { + type: 'detail_requested', + id: 'eval-2', + requestId: 2, + }) + const stale = intelligentEvalReadReducer(second, { + type: 'detail_succeeded', + id: 'eval-1', + requestId: 1, + value: evaluation, + }) + + expect(stale).toBe(second) + expect(stale.detail.value).toBeNull() + expect(stale.detail.selectedId).toBe('eval-2') }) }) diff --git a/frontend/web/src/read/intelligentEval.ts b/frontend/web/src/read/intelligentEval.ts index fa55221..08595aa 100644 --- a/frontend/web/src/read/intelligentEval.ts +++ b/frontend/web/src/read/intelligentEval.ts @@ -10,16 +10,20 @@ export interface ReadSlot { export interface IntelligentEvalReadState { list: ReadSlot - detail: ReadSlot + detail: ReadSlot & { + selectedId: string | null + requestId: number + } } export type IntelligentEvalReadAction = | { type: 'list_requested'; silent?: boolean } | { type: 'list_succeeded'; value: IntelligentEval[] } | { type: 'list_failed'; error: string } - | { type: 'detail_requested'; silent?: boolean } - | { type: 'detail_succeeded'; value: IntelligentEval } - | { type: 'detail_failed'; error: string } + | { type: 'detail_cleared'; requestId: number } + | { type: 'detail_requested'; id: string; requestId: number; silent?: boolean } + | { type: 'detail_succeeded'; id: string; requestId: number; value: IntelligentEval } + | { type: 'detail_failed'; id: string; requestId: number; error: string } export interface IntelligentEvalReadAdapter { list: () => Promise @@ -33,7 +37,7 @@ export const intelligentEvalReadAdapter: IntelligentEvalReadAdapter = { export const initialIntelligentEvalReadState: IntelligentEvalReadState = { list: { phase: 'idle', value: [], error: null }, - detail: { phase: 'idle', value: null, error: null }, + detail: { phase: 'idle', value: null, error: null, selectedId: null, requestId: 0 }, } function requestPhase(slot: ReadSlot, silent: boolean | undefined): ReadSlot { @@ -56,11 +60,35 @@ export function intelligentEvalReadReducer( ? { ...state.list, phase: 'ready', error: null } : { ...state.list, phase: 'error', error: action.error }, } - case 'detail_requested': - return { ...state, detail: requestPhase(state.detail, action.silent) } + case 'detail_cleared': + return { + ...state, + detail: { phase: 'idle', value: null, error: null, selectedId: null, requestId: action.requestId }, + } + case 'detail_requested': { + const sameSelection = state.detail.selectedId === action.id + const current = sameSelection + ? state.detail + : { ...state.detail, value: null, selectedId: action.id } + return { + ...state, + detail: { ...requestPhase(current, action.silent), selectedId: action.id, requestId: action.requestId }, + } + } case 'detail_succeeded': - return { ...state, detail: { phase: 'ready', value: action.value, error: null } } + if (state.detail.selectedId !== action.id || state.detail.requestId !== action.requestId) return state + return { + ...state, + detail: { + phase: 'ready', + value: action.value, + error: null, + selectedId: action.id, + requestId: action.requestId, + }, + } case 'detail_failed': + if (state.detail.selectedId !== action.id || state.detail.requestId !== action.requestId) return state return { ...state, detail: state.detail.value diff --git a/frontend/web/src/read/useIntelligentEvalRead.test.tsx b/frontend/web/src/read/useIntelligentEvalRead.test.tsx index 6010799..09b2b4a 100644 --- a/frontend/web/src/read/useIntelligentEvalRead.test.tsx +++ b/frontend/web/src/read/useIntelligentEvalRead.test.tsx @@ -6,6 +6,16 @@ import type { IntelligentEval } from '../api' const evaluation = { id: 'eval-1', name: '评估', status: 'executing' } as unknown as IntelligentEval +function deferred() { + let resolve!: (value: T) => void + let reject!: (error: Error) => void + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise + reject = rejectPromise + }) + return { promise, resolve, reject } +} + afterEach(() => { vi.useRealTimers() }) @@ -54,4 +64,50 @@ describe('useIntelligentEvalRead', () => { expect(result.current.list.value).toEqual([evaluation]) expect(result.current.list.error).toBeNull() }) + + it('clears the old detail and ignores a response for the previous selection', async () => { + const first = deferred() + const second = deferred() + const secondEvaluation = { ...evaluation, id: 'eval-2', name: '评估二' } as IntelligentEval + const adapter: IntelligentEvalReadAdapter = { + list: vi.fn().mockResolvedValue([]), + get: vi.fn((id: string) => (id === 'eval-1' ? first.promise : second.promise)), + } + const { result, rerender } = renderHook( + ({ selectedId }) => useIntelligentEvalRead(selectedId, adapter), + { initialProps: { selectedId: 'eval-1' as string | null } }, + ) + + rerender({ selectedId: 'eval-2' }) + expect(result.current.detail.value).toBeNull() + expect(result.current.detail.selectedId).toBe('eval-2') + + await act(async () => { second.resolve(secondEvaluation) }) + expect(result.current.detail.value?.id).toBe('eval-2') + + await act(async () => { first.resolve(evaluation) }) + expect(result.current.detail.value?.id).toBe('eval-2') + }) + + it('surfaces failure for a newly selected detail instead of retaining the old snapshot', async () => { + const adapter: IntelligentEvalReadAdapter = { + list: vi.fn().mockResolvedValue([]), + get: vi.fn() + .mockResolvedValueOnce(evaluation) + .mockRejectedValueOnce(new Error('不存在')), + } + const { result, rerender } = renderHook( + ({ selectedId }) => useIntelligentEvalRead(selectedId, adapter), + { initialProps: { selectedId: 'eval-1' as string | null } }, + ) + await settle() + expect(result.current.detail.value?.id).toBe('eval-1') + + rerender({ selectedId: 'eval-2' }) + await settle() + + expect(result.current.detail.phase).toBe('error') + expect(result.current.detail.value).toBeNull() + expect(result.current.detail.error).toBe('不存在') + }) }) diff --git a/frontend/web/src/read/useIntelligentEvalRead.ts b/frontend/web/src/read/useIntelligentEvalRead.ts index c6ab4ba..19bf80d 100644 --- a/frontend/web/src/read/useIntelligentEvalRead.ts +++ b/frontend/web/src/read/useIntelligentEvalRead.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useReducer } from 'react' +import { useCallback, useEffect, useLayoutEffect, useReducer, useRef } from 'react' import { intelligentEvalReadAdapter, intelligentEvalReadReducer, @@ -23,6 +23,7 @@ export function useIntelligentEvalRead( adapter: IntelligentEvalReadAdapter = intelligentEvalReadAdapter, ): IntelligentEvalReadState & { reloadList: () => Promise; reloadDetail: () => Promise } { const [state, dispatch] = useReducer(intelligentEvalReadReducer, initialIntelligentEvalReadState) + const detailRequestId = useRef(0) const loadList = useCallback(async (silent = false) => { dispatch({ type: 'list_requested', silent }) @@ -35,12 +36,13 @@ export function useIntelligentEvalRead( }, [adapter]) const loadDetail = useCallback(async (id: string, silent = false) => { - dispatch({ type: 'detail_requested', silent }) + const requestId = ++detailRequestId.current + dispatch({ type: 'detail_requested', id, requestId, silent }) try { const value = await adapter.get(id) - dispatch({ type: 'detail_succeeded', value }) + dispatch({ type: 'detail_succeeded', id, requestId, value }) } catch (error) { - dispatch({ type: 'detail_failed', error: errorMessage(error) }) + dispatch({ type: 'detail_failed', id, requestId, error: errorMessage(error) }) } }, [adapter]) @@ -48,8 +50,11 @@ export function useIntelligentEvalRead( void loadList() }, [loadList]) - useEffect(() => { - if (selectedId == null) return + useLayoutEffect(() => { + if (selectedId == null) { + dispatch({ type: 'detail_cleared', requestId: ++detailRequestId.current }) + return + } void loadDetail(selectedId) }, [loadDetail, selectedId]) diff --git a/scripts/deploy-volcengine-102.sh b/scripts/deploy-volcengine-102.sh index 972380d..4dd6e10 100755 --- a/scripts/deploy-volcengine-102.sh +++ b/scripts/deploy-volcengine-102.sh @@ -75,6 +75,14 @@ done ROOT="$(cd "$(dirname "$0")/.." && pwd)" cd "$ROOT" +RELEASE_DIR="" +cleanup_release_source() { + if [[ -n "$RELEASE_DIR" && -d "$RELEASE_DIR" ]]; then + rm -rf "$RELEASE_DIR" + fi +} +trap cleanup_release_source EXIT + validate_tag() { [[ "$1" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]] || die "invalid image tag: $1" } @@ -151,21 +159,17 @@ run ssh "$HOST" "set -eu; mkdir -p '$REMOTE_DIR'; \ test -f '$REMOTE_DIR/deploy/volcengine-102/.env' \ || { echo 'missing production deploy/volcengine-102/.env' >&2; exit 1; }" +log "prepare committed release source from HEAD" +RELEASE_DIR=$(mktemp -d "${TMPDIR:-/tmp}/agenteval-release.XXXXXX") +git archive --format=tar HEAD | tar -xf - -C "$RELEASE_DIR" + log "sync committed release source → $HOST:$REMOTE_DIR" run rsync -az --delete \ - --exclude='.git' \ - --exclude='.scratch' \ --exclude='.env' \ - --exclude='node_modules' \ - --exclude='frontend/web/dist' \ - --exclude='.venv' \ - --exclude='__pycache__' \ - --exclude='.pytest_cache' \ - --exclude='.ruff_cache' \ --exclude='data' \ --exclude='config/config.json' \ --exclude='*.db*' \ - ./ "$HOST:$REMOTE_DIR/" + "$RELEASE_DIR/" "$HOST:$REMOTE_DIR/" backup_remote_data "$IMAGE_TAG" diff --git a/scripts/production-entrypoint.sh b/scripts/production-entrypoint.sh index 446506c..a9c921c 100755 --- a/scripts/production-entrypoint.sh +++ b/scripts/production-entrypoint.sh @@ -14,9 +14,8 @@ if [ ! -f "$DB_PATH" ]; then else HAS_ALEMBIC_VERSION=$(python -c "import sqlite3, sys; connection = sqlite3.connect(sys.argv[1]); row = connection.execute(\"SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'alembic_version'\").fetchone(); connection.close(); print('yes' if row else 'no')" "$DB_PATH") if [ "$HAS_ALEMBIC_VERSION" = "no" ]; then - echo "legacy production db detected; recording the current schema baseline" - python -c "from agenteval.storage.db import init_db; init_db()" - alembic stamp head + echo "legacy production db detected; starting migrations from the pre-Alembic baseline" + alembic stamp base fi fi diff --git a/tests/integration/test_campaign_analysis_api.py b/tests/integration/test_campaign_analysis_api.py index 7c3f123..11e9bf6 100644 --- a/tests/integration/test_campaign_analysis_api.py +++ b/tests/integration/test_campaign_analysis_api.py @@ -151,7 +151,7 @@ async def test_post_then_get_completed_analysis(client, seeded_db, monkeypatch): # 假后台任务：同步写入 completed 行（真任务的单测覆盖在 test_campaign_analysis.py） monkeypatch.setattr( campaigns_module, - "start_campaign_analysis", + "enqueue_campaign_analysis", lambda cid, *, triggered_by: _complete_analysis_row(seeded_db, cid), ) @@ -172,7 +172,7 @@ async def test_rerun_upserts_without_new_row(client, seeded_db, monkeypatch): campaign_id = await _create_campaign(client, seeded_db) monkeypatch.setattr( campaigns_module, - "start_campaign_analysis", + "enqueue_campaign_analysis", lambda cid, *, triggered_by: _complete_analysis_row(seeded_db, cid), ) diff --git a/tests/integration/test_campaign_analysis_auto_trigger.py b/tests/integration/test_campaign_analysis_auto_trigger.py index 9fa5add..24584f5 100644 --- a/tests/integration/test_campaign_analysis_auto_trigger.py +++ b/tests/integration/test_campaign_analysis_auto_trigger.py @@ -63,7 +63,7 @@ def analysis_spy(monkeypatch): """Spy the analysis seam: resolvable model, recorded enqueue calls.""" calls: list[tuple[str, str]] = [] monkeypatch.setattr( - campaign_runner, "start_campaign_analysis", + campaign_runner, "enqueue_campaign_analysis", lambda cid, *, triggered_by: calls.append((cid, triggered_by)), ) monkeypatch.setattr(campaign_runner, "resolve_analysis_model", lambda campaign, session: object()) diff --git a/tests/unit/test_http_channel_and_rules.py b/tests/unit/test_http_channel_and_rules.py index a064ad1..cbf8742 100644 --- a/tests/unit/test_http_channel_and_rules.py +++ b/tests/unit/test_http_channel_and_rules.py @@ -4,9 +4,10 @@ from unittest.mock import AsyncMock, MagicMock, patch import httpx import pytest -from agenteval.channels.base import ExchangeStatus +from agenteval.channels.base import ExchangeStatus, SendResult from agenteval.channels.http import HttpChannel, _get_path from agenteval.channels.openclaw import OpenClawChannel +from agenteval.channels.tutu import TutuApiChannel from agenteval.evaluation.rules.keyword import KeywordMatchRule from agenteval.evaluation.rules.response_time import ResponseTimeRule from agenteval.models import Case, CaseType, Expectation, Turn @@ -105,6 +106,30 @@ async def test_http_exchange_does_not_mask_adapter_programming_errors(): await ch.exchange("hi") +async def test_tutu_exchange_classifies_non_200_poll_response_as_failure(): + channel = TutuApiChannel( + { + "base_url": "http://mock", + "token": "token", + "tenant": "tenant", + "chat_channel_id": "channel", + "chat_contact_id": "contact", + } + ) + response = MagicMock(status_code=502, text="bad gateway") + client = MagicMock() + client.get = AsyncMock(return_value=response) + + with ( + patch.object(channel, "_send", new=AsyncMock(return_value=SendResult(ok=True, question_msg_id="msg-1"))), + patch.object(channel, "_get_client", new=AsyncMock(return_value=client)), + ): + outcome = await channel.exchange("hi", timeout=0.1, poll_interval=0.01) + + assert outcome.status is ExchangeStatus.POLL_FAILED + assert "HTTP 502" in outcome.reason + + async def test_http_poll_reply_found(): ch = _make_channel(reply_path="answer") call_count = {"n": 0} diff --git a/tests/unit/test_production_entrypoint.py b/tests/unit/test_production_entrypoint.py new file mode 100644 index 0000000..d15b058 --- /dev/null +++ b/tests/unit/test_production_entrypoint.py @@ -0,0 +1,41 @@ +import os +import sqlite3 +import subprocess +import sys +from pathlib import Path + + +def _write_executable(path: Path, content: str) -> None: + path.write_text(content) + path.chmod(0o755) + + +def test_legacy_database_starts_migrations_from_pre_alembic_baseline(tmp_path: Path) -> None: + database_path = tmp_path / "legacy.db" + connection = sqlite3.connect(database_path) + connection.execute("CREATE TABLE scenarios (id TEXT PRIMARY KEY, name TEXT NOT NULL)") + connection.close() + + command_log = tmp_path / "commands.log" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + recorder = '#!/bin/sh\nprintf "%s\\n" "$*" >> "$COMMAND_LOG"\n' + _write_executable(bin_dir / "alembic", recorder) + _write_executable(bin_dir / "agenteval", recorder) + _write_executable(bin_dir / "python", f'#!/bin/sh\nexec "{sys.executable}" "$@"\n') + + environment = { + **os.environ, + "AGENTEVAL_DB_PATH": str(database_path), + "COMMAND_LOG": str(command_log), + "PATH": f"{bin_dir}:{os.environ['PATH']}", + } + script = Path(__file__).resolve().parents[2] / "scripts" / "production-entrypoint.sh" + + subprocess.run(["sh", str(script)], check=True, env=environment) + + assert command_log.read_text().splitlines() == [ + "stamp base", + "upgrade head", + "server start --host 0.0.0.0 --port 8000", + ]