perry: experimental patches for context-mode native compilation - #7225
perry: experimental patches for context-mode native compilation#7225bradmccormack wants to merge 5 commits into
Conversation
Brings in 39 upstream commits (a3b31c0..7d1dc9c) on top of the local context-mode experimental patches (b72f6f9, c3d1141, 6d28ca7). Resolution notes: * The local CHANGELOG.md diverged only in home-directory paths (`/Users/amlug/...` vs `~/projects/...`) that upstream scrubbed in PerryTS#7165. CHANGELOG.md is FROZEN at v0.5.1264 (see `changelog.d/README.md`), so the file is restored to its state at the merge base (df7214b, the v0.5.1264 version bump). The new context-mode entries go in `changelog.d/7220-context-mode-patches.md` instead, per the per-PR fragment policy. * The five perry-related file changes (eval_classifier, expr_new, alias_tracking, freshness, lib) all auto-merged cleanly. The workspace reorg from `perry/...` to `crates/perry/...` is picked up via git's rename detection, so the `freshness.rs` patch rides along at its new path. * No upstream-side changes were dropped or hand-edited.
The fragment was committed with a placeholder name (7220) before the PR was opened; the assigned number is 7225. Pure rename, no content change.
📝 WalkthroughWalkthroughThe compiler now tracks known library code-generation sites, recognizes aliased ChangesContext-mode compilation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Source
participant PerryHir
participant OptimizedCompiler
participant NativeRuntime
Source->>PerryHir: uses aliased Function or known library code generation
PerryHir->>OptimizedCompiler: reports known code-generation sites
OptimizedCompiler->>OptimizedCompiler: updates cache key and selects dyn-eval
OptimizedCompiler->>NativeRuntime: builds runtime with required features
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@CHANGELOG.md`:
- Line 5686: Remove the machine-specific /Users/amlug paths and
workstation-specific details from the top-level CHANGELOG.md entry, replacing
them with repository-relative paths or placeholders such as $PERRY_ROOT,
$MANGO_ROOT, and $WORKTREE_ROOT. Move the compatibility details into
changelog.d/7225-context-mode-patches.md without adding a version header, and
apply the same cleanup to the referenced entries.
In `@CONTEXT-MODE-COMPAT.md`:
- Around line 25-33: Update the fenced code blocks in CONTEXT-MODE-COMPAT.md
around the command transcripts to specify the text language identifier,
including both blocks referenced by the review. Leave the transcript contents
unchanged.
In `@crates/perry-hir/src/eval_classifier.rs`:
- Around line 112-123: Scope KNOWN_CODEGEN_SITE_COUNT to the current compilation
rather than process lifetime. Update the state used by check_site and
has_known_codegen_sites so each compilation starts with no known codegen sites,
either by storing it in compilation-owned state or resetting it at the
compilation boundary before the next check_site; ensure later programs do not
inherit the flag or enable unnecessary runtime features.
In `@crates/perry-hir/src/lower/expr_new.rs`:
- Around line 207-214: Preserve lexical shadowing in the constructor-lowering
flow around resolve_class_alias: capture the original identifier’s
LocalId/binding before normalizing class_name, then treat the name as the
intrinsic Function constructor only when that binding matches the explicitly
tracked globalThis.Function alias. Ensure local parameters or variables
shadowing the alias continue through lookup_local and shadowed_by_user_binding,
and add a regression test for new F(...) inside a function parameter named F.
🪄 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: 7db62a79-76e8-4324-874b-77b985a0d830
📒 Files selected for processing (8)
CHANGELOG.mdCONTEXT-MODE-COMPAT.mdchangelog.d/7225-context-mode-patches.mdcrates/perry-hir/src/destructuring/var_decl/alias_tracking.rscrates/perry-hir/src/eval_classifier.rscrates/perry-hir/src/lib.rscrates/perry-hir/src/lower/expr_new.rscrates/perry/src/commands/compile/optimized_libs/freshness.rs
| PR #985 (committed at `a7cd054a`, 2026-05-18 05:48) had specifically claimed this case was fixed by adding `CompileOptions::namespace_v8_specifiers` and routing `R.<member>(args)` through `js_call_v8_export` from both `expr.rs:6588` (StaticMethodCall arm) and `lower_call.rs:549` (namespace-member-call arm). | ||
|
|
||
| **Root cause — stale sweep binary, not a regression.** The sweep at `/tmp/perry-compat-sweep/run.sh` defaults `PERRY_BIN` to `~/projects/perry/perry/target/release/perry`, and that binary on disk was built at 05:25 — 23 minutes *before* #985's commit landed. Rebuilding the compiler (`cargo build --release -p perry-runtime -p perry-stdlib -p perry-jsruntime -p perry`) and rerunning the sweep against the freshly-built binary turns the ramda fixture green immediately: | ||
| **Root cause — stale sweep binary, not a regression.** The sweep at `/tmp/perry-compat-sweep/run.sh` defaults `PERRY_BIN` to `/Users/amlug/projects/perry/perry/target/release/perry`, and that binary on disk was built at 05:25 — 23 minutes *before* #985's commit landed. Rebuilding the compiler (`cargo build --release -p perry-runtime -p perry-stdlib -p perry-jsruntime -p perry`) and rerunning the sweep against the freshly-built binary turns the ramda fixture green immediately: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove machine-specific paths from CHANGELOG.md.
These lines hard-code /Users/amlug/.... The commands and reproduction paths only work on one workstation and expose a local username.
Use repository-relative paths or placeholders such as $PERRY_ROOT, $MANGO_ROOT, and $WORKTREE_ROOT. Keep new compatibility details in changelog.d/7225-context-mode-patches.md.
Based on learnings: contributors must not update top-level CHANGELOG.md release entries. As per coding guidelines: add each change to a PR-keyed changelog.d/*.md fragment without a version header.
Also applies to: 9020-9020, 10506-10506, 10521-10521, 10561-10561
🤖 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 `@CHANGELOG.md` at line 5686, Remove the machine-specific /Users/amlug paths
and workstation-specific details from the top-level CHANGELOG.md entry,
replacing them with repository-relative paths or placeholders such as
$PERRY_ROOT, $MANGO_ROOT, and $WORKTREE_ROOT. Move the compatibility details
into changelog.d/7225-context-mode-patches.md without adding a version header,
and apply the same cleanup to the referenced entries.
Sources: Coding guidelines, Learnings
| ``` | ||
| $ ./context-mode-http-real | ||
| CONTEXT_MODE_PORT=40475 | ||
|
|
||
| # Initialize → valid JSON-RPC response with protocolVersion + serverInfo | ||
| # ctx_index → "Indexed 1 chunks from source 'verification'" | ||
| # ctx_search → "FTS Verified: BM25 FTS5 indexing works. Keywords: Rust TypeScript SQLite native." | ||
| # DB file → /tmp/context-mode-{pid}.db, 4096 bytes on disk | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to the fenced blocks.
The fenced blocks at Lines 25 and 119 have no language identifier. markdownlint reports MD040. Use text for these command transcripts.
Also applies to: 119-134
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 25-25: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@CONTEXT-MODE-COMPAT.md` around lines 25 - 33, Update the fenced code blocks
in CONTEXT-MODE-COMPAT.md around the command transcripts to specify the text
language identifier, including both blocks referenced by the review. Leave the
transcript contents unchanged.
Source: Linters/SAST tools
| /// #6559 companion: KnownLibraryCodegen sites (ajv, fast-json-stringify, | ||
| /// find-my-way) also need the dyn-eval interpreter at runtime until their | ||
| /// build-time evaluation phases (#1680/#1681/#1682) ship. Track them in a | ||
| /// deferred-style sink so the auto-optimizer can include `dyn-eval`. | ||
| static KNOWN_CODEGEN_SITE_COUNT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); | ||
|
|
||
| /// Whether this compile contains at least one KnownLibraryCodegen site. | ||
| /// The auto-optimizer consults this alongside [`has_deferred_dynamic_code_sites`] | ||
| /// to decide whether to include the `dyn-eval` interpreter. | ||
| pub fn has_known_codegen_sites() -> bool { | ||
| KNOWN_CODEGEN_SITE_COUNT.load(std::sync::atomic::Ordering::Relaxed) > 0 | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 5 \
'KNOWN_CODEGEN_SITE_COUNT|has_known_codegen_sites|take_deferred_eval_sites|check_site\(' \
crates/perry-hir crates/perryRepository: PerryTS/perry
Length of output: 21257
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^|/)crates/(perry|perry-hir)/.*\.(rs|toml)$|Cargo\.toml$|Cargo\.lock$' | sed -n '1,160p'
echo "== eval_classifier relevant sections =="
wc -l crates/perry-hir/src/eval_classifier.rs
sed -n '1,140p' crates/perry-hir/src/eval_classifier.rs
sed -n '590,745p' crates/perry-hir/src/eval_classifier.rs
echo "== take_deferred_eval_sites references =="
rg -n -C 4 'take_deferred_eval_sites\(|record_deferred_aot_site|has_deferred_dynamic_code_sites|has_known_codegen_sites|print_deferred_eval_notice|dynamicLib|buildOptimizedRuntime|freshness' crates/perry crates/perry-hir
echo "== compile module outline candidates =="
for f in $(git ls-files 'crates/perry/src/commands/compile/*.rs' | sed -n '1,80p'); do
echo "--- $f"
wc -l "$f"
doneRepository: PerryTS/perry
Length of output: 50369
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== build_optimized_libs / fresh call sites =="
rg -n -C 6 'auto_optimized_cache_key|auto_optimized_archives_are_fresh|build_optimized_libs|auto_optimized_fresh|auto_optimized_cross_features|auto_optimized_source_fingerprint|build_stamp' crates/perry/src/commands/compile crates/perry/src -g '*.rs' | sed -n '1,260p'
echo "== perry_hir module globals state =="
rg -n -C 3 'static .*: .*Mutex|static .*: .*Mutex|take_deferred_eval_sites|has_deferred_dynamic_code_sites|has_known_codegen_sites|EVAL_DEFERRED_SITES|KNOWN_CODEGEN_SITE_COUNT' crates/perry-hir/src -g '*.rs'
echo "== run_pipeline top/main relevant section =="
wc -l crates/perry/src/commands/compile/run_pipeline.rs
sed -n '6220,6285p' crates/perry/src/commands/compile/run_pipeline.rs
echo "== compile command functions around pipelines =="
wc -l crates/perry/src/commands/compile/modules.rs crates/perry/src/commands/compile/helpers.rs crates/perry/src/commands/compile/run_pipeline.rs crates/perry/src/commands/compile/mod.rs
sed -n '1,220p' crates/perry/src/commands/compile/modules.rs
sed -n '1,230p' crates/perry/src/commands/compile/helpers.rsRepository: PerryTS/perry
Length of output: 40295
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== compile commands entry points =="
rg -n -C 5 'run_pipeline|CompileContext|CompilationContext|print_deferred_eval_notice|build_optimized_libs|auto_optimized_cross_features|auto_optimized_cache_key|run_with_parse_cache|entry' crates/perry/src/commands crates/perry/src -g '*.rs' | sed -n '1,260p'
echo "== eval_classifier known packages / classify section =="
sed -n '200,450p' crates/perry-hir/src/eval_classifier.rs
sed -n '745,810p' crates/perry-hir/src/eval_classifier.rs
echo "== tests using KnownLibraryCodegen ="
rg -n -C 5 'KnownLibraryCodegen|known-library-codegen|fast-json-stringify|ajv|find-my-way|has_known_codegen_sites' crates -g '*.rs'Repository: PerryTS/perry
Length of output: 50370
Scope KNOWN_CODEGEN_SITE_COUNT to one compilation.
KNOWN_CODEGEN_SITE_COUNT has process lifetime and no reset path. If one process compiles more than one program, one KnownLibraryCodegen site leaves has_known_codegen_sites() true for later programs, changing later optimized-cache keys and enabling perry-runtime/dyn-eval and perry-runtime/regex-engine unnecessarily. Store the result in compilation-owned state, or reset it at the compilation boundary before the next check_site.
🤖 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 `@crates/perry-hir/src/eval_classifier.rs` around lines 112 - 123, Scope
KNOWN_CODEGEN_SITE_COUNT to the current compilation rather than process
lifetime. Update the state used by check_site and has_known_codegen_sites so
each compilation starts with no known codegen sites, either by storing it in
compilation-owned state or resetting it at the compilation boundary before the
next check_site; ensure later programs do not inherit the flag or enable
unnecessary runtime features.
| // Global constructor aliases such as Zod's `const F = Function` | ||
| // retain the Function constructor identity. Treat only aliases | ||
| // explicitly tracked from globalThis.Function as intrinsic; an | ||
| // arbitrary local named F remains an ordinary user value. | ||
| let function_alias = ctx.resolve_class_alias(&class_name).as_deref() == Some("Function"); | ||
| if function_alias { | ||
| class_name = "Function".to_string(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve lexical shadowing for Function aliases.
The name-keyed ctx.resolve_class_alias lookup runs before the code captures the original callee binding. For example:
const F = globalThis.Function;
function make(F) {
return new F("return 1");
}When the parameter F shadows the outer alias, function_alias can still be true. The code then changes class_name to Function, so ctx.lookup_local(&class_name) does not find the parameter. The !function_alias condition also disables shadowed_by_user_binding. new F(...) can therefore use the global Function constructor instead of the parameter value.
Resolve the alias against the original identifier and binding identity. Capture the original LocalId before normalizing class_name, and use the intrinsic path only for the tracked alias binding. Add a regression test for this shadowing case.
Also applies to: 279-280
🤖 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 `@crates/perry-hir/src/lower/expr_new.rs` around lines 207 - 214, Preserve
lexical shadowing in the constructor-lowering flow around resolve_class_alias:
capture the original identifier’s LocalId/binding before normalizing class_name,
then treat the name as the intrinsic Function constructor only when that binding
matches the explicitly tracked globalThis.Function alias. Ensure local
parameters or variables shadowing the alias continue through lookup_local and
shadowed_by_user_binding, and add a regression test for new F(...) inside a
function parameter named F.
|
Apologies — this PR was opened against the wrong repository. It was meant to be opened against my fork These are local experimental patches (5 surgical Rust edits + a downstream-consumer compatibility report) for a personal downstream project (context-mode-native). They are not intended as a contribution to upstream perry, and I should not have opened a PR here. Closing to avoid upstream noise. Sorry for the inconvenience. |
1 participant
Summary
Five small forward-compat patches to
perry-hirthat let the auto-optimizer'sdyn-evalinterpreter ride along forKnownLibraryCodegen(ajv,fast-json-stringify, find-my-way) call sites, and that track
const F = Function-style aliases of the globalFunctionconstructor throughthe
globalThis.Functionpath.Together they unblock compiling
context-mode(a Node.js MCP server) into anative 16 MB binary with
node:sqlite+ FTS5 + HTTP MCP transport workingend-to-end.
Full write-up, including the 7 upstream Perry issues worked around, the
file-level diff inventory, and the end-to-end MCP transcript, is in
CONTEXT-MODE-COMPAT.mdat the repo root.The five patches
crates/perry-hir/src/eval_classifier.rs— addKNOWN_CODEGEN_SITE_COUNT(an
AtomicUsizesibling of the deferred-style sink) and ahas_known_codegen_sites()accessor.check_siteincrements the counter onevery
EvalBucket::KnownLibraryCodegensite, mirroring therecord_deferred_aot_sitepattern.crates/perry-hir/src/lower/expr_new.rs— before theshadowed-by-user-binding check, ask
ctx.resolve_class_alias(&class_name)whether the callee resolves to
"Function". If so, route through theFunction-intrinsic path. The check is gated on a new localfunction_aliasso it is short-circuited together with
force_global_intrinsic.crates/perry-hir/src/destructuring/var_decl/alias_tracking.rs— extendthe
is_global_this_valueproperty.as_str()arm to also accept"Function", so aglobalThis.Functionlookup participates in theprototype-alias map.
crates/perry/src/commands/compile/optimized_libs/freshness.rs— teachthe freshness stamp about the new
has_known_codegen_sites()(keyed asknowngen=…) and broaden thecross_features.push("perry-runtime/dyn-eval"…)gate fromhas_deferred_dynamic_code_sites()tohas_dyn_eval || has_known. Whenthe known-codegen bucket is the only one present, the regex engine still
rides along (ajv uses regex literals too).
crates/perry-hir/src/lib.rs— re-exporthas_known_codegen_sitesnext to
has_deferred_dynamic_code_sites, so the newfreshness.rscallsite resolves through the same
pub use eval_classifier::{…}block.Use case:
context-mode→ 16 MB native binaryBuild time: ~2 minutes (release) / ~15s (debug). Output: a 16.0 MB
context-modebinary that boots without Node.js installed, persists itsSQLite under
/tmp/context-mode-*-<pid>.dbin WAL mode, and serves MCPJSON-RPC over plain HTTP.
Why "experimental"
The downstream build is a real consumer of these patches, but the surface is
deliberately scoped so it can be reverted without leaving dead code:
pub useblock.cross_featuresgate infreshness.rsis a pure superset of the priorhas_deferred_dynamic_code_sites()check.If a future Perry change closes the underlying gaps upstream (e.g. via
#1680/#1681/#1682build-time eval of the known-codegen packages), thispatch series can be reverted commit-for-commit.
Known limitation, not addressed
A separate
RangeError: toString() radix argument must be between 2 and 36GC crash on the full
context-mode/server.ts(4948 lines) is not fixedby this PR. The crash is real and reproducible, but its root cause sits in
perry-codegen's conservative-scan/object-shape path and a fix there is anon-trivial GC change of its own. The downstream user works around it by
splitting the build into 4 sub-400-line tool bundles that don't share an
import graph with the crashing modules — see
CONTEXT-MODE-COMPAT.mdforthe file inventory and the tool-level verification transcript.
Test plan
context-mode(full server with 11 tools) compiles in 2 minutescontext-modeboots and answers MCPinitialize/tools/list/tools/callwith FTS5 storage workingspillover)
changelog.d/7220-context-mode-patches.mddescribes the changeCHANGELOG.mdleft untouched (frozen at v0.5.1264 per the policy inchangelog.d/README.md)The downstream consumer's own test suite (
context-mode'stests/) isgreen; the upstream Perry
lintjob is expected to pass because the patchesadd the required
changelog.d/fragment and thecrates/diff issurgically scoped.
Summary by CodeRabbit
New Features
Functionaliases during builds.Documentation