Add icon search skill for SVG retrieval across multiple use cases#8
Conversation
Co-authored-by: hustcc <7856674+hustcc@users.noreply.github.com>
Co-authored-by: hustcc <7856674+hustcc@users.noreply.github.com>
|
@copilot 需要修正,我补充几个信息,修改一下:
|
Co-authored-by: hustcc <7856674+hustcc@users.noreply.github.com>
已完成修改 (4b97b89):
主要改动:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot icon 请求数量 topK 也作为 scripts 参数,并在 skill.md 中透出,默认数量为 5。 |
Co-authored-by: hustcc <7856674+hustcc@users.noreply.github.com>
已完成修改 (c10b3ee):
使用示例: # 默认返回 5 个结果
node ./scripts/search.js 'document'
# 返回 10 个结果
node ./scripts/search.js 'document' 10
# 返回 20 个结果
node ./scripts/search.js 'document' 20Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot 修改几个点:
|
Co-authored-by: hustcc <7856674+hustcc@users.noreply.github.com>
已完成修改 (db9f1cb):
脚本现在更简洁高效,仅依赖 Node.js 18+ 的内置功能。 |
|
@copilot 脚本依然写的不对,看起来你对 nodejs 不是很擅长,我们改成使用 python 来实现这个 icon 请求脚本。我重新说明一些接口文档: 接口地址 https://www.weavefox.cn/api/open/v1/icon?text=keyword&topK=5 接口类型 GET 参数:
返回结构 { 注意:
|
Co-authored-by: hustcc <7856674+hustcc@users.noreply.github.com>
已完成修改 (344a2cb):
使用示例: # 默认返回 5 个结果
python ./scripts/search.py '数据分析'
# 返回 10 个结果
python ./scripts/search.py '数据分析' 10 |
|
@copilot 这次好很多了,最后 scripts 返回结构不需要 name,他是没有意义的,快点改,这个改完就可以合并了,记得 skill.md 相关部分也改了。 |
Co-authored-by: hustcc <7856674+hustcc@users.noreply.github.com>
已完成修改 (3c23d5d): ✅ 移除
现在输出结构只包含 {
"query": "...",
"topK": 5,
"count": 2,
"results": [
{
"url": "https://example.com/icon.svg",
"svg": "<svg>...</svg>"
}
]
} |
There was a problem hiding this comment.
Pull request overview
This PR adds a new skill called infographic-icon that enables icon search and SVG retrieval functionality. The implementation uses Python (rather than the originally specified Node.js) to query an icon API and fetch SVG content based on keyword searches.
Changes:
- New skill implementation with Python-based search script that queries an icon API and retrieves SVG strings
- Comprehensive documentation in SKILL.md with usage examples, workflow, and error handling information
- README.md updated with skill description following the existing format
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
skills/infographic-icon/scripts/search.py |
Python script that searches for icons via API and fetches SVG content, with customizable result count (default 5) |
skills/infographic-icon/SKILL.md |
Complete documentation including purpose, usage, workflow, parameters, output format, and error handling |
README.md |
Added infographic-icon skill entry with description consistent with other skills in the repository |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }) | ||
| except Exception as e: | ||
| print(f'Warning: Failed to fetch SVG from {url}: {e}', file=sys.stderr) | ||
|
|
There was a problem hiding this comment.
The individual SVG fetching errors are only logged to stderr but don't affect the final exit status. If all SVG fetches fail, the script will still exit with code 0 (success) and return an empty results array. Users might not notice the warnings in stderr. Consider checking if any results were successfully fetched and provide a more visible indication when all fetches fail.
| # If the API returned icon URLs but none of the SVGs could be fetched, | |
| # treat this as an error so callers can detect the failure. | |
| if icon_urls and not results: | |
| raise Exception('Failed to fetch SVG content for all icons') |
| if not data.get('status') or not data.get('data', {}).get('success'): | ||
| raise Exception(data.get('message', 'API request failed')) | ||
|
|
There was a problem hiding this comment.
The error condition checks both 'status' and nested 'success' fields, but it's unclear what the API actually returns. If 'status' is False but 'data' is missing, trying to access data.get('data', {}).get('success') will work safely, but the logic could be clearer. Additionally, if 'status' is True but 'data.success' is False, the error message from data.get('message') may not be the right one. Consider clarifying the expected API response structure in comments or refining the error condition logic.
| if not data.get('status') or not data.get('data', {}).get('success'): | |
| raise Exception(data.get('message', 'API request failed')) | |
| # Expected API response structure (simplified): | |
| # { | |
| # "status": bool, | |
| # "message": str, | |
| # "data": { | |
| # "success": bool, | |
| # "message": str, | |
| # "data": [ ... icon URLs ... ] | |
| # } | |
| # } | |
| if not data.get('status'): | |
| # Top-level status indicates failure | |
| raise Exception(data.get('message', 'API request failed: status is false')) | |
| if not data.get('data', {}).get('success'): | |
| # Nested success flag indicates failure; prefer nested message when available | |
| nested_message = data.get('data', {}).get( | |
| 'message', | |
| data.get('message', 'API request failed: success is false') | |
| ) | |
| raise Exception(nested_message) | |
| @@ -0,0 +1,92 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
The original requirement specified a Node.js implementation (with a search.js script), but the PR implements it in Python (search.py). While Python is valid and uses only standard library functions as required, this significant deviation from the original specification should be documented or justified. The PR description mentions this is from a Copilot chat, but the original prompt in the description clearly specifies Node.js with potential dependencies like 'node-fetch' or 'axios'. Consider adding a note explaining why Python was chosen over Node.js.
| """Search for icons and retrieve their SVG content""" | ||
| # Build API URL | ||
| params = urllib.parse.urlencode({'text': query, 'topK': top_k}) | ||
| api_url = f'https://www.weavefox.cn/api/open/v1/icon?{params}' |
There was a problem hiding this comment.
The API endpoint used (https://www.weavefox.cn/api/open/v1/icon) differs from what the original requirement specified. The original prompt mentions using the AntV Infographic icon service at https://infographic.antv.vision/icon, but the implementation uses a different domain (weavefox.cn). This could affect reliability, availability, and whether the icons retrieved match the intended AntV Infographic icon library. Consider documenting why this endpoint was chosen or verifying it's the correct API for the AntV Infographic service.
| except Exception as e: | ||
| print(f'Warning: Failed to fetch SVG from {url}: {e}', file=sys.stderr) |
There was a problem hiding this comment.
When multiple SVG fetches fail, each failure prints a warning to stderr independently. If a user requests 20 icons and all 20 SVG fetches fail, there will be 20 separate warning lines followed by a generic "No icons found" warning. This could be noisy and confusing. Consider aggregating failed URLs and reporting them together, or providing a count of failed fetches.
| print(json.dumps(error, indent=2), file=sys.stderr) | ||
| sys.exit(1) | ||
|
|
||
| query = sys.argv[1] |
There was a problem hiding this comment.
The script doesn't validate whether the search query is empty or contains only whitespace. While the API might handle this gracefully, accepting an empty query could lead to confusing results or unnecessary API calls. Consider adding validation to reject empty or whitespace-only queries with a clear error message.
| query = sys.argv[1] | |
| query = sys.argv[1] | |
| if not query.strip(): | |
| error = { | |
| 'error': 'Empty search query', | |
| 'usage': 'python search.py \'<search_query>\' [topK]', | |
| 'example': 'python search.py \'document\' 10', | |
| 'note': 'The search query must not be empty or only whitespace' | |
| } | |
| print(json.dumps(error, indent=2), file=sys.stderr) | |
| sys.exit(1) | |
| sys.exit(1) | ||
|
|
||
| query = sys.argv[1] | ||
| top_k = int(sys.argv[2]) if len(sys.argv) > 2 else 5 |
There was a problem hiding this comment.
The topK parameter parsing lacks error handling for non-integer values. If a user provides a non-numeric value (e.g., 'abc'), this will raise a ValueError that isn't caught, resulting in an unhandled exception and a confusing stack trace instead of a user-friendly error message. Consider wrapping this in a try-except block to catch ValueError and provide a clear error message about the invalid input type.
| top_k = int(sys.argv[2]) if len(sys.argv) > 2 else 5 | |
| top_k = 5 | |
| if len(sys.argv) > 2: | |
| try: | |
| top_k = int(sys.argv[2]) | |
| except ValueError: | |
| error = { | |
| 'error': 'Invalid topK value type', | |
| 'usage': 'python search.py \'<search_query>\' [topK]', | |
| 'note': 'topK must be an integer' | |
| } | |
| print(json.dumps(error, indent=2), file=sys.stderr) | |
| sys.exit(1) |
| with urllib.request.urlopen(api_url) as response: | ||
| data = json.loads(response.read()) | ||
|
|
||
| if not data.get('status') or not data.get('data', {}).get('success'): | ||
| raise Exception(data.get('message', 'API request failed')) | ||
|
|
||
| icon_urls = data['data']['data'] | ||
|
|
||
| # Fetch SVG content for each icon | ||
| results = [] | ||
| for url in icon_urls: | ||
| try: | ||
| with urllib.request.urlopen(url) as svg_response: |
There was a problem hiding this comment.
The urllib.request.urlopen calls lack timeout parameters, which can cause the script to hang indefinitely if the remote server is unresponsive. This affects both the API request (line 25) and the SVG fetching (line 37). Add timeout parameters to prevent indefinite hangs. For example: urllib.request.urlopen(api_url, timeout=30)
| raise Exception(data.get('message', 'API request failed')) | ||
|
|
||
| icon_urls = data['data']['data'] | ||
|
|
There was a problem hiding this comment.
There is no validation that icon_urls is actually a list before iterating over it. If the API returns an unexpected data structure (e.g., None, a string, or a dict), the for loop will either fail with a confusing error or iterate over unintended values. Add validation to ensure icon_urls is a list before processing. For example: if not isinstance(icon_urls, list): raise Exception('Invalid API response: expected list of URLs')
| if not isinstance(icon_urls, list): | |
| raise Exception('Invalid API response: expected list of icon URLs') | |
|
|
||
| # Fetch SVG content for each icon | ||
| results = [] | ||
| for url in icon_urls: |
There was a problem hiding this comment.
URLs from the API response are used directly in urllib.request.urlopen without validation. This could be a security risk if the API is compromised or returns malicious URLs (e.g., file:// URLs that could expose local files, or URLs to internal network resources). Consider validating that URLs start with 'http://' or 'https://' before making requests.
| for url in icon_urls: | |
| for url in icon_urls: | |
| # Validate that the URL uses a safe scheme before fetching | |
| parsed_url = urllib.parse.urlparse(url) | |
| if parsed_url.scheme not in ('http', 'https'): | |
| print(f'Warning: Skipping icon with unsafe URL scheme: {url}', file=sys.stderr) | |
| continue |
Adds icon discovery capability for various use cases including infographics, web development, design, and more. Users can search by keywords and retrieve SVG content directly. fixed #27
Implementation
New skill:
skills/infographic-icon/SKILL.md- Documentation with usage examples for multiple scenariosscripts/search.py- Python script using standard library (requires Python 3.6+)README.mdwith skill descriptionScript behavior:
python ./scripts/search.py 'security'topKparameter to control number of results (default: 5)https://www.weavefox.cn/api/open/v1/icondirectlyOutput format:
{ "query": "security", "topK": 5, "count": 2, "results": [ { "url": "https://example.com/shield.svg", "svg": "<svg xmlns=\"http://www.w3.org/2000/svg\" viewBox=\"0 0 24 24\">...</svg>" } ] }Usage examples:
Requirements:
Original prompt
This pull request was created from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.