Repository navigation
CX-K–N: drive complexity violations 271 → 180 - #318
Conversation
Sessions CX-K through CX-N implement incremental evidence collection, wrapper transparency, and branch-filtering to fix branching-path evidence poisoning. All branching recursion root causes eliminated except skip_until_rbrace (1 violation). Remaining 180 violations are predominantly mutual recursion SCCs (parser, emitters, inference). Key changes: - collect_evidence_incremental: thread descent vars in evaluation order - classify_self_call_evidence: ExprCall wrapper transparency - branching_only mode: filter dominated branches in ExprIf/ExprMatch - construct_branching_termination_proof: TreeSize/ListLength only - Updated test ratchet expectation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3290c0acb0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let dominated = if branching_only { | ||
| max_path_self_calls(body: arm_body(n: arm_node), func_name: func_name) < max_arm_calls | ||
| } else { false } | ||
| if dominated { acc } |
There was a problem hiding this comment.
In branching_only mode this drops any arm whose self-call count is below the maximum, so descent evidence is computed from only the heaviest arms. That makes construct_branching_termination_proof able to return Strict even when another reachable arm is non-descending (for example, one branch does two descending calls while another does a single self(param) call), and classify_recursion_pattern can then mark the function as LinearRecursion. This underestimates worst-case behavior and can hide real complexity violations.
Useful? React with 👍 / 👎.
| pub fn node_type_equals(left: Rc<Node>, right: Rc<Node>) -> bool { | ||
| stacker::maybe_grow(512 * 1024, 2 * 1024 * 1024, || { | ||
| { | ||
| let left_err = if (left.inferred.clone() != None) { | ||
| is_compiler_error(left.inferred.clone().clone().unwrap()) | ||
| { |
There was a problem hiding this comment.
This refactor removed the stacker::maybe_grow wrapper from node_type_equals, but recursion depth is still driven by nested type structure via node_type_equals_core calling back into node_type_equals for children/returns. Deeply nested inputs that previously expanded stack space can now hit a native stack overflow during type inference, turning valid large programs into crashes instead of diagnostics.
Useful? React with 👍 / 👎.
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review (INVARIANTS: 3, MODELING: 2+/1-, ROADMAP: 2✓/1!)
INVARIANTS — Violations (3)
MODELING — Strengths
src/v2/complexity.dagReadingMethodSemanticsto recognize iteration is more compositional than adding another method-name classifier in the analyzer.dsl/std/computation.dagThe explicitCallPattern -> LoweringTargettable is the right abstraction for moving recursion lowering into a std-layer authority.
MODELING — Improvements
dsl/std/computation.dagCallPatternandSizeBoundstill useStringidentity proxies (accessor,param,witness,element) where the roadmap’s structural-identity direction wants references, not names.
ROADMAP — Verified
- CX-A progress: The diff materially advances TokenPosition-based parser proofs by adding shared SCC parser-edge collection and parser regression tests.
- CX-C progress: The diff backs the structural-children iteration progress claim by adding
MethodSemantics-driven iteration handling and a fold-over-children regression test.
ROADMAP — Incomplete
- CX-A SCC parser-proof coverage: The new tests cover single-function parser progress and a negative SCC case, but there is still no positive SCC test exercising the new SCC parser-proof path end-to-end.
The PR moves the CX lane forward and adds good regression coverage, but it still introduces one unconsumed std authority and relies on proof-critical heuristics in complexity.dag.
| _ => false | ||
| } | ||
| // CX-K: a variable previously bound to param.children is also children. | ||
| ExprVar { binding_kind: _ } => set_has(m: vars, key: expr_var_name(texpr: expr)) |
There was a problem hiding this comment.
Invariant violation: The new child-descent proof still hardcodes "children" and list-method cases like skip instead of reading a structural child/iteration authority, violating Heuristics indicate lost structure and No case enumeration for open sets.
| let is_extraction_descent = if is_sub_value_extractor(expr: val) { | ||
| let inner = extractor_inner_arg(expr: val) | ||
| let inner_is_descent = (check_child && is_child_descent_expr(expr: inner, param_name: param_name, vars: vars)) | ||
| || (check_list && is_list_shrink_expr(expr: inner, param_name: param_name)) |
There was a problem hiding this comment.
Invariant violation: Using lambda_param_names |> last to decide which lambda binding is the descended child is a heuristic for missing method-signature facts, violating Heuristics indicate lost structure and Explicit boundary contracts.
|
Violations (could not place on specific lines):
|
ChatGPT ReviewFindings: src/v2/complexity.dag: the new branching_only fallback is unsound. collect_evidence_incremental(..., branching_only: true) drops lighter if/match branches, and classify_recursion_pattern can then upgrade the whole function to LinearRecursion. That breaks the fail-closed invariant: a dominated branch can still do self(param) or otherwise fail to descend, but it is now ignored. Example shape: heavy branch recurses on children twice, light branch recurses on the unchanged param once. This PR would accept the heavy branch as proof and miss the bad light branch. is_match_option_descent is also fail-open. It treats match descent_expr { Some { value: v } => v, None => default } as a descent-producing let-value if any arm returns the bound value, but it never constrains default. So None => param (or any other non-descending fallback) will still mark the binding as a descent var, and a later self(x) gets misclassified as strict descent. Compositional modeling regressed: the new reasoning is mostly hard-coded analyzer knowledge (with_required_cardinality, rt_type, param_node_type_expr, plus specific method names) in src/v2/complexity.dag and the Rust mirror, while dsl/std/computation.dag only changes comments. I would not claim “full model” / “exhaustive lowering table” in INVARIANTS.md and docs/cx-computation-model.md yet; the behavior here is still ad hoc, not model-driven. |
Two root causes prevented SCC termination proofs for emitter functions:
1. expr_contains_descent did not recognize the parameter itself as a
valid descent base. `remaining |> first` where `remaining` is the
parameter was not classified as descent, so `stmt` extracted via
`match remaining |> first { Some { value: stmt } }` was never added
to descent_vars.
2. collect_scc_child_edges only checked arguments whose name matched
the caller's param_name. When emitter functions pass descent
expressions via differently-named parameters (e.g. `texpr: stmt`
when param is `remaining`), the evidence was invisible.
Fix 1: In ExprVar case of expr_contains_descent, check `vname ==
param_name` in addition to descent_vars membership.
Fix 2: In collect_scc_child_edges, check ALL named arguments and take
the best progress (Strict > Same > Unknown).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review (INVARIANTS: 4, MODELING: 2+/2-, ROADMAP: 0✓/0!)
INVARIANTS — Violations (4)
MODELING — Strengths
dsl/std/computation.dagSeparating CallPattern from LoweringTarget composes recursion classification from std.iteration and std.termination instead of baking analyzer-only cases into one monolith.src/v2/complexity.dagRouting parser SCC proofs through collect_parser_edges_for_scc is the right single-authority move for TokenPosition evidence.
MODELING — Improvements
dsl/std/computation.dagSizeBound and CallPattern still identify params, accessors, and witnesses by String, so the lowering table is not yet a structural authority and will keep leaking name-proxy heuristics downstream.src/v2/complexity.dagThe new proof helpers still infer critical facts from coverage counts, raw ProgressKind, and lambda-position conventions; callee-parameter correspondence and branch disjointness need first-class structure instead of heuristic recovery.
ROADMAP — Unclaimed
- CX lane ratchet reduction: The diff clearly advances CX-A/C/D work and adds soundness tests, but ROADMAP.md is not updated to record the claimed 271 → 180 movement or which CX items changed status.
The PR adds real complexity-proof infrastructure, but several new proof paths are still unsound because they replace missing structural facts with heuristic edge selection and branching reclassification.
| match arg_name(n: arg_node) { | ||
| Some { value: _ } => | ||
| let arg_progress = classify_scc_call_progress(arg_expr: arg_value(n: arg_node), param_name: param_name, descent_vars: descent_vars) | ||
| match arg_progress { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| TreeSize { param: _ } => true | ||
| ListLength { param: _ } => true | ||
| _ => false | ||
| } |
There was a problem hiding this comment.
Invariant violation: The new branching_proof_safe fallback reclassifies multi-call recursion as LinearRecursion without any modeled witness that sibling calls are disjoint subproblems, so it fabricates a linear bound and violates Modeling Faithfulness / No fallbacks that fabricate.
|
Violations (could not place on specific lines):
|
ChatGPT ReviewReviewed from the supplied diff only. I would not approve this as-is. src/v2/complexity.dag: the new branching_only fallback is unsound. construct_branching_termination_proof + collect_evidence_incremental(..., branching_only: true) deliberately skip lighter branches. That is okay for estimating the branching factor, but it is not okay for a termination/descent proof. Counterexample: dag path_calls is driven by the then branch, so the else branch gets ignored by the fallback. The new code can still recover a TreeSize proof from the heavy branch and classify this as LinearRecursion, even though there is a non-descending recursive path. That violates the fail-closed story in INVARIANTS.md. I would only use “branching-only” evidence after a full all-branches descent proof already succeeded, never as a recovery path when the full proof failed. src/v2/complexity.dag: is_match_option_descent over-approves descent. The new helper returns true when: the scrutinee “contains descent”, and any variant arm returns its bound value directly. It never checks the other arms. Counterexample: dag On the None path this is a same-argument call, but k now gets promoted to a descent var. The helper needs an “all arms are safe” condition, not an “exists one extracting arm” condition. src/v2/complexity.dag: expr_contains_descent now treats the bare parameter as descent. This line is the problem: dag That makes plain n look like a descent source. It then feeds directly into is_if_option_descent / is_match_option_descent. Counterexample: dag k can be exactly n, but the new logic can classify it as strict descent. I would split this into two helpers: “is the source param-or-known-subvalue?” “is this a strict descent expression?” Right now those concepts got conflated. src/v2/complexity.dag: collect_scc_child_edges became unsound for SCC proofs. Before, SCC progress was checked against the argument whose name matched the chosen parameter. Now it scans all named args and lets any strict arg upgrade the whole edge to ProgressStrict. That is not safe. Progress has to be tracked against the specific callee parameter participating in the ranking dimension. Counterexample: dag The cycle on n is Same, but the new code can mark f -> g as Strict because acc descends. A single scalar edge label is no longer enough once you stop matching the parameter slot. Invariant/model drift: docs moved farther than the implementation. INVARIANTS.md, docs/cx-computation-model.md, and dsl/std/computation.dag now say unchanged-argument self-calls lower to repeat(Forever) and that “no pattern is rejected.” I do not see executable changes in this diff that actually introduce that lowering. The code here still reasons in terms of Same / NonIncreasing / fail-closed behavior. So either: the lowering already exists elsewhere and this PR should point to it, or the docs are now ahead of the implementation. Compositional-modeling concern: most of the new behavior is hard-coded in complexity.dag by function/method name (with_required_cardinality, rt_type, param_node_type_expr, filter, reverse, enumerate) while dsl/std/computation.dag only got comment edits. That weakens the “model-driven” story and will drift again the next time another wrapper/extractor appears. Smaller concern: collect_descent_vars now merges val_inner_vars from inside a let-value back into outer scope. If the language allows shadowing, that can leak arm-local pattern bindings past their lexical scope and fabricate descent vars later. ROADMAP.md is not modified, so there is no roadmap-alignment item to review here. The first four are blocking for me. |
ChatGPT ReviewI found 4 blockers.
The comment says all non-extracting arms must be safe, but That means this is accepted as “descent”:
Before, any So a 1-node parser SCC like
That is not sufficient. A function like: still has overlapping subproblems, but both calls are
The DAG side adds If stage0 is the bootstrap compiler, the new CX-N behavior will not actually run there, so source and bootstrap analyzer behavior diverge.
On compositional modeling: the wrapper/core splits in One invariant/doc note: |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review (INVARIANTS: 7, MODELING: 1+/1-, ROADMAP: 0✓/0!)
INVARIANTS — Violations (7)
MODELING — Strengths
dsl/std/computation.dagMappingSameArgumentCallonto existingRepeatplusForeverreuses the bounded-iteration algebra instead of inventing a fourth primitive.
MODELING — Improvements
src/v2/complexity.dagThe new descent machinery should consumestd/computation.dagandstd/termination.dagas authorities end-to-end instead of adding more analyzer-local name lists for wrappers, extractors, fields, and collection methods.
ROADMAP — Unclaimed
- CX: Complexity Analyzer / Ratchet Counts: This branch clearly advances the CX lane and claims a new violation-count movement in the PR title, but ROADMAP.md still reports the older 164 complexity violations and 314 diagnostics with no CX-K–N update.
The branch fixes one real SCC-argument soundness bug, but most of the new CX movement still comes from added heuristics, scope leaks, and analyzer-shaped source rewrites rather than from new upstream structural facts.
| // Keyed-container detection derives from container_type_set (single authority | ||
| // for container identity) plus structural child count — no parallel name list. | ||
| // | ||
| // Split into wrapper (Optional prefix) + inner (child descent) so the CX |
There was a problem hiding this comment.
Invariant violation: Splitting node_to_name_str specifically so the CX analyzer sees a friendlier SCC shape patches an analyzer gap in a producer instead of fixing the analyzer root cause, violating Root-Cause Depth / Heuristics indicate lost structure.
| let graph = build_call_graph_from_parser_edges(names: members, edges: same_edges) | ||
| graph_has_multi_node_scc(names: members, graph: graph) | ||
| } | ||
| let same_cross_edges = edges |> filter(edge => edge.progress == ProgressSame && edge.caller != edge.callee) |
There was a problem hiding this comment.
Invariant violation: Filtering out ProgressSame self-edges fabricates acyclicity without modeling the witness that discharges those edges, violating No fallbacks that fabricate / Explicit boundary contracts.
| match expr.expr_data { | ||
| ExprCall { call_semantics: _ } => | ||
| let callee = expr_call_func(texpr: expr) | ||
| callee == "rt_type" || callee == "param_node_type_expr" || callee == "field_binding_pattern" |
There was a problem hiding this comment.
Invariant violation: Hardcoding rt_type, param_node_type_expr, and field_binding_pattern as strict sub-value extractors re-derives descent from function names instead of a structural authority, violating Heuristics indicate lost structure / No case enumeration for open sets.
| ExprVar { binding_kind: _ } => | ||
| set_has(m: vars, key: expr_var_name(texpr: expr)) | ||
| // CX-E: param.field or descent_var.field is a sub-value (acyclic construction). | ||
| ExprFieldAccess => |
There was a problem hiding this comment.
Invariant violation: Treating any param.field or descent_var.field as descent assumes every field projection is a smaller recursive witness, which is unmodeled and violates Heuristics indicate lost structure / No fallbacks that fabricate.
| // Requires: at least one arm extracts a variant binding, AND all non-extracting | ||
| // arms are safe (don't return param_name directly — that would pass through | ||
| // the original value without descent). | ||
| fn is_match_option_descent(val: Node, param_name: String, vars: Map<String, Bool>, check_child: Bool, check_list: Bool) -> Bool { |
There was a problem hiding this comment.
Invariant violation: is_match_option_descent treats any match with one extracting variant arm and “safe” other arms as descent, which is a shape heuristic for missing Option/Result facts, violating Heuristics indicate lost structure / Explicit boundary contracts.
| // structures (e.g., match arms inside `let k = match scrutinee { ... }`). | ||
| // Without this, bindings like `kn` from `Some { value: kn }` inside a | ||
| // let-value match are invisible, causing SCC call arguments to be Unknown. | ||
| let val_inner_vars = collect_descent_vars(body: val, param_name: param_name, vars: vars, check_child: check_child, check_list: check_list) |
There was a problem hiding this comment.
Invariant violation: Merging val_inner_vars from a let-value match into combined_vars leaks arm-local pattern bindings into the outer scope, fabricating variable visibility and violating Explicit boundary contracts / No fallbacks that fabricate.
|
|
||
| // Diagnostic: compile only 02_parse.dag and dump parser SCC edge classifications. | ||
| // Avoids OOM from full self-compile while giving ground truth on edge progress. | ||
| #[test] |
There was a problem hiding this comment.
Invariant violation: The new ignored diagnostic tests read the workspace source tree directly from disk in a src/ test module, violating Testing Invariants: Hermetic unit tests only.
…_str emit_pattern refactor: emit_variant_pattern and emit_variant_pattern_rc_aware now take MatchPattern instead of destructured fields, so the SCC has uniform parameter names and collect_scc_child_edges can match edges correctly. node_to_name_str: add let binding before if expression to work around stage0 parser limitation (function bodies must have let bindings before if). Regenerated stage0 and fixed clippy warnings in pipeline.rs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review (INVARIANTS: 4, MODELING: 2+/1-, ROADMAP: 0✓/0!)
INVARIANTS — Violations (4)
MODELING — Strengths
dsl/std/computation.dagLoweringTarget still composes recursion as IterationPrimitive × SizeBound × DescentEvidence, which stays grounded in the std iteration algebra instead of inventing a new executor model.src/v2/complexity.dagconstruct_branching_termination_proof reuses existing TerminationProof and RankingDimension structures rather than introducing a parallel branching-proof type.
MODELING — Improvements
src/v2/complexity.dagThe new wrapper/extractor/field/projection cases should come from a structural descent authority shared with upstream stages, not from more callee-name and field-name ladders inside complexity.
ROADMAP — Unclaimed
- CX: Complexity Analyzer: This PR is explicitly CX milestone work, but ROADMAP.md was not updated to record the new ratchet state or what CX-K–N now covers.
One prior SCC-argument issue is fixed, but the PR still adds new wrong-layer analyzer patches, a duplicate authority, and a fail-open emit fallback, so it is not ready to land.
| else if left_opt && right_opt { | ||
| // Optional unwrap: same tree structure with cardinality stripped. | ||
| // Separate from node_type_equals_core so CX sees SCC edge as Same | ||
| // (wrapper transparency) rather than dragging core's Strict to NonIncreasing. |
There was a problem hiding this comment.
Invariant violation: Splitting optional-cardinality equality into a wrapper/core pair so the analyzer “sees” a Same edge patches analyzer shape in a producer instead of fixing the upstream authority, violating Root-Cause Depth.
| | ParserResultExpect { input: ProgressKind } | ||
| | ParserResultEat { input: ProgressKind } | ||
| | ParserResultCall { input: ProgressKind, callee: String } | ||
| | ParserResultDirectState { progress: ProgressKind } |
There was a problem hiding this comment.
Invariant violation: ParserResultDirectState duplicates the state-progress fact already carried in ParserProgressEnv.state_aliases, violating No duplicate representations.
| } | ||
| _ => false | ||
| } | ||
| } |
There was a problem hiding this comment.
Invariant violation: is_tree_size_preserving_wrapper hardcodes with_required_cardinality by callee name as descent authority instead of reading a structural fact, violating Heuristics indicate lost structure and No case enumeration for open sets.
| } | ||
| _ => "" | ||
| } | ||
| } |
There was a problem hiding this comment.
Invariant violation: emit_variant_pattern and emit_variant_pattern_rc_aware now return an empty string on impossible non-VariantPattern input instead of failing closed, violating No fallbacks that fabricate.
…ndness
Merge origin/main into neat-yak-618, resolving conflicts in 02_parse.dag
(comment wording) and stage0/v2_compiler_complexity.rs (regenerated).
Review fix: is_match_option_descent `all_safe` check auto-approved all
VariantPattern arms (`VariantPattern { ... } => true`), but variant arms
like `None => n` can return the original parameter, making the match
non-descending. Now all arms are checked uniformly — arm body must not
return param_name directly, regardless of pattern kind.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review (INVARIANTS: 2, MODELING: 1+/1-, ROADMAP: 0✓/1!)
INVARIANTS — Violations (2)
MODELING — Strengths
src/v2/complexity.dagcollect_field_binding_namescomposes better with the existingVariantPatternstructure by deriving nested bindings structurally instead of stopping at flatBindcases.
MODELING — Improvements
dsl/std/computation.dagSameArgumentCall -> repeat(Forever)is better grounded in the shared lowering table, but the model is still not fully compositional whilecomplexity.dagkeeps parallel recursion classifiers instead of consuminglower_call_patternas the single authority.
ROADMAP — Incomplete
- CX-K–N: drive complexity violations 271 → 180: The diff adds CX heuristics and diagnostics, but it does not update the ratchet/assertion in the same change to prove the claimed
180ceiling.
One SCC-argument bug is fixed, but the PR still leans on analyzer-shape workarounds and debug-only test scaffolding rather than landing the missing structural authorities.
| type_name: "Callable", params: param_renderings, return_type: ret, | ||
| element: none, key: none, value: none, inner: none, generic_args: [], | ||
| shared: false, boxed: false, is_tuple: false, is_error: false, error_label: "" | ||
| } |
There was a problem hiding this comment.
Invariant violation: Splitting build_type_rendering into build_type_rendering_dispatch specifically so the analyzer avoids a NonIncreasing self-loop patches analyzer shape in emit instead of fixing the upstream authority, violating Root-Cause Depth.
|
|
||
| // Diagnostic: compile only 02_parse.dag and dump parser SCC edge classifications. | ||
| // Avoids OOM from full self-compile while giving ground truth on edge progress. | ||
| #[test] |
There was a problem hiding this comment.
Invariant violation: The new ignored diag_* cases are debug scripts that print internal compiler state instead of asserting observable behavior, violating Testing Invariants: Behavioral only.
ChatGPT ReviewHere are the review findings I’d leave on this PR.
In
In
That is too weak. Arms like
In
A few broader notes:
I’d block on the first three. The fourth is at least a code/comment mismatch, and possibly an incomplete implementation of CX-N. |
Rewrite CX section to reflect actual state: 315→164→76 violations. Catalog all 15 deferred review items in 4 themes (heuristic descent, producer patches, fabrication/scope, boundary/testing). Clarify CX-A through CX-E status and remaining path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review (INVARIANTS: 2, MODELING: 1+/2-, ROADMAP: 1✓/1!)
INVARIANTS — Violations (2)
MODELING — Strengths
dsl/std/computation.dagThe CallPattern -> LoweringTarget model composes recursion analysis through the bounded std iteration primitives instead of scattering ad-hoc lowerings across consumers.
MODELING — Improvements
dsl/std/computation.dagSameArgumentCall should be grounded in an explicit ranking witness or fail closed; Forever is less faithful than the rest of the lowering algebra.src/v2/complexity.dagThe new parser-progress and descent helpers still depend on compiler-local string/function classifiers rather than a single structural authority imported from std.
ROADMAP — Verified
- Review feedback (deferred — 15 items from PR #318): The updated roadmap matches the code by recording still-present heuristic descent sites and the impossible-pattern empty-string fallback as deferred review debt rather than claiming them fixed.
ROADMAP — Incomplete
- CX status: 315 -> 164 -> 76: The diff still bypasses complexity in the main compile path and measures violations through a hand-assembled test pipeline, so the claimed improvement is not yet reflected end-to-end in compilation.
This diff fixes the SCC-argument progress bug, but it still advances CX with fabricated bounds and test-side compensation instead of landing the missing structural authorities upstream.
| LoweringTarget { primitive: Repeat, | ||
| bound: Forever, | ||
| evidence: Strict } | ||
| evidence: NonIncreasing } |
There was a problem hiding this comment.
Invariant violation: Lowering SameArgumentCall to Repeat(Forever) compensates for a missing structural ranking witness with a fabricated machine-max bound, violating Modeling Faithfulness and Root-Cause Depth.
| .map(|(p, c)| (p.as_str(), c.as_str())) | ||
| .collect(); | ||
| let result = crate::helpers::compile_multi(&file_refs); | ||
|
|
There was a problem hiding this comment.
Invariant violation: Rebuilding the front end and calling build_complexity_report directly in the test compensates for compile.dag bypassing complexity instead of fixing the upstream pipeline, violating Root-Cause Depth and No parallel implementations.
ChatGPT ReviewI would not approve this as-is. The violation-count drop looks driven more by analyzer relaxations than by grounded structural facts, and a few of those relaxations look unsound.
In
In
In
In On the invariant / roadmap side, the docs are ahead of the implementation. On compositional modeling quality, I would not count these Low-severity extras: |
…t tables Dissolve 4 hardcoded heuristic classifiers in the complexity analyzer by introducing declarative fact tables in 00_core.dag that the analyzer queries. Phase 1 — NodeFieldRole table: replaces 8 sites of `field == "children" || field == "params"` with `is_children_list_field(field)`, and gates bare `param.field` descent on `is_sub_value_field(field)`. Phase 2 — FunctionSizeEffect table: replaces `is_tree_size_preserving_wrapper` (hardcoded "with_required_cardinality") and `is_sub_value_extractor` (hardcoded 3 function names) with table lookups. Cost-of-change: adding a new structural field or size-affecting function = one line in 00_core.dag, zero changes to the analyzer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…utation model Replace the ad-hoc RecursionPattern (LinearRecursion/DivideAndConquer/ UnresolvableRecursion) with the structural CallPattern → LoweringTarget pipeline from computation.dag. Every recursive function now classifies to a CallPattern, lowers via lower_call_pattern(), and costs from the resulting LoweringTarget's SizeBound. Key changes: - Import types from std.termination (delete 5 local duplicates + merge_evidence) - Import CallPattern/LoweringTarget/lower_call_pattern from std.computation - Add proof_to_call_pattern bridge (TerminationProof → CallPattern) - Add size_bound_param helper to computation.dag (avoids TreeSize name collision) - Rewrite classify_recursion_pattern, classify_scc_recursion_pattern, classify_parser_scc_recursion_pattern → return LoweringTarget - Rewrite bounded_recursive_cost/bounded_scc_cost → consume LoweringTarget - Delete RecursionPattern type (3 variants → 0) - UnresolvableRecursion eliminated: SameArgumentCall → repeat(Forever) No pattern is rejected. Bounded truth principle: SameArgumentCall maps to repeat(Forever) — in a Bit/Word64 system, "always" = 2^63-1. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review (INVARIANTS: 2, MODELING: 2+/2-, ROADMAP: 1✓/2!)
INVARIANTS — Violations (2)
MODELING — Strengths
src/v2/complexity.dagReplacing the local RecursionPattern enum with LoweringTarget composes recursion classification with the shared computation algebra instead of keeping a parallel local model.dsl/std/termination.dagImporting DescentEvidence from std gives the termination lattice a single reusable authority across the analyzer and the docs.
MODELING — Improvements
src/v2/00_core.dagNodeFieldRole and FunctionSizeEffect should dissolve into structural witnesses on the analyzed graph, because string-keyed tables are still proxy modeling rather than faithful composition.dsl/std/computation.dagSameArgumentCall -> Forever only becomes faithful once the downstream cost algebra can carry that bound end-to-end instead of collapsing it back to CostUnknown.
ROADMAP — Verified
- CX migration toward std authorities: src/v2/complexity.dag now imports std.computation and std.termination and replaces the local recursion-pattern enum with LoweringTarget, which matches the roadmap’s stated migration direction.
ROADMAP — Incomplete
- CX "No rejected patterns": bounded_recursive_cost and bounded_scc_cost still turn the Forever case into CostUnknown, so the analyzer does not yet carry an end-to-end finite bound for same-argument recursion.
- CI Gates diagnostic ratchet 316: src/v2/tests/src/pipeline.rs still keeps COMPLEXITY_RATCHET at 315, so the roadmap table's ratchet value is ahead of the code.
The PR improves compositional grounding by reusing std computation and termination concepts, but it still relies on new name-keyed proxy authorities and a fabricated proof-to-call-pattern bridge, so the CX roadmap text is ahead of the implemented model.
| | MetadataField // non-recursive data (name, span, etc.) | ||
|
|
||
| data node_field_roles: Map<String, NodeFieldRole> = { | ||
| "children": ChildrenListField, |
There was a problem hiding this comment.
Invariant violation: node_field_roles and function_size_effects encode descent authority as Map<String, ...> name lookups, so the analyzer still depends on identity proxies instead of structural witnesses, violating Boundary sufficiency / Explicit boundary contracts.
| ) | ||
| } | ||
|
|
||
| // Bridge: extract a CallPattern from a TerminationProof's primary ranking dimension. |
There was a problem hiding this comment.
Invariant violation: proof_to_call_pattern reconstructs ChildAccessorCall, ParserAdvanceCall, and WorklistDrainCall by stuffing the measured parameter into accessor or witness slots, which fabricates a lowering witness downstream instead of carrying it structurally and violates Root-Cause Depth / Explicit boundary contracts.
1 participant
Summary
Test plan
strict_complexity_violation_countpasses at 180🤖 Generated with Claude Code