ci: windows arm e2e fix#1655
Open
zerob13 wants to merge 6 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR upgrades DuckDB for Windows ARM64, adds a DuckDB/VSS CI smoke test, changes the Windows ARM64 E2E workflow to run Playwright launch and a packaged-exe process smoke, collects extensive diagnostics on failure, enhances E2E fixtures to attach main-process logs, and updates related docs and release/build workflows. ChangesWindows ARM64 DuckDB Support & E2E Diagnostics
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant DuckDB as smoke:duckdb:vss
participant Playwright as Playwright Launch Test
participant PackagedExe as Packaged DeepChat.exe
participant Diagnostics as Diagnostics Collection
participant Upload as Artifact Upload
CI->>DuckDB: Run DuckDB/VSS smoke test
DuckDB-->>CI: INSTALL vss & LOAD vss success
CI->>Playwright: Run launch.smoke.spec.ts
Playwright-->>CI: Test artifacts, logs captured
CI->>PackagedExe: Start process, wait for stability
PackagedExe-->>CI: Process remains alive
CI->>Diagnostics: Collect filesystem/logs/events
Diagnostics-->>CI: Diagnostics prepared
CI->>Upload: Upload deepchat-win-arm64-e2e-diagnostics
Upload-->>CI: Artifact stored
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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.
🧹 Nitpick comments (2)
.github/workflows/release.yml (2)
429-454: ⚖️ Poor tradeoffConsider extracting the YAML merge logic to reduce duplication.
The
merge_windows_ymlfunction (lines 429-454) is nearly identical tomerge_mac_yml(lines 483-508), differing only in the input directory paths. This code duplication makes maintenance harder.While fixing this would require refactoring both merge functions into a single parameterized helper, it would improve maintainability for future updates to the merge logic.
♻️ Example refactored approach
+ merge_platform_yml() { + local name="$1" + local platform="$2" + local x64="artifacts/deepchat-${platform}-x64/$name" + local arm64="artifacts/deepchat-${platform}-arm64/$name" + if [ -f "$x64" ] && [ -f "$arm64" ]; then + ruby -ryaml -e ' + x64 = YAML.load_file(ARGV[0]) || {} + arm = YAML.load_file(ARGV[1]) || {} + merged = x64.dup + merged["version"] ||= arm["version"] + merged["releaseDate"] ||= arm["releaseDate"] + merged["releaseNotes"] ||= arm["releaseNotes"] + merged["path"] ||= arm["path"] + merged["sha512"] ||= arm["sha512"] + files = [] + files.concat(x64["files"]) if x64["files"].is_a?(Array) + files.concat(arm["files"]) if arm["files"].is_a?(Array) + merged["files"] = files.uniq { |f| f["url"] } + File.write(ARGV[2], merged.to_yaml) + ' "$x64" "$arm64" "release_assets/$name" + elif [ -f "$x64" ]; then + cp "$x64" "release_assets/$name" + elif [ -f "$arm64" ]; then + cp "$arm64" "release_assets/$name" + fi + } + - merge_windows_yml() { - local name="$1" - local x64="artifacts/deepchat-win-x64/$name" - local arm64="artifacts/deepchat-win-arm64/$name" - ... - } - merge_windows_yml latest.yml - merge_windows_yml beta.yml + merge_platform_yml latest.yml win + merge_platform_yml beta.yml winThen replace
merge_mac_ymlcalls similarly.🤖 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 429 - 454, The merge_windows_yml function duplicates the YAML-merge logic in merge_mac_yml; extract the common Ruby/YAML merge logic into a single parameterized helper (e.g., merge_yml) that accepts the two source paths and output path/name, then have merge_windows_yml and merge_mac_yml call that helper with their platform-specific input directories (x64/arm64 or darwin/x64/arm64) so the Ruby snippet and file existence/copy handling are implemented once and reused.
124-124: ⚡ Quick winRefute invalid-runner concern for
windows-2025-vs2026
GitHub-hosted Windows runner labelwindows-2025-vs2026is a documented/available label (used for testing the Windows Server 2025 + Visual Studio 2026 image before the main migration). It shouldn’t cause the job to fail due to an unknown runner.
- If the intent is the default/stable Windows 2025 image, consider switching to
windows-2025/windows-latest; if the intent is to target the VS2026 image early, keep as-is.🤖 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 at line 124, The runner label "windows-2025-vs2026" in the workflow may be intentional for testing the VS2026 image; either leave the runner as-is if you want the pre-release Visual Studio 2026 image, or replace the runner value in the workflow definition with a stable label such as "windows-2025" or "windows-latest" to target the default/stable Windows 2025 image and avoid any unknown-runner concerns.
🤖 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.
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 429-454: The merge_windows_yml function duplicates the YAML-merge
logic in merge_mac_yml; extract the common Ruby/YAML merge logic into a single
parameterized helper (e.g., merge_yml) that accepts the two source paths and
output path/name, then have merge_windows_yml and merge_mac_yml call that helper
with their platform-specific input directories (x64/arm64 or darwin/x64/arm64)
so the Ruby snippet and file existence/copy handling are implemented once and
reused.
- Line 124: The runner label "windows-2025-vs2026" in the workflow may be
intentional for testing the VS2026 image; either leave the runner as-is if you
want the pre-release Visual Studio 2026 image, or replace the runner value in
the workflow definition with a stable label such as "windows-2025" or
"windows-latest" to target the default/stable Windows 2025 image and avoid any
unknown-runner concerns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fda9565d-6396-42c4-9229-adfe8f08921c
📒 Files selected for processing (5)
.github/workflows/build.yml.github/workflows/release.ymldocs/issues/windows-release-build-arch/plan.mddocs/issues/windows-release-build-arch/spec.mddocs/issues/windows-release-build-arch/tasks.md
✅ Files skipped from review due to trivial changes (2)
- docs/issues/windows-release-build-arch/tasks.md
- .github/workflows/build.yml
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
Documentation
Chores