Skip to content

Distribute single binary with bun#942

Closed
tomusdrw wants to merge 19 commits into
mainfrom
td-bun-migration
Closed

Distribute single binary with bun#942
tomusdrw wants to merge 19 commits into
mainfrom
td-bun-migration

Conversation

@tomusdrw
Copy link
Copy Markdown
Member

@tomusdrw tomusdrw commented Apr 7, 2026

Related #941

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 7, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2027e153-8c0f-480d-b92c-481d20c6c353

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds Bun-compiled standalone binary support and release packaging: a new CI job builds platform-specific binaries (darwin-arm64 and linux-x64-gnu), runs a smoke test, and uploads zip artifacts to releases; release notes are updated to include a "Standalone Binaries" section. A new build script (bin/jam/build-for-bun.sh) compiles the app with Bun, bundles native .node addons, writes version info, and zips the distribution. A Bun entry script (bin/jam/bun-entry.ts) patches module resolution and dlopen to load sibling native addons, then imports the app. Worker bootstrap selection is made Bun-aware by adding WORKER_BUN exports in several worker packages and rewriting worker URLs when running under Bun. package.json gains a trustedDependencies entry for protobufjs.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • mateuszsikora
  • DrEverr

Poem

🐰 I hopped through code, built with Bun delight,
Packaged nodes and zips for day and night,
I patched the loader, scoped the native file,
Now binaries bounce across each platform mile,
Cheers — a rabbit’s nibble, compact and spry! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Distribute single binary with bun' accurately and concisely summarizes the main objective of the changeset, which adds Bun-based binary distribution capabilities.
Description check ✅ Passed The description 'Related #941' is minimal but directly related to the changeset, providing context by linking to the associated issue.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch td-bun-migration

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/publish.yml (1)

179-201: ⚠️ Potential issue | 🟡 Minor

Make the release-notes update idempotent.

append_body: true already preserves the current release notes. Interpolating ${{ github.event.release.body }} into the appended block duplicates the existing body on the first run, and reruns append another copy of the standalone/docker/npm section again.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish.yml around lines 179 - 201, The body block passed
to softprops/action-gh-release@v2 currently includes interpolation of ${{
github.event.release.body }} and also sets append_body: true, which causes
duplicate release notes on reruns; remove the interpolation of ${{
github.event.release.body }} from the body field and let append_body: true
handle preserving the existing release notes so only the new
standalone/docker/npm section is appended (update the body value in the action
that defines body: | and keep append_body: true).
🧹 Nitpick comments (2)
.github/workflows/publish.yml (1)

152-155: Exercise at least one worker path in the binary smoke test.

./dist/jam-bun/jam --help proves the executable starts, but it won't touch the new WORKER_BUN selection in packages/jam/node/workers.ts or most of the .node redirection in bin/jam/bun-entry.ts. A minimal command that spawns a worker would catch the PR's highest-risk runtime failure before upload.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish.yml around lines 152 - 155, The smoke test only
checks that the binary starts but doesn't exercise the worker code paths; update
the workflow step that runs ./dist/jam-bun/jam to invoke a command that spawns a
Bun worker so WORKER_BUN in packages/jam/node/workers.ts and the .node
redirection in bin/jam/bun-entry.ts are executed (for example run a lightweight
subcommand or flag that creates a worker and exits). Ensure the chosen
subcommand is non-interactive and deterministic so the CI step completes quickly
and reliably while touching the worker path.
packages/jam/node/workers.ts (1)

11-17: Use an explicit property read for Bun detection.

"Bun" in globalThis is the in-based runtime detection pattern the repo asks us to avoid. A direct property read keeps the same behavior without adding another type probe.

♻️ Proposed fix
-const isBun = "Bun" in globalThis;
+const isBun = (globalThis as typeof globalThis & { Bun?: unknown }).Bun !== undefined;

As per coding guidelines, "Avoid instanceof and use explicit checks rather than type detection via in operator."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/jam/node/workers.ts` around lines 11 - 17, Replace the `in`-based
runtime detection for Bun with an explicit property read: update the isBun
variable (used by workerUrl) to check the Bun property on globalThis via an
explicit undefined/type check (so it does not throw) and coerce to boolean; keep
workerUrl unchanged but rely on the new isBun value. Ensure you reference the
isBun variable and workerUrl function when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/publish.yml:
- Around line 138-142: Pin the Bun setup action and enforce frozen dependency
resolution: update the "Setup Bun" step that uses oven-sh/setup-bun to reference
an exact, immutable tag (replace the floating `@v2` with a specific release tag)
and change the "Install dependencies" step (currently running `bun install`) to
run with the frozen-lockfile flag (use `bun install --frozen-lockfile`) so
builds use the committed lockfile and the toolchain is pinned.

In `@bin/jam/build-for-bun.sh`:
- Around line 9-12: The VERSION lookup and the `cd ../..` are resolving from the
caller's cwd; change both to resolve paths relative to the script location
instead: compute the script directory (referenced in this file as the context
for VERSION and the subsequent cd) and use that base when invoking bun to read
package.json for VERSION and when changing directories (replace the current
`VERSION=$(bun -e "console.log(require('./package.json').version)")` and `cd
../..` usage with versions that reference the computed script directory variable
so the script always reads the correct package.json and cd's into the intended
repo location).
- Around line 21-23: The bun build invocation currently compiles for the CI host
instead of the intended $PLATFORM; add a small mapping from $PLATFORM to a bun
compile target (e.g., map darwin-arm64 -> bun-darwin-arm64, linux-x64 ->
bun-linux-x64) and pass it to bun via --target "$TARGET" in the existing bun
build --compile ... --outfile "$DIST_FOLDER/jam" ./bin/jam/bun-entry.ts command;
implement the mapping as a case/switch before the bun build and validate TARGET
is set (exit with an error if unknown) so the produced binary matches the staged
native addons and zip naming.

In `@bin/jam/bun-entry.ts`:
- Around line 3-5: The build imports "reflect-metadata" directly in
bin/jam/bun-entry.ts but doesn't declare it as a dependency in
bin/jam/package.json, relying on a fragile transitive chain; add
"reflect-metadata" to the dependencies section of bin/jam/package.json (pick a
version consistent with your repo's root or use a caret range matching the
version used elsewhere), save the file, and run your Bun install command to
update the lockfile so the Bun binary build no longer depends on a transitive
package for the reflect-metadata import.

---

Outside diff comments:
In @.github/workflows/publish.yml:
- Around line 179-201: The body block passed to softprops/action-gh-release@v2
currently includes interpolation of ${{ github.event.release.body }} and also
sets append_body: true, which causes duplicate release notes on reruns; remove
the interpolation of ${{ github.event.release.body }} from the body field and
let append_body: true handle preserving the existing release notes so only the
new standalone/docker/npm section is appended (update the body value in the
action that defines body: | and keep append_body: true).

---

Nitpick comments:
In @.github/workflows/publish.yml:
- Around line 152-155: The smoke test only checks that the binary starts but
doesn't exercise the worker code paths; update the workflow step that runs
./dist/jam-bun/jam to invoke a command that spawns a Bun worker so WORKER_BUN in
packages/jam/node/workers.ts and the .node redirection in bin/jam/bun-entry.ts
are executed (for example run a lightweight subcommand or flag that creates a
worker and exits). Ensure the chosen subcommand is non-interactive and
deterministic so the CI step completes quickly and reliably while touching the
worker path.

In `@packages/jam/node/workers.ts`:
- Around line 11-17: Replace the `in`-based runtime detection for Bun with an
explicit property read: update the isBun variable (used by workerUrl) to check
the Bun property on globalThis via an explicit undefined/type check (so it does
not throw) and coerce to boolean; keep workerUrl unchanged but rely on the new
isBun value. Ensure you reference the isBun variable and workerUrl function when
making the change.
🪄 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: CHILL

Plan: Pro

Run ID: 1ddf8873-5762-4ff6-8f70-f4d1a4740195

📥 Commits

Reviewing files that changed from the base of the PR and between 9567cee and 66e95b7.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • .github/workflows/publish.yml
  • bin/jam/build-for-bun.sh
  • bin/jam/bun-entry.ts
  • package.json
  • packages/jam/node/workers.ts
  • packages/workers/block-authorship/index.ts
  • packages/workers/importer/index.ts
  • packages/workers/jam-network/index.ts

Comment thread .github/workflows/publish.yml Outdated
Comment on lines +138 to +142
- name: Setup Bun
uses: oven-sh/setup-bun@v2

- name: Install dependencies
run: bun install
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Workflow step under review:"
sed -n '138,143p' .github/workflows/publish.yml

echo
echo "Committed lockfiles:"
fd -a 'bun\.lockb?$|package-lock\.json$|npm-shrinkwrap\.json$' .

Repository: FluffyLabs/typeberry

Length of output: 291


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking package.json for Bun version requirement ==="
if [ -f package.json ]; then
  jq -r '.engines.bun // "No bun version specified"' package.json
fi

echo
echo "=== Full engines section if present ==="
if [ -f package.json ]; then
  jq '.engines // "No engines section"' package.json
fi

echo
echo "=== Checking full publish.yml workflow for Bun version references ==="
rg -i 'bun.*version|version.*bun' .github/workflows/publish.yml || echo "No version references found"

echo
echo "=== Lockfile sizes and timestamps ==="
ls -lh bun.lock package-lock.json 2>/dev/null || echo "Lockfiles info unavailable"

echo
echo "=== Checking if bun.lock is actually a binary or text lockfile ==="
file bun.lock 2>/dev/null || echo "Could not determine file type"

Repository: FluffyLabs/typeberry

Length of output: 586


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== All uses of setup-bun in workflow files ==="
rg 'setup-bun' .github/workflows/

echo
echo "=== All workflow files listing ==="
fd '\.ya?ml$' .github/workflows/

echo
echo "=== Checking if bun-version is documented or used anywhere ==="
rg 'bun-version' .github/workflows/ || echo "bun-version parameter not used"

Repository: FluffyLabs/typeberry

Length of output: 729


Pin the Bun toolchain and enforce frozen dependency resolution for releases.

This job currently floats both the compiler/runtime (setup-bun@v2 without a version pin) and does not enforce the committed lockfile (bun install lacks --frozen-lockfile). This allows the same release tag to produce different binaries when built at different times.

🔒 Proposed hardening
     - name: Setup Bun
       uses: oven-sh/setup-bun@v2
+      with:
+        bun-version: 1.2.x
 
     - name: Install dependencies
-      run: bun install
+      run: bun install --frozen-lockfile
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Setup Bun
uses: oven-sh/setup-bun@v2
- name: Install dependencies
run: bun install
- name: Setup Bun
uses: oven-sh/setup-bun@v2
with:
bun-version: 1.2.x
- name: Install dependencies
run: bun install --frozen-lockfile
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish.yml around lines 138 - 142, Pin the Bun setup
action and enforce frozen dependency resolution: update the "Setup Bun" step
that uses oven-sh/setup-bun to reference an exact, immutable tag (replace the
floating `@v2` with a specific release tag) and change the "Install dependencies"
step (currently running `bun install`) to run with the frozen-lockfile flag (use
`bun install --frozen-lockfile`) so builds use the committed lockfile and the
toolchain is pinned.

Comment thread bin/jam/build-for-bun.sh Outdated
Comment on lines +9 to +12
VERSION=$(bun -e "console.log(require('./package.json').version)")

# Start from the top-level project directory
cd ../..
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.

⚠️ Potential issue | 🟠 Major

Resolve paths from the script location, not the caller's cwd.

Both the version lookup and cd ../.. only work when this script is launched from bin/jam. Running bin/jam/build-for-bun.sh from the repo root reads a different package.json and then cds outside the repository.

🧭 Proposed fix
-VERSION=$(bun -e "console.log(require('./package.json').version)")
-
-# Start from the top-level project directory
-cd ../..
+SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
+ROOT_DIR="$(cd -- "$SCRIPT_DIR/../.." && pwd)"
+VERSION="$(cd "$SCRIPT_DIR" && bun -e 'console.log(require("./package.json").version)')"
+
+# Start from the top-level project directory
+cd "$ROOT_DIR"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
VERSION=$(bun -e "console.log(require('./package.json').version)")
# Start from the top-level project directory
cd ../..
SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
ROOT_DIR="$(cd -- "$SCRIPT_DIR/../.." && pwd)"
VERSION="$(cd "$SCRIPT_DIR" && bun -e 'console.log(require("./package.json").version)')"
# Start from the top-level project directory
cd "$ROOT_DIR"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bin/jam/build-for-bun.sh` around lines 9 - 12, The VERSION lookup and the `cd
../..` are resolving from the caller's cwd; change both to resolve paths
relative to the script location instead: compute the script directory
(referenced in this file as the context for VERSION and the subsequent cd) and
use that base when invoking bun to read package.json for VERSION and when
changing directories (replace the current `VERSION=$(bun -e
"console.log(require('./package.json').version)")` and `cd ../..` usage with
versions that reference the computed script directory variable so the script
always reads the correct package.json and cd's into the intended repo location).

Comment thread bin/jam/build-for-bun.sh Outdated
Comment on lines +21 to +23
bun build --compile \
--outfile "$DIST_FOLDER/jam" \
./bin/jam/bun-entry.ts
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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Binary matrix:"
sed -n '124,150p' .github/workflows/publish.yml

echo
echo "Compile command:"
sed -n '18,24p' bin/jam/build-for-bun.sh

Repository: FluffyLabs/typeberry

Length of output: 1120


🌐 Web query:

bun build --target option documentation

💡 Result:

bun build --target (bundling)

--target sets the intended runtime environment for the generated bundle (default: browser). Valid values are: browser, bun, node. It affects resolution conditions and optimizations. The CLI form is, for example: bun build ./index.ts --outdir ./out --target browser. [1]

Reference: Bun “Bundler” docs → CLI Usage / --target, and the target option description. [1]

bun build --compile --target (single-file executable)

When using --compile, --target is used for cross-compiling a Bun executable, and takes values like bun-linux-arm64, bun-windows-x64, etc. Example: bun build --compile --target=bun-linux-arm64 ./index.ts --outfile myapp. [2]

Reference: Bun “Single-file executable” docs → supported compile targets + examples. [2]

Sources: [1] [2]


🏁 Script executed:

# Check if there's any existing usage of --target flag in the repository
rg --type sh "bun build.*--target" || echo "No --target usage found"

# Also check for any bun configuration files
fd -e bunfig.toml -e bunfig.json

Repository: FluffyLabs/typeberry

Length of output: 87


🏁 Script executed:

cat -n bin/jam/build-for-bun.sh

Repository: FluffyLabs/typeberry

Length of output: 2725


🌐 Web query:

bun build --compile --target bun-darwin-arm64 supported targets

💡 Result:

bun build --compile --target=... supports these targets (including bun-darwin-arm64): [1]

  • macOS
    • bun-darwin-arm64
    • bun-darwin-x64
    • bun-darwin-x64-baseline
  • Linux (glibc)
    • bun-linux-x64
    • bun-linux-x64-baseline
    • bun-linux-x64-modern
    • bun-linux-arm64
  • Linux (musl)
    • bun-linux-x64-musl
    • bun-linux-arm64-musl
  • Windows
    • bun-windows-x64
    • bun-windows-x64-baseline
    • bun-windows-x64-modern
    • bun-windows-arm64

Sources: [1]


🏁 Script executed:

# Check if there's any bun documentation or examples in the repository
find . -type f \( -name "*.md" -o -name "*.ts" -o -name "*.js" \) -exec grep -l "bun build.*compile" {} \; | head -10

Repository: FluffyLabs/typeberry

Length of output: 46


Add --target flag to cross-compile for the intended platform, not the host runner.

The zip name and staged native addons are keyed off $PLATFORM, but the executable is currently built for whatever machine runs CI. If the CI runner's platform diverges from the target (e.g., macOS builder for darwin-arm64 vs. Linux builder for linux-x64), you'll ship a binary/addon mismatch. Cross-compiling ensures consistency:

🎯 Proposed fix
 bun build --compile \
+  --target="bun-${PLATFORM}" \
   --outfile "$DIST_FOLDER/jam" \
   ./bin/jam/bun-entry.ts

This maps $PLATFORM values (darwin-arm64, linux-x64) to the corresponding bun compile targets (bun-darwin-arm64, bun-linux-x64).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bun build --compile \
--outfile "$DIST_FOLDER/jam" \
./bin/jam/bun-entry.ts
bun build --compile \
--target="bun-${PLATFORM}" \
--outfile "$DIST_FOLDER/jam" \
./bin/jam/bun-entry.ts
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bin/jam/build-for-bun.sh` around lines 21 - 23, The bun build invocation
currently compiles for the CI host instead of the intended $PLATFORM; add a
small mapping from $PLATFORM to a bun compile target (e.g., map darwin-arm64 ->
bun-darwin-arm64, linux-x64 -> bun-linux-x64) and pass it to bun via --target
"$TARGET" in the existing bun build --compile ... --outfile "$DIST_FOLDER/jam"
./bin/jam/bun-entry.ts command; implement the mapping as a case/switch before
the bun build and validate TARGET is set (exit with an error if unknown) so the
produced binary matches the staged native addons and zip naming.

Comment thread bin/jam/bun-entry.ts
Comment on lines +3 to +5
// reflect-metadata polyfill required by tsyringe (transitive dep of @matrixai/quic)
// eslint-disable-next-line import/no-extraneous-dependencies
import "reflect-metadata";
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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Imports of reflect-metadata:"
rg -n '["'"'"']reflect-metadata["'"'"']' .

echo
echo "Package manifests declaring reflect-metadata:"
python - <<'PY'
import json, pathlib
for path in pathlib.Path('.').rglob('package.json'):
    try:
        data = json.loads(path.read_text())
    except Exception:
        continue
    for key in ("dependencies", "devDependencies", "peerDependencies", "optionalDependencies"):
        version = data.get(key, {}).get("reflect-metadata")
        if version:
            print(f"{path}: {key} -> {version}")
PY

Repository: FluffyLabs/typeberry

Length of output: 952


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Workspace structure ==="
find . -maxdepth 3 -name "package.json" -type f | head -20

echo
echo "=== Root package.json dependencies (first 50 lines) ==="
if [ -f "package.json" ]; then
  cat package.json | head -50
fi

echo
echo "=== bin/jam manifest (if exists) ==="
if [ -f "bin/jam/package.json" ]; then
  cat bin/jam/package.json
else
  echo "No bin/jam/package.json found"
fi

echo
echo "=== Search for `@matrixai/quic` declarations ==="
rg -n '@matrixai/quic' --type json

echo
echo "=== Content of bin/jam/bun-entry.ts ==="
cat bin/jam/bun-entry.ts

Repository: FluffyLabs/typeberry

Length of output: 7269


Declare reflect-metadata explicitly in bin/jam/package.json.

The import is currently a transitive dependency via @typeberry/networking@matrixai/quicreflect-metadata. While the chain exists today, this creates brittleness: if the transitive path breaks or lockfile churn removes it, the Bun build fails without an explicit declaration. Add "reflect-metadata" to bin/jam dependencies to ensure the Bun binary build remains stable across lockfile updates.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bin/jam/bun-entry.ts` around lines 3 - 5, The build imports
"reflect-metadata" directly in bin/jam/bun-entry.ts but doesn't declare it as a
dependency in bin/jam/package.json, relying on a fragile transitive chain; add
"reflect-metadata" to the dependencies section of bin/jam/package.json (pick a
version consistent with your repo's root or use a caret range matching the
version used elsewhere), save the file, and run your Bun install command to
update the lockfile so the Bun binary build no longer depends on a transitive
package for the reflect-metadata import.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
bin/jam/bun-entry.ts (1)

21-24: Consider tightening the regex patterns to reduce false-positive risk.

The current patterns /quic/ and /bandersnatch/ are broad and could theoretically match unrelated module identifiers (e.g., a hypothetical package containing "oblique" or "bandersnatcher"). While unlikely given the current codebase, anchoring the patterns to expected package prefixes would be more defensive.

🔧 Suggested pattern refinement
 const NATIVE_SIBLING_FILES: [pattern: RegExp, file: string][] = [
-  [/quic/, "quic.node"],
-  [/bandersnatch/, "bandersnatch.node"],
+  [/@matrixai\/quic/, "quic.node"],
+  [/@typeberry\/bandersnatch-native/, "bandersnatch.node"],
 ];
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bin/jam/bun-entry.ts` around lines 21 - 24, Tighten the broad RegExp entries
in the NATIVE_SIBLING_FILES constant to avoid false positives: replace the loose
substring matches for "quic" and "bandersnatch" with anchored patterns that
match package names (including optional scoped prefixes) and ensure they only
match the module identifier at the start or at a path boundary, then update the
two entries in NATIVE_SIBLING_FILES accordingly (the array and its two elements
that currently use /quic/ and /bandersnatch/).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@bin/jam/bun-entry.ts`:
- Around line 21-24: Tighten the broad RegExp entries in the
NATIVE_SIBLING_FILES constant to avoid false positives: replace the loose
substring matches for "quic" and "bandersnatch" with anchored patterns that
match package names (including optional scoped prefixes) and ensure they only
match the module identifier at the start or at a path boundary, then update the
two entries in NATIVE_SIBLING_FILES accordingly (the array and its two elements
that currently use /quic/ and /bandersnatch/).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 95a10e8e-e4ad-4a09-8f1c-89e52823ec33

📥 Commits

Reviewing files that changed from the base of the PR and between 66e95b7 and 0f17644.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • .github/workflows/publish.yml
  • bin/jam/build-for-bun.sh
  • bin/jam/bun-entry.ts
  • package.json
  • packages/jam/node/workers.ts
  • packages/workers/block-authorship/index.ts
  • packages/workers/importer/index.ts
  • packages/workers/jam-network/index.ts
✅ Files skipped from review due to trivial changes (3)
  • packages/workers/importer/index.ts
  • packages/workers/block-authorship/index.ts
  • packages/workers/jam-network/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • package.json
  • .github/workflows/publish.yml
  • bin/jam/build-for-bun.sh

@tomusdrw tomusdrw marked this pull request as draft April 8, 2026 13:48
@github-actions
Copy link
Copy Markdown

View all
File Benchmark Ops
bytes/hex-from.ts[0] parse hex using Number with NaN checking 66282.94 ±2.45% 85.65% slower
bytes/hex-from.ts[1] parse hex from char codes 461794.88 ±0.35% fastest ✅
bytes/hex-from.ts[2] parse hex from string nibbles 259858.73 ±0.48% 43.73% slower
bytes/hex-to.ts[0] number toString + padding 91936.84 ±0.44% fastest ✅
bytes/hex-to.ts[1] manual 7396.56 ±1.59% 91.95% slower
codec/bigint.compare.ts[0] compare custom 77721637.72 ±4.04% 19.6% slower
codec/bigint.compare.ts[1] compare bigint 96668778.79 ±3.93% fastest ✅
codec/bigint.decode.ts[0] decode custom 74799515.73 ±3.17% 14.92% slower
codec/bigint.decode.ts[1] decode bigint 87913279.45 ±5.17% fastest ✅
codec/decoding.ts[0] manual decode 9797585.43 ±0.96% 85.11% slower
codec/decoding.ts[1] int32array decode 65815757.68 ±2.87% fastest ✅
codec/decoding.ts[2] dataview decode 64562300.26 ±4.15% 1.9% slower
codec/encoding.ts[0] manual encode 1103481.39 ±0.49% 13.54% slower
codec/encoding.ts[1] int32array encode 1276232.87 ±0.29% fastest ✅
codec/encoding.ts[2] dataview encode 1255933.41 ±0.21% 1.59% slower
collections/map-set.ts[0] 2 gets + conditional set 71492.14 ±0.35% fastest ✅
collections/map-set.ts[1] 1 get 1 set 47619.53 ±0.26% 33.39% slower
logger/index.ts[0] console.log with string concat 2603232.01 ±57.78% fastest ✅
logger/index.ts[1] console.log with args 1238126.83 ±100.88% 52.44% slower
math/add_one_overflow.ts[0] add and take modulus 101067787.85 ±4.76% fastest ✅
math/add_one_overflow.ts[1] condition before calculation 90951601.14 ±5.19% 10.01% slower
math/count-bits-u32.ts[0] standard method 39993963.22 ±2.98% 47.83% slower
math/count-bits-u32.ts[1] magic 76660335.09 ±6.1% fastest ✅
math/count-bits-u64.ts[0] standard method 4232474.55 ±0.88% 84.29% slower
math/count-bits-u64.ts[1] magic 26944824.28 ±1% fastest ✅
math/mul_overflow.ts[0] multiply and bring back to u32 99538623.24 ±5.12% fastest ✅
math/mul_overflow.ts[1] multiply and take modulus 99487901.61 ±4.88% 0.05% slower
math/switch.ts[0] switch 100769254.91 ±4.47% fastest ✅
math/switch.ts[1] if 98561620.89 ±4.96% 2.19% slower
hash/index.ts[0] hash with numeric representation 68.46 ±0.07% 31.33% slower
hash/index.ts[1] hash with string representation 43.13 ±0.8% 56.74% slower
hash/index.ts[2] hash with symbol representation 67.26 ±0.06% 32.53% slower
hash/index.ts[3] hash with uint8 representation 75.91 ±1.09% 23.85% slower
hash/index.ts[4] hash with packed representation 99.69 ±0.44% fastest ✅
hash/index.ts[5] hash with bigint representation 69.41 ±0.21% 30.37% slower
hash/index.ts[6] hash with uint32 representation 94.17 ±0.35% 5.54% slower
bytes/bytes-to-number.ts[0] Conversion with bitops 3654.86 ±5.59% fastest ✅
bytes/bytes-to-number.ts[1] Conversion without bitops 2931.79 ±4.52% 19.78% slower
bytes/compare.ts[0] Comparing Uint32 bytes 11582.25 ±1.72% fastest ✅
bytes/compare.ts[1] Comparing raw bytes 11022.34 ±0.54% 4.83% slower
codec/view_vs_collection.ts[0] Get first element from Decoded 13645.36 ±0.35% 55.43% slower
codec/view_vs_collection.ts[1] Get first element from View 30617.41 ±0.37% fastest ✅
codec/view_vs_collection.ts[2] Get 50th element from Decoded 13710.82 ±0.26% 55.22% slower
codec/view_vs_collection.ts[3] Get 50th element from View 16747.83 ±0.22% 45.3% slower
codec/view_vs_collection.ts[4] Get last element from Decoded 13747.01 ±0.3% 55.1% slower
codec/view_vs_collection.ts[5] Get last element from View 11847.95 ±0.25% 61.3% slower
codec/view_vs_object.ts[0] Get the first field from Decoded 196892.12 ±0.61% fastest ✅
codec/view_vs_object.ts[1] Get the first field from View 50725.43 ±0.41% 74.24% slower
codec/view_vs_object.ts[2] Get the first field as view from View 50473.98 ±0.34% 74.36% slower
codec/view_vs_object.ts[3] Get two fields from Decoded 192691.8 ±1.38% 2.13% slower
codec/view_vs_object.ts[4] Get two fields from View 40435.49 ±0.42% 79.46% slower
codec/view_vs_object.ts[5] Get two fields from materialized from View 80910.56 ±0.37% 58.91% slower
codec/view_vs_object.ts[6] Get two fields as views from View 40450.5 ±0.34% 79.46% slower
codec/view_vs_object.ts[7] Get only third field from Decoded 99678.3 ±98.69% 49.37% slower
codec/view_vs_object.ts[8] Get only third field from View 33442.3 ±16.32% 83.01% slower
codec/view_vs_object.ts[9] Get only third field as view from View 44249.17 ±0.28% 77.53% slower
collections/map_vs_sorted.ts[0] Map 134748.84 ±0.37% fastest ✅
collections/map_vs_sorted.ts[1] Map-array 45582.73 ±1.94% 66.17% slower
collections/map_vs_sorted.ts[2] Array 48597.42 ±1.45% 63.93% slower
collections/map_vs_sorted.ts[3] SortedArray 80262.37 ±0.23% 40.44% slower
collections/hash-dict-vs-blob-dict_delete.ts[0] StringHashDictionary 2451.48 ±0.53% 1.32% slower
collections/hash-dict-vs-blob-dict_delete.ts[1] BlobDictionary(1) 2456.04 ±0.18% 1.13% slower
collections/hash-dict-vs-blob-dict_delete.ts[2] BlobDictionary(2) 2484.15 ±0.16% fastest ✅
collections/hash-dict-vs-blob-dict_delete.ts[3] BlobDictionary(3) 2451.94 ±0.23% 1.3% slower
collections/hash-dict-vs-blob-dict_delete.ts[4] BlobDictionary(4) 2449.95 ±0.35% 1.38% slower
collections/hash-dict-vs-blob-dict_delete.ts[5] BlobDictionary(5) 2470.19 ±0.23% 0.56% slower
collections/hash-dict-vs-blob-dict_get.ts[0] StringHashDictionary 2988.18 ±0.38% 0.17% slower
collections/hash-dict-vs-blob-dict_get.ts[1] BlobDictionary(1) 2989.83 ±0.36% 0.11% slower
collections/hash-dict-vs-blob-dict_get.ts[2] BlobDictionary(2) 2983.37 ±0.34% 0.33% slower
collections/hash-dict-vs-blob-dict_get.ts[3] BlobDictionary(3) 2979.57 ±0.37% 0.45% slower
collections/hash-dict-vs-blob-dict_get.ts[4] BlobDictionary(4) 2956.88 ±0.45% 1.21% slower
collections/hash-dict-vs-blob-dict_get.ts[5] BlobDictionary(5) 2993.17 ±0.4% fastest ✅
collections/hash-dict-vs-blob-dict_set.ts[0] StringHashDictionary 2302.3 ±0.4% fastest ✅
collections/hash-dict-vs-blob-dict_set.ts[1] BlobDictionary(1) 2287.09 ±0.41% 0.66% slower
collections/hash-dict-vs-blob-dict_set.ts[2] BlobDictionary(2) 2280.62 ±0.46% 0.94% slower
collections/hash-dict-vs-blob-dict_set.ts[3] BlobDictionary(3) 2282.47 ±0.44% 0.86% slower
collections/hash-dict-vs-blob-dict_set.ts[4] BlobDictionary(4) 741.22 ±98.6% 67.81% slower
collections/hash-dict-vs-blob-dict_set.ts[5] BlobDictionary(5) 1503.58 ±1.48% 34.69% slower
hash/blake2b.ts[0] our hasher 1.08 ±0.25% fastest ✅
hash/blake2b.ts[1] blake2b js 0.03 ±2.26% 97.22% slower
crypto/ed25519.ts[0] native crypto 5.72 ±1.05% fastest ✅
crypto/ed25519.ts[1] wasm lib 2.26 ±1.84% 60.49% slower
crypto/ed25519.ts[2] wasm lib batch 2.27 ±0.29% 60.31% slower

Benchmarks summary: 83/83 OK ✅

@tomusdrw
Copy link
Copy Markdown
Member Author

@fluffylabs-bot benchmark

@tomusdrw tomusdrw closed this Apr 27, 2026
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.

1 participant