docs: align documentation with current runtime surface#107
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDocumentation-only rewrite across README files and the docs directory. Content is reorganized into consistent "When to Read"/"Source of Truth" sections with condensed tables and checklists, expanding coverage of coroutines, maps, structured errors, verification, and profiling while removing older prose-heavy explanations and code examples. ChangesDocumentation reference refresh
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #107 +/- ##
==========================================
- Coverage 46.35% 46.34% -0.02%
==========================================
Files 77 77
Lines 17390 17390
==========================================
- Hits 8061 8059 -2
- Misses 8407 8408 +1
- Partials 922 923 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.ko.md`:
- Around line 190-192: The JIT/trace description contains a spelling/notation
issue in the affected sentence, so review the wording around the README.ko.md
JIT coverage paragraph and correct the malformed term or punctuation while
preserving the intended meaning. Check the sentences mentioning trace-based JIT,
“native back-edge,” and the continuation behavior, and normalize the
Korean/English mixed phrasing so the documentation reads cleanly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dc62631c-bac9-4b16-8a53-4ef158afdef4
📒 Files selected for processing (3)
README.ko.mdREADME.mddocs/documentation-review.md
| JIT는 **트레이스 기반**입니다. 함수 진입점이나 루프 헤더가 뜨거워지면 한 번의 실행으로 라이브 핫 경로를 기록하고, 그 트레이스를 네이티브 코드로 컴파일해 디스패치 테이블에 설치합니다. 기록된 모든 가정(호출 대상, 분기 방향, 값 종류, 배열 범위)은 런타임 가드이며, 가드가 실패하면 저널을 통해 스레디드 인터프리터로 deopt해 트레이스가 멈춘 지점에서 정확히 재개합니다. 반복되는 분기 이탈은 이후 재컴파일에서 같은 트레이스 루트의 네이티브 continuation으로 합쳐질 수 있습니다. 분기가 있는 non-self callee는 호출자 트레이스 안으로 인라인될 수 있고, 학습된 callee continuation은 pending-continuation 제한 안에서 `RETURN`을 지나 호출자 tail까지 이어질 수 있습니다. 완전한 정적 tree-shaped compilation은 아직 미래 작업입니다. | ||
|
|
||
| 커버리지는 i32/i64/f32/f64 산술·비트·비교·변환(좁은 i1/i8 종류는 i32 표현을 공유해 i32와 함께 네이티브로 계산되며, width-closed 비트 연산에서 종류가 보존됩니다), 스택 연산·로컬·글로벌·업밸류·상수·`select`·분기, 직접/클로저/가드 간접 호출, 읽기 전용 힙 빠른 경로(`array.get/len`, `struct.get`, 레퍼런스 읽기), 그리고 **루프**까지 포함합니다. 핫 루프는 본문을 레지스터에 유지한 채 네이티브 back-edge를 돌며 매 반복 사이에 세이프포인트를 폴링합니다. 할당·변형·호스트 호출은 트레이스를 끝내고 인터프리터가 담당합니다. 스레디드 인터프리터는 스위치 테이블 대신 클로저 디스패치를 써서 JIT 전에도 충분히 빠릅니다. | ||
| 커버리지는 i32/i64/f32/f64 산술·비트·비교·변환(좁은 i1/i8 종류는 i32 표현을 공유해 i32와 함께 네이티브로 계산되며, width-closed 비트 연산에서 종류가 보존됩니다), 스택 연산·로컬·글로벌·업밸류·상수·`select`·분기, 직접/클로저/가드 간접 호출, 읽기 전용 힙 빠른 경로(`array.get/len`, `struct.get`, `error.get`, ref 읽기), 그리고 **루프**까지 포함합니다. 핫 루프는 본문을 레지스터에 유지한 채 네이티브 back-edge를 돌며 매 반복 사이에 세이프포인트를 폴링합니다. 할당·변형·호스트 호출·`error.new`·`error.code`·`throw`는 트레이스를 끝내고 인터프리터가 담당합니다. 스레디드 인터프리터는 스위치 테이블 대신 클로저 디스패치를 써서 JIT 전에도 충분히 빠릅니다. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
문장 표현을 다시 확인해 주세요.
Line 192의 문장에 맞춤법/표기 문제가 있습니다. 머지 전에 문구를 다시 점검해 주세요.
🧰 Tools
🪛 LanguageTool
[grammar] ~192-~192: Ensure spelling is correct
Context: ...·throw`는 트레이스를 끝내고 인터프리터가 담당합니다. 스레디드 인터프리터는 스위치 테이블 대신 클로저 디스패치를 써서 JIT 전에도 충분히 빠릅니...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.ko.md` around lines 190 - 192, The JIT/trace description contains a
spelling/notation issue in the affected sentence, so review the wording around
the README.ko.md JIT coverage paragraph and correct the malformed term or
punctuation while preserving the intended meaning. Check the sentences
mentioning trace-based JIT, “native back-edge,” and the continuation behavior,
and normalize the Korean/English mixed phrasing so the documentation reads
cleanly.
Source: Linters/SAST tools
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/guides/add-architecture.md (2)
43-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRephrase the scratch-slot ownership sentence.
interp/jit.gois the actual contract boundary here; saying the order is “shared with the existing ARM64 backend” makes the ABI sound ARM64-owned instead of JIT-driver-owned. Point the prose atinterp/jit.goas the source of truth.Suggested wording
- Scratch slot order is shared with the existing ARM64 backend: + Scratch slot order follows the shared `interp/jit.go` contract:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/add-architecture.md` around lines 43 - 61, The scratch-slot ownership sentence should be rephrased so `interp/jit.go` is the source of truth rather than implying ARM64 owns the ABI. Update the Step 2 prose to point readers to the shared JIT-driver contract defined in `interp/jit.go`, and keep the architecture-specific backend note limited to implementation details in `archLowerer`/`newCompiler` rather than ownership language.
100-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winParameterize the build example.
Hard-coding
GOOS=linuxmakes the verification step read as Linux-only, even though Step 3 says the supportedGOOS/GOARCHpairs should be documented. Use the backend's supported target(s) or label Linux explicitly as an example.Suggested wording
-GOOS=linux GOARCH=<arch> go build ./... +GOOS=<supported-goos> GOARCH=<arch> go build ./...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guides/add-architecture.md` around lines 100 - 113, The verification example in the “Step 5 — Verify” section is hard-coded to Linux, so update the guidance to either use the backend’s documented supported GOOS/GOARCH targets or clearly label the command as a Linux example; adjust the wording around the build command in this doc snippet so it matches the platform support described in the earlier steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/guides/add-architecture.md`:
- Around line 43-61: The scratch-slot ownership sentence should be rephrased so
`interp/jit.go` is the source of truth rather than implying ARM64 owns the ABI.
Update the Step 2 prose to point readers to the shared JIT-driver contract
defined in `interp/jit.go`, and keep the architecture-specific backend note
limited to implementation details in `archLowerer`/`newCompiler` rather than
ownership language.
- Around line 100-113: The verification example in the “Step 5 — Verify” section
is hard-coded to Linux, so update the guidance to either use the backend’s
documented supported GOOS/GOARCH targets or clearly label the command as a Linux
example; adjust the wording around the build command in this doc snippet so it
matches the platform support described in the earlier steps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e52abd7-a53a-4a9e-8ee9-ca5f2f568ecf
📒 Files selected for processing (3)
docs/guides/add-architecture.mddocs/guides/add-opcode.mddocs/guides/repl.md
✅ Files skipped from review due to trivial changes (1)
- docs/guides/repl.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/compatibility.md`:
- Around line 65-72: The Markdown table in the compatibility section is broken
because the `||` condition strings are being parsed as extra columns. Update the
table entries around the `darwin || linux` and `!darwin && !linux` conditions by
escaping the pipe characters or moving those conditions out of the table, and
verify the section still renders as valid Markdown. Keep the formatting
consistent with the surrounding table in the compatibility docs.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 005c93ff-dbe9-4f88-a165-7bad2f39e0e7
📒 Files selected for processing (5)
docs/README.mddocs/compatibility.mddocs/memory-model.mddocs/pass-system.mddocs/roadmap.md
✅ Files skipped from review due to trivial changes (3)
- docs/memory-model.md
- docs/pass-system.md
- docs/roadmap.md
| | Condition | Effect | | ||
| |---|---| | ||
| | `arm64` | enables ARM64 encoder, ABI, trampoline, and JIT lowering | | ||
| | `!arm64` | uses ARM64 stubs; ARM64 JIT lowering is not compiled | | ||
| | `darwin && arm64 && cgo` | enables real instruction-cache flush | | ||
| | `!darwin || !arm64 || !cgo` | uses no-op instruction-cache flush | | ||
| | `darwin || linux` | enables executable-memory mapping | | ||
| | `!darwin && !linux` | executable-memory allocation returns `ErrMmapFailed` | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the broken Markdown table.
The || conditions are being parsed as extra table separators, which is why markdownlint reports MD056 on lines 70-71. Escape the pipes or move these conditions out of the table so the section renders correctly. As per coding guidelines, documentation edits should preserve formatting and be verified for valid Markdown.
🧰 Tools
🪛 LanguageTool
[style] ~68-~68: Using many exclamation marks might seem excessive (in this case: 7 exclamation marks for a text that’s 4760 characters long)
Context: ..., ABI, trampoline, and JIT lowering | | !arm64 | uses ARM64 stubs; ARM64 JIT lowering is not compiled | | darwin && arm64 && cgo | enables real instruction-cache flush | | !darwin || !arm64 || !cgo | uses no-op instruction-cache flush | | darwin || linux | enables executable-memory mapping | | !darwin && !linux | executable-memory allocation re...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.22.1)
[warning] 70-70: Table column count
Expected: 2; Actual: 6; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 71-71: Table column count
Expected: 2; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/compatibility.md` around lines 65 - 72, The Markdown table in the
compatibility section is broken because the `||` condition strings are being
parsed as extra columns. Update the table entries around the `darwin || linux`
and `!darwin && !linux` conditions by escaping the pipe characters or moving
those conditions out of the table, and verify the section still renders as valid
Markdown. Keep the formatting consistent with the surrounding table in the
compatibility docs.
Sources: Coding guidelines, Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/benchmarks.md`:
- Around line 187-193: The Cross-runtime library versions entry has an incorrect
goja version string, missing the revision suffix. Update the goja value in the
benchmarks documentation to match the version declared in benchmarks/go.mod,
using the full pseudo-version with the commit suffix so the documented
dependency aligns with the actual module version.
In `@docs/profile.md`:
- Around line 105-112: The `vm_jit_skips_total` entry in the metrics table is
outdated and still describes it as reserved/always zero; update the
documentation wording in the `vm_jit_skips_total` row to reflect it as a real
counter, using “JIT skip count” instead. Make the change alongside the other VM
JIT metric descriptions so the `vm_jit_attempts_total`, `vm_jit_emits_total`,
`vm_jit_links_total`, `vm_jit_errors_total`, and `vm_jit_bytes_total` entries
remain consistent.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: abfac6ec-9968-4b41-8973-43fc4485fe83
📒 Files selected for processing (3)
docs/benchmarks.mddocs/debugging.mddocs/profile.md
| Cross-runtime library versions: | ||
|
|
||
| - wazero v1.12.0 | ||
| - gopher-lua v1.1.2 | ||
| - tengo v2.17.0 | ||
| - goja v0.0.0-20260311135729 | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the benchmark doc and module file.
git ls-files 'docs/benchmarks.md' 'go.mod' '**/go.mod'
# Show the relevant benchmark lines.
python3 - <<'PY'
from pathlib import Path
p = Path('docs/benchmarks.md')
lines = p.read_text().splitlines()
for i in range(180, 196):
if i <= len(lines):
print(f"{i}: {lines[i-1]}")
PY
# Search for goja version references in the repo.
rg -n --hidden --glob '!**/.git/**' 'goja|Cross-runtime library versions' .
# If go.mod exists at repo root, show the relevant require lines.
if [ -f go.mod ]; then
rg -n '^(\s*require\s*\(|\s*github\.com/dop251/goja|\s*\)|\s*replace\s+.*goja)' go.mod
fi
# Also inspect any docs or release notes that mention the cross-runtime versions.
rg -n --hidden --glob 'docs/**/*.md' 'wazero v1\.12\.0|gopher-lua v1\.1\.2|tengo v2\.17\.0|goja v0\.0\.0' docsRepository: siyul-park/minivm
Length of output: 2907
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Try to identify the goja module version from the module graph metadata if present.
if [ -f go.mod ]; then
# Read the module path and any direct/indirect goja requirement.
python3 - <<'PY'
from pathlib import Path
import re
text = Path('go.mod').read_text()
m = re.search(r'github\.com/dop251/goja\s+([^\s]+)', text)
print(m.group(1) if m else "NO_G0JA_REQUIRE")
PY
fi
# Check whether the pseudo-version format in the doc is syntactically valid.
python3 - <<'PY'
import re
s = "v0.0.0-20260311135729"
# Go pseudo-versions need a timestamp and revision suffix.
pattern = re.compile(r'^v\d+\.\d+\.\d+-\d{14}-[0-9a-f]{12}$')
print("matches_pseudoversion_pattern=", bool(pattern.match(s)))
PYRepository: siyul-park/minivm
Length of output: 208
Fix the goja version entry. Use v0.0.0-20260311135729-065cd970411c to match benchmarks/go.mod; the current value is missing the revision suffix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/benchmarks.md` around lines 187 - 193, The Cross-runtime library
versions entry has an incorrect goja version string, missing the revision
suffix. Update the goja value in the benchmarks documentation to match the
version declared in benchmarks/go.mod, using the full pseudo-version with the
commit suffix so the documented dependency aligns with the actual module
version.
| | Metric | Meaning | | ||
| |---|---| | ||
| | `vm_jit_attempts_total` | compilation attempts | | ||
| | `vm_jit_emits_total` | emitted trace objects | | ||
| | `vm_jit_links_total` | installed callable entries | | ||
| | `vm_jit_skips_total` | reserved; currently zero | | ||
| | `vm_jit_errors_total` | compile or link errors | | ||
| | `vm_jit_bytes_total` | generated code bytes | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document vm_jit_skips_total as a real counter. The table’s “reserved; currently zero” wording is stale; use “JIT skip count” instead.
Suggested wording
-| `vm_jit_skips_total` | reserved; currently zero |
+| `vm_jit_skips_total` | JIT skip count |📝 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.
| | Metric | Meaning | | |
| |---|---| | |
| | `vm_jit_attempts_total` | compilation attempts | | |
| | `vm_jit_emits_total` | emitted trace objects | | |
| | `vm_jit_links_total` | installed callable entries | | |
| | `vm_jit_skips_total` | reserved; currently zero | | |
| | `vm_jit_errors_total` | compile or link errors | | |
| | `vm_jit_bytes_total` | generated code bytes | | |
| | Metric | Meaning | | |
| |---|---| | |
| | `vm_jit_attempts_total` | compilation attempts | | |
| | `vm_jit_emits_total` | emitted trace objects | | |
| | `vm_jit_links_total` | installed callable entries | | |
| | `vm_jit_skips_total` | JIT skip count | | |
| | `vm_jit_errors_total` | compile or link errors | | |
| | `vm_jit_bytes_total` | generated code bytes | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/profile.md` around lines 105 - 112, The `vm_jit_skips_total` entry in
the metrics table is outdated and still describes it as reserved/always zero;
update the documentation wording in the `vm_jit_skips_total` row to reflect it
as a real counter, using “JIT skip count” instead. Make the change alongside the
other VM JIT metric descriptions so the `vm_jit_attempts_total`,
`vm_jit_emits_total`, `vm_jit_links_total`, `vm_jit_errors_total`, and
`vm_jit_bytes_total` entries remain consistent.
Changes Made
instr/opcode.go,instr/type.go,program/verify.go,interp/marshal.go, andgo.mod.docs/README.mdas the documentation index and ownership map to reduce repeated explanations across topic docs.docs/documentation-review.mdnotes because the review plan is now captured by this PR and the lasting ownership rules live indocs/README.md.README.mdandREADME.ko.mdso public capability summaries include maps, coroutines, structured errors, verifier guidance, and current opcode families.When to Read,Source of Truth, main content,Maintenance Notes, andRelated Docswhere each section is relevant.boolmapping toI1.Related Issues
None.
Additional Information
Docs-only change. No tests were run.
Review notes:
docs/README.mddefines which document owns each topic to prevent future drift.docs/instruction-set.md; heap ownership belongs indocs/memory-model.md; boxed layout belongs indocs/value-representation.md; benchmark numbers belong indocs/benchmarks.md; JIT internals belong indocs/jit-internals.md; profiling behavior belongs indocs/profile.md.Summary by CodeRabbit