Skip to content

Validate wheel ABI, linkage, and runtime compatibility - #473

Open
harsh21234i wants to merge 24 commits into
reflex-dev:mainfrom
harsh21234i:fix/wheel-abi-linkage-verification-448
Open

harsh21234i wants to merge 24 commits into
reflex-dev:mainfrom
harsh21234i:fix/wheel-abi-linkage-verification-448

Conversation

@harsh21234i

@harsh21234i harsh21234i commented Aug 6, 2026 •

Copy link
Copy Markdown

Fixes #448

Summary

  • Inspect ELF, Mach-O, and PE headers and verify the wheel's platform tag, architecture, and bitness.
  • Require the exported xy_abi_version symbol and validate Linux, macOS, and Windows linkage policy.
  • Include the glibc 2.17 system libraries used by Rust and the Windows bcryptprimitives.dll and ntdll.dll imports.
  • Run an install and representative native-kernel smoke test for every published native wheel.
  • Keep the release matrix and expect-artifacts contract aligned, and document the supported prebuilt wheel targets.

Local validation

  • python -m pytest tests/test_verify_wheel.py tests/test_release_process.py tests/test_kernels.py -q: 193 passed, 1 skipped.
  • python scripts/verify_ci_workflow.py: passed.
  • Ruff check/format and git diff --check: passed.
  • cargo test: 137 passed.
  • scripts/abi_smoke.py: 140 checks passed against the built Windows DLL.
  • Built xy-0.0.0-py3-none-win_amd64.whl; the verifier passed platform, ABI export, and linkage checks against the wheel and real DLL.
  • uvx reflex-release@0.1.0a4 sync --check, Ruff, and git diff --check: passed.

The native install/load matrix cannot run on this Windows development machine; the hosted release workflow must supply the cross-platform runtime proof. The docs-site pytest could not be collected locally because the docs environment dependency reflex_base is not installed.

Platform coverage

The release matrix retains all eleven previously published native wheel targets. Each wheel gets an install and native-kernel smoke on a matching native runner, with ARMv7 targets exercised in QEMU containers and musllinux wheels installed in Alpine. The artifact contract and installation/release documentation list the same complete target set.

Review in cubic

Summary by CodeRabbit

  • Release Improvements

    • Release packages target Linux glibc and musl (x86-64, aarch64, armv7), macOS Intel and Apple Silicon, and Windows x86, x64, and ARM64.
    • Packages are verified for platform compatibility, required native components, and system linkage before release.
    • Every package undergoes runtime smoke testing, including native NumPy kernel checks and installed-version validation.
  • Reliability

    • Improved detection of incompatible architectures, missing native symbols, and unsupported system library requirements.
    • Release checks now prevent packages built without native runtime support from being published.
  • Documentation

    • Updated platform support and release-readiness guidance to reflect the verified package targets.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
Walkthrough

The release workflow now builds three native wheel targets. Wheel validation parses ELF, Mach-O, and PE binaries, checks ABI exports and linkage, and runs installation smoke tests with NumPy and factorize_fixed.

Changes: Release wheel validation

Layer / File(s) Summary
Native binary parsing and linkage validation
scripts/verify_wheel.py, tests/test_verify_wheel.py
The verifier parses ELF, Mach-O, and PE metadata, checks architecture, exported ABI symbols, dependencies, and platform linkage. Tests cover compatible headers, wrong architectures, missing symbols, and invalid glibc requirements.
Wheel verifier and CI contract integration
scripts/verify_wheel.py, scripts/verify_ci_workflow.py, tests/test_verify_ci_workflow.py, spec/process/production-readiness.md
The verifier accepts platform, required-symbol, and linkage options. CI parses inline matrix mappings, requires these checks, and rejects release matrices containing native: false targets. The readiness checklist records the same requirements.
Release matrix and runtime smoke checks
.github/workflows/release.yml, scripts/wheel_smoke.py, spec/design-dossier.md, spec/process/production-readiness.md
The matrix retains native Linux x86-64, macOS arm64, and Windows x64 builds. Every wheel runs platform validation, installation, version, NumPy backend, and factorize_fixed checks. The specifications record the reduced runtime-verified matrix.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Workflow as release.yml
  participant Verifier as verify_wheel.py
  participant Wheel as Release wheel
  participant Runtime as Installed runtime
  Workflow->>Verifier: Validate wheel platform, ABI, and linkage
  Verifier->>Wheel: Inspect native binary
  Workflow->>Runtime: Install wheel and run factorize_fixed
  Runtime-->>Workflow: Return version and kernel results
Loading

Possibly related PRs

  • reflex-dev/xy#275: Both changes modify scripts/verify_wheel.py and release wheel validation.
  • reflex-dev/xy#278: Both changes modify release-workflow and CI verification guardrails for wheel publishing.
  • reflex-dev/xy#383: Both changes address platform-specific native availability and wheel smoke testing.

Suggested reviewers: alek99

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 32.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed The PR satisfies issue #448 by adding binary, ABI, linkage, runtime, publishing, and negative-fixture validation.
Out of Scope Changes check Passed The workflow, verifier, tests, smoke test, and documentation changes directly support issue #448 objectives.
Title check Passed The title clearly and concisely describes the main changes: validation of wheel ABI, linkage, and runtime compatibility.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the remaining previous finding is a non-blocking gap in the matrix test.

Findings

  1. P2 Missing smoke modes go unnoticed ▶

Summary

The PR adds platform, exported-symbol, and linkage checks for native wheels.

  • Native wheels are checked against their platform tags and linkage rules.
  • Every native wheel gets an installed runtime smoke on a matching environment.
  • Native-core errors now list the supported wheel targets.

The earlier non-ELF export, shared-library interpreter, macOS version comparison, option-check, and delay-load fixture threads were manually resolved without explanatory replies. The current code addresses those findings.

Reviews (11) · Last reviewed commit: "Merge upstream main and resolve release ..." · Reviewed by Greptile

Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (6)
scripts/verify_wheel.py (2)

970-995: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider catching struct.error in main.

The new parsers call struct.unpack_from on attacker- or build-controlled offsets. Any residual out-of-range read raises struct.error, which the except clause at line 996 does not cover. The verifier then exits with a traceback and a nonzero code from an unhandled exception, not the intended wheel verification failed message.

♻️ Proposed change
-    except (AssertionError, KeyError, zipfile.BadZipFile) as e:
+    except (AssertionError, KeyError, struct.error, zipfile.BadZipFile) as e:
🤖 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 `@scripts/verify_wheel.py` around lines 970 - 995, Update the exception
handling in main around verify_wheel to also catch struct.error raised by parser
offset reads, producing the existing “wheel verification failed” handling
instead of an uncaught traceback. Preserve the current handling for the
exceptions already covered.

227-243: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Guard the last symbol entry against a partial read.

symbol_length is not required to be a multiple of entry_size. If the table is ragged, the final iteration calls struct.unpack_from past the declared table end and can raise struct.error. main catches only AssertionError, KeyError, and zipfile.BadZipFile, so the verifier exits with a traceback instead of the intended failure message.

♻️ Proposed bound
-    for offset in range(symbol_offset, symbol_offset + symbol_length, entry_size):
+    for offset in range(symbol_offset, symbol_offset + symbol_length - symbol_size + 1, entry_size):
🤖 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 `@scripts/verify_wheel.py` around lines 227 - 243, Update the symbol-entry loop
around the symbol parsing logic to process only complete entries, ensuring each
iteration’s offset plus entry_size does not exceed symbol_offset +
symbol_length. Skip or stop before parsing a ragged final entry so
struct.unpack_from cannot read beyond the declared symbol table.
tests/test_verify_wheel.py (2)

108-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add positive export coverage for the three formats.

Every fixture here is a bare header. _elf_exported_symbols returns early at line 171 because the data is 20 bytes, _macho_exported_symbols returns early because ncommands is 0, and _pe_exported_symbols returns early because the data directory is out of range. The tests therefore never execute the section-table, symbol-table, or export-directory paths. The sh_link/sh_entsize defect flagged in scripts/verify_wheel.py lines 204-222 is invisible to this suite.

Add a fixture that carries a real SHT_DYNSYM section plus string table and assert that _require_exported_symbols accepts xy_abi_version. Add equivalent Mach-O LC_SYMTAB and PE export-directory fixtures.

🤖 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 `@tests/test_verify_wheel.py` around lines 108 - 146, Add positive
export-symbol coverage for ELF, Mach-O, and PE in the tests around
test_native_binary_header_matches_wheel_platform. Build fixtures containing a
valid ELF SHT_DYNSYM with its linked string table, a Mach-O LC_SYMTAB with the
exported symbol, and a PE export directory exposing xy_abi_version, then call
_require_exported_symbols and assert acceptance. Ensure the ELF fixture
exercises section-table symbol parsing, including valid sh_link and sh_entsize
values, rather than relying on the existing header-only binaries.

174-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The test name promises dependency-family coverage that the body does not assert.

test_elf_linkage_validates_glibc_floor_and_dependency_family asserts the glibc floor twice but never asserts a rejection for a dependency outside _ELF_ALLOWED_GLIBC, and never exercises the musllinux_ branch. Add a case with an unexpected DT_NEEDED name and a case with a musl interpreter, or rename the test to match its scope.

🤖 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 `@tests/test_verify_wheel.py` around lines 174 - 184, Expand
test_elf_linkage_validates_glibc_floor_and_dependency_family to assert rejection
of an unexpected DT_NEEDED dependency and to exercise the musllinux_ interpreter
branch, using the existing _elf_linkage_fixture and _require_elf_linkage
helpers. If dependency-family and musl coverage are not added, rename the test
to describe only glibc-floor validation.
.github/workflows/release.yml (1)

104-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Select the interpreter path explicitly instead of chaining with ||.

The || fallback runs the second interpreter whenever the first command exits nonzero, including when the assertions fail rather than when the interpreter is missing. On Linux and macOS a genuine assertion failure is then reported as a missing ./smoke/Scripts/python.exe, which hides the real cause. The 300-character probe is also duplicated verbatim, so the two copies can drift.

♻️ Proposed restructure
       - name: Verify the wheel installs and loads the native core
         # Every published wheel is built for the runner architecture and must
         # pass an import plus representative native-kernel call.
         shell: bash
         run: |
           uv venv smoke
           uv pip install -p smoke dist/*.whl numpy anywidget "reflex>=0.9.6"
-          ./smoke/bin/python -c "import importlib.metadata as m, numpy as np, reflex_xy, xy.kernels as k; assert k.BACKEND=='native', k.BACKEND; assert reflex_xy.__version__ == m.version('xy'); codes, unique = k.factorize_fixed(np.asarray(['a', 'b', 'a'], dtype='S1')); assert codes.tolist() == [0, 1, 0] and unique.tolist() == [0, 1]; print('native', k.__file__)" \
-            || ./smoke/Scripts/python.exe -c "import importlib.metadata as m, numpy as np, reflex_xy, xy.kernels as k; assert k.BACKEND=='native', k.BACKEND; assert reflex_xy.__version__ == m.version('xy'); codes, unique = k.factorize_fixed(np.asarray(['a', 'b', 'a'], dtype='S1')); assert codes.tolist() == [0, 1, 0] and unique.tolist() == [0, 1]; print('native', k.__file__)"
+          py=./smoke/bin/python
+          [ -x "$py" ] || py=./smoke/Scripts/python.exe
+          cat > smoke_probe.py <<'PY'
+          import importlib.metadata as m
+          import numpy as np
+          import reflex_xy
+          import xy.kernels as k
+          assert k.BACKEND == "native", k.BACKEND
+          assert reflex_xy.__version__ == m.version("xy")
+          codes, unique = k.factorize_fixed(np.asarray(["a", "b", "a"], dtype="S1"))
+          assert codes.tolist() == [0, 1, 0]
+          assert unique.tolist() == [0, 1]
+          print("native", k.__file__)
+          PY
+          "$py" smoke_probe.py

If you move the probe into a checked-in script such as scripts/wheel_smoke.py, update the required substrings in scripts/verify_ci_workflow.py lines 1274-1278 and the fixture string in tests/test_verify_ci_workflow.py lines 1307-1310.

🤖 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 104 - 108, Update the smoke-test
step to select the platform-appropriate interpreter explicitly rather than
chaining duplicate probes with ||, so assertion failures remain visible and only
the valid executable is invoked. Extract the shared validation into a checked-in
script such as scripts/wheel_smoke.py, and update the corresponding
required-substring checks in verify_ci_workflow.py and its test fixture to match
the new workflow content.
scripts/verify_ci_workflow.py (1)

1282-1286: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The native: false guard now watches a matrix key that no step reads.

.github/workflows/release.yml removed the if: matrix.native condition from the smoke step, so matrix.native is unused metadata. This check passes as long as nobody writes the exact text native: false, and it does not verify that the smoke step actually runs for each entry. A future entry added without the native key at all bypasses the guard.

Prefer one of two options. Remove the native key from the matrix and this guard, since the smoke step is now unconditional. Or keep the key and assert that the smoke step carries no if: condition.

🤖 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 `@scripts/verify_ci_workflow.py` around lines 1282 - 1286, Update the wheels
workflow validation around wheels_job and the release matrix so it matches the
unconditional smoke step: remove the obsolete native matrix key and the native:
false guard, or retain that metadata while validating that the smoke step has no
if: condition. Ensure entries missing native cannot bypass the intended
smoke-step coverage check.
🤖 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 `@scripts/verify_wheel.py`:
- Around line 970-995: Update scripts/verify_wheel.py at lines 970-995 to catch
struct.error alongside the existing exceptions in main, preserving the standard
verification-failure message and exit code 1. At lines 227-243, bound the ELF
symbol loop to process only complete Elf*_Sym entries when symbol_length is not
a multiple of entry_size. At lines 584-589, validate that len(data) is at least
0x40 before reading e_lfanew at offset 0x3C.
- Around line 137-149: Update the Mach-O endianness selection in the detection
branch around `_macho_exported_symbols` so both big-endian magics,
`\xfe\xed\xfa\xce` and `\xfe\xed\xfa\xcf`, select `">"`, matching
`_require_macho_linkage`; preserve little-endian handling for the corresponding
reversed magics.
- Around line 929-946: Update the wheel verification flow around the
native-library checks to reject require_linkage when expect_platform is None
with an explicit assertion or validation error. Open the ZipFile once, read
native_libs[0] once, and reuse the extracted bytes for _require_native_target,
_require_exported_symbols, and the platform-specific linkage helper while
preserving the existing platform dispatch.
- Around line 428-435: Remove the ELF-only format guard from
_require_exported_symbols and validate required against info.exported_symbols
for ELF, Mach-O, and PE, using the existing _macho_exported_symbols and
_pe_exported_symbols paths. Extend the related tests to cover successful
required-symbol matches for all three formats.
- Around line 204-222: Store section_link and section_entry_size as separate
tuple fields in the sections collection, update its annotation and all
unpacking/indexing in the dynamic-symbol handling flow accordingly, and use
sh_link only for string_index and sh_entsize for entry_size. Add a fixture
containing a valid ELF section header and dynamic symbol table so
_require_exported_symbols validates a genuine binary without the false
invalid-table assertion.

---

Nitpick comments:
In @.github/workflows/release.yml:
- Around line 104-108: Update the smoke-test step to select the
platform-appropriate interpreter explicitly rather than chaining duplicate
probes with ||, so assertion failures remain visible and only the valid
executable is invoked. Extract the shared validation into a checked-in script
such as scripts/wheel_smoke.py, and update the corresponding required-substring
checks in verify_ci_workflow.py and its test fixture to match the new workflow
content.

In `@scripts/verify_ci_workflow.py`:
- Around line 1282-1286: Update the wheels workflow validation around wheels_job
and the release matrix so it matches the unconditional smoke step: remove the
obsolete native matrix key and the native: false guard, or retain that metadata
while validating that the smoke step has no if: condition. Ensure entries
missing native cannot bypass the intended smoke-step coverage check.

In `@scripts/verify_wheel.py`:
- Around line 970-995: Update the exception handling in main around verify_wheel
to also catch struct.error raised by parser offset reads, producing the existing
“wheel verification failed” handling instead of an uncaught traceback. Preserve
the current handling for the exceptions already covered.
- Around line 227-243: Update the symbol-entry loop around the symbol parsing
logic to process only complete entries, ensuring each iteration’s offset plus
entry_size does not exceed symbol_offset + symbol_length. Skip or stop before
parsing a ragged final entry so struct.unpack_from cannot read beyond the
declared symbol table.

In `@tests/test_verify_wheel.py`:
- Around line 108-146: Add positive export-symbol coverage for ELF, Mach-O, and
PE in the tests around test_native_binary_header_matches_wheel_platform. Build
fixtures containing a valid ELF SHT_DYNSYM with its linked string table, a
Mach-O LC_SYMTAB with the exported symbol, and a PE export directory exposing
xy_abi_version, then call _require_exported_symbols and assert acceptance.
Ensure the ELF fixture exercises section-table symbol parsing, including valid
sh_link and sh_entsize values, rather than relying on the existing header-only
binaries.
- Around line 174-184: Expand
test_elf_linkage_validates_glibc_floor_and_dependency_family to assert rejection
of an unexpected DT_NEEDED dependency and to exercise the musllinux_ interpreter
branch, using the existing _elf_linkage_fixture and _require_elf_linkage
helpers. If dependency-family and musl coverage are not added, rename the test
to describe only glibc-floor validation.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5010e04b-b00c-4ee4-af95-07c3ac3cdbb7

📥 Commits

Reviewing files that changed from the base of the PR and between 37c3d91 and d99508f.

📒 Files selected for processing (5)
  • .github/workflows/release.yml
  • scripts/verify_ci_workflow.py
  • scripts/verify_wheel.py
  • tests/test_verify_ci_workflow.py
  • tests/test_verify_wheel.py

Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_ci_workflow.py Resolved
Comment thread scripts/verify_wheel.py Resolved
Comment thread tests/test_verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@scripts/verify_ci_workflow.py`:
- Around line 1290-1297: Update the release wheel validation around
_matrix_include_entries and reject an empty matrix_entries list before
evaluating native flags. Preserve the existing failure for any entry whose
native value is not true, ensuring workflows using unsupported matrix forms
cannot bypass runtime verification.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 18a4319a-e011-47ef-b8ac-caee81cd0dc2

📥 Commits

Reviewing files that changed from the base of the PR and between d99508f and 911ea18.

📒 Files selected for processing (3)
  • scripts/verify_ci_workflow.py
  • scripts/verify_wheel.py
  • tests/test_verify_wheel.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/test_verify_wheel.py

Comment thread scripts/verify_ci_workflow.py Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@scripts/wheel_smoke.py`:
- Around line 12-16: Replace the bare assertions in the smoke-test checks with
explicit conditionals that raise an appropriate exception when the backend,
version, codes, or unique values do not match the expected results. Update the
checks around kernels.BACKEND, reflex_xy.__version__, and factorize_fixed so
validation remains active under optimized Python execution.

In `@spec/process/production-readiness.md`:
- Line 390: Update the release checklist and design-dossier platform matrix to
match release.yml: retain only Linux x86-64, macOS arm64, Windows x64, and the
separate PyEmscripten wheel, and remove aarch64, armv7, musllinux, macOS x86-64,
Windows x86, and Windows arm64 entries.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 55d7d25c-508c-4b91-aeca-04acf32fb20f

📥 Commits

Reviewing files that changed from the base of the PR and between 983c244 and a31c662.

📒 Files selected for processing (6)
  • .github/workflows/release.yml
  • scripts/verify_ci_workflow.py
  • scripts/verify_wheel.py
  • scripts/wheel_smoke.py
  • spec/process/production-readiness.md
  • tests/test_verify_wheel.py
🚧 Files skipped from review as they are similar to previous changes (4)
  • .github/workflows/release.yml
  • scripts/verify_ci_workflow.py
  • scripts/verify_wheel.py
  • tests/test_verify_wheel.py

Comment thread scripts/wheel_smoke.py Resolved
Comment thread spec/process/production-readiness.md Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 6 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/verify_ci_workflow.py Resolved
Comment thread tests/test_verify_wheel.py Resolved
Comment thread scripts/wheel_smoke.py Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/verify_ci_workflow.py">

<violation number="1" location="scripts/verify_ci_workflow.py:580">
P1: The release validator can accept a workflow that does not execute `scripts/wheel_smoke.py`: the new check only looks for its text inside active run lines, so an `echo` or other non-invoking command passes. Requiring the smoke command as an exact executable line, preferably through a hard-gated exact-command step or shell control-flow validation, would preserve the native runtime publication gate.</violation>

<violation number="2" location="scripts/verify_ci_workflow.py:1299">
P3: An invalid wheel smoke gate is reported twice because the same `_require_step_run_contains` check is duplicated; removing the second invocation keeps one actionable diagnostic instead of duplicate errors.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/verify_ci_workflow.py Resolved
Comment thread scripts/verify_ci_workflow.py Resolved

@FarhanAliRaza FarhanAliRaza left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I built the PR head and ran the new gate against the wheel. The header and ABI parsing works. The linkage policy rejects the wheel the release job actually builds, and the branch no longer applies to main.

Required changes (details inline):

  1. glibc allowlist fails the CI wheel. A cargo-zigbuild --target x86_64-unknown-linux-gnu.2.17 build, packed exactly as the release job packs it, fails --require-linkage with unsupported glibc dependencies: ['libdl.so.2', 'libpthread.so.0']. Use the PEP 599 manylinux2014 library set.
  2. Windows allowlist probably misses bcryptprimitives.dll. Rust std imports it for the default-hasher HashMap that the core uses. Check the import table of a real win_amd64 build and allowlist what it needs.
  3. Merge main. release.yml is now build_release_artifacts.yml, and validate_release_workflow moved out of verify_ci_workflow.py. There are four conflicting files.
  4. Dropping 8 of 11 wheel targets needs explicit maintainer sign-off, or runtime legs on ubuntu-24.04-arm, windows-11-arm, Alpine containers or QEMU. The dossier §F1 edits currently turn a CI gap into product scope.
  5. Ruff fails (RUF043, and three files need reformatting). Also remove the duplicated _require_step_run_contains block.

What works:

  • pytest tests/test_verify_wheel.py tests/test_verify_ci_workflow.py → 157 passed.
  • verify_ci_workflow.py passes on the PR head.
  • The glibc floor check correctly rejects a wheel built against host glibc 2.39 but tagged manylinux_2_17 (requires glibc 2.35).
  • xy_abi_version is found in the ELF dynsym.
  • scripts/wheel_smoke.py passes in a clean venv with the zig-built wheel installed.
  • A 200k-point categorical scatter exported from that installed wheel renders correctly in headless Chromium, with no console errors.

No news fragment is needed, because the diff touches no files under python/, src/ or js/.


Generated by Claude Code

Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved
Comment thread .github/workflows/release.yml Resolved
Comment thread scripts/verify_ci_workflow.py Resolved
Comment thread tests/test_verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 11 files

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread news/448.bugfix.md Resolved
Comment thread tests/test_verify_wheel.py Resolved
Comment thread scripts/wheel_smoke.py Resolved
Comment thread docs/overview/installation.md Resolved
Comment thread docs/overview/installation.md Resolved
Comment thread scripts/verify_wheel.py Resolved
Comment thread scripts/verify_wheel.py Resolved
Comment thread spec/design-dossier.md Resolved
Comment thread docs/overview/installation.md Resolved
Comment thread python/xy/kernels.py Resolved
Comment thread tests/test_verify_wheel.py Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 9 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread news/448.breaking.md Resolved
Comment thread spec/design-dossier.md Resolved
Comment thread tests/test_verify_wheel.py Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files (changes from recent commits).

Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

View guided diff | Re-trigger cubic

Comment thread docs/overview/installation.md Resolved
Comment thread docs/overview/installation.md Resolved
Comment thread docs/app/tests/test_docs_site.py Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread spec/design-dossier.md Resolved
Comment thread docs/app/tests/test_docs_site.py Resolved
Comment thread python/xy/kernels.py Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 7 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread tests/test_release_process.py Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread docs/app/tests/test_docs_site.py Resolved
Comment on lines +222 to +223
smoke_modes = set(re.findall(r"\bsmoke: ([A-Za-z0-9-]+)", "\n".join(wheel_entries)))
assert smoke_modes == {"host", "manylinux-armv7", "alpine", "alpine-armv7"}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing smoke modes go unnoticed

The new smoke_modes set checks which modes appear anywhere, but no longer checks that every wheel entry has one. Removing smoke: host from one target still passes because other targets retain it. That target then skips every smoke step and still uploads its wheel.

Keep the per-entry assertion alongside the set check so future matrix edits cannot silently remove a wheel’s install test.

Suggested change
smoke_modes = set(re.findall(r"\bsmoke: ([A-Za-z0-9-]+)", "\n".join(wheel_entries)))
assert smoke_modes == {"host", "manylinux-armv7", "alpine", "alpine-armv7"}
assert all(re.search(r"\bsmoke: [A-Za-z0-9-]+", line) for line in wheel_entries), wheel_entries
smoke_modes = set(re.findall(r"\bsmoke: ([A-Za-z0-9-]+)", "\n".join(wheel_entries)))
assert smoke_modes == {"host", "manylinux-armv7", "alpine", "alpine-armv7"}

Knowledge Base Used: Build, packaging, and release validation

@codspeed

codspeed Bot commented Oct 27, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

1 warning, 1 note

⚠️ 13 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

ℹ️ 109 benchmarks have no flamegraph

The flamegraph wasn't generated: the symbols of the profiling trace could not be resolved, because no information about the language frames was collected.

See affected benchmarks

✅ 109 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing harsh21234i:fix/wheel-abi-linkage-verification-448 (6649f5b) with main (6dfae96)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validate architecture, exported ABI, and dynamic linkage for every release wheel

2 participants