docs: update skills guide to reflect catalog-driven detection engine#341
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughDocumentation and a journal entry updated to reflect that Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/journal/scribe-journal.md:
- Around line 16-17: Add a single blank line immediately after the heading "##
2025-05-15 - Catalog-driven Skill Detection" so the heading is followed by an
empty line to satisfy markdownlint rule MD022; locate the heading in
scribe-journal content and insert one newline between that heading line and the
following paragraph.
In `@website/docs/src/content/docs/guides/skills.mdx`:
- Line 140: The guide currently contradicts itself: the table row "`vue` | Vue |
`package.json` (vue)" marks Vue as detected while the later "future catalog
additions such as Vue" section states Vue is not yet detected; update one to be
consistent—either change the table entry to indicate Vue is "planned" or "not
detected" or revise the future-section text to state that Vue is now detected.
Make the change where the table row "`vue` | Vue | `package.json` (vue)" and the
paragraph titled "future catalog additions such as Vue" are defined (update
wording so both state the same detection status and, if applicable, add a short
note about when Vue support was added or expected).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b3c28fce-b075-4985-910b-ecf9f2f35d32
📒 Files selected for processing (2)
.agents/journal/scribe-journal.mdwebsite/docs/src/content/docs/guides/skills.mdx
- Add blank line after '## 2025-05-15' heading in scribe-journal.md (MD022) - Replace outdated Vue example in 'future catalog additions' section; Vue is already detected via catalog.v1.toml, so use a hypothetical solidjs example to accurately illustrate the model without contradicting the technology table
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
website/docs/src/content/docs/guides/skills.mdx (1)
295-299:⚠️ Potential issue | 🟠 MajorCorrect the “Rust change first” claim for new technology additions.
Line 295 and Line 319 currently imply new technology detection requires adding Rust
TechnologyId/detector support first. With the current detector, technologies are iterated from catalog entries and evaluated via generic rules, so adding a new technology is primarily a catalog change (Rust changes are only needed for new detection mechanisms, not a new technology id).Proposed doc fix
-Detection support for a new technology is still a Rust change first. For example, adding support for a new technology (say, `solidjs`) would typically require: +Detection support for a new technology is primarily a catalog change. For example, adding support for a new technology (say, `solidjs`) would typically require: -1. adding a new Rust `TechnologyId` and detector markers -2. adding a catalog skill definition and technology entry -3. optionally adding combo entries later +1. adding a catalog skill definition and technology entry (including `[technologies.detect]` markers) +2. ensuring recommendation mappings point to the appropriate skills +3. optionally adding combo entries later -That TOML shape is already supported by the catalog model. But the repository would not start detecting the technology until the Rust detector also learned how to report its `id`. +That TOML shape is already supported by the catalog model. Detection begins once the catalog entry is present and its markers match repository contents. Rust changes are only needed when introducing new detection rule capabilities.Also applies to: 319-319
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/src/content/docs/guides/skills.mdx` around lines 295 - 299, Update the paragraph that claims adding a new technology requires a "Rust change first" so it accurately states that detection is primarily driven by catalog entries and generic detector rules: change the text around the mentions of Rust `TechnologyId` and detector markers to explain that adding a new technology like `solidjs` typically involves adding a catalog skill definition and technology entry (and optional combo entries), while Rust changes to `TechnologyId`/detector code are only necessary when implementing a new detection mechanism rather than for every new technology id or catalog entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@website/docs/src/content/docs/guides/skills.mdx`:
- Around line 295-299: Update the paragraph that claims adding a new technology
requires a "Rust change first" so it accurately states that detection is
primarily driven by catalog entries and generic detector rules: change the text
around the mentions of Rust `TechnologyId` and detector markers to explain that
adding a new technology like `solidjs` typically involves adding a catalog skill
definition and technology entry (and optional combo entries), while Rust changes
to `TechnologyId`/detector code are only necessary when implementing a new
detection mechanism rather than for every new technology id or catalog entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 81360b1f-4f77-47d2-8291-dd9ea7527673
📒 Files selected for processing (2)
.agents/journal/scribe-journal.mdwebsite/docs/src/content/docs/guides/skills.mdx
…-accuracy-5668266863270425982
…github.com:dallay/agentsync into docs/fix-skills-guide-accuracy-5668266863270425982
|
yacosta738
deleted the
docs/fix-skills-guide-accuracy-5668266863270425982
branch
1 participant
Updated the skills documentation to align with the current implementation of the technology detection engine. The previous documentation claimed support for only 7 technologies, whereas the current catalog-driven engine (
src/skills/catalog.v1.toml) supports 73+. Also synchronized the list of ignored directories and updated the project journal with findings.PR created automatically by Jules for task 5668266863270425982 started by @yacosta738