CopyArtifacts: write trailing newline to match prettier (fix copy-artifacts CI) - #518
Conversation
The copy-artifacts determinism check has been failing on main: CopyArtifacts.sol wrote each abi JSON without a trailing newline, but the committed files (and the prettier-rainix pre-commit hook, now enforced in CI via rainix#195) expect a final newline on JSON. The two checks fought over crates/bindings/abi/*.json. Append a newline in vm.writeFile so the generated artifact matches both the committed file and prettier. Re-running CopyArtifacts now produces a no-op diff against the committed abis (verified), so the determinism check passes without changing any committed artifact. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 39 minutes and 10 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAppend a trailing newline when writing committed artifact files in ChangesArtifact & Workflow Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
rainlang-prelude builds the rain CLI via nix buildRustPackage, which fetches each crate dep through nix builtin fetcher from crates.io — whose download endpoint 403s nix UAs for crates not mirrored on cache.nixos.org (e.g. alloy-json-abi). Add cachix-action so those derivations substitute from the rainlanguage cache instead, and raise the store GC cap 1G->8G so the toolchain survives between runs (matches rainlanguage/rainix#196/#197). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/copy-artifacts.yaml:
- Around line 18-22: Replace the mutable tag cachix/cachix-action@v15 with the
exact commit SHA to pin the action (use
cachix/cachix-action@ad2ddac53f961de1989924296a1f236fcfbaa4fc) so the workflow
cannot change unexpectedly; update the uses entry in the workflow to reference
that SHA, keep continue-on-error and with: blocks unchanged, and commit the
change to lock the action to the specified revision.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 197a2d48-2df1-48bd-a6cf-e53665c50d94
📒 Files selected for processing (1)
.github/workflows/copy-artifacts.yaml
Replace the whole local workflow with a thin caller of rainix-copy-artifacts.yaml (rainix#201): centralizes the build-and-assert-clean logic + the Cachix/GC-cap resilience, and drops the rainlang-prelude step — committing the generated artifacts is what removes the need for a prelude. CopyArtifacts.sol still emits the trailing newline so the abis match.
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=S |
The
copy-artifactsdeterminism check has been failing on main. Root cause: a conflict between two CI checks overcrates/bindings/abi/*.json:CopyArtifacts.solwrote each abi JSON with no trailing newlineprettier-rainix(now enforced in CI via rainix#195) adds a trailing newline to JSONThe committed files have newlines (prettier-happy), so the determinism check (
git diff --exit-codeafter regenerating) failed because CopyArtifacts stripped them.Fix: append a newline in
vm.writeFileso the generated artifact matches both the committed file and prettier. Re-runningCopyArtifacts.sol --ffinow produces a no-op diff against the committed abis (verified locally) — the determinism check passes, prettier stays happy, and no committed artifact changes.This unblocks all rainlang PRs (incl. #516 parser publish), which were red on the pre-existing
copy-artifactsfailure.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores