test(evaluator): E7 analyze_complexity root selection coverage by briansrls · Pull Request #1505 · gunb-ai/gunbc

briansrls · 2026-05-02T12:37:45Z

Follow-up to #1503 for PR-E E7 public API coverage.

Adds an integration test proving analyze_complexity(dag, workflow_root) honors the supplied workflow root on a two-bind program instead of implicitly choosing the only/first bind. The test compares the public wrapper to analyze_symbolic_cost_dimension for each root and asserts the second root reaches a larger witness spine.

Scope: tests only in src/v3/compiler/tests/integration.rs; no analyzer semantics, Witness/DimensionReport shapes, TenantFlow/IFC, E6 fold, substrate, runner, or fixture changes.

…analyze-complexity-integration

…ath claim Reviewer noted the test was named *_fail_diagnostics_are_typed but only exercises DimensionOk. Renamed to preserves_typed_witness_envelope_on_ok and tightened the docstring to match what the test actually proves: the public API preserves the typed Witness<SymbolicCost> envelope on the success arm. Fail-arm typed-diagnostic discipline stays pinned by the in-module analyze_complexity_tests, which can construct ghost-port DAGs via crate-private builders the integration surface can't reach. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…analyze-complexity-integration

…itness content equality Reviewer noted the delegation test compared witness counts only; a wrapper preserving dimension_name + composed + count while changing witness contents would have passed. Strengthens the per-pair assertion: on the Inhabits arm (only arm reachable on well-typed public-API programs), assert SymbolicCost equality (PartialEq is derived). Mismatched arms or both-Violates panic so the test catches real divergence; fail-arm content equality stays in the in-module unit tests where ghost-port DAGs are constructible (Behavior does not derive PartialEq). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…analyze-complexity-integration

… import Reviewer flagged the trailing `let _: fn(&Diagnostic) -> () = |_| {}` as a no-op kept only to silence the unused-import warning, which sits awkwardly with TESTING.md §2 (behavior-driven). Dropped both the stanza and the import; the typed-Witness assertion above already carries the typed-envelope claim. Fail-arm Diagnostic coverage stays in the in-module unit tests where ghost-port DAGs are constructible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…analyze-complexity-integration

…wo-bind fixture Reviewer noted single-bind fixtures left the workflow_root arg non-observable: a wrapper that ignored the supplied root and always picked "the only bind" or "the first bind" would still pass. Adds analyze_complexity_public_api_honors_supplied_workflow_root over a two-bind program ("let a = 1 + 2\nlet b = a + 3 + 4"). For each root, asserts the wrapper agrees with the underlying analyzer; then asserts root `a`'s witness-spine count is strictly less than root `b`'s, since b's reachable-behaviors slice covers a's slice plus the additional adds. A wrapper that ignored the supplied root would return equal counts and fail the strict-less assertion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

briansrls · 2026-05-02T12:39:13Z

Review metadata

Provider / model: cursor / composer-2
Commit: 6cfd22a6 · Trigger: schedule
Comparison: origin/main @ 9cf6dd22 ... review/pr-1505-6cfd22a6 @ 6cfd22a6
Thinking: 34s wall

Findings: None. This diff only adds an integration test in src/v3/compiler/tests/integration.rs: it checks that analyze_complexity matches analyze_symbolic_cost_dimension for each chosen bind root and that witness-spine sizes differ between roots a and b on a two-bind fixture. That is normal test surface; it does not touch substrate modeling, boundaries, or production fail-closed paths. Against TESTING.md, it states the interface (analyze_complexity vs root) and observable behavior (agreement with the lens path and strictly larger spine for b), uses a small compiled fixture in tests/ (allowed by the doc’s practical guidance), and stays in the existing E7 integration module pattern.

Verdict: APPROVE — Narrow, regression-oriented test coverage for supplied workflow_root; nothing in the diff conflicts with the pinned rubric.

briansrls

Review metadata

Provider / model: codex / unknown
Commit: 6cfd22a6 · Trigger: schedule
Thinking: 168s wall

Non-blocking — Strengths

src/v3/compiler/tests/integration.rs The added two-bind fixture makes workflow_root observable and checks the public wrapper against the symbolic-cost analyzer without introducing a second analyzer path.

✅ No blocking concerns; this is focused root-selection coverage over the existing E7 wrapper contract.

briansrls · 2026-05-02T12:55:33Z

Review metadata

Provider / model: openai-pro / gpt-5-5-pro
Commit: 6cfd22a6 · Trigger: manual
Comparison: main @ 9cf6dd22 ... feat/r3-evaluator-e7-analyze-complexity-integration @ 6cfd22a6
Conversation: View conversation

1. Story of the diff

This PR adds one E7 integration regression in src/v3/compiler/tests/integration.rs:629-698. The test builds a two-bind fixture where a and b are distinct workflow roots, then checks that the public analyze_complexity(&dag, selected_root) wrapper delegates to analyze_symbolic_cost_dimension(&dag, selected_root) for each supplied root rather than silently choosing a default/root-by-convention path. The load-bearing mechanism is the second bind: b reaches a strictly larger witness spine than a, so a wrapper that ignores the caller’s workflow_root should be caught by the final witnesses_a.len() < witnesses_b.len() assertion.

2. Invariant categories

LAYER MODEL (substrate vs implementation).

N/A — this is implementation-only Rust test coverage; the diff adds a #[test] at src/v3/compiler/tests/integration.rs:637 and does not touch substrate declarations, Dag storage, cross-pass model carriers, or new variants.

INVARIANTS.md + modeling-discipline.md.

Compliant — facts-flow-forward / single-authority shape is respected in the test setup: the selected root is explicitly threaded into both the public wrapper and the underlying dimension analyzer at src/v3/compiler/tests/integration.rs:648-650, so the test is aimed at preventing root selection from becoming an implicit/defaulted fact.

CODING.md.

Compliant — the test uses explicit free-function inputs/outputs (compile_to_dag, find_bind_root, analyze_complexity, analyze_symbolic_cost_dimension) rather than hidden state or object-style mutation, and it matches the typed DimensionReport::DimensionOk carrier at src/v3/compiler/tests/integration.rs:652-674 instead of probing strings.

TESTING.md.

Finding — NON-BLOCKING, behavior-driven oracle is weaker than the stated claim.

src/v3/compiler/tests/integration.rs:666-668 asserts only:

cw.len(),dw.len(),

But the surrounding claim says “the public wrapper agrees with the underlying analyzer on the same root” at src/v3/compiler/tests/integration.rs:646-647. Equal witness counts are only a proxy: a wrapper could return the wrong witness contents in the right quantity and still pass this part of the regression. Since witnesses is part of the published DimensionReport::DimensionOk carrier being compared, this should ideally assert cw == dw or compare a stable per-witness projection, not just the lengths.

LOCKED DESIGN DECISIONS.

N/A — the diff references E7 in test prose at src/v3/compiler/tests/integration.rs:629, but it does not alter a locked design document, substrate target, target-private realization schema, or external-realization authority.

TRACKED vs UNTRACKED DEBT.

N/A — no new scaffold, TODO, temporary bridge, deprecation marker, or parallel representation is introduced. The explanatory comments at src/v3/compiler/tests/integration.rs:629-636 describe the regression fixture rather than adding debt.

3. Verdict

APPROVE_WITH_COMMENTS

The root-selection regression is well targeted and does make the supplied workflow_root observable. The only issue I see is that one delegation assertion checks witness-spine count rather than witness content, so the test does not fully enforce the agreement claim it documents.

… to witness content Reviewer noted the per-root delegation in honors_supplied_workflow_root checked witness count only; a wrapper returning correct count but wrong contents would pass. Adds per-pair Witness::Inhabits content equality (SymbolicCost has PartialEq) matching the pattern from the main delegation test. Fail-arm equality stays in in-module unit tests where ghost-port DAGs are constructible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

briansrls · 2026-05-02T13:02:37Z

Review metadata

Provider / model: codex / codex-default
Commit: b0e5aced · Trigger: schedule
Comparison: origin/main @ 9cf6dd22 ... review/pr-1505-b0e5aced @ b0e5aced
Thinking: 44s wall

Verdict: APPROVE

No findings. The diff adds focused integration coverage for analyze_complexity honoring the supplied workflow root, and the assertions stay on the public API behavior and typed witness structure. I did not run tests, per the review instructions.

briansrls

Review metadata

Provider / model: codex / unknown
Commit: b0e5aced · Trigger: schedule
Thinking: 100s wall

✅ The PR is test-only E7 root-selection coverage, and I found no blocking or non-blocking concerns in the provided diff.

briansrls and others added 11 commits May 2, 2026 07:09

WIP: merry-heron-351

4d4420b

Merge remote-tracking branch 'origin/main' into feat/r3-evaluator-e7-…

0af999a

…analyze-complexity-integration

chore: apply cargo fmt

1ede3e5

Merge remote-tracking branch 'origin/main' into feat/r3-evaluator-e7-…

7f896e2

…analyze-complexity-integration

Merge remote-tracking branch 'origin/main' into feat/r3-evaluator-e7-…

fec9eee

…analyze-complexity-integration

Merge remote-tracking branch 'origin/main' into feat/r3-evaluator-e7-…

218b9ea

…analyze-complexity-integration

chore: apply cargo fmt

6cfd22a

briansrls changed the title ~~merry-heron-351~~ test(evaluator): PR-E E7 integration — observable workflow_root via two-bind fixture May 2, 2026

briansrls marked this pull request as ready for review May 2, 2026 12:38

briansrls changed the title ~~test(evaluator): PR-E E7 integration — observable workflow_root via two-bind fixture~~ test(evaluator): E7 analyze_complexity root selection coverage May 2, 2026

briansrls mentioned this pull request May 2, 2026

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

Closed

briansrls commented May 2, 2026

View reviewed changes

briansrls merged commit 517c96a into main May 2, 2026
4 checks passed

briansrls commented May 2, 2026

View reviewed changes

Navigation Menu

Repository navigation

test(evaluator): E7 analyze_complexity root selection coverage - #1505

test(evaluator): E7 analyze_complexity root selection coverage#1505
briansrls merged 12 commits into
mainfrom
feat/r3-evaluator-e7-analyze-complexity-integration

briansrls commented May 2, 2026 •

edited

Loading

Uh oh!

briansrls commented May 2, 2026

Uh oh!

briansrls left a comment

Uh oh!

briansrls commented May 2, 2026

Uh oh!

briansrls commented May 2, 2026

Uh oh!

Uh oh!

briansrls left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

briansrls commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

briansrls commented May 2, 2026

Uh oh!

briansrls left a comment

Choose a reason for hiding this comment

Uh oh!

briansrls commented May 2, 2026

1. Story of the diff

2. Invariant categories

3. Verdict

Uh oh!

briansrls commented May 2, 2026

Uh oh!

Uh oh!

briansrls left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

briansrls commented May 2, 2026 •

edited

Loading