diff --git a/packages/core/src/opendirector/agent/tools.py b/packages/core/src/opendirector/agent/tools.py index 91b3383d..369f13dc 100644 --- a/packages/core/src/opendirector/agent/tools.py +++ b/packages/core/src/opendirector/agent/tools.py @@ -1,11 +1,12 @@ """Business commands shared by every agent runtime.""" import asyncio +import difflib import shutil from pathlib import Path +from typing import get_args from uuid import UUID, uuid5 -from pydantic import JsonValue, TypeAdapter - +from pydantic import JsonValue, TypeAdapter, ValidationError from opendirector.application.creation_artifacts import publish_artifact from opendirector.application.creation_assets import CreationAssets from opendirector.application.creation_canvas import CreationCanvas @@ -22,6 +23,29 @@ ROOT = SOURCE_ROOT or BUNDLED EXCLUDED = {'project.create', 'project.trash', 'project.restore', 'asset.import', 'job.checkpoint', 'canvas.applyEdit', 'canvas.publishMedia', 'timeline.publishRender'} + +QUERY_MODELS = get_args(get_args(Query)[0]) +#: The legal `type` values, read from the union itself. A new query type cannot +#: leave the tool description or the error guidance behind. +QUERY_TYPES = tuple(sorted(get_args(model.model_fields['type'].annotation)[0] for model in QUERY_MODELS)) + + +def query_guidance(failure): + """Explain an unusable query without losing the detail the caller needs. + + A guessed type (`canvas_items`) is answered with the accepted values and the + closest match. Every other problem keeps pydantic's field-level message: the + field name and constraint are what let the caller fix the argument, so a + generic summary here would be a regression (see test_agent_artifacts, which + asserts that an unexpected `waitSeconds` is named). + """ + for issue in failure.errors(include_input=False, include_url=False): + if issue['type'] == 'union_tag_invalid': + given = issue.get('ctx', {}).get('tag', '') + closest = difflib.get_close_matches(str(given), QUERY_TYPES, n=1, cutoff=0.4) + hint = f' Did you mean {closest[0]}?' if closest else '' + return f'Unsupported query type {given!r}.{hint} Valid types: {", ".join(QUERY_TYPES)}.' + return str(failure) COMMANDS = {name: model for name, model in COMMAND_MODELS.items() if name not in EXCLUDED} for command_name, command_model in COMMANDS.items(): @@ -208,7 +232,8 @@ def tools() -> list[dict]: for field in ('queryId', 'projectId'): definition['properties'].pop(field) definition['required'].remove(field) - result.append(tool_schema('query', 'Query objects, jobs, assets or versions. canvas.status can wait; asset.preview provides media for native viewing.', query_schema)) + result.append(tool_schema('query', f'Query objects, jobs, assets or versions. Valid types: {", ".join(QUERY_TYPES)}. ' + 'canvas.status waits and needs objectId; asset.preview returns media for native viewing.', query_schema)) command_type = {'type': 'string', 'enum': list(COMMANDS)} result.append(tool_schema('command_schema', 'Read command help before the first execute_command call of that type in this turn. ' 'Find commands with read_project. For the first lookup, pass only the exact type, e.g. {"type":"canvas.show"}. ' @@ -243,8 +268,13 @@ async def execute(self, project_id: UUID, turn_id: UUID, session: str, call_id: if name == 'query': if {'projectId', 'queryId'} & arguments.keys(): raise ProjectError('查询身份与工程范围由宿主提供。') - query = TypeAdapter(Query).validate_python({**arguments, 'projectId': project_id, - 'queryId': uuid5(turn_id, call_id)}) + try: + query = TypeAdapter(Query).validate_python({**arguments, 'projectId': project_id, + 'queryId': uuid5(turn_id, call_id)}) + except ValidationError as failure: + # A rejected query must name the accepted types rather than echo a + # tag list the model has to reinterpret. + raise ProjectError(query_guidance(failure)) from None result = await CreationCanvas(self.projects).query(query) if query.type == 'asset.preview' and 'preview' in result: with self.projects.local.transaction() as db: diff --git a/tests/test_query_guidance.py b/tests/test_query_guidance.py new file mode 100644 index 00000000..08623e3f --- /dev/null +++ b/tests/test_query_guidance.py @@ -0,0 +1,41 @@ +"""Issue #132: a rejected query must name the types and fields it actually accepts.""" +from uuid import uuid4 + +import pytest +from pydantic import TypeAdapter, ValidationError + +from opendirector.agent.tools import QUERY_TYPES, query_guidance, tools +from opendirector.domain.creation.commands import Query + + +def guidance(arguments): + try: + TypeAdapter(Query).validate_python({'projectId': uuid4(), 'queryId': uuid4(), **arguments}) + except ValidationError as failure: + return query_guidance(failure) + raise AssertionError('expected the query to be rejected') + + +def test_the_tool_description_lists_every_legal_type(): + tool = next(item['function'] for item in tools() if item['function']['name'] == 'query') + for name in QUERY_TYPES: + assert name in tool['description'], name + # The type the assistant guessed in #132 is not one of them. + assert 'canvas_items' not in QUERY_TYPES + + +def test_an_unknown_type_is_answered_with_the_closest_legal_one(): + message = guidance({'type': 'canvas_items'}) + assert 'canvas_items' in message + assert 'canvas.status' in message # the suggestion + assert all(name in message for name in QUERY_TYPES) + + +def test_a_missing_field_is_named(): + message = guidance({'type': 'canvas.status'}) + assert 'objectId' in message + +def test_an_unexpected_field_keeps_its_name(): + """Field-level detail must survive: test_agent_artifacts relies on it.""" + message = guidance({'type': 'object.get', 'objectId': str(uuid4()), 'waitSeconds': 1}) + assert 'waitSeconds' in message