Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…_metadata_object tool pairs - Created unified `list-objects.tool.ts` (name: 'list_objects') with filter/includeFields - Created unified `describe-object.tool.ts` (name: 'describe_object') with snake_case validation - Deleted old `list-metadata-objects.tool.ts` and `describe-metadata-object.tool.ts` - Removed duplicate LIST_OBJECTS_TOOL, DESCRIBE_OBJECT_TOOL from data-tools.ts - Removed duplicate ObjectDef/FieldDef types from data-tools.ts - DATA_TOOL_DEFINITIONS reduced from 5 to 3 (query_records, get_record, aggregate_data) - METADATA_TOOL_DEFINITIONS retains 6 tools with unified names - Updated metadata_assistant agent to use list_objects/describe_object - Updated all tests across service-ai and studio - Updated CHANGELOG.md Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/9ed2bcd9-23a4-4f4d-b8ba-770577f41cb3 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR removes the duplicate list_objects/describe_object implementations that previously existed in both “data tools” and “metadata tools”, and standardizes on a single richer implementation registered via registerMetadataTools() so both data_chat and metadata_assistant share identical capabilities.
Changes:
- Consolidated
list_objectsanddescribe_objecttool metadata under unified tool definitions and removed the duplicate data-tools versions/types/handlers. - Simplified
DATA_TOOL_DEFINITIONSto the 3 record-level tools and updatedDataToolContextto no longer requiremetadataService. - Updated agents/tests/exports/changelog to use the unified tool names and new output shapes.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/services/service-ai/src/tools/metadata-tools.ts | Swaps tool exports/registrations to unified list_objects/describe_object. |
| packages/services/service-ai/src/tools/list-objects.tool.ts | Renames tool metadata to list_objects (unified). |
| packages/services/service-ai/src/tools/describe-object.tool.ts | Adds unified describe_object tool metadata file. |
| packages/services/service-ai/src/tools/describe-metadata-object.tool.ts | Removes the old duplicate describe_metadata_object tool metadata. |
| packages/services/service-ai/src/tools/index.ts | Updates tool exports to the unified names. |
| packages/services/service-ai/src/tools/data-tools.ts | Removes list/describe tools and drops metadataService from DataToolContext/registration. |
| packages/services/service-ai/src/plugin.ts | Updates registerDataTools() call site to pass only { dataEngine }. |
| packages/services/service-ai/src/index.ts | Re-exports unified tool metadata from metadata-tools. |
| packages/services/service-ai/src/agents/metadata-assistant-agent.ts | Updates agent instructions/tool list to list_objects/describe_object. |
| packages/services/service-ai/src/tests/metadata-tools.test.ts | Updates tool names and asserts unified registration behavior. |
| packages/services/service-ai/src/tests/chatbot-features.test.ts | Updates tool counts and moves list/describe expectations to metadata-tool registration. |
| apps/studio/test/ai-chat-panel.test.tsx | Updates tool invocation part name to list_objects. |
| CHANGELOG.md | Documents the tool unification and tool-definition split change. |
| @@ -258,7 +258,7 @@ export class AIServicePlugin implements Plugin { | |||
| try { | |||
| const dataEngine = ctx.getService<IDataEngine>('data'); | |||
| if (dataEngine && metadataService) { | |||
| registerDataTools(this.service.toolRegistry, { dataEngine, metadataService }); | |||
| registerDataTools(this.service.toolRegistry, { dataEngine }); | |||
| ctx.logger.info('[AI] Built-in data tools registered'); | |||
There was a problem hiding this comment.
registerDataTools() no longer requires metadataService, but this block still gates data-tool registration on metadataService being present (if (dataEngine && metadataService)). This can prevent query_records/get_record/aggregate_data from being registered in runtimes where the data engine exists but metadata is unavailable. Consider registering data tools whenever dataEngine is available, and keep the agent auto-registration (which does require metadataService) behind a separate if (metadataService) check.
| registry: ToolRegistry, | ||
| context: DataToolContext, | ||
| ): void { | ||
| registry.register(LIST_OBJECTS_TOOL, createListObjectsHandler(context)); | ||
| registry.register(DESCRIBE_OBJECT_TOOL, createDescribeObjectHandler(context)); | ||
| registry.register(QUERY_RECORDS_TOOL, createQueryRecordsHandler(context)); | ||
| registry.register(GET_RECORD_TOOL, createGetRecordHandler(context)); | ||
| registry.register(AGGREGATE_DATA_TOOL, createAggregateDataHandler(context)); |
There was a problem hiding this comment.
registerDataTools() signature now only takes { dataEngine }, but the doc comment immediately above still states it’s called after the metadata service is available and the example passes metadataService. Update the JSDoc/example to match the new DataToolContext so callers don’t attempt to provide a non-existent dependency.
|
@copilot fix all |
…stale JSDoc - plugin.ts: Register data tools whenever dataEngine is available, not gated on metadataService. Agent auto-registration still requires metadataService and is nested inside a separate check. - data-tools.ts: Update JSDoc/example to match new DataToolContext (no longer references metadataService). Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/f69965e4-42f5-4f19-84ef-74c854bdf5ac Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Fixed both review comments in
|
list_objectsandlist_metadata_objectsshare the same data source (metadataService.listObjects()), withlist_objectsbeing a strictly weaker subset (nofilter/includeFields). Same duplication exists fordescribe_object/describe_metadata_object. This created naming confusion, duplicateObjectDef/FieldDeftypes, and gavedata_chatagent inferior capabilities.Changes
Unified tool definitions: Created
list-objects.tool.tsanddescribe-object.tool.tswith the richer parameter sets (filter, includeFields, snake_case validation, enableFeatures). Deletedlist-metadata-objects.tool.tsanddescribe-metadata-object.tool.ts.data-tools.ts: RemovedLIST_OBJECTS_TOOL,DESCRIBE_OBJECT_TOOL, their handlers, and duplicateObjectDef/FieldDeftypes.DATA_TOOL_DEFINITIONS: 5 → 3 tools (query_records, get_record, aggregate_data).DataToolContextno longer requiresmetadataService.metadata-tools.ts:METADATA_TOOL_DEFINITIONSretains 6 tools, now registeringlist_objectsanddescribe_object(unified names) with the full-capability handlers.Agents:
metadata_assistantreferences updated fromlist_metadata_objects/describe_metadata_object→list_objects/describe_object.data_chatalready used these names — no change needed.Exports: Updated
tools/index.tsandsrc/index.ts—listObjectsTool/describeObjectToolreplacelistMetadataObjectsTool/describeMetadataObjectTool.Registration flow
Both agents get
list_objects/describe_objectfromregisterMetadataTools(), which only requiresmetadataService(always available when data tools are registered):