[fix] agent: name the legal query types in the description and in rejections (#132)

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:

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