feat: soft-deprecate tree-walker (remove --run-tree CLI, internalise interpreter module)#423
Closed
danieljohnmorris wants to merge 6 commits into
Closed
feat: soft-deprecate tree-walker (remove --run-tree CLI, internalise interpreter module)#423danieljohnmorris wants to merge 6 commits into
danieljohnmorris wants to merge 6 commits into
Conversation
The tree-walking interpreter has been the slowest engine since the VM became the default, and the --run-tree flag was mostly a foot-gun: agents reached for it for debuggability and paid an 8x wall-clock tax on real workloads (see the 2026-05-12 routing-tsp / opt4.ilo notes). This soft-deprecates the engine. --run-tree and its --run alias are removed from the public CLI: they used to compile to Engine::Tree via clap, now they fall through to the unknown-flag guard with a clean error pointing at --run-vm and --jit. The tree-walker itself stays in-tree as the dispatch target for the HOFs / regex / fmt / IO shapes the VM and Cranelift haven't lifted natively yet (see the 2026-05-18 STOPPED entry); the VM bails to it transparently. interpreter is gated behind #[doc(hidden)] in lib.rs so it disappears from rustdoc / IDE completion, but stays pub for the in-tree bin and the cross-engine tests. The run_tree / run fields on RunArgs are kept with #[arg(skip)] so internal construction sites (REPL, dispatch, tests asserting effective_engine) still compile. Real removal is deferred to 0.13.0+ once the bridge consumers are lifted natively and the shared runtime types are extracted to a non-engine module. The wr (3-arg dynamic-format) VM compile error used to suggest --run-tree as the workaround; that hint is now stale, so the message just states the gap.
…removal cli_run_flag_placement got a full rewrite: the per-position --run-tree tests now exercise --run-vm, the two conflicting-flags tests pin the --run-vm vs --jit pair, the --run-alias tests assert clean rejection via the unknown-flag guard, and the file-path positional tests track the same flag. cli_integration.run_tree_basic_execution was renamed to run_vm_basic_execution_via_subcommand and repointed at --run-vm so the clap subcommand path still gets coverage. regression_unknown_flag_guard gains a positive test that --run-tree now exits non-zero with the flag named in stderr. eval_inline had a handful of legacy --run alias invocations (the tree-walker shorthand); those flip to --run-vm.
Bulk-replace --run-tree with --run-vm in every cross-engine regression test. The tree-walker is no longer publicly selectable, but the VM bails to it transparently for the bridge ops (regex, fmt, ctx-arg HOFs, io shapes), so coverage of tree-walker code paths is preserved through VM invocations. ENGINES_ALL arrays shrink from [tree, vm, jit] / [tree, vm] to [vm, jit] / [vm]; single-element arrays trip clippy's single_element_loop where they used to be multi-engine loops, so the affected files gain a file-level #[allow] with a comment pointing at this change. coverage_interpreter widens its error-code asserts from a tight ILO-R009 (interpreter error code) to a union of ILO-R003 / R004 / R009 / R012 because the VM and Cranelift use different codes for the same shape errors. wr_unknown_format_errors now accepts the VM's 'undefined function' compile-time refusal since the VM has no native 3-arg wr-with-dynamic-format path and --run-tree is gone. examples_engines drops the duplicate tree row from its engine list. engine-matrix/run-matrix.sh + README drop the tree column from the audit matrix.
SPEC.md and ai.txt: drop --run-tree from the in-process-runner list, drop the tree-interpreter sentence from the default-engine section (it falsely implied Phase 2 closures only run on tree; VM and JIT have handled captures natively since PR1 / PR2 last year). Add a new 'Tree-walker is internal-only' paragraph that names the remaining bridge consumers so an agent reading the spec knows where the engine still hides. skills/ilo/SKILL.md: drop the --run-tree line from the CLI examples block and note the removal in the subcommand-dispatch paragraph. The engines table in ilo-engines.md trims to three rows (VM, Cranelift JIT, Cranelift AOT) with the internal-only tree-walker called out underneath. Capturing-lambda guidance was a leftover from the pre-PR1 days; the matrix is updated to state that every public backend runs captures. README.md: drop the 'ilo Interpreter' row from the benchmarks table. The numbers there were never the headline (the VM and JIT rows are what agents actually choose between), and keeping the row implied --run-tree was a supported choice.
…oft-deprecate-tree-walker # Conflicts: # SPEC.md # ai.txt # skills/ilo/SKILL.md # skills/ilo/ilo-engines.md
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
…oft-deprecate-tree-walker # Conflicts: # ai.txt # skills/ilo/SKILL.md # tests/eval_inline.rs
Collaborator
Author
|
Abandoning this PR — main's skill docs (SKILL.md, ilo-engines.md, ai.txt, SPEC.md) have restructured substantially since this branch was opened, leaving the doc-sync commit obsolete and producing 135-line semantic conflicts. Redoing fresh off current main is cleaner than merging through the drift. New PR coming under feature/soft-deprecate-tree-walker-v2. |
5 tasks
danieljohnmorris
added a commit
that referenced
this pull request
May 19, 2026
… removal The original PR #423 doc-sync commit hit semantic conflicts after main's skill docs were restructured (the engines page rewritten, ilo-edit-loop and ilo-errors added). Redo the doc edits against current main: - SPEC.md: drop --run-tree from the in-process runner list, retire the canonical-semantics-reference sentence (every public backend handles Phase 2 captures natively), and add a new "Tree-walker is internal-only" paragraph naming every entry in is_tree_bridge_eligible plus the two bridge-coverage test files. - ai.txt: same three edits, single-line. - README.md: drop the "ilo Interpreter" row from the benchmarks table. The VM and JIT rows are what agents actually choose between, and keeping the interpreter row implied --run-tree was a supported choice. - skills/ilo/ilo-engines.md: full rewrite. Drops the Tree-walk row and the `--run` alias note, lists three public backends, calls out the internal-only tree-walker with the bridge ops below the table, and trims the benchmarking line to vm/jit. Captures-run- natively guidance loses the tree caveat. - skills/ilo/ilo-edit-loop.md: R012 guidance loses the "or --run-tree" workaround (every public backend supports Phase 2 captures). - skills/ilo/ilo-errors.md: the `--engine tree` mystery-arity pattern points at --run-vm / --jit instead of --run-tree. - CHANGELOG.md: move the --run-tree removal line from the 0.12.0 Breaking section (which never shipped that change) to 0.12.1 where it actually lands, with the longer framing that calls out the bridge consumers.
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
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
The tree-walking interpreter has been the slowest engine since the VM became the default (PR #390). The
--run-treeflag was mostly a foot-gun: agents reached for it for debuggability and silently paid an 8x wall-clock tax on real workloads. This soft-deprecates the engine without ripping it out, picking the safe path through the blocker called out by the 2026-05-18 STOPPED entry onilo_assessment_feedback.md(14 tree-bridge consumers + 191 references to shared infra still living undersrc/interpreter/).Result: agents pick from VM / JIT / AOT, the tree-walker keeps running internally for the HOF / regex / fmt / IO shapes the VM hasn't lifted natively yet, and the door is open for a clean hard-drop in 0.13.0+ once the bridge consumers and shared runtime types get extracted.
What's in the diff
Four commits, each a coherent slice:
cli: drop --run-tree / --run from the public engine surface—src/cli/args.rsstrips the two flags from the clap derive (they fall through to the existing unknown-flag guard),src/lib.rsgatesinterpreterbehind#[doc(hidden)],src/main.rsremoves the tree branch fromextract_run_engine_flagand the bare-args dispatcher, and thewr3-arg dynamic-format compile error stops suggesting--run-treeas the workaround.tests: rewrite cli flag-placement / unknown-guard for the --run-tree removal—cli_run_flag_placement.rsgot a full rewrite (per-position tests now exercise--run-vm; the conflicting-flags tests pin--run-vmvs--jit; the--runalias tests assert clean rejection via the unknown-flag guard).regression_unknown_flag_guardgains a positive test that--run-treeexits non-zero with the flag named in stderr.tests: sweep --run-tree to --run-vm across the regression suite— 159 test files.ENGINES_ALLarrays shrink from[tree, vm, jit]/[tree, vm]to[vm, jit]/[vm]; single-element loops trip clippy'ssingle_element_looplint where they used to be multi-engine, so the affected files gain a file-level#[allow]with a pointer comment.coverage_interpreterwidens its error-code asserts because tree and VM use differentILO-R###codes for the same shape errors;wr_unknown_format_errorsaccepts the VM'sundefined functioncompile-time refusal since the VM has no native 3-argwr-with-dynamic-format path.docs: sync SPEC / ai.txt / SKILL / README for --run-tree removal— drops--run-treefrom in-process runner lists, retires the stale Phase-2-closures-only-on-tree sentence (VM/JIT have handled captures natively since PR1/PR2), adds aTree-walker is internal-onlyparagraph naming the remaining bridge consumers, and removes theilo Interpreterrow from the README benchmarks table.Companion commit on the site repo: ilo-lang/ilo-site@c2d6fec drops
--run-treefromengines.md,cli.md, andbenchmarks.md. Already pushed; the site has no CI so no follow-up there.Before / after
Why soft-deprecate rather than hard-drop
The 2026-05-18 STOPPED entry catalogues why the hard-drop was blocked:
is_tree_bridge_eligibleinsrc/vm/mod.rs:532still routes 14 builtin-shape pairs throughcall_builtin_for_bridge+ACTIVE_AST_PROGRAMTLS (Rgx 2, Rgxall 2, Rgxall1 2, Fmt, Rd 2, Rdb 2, Sleep 1, Ct 2/3, Rsrt 2/3, Map 3 ctx, Flt 3 ctx, Fld 4 ctx, Srt 3 ctx). Theinterpreter::module also exports shared infra used by VM and Cranelift (Value,MapKey,RuntimeError, math helpers) — 191 references across 8 files.Soft-deprecation keeps both of those working while removing the user-facing flag and the rustdoc / IDE-completion noise. 0.13.0+ can hard-drop after PR3d/PR3e migrate the bridge consumers natively and the shared runtime types get extracted to a non-engine module.
Test plan
cargo build --release --features craneliftcleancargo test --release --features cranelift— 6933 passed / 0 failedcargo fmt --all -- --checkcleancargo clippy --release --features cranelift --all-targets -- -D warningsclean--run-tree,--runrejected with clean errors;--run-vmstill worksFollow-ups
Value/MapKey/RuntimeErrorand math helpers tosrc/runtime/, then hard-dropsrc/interpreter/.## ✅ Addressedmove on merge.