Check vscode extension sync #257
Conversation
WalkthroughA new Python script was added to check synchronization between VSCode recommended extensions and the devcontainer configuration. The script is integrated as a pre-commit hook and as a Justfile recipe. The devcontainer configuration file was also introduced, and pre-commit hook versions were updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Pre-commit Hook
participant check_extension_sync.py
participant .vscode/extensions.json
participant .devcontainer/devcontainer.json
User->>Pre-commit Hook: Initiate commit
Pre-commit Hook->>check_extension_sync.py: Run script
check_extension_sync.py->>.vscode/extensions.json: Read recommended extensions
check_extension_sync.py->>.devcontainer/devcontainer.json: Read devcontainer extensions
check_extension_sync.py->>Pre-commit Hook: Report result (success or error)
Pre-commit Hook->>User: Allow or block commit based on result
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 664b7f1 in 1 minute and 54 seconds. Click for details.
- Reviewed
444lines of code in16files - Skipped
1files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/ci.yaml:2
- Draft comment:
Explicitly setting 'permissions: contents: write' is a good security measure. Ensure this is necessary for the workflow steps that modify repository data. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. .pre-commit-config.yaml:57
- Draft comment:
The new hook 'check-vscode-extension-sync' uses language 'system' with types set to [json]. Since the script doesn’t actually process JSON input from stdin, consider if specifying file types is needed or if a broader match might be more appropriate. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. Justfile:640
- Draft comment:
The PR description mentions adding a 'check-extension-sync' recipe to the Justfile, but this recipe appears to be missing. Consider adding a recipe that executes 'uvx scripts/check_extension_sync.py' for consistency with documentation. - Reason this comment was not posted:
Comment was on unchanged code.
4. scripts/check_extension_sync.py:38
- Draft comment:
The extension sync check correctly computes the missing extensions. Optionally, consider testing behavior when the devcontainer file is malformed or missing expected keys. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. scripts/check_license.py:39
- Draft comment:
The license check script uses xargs to pass a possibly large list of files to addlicense. Ensure that the command line length is not exceeded on some platforms. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
6. tests/test_config.py:4
- Draft comment:
Typographical: Consider capitalizing the start of the sentence on line 4 ('you may not use this file...') to match the style used in the rest of the header. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The Apache 2.0 license has standard, official text that should not be modified. The comment is actually suggesting to make an incorrect change that would deviate from the official license text. This would be harmful to follow. The current lowercase version is correct. Could there be a house style guide that requires all sentences to be capitalized, even in licenses? Could consistency within the file be more important than matching the official license? No - license text should never be modified from the official version, as that could affect its legal validity. House style guides don't override legal documents. Delete this comment as it suggests an incorrect change that would modify standard license text that should remain unchanged.
Workflow ID: wflow_lWeO7m2XiFUxLsrZ
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (9)
scripts/check_license.py (1)
39-42: Improve subprocess security and code formatting.The current implementation has several minor issues:
- It uses a partial executable path for
addlicense- Line 40 exceeds the 88 character limit
-result = subprocess.run( - ["addlicense", "-check", "-c", "Steve Morin", "-l", "mit", "-y", "2025", "-s", "-v", *files] -) +# Get full path to addlicense binary +import shutil +addlicense_path = shutil.which("addlicense") +if not addlicense_path: + print("Error: addlicense tool not found in PATH", file=sys.stderr) + sys.exit(1) + +result = subprocess.run([ + addlicense_path, "-check", "-c", "Steve Morin", "-l", "mit", + "-y", "2025", "-s", "-v", *files +])🧰 Tools
🪛 Ruff (0.11.9)
39-39:
subprocesscall: check for execution of untrusted input(S603)
40-40: Starting a process with a partial executable path
(S607)
40-40: Line too long (96 > 88)
(E501)
Justfile (2)
640-645: Consider using targets in the find commandsThe
license-targetsvariable isn't used in the find commands, which instead use a hardcoded list of file types.Consider restructuring to use the
license-targetsvariable in the find commands, which would make the script more maintainable and consistent.
648-663: Avoid duplication in file finding logicBoth recipes duplicate the exact same find command. Consider extracting this into a separate recipe or variable to reduce duplication.
You could create a recipe for generating the file list that both check-license and fix-license would call, or use a variable to store the find command.
scripts/check_extension_sync.py (6)
1-7: Add script docstring with usage informationWhile the script is well-structured, adding a docstring at the top would provide immediate clarity about its purpose and usage.
Add a docstring at the top of the script:
#!/usr/bin/env python3 +""" +Validates that VSCode recommended extensions are configured in devcontainer.json. + +This script checks that all extensions recommended in .vscode/extensions.json +are also configured in .devcontainer/devcontainer.json. It exits with: + - 0: All recommended extensions are configured in the devcontainer + - 1: Some recommended extensions are missing from the devcontainer + - 2: Error reading or parsing configuration files +""" + import json import sys import os from pathlib import Path
6-6: Remove unused importThe
osmodule is imported but never used in the script.import json import sys -import os from pathlib import Path
25-30: Add more detailed error message for missing devcontainer configurationThe current implementation silently returns an empty set when the devcontainer configuration is missing the necessary keys. A warning would be helpful for troubleshooting.
def extract_devcontainer_extensions(data): try: return set(data["customizations"]["vscode"]["extensions"]) except KeyError: + print("⚠️ Warning: Missing or incomplete 'customizations.vscode.extensions' in devcontainer.json") return set()
44-44: Line too longThe static analysis tool flagged this line as exceeding the recommended length (132 > 88 characters).
While the line content is simple enough that line length isn't a critical issue, you might consider breaking the f-string into multiple parts for consistency with code style guidelines:
- print(f"To fix: Add these extensions to the `customizations.vscode.extensions` array in `.devcontainer/devcontainer.json`") + print("To fix: Add these extensions to the `customizations.vscode.extensions` array " + "in `.devcontainer/devcontainer.json`")🧰 Tools
🪛 Ruff (0.11.9)
44-44: Line too long (132 > 88)
(E501)
38-47: Consider providing copy-pastable JSON array for easier fixesWhen extensions are missing, it would be helpful to print them in a format that can be directly copied into the devcontainer JSON file.
if missing: print("❌ VSCode extensions not synced with devcontainer configuration!\n") print("Missing from .devcontainer/devcontainer.json:") for ext in sorted(missing): print(f"- {ext}") - print("\nTo fix: Add these extensions to the `customizations.vscode.extensions` array in `.devcontainer/devcontainer.json`") + print("\nTo fix: Add these extensions to the `customizations.vscode.extensions` array in `.devcontainer/devcontainer.json`") + print("\nCopy-paste friendly format:") + print(json.dumps(sorted(list(missing)), indent=2)) sys.exit(1)🧰 Tools
🪛 Ruff (0.11.9)
44-44: Line too long (132 > 88)
(E501)
31-49: Consider checking for extensions in devcontainer but not recommendedThe script currently checks for VSCode extensions missing from the devcontainer, but not the reverse. Consider checking both directions to ensure full synchronization.
def main(): vscode_data = load_json(EXTENSIONS_FILE) devcontainer_data = load_json(DEVCONTAINER_FILE) vscode_exts = extract_recommended_extensions(vscode_data) dev_exts = extract_devcontainer_extensions(devcontainer_data) missing = vscode_exts - dev_exts + extra = dev_exts - vscode_exts + has_issues = False if missing: + has_issues = True print("❌ VSCode extensions not synced with devcontainer configuration!\n") print("Missing from .devcontainer/devcontainer.json:") for ext in sorted(missing): print(f"- {ext}") print("\nTo fix: Add these extensions to the `customizations.vscode.extensions` array in `.devcontainer/devcontainer.json`") - sys.exit(1) + + if extra: + has_issues = True + print("\n⚠️ Extensions in devcontainer but not recommended in VSCode:") + for ext in sorted(extra): + print(f"- {ext}") + print("\nConsider adding these to .vscode/extensions.json if they should be recommended.") + + if has_issues: + sys.exit(1) print("✅ VSCode extensions are in sync.") sys.exit(0)🧰 Tools
🪛 Ruff (0.11.9)
44-44: Line too long (132 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (17)
.github/workflows/ci.yaml(3 hunks).gitignore(1 hunks).gitmodules(1 hunks).licenseignore(1 hunks).pre-commit-config.yaml(1 hunks)Justfile(1 hunks)addlicense(1 hunks)py_launch_blueprint/__init__.py(1 hunks)py_launch_blueprint/_version.py(1 hunks)py_launch_blueprint/projects.py(1 hunks)scripts/check_extension_sync.py(1 hunks)scripts/check_license.py(1 hunks)tests/__init__.py(1 hunks)tests/test_api.py(1 hunks)tests/test_cli.py(1 hunks)tests/test_config.py(1 hunks)update_contributors.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
scripts/check_license.py
39-39: subprocess call: check for execution of untrusted input
(S603)
40-40: Starting a process with a partial executable path
(S607)
40-40: Line too long (96 > 88)
(E501)
scripts/check_extension_sync.py
44-44: Line too long (132 > 88)
(E501)
🔇 Additional comments (18)
addlicense (1)
1-1: Approve submodule bump foraddlicense
Theaddlicensesubproject has been updated to commit499ed7f28389eb4a08c2d7e40b1637cfd7f65381to support the new license compliance checks. Looks correct and necessary for the CI enhancements.tests/test_cli.py (1)
1-16: License header insertion is correct.The Apache License 2.0 header and SPDX identifier have been properly added, matching the project's standard across other modules. No issues found.
py_launch_blueprint/__init__.py (1)
1-15: License header insertion is correct.The file now includes the standard Apache License 2.0 header with the SPDX identifier, consistent with repository conventions.
tests/__init__.py (1)
1-15: License header insertion is correct.The Apache License 2.0 header and SPDX identifier have been added consistently with other test modules.
tests/test_config.py (1)
1-16: License header insertion is correct.Standard Apache License 2.0 header and SPDX identifier are present and align with the project's license compliance requirements.
.gitmodules (1)
1-4: Addlicense submodule configuration is correct.The new Git submodule entry for
addlicenseis properly defined with the correct path and URL, enabling license header tooling integration.py_launch_blueprint/projects.py (1)
2-17: Add Apache License 2.0 header
The standard Apache License 2.0 header, including the SPDX identifier, has been correctly inserted above the module docstring and below the shebang. This aligns with the project’s license compliance framework.py_launch_blueprint/_version.py (1)
1-16: Add Apache License 2.0 header
The file now includes the full Apache License 2.0 header with copyright attribution and SPDX identifier. No functional code was altered.tests/test_api.py (1)
1-16: Add Apache License 2.0 header
A consistent Apache License 2.0 header has been added to the top of the test file. This matches the other test modules and supports the automated license checks..gitignore (1)
25-25: Ignorefilelist.txt
Addingfilelist.txtto.gitignoreprevents intermediate files generated by the license tooling from being committed. This is correct and aligns with the new Justfile recipes.update_contributors.py (1)
2-16: LGTM! Proper license header added.The Apache License 2.0 header is correctly formatted and includes all required components: copyright notice, license terms, and SPDX identifier.
scripts/check_license.py (2)
28-34: LGTM! Good file filtering approach.The function correctly identifies files to process based on extensions and excludes appropriate directories.
35-37: LGTM! Early exit if no files to process.Good optimization to exit early if no matching files are found.
.pre-commit-config.yaml (2)
50-57: LGTM! License check hook properly configured.The local hook correctly runs the license checking script and targets appropriate file types.
57-64: LGTM! VSCode extension sync check properly configured.The hook correctly uses
uvxto run the extension sync script and targets JSON files, which aligns with the PR objective of ensuring synchronization between VSCode recommended extensions and devcontainer configuration..github/workflows/ci.yaml (3)
2-3: Verify if write permissions are necessary.The workflow has been granted repository write permissions (
contents: write), which may be broader than needed for CI checks.Does the workflow need to modify repository contents? If not, consider using more restrictive permissions like
contents: read.
20-21: LGTM! Full history checkout enabled.Setting
fetch-depth: 0ensures the complete Git history is available, which is good for accurate contributor information.
69-98: LGTM! Well-structured license compliance job.The license check job properly:
- Sets up Go and installs the addlicense tool
- Dynamically determines the current year
- Checks appropriate file types
- Excludes irrelevant directories
Nice use of dynamic year calculation instead of hardcoding, which improves maintainability.
Tatianah3
force-pushed
the
feature/check-vscode-extension-sync
branch
from
664b7f1 to
e22990c
Compare
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed e22990c in 1 minute and 37 seconds. Click for details.
- Reviewed
145lines of code in3files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .pre-commit-config.yaml:59
- Draft comment:
The local hook for 'check-vscode-extension-sync' uses 'uvx'. Ensure that this command is installed and available in the system PATH in all developer environments, as 'system' language hooks assume pre-installed dependencies. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to ensure that a command is installed and available in the system PATH. This falls under the category of asking the author to ensure something is set up correctly, which violates the rules.
Workflow ID: wflow_wINXfveEP9aDeOXQ
You can customize
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 88ae927 in 1 minute and 47 seconds. Click for details.
- Reviewed
60lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .pre-commit-config.yaml:52
- Draft comment:
Removed license-check hook. Confirm that its removal is intentional and that the 'types: [json]' specification suits the extension sync check. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment asks for confirmation about intentional removal - this violates our rule about not asking for confirmations or intentions. For the types specification, this is a new addition and could be wrong, but we don't have enough context to know if json is the right type for checking VSCode extensions. The types: [json] specification could be genuinely wrong and cause the hook to miss files it should check. Maybe we should keep the comment just for that part? While the types specification could be wrong, we don't have strong evidence that it is. The author likely tested this locally, and if it's wrong, it would be immediately apparent when the hook fails to run on the right files. Delete the comment. It asks for confirmation of intention (which we shouldn't do) and speculates about potential issues without strong evidence.
2. Justfile:303
- Draft comment:
For non-Windows, the CONTRIBUTORS.md update command only prints a TODO placeholder. Consider implementing or clearly documenting the required command for macOS/Linux. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_Z8riYSfDBn4TMqfp
You can customize
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
scripts/check_extension_sync.py (1)
25-29: Fix extraction function to handle extension objects with version specs.The function assumes extensions are always strings, but they can also be objects with version specifications according to the devcontainer spec.
-def extract_devcontainer_extensions(data): +def extract_devcontainer_extensions(data: Dict[str, Any]) -> Set[str]: try: - return set(data["customizations"]["vscode"]["extensions"]) + return set(e if isinstance(e, str) else e["id"] for e in data["customizations"]["vscode"]["extensions"]) except KeyError: return set()🧰 Tools
🪛 GitHub Actions: CI/CD
[error] 25-25: mypy: Function is missing a type annotation. [no-untyped-def]
[error] check-vscode-extension-sync: Running the script directly is not supported by 'uvx'. Use 'uv run scripts/check_extension_sync.py' instead.
🧹 Nitpick comments (4)
scripts/check_extension_sync.py (4)
44-44: Fix line length to comply with style guide.Line 44 exceeds the maximum line length of 88 characters as flagged by ruff.
- for ext in sorted(missing): - print(f"- {ext}") + for ext in sorted(missing): + print(f"- {ext}")Since the line doesn't actually exceed 88 characters in the provided code snippet, this seems to be an incorrect linting report. However, if the line is indeed too long in the actual codebase, please consider wrapping it or shortening it.
🧰 Tools
🪛 Ruff (0.11.9)
44-44: Line too long (132 > 88)
(E501)
🪛 GitHub Actions: CI/CD
[error] 44-44: ruff: Line too long (132 > 88) [E501]
[error] check-vscode-extension-sync: Running the script directly is not supported by 'uvx'. Use 'uv run scripts/check_extension_sync.py' instead.
1-1: Add uvx compatibility comment to the shebang line.The pipeline error indicates that running the script directly is not supported by 'uvx'. Add a comment to clarify the proper execution method.
#!/usr/bin/env python3 +# Note: When using uv, run with: uv run scripts/check_extension_sync.py🧰 Tools
🪛 GitHub Actions: CI/CD
[error] check-vscode-extension-sync: Running the script directly is not supported by 'uvx'. Use 'uv run scripts/check_extension_sync.py' instead.
38-46: Consider checking for extensions added to devcontainer but not in VSCode recommendations.The script only checks for extensions missing from the devcontainer but doesn't verify if there are extraneous extensions in the devcontainer that aren't in the VSCode recommendations.
missing = vscode_exts - dev_exts + extra = dev_exts - vscode_exts if missing: print("❌ VSCode extensions not synced with devcontainer configuration!\n") print("Missing from .devcontainer/devcontainer.json:") for ext in sorted(missing): print(f"- {ext}") print("\nTo fix: Add these extensions to the `customizations.vscode.extensions` array in `.devcontainer/devcontainer.json`") sys.exit(1) + + if extra: + print("⚠️ Found extensions in devcontainer that are not in VSCode recommendations:\n") + for ext in sorted(extra): + print(f"- {ext}") + print("\nConsider adding these to .vscode/extensions.json if they should be recommended.") + # Optional: Uncomment to make this a failure case as well + # sys.exit(1)This addition would provide a more comprehensive check by ensuring both files are truly in sync, not just that the devcontainer contains all VSCode recommendations.
🧰 Tools
🪛 Ruff (0.11.9)
44-44: Line too long (132 > 88)
(E501)
🪛 GitHub Actions: CI/CD
[error] 44-44: ruff: Line too long (132 > 88) [E501]
[error] check-vscode-extension-sync: Running the script directly is not supported by 'uvx'. Use 'uv run scripts/check_extension_sync.py' instead.
11-14: Use context manager pattern consistently for file handling.The current implementation uses a context manager for file opening but doesn't specify encoding explicitly. For better compatibility across platforms, consider specifying the encoding.
def load_json(file_path): try: - with open(file_path) as f: + with open(file_path, encoding="utf-8") as f: return json.load(f)🧰 Tools
🪛 GitHub Actions: CI/CD
[error] 11-11: mypy: Function is missing a type annotation. [no-untyped-def]
[error] check-vscode-extension-sync: Running the script directly is not supported by 'uvx'. Use 'uv run scripts/check_extension_sync.py' instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
.github/workflows/ci.yaml(3 hunks).pre-commit-config.yaml(1 hunks)Justfile(2 hunks)scripts/check_extension_sync.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .pre-commit-config.yaml
- Justfile
- .github/workflows/ci.yaml
🧰 Additional context used
🪛 Ruff (0.11.9)
scripts/check_extension_sync.py
44-44: Line too long (132 > 88)
(E501)
🪛 GitHub Actions: CI/CD
scripts/check_extension_sync.py
[error] 11-11: mypy: Function is missing a type annotation. [no-untyped-def]
[error] 22-22: mypy: Function is missing a type annotation. [no-untyped-def]
[error] 25-25: mypy: Function is missing a type annotation. [no-untyped-def]
[error] 31-31: mypy: Function is missing a return type annotation. Use '-> None' if function does not return a value. [no-untyped-def]
[error] 32-32: mypy: Call to untyped function 'load_json' in typed context. [no-untyped-call]
[error] 33-33: mypy: Call to untyped function 'load_json' in typed context. [no-untyped-call]
[error] 35-35: mypy: Call to untyped function 'extract_recommended_extensions' in typed context. [no-untyped-call]
[error] 36-36: mypy: Call to untyped function 'extract_devcontainer_extensions' in typed context. [no-untyped-call]
[error] 52-52: mypy: Call to untyped function 'main' in typed context. [no-untyped-call]
[error] 44-44: ruff: Line too long (132 > 88) [E501]
[error] check-vscode-extension-sync: Running the script directly is not supported by 'uvx'. Use 'uv run scripts/check_extension_sync.py' instead.
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 5fc7625 in 1 minute and 36 seconds. Click for details.
- Reviewed
78lines of code in2files - Skipped
1files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/ci.yaml:66
- Draft comment:
Removed license compliance check job. Ensure this removal is intentional so that all required license verifications are handled elsewhere. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
2. .github/workflows/ci.yaml:93
- Draft comment:
Typo: In the commented command on this line, it says 'uv pip install build twine'. Was 'uv' meant to be 'sudo'? If not, please clarify the intended command. - Reason this comment was not posted:
Comment was on unchanged code.
3. Justfile:639
- Draft comment:
Typo: 'devcontainerm' should be 'devcontainer' - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_bvlsT1WOdDvu6Yks
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 33bfca2 in 50 seconds. Click for details.
- Reviewed
55lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. scripts/check_extension_sync.py:59
- Draft comment:
Missing call to main() inside the 'main' block. Append 'main()' to ensure the script runs as intended. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. scripts/check_extension_sync.py:1
- Draft comment:
Consider adding a usage instructions comment at the top of the file to guide users on script execution and its exit codes. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_SPKEbDecKBPXvXJT
You can customize
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 654d60c in 1 minute and 21 seconds. Click for details.
- Reviewed
11lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. scripts/check_extension_sync.py:59
- Draft comment:
The script now correctly calls main() under the if name == 'main' block. However, please ensure a trailing newline at the end of the file for POSIX compliance and to adhere to common code style guidelines. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the comment is technically correct about POSIX compliance, the rules explicitly state to delete comments that start with "Ensure that..." as they are asking the author to verify something. Additionally, this is the kind of issue that would be caught by standard linting tools in the build process. The missing newline could cause issues with some tools and is a real problem that should be fixed. The comment is pointing out a legitimate issue. While the issue is real, the rules clearly state not to make comments that would be caught by the build process, and trailing newline issues are typically caught by linters. The "Ensure that..." phrasing also violates the rules. Delete this comment. While it points out a real issue, it violates multiple rules: it starts with "Ensure that..." and it's the type of issue that would be caught by standard linting.
Workflow ID: wflow_aphrzGxDOzOz7K9M
You can customize
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
scripts/check_extension_sync.py (1)
59-60:⚠️ Potential issueAdd newline at end of file
The pipeline reports an end-of-file issue. Files should end with a newline.
if __name__ == "__main__": main() +🧰 Tools
🪛 GitHub Actions: CI/CD
[error] Fix End of Files hook modified this file to fix end-of-file issues.
[error] ruff: Found 10 errors (8 fixed, 2 remaining). No fixes available for some errors.
[error] Check VSCode Extension Sync hook failed: Tried to run a Python script with unsupported command
uvx. Useuv run scripts/check_extension_sync.pyinstead.
🧹 Nitpick comments (2)
scripts/check_extension_sync.py (2)
30-33: Replace unnecessary generator with set comprehensionThe linter flagged this as an unnecessary generator. A set comprehension would be more concise.
try: - return set( - ext if isinstance(ext, str) else ext["id"] - for ext in data["customizations"]["vscode"]["extensions"] - ) + return { + ext if isinstance(ext, str) else ext["id"] + for ext in data["customizations"]["vscode"]["extensions"] + }🧰 Tools
🪛 Ruff (0.11.9)
30-33: Unnecessary generator (rewrite as a set comprehension)
Rewrite as a set comprehension
(C401)
🪛 GitHub Actions: CI/CD
[error] Fix End of Files hook modified this file to fix end-of-file issues.
[error] 30-30: ruff: C401 Unnecessary generator (rewrite as a
setcomprehension)
[error] ruff: Found 10 errors (8 fixed, 2 remaining). No fixes available for some errors.
[error] Check VSCode Extension Sync hook failed: Tried to run a Python script with unsupported command
uvx. Useuv run scripts/check_extension_sync.pyinstead.
52-52: Line exceeds maximum lengthThis line exceeds the 88 character limit according to the linter.
- print("\nTo fix: Add these extensions to the `customizations.vscode.extensions` array in `.devcontainer/devcontainer.json`") + print("\nTo fix: Add these extensions to the `customizations.vscode.extensions` array") + print(" in `.devcontainer/devcontainer.json`")🧰 Tools
🪛 Ruff (0.11.9)
52-52: Line too long (132 > 88)
(E501)
🪛 GitHub Actions: CI/CD
[error] Fix End of Files hook modified this file to fix end-of-file issues.
[error] 52-52: ruff: E501 Line too long (132 > 88)
[error] ruff: Found 10 errors (8 fixed, 2 remaining). No fixes available for some errors.
[error] Check VSCode Extension Sync hook failed: Tried to run a Python script with unsupported command
uvx. Useuv run scripts/check_extension_sync.pyinstead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.devcontainer/devcontainer.json(1 hunks).github/workflows/ci.yaml(1 hunks)scripts/check_extension_sync.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .devcontainer/devcontainer.json
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yaml
[error] 95-95: no new line character at the end of file
(new-line-at-end-of-file)
🪛 GitHub Actions: CI/CD
.github/workflows/ci.yaml
[error] Fix End of Files hook modified this file to fix end-of-file issues.
scripts/check_extension_sync.py
[error] 15-15: mypy: Returning Any from function declared to return "dict[str, Any]" (no-any-return)
[error] Fix End of Files hook modified this file to fix end-of-file issues.
[error] 30-30: ruff: C401 Unnecessary generator (rewrite as a set comprehension)
[error] 52-52: ruff: E501 Line too long (132 > 88)
[error] ruff: Found 10 errors (8 fixed, 2 remaining). No fixes available for some errors.
[error] Check VSCode Extension Sync hook failed: Tried to run a Python script with unsupported command uvx. Use uv run scripts/check_extension_sync.py instead.
🪛 Ruff (0.11.9)
scripts/check_extension_sync.py
30-33: Unnecessary generator (rewrite as a set comprehension)
Rewrite as a set comprehension
(C401)
52-52: Line too long (132 > 88)
(E501)
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 125de43 in 1 minute and 15 seconds. Click for details.
- Reviewed
39lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .pre-commit-config.yaml:54
- Draft comment:
Consider adding 'pass_filenames: false' to the check-vscode-extension-sync hook to ensure it runs unconditionally, since it doesn't process file arguments. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% Without seeing the actual script (check_extension_sync.py), I can't verify if it processes file arguments or not. The comment makes an assumption about the script's behavior that I can't validate. Following the principle of needing strong evidence, I should err on the side of removing speculative comments. The comment could be correct - many validation scripts do run globally rather than on specific files. And if it is correct, this would be a useful configuration improvement. While the suggestion might be valid, I don't have enough context to verify it. The core rule is that we need strong evidence to keep comments. Delete the comment since we don't have strong evidence that the script doesn't process file arguments - we'd need to see the actual script contents to verify this claim.
Workflow ID: wflow_riV1aBwKicuFOFpl
You can customize
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
.pre-commit-config.yaml (2)
59-59: Remove trailing whitespace.Line 59 has extra spaces, leading to lint errors:
- types: [json] + types: [json]🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 59-59: trailing spaces
(trailing-spaces)
90-90: Add newline at end of file.Ensure the file ends with a newline to comply with POSIX standards and avoid linter warnings.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 90-90: no new line character at the end of file
(new-line-at-end-of-file)
scripts/check_extension_sync.py (4)
12-21: Redirect error messages to stderr.Printing errors to stdout can mix with standard output; prefer
stderrfor error messages:- print(f"❌ Error: {file_path} not found.") + print(f"❌ Error: {file_path} not found.", file=sys.stderr) ... - print(f"❌ Error parsing {file_path}: {e}") + print(f"❌ Error parsing {file_path}: {e}", file=sys.stderr)
27-34: Use a set comprehension instead ofset(generator).Ruff flags the generator; a comprehension is more idiomatic:
-def extract_devcontainer_extensions(data: Dict[str, Any]) -> Set[str]: - try: - return set( - ext if isinstance(ext, str) else ext["id"] - for ext in data["customizations"]["vscode"]["extensions"] - ) - except KeyError: - return set() +def extract_devcontainer_extensions(data: Dict[str, Any]) -> Set[str]: + try: + extensions = data["customizations"]["vscode"]["extensions"] + except KeyError: + return set() + return {ext if isinstance(ext, str) else ext["id"] for ext in extensions}🧰 Tools
🪛 Ruff (0.11.9)
29-32: Unnecessary generator (rewrite as a set comprehension)
Rewrite as a set comprehension
(C401)
49-51: Split the long print statement.Line 51 exceeds typical line-length limits. Break it into multiple parts:
- print("\nTo fix: Add these extensions to the `customizations.vscode.extensions` array in `.devcontainer/devcontainer.json`") + print( + "\nTo fix: Add these extensions to the " + "`customizations.vscode.extensions` array in " + "`.devcontainer/devcontainer.json`" + )🧰 Tools
🪛 Ruff (0.11.9)
51-51: Line too long (132 > 88)
(E501)
58-59: Add newline at end of file.Make sure the script ends with a newline character to satisfy POSIX requirements and avoid linter complaints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.devcontainer/devcontainer.json(1 hunks).pre-commit-config.yaml(4 hunks)Justfile(1 hunks)scripts/check_extension_sync.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Justfile
🚧 Files skipped from review as they are similar to previous changes (1)
- .devcontainer/devcontainer.json
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.pre-commit-config.yaml
[error] 52-52: syntax error: expected , but found '-'
(syntax)
[warning] 54-54: wrong indentation: expected 6 but found 5
(indentation)
[warning] 54-54: too many spaces after hyphen
(hyphens)
[error] 59-59: trailing spaces
(trailing-spaces)
[error] 90-90: no new line character at the end of file
(new-line-at-end-of-file)
🪛 GitHub Actions: CI/CD
.pre-commit-config.yaml
[error] 43-52: InvalidConfigError: while parsing a block mapping, did not find expected key. Check syntax near line 43 to 52.
🪛 Ruff (0.11.9)
scripts/check_extension_sync.py
29-32: Unnecessary generator (rewrite as a set comprehension)
Rewrite as a set comprehension
(C401)
51-51: Line too long (132 > 88)
(E501)
🔇 Additional comments (4)
.pre-commit-config.yaml (2)
4-4: Approve updated hook version.Bumping
mypytov1.16.0aligns with the latest type-checking improvements.
24-24: Approve updated hook version.Updating
rufftov0.11.12ensures your linting stays current.scripts/check_extension_sync.py (2)
1-2: Shebang present for direct execution.Including
#!/usr/bin/env python3is correct and enables running the script as an executable.
3-6: Imports satisfymypy.Grouping standard library imports together and adding
Dict,Set,Anyfromtypingcovers all static type checks.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.pre-commit-config.yaml (1)
52-58: Scope the new hook to only the relevant JSON files
Currently the sync check runs on all.jsonfiles. Restrict it to just your VSCode extension lists to avoid unnecessary executions:- id: check-vscode-extension-sync name: Check VSCode Extension Sync entry: uv run --with-editable . scripts/check_extension_sync.py language: system - types: [json] + files: '(\.vscode/extensions\.json|\.devcontainer/devcontainer\.json)$'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.pre-commit-config.yaml(4 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.pre-commit-config.yaml
[error] 59-59: trailing spaces
(trailing-spaces)
🪛 GitHub Actions: CI/CD
.pre-commit-config.yaml
[error] 1-1: Pre-commit hook 'trailing-whitespace' failed and fixed issues in this file.
🔇 Additional comments (2)
.pre-commit-config.yaml (2)
4-4: Bump mypy hook version
Therevwas updated fromv1.15.0tov1.16.0. This is a straightforward patch release—ensure CI and local environments remain compatible with the new version.
24-24: Bump ruff hook version
Therevwas updated fromv0.11.11tov0.11.12. Verify that linting behavior and autofixes remain consistent in your pipeline.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 08ff1c3 in 56 seconds. Click for details.
- Reviewed
69lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. scripts/check_extension_sync.py:14
- Draft comment:
Consider specifying an encoding (e.g., encoding='utf-8') when opening files for better portability. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. scripts/check_extension_sync.py:17
- Draft comment:
For error messages, consider printing to stderr (e.g., print(..., file=sys.stderr)) to differentiate error output from normal stdout. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. scripts/check_extension_sync.py:48
- Draft comment:
Optionally, add colored output for error messages per requirements to enhance clarity if the terminal supports it. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_BKbHnCQUlSevEE3J
You can customize
Feature Pull Request
Branch Naming Convention
Current branch:
feature/check-vscode-extension-syncRelated Issue
Closes #239
Feature Description
This PR adds an automated check to ensure that VSCode recommended extensions in .vscode/extensions.json are properly synced with the extensions configured in .devcontainer/devcontainer.json. This prevents inconsistencies between local VSCode environments and the devcontainer setup.
Implementation Details
Changes Made
Testing Performed
Pre-commit and Testing Instructions
just check-extension-syncDocumentation Updates
scripts/check_extension_sync.pyBreaking Changes
None.
Checklist
feature/descriptionorfeature/issue-number-description)pre-commit run --all-filespasses)pytest --covto verify)flake8or equivalent passes)Reviewer Notes
Important
Adds a script to ensure VSCode extensions are synced between local and devcontainer setups, integrated into pre-commit and
justtask runner.scripts/check_extension_sync.pyto verify VSCode extensions in.vscode/extensions.jsonmatch those in.devcontainer/devcontainer.json.check-extension-synctask toJustfilefor running the script..pre-commit-config.yamlto include the extension sync check in pre-commit hooks.This description was created by
for 08ff1c3. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Chores