Skip to content

feat(evaluator): PR-E E7 symbolic-cost-only — analyze_complexity wrapper - #1484

Merged
briansrls merged 5 commits into
mainfrom
feat/r3-evaluator-e7-analyze-complexity
May 2, 2026
Merged

briansrls merged 5 commits into
mainfrom
feat/r3-evaluator-e7-analyze-complexity

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Summary

First executable PR-E E7 slice per #1471. Adds the public analyze_complexity(dag, workflow_root) -> DimensionReport<SymbolicCost> entrypoint as a thin wrapper over the live analyze_symbolic_cost_dimension. Pre-E5 by construction; no new substrate, no Bool bridge, no new variants.

What lands

  • pub fn analyze_complexity in src/v3/compiler/src/dimension.rs — single-authority wrapper that delegates directly to analyze_symbolic_cost_dimension. No parallel analyzer.
  • 5 focused tests in mod analyze_complexity_tests:
    1. matches_analyze_symbolic_cost_dimension_for_known_workflow — single-authority pin.
    2. returns_ok_for_bounded_int_add_workflow — happy-path DimensionOk shape, no Violates witnesses.
    3. fails_closed_with_typed_diagnostic_on_missing_cost — every violation is Diagnostic::ParseError via typed pattern match.
    4. fail_arm_has_no_composed_field — exhaustive match on DimensionFail asserts the partition (no fabricated composed).
    5. violation_entries_are_typed_diagnostic_enums — every entry is a Diagnostic enum variant; only non-empty checks on human-facing message fields, no string parsing.

Out of scope

  • analyze_with_evaluator / LensRunnerView<C> — gated on E5.
  • analyze_tenant_flow / analyze_ifc — gated on TenantFlow / IfcLabel carriers.
  • New Witness / DimensionReport / Diagnostic variants.
  • String parsing of Witness::Violates.reason.
  • Changes to analyze_symbolic_cost_dimension semantics.
  • Bool bridge, runner / TestPredicate, substrate work.

Test plan

  • RUST_MIN_STACK=33554432 cargo test -p v3-compiler --lib dimension::analyze_complexity_tests — 5 passed locally. (Same RUST_MIN_STACK workaround the existing fail_closed_tests need on this dev environment; CI has the right setup.)
  • CI v3 + ci.

Cross-references

🤖 Generated with Claude Code

Adds the public `analyze_complexity(dag, workflow_root) ->
DimensionReport<SymbolicCost>` entrypoint authorized by #1471. Single-
authority wrapper that delegates to the live
`analyze_symbolic_cost_dimension`; no parallel analyzer. Pre-E5 by
construction: the lens-spine path walks reachable behaviors via
`workflow_reachable_behavior_ids` and consumes `symbolic_cost_of` —
does not depend on `eval_node` / `evaluate_body` Loop coverage.

Five focused tests (per #1471 test plan, all passing locally with
default-test-stack workaround `RUST_MIN_STACK=33554432`, same
constraint the existing `fail_closed_tests` exhibit):

1. matches_analyze_symbolic_cost_dimension — single-authority pin.
2. returns_ok_for_bounded_int_add_workflow — happy path; DimensionOk
   with no Violates witnesses.
3. fails_closed_with_typed_diagnostic_on_missing_cost — every
   violation is `Diagnostic::ParseError` via typed pattern match.
4. fail_arm_has_no_composed_field — exhaustive match on
   DimensionFail asserts the partition (no fabricated composed).
5. violation_entries_are_typed_diagnostic_enums — every entry
   matches a Diagnostic enum variant; only non-empty checks on
   human-facing message fields, no string parsing.

No analyze_with_evaluator, no LensRunnerView, no TenantFlow/IFC
carriers, no new Witness/DimensionReport/Diagnostic variants. No
string parsing of Witness::Violates.reason. analyze_symbolic_cost_dimension
semantics unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Manager review: the implementation is correctly thin over analyze_symbolic_cost_dimension and the tests stay on typed DimensionReport / Diagnostic matching. No analyzer semantics drift or new carrier work spotted.

One blocking API fix before merge: src/v3/compiler/src/dimension.rs is private to the crate (mod dimension;), and lib.rs currently re-exports analyze_symbolic_cost_dimension, behavior_spine_in_node_order, DimensionReport, and Witness but not the new analyze_complexity. Since this PR’s deliverable is the public E7 entrypoint, please add analyze_complexity to the existing pub use dimension::{ ... } list in src/v3/compiler/src/lib.rs. Otherwise the function exists but is not reachable as a public crate API.

— sent from snappy-moth-795

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: 4332220c · Trigger: schedule
  • Comparison: origin/main @ d4eeec26 ... review/pr-1484-4332220c @ 4332220c
  • Thinking: 49s wall

Findings:

  • src/v3/compiler/src/dimension.rs:256 — analyze_complexity is documented as the E7 public entrypoint, but adding pub fn inside private mod dimension does not expose it from the crate API. lib.rs still only re-exports analyze_symbolic_cost_dimension, behavior_spine_in_node_order, DimensionReport, and Witness. This misses the intended boundary/API-level surface for the wrapper; external callers still cannot use the named entrypoint.

Verdict: REQUEST_CHANGES. The implementation is correctly thin and single-authority, but the PR’s named public surface is not actually exported. Re-exporting analyze_complexity from lib.rs should resolve the concern.

CI lint flagged analyze_complexity as unused (no external consumers
in lib). Add to the existing dimension re-export block alongside
analyze_symbolic_cost_dimension.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: b226a8b2 · Trigger: schedule
  • Comparison: origin/main @ d4eeec26 ... review/pr-1484-b226a8b2 @ b226a8b2
  • Thinking: 45s wall

Verdict: APPROVE

The diff is narrowly scoped: analyze_complexity is a true thin wrapper over analyze_symbolic_cost_dimension, and the crate-root export does not introduce a second analyzer or duplicate cost authority. The added tests exercise the fail-closed and typed-diagnostic shape without parsing diagnostic strings. No concrete violations of the pinned invariants, coding guidance, or testing guidance observed.

@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: b226a8b2 · Trigger: schedule
  • Thinking: 219s wall

Non-blocking — Strengths

  • src/v3/compiler/src/dimension.rs analyze_complexity stays an implementation-layer wrapper over analyze_symbolic_cost_dimension, so it does not introduce a parallel complexity authority.

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

  • src/v3/compiler/src/dimension.rs The Pre-E5 rustdoc is stale against this post-image because eval_node now dispatches Behavior::Loop; update the rationale in the PR-E evaluator docs cleanup lane if deferred.

✅ No blocking issues found.

briansrls and others added 3 commits May 2, 2026 00:29
Reviewer noted "Pre-E5" framing in the rustdoc was stale: eval_node
now dispatches Behavior::Loop (E5 landed at lib.rs:251). Wording now
clarifies the wrapper is lens-spine-driven (not body-evaluator-driven),
and stays the single-authority complexity entrypoint until
analyze_with_evaluator lands — independent of E5 status.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: c14f2234 · Trigger: schedule
  • Comparison: origin/main @ df14a24b ... review/pr-1484-c14f2234 @ c14f2234
  • Thinking: 6.6s wall

APPROVE — diff is small and narrowly scoped: a thin analyze_complexity wrapper that delegates to analyze_symbolic_cost_dimension (preserves single authority), plus four typed-pattern-match tests that align with TESTING.md (hermetic unit tests, no string-parsing of diagnostics). No invariant or modeling-discipline violation observed.

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