feat(skills): hierarchical skill optimization and search command#507
feat(skills): hierarchical skill optimization and search command#507dumko2001 wants to merge 11 commits intogoogleworkspace:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 20e4c25 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
googleworkspace-bot
added
area: skills
area: docs
area: core
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the skill management system by introducing a hierarchical structure and a powerful search capability. The changes aim to optimize agent performance by reducing unnecessary context loading and improving the discoverability of specific tools. This refactoring also streamlines the skill generation process and cleans up the repository by removing redundant files. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a hierarchical structure for skills to address context pollution, along with a new gws skills search command for discovery. The changes are extensive, refactoring the skill generation logic and updating numerous skill definition files.
My review has identified a few issues:
- A critical bug in the new
check_links.pyscript that prevents it from correctly identifying most local links. - A high-severity issue in the same script where file encoding is not specified, which could lead to errors on different operating systems.
- A high-severity typo in
.gitignorethat will prevent theskills/directory from being ignored correctly.
I've provided suggestions to fix these issues. The rest of the changes, particularly the refactoring of the Rust code for skill generation and the updates to the markdown files, appear to be correct and consistent with the goals of the pull request.
…x .gitignore formatting
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed refactoring to implement a hierarchical skill structure, moving granular API skills into a references/ subdirectory to solve context pollution. It also adds a new gws skills search <query> command for skill discovery. The changes are extensive, including updates to the skill generation logic, documentation, and numerous skill files, but they appear consistent and correct. I have one high-severity suggestion regarding error handling in the new search functionality to make it more robust against silent failures.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed refactoring of the skill management system. The move to a hierarchical skill structure and on-demand generation is a great improvement for maintainability and reduces repository bloat. The new gws skills search command is a valuable addition for discoverability. The code changes are generally of high quality, with improved error handling in the generation script. I have one high-severity suggestion regarding code duplication to further improve the long-term maintainability of the new search functionality.
…in src/registry.rs
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed refactoring of the skill management system, moving to a hierarchical structure to address context pollution. The addition of the gws skills search command is a valuable enhancement for discoverability. The code changes are generally of high quality, with notable improvements in error handling within the skill generation logic and better code organization by introducing a registry module. I have identified one high-severity issue in the new search command's argument parsing that could lead to incorrect behavior for multi-word queries, and I've provided a suggestion for a fix.
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed refactoring of the skill structure to be hierarchical, which should help with agent context pollution. The addition of the gws skills search command is a great step towards improving skill discoverability. The code changes are mostly clean and the refactoring of generate_skills.rs improves modularity and error handling. I've found one high-severity issue in the new search functionality that should be addressed.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a hierarchical skill structure to address context pollution, along with a new gws skills search command for skill discovery. The changes are extensive, refactoring skill generation logic, updating all skill definitions, and adding a new link checker script. My review focuses on the new functionality and refactoring. I've found a high-severity issue in the argument parsing of the new skills search command that should be addressed.
Add explicit --help/-h handling to `handle_skills_command` so that `gws skills --help` and `gws skills search --help` print a proper help screen instead of treating the flag as a search query. Keeps the same manual dispatch pattern as all other top-level commands in main.rs rather than introducing a full clap subcommand tree for a single new command.
|
/gemini review |
Summary of all Gemini review fixesAddressed all reviewer feedback across the 4 review rounds: Round 1 (on initial commit):
Round 2 (error handling):
Round 3 (code duplication):
Round 4 (multi-word queries):
Round 5 (helper skills missing from search):
Round 6 (latest —
All 630 tests pass. |
There was a problem hiding this comment.
Code Review
This pull request introduces a hierarchical structure for skills to reduce context pollution for agents, along with a new gws skills search command for discovery. The refactoring of skills into a references subdirectory is well-executed across the markdown files and the generation script.
Per CLI convention, invoking a subcommand with no arguments should display usage/help rather than return an error. Since 'gws skills --help', 'gws skills', and 'gws skills <unknown-subcommand>' all indicate the user wants guidance, route all non-search invocations to print_skills_help() and return Ok(()).
|
/gemini review |
Replace single-string substring match with a token-AND approach: split the query into individual tokens and require all tokens to appear somewhere in the combined fields (name + description + aliases). Previously, `gws skills search send email` built the query string "send email" and required that exact contiguous phrase to appear in a field. This failed for descriptions like "Gmail: Send an email" where the words are present but not adjacent. With token-AND matching, both tokens must match anywhere across the combined text, which is the correct behavior for natural-language queries.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a hierarchical skill structure to solve context pollution for AI agents, and adds a gws skills search command for discovery. The changes are extensive, refactoring skill generation, updating all skill definitions, and implementing the new search functionality. The implementation is solid and consistently applied across the codebase. The addition of a link checker script is a good step towards maintaining documentation quality. I've identified one high-severity issue in this new script where it can fail to parse markdown links that include a title, potentially causing it to miss broken links.
There was a problem hiding this comment.
Code Review
This pull request introduces a hierarchical skill structure to optimize agent performance and adds a new gws skills search command for discovery. The changes are well-executed, including refactoring the skill generation logic, updating all markdown links, and improving error handling. My main feedback concerns a performance and architectural issue in the new skill search and generation logic. The current method for discovering helper commands is inefficient as it dynamically builds command structures at runtime. I've suggested creating a static registry for helpers to improve performance and maintainability.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and valuable improvement by restructuring the skills into a hierarchical system and adding a new gws skills search command for discovery. This effectively addresses the context-pollution problem. The refactoring of the skill generation logic in generate_skills.rs is well-executed, and the introduction of the registry module improves code organization. The changes are consistent and the new search functionality is a great addition. I have one high-severity suggestion regarding adding tests for the new search functionality to ensure its long-term maintainability and correctness.
- check_links.py: strip optional title from markdown link paths so links like `[text](path "title")` resolve correctly - registry.rs: add HelperMeta struct and HELPERS static const as single source of truth for helper command metadata - skills_search.rs: replace runtime clap::Command construction with HELPERS registry iteration; extract token_matches() for unit testing; add 7 unit tests covering single/multi-token, case, cross-field, no-match, and empty-token cases - generate_skills.rs: replace build_cli() helper-filter loop with HELPERS registry iteration, eliminating redundant CLI tree construction
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces hierarchical skill optimization and a new search command to improve the gws CLI. The changes include restructuring the skills directory, adding a gws skills search command, enhancing gws-shared, automating artifact generation, and cleaning up committed Markdown files. The review focuses on ensuring the new structure and search functionality work as intended, and that the code adheres to best practices.
| let service_md = | ||
| render_service_skill(alias, entry, &helpers, &resources, &product_name, &doc); | ||
| write_skill(output_path, &skill_name, &service_md)?; | ||
| write_skill(output_path, &skill_name, &service_md, true)?; |
There was a problem hiding this comment.
high
| write_skill(output_path, &skill_name, &service_md, true)?; | |
| write_skill(output_path, &skill_name, &service_md, false); |
There was a problem hiding this comment.
Declining this suggestion — the true flag is intentional and correct. write_skill(..., true) routes output to skills/references/{name}/SKILL.md, which is the core feature of this PR (hierarchical structure). Changing it to false would write service skills back into skills/{name}/SKILL.md, reverting the hierarchical layout entirely. The ? is also kept to propagate I/O errors rather than silently swallowing them.
| let helper_md = | ||
| render_helper_skill(alias, helper_name, helper, entry, &product_name); | ||
| write_skill(output_path, &helper_skill_name, &helper_md)?; | ||
| write_skill(output_path, &helper_skill_name, &helper_md, true)?; |
There was a problem hiding this comment.
high
| write_skill(output_path, &helper_skill_name, &helper_md, true)?; | |
| write_skill(output_path, &helper_skill_name, &helper_md, false); |
There was a problem hiding this comment.
Same as above — declining this suggestion. Helper skills are also part of the hierarchical structure and must go into skills/references/gws-{service}-{helper}/SKILL.md. Using is_reference: false here would break that. The ? stays to propagate errors correctly.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a hierarchical structure for skills, moving service and helper skills into a references/ subdirectory, and implements a gws skills search command for discovery. Key changes include refactoring the skill generation logic in src/generate_skills.rs to support the new structure and filtering, updating skill documentation and metadata paths across numerous .md files, and adding new modules (src/registry.rs, src/skills_search.rs) for centralized skill metadata and search functionality. A check_links.py script was also added for link validation. The review comments point out that the persona and recipe skills' metadata and prerequisite text still incorrectly reference the old skill paths and need to be updated to reflect the new references directory.
| .services | ||
| .iter() | ||
| .map(|s| format!("\"gws-{s}\"")) | ||
| .map(|s| format!("\"references/gws-{s}\"")) |
There was a problem hiding this comment.
high: The persona skills should reference the skills in the references directory.
| .map(|s| format!("\"references/gws-{s}\"")) | |
| .map(|s| format!("\"references/gws-{s}\"" |
There was a problem hiding this comment.
This is already implemented — the current code at line 778 already reads .map(|s| format\!("\"references/gws-{s}\"")). The diff you're reviewing shows the old line (gws-{s}) removed and the new line (references/gws-{s}) added as part of this PR.
| .services | ||
| .iter() | ||
| .map(|s| format!("\"gws-{s}\"")) | ||
| .map(|s| format!("\"references/gws-{s}\"")) |
There was a problem hiding this comment.
high: The recipe skills should reference the skills in the references directory.
| .map(|s| format!("\"references/gws-{s}\"")) | |
| .map(|s| format!("\"references/gws-{s}\"" |
There was a problem hiding this comment.
Same as above — already implemented. Line 848 already reads .map(|s| format\!("\"references/gws-{s}\"")). The suggestion matches what this PR already puts in place.
2 participants
Description
This PR implements hierarchical skill optimization to solve the context-pollution problem raised in #82. Instead of loading all 40+ API service skills at once, we now use a "Discovery-First" architecture inspired by Claude and GitHub Copilot.
Key changes:
skills/directory, while granular technical references for individual services/helpers are moved toskills/references/.gws skills search <query>: A new command for semantic/keyword discovery of tools. This allows agents to find the right skill without pre-loading the entire library.gws-shared: Restored all safety rails (zsh expansion, JSON quoting) and added a "Discovery & Search" section.generate-skillsto handle the new hierarchy and update all internal Markdown cross-references automatically.Dry Run Output:
(Since
gws skills searchis a local discovery tool, it does not produce a JSON API request. Here is the command output instead):Checklist:
AGENTS.mdguidelines (no generatedgoogle-*crates).cargo fmt --allto format the code perfectly.cargo clippy -- -D warningsand resolved all warnings.pnpx changeset) to document my changes.