Repository navigation
fix(agent): 修复结构化响应校验失败后无法重试 #369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
|
|
||
| import asyncio | ||
| import json | ||
| from typing import Any | ||
| from typing import List | ||
| from unittest.mock import Mock | ||
|
|
||
|
|
@@ -24,9 +25,11 @@ | |
| from trpc_agent_sdk.context import InvocationContext, create_agent_context | ||
| from trpc_agent_sdk.events import Event | ||
| from trpc_agent_sdk.models import LLMModel, LlmRequest, LlmResponse, ModelRegistry | ||
| from trpc_agent_sdk.runners import Runner | ||
| from trpc_agent_sdk.sessions import InMemorySessionService | ||
| from trpc_agent_sdk.tools import FunctionTool | ||
| from trpc_agent_sdk.tools import TOOL_NAME as SET_MODEL_RESPONSE_TOOL_NAME | ||
| from trpc_agent_sdk.types import Content, FunctionResponse, Part | ||
| from trpc_agent_sdk.types import Content, FunctionCall, FunctionResponse, Part | ||
|
|
||
|
|
||
| class MockLLMModel(LLMModel): | ||
|
|
@@ -41,6 +44,40 @@ def validate_request(self, request): | |
| pass | ||
|
|
||
|
|
||
| class ScriptedToolCallModel(LLMModel): | ||
| """Model test double that returns scripted function calls.""" | ||
|
|
||
| def __init__(self, responses: list[dict[str, Any]]): | ||
| super().__init__(model_name="scripted-output-model") | ||
| self.responses = responses | ||
| self.calls = 0 | ||
|
|
||
| @classmethod | ||
| def supported_models(cls) -> List[str]: | ||
| return [r"scripted-output-model"] | ||
|
|
||
| async def _generate_async_impl(self, request, stream=False, ctx=None): | ||
| args = self.responses[self.calls] | ||
| self.calls += 1 | ||
| yield LlmResponse( | ||
| content=Content( | ||
| role="model", | ||
| parts=[ | ||
| Part( | ||
| function_call=FunctionCall( | ||
| id=f"set-response-{self.calls}", | ||
| name=SET_MODEL_RESPONSE_TOOL_NAME, | ||
| args=args, | ||
| ) | ||
| ) | ||
| ], | ||
| ) | ||
| ) | ||
|
|
||
| def validate_request(self, request): | ||
| pass | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module", autouse=True) | ||
| def register_test_model(): | ||
| original_registry = ModelRegistry._registry.copy() | ||
|
|
@@ -108,7 +145,11 @@ def test_returns_none_for_none_event(self): | |
| assert get_structured_model_response(None) is None | ||
|
|
||
| def test_returns_none_for_no_function_responses(self): | ||
| event = Event(invocation_id="inv-1", author="agent", content=Content(parts=[Part(text="text")])) | ||
| event = Event( | ||
| invocation_id="inv-1", | ||
| author="agent", | ||
| content=Content(parts=[Part(text="text")]), | ||
| ) | ||
| assert get_structured_model_response(event) is None | ||
|
|
||
| def test_returns_none_for_non_set_model_response(self): | ||
|
|
@@ -134,6 +175,108 @@ def test_returns_json_for_set_model_response(self): | |
| assert parsed["answer"] == "42" | ||
| assert parsed["confidence"] == 0.95 | ||
|
|
||
| def test_returns_none_for_failed_set_model_response(self): | ||
| fr = FunctionResponse( | ||
| name=SET_MODEL_RESPONSE_TOOL_NAME, | ||
| response={ | ||
| "error": "tool_execution_error", | ||
| "message": "validation failed", | ||
| "status": "failed", | ||
| }, | ||
| ) | ||
| event = Event( | ||
| invocation_id="inv-1", | ||
| author="agent", | ||
| content=Content(parts=[Part(function_response=fr)]), | ||
| error_code="tool_execution_error", | ||
| error_message="validation failed", | ||
| ) | ||
|
|
||
| assert get_structured_model_response(event) is None | ||
|
|
||
| def test_returns_none_for_failed_response_in_merged_event(self): | ||
| """Parallel merging drops child error_code but retains its response.""" | ||
| success = FunctionResponse( | ||
| id="other-call", | ||
| name="other_tool", | ||
| response={"result": "ok"}, | ||
| ) | ||
| failed = FunctionResponse( | ||
| id="set-response", | ||
| name=SET_MODEL_RESPONSE_TOOL_NAME, | ||
| response={ | ||
| "error": "tool_execution_error", | ||
| "message": "validation failed", | ||
| "status": "failed", | ||
| }, | ||
| ) | ||
| event = Event( | ||
| invocation_id="inv-1", | ||
| author="agent", | ||
| content=Content( | ||
| parts=[ | ||
| Part(function_response=success), | ||
| Part(function_response=failed), | ||
| ] | ||
| ), | ||
| ) | ||
|
|
||
| assert get_structured_model_response(event) is None | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_invalid_set_model_response_retries_until_valid(): | ||
| model = ScriptedToolCallModel( | ||
| [ | ||
| {"answer": "missing confidence"}, | ||
| {"answer": "42", "confidence": 0.95}, | ||
| ] | ||
| ) | ||
|
|
||
| def dummy_tool() -> str: | ||
| """Keep the agent on the output-schema-plus-tools code path.""" | ||
| return "ok" | ||
|
|
||
| from trpc_agent_sdk.agents import LlmAgent | ||
|
|
||
| agent = LlmAgent( | ||
| name="retry_output_agent", | ||
| model=model, | ||
| output_schema=SampleOutputSchema, | ||
| tools=[FunctionTool(dummy_tool)], | ||
| ) | ||
| session_service = InMemorySessionService() | ||
| await session_service.create_session( | ||
| app_name="test", | ||
| user_id="u1", | ||
| session_id="retry", | ||
| ) | ||
| runner = Runner( | ||
| app_name="test", | ||
| agent=agent, | ||
| session_service=session_service, | ||
| ) | ||
|
|
||
| events = [ | ||
| event | ||
| async for event in runner.run_async( | ||
| user_id="u1", | ||
| session_id="retry", | ||
| new_message=Content( | ||
| role="user", | ||
| parts=[Part(text="Return a structured answer")], | ||
| ), | ||
| ) | ||
| ] | ||
|
|
||
| assert model.calls == 2 | ||
| assert any(event.error_code == "tool_execution_error" for event in events) | ||
| final_event = events[-1] | ||
| assert json.loads(final_event.get_text()) == { | ||
| "answer": "42", | ||
| "confidence": 0.95, | ||
| } | ||
|
Comment on lines
+227
to
+278
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 问题: 新增测试只覆盖「错误被识别 → 重试 → 成功」的可恢复路径,未覆盖本变更引入的两个关键行为面:合法载荷与错误信封形状碰撞时不得被误判(即校验通过的输出无论内容如何都应作为最终响应),以及连续失败耗尽重试预算时的终止行为。 触发条件: 运行 实际影响: 碰撞误判与无界重试两类缺陷在测试中不可见,回归风险滞留到线上:结构化输出间歇性被吞、成本激增或 修正方向: 补充用例: (a) 输出 schema 含 |
||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # OutputSchemaRequestProcessor | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,7 +100,18 @@ def get_structured_model_response(function_response_event: Event) -> str | None: | |
|
|
||
| for func_response in function_response_event.get_function_responses(): | ||
| if func_response.name == SET_MODEL_RESPONSE_TOOL_NAME: | ||
| # Tool failures are returned as function responses so that the | ||
| # model can inspect the validation error and retry. Do not treat | ||
| # that error envelope as the final structured response. Checking | ||
| # the payload also covers errors inside merged parallel-tool | ||
| # events, where the child Event.error_code is not preserved. | ||
| response = func_response.response | ||
| if function_response_event.error_code or (isinstance(response, dict) and response.get("status") == "failed" | ||
| and "error" in response and "message" in response): | ||
|
Comment on lines
+109
to
+110
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 问题: 新增失败判定通过载荷内容嗅探识别错误封套:只要 触发条件: 用户 实际影响: Agent 的有效最终结构化答案被静默丢弃,被要求“修复”一个本已正确的答案,循环重试直到默认 修正方向: 不要按任意键名嗅探。改为比较 |
||
| logger.debug("set_model_response failed; continuing the agent loop for retry") | ||
| return None | ||
|
Comment on lines
+111
to
+112
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 问题: 修复将校验失败从“立即以错误封套终止”改为“返回 None 让 Agent 循环重试”,但该重试没有针对此失败模式的重试预算或优雅终止路径:循环内仅受 触发条件: 模型持续输出无法通过 schema 校验的 实际影响: 默认配置下累计约 500 次 LLM 调用与工具执行(真实计费成本)后以异常终止,不产出任何最终事件, 修正方向: 为 |
||
|
|
||
| # Convert dict to JSON string | ||
| return json.dumps(func_response.response, ensure_ascii=False) | ||
| return json.dumps(response, ensure_ascii=False) | ||
|
|
||
| return None | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
问题: 新集成测试
test_invalid_set_model_response_retries_until_valid只覆盖“1 次失败后成功”路径,未覆盖持续失败路径;ScriptedToolCallModel._generate_async_impl以self.responses[self.calls]取响应,第 3 次调用即抛IndexError,使持续失败场景无法用该测试双端自然表达;并行合并分支仅用手工构造的合并事件覆盖,未经过真实_merge_parallel_function_response_events端到端路径。触发条件: 需要验证持续校验失败时的系统行为(期望的受限重试、优雅终止事件),或
parallel_tool_calls=True下真实合并事件对失败封套的检测时,现有测试均无法提供覆盖。实际影响: 本提交新增的两个高风险行为(无界重试、载荷误判)均无自动化保护,后续改动易回归;测试双端在响应耗尽时抛出原始
IndexError还会掩盖断言失败、误导定位。修正方向: 增加“恒失败模型”测试,断言达到重试上限后产生含最后一次校验错误的终态事件且不抛出异常;增加
parallel_tool_calls=True下真实合并事件的端到端测试;测试双端对越界响应做截断或抛出语义明确的错误。