GAMS sync#39
Conversation
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds BARON, Gurobi, and HiGHS solver option snapshots, a manually triggered GAMS synchronization workflow, workspace exclusion for the scraper, and documentation for updating option definitions. ChangesGAMS option synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
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: 3
🧹 Nitpick comments (1)
.github/workflows/gams-sync.yml (1)
40-43: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueDistinguish diff differences from diff errors.
diffreturns exit code 1 for content differences and 2 for errors (e.g., file not found, permission denied). The current||block reports "options are out of date" for both cases, which could mislead debugging on an I/O error.♻️ Proposed fix
- diff "$snapshot" "$new" || { - echo "::error::$solver options are out of date. Regenerate and update ${snapshot} and the corresponding options.rs." - exit 1 - } + diff "$snapshot" "$new" + diff_status=$? + if [ $diff_status -eq 1 ]; then + echo "::error::$solver options are out of date. Regenerate and update ${snapshot} and the corresponding options.rs." + exit 1 + elif [ $diff_status -eq 2 ]; then + echo "::error::Error comparing $solver snapshot ($snapshot vs $new)." + exit 1 + fi🤖 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/gams-sync.yml around lines 40 - 43, Update the diff check in the workflow step around the diff command to distinguish its exit statuses: report that solver options are out of date only when diff returns 1, and report or propagate a separate error for other nonzero statuses such as I/O failures. Preserve the existing failure exit behavior for both cases.
🤖 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/gams-sync.yml:
- Around line 1-44: Add a top-level permissions block to the “Verify GAMS solver
options” workflow granting only contents read access. In both checkout steps,
set persist-credentials to false so the GitHub token is not retained for
subsequent scraper execution.
- Around line 13-17: Update the “Checkout gams_scraper” action to pin the
Godwinss24/GAM-SOLVERS checkout to a specific trusted commit SHA instead of the
default branch tip, and add a brief comment documenting that the SHA must be
deliberately reviewed and updated when upgrading the dependency.
In `@README.md`:
- Line 344: Update the README workflow link so both its display text and target
reference the existing .github/workflows/gams-sync.yml file, replacing the
incorrect gams-options.yml target.
---
Nitpick comments:
In @.github/workflows/gams-sync.yml:
- Around line 40-43: Update the diff check in the workflow step around the diff
command to distinguish its exit statuses: report that solver options are out of
date only when diff returns 1, and report or propagate a separate error for
other nonzero statuses such as I/O failures. Preserve the existing failure exit
behavior for both cases.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d92a40d-625f-454b-875b-16a43bac0d57
📒 Files selected for processing (6)
.github/workflows/gams-sync.ymlCargo.tomlREADME.mdsnapshots/baron.txtsnapshots/gurobi.txtsnapshots/highs.txt
| - name: Checkout gams_scraper | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: Godwinss24/GAM-SOLVERS | ||
| path: gams_scraper |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin the external gams_scraper checkout to a specific commit.
The workflow checks out Godwinss24/GAM-SOLVERS at its default branch tip and then executes that code via cargo run. If the external repo is compromised, arbitrary code runs in CI with the workflow's token. Pin to a specific commit SHA and document the update process.
🔒️ Proposed fix
- name: Checkout gams_scraper
uses: actions/checkout@v4
with:
repository: Godwinss24/GAM-SOLVERS
+ ref: <pinned-commit-sha>
path: gams_scraper
persist-credentials: false🧰 Tools
🪛 zizmor (1.26.1)
[warning] 13-17: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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/gams-sync.yml around lines 13 - 17, Update the “Checkout
gams_scraper” action to pin the Godwinss24/GAM-SOLVERS checkout to a specific
trusted commit SHA instead of the default branch tip, and add a brief comment
documenting that the SHA must be deliberately reviewed and updated when
upgrading the dependency.
|
Hello @Godwinss24, I opened a PR at Godwinss24/GAM-SOLVERS#1 to add support for more solvers. |
|
Hello @GermanHeim, I've reviewed it and merged the PR. The added solver support looks great. |
2 participants
Description
Adds a CI check that verifies the hand-written solver option lists in
options.rs(BARON, Gurobi, HiGHS) stay in sync with GAMS' published docs. A manually-triggered workflow scrapes the current GAMS solver option pages using an external tool (gams_scraper) and diffs the result against a checked-in snapshot per solver. This is a verify-only check — it flags drift but does not auto-generate or auto-commit changes; a maintainer still manually updates the relevant*_params!macroand the snapshot file when it fails.
Checklist
cargo fmtandcargo clippyto ensure code qualitycargo test)cargo test --features gamspasses (requires GAMS)cargo test --features gurobipasses (requires Gurobi)cargo test --features baronpasses (requires BARON)Changes Made
.github/workflows/gams-sync.yml: manually-triggered(
workflow_dispatch) workflow that runsgams_scraperfor BARON, GUROBI,and HIGHS and diffs the output against checked-in snapshots
snapshots/folder with current baseline scraper output for eachsupported solver
Cargo.tomlto exclude the externalgams_scrapercheckout fromthe workspace during CI runs
and what to do when it fails
Related Issues
Closes #37
@GermanHeim.