Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a portable runtime shim and JS bootstrap to unify AssemblyScript and JavaScript execution paths, introducing a new Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assembly/instructions/logic.ts (1)
41-44:⚠️ Potential issue | 🟡 MinorInconsistent
u64()wrapping for bitwise NOT operations.
or_inv(line 48) andxnor(line 54) wrap their results withu64()when using bitwise NOT (~), butand_inv(line 42) also uses bitwise NOT without the wrapper. All three operations should be consistent; adding theu64()wrapper ensures proper type handling across the board.Proposed fix
// AND_INV export const and_inv: InstructionRun = (r, args, registers) => { - registers[reg(args.c)] = registers[reg(args.b)] & ~registers[reg(args.a)]; + registers[reg(args.c)] = u64(registers[reg(args.b)] & ~registers[reg(args.a)]); return ok(r); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assembly/instructions/logic.ts` around lines 41 - 44, and_inv uses a bitwise NOT without the u64() wrapper, causing inconsistent type handling compared to or_inv and xnor; update the assignment inside and_inv (the line setting registers[reg(args.c)]) to wrap the negated value with u64(), i.e. apply u64(~registers[reg(args.a)]) so the result matches the type handling of or_inv and xnor while keeping reg, args, and registers usage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@assembly/api-internal.ts`:
- Around line 180-181: The portable.asArray signature currently declared as
asArray<T>(v: T[]): T[] is too narrow and causes `@ts-ignore`; update the
declaration to asArray<T>(v: T[] | Iterable<T>): T[] (or equivalent union type)
so it accepts both arrays and iterables, remove the `@ts-ignore` usage, and keep
the existing implementation (which uses Array.from/handles iterables) intact;
reference the portable.asArray function to locate and change its type signature
so calls like memory.pages.keys(), this.pages.values(), and
computeGasCosts(...).values() type-check without suppression.
In `@assembly/api-utils.ts`:
- Around line 62-68: The two "@ts-ignore" lines in the HasMetadata branch hide a
TypeScript 5.9 generic-parameter mismatch for Uint8Array when assigning the
result of extractCodeAndMetadata to code and metadata; update this by replacing
the ignores with a short explanatory comment referencing TS 5.9's changed
Uint8Array generic (or a link to the relevant TypeScript issue) and either
narrow the types via an explicit, documented cast (e.g. cast the returned values
from extractCodeAndMetadata to the expected Uint8Array types) or adjust
extractCodeAndMetadata's return signature to match the expected types so you can
remove the ignores—locate this logic in the if (hasMetadata === HasMetadata.Yes)
block around extractCodeAndMetadata, data.code, and data.metadata.
In `@assembly/instructions/store.ts`:
- Around line 62-63: Extract a shared helper that computes the effective address
from a base register and signed 32-bit offset and use it in all indirect store
handlers; implement a function (e.g., computeEffectiveAddress or
effectiveAddress) that takes registers and the two operands (the register index
for reg(args.a) and args.b), performs u32SignExtend(args.b), adds via
portable.u64_add to registers[reg(args.a)], and returns the final u32 address;
then replace the repeated expression
u32(portable.u64_add(registers[reg(args.a)], u32SignExtend(args.b))) in the
handlers that call memory.setU8/memory.setU16/etc. with a call to this new
helper to centralize the address calculation.
In `@assembly/instructions/utils.ts`:
- Around line 73-87: The three sign-extension helpers u8SignExtend,
u16SignExtend, and u32SignExtend currently declare return type i64 but wrap
their result with u64(...), causing a type mismatch; update their function
signatures to return u64 (i.e., change the annotated return type from i64 to u64
for u8SignExtend, u16SignExtend, and u32SignExtend) so the signature matches the
actual returned value and existing u64 usage patterns.
In `@assembly/portable.ts`:
- Around line 8-10: installPolyfills() (and other top-level checks) directly
reference ASC_TARGET at module load, which can throw if portable/bootstrap.ts
hasn't set g.ASC_TARGET yet; create a safe accessor (e.g., getAscTarget() or use
typeof checks) and replace direct uses of ASC_TARGET in installPolyfills(), and
other module-load spots, with that accessor or with guards like typeof
ASC_TARGET !== "undefined" or (globalThis as any)?.g?.ASC_TARGET existence
checks so the module never reads an uninitialized global during import; ensure
all occurrences (including the ones you noted and any top-level conditionals)
use the same guarded access pattern.
In `@biome.jsonc`:
- Line 2: Update package.json to align the declared Biome version with the
schema by changing the dependency "@biomejs/biome" to ^2.4.3 (currently ^2.3.11)
so the declared package matches the biome.jsonc "$schema" version 2.4.3 and the
installed/locked version; modify the version string in the
dependencies/devDependencies entry for "@biomejs/biome" and run an install to
verify lockfile consistency.
- Around line 56-57: Remove the global "suspicious.noTsIgnore": "off" setting
and instead apply documented, narrowly scoped suppressions where needed: search
for the AssemblyScript sources that contain the ~38 `@ts-ignore` comments and
replace the global rule by adding per-line biome-ignore or inline rule disables
(e.g., use biome-ignore comments or per-line eslint/TS directives) alongside the
existing explanatory notes ("AS-only decorator", "AS returns T[], JS returns
iterator", etc.); keep each suppression adjacent to the `@ts-ignore` with the same
human-readable justification so future changes are still linted globally while
these specific, documented exceptions remain allowed.
In `@package.json`:
- Line 37: The package.json build script "tsbuild:portable" currently invokes
esbuild via "npx esbuild", which causes non-deterministic installs; add esbuild
as a pinned devDependency (choose a specific version) in package.json's
devDependencies and update the "tsbuild:portable" script to call "esbuild"
directly instead of "npx esbuild" so the project uses the pinned, deterministic
esbuild binary during CI and local builds.
In `@portable/types/assemblyscript.d.ts`:
- Around line 8-19: The i64 and u64 aliases currently use number which causes
precision loss; change the type aliases i64 and u64 to bigint and update any
related AssemblyScript cast/type interfaces that handle 64-bit values (the cast
interfaces referenced in the diff) to use bigint as well so 64-bit exactness is
preserved; keep all other fixed-size aliases
(i8,i16,i32,u8,u16,u32,isize,usize,f32,f64) as number and ensure any functions
or interfaces that previously accepted/returned i64/u64 are updated to
accept/return bigint to maintain type consistency.
In `@test/test-w3f-common.ts`:
- Around line 45-59: The ignored array (variable name: ignored) contains 13 test
identifiers related to memory access errors but lacks rationale; update the test
file to add a concise comment above the ignored declaration explaining why each
test is skipped (e.g., known bug IDs, platform limitations, expected spec
divergence, or TODOs with links to issues/PRs), or group them with brief reasons
next to each test name if they differ, so future maintainers can quickly
understand whether these are temporary skips or permanent exclusions.
- Around line 165-177: The asChunks function currently mutates each input Chunk
by assigning chunk.data and deleting chunk.contents; change it to return new
objects instead to avoid side effects: inside asChunks map over chunks and for
each Chunk create and return a new object (preserving address and setting data
to read(chunk, "contents") as number[]) without modifying the original chunk,
and do not perform delete operations; reference the asChunks function and the
Chunk type when making the change.
---
Outside diff comments:
In `@assembly/instructions/logic.ts`:
- Around line 41-44: and_inv uses a bitwise NOT without the u64() wrapper,
causing inconsistent type handling compared to or_inv and xnor; update the
assignment inside and_inv (the line setting registers[reg(args.c)]) to wrap the
negated value with u64(), i.e. apply u64(~registers[reg(args.a)]) so the result
matches the type handling of or_inv and xnor while keeping reg, args, and
registers usage unchanged.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (43)
.githooks/pre-push.github/workflows/build.ymlassembly/api-debugger.tsassembly/api-internal.tsassembly/api-types.tsassembly/api-utils.tsassembly/arguments.tsassembly/codec.tsassembly/gas-costs.tsassembly/gas.tsassembly/index-shared.tsassembly/index.tsassembly/instructions.tsassembly/instructions/bit.tsassembly/instructions/branch.tsassembly/instructions/jump.tsassembly/instructions/load.tsassembly/instructions/logic.tsassembly/instructions/math.tsassembly/instructions/mov.tsassembly/instructions/rot.tsassembly/instructions/set.tsassembly/instructions/shift.tsassembly/instructions/store.tsassembly/instructions/utils.tsassembly/interpreter.tsassembly/memory-page.tsassembly/memory.tsassembly/portable.tsassembly/program.tsassembly/registers.tsassembly/spi.tsassembly/test.tsbin/src/trace-replay.tsbiome.jsoncpackage.jsonportable/bootstrap.tsportable/index.tsportable/tsconfig.jsonportable/types/assemblyscript.d.tstest/test-w3f-common.tstest/test-w3f-portable.tstest/test-w3f.ts
mateuszsikora
approved these changes
Feb 27, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package.json (1)
54-54:⚠️ Potential issue | 🟠 MajorPin
esbuildto an exact version to keep builds reproducible.Line 54 still uses a semver range (
^0.27.0), so patch upgrades can change CI/local behavior over time. Use an exact version pin.Proposed fix
- "esbuild": "^0.27.0", + "esbuild": "0.27.0",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 54, The dependency entry for esbuild uses a caret range ("esbuild": "^0.27.0") which allows patch upgrades; change it to an exact version string (e.g., "esbuild": "0.27.0") in package.json to pin the version, then reinstall and update the lockfile (npm-shrinkwrap.json / package-lock.json or yarn.lock) so CI and local builds remain reproducible; locate the "esbuild" entry in package.json to make this 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 `@package.json`:
- Line 39: The package.json adds a new "test:w3f-portable" script but lacks a
corresponding "pretest:w3f-portable" build step; add a "pretest:w3f-portable"
script that runs the same build/preparation command used by the existing pretest
hooks (e.g., the command used by "pretest:w3f" and "pretest:gas-cost") so that
running "npm run test:w3f-portable" on a clean checkout performs the necessary
build steps first.
---
Duplicate comments:
In `@package.json`:
- Line 54: The dependency entry for esbuild uses a caret range ("esbuild":
"^0.27.0") which allows patch upgrades; change it to an exact version string
(e.g., "esbuild": "0.27.0") in package.json to pin the version, then reinstall
and update the lockfile (npm-shrinkwrap.json / package-lock.json or yarn.lock)
so CI and local builds remain reproducible; locate the "esbuild" entry in
package.json to make this change.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
assembly/instructions/store.tspackage.json
auto-merge was automatically disabled
February 27, 2026 17:27
Head branch was modified
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
package.json (2)
52-56: 🧹 Nitpick | 🔵 TrivialPrefer exact
esbuildpin if reproducible artifacts are required.Line 56 uses
^0.27.0. This is better than runtimenpx, but caret ranges can still shift bundler behavior when lockfiles are regenerated.#!/usr/bin/env bash set -euo pipefail echo "package.json esbuild range:" jq -r '.devDependencies.esbuild // .dependencies.esbuild // "MISSING"' package.json echo "" echo "Detected lockfiles:" fd '^(package-lock\.json|pnpm-lock\.yaml|yarn\.lock)$' -HI . || true if [ -f package-lock.json ]; then echo "" echo "package-lock resolved esbuild version:" jq -r '.packages["node_modules/esbuild"].version // "MISSING"' package-lock.json fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 52 - 56, The esbuild dependency in package.json is specified with a caret range ("esbuild": "^0.27.0"), which can allow minor/patch upgrades to change bundler behavior; update the esbuild entry in package.json to an exact version (e.g., "esbuild": "0.27.0") to ensure reproducible artifacts, then regenerate and commit the lockfile so the resolved esbuild version is fixed (verify the change by checking the esbuild entry in package.json and the resolved version in the lockfile).
39-39:⚠️ Potential issue | 🟠 MajorAdd
pretest:w3f-portablefor clean-checkout reliability.Line 39 introduces
test:w3f-portable, but unlike Line 29 and Line 30 there is no matching pretest hook. Running this target directly can fail before build artifacts exist.Proposed fix
"pretest:w3f": "npm run build", "pretest:gas-cost": "npm run build", + "pretest:w3f-portable": "npm run build", "preweb": "npm run build",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 39, Add a missing pretest hook for the new test script: create a "pretest:w3f-portable" script in package.json that runs the same preparation/build steps used by the existing "pretest:w3f-ci" and "pretest:w3f-local" scripts so "test:w3f-portable" has the required build/artifacts available when run directly; copy the exact command from "pretest:w3f-ci"/"pretest:w3f-local" into the new "pretest:w3f-portable" entry to ensure parity with the other test targets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@package.json`:
- Around line 52-56: The esbuild dependency in package.json is specified with a
caret range ("esbuild": "^0.27.0"), which can allow minor/patch upgrades to
change bundler behavior; update the esbuild entry in package.json to an exact
version (e.g., "esbuild": "0.27.0") to ensure reproducible artifacts, then
regenerate and commit the lockfile so the resolved esbuild version is fixed
(verify the change by checking the esbuild entry in package.json and the
resolved version in the lockfile).
- Line 39: Add a missing pretest hook for the new test script: create a
"pretest:w3f-portable" script in package.json that runs the same
preparation/build steps used by the existing "pretest:w3f-ci" and
"pretest:w3f-local" scripts so "test:w3f-portable" has the required
build/artifacts available when run directly; copy the exact command from
"pretest:w3f-ci"/"pretest:w3f-local" into the new "pretest:w3f-portable" entry
to ensure parity with the other test targets.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
package.json
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
2 participants
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.
Resolves #3