-
Notifications
You must be signed in to change notification settings - Fork 78
feat: Return Actor card in markdown #195
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
Replace inconsistent template literal indentation with string concatenation for all tool descriptions to ensure clean code and output text.
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 Actor search and detail tools to return markdown-formatted Actor cards instead of JSON objects, improving user-friendliness and readability. The changes standardize tool description formatting using string concatenation and introduce a new utility for creating Actor cards in markdown format.
- Replaces JSON responses with markdown Actor cards for better user experience
- Standardizes tool descriptions using string concatenation for consistency
- Creates new utility functions for formatting Actor information as markdown cards
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/actor-card.ts |
New utility file for formatting Actor information into markdown cards |
src/tools/store_collection.ts |
Updated search actors tool to return markdown cards instead of JSON |
src/tools/get-actor-details.ts |
Updated get actor details tool to return markdown cards instead of JSON |
src/tools/search-apify-docs.ts |
Standardized tool description formatting with string concatenation |
src/tools/run_collection.ts |
Standardized tool description formatting with string concatenation |
src/tools/helpers.ts |
Standardized tool description formatting with string concatenation |
src/tools/actor.ts |
Standardized tool description formatting and removed unused code |
src/const.ts |
Added Apify Store URL constant |
tests/integration/suite.ts |
Removed type import that's no longer needed |
README.md |
Updated documentation sections for MCP server setup |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
MQ37
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.
I would add back the failed number of runs stats that was in the original implementation. Otherwise LGTM 👍
| description: `Gets a paginated list of Actor runs with run details, datasetId, and keyValueStoreId. | ||
| Filter by status: READY (not allocated), RUNNING (executing), SUCCEEDED (finished), FAILED (failed), | ||
| TIMING-OUT (timing out), TIMED-OUT (timed out), ABORTING (being aborted), ABORTED (aborted).`, | ||
| description: `Gets a paginated list of Actor runs with run details, datasetId, and keyValueStoreId.\n` |
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.
why do we use the + to join the strings if we can just use single multiline ``` string?
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.
Other tools already used that. I find noindent ugly. I'm sorry. If we agree on using template literals with noindent across the codebase, I'll adapt.
So what do you say @MQ37 @MichalKalita
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.
I also like how the + looks but updating that and splitting the lines is a hassle. Let's keep that for now 👍
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.
I usually prefer ``` because of simplicity. It's easier to copy/paste content.
But there is no need to refactor all places and update them.
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.
Ok, I've created issue for that: #197
MQ37
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 👍


close: #181