Skip to content

feat(evaluator): PR-E Bind callable entry - #1496

Merged
briansrls merged 2 commits into
mainfrom
feat/r3-evaluator-bind-entry
May 2, 2026
Merged

briansrls merged 2 commits into
mainfrom
feat/r3-evaluator-bind-entry

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Summary

Implements the PR-E Bind/callable-entry evaluator prerequisite inside the existing Rust evaluator surface.

  • wires Behavior::Bind through eval_node
  • adds eval_bind for bounded body-entry evaluation through BindNode.value
  • copies declared BindNode.params from caller-visible frames into a fresh callable frame before evaluating the body port
  • preserves stack balance on success and diagnostic paths
  • keeps scope out of E6 lens fold, runner work, substrate changes, and new runtime carriers

Validation

  • cargo fmt --all --check
  • RUST_MIN_STACK=33554432 RUSTC_WRAPPER= cargo test -p v3-compiler --lib evaluator::tests
  • RUST_MIN_STACK=33554432 RUSTC_WRAPPER= cargo clippy -p v3-compiler --lib -- -D warnings
  • RUST_MIN_STACK=33554432 RUSTC_WRAPPER= cargo test -p v3-compiler --lib

Note: this checkout's inline v3-compiler lib test binary overflows the default Rust test thread stack even on unchanged evaluator frame-helper tests, so the local lib/evaluator test runs used RUST_MIN_STACK=33554432.

@briansrls
briansrls marked this pull request as ready for review May 2, 2026 07:35
@briansrls

Copy link
Copy Markdown
Contributor Author

Manager pass: this is the right Bind/callable-entry prerequisite shape. It wires only Behavior::Bind, evaluates through the existing eval_port/eval_node surface, copies declared params from the caller-visible frames into a fresh callable frame, and restores stack depth on success, duplicate-param failure, unbound-param failure, and body diagnostics.

Scope looks clean: no E6 fold driver, no runner work, no substrate/value/report shape changes. Hold for CI/reviews, but I do not see a manager-blocking issue from this pass.

— sent from snappy-moth-795

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: aff92b01 · Trigger: schedule
  • Comparison: origin/main @ 331b9ebe ... review/pr-1496-aff92b01 @ aff92b01
  • Thinking: 48s wall

Findings: None. The diff implements Bind with explicit dispatch (no open behavior => fallback), keeps failures on typed EvalError / EvalFrameError paths, resolves parameter bindings before pushing the inner frame so unbound parameters never allocate a frame, and always pop_frames after the body so the stack is restored on both success and failure. Tests use minimal Dag construction and assert observable outcomes (value, frame depth, caller binding preservation, duplicate/unbound errors, body diagnostics), consistent with TESTING.md for this kind of unit.

Verdict: APPROVE — Scoped evaluator change: removes the UnsupportedBehavior stub for Bind, adds the documented callable-frame behavior, and replaces it with targeted regression tests. No concrete conflict with INVARIANTS.md, docs/modeling-discipline.md, CODING.md, or TESTING.md visible in the diff.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: aff92b01 · Trigger: schedule
  • Thinking: 137s wall

Non-blocking — Strengths

  • src/v3/compiler/src/lib.rs Code-only evaluator slice: Bind now covers the remaining L1 behavior, keeps failures typed through EvalError/EvalFrameError, and tests stack restoration on success and diagnostics.

✅ No blocking concerns for this Bind callable-entry implementation.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: 57f1a584 · Trigger: schedule
  • Comparison: origin/main @ 11c12d08 ... review/pr-1496-57f1a584 @ 57f1a584
  • Thinking: 60s wall

Verdict: APPROVE

Diff is narrowly scoped to making Behavior::Bind executable in the evaluator, with typed failure paths for missing/duplicate frame bindings and stack restoration tests around diagnostics. I did not see concrete violations of the pinned invariants, modeling discipline, coding guidance, or testing guidance. Builds/tests were not run per instruction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant