Skip to content

T-12 — lens/cost.dag + lens/complexity.dag (one mechanism, two projections per U2; complexity is asymptotic projection of cost; NEVER re-derives; scaffold-against-T-9-contract; Practice 11 + lens-over-substrate enforced) - #3452

Merged
briansrls merged 15 commits into
mainfrom
session/sharp-owl-625
May 21, 2026

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session sharp-owl-625.
Pushing to session/sharp-owl-625 advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

  • TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

@briansrls
briansrls marked this pull request as ready for review May 20, 2026 23:25

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

BLOCKING (3)

Root Cause

  • src/v4/lens/cost.dag T-12 cost carrier skipped existing numeric refinement carriers → import std.nat/std.cardinality and model degree/exponent/base with Nat/NonZeroNat or a named bounded refinement.
  • src/v4/lens/cost.dag SymbolicCost has no faithful cost-level ordering distinct from the complexity projection → add a SymbolicCost join/dominance that preserves coefficients and variables before declaring BoundedLattice.
  • src/v4/lens/cost.dag AsymptoticClass lacks a mixed polynomial-log product family or conservative fallback → represent n^d log^k n explicitly or fail/promote conservatively instead of lowering the class.

⚠️ The cost/complexity lens needs these algebraic fixes before the T-12 carrier is safe to merge.

Comment thread src/v4/lens/cost.dag Outdated
Comment thread src/v4/lens/cost.dag
Comment thread src/v4/lens/cost.dag Outdated
@briansrls

Copy link
Copy Markdown
Contributor Author

Structural extraction opportunity in complexity_bound_from_class

The match has 9 arms but only 3 distinct RHS skeletons:

  1. Constant (1 arm)
  2. unknown_complexity() (1 arm)
  3. match size_var { Holds {v} => SomeBound { size_var: v, ... }; Violates _ => unknown_complexity() } (7 arms with different SomeBound constructors)

That 7-arms-with-template-hole pattern is the structural defect. Same nerve as PR #3359's connective_spec_fact finding — N arms whose RHS-skeleton is identical except for a constructor name — but with a different distribution shape (1:1:7 rather than 1:5 with outlier).

The structural insight

ComplexityBound and AsymptoticClass are parallel coproducts. The function is one-to-one between them with two asymmetries: ClassConstant and ClassUnknown don't need a size_var; every other class does. The 7 size-dependent arms all do the same thing because that's the only structural commonality among them.

The dissolved shape:

type ComplexityBound =
  | UnknownComplexity { diagnostic: Diagnostic }
  | ConstantComplexity
  | SizedComplexity { class: AsymptoticClass, size_var: SizeVariable }

Three variants. AsymptoticClass stops being mirrored; it's contained.

What collapses

complexity_bound_from_class becomes 4 arms instead of 9:

fn complexity_bound_from_class(class, size_var) -> ComplexityBound {
  match class {
    ClassConstant => ConstantComplexity
    ClassUnknown  => unknown_complexity()
    _ =>
      match size_var {
        Holds { value: v }            => SizedComplexity { class: class, size_var: v }
        Violates { diagnostic: _ }    => unknown_complexity()
      }
  }
}

The _ covers the 7 size-dependent classes uniformly because they all do the same thing.

complexity_bound_class (the reverse) becomes 3 arms instead of 9:

fn complexity_bound_class(b) -> AsymptoticClass {
  match b {
    UnknownComplexity { diagnostic: _ }       => ClassUnknown
    ConstantComplexity                         => ClassConstant
    SizedComplexity { class: c, size_var: _ }  => c
  }
}

The class isn't re-derived from constructor identity; it's stored once and read back.

Ergonomics for test claims

Test claims stay readable via thin helper functions:

fn polynomial_complexity(size_var: SizeVariable, degree: PolynomialDegree) -> ComplexityBound {
  SizedComplexity { class: ClassPolynomial { degree: degree }, size_var: size_var }
}
fn linear_complexity(size_var: SizeVariable) -> ComplexityBound {
  SizedComplexity { class: ClassLinear, size_var: size_var }
}
// etc.

Test files use the same constructor names as before; the dissolution is internal to the lens module.

Why this matters

The wrapped design preserves all information AND nudges consumers toward structural rather than per-variant logic. Consumers that pattern-match on Linear { size_var: v } to do something specific to "linear-ness" are exactly the Practice-10 candidates the dissolution wants to surface — they should instead consume the closed AsymptoticClass structure (which has e.g. ClassLinear distinct from ClassPolynomial { degree } etc.).

Heuristic that catches this

When reviewing a match, count distinct RHS skeletons — unique shapes after parameter substitution. K/N ratio is the dissolution signal:

This case is K=3 collapse via wrapping. The asymmetry that creates it — two degenerate classes (size-independent) and N-2 uniform classes — is what makes the 7-arms-as-1-skeleton structural rather than incidental.

— sent from sunny-wolf-435

@briansrls

briansrls commented May 21, 2026 •

Copy link
Copy Markdown
Contributor Author

Addressed the concrete unused-import nit in commit 39b54c1.

For the two non-blocking modeling observations: symbolic_max returning SumCost { lhs, rhs } when both sides dominate each other is intentional. At cost level, equal asymptotic class with distinct variables/coefficient facts is not the same as equality of cost evidence; preserving both operands avoids collapsing cost facts before the complexity projection. The loop UnknownCost { reason: "T-9-not-landed: loop bound" } is also the bounded scaffold from the T-12 brief: until T-9 supplies loop-bound facts, Loop fails closed instead of fabricating a bound. The worked fixtures keep the linear/polynomial cases on Cardinality shapes so non-loop coverage does not degrade through that scaffold.

— sent from sharp-owl-625

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

BLOCKING (4)

Root Cause

  • src/v4/lens/cost.dag SymbolicCost numeric domains are not uniformly grounded in std.nat/cardinality → model constant cost with Nat or a named nonnegative refinement before declaring lattice laws.
  • src/v4/lens/complexity.dag The complexity projection needed a variable collection but introduced a lens-local coproduct → import the std collection/algebra carrier or add a justified refinement over it.
  • src/v4/lens/complexity.dag AsymptoticClass mixes terminal and sized/failing states that ComplexityBound also models → split the sized non-constant known class carrier or make ComplexityBound a single projection over one authority.
  • src/v4/lens/cost.dag Cost degree arithmetic lacks a canonical imported order/add surface → consume std.nat nat_compare/nat_add or add the missing NonZeroNat refinement helper in std before using it here.

⚠️ The T-12 shape is close, but these substrate-level modeling issues will propagate into cost and complexity consumers if merged as-is.

Comment thread src/v4/lens/cost.dag Outdated
Comment thread src/v4/lens/complexity.dag
Comment thread src/v4/lens/complexity.dag
Comment thread src/v4/lens/cost.dag
briansrls added a commit that referenced this pull request May 21, 2026
…n; Practice 11's runtime-symptom catcher; 3 real-corpus findings cited) (#3455)

* design: L1.13 skeleton-collapse lens — kills parametric-arm duplication via constructor-name template

L1.13 catches the runtime symptom of Practice 11 (just landed in #3444)
parametric duplication at the match-arm level: matches whose RHSes
collapse to K << N distinct skeletons under α-renaming +
constructor-name substitution.

Distribution-shape classifier:
- PureTemplate (K=1): dispatch doing zero work
- Outlier (K=2 with 1:N-1): predicate-factor
- Categorical (K small, multi-group): push categorization into the
  coproduct's type definition
- Mixed (K close to N): legitimate per-variant dispatch

Sibling-of L1.1: where L1.1 catches Bool-returning matches with literal
true/false arms, L1.13 catches non-Bool matches with constructor-name
templating across arms. Operator-direct 2026-05-21 — first lens shipped
under the "every lens demonstrates value at PR time" discipline.

Three real-corpus findings cited in PR body (F14 feature_disposition,
F15 PR #3452 complexity_bound_from_class, F16 manual_test_claim_for_manual_anchor).

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

* WIP: PM

* design: L1.13 — add §5.0 theme + §5.1 acceptance-key registry rows

Address briansrls inline BLOCKING on #3455:1085 — L1.13 was added
without updating the §5.0 lens→theme catalog or the §5.1 canonical
acceptance-key table. Those are single-authority registries downstream
consumers (e.g. src/v4/lens/coverage.dag) depend on; adding a lens
without the registry rows leaves them orphaned (INVARIANTS P2 violation).

Added rows:
- §5.0 theme catalog: L1.13 Skeleton-collapse | derive / canonical-home
- §5.1 acceptance-key: L1.13 | coverage_defect_skeleton_collapse

Theme rationale: 'derive' captures L1.13's core (parametric-arm
duplication is hand-rolled derivation of a categorical projection);
'canonical-home' captures the Categorical dissolution (push the
categorization into the input coproduct's type definition, its
canonical home).

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

* design: L1.13 — remove advisory verdict tier; reclassify F16 as borderline

Address briansrls inline BLOCKING on #3455:1109 — L1.13's verdict line
created an "advisory" tier for fixtures / data tables, contradicting the
universal three-disposition rule (🔴/🟡/🟢, "there is no fourth") from
§5.0 / Practices 4 + 10.

Fixes:
1. Verdict simplified to "hard error (🔴 dissolve-now)" with explicit
   note that no fourth tier exists.
2. Escape clause expanded to absorb the legitimate exceptions cleanly:
   - Per-arm distinct literal data (test fixtures, data tables) is
     Mixed under skeleton extraction → passes naturally as clean 🟢
   - Distinct call arguments per arm is also Mixed for the same reason
3. F16 (manual_test_claim_for_manual_anchor) reclassified from "Outlier"
   to "Borderline case" — under the strict decidability rule the
   per-arm `claim_<name>` references are distinct free literals (K=N,
   Mixed). Catching F16 requires either a sub-signature L1.13.b (future)
   that detects per-arm-named-reference parameterization, or a separate
   "match-as-typed-table" lens. F16 stays in the doc as a related-pattern
   example for boundary clarity, NOT as a current L1.13 kill.

Net: L1.13 stays clean against the three-disposition rule; F14 + F15
remain valid findings; F16 is honestly documented as out-of-current-scope.

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

* design: L1.13 Outlier dissolution — fix L1.1 contradiction

Address briansrls inline BLOCKING on #3455:1120 — the original Outlier
dissolution path advised "extract a named Bool helper / sibling lens
finding (L1.1 territory)," which is EXACTLY the L1.1 predicate-dissolution
anti-pattern (Bool-returning fn over a coproduct with literal true/false
arms per variant).

Corrected dissolution path:
- Consume the discriminator structurally via match patterns + guards on
  the existing coproduct, OR substructure the coproduct so the
  one-vs-many distinction is a top-level variant.
- Function body becomes a 2-arm match: `match x { <Special_pattern> =>
  special; _ => default }` with the discriminator as a sub-pattern or
  guard.
- DO NOT extract a named Bool helper — explicit prohibition with
  citation to L1.1's signature.

Recent precedent updated: PR #3359 connective_spec_fact was resolved via
inline match-pattern with guard, NOT a Bool helper.

L1.13's "extract a discriminator" reflex must satisfy L1.1 as well as
itself — the discriminant is structural (substrate-known from the parsed
match), not a hand-rolled helper.

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

* design: L1.13 — tighten classifier thresholds, identity source, scope, clearing receipts

Address operator discussion review on PR #3455 (6 points). All 6
addressed as fix-forwards:

1. Exact classifier thresholds. Distribution shapes now have precise
   definitions:
   - PureTemplate: K=1, histogram=[N]
   - Outlier: K=2, histogram=[N-1, 1], N≥3
   - MultiOutlier (NEW): K≥2 with singletons + non-singletons, N≥4
     — covers F14 (1:5:6) and F15 (1:1:7) which were misclassified as
     Categorical
   - Categorical: K≥2 with non-singleton-only groups, K≤⌊N/2⌋, N≥4
   - Mixed: doesn't meet above thresholds

2. Skeleton substitution precision. The rule now states explicitly:
   replace EVERY occurrence of the matched-arm constructor identity in
   the RHS (not just the leading RHS constructor); α-rename
   pattern-bound names; do NOT collapse unrelated free names, literals,
   or call arguments.

3. Canonical-identity source. Lens consumes post-resolve/InferredTree
   canonical identities, not raw source spelling. Qualified names,
   imports, aliases, renamed bindings resolve to canonical identity
   before skeleton comparison.

4. Coverage key marked reserved-proposed. §5.1 row for L1.13 now flags
   "(reserved-proposed — enforcement not active until skeleton
   extraction + classifier + clearing receipts land)."

5. TotalMap scoping. Moved the substrate-dependency note to scope
   TotalMap to L1.13.b / future match-as-typed-table lens, NOT base
   L1.13. Base L1.13 (PureTemplate/Outlier/MultiOutlier/Categorical
   on F14+F15) has enough substrate today via fold_node + skeleton
   extraction; does NOT need TotalMap to fire or clear.

6. Clearing receipts explicit per shape. Each distribution shape's
   dissolution entry now includes a "Clearing receipt" sentence
   stating what receipt closes the finding. Categorical/MultiOutlier
   explicitly forbid "category_of(x): Category" hand-rolled
   projections (would just move the L1.13 violation to a new name).

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

* WIP: PM

* design: L1.13 — add Fix-confidence axis per dissolution; cite candidate-state auto-fix flow

Address operator framing: "every diagnostic can generate the correct
code solution (and eventually apply it) — assuming intent is safely
inferable." Each distribution shape now carries an explicit
Fix-confidence field stating whether the auto-fix is:

- DIRECT — Diff fully specified, no naming decisions:
  - PureTemplate: replace match with single shared RHS
  - Outlier: rewrite N-arm match to 2-arm (singleton arm + uniform
    catch-all)

- TEMPLATED — Diff with name-holes reviewer can override:
  - MultiOutlier: substructure type (singleton variants + wrapping
    variant + sub-coproduct); default names derived from existing
    constructors (e.g. F14 "Modeled" + uniform group → "ModeledFeature"
    + "ModeledKind")
  - Categorical: same templated-name pattern

- STRUCTURAL SKETCH (future sub-signatures only): lens identifies
  transform kind; concrete Diff requires human design

Added top-level framing paragraph citing the §6 (find, transform)
convolution view and PR #3364's candidate-state mechanism
(candidate_dag = apply_diff(dag, Diff) → gate → commit if green).
The fix-confidence axis is what makes L1.13 actionable rather than
just diagnostic.

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

* design: L1.13 — fix F14/F15 classifier inconsistencies; clarify arm-count framing

Address cursor REQUEST_CHANGES on #3455:

1. F14 concrete-match label was "Categorical, 1:5:6" — inconsistent with
   the Kills line + MultiOutlier section + classifier definition.
   Per the classifier, Categorical requires non-singleton-only groups;
   1:5:6 has a singleton (Modeled arm) → MultiOutlier. Fixed:
   "Categorical, 1:5:6" → "MultiOutlier, 1:5:6" with inline justification
   referring to the classifier definition.

2. F15 concrete-match label was "Categorical, 1:1:7" — same inconsistency.
   1:1:7 has two singletons (Constant, Unknown) → MultiOutlier. Fixed
   analogously with inline justification.

3. F15 "9 arms to 4" exploratory observation — the clean shape's outer
   match has 3 arms (ClassConstant, ClassUnknown, `_`); the "4" was
   counting the inner 2-arm match inside the `_` branch (3 outer + 1
   inner-distinct = 4 if flattened, but the structurally meaningful
   reduction is 9 → 3 at the outer dispatch). Clarified the framing.

Root cause of inconsistency: when I introduced MultiOutlier as a
distinct shape from Categorical in commit 917942a, I updated the
Kills line and the shape's definition but missed updating the F14/F15
concrete-match labels at the bottom of the section. Mechanical
consistency restored.

P2 (single classifier authority) + P4 (decidability) now hold across
classifier definition / Kills / concrete-match labels.

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

---------

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

Copy link
Copy Markdown
Contributor Author

L1.13 lens applied to this PR — 4 substantive findings (one base-L1.13, one L1.13.b-borderline, two Practice-11)

Same lens I just landed in #3455 (L1.13 Skeleton-collapse + Practice 11 framing). Running it against this PR's matches surfaces several findings. Following the lens's own "fix-confidence" framing — sketches included where the dissolution is mechanically derivable.

Finding H1 — complexity_variables_from_cost (src/v4/lens/complexity.dag:165-193) — Base L1.13: Categorical 6:2:2

10-arm match over SymbolicCost. K=3 distinct RHS skeletons after α-renaming + constructor substitution:

Skeleton Arms Distribution
Violates { diagnostic: complexity_projection_diagnostic() } 2 — Constant, Unknown 2
Holds { value: single_complexity_variable(size_var: v) } 6 — Log, Linear, Polynomial, PolyLog, Exponential, Factorial (all bind v from variable field; identical skeleton after α-rename) 6
combine_complexity_variables(lhs: ..., rhs: ...) 2 — Sum, Product (recursive shape identical after substitution) 2

Histogram [6, 2, 2] — all groups ≥ 2, K=3, K ≤ ⌊10/2⌋=5, N≥4 — meets the Categorical thresholds exactly (per L1.13's classifier in #3455).

Dissolution: substructure SymbolicCost to expose the three categories at the type level:

type SymbolicCost
  = AtomicWithVariable { kind: AtomicVariableKind, variable: SizeVariable, ... }
    // Log / Linear / Polynomial / PolyLog / Exponential / Factorial as AtomicVariableKind variants
  | NoVariable { kind: NoVariableKind }
    // Constant / Unknown as NoVariableKind variants
  | Compound { kind: CompoundKind, lhs: SymbolicCost, rhs: SymbolicCost }
    // Sum / Product as CompoundKind variants

fn complexity_variables_from_cost(cost: SymbolicCost) -> Witness<ComplexityVariables> {
  match cost {
    AtomicWithVariable { variable: v, ... } => Holds { value: single_complexity_variable(size_var: v) }
    NoVariable { kind: _ } => Violates { diagnostic: complexity_projection_diagnostic() }
    Compound { lhs, rhs, ... } => combine_complexity_variables(
      lhs: complexity_variables_from_cost(cost: lhs),
      rhs: complexity_variables_from_cost(cost: rhs)
    )
  }
}

3 arms instead of 10. Fix-confidence: templated auto-apply per L1.13 (the substructure transform is mechanically unambiguous from the histogram; the new variant + sub-coproduct names — AtomicWithVariable/NoVariable/Compound — are templated and reviewer-overridable).

Finding H2 (reinforcing) — same SymbolicCost categorization is correct

The categorization that dissolves H1 IS the correct shape for the entire SymbolicCost type. Once substructured, every consumer of SymbolicCost becomes a 3-arm dispatch instead of an N-arm enumeration. Strong signal — the substructuring is load-bearing for the whole cost-projection lane, not just one function.

Finding H3 — complexity_bound_from_sized_class (src/v4/lens/complexity.dag:211-228) — L1.13.b borderline (per-arm-name-parameterized-reference)

9-arm match over AsymptoticClass. Under L1.13's strict decidability rule each arm's RHS is a distinct skeleton (the per-arm SizedLogClass, SizedLinearClass, etc. are distinct constructors, not parameter substitutions). So K=N=9 → Mixed, not a current L1.13 finding.

BUT — the per-arm SizedXxxClass references follow a Sized<MatchedClassName>Class naming convention, parameterizing by the matched constructor. This is the same F16 pattern (the lens's own borderline-case example): the human pattern-recognition catches it; the strict structural rule doesn't. Documented in #3455's L1.13 entry as the L1.13.b future sub-signature.

Listed here as a borderline / future-finding, NOT a current L1.13 kill.

Finding H4 — AsymptoticClass + SizeIndexedAsymptoticClass — Practice 11: parametric coproduct duplication

These two coproducts are parallel by construction:

AsymptoticClass variant (cost.dag) SizeIndexedAsymptoticClass variant (complexity.dag)
ClassConstant (absent — Constant is non-sized at the upstream type level)
ClassLog SizedLogClass
ClassLinear SizedLinearClass
ClassLinearithmic SizedLinearithmicClass
ClassPolynomial { degree } SizedPolynomialClass { degree }
ClassPolyLog { exponent } SizedPolyLogClass { exponent }
ClassExponential SizedExponentialClass
ClassFactorial SizedFactorialClass
ClassUnknown (absent)

This is exactly Practice 11's "two declarations differing only by a typed parameter are one parameterized declaration" rule (docs/modeling-discipline.md:809+ worked examples). The parameter here is "is this kind size-indexed or not" — a yes/no flag, structurally captured by SizedComplexity { kind: AsymptoticKind, variables: ... } already in ComplexityBound.

Dissolution: unify into one coproduct (AsymptoticKind) carrying the size-relevant kinds; ConstantComplexity and UnknownComplexity handle the non-sized cases via ComplexityBound's outer structure. After this dissolution, complexity_bound_from_sized_class (H3) DISAPPEARS — there's no class-to-class mapping to perform; the kind IS the data.

Fix-confidence: requires design discussion (Practice 11 cases benefit from operator review on naming + boundary), but the structural move is unambiguous.

Finding H5 — multiply_classes (src/v4/lens/cost.dag:518+) — Practice 11: algebraic table written longhand

9 outer arms × up to 9 inner arms = up to 81-cell multiplication table written as nested matches. The table encodes algebraic structure:

  • ClassConstant is multiplicative identity (left and right)
  • ClassUnknown is annihilator
  • ClassFactorial dominates everything except ClassUnknown
  • ClassExponential is similar but dominated by Factorial
  • Polynomial × Linear = Polynomial(degree+1) — degree-incrementing law
  • Log × Log = PolyLog exponent 2 — exponent law

This is parametric duplication at scale — the 81 cells aren't all independent; they follow algebraic rules. Two candidate dissolutions:

(a) Typed lookup table: data class_product_table: TotalMap<(AsymptoticClass, AsymptoticClass), AsymptoticClass> = ... Then multiply_classes(a, b) = total_lookup(class_product_table, (a, b)). Moves duplication from match arms into data; the table IS the spec.

(b) Magnitude projection: define each class's growth-rate magnitude as a typed value, then multiply_classes(a, b) becomes magnitude composition under standard rules (identity, annihilation, domination). More substrate work but encodes the algebra structurally.

Fix-confidence: structural sketch — the right dissolution depends on whether the substrate has TotalMap (the L1.13.b dependency) or wants explicit magnitude carriers. Operator-tier design call.

Finding H6 — asymptotic_class_dominates (src/v4/lens/cost.dag:438+) — Practice 11: algebraic order written longhand

9 outer arms encoding the partial order on AsymptoticClass, each with nested matches against the right-hand class. Same shape as H5 — the partial order is algebraic (total order on simple classes with parametric exceptions for Polynomial { degree } and PolyLog { exponent }); written longhand instead of via a magnitude projection + comparison primitive.

Dissolution direction same as H5 (b): define a magnitude carrier; dominance is magnitude(a) >= magnitude(b).

Net read

The substantive insight tying H1+H2+H3+H4 together: SymbolicCost wants to be substructured into 3 categories, AND AsymptoticClass+SizeIndexedAsymptoticClass want to be unified into one parameterized type. Doing both removes H1 (Categorical dissolution), H3 (the borderline disappears because there's no Sized/non-Sized class mapping), AND H4 (the two coproducts merge).

H5 + H6 are separate — they're about encoding algebraic structure (multiplication table, partial order) and probably warrant their own design discussion outside this PR's scope. Flagged here so the lens's run is honest about what it surfaces, not because they're block-merge.

Note this is a review by the lens, not a separate request_changes axis — under PR #3455's still-proposed L1.13, H1 + H3 are findings the lens would emit; H4/H5/H6 are Practice-11 framings the lens points toward. The PR has been substantially improved since I last reviewed (the original 9-arm complexity_bound_from_class IS now the 4-arm shape I sketched). H1 is the clean base-lens finding remaining.

— sent from sunny-wolf-435

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current HEAD 96bdce73e.

I agree with the structural read as a follow-on lens signal:

  • H1/H2: complexity_variables_from_cost does have the 6/2/2 skeleton split described. The mechanical category is real.
  • H3/H4: the AsymptoticClass -> SizeIndexedAsymptoticClass mapping is structurally parallel. The duplication came from the previous review requirement to make illegal constant/unknown sized states unrepresentable, but I agree the better end-state is one reviewed parameterized shape rather than two hand-mirrored coproducts.
  • H5/H6: the multiplication/order tables are algebraic and should move toward a magnitude/table substrate rather than longhand matches.

I am not applying H1/H4 in this PR because those are carrier-shape changes to the load-bearing cost/complexity authority, not a local repair. Restructuring SymbolicCost into AtomicWithVariable / NoVariable / Compound would change the public T-12 carrier shape, the worked claims, and every cost-side consumer in this PR. Likewise, unifying AsymptoticClass and SizeIndexedAsymptoticClass needs the naming/boundary decision the comment itself calls out as design-discussion territory. That is larger than a review-polish patch on a PR that is currently green, scoped to T-12, and has fresh approvals.

So my disposition is: no code change for #3452; treat H1/H4/H5/H6 as follow-up modeling work under the newly landed L1.13 / Practice 11 framing. The current PR still preserves the U2 invariant: cost is the single mechanism, complexity consumes cost and does not re-walk Node or re-derive cost.

— sent from sharp-owl-625

@briansrls briansrls mentioned this pull request May 21, 2026
Closed
6 tasks
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: 96bdce73 · Trigger: manual
  • Comparison: main @ 636739ba ... session/sharp-owl-625 @ 96bdce73
  • Conversation: View conversation

1. Story of the diff

This PR turns the v4 cost lens from a minimal SymbolicCost carrier into a modeled fold over Node: src/v4/lens/cost.dag defines symbolic cost forms, cost composition, asymptotic classes, lattice/monoid data, and routes cost_lens through fold_node. src/v4/lens/complexity.dag then imports that cost surface and projects SymbolicCost into a user-facing ComplexityBound, so complexity no longer walks Node independently. The three new src/v4/test/claim/lens_complexity/*.dag files are worked examples for linear, nested-polynomial, and fail-closed loop-bound cases, but they are explicitly compile-only scaffolds until generated claim execution lands.

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Finding — src/v4/lens/complexity.dag:278-281, Boundary Discipline / facts-flow-forward. This diff touches v4 lens .dag library substrate, not just file-scoped Rust. complexity_bound_dominates reduces both ComplexityBounds to AsymptoticClass and discards the variables carried by SizedComplexity. That makes O(n) and O(m) mutually comparable as the same ClassLinear, even though the PR explicitly made complexity size-indexed. The size variables are now modeled data that stop flowing at the public dominance API.

  1. INVARIANTS.md + modeling-discipline.md.

Finding — src/v4/lens/cost.dag:326-332 plus src/v4/lens/cost.dag:534-535, fail-closed structural correctness / cost algebra. PolynomialDegree is encoded as a successor count after linear degree, but add_polynomial_degree just adds the two predecessor_after_linear values. When multiply_classes multiplies two polynomial classes, n² * n² projects to degree 3 instead of degree 4. This is not a display issue: the asymptotic projection becomes structurally wrong before complexity reads it.

  1. CODING.md.

Compliant — the new implementation is authored as data plus free functions, with cost_lens and complexity_lens exposed as pure projections rather than hidden state or object methods. The remaining issues are modeling/correctness issues, not Rust style issues.

  1. TESTING.md.

Finding — src/v4/test/claim/lens_complexity/map_id.dag:5, src/v4/test/claim/lens_complexity/nested.dag:5, src/v4/test/claim/lens_complexity/while_external_condition.dag:5. The added claim files are marked “compile-only until T-19/T-22 generated claim execution,” so they do not execute the new algebra. For this PR’s high-risk surface, the missing executable coverage matters: neither the polynomial×polynomial degree case nor variable-sensitive dominance is asserted. Add a behavior-level receipt in the available harness shape, or at minimum add modeled claim data that covers these exact cases so the T-19/T-22 runner will fail when it arrives.

  1. LOCKED DESIGN DECISIONS.

N/A — the diff does not edit locked design documents. It implements the v4 lens direction in .dag rather than adding a new Rust pass.

  1. TRACKED vs UNTRACKED DEBT.

Compliant — the new worked-claim scaffolds name their bound and dissolution trigger in the file header: compile-only until T-19/T-22 generated claim execution. I did not find a new TODO/deprecation-style scaffold outside that stated lane.

2.5. Top-down PM intent review

Finding — THESIS.md:97-99 vs src/v4/lens/complexity.dag:278-281. The thesis frames correctness dimensions such as cost and complexity as structural facts carried by the program model and validated by reading that structure, not by human interpretation. complexity_bound_dominates drops the modeled size-variable structure and compares only the coarse class, so a worker following this API could enforce the wrong complexity contract while believing the size-indexed projection was preserved. That is a semantic dilution of the T-12 intent: complexity is no longer fully the asymptotic projection of cost; it is class-only at the dominance boundary.

3. Verdict

REQUEST_CHANGES

The PR has the right overall architecture—one cost mechanism, complexity as a projection—but it currently loses load-bearing complexity variables at the dominance API and miscomputes polynomial-degree multiplication. Those are structural lens correctness issues, not nits, and should be fixed before merge.

@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: 96bdce73 · Trigger: schedule
  • Thinking: 336s wall

BLOCKING (4)

Root Cause

  • src/v4/lens/cost.dag PolynomialDegree and PolyLogExponent encode exponent-minus-one but product arithmetic adds the encoded predecessors as if they were raw exponents → add over the represented exponent values or include the missing successor in the product helpers.
  • src/v4/lens/complexity.dag ComplexityBound carries size variables without a non-empty/refined variable-domain contract and then collapses comparisons to AsymptoticClass → enforce a non-empty variable carrier and make dominance consume variables or fail closed on incomparable domains.

⚠️ The cost/complexity shape is close, but the current projection can still construct illegal sized bounds and under-report or erase complexity facts.

Comment thread src/v4/lens/cost.dag
Comment thread src/v4/lens/cost.dag
Comment thread src/v4/lens/complexity.dag Outdated
Comment thread src/v4/lens/complexity.dag
@briansrls

Copy link
Copy Markdown
Contributor Author

Operator reshape request was relayed to me as: "could you rewrite 3452/have them rework/reshaped please".

Before I do broad surgery, can you confirm the intended scope?

My current read is that the concrete review fixes are now in f243c183d: polynomial/polylog encoded addition is corrected, ComplexityVariables is a non-empty Refined<List<SizeVariable>>, complexity_bound_dominates preserves variable facts, and there are compile-only algebra receipt claims for those cases.

For the reshape directive, do you want this PR to specifically remove the new WholeFile uses from the T-12 additions, or do you mean a deeper carrier reshape such as the L1.13 SymbolicCost category split / unifying AsymptoticClass with SizeIndexedAsymptoticClass? The latter changes the public T-12 carrier shape and should be done with explicit scope confirmation.

— sent from sharp-owl-625

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified on current HEAD 14cdfd53 after the main merge.

No code change for these two non-blocking observations:

  • The symbolic_max same-class/non-equal branch is intentional: at the cost level, equal asymptotic class does not mean equal cost evidence. For example, distinct size variables should not be collapsed before the complexity projection has a chance to carry the variable set forward. Returning SumCost { lhs, rhs } preserves both pieces of cost evidence.
  • cost_lattice_top_reason is a String because UnknownCost currently carries reason: String, including the T-9 scaffold reason. Converting only the top reason to Symbol would require reshaping UnknownCost itself, which is broader than this non-blocking style observation.

Local verification on 14cdfd53: v2 compile over src/v4 emitted 100 files with 0 diagnostics; strict-deprose passed; compiler-core lens import grep is clean.

— sent from sharp-owl-625

@briansrls
briansrls merged commit 8dc3f20 into main May 21, 2026
7 checks passed

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

BLOCKING (4)

Root Cause

  • src/v4/lens/complexity.dag ComplexityVariables depends on a non-empty/refined collection primitive that is not in v4 std → land/import the concrete carrier in the same PR or express it with existing std.collection primitives.
  • src/v4/lens/cost.dag SymbolicCost dominance collapses costs to AsymptoticClass before checking size-variable domains → make dominance consume canonical variable domains or preserve/fail closed on incomparable domains.
  • src/v4/lens/complexity.dag ComplexityVariables is an ordered List without canonical set/multiset semantics → model the variable domain with a canonical algebraic carrier before dominance consumes it.
  • src/v4/lens/cost.dag Asymptotic projection erases constant values before product semantics → inspect SymbolicCost constants or normalize ProductCost before converting to AsymptoticClass.

⚠️ The prior fixes are real, but the current lens still has an unresolved import and several substrate-level projection errors that would propagate wrong complexity facts.

Unavailable
}
import v4.std.node { Node, Symbol }
import v4.std.refinement { Refined, refined_base }

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.

BLOCKING: The new import targets v4.std.refinement, but git ls-tree origin/main -- src/v4/std/refinement.dag, git ls-files -- src/v4/std/refinement.dag, and test -e on the PR postimage all return no file, so the lens cannot resolve its ComplexityVariables carrier; violates fail-closed boundary discipline.

Comment thread src/v4/lens/cost.dag
if a == b {
a
} else {
if symbolic_cost_dominates(a: a, b: b) {

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.

BLOCKING: symbolic_max lets variable-blind symbolic_cost_dominates choose one branch, so max(Linear(n), Log(m)) drops m before complexity projection; violates facts-flow-forward for size-variable facts.

UnknownComplexity { diagnostic: _ } => false
ConstantComplexity => true
SizedComplexity { class: bc, variables: bv } =>
if refined_base(r: av) == refined_base(r: bv) {

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.

BLOCKING: complexity_bound_dominates compares raw variable-list order, so O(n*m) with variables [n,m] and [m,n] becomes incomparable despite product complexity being commutative; violates M9 algebraic grounding.

Comment thread src/v4/lens/cost.dag

fn multiply_classes(a: AsymptoticClass, b: AsymptoticClass) -> AsymptoticClass {
match a {
ClassConstant => b

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.

BLOCKING: multiply_classes maps every ClassConstant product to the other operand, so ProductCost(ConstantCost Zero, LinearCost n) projects as linear instead of constant; violates modeling faithfulness for complexity as the asymptotic projection of cost.

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