docs(evaluator): R3 E4 branch readiness / blocker audit (docs-only) by briansrls · Pull Request #1388 · gunb-ai/gunbc

briansrls · 2026-05-01T07:12:30Z

Summary

Per Director re-task after PR-E E4 STOP+PING was accepted: docs-only audit recording the exact E1 prerequisites E4 (Branch arm coverage) depends on. E4 implementation waits for E1 / E1.0 to land the runtime Value Rust mirror and eval_node / eval_port shell.

Brief contents

docs/briefs/r3-evaluator-e4-branch-readiness-audit.md:

State at HEAD — E0 docs-only landed (docs(evaluator): R3 E0 body-evaluator API scaffold (docs-only) #1371); E2 frame helpers landed (feat(evaluator): PR-E E2 EvalFrame lookup and Bind environment #1374) as EvalFrame<V> / EvalStateStack<V> generic on V; E1 has not landed (verified via grep — no runtime Value mirror, no eval_value/eval_node/eval_port outside lens_apply.rs::EvalCtx which operates on FieldValue).
Four E1 prerequisites: (1) Value::VariantValue { tag: DeclarationId, payload: Box<Value> } access; (2) eval_port port-level demand-eval authority; (3) eval_node dispatch shell with all five Behavior arms (four NotYetImplemented placeholders); (4) Frame discipline reaching EvalStateStack<Value> instantiation.
E4 preview — eval_branch per PR-B.1 §B.1.3, acceptance tests, balanced-stack invariant.
Cross-references — PR-E dispatch, PR-B.1 §B.1.3, E0, E2.
STOP+PING boundary — what E4 must NOT do when implemented.

Constraints upheld

Docs-only.
No Rust, no substrate, no tests, no fixtures.
No new substrate carriers proposed.
No bundling of E1 + E4.

Test plan

Docs-only; CI fmt unaffected; v3 may still fail SG-0 census ratchet upstream.
Reviewer confirms the four prerequisites are sufficient for E4 to be a small follow-on once E1 lands.

🤖 Generated with Claude Code

Records the exact E1 prerequisites E4 (Branch arm coverage) depends on, after STOP+PING acceptance in #1371/#1374 follow-on dispatch: 1. Runtime Value Rust mirror with at least LiteralValue + VariantValue { tag: DeclarationId, payload: Box<Value> } arms. 2. eval_port port-level demand-eval authority (consumes existing EvalStateStack::lookup + dag.port_opt(port).produced_by). 3. eval_node dispatch shell with all five Behavior arms, four returning NotYetImplemented placeholders until later slices fill. 4. Frame discipline reaching EvalStateStack<Value> instantiation — E2's helpers are generic on V; E4 consumes the instantiated stack. Also previews E4's eval_branch implementation per PR-B.1 §B.1.3 + acceptance tests, and names the STOP+PING boundary E4 must respect when unblocked. Cross-links E0 / E2 / PR-B.1 / dispatch brief. Docs-only; no Rust, no substrate, no tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

briansrls · 2026-05-01T07:14:04Z

Review metadata

Provider / model: cursor / composer-2
Commit: c8d8852e · Trigger: schedule
Comparison: origin/main @ 569b753b ... review/pr-1388-c8d8852e @ c8d8852e
Thinking: 13s wall

The diff only adds docs/briefs/r3-evaluator-e4-branch-readiness-audit.md: an E4 readiness / blocker audit that ties E4 to E1 (Value, eval_port, eval_node shell, EvalStateStack<Value>), cites existing briefs and code locations, and repeats STOP+PING boundaries from E0.

Findings: None. Nothing here contradicts INVARIANTS.md (it reinforces single-authority / no parallel demand-eval and fail-closed diagnostics), docs/modeling-discipline.md (no new substrate types or Rust enums in the diff), CODING.md (Rust style applies to implementation code), or TESTING.md (docs-only; no new tests claimed as shipped).

Verdict: APPROVE — Narrow, honest docs-only audit with explicit scope (“AUDIT”, “Out of scope”), point-in-time verification anchored to a named origin/main commit, and preview sections clearly labeled. No rubric violations grounded in this diff.

…ip before frame lookup Reviewer flagged on the merged E0 brief that "frame lookup first" allows a caller-supplied initial frame containing a nonexistent PortId to evaluate successfully — Fail-Closed violation. Carries the fix forward in this E4 readiness audit since E4's eventual implementation consumes eval_port: order is dag.port_opt(&port) → frame_lookup → producer-node demand-eval. A follow-on docs amendment should clarify E0's wording. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ult_port Reviewer flagged on merged E0: eval_value's signature lacked stack parameter, so a Value-produced PortId demand-eval would return Value without making the port readable — Facts-Flow-Forward violation. Carries the fix forward: E1 signature must be eval_value(node, stack: &mut EvalStateStack) and call stack.bind_top(node.result_port, value) before returning. Same producer-bind invariant for eval_transform / eval_loop / eval_branch (eval_bind already binds explicitly). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

briansrls

Review metadata

Provider / model: codex / unknown
Commit: 7fbd92a8 · Trigger: schedule
Thinking: 216s wall

BLOCKING (1)

Root Cause

docs/briefs/r3-evaluator-e4-branch-readiness-audit.md E0 remains an unsuperseded boundary contract after this audit changes the required port-resolution order -> amend the E0 contract now or mark the exact E0 section superseded by this audit.

⚠️ One blocking docs-contract conflict remains.

briansrls · 2026-05-01T07:23:39Z

+  Fail-Closed and "Every Dependency Is A Substrate Fact"). The E0
+  brief's port-level contract said "frame lookup first" without
+  this prerequisite — E1's implementation must perform the
+  membership check ahead of frame lookup, and a follow-on docs


BLOCKING: The audit knowingly leaves E0's live eval_port contract with the opposite lookup order, so Documentation Describes Live State / Explicit Boundary Contracts require updating or explicitly superseding the E0 wording here.

…corrections Reviewer flagged that E0 remains authoritative while this audit changes the required eval_port order and the eval_value signature. Adds an explicit "Supersedes" §naming the two E0 sections this audit corrects (Port-level evaluation contract step 1; Per-Behavior slice fills eval_value signature) and notes E1 implementation MUST follow the audit wording until a follow-on E0 amendment lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

briansrls · 2026-05-01T07:30:09Z

Review metadata

Provider / model: claude / claude-opus-4-7
Commit: 535b4f5f · Trigger: schedule
Comparison: origin/main @ 569b753b ... review/pr-1388-535b4f5f @ 535b4f5f
Thinking: 13s wall

APPROVE — Docs-only audit brief. New file under docs/briefs/, no Rust/substrate touched. Content is internally consistent: it explicitly notes it supersedes two specific E0 contract points (port-membership-check-first for Fail-Closed, and eval_value taking &mut stack for Facts-Flow-Forward) and grounds each in the relevant invariant. Cross-references and file/line citations (runtime.dag:46-47, dag.rs:3237, :3061) look plausible and are bounded by the brief's "Out of scope" section. No findings.

…51-pr-e-e4-readiness

briansrls · 2026-05-01T07:48:46Z

Review metadata

Provider / model: claude / claude-opus-4-7
Commit: e298be04 · Trigger: schedule
Comparison: origin/main @ 7c750541 ... review/pr-1388-e298be04 @ e298be04
Thinking: 12s wall

APPROVE — docs-only audit brief, single new file under docs/briefs/. No code, no substrate, no fixtures touched. Content is internally consistent: it correctly identifies E1 prerequisites, flags two E0 contract holes (port-membership-before-frame-lookup for Fail-Closed, and eval_value needing &mut stack for Facts-Flow-Forward) and explicitly names itself as superseding E0 wording until a follow-on amendment lands — that's a documented, bounded, named dissolution trigger, which is the right shape for a tracked-debt bridge. STOP+PING boundary at the end correctly preserves the no-wildcard-pattern and no-parallel-carrier invariants.

No findings.

briansrls mentioned this pull request May 1, 2026

session/snappy-moth-795 · snappy-moth-795 #1131

Closed

briansrls mentioned this pull request May 1, 2026

docs(evaluator): R3 E0 body-evaluator API scaffold (docs-only) #1371

Merged

2 tasks

briansrls commented May 1, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into session/merry-heron-3…

e298be0

…51-pr-e-e4-readiness

briansrls merged commit 8523852 into main May 1, 2026
4 checks passed

This was referenced May 1, 2026

session/merry-heron-351 · merry-heron-351 #1199

Closed

session/zesty-bear-812 · gunbc Director #828

Closed

feat(evaluator): PR-E E4 Branch arm coverage (eager LeftFirst) #1426

Merged

session/merry-gull-128 · merry-gull-128 #1743

Closed

Navigation Menu

Repository navigation

docs(evaluator): R3 E4 branch readiness / blocker audit (docs-only) - #1388

docs(evaluator): R3 E4 branch readiness / blocker audit (docs-only)#1388
briansrls merged 5 commits into
mainfrom
session/merry-heron-351-pr-e-e4-readiness

briansrls commented May 1, 2026

Uh oh!

briansrls commented May 1, 2026

Uh oh!

briansrls left a comment

Uh oh!

briansrls May 1, 2026

Uh oh!

briansrls commented May 1, 2026

Uh oh!

briansrls commented May 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

briansrls commented May 1, 2026

Summary

Brief contents

Constraints upheld

Test plan

Uh oh!

briansrls commented May 1, 2026

Uh oh!

briansrls left a comment

Choose a reason for hiding this comment

Uh oh!

briansrls May 1, 2026

Choose a reason for hiding this comment

Uh oh!

briansrls commented May 1, 2026

Uh oh!

briansrls commented May 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant