-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/add vs code insiders #16
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
- Add vs-code-insiders entry to _SUPPORTED_AGENT_DATA with cross-platform paths - Configure detection directories for Linux, macOS, and Windows - Update test to handle VS Code Insiders as special case - Verify configuration is valid and discoverable via CLI Related to T1.0 in Spec 08
- Add test_vs_code_insiders_detection_multiplatform for linux/darwin/win32 - Add test_vs_code_insiders_detection_empty_when_no_directories - Add test_vs_code_insiders_get_command_dir_platform_specific - Use pytest parametrization and monkeypatch for platform simulation - All 9 new tests pass, no regressions in existing 14 tests Related to T2.0 in Spec 08
- Add vs-code-insiders to test_generate_all_supported_agents - Positioned in alphabetical order after vs-code - Integration test validates end-to-end file generation - Docker-isolated tests prevent user data modification Related to T3.0 in Spec 08
- Update README.md with VS Code Insiders platform-specific paths - Add VS Code Insiders entry to agents table in slash-command-generator.md - Document independence between VS Code and VS Code Insiders - Include platform-specific paths for Linux, macOS, and Windows - Add links to official VS Code Insiders resources Related to T4.0 in Spec 08
📝 WalkthroughWalkthroughThis PR adds VS Code Insiders as a supported agent: new configuration entry with platform-specific command/detection paths, unit and integration tests to validate detection and generation, and comprehensive documentation/specs/proofs describing the addition and verification artifacts. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
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 |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In
@docs/specs/08-spec-vs-code-insiders-support/08-tasks-vs-code-insiders-support.md:
- Line 21: Fix the concatenated text on line 21 in the doc string where
"introducedollow" appears; split and correct the sentence so it reads naturally
(for example: "introduced. Follow pytest parametrization patterns from existing
VS Code tests in `test_detection.py`"), ensuring proper spacing and punctuation
between the two clauses.
🧹 Nitpick comments (1)
docs/specs/08-spec-vs-code-insiders-support/08-spec-vs-code-insiders-support.md (1)
1-179: Excellent specification document with clear requirements and approach.The specification is comprehensive, well-organized, and provides clear guidance for implementation. It covers all necessary aspects including goals, user stories, technical considerations, testing strategy, and success metrics. The approach to mirror existing VS Code support with Insiders-specific paths is sound and follows established patterns.
Optional: Minor style improvement for user stories
The static analysis tool noted that three consecutive user stories begin with "As a..." which can be slightly repetitive. While this follows the standard user story format and is not incorrect, you could optionally vary the sentence structure for improved readability. However, maintaining consistency with user story conventions is also valuable, so this is purely optional.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
README.mddocs/slash-command-generator.mddocs/specs/08-spec-vs-code-insiders-support/08-proofs/08-task-01-proofs.mddocs/specs/08-spec-vs-code-insiders-support/08-proofs/08-task-02-proofs.mddocs/specs/08-spec-vs-code-insiders-support/08-proofs/08-task-03-proofs.mddocs/specs/08-spec-vs-code-insiders-support/08-proofs/08-task-04-proofs.mddocs/specs/08-spec-vs-code-insiders-support/08-questions-1-vs-code-insiders-support.mddocs/specs/08-spec-vs-code-insiders-support/08-spec-vs-code-insiders-support.mddocs/specs/08-spec-vs-code-insiders-support/08-tasks-vs-code-insiders-support.mddocs/specs/08-spec-vs-code-insiders-support/08-validation-vs-code-insiders-support.mdslash_commands/config.pytests/integration/test_generate_command.pytests/test_config.pytests/test_detection.py
🧰 Additional context used
📓 Path-based instructions (1)
slash_commands/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Follow existing code patterns in slash_commands/
Files:
slash_commands/config.py
🧬 Code graph analysis (1)
tests/test_detection.py (2)
slash_commands/detection.py (1)
detect_agents(11-25)slash_commands/config.py (2)
get_agent_config(177-183)get_command_dir(35-43)
🪛 LanguageTool
docs/specs/08-spec-vs-code-insiders-support/08-questions-1-vs-code-insiders-support.md
[style] ~3-~3: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...r more options, or add your own notes). Feel free to add additional context under any questi...
(FEEL_FREE_TO_STYLE_ME)
[style] ~15-~15: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ... Additional context: Following the exact same cross-platform support as regular VS Co...
(EN_WORDINESS_PREMIUM_EXACT_SAME)
docs/specs/08-spec-vs-code-insiders-support/08-spec-vs-code-insiders-support.md
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...pt configurations without conflicts. As a cross-platform user, I want VS Code...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~23-~23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e regardless of my operating system. As a CLI user, I want VS Code Insiders t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/specs/08-spec-vs-code-insiders-support/08-validation-vs-code-insiders-support.md
[style] ~164-~164: This phrase is redundant (‘OS’ stands for ‘operating system’). Use simply “macOS”.
Context: ...itioned after vs-code - ✅ Detected on macOS system (system has VS Code Insiders installed)...
(ACRONYM_TAUTOLOGY)
[uncategorized] ~236-~236: The official name of this software platform is spelled with a capital “H”.
Context: ...regressions in CLI, config, generators, github_utils, prompts, validation, version, or...
(GITHUB)
[grammar] ~327-~327: Use a hyphen to join words.
Context: ...iant (verified by passing tests) - ✅ 100 character line limit respected - ✅ Type ...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (13)
slash_commands/config.py (1)
80-96: LGTM! VS Code Insiders configuration follows the established pattern.The new agent entry correctly mirrors the VS Code configuration with adjusted paths for the Insiders variant. All required fields are present with appropriate values:
- Platform-specific detection directories for all three platforms
- Platform-specific command directories matching VS Code Insiders installation paths
- Consistent file extension (
.prompt.md) and format (CommandFormat.MARKDOWN)- Proper key naming (
vs-code-insiders) that maintains the existing naming conventiontests/integration/test_generate_command.py (1)
223-232: LGTM! VS Code Insiders correctly added to integration tests.The agent is properly added in alphabetical order (after
"vs-code") and will be validated through the existing test loop that verifies:
- Successful generation (exit code 0)
- Correct agent-specific directory creation
- File generation with proper extension
README.md (1)
195-198: LGTM! Clear and consistent documentation.The VS Code Insiders entry is well-documented with platform-specific paths that correctly mirror the VS Code structure. The formatting is consistent with other platform-specific agents in the list.
tests/test_config.py (1)
115-119: LGTM! Test follows established pattern.The test case for VS Code Insiders correctly mirrors the VS Code test pattern and validates the detection directory structure. The assertion properly checks for both the root
.configdirectory and the specific.config/Code - Insiderspath.docs/specs/08-spec-vs-code-insiders-support/08-proofs/08-task-04-proofs.md (1)
1-105: Comprehensive proof artifacts.This documentation provides thorough verification artifacts for Task 4.0, including code samples, verification checklists, and consistency reviews. The proof document clearly demonstrates that the documentation updates are complete and accurate.
docs/slash-command-generator.md (2)
198-198: LGTM! Table entry is complete and accurate.The VS Code Insiders table entry correctly follows the established format with appropriate agent key, display name, format, extension, and reference links. The placement in alphabetical order after
vs-codeis correct.
201-215: Excellent documentation of platform-specific paths and independence.The updated platform notes section clearly documents VS Code Insiders paths for all three platforms and explicitly states that VS Code and VS Code Insiders "operate independently" and "do not share configurations." This clarity is important to prevent user confusion about the relationship between the two variants.
docs/specs/08-spec-vs-code-insiders-support/08-proofs/08-task-01-proofs.md (1)
1-110: Thorough proof artifacts for configuration validation.This documentation provides comprehensive validation for Task 1.0, including the configuration code structure, CLI output demonstrating agent discoverability, and test results confirming all configuration tests pass with no regressions. The proof artifacts clearly demonstrate that the VS Code Insiders configuration follows the correct pattern with platform-specific paths.
docs/specs/08-spec-vs-code-insiders-support/08-proofs/08-task-02-proofs.md (1)
1-161: Excellent documentation of test implementation and results.The proof artifacts are thorough and well-organized. The documentation clearly shows:
- Test code implementation with proper parametrization
- Comprehensive test results (9 new tests, all passing)
- No regressions in existing tests
- Clear verification checklist
This provides strong evidence that the VS Code Insiders detection tests are properly implemented and validated.
tests/test_detection.py (3)
98-117: LGTM! Well-structured multiplatform detection test.The test correctly validates VS Code Insiders detection across all three platforms using parametrization and monkeypatch. The platform-specific directory paths align with VS Code Insiders conventions, and the test structure mirrors the established pattern from the existing VS Code tests.
119-130: LGTM! Proper negative test case for detection.The test correctly verifies that VS Code Insiders is not detected when its configuration directories don't exist. This negative test is important for ensuring accurate detection behavior and follows the established pattern from existing tests.
152-170: LGTM! Comprehensive test of platform-specific command directory resolution.The test correctly validates that
get_command_dir()returns the appropriate platform-specific path for VS Code Insiders, including the/User/promptssubdirectory. The test covers all three platforms and follows the established pattern from existing VS Code tests.docs/specs/08-spec-vs-code-insiders-support/08-proofs/08-task-03-proofs.md (1)
1-117: Thorough documentation of integration test updates.The proof artifacts clearly document the integration test changes, including:
- Addition of vs-code-insiders to the test agent list in proper alphabetical order
- Rationale for Docker isolation (preventing user data modification)
- Expected test behavior across platforms
- Comprehensive verification checklist
The documentation provides clear evidence that integration testing is properly configured for VS Code Insiders support.
docs/specs/08-spec-vs-code-insiders-support/08-tasks-vs-code-insiders-support.md
Outdated
Show resolved
Hide resolved
ryderstorm
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.
![]()
Thanks for knocking this out @m-schaller-3 !
Why?
Adds support for VS Code Insiders for agent platform detection
What Changed?
Mirrored config for vs-code in config scripts for detection
Testing
Checklist
pre-commit run --all-filesSummary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.