Conversation
WalkthroughThis update modernizes the GenVM documentation and build workflows by adopting Nix for environment management and adding scripts for documentation generation and merging. It restructures and expands documentation with new sections, removes outdated files, and clarifies specifications. The executor gains dynamic WASM version detection and improved Wasmtime configuration. Test cases are generalized to use variable filenames, and new test cases and tooling scripts are added for runner documentation and version mapping. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Nix
participant Ruby/Python Scripts
participant Sphinx
participant Documentation Output
GitHub Actions->>Nix: Setup environment (via Nix action)
Nix->>Ruby/Python Scripts: Run codegen, merging, and tag-mapping scripts
Ruby/Python Scripts->>Sphinx: Prepare source files and configuration
Sphinx->>Documentation Output: Build HTML and text documentation
GitHub Actions->>Documentation Output: Deploy built documentation
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (2)
doc/website/src/spec/appendix/index.rst (1)
1-9: Missing referenced document in appendix toctreeThe verification script found
runner-schema.rstbut did not locate aconstants.rstfile underdoc/website/src/spec/appendix/. Please add the missing file or remove theconstantsentry.• Location:
doc/website/src/spec/appendix/index.rst
- Entry
constantshas no correspondingconstants.rstin the same directory..github/workflows/pr.yaml (1)
27-31: Setup-Poetry should come after Python setup to avoid mixed runtimes
Gr1N/setup-poetry@v9bootstraps Poetry viapipx; it uses the current Python on the runner.
Running it beforeactions/setup-pythonmeans Poetry is installed against the stock 3.11, but the project later activates 3.12, leading to virtual-env mismatch and flaky cache hits.A minimal re-ordering (and pinning Poetry) prevents this:
- - uses: Gr1N/setup-poetry@v9 - - uses: actions/setup-python@v5 - with: - python-version: '3.12' - cache: poetry + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + cache: poetry + - uses: Gr1N/setup-poetry@v9 + with: + poetry-version: "1.8.2"
🧹 Nitpick comments (19)
doc/website/.gitignore (1)
1-2: Clarify intent & future-proof the ignore patternsThese two patterns are rooted at
doc/website/because of the leading “/” in a sub-folder.gitignore.
If the same generated files ever move outsidedoc/website/, or a parallel docs build is introduced at project root, they will start leaking into git.
Consider either:- /src/spec/appendix/constants.rst - /src/impl-spec/appendix/runners-versions.json +doc/website/src/spec/appendix/constants.rst +doc/website/src/impl-spec/appendix/runners-versions.jsonor add a short comment explaining the anchoring decision so future contributors don’t get bitten.
doc/website/src/impl-spec/appendix/runners-versions.rst (1)
4-5: Missing option flags may break HTML build
.. include:: … :literal:works, but the raw JSON block renders as a single very-wide line in HTML, making the page almost unreadable.
Adding:code:or wrapping with.. code-block:: jsongives automatic line-wrapping & syntax highlighting:-.. include:: runners-versions.json - :literal: +.. include:: runners-versions.json + :code: jsondoc/website/src/impl-spec/02-vm/index.rst (1)
4-7: Add caption for clearer navigation sidebarSphinx shows bare directory names when a toctree lacks a caption.
A short caption improves the left-hand nav & breadcrumbs:.. toctree:: :maxdepth: 2 + :caption: Runtime 01-wasm-runtimedoc/website/src/spec/01-core-architecture/01-components.rst (1)
7-25: Minor reST nit – backslash escape is visible
\safter:term:`sub-VM`\leaks into the rendered text.
Usesub-VMsor:term:`sub-VM`\ s(note the space) to keep the plural marker outside the inline-literal.doc/website/merge-txts.rb (1)
3-6: Argument validation would harden the scriptIf callers pass fewer than two arguments, Ruby raises an unhelpful
undefined methoderror later. Guard early:abort "usage: merge-txts.rb <src_dir> <dst_file>" unless ARGV.size == 2doc/website/src/spec/03-vm/03-ram-limiting.rst (3)
7-7: Fix awkward phrasing and remove unnecessary escape.The line has awkward phrasing and an unnecessary backslash escape that makes it less readable.
-RAM limit is 4294967295 octets (4 GB) for each :ref:`gvm-def-non-det-mode`\. +The RAM limit for :ref:`gvm-def-non-det-mode` is 4294967295 octets (4 GB).
14-14: Clarify RAM consumption definition.The current phrasing is unclear and could be misunderstood. Consider rephrasing for better clarity.
-RAM is consumed by subtracting specified amount of memory from limit of current :ref:`gvm-def-vm-mode` of given :term:`sub-VM`\. +RAM consumption is tracked by deducting the allocated memory amount from the current :ref:`gvm-def-vm-mode` limit for the given :term:`sub-VM`.
24-24: Add missing period for consistency.-However, during the execution process, the RAM memory can not be released. +However, during the execution process, the RAM memory can not be released.executor/codegen/templates/rst.rb (3)
1-1: Add execute permission to the script file.The static analysis correctly identifies that this script file lacks execute permission, which is needed for it to run as a standalone script.
chmod +x executor/codegen/templates/rst.rb
7-8: Add input validation for command line arguments.The script doesn't validate that the required command line arguments are provided, which could lead to confusing errors if run incorrectly.
json_path, out_path = ARGV +if json_path.nil? || out_path.nil? + puts "Usage: #{$0} <json_path> <out_path>" + exit 1 +end
17-17: Add error handling for JSON file operations.The script should handle potential file reading errors gracefully.
-JSON.load_file(Pathname.new(json_path)).each { |t| +begin + JSON.load_file(Pathname.new(json_path)).each { |t|And add corresponding error handling at the end of the block.
runners/support/match-tags.py (3)
3-3: Fix import organization per Python style guidelines.Multiple imports should be on separate lines for better readability and maintainability.
-import subprocess, json, sys +import json +import subprocess +import sys
37-37: Move import to the top of the file.Module-level imports should be at the top of the file according to PEP 8.
+import os import subprocess, json, sys from pathlib import Path - -import os
35-35: Remove debug print statement.This appears to be debug code that should be removed from the final script.
-print(commit2tag)doc/website/src/spec/02-execution-environment/01-wasm.rst (1)
33-33: Improve phrasing for WASM memory cost description.The current phrasing is awkward and could be clearer.
-Each WASM Memory costs length of bytes it has. WASM ``memory.grow`` instruction which would exceed limit returns :math:`-1` +Each WASM Memory consumes RAM proportional to its byte length. The WASM ``memory.grow`` instruction returns :math:`-1` if the requested growth would exceed the memory limit.doc/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk.rst (1)
80-154: Comprehensive function documentation enhances API usability.The new "Function Descriptions" section provides essential details for each API function including parameters, return values, and requirements. This significantly improves the developer experience by making the API self-documenting.
Minor suggestions for consistency:
- Consider adding a description for the
gl_callfunction (line 84) which currently only has a header- Ensure all parameter types are consistently formatted (e.g., "u32" vs "uint32_t")
doc/website/src/impl-spec/appendix/host-loop.rst (2)
135-143: Minor syntax / clarity glitches in thepost_eventbranch
read_byte topics_lenlooks like a procedure call missing an assignment operator – all other reads assign the result (:=).blob := read_sliceis missing(); elsewhereread_slice()is used.- json/methods/post_event: - read_byte topics_len + json/methods/post_event: + topics_len := read_byte … - blob := read_slice + blob := read_slice()These small inconsistencies break the otherwise uniform pseudo-code style and could trip up readers translating the spec into code.
31-41: Return-status pattern is clear – call it out once and factorEvery branch manually emits
if err != json/errors/ok: write_byte err else: write_byte json/errors/ok …Repeating this ~15 times bloats the spec. Consider introducing a helper macro/pseudocode primitive such as
reply(err, payload_writer)to keep the listing concise and highlight the uniform contract.doc/website/src/spec/02-execution-environment/02-wasip1.rst (1)
90-100: Tiny typographical slip: “Acces” header pluralThe bullet list reads “Fail with
Acceserror code:” – while the WASI errno identifier is indeedacces, consider rewriting the prose to “Fail with theAcceserror code:” or “Fail withErrno::Acces” for absolute clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
executor/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (86)
.github/workflows/deploy-docs.yml(1 hunks).github/workflows/genvm-build-runners.yaml(0 hunks).github/workflows/pr.yaml(1 hunks).github/workflows/release.yml(1 hunks)build-scripts/devenv/flake.nix(1 hunks)doc/website/.gitignore(1 hunks)doc/website/generate-other.rb(0 hunks)doc/website/merge-txts.rb(1 hunks)doc/website/src/api/genlayer.rst(0 hunks)doc/website/src/api/index.rst(1 hunks)doc/website/src/impl-spec/01-core-architecture/02-host-interface.rst(2 hunks)doc/website/src/impl-spec/01-core-architecture/03-modules.rst(1 hunks)doc/website/src/impl-spec/02-vm/01-wasm-runtime.rst(1 hunks)doc/website/src/impl-spec/02-vm/index.rst(1 hunks)doc/website/src/impl-spec/appendix/host-loop.rst(1 hunks)doc/website/src/impl-spec/appendix/index.rst(1 hunks)doc/website/src/impl-spec/appendix/message-schema.rst(1 hunks)doc/website/src/impl-spec/appendix/runners-versions.rst(1 hunks)doc/website/src/impl-spec/index.rst(2 hunks)doc/website/src/spec-old/abi/calldata.rst(0 hunks)doc/website/src/spec-old/abi/contract_startup.rst(0 hunks)doc/website/src/spec-old/abi/index.rst(0 hunks)doc/website/src/spec-old/abi/runners.rst(0 hunks)doc/website/src/spec-old/abi/upgradability.rst(0 hunks)doc/website/src/spec-old/index.rst(0 hunks)doc/website/src/spec/01-core-architecture/01-components.rst(2 hunks)doc/website/src/spec/01-core-architecture/02-vm.rst(7 hunks)doc/website/src/spec/02-execution-environment/01-wasip1.rst(0 hunks)doc/website/src/spec/02-execution-environment/01-wasm.rst(1 hunks)doc/website/src/spec/02-execution-environment/02-wasip1.rst(1 hunks)doc/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk.rst(3 hunks)doc/website/src/spec/02-execution-environment/04-runners.rst(10 hunks)doc/website/src/spec/02-execution-environment/index.rst(1 hunks)doc/website/src/spec/03-contract-interface/02-abi.rst(0 hunks)doc/website/src/spec/03-contract-interface/03-storage.rst(0 hunks)doc/website/src/spec/03-contract-interface/04-upgradability.rst(0 hunks)doc/website/src/spec/03-contract-interface/05-execution-flow.rst(0 hunks)doc/website/src/spec/03-vm/01-result.rst(1 hunks)doc/website/src/spec/03-vm/02-sandboxing.rst(1 hunks)doc/website/src/spec/03-vm/03-ram-limiting.rst(1 hunks)doc/website/src/spec/03-vm/04-determinism-mode-switching.rst(1 hunks)doc/website/src/spec/03-vm/index.rst(1 hunks)doc/website/src/spec/04-contract-interface/01-calldata.rst(1 hunks)doc/website/src/spec/04-contract-interface/02-abi.rst(1 hunks)doc/website/src/spec/04-contract-interface/03-storage.rst(1 hunks)doc/website/src/spec/04-contract-interface/04-upgradability.rst(1 hunks)doc/website/src/spec/04-contract-interface/05-execution-flow.rst(1 hunks)doc/website/src/spec/appendix/index.rst(1 hunks)doc/website/src/spec/appendix/runner-schema.rst(1 hunks)doc/website/src/spec/index.rst(2 hunks)doc/website/yabuild.rb(1 hunks)executor/Cargo.toml(1 hunks)executor/codegen/data/public-abi.json(2 hunks)executor/codegen/templates/rst.rb(1 hunks)executor/src/vm.rs(3 hunks)executor/src/wasi/preview1.rs(2 hunks)executor/testdata/cases/_hello_world_class.jsonnet(1 hunks)executor/testdata/cases/_hello_world_class_nondet.jsonnet(1 hunks)executor/testdata/cases/exploits/flt.jsonnet(1 hunks)executor/testdata/cases/exploits/unreachable.jsonnet(1 hunks)executor/testdata/cases/exploits/version.jsonnet(1 hunks)executor/testdata/cases/exploits/version.stdout(1 hunks)executor/testdata/cases/exploits/version.wat(1 hunks)executor/testdata/cases/module/np.jsonnet(1 hunks)executor/testdata/cases/module/pil.jsonnet(1 hunks)executor/testdata/cases/py-core/balances/balance.jsonnet(1 hunks)executor/testdata/cases/py-core/deploy.jsonnet(1 hunks)executor/testdata/cases/py-core/embeddings/simple.jsonnet(1 hunks)executor/testdata/cases/py-core/embeddings/vecdb.jsonnet(1 hunks)executor/testdata/cases/py-core/erroring/oom.jsonnet(1 hunks)executor/testdata/cases/py-core/nondet/trivial.jsonnet(1 hunks)executor/testdata/cases/py-core/nondet/validator/sync.jsonnet(1 hunks)executor/testdata/cases/py-core/rollbacks/nondet.jsonnet(1 hunks)executor/testdata/cases/py-core/schemas/trivial.jsonnet(1 hunks)executor/testdata/cases/self-run/datetime.jsonnet(1 hunks)executor/testdata/cases/self-run/floats.jsonnet(1 hunks)executor/testdata/cases/self-run/formats.jsonnet(1 hunks)executor/testdata/cases/self-run/re.jsonnet(1 hunks)executor/testdata/cases/storage/base.jsonnet(1 hunks)executor/testdata/cases/storage/floats.jsonnet(1 hunks)executor/testdata/cases/storage/np.jsonnet(1 hunks)executor/testdata/cases/storage/persists.jsonnet(1 hunks)executor/testdata/runner/run.py(3 hunks)runners/docs.nix(1 hunks)runners/genlayer-py-std/src/genlayer/py/storage/__init__.py(1 hunks)runners/support/match-tags.py(1 hunks)
💤 Files with no reviewable changes (14)
- doc/website/src/api/genlayer.rst
- doc/website/src/spec-old/abi/index.rst
- doc/website/src/spec-old/index.rst
- doc/website/src/spec/03-contract-interface/04-upgradability.rst
- doc/website/src/spec/03-contract-interface/02-abi.rst
- .github/workflows/genvm-build-runners.yaml
- doc/website/generate-other.rb
- doc/website/src/spec/03-contract-interface/03-storage.rst
- doc/website/src/spec-old/abi/contract_startup.rst
- doc/website/src/spec-old/abi/upgradability.rst
- doc/website/src/spec-old/abi/runners.rst
- doc/website/src/spec/02-execution-environment/01-wasip1.rst
- doc/website/src/spec-old/abi/calldata.rst
- doc/website/src/spec/03-contract-interface/05-execution-flow.rst
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/release.yml
160-160: property "gen-tag" is not defined in object type {release-publish: {outputs: {}; result: string}}
(expression)
.github/workflows/deploy-docs.yml
46-46: description is required in metadata of "GenVM get source" action at "/home/jailuser/git/.github/actions/get-src/action.yaml"
(action)
🪛 RuboCop (1.76.1)
executor/codegen/templates/rst.rb
[warning] 1-1: Script file rst.rb doesn't have execute permission.
(Lint/ScriptPermission)
🪛 Ruff (0.12.2)
runners/support/match-tags.py
3-3: Multiple imports on one line
Split imports
(E401)
37-37: Module level import not at top of file
(E402)
🔇 Additional comments (102)
executor/Cargo.toml (1)
36-36: Double-check zip crate v4.0.0 API compatibilityThe
zipdependency has been bumped from 2.5.0 → 4.0.0, a major release that likely introduces breaking changes. Confirm that all existing calls still match the new API before merging—compile-time errors or behavioral changes here could break archive handling.Key usage sites to verify:
- executor/src/vm.rs
zip::ZipArchive::new(std::io::Cursor::new(...))- executor/src/ustar.rs
zip::ZipArchive<R>zip::CompressionMethod::Storedzip.by_index(i)?Action items:
- Review the zip 4.0.0 changelog for any renamed modules, changed function signatures, or altered enum variants.
- Update imports or call sites as needed.
- Run the full test suite (and exercise wasm archive flows) to catch any regressions.
runners/genlayer-py-std/src/genlayer/py/storage/__init__.py (1)
57-58: copy_to_memory callers are all storage instances with__type_desc__; removal of fallback is safe
- All
copy_to_memorycalls were inexecutor/testdata/cases/py-core/pitfalls/state_in_nondet.py, operating onself.user/self.gen_user—both storage record objects.- Storage classes produced by
_internal/generate.py(and enforced in_internal/desc_record.py) set__type_desc__on every instance.No changes required.
build-scripts/devenv/flake.nix (1)
59-73: Well-structured documentation environment.The new
docsdevShell provides a clean, minimal environment focused specifically on documentation tasks. The separation from the default development environment is a good practice, and the package selection (Python, Poetry, Ninja, Ruby, Mermaid) aligns well with documentation build requirements.doc/website/src/spec/04-contract-interface/01-calldata.rst (1)
1-4: Improved documentation structure and cross-referencing.The addition of the reference label
.. _gvm-def-calldata-encoding:and the title change from "Calldata Format" to "Calldata Encoding" improve documentation consistency and enable proper cross-referencing from other specification documents.doc/website/src/api/index.rst (1)
4-5: Enhanced documentation accessibility for AI consumers.The addition of the note directive with a link to consolidated single text file documentation improves machine readability and follows the consistent pattern established across other documentation indices in this PR.
executor/testdata/cases/storage/base.jsonnet (1)
2-2: fileBaseName properly defined in test runnerThe
fileBaseNamevariable is set inexecutor/testdata/runner/run.py(viajsonnet_path.stem), so the${fileBaseName}.pyinterpolation will resolve correctly. LGTM—approving this change.executor/testdata/cases/py-core/deploy.jsonnet (1)
2-2: Consistent generalization with proper JSON structure.The change follows the same pattern of replacing hardcoded filename with
${fileBaseName}.pywhile maintaining the deployment-specific configuration (calldata with__init__method andis_init: truemessage property).executor/testdata/cases/self-run/formats.jsonnet (1)
2-2: Consistent with the test generalization pattern.The change matches the same refactoring pattern applied across test cases, replacing hardcoded
formats.pywith dynamic${fileBaseName}.pyfor improved test flexibility.doc/website/src/impl-spec/appendix/index.rst (1)
1-10: Verified referenced implementation appendix documentsAll three files referenced in the toctree are present under
doc/website/src/impl-spec/appendix/:
- host-loop.rst
- message-schema.rst
- runners-versions.rst
Great work separating specification and implementation documentation—approving these changes.
executor/testdata/cases/exploits/version.stdout (1)
1-2: LGTM! Test output format is correct.The test output correctly validates the "version_too_big" error case for the new version detection functionality. The VMError format and trailing newline are consistent with other test output files.
executor/testdata/cases/storage/np.jsonnet (1)
2-2: LGTM! Dynamic filename improves test flexibility.The change from hardcoded filename to
${fileBaseName}variable makes the test more flexible and reusable, allowing it to work with different Python files based on context.executor/testdata/cases/py-core/schemas/trivial.jsonnet (1)
2-2: LGTM! Consistent with test generalization pattern.The dynamic filename change follows the same pattern as other test files, improving maintainability while preserving the existing test configuration.
executor/testdata/cases/self-run/floats.jsonnet (1)
2-2: LGTM! Follows consistent test generalization pattern.The change from hardcoded filename to dynamic
${fileBaseName}variable is consistent with the broader test refactoring effort and improves test flexibility.executor/testdata/cases/py-core/embeddings/vecdb.jsonnet (1)
2-2: LGTM! Consistent dynamic filename pattern maintained.The change to use
${fileBaseName}instead of hardcoded filename follows the established pattern across test files, improving test flexibility while preserving the existing calldata configuration.doc/website/src/impl-spec/01-core-architecture/03-modules.rst (1)
30-30: LGTM: Improved cross-referencing enhances documentation navigation.The replacement of generic text with a specific cross-reference label
:ref:gvm-def-calldata-encoding`` improves documentation consistency and provides better navigation to the calldata encoding specification.executor/testdata/cases/exploits/unreachable.jsonnet (1)
2-2: Approved:fileBaseNameis properly injected in the test runnerThe grep output from
executor/testdata/runner/run.pyconfirms thatfileBaseNameis set tojsonnet_path.stemin the variable substitution context. No further changes needed.executor/testdata/cases/exploits/flt.jsonnet (1)
2-2: LGTM: Consistent test infrastructure improvement.The dynamic filename pattern
${fileBaseName}.wataligns with the broader test infrastructure generalization and improves template reusability.doc/website/src/impl-spec/index.rst (2)
4-5: LGTM: AI-oriented documentation link enhances accessibility.The addition of a single text file documentation link for AI consumption improves accessibility and provides an alternative format for the implementation specification.
16-17: LGTM: Expanded documentation structure improves organization.The addition of "02-vm/index" and "appendix/index" sections enhances the documentation's navigational structure and provides better organization for VM-related content and supplementary materials.
executor/testdata/cases/self-run/re.jsonnet (1)
2-2: LGTM: Consistent dynamic filename pattern for Python tests.The change from
're.py'to'${fileBaseName}.py'follows the same beneficial pattern applied across the test infrastructure, enabling flexible test execution for Python files.doc/website/src/spec/appendix/runner-schema.rst (1)
4-5: Verify relative path – one “..” short
runner-schema.rstlives indoc/website/src/spec/appendix/.
Four dots climb todoc/, so the current include resolves todoc/schemas/runners.json.
If the schema actually sits at repository rootschemas/runners.json, you need five levels:-.. include:: ../../../../schemas/runners.json +.. include:: ../../../../../schemas/runners.jsonPlease confirm the location; Sphinx will fail silently if the file isn’t found.
executor/testdata/cases/py-core/balances/balance.jsonnet (1)
2-2: LGTM! Good refactoring for test flexibility.The change from hardcoded filename to dynamic
${fileBaseName}variable improves test maintainability and allows for better parameterization of test cases.executor/testdata/cases/exploits/version.jsonnet (1)
1-2: LGTM! New exploit test case for version handling.This new test case follows the established pattern of using dynamic
${fileBaseName}variables and appropriately targets WebAssembly text format files (.wat) for testing version validation logic.executor/testdata/cases/module/np.jsonnet (1)
2-2: LGTM! Consistent with test suite refactoring.The change from hardcoded
'np.py'to dynamic'${fileBaseName}.py'maintains consistency with the broader test suite refactoring and improves test flexibility.executor/testdata/cases/py-core/nondet/trivial.jsonnet (1)
2-2: LGTM! Consistent filename parameterization.The change from hardcoded
'trivial.py'to dynamic'${fileBaseName}.py'aligns with the test suite refactoring pattern and improves test maintainability.executor/testdata/cases/py-core/rollbacks/nondet.jsonnet (1)
2-2: LGTM! Completes the consistent test suite refactoring.The change from hardcoded
'nondet.py'to dynamic'${fileBaseName}.py'completes the consistent pattern across all reviewed test cases, improving test flexibility and maintainability.doc/website/src/impl-spec/appendix/message-schema.rst (2)
1-2: LGTM! Proper RST documentation structure.The title formatting and structure follow RST conventions correctly.
4-5: Include directive path verified
The relative path../../../../schemas/message.jsonresolves todoc/schemas/message.json, which exists. No changes are needed.doc/website/src/spec/index.rst (1)
17-19: All toctree entries verified
All referenced files exist underdoc/website/src/spec:
03-vm/index.rst04-contract-interface/index.rstappendix/index.rstNo further action required.
executor/testdata/cases/storage/floats.jsonnet (1)
2-2: fileBaseName is correctly defined and used consistently.The
fileBaseNamevariable is injected in the test runner and all Jsonnet cases—includingstorage/floats.jsonnet—use the same${jsonnetDir}/${fileBaseName}.pypattern.• Defined in executor/testdata/runner/run.py:
{'jsonnetDir': str(jsonnet_path.parent), 'fileBaseName': jsonnet_path.stem}
• Usage in executor/testdata/cases/storage/floats.jsonnet matches all other cases.All checks pass; approving the parameterization.
doc/website/src/spec/03-vm/index.rst (2)
1-2: LGTM! Proper RST documentation structure.The title formatting and document structure follow RST conventions correctly.
7-10: All referenced VM specification documents existVerified that the following files are present under
doc/website/src/spec/03-vm:
- 01-result.rst
- 02-sandboxing.rst
- 03-ram-limiting.rst
- 04-determinism-mode-switching.rst
The RST toctree is valid and follows project conventions. No further changes needed.
executor/testdata/cases/py-core/nondet/validator/sync.jsonnet (2)
2-2: LGTM! Consistent test case parameterization.The filename parameterization follows the same pattern as other test cases, improving flexibility while maintaining the test configuration integrity.
3-16: Test configuration looks well-structured.The test configuration with calldata, sync mode, and leader_nondet settings is properly formatted and follows the expected JSON structure.
executor/testdata/cases/module/pil.jsonnet (1)
2-2: LGTM! Clean refactoring for dynamic filename support.The change from hardcoded filename to
${fileBaseName}.pyimproves test flexibility and aligns with the systematic refactoring across test cases.doc/website/src/spec/02-execution-environment/index.rst (1)
9-12: LGTM! Well-structured documentation hierarchy.The addition of "01-wasm" as the foundational entry and renumbering of existing entries creates a logical documentation flow for the execution environment specification.
executor/testdata/cases/self-run/datetime.jsonnet (1)
2-2: LGTM! Consistent with the dynamic filename refactoring.The change maintains the same beneficial pattern of replacing hardcoded filenames with
${fileBaseName}for improved test flexibility.executor/testdata/cases/_hello_world_class_nondet.jsonnet (1)
2-2: LGTM! Consistent dynamic filename implementation.The change follows the established pattern of using
${fileBaseName}while preserving the existing calldata configuration.executor/testdata/cases/py-core/embeddings/simple.jsonnet (1)
2-2: LGTM! Completes the consistent dynamic filename refactoring.The change maintains the established pattern while preserving the existing calldata configuration for the embeddings test case.
executor/testdata/cases/_hello_world_class.jsonnet (1)
2-2: LGTM: Dynamic filename improves test flexibilityThe change from hardcoded filename to
${fileBaseName}.pymakes the test case more reusable and maintainable. This standardization across test cases is a good improvement.executor/testdata/cases/py-core/erroring/oom.jsonnet (1)
2-2: LGTM: Consistent with test case standardizationThe dynamic filename pattern is correctly applied, maintaining consistency with the broader refactoring effort across test cases.
doc/website/src/impl-spec/02-vm/01-wasm-runtime.rst (1)
1-11: LGTM: Clear documentation of runtime modificationsThe documentation effectively describes the key modifications made to Wasmtime for blockchain use. The structure is clear and covers the essential aspects: deterministic execution, resource limits, error handling, and floating point restrictions.
executor/testdata/cases/storage/persists.jsonnet (1)
3-3: LGTM: Consistent dynamic filename implementationBoth test runs in the array are correctly updated to use the dynamic filename pattern, maintaining consistency with the broader test case standardization effort.
Also applies to: 11-11
doc/website/src/spec/03-vm/02-sandboxing.rst (1)
1-14: LGTM: Comprehensive sandboxing specificationThe documentation clearly describes the sandboxing feature with well-defined rules for sub-VM behavior, inheritance, restrictions, and error handling. The use of proper reStructuredText formatting and cross-references enhances readability.
executor/testdata/cases/exploits/version.wat (1)
1-9: Looks good – well-formed WAT fixtureThe custom
genvm.versionsection and the intentionalunreachabletrap correctly exercise the version-parsing logic. No issues spotted.executor/codegen/data/public-abi.json (1)
56-59: Newvm_error.exit_codeentry – double-check downstream handlingThe string looks fine here, but be sure every pattern-match / switch in the runtime now covers the new variant to avoid unexpected
unreachable!()or default branches.executor/testdata/runner/run.py (3)
99-102: Good enhancement for dynamic test file referencing.The addition of
'fileBaseName': jsonnet_path.stemenables test configurations to dynamically reference the base filename, which promotes reusability and reduces hardcoded values in test files.
130-132: Consistent addition of WASM annotations support.The
--enable-annotationsflag enables preservation of annotation information in the generated WASM binary, which aligns with the broader executor enhancements for version detection and metadata processing.
168-170: Maintains consistency in WASM processing.Good consistency in applying the
--enable-annotationsflag to both wat2wasm calls in the codebase, ensuring uniform behavior across the test execution pipeline.doc/website/src/spec/03-vm/04-determinism-mode-switching.rst (1)
1-25: Well-structured documentation for determinism mode switching.The documentation clearly explains the three operational modes (Leader, Sync, Validator) when switching to non-deterministic execution. The structure is logical and the content is technically accurate with proper RST formatting.
executor/src/wasi/preview1.rs (3)
369-369: Appropriate fixed value for deterministic execution.Returning
Ok(1)for clock resolution provides deterministic behavior, which is essential for the GenVM execution environment where consistent results across different systems are required.
375-375: Good practice for unused parameter handling.Prefixing the unused parameter with underscore follows Rust conventions and helps suppress compiler warnings while maintaining the required function signature.
1134-1134: Appropriate restriction for sandboxed execution.Returning
Errno::Notsupforpoll_oneoffaligns with the restrictive nature of the GenVM execution environment, where certain system operations are intentionally limited to maintain security and determinism.doc/website/src/spec/04-contract-interface/04-upgradability.rst (1)
1-17: Clear documentation of contract upgradability system.The documentation effectively explains the native contract upgradability system, covering both the data structure (root storage slot) and the access control mechanism. The structure is logical and the content is technically accurate with proper RST formatting.
doc/website/src/spec/04-contract-interface/02-abi.rst (1)
1-40: Comprehensive ABI documentation with clear examples.The documentation provides a thorough explanation of the GenVM ABI, including method calling conventions and special methods. The JSON examples are well-formatted and the distinction between deployment and non-deployment calls is clearly explained.
doc/website/src/spec/03-vm/03-ram-limiting.rst (1)
4-7: Missing deterministic mode RAM limit specification.The document specifies the RAM limit for non-deterministic mode but doesn't mention the limit for deterministic mode. This creates an incomplete specification.
Please verify if the deterministic mode RAM limit should be documented here or if it's intentionally omitted. If it has a different limit, it should be specified for completeness.
executor/codegen/templates/rst.rb (1)
17-45: Good implementation of JSON to RST conversion.The script effectively processes enums and constants from JSON input and generates well-structured RST documentation with proper labels and references. The use of OpenStruct for JSON object handling is appropriate and the string building approach works well for this use case.
runners/support/match-tags.py (1)
6-71: Well-structured subprocess handling and error management.The script demonstrates good practices with subprocess calls using
check=True, proper output capture, and appropriate error handling. The complex Nix command construction with JSON escaping is handled correctly.doc/website/src/spec/02-execution-environment/01-wasm.rst (2)
7-11: Well-documented WASM features and proposals.The enumeration of enabled WASM features is clear and comprehensive, providing good reference material for developers.
16-24: Clear specification of deterministic mode limitations.The floating-point operation restrictions are well-documented with specific allowed operations listed. This provides clear guidance for developers working with deterministic execution.
doc/website/src/impl-spec/01-core-architecture/02-host-interface.rst (3)
34-34: Good modular approach to documentation organization.Moving the detailed pseudocode to a separate appendix document improves readability and maintainability while keeping the reference easily accessible.
58-67: Improved formatting enhances readability.The conversion from inline text to a structured bullet list format makes the result encoding specification much clearer and easier to follow.
34-34: Appendix Reference VerifiedThe referenced document
host-loop.rstexists at:
- doc/website/src/impl-spec/appendix/host-loop.rst
No further action is needed.
doc/website/src/spec/02-execution-environment/03-wasi_genlayer_sdk.rst (2)
18-18: Good improvement: Added proper cross-reference to calldata encoding.The update correctly links to the calldata encoding specification, improving documentation navigation and clarity.
66-66: Consistent cross-referencing improves documentation quality.The serialization section now properly references the same calldata encoding format, maintaining consistency throughout the document.
doc/website/yabuild.rb (4)
11-11: Path construction looks correct.The codegen source path properly navigates from current source to the executor codegen directory.
17-17: Code generation integration enhances documentation automation.Adding the RST template generation step automates the creation of constants documentation from the JSON ABI, improving maintainability and consistency.
19-28: Build process improvements streamline documentation generation.The changes effectively:
- Remove intermediate build directories for cleaner process
- Add text file merging for AI documentation consumption
- Update Sphinx build commands to work directly with source
The command structure and dependencies appear well-organized.
30-30: Relative paths verified under root_srcAll referenced files and directories are present when using the repository root as the working directory, so switching
cwdtoroot_srcdoes not break any path resolutions:
- runners/support/match-tags.py
- doc/website/src
- doc/website/merge-txts.rb
- executor/codegen/templates/rst.rb
- executor/codegen/data/public-abi.json
.github/workflows/deploy-docs.yml (2)
40-43: Nix integration modernizes environment management.The switch to Nix-based environment setup provides better reproducibility and consistency. The configuration looks correct with proper GitHub token access and specific Nix version.
51-61: Build process correctly uses Nix development environment.The build steps properly:
- Enter the Nix development environment
- Install Poetry dependencies in the correct directory
- Execute the build commands
The command structure is sound and follows best practices.
doc/website/src/spec/04-contract-interface/05-execution-flow.rst (1)
1-53: Excellent new documentation for contract execution flow.This comprehensive document effectively describes the complete contract execution lifecycle with:
- Clear step-by-step process from deployment to execution
- Proper cross-references to related specification documents
- Good use of RST formatting and terminology
- Detailed coverage of entry point processing and calldata fields
The technical content appears accurate and well-structured, providing valuable guidance for developers working with GenVM contracts.
doc/website/src/spec/04-contract-interface/03-storage.rst (1)
1-55: Comprehensive storage system documentation.This new document provides excellent coverage of GenVM's storage system with:
- Clear architecture explanation with specific technical details (4GB slots, 32-byte SlotIDs)
- Well-explained default derivation algorithm with concrete examples
- Detailed root slot structure covering contract metadata, code, and upgrade permissions
- Proper cross-references to related upgradability documentation
The technical content is well-structured and the examples help clarify complex concepts like storage slot indirection.
doc/website/src/spec/02-execution-environment/04-runners.rst (16)
14-14: Good clarification on the hash algorithm.Specifying "sha3-256" instead of just "hash" improves the documentation's precision and helps implementers know exactly which algorithm to use.
44-52: Well-defined file naming constraints for security.The file naming requirements effectively prevent path traversal attacks and ensure consistent path handling across different systems.
59-61: Clear specification of ZIP compression constraints.Restricting to "stored" (no compression) ensures predictable behavior and prevents potential compression-based security issues.
88-88: Good addition of Lua-style comment support.Adding "--" comment style increases language compatibility, supporting Lua, SQL, and Haskell-style comments.
115-115: More concrete example with exit code.Using
exit(30)instead of a print statement better demonstrates actual program termination behavior.
117-122: Clear version file specification.The version file format follows standard semantic versioning with the "v" prefix, and the default behavior is well-documented.
128-128: Good practice referencing the formal schema.Linking to the schema document helps developers validate their runner.json files.
137-147: Clear example for AddEnv action.The example effectively demonstrates setting an environment variable.
154-173: Comprehensive MapFile documentation with resource implications.The properties are well-documented, the example is clear, and the RAM consumption note helps developers understand resource usage.
181-188: Clear SetArgs example.The example effectively shows how to set process arguments.
203-204: Important initialization behavior documented.Documenting that
_initializeis called immediately after linking helps developers understand the module initialization sequence.
205-224: Clear documentation of StartWasm as terminal action.The documentation clearly explains that StartWasm is terminal and results in module linking and _start execution.
230-240: Important note on dependency processing.Documenting that dependencies are processed only once prevents confusion about duplicate processing.
260-276: Well-documented conditional execution.The When action properties and example clearly show how to conditionally execute actions based on execution mode.
283-293: Clear example of runner reuse pattern.The example effectively demonstrates using With to reuse files from another runner.
298-300: Clear specification of runner execution flow.The documentation clearly states the left-recursive execution and error condition when StartWasm is not reached.
runners/docs.nix (1)
9-21: Well-structured fold operation with proper safeguards.The fold logic correctly builds the nested structure with appropriate checks to prevent overwriting existing entries.
doc/website/src/spec/03-vm/01-result.rst (2)
1-40: Clear and comprehensive result type definitions.The four result kinds are well-defined with appropriate cross-reference labels. The note about InternalError visibility is particularly important for understanding error handling behavior.
41-91: Comprehensive encoding and security specifications.The encoding formats are clearly specified, and the fingerprint structure with BLAKE3 hashing provides a robust anti-cheating mechanism for consensus validation.
executor/src/vm.rs (3)
128-146: Optimize version detection with early return.The function correctly extracts the version but continues parsing after finding it. Consider returning immediately after finding the version section.
fn detect_version_from_wasm(code: &[u8]) -> anyhow::Result<String> { let parser = wasmparser::Parser::new(0); for payload in parser.parse_all(code) { match payload? { wasmparser::Payload::CustomSection(section) if section.name() == "genvm.version" => { let version = section.data().to_vec(); if let Ok(version_str) = str::from_utf8(&version) { return Ok(version_str.to_string()); } else { return Err(anyhow::anyhow!("Invalid UTF-8 in version section")); } } _ => {} } } Err(anyhow::anyhow!("version section not found")) }The current implementation already returns early, so this is working correctly.
402-423: Improved engine configuration with explicit settings.The refactored configuration provides better control over WASM features, with appropriate settings for deterministic execution (disabled SIMD, deterministic relaxed SIMD).
910-920: Robust version detection with graceful fallback.The implementation handles version detection failures gracefully by logging a warning and using a default version, ensuring the archive creation process continues.
doc/website/src/spec/01-core-architecture/02-vm.rst (5)
17-18: Good abstraction from specific implementation.Removing the Wasmtime reference makes the specification more implementation-agnostic.
29-34: Improved structure with reference anchors.Adding explicit anchors (
gvm-def-vm-mode,gvm-def-det-mode,gvm-def-non-det-mode) enables better cross-referencing throughout the documentation.
111-131: Cleaner formatting and simplified descriptions.The reformatted sections are more readable while preserving the important technical details.
137-137: Consistent use of cross-reference labels.Using
:ref:syntax improves documentation navigation and consistency.
203-203: Proper cross-reference for encoding format.The reference to calldata encoding maintains consistency with the documentation structure.
doc/website/src/impl-spec/appendix/host-loop.rst (1)
55-64: Possible missing parameters instorage_write
storage_writeonly receives(slot, index, data)while the correspondingstorage_readreceives(read_type, address, slot, …).
If the write operation is also contract-address-scoped or storage-type-scoped, omitting these parameters here will make the API asymmetric and may be insufficient for real-world handlers. Please double-check the intended signature.doc/website/src/spec/02-execution-environment/02-wasip1.rst (1)
226-236: FD table initial state: clarify persistence across callsThe spec states FD 0 contains calldata, FD 1/2 are stdio, FD 3 is root dir.
It is worth spelling out whether subsequent invocations within the same transaction share the same mutable FD table or receive a fresh instance each time. This affects determinism and resource-leak expectations.
| `Single text file docs for AI <../_static/ai/spec.txt>`_ | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the AI documentation file exists.
The new link references ../_static/ai/spec.txt for AI consumption. Please ensure this file exists and is properly generated.
🏁 Script executed:
#!/bin/bash
# Verify that the AI documentation file exists
ai_doc_path="doc/website/src/_static/ai/spec.txt"
echo "Checking if AI documentation file exists at: $ai_doc_path"
if [ -f "$ai_doc_path" ]; then
echo "✓ AI documentation file exists"
echo "File size: $(wc -c < "$ai_doc_path") bytes"
echo "First few lines:"
head -5 "$ai_doc_path"
else
echo "✗ AI documentation file not found"
echo "Searching for spec.txt files:"
fd -name "spec.txt" -t f
echo "Checking _static directory structure:"
fd -t d -name "_static"
fiLength of output: 1011
The previous search didn’t locate the file (and the fd flags were misused). Let’s re-run using find to pinpoint where (if anywhere) spec.txt and the _static/ai directory live:
🏁 Script executed:
#!/usr/bin/env bash
# Search for any spec.txt files under doc/website
echo "=== Searching for spec.txt files ==="
find doc/website -type f -name spec.txt
# Search for any ai directories under doc/website
echo -e "\n=== Searching for 'ai' directories ==="
find doc/website -type d -name ai
# Search for all _static directories under doc/website
echo -e "\n=== Searching for '_static' directories ==="
find doc/website -type d -name _static
# If any _static dirs found, list their contents
for dir in $(find doc/website -type d -name _static); do
echo -e "\nContents of $dir:"
ls -R "$dir"
doneLength of output: 451
Missing AI documentation file at doc/website/src/_static/ai/spec.txt
The new link in doc/website/src/spec/index.rst points to ../_static/ai/spec.txt, but no such file or directory was found in the repo.
• File needing update:
- doc/website/src/spec/index.rst (lines 4–5)
• Expected target path: - doc/website/src/_static/ai/spec.txt
Please add the generated spec.txt under _static/ai/ or correct the link to wherever the AI-friendly docs reside.
🤖 Prompt for AI Agents
In doc/website/src/spec/index.rst around lines 4 to 5, the link points to
../_static/ai/spec.txt, but the file doc/website/src/_static/ai/spec.txt does
not exist. To fix this, either add the missing spec.txt file under
doc/website/src/_static/ai/ or update the link in index.rst to point to the
correct existing location of the AI documentation file.
| src_dir = Pathname.new(src_dir) | ||
|
|
||
| src_dir.glob('**/*.txt').each do |file| | ||
| name = file.relative_path_from(src_dir).to_s | ||
| buf << "### #{name}\n\n" | ||
| buf << file.read | ||
| buf << "\n\n" | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Portability issue – Pathname#glob not available in older Ruby
Pathname#glob is only present from Ruby 3.1+. If you need Ruby 3.0 or earlier on CI/macOS, the script will crash.
A version-agnostic alternative:
-src_dir.glob('**/*.txt').each do |file|
+Dir.glob(src_dir.join('**/*.txt')).sort.each do |path|
+ file = Pathname.new(path)Also added .sort for deterministic output order.
🤖 Prompt for AI Agents
In doc/website/merge-txts.rb around lines 7 to 14, the use of Pathname#glob
causes compatibility issues with Ruby versions earlier than 3.1. Replace
Pathname#glob with Dir.glob combined with Pathname to maintain compatibility
across Ruby versions. Also, add .sort to the file list to ensure deterministic
output order.
| install_url: https://releases.nixos.org/nix/nix-2.24.11/install | ||
|
|
||
| - name: Get source | ||
| uses: ./.github/actions/get-src |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Static analysis flagged missing action description.
The get-src action is missing a description in its metadata, which is a documentation best practice for GitHub Actions.
Please verify and update the action metadata:
🏁 Script executed:
#!/bin/bash
# Check the get-src action metadata
cat .github/actions/get-src/action.yaml | grep -A 5 -B 5 "description"Length of output: 357
Add a meaningful description to the get-src action metadata
The get-src action’s action.yaml currently has an empty top-level description, which makes it harder for others to understand its purpose. Please update it with a concise summary of what the action does.
• File: .github/actions/get-src/action.yaml
- Line where
description: ""appears - Replace the empty description with something like:
name: GenVM get source
-description: ""
+description: "Fetches the repository’s source code (including submodules) for documentation deployment"
inputs:
load_submodules:
description: if should update submodules🧰 Tools
🪛 actionlint (1.7.7)
46-46: description is required in metadata of "GenVM get source" action at "/home/jailuser/git/.github/actions/get-src/action.yaml"
(action)
🤖 Prompt for AI Agents
In .github/actions/get-src/action.yaml at the line where description is
currently empty, replace the empty string with a concise and meaningful summary
of the action's purpose to improve clarity for users. This description should
briefly explain what the get-src action does.
| let | ||
| commitToTag = builtins.fromJSON commitToTagStr; | ||
| registryRunnersList = (import ./support/registry/default.nix).allRunnersList; | ||
| currentRunnersList = builtins.map (x: x // { rev = currentCommit; }) (import ./default.nix); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential overwrite of existing revision values.
The code unconditionally sets rev = currentCommit which could overwrite existing revision values in the runners from default.nix. Consider preserving existing revisions or documenting this intentional behavior.
🏁 Script executed:
#!/bin/bash
# Check if runners in default.nix already have rev attributes
fd -e nix -x ast-grep --pattern 'rev = $_' {}Length of output: 49
I’ll verify whether any rev attributes exist in default.nix before concluding.
🏁 Script executed:
#!/bin/bash
# Locate default.nix and check for any existing `rev = ...` lines
file=$(fd default.nix -t f | head -n1)
echo "Inspecting: $file"
grep -n "rev *=" "$file" || echo "No existing `rev` assignments found in $file"Length of output: 260
To ensure we’re inspecting the exact default.nix that runners/docs.nix imports, let’s locate docs.nix and check for a sibling default.nix.
🏁 Script executed:
#!/bin/bash
# Find all docs.nix files and look for default.nix alongside
fd docs.nix -t f | while read -r docs; do
dir=$(dirname "$docs")
echo "Found docs.nix at: $docs"
if [ -f "$dir/default.nix" ]; then
echo "Inspecting $dir/default.nix"
grep -n "rev *=" "$dir/default.nix" || echo "No rev= lines in $dir/default.nix"
else
echo "No default.nix in $dir"
fi
doneLength of output: 370
Preserve existing rev attributes when merging runners
We’ve confirmed that runners/default.nix already defines a rev (e.g. line 12 in runners/default.nix). The mapping in runners/docs.nix (line 6) unconditionally does:
currentRunnersList = builtins.map (x: x // { rev = currentCommit; }) (import ./default.nix);This will overwrite any existing rev values. If that’s not intentional, consider changing to a conditional override, for example:
-currentRunnersList = builtins.map (x: x // { rev = currentCommit; }) (import ./default.nix);
+currentRunnersList =
+ builtins.map
+ (x:
+ if builtins.hasAttr "rev" x then
+ x
+ else
+ x // { rev = currentCommit; }
+ )
+ (import ./default.nix);Or, if overwriting is intended, add a comment/documentation noting that any rev from default.nix will be ignored here.
- runners/docs.nix: line 6
- runners/default.nix: existing
revat line 12
🤖 Prompt for AI Agents
In runners/docs.nix at line 6, the current code unconditionally overwrites the
rev attribute for each runner, which may unintentionally discard existing rev
values defined in runners/default.nix at line 12. To fix this, modify the
mapping function to conditionally set rev only if it is not already defined on
the runner object, preserving existing rev attributes. Alternatively, if
overwriting is intended, add a clear comment explaining that any rev from
default.nix will be ignored here.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
doc/website/src/impl-spec/appendix/host-loop.rst (1)
53-54: Prefer length-prefixed write for symmetryAll other variable-length outputs use
write_byte_slice, butstorage_readreplies with a rawwrite_byteseven though the size is dynamic (len). Returning the length explicitly keeps the protocol self-describing and avoids silent divergence iflenis mis-matched.- write_bytes data # must be exactly len in size + write_byte_slice data
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/release.yml(1 hunks)doc/website/merge-txts.rb(1 hunks)doc/website/src/impl-spec/appendix/host-loop.rst(1 hunks)doc/website/src/spec/03-vm/03-ram-limiting.rst(1 hunks)executor/codegen/data/public-abi.json(2 hunks)runners/support/registry(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- runners/support/registry
- doc/website/merge-txts.rb
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/release.yml
- executor/codegen/data/public-abi.json
- doc/website/src/spec/03-vm/03-ram-limiting.rst
🔇 Additional comments (1)
doc/website/src/impl-spec/appendix/host-loop.rst (1)
100-104:consume_fuellacks an acknowledgementThis is the only branch that never emits a response byte. If the client side assumes every request is acknowledged, it could block waiting for data. Either:
- Add a single status byte (
json/errors/ok), or- Explicitly document that this method is fire-and-forget and that the client must not wait for a response.
| json/methods/post_event: | ||
| read_byte topics_len | ||
| topics := [] | ||
| for i in range(topics_len): | ||
| topic := read_bytes(32) # 32 bytes each | ||
| topics.append(topic) | ||
| blob := read_slice | ||
| err := host_post_event(topics, blob) | ||
| if err != json/errors/ok: |
There was a problem hiding this comment.
Fix two syntactic errors in the post_event handler
read_byte topics_lenshould follow the same assignment style used elsewhere (topics_len := read_byte).blob := read_sliceis missing the()– without the call it evaluates to the function itself.
- json/methods/post_event:
- read_byte topics_len
+ json/methods/post_event:
+ topics_len := read_byte
@@
- blob := read_slice
+ blob := read_slice()📝 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.
| json/methods/post_event: | |
| read_byte topics_len | |
| topics := [] | |
| for i in range(topics_len): | |
| topic := read_bytes(32) # 32 bytes each | |
| topics.append(topic) | |
| blob := read_slice | |
| err := host_post_event(topics, blob) | |
| if err != json/errors/ok: | |
| json/methods/post_event: | |
| topics_len := read_byte | |
| topics := [] | |
| for i in range(topics_len): | |
| topic := read_bytes(32) # 32 bytes each | |
| topics.append(topic) | |
| blob := read_slice() | |
| err := host_post_event(topics, blob) | |
| if err != json/errors/ok: |
🤖 Prompt for AI Agents
In doc/website/src/impl-spec/appendix/host-loop.rst around lines 135 to 143, fix
the syntax errors in the post_event handler by changing `read_byte topics_len`
to `topics_len := read_byte` to match the assignment style, and add parentheses
to `read_slice` by changing `blob := read_slice` to `blob := read_slice()` to
correctly call the function.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
doc/website/src/impl-spec/appendix/host-loop.rst (1)
135-141:post_eventhandler still carries the original syntax bugsThe assignment style and the
read_sliceinvocation were pointed out in the previous review but are still unfixed. They break consistency with the rest of the pseudocode and make the snippet technically ill-formed.- read_byte topics_len + topics_len := read_byte @@ - blob := read_slice + blob := read_slice()Please apply the diff to keep the specification self-consistent and executable.
🧹 Nitpick comments (1)
doc/website/src/impl-spec/appendix/host-loop.rst (1)
66-70:consume_resultreads data only to throw it away
host_result := read_slice()fetches the payload but the variable is never referenced afterwards.
If the intention is to flush the inbound buffer only, consider omitting the binding entirely or adding a clarifying comment so future readers do not assume the result must be processed.Example (minimal):
- host_result := read_slice() + _ := read_slice() # discard result; loop terminates next
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/release.yml(1 hunks)doc/website/merge-txts.rb(1 hunks)doc/website/src/impl-spec/appendix/host-loop.rst(1 hunks)doc/website/src/spec/03-vm/03-ram-limiting.rst(1 hunks)executor/codegen/data/public-abi.json(2 hunks)runners/support/registry(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- runners/support/registry
- .github/workflows/release.yml
- doc/website/merge-txts.rb
- doc/website/src/spec/03-vm/03-ram-limiting.rst
- executor/codegen/data/public-abi.json
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Tests