Add expunge host call

[DrEverr]

Merge after: #935

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Implemented the machine expunge functionality, enabling removal of nested virtual machines and retrieval of their program counter status.

• Tests

  • Added test coverage for expunge operations, including successful removal, error handling for non-existent machines, and duplicate expunge prevention.

Walkthrough

This PR implements the machineExpunge method across the RefineExternalities interface. The implementation searches for a machine by index in the internal machines collection, returns an error if not found, extracts and removes the machine's program counter, and returns the PC value. Supporting changes include importing required dependencies, comprehensive test coverage for success and failure scenarios, and JSDoc clarification.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Add Machine HostCall #935: Implements machineInit to add machines to the same internal collection that machineExpunge now removes from, forming complementary lifecycle operations.

Suggested reviewers

• tomusdrw
• mateuszsikora
• r0tc

Poem

🐰 A machine meets its end, with a hop and a bound,
Its program counter extracted without a sound,
The collection grows lighter, the storage runs lean,
One more machineExpunge—the cleanest we've seen!
✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Description check	❓ Inconclusive	The description 'Merge after: #935' is a procedural note related to the changeset, indicating a dependency rather than explaining the changes themselves.	Consider adding a brief description of what the expunge host call does and why it's being added to provide context for reviewers.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Add expunge host call' directly describes the main change across all modified files, which implement and test the machineExpunge functionality.

_{✏️ 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 maso-expunge

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

packages/jam/in-core/externalities/refine.test.ts (1)

247-255: Assert the exact error value in the negative-path cases.

These tests only check isError, so they will still pass if machineInit starts returning the wrong decoder error or machineExpunge returns the wrong host-call error. Please pin the contract with the concrete error value as well.

Also applies to: 257-265, 306-325

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/jam/in-core/externalities/refine.test.ts` around lines 247 - 255,
Update the negative-path tests to assert the concrete error value (not just
isError) returned by ext.machineInit and ext.machineExpunge: for the test
"should return error for invalid program blob" (using createExt,
BytesBlob.blobFrom, tryAsProgramCounter, and ext.machineInit) assert the
specific error enum/variant or error.code/message expected from the decoder
instead of only checking result.isError; make the analogous changes in the other
failing tests referenced (lines 257-265 and 306-325) to pin the expected error
values for each negative case so the assertions fail if the wrong error is
returned.

packages/jam/in-core/externalities/refine.ts (1)

118-120: Avoid reparsing the same program blob during machineInit.

ProgramDecoder.deblob(code.raw) already parses the blob once, and resetGeneric(code.raw, ...) immediately decodes it again. On larger inner programs this doubles the init-path decode/copy cost for no behavioral gain.

As per coding guidelines, "be mindful of allocations/copying of large memory (prefer ArrayBuffer/views and subarray over slice)".

Also applies to: 123-125

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/jam/in-core/externalities/refine.ts` around lines 118 - 120,
ProgramDecoder.deblob(code.raw) is being called twice during machineInit (once
as deblobResult and again inside resetGeneric(code.raw, ...)), causing redundant
parsing and copies for large inner programs; change the flow to reuse the
already-parsed result from ProgramDecoder.deblob (deblobResult) when calling
resetGeneric (or add/use an overload of resetGeneric that accepts the
parsed/deblobbed representation) so you pass the decoded buffer/view instead of
re-decoding code.raw, and apply the same change for the other occurrence around
lines 123-125 to avoid double parsing/copying.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/jam/in-core/externalities/refine.ts`:
- Line 125: The code narrows a u64 program counter to a JS number in
innerPvm.resetGeneric(Number(programCounter)), risking precision loss for values
> 2^53-1; either (A) add a guard before calling innerPvm.resetGeneric that
rejects/non-accepts programCounter values that cannot round-trip (e.g. check
Number.isSafeInteger(Number(programCounter)) or compare to
Number.MAX_SAFE_INTEGER and throw/return an error from
machineExpunge/machineInit), or (B) refactor the runtime to preserve the full
u64 by changing innerPvm.resetGeneric (and any callers such as machineExpunge,
machineInit and places using tryAsProgramCounter/innerPvm.getPC) to accept a
BigInt/u64 representation instead of narrowing to number; update all affected
call sites (resetGeneric, tryAsProgramCounter, innerPvm.getPC, machineExpunge,
machineInit) to use the chosen approach.

---

Nitpick comments:
In `@packages/jam/in-core/externalities/refine.test.ts`:
- Around line 247-255: Update the negative-path tests to assert the concrete
error value (not just isError) returned by ext.machineInit and
ext.machineExpunge: for the test "should return error for invalid program blob"
(using createExt, BytesBlob.blobFrom, tryAsProgramCounter, and ext.machineInit)
assert the specific error enum/variant or error.code/message expected from the
decoder instead of only checking result.isError; make the analogous changes in
the other failing tests referenced (lines 257-265 and 306-325) to pin the
expected error values for each negative case so the assertions fail if the wrong
error is returned.

In `@packages/jam/in-core/externalities/refine.ts`:
- Around line 118-120: ProgramDecoder.deblob(code.raw) is being called twice
during machineInit (once as deblobResult and again inside resetGeneric(code.raw,
...)), causing redundant parsing and copies for large inner programs; change the
flow to reuse the already-parsed result from ProgramDecoder.deblob
(deblobResult) when calling resetGeneric (or add/use an overload of resetGeneric
that accepts the parsed/deblobbed representation) so you pass the decoded
buffer/view instead of re-decoding code.raw, and apply the same change for the
other occurrence around lines 123-125 to avoid double parsing/copying.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7575fb9c-8fb3-47c7-b257-97983949cfcb

📥 Commits

Reviewing files that changed from the base of the PR and between 6cb1bd5 and bfeb686.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (4)

• packages/jam/in-core/externalities/refine.test.ts
• packages/jam/in-core/externalities/refine.ts
• packages/jam/in-core/package.json
• packages/jam/jam-host-calls/externalities/refine-externalities.ts

[github-actions]

View all

File	Benchmark	Ops
bytes/hex-from.ts[0]	parse hex using Number with NaN checking	66100.3 ±1.09%	85.65% slower
bytes/hex-from.ts[1]	parse hex from char codes	460498.11 ±0.39%	fastest ✅
bytes/hex-from.ts[2]	parse hex from string nibbles	262264.44 ±0.66%	43.05% slower
bytes/hex-to.ts[0]	number toString + padding	90351.95 ±0.72%	fastest ✅
bytes/hex-to.ts[1]	manual	7440.46 ±0.54%	91.77% slower
codec/bigint.compare.ts[0]	compare custom	95079775.68 ±4.01%	0.05% slower
codec/bigint.compare.ts[1]	compare bigint	95124087.59 ±4.37%	fastest ✅
codec/bigint.decode.ts[0]	decode custom	70600253.42 ±3.69%	24.89% slower
codec/bigint.decode.ts[1]	decode bigint	94000760.81 ±4.69%	fastest ✅
codec/decoding.ts[0]	manual decode	9755808.68 ±0.88%	85.48% slower
codec/decoding.ts[1]	int32array decode	61792953.22 ±4.45%	8.01% slower
codec/decoding.ts[2]	dataview decode	67172890.85 ±2.98%	fastest ✅
codec/encoding.ts[0]	manual encode	1081800.11 ±0.34%	11.8% slower
codec/encoding.ts[1]	int32array encode	1226595.2 ±1.11%	fastest ✅
codec/encoding.ts[2]	dataview encode	1211119.06 ±0.81%	1.26% slower
collections/map-set.ts[0]	2 gets + conditional set	70510.46 ±0.2%	fastest ✅
collections/map-set.ts[1]	1 get 1 set	46956.44 ±0.11%	33.41% slower
logger/index.ts[0]	console.log with string concat	4698603.52 ±46.66%	fastest ✅
logger/index.ts[1]	console.log with args	1467014.75 ±84.6%	68.78% slower
math/add_one_overflow.ts[0]	add and take modulus	71721258.18 ±38.56%	25.3% slower
math/add_one_overflow.ts[1]	condition before calculation	96015008.73 ±4.37%	fastest ✅
math/count-bits-u32.ts[0]	standard method	41791930.23 ±1.78%	52.96% slower
math/count-bits-u32.ts[1]	magic	88845384.9 ±4.2%	fastest ✅
math/count-bits-u64.ts[0]	standard method	4248486.4 ±1.36%	84% slower
math/count-bits-u64.ts[1]	magic	26552725.16 ±1.13%	fastest ✅
math/mul_overflow.ts[0]	multiply and bring back to u32	100092298.03 ±4.55%	fastest ✅
math/mul_overflow.ts[1]	multiply and take modulus	93135325.91 ±5.22%	6.95% slower
math/switch.ts[0]	switch	101458332.78 ±4.12%	fastest ✅
math/switch.ts[1]	if	93337653.01 ±4.98%	8% slower
hash/index.ts[0]	hash with numeric representation	67.79 ±0.16%	31.16% slower
hash/index.ts[1]	hash with string representation	42.95 ±1.35%	56.38% slower
hash/index.ts[2]	hash with symbol representation	65.94 ±0.66%	33.04% slower
hash/index.ts[3]	hash with uint8 representation	74.86 ±1.08%	23.98% slower
hash/index.ts[4]	hash with packed representation	98.47 ±1.37%	fastest ✅
hash/index.ts[5]	hash with bigint representation	72.39 ±0.61%	26.49% slower
hash/index.ts[6]	hash with uint32 representation	92.98 ±0.2%	5.58% slower
bytes/bytes-to-number.ts[0]	Conversion with bitops	3548.39 ±5.36%	fastest ✅
bytes/bytes-to-number.ts[1]	Conversion without bitops	2870.62 ±4.59%	19.1% slower
bytes/compare.ts[0]	Comparing Uint32 bytes	12217.1 ±0.75%	fastest ✅
bytes/compare.ts[1]	Comparing raw bytes	12000.28 ±0.7%	1.77% slower
codec/view_vs_collection.ts[0]	Get first element from Decoded	14748.1 ±0.69%	54.01% slower
codec/view_vs_collection.ts[1]	Get first element from View	32065.85 ±0.77%	fastest ✅
codec/view_vs_collection.ts[2]	Get 50th element from Decoded	14644.64 ±0.65%	54.33% slower
codec/view_vs_collection.ts[3]	Get 50th element from View	17081.03 ±0.84%	46.73% slower
codec/view_vs_collection.ts[4]	Get last element from Decoded	14832.4 ±0.35%	53.74% slower
codec/view_vs_collection.ts[5]	Get last element from View	11918.03 ±0.32%	62.83% slower
codec/view_vs_object.ts[0]	Get the first field from Decoded	199731.25 ±0.56%	1.65% slower
codec/view_vs_object.ts[1]	Get the first field from View	49351.02 ±1.11%	75.7% slower
codec/view_vs_object.ts[2]	Get the first field as view from View	49049.74 ±0.85%	75.85% slower
codec/view_vs_object.ts[3]	Get two fields from Decoded	198219.07 ±0.69%	2.39% slower
codec/view_vs_object.ts[4]	Get two fields from View	39450.88 ±0.63%	80.57% slower
codec/view_vs_object.ts[5]	Get two fields from materialized from View	79267.79 ±0.42%	60.97% slower
codec/view_vs_object.ts[6]	Get two fields as views from View	39676.39 ±0.39%	80.46% slower
codec/view_vs_object.ts[7]	Get only third field from Decoded	203080.03 ±0.36%	fastest ✅
codec/view_vs_object.ts[8]	Get only third field from View	49790.25 ±0.36%	75.48% slower
codec/view_vs_object.ts[9]	Get only third field as view from View	49444.75 ±0.38%	75.65% slower
collections/map_vs_sorted.ts[0]	Map	124647.39 ±1.23%	fastest ✅
collections/map_vs_sorted.ts[1]	Map-array	46812.53 ±0.38%	62.44% slower
collections/map_vs_sorted.ts[2]	Array	54399.08 ±0.23%	56.36% slower
collections/map_vs_sorted.ts[3]	SortedArray	82804.57 ±0.33%	33.57% slower
collections/hash-dict-vs-blob-dict_delete.ts[0]	StringHashDictionary	2677.01 ±0.48%	0.37% slower
collections/hash-dict-vs-blob-dict_delete.ts[1]	BlobDictionary(1)	2661.45 ±0.37%	0.95% slower
collections/hash-dict-vs-blob-dict_delete.ts[2]	BlobDictionary(2)	2673.41 ±0.38%	0.5% slower
collections/hash-dict-vs-blob-dict_delete.ts[3]	BlobDictionary(3)	2686.94 ±0.33%	fastest ✅
collections/hash-dict-vs-blob-dict_delete.ts[4]	BlobDictionary(4)	2666.32 ±0.29%	0.77% slower
collections/hash-dict-vs-blob-dict_delete.ts[5]	BlobDictionary(5)	2669.61 ±0.3%	0.64% slower
collections/hash-dict-vs-blob-dict_get.ts[0]	StringHashDictionary	2972.21 ±0.48%	1.43% slower
collections/hash-dict-vs-blob-dict_get.ts[1]	BlobDictionary(1)	3000.37 ±0.41%	0.5% slower
collections/hash-dict-vs-blob-dict_get.ts[2]	BlobDictionary(2)	2997.85 ±0.36%	0.58% slower
collections/hash-dict-vs-blob-dict_get.ts[3]	BlobDictionary(3)	3005.41 ±0.37%	0.33% slower
collections/hash-dict-vs-blob-dict_get.ts[4]	BlobDictionary(4)	2993.02 ±0.52%	0.74% slower
collections/hash-dict-vs-blob-dict_get.ts[5]	BlobDictionary(5)	3015.42 ±0.39%	fastest ✅
collections/hash-dict-vs-blob-dict_set.ts[0]	StringHashDictionary	2192.01 ±0.43%	fastest ✅
collections/hash-dict-vs-blob-dict_set.ts[1]	BlobDictionary(1)	2161.92 ±0.47%	1.37% slower
collections/hash-dict-vs-blob-dict_set.ts[2]	BlobDictionary(2)	2172.8 ±0.41%	0.88% slower
collections/hash-dict-vs-blob-dict_set.ts[3]	BlobDictionary(3)	2168.94 ±0.61%	1.05% slower
collections/hash-dict-vs-blob-dict_set.ts[4]	BlobDictionary(4)	2149.37 ±0.43%	1.95% slower
collections/hash-dict-vs-blob-dict_set.ts[5]	BlobDictionary(5)	2165.49 ±0.45%	1.21% slower
hash/blake2b.ts[0]	our hasher	1.06 ±0.16%	fastest ✅
hash/blake2b.ts[1]	blake2b js	0.03 ±0.05%	97.17% slower
crypto/ed25519.ts[0]	native crypto	5.691 ±1.01%	fastest ✅
crypto/ed25519.ts[1]	wasm lib	2.262 ±0.39%	60.25% slower
crypto/ed25519.ts[2]	wasm lib batch	2.263 ±0.17%	60.24% slower

Benchmarks summary: 83/83 OK ✅

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (1)

packages/jam/in-core/externalities/refine.ts (1)

98-100: ⚠️ Potential issue | 🟠 Major

ProgramCounter round-trip can still corrupt values above safe integer range.

machineExpunge now returns PC from interpreter, but machineInit still narrows at Line 150 via Number(programCounter). Values beyond 2^53 - 1 cannot round-trip exactly, so returned PC may differ from input.

#!/bin/bash
set -euo pipefail

# Verify JS-safe-integer precision loss for ProgramCounter-like values.
python - <<'PY'
pc = 2**53 + 1
js_num = float(pc)         # mirrors JS Number precision model
roundtrip = int(js_num)
print({"input_pc": pc, "roundtrip_pc": roundtrip, "equal": pc == roundtrip})
PY

# Verify narrowing and retrieval sites in this file.
rg -n -C2 'Number\(programCounter\)' packages/jam/in-core/externalities/refine.ts
rg -n -C2 'tryAsProgramCounter\(entry\[1\]\.getPC\(\)\)|machineExpunge\(' packages/jam/in-core/externalities/refine.ts

Expected: the Python output should show "equal": False, confirming precision loss risk.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/jam/in-core/externalities/refine.ts` around lines 98 - 100,
machineExpunge returns a ProgramCounter from the interpreter but machineInit
still narrows incoming programCounter with Number(programCounter), which will
corrupt values > 2^53-1; update the code paths (machineInit and any places that
call Number(programCounter)) to preserve full-precision by treating program
counters as BigInt or as string-backed BigInt semantics instead of Number,
remove the Number(...) cast, use
tryAsProgramCounter/tryAsProgramCounter(entry[1].getPC()) to validate/convert
from BigInt/string safely, and add range/format validation where needed so
round-trips via machineExpunge and machineInit do not lose precision.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/jam/in-core/externalities/refine.test.ts`:
- Around line 332-335: The test only asserts the second machineInit result
(initResult) and ignores the first and third, allowing partial setup failures to
pass; capture each machineInit return (e.g., initResult0 = await
ext.machineInit(code, tryAsProgramCounter(0)), initResult1 = await
ext.machineInit(code, tryAsProgramCounter(10)), initResult2 = await
ext.machineInit(code, tryAsProgramCounter(20))) and add assertions that
initResult0.isOk, initResult1.isOk, and initResult2.isOk are true (or fail test
with their error details) so every call to ext.machineInit is validated.
- Around line 307-327: The tests currently only check result.isError for
machineExpunge; update both assertions to assert the concrete error type
NoMachineError is returned: in the non-existent machine case (where
ext.machineExpunge(tryAsMachineId(999)) returns result) assert result.error is
an instance of or equals NoMachineError, and in the double-expunge case after r2
= await ext.machineExpunge(machineId) assert r2.error is NoMachineError; locate
uses of machineExpunge, createExt, tryAsMachineId, tryAsProgramCounter,
BytesBlob.blobFrom and MINIMAL_PROGRAM to modify the assertions accordingly so
the test fails for wrong error kinds.

---

Duplicate comments:
In `@packages/jam/in-core/externalities/refine.ts`:
- Around line 98-100: machineExpunge returns a ProgramCounter from the
interpreter but machineInit still narrows incoming programCounter with
Number(programCounter), which will corrupt values > 2^53-1; update the code
paths (machineInit and any places that call Number(programCounter)) to preserve
full-precision by treating program counters as BigInt or as string-backed BigInt
semantics instead of Number, remove the Number(...) cast, use
tryAsProgramCounter/tryAsProgramCounter(entry[1].getPC()) to validate/convert
from BigInt/string safely, and add range/format validation where needed so
round-trips via machineExpunge and machineInit do not lose precision.

🪄 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: 9d967cb2-9cf5-438f-8fe1-ed63ff691be3

📥 Commits

Reviewing files that changed from the base of the PR and between bfeb686 and 94a0dc4.

📒 Files selected for processing (2)

• packages/jam/in-core/externalities/refine.test.ts
• packages/jam/in-core/externalities/refine.ts