Repository navigation
Lane 3 Stage 3a partial — 3a.2 data values + 3a.4/5 tests + 3a.3 foundation - #496
Conversation
…tion
Implements four of the five M2 feature-parity sub-stages, with 3a.3
landing foundation only (parser + substrate edge) and 3a.3 full
semantics + 3a.1 mutual recursion deferred as size-class overruns.
**3a.4 surface generics (DB-12)** — acceptance tests only; the
parser (`parse_optional_type_params`) and lowering
(`lower.rs:306-323` already wire surface `<T, U>` through to
`Declaration.type_params` as `AtomPayload::TypeParam` declarations).
Tests lock in `fn id<T>(x: T) -> T = x` compiles, multi-param fn
compiles, and bounded form `<T: Ord>` is rejected at parse time.
**3a.5 Disj dotted-path (DB-13)** — acceptance tests only; match-arm
body lowering (`lower.rs:3375`) already extends `arm_scope` with
pattern bindings, and `lower_field_path_expr` resolves `s.field`
inside the arm via the existing `resolve_static_field_project`
walk. Tests lock in `match opt { Some(p) => p.x, None => 0 }` +
nested `o.inner.a` paths.
**3a.2 data value semantics (DB-10)** — full implementation:
- New `ValueBody::Scalar(FieldValue)` variant for scalar data
(`data answer: Int = 42`).
- Parser extends `parse_data_item` to accept non-`{`-shaped RHS
as a scalar expression body.
- `lower_data_item` builds `ValueBody::Scalar` from
`SurfaceExpr::Literal` bodies via existing
`lower_scalar_literal_for_type` (type-checked against the
declared type).
- New `Dag::data_value_at(id)` accessor reads the compile-time
value body from a declaration.
- `SurfaceExpr::Var` lowering falls back to inlining
`ValueBody::Scalar` as a `Value(LiteralBits)` node when the
name is not in local scope — enables `fn f() -> Int = answer`
to emit `42` directly without separate emit-side wiring.
- New `lower_field_path_expr` fallback: when the head ident is
a data declaration with `ValueBody::Structural { fields }`,
walk the compile-time fields instead of the runtime type
(enables `cfg.host` to resolve to the record literal's value).
- Nested `Record` field values supported via
`resolve_structural_field_path`.
**3a.3 `where` refinement (DB-11) — foundation only**:
- Parser: `parse_params` accepts `where <expr>` after a parameter
type; `SurfaceParam.refinement: Option<SurfaceExpr>` carries
the parsed predicate.
- Substrate: new `Declaration.refinement: Option<DeclarationId>`
edge (all existing Declaration construction sites set to
`None`).
- Tests lock that `fn div(n: Int, d: Int where d != 0) -> Int`
parses without dropping the `where` clause.
- **Deferred**: lowering the parsed predicate into an attached
predicate `Declaration`, call-site structural-DAG check, and
Branch-arm predicate narrowing extension to M1(2.8) pattern
resolution. These three items represent a size overrun
(M → L) against 3a.3's sizing and warrant a dedicated follow-up
PR where the predicate pipeline can be designed end-to-end.
**3a.1 mutual recursion (DB-9)** — deferred. SCC detection +
cluster descent verification is L-sized and landing it inside
this PR would push the combined change past its review budget.
Follow-up tracked separately.
All workspace tests pass; clippy clean; no behavior variant added;
substrate commitment per THESIS.md:604-629 preserved.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
![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: 0d991e518f
ℹ️ 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 refinement = if matches!(self.peek().kind, TokenKind::KwWhere) { | ||
| self.bump(); | ||
| Some(self.parse_expr()?) | ||
| } else { |
There was a problem hiding this comment.
where refinements into lowering
The parser now accepts and stores parameter refinements here, but the lowering path never consumes them (function signatures are still built from parameter types only, and declarations are initialized with refinement: None), so where predicates are silently dropped after parsing. That makes constrained signatures appear valid while providing no enforcement or structural trace in the DAG, which is a correctness issue for any code relying on these constraints.
Useful? React with 👍 / 👎.
| if let Some(decl_id) = symbols.get(name) { | ||
| if let Some(crate::dag::ValueBody::Scalar( | ||
| crate::dag::FieldValue::Literal(bits), | ||
| )) = &dag.declaration(*decl_id).value_body |
There was a problem hiding this comment.
This lookup inlines a scalar data value only if value_body is already present on the declaration, but bodies are lowered in source order, so references to a later data item (for example fn f() -> Int = answer before data answer: Int = 42) are marked unresolved and never repaired. The new feature therefore introduces order-dependent name resolution for top-level data values.
Useful? React with 👍 / 👎.
Ran locally; no substantive changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reflects what was actually landed in PR #496 vs. what the design originally proposed: - **DB-10** — implementation chose lowering-time inlining (`SurfaceExpr::Var` + `lower_field_path_expr`) rather than emit-time inlining (three emit files). Amendment records the trade-off (simpler vs lossier provenance) and notes that `Dag::data_value_at` remains the shared contract for any future emission-time consumer. - **DB-12** — investigation found the parser + lowering for `fn f<T, U>` was already wired (`parse_optional_type_params` + `lower.rs:306-323`). Stage collapsed to acceptance tests only. - **DB-13** — investigation found the match-arm body scope extension was already wired (`lower.rs:3375` + arm_scope + `lower_field_path_expr`). Stage collapsed to acceptance tests only. - **Declaration.refinement** — docstring now carries an explicit 🟡 scaffold marker with the dissolution trigger (3a.3-followup per issue #498). Makes the scaffold discoverable by grep so a future reader doesn't assume the field is fully wired. Tracking: #498 (3a.3 semantics), #499 (3a.1 mutual recursion). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Addresses review finding on PR #496 (chatgpt-codex P2): `data` scalar references were order-dependent. `fn f() -> Int = answer` declared *before* `data answer: Int = 42` resolved to a declaration whose `value_body` was still `None` because bodies lowered in source order; the Var fallback fell through to an unresolved diagnostic. Fix: restructure `lower_bodies_phase` into three dependency-ordered passes: - Pass 1: types (TypeRecord / TypeSum / TypeAlias / TypeAtom). Pure type connectives — no reference to values. - Pass 2: data items. Depend on types for inhabitance checking; populate `value_body` so later fn-body `SurfaceExpr::Var` lookups find it. - Pass 3: everything else (fns, fn external body, let, module, import) in source order. Types and data are skipped (already lowered in the pre-passes). Pass 1 prevents the naive data-only pre-pass from trying to walk type connectives that are still placeholders. Regression test: `test_3a2_data_reference_is_order_independent` locks in both forward (`fn f` before `data answer`) and backward (`data answer` before `fn f`) orderings compile and emit the same `Value(Int(42))` node. Full v3-compiler test suite remains green; clippy clean; no substrate shape change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Responses to the two review findings: P2 (lower.rs, "Seed data values before resolving scalar references") — fixed in c521c73. Restructured P1 (parse.rs:1155, "Propagate parsed |
New principle: ROADMAP is the tracker. All in-flight work, deferrals,
and follow-ups from merged PRs live in ROADMAP (and the lane / design
docs it points at) — not in GitHub issues. A PR merging in/out of
main MUST leave ROADMAP reflecting the new state.
Rationale: GitHub issues fork authority from the code+docs ROADMAP
points at; they rot silently when sessions forget to sync. A single
file the project reads every day doesn't rot.
Also:
- New "Active deferrals" section below the Post-A/B Lane Plan,
populated with:
- Lane 3 Stage 3a sub-stage status table (3a.2/4/5 ✅, 3a.3 🟡,
3a.1 ⏸).
- Deferral entries for 3a.3-full and 3a.1 mutual recursion (from
PR #496 overruns) and 1b full implementation (from PR #495
escalation; unblocked by DB-14/#497).
- Deferral format includes size, scope, design doc link, acceptance
pointer, and yellow-flag threshold.
- Follow-up PRs clear deferrals by removing entries and updating the
sub-stage tables. PR reviewers block merge on ROADMAP staleness.
GitHub issues #498 (3a.3 semantics) and #499 (3a.1 mutual recursion)
to be closed with a pointer to this section, per the discipline.
) New principle: ROADMAP is the tracker. All in-flight work, deferrals, and follow-ups from merged PRs live in ROADMAP (and the lane / design docs it points at) — not in GitHub issues. A PR merging in/out of main MUST leave ROADMAP reflecting the new state. Rationale: GitHub issues fork authority from the code+docs ROADMAP points at; they rot silently when sessions forget to sync. A single file the project reads every day doesn't rot. Also: - New "Active deferrals" section below the Post-A/B Lane Plan, populated with: - Lane 3 Stage 3a sub-stage status table (3a.2/4/5 ✅, 3a.3 🟡, 3a.1 ⏸). - Deferral entries for 3a.3-full and 3a.1 mutual recursion (from PR #496 overruns) and 1b full implementation (from PR #495 escalation; unblocked by DB-14/#497). - Deferral format includes size, scope, design doc link, acceptance pointer, and yellow-flag threshold. - Follow-up PRs clear deferrals by removing entries and updating the sub-stage tables. PR reviewers block merge on ROADMAP staleness. GitHub issues #498 (3a.3 semantics) and #499 (3a.1 mutual recursion) to be closed with a pointer to this section, per the discipline.
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
codex · gpt-5.4 · 0d991e51
BLOCKING (2)
Root Cause
src/v3/compiler/src/lower.rsSurfaceParam.refinementis never consumed when function signatures and parameter ports are built, so the fact dies at parse→lower; either wire it intoDeclaration.refinementplus a real consumer, or emit an explicit unsupported-refinement diagnostic until #498 lands.src/v3/compiler/src/dag.rsThe new top-level data-body shape reuses the recursive per-field carrier instead of a scalar-only carrier; narrow it toLiteralBitsor a dedicated top-level value enum before downstream consumers copy the wider shape.
Non-blocking — Strengths
docs/design-m2-feature-parity.mdThe DB-10/12/13 amendments are honest about what actually landed versus what turned out to already exist, which keeps the design doc aligned with the compiler state.
Non-blocking — Improvements (fix in-PR if easy, else defer to roadmap)
src/v3/compiler/src/dag.rsDag::data_value_at's docstring saysValueBody::Unparsedcomes back asNone, but the implementation currently returns the rawUnparsedarm, so the helper contract should be tightened before a consumer relies on it.
where compiles as ignored syntax and ValueBody::Scalar still admits overlapping top-level representations.
| // parameter's type. Parsed as an ordinary expression in | ||
| // the parameter's scope; lowering resolves operators | ||
| // against the parameter's type algebra. | ||
| let refinement = if matches!(self.peek().kind, TokenKind::KwWhere) { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Addressed in e0b4ded — lowering now emits an explicit ResolveError via report_unsupported_parameter_refinements during seed_function_signatures_phase for every parsed SurfaceParam.refinement. fn div(n: Int, d: Int where d != 0) -> Int no longer compiles silently; it compiles into a diagnostic pointing at the predicate span and naming #498 as the dissolution trigger. Fail-closed per C-8. Regression test test_3a3_where_clause_parses_and_reports_unsupported locks the behavior.
|
Violations (could not place on specific lines):
|
This comment has been minimized.
This comment has been minimized.
Review: approve ✅Final state: fmt ✅, v3 ✅, ci pending. MERGEABLE. All three conditional-approval items addressed:
Substantive approvalsM→L escalation discipline honored. 3a.3 hit its size class ceiling during implementation; session correctly stopped at parser foundation rather than silently absorb. That's the right signal. Scaffold hygiene is correct. 3a.2 inlining-at-lowering is a legitimate trade-off. Session chose lowering-time inlining of 3a.4/3a.5 tests-only landing is a useful data point. The design docs called for "parser + lowering wiring" and it turned out the infrastructure was already present. DB-12/DB-13 now reflect that. Future sub-stages in this vein can cite the pattern: "verify existing infrastructure before authoring new." Post-merge state (once ci passes)
Green to merge once ci turns green. Good deliverable. |
…x docstring Addresses three blocking findings from the PR #496 reviewers: **ValueBody::Scalar narrowed from FieldValue to LiteralBits** (codex blocking #2 + chatgpt-review coproduct-dissolution). `Scalar(FieldValue)` admitted `Scalar(FieldValue::Record{..})`, which overlaps with `ValueBody::Structural { fields }` — two encodings of the same top-level record body. Narrowing to `Scalar(LiteralBits)` makes that illegal state unrepresentable and aligns the variant with its actual acceptance: scalar literals only. A new `emit_literal_as_value_port` helper is shared by the `Scalar` inlining, `FieldValue::Literal` terminal reads, and `SurfaceExpr::Var` data inlining. If a future parser accepts list/map/variant literals at the top level, grow `ValueBody` with a new variant — do not widen `Scalar`. **`where` refinement now emits an explicit diagnostic** (codex blocking #1 + chatgpt-review REQUEST_CHANGES on facts- flow-forward). `report_unsupported_parameter_refinements` runs in `seed_function_signatures_phase` and emits a `ResolveError` pointing at the predicate span for every parsed `SurfaceParam.refinement`. Makes the parsed fact visible at compile time instead of silently dropping it — fail-closed per C-8. The test `test_3a3_where_clause_on_parameter_parses` inverts to `test_3a3_where_clause_parses_and_reports_unsupported`: the parser still accepts the syntax, lowering explicitly rejects it with a diagnostic naming issue #498 as the dissolution trigger. Removes the "parsed but silently unread" substrate debt the reviewers flagged. **`Dag::data_value_at` docstring corrected** (codex non-blocking). The prior docstring claimed `Unparsed` scaffolds return `None`, but the implementation returns the raw `ValueBody::Unparsed` variant. Docstring rewritten to accurately describe the contract: `None` iff no value_body, `Some(&ValueBody)` otherwise including `Unparsed`. Consumers that need inhabitance-checked bodies match on the variant directly. All 12 m2_feature_parity tests remain green; full v3-compiler test suite green; clippy clean; fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
All three new blocking findings addressed in e0b4ded: 1. 2. 3. All 12 |
ChatGPT ReviewPrinciple audit. Against the active discipline’s fail-closed / facts-flow-forward bar, and the thesis’s warning that lossy compression is only acceptable when it is explicit rather than accidental, this revision is materially cleaner than the earlier one. chatgpt-review-5b371c1c-35a7-4a… chatgpt-review-9a09bd3f-f5f7-49… Fail-closed. This looks good now. The Illegal states unrepresentable. Also good. Narrowing Facts flow forward. 3a.2 now flows end-to-end: parse accepts scalar data bodies ( Coproduct dissolution. I think this is in better shape now. The important move was not just adding a new arm, but narrowing it so Single authority. Satisfied. Compile-time data values still have one canonical home — API-level enforcement. Mostly satisfied, with 3a.3 still protected behaviorally rather than structurally. The 3a.2 side is reasonably API-shaped now because the carrier itself rules out overlapping record/scalar states. The refinement side still relies on an explicit diagnostic gate plus regression tests ( LOOP HEALTH: converging — this round removes both earlier blockers, turns 3a.3 from a silent-drop into a tracked fail-closed scaffold, and adds real consumer-enabling tests for 3a.2 plus ratchet tests for 3a.4/5. Design question. Do you want compile-time data references to collapse to plain The design amendment now consciously chooses eager literalization and records the trade-off as “simpler” versus “lossier provenance.” That is a legitimate choice for Verdict. APPROVE_WITH_COMMENTS The substance looks good now: |
Meta-Review (Loop Health)According to a document from 2026-04-17, the loop is not bluffing anymore, but it is no longer worth another tightening round on this PR. The review history shows a real repeated failure class — facts introduced at parse/substrate boundaries without downstream authority — and the current diff appears to have converted the worst instance from “ignored semantics” into tracked, fail-closed debt. That is enough to merge with debt, not enough to call the lane clean. Loop summary. From the attached history I can see 4 recorded review events: 1 codex-cli review, 2 complete chatgpt-browser reviews, and 1 chatgpt-browser placeholder “review in progress.” That is 3 substantive reviews plus 1 non-substantive placeholder, spanning about 54 minutes (04:46Z → 05:40Z on 2026-04-17). The exact commit count is not recoverable from the attachments; what is visible is at least one substantive post-review patch turn, because the current diff appears to address the blockers the 05:13/05:15 reviews named. Forward progress evidence. There is real progress, not just motion. The loop did enable a real consumer path for 3a.2. The first browser review explicitly distinguishes 3a.2 from 3a.3: scalar data now flows parse → lowering → The loop also improved the quality of scope control. For DB-12/13, the reviews note that the PR amended the design doc to say what really landed versus what already existed, and 3a.4/5 became tests rather than substrate growth. That is forward progress under your Principle 1 and 7: a consumer/test beats more perfect-looking substrate with no user. Most importantly, the current diff appears to have taken the central blocker from the recorded reviews — “ Debt accumulation evidence. The loop still accumulated debt; it just did so with accounting. The same issue class was re-flagged across rounds: parsed refinement facts do not survive parse → lower → consumer. The first browser review calls 3a.3 “substrate/parser shape without corresponding fact flow.” The second says the compiler “accepts That matters because the active discipline already says what should happen with a recurring class like this: facts must flow forward, scaffolds need explicit boundaries, and substrate issues are blocking because later fixes get more expensive. The invariant set is already telling the project to stop patching leaf instances and fix the deepest broken boundary. The debt is also visible in the PR shape itself: And at the thesis level, the project’s own roadmap is explicit that types with zero consumers are decorative, not proof. If a modeled fact is not consumed, it is a paper exercise. That is exactly what 3a.3 became for one round: parser/substrate shape without enforcement. Cheating signal. Low, but real. The implementer is not hiding compromises. The recent loop behavior is openly scaffolded and accounted for: the browser reviews themselves describe the refinement slot as a tracked scaffold, and the current diff appears to have moved from “ignored surface syntax” to “explicit unsupported diagnostic.” That is not quiet cheating. That is implementer triage with receipts. The strongest positive signal is that the loop did not answer repeated blockers with more silent behavior or more fake success. It appears to have narrowed The negative signal is that the fix is still bounded debt, not payoff. Why this is not a Because the loop is not stagnant. It found the same class repeatedly, and the current diff appears to have actually responded in the right way: narrow the representation, add fail-closed handling, and lock behavior with tests. A regroup is for loops that keep rediscovering the same class without changing the architecture of the failure. This one did change the local architecture of the failure. Why this is not a Because the marginal value of another round on this PR looks low. The important thing was to stop shipping ignored semantics. Once that becomes a hard diagnostic and the over-wide value-body shape is narrowed, the remaining work is the real follow-up consumer. Another pass here is likely to turn into “finish #498 inside this PR,” which is exactly the kind of scope bleed the invariants argue against. The project’s own discipline says don’t normalize transitional states — either make them load-bearing now or bound them clearly. This diff now appears to do the second. Meta-verdict: ⚖️ SHIP_WITH_DEBT This loop made real progress. It enabled a real 3a.2 consumer path, correctly collapsed 3a.4/5 into tests, and appears to have converted the 3a.3 problem from silent semantic drop into tracked, fail-closed debt. But it did not finish the refinement feature; it introduced or preserved one scaffold ( |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
codex · gpt-5.4 · e0b4ded2
✅ Review (blocking: 0, non-blocking: 0+/2-)
Non-blocking — Improvements (fix in-PR if easy, else defer to roadmap)
src/v3/compiler/src/lower.rsWhen a scalar data body fails its declared-type check, the new branch still lands ValueBody::Unparsed, so the user path can also pick up the generic opaque-body scaffold diagnostic; if deferred, clean that up before DB-10 grows more body shapes.src/v3/compiler/tests/m2_feature_parity_test.rstest_3a2_data_field_access_resolves_statically only asserts successful compile, so it does not actually ratchet the intended literal inlining versus a runtime field read; if deferred, tighten it in Lane 3's DB-10 test ratchet before more consumers depend on the lowering-time choice.
ROADMAP — Incomplete
- #498: Full refinement semantics are still deferred, but the field comment, PR body, and fail-closed lowering gate make the remaining scope explicit and bounded.
✅ The prior blockers are fixed, the remaining 3a.3 debt is explicitly bounded, and I did not find a new merge-stopping thesis or invariant violation in the changed code.
|
✅ Review (blocking: 0, non-blocking: 0+/2-) Non-blocking — Improvements (fix in-PR if easy, else defer to roadmap)
ROADMAP — Incomplete
✅ The prior blockers are fixed, the remaining 3a.3 debt is explicitly bounded, and I did not find a new merge-stopping thesis or invariant violation in the changed code. |
…lining Addresses codex non-blocking suggestion on e0b4ded: the field- access acceptance test asserted only "compile succeeds," which didn't actually ratchet the DB-10 lowering-time-inlining choice. A silent regression to runtime `FieldProject` on a data value would pass the old test. New assertions: 1. DAG must contain a `Value(Int(1))` node — the inlined `host` field from `data cfg: Config = { host: 1, port: 8080 }`. 2. DAG must NOT contain a `FieldProject<host>` Transform. Any runtime field access on a data value fails this assertion and breaks the invariant. Deferred (per reviewer "defer to roadmap if not easy"): scalar data body type-check failures currently still fall through to `ValueBody::Unparsed`, producing a possible second user-range scaffold diagnostic on top of the specific type-mismatch diagnostic. Clean up before DB-10 grows more body shapes. All 12 m2_feature_parity tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the re-review ✅. Addressed the test-ratchet suggestion in 2ae5e2d:
Deferred the scalar-body-type-mismatch double-diagnostic cleanup per your "if deferred, before DB-10 grows" guidance — noted in the commit message. Scope is bounded and follows the same "more body shapes before we clean this up" trigger. |
ChatGPT ReviewPrinciple audit. Against the thesis’s “if it compiles, the intent is sound” bar and the active six-principle review lens, this revision looks materially cleaner than the earlier one. chatgpt-review-9a09bd3f-f5f7-49… chatgpt-review-b940f0a1-1212-41… Fail-closed. Satisfied. The prior 3a.3 concern looks fixed: unsupported parameter refinements are now rejected through an explicit lowering diagnostic rather than compiling as ignored syntax, which is the right shape under both fail-closed and the scaffold-boundary rule. chatgpt-review-b940f0a1-1212-41… chatgpt-review-8bb52af0-b5a4-43… Illegal states unrepresentable. Satisfied. Narrowing Facts flow forward. Satisfied. 3a.2 now reads like a full pipeline change rather than a partial parse-only addition: scalar data bodies are carried into lowering, consumed by var/path resolution, and the dependency-ordered type/data prepasses remove the incidental source-order dependency that would otherwise leak through. Coproduct dissolution. Satisfied for this diff. The new Single authority. Mostly satisfied. My one non-blocking comment is at the acceptance boundary: API-level enforcement. Mostly satisfied. The user-path invariants are enforced well enough here — especially the fail-closed refinement gate — and I do not see an easy way for a contributor to reintroduce the earlier silent-success bug. My only reservation is the same non-blocking test ratchet: this repo’s testing rules say tests should pin observable behavior, not internal implementation detail, unless you are intentionally promoting that detail into the contract. chatgpt-review-31afd463-e734-45… LOOP HEALTH: converging — the prior blockers are fixed, 3a.2 now unblocks a real consumer path with order-independent lowering, and the only new wrinkle I see is a test/doc contract mismatch rather than fresh substrate debt. Design question. Is lowering-time literalization now a committed architectural choice, or still just one valid implementation of DB-10? That is the only structural question I think this PR still raises. If the answer is “committed,” then the new ratchet in Verdict. APPROVE_WITH_COMMENTS The substantive code issues I would have stopped earlier revisions on look fixed, and this round reads as real forward progress rather than debt shuffling. I would merge it, but I would align the DB-10 design text and the new lowering-time-inlining ratchet so there is exactly one authority for what the PR is promising. |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
codex · gpt-5.4 · 2ae5e2de
✅ Review (blocking: 0, non-blocking: 2+/0-)
Non-blocking — Strengths
src/v3/compiler/src/dag.rsNarrowing top-level scalar data to ValueBody::Scalar while keeping record payloads in Structural removes the overlapping representation and improves illegal-states-unrepresentable at the substrate boundary.src/v3/compiler/tests/m2_feature_parity_test.rsThe new 3a.2/3a.3/3a.4/3a.5 coverage ratchets real consumer behavior, especially order-independent data lookup and fail-closed where handling.
ROADMAP — Verified
- DB-10: The lowering-time data inlining path now has both code-path coverage and an order-independence ratchet, so 3a.2 no longer depends on declaration order.
- DB-12/DB-13: This PR correctly collapsed both sub-stages to test-only landings after verifying the parser and lowering behavior was already present.
ROADMAP — Incomplete
- #498: Parameter refinements are now explicitly bounded debt rather than silent behavior, but predicate lowering, call-site checks, and branch-arm narrowing are still follow-up work.
✅ The prior blockers are fixed, the remaining refinement work is explicitly bounded, and I did not find a new thesis- or invariant-level reason to stop this PR.
|
✅ Review (blocking: 0, non-blocking: 2+/0-) Non-blocking — Strengths
ROADMAP — Verified
ROADMAP — Incomplete
✅ The prior blockers are fixed, the remaining refinement work is explicitly bounded, and I did not find a new thesis- or invariant-level reason to stop this PR. |
Meta-Review (Loop Health)According to a document from 2026-04-17, this loop is making real progress, but it has already extracted most of the value available from another review turn. Meta-verdict: ⚖️ SHIP_WITH_DEBT. Loop summary. The attached history shows 7 review events over about 2 hours 41 minutes: 4 chatgpt-browser reviews and 3 codex-cli reviews. The artifacts do not expose exact commit hashes, but they do show about 4 distinct reviewed revision states. Forward progress evidence. This loop did not just reshuffle comments. Early rounds correctly blocked on two substrate-level issues: 3a.3 accepted Debt accumulation evidence. Some debt remains. The big one is explicit: full refinement semantics are still deferred to #498, so Cheating signal. Low. The implementer is not hiding compromises. Reviewers explicitly call out the scaffold marker, the named follow-up, and the fail-closed gate, and later rounds describe the remaining 3a.3 debt as “explicitly bounded.” The most recent fixes are also structural before they become cosmetic: first reject ignored syntax, first narrow the carrier, first add real ratchets; only then does the loop devolve into doc/test alignment. That is implementer triage with accounting, not quiet “good enough for now” drift. Invariant graduation. No new invariant was added in this PR’s loop. I do not read that as a failure here, because the recurring 3a.3 problem was already covered by an existing rule: Scaffold boundaries already requires an explicit user-range unreachability gate, regression test, and named dissolution trigger for any new scaffold. This loop eventually applied that existing invariant instead of rediscovering a new one. Meta-verdict — ⚖️ SHIP_WITH_DEBT. The loop’s bluff has already been called, and it passed. It found the real blockers, fixed them structurally, converted fake feature work into test locks where appropriate, and left only explicit follow-up debt plus a small test/doc authority mismatch. Another iteration is more likely to produce polish than new structural value. Merge this PR with the tracked |
Addresses the doc/test contract mismatch flagged by ChatGPT round-2 review: the test ratchet in e0b4ded/2ae5e2de9 asserts that `cfg.host` lowers to `Value(Int(1))` AND that no `FieldProject<host>` Transform exists in the DAG. That makes lowering-time inlining load-bearing architecture, not the "swappable implementation detail" the prior amendment claimed. Three doc edits, no code changes: 1. **DB-10 architectural commitment.** Rewrites the "Implementation amendment" section in `docs/design-m2-feature-parity.md` to explicitly commit to lowering-time inlining. Emission-time inlining is moved to the rejected-alternatives list with its trade-off recorded: richer provenance but forces every downstream consumer (3× emitters, future property lenses, cost analysis) to handle data-declaration references with a second code path. The test is the lock. 2. **Stale `Scalar(FieldValue::Literal(bits))` reference.** The post-narrowing shape is `ValueBody::Scalar(LiteralBits)` (direct), not `Scalar(FieldValue::Literal(bits))` (wrapped). The DB-10 "Identifier resolution" bullet is updated to match. 3. **SurfaceParam.refinement docstring drift.** The field doc previously claimed the predicate is "Lowered to a predicate `Declaration` whose resolved expression DAG becomes `Declaration.refinement` on a cloned type declaration." The actual behavior (per `report_unsupported_parameter_refinements` in `lower.rs`) is that the predicate is captured at parse and rejected with a `ResolveError` at lowering — fail-closed, not silently dropped, but not propagated into the substrate either. Doc now accurately describes the foundation-only scope and names issue #498 as the dissolution trigger. No test or behavior changes; all 12 m2_feature_parity tests still green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ss-5 confidence-builder attack point) Per operator directive 2026-05-15 — "lets attack DB-10 for confidence, give it to one of the existing path B brief members". Brief 4 dispatches DB-10 (data value semantics, M2 feature parity item 3a.2) to witty-moth-725 — they closed Brief 1 cleanly (PR #3142 promoted to ready) and are available for new substrate-language work. Authoritative design at docs/design-m2-feature-parity.md §DB-10 (lines 20-71); doc carries "Design ready for implementer review" status since authoring. This is the FIRST attack against the M2 / class-5 deferral pit per the survey at docs/r3-m2-class-5-deferral-survey.md. Scope: - Phase A: verify design-doc cited locations (dag.rs:122, lower.rs:1404, test_3a2_data_field_access_resolves_statically test ratchet) against HEAD; surface drift findings to deep-wolf-155 - Phase B: land 3 consumers per design — Dag::data_value_at accessor + SurfaceExpr::Var fallback for scalar inlining + lower_field_path_expr walks ValueBody::Structural for static field access - Phase C: test fixture demonstrating data answer = 42 + answer + 1 emits 42 + 1 inline across all 3 target languages Architectural commitment honored (per design + PR #496 2026-04-17): inlining at LOWERING, not emission. Load-bearing per locked test ratchet. Emission-time inlining REJECTED. Coordination: parallel to PR #3142 merge (Brief 1 closure); separate branch; report findings directly to deep-wolf-155 (no Director/Mgr layer). Coordinate with sunny-tern-495 (Brief 2) + bright-swift-668 (Brief 3) only if substrate work overlaps. Anti-paper-shrink discriminator: locked test asserts no FieldProject Transform exists in the lowered DAG for data-resolved field accesses; that ratchet catches trampoline-style workarounds. Estimated effort: 1-2 weeks if design doc is accurate.
…d LANDED at HEAD 2026-05-15 Per gentle-bat-24 Brief 4 Phase A finding (DB-10 substrate already implemented at HEAD with `Dag::data_value_at` at dag.rs:4310 + lower.rs:8489+/8642+/9125+ + passing test at m2_feature_parity_test.rs:771) + subsequent verification of DB-12/DB-13 status amendments in PR #496 against HEAD test names: DB-10: LANDED DB-12: LANDED (parse_generated.rs:927; m2_feature_parity_test.rs:90-156) DB-13: LANDED (lower.rs:3375; m2_feature_parity_test.rs:163-208) DB-9 + DB-11: HEAD landed-state NOT YET AUDITED; may also be silently landed Sites updated: - docs/r3-m2-class-5-deferral-survey.md: M2 feature parity table reframed with cited HEAD evidence per DB; explicit retraction of "no DB item has landed yet" claim; active deferral pits shrink to DB-9 + DB-11 (pending audit) plus class-5 sub-gaps which remain genuinely deferred per Brief 3 findings - docs/design-m2-feature-parity.md: top-level Status line updated from "Design ready for implementer review" to "3 of 4 LANDED" with per-DB cited HEAD evidence; explicit advisory that workers should audit HEAD state before authoring implementation against this doc Residual M2 work for the 3 LANDED items: emission-test-coverage gap for DB-12 + DB-13 (design §Acceptance promises 3-target render verification; current tests check compile + lower only). Small follow-up scope; not pit-shaped. The "M2 deferral pit" framing was substantially wrong as initially described — much of M2 has been silently landing. The class-5 sub-gaps remain genuinely deferred per Brief 3's bright-swift-668 investigation (brace-body fn parse on .dag files). Per `feedback_corrections_must_grep_verify_source` — this is the canonical lesson: design docs drift; verify against HEAD before authoring against them.
…afts) (#3139) * docs(r3): rust retirement catalogue + emit.rs modeling doc (format-validation drafts) Per operator 2026-05-15 directive — replace SG-0 file-path ratchet (which rewards file-path manipulation / paper-shrink) with a substrate-authority planning artifact. Focus on actual `.dag` substrate growth, not ratchet file-list shrinkage. Two docs: (1) `docs/r3-rust-retirement-catalogue.md` (159 lines) - Framing: explicit anti-paper-shrink stance citing `feedback_template_relocation_paper_shrink_discriminator` (cycles 4/5/6 reverts at PR #3059) - Format spec for per-entry catalogue: Role / Public surface / Inputs / Calls into / Called by / Existing `.dag` substrate / Retirement target / Replacement plan (R1=substrate, R2=consumer-reads-substrate-not-template, R3=parity invariant — semantic NOT textual) / Anti-paper-shrink check / Phase 1-5 / Substantive retirement risks - 5-phase retirement plan ordered cheapest-first: Phase 1: hand-Rust monotonicity invariant (foreclose growth) Phase 2: test harness layer (122 → 1-3 via .dag TestClaim) Phase 3: lens-as-Rust (7-10 → 0 via data lens_X: Lens<C> + fold_lens) Phase 4: substrate-mirror generation (5-8 → 0 via regen reads substrate.dag) Phase 5: pipeline-stage retirement (12-15 → meta-circular bootstrap) - One worked example entry: src/v3/compiler/src/emit.rs (3,992 lines) with 4 substantive retirement risks flagged - 5 open format questions for operator before scaling to remaining ~176 entries (2) `docs/r3-retirement-modeling-emit-rs.md` (310 lines) - Per-file modeling doc; first instance of a series (one per Rust file) - §1 What emit.rs does (4 internal layers identified) - §2 Reference pattern (tokenize.dag) — the proven `.dag`-drives-retirement example - §3 6-layer identification ordered by tractability (Layer 1 dispatcher = most tractable; Layer 6 = whole-file retirement) - §4 Layer 1 (dispatcher) FULLY MODELED: * 4 new substrate carriers in src/v3/std/emit_model.dag: EmitTarget, EmitMode, EmittedSource, EmitTargetDispatchRow * 1 data value: emit_target_dispatch_table * 3 pure functions: emit_with_mode, emit, emit_module * Codegen driver outline: new regen_emit_dispatcher.rs following regen_tokenize.rs pattern * Parity verification: SEMANTIC (m1_3/m1_4 integration tests pass unchanged) NOT TEXTUAL (no source-clone) - §5 Sketches for Layers 2-N - §6 4 open questions about .dag substrate-language features needed - §7 Substantive retirement risks (paper-shrink discriminators) - §8 What Layer 1 demonstrates (proof-of-pattern for emit translation) Neither doc proposes changing the ratchet now — that's separate operator direction. These are planning artifacts for the substantive retirement work, not enforcement bridges. Tokenize/parse pipeline stages are already fully substrate-driven (the proof-of-concept). Lower/infer/emit have L2.5 design docs (PR #3066, #3077, #3085, #3126 just merged) but no executable `.dag` substrate yet — the modeling doc series identifies the smallest tractable layer per file to begin substantive migration. Co-authored-by: Brian (operator briansrls) — directive 2026-05-15 * docs(r3-catalogue): restructure per claude APPROVE-exploratory PR #3139 — move 5-phase plan above format spec (single authority for Phase enum) claude APPROVE on commit 458b08d (PR #3139, 2026-05-15T02:14:43Z) flagged a non-blocking exploratory note: "the catalogue's Phase enum is defined inside the per-entry format spec block, but the 5-phase plan section at the bottom is the canonical authority — if this doc lives long, consider moving the phase definitions above the format spec so the format references rather than redefines them." Applied — the doc now has the 5-phase plan section FIRST, with full definitions, and the format spec section references Phase numbers without duplicating definitions. Eliminates the dual-authority problem the exploratory note identified. Section order (post-fix): - §"5-phase plan (canonical authority for Phase enum referenced below)" - §"Format spec (per entry — references Phase numbers from §"5-phase plan" above)" - §"Catalogue" (worked example: emit.rs) - §"(Remaining ~176 entries — to be filled per format-ratified spec)" - §"Open questions for operator before format is ratified" The format spec's Phase field now reads: "Phase: one of Phase 1–5 per §"5-phase plan" above. (Definitions are NOT duplicated here — the §"5-phase plan" section is the single authority.)" No content changes beyond structure; doc is 154 lines (down 5 from de-duplication of the Phase definitions). * docs(r3): add Path B tokenize/parse brief set (9 briefs, no workarounds) Per operator directive 2026-05-15 — "could we try to do path B for tokenize/ parse (NO workarounds) now — i'd like to spawn workers under you directly". Planning doc enumerating the 9 briefs needed for true 100% .dag retirement of tokenize + parse including codegen drivers. Workers spawn directly under deep-wolf-155 (PM) per operator; no Director/Mgr layer for this lane. Dispatch graph: - Phase 0 (parallel, no deps): Briefs 1-3 substrate-language gaps - Brief 1: generic methods on FreeMonoid<T> (non-endomorphic map + per-method type params; named gap at dsl/std/algebra.dag:387-393) - Brief 2: format(template, args) + Int/Char/Bool → String primitives - Brief 3: char-class structural completion (finish in-progress scaffold at tokenize.dag:23-58) - Phase 1 (parallel, no deps on Phase 0): Briefs 4-6 host effects + bridge - Brief 4: FileReadEffect + FileWriteEffect (bundle) - Brief 5: ProcessSpawnEffect (for rustfmt) - Brief 6: compile_to_dag FFI bridge — operator decision point (FFI vs self-hosted) - Phase 2 (depends on Phases 0+1): Briefs 7-9 driver authoring - Brief 7: tokenize_codegen.dag (retires regen_tokenize.rs 1,186 lines) - Brief 8: parse_tables_codegen.dag (retires regen_parse_tables_emit.rs 1,284 lines) - Brief 9: parse_codegen.dag (retires regen_parse_emit.rs 124 lines) Cumulative: 6-12 months optimistic, 12-18 realistic. Substrate-language work in Briefs 1-6 unblocks Phase 2/3/5 broader retirement paths beyond tokenize/parse alone. 5 open questions for operator at end of doc: 1. Brief 6 FFI vs self-hosted architectural choice 2. Spawn cadence (parallel vs sequential) 3. Test harness scope (in or out of this brief set) 4. Substrate-language work attribution to Phase 2/3/5 5. Worker parallelism cap (3-4 in flight realistic) Anti-paper-shrink discriminator applies to all 9 briefs: substrate-growth PR-by-PR is the receipt, not file-path shrinkage. * docs(briefs): add Path B Phase 0 worker briefs (1-3) — individual files for dispatched workers Workers witty-moth-725 (Brief 1), sunny-tern-495 (Brief 2), bright-swift-668 (Brief 3) auto-spawned under deep-wolf-155 per operator directive. The work- item titles were truncated and didn't carry full brief content; this commit provides the canonical single-file brief per worker. - docs/briefs/r3-path-b-brief-1-freemonoid-generic-methods.md Worker: witty-moth-725 Gap: dsl/std/algebra.dag:387-393 (FreeMonoid<T>.map endomorphic only) Estimated effort: 2-6 months substrate-language work - docs/briefs/r3-path-b-brief-2-string-templating-conversions.md Worker: sunny-tern-495 Gap: no format(template, args) + verify Int/Char/Bool → String primitives Estimated effort: 1-3 months - docs/briefs/r3-path-b-brief-3-char-class-structural.md Worker: bright-swift-668 Gap: tokenize.dag:23-58 named scaffold (char_in_class NYI for structural execution) Estimated effort: 1-2 months Each brief: - Investigation-first (Phase A) — surface findings to deep-wolf-155 before authoring - Concrete deliverables + acceptance criteria (substrate-fact-at-HEAD) - Anti-paper-shrink discriminator (substrate growth, not file-path manipulation) - Coordination protocol: direct dashboard-message to deep-wolf-155, no Director/Mgr layer - PR title prefix r3-path-b-brief-N for traceability * docs(r3): address codex + briansrls BLOCKING PR #3139 — align catalogue with 0-floor target + dissolve EmitTarget closed enum into LanguageSpec open registry Two valid BLOCKING findings: (1) codex REQUEST_CHANGES (catalogue.md:31, :52, :55) — "irreducible 10-15 file seed" + "Irreducible bootstrap seed (cannot retire)" framings CONTRADICTED `docs/design-pure-bootstrap-zero.md` (LIVE 2026-04-25 cascade promotion) which establishes the in-tree floor as ZERO. Authoritative text: - line 41: "Goal: zero hand-authored files in v3's source tree." - line 95: "the in-tree floor target is 0 regardless of which N=0 resolution lands" - line 191: "If first-time bootstrap (N=0) resolution requires hand-Rust in v3's source tree — STOP. The resolution is supposed to live outside v3's source tree (install script, gunbc-runtime crate, or rustc macro)." (2) briansrls INLINE BLOCKING (modeling-emit-rs.md:125) — proposed `type EmitTarget = Go | Rust | Python` closed sum REINTRODUCED a target roster that the live `src/v3/std/emit_model.dag` header (lines 8-16) explicitly dissolves via LanguageSpec references: "Adding a new shared target is a pure spec-file change — drop a `<target>.dag` with a LanguageSpec data item and reference it from each realization. No compiler enum roster to edit." Fixes: Catalogue (docs/r3-rust-retirement-catalogue.md): - §"5-phase plan" end-state: replaced "irreducible 10-15 file seed" with 0-floor target. Files that look irreducible (bootstrap drivers etc.) retire by MOVING OUT-OF-TREE per design-pure-bootstrap-zero.md N=0 resolution paths (install script / gunbc-runtime crate / rustc macro), NOT by accepting a permanent in-tree residual. - §"Format spec" per-entry "Retirement target" options: replaced "Irreducible bootstrap seed (cannot retire; document why)" with "Moved out-of-tree". Added explicit (N.B.) citing codex BLOCKING + design-pure-bootstrap-zero.md line 191: planning artifact does NOT have authority to introduce permanent in-tree carve-outs against the live 0-floor design. Modeling doc (docs/r3-retirement-modeling-emit-rs.md): - §4.1 substrate carriers: retracted `type EmitTarget = Go | Rust | Python` closed sum. Replaced `EmittedSource.target: EmitTarget` with `EmittedSource.language: DeclarationRef` matching the `TypeRealization.language: DeclarationRef` pattern at emit_model.dag:18. Removed `EmitTargetDispatchRow` separate roster carrier — LanguageSpec registry IS the roster. - §4.2/§4.3: dispatch reads LanguageSpec declarations from src/v3/spec/*.dag directly via declaration-namespace walk; functions now take `language: DeclarationRef` parameter. - §4.4 codegen driver: GENERATED Rust uses open-registry pattern too — no hardcoded `match EmitTarget { Go => ..., Rust => ..., Python => ... }` in emitted code; dispatches via DeclarationId lookup against the live LanguageSpec registry. - §6 open question #2: variant-arm exhaustiveness check replaced with declaration-namespace-walk-by-type-tag (open-registry concern). - §7 substantive retirement risks: paper-shrink discriminator updated (no closed enum reintroduction in generated Rust). Cross-target convenience wrappers collapse to ONE `emit_text(dag, language)` parameterized function. EmitMode = Program | Module stays as a closed sum (intentionally bounded; structurally distinct from target-roster concern). * docs(r3): address briansrls BLOCKING PR #3139 at catalogue.md:21 — expand Phase 1 monotonicity invariant to full hand-Rust surface (include src/v3/compiler/tests/**) briansrls inline BLOCKING at 2026-05-15T02:48:45Z flagged that Phase 1 scoped only src/v3/compiler/src/, leaving Rust tests outside the net-shrink gate even though INVARIANTS.md P5 + T-PB-B explicitly include Rust tests in the hand-Rust surface. Verified at INVARIANTS.md:323 (Dispatch-Discipline Mechanisms (b)): "Hand-Rust includes Rust tests (src/v3/compiler/tests/**) — these are the T-PB-B test subset of the SG-0 census; the gate applies the same way it applies to T-PB-A non-test files." Fix: Phase 1 monotonicity invariant now scopes the FULL hand-Rust surface (src + tests). Eliminates the loophole where gate-closure PRs adding Rust tests would skirt the invariant — which is exactly the growth pattern observed in the SG-0 trajectory data (TEST array went 122 → 130 → 131 across recent gate-closure PRs). * docs(r3): M2 / class-5 deferral debt survey (operator-requested 2026-05-15) Per operator directive: "Can we please find all instances of this happening in this project - i want to attack one of these problems specifically so we can gather some confidence." Survey enumerates the M2-deferred / class-5-blocked work in the project: Quantitative scope: - 119 references to "class-5" across docs/ + src/v3/ + dsl/ - 61 citations naming class-5 as a blocker / dissolution trigger - 161 ArrowBody::Unparsed std fn bodies (58% of 276 total) - 1 dedicated design doc (docs/design-m2-feature-parity.md, status "Design ready for implementer review") sitting unimplemented Enumeration: 1. M2 Feature Parity (DB-9 through DB-13, design doc ready): - DB-9: Mutual recursion lowering (L) - DB-10: data value semantics (S) — RECOMMENDED ATTACK POINT - DB-11: where refinement predicates (M) - DB-12: surface generics (S) - DB-13: Disj dotted-path (S) 2. Class-5 grammar gaps (substrate-language sub-program): - #1: Brace-bodied fn on .dag files (161 Unparsed std fns) - #2: Record literals inside data bodies - #3: Lens-fold prereq - #4: Variant-constructor expressions - #5: Dimension<C> data declarations (blocked on #2) - #6: Name rosters 3. Hand-Rust trampolines that exist BECAUSE class-5 isn't closed 4. Downstream symptoms (R3 retirement stalls, lens trampolines, etc.) Recommendation: DB-10 (data value semantics) as confidence-builder: - Smallest (size S per doc) - Substrate carrier already exists (Declaration.value_body at dag.rs:122) - Gap is downstream consumers, not substrate-shape change - Closes class-5 sub-gap #2 + unblocks dimension framework + cost-lens trampoline retirement - Estimated 1-2 weeks if design is accurate - Demonstrates "we can close M2 deferrals" pattern Alternative: class-5 brace-body parse for .dag files (bigger lever, more invasive; coordinates with Brief 3 worker bright-swift-668). * docs(r3-catalogue): clean up stale 'irreducible bootstrap seed' reference in Open Questions §5 (briansrls BLOCKING PR #3139 follow-through at catalogue.md:163) briansrls BLOCKING at catalogue.md:31 about the "irreducible 10-15 file seed" framing was already addressed in commit 6bafd12 (relay arrived after fix per dashboard typical relay-after-fix pattern). However, a stale reference to "irreducible bootstrap seed" remained at line 163 in the Open Questions section — reframed as "move out-of-tree per design-pure-bootstrap-zero.md N=0 resolution paths" with explicit no-permanent-residual disclaimer. The retraction at lines 31-38 + §"Format spec" (N.B.) at line 64 remains the canonical retraction text per the prior commit. * docs(briefs): add Path B Brief 4 — DB-10 data value semantics (M2/class-5 confidence-builder attack point) Per operator directive 2026-05-15 — "lets attack DB-10 for confidence, give it to one of the existing path B brief members". Brief 4 dispatches DB-10 (data value semantics, M2 feature parity item 3a.2) to witty-moth-725 — they closed Brief 1 cleanly (PR #3142 promoted to ready) and are available for new substrate-language work. Authoritative design at docs/design-m2-feature-parity.md §DB-10 (lines 20-71); doc carries "Design ready for implementer review" status since authoring. This is the FIRST attack against the M2 / class-5 deferral pit per the survey at docs/r3-m2-class-5-deferral-survey.md. Scope: - Phase A: verify design-doc cited locations (dag.rs:122, lower.rs:1404, test_3a2_data_field_access_resolves_statically test ratchet) against HEAD; surface drift findings to deep-wolf-155 - Phase B: land 3 consumers per design — Dag::data_value_at accessor + SurfaceExpr::Var fallback for scalar inlining + lower_field_path_expr walks ValueBody::Structural for static field access - Phase C: test fixture demonstrating data answer = 42 + answer + 1 emits 42 + 1 inline across all 3 target languages Architectural commitment honored (per design + PR #496 2026-04-17): inlining at LOWERING, not emission. Load-bearing per locked test ratchet. Emission-time inlining REJECTED. Coordination: parallel to PR #3142 merge (Brief 1 closure); separate branch; report findings directly to deep-wolf-155 (no Director/Mgr layer). Coordinate with sunny-tern-495 (Brief 2) + bright-swift-668 (Brief 3) only if substrate work overlaps. Anti-paper-shrink discriminator: locked test asserts no FieldProject Transform exists in the lowered DAG for data-resolved field accesses; that ratchet catches trampoline-style workarounds. Estimated effort: 1-2 weeks if design doc is accurate. * docs(r3): retract stale 'M2 not landed' framing — DB-10/12/13 verified LANDED at HEAD 2026-05-15 Per gentle-bat-24 Brief 4 Phase A finding (DB-10 substrate already implemented at HEAD with `Dag::data_value_at` at dag.rs:4310 + lower.rs:8489+/8642+/9125+ + passing test at m2_feature_parity_test.rs:771) + subsequent verification of DB-12/DB-13 status amendments in PR #496 against HEAD test names: DB-10: LANDED DB-12: LANDED (parse_generated.rs:927; m2_feature_parity_test.rs:90-156) DB-13: LANDED (lower.rs:3375; m2_feature_parity_test.rs:163-208) DB-9 + DB-11: HEAD landed-state NOT YET AUDITED; may also be silently landed Sites updated: - docs/r3-m2-class-5-deferral-survey.md: M2 feature parity table reframed with cited HEAD evidence per DB; explicit retraction of "no DB item has landed yet" claim; active deferral pits shrink to DB-9 + DB-11 (pending audit) plus class-5 sub-gaps which remain genuinely deferred per Brief 3 findings - docs/design-m2-feature-parity.md: top-level Status line updated from "Design ready for implementer review" to "3 of 4 LANDED" with per-DB cited HEAD evidence; explicit advisory that workers should audit HEAD state before authoring implementation against this doc Residual M2 work for the 3 LANDED items: emission-test-coverage gap for DB-12 + DB-13 (design §Acceptance promises 3-target render verification; current tests check compile + lower only). Small follow-up scope; not pit-shaped. The "M2 deferral pit" framing was substantially wrong as initially described — much of M2 has been silently landing. The class-5 sub-gaps remain genuinely deferred per Brief 3's bright-swift-668 investigation (brace-body fn parse on .dag files). Per `feedback_corrections_must_grep_verify_source` — this is the canonical lesson: design docs drift; verify against HEAD before authoring against them. * docs(r3): address codex BLOCKING PR #3139 (review #12458) — retract String-typed EmitDispatchError + retract Brief 6 FFI bridge as deliverable path Two findings, both valid: (1) modeling-emit-rs.md:203 — proposed `type EmitDispatchError { language: DeclarationRef, detail: String }` as 🟡 SCAFFOLD. RETRACTED. INVARIANTS P3 + docs/modeling-discipline.md Practice 1 forbid String-typed diagnostic channels at the substrate-API level. SCAFFOLD-with-String at the API level is still a String-API at HEAD; the scaffold marker doesn't excuse the typed-carrier requirement. Fix: defer the EmitDispatchError shape entirely until per-language typed error carriers land (Brief 7+). Dispatcher signature stays Result<EmittedSource, ???> with ??? as forward-reference. Workers attempting Brief 7+ surface back if the error-carrier shape becomes load-bearing before per-language errors exist — substantive substrate-language question, not papered over with String channel. (2) brief-set.md:206 — Brief 6 presented an "FFI bridge" option (a) as a deliverable path. The brief text itself admitted "does not retire the Rust compile_to_dag function itself" — that's a bridge preserving hand-Rust compiler authority, NOT retirement. Operator's NO WORKAROUNDS stance + INVARIANTS P5 forbid FFI as deliverable path. The brief currently muddled this; codex correctly flagged that the wording would send a worker to execute the wrong work. Fix: Brief 6 reframed as SCOPE MARKER ONLY (Phase 5 program, NOT dispatchable within this brief set). Recommendation surfaced to operator: declare Phase 5 / meta-circular bootstrap as separate program from Path B; tokenize/parse codegen-driver retirements (Briefs 7-9) are downstream of Phase 5 and not landable until self-hosted compile_to_dag lands. Honest framing: forcing Briefs 7-9 to land before Phase 5 via FFI bridges produces paper-shrink, not retirement. Sites updated: - modeling-emit-rs.md §4.3 EmitDispatchError block: explicit RETRACTION text; signature now Result<EmittedSource, ???> with forward-reference framing - brief-set.md §"Brief 6 — Meta-circular: compile_to_dag Foreign-Function Bridge" retitled to "self-hosted compile_to_dag (Phase 5 scope expansion)" + scope reframed as SCOPE MARKER ONLY - brief-set.md dispatch graph: Brief 6 marked NOT DISPATCHABLE - brief-set.md §Brief 7 prerequisites: Briefs 1-5 + Phase 5 separately - brief-set.md §Brief 7 scope: cites separately-scoped Phase 5 program - brief-set.md §"Open questions for operator before dispatch" #1: RESOLVED per this codex BLOCKING; no FFI workaround acceptable * WIP: Path B Brief 4 — M2 DB-10 data value semantics (data foo: T = v usable a * docs(r3): finish typed emit error retraction
1 participant
Summary
Implements four of the five M2 feature-parity sub-stages from DB-10..DB-13 and DB-9. 3a.3 full semantics and 3a.1 mutual recursion are deferred as size-class overruns — tracking issues below.
3a.4 surface generics (DB-12) — tests-only
Investigation found the parser (
parse_optional_type_params) and lowering (lower.rs:306-323) already wire surface<T, U>through toDeclaration.type_params. No code change was required. DB-12 in the design doc has been amended to reflect this. New acceptance tests lock in:fn id<T>(x: T) -> T = xcompiles.fn pair<A, B>(a: A, b: B) -> A = acompiles.fn bound<T: Ord>(x: T) -> T = xrejected at parse time.3a.5 Disj dotted-path (DB-13) — tests-only
Match-arm body lowering (
lower.rs:3375) already extendsarm_scopewith pattern bindings, andlower_field_path_expralready consults that scope. No code change was required. DB-13 in the design doc has been amended to reflect this. New acceptance tests lock in:match opt { Some(p) => p.x, None => 0 }compiles and lowers to aFieldProjectTransform.match w { Wrap(o) => o.inner.a, Empty => 0 }compile.3a.2 data value semantics (DB-10) — full
ValueBody::Scalar(FieldValue)variant for scalar data (data answer: Int = 42).parse_data_itemaccepts non-{-shaped RHS as a scalar expression.lower_data_itemroutes scalar literals toValueBody::Scalarvia existinglower_scalar_literal_for_type.Dag::data_value_at(id)accessor reads a declaration's compile-time value.SurfaceExpr::Varfallback inlinesValueBody::Scalaras aValue(LiteralBits)node when the name is not in local scope —fn f() -> Int = answeremits the literal inline without emit-side wiring.lower_field_path_exprfallback: when the head ident is a data declaration withValueBody::Structural, walk the compile-time fields;cfg.hostresolves to the record literal's value.Recordfield values supported viaresolve_structural_field_path.Implementation amendment recorded in DB-10. The design originally specified emit-time inlining (3× emit-file changes). The landed implementation does lowering-time inlining instead — simpler plumbing, lossier provenance (the DAG carries
Value(42)rather than a namedanswerreference). Trade-off is acceptable forcompiler.dag's use case;Dag::data_value_atremains available for any future emission-time consumer. See DB-10's "Implementation amendment" section indocs/design-m2-feature-parity.md.3a.3
whererefinement (DB-11) — foundation onlywhere <expr>after a parameter type (SurfaceParam.refinement: Option<SurfaceExpr>).Declaration.refinement: Option<DeclarationId>edge (~15 Declaration construction sites touched to setrefinement: None; documented as scaffold indag.rs's docstring).fn div(n: Int, d: Int where d != 0) -> Intparses without dropping the clause.Deferred — tracked by #498 (size overrun M → L): predicate lowering to an attached
Declaration, call-site structural-DAG check, Branch-arm predicate narrowing extension to M1(2.8).3a.1 mutual recursion (DB-9) — deferred
Full DB-9 implementation (SCC detection + cluster descent + bottom-up SCC inference + test flip) tracked by #499. L-sized; kept independent to preserve this PR's review budget.
Follow-up tracking
Preserved invariants
Behaviorvariant. Substrate still has exactly five behaviors —Value | Transform | Branch | Loop | Bind.Declarationis structural (value_bodyalready existed;refinementjoins it, with an explicit scaffold marker pointing at its dissolution trigger).Test plan
cargo test --workspace --exclude v2-compiler-tests— all green (including newm2_feature_parity_test.rs).cargo clippy --all-targets -- -D warnings— clean.cargo fmt --all— clean (second commit in this PR).Commits
Lane 3 Stage 3a partial — 3a.2 data values, 3a.4/5 tests, 3a.3 foundationstyle: cargo fmt --all(formatting-only)docs: DB-10/12/13 amendments + scaffold marker on Declaration.refinement🤖 Generated with Claude Code