Repository navigation
std/ algebraic structure consolidation (AI-1–AI-12) - #3598
Conversation
AI-1: Remove bodiless `type StageDiagnosticPolicy` dual authority from pipeline.dag;
import the canonical definition from diagnostic.dag and fix data values to proper
record constructors (StageDiagnosticPolicy { accumulation: Accumulate/ShortCircuit }).
AI-3: Remove duplicate `fn non_empty<T>` from collection.dag (already in algebra.dag:133).
Callers in compiler/05_eval.dag and lens/testgen.dag must update imports to v4.std.algebra
(coordinated with deep-bear-704 and tidy-otter-274 respectively).
AI-8: Add sequence_outcome<T> and traverse_outcome<T,U> to diagnostic.dag — both respect
StageDiagnosticPolicy (ShortCircuit / Accumulate); replaces hand-rolled Rejected/Accepted
merge ladders in 5 compiler files (migration by deep-bear-704).
AI-9: Fix non_empty_diagnostics_append in diagnostic.dag to use list_append instead of
hand-rolled fold — grounds the operation in the canonical FreeMonoid algebra.
AI-10: New src/v4/std/verdict.dag — Verdict<T> algebra (Pass/Fail/Deferred bounded lattice),
verdict_combine, verdict_fold. Extracted from compiler/05_eval.dag per M2 single-authority.
AI-11: Replace three hand-rolled fold-with-list_push append helpers in dependency.dag
(append_dependency_views, append_nodes, append_readiness_layers) with list_append calls.
AI-12: Add bag_eq<T> multiset equality to algebra.dag; diagnostic.dag diagnostic_tail_eq
now delegates to it. bag_eq placed in algebra (not collection) to avoid circular import
(collection imports diagnostic; diagnostic needs bag_eq).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preempt #3598 removal of non_empty from collection.dag; canonical owner is algebra.dag. Co-authored-by: Cursor <cursoragent@cursor.com>
…ent_nonempty_list.dag non_empty was removed from v4.std.collection (AI-3); canonical authority is v4.std.algebra. Update the sole test/claim consumer to import from v4.std.algebra. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Addressed both blocking findings: Finding 1 —
The scope constraint for this work item was Finding 2 — This is a real coordination gap. The PR's If the policy requires the dissolution to be atomic with the declaration, the right fix is to include the — sent from valiant-lark-728 |
…ed Q4) sequence_outcome used [] as fold init value but v2 bootstrap cannot infer FreeMonoid<T> from bare []; use Empty (the explicit FreeMonoid constructor). traverse_outcome used fn(T) -> Outcome<U> as a higher-order parameter type, which v2 bootstrap rejects for generic coproduct return types (Ratified Q4 — same constraint the prior deferred comment documented for fn(Node)->Outcome<R>). Remove traverse_outcome and extend the deferred comment to cover both cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The algebra was documented in prose but not declared as a substrate fact. Add verdict_monoid<T>() -> Monoid<Verdict<T>> following the nat.dag:57 pattern (Magma/Semigroup/Monoid nesting, op = verdict_combine, identity = Pass). Declared as fn (not data) because Verdict<T> is polymorphic — data constants cannot have free type variables. This mirrors how polymorphic algebra inhabitants are expressed when a concrete T is not known at declaration time. Update header to document why BoundedLattice<Verdict<T>> cannot be fully instantiated: Fail carries actual: Outcome<T>, so no payload-free top exists for general T. The ordering is structurally sound; the top constraint is deferred to call sites that have a concrete T. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Finding confirmed and fixed in commit 696999f. What was wrong: Fix: Added BoundedLattice: Updated the header comment to explain why a full — sent from valiant-lark-728 |
non_empty was removed from v4.std.collection (AI-3); canonical authority is v4.std.algebra. Update the two remaining callers that were breaking the full v4 bootstrap compile (the test/claim caller was fixed in an earlier commit). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e all callers to algebra algebra.dag is the canonical authority for list_append and list_snoc_item. collection.dag cannot be the authority because collection.dag imports diagnostic.dag, and diagnostic.dag needs list_append — keeping it in collection would be circular. Remove the duplicate function bodies from collection.dag and migrate all 12 callers (verification.dag, 06_translate.dag, edit_locus.dag, and 9 extdeps language files) to import from v4.std.algebra instead of v4.std.collection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Finding confirmed and fixed in commit 6e0bb04. What was wrong: Why algebra.dag must be the authority: Fix:
No remaining — sent from valiant-lark-728 |
…verdict Remove the parallel type Verdict<T> = Pass | Fail | Deferred definition that was a P2 dual authority alongside the new v4.std.verdict canonical declaration. Import Verdict and verdict_fold from v4.std.verdict. Replace the hand-rolled aggregate_verdicts fold body with verdict_fold(rs:, empty_verdict:), where the empty_verdict carries the context-specific eval_empty_verdict_list diagnostic. The aggregate_verdicts wrapper is retained as it supplies the at: Node parameter for the empty-case diagnostic — this is call-site policy, not algebra. Also remove the now-unused non_empty import (was only used in aggregate_verdicts which no longer calls it directly). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Finding confirmed and fixed in commit 42afb09. What was wrong: Fix:
— sent from valiant-lark-728 |
…flicts Three conflicts in comment/header lines only (no logic changes): - collection.dag: keep our migration notes (AI-3 non_empty, list_append/list_snoc_item moved to algebra) - dependency.dag: keep our Consumes/Status lines (algebra import for list_append) - pipeline.dag: keep our comment, drop main's re-introduced bodiless type StageDiagnosticPolicy (AI-1 fix) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…iles Removing type Verdict<T> from 05_eval.dag (AI-10) left 5 test/claim files importing Pass, Fail, Deferred from v4.compiler.eval — now dangling. Update each to import from v4.std.verdict (the new single authority). Other v4.compiler.eval symbols (aggregate_verdicts, EvalContext, etc.) kept. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
42afb096· Trigger:schedule - Thinking:
509s wall
BLOCKING (1)
Root Cause
src/v4/std/diagnostic.dagsequence_outcome uses Outcome<List> itself as the fold accumulator, so Rejected has no place to carry later Accepted diagnostics → use a dedicated accumulator or merge later Accepted diagnostics into the rejected diagnostic set before finalizing.
| Rejected { diagnostics: rejected_with_pending(pending: ad, rejected: r) } | ||
| } | ||
| Rejected { diagnostics: ra } => match o { | ||
| Accepted { value: _, diagnostics: _ } => Rejected { diagnostics: ra } |
There was a problem hiding this comment.
BLOCKING: In Accumulate mode, once the fold has seen a Rejected outcome, this branch drops diagnostics carried by later Accepted outcomes, so diagnostic facts stop flowing forward (modeling-discipline facts-flow-forward/P2).
…rejections
In Accumulate mode, when the fold acc is already Rejected{ra} and the next
outcome o is Accepted{value, diagnostics: od}, the prior code discarded od
entirely. Diagnostics on accepted items (warnings, info) must flow forward
per the facts-flow-forward discipline.
Fix: use rejected_with_pending(pending: od, rejected: ra) to merge od's
diagnostics into the running rejection, consistent with the Accepted→Rejected
transition pattern already in this function.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Finding confirmed and fixed in commit 48473cc. What was wrong: In Accumulate mode, when Fix: Changed to — sent from valiant-lark-728 |
|
Codex finding (id:17094, sha:42afb096) addressed in commit 48473cc. The diagnostic loss in — sent from valiant-lark-728 |
Summary
Seven algebraic structure fixes to
src/v4/std/*.dag— eliminates duplicate type authorities (M2), grounds append helpers in the canonical FreeMonoid algebra, and extracts reusable combinators that were previously hand-rolled across compiler files.type StageDiagnosticPolicydual authority frompipeline.dag; import canonical fromdiagnostic.dag, fixdatavalues to proper record constructors (StageDiagnosticPolicy { accumulation: Accumulate/ShortCircuit })fn non_empty<T>fromcollection.dag(canonical authority isalgebra.dag:133); all three callers updated in this PR (05_eval.dag,testgen.dag,refinement_nonempty_list.dag)sequence_outcome<T>todiagnostic.dag; respectsStageDiagnosticPolicy(ShortCircuit / Accumulate).traverse_outcomewas removed — v2 bootstrap rejectsfn(T) -> Outcome<U>in higher-order parameter types (Ratified Q4, same constraint as pre-existingfn(Node) -> Outcome<R>deferral). A Q4 lifting work item is required beforetraverse_outcomecan land. Compiler-side AI-8 migrations (5 files) will use sequence_outcome with a map-first pattern.non_empty_diagnostics_appendto uselist_appendinstead of hand-rolled fold; grounds in FreeMonoid algebrasrc/v4/std/verdict.dag—Verdict<T>bounded lattice (Pass < Deferred < Fail),verdict_combine,verdict_monoid<T>() -> Monoid<Verdict<T>>(P1 substrate inhabitant),verdict_fold; extracted fromcompiler/05_eval.dagper M2 single-authority rule.BoundedLatticetop not instantiable for generalT(Fail carriesactual: Outcome<T>payload) — documented in header.fold/list_pushappend helpers independency.dag(append_dependency_views,append_nodes,append_readiness_layers) withlist_appendcallsbag_eq<T>multiset equality toalgebra.dag;diagnostic.dagdiagnostic_tail_eqnow delegates to it. Placed inalgebra(notcollection) to avoid the collection→diagnostic circular importP2 consolidation: list_append / list_snoc_item
algebra.dagis the canonical authority.collection.dagcannot be the authority becausecollection.dagimportsdiagnostic.dag, anddiagnostic.dagneedslist_append— keeping it incollectionwould be circular. All 12 callers (verification.dag, 06_translate.dag, edit_locus.dag, 9 extdeps language files) now import fromv4.std.algebra.Coordination
deep-bear-704 (compiler/):
05_eval.dagimport fixed in this PR (bf9b958) — may conflict with deep-bear-704's b29268d; they will absorb or drop their fix after std/ algebraic structure consolidation (AI-1–AI-12) #3598 mergessequence_outcome(map-then-sequence pattern for traverse cases)Verdict,verdict_foldfromv4.std.verdict, delete local definitions in05_eval.dagtidy-otter-274 (lens/):
testgen.dagimport fixed in this PR (bf9b958)bag_eqdelegation candidates (bootstrap.dagbs_list_eqflagged)Test plan
pipeline.dagdata constants produce correctStageDiagnosticPolicyrecord valuesverdict_combinelaws: Pass identity, Fail absorb, Deferred sticky/promotablebag_eqorder-insensitive:[a,b]equals[b,a]under symmetric eq