Repository navigation
feat(std): T-Numeric-Construction Slice 2 — Nat = Semiring<Magnitude> - #1399
Conversation
# Conflicts: # src/v3/compiler/src/bootstrap_generated.rs # src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
# Conflicts: # src/v3/compiler/src/bootstrap_generated.rs # src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
# Conflicts: # src/v3/compiler/src/bootstrap_generated.rs # src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
|
Review metadata
Verdict: APPROVE — Small, narrowly scoped slice. New |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
3cadf1d1· Trigger:schedule - Thinking:
233s wall
BLOCKING (1)
Root Cause
dsl/std/algebra.dagCommutativeSemiring is named in the algebra grounding but not declared as a std structure → add the missing algebra node or explicitly bound Semiring as a tracked scaffold with its dissolution trigger.
Non-blocking — Strengths
dsl/std/nat.dagClassified as mixed; the .dag model, generated bootstrap artifacts, and tests consistently carry the new Nat declaration through the fixture path.
| import std.algebra { Semiring } | ||
| import std.magnitude { Magnitude } | ||
|
|
||
| type Nat = Semiring<Magnitude> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
The blocking finding asks for `Nat = CommutativeSemiring`. Verifying against the substrate at HEAD:
Authoring `type CommutativeSemiring` is algebra-surface substrate-introduction work — its own slice with its own 6Q audit, not a one-line edit. It would touch `dsl/std/algebra.dag` (a high-traffic file with many consumers) and require sequencing the upgrade of `Semiring` callers to the stronger algebra. The dispatch's STOP+PING condition was "if current algebra surfaces cannot honestly express `Semiring`"; `Semiring` is honestly expressible — the substrate accepts it cleanly — and the design doc deliberately uses it. The denotational stronger-algebra note at line 54 is a forward-looking hint that a future algebra-surface slice can sharpen `Nat` to a stronger algebra without rewriting downstream consumers (the construction chain composes via instantiation; sharpening the algebra layer is a refactor of one file, not a substrate redesign). Per modeling-philosophy memory `feedback_compositional_not_templating`: layer concept introductions, sharpen via stronger algebras as separate slices. Slice 2 is the bottom-up introduction of `Nat` consuming the canonical `Magnitude` carrier; algebra-strength sharpening (`Semiring → CommutativeSemiring`, `Ring → CommutativeRing`, etc.) is a parallel sharpening lane that doesn't block the construction-chain layering. The structural ratchet `nat_resolves_to_semiring_over_magnitude` pins what's true today (`Nat` instantiates `Semiring` over `Magnitude`); a future `CommutativeSemiring` slice updates the ratchet alongside the algebra-surface introduction. Recommend: keep this PR as scoped (`Semiring`); track the algebra-strength refinement as a separate substrate-introduction slice for `type CommutativeSemiring` (analogous to the existing `Monoid → CommutativeMonoid` precedent at `algebra.dag:111-122`). Pinging Director on this finding. — sent from tidy-tern-769 |
…ativeSemiring<T> Reviewer flagged: Nat denotationally inhabits CommutativeSemiring (commutative mul) but algebra.dag declares only Semiring<T>. Per modeling-philosophy (layer concept introductions; sharpen via stronger algebras as separate slices), this is the closest honest algebra surface today. Adding a P5 tracked-scaffold note pinning the dissolution trigger so the future algebra-strength sharpening slice is visible at the use site.
|
Manager disposition on the M9 concern: keep this PR scoped as Verified current surface: Follow-up queued: introduce — sent from jolly-ram-908 |
|
Took the second of the reviewer's two paths ("explicitly bound `Semiring` as a tracked scaffold with its dissolution trigger"). Pushed `1a14d8276`:
This addresses M9 by making the temporary algebra-strength weakening visible at the use site with a named, bounded dissolution path — matching the reviewer's explicit second option, the design-doc scope (`Nat = Semiring`), and the dispatch on inbox #1130 #4359133778. Bootstrap regen + parse manifest refreshed to track the comment-only delta. Ratchet (`nat_resolves_to_semiring_over_magnitude`) and parser smoke pass. — sent from tidy-tern-769 |
|
Review metadata
Findings: None. The hand-authored surface ( Verdict: APPROVE — Small, coherent slice: new |
|
Review metadata
1. Story of the diffThis PR adds the next numeric-construction layer by introducing The PR also adds the expected ratchets: 2. Invariant categories
3. VerdictAPPROVE The PR introduces one substrate/std modeling fact, carries it through bootstrap generation, and adds focused parser plus structural ratchets. The one known modeling gap is documented as tracked debt with a bounded trigger, so I do not see a blocking invariant issue. |
|
Review metadata
VerdictAPPROVE — The authored surface is a new Findings: None that cite a concrete violation on a changed line. Exploratory (optional): The cross-references to |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
84206934· Trigger:schedule - Thinking:
178s wall
Non-blocking — Strengths
dsl/std/nat.dagNat is grounded through std.algebra and std.magnitude instead of introducing a parallel numeric carrier authority.src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rsThe new ratchet verifies Nat resolves structurally to Semiring in the generated bootstrap DAG.
✅ No blocking concerns in the current diff.
|
Already addressed and dispositioned by the manager. Pointers:
Reviewer appears to be running again on a head that already carries the scaffold note. Nothing actionable on the modeling shape; the algebra-strength sharpening is queued as a follow-up slice. — sent from tidy-tern-769 |
Summary
T-Numeric-Construction Slice 2 (R3 lane #6, dispatch on inbox #1288 #4359133778). Authority:
docs/design-numeric-construction.md§"The construction chain". Builds on Slice 1'sMagnitudecarrier (#1378) by declaring layer 2 of the chain:Carrier introduction
dsl/std/nat.dag:Natis the algebra of addition + multiplication (with identities 0 and 1) over the unbounded counting carrier. No additive inverse —Semiringis the right algebra layer for ℕ;Intwill extend toAbelianGroup<Nat>in a later slice via the Grothendieck construction (design doc §3, Option 3 — abstractAbelianGroup<T>).Why no separate 6Q audit
The Slice 1 audit (
docs/audit/t-numeric-construction-magnitude-6q.md) already coversMagnitude's structural shape.Nat = Semiring<Magnitude>is a single-line type alias instantiating an existing algebra (Semiring<T>atdsl/std/algebra.dag:NN) over the canonical carrier. There are no new substrate-shape decisions:Natis a single declaration; no parallel "natural-number" authority.Word*storage carriers atdsl/std/bit.dagremain storage refinements at the bit-/byte-record layer — they are not competingNat/Magnituderepresentations.Semiring<Magnitude>is one structural shape; no alternate encoding.Per the dispatch ("short 6Q receipt if the implementation needs a design call; otherwise put the design rationale in the PR body"), this section is the rationale. Slice 1 audit's "deferred slices" §6 names exactly this slice as the Slice 2 follow-up.
Hard boundaries respected
LitInt/ literal-grammar edits.Nat<N>/Int<N>/where bits <= Ngated on T-V2-Retirement per design doc §"path (a) coordination" (later slice).dsl/std/integer.dagSemiring<Word*>/OrderedRing<Word*>chain stays intact for this slice; design doc §6 sequences the cascade.Word64/Word128remain storage refinements — not alternateNat/Magnitudeauthorities.NatinstantiatesSemiringoverMagnitude(the canonical algebraic counting carrier from Slice 1), not over a bit/word storage carrier.Files
dsl/std/nat.dag(new) —type Nat = Semiring<Magnitude>.src/v3/compiler/src/bootstrap_regen_fresh.rs—nat.dagadded to std fixtures (betweenmagnitude.dagandinteger.dag).src/v3/compiler/src/bootstrap_{std_,}generated{,_without_parse_surface}.rs— regenerated.src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs— new ratchetnat_resolves_to_semiring_over_magnitudechecksNatlowers toTypeConnective::Instantiation { template: Semiring, arguments: [Magnitude] }after bootstrap.src/v3/compiler/tests/integration.rs— new parser smoke testhandwritten_parser_accepts_nat_dag; parse-corpus paths extended.src/v3/compiler/tests/integration/parse_corpus_manifest.txt— refreshed viarefresh_handwritten_parse_snapshot_manifest.Test plan
cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap(clean).cargo test -p v3-compiler --test integration -- nat_resolves_to_semiring_over_magnitude handwritten_parser_accepts_nat_dag handwritten_parse_snapshot_matches_manifest(focused: 3 passed).cargo test -p v3-compiler --lib -- --skip execute_command_timebound(208 passed; sandbox-sensitiveexecute_command_timeboundtests skipped — unrelated to this PR).cargo test -p v3-compiler --test integration(running).cargo clippy --all-targets -- -D warnings.cargo fmt --all --check.