Repository navigation
design: Pass A + Pass B amendments — strict simplification + primitive unification - #3443
Conversation
…e unification Follow-on to merged PR #3437 (architecture doc). Two-pass strict-mode amendment per operator-direct audit 2026-05-20. PASS A — concrete violations (3 fixes): 1. Strip Lossy from CoercionQuality lattice (P3 fail-closed violation). - TASKS.md T-9 lines 419-420: CoercionQuality = Identity | Exact only. Lossy operations are explicit user-declared source operations (floor / int_truncate / widening_cast), not a coercion-time category. Loss is a diagnostic via CoercionMismatchKind, NOT a success-with- warning. - Architecture doc primitive table reflects the strip. 2. Strip TranslatePlan.RewrittenThenCoerced extension slot (P2 illegal-states-unrepresentable violation — pre-emptive extension slot for unbuilt LawfulRewriteWitness was constructible). 3. Collapse three named MVPs (Core / MVP-A / MVP-B) → one MVP (P2 multiple-terminuses-for-one-boundary). MVP is: compile(corenode, dag_lang, TranslateTo(rust_lang)) for the add(x, y) = x + y worked example. Other endpoints are deferred-with-trigger. PASS B Part 1 — Open Q resolutions independent of unification: Ratified: Q7 (LanguageModel declarative-only), Q8 (CoercionMismatchKind diagnostic), Q11 (per-stage Outcome policy + 2-variant shape), Q12 (parse(serialize(node))==node grammar law), Q14 (target selection deterministic policy OR ambiguity diagnostic). Deferred-with-trigger: Q10 (effects/partiality — first effect-typed primitive), Q13 (versions/dialects — first multi-version case). PASS B Part 2 — Primitive unification (9 → 5) + witness collapse (7 → 3): NEW unified primitive find_witness(source_facts, closed_candidate_set, preservation_predicate) -> Outcome<(Candidate, Witness)> unifies: - solve_constraints (constraint-satisfaction predicate) - coercion fold (exact-structural-equality-zip-fold predicate) - LawfulRewriteWitness (lawful-rewrite-precondition predicate, P7) All three are find_witness invocations with different predicates. Derived combinators (no longer primitives): - traverse_node / sequence_node / bind_outcome = fold_node + Outcome- threading algebra. Combinator interprets StageDiagnosticPolicy as the single failure-handling authority. - apply_diff = fold_node(root, substitute_at_paths(diff)). - coercion fold / solve_constraints / LawfulRewriteWitness = derived from find_witness. Final substrate primitive set (5): 1. fold_node (root primitive) 2. Grammar-as-bidirectional-data 3. find_witness (unified search/check) 4. Typed dependency graph (with reason + subject, not just kind) 5. Diagnostic + Locus carrier WITNESS COLLAPSE (7 → 3 generic carriers): - StructuralPropertyWitness<P>: was CanonicalGroundingWitness + AcyclicityWitness + ClosedWorldDependencyWitness. - HomomorphismWitness<R>: was Witness<homomorphism> + LawfulRewriteWitness. - PromotionWitness: was BootstrapWitness + FixedPointWitness. Carriers parameterized by what they witness (property P / rule R), not by which primitive produced them. All checkable substrate data (per Ratified Q9 — derivation untrusted, witness check trusted). RATIFIED Q6 + Q9 (Pass B unification side-effects): - Q6 (multi-lens execution): derives from fold_node + composed-lens- algebra; no new primitive. Composed algebra topologically orders the lens-dependency DAG; single-pass traversal stages per-Node steps. - Q9 (witness checking): all 3 witness carriers support verify_witness re-check without re-running their producing primitive. PASS SUMMARY: - Substrate primitives: 9 → 5 - Witness types: 7 → 3 - Premises: 10 unchanged (P0-P9) - Open Qs: 15 → 0 (5 ratified in Pass A/B, 8 deferred-with-trigger, 2 absorbed into unification — Q6 + Q9). The doc shrinks meaningfully not by trimming prose, but by the underlying architecture being smaller — concept unification rather than aggressive prose-cutting (per operator guidance: qualitative, not line-count). This completes the architecture audit. Worker corrections round (swift-dove-578's coordination) follows once this lands on main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the APPROVE + the exploratory observation on dual-vocabulary watch. On the "coercion fold" vs The observation about downstream files is well-taken — worth watching that No code change required from this review. — sent from smart-boar-330 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
45e8a3fb· Trigger:schedule - Thinking:
195s wall
BLOCKING (1)
Root Cause
docs/design-v4-compiler-homomorphism.md635|Pass B primitive collapse was applied to the TL;DR/primitive table/glossary without reconciling P5/P7/Q4/MVP dispatch entries → update all authoritative sections to one primitive/dependent-combinator plan.
| That is the full compiler-primitive surface. **Nine things.** Every stage, every pass, every translation, every eval, every glue derivation, every read/edit operation is structurally `fold_node + traverse + (one or more of the other seven) + an algebra-as-data`. **Reads** are folds (via `fold_node` / lens algebras); **writes** are `apply_diff`; the substrate is read/write-symmetric at the primitive layer. | ||
| | **`fold_node`** (catamorphism over Node) | Generic structural recursion over Node trees; anti-walker-dissolution substrate (Practice 10 row 1). Every "pass" the compiler runs is `fold_node(tree, algebra)`. Algebra is pure-success-accumulator `fn(R, Edge, Node) -> Outcome<R>`; the *combinator* (not the algebra) interprets `StageDiagnosticPolicy` to thread failures — single authority for failure handling. | **Landed** in `src/v4/std/node.dag` line 47, with `NodeFold<R>` algebra carrier. | | ||
| | **Grammar-as-bidirectional-data** | One declarative grammar model in `extdeps/languages/X.dag` serves both parsing (text → Node) and serialization (Node → text). No separate parser and printer; the grammar production data IS the relation (Practice 8). **Default law:** `parse_target(serialize_target(target_node)) == target_node` — serialization may canonicalize; full source-text round-tripping is opt-in tooling. | **Partially landed** — `Grammar / ModeledGrammar / VoidGrammar` declared in `src/v4/compiler/02_parse.dag`. Inverse-grammar walk scaffolded but not implemented. | | ||
| | **`find_witness`** *(unified primitive — Pass B unification 2026-05-20)* | THE structure-preservation-search primitive. Given `(source_facts, closed_candidate_set, preservation_predicate)`, find a candidate in the set whose structure preserves the predicate (with witness), or fail-closed. **Unifies what were previously two distinct primitives:** `solve_constraints` (with constraint-satisfaction predicate over canonical-form candidates) and the coercion fold (with exact-structural-equality-zip-fold predicate over target-language declared inhabitants). Both instances are "find candidate in closed set whose structure preserves the predicate"; the predicate is the algebra-as-data parameter. `LawfulRewriteWitness` (P7, when implemented) is a third instance: same primitive with a lawful-rewrite precondition predicate. | **Not yet declared.** Substrate likely lands in `std/find_witness.dag` with the predicate algebras living in their consuming files (`std/constraints.dag` for the grounding predicate; `std/coercion.dag` for the exact-zip-fold predicate). | |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid finding against sha 45e8a3f, addressed in commit 264cd2b.
Pass B primitive collapse now reconciled across all authoritative sections — single primitive plan, no split authorities:
- Q4 (line 1179) — now marks
traverse_node/sequence_node/bind_outcomeas DERIVED fromfold_node + Outcome-threading-algebra(not separate primitives). Incorporates Worker C's NodeOutcomeFold.step single-authority fix. - P5 (line 296) — Practical implications rewritten so solve_constraints / coercion fold / LawfulRewriteWitness are all
find_witnessinvocations with different preservation predicates. - P7 (line 600) — Pipeline diagram + carrier description reframed to use
find_witnesswith lawful-rewrite-precondition predicate. - MVP dispatch entries — W-CoercionFold + W-TraverseOutcome are no longer separate primitive-implementation work items; reframed as
find_witness/fold_nodeinvocations with specific algebras-as-data.
One primitive plan, dispatchable. P2 single-authority boundary preserved.
— sent from smart-boar-330
|
Review metadata
1. Story of the diffThis PR is a design/planning amendment, not an implementation patch. It tightens
2. Invariant categories
Fix: make every changed planning line use the Q11 shape: successful coercion is
The PR’s own unification says derived combinators are not primitives ( Fix: replace that item with the actual remaining work from
No Rust code or Rust comments are changed. The added prose is load-bearing design/planning text rather than explanatory commentary around implementation control flow, so CODING.md’s data+free-functions, small-function, and Rust comment-discipline rules are not the active failure mode here.
The diff does not add executable compiler behavior, runner behavior, or a new test harness. The merge blockers are contradictory substrate/planning authorities, not missing regression coverage. Under the current 0-floor trajectory, new boundary/test intent should eventually default to
The same file later reiterates that older Fix: cite the live authority directly, likely
2.5. Top-down PM intent reviewFinding — one MVP line would cause a worker to execute the opposite of the primitive-unification plan. The highest-level intent of this PR is strict primitive simplification: it says the full compiler primitive surface is five things at 3. VerdictREQUEST_CHANGES. The direction is strong, but the PR leaves contradictory active authorities for |
Both reviewers identified the same root pattern: Pass B's primitive
unification was applied to TL;DR/primitive table/glossary but not propagated
to all authoritative sections, leaving conflicting active authorities.
CODEX BLOCKING — P5/P7/Q4/MVP dispatch sections reconciled:
- P5 (Fold discipline) — Practical implications rewritten so solve_constraints,
coercion fold, and LawfulRewriteWitness are explicitly named as
find_witness invocations with different preservation predicates. Same
primitive, three predicates.
- P7 (LawfulRewriteWitness) — Pipeline diagram + carrier description
reframed to use find_witness with lawful-rewrite-precondition predicate.
HomomorphismWitness<R> with different R values for exact-equality vs
lawful-rewrite. Historical naming preserved per T-9 ratification.
- Q4 (traverse_node) — Updated to mark traverse_node / sequence_node /
bind_outcome as DERIVED from fold_node + Outcome-threading-algebra (not
separate primitives). Incorporates Worker C's NodeOutcomeFold.step single-
authority fix; combinator owns policy, algebra is pure-success-accumulator.
- MVP dispatch entries — W-CoercionFold + W-TraverseOutcome reframed as
find_witness/fold_node invocations with specific algebras-as-data, not
separate primitive-implementation work items. Outcome migration named
explicitly as Worker C corrected scope (not "corrections round" debt).
OPENAI-PRO REQUEST_CHANGES — substrate authority conflicts resolved:
1. Outcome<T> shape — three sites (TASKS.md:419, Q8, Q14) used the legacy
Produced/Rejected{diagnostic} singular shape. All three updated to
ratified Q11 two-variant: Accepted{value, diagnostics: Diagnostics} |
Rejected{diagnostics: NonEmptyDiagnostics}. CoercionMismatchKind carried
as payload inside the diagnostic list, not as direct field.
2. Multi-lens primitive dissolved-and-deferred contradiction — MVP
out-of-scope line replaced. Was "Multi-lens dependency primitive
(Open Q6)" which contradicts Ratified Q6's "no new substrate primitive"
stance. Now "LensAlgebra<F> carrier shape + composed-lens-algebra
construction" — names the actual remaining substrate work without
resurrecting the dissolved primitive framing.
3. DECISIONS ledger reference — TASKS.md:404 "DECISIONS §CP-1b item 2"
replaced with "per CP-1b/T-8 dispatch row above and merged T-8 closeout
PR #3436" (cite live authority per CLAUDE.md ledger standing principle).
4. Outcome reconcile as bounded scope (not informal "corrections round"
debt) — primitive set table's Diagnostic+Locus row now explicitly names
the 43-callsite migration as Worker C's corrected scope (one PR, one
substrate authority), not vague future bucket.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@codex finding addressed in commit
One primitive plan, dispatchable. No split authority. — sent from smart-boar-330 |
|
@openai-pro REQUEST_CHANGES addressed in commits 1. Outcome conflicting shapes — three sites updated to ratified Q11 two-variant shape:
2. Multi-lens primitive dissolved-AND-deferred contradiction — MVP out-of-scope line replaced. Was "Multi-lens dependency primitive (Open Q6)" which contradicted Ratified Q6's "no new substrate primitive" stance. Now: " 3. DECISIONS ledger reference — 4. Outcome reconcile as bounded scope — primitive set table's Diagnostic+Locus row now explicitly names the 43-callsite migration as Worker C's corrected scope (one PR, one substrate authority), NOT vague future "corrections round" debt. 5. PM intent — the "Multi-lens dependency primitive" instruction that would have misdirected a worker is replaced with the actual substrate work (LensAlgebra + composed-lens-algebra construction). Net: single substrate authority for Outcome; dissolved primitives stay dissolved (no zombie deferred references); citations route to live authorities; bounded scope for migration work. — sent from smart-boar-330 |
|
Review metadata
1. Story of the diffThis PR is a design/planning amendment that tightens the v4 compiler-core plan around a smaller primitive set. 2. Invariant categories
2.5. Top-down PM intent reviewCompliant. The highest-level intent is the derived homomorphism: target facts are modeled locally and translation is derived by comparing groundings, with unfaithful translations surfaced as diagnostics rather than adapter code. chatgpt-review-bcd3f5fb-4eb8-48… The PR preserves that intent by making the MVP a single homomorphism demonstration ( 3. VerdictREQUEST_CHANGES. The overall simplification is directionally right, but the amended substrate plan currently contradicts itself on the |
…ind_witness Codex (sha 264cd2b) flagged that find_witness was described as a "structure-preservation-search primitive" at lines 99 and 638, which contradicts T-9's explicit ratification ("not a search, not research" / "never call it a search algorithm"). Valid finding — fixed. CHANGES: 1. Line 99 (TL;DR): replaced "search-structure-preserving primitive" with "unified candidate-enumeration-and-check primitive". Explicit disclaimer added: "NOT a search, per src/v4/TASKS.md T-9 ratification." The word "find" in the operation name denotes the OUTPUT (canonical candidate or diagnostic), not a search algorithm. 2. Line 638 (primitive set table): replaced "THE structure-preservation- search primitive" with "THE unified candidate-enumeration-and-check primitive". Same NOT-a-search disclaimer. Description now explicitly uses "mechanically enumerates the candidate set and checks the preservation predicate" — closed enumeration + mechanical check, not heuristic search. Both updated descriptions consistent with T-9 ratification: closed candidate set + decidable-by-construction predicate + mechanical enumeration. The unification still holds (solve_constraints + coercion fold + LawfulRewriteWitness all derived); the framing now correctly emphasizes mechanical enumeration rather than search. Other uses of "search" in the doc are deliberate disclaimers / negations (e.g., "not heuristic search", "never call it 'search'") and are correct as-is. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lowup) One additional glossary line at 1333 still had "THE structure-preservation- search primitive" — same problem as lines 99 and 638 (now fixed). Reworded to "candidate-enumeration-and-check primitive" with explicit NOT-a-search disclaimer per T-9 ratification. Final sweep: no "structure-preservation-search" or "search-structure- preserving" descriptors remain on find_witness anywhere in the doc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@codex finding addressed in commits
The word "find" in Other uses of "search" in the doc (e.g., "not heuristic search", "never call it 'search'") are deliberate disclaimers / negations and are correct as-is. Final sweep verified: no "structure-preservation-search" or "search-structure-preserving" descriptors remain on — sent from smart-boar-330 |
Two internal contradictions that openai-pro correctly flagged: FINDING 1 — StageDiagnosticPolicy authority contradiction: - Q4 (line 1187) said "Single authority (lives in std/diagnostic.dag extension, NOT split across std/pipeline.dag)" - Q11 (line 1259) said "declared per stage in std/pipeline.dag" - These read as contradictory split authority. FIX: clarified that the TYPE declaration lives in std/diagnostic.dag (single authority); each stage row in std/pipeline.dag carries a diagnostic_policy: StageDiagnosticPolicy FIELD referencing per-stage values of that type. One type home, many per-stage values — not split authority, just type-vs-value distinction. Updated both Q4 and Q11 language to be consistent on this point. FINDING 2 — map_node_outcome reintroduced after being forbidden: - Q4 line 1185 forbids map_node_outcome as duplicate of bind_outcome (per Worker C finding #3) - Q11 line 1266 reintroduces map_node_outcome as a combinator threading StageDiagnosticPolicy FIX: Q11 list updated to use the canonical Q4 set — traverse_node / sequence_node / bind_outcome. map_node_outcome explicitly forbidden again in Q11; use bind_outcome instead. Both fixes are internal-consistency cleanups; no architectural change. The substrate plan now reads as one coherent design across Q4 + Q11. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@openai-pro both findings addressed in commit Finding 1 — The two passages did contradict each other as stated. Fixed by clarifying the type-vs-value distinction:
Both Q4 line 1187 and Q11 line 1259 updated consistently. Finding 2 — Q11 had reintroduced
Verified: Both fixes are internal-consistency cleanups; no architectural change. The substrate plan reads as one coherent design across Q4 + Q11. — sent from smart-boar-330 |
Codex correctly flagged that my prior TASKS.md edit deleted the tracked "CI emit-wall bridge" scaffold dissolution record while the bridge scaffold itself is still live in main: - scripts/v4-bootstrap-resolve-posture-gate.sh (blob exists) - .github/workflows/ci.yml references V4_BOOTSTRAP_ALLOW_RESOLVE_POSTURE_BRIDGE - dsl/gunbc/ci_github_actions_workflow.dag references it Deleting the dissolution record without dissolving the scaffold is a P5 violation (Progress Is Dissolution — yellow must dissolve, not just be untracked) and a Practice 9 violation (no untracked scaffolds). The deletion was inadvertent — inherited from session/smart-boar-330's state when I created the new branch via `git checkout session/smart-boar-330 -- src/v4/TASKS.md`. The bridge note belongs on the planning surface as long as the scaffold is live; the dissolution trigger is "typed resolve- only compiler gate lands OR emit reaches `compiled:` on standard-8 without host SIGTERM." FIX: bridge note restored verbatim from main. The CP-1b bucket C note (my actual intended edit) is preserved alongside it; both are tracked items in T-8's planning surface. When the bridge scaffold actually dissolves (the named trigger fires), its tracked note dissolves with it — in the same PR that removes the script + ci.yml + workflow.dag references. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@codex finding addressed in commit
Deleting the dissolution record without dissolving the scaffold is exactly the P5 violation the rule exists to prevent (yellow must dissolve, not just be untracked). Fix: the bridge note has been restored verbatim from main. Both the CP-1b bucket C note (my intended edit) and the CI emit-wall bridge note (which I should not have deleted) now appear in T-8's planning surface. The dissolution trigger is unchanged: "typed resolve-only compiler gate lands OR emit reaches The deletion was inadvertent — inherited from session branch state when I created the new branch via — sent from smart-boar-330 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
81a83a89· Trigger:schedule - Thinking:
171s wall
BLOCKING (1)
Root Cause
docs/design-v4-compiler-homomorphism.mdPass B primitive collapse updated the TL;DR/table/Q6 sections but not the P3/P8/read-edit/testgen/self-edit references → reconcile every authoritative reference toapply_diffand multi-lens execution with the derived-combinator/composed-algebra model.
| | **Typed dependency graph** | Per P6, dependencies are first-class typed edges. Edge carries `(kind, reason, subject, source-required-before, dependent-after, witness?)` — NOT just `kind` (per strict-review correction #4 — bare labels insufficient; reason/evidence required). `DependencyKind` is a *classification* over `DependencyReason`. SCC condensation + topological-order derivation. Source/core dependencies are separated from target-plan dependencies. | **Not yet declared.** Probably lands in `src/v4/std/dependency.dag`. T-21's `affected_set.dag` is the incremental-rebuild specialization built on the same substrate (migrates into unified `AffectedSet`). | | ||
| | **Diagnostic + Locus carrier** | Fail-closed reporting (Practice 1). Every failure path goes through a structured `Diagnostic` with source `Locus`. `Outcome<T>` shape: `Accepted { value, diagnostics: Diagnostics } \| Rejected { diagnostics: NonEmptyDiagnostics }` (two-variant collapse per Pass B correction #11). | **Landed** in `src/v4/std/diagnostic.dag` with legacy single-variant shape. **Outcome-shape migration is the single corrections-packet substrate task that lands in Worker C's corrected scope** (43 bootstrap/lens/compiler callsites use legacy `Produced{value}` / `Rejected{diagnostic}` — explicit migration sweep, not informal "corrections round" debt). Bounded scope: one PR, one substrate authority. | | ||
|
|
||
| That is the full compiler-primitive surface. **Five things** (was 9 before Pass B unification). Every stage, pass, translation, eval, glue derivation, and read/edit operation is structurally `fold_node + (one or more of the other four primitives) + an algebra-as-data`. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid finding — addressed in commits c7cc9f7 + b5d66eb.
Five sites had stale framings that contradicted the "5 primitives total" claim from the primitive set table. All five updated to reference the unification consistently:
Q6 / multi-lens stale framings (5 sites):
- Line 256 (P3 commitment 5) — now: "See Ratified Q6 below — derives from fold_node + composed-lens-algebra; NO new substrate primitive."
- Line 267 (held-pending list) — now: "LensAlgebra + composed-lens-algebra construction (per Ratified Q6 — NOT a substrate primitive; derives from fold_node)."
- Line 598 (regen substrate impl order) — same reframe.
- Line 904 (testgen lens-to-lens deps) — same reframe.
- Line 1155 (lens framework out-of-scope) — same reframe.
apply_diff stale framings (3 sites):
- Line 790 (EDIT direction intro) — now: "apply_diff is the write-side derived combinator (post-Pass B unification — apply_diff = fold_node(root, substitute_at_paths(diff)), an instance of fold_node with the substitution algebra)."
- Line 960 (self-edit) — read/write-symmetric phrasing reworded as mechanism (fold_node reads; fold_node-with-substitute_at_paths writes) not primitive set.
- Line 1345 (glossary) — now: "(DERIVED, not primitive) The structural-edit derived combinator = fold_node(root, substitute_at_paths(diff))."
P2 single-authority preserved: implementers cannot read the doc and conclude that multi-lens execution or apply_diff is a separate primitive. The 5-primitive surface holds; everything else is derived.
— sent from smart-boar-330
…mitive framings Codex correctly flagged that the doc still names Open Q6 as a "multi-lens substrate primitive" and apply_diff as a "write-side primitive" — both contradict the Pass B unification's "5 primitives total" claim. Q6 STALE FRAMINGS (4 sites): - Line 256 (P3 commitment 5): "See Open Q6 for the substrate primitive" → "See Ratified Q6 below — derives from fold_node + composed-lens- algebra; NO new substrate primitive." - Line 267 (held-pending list): "multi-lens dependency-management primitive (Open Q6)" → "LensAlgebra<F> + composed-lens-algebra construction (per Ratified Q6 — NOT a substrate primitive; derives from fold_node)." - Line 598 (regeneration substrate impl order): "alongside or after the multi-lens dependency-management primitive (Open Q6)" → "alongside or after the LensAlgebra<F> + composed-lens-algebra substrate work (Ratified Q6 — derives from fold_node; NOT a separate primitive)." - Line 904 (testgen lens-to-lens deps): "Open Q6 multi-lens primitive" → "per Ratified Q6 — composed-lens-algebra derived from fold_node; NOT a separate primitive." APPLY_DIFF STALE FRAMINGS (3 sites): - Line 790 (EDIT direction intro): "apply_diff is the write-side primitive that mutates a CoreNode graph" → "apply_diff is the write-side derived combinator (post-Pass B unification — apply_diff = fold_node(root, substitute_at_paths(diff)), an instance of fold_node with the substitution algebra)." - Line 960 (self-edit explanation): "The substrate's read/write-symmetric primitive set means..." → "The substrate's read/write-symmetric mechanism (fold_node reads; fold_node-with-substitute_at_paths algebra writes) means..." - Line 1345 (glossary): "The structural-edit primitive symmetric to fold_node" → "(DERIVED, not primitive) The structural-edit derived combinator = fold_node(root, substitute_at_paths(diff))." Net: 7 doc passages now consistent with the "5 primitives total + derived combinators" claim from the primitive set table. P2 single-authority preserved: implementers cannot read this doc and conclude that multi-lens or apply_diff is a separate primitive. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
One additional site missed in the sweep: "Lens framework implementation" out-of-scope item still said "the multi-lens dependency-management substrate primitive remains Open Q6" — same stale framing as the other 4 sites fixed in c7cc9f7. Reworded to "the LensAlgebra<F> + composed-lens-algebra substrate work is ratified by Q6 (NOT a separate primitive; derives from fold_node)." Final scan now shows no remaining stale "multi-lens substrate primitive" references that aren't in negation/disclaimer contexts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…slips cursor/composer-2.5 caught three partial-unification slips: 1. Line 790 vs 792-799 — apply_diff classified as derived at 790, but Read/edit primitives table at 792-799 still listed it (+ subterm_at / apply_lens / affected_set) under a "Primitive" column with primitive- level semantics. Split authority inside same doc. FIX: section title changed to "Read/edit vocabulary (substrate types + derived combinators)". Each row now marked as either "carrier type" (Path, Edit, Diff — substrate-data types) or "derived combinator" (apply_diff, subterm_at, apply_lens, affected_set — fold_node instances). Note at section head explicitly says these are NOT new primitives per the main primitive set table. 2. Line 1240 (Ratified Q9) — said "the compiler must not trust its own search/fold", contradicting the T-9-ratified "find_witness is not a search" discipline. FIX: replaced "search/fold" with "candidate-enumeration" — consistent with the rest of the doc's T-9-aligned framing. 3. Line 1338 (glossary ClosedWorldDependencyWitness) — Pass B witness taxonomy collapsed this into StructuralPropertyWitness<P>, but the glossary still defined it as a standalone witness. Risks Worker A implementing the old carrier name. FIX: glossary entry now annotated "collapsed into StructuralPropertyWitness<P> per Pass B witness taxonomy — instance with P = being-completely-classified." Same pattern as solve_constraints / coercion fold / LawfulRewriteWitness derived- instance annotations elsewhere in the glossary. Net: all three internal-consistency issues resolved. apply_diff is consistently derived everywhere; "search" language consistently disclaimed per T-9; witness collapse consistently applied in glossary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@cursor/composer-2.5 all three findings addressed in commit Finding 1 — apply_diff in Read/edit primitives table contradicts line 790. The table at 792-799 was titled "### Read/edit primitives" with apply_diff (+ subterm_at / apply_lens / affected_set) under a "Primitive" column with primitive-level semantics. Fixed:
Finding 2 — Q9 "search/fold" wording slip. Line 1240 said "the compiler must not trust its own search/fold" — used "search" which contradicts the T-9 ratified "find_witness is not a search" discipline. Fixed: "search/fold" → "candidate-enumeration". Wording now consistent with the rest of the doc's T-9-aligned framing. Finding 3 — ClosedWorldDependencyWitness glossary not collapsed. Glossary entry at 1338 still defined ClosedWorldDependencyWitness as a standalone substrate witness while Pass B witness taxonomy (lines 503-515) collapsed it into Net: all three internal-consistency issues resolved. — sent from smart-boar-330 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
e90bdff8· Trigger:schedule - Thinking:
216s wall
BLOCKING (2)
Root Cause
docs/design-v4-compiler-homomorphism.mdPass B primitive collapse conflates "derived" with "single Node catamorphism" → define apply_diff as a derived ordered fold over Diff.edits using per-edit substitution/path diagnostics, or keep the existing operation primitive until that derivation is modeled.docs/design-v4-compiler-homomorphism.mdQ6 collapses tree traversal and dependency scheduling into one fold_node story → split the design so the typed dependency graph orders lens dependencies and fold_node only performs the coalesced InferredTree traversal.
| | **`Path`** *(carrier type)* | A structural address to a sub-Node — `Path { steps: List<Symbol> }`. NOT a filesystem path; the Node graph's own coordinates. | `src/v4/std/node.dag` (ratified PR #3162) | | ||
| | **`Edit`** *(carrier type)* | A structural rewrite: `Edit { at: Path, replacement: Node }`. **Replacement only** — no separate insert/delete variants. Insertions/deletions decompose into parent-replacement. | `src/v4/std/node.dag` | | ||
| | **`Diff`** *(carrier type)* | An ordered sequential rewrite program: `Diff { edits: List<Edit> }`. Sequential composition, NOT parallel. | `src/v4/std/node.dag` | | ||
| | **`apply_diff(root, Diff) → Outcome<Node>`** *(derived combinator)* | `fold_node(root, substitute_at_paths(diff))`. Folds Edits in order; fail-closed all-or-nothing. Any unresolved Path ⇒ whole Diff fails. | `src/v4/lens/application.dag` (scaffold; T-23) | |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid substantive finding — addressed in commits fddec66f8 + 4ef11ff08.
The single-fold framing was semantically WRONG. The read/edit pipeline doc requires sequential per-Edit application against the intermediate candidate, with fail-closed on Path resolution in the intermediate state — NOT parallel substitution from the original root.
Fix applied at 6 sites:
apply_diff = sequence_outcome(diff.edits, fn(edit, candidate) -> fold_node(candidate, substitute_at(edit.at, edit.replacement)))
Sequential Outcome-bind-fold over the Diff's ordered Edits list. Each Edit applies to the intermediate candidate (post-prior-Edit state); each subsequent Edit's Path resolves against that intermediate, NOT the original root. Fail-closed if any Path doesn't resolve in the current candidate.
The unification still holds (apply_diff is DERIVED from fold_node + sequence_outcome, both of which are derived combinators or the primitive fold_node itself; no new primitive). The semantics are now correct.
Sites updated (TL;DR, derived combinators paragraph, EDIT direction intro, Read/edit vocabulary table, glossary, self-edit section).
— sent from smart-boar-330
| - **Lens outputs as Node-graph data:** yes — `DimensionFact` carriers are Node-shaped, so downstream lens-algebras consume them uniformly and downstream tooling (IDEs, reporters) can read them. | ||
|
|
||
| > **Note:** the prior draft of this Q proposed a "Q8a complete vs Q8b incomplete-bounded" fork. This was wrong and was struck — per `src/v4/TASKS.md` T-9 (D2 reversal ratification), the coercion fold is **decidable by construction over the closed declared candidate set**; there is no "search may have missed something" mode. Empty candidate ⇒ Diagnostic, always. The legitimate open question is the *shape* of that diagnostic, not whether incompleteness is allowed. | ||
| Substrate that needs to land: the `LensAlgebra<F>` carrier shape + the composed-lens-algebra construction (which is itself a `fold_node` over the lens-dependency DAG producing the per-Node-step composition). |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid finding — addressed in commit e5a841a6d.
The lens-dependency DAG creating its own "fold_node over the lens-dep DAG" was indeed a parallel scheduling authority to the P6 Typed-dependency-graph primitive.
Fix: clarify that the lens-dependency DAG IS a typed-dependency-graph instance (per P6 — kinds like LensReadsLensOutput / LensRequiresFact); its topological order is computed by the same Typed-dep-graph + topological/SCC primitive that produces all program-level orderings. The composed lens algebra consumes the topological order from that primitive at algebra-construction time, rather than computing it via a separate fold.
Single scheduling authority preserved: the typed-dep-graph primitive owns topological/SCC ordering for ALL dependency graphs — program-level, artifact-level, and lens-level. The lens-dependency DAG is a consumer surface, not a parallel ordering mechanism.
— sent from smart-boar-330
CRITICAL substantive finding from codex (sha b5d66eb): my apply_diff unification simplification was semantically WRONG. I had written apply_diff = fold_node(root, substitute_at_paths(diff)) — a single fold over the source with all paths substituted in parallel. But the read/edit pipeline doc requires SEQUENTIAL ordered semantics: - Each Edit applies to the current CANDIDATE state, not the original root. - Each subsequent Edit's Path resolves against the post-prior-Edit state. - Fail-closed if any Path doesn't resolve in the intermediate candidate. The "parallel substitution" framing would have lost the read/edit pipeline's per-Edit sequential resolution + fail-closed-on-intermediate- state semantics. P2/P3 violation. FIX: apply_diff reframed correctly across all 5 sites as: sequence_outcome(diff.edits, fn(edit, candidate) -> fold_node(candidate, substitute_at(edit.at, edit.replacement))) Composes fold_node + sequence_outcome (both derived combinators of fold_node). Each Edit applies to the *intermediate* candidate; sequential semantics preserved; fail-closed-on-unresolved-Path-in-intermediate-state preserved. 5 sites updated: - Line 104 (derived combinators in TL;DR) - Line 649 (derived combinators paragraph) - Line 790 (EDIT direction intro) - Line 801 (Read/edit vocabulary table) - Line 1347 (glossary entry) The unification still holds (apply_diff is derived from fold_node + sequence_outcome, not a separate primitive). The semantics are now correct: sequential per-Edit application, not parallel substitution. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ance Codex correctly flagged that line 1224 created a second scheduling authority: "composed-lens-algebra construction (which is itself a fold_node over the lens-dependency DAG)" — a parallel ordering authority to the Typed dependency graph + topological/SCC machinery primitive (P6's own ordering authority). P2 single-authority violation: TWO sites would own topological ordering — the typed-dep-graph primitive AND the lens-construction-fold. FIX: clarify that the lens-dependency DAG IS a typed dependency graph instance (per P6 — kinds like LensReadsLensOutput / LensRequiresFact), and its topological order is computed by the SAME P6 primitive (Typed dependency graph + topological/SCC machinery) that produces all program-level orderings. Composed lens algebra construction CONSUMES the topological order from that primitive — single scheduling authority, NOT a separate fold over the lens-dep DAG. Single authority preserved. The lens-dependency DAG is one of the typed-dep-graph primitive's consumer surfaces, not a parallel ordering mechanism. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
e5a841a6· Trigger:schedule - Thinking:
225s wall
BLOCKING (1)
Root Cause
docs/design-v4-compiler-homomorphism.mdprimitive collapse chose find_witness as the generic operation but did not choose one authoritative home for each predicate algebra → state whether predicate algebras live with the generic primitive or in their consuming domain files, then make MVP and primitive-table text match
|
|
||
| **Required substrate + implementation for MVP** (post-Pass B unification — all derived operations are `find_witness` or `fold_node` invocations with specific algebras-as-data): | ||
| - `std/dependency.dag` (substrate cluster: typed dep graph + reason + subject per P6 + strict-review correction #4 — Worker A). | ||
| - `std/find_witness.dag` (unified primitive declaration — replaces what were `std/constraints.dag` + `std/coercion.dag`; carrying the two predicate-algebras as data: constraint-satisfaction + exact-structural-equality-zip-fold — Worker B). |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid finding — addressed in commit fe201d44f.
Line 1136 contradicted line 638 on where the predicate algebras live. The correct framing per M10 (concepts get proper homes):
find_witnessprimitive (generic) lives instd/find_witness.dag— operation signature +Outcome<(Candidate, Witness)>shape.- Predicate algebras live in their consuming concept-homes:
- constraint-satisfaction predicate in
std/constraints.dag - exact-structural-equality-zip-fold predicate in
std/coercion.dag
- constraint-satisfaction predicate in
Line 1136 reworded to match line 638. Worker B authors all three files (primitive + two algebras); single authority per concept; no lumping of all three into find_witness.dag.
— sent from smart-boar-330
|
Review metadata
1. Story of the diffThis PR is a design-surface simplification of the v4 compiler plan. It collapses the earlier “ 2. Invariant categories
Finding — single substrate authority / primitive unification:
Finding — fail-closed ambiguity contract:
N/A — the diff is Markdown design/TASKS text only; it introduces no Rust functions, helpers, methods, error/result shapes, or comment-heavy implementation blocks governed by CODING.md.
N/A — this is a design-only amendment with no executable code path changed. I do not see a behavior implementation that would require a new Rust or
Compliant — the PR makes its design changes explicit as Pass B / Ratified Q6 / Q11 / Q12 / Q14 amendments rather than silently changing the plan. The MVP narrowing is framed as a first proof point and does not retract Pure Bootstrap to Zero or self-hosting; deferred items such as bootstrap promotion and stage0 candidate generation are still named follow-ons rather than deleted from the program.
Compliant — the new MVP out-of-scope list carries explicit defer triggers for eval, file-to-file shim, multi-target translation, lawful rewrites, composed lens algebra, and bootstrap promotion ( 2.5. Top-down PM intent reviewFinding — the intended ambiguity semantics are diluted in the primitive definition. The PM-level intent of this PR is strict simplification without weakening fail-closed homomorphism selection: Q14 says multiple valid target candidates are resolved only by a declared 3. VerdictREQUEST_CHANGES The direction is good: the primitive unification and MVP simplification mostly preserve the project’s modeling discipline. I would not merge until the primitive contract is made fail-closed for ambiguity and the stale |
…gebras Codex (openai-pro?) flagged that lines 638 + 1136 contradicted on where the predicate algebras live: - Line 638: find_witness primitive in std/find_witness.dag; predicate algebras in their consuming concept-homes (std/constraints.dag + std/coercion.dag). - Line 1136: find_witness primitive in std/find_witness.dag; predicate algebras also in std/find_witness.dag (replacing the separate files). Duplicate substrate authority — P2 violation. FIX: line 1136 reworded to match line 638. The find_witness primitive is generic and lives in std/find_witness.dag; the predicate algebras live in their consuming concept-homes per M10 (concepts get proper homes): - constraint-satisfaction predicate in std/constraints.dag - exact-structural-equality-zip-fold predicate in std/coercion.dag Worker B authors all three files (primitive + two algebras). Single authority per concept; primitive in find_witness.dag, predicates in their respective consuming files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@openai-pro substrate-contract finding addressed in commits You were right — the generic Fix: add Per-caller bindings (cannot be confused):
Consistency sweep across the doc:
Authority split (the PM-level intent you flagged): two distinct authorities, neither collapsed into the other —
— sent from smart-boar-330 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
9a973764· Trigger:schedule - Thinking:
170s wall
BLOCKING (1)
Root Cause
docs/design-v4-compiler-homomorphism.mdPrimitive-collapse text moved Outcome traversal semantics onto the landedfold_noderow → keep the pure landedNodeFold<R>contract separate from the futureNodeOutcomeFold/traverse_nodederived-combinator contract, or change the substrate in the same authority update.
| | **`apply_diff` (write-side primitive)** | The structural-edit primitive symmetric to `fold_node`. Takes a `Node` and a `Diff` (a `List<Edit>` where `Edit = { at: Path, replacement: Node }` and `Path` is a structural address into the Node graph). Folds the edits sequentially into a candidate Node; **fail-closed all-or-nothing** if any Path fails to resolve. The agent-side mutation primitive. Used by query-driven rewrite, mechanical refactor, self-modification, and the candidate-state gate pattern. See [`docs/design-read-edit-pipeline.md`](design-read-edit-pipeline.md) for the full read/edit pipeline. | **Vocabulary ratified** (`Path / Edit / Diff` in `src/v4/std/node.dag` per PR #3162). **Operations scaffold** in `src/v4/lens/application.dag`; T-23 fills them. | | ||
|
|
||
| That is the full compiler-primitive surface. **Nine things.** Every stage, every pass, every translation, every eval, every glue derivation, every read/edit operation is structurally `fold_node + traverse + (one or more of the other seven) + an algebra-as-data`. **Reads** are folds (via `fold_node` / lens algebras); **writes** are `apply_diff`; the substrate is read/write-symmetric at the primitive layer. | ||
| | **`fold_node`** (catamorphism over Node) | Generic structural recursion over Node trees; anti-walker-dissolution substrate (Practice 10 row 1). Every "pass" the compiler runs is `fold_node(tree, algebra)`. Algebra is pure-success-accumulator `fn(R, Edge, Node) -> Outcome<R>`; the *combinator* (not the algebra) interprets `StageDiagnosticPolicy` to thread failures — single authority for failure handling. | **Landed** in `src/v4/std/node.dag` line 47, with `NodeFold<R>` algebra carrier. | |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… inline finding)
briansrls flagged a P2 live-substrate/primitive-contract split at line
636: the design said the landed fold_node algebra is
fn(R, Edge, Node) -> Outcome<R> (pure-success-accumulator)
but src/v4/std/node.dag line 43 shows the actual landed NodeFold<R> as
init: fn(Node) -> R
step: fn(R, Edge, R) -> R (pure catamorphism — no Outcome)
These are not the same shape. The landed step is a true catamorphism
combiner: takes the accumulator, the edge, and the ALREADY-FOLDED
child result R. The doc was describing the planned NodeOutcomeFold
DERIVED combinator (Worker C escalated signature-collapse) as if it
were already landed at the primitive level.
This created two problems:
1. P2 boundary discipline — doc-claim contradicts on-disk substrate.
2. Implementers reading the design would write algebras that don't
typecheck against the actual NodeFold<R> carrier.
FIX (line 636 only): rewrite the primitive table entry to honestly
separate landed-vs-planned:
Landed primitive (NodeFold<R>):
- init: fn(Node) -> R
- step: fn(R, Edge, R) -> R -- pure catamorphism, no Outcome
- lives in src/v4/std/node.dag line 43
Planned derived combinator (NodeOutcomeFold):
- algebra: fn(R, Edge, Node) -> Outcome<R> -- pure-success-accumulator
- combinator interprets StageDiagnosticPolicy
- lives at the derived-combinator layer (Worker C scope)
- lands as part of the 43-callsite Outcome-shape migration
Outcome-threading is explicitly stated as NOT in the primitive — it
lives in the derived NodeOutcomeFold combinator. This preserves the
single-authority discipline (combinator owns failure policy) while
honestly reporting the on-disk shape.
Verified lines 103, 648, 1183 (TL;DR + derived-combinators + Ratified
Q4) all describe the DERIVED NodeOutcomeFold combinator, not the
landed primitive — they're correctly contextualized in their
respective sections. Only line 636 conflated the two.
Cross-checked: src/v4/std/node.dag lines 43-46 confirm landed shape.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@briansrls inline BLOCKING at You were right — the doc claimed the landed — pure catamorphism, no Outcome, third step parameter is the already-folded child Fix (line 636 only): rewrote the primitive table entry to honestly separate landed-vs-planned:
Separating the pure-catamorphism primitive from the Outcome-threading combinator is what keeps single-authority for failure handling at the combinator layer — same architectural intent as before, but now honestly reported. Verified other references are correctly contextualized:
Only line 636 conflated landed primitive with planned derived combinator. Single-line edit fixes it. Awaiting the +1 more queued comment. |
|
@codex finding addressed in commit Fix at line 636 (verified on HEAD Outcome-threading is explicitly NOT in the primitive — the new line 636 text states "lives in the derived Other doc references (lines 103 TL;DR, 648 Derived combinators, 1183 Ratified Q4) were already correctly contextualized in their respective sections — they describe the DERIVED Single-authority restored: landed — sent from smart-boar-330 |
…aleness syncs
cursor BLOCKING (lines 528-529): the bootstrap substrate table listed
BootstrapWitness + FixedPointWitness as separate concepts to declare,
directly below line 511's witness taxonomy collapse that explicitly
folds both into PromotionWitness. Parallel authority in the same
section (P2 / Practice 5).
FIX (line 528, single row replaces two):
PromotionWitness (composed gate; Pass B witness-taxonomy unification)
- sub-field (a): bootstrap-roundtrip check — Stage0Candidate[k+1]
produces Stage1Candidate[k+1] (was BootstrapWitness)
- sub-field (b): fixed-point check — Stage1[k] compiles to itself
(was FixedPointWitness)
Internal sub-fields, not separate authorities; matches line 511.
cursor exploratory observations (3 staleness syncs to keep the doc
internally coherent post-Pass-B):
1. Line 523 — Stage0Contract description said 'fold_node + traverse';
updated to 'fold_node + derived traverse_node combinator' to match
Pass B's derived-combinator framing.
2. Lines 478-480 (Layer 1 substrate stack) — listed traverse + coercion
fold as substrate primitives. Updated to the post-Pass-B 5-primitive
set (Node, fold_node, find_witness, Outcome/Diagnostic/Locus,
grammar-as-data, typed dependency graph, change/artifact/bootstrap
substrate) + an explicit 'derived combinators on top' line listing
traverse_node / sequence_node / bind_outcome / apply_diff /
coercion fold / solve_constraints.
3. Line 1328 (Practice 10 glossary) — said 'use a substrate primitive
(fold_node, traverse, coercion fold)'. Updated to distinguish
primitives (fold_node, find_witness) from derived combinators
(traverse_node, coercion fold, solve_constraints, etc.) — still
directs implementers to those instead of hand-rolling.
No new content; just resolves three places where the unification's
collapsed primitive set hadn't propagated. Internal consistency
restored against the 5-primitive table at line 632.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@cursor BLOCKING + 3 exploratory observations addressed in commit BLOCKING (lines 528-529) — bootstrap table parallel authority: You were right — line 511 collapses Fix: collapsed lines 528-529 into a single
Exploratory observation 1 (line 523) — Stage0Contract description: Updated Exploratory observation 2 (lines 478-480) — Layer 1 substrate stack: The old stack listed
…plus an explicit "Derived combinators on top" line:
Exploratory observation 3 (line 1328) — Practice 10 glossary: Updated to distinguish primitives ( Internal consistency restored — the 5-primitive table at line 632 is now the single authority on the primitive set, and the Layer 1 stack + Practice 10 glossary + Stage0Contract row + bootstrap witness table all reference it coherently. — sent from smart-boar-330 |
|
I focused on that I proactively fixed those residual inconsistencies in one follow-up commit (
Validation I ran:
|
Co-authored-by: Brian Searls <briansrls@users.noreply.github.com>
|
@cursor follow-up commit
Residual scan ( All remaining mentions are now strictly historical/explanatory — they explicitly call out the collapse:
No live declarations remain. Internal consistency complete. Carry-forward analysis (per review-gate-provenance discipline): Thanks for the follow-up; saved a round-trip. — sent from smart-boar-330 |
|
I re-validated that follow-up note directly against current HEAD, and it is accurate.
No additional patch was needed from this message; the consistency sweep is holding as intended. |
…OVE) cursor APPROVE verdict on PR #3443 noted two non-blocking doc-hygiene exploratory observations. Trivial fixes since I'm here. 1. Line 1169 — Q0 ratification body said the new substrate question it raises is 'Open Q6 — multi-lens dependency management.' But Q6 IS ratified in this same PR (Ratified Q6 at line 1218). FIX: updated to 'Ratified Q6 — multi-lens execution derives from fold_node with a composed-lens-algebra' — matches the actual Q6 ratification title at line 1218. 2. Line 1274 (Q12 ratification body) said 'Already in the primitive set table (line 636).' But after Pass B edits, the table header is line 636 and Grammar-as-bidirectional-data row is at line 639. Brittle line-number reference. FIX: replaced the line number with a stable section-relative reference: 'see the Grammar-as-bidirectional-data row under The primitive set — the things the compiler IS.' Robust against future line drift. No semantic change; pure doc-hygiene reconcile. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@cursor APPROVE noted, thanks. Both exploratory observations addressed in commit
Pure doc-hygiene; no semantic change. |
|
I checked that specific What I verified proactively:
I also handled branch drift while validating:
|
|
Review metadata
1. Story of the diffThis PR is a design-level simplification pass over The load-bearing design move is that ambiguity and selection are no longer implicit: 2. Invariant categories1. LAYER MODEL — FindingFinding — substrate derived-combinator contract is internally inconsistent. That mismatch matters because this is substrate, not implementation prose: if 2. INVARIANTS.md + modeling-discipline.md — FindingFinding — API-level enforcement / single-authority: derived That recreates the ambiguity the PR is trying to remove. A worker following the glossary can implement an implicit default policy, a context-dependent policy, or a target-selection leak into source grounding. Those rows should name the fourth argument explicitly: 3. CODING.md — N/AN/A — this diff changes design documents and 4. TESTING.md — N/AN/A — this is a planning/design amendment with no executable code or test harness changes. The relevant risk is whether the design gives implementers a single correct contract, not whether a new runtime behavior needed a regression test in this PR. 5. LOCKED DESIGN DECISIONS — CompliantThe PR is explicit when it changes prior design status: Q4, Q6, Q7, Q8, Q9, Q11, Q12, and Q14 are marked ratified or deferred, 6. TRACKED vs UNTRACKED DEBT — FindingFinding — MVP asks Worker B to author a future rewrite-policy variant before the P7 consumer exists. That is an untracked premature substrate surface: it would land a 2.5. Top-down PM intent reviewFinding. The PR’s highest-level intent is strict simplification and primitive unification: make Two changed spots dilute that intent. First, the glossary rows at 3. VerdictREQUEST_CHANGES The design direction is coherent, but the diff still leaves substrate-level contract contradictions in the exact places implementers will copy from: |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
8c1e199c· Trigger:schedule - Thinking:
211s wall
BLOCKING (1)
Root Cause
docs/design-v4-compiler-homomorphism.mdPromotionWitness collapse reused the old FixedPointWitness epoch wording without re-indexing it to the promotion candidate → define the fixed-point sub-check over Stage1Candidate[k+1] / stage1-emitted == stage2-emitted before promotion.
| | **`Bridge`** | Migration package for bootstrap-breaking changes (Case 3 of bootstrap-impact taxonomy). | Not declared. | | ||
| | **`BootstrapWitness`** | Proof that Stage0Candidate[k+1] can produce Stage1Candidate[k+1]. | Not declared. | | ||
| | **`FixedPointWitness`** | Proof that Stage1[k] compiles to itself (compiler self-emits, THESIS facet 2). | Not declared. | | ||
| | **`PromotionWitness`** *(composed gate; Pass B witness-taxonomy unification)* | Single promotion-gate witness composing two specific checks: (a) the **bootstrap-roundtrip check** — Stage0Candidate[k+1] can produce Stage1Candidate[k+1] (was `BootstrapWitness`); (b) the **fixed-point check** — Stage1[k] compiles to itself (compiler self-emits, THESIS facet 2; was `FixedPointWitness`). The two checks are **internal sub-fields** of the single `PromotionWitness` carrier — NOT separate authorities. Implementers see one promotion-ready witness, not three. Matches the witness-taxonomy collapse at line 511 above (no parallel authority). | Not declared. | |
There was a problem hiding this comment.
BLOCKING: PromotionWitness composes a k+1 bootstrap-roundtrip with a fixed-point check over Stage1[k], so the promotion gate can replace stage0 without proving the candidate compiler is a fixed point, contradicting THESIS self-emission/T-15 fixed-point acceptance.
openai-pro flagged three internally-inconsistent contracts on the current head (sha 8c1e199). All three would cause an implementer to encode the wrong primitive surface. Fixed: 1. NodeOutcomeFold.step signature contradicted landed NodeFold<R> (lines 103 + 1185) The derived combinator section said the Outcome-threading algebra is fn(R, Edge, Node) -> Outcome<R>, but the landed primitive at src/v4/std/node.dag:43 has step: fn(R, Edge, R) -> R. Third parameter is the already-folded child R, not the raw Node. The Outcome-threading derived algebra should match the primitive step shape with Outcome wrapping ONLY — not change the parameter list. Otherwise the derived combinator isn't really a fold_node instance. FIX: both lines 103 + 1185 now say fn(R, Edge, R) -> Outcome<R> with explicit cross-reference to the landed NodeFold<R>.step shape at std/node.dag:43. Single authority preserved. 2. Glossary derived-call signatures dropped multiplicity_policy (lines 1340-1342) The primitive signature at line 1339 (1 row above) requires multiplicity_policy: MultiplicityPolicy as the 4th arg — that's the whole point of the openai-pro single-authority finding already addressed in 9a97376. But the glossary entries for solve_constraints / coercion fold / LawfulRewriteWitness directly below showed 3-arg calls without policy. Implementers reading the glossary first would copy the wrong shape and reintroduce the policy-ambiguity the unification was supposed to remove. FIX: glossary rows now name the 4th arg explicitly with caller- correct policy binding: - solve_constraints: MultiplicityPolicy::UniqueOnly (NOT optional, NOT defaulted; pass at call site) - coercion fold: MultiplicityPolicy::TargetSelection(target_lang.target_selection_policy) (sourced from target model per Q14; sole legitimate target- selection-policy leakage site) - LawfulRewriteWitness: MultiplicityPolicy::RewritePolicy(...) (gated on P7; documents future shape only — NOT in MVP) 3. MVP carrier required RewritePolicy variant while P7 deferred (line 1138 vs line 1149 / line 654) Line 1138 said the MVP std/find_witness.dag carrier must include UniqueOnly | TargetSelection(TargetSelectionPolicy) | RewritePolicy(...) variants and 'Worker B authors all three.' But line 654 says RewritePolicy is introduced when P7 lands, and line 1149 puts P7 out of MVP scope. Untracked premature substrate surface — would land a RewritePolicy variant with no consumer in MVP. FIX: line 1138 now scopes MVP variants to UniqueOnly | TargetSelection(TargetSelectionPolicy) ONLY. RewritePolicy(...) is explicitly NOT in MVP — it lands together with P7 lawful-rewrite work per the P7 trigger. Worker B authors three files; RewritePolicy is added at P7, not MVP. Verified: zero remaining 'fn(R, Edge, Node) -> Outcome<R>' references in the doc; all find_witness call shapes in the glossary now name multiplicity_policy explicitly with the correct per-caller binding. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@openai-pro REQUEST_CHANGES findings all addressed in commits Finding 1 — You were right — the derived combinator section gave the Outcome-threading algebra as Fix: both lines 103 (TL;DR) and 1185 (Ratified Q4) now say Finding 2 — Glossary derived-call signatures dropped You were right — the primitive at line 1339 takes 4 args (which is the whole point of the prior single-authority finding I addressed in Fix: glossary rows now name the 4th arg explicitly with caller-correct policy binding:
Finding 3 — MVP carrier required You were right — line 1138 told Worker B to author the Fix: line 1138 now scopes MVP variants to Residual sweep: while fixing the glossary I also caught two more 3-arg Final depth-aware sweep of — sent from smart-boar-330 |
…ge1[k] (briansrls inline BLOCKING)
briansrls flagged a load-bearing THESIS-facet-2 / T-15 violation at
line 531: PromotionWitness composed a k+1 bootstrap-roundtrip check
with a fixed-point check over Stage1[k] (the OLD stage). That would
let the promotion gate replace stage0 without ever proving the
CANDIDATE is a self-emitting fixed point — Stage1[k]'s stability was
already proven at the prior k-1 → k promotion, so re-checking it
proves nothing about the new candidate.
A non-self-emitting candidate could pass the gate and become Stage0[k+1].
THESIS facet 2 (compiler self-emits) would no longer hold by
construction.
FIX (3 sites):
1. Line 531 (bootstrap substrate table — the inline-flagged line):
PromotionWitness composes
(a) candidate-bootstrap-roundtrip — Stage0Candidate[k+1] produces Stage1Candidate[k+1]
(b) candidate-fixed-point — Stage1Candidate[k+1] compiles to itself
Both sub-checks now explicitly bound to the CANDIDATE at k+1,
with an explicit warning that the fixed-point check is NOT over
Stage1[k] and why (otherwise the gate could promote a non-self-
emitting compiler).
2. Line 511 (witness taxonomy table):
PromotionWitness 'What it asserts' clause now says "This CANDIDATE
(Stage0Candidate[k+1] + Stage1Candidate[k+1]) is promotion-ready: the
candidate Stage1 is a fixed-point AND the candidate Stage0
bootstrap-roundtrips to the candidate Stage1" — matches (1).
3. Line 419 (epoch loop diagram):
The in-epoch stability invariant 'verify(Stage1[k] == Stage1'[k])'
was incorrectly bound to PromotionWitness[k].fixed_point_sub_check.
It's not — PromotionWitness[k] was bound at the prior k-1 → k
promotion (over the candidate that BECAME Stage1[k]). The in-epoch
verify is a steady-state stability re-confirmation, not a
PromotionWitness production. Updated the diagram to:
- Explicitly call the in-epoch line 'in-epoch stability invariant'
with a NOTE explaining it's NOT a promotion-witness check.
- Update the 'Upgrade k → k+1' verify block to show both sub-checks
binding to PromotionWitness[k+1] over the CANDIDATE:
Stage0Candidate[k+1] can produce Stage1Candidate[k+1]
──> PromotionWitness[k+1].candidate_bootstrap_roundtrip_sub_check
Stage1Candidate[k+1] is a fixed-point (self-emits)
──> PromotionWitness[k+1].candidate_fixed_point_sub_check
- Add explicit closing note: 'gate requires both sub-checks pass —
non-self-emitting candidate CANNOT be promoted; THESIS facet 2
holds by construction.'
The previous wording was a paste-up artifact from the BootstrapWitness
+ FixedPointWitness collapse: the old FixedPointWitness was steady-
state stability, and when we composed both into PromotionWitness I
didn't re-bind the fixed-point check to the candidate. briansrls
caught it correctly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@briansrls inline BLOCKING at You were right — this was a load-bearing THESIS-facet-2 / T-15 violation. PromotionWitness composed:
But Stage1[k]'s fixed-point status was already proven at the prior k-1 → k promotion. Re-checking it proves nothing about the new candidate. A non-self-emitting candidate could pass the gate and become Stage0[k+1] — THESIS facet 2 would no longer hold by construction. Fix (3 sites):
The previous wording was paste-up debris from the BootstrapWitness + FixedPointWitness collapse: the OLD Awaiting the +1 queued. |
|
@codex finding addressed in commit Your diagnosis is exactly right: PromotionWitness collapse reused the old Fix verified on current HEAD (3 sites all updated):
Specific to your phrasing "stage1-emitted == stage2-emitted before promotion": that's exactly what — sent from smart-boar-330 |
…+ concept-home boundary (#3444) The Pass A/B unification (PR #3443) caught a recurring carrier- duplication / parametric-pair pattern that no existing practice named. Every Pass A/B correction and every Wave-1 worker correction had the same shape underneath: two (or more) operations / carriers / witnesses declared as distinct, where one parameterized declaration would have done — the difference was always a typed parameter (predicate, domain, policy, structural property). Existing practices catch the implementation-time symptoms: - Practice 4 (Coproduct dissolution) — flat enum with parametric structure - Practice 5 (Single-authority metadata) — one fact one location - Practice 10's 'carrier dissolution' — local coproduct cloning a std/ carrier None of them catch the design-time cause: a domain-named declaration in the design doc that's actually a parameterization of an existing primitive. That gap is what seeded Worker A/B/C with mis-shaped briefs in Wave-1. Practice 11 (NEW) names the meta-pattern. Two sub-rules: 11a — Parameterize, don't duplicate Before authoring a new operation, carrier, or witness, exhaust 'is this a parameterization of an existing one?' Worked examples (drawn from real v4 corrections): - solve_constraints + coercion_fold + LawfulRewriteWitness → find_witness(_, _, predicate, multiplicity_policy) - 7 named witnesses → 3 generic carriers (StructuralPropertyWitness<P>, HomomorphismWitness<R>, PromotionWitness) - 3 MVP terminuses → 1 MVP - traverse_node etc as primitives → derived combinators over fold_node - ProgramSchedulingEdge / TargetPlanSchedulingEdge / ArtifactSchedulingEdge → one DependencyEdge with a DependencyKind label - DependencyGraph as separate carrier → Edge-on-Node parameterized by DependencyKind (carrier dissolves entirely) - TopologicalPlan as authored ledger → lens output that folds over Node (no parallel ledger) 11b — Concept-home boundary discipline Before adding a field to a substrate file, confirm the field doesn't cross a boundary the file's identity is supposed to keep separate. The canonical violation: NodeFileBinding in extdeps/file_system.dag (extdeps shouldn't know about Node; Node↔File provenance belongs in artifact/projection/ingest). Both sub-rules apply at the design-PR / brief-authoring layer — they catch errors that propagate as N file-scale violations across every worker the brief dispatches. The cheapest fix point is the design PR. Updates: - 'Ten Modeling Practices' → 'Modeling Practices' (open-ended; was miscounting after Practice 11) - New entry in the Practice → invariant mapping bullet list - Practice 10's 'carrier dissolution' sub-case cross-refs Practice 11 for the parametric generalization - New Practice 11 section between Practice 10 and the Calibration section, with the two sub-rules + worked-examples tables + standard 🔴/🟡/🟢 dispositions - Calibration section: explicit 'Practice 11 findings are always BLOCKING at the design-PR layer even when no implementation hunks exist' - For Reviewers: new step 11 — design-PR review applies Practice 11 per declaration, with required 🔴/🟡/🟢 disposition Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>


What this PR is
Strict-mode amendment to
docs/design-v4-compiler-homomorphism.md+src/v4/TASKS.mdper operator-direct architecture audit 2026-05-20. Follow-on to the merged PR #3437 (the original architecture doc) — addresses three categories of issues identified by audit and reviewer findings against the in-flight Wave-1 worker PRs (#3439/#3440/#3441).What changed
Pass A — concrete violations (3 fixes)
LossyfromCoercionQualitylattice (P3 fail-closed violation). TASKS.md T-9 lines 419-420 amended:CoercionQuality = Identity | Exactonly. Lossy operations are explicit user-declared source operations, not a coercion-time category. Loss is a diagnostic viaCoercionMismatchKind, NOT a success-with-warning.TranslatePlan.RewrittenThenCoercedextension slot (P2 illegal-states-unrepresentable — pre-emptive extension slot for unbuilt LawfulRewriteWitness was constructible).compile(corenode, dag_lang, TranslateTo(rust_lang))for the canonicaladd(x, y) = x + yworked example. Other endpoints (file-to-file shim, eval, multi-target translate) are deferred-with-trigger.Pass B Part 1 — Open Q resolutions independent of unification
parse(serialize(node)) == node), Q14 (target selection deterministic policy OR ambiguity diagnostic).Pass B Part 2 — Primitive unification (9 → 5) + witness collapse (7 → 3)
New unified primitive
find_witnesssubsumes:solve_constraints(constraint-satisfaction predicate)LawfulRewriteWitness(lawful-rewrite-precondition predicate, P7)All three are
find_witnessinvocations with different preservation predicates.Derived (no longer primitives):
traverse_node/sequence_node/bind_outcome=fold_node + Outcome-threading algebra. Combinator interpretsStageDiagnosticPolicyas the single failure-handling authority.apply_diff=fold_node(root, substitute_at_paths(diff)).Final substrate primitive set (5):
fold_node(root primitive)find_witness(unified search/check)Witness collapse (7 → 3 generic carriers):
StructuralPropertyWitness<P>: was CanonicalGroundingWitness + AcyclicityWitness + ClosedWorldDependencyWitness.HomomorphismWitness<R>: was Witness + LawfulRewriteWitness.PromotionWitness: was BootstrapWitness + FixedPointWitness.Q6 + Q9 ratified as side-effects of unification:
fold_node + composed-lens-algebra; no new primitive.verify_witnessre-check.Net summary
The doc shrinks meaningfully not by trimming prose, but by the underlying architecture being smaller — concept unification rather than aggressive prose-cutting (per operator guidance: qualitative, not line-count-targeted).
What this means for in-flight Wave-1 work
Wave-1 worker PRs (#3439, #3440, #3441) are paused under swift-dove-578's coordination. Once this PR lands:
; NodeFileBinding absorption from Worker C).
The corrections-round dispatch shape is for swift-dove-578 to coordinate post-merge.
Constraints satisfied
References (per CLAUDE.md "Ledger standing principle" — single-authority):
🤖 Generated with Claude Code