Add lite interpreter#921
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces support for a new "Lite" PVM backend variant across the codebase. Changes include adding a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/jam/config/pvm-backend.ts (1)
2-2: Add Lite to accumulate backend test matrix.
packages/jam/transition/accumulate/accumulate.test.tscurrently loops only over BuiltIn/Ananas, so the new Lite backend path is not covered there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jam/config/pvm-backend.ts` at line 2, The test matrix in accumulate.test.ts omits the new "lite" backend; update the test to include Lite by either referencing the exported PvmBackendNames array (export const PvmBackendNames) so the loop automatically covers "built-in", "ananas", and "lite", or explicitly add "lite" to the list used in the test runner so the accumulate tests exercise the Lite backend path as well.bin/test-runner/w3f/runners.ts (1)
50-50: Consider reusingALL_PVMSto avoid backend list drift.This list is now duplicated from
bin/test-runner/common.ts; importingALL_PVMShere would reduce future update misses when new backends are added.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/test-runner/w3f/runners.ts` at line 50, The pvms array duplicates the backend list; replace the hard-coded const pvms: SelectedPvm[] = [...] with an import and usage of the canonical ALL_PVMS to avoid drift: import ALL_PVMS (or the exported name holding all SelectedPvm values) from the module that defines it and use that constant wherever pvms is referenced (replace references to pvms with ALL_PVMS and remove the local SelectedPvm array declaration).
🤖 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/config/pvm-backend.ts`:
- Around line 10-11: The new enum value PvmBackend.Lite (numeric value 2) is not
handled by the importer deserialization logic and causes "Invalid PvmBackend: 2"
at runtime; update the importer protocol deserializer (the function that
validates/parses PvmBackend in packages/workers/importer/protocol.ts) to accept
numeric value 2 and map it to PvmBackend.Lite (add a case/branch for 2 or
include 2 in the allowed values), ensuring the deserializer returns the
PvmBackend.Lite enum value instead of rejecting it and adjust any related error
text to include unexpected values only.
---
Nitpick comments:
In `@bin/test-runner/w3f/runners.ts`:
- Line 50: The pvms array duplicates the backend list; replace the hard-coded
const pvms: SelectedPvm[] = [...] with an import and usage of the canonical
ALL_PVMS to avoid drift: import ALL_PVMS (or the exported name holding all
SelectedPvm values) from the module that defines it and use that constant
wherever pvms is referenced (replace references to pvms with ALL_PVMS and remove
the local SelectedPvm array declaration).
In `@packages/jam/config/pvm-backend.ts`:
- Line 2: The test matrix in accumulate.test.ts omits the new "lite" backend;
update the test to include Lite by either referencing the exported
PvmBackendNames array (export const PvmBackendNames) so the loop automatically
covers "built-in", "ananas", and "lite", or explicitly add "lite" to the list
used in the test runner so the accumulate tests exercise the Lite backend path
as well.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e025cbea-5678-4027-a9a8-c3c8c5a85f8c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
bin/test-runner/common.tsbin/test-runner/javajam-071.tsbin/test-runner/w3f/accumulate.tsbin/test-runner/w3f/runners.tspackages/core/pvm-host-calls/package.jsonpackages/core/pvm-host-calls/pvm-instance-manager.tspackages/jam/config/pvm-backend.ts
|
@fluffylabs-bot benchmark |
|
✅ Benchmark workflow triggered successfully! 🎉 🔗 Check the Actions tab for workflow progress. |
Picofuzz Benchmark Resultsfallback
safrole
storage
storage_light
🤖 Automated benchmark |
c5cd60c to
a412f33
Compare
|
@fluffylabs-bot benchmark |
|
✅ Benchmark workflow triggered successfully! 🎉 🔗 Check the Actions tab for workflow progress. |
|
@fluffylabs-bot benchmark |
|
✅ Benchmark workflow triggered successfully! 🎉 🔗 Check the Actions tab for workflow progress. |
|
@fluffylabs-bot benchmark |
|
✅ Benchmark workflow triggered successfully! 🎉 🔗 Check the Actions tab for workflow progress. |
View all
Benchmarks summary: 83/83 OK ✅ |
No description provided.