Update project configuration and documentation#15
Merged
Conversation
- Added `.venv/` to `.gitignore` to exclude virtual environment files. - Updated installation instructions in `contributors.md` to use `uv` for setting up the environment and running tests. - Enhanced release notes in `contributors.md` to clarify the automated release workflow and manual publishing steps. - Refined the CI workflow in `.github/workflows/ci.yml` to utilize `uv` for dependency installation and test execution. - Improved function scheduling in `compile_interpreter.py` to manage function generation more effectively. - Added version normalization for release tags in `check_release_versions.py` to ensure consistency between tag and project versions.
|
Warning Review limit reached
More reviews will be available in 19 minutes and 53 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR implements automated release pipeline infrastructure using the ChangesAutomated Release Pipeline
Compiler Tree-Shaking Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
1-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit least-privilege
permissionsfor the workflow/job.No
permissionsblock means broader default token access than needed. Define minimal permissions (for this job, read-only is likely enough).Suggested fix
name: CI +permissions: + contents: read🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 1 - 39, Add an explicit least-privilege permissions block to the CI workflow (either at the top-level or inside the test job) so the GITHUB_TOKEN is scoped only to what the job needs; for this test job (name "test", matrix python-version) grant minimal read-only rights such as contents: read (and any other specific permissions your steps require) instead of relying on defaults. Update .github/workflows/ci.yml to include a permissions mapping at workflow or job level that restricts access (e.g., contents: read) so actions/checkout and the test steps run with least privilege.Source: Linters/SAST tools
🧹 Nitpick comments (2)
.github/workflows/release.yml (2)
57-61: ⚡ Quick winConsider using npm Provenance and trusted publishing.
The current workflow uses an NPM_TOKEN secret for authentication. npm now supports Provenance (trusted publishing) which eliminates the need for long-lived tokens and provides supply-chain transparency.
📦 Example implementation
- name: Publish to npm working-directory: npm - run: npm publish --access public - env: - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + run: npm publish --provenance --access publicNote: This requires configuring the npm package for trusted publishing. See npm documentation for setup details.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 57 - 61, The Publish to npm step currently authenticates with a long-lived token via the NODE_AUTH_TOKEN env and runs npm publish --access public; replace this with npm's trusted publishing (provenance) flow by removing the NPM_TOKEN usage and switching the step to call the trusted-publish command or configure GitHub OIDC-based trusted publishing as documented by npm, updating the step named "Publish to npm" and the command that currently invokes npm publish --access public to use the provenance/trusted-publish mechanism and any required package/registry metadata, and remove the NODE_AUTH_TOKEN env reference to avoid long-lived secret usage.Source: Linters/SAST tools
15-72: ⚖️ Poor tradeoffConsider pinning GitHub Actions to commit hashes.
The workflow uses semantic version tags (e.g.,
@v4,@v5) for actions. For maximum supply-chain security, consider pinning to immutable commit SHAs. This prevents automatic updates that could introduce malicious code, though it requires manual dependency updates.Example:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 with: persist-credentials: falseNote: This is a tradeoff between security and maintenance burden. Many projects accept the risk of version tags for easier maintenance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 15 - 72, The workflow currently pins actions by tag (e.g., actions/checkout@v4, actions/setup-python@v5, astral-sh/setup-uv@v5, actions/setup-node@v4, softprops/action-gh-release@v2); replace those tag references with the corresponding immutable commit SHA for each action to mitigate supply-chain risk (update each uses: line to uses: actions/checkout@<commit-sha> etc.), verifying the chosen SHAs correspond to the intended tag/release and updating any action inputs if the pinned commit requires them.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 25-26: The workflow step named "Set up uv" currently uses the
mutable reference "astral-sh/setup-uv@v5"; update that step to pin the action to
an immutable full commit SHA (replace "`@v5`" with the specific 40-character
commit hash for astral-sh/setup-uv) so the action is fixed to a known commit;
ensure you use the full commit SHA string in place of the tag in the action
reference to prevent supply-chain drift.
In @.github/workflows/release.yml:
- Line 40: In the publish-npm job's checkout step (the step that uses
actions/checkout@v4), add the persist-credentials: false input to the checkout
action to prevent Git credentials from being persisted to the runner; update the
step configuration that references uses: actions/checkout@v4 so it includes
persist-credentials: false under its inputs.
- Line 15: In the verify job, update the actions/checkout@v4 step to disable
credential persistence by adding persist-credentials: false to its step
configuration; locate the checkout step (uses: actions/checkout@v4) and add the
persist-credentials: false key under that step so Git credentials are not
persisted for subsequent steps.
- Around line 8-9: Remove the workflow-level permissions block that sets
"contents: write" and instead add a job-level permissions entry for the
github-release job; specifically, delete the top-level "permissions: contents:
write" and add a "permissions: contents: write" mapping under the github-release
job definition (leaving other jobs with default permissions), so only
github-release has write access to repository contents.
In `@contributors.md`:
- Line 86: The documentation uses the incorrect platform casing "Github" in the
contributors.md sentence describing the Release workflow; update that occurrence
to the proper "GitHub" spelling (the line referencing the Release workflow and
.github/workflows/release.yml) so the sentence reads "Pushing a `v*` tag
triggers the [Release workflow](.github/workflows/release.yml)..." with "GitHub"
capitalized correctly.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 1-39: Add an explicit least-privilege permissions block to the CI
workflow (either at the top-level or inside the test job) so the GITHUB_TOKEN is
scoped only to what the job needs; for this test job (name "test", matrix
python-version) grant minimal read-only rights such as contents: read (and any
other specific permissions your steps require) instead of relying on defaults.
Update .github/workflows/ci.yml to include a permissions mapping at workflow or
job level that restricts access (e.g., contents: read) so actions/checkout and
the test steps run with least privilege.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 57-61: The Publish to npm step currently authenticates with a
long-lived token via the NODE_AUTH_TOKEN env and runs npm publish --access
public; replace this with npm's trusted publishing (provenance) flow by removing
the NPM_TOKEN usage and switching the step to call the trusted-publish command
or configure GitHub OIDC-based trusted publishing as documented by npm, updating
the step named "Publish to npm" and the command that currently invokes npm
publish --access public to use the provenance/trusted-publish mechanism and any
required package/registry metadata, and remove the NODE_AUTH_TOKEN env reference
to avoid long-lived secret usage.
- Around line 15-72: The workflow currently pins actions by tag (e.g.,
actions/checkout@v4, actions/setup-python@v5, astral-sh/setup-uv@v5,
actions/setup-node@v4, softprops/action-gh-release@v2); replace those tag
references with the corresponding immutable commit SHA for each action to
mitigate supply-chain risk (update each uses: line to uses:
actions/checkout@<commit-sha> etc.), verifying the chosen SHAs correspond to the
intended tag/release and updating any action inputs if the pinned commit
requires them.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb6c1d53-2771-4a23-a49b-ad15b64b1a37
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.github/workflows/ci.yml.github/workflows/release.yml.gitignorecontributors.mdminecraft_script/compiler/builtin_functions.pyminecraft_script/compiler/compile_interpreter.pyscripts/check_release_versions.pytests/test_tree_shaking.pytodo.md
💤 Files with no reviewable changes (1)
- todo.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="minecraft_script/compiler/compile_interpreter.py">
<violation number="1" location="minecraft_script/compiler/compile_interpreter.py:492">
P1: Tree-shaken function scheduling misses callback functions passed into builtins like `raycast_block`/`raycast_entity`, producing runtime calls to non-generated mcfunctions.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
Comment on lines
+492
to
+494
| if not is_builtin and fnc.body is not None: | ||
| self.schedule_function_generation(fnc) | ||
| commands, return_value = fnc.call(self, arguments, context) |
There was a problem hiding this comment.
P1: Tree-shaken function scheduling misses callback functions passed into builtins like raycast_block/raycast_entity, producing runtime calls to non-generated mcfunctions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At minecraft_script/compiler/compile_interpreter.py, line 492:
<comment>Tree-shaken function scheduling misses callback functions passed into builtins like `raycast_block`/`raycast_entity`, producing runtime calls to non-generated mcfunctions.</comment>
<file context>
@@ -459,11 +485,13 @@ def visit_ImportNode(self, node, context: CompileContext) -> CompileResult:
+ hasattr(fnc.call, "__module__")
+ and fnc.call.__module__.endswith(".builtin_functions")
)
+ if not is_builtin and fnc.body is not None:
+ self.schedule_function_generation(fnc)
+ commands, return_value = fnc.call(self, arguments, context)
</file context>
Suggested change
| if not is_builtin and fnc.body is not None: | |
| self.schedule_function_generation(fnc) | |
| commands, return_value = fnc.call(self, arguments, context) | |
| if not is_builtin and fnc.body is not None: | |
| self.schedule_function_generation(fnc) | |
| elif is_builtin: | |
| for argument in arguments: | |
| if isinstance(argument, MCSFunction): | |
| self.schedule_function_generation(argument) | |
| commands, return_value = fnc.call(self, arguments, context) |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
Status: No Issues Found | Recommendation: Merge
Files Reviewed (6 files)
.github/workflows/ci.yml- CI workflow updated for uv.github/workflows/release.yml- New release workflow.gitignore- Added .venv/ exclusionscripts/check_release_versions.py- Added --tag validationminecraft_script/compiler/compile_interpreter.py- Tree shaking implementationminecraft_script/compiler/builtin_functions.py- Click handler schedulingtests/test_tree_shaking.py- Tree shaking tests
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (10 files)
Reviewed by step-3.7-flash-20260528 · 179,707 tokens |
- Added permissions for content access in CI and release workflows. - Updated action versions for `checkout`, `setup-python`, `setup-node`, and `action-gh-release` to specific commits for stability. - Enhanced `ci.yml` to ensure proper content permissions for pull requests.
- Updated the GitHub Actions workflow to ensure the `github-release` job depends on both `verify` and `publish-npm`. - Improved the `raycast_block` and `raycast_entity` functions to schedule function generation for loop functions when referenced. - Added a new test to verify that raycast callback functions are generated correctly when referenced in the datapack.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.venv/to.gitignoreto exclude virtual environment files.contributors.mdto useuvfor setting up the environment and running tests.contributors.mdto clarify the automated release workflow and manual publishing steps..github/workflows/ci.ymlto utilizeuvfor dependency installation and test execution.compile_interpreter.pyto manage function generation more effectively.check_release_versions.pyto ensure consistency between tag and project versions.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
Chores