-
Notifications
You must be signed in to change notification settings - Fork 10
feat(chat): rely fully on jsdoc to bring in description and schema #1773
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
Conversation
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.
Pull Request Overview
This PR refactors the LLM tool system to automatically extract tool descriptions and schemas from TypeScript/JSDoc comments instead of requiring manual specification. The change simplifies tool definitions by relying on TypeScript types and JSDoc annotations for metadata.
- Removes manual description and inputSchema properties from tool definitions
- Adds logic to extract schemas and descriptions from patterns/handlers automatically
- Updates type definitions to make description and inputSchema optional in BuiltInLLMTool
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/runner/src/builtins/llm-dialog.ts | Implements schema extraction logic and fallback to JSDoc descriptions |
| packages/patterns/chatbot-tools.tsx | Removes manual tool schemas and adds JSDoc comments with type definitions |
| packages/api/index.ts | Updates BuiltInLLMTool type to make description and inputSchema optional |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
3 issues found across 4 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="packages/runner/src/builtins/llm-dialog.ts">
<violation number="1" location="packages/runner/src/builtins/llm-dialog.ts:440">
Converting boolean false schema to {} changes semantics from "deny all" to "allow anything"; use a non-boolean schema that matches nothing instead.</violation>
<violation number="2" location="packages/runner/src/builtins/llm-dialog.ts:443">
Potential runtime error: schema may be undefined when only inputSchema is provided; use optional chaining and prefer inputSchema.description first.</violation>
</file>
<file name="packages/api/index.ts">
<violation number="1" location="packages/api/index.ts:270">
Making description optional is a breaking type change for consumers expecting it to always exist; consider keeping description required or providing a transition path.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
bfollington
left a comment
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.
LGTM but I am going to see if I can action some of the AI feedback before merging.
- coerce tools without explicit schemas into a normalized shape and warn when descriptions are missing before sending the request payload - provide an isBoolean helper for schema normalization and update the public tool types now that input schemas are guaranteed - tweak the chatbot-tools section comment to the new banner style
9c96a49 to
08918d6
Compare
Summary by cubic
Tools now auto-generate their description and input schema from JSDoc/type info, removing manual JSONSchema and string descriptions. This simplifies tool authoring and enforces a clear “pattern or handler” contract.
Refactors
Migration