fix(ci): fix broken releases - #417
Merged
Merged
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Ryan Millard <142347829+Ryan-Millard@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Ryan Millard <142347829+Ryan-Millard@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Ryan Millard <142347829+Ryan-Millard@users.noreply.github.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 18 minutes and 45 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: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds MSVC compiler compatibility across the C++ core and adjusts build configuration. The changes convert struct packing from GCC/Clang-only attributes to compiler-conditional pragmas, supply missing math constants for MSVC, disable example builds, and refine npm release authentication. ChangesMSVC Compiler Compatibility
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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
added
github_actions
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/src/internal/bilateral_filter_gpu.cpp (1)
20-39: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd compile-time layout checks for
FilterParamsuniform ABI
FilterParamsis packed via#pragma pack(push, 1)/__attribute__((packed))and then used directly for the uniform buffer (bufDesc.size = sizeof(FilterParams)andWriteBuffer(..., sizeof(FilterParams))). Add compile-timesizeof/offsetofguards so host/shader layout can’t silently drift.Suggested guard rails
struct FilterParams { float sigmaSpatial; float sigmaRange; // std140 requires 16-byte alignment for structs/vec4s. // Two floats = 8 bytes. We likely need padding if this struct grows, // but for a standalone bind group of 2 floats, standard alignment usually suffices. // However, it is safer to pad to 16 bytes to be sure. float _pad1; float _pad2; } `#ifndef` _MSC_VER __attribute__((packed)) `#endif` ; `#ifdef` _MSC_VER `#pragma` pack(pop) `#endif` + +static_assert(sizeof(FilterParams) == 16, + "FilterParams must stay 16 bytes to match the WGSL uniform layout"); +static_assert(offsetof(FilterParams, sigmaSpatial) == 0, "Unexpected sigmaSpatial offset"); +static_assert(offsetof(FilterParams, sigmaRange) == 4, "Unexpected sigmaRange offset"); +static_assert(offsetof(FilterParams, _pad1) == 8, "Unexpected _pad1 offset"); +static_assert(offsetof(FilterParams, _pad2) == 12, "Unexpected _pad2 offset");🤖 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 `@core/src/internal/bilateral_filter_gpu.cpp` around lines 20 - 39, Add compile-time layout guards for FilterParams by inserting static_asserts that verify sizeof(FilterParams) matches the expected uniform size (16 bytes) and that offsetof(FilterParams, sigmaSpatial), offsetof(FilterParams, sigmaRange), etc. match the byte offsets the shader expects; this prevents bufDesc.size = sizeof(FilterParams) and the subsequent WriteBuffer(..., sizeof(FilterParams)) from silently using a mismatched layout if the struct changes. Reference the FilterParams type and use sizeof and offsetof in the assertions; update the expected constants if the shader/UAV layout changes.
🤖 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 `@core/src/internal/kmeans_gpu.cpp`:
- Around line 29-76: Add compile-time ABI layout checks for the GPU-facing
structs Params, ClusterAccumulator, and CentroidParams: include <cstddef> and
add static_asserts that verify sizeof(Params), sizeof(ClusterAccumulator), and
sizeof(CentroidParams) match the expected packed sizes and use static_assert
with offsetof to verify offsets of each field (e.g., offsetof(Params,
numPoints), offsetof(ClusterAccumulator, sumR), offsetof(CentroidParams,
r/g/width)) to ensure host↔shader layout compatibility; place these assertions
near the struct definitions so any layout drift fails at compile time.
---
Outside diff comments:
In `@core/src/internal/bilateral_filter_gpu.cpp`:
- Around line 20-39: Add compile-time layout guards for FilterParams by
inserting static_asserts that verify sizeof(FilterParams) matches the expected
uniform size (16 bytes) and that offsetof(FilterParams, sigmaSpatial),
offsetof(FilterParams, sigmaRange), etc. match the byte offsets the shader
expects; this prevents bufDesc.size = sizeof(FilterParams) and the subsequent
WriteBuffer(..., sizeof(FilterParams)) from silently using a mismatched layout
if the struct changes. Reference the FilterParams type and use sizeof and
offsetof in the assertions; update the expected constants if the shader/UAV
layout changes.
🪄 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: 7eb926be-1e71-419a-830f-fa8c711e48d4
📒 Files selected for processing (10)
.github/workflows/release.ymlcore/include/internal/LABAPixel.hcore/include/internal/LABPixel.hcore/include/internal/RGBAPixel.hcore/include/internal/RGBPixel.hcore/src/internal/bilateral_filter_gpu.cppcore/src/internal/contours.cppcore/src/internal/image_utils.cppcore/src/internal/kmeans_gpu.cpppyproject.toml
📜 Review details
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build C/C++ / Build C & C++
- GitHub Check: Build C/C++ / Build Python
- GitHub Check: Build C/C++ / Build WASM (bindings/js)
- GitHub Check: Build Documentation Site / Build Docusaurus Site
- GitHub Check: Lint & Validate Code
- GitHub Check: Analyze (c-cpp)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{c,cc,cpp,cxx,h,hpp,hxx}
📄 CodeRabbit inference engine (.clang-format)
**/*.{c,cc,cpp,cxx,h,hpp,hxx}: Follow Google style guide for C/C++ code formatting
Use 4 spaces for indentation
Maintain a column limit of 100 characters per line
Do not allow short functions on a single line
Files:
core/include/internal/RGBAPixel.hcore/include/internal/LABPixel.hcore/src/internal/image_utils.cppcore/src/internal/contours.cppcore/include/internal/LABAPixel.hcore/src/internal/bilateral_filter_gpu.cppcore/include/internal/RGBPixel.hcore/src/internal/kmeans_gpu.cpp
**/*.{hpp,cpp,c,h}
📄 CodeRabbit inference engine (.editorconfig)
**/*.{hpp,cpp,c,h}: Use 4-space indentation for C/C++ files
Maintain 120 character maximum line length for C/C++ files
Files:
core/include/internal/RGBAPixel.hcore/include/internal/LABPixel.hcore/src/internal/image_utils.cppcore/src/internal/contours.cppcore/include/internal/LABAPixel.hcore/src/internal/bilateral_filter_gpu.cppcore/include/internal/RGBPixel.hcore/src/internal/kmeans_gpu.cpp
core/**/*.{cpp,c,h,hpp}
⚙️ CodeRabbit configuration file
core/**/*.{cpp,c,h,hpp}: This is the Img2Num core C/C++ library. Review for:
- Memory safety: null pointer dereferences, use-after-free, buffer overflows.
- Correct RAII usage and smart pointer idioms.
- Adherence to the .clang-format style; formatting must be applied via
./img2num format-cpp(or./img2num format-wasmfor WASM modules),
NOT by calling clang-format directly.- When suggesting build/test steps, always use the Docker-first wrapper scripts
(./img2num,img2num.ps1, orimg2num.bat) rather than direct tool invocations,
since contributors may not have dependencies installed locally.- Must have Doxygen docstrings in the API (
img2num.h) files. This is important!
Files:
core/include/internal/RGBAPixel.hcore/include/internal/LABPixel.hcore/src/internal/image_utils.cppcore/src/internal/contours.cppcore/include/internal/LABAPixel.hcore/src/internal/bilateral_filter_gpu.cppcore/include/internal/RGBPixel.hcore/src/internal/kmeans_gpu.cpp
**/*.{cpp,c,h,hpp}
⚙️ CodeRabbit configuration file
**/*.{cpp,c,h,hpp}: - For any C/C++ code outside core/ (e.g. bindings, example-apps), apply the same
memory-safety and style standards as the core. Formatting via./img2num format-cpp.
- Doxygen docstrings are required.
Files:
core/include/internal/RGBAPixel.hcore/include/internal/LABPixel.hcore/src/internal/image_utils.cppcore/src/internal/contours.cppcore/include/internal/LABAPixel.hcore/src/internal/bilateral_filter_gpu.cppcore/include/internal/RGBPixel.hcore/src/internal/kmeans_gpu.cpp
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: GitHub Actions workflows. Review for:
- SHA-pinned action versions for third-party actions (security best practice).
- Secrets accessed only via ${{ secrets.* }} — never hardcoded.
- Least-privilege permissions on each job/workflow.
- Correct job dependency ordering (needs:) and if/condition logic.
Files:
.github/workflows/release.yml
🧠 Learnings (9)
📚 Learning: 2026-02-25T21:24:34.055Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 272
File: core/include/internal/bilateral_filter_gpu.h:108-109
Timestamp: 2026-02-25T21:24:34.055Z
Learning: In WGSL shaders, .rgb swizzle on a vec4 returns the first three components regardless of color space. Do not assume data is RGB color space when the texture contains non-RGB data (e.g., CIELAB L, A, B). If LAB data is stored, treat and compute distances in LAB space, not RGB. Clearly document shader code paths that rely on specific color spaces and prefer explicit conversions or comments when using swizzled components with non-RGB textures. Apply this guidance to WGSL shader files across the codebase (not just this header) when handling color data or color-like channels.
Applied to files:
core/include/internal/RGBAPixel.hcore/include/internal/LABPixel.hcore/include/internal/LABAPixel.hcore/include/internal/RGBPixel.h
📚 Learning: 2026-04-09T19:05:40.514Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 302
File: core/include/internal/gpu.h:203-216
Timestamp: 2026-04-09T19:05:40.514Z
Learning: In Img2Num’s internal GPU initialization code, if querying adapter limits fails (e.g., `adapter.GetLimits(...)` returns `false`), treat this as a GPU initialization failure and route execution to the CPU fallback path. Do not continue with default limits in this case; the failure should cause the same fallback behavior as other GPU init errors.
Applied to files:
core/include/internal/RGBAPixel.hcore/include/internal/LABPixel.hcore/include/internal/LABAPixel.hcore/include/internal/RGBPixel.h
📚 Learning: 2025-12-31T17:46:54.476Z
Learnt from: fransafu
Repo: Ryan-Millard/Img2Num PR: 176
File: src/wasm/modules/image/src/bilateral_filter.cpp:98-101
Timestamp: 2025-12-31T17:46:54.476Z
Learning: In bilateral_filter.cpp (src/wasm/modules/image/src/bilateral_filter.cpp), rely on the fact that the center pixel contributes a weight of exactly 1.0 to both spatial and range components (exp(0) = 1) so the normalization sum cannot be zero. This implies you do not need explicit guards against division by zero for normalization in this implementation.
Applied to files:
core/src/internal/image_utils.cppcore/src/internal/contours.cppcore/src/internal/bilateral_filter_gpu.cppcore/src/internal/kmeans_gpu.cpp
📚 Learning: 2026-01-06T04:56:57.269Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 191
File: src/wasm/modules/image/src/kmeans.cpp:128-134
Timestamp: 2026-01-06T04:56:57.269Z
Learning: In C++ sources compiled for WASM with Emscripten/Clang, designated initializers (e.g., RGBXY{.r = ..., .g = ...}) are allowed as a C++17 extension. When reviewing code that relies on designated initializers, verify that the target toolchain enables CXX_STANDARD 17 or higher and that the build system (CMake/emsdk) uses Emscripten with a compatible clang. If not, avoid such initializers or provide portable alternatives.
Applied to files:
core/src/internal/image_utils.cppcore/src/internal/contours.cppcore/src/internal/bilateral_filter_gpu.cppcore/src/internal/kmeans_gpu.cpp
📚 Learning: 2026-01-06T21:06:24.476Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 191
File: src/wasm/modules/image/src/cielab.cpp:43-57
Timestamp: 2026-01-06T21:06:24.476Z
Learning: In the Img2Num project, prefer recommending and using the provided docker/script wrappers (e.g., ./img2num format-wasm, ./img2num clean-wasm) instead of invoking local tools directly (e.g., clang-format -i). This reduces dependency requirements for users and ensures consistent tooling across environments. Apply this guidance to C++ source files under the project when reviewing changes.
Applied to files:
core/src/internal/image_utils.cppcore/src/internal/contours.cppcore/src/internal/bilateral_filter_gpu.cppcore/src/internal/kmeans_gpu.cpp
📚 Learning: 2026-01-19T00:02:34.957Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 234
File: src/wasm/modules/image/src/kmeans.cpp:79-81
Timestamp: 2026-01-19T00:02:34.957Z
Learning: In C++ multithreading contexts, prefer unsigned types for thread-count-like parameters (e.g., n_threads) to prevent negative values. Validate that the value is at least 1 before any division to avoid divide-by-zero at runtime. At the start of functions handling such values, create a safe count like: const unsigned int thread_count{std::max(1u, n_threads)}; This ensures non-negative, non-zero usage for divisions and related arithmetic. Apply this pattern to similar parameters across C++ modules, not just the specific file.
Applied to files:
core/src/internal/image_utils.cppcore/src/internal/contours.cppcore/src/internal/bilateral_filter_gpu.cppcore/src/internal/kmeans_gpu.cpp
📚 Learning: 2026-02-25T21:24:19.036Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 272
File: core/src/internal/bilateral_filter_gpu.cpp:171-185
Timestamp: 2026-02-25T21:24:19.036Z
Learning: Adopt brace initialization (e.g., Type var{}) over copy initialization (e.g., Type var = {}) for zero-initialization in C++ across the repository. This should apply to variables, arrays, and structs in most C++ files (e.g., core/src/internal/bilateral_filter_gpu.cpp). It improves safety by avoiding narrowing conversions and makes initialization intent explicit.
Applied to files:
core/src/internal/image_utils.cppcore/src/internal/contours.cppcore/src/internal/bilateral_filter_gpu.cppcore/src/internal/kmeans_gpu.cpp
📚 Learning: 2026-05-01T22:50:11.527Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 339
File: release-please-config.json:18-47
Timestamp: 2026-05-01T22:50:11.527Z
Learning: In this repo, release-please-action v4 preserves '/' verbatim in slash-containing path-based package keys when emitting GitHub Actions output names (e.g., `bindings/c--release_created`). When referencing these step outputs in `job.outputs` (and other expressions), use bracket notation with the exact output name: `${{ steps.release.outputs['bindings/c--release_created'] }}` rather than dot notation. If needed, map the complex step output to a clean job-level output alias so downstream jobs can use dot notation via that alias.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2026-05-19T17:30:09.565Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 375
File: .github/workflows/cmake-build.yml:86-88
Timestamp: 2026-05-19T17:30:09.565Z
Learning: In Ryan-Millard/Img2Num CI/workflow YAMLs, any `uv sync` command used for the Python package build must include `--no-build-isolation` (do not remove it). If you need deterministic dependency installs for CI, you may add `--frozen` alongside it (e.g., `uv sync --frozen --no-build-isolation`), and it should not conflict with the repo’s build setup.
Applied to files:
.github/workflows/release.yml
🔇 Additional comments (8)
core/include/internal/LABAPixel.h (1)
8-42: LGTM!core/include/internal/LABPixel.h (1)
16-57: LGTM!core/include/internal/RGBAPixel.h (1)
8-42: LGTM!core/include/internal/RGBPixel.h (1)
11-53: LGTM!core/src/internal/contours.cpp (1)
10-13: LGTM!core/src/internal/image_utils.cpp (1)
16-19: LGTM!pyproject.toml (1)
44-44: LGTM!Also applies to: 53-54
.github/workflows/release.yml (1)
151-152: LGTM!
Owner
Author
|
@coderabbitai please suggest a good commit message for this.
|
Contributor
|
Tip For best results, initiate chat on the files or code changes.
A few notes:
|
This was referenced May 29, 2026
This was referenced Jun 21, 2026
This was referenced Jul 30, 2026
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.
What was changed & why
v0.1.0 released very poorly - not even published to npm or pypi
This fixed that
Fixes: none
Changes
third_party/for Python buildsTesting & Verification
Additional Resources