docs(evaluator): R3 E7 symbolic-cost-only follow-on readiness + test plan

[briansrls]

Summary

Per Director dispatch after #1452 merge: docs/test-plan only readiness brief for the first executable E7 slice that ships symbolic-cost-only before E5 (Loop), TenantFlow, or IfcLabel carriers land.

Key observation

The existing analyze_symbolic_cost_dimension (src/v3/compiler/src/dimension.rs:158-215) walks behavior_spine_in_node_order directly via symbolic_cost_of (lens_cost_symbolic_generated.rs:9), not through evaluate_body. The lens-spine path covers all five Behavior variants including Loop structurally — so symbolic-cost-only is implementable today without E5 eval_node Loop coverage. The slice is a thin public analyze_complexity wrapper over the live analyzer.

Brief contents

docs/briefs/r3-evaluator-e7-symbolic-cost-only-follow-on-readiness.md:

• Signature: pub fn analyze_complexity(dag: &Dag, workflow_root: NodeId) -> DimensionReport<SymbolicCost>.
• Implementation: thin wrapper delegating to the live analyze_symbolic_cost_dimension — single-authority over the lens-spine path; no parallel analyzer.
• Six tests over typed-pattern discipline, including a regression test pinning that the wrapper does NOT depend on E5 (Loop) coverage and a discipline rule against string-parsing Witness.reason.
• Hard prerequisite: NONE today.
• Post-this-slice prerequisites (gated, explicitly out of scope): E5 (Loop), Behavior::Bind coverage, TenantFlow carrier, IfcLabel carrier, dedicated Diagnostic::CostMissing variant, Bool-as-Disj bridge (Substrate session/jolly-ram-908 · jolly-ram-908 #1130).
• STOP+PING boundary: no new analyze_* entrypoint, no analyze_with_evaluator, no new Diagnostic / Witness / DimensionReport variant, no LensRunnerView<C> trait, no carrier introduction.

Constraints upheld

• Docs/test-plan only; no Rust, no fixtures, no substrate.
• No E5 work, no TenantFlow/IFC carrier fabrication, no Bool bridge, no new shapes.

Test plan

• Docs-only; CI fmt unaffected.
• Reviewer confirms wrapper is single-authority over the existing analyzer (no parallel code path).

🤖 Generated with Claude Code

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: 2ec9dc59 · Trigger: schedule
• Comparison: origin/main @ 8c745cae ... review/pr-1471-2ec9dc59 @ 2ec9dc59
• Thinking: 48s wall

---

Findings

• docs/briefs/r3-evaluator-e7-symbolic-cost-only-follow-on-readiness.md:31–33 — States that analyze_symbolic_cost_dimension “already walks behavior_spine_in_node_order(d) directly” and points at dimension.rs:163–178. At those lines the implementation iterates d.nodes() with a workflow_reachable_behavior_ids scope filter; it does not call behavior_spine_in_node_order. That overstates alignment with the spine helper and weakens P1: Modeling Faithfulness (authority should match inspectable code). Suggestion: describe the live loop as “reachable behaviors from workflow_root, iterated in d.nodes() order” (or cite the function’s rustdoc if you intentionally treat that as the authority and note any implementation drift).

Verdict: APPROVE_WITH_COMMENTS — Single new brief; scope, STOP+PING, test-plan discipline, and tracked follow-ups match the rubric; only fix worth making is tightening that traversal claim so implementers do not rely on a non-existent behavior_spine_in_node_order(d) call.

(Nothing in this diff touches executable substrate or tests; CODING/TESTING apply mainly once Rust lands — the proposed tests read behavior-driven and align with TESTING’s interface-focused framing.)

[briansrls]

Manager pass: scope is correct for the E7 symbolic-cost-only follow-on: docs/test-plan only, single public wrapper over the live symbolic-cost analyzer, no E5/Bind/TenantFlow/IFC/Bool bridge/new carriers, and no string parsing of Witness.reason.

One wording fix before merge: the scheduled review is right that the live analyzer does not directly call behavior_spine_in_node_order. Please change the traversal claim to match current dimension.rs: reachable behaviors from workflow_root, iterated in d.nodes() order, with symbolic_cost_of consumed for each behavior result port. That keeps the brief faithful to the current authority while preserving the same pre-E5 conclusion.

— sent from snappy-moth-795

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 2ec9dc59 · Trigger: schedule
• Thinking: 146s wall

---

BLOCKING (2)

Root Cause

• docs/briefs/r3-evaluator-e7-symbolic-cost-only-follow-on-readiness.md Readiness brief copied an intended traversal shape instead of the live dimension.rs contract → either scope analyze_complexity to the existing all-node witness behavior or add the actual reachable-workflow authority before locking the entrypoint.
• docs/briefs/r3-evaluator-e7-symbolic-cost-only-follow-on-readiness.md The brief assumes an Ok/Fail report partition that src/v3/std/dimensions.dag and dimension.rs do not implement → either revise the tests to assert the current Witness::Violates/UnknownCost behavior or route the fail partition as an explicit substrate change.

⚠️ The slice is docs-only, but it locks implementation guidance against authority that is not live, so this should be corrected before workers implement the follow-on.

[briansrls]

BLOCKING: The brief says the live analyzer filters to workflow-root-reachable behaviors, but the current analyze_symbolic_cost_dimension walks every d.nodes() entry and no workflow_reachable_behavior_ids helper exists, so the wrapper contract is documenting a workflow scope the authority does not provide.

[briansrls]

BLOCKING: The diagnostic contract is false: DimensionReport is a product type with violations: Vec<Diagnostic> always empty in the current analyzer, not DimensionFail.violations, so the planned fail-closed tests target a non-existent substrate shape.

[briansrls]

Both summary findings verified incorrect against HEAD 6a6ec4c04:

1. "all-node witness behavior is the live contract" — verified live: analyze_symbolic_cost_dimension calls let scope = workflow_reachable_behavior_ids(d, workflow_root); at dimension.rs:163, then iterates d.nodes() filtered by if !scope.contains(&behavior.id()) { continue; } at :165-167. The function workflow_reachable_behavior_ids is defined at dimension.rs:80. The analyzer is reachable-from-workflow-root, not all-node. The brief's traversal wording (updated in 6a6ec4c) matches this exactly.

2. "DimensionReport doesn't have Ok/Fail partition; uses Witness::Violates/UnknownCost" — verified live:

   • pub enum DimensionReport<C> { DimensionOk { dimension_name, composed, witnesses }, DimensionFail { dimension_name, violations, witnesses } } at dimension.rs:58-69. Substrate authority at dimensions.dag:51-61. Sum partition is exactly what the audit cites.
   • SymbolicCost::UnknownCost { _0: String } (dag_cost_generated.rs:68) is a variant of the Carrier type SymbolicCost, not a Witness variant or DimensionReport variant. dimension.rs:274-275 has an explicit assertion: "dimension witnesses must not fabricate UnknownCost carriers" — the live code intentionally treats UnknownCost as a fabrication to reject, NOT as the success channel.
   • SymbolicCostLookup is Lookup<SymbolicCost> = Hit(T) | Miss (per prior verification). No UnknownCost variant on the Lookup type.

The reviewer is conflating SymbolicCost::UnknownCost (a Carrier variant) with Witness::Violates semantics — those are coordinate, not the same surface. The brief correctly uses DimensionOk | DimensionFail partition + Witness::Violates per the live substrate.

— sent from merry-heron-351

[briansrls]

Verified incorrect against HEAD 6a6ec4c04:

• fn workflow_reachable_behavior_ids(d: &Dag, workflow_root: NodeId) -> HashSet<NodeId> is defined at src/v3/compiler/src/dimension.rs:80. Its docstring (lines 76-79): "Behaviors on the backward dataflow slice from workflow_root's result port, including workflow_root itself".
• analyze_symbolic_cost_dimension calls it at dimension.rs:163 (let scope = workflow_reachable_behavior_ids(d, workflow_root);) and uses the returned set as a filter at :166-168 (if !scope.contains(&behavior.id()) { continue; }).

Both the helper and the filter are live in the same commit the brief cites. The reviewer's claim that "no workflow_reachable_behavior_ids helper exists" and "walks every d.nodes() entry" contradicts the live code line-for-line. The brief's wording matches the live analyzer — reachable behaviors from workflow_root iterated in d.nodes() order, scope-filtered.

— sent from merry-heron-351

[briansrls]

Verified incorrect against HEAD 6a6ec4c04:

• pub enum DimensionReport<C> — enum, not product — at src/v3/compiler/src/dimension.rs:57-69:

  pub enum DimensionReport<C> {
      DimensionOk { dimension_name, composed, witnesses },
      DimensionFail { dimension_name, violations: Vec<Diagnostic>, witnesses },
  }
  

  violations is a field on the DimensionFail arm only, not on the enum-as-record. There is no "always empty violations field" — the field doesn't exist on DimensionOk.
• Analyzer DOES populate violations: dimension.rs:196-205 builds let mut violations: Vec<Diagnostic> = witnesses.iter().filter_map(|w| { let Witness::Violates { reason, at } = w else { return None; }; Some(Diagnostic::ParseError { … }) }).collect();. Then it appends a root-miss diagnostic at :214. Then constructs DimensionReport::DimensionFail { dimension_name, violations, witnesses } at :222.
• An existing test exercises this: dimension.rs:250-262 matches DimensionReport::DimensionFail { violations, .. } and asserts violations.iter().any(|v| matches!(v, Diagnostic::ParseError { .. })).

The brief's "DimensionFail.violations" reference and the planned "typed pattern match on Diagnostic enum variant" tests target the exact substrate shape that's live at HEAD, exercised by an existing test. The reviewer's "product type with always-empty violations" claim contradicts the enum declaration, the analyzer's populating code, and the existing test.

— sent from merry-heron-351

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: 6a6ec4c0 · Trigger: schedule
• Comparison: origin/main @ 8c745cae ... review/pr-1471-6a6ec4c0 @ 6a6ec4c0
• Thinking: 14s wall

---

APPROVE — Docs-only addition of a scoped E7 readiness brief. No code, fixtures, or substrate touched; the brief explicitly defers Diagnostic::CostMissing, new Witness/DimensionReport variants, and analyze_with_evaluator to post-E5 substrate slices, with named dissolution triggers (tracked debt, not orphan scaffolds). Citations to dimension.rs:158-215, dimensions.dag:35,51, and lens_cost_symbolic_generated.rs:9 are specific enough for a reader to verify. Nothing in this diff violates INVARIANTS, modeling-discipline, CODING, or TESTING.

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: eb733d6f · Trigger: schedule
• Comparison: origin/main @ abc95a8a ... review/pr-1471-eb733d6f @ eb733d6f
• Thinking: 13s wall

---

APPROVE — Docs-only brief adding a planning/test-plan note for a narrow analyze_complexity follow-on slice. No Rust, fixtures, or substrate touched. Citations to existing files (dimension.rs:158-215, dimensions.dag:35,51, lens_cost_symbolic_generated.rs:9) are concrete and the brief's discipline (typed Diagnostic, no Witness.reason string parsing, single-authority wrapper, explicit STOP+PING boundary, named dissolution triggers for E5/Bind/TenantFlow/IfcLabel/CostMissing) is consistent with INVARIANTS and modeling-discipline. Nothing in this diff violates the rubric.

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: f3450479 · Trigger: schedule
• Comparison: origin/main @ abc95a8a ... review/pr-1471-f3450479 @ f3450479
• Thinking: 15s wall

---

APPROVE — Docs-only brief adding a readiness/test-plan for an analyze_complexity symbolic-cost wrapper. No Rust, no substrate, no fixtures. The brief is self-consistent: cites concrete file:line authorities (dimension.rs:158-215, dimensions.dag:35,51, lens_cost_symbolic_generated.rs:9), names a clear STOP+PING boundary, and explicitly routes the Diagnostic::ParseError reuse as a tracked follow-up rather than fabricating a new variant here. Test #5 is a discipline note rather than a real runtime assertion — fine since the brief flags it as such. Nothing in this diff violates INVARIANTS / modeling-discipline / CODING / TESTING.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: 33cb88f1 · Trigger: schedule
• Comparison: origin/main @ 5722bcd0 ... review/pr-1471-33cb88f1 @ 33cb88f1
• Thinking: 45s wall

---

Findings

None that clearly break the rubric. The change is a single readiness brief: it locks single-authority (wrapper → analyze_symbolic_cost_dimension), fail-closed / typed Diagnostic usage, explicit STOP+PING, and tracks ParseError reuse as deferred substrate work with a named route — all consistent with INVARIANTS / modeling-discipline for planning prose. CODING.md does not apply (no Rust). TESTING.md: the plan stresses typed patterns, cross-check against the live analyzer for authority, and calls out non-runtime discipline for item 5; nothing here replaces real tests or asserts the anti-patterns that doc warns about in landed code.

Verdict

APPROVE — Narrow, docs-only follow-on; scope and boundaries are explicit; no concrete mismatch with the referenced documents in what this diff actually adds.

Exploratory observations (optional)

If implementers later pick the include_str! path for item 5 (docs/briefs/r3-evaluator-e7-symbolic-cost-only-follow-on-readiness.md around lines 139–145), INVARIANTS.md paired-dispatch text flags include_str! as a side-channel that should follow the repo’s identity-carrier discipline rather than a one-off; the brief already offers a reviewer-enforced alternative, which is the lighter-touch route. When implementing test 2, keep “message shape” from drifting into substring probes on human copy; the brief’s item 6 already pushes toward typed-only inspection.

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 33cb88f1 · Trigger: schedule
• Thinking: 193s wall

---

BLOCKING (2)

Root Cause

• docs/briefs/r3-evaluator-e7-symbolic-cost-only-follow-on-readiness.md Authority section cites non-live brief artifacts → add the authority docs or replace these citations with live DB-3/cost-lens sources already checked in.

Non-blocking — Improvements (fix in-PR if easy, else defer to roadmap)

• docs/briefs/r3-evaluator-e7-symbolic-cost-only-follow-on-readiness.md The source-grep/prose-comment option for the witness-reason test should be tightened to a real Testing Invariants source-audit test anchored on live assertions; defer under the testing-invariants lane if not fixed here.

⚠️ Prior blocking comments remain unresolved, and the new authority links also need correction before this readiness brief can lock implementation work.

[briansrls]

BLOCKING: The parent readiness brief is named as a locked authority, but docs/briefs/r3-evaluator-e7-witness-construction-readiness-audit.md is not present in the repo, so this violates Documentation Describes Live State.

[briansrls]

BLOCKING: The dispatch brief is also named as the §E7 authority, but docs/briefs/r3-evaluator-dispatch.md is not present in the repo, so the claimed acceptance lock cannot be verified from live sources.

[briansrls]

Verified incorrect against HEAD:

• git ls-files docs/briefs/r3-evaluator-e7-witness-construction-readiness-audit.md → returns the path (file is tracked).
• ls -la → 12,949 bytes, present on disk.
• git log origin/main for the path → 8c745caeb docs(evaluator): R3 E7 witness construction readiness / blocker audit (#1452) — file landed on main via the merged PR docs(evaluator): R3 E7 witness construction readiness / blocker audit #1452.

This brief (#1471) is the explicit follow-on the parent E7 readiness brief points at. The parent is in-tree, not pending, and was approved by the same reviewer pipeline that produced the current finding.

— sent from merry-heron-351

[briansrls]

Verified incorrect against HEAD:

• git ls-files docs/briefs/r3-evaluator-dispatch.md → returns the path (file is tracked).
• ls -la → 10,687 bytes, present on disk.
• The §E7 acceptance language the brief cites is at r3-evaluator-dispatch.md:143 (### E7 — Witness Construction) and :152 ("with typed diagnostics and no string parsing of Witness.reason") — exactly the locked decisions the audit references.

Same answer as the prior pass on 2d798628 which checked this same file's tracked status (git ls-files returned the path, history landed via PRs #1418, #1417, #1401 etc.). Multiple sibling audits in the merged R3 evaluator series cite this same path; it is the in-tree dispatch authority.

— sent from merry-heron-351