[codex] bump Windows ROCm to 7.2.1 - #1690
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughUpdated AMD ROCm package references from 7.2 → 7.2.1 across assets and TypeScript; added exported AMD Windows constants and URL helper; and replaced generic AMD Windows install failure messages with a formatted error including driver and ROCm-release context. Changes
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
dosubot
Bot
added
the
size:M
|
Manually testing today. |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/constants.ts (1)
200-217: Optional: DRY the embedded7.2.1version in filenames.
AMD_ROCM_WINDOWS_RELEASEis defined as'7.2.1'but the same literal is then hardcoded inside every filename passed togetAmdRocmWindowsPackageUrl. Next bump (e.g. 7.2.2) will require editing the version in 8 places instead of 1, which is exactly the kind of drift this constant was introduced to prevent.♻️ Proposed refactor
-export const AMD_ROCM_SDK_PACKAGES: string[] = [ - getAmdRocmWindowsPackageUrl('rocm_sdk_core-7.2.1-py3-none-win_amd64.whl'), - getAmdRocmWindowsPackageUrl('rocm_sdk_devel-7.2.1-py3-none-win_amd64.whl'), - getAmdRocmWindowsPackageUrl('rocm_sdk_libraries_custom-7.2.1-py3-none-win_amd64.whl'), - getAmdRocmWindowsPackageUrl('rocm-7.2.1.tar.gz'), -]; +const ROCM = AMD_ROCM_WINDOWS_RELEASE; +export const AMD_ROCM_SDK_PACKAGES: string[] = [ + getAmdRocmWindowsPackageUrl(`rocm_sdk_core-${ROCM}-py3-none-win_amd64.whl`), + getAmdRocmWindowsPackageUrl(`rocm_sdk_devel-${ROCM}-py3-none-win_amd64.whl`), + getAmdRocmWindowsPackageUrl(`rocm_sdk_libraries_custom-${ROCM}-py3-none-win_amd64.whl`), + getAmdRocmWindowsPackageUrl(`rocm-${ROCM}.tar.gz`), +];The same applies to the
+rocm7.2.1build tag inAMD_TORCH_PACKAGES.Note that the assets
.txt/.compiledfiles (which are uv-generated) must continue to match exactly — those are autogenerated and not affected by this refactor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/constants.ts` around lines 200 - 217, The filenames hardcode the ROCm version instead of reusing AMD_ROCM_WINDOWS_RELEASE; update getAmdRocmWindowsPackageUrl usage so filenames are constructed from AMD_ROCM_WINDOWS_RELEASE (e.g., use template fragments that insert ${AMD_ROCM_WINDOWS_RELEASE} for "rocm_sdk_core-<version>-..." and "rocm-<version>.tar.gz") and change the AMD_TORCH_PACKAGES entries to include the "+rocm${AMD_ROCM_WINDOWS_RELEASE}" build tag and the version in the wheel names, keeping AMD_ROCM_WINDOWS_BASE_URL and getAmdRocmWindowsPackageUrl intact and ensuring AMD_ROCM_SDK_PACKAGES and AMD_TORCH_PACKAGES are generated from those templates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/constants.ts`:
- Around line 200-217: The filenames hardcode the ROCm version instead of
reusing AMD_ROCM_WINDOWS_RELEASE; update getAmdRocmWindowsPackageUrl usage so
filenames are constructed from AMD_ROCM_WINDOWS_RELEASE (e.g., use template
fragments that insert ${AMD_ROCM_WINDOWS_RELEASE} for
"rocm_sdk_core-<version>-..." and "rocm-<version>.tar.gz") and change the
AMD_TORCH_PACKAGES entries to include the "+rocm${AMD_ROCM_WINDOWS_RELEASE}"
build tag and the version in the wheel names, keeping AMD_ROCM_WINDOWS_BASE_URL
and getAmdRocmWindowsPackageUrl intact and ensuring AMD_ROCM_SDK_PACKAGES and
AMD_TORCH_PACKAGES are generated from those templates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 74b8da4a-723f-477a-b711-5a344f0f1263
📒 Files selected for processing (5)
assets/override_amd.txtassets/requirements/amd_requirements.txtassets/requirements/windows_amd.compiledsrc/constants.tssrc/virtualEnvironment.ts
|
Manual test succeeded |
|
LGTM, i recognize the rocm link from desktop 2.0 |
|
oh, im not in the comfy desktop devs group, RIP |
3 participants
Summary
rocm-rel-7.2artifacts to the latest public Windows release,rocm-rel-7.2.1rocm_sdk_devel26.2.2requirement for ROCm7.2.1on WindowsWhy
ROCm overall has a
7.2.2release, but AMD's public Windows ROCm/PyTorch feed currently exposesrocm-rel-7.2.1as the newest installable Windows package set. Desktop was still pinned to the olderrocm-rel-7.2artifacts, so this updates the app to the latest Windows ROCm release that is publicly available today.Impact
7.2.1artifacts instead of the older7.2preview/dev package setValidation
yarn install --immutableyarn formatyarn lintyarn typecheckyarn test:unit┆Issue is synchronized with this Notion page by Unito
Summary by CodeRabbit
Chores
Improvements