ci: build for windows arm64#1084
Merged
Merged
Conversation
Add arm64 architecture to the build matrix using the windows-11-arm hosted runner, and fix Inno Setup installer to use arm64 install mode instead of x64compatible for ARM64 builds. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add OPENSSL_VENDORED=1 for ARM64 builds since cryptography 46.x has no win_arm64 wheel on PyPI and must be compiled from source. Also declare uv environments to include win_arm64 in lock file resolution, removing macOS/Linux-only packages from the lock. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace ineffective OPENSSL_VENDORED approach (blocked by cargo --locked in maturin) with a vcpkg-based OpenSSL installation. The compiled output is cached under a stable key to avoid recompiling on every run. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add platform marker to exclude accessible-output3 on ARM64 Windows. Guard the import with try/except so the app runs gracefully when the package is absent; use_accessible_output now returns False when accessible_output3 is not installed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Inno Setup does not support the arm64compatible identifier. Use a
preprocessor conditional to set ArchitecturesAllowed=arm64 for ARM64
and {arch}compatible for x86/x64 builds.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
platform.architecture() returns '64bit' on ARM64, causing the updater to download the x64 installer. Check platform.machine() first to correctly identify ARM64 Windows. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
clementb49
marked this pull request as ready for review
April 11, 2026 20:23
Contributor
WalkthroughAdds ARM64 support across Windows CI, dependency selection, runtime accessible-output availability handling, app-architecture detection, and installer preprocessor macros; also adjusts workflow checkout/artifact paths and uv dependency flags. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@basilisk/accessible_output.py`:
- Around line 13-15: The module-level availability flags (e.g., _ao3_available)
use lowercase but should follow constant naming convention; rename these flags
to UPPER_SNAKE_CASE (for example _ao3_available -> AO3_AVAILABLE) and update all
usages and imports in this file accordingly (also apply the same change for the
other availability flags referenced around lines 71-72 and 77). Ensure you
preserve the try/except assignment behavior, keep the leading underscore only if
you want module-private visibility (e.g., _AO3_AVAILABLE vs AO3_AVAILABLE) and
update any conditional checks or assignments that reference the old names
(search for occurrences of _ao3_available and the other two flags and replace
them consistently).
- Around line 77-83: When _ao3_available is False the code currently sets
self._accessible_output to None and returns, which allows handle(...,
force=True) to later call self.accessible_output.speak(...) and crash; instead,
replace the None with a safe no-op object so accessible_output.speak is always
callable on unsupported platforms: introduce a simple NullAccessibleOutput (or
similar) and assign it to self._accessible_output in the branch where
_ao3_available is False (preserving display_log behavior), or alternatively
update handle to check self.accessible_output before calling speak; reference
symbols: _ao3_available, display_log, self._accessible_output,
accessible_output.speak, and handle(..., force=True).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: fab532e5-586a-483a-ad19-83176559cb81
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/workflows/build_app.yml.github/workflows/ci.yml.github/workflows/localization.ymlbasilisk/accessible_output.pybasilisk/updater.pypyproject.tomlwin_installer.iss
Introduce _NullAccessibleOutput with no-op speak/braille methods and assign it instead of None when _AO3_AVAILABLE is False, so handle(..., force=True) can never crash with AttributeError on unsupported platforms. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@basilisk/accessible_output.py`:
- Around line 87-93: The accessible_output property is currently annotated as
accessible_output3.outputs.auto.Auto but can also be None (when
use_accessible_output is False) or an instance of _NullAccessibleOutput; define
a small Protocol (e.g., AccessibleOutputLike) that declares the speak(...) and
braille(...) methods, then change the accessible_output property's return type
to Optional[AccessibleOutputLike] (or Union[AccessibleOutputLike, None]) so
callers see the correct contract; update imports to include typing.Protocol and
Optional and use AccessibleOutputLike in the property/type hints instead of
Auto.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 0582fb13-cbe7-4202-af57-f7b85064ead9
📒 Files selected for processing (1)
basilisk/accessible_output.py
Comment on lines
+87
to
+93
| if not _AO3_AVAILABLE: | ||
| if display_log: | ||
| log.debug( | ||
| "accessible_output3 is not available on this platform." | ||
| ) | ||
| self._accessible_output = _NullAccessibleOutput() | ||
| return |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether callers rely on Auto-specific APIs beyond speak/braille.
rg -nP --type=py '\.accessible_output\.[A-Za-z_]\w*\(' -C2Repository: SigmaNight/basiliskLLM
Length of output: 703
🏁 Script executed:
# First, let's look at the file to understand the current type annotations
cat -n basilisk/accessible_output.py | head -120Repository: SigmaNight/basiliskLLM
Length of output: 4350
🏁 Script executed:
# Check for any usages of accessible_output property outside this file
rg -nP --type=py 'accessible_output\.' --glob='!basilisk/accessible_output.py'Repository: SigmaNight/basiliskLLM
Length of output: 348
🏁 Script executed:
# Also check what accessible_output3 is and what Auto provides
rg -nP --type=py 'accessible_output3|import Auto' basilisk/Repository: SigmaNight/basiliskLLM
Length of output: 610
Fix type annotation for accessible_output property; it can return None or _NullAccessibleOutput, not just Auto.
The accessible_output property is typed as accessible_output3.outputs.auto.Auto (line 106), but can actually return None (when use_accessible_output is False; see line 99) or _NullAccessibleOutput (line 92). Both _NullAccessibleOutput and Auto provide speak() and braille() methods, which are the only methods actually called (lines 150, 158).
Use a Protocol to properly express this contract:
Proposed typing refactor
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Protocol
@@
class _NullAccessibleOutput:
"""No-op fallback used when accessible_output3 is unavailable."""
def speak(self, text: str) -> None:
pass
def braille(self, text: str) -> None:
pass
+
+
+class _AccessibleOutputBackend(Protocol):
+ def speak(self, text: str) -> None: ...
+ def braille(self, text: str) -> None: ...
@@
def __init__(self):
"""Initialize the accessible output handler."""
- self._accessible_output = None
+ self._accessible_output: _AccessibleOutputBackend | None = None
@@
- def accessible_output(self) -> accessible_output3.outputs.auto.Auto:
+ def accessible_output(self) -> _AccessibleOutputBackend | None:
"""Get the Accessible Output instance, initializing it if necessary."""
if self._accessible_output is None:
self._init_accessible_output(False)
return self._accessible_output🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@basilisk/accessible_output.py` around lines 87 - 93, The accessible_output
property is currently annotated as accessible_output3.outputs.auto.Auto but can
also be None (when use_accessible_output is False) or an instance of
_NullAccessibleOutput; define a small Protocol (e.g., AccessibleOutputLike) that
declares the speak(...) and braille(...) methods, then change the
accessible_output property's return type to Optional[AccessibleOutputLike] (or
Union[AccessibleOutputLike, None]) so callers see the correct contract; update
imports to include typing.Protocol and Optional and use AccessibleOutputLike in
the property/type hints instead of Auto.
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.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Localization