Streamline project docs and contributor guidance#250
Merged
Conversation
- Consolidate AGENTS guidance and architecture notes - Trim duplicated detail across docs and align references - Update built-in, GC, types, decorators, and platform sections
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdded concise "Executive Summary" sections across many docs, documented new built-ins and flags (Proxy, Reflect, FFI, JSONL), condensed and relocated detailed design/GC/language specifics into targeted docs, and updated documented Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/decision-log.md (1)
7-8: Clarify VM value model (registers are notTGocciaValuedirectly).Line 7 says bytecode execution operates directly on
TGocciaValue, which conflicts with the current VM model (TGocciaRegisterregister file, withTGocciaValuematerialization at runtime boundaries). Please reword to avoid architectural drift.Based on learnings: In frostney/GocciaScript (units/Goccia.VM*.pas), the bytecode VM register file uses a tagged `TGocciaRegister` type, not `TGocciaValue` directly.Suggested wording adjustment
-- **Bytecode fold-in** — The bytecode backend is now Goccia-owned, operating directly on `TGocciaValue` with dedicated opcodes +- **Bytecode fold-in** — The bytecode backend is now Goccia-owned, using dedicated opcodes over tagged VM registers (`TGocciaRegister`) while materializing `TGocciaValue` at object/runtime boundaries🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/decision-log.md` around lines 7 - 8, Update the decision log wording to reflect the actual VM value model: replace the assertion that the bytecode backend "operates directly on TGocciaValue" with wording that the bytecode VM uses a tagged TGocciaRegister register file and only materializes TGocciaValue at runtime boundaries; reference the bytecode backend and the types TGocciaRegister and TGocciaValue (and TGocciaControlFlow if present) so the log accurately states registers hold TGocciaRegister entries and TGocciaValue materialization occurs at boundary points rather than direct register storage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/built-ins.md`:
- Line 8: The executive summary incorrectly states "22 built-in flags" while the
TGocciaGlobalBuiltin enum actually defines 23 entries; update the wording in
docs/built-ins.md to reflect 23 built-in flags and/or explicitly mention both
ggTestAssertions and ggBenchmark so the count matches the TGocciaGlobalBuiltin
enum definition (ensure the list of names corresponds to the enum entries).
In `@docs/bytecode-vm.md`:
- Line 9: Update the docs to reflect the three-tier opcode split used in the
codebase: change the opcode range sentence to state core instructions are
0..127, non-core generic arithmetic/bitwise are 128..166, and semantic/helper
ops start at 167..255 (matching OP_SEM_FIRST = 167 in
units/Goccia.Bytecode.pas); reference OP_SEM_FIRST and the three ranges (0..127,
128..166, 167..255) in the wording so the doc aligns exactly with the
implementation.
In `@docs/language-restrictions.md`:
- Line 9: Update the "Graceful handling — Excluded syntax parses successfully
but executes as a no-op with a warning and suggestion" line to narrow the claim
to only parser-recognized exclusions (e.g., change wording to something like
"Graceful handling — Some parser-recognized excluded syntax parses successfully
but is treated as a no-op with a warning and suggestion") so it no longer
implies blanket no-op behavior for exclusions such as eval or arguments; locate
and edit the exact "Graceful handling — Excluded syntax parses successfully but
executes as a no-op with a warning and suggestion" phrase in the document and
replace it with the qualified wording.
In `@docs/testing.md`:
- Line 10: Update the documented full-suite command string `./build.pas
testrunner && ./build/TestRunner tests` to include the `--asi` flag (e.g.,
`./build.pas testrunner && ./build/TestRunner tests --asi`) or alternatively add
an explicit one-line caveat after that command stating that the full suite
requires `--asi` so ASI tests under `tests/language/asi/` run successfully; edit
the single occurrence of the command in the docs/testing.md content to make this
change.
---
Nitpick comments:
In `@docs/decision-log.md`:
- Around line 7-8: Update the decision log wording to reflect the actual VM
value model: replace the assertion that the bytecode backend "operates directly
on TGocciaValue" with wording that the bytecode VM uses a tagged TGocciaRegister
register file and only materializes TGocciaValue at runtime boundaries;
reference the bytecode backend and the types TGocciaRegister and TGocciaValue
(and TGocciaControlFlow if present) so the log accurately states registers hold
TGocciaRegister entries and TGocciaValue materialization occurs at boundary
points rather than direct register storage.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29f6ea78-4d97-4406-828d-ff6de3d78dc8
📒 Files selected for processing (17)
AGENTS.mdREADME.mddocs/adding-built-in-types.mddocs/architecture.mddocs/benchmarks.mddocs/build-system.mddocs/built-ins.mddocs/bytecode-vm.mddocs/code-style.mddocs/decision-log.mddocs/design-decisions.mddocs/embedding.mddocs/language-restrictions.mddocs/profiling.mddocs/testing.mddocs/tutorial.mddocs/value-system.md
- Fix built-in flag count 22→23 in built-ins.md executive summary - Correct opcode space to three-tier split (0-127 core, 128-166 non-core, 167-255 semantic) in bytecode-vm.md - Narrow graceful handling claim to parser-recognized exclusions in language-restrictions.md - Add --asi flag to default test command in testing.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/language-restrictions.md`:
- Line 10: Update the wording to separate actual opt-in flags from
always-available language features: list ASI (`--asi`) and JSX (`ggJSX`) as the
only opt-in toggles, and move decorators, enums, and "types as comments" out of
the opt-in group into an "always available" or "language features" group;
explicitly state that GocciaScript supports TC39 Stage 3 decorators and
decorator metadata and the TC39 "Types as Comments" proposal to reflect current
support.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ac5b14a-ca51-436a-aa5b-1a72394eacd4
📒 Files selected for processing (6)
AGENTS.mddocs/build-system.mddocs/built-ins.mddocs/bytecode-vm.mddocs/language-restrictions.mddocs/testing.md
✅ Files skipped from review due to trivial changes (4)
- docs/build-system.md
- docs/testing.md
- docs/bytecode-vm.md
- docs/built-ins.md
🚧 Files skipped from review as they are similar to previous changes (1)
- AGENTS.md
Split the executive summary bullet in language-restrictions.md: ASI and JSX are opt-in toggles (--asi flag, ggJSX flag), while types as comments, decorators, and enums are always-available language features with no gating flag. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Suite Timing
Measured on ubuntu-latest x64. |
Contributor
Benchmark Results274 benchmarks Interpreted: 🟢 31 improved · 🔴 194 regressed · 49 unchanged · avg -4.2% arraybuffer.js — Interp: 🟢 1, 🔴 12, 1 unch. · avg -6.3% · Bytecode: 🟢 14 · avg +24.1%
arrays.js — Interp: 🔴 19 · avg -6.0% · Bytecode: 🟢 19 · avg +30.1%
async-await.js — Interp: 🔴 6 · avg -6.1% · Bytecode: 🟢 6 · avg +25.2%
classes.js — Interp: 🟢 3, 🔴 21, 7 unch. · avg -2.6% · Bytecode: 🟢 25, 6 unch. · avg +19.0%
closures.js — Interp: 🔴 10, 1 unch. · avg -3.0% · Bytecode: 🟢 11 · avg +22.8%
collections.js — Interp: 🟢 2, 🔴 9, 1 unch. · avg -9.5% · Bytecode: 🟢 12 · avg +26.4%
destructuring.js — Interp: 🟢 7, 🔴 4, 11 unch. · avg -0.3% · Bytecode: 🟢 20, 2 unch. · avg +17.7%
fibonacci.js — Interp: 🔴 4, 4 unch. · avg -4.9% · Bytecode: 🟢 8 · avg +27.9%
for-of.js — Interp: 🟢 5, 🔴 1, 1 unch. · avg +2.4% · Bytecode: 🟢 7 · avg +26.2%
helpers/bench-module.js — Interp: 0 · Bytecode: 0
iterators.js — Interp: 🟢 1, 🔴 16, 3 unch. · avg -3.0% · Bytecode: 🟢 20 · avg +26.0%
json.js — Interp: 🔴 20 · avg -8.0% · Bytecode: 🟢 20 · avg +19.4%
jsx.jsx — Interp: 🟢 2, 🔴 10, 9 unch. · avg -1.7% · Bytecode: 🟢 21 · avg +15.7%
modules.js — Interp: 🔴 8, 1 unch. · avg -6.0% · Bytecode: 🟢 9 · avg +27.8%
numbers.js — Interp: 🔴 11 · avg -8.6% · Bytecode: 🟢 10, 🔴 1 · avg +15.5%
objects.js — Interp: 🟢 5, 🔴 2 · avg +2.0% · Bytecode: 🟢 7 · avg +19.5%
promises.js — Interp: 🔴 11, 1 unch. · avg -6.2% · Bytecode: 🟢 12 · avg +18.4%
regexp.js — Interp: 🔴 10, 1 unch. · avg -7.2% · Bytecode: 🟢 11 · avg +18.9%
strings.js — Interp: 🔴 10, 1 unch. · avg -8.1% · Bytecode: 🟢 11 · avg +22.3%
typed-arrays.js — Interp: 🟢 5, 🔴 10, 7 unch. · avg -1.1% · Bytecode: 🟢 22 · avg +21.7%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AGENTS.mdto make core contributor rules, GC notes, language restrictions, and design patterns easier to scan.README.mdwhile pushing detailed references into the dedicated pages underdocs/.Testing
AGENTS.md,README.md, anddocs/.