Skip to content

docs(design): Prereq-X audit — HO field-call for fold_lens<C> - #1264

Merged
briansrls merged 141 commits into
mainfrom
session/tidy-wolf-507
Apr 30, 2026
Merged

briansrls merged 141 commits into
mainfrom
session/tidy-wolf-507

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Summary

Audit/planning PR — no code, no parser edits, no fold_lens implementation. Director-approved option (b) on parent inbox #1130 (2026-04-30) after the fold_lens<C> HO field-call smoke confirmed v3 surface grammar does not support call-on-field-access.

The deliverable is docs/design-prereq-x-ho-field-call.md. It records exact parse failures, splits the prerequisite into three implementation slices, and maps each to fold_lens<C> and lens-instance consumer dispatch shapes.

Smoke evidence

Four shapes tested via cached_compile_to_dag against current origin/main post-Prereq-1 / Prereq-2 / Prereq-3a. All four fail:

  • S1 w.f(x) — parse error: expected let/fn/type/module/import/data, got LParen. Field-call grammar absent.
  • S2 (w.f)(x) — parse error: expected primary expression, got LParen. Parenthesized callee not in primary position.
  • S3 let g = wrap_double.f; g(x) — semantic errors: data field projection requires "compile-time value"; g doesn't resolve.
  • S4 fn r(...) -> T = { let g = ...; g(x) } — parse error: expected field label, got KwLet. Brace-after-= is record literal, not block expression.

Three implementation slices

  • Prereq-X1 (call-on-field-access dispatch): generalize call grammar from <ident>(<args>) to <expr>(<args>) for Arrow-typed <expr>. Primary case: lens.read(d, b).
  • Prereq-X2 (call-on-Var): likely implicit in X1 if call-callee position generalizes; flagged for Director.
  • Prereq-X3 (brace-block expressions inside = bodies): block-vs-record-literal disambiguation. Two strategies presented (lookahead at first non-WS token vs explicit do { ... } keyword). Director call.

Mapping to fold_lens

Every Lens instance dispatch path is a call-on-field-access:

  • lens.read(d, b) — X1
  • lens.sequential.op(a, b) — X1 with two-level field projection
  • lens.branch(a, b) / lens.iterate(body, bound) / lens.validate(d, c) — X1

fold_lens<C> cannot be authored without X1. The lens-fold-prerequisites audit at docs/design-lens-fold-prerequisites.md (PR #1207) conflated field assignment (Prereq-1, landed) with field invocation (Prereq-X, missing); the audit doc lays out the correction.

Out of scope

  • No parser, lowerer, or emitter modifications.
  • No fold_lens<C> authoring (blocked on Prereq-X).
  • No Lens instance authoring or PROXY/STUB lens migration.
  • No commitment to X3 disambiguation strategy (a) vs (b) — Director call.

Test plan

  • Audit doc lands at docs/design-prereq-x-ho-field-call.md with exact parse-failure messages and acceptance test matrix per slice.
  • Cross-references existing prereqs and the lens-fold-prerequisites audit.
  • Director routing for X1 implementation slice (and X3 disambiguation strategy).

🤖 Generated with Claude Code

briansrls and others added 30 commits April 28, 2026 23:33
# Conflicts:
#	src/v3/compiler/src/bootstrap_generated.rs
#	src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
- Add method_template_contract_test.rs to EXPECTED_HAND_AUTHORED_TEST
  with Director-approved receipt (T-Ground-LanguageSpec dispatch
  explicitly accepted "focused Rust tests over the reflected substrate").
- Refresh parse_corpus_manifest.txt entry for src/v3/std/emit_model.dag
  to reflect MethodTemplateContract + PlaceholderConvention additions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts:
#	src/v3/compiler/src/bootstrap_generated.rs
#	src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
Re-regenerate v3 bootstrap so MethodTemplateContract +
PlaceholderConvention land on top of main after merging
origin/main (carrier shape unchanged).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First substrate slice for the lens framework
(docs/design-lens-framework.md, docs/briefs/r2-substrate-manager.md).
Director-locked option (c) on parent inbox #1130.

Substrate changes:
- New src/v3/std/lens.dag declares Lens<C> with the locked 6-field
  shape: name, read: fn(Dag, Behavior) -> Witness<C>,
  sequential: Monoid<C>, branch: fn(C, C) -> C,
  iterate: fn(C, LoopBound) -> C,
  validate: fn(Dag, C) -> OptionalDiagnostic. Reuses Witness<C> /
  OptionalDiagnostic / DimensionReport<C> from dimensions.dag and
  Monoid<C> from dsl/std/algebra.dag — no parallel reps introduced.
- diagnostics.dag: Q6.5 two-layer authority. Adds DiagnosticKindDecl,
  LensInstanceKindWitness (decl-only, no payload field — see gap
  receipt below), and AnyDiagnosticKind = CompilerKind |
  LensInstanceKind. Widens Diagnostic.kind from CompilerDiagnosticKind
  to AnyDiagnosticKind. CompilerDiagnosticKind closed sum unchanged
  (anti-bridge invariant).

Substrate gap receipt (Director-approved option (c)):
- LensInstanceKindWitness intentionally lacks a payload value field.
  Today's .dag grammar cannot express
  `payload: <inhabits kind_decl.payload>` (refinement-type-on-sibling-
  field). The flat alternative ratifies the illegal-state Q6.5
  rejected (Lens / name / payload-shape three independent coords).
  Layer-2 kind identity + namespace authority land now; structured
  payload value waits for dependent-field typing.

Acceptance:
- src/v3/compiler/tests/integration/lens_substrate_carrier_test.rs:
  Lens<C> 6-field shape, Diagnostic.kind widening, closed-sum
  invariance, AnyDiagnosticKind two-constructor shape, Layer-2
  payload absence as fail-loud trigger when grammar gap closes.
- SG-0 ratchet receipt added with Director acceptance citation.
- parse_corpus_manifest.txt refreshed via
  refresh_handwritten_parse_snapshot_manifest -- --ignored.

Out of scope (deferred to subsequent lanes):
- Migration of cost.dag / complexity.dag / idempotency.dag /
  parallelism.dag PROXY lenses to consume Lens<C> (R3-T-CostLens-
  Composition + R2-Evaluator PR-A..E).
- fold_lens<C> generic fold machinery (I2 in design doc).
- User-authored lens TestClaim wiring (I7 in design doc).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Strengthen LensInstanceKindWitness SCAFFOLD comment to call out that
bare `DeclarationRef` for `kind_decl` is part of the SAME dissolution
trigger as the deferred payload typing — substrate-level
refinement-typing-on-DeclarationRef closes both the payload-typing
gap and the kind-decl resolution gap in one move. Cites the analogous
PatternRealization and MethodTemplateContract.dag_method patterns.

Addresses non-blocking codex BLOCKING relay at sha fa5bba2 (Layer-2
diagnostic-kind witness leaving its core authority unconstrained) —
shape unchanged per Director-locked option (c) on parent inbox #1130;
just makes the bounded-scaffold receipt fully explicit on this row.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts:
#	src/v3/compiler/src/bootstrap_generated.rs
#	src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
Re-regenerate v3 bootstrap so Lens<C> + Q6.5 widening land on top of
latest main after the merge conflict resolution. Refresh parse
manifest. Carrier shape unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts:
#	src/v3/compiler/src/bootstrap_generated.rs
#	src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
Re-regenerate v3 bootstrap so Lens<C> + Q6.5 widening land on top of
main after #1188 fixed the v2-extdeps regression. Refresh parse
manifest. Carrier shape unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Addressed in 8dbdfed. Reviewer was right: pub enum with named-field variants exposes those fields publicly, so 'crate-private fields' was a false claim — outside callers could still write TransformDispatch::Callable { callee, args } literals.

Replaced the named-field variants with tuple-struct payloads:

pub enum TransformDispatch {
    Callable     (CallableDispatch),
    FieldProject (FieldProjectDispatch),
    Operator     (OperatorCall),
    Indirect     (IndirectDispatch),
}

pub struct CallableDispatch {
    pub(crate) callee: DeclarationId,
    pub(crate) args:   Vec<PortId>,
}
// + analogous FieldProjectDispatch / IndirectDispatch

Outside the dag module, pub(crate) fields make literal construction a compile error; the only path is the Dag builder. Read access is via accessor methods. OperatorCall stays a plain pub enum because its variants witness no signature — no proof to protect.

— sent from tidy-wolf-507

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: 7ce098ff · Trigger: schedule
  • Comparison: origin/main @ d38e3291 ... review/pr-1264-7ce098ff @ 7ce098ff
  • Thinking: 54s wall

Findings

  • docs/design-prereq-x-ho-field-call.md:243 / :255 — The proposed payload structs use pub(crate) fields, but the text at :280 says this prevents construction outside the dag module. In Rust, pub(crate) permits any module in the crate to construct CallableDispatch { ... } / IndirectDispatch { ... }, bypassing the Dag builder that is supposed to bind args to the target signature. That violates modeling-discipline API-level enforcement / illegal-states-unrepresentable. Use private fields scoped to the defining module, or another sealed construction pattern, if the builder is meant to be the only construction path.

Verdict: REQUEST_CHANGES. The audit is otherwise well-scoped, but this visibility mismatch undermines the central structural guarantee the design is locking.

…b(crate))

Reviewer caught: pub(crate) fields permit any in-crate module to
construct CallableDispatch { ... } literally, bypassing the Dag
builder that binds args to target. Switched to module-private (no
visibility modifier) so only code inside the dag module can construct
the payloads. Aligns ArrowPortRef precedent (already module-private).
@briansrls

Copy link
Copy Markdown
Contributor Author

Addressed in 76a65b2. Reviewer was right — pub(crate) is crate-wide, so any module in the crate could literal-construct CallableDispatch { ... } and bypass the Dag builder. Switched payload fields to module-private (no visibility modifier; private to the dag module only), matching the ArrowPortRef(/* private */ PortId) precedent already in the doc. Outside the dag module the literal construction is now a compile error; the only public path remains the Dag builder.

— sent from tidy-wolf-507

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-thinking
  • Commit: 76a65b29 · Trigger: manual
  • Comparison: main @ d38e3291 ... session/tidy-wolf-507 @ 76a65b29
  • Conversation: View conversation

1. Story of the diff

This PR adds one audit document, docs/design-prereq-x-ho-field-call.md, for the missing .dag surface needed before fold_lens<C> can call Lens fields. The doc starts from four smoke failures around w.f(x), (w.f)(x), field projection through data, and brace-block expression bodies, then decomposes the missing work into call-on-field-access, call-on-Arrow-valued variables, and explicit block expressions. Its load-bearing design move is not just parser grammar: it proposes collapsing TransformNode.target + TransformNode.inputs into a typed TransformDispatch carrier so indirect/runtime callees flow as first-class dependencies rather than positional conventions. It also records acceptance tests for the future implementation slice and explicitly says this PR is the audit deliverable only: no parser, lowerer, emitter, or test changes land here (docs/design-prereq-x-ho-field-call.md:624-628).

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Finding — BLOCKING, Modeling Faithfulness / coproduct dissolution. The diff is a docs-only PR, but the doc proposes substrate shape for TransformDispatch, so substrate discipline applies. The OperatorCall branch is described as constructable “from primitives without target resolution” and then classified as terminal GREEN because “operator dispatch is a true user-input-boundary distinction” and “callee identity is a fixed enum of language primitives” (docs/design-prereq-x-ho-field-call.md:270-278, docs/design-prereq-x-ho-field-call.md:403-407). That conflicts with the modeling discipline’s algebraic-form dissolution pattern, whose explicit example says ArithOp::Add | Sub | Mul | Div should become Apply { function: FunctionRef } pointing at std::int::add, etc. chatgpt-review-5d3f57fc-15e0-4d…

It also conflicts with the invariant example that operator symbols come from algebra field declarations, not a target/private enum table. chatgpt-review-1bfa188d-313b-4e…

At minimum, Operator cannot be marked 🟢 terminal without explaining why these specific operators are irreducible user-input boundary facts rather than algebraic calls; otherwise the audit locks a substrate enum that the project’s own examples treat as dissolvable.

  1. INVARIANTS.md + modeling-discipline.md.

Finding — BLOCKING, P1 Modeling Faithfulness / Practice 4 Coproduct dissolution. The document does include a dissolution ledger for TransformDispatch (docs/design-prereq-x-ho-field-call.md:399-433), which is the right kind of evidence, but the ledger’s GREEN classification for Operator is unsupported. GREEN requires “no richer source exists” and a written record of attempted dissolution patterns. chatgpt-review-5d3f57fc-15e0-4d…

The diff’s rationale is only that no DeclarationId currently exists for + / - / unary ! (docs/design-prereq-x-ho-field-call.md:403-411); absence of a current DeclarationId is not the same as “no richer source exists,” especially when the documented dissolution route is to reference the relevant std/ function or algebra witness, not synthesize a fictional declaration. The Callable / FieldProject / Indirect YELLOW classification is otherwise well tracked with a named trigger: dissolve when emitter callee-rendering splits from the dispatch match (docs/design-prereq-x-ho-field-call.md:413-427).

  1. CODING.md.

Compliant. Although no Rust code changes land, the proposed API shape follows the coding preference for structured carriers and typed handles: ArrowPortRef wraps a PortId only after validation (docs/design-prereq-x-ho-field-call.md:335-340), and TransformDispatch::input_ports() becomes the named dependency accessor instead of callers walking raw inputs conventions (docs/design-prereq-x-ho-field-call.md:341-344).

  1. TESTING.md.

Compliant. No tests are required in this audit-only PR because it explicitly does not implement behavior (docs/design-prereq-x-ho-field-call.md:624-628). The future test matrix is at the right level for the future implementation: parser/lowerer/emitter behavior for call-on-field projection, parameter-field calls, nested field calls, and non-Arrow callee diagnostics (docs/design-prereq-x-ho-field-call.md:492-501), which matches the testing guidance that parser/lowering/inference behavior can legitimately use compile-pipeline fixtures. chatgpt-review-27e2f60c-7fca-4c…

  1. LOCKED DESIGN DECISIONS.

Compliant. The doc explicitly records the Director-locked explicit block syntax decision for X3 (docs/design-prereq-x-ho-field-call.md:536-556) and repeats that the audit locks the explicit-marker discipline rather than a specific token (docs/design-prereq-x-ho-field-call.md:554-556). I do not see an unacknowledged divergence from a locked design decision in this diff.

  1. TRACKED vs UNTRACKED DEBT.

Finding — BLOCKING, tracked debt / dissolution trigger. The Callable / FieldProject / Indirect temporary split is tracked: documentation, bounds, and dissolution trigger are present (docs/design-prereq-x-ho-field-call.md:413-427). The Operator case is the problem: by marking it 🟢 permanent (docs/design-prereq-x-ho-field-call.md:403-411) and then saying no variant is dissolve-now (docs/design-prereq-x-ho-field-call.md:429-433), the audit creates no debt row or trigger for the operator enum even though the modeling docs identify arithmetic-operator enums as a canonical algebraic-form dissolution case. chatgpt-review-5d3f57fc-15e0-4d…

Either reclassify Operator as YELLOW with a concrete dissolution trigger, or add a real GREEN ledger explaining why this operator set is not the ArithOp-style case.

3. Verdict

REQUEST_CHANGES. The audit is otherwise careful and valuable, especially around Indirect as a typed runtime-callee dispatch, but it currently blesses OperatorCall as terminal substrate shape in direct tension with the repo’s own coproduct-dissolution examples. Because this is a substrate-design document, fixing the classification before merge is cheaper than letting the implementation slice inherit a permanent enum authority.

… example

Modeling-discipline Practice 4's canonical example is ArithOp →
Apply { function: FunctionRef } pointing at std::int::add. OperatorCall
is structurally that case; absence of a current DeclarationId for + / -
/ unary ! is not the same as 'no richer source exists.' Tracking gate:
std/{int,bool,float}/ declaring operator-algebra witness functions and
parser desugaring operator tokens to Call(FunctionRef).
@briansrls

Copy link
Copy Markdown
Contributor Author

Addressed in 6bf392f. Reviewer is right — OperatorCall matches the canonical ArithOp → Apply { function: FunctionRef } dissolution example in modeling-discipline Practice 4. Reclassified from 🟢 to 🟡 with a concrete tracking gate:

Dissolve when std/{int,bool,float}/ declares the operator-algebra witness functions (std::int::add, std::bool::not, …) and the parser desugars operator tokens to Call(FunctionRef) at parse time. At that point Operator collapses into the same Call { callee: CalleeRef = Decl(...) } shape as the rest of the call-shapes.

The "absence of a current DeclarationId" reasoning was wrong as a 🟢 justification — it conflated not yet declared with no richer source exists. The richer source is the std-grounded algebra witness, which is exactly the dissolution route the modeling docs prescribe. 🔴 stays None.

— sent from tidy-wolf-507

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 5c53727b · Trigger: schedule
  • Comparison: origin/main @ 68c1b9cc ... review/pr-1264-5c53727b @ 5c53727b
  • Thinking: 72s wall

Findings: None. The diff only adds docs/design-prereq-x-ho-field-call.md. It is explicitly AUDIT / “does not author code,” ties smoke results to current failures, and keeps proposed substrate shapes as design (not described as live). That matches INVARIANTS “documentation describes live state” intent: it does not present unimplemented behavior as shipped. docs/modeling-discipline.md Practice 4 is honored in-doc: 🟢/🟡/🔴 ledger plus named dissolution triggers for the 🟡 Operator and unified-call future work. CODING.md / TESTING.md do not apply to this docs-only deliverable in a way that creates a violation (no new Rust or tests claimed as done).

Verdict: APPROVE — Narrow, Director-scoped audit with explicit non-scope, cross-links, and test-matrix hooks for follow-on work; no rubric violations spotted in the diff.

Exploratory (optional): The large Rust sketch uses tuple variants like Indirect(IndirectDispatch) (e.g. around docs/design-prereq-x-ho-field-call.md:239), while the emitter sketch uses TransformDispatch::Indirect { callee, args } (around docs/design-prereq-x-ho-field-call.md:463). Harmonizing those two snippets would reduce implementer confusion; not a policy violation.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 5c53727b · Trigger: schedule
  • Thinking: 313s wall

BLOCKING (2)

Root Cause

  • docs/design-prereq-x-ho-field-call.md The proposed substrate lacks a clean separation between field projection and field invocation → preserve projection as a one-carrier dispatch fact, then invoke Arrow-valued projections via an Indirect/Call shape whose dependency iterator includes callee and arguments.

Non-blocking — Strengths

  • docs/design-prereq-x-ho-field-call.md Classified as design/docs; the audit now cleanly separates Lens field assignment from consumer-side invocation and names concrete acceptance cases for X1/X2/X3.

Non-blocking — Improvements (fix in-PR if easy, else defer to roadmap)

  • docs/design-prereq-x-ho-field-call.md The emitter-contract snippet should use tuple payload matches/accessors instead of struct-variant destructuring so it matches the private-payload API; defer to the Prereq-X1 implementation lane if not fixed here.

⚠️ Two substrate-shape issues remain around FieldProject dependencies and projection-vs-invocation before this audit should lock the implementation target.

/* private */ field_label: String,
/* private */ field_child: Option<DeclarationId>,
/* private */ carrier: PortId,
/* private */ args: Vec<PortId>,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING: FieldProjectDispatch adds args: Vec<PortId> to a substrate form that current code uses for one-carrier Conj projection, collapsing plain field access and field invocation into one state family (illegal states unrepresentable / modeling faithfulness).

Comment thread docs/design-prereq-x-ho-field-call.md Outdated
impl TransformDispatch {
/// Single-authority dependency walk for Facts Flow Forward.
/// For `Indirect`, yields the callee port first, then args.
/// For other variants, yields args only (callee identity is

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING: input_ports() excludes non-Indirect runtime inputs, but FieldProjectDispatch carries carrier: PortId, so reflected consumers would miss the carrier dependency (Facts Flow Forward / Every Dependency Is A Substrate Fact).

@briansrls

Copy link
Copy Markdown
Contributor Author

Stale — dissolution ledger has been in the doc since 783fabdad and was tightened in 6bf392fdc (Operator reclassified 🟢→🟡). Current head 5b8668c24 has the full enum-level ledger at lines 399–444:

  • 🟡 Operator { op: OperatorCall } — future-dissolve to Apply { function: FunctionRef } per ArithOp canon; tracking gate: std-grounded operator-algebra witnesses + parser desugar.
  • 🟡 Callable / FieldProject / Indirect — future-collapse to Call { callee: CalleeRef, args }; tracking gate: emitter callee-rendering split.
  • 🔴 None.

— sent from tidy-wolf-507

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: 5b8668c2 · Trigger: schedule
  • Comparison: origin/main @ 68c1b9cc ... review/pr-1264-5b8668c2 @ 5b8668c2
  • Thinking: 56s wall

Findings:

  • NON-BLOCKING: docs/design-prereq-x-ho-field-call.md:482 says the Indirect variant is permanent, but line 424 classifies Callable / FieldProject / Indirect as 🟡 future-dissolve into Call { callee: CalleeRef, args }. That muddles the tracked-debt/dissolution story from INVARIANTS.md P5 / modeling-discipline Practice 4. I’d clarify whether the HO dispatch capability is permanent while this specific variant spelling is transitional.

Verdict: APPROVE_WITH_COMMENTS. The audit is otherwise narrowly scoped, docs-only, and it names the substrate target, typed construction path, fail-closed tests, and dissolution gates clearly.

…ocation)

Reviewer caught: collapsing plain field access and field invocation
into one variant with optional args admits a malformed state where
projection has args or invocation has none. Different state families
should be different variants.

- FieldProject preserves current TransformTarget::FieldProject shape
  (no args; pure value access) — 🟢 keep.
- FieldCall is the new X1 variant — projection-then-call; always has
  args. 🟡 future-collapses with Callable/Indirect into Call{CalleeRef}.

Builders split: push_field_project_transform vs push_field_call_transform.
@briansrls

Copy link
Copy Markdown
Contributor Author

Addressed in e03703e. Reviewer is right — FieldProjectDispatch { ..., args: Vec<PortId> } collapsed two distinct state families (plain projection vs project-then-call) under a single variant where the only distinguisher was args-emptiness.

Split into two variants:

  • FieldProject(FieldProjectDispatch) — pure value access; no args. Preserves the current TransformTarget::FieldProject shape exactly. 🟢 in the ledger.
  • FieldCall(FieldCallDispatch) — Arrow-typed-field projection followed by call; always has args. New for X1. 🟡 (future-collapses with Callable / Indirect into Call { callee: CalleeRef, args }).

Builder surface split accordingly: push_field_project_transform (no args param) vs push_field_call_transform (raw_ports param, validated against the field's Arrow signature).

— sent from tidy-wolf-507

Reviewer caught: FieldProject.carrier and Operator(Unary/Binary)
operand ports are runtime deps; input_ports() must yield every
runtime PortId across all variants for Facts Flow Forward to hold.
Documented per-variant enumeration explicitly.
@briansrls

Copy link
Copy Markdown
Contributor Author

Addressed in 02f04e7. Reviewer is right — FieldProject.carrier and Operator(Unary/Binary) operand ports are runtime port deps; an input_ports() that only yielded Indirect.callee + args would silently drop them and break Facts Flow Forward.

Documented the full per-variant enumeration:

  • Callable(d) — d.args.
  • FieldProject(d) — d.carrier.
  • FieldCall(d) — d.carrier, then d.args.
  • Operator(Unary{arg}) — arg.
  • Operator(Binary{lhs,rhs}) — lhs, then rhs.
  • Indirect(d) — d.callee (wrapped ArrowPortRef's port), then d.args.

DeclarationId callees are resolved at lowering time, not runtime ports, so they are not yielded — that distinction is now stated explicitly.

— sent from tidy-wolf-507

@briansrls

Copy link
Copy Markdown
Contributor Author

Stale — review ran against 5c53727b; head is 02f04e774. All three items already addressed:

Blocking #1 (projection vs invocation): done in e03703e35. Split into two variants — FieldProject(FieldProjectDispatch) (no args; pure value access; preserves current TransformTarget::FieldProject shape; 🟢 in ledger) and FieldCall(FieldCallDispatch) (project-then-call; always has args; 🟡 future-collapses into Call{CalleeRef} with Callable/Indirect). Different state families, different variants.

Blocking #2 (input_ports must include callee + carrier + args): done in 02f04e774. Documented full per-variant enumeration:

  • Callable → args; FieldProject → carrier; FieldCall → carrier + args; Operator(Unary) → arg; Operator(Binary) → lhs + rhs; Indirect → callee port + args.

Non-blocking (emitter snippet uses tuple form): done in 5b8668c24. Emitter snippet now uses TransformDispatch::Indirect(d) => render_indirect_call(d.callee(), d.args(), ...) etc., matching the tuple-payload API.

— sent from tidy-wolf-507

…ect variant transitional'

Reviewer flagged: 'Indirect is permanent' (line 482) muddled with
'Callable/FieldCall/Indirect 🟡 future-dissolve' (line 424). Split into
two claims: the capability is permanent (some variant must carry HO
dispatch), but the specific variant spelling 'Indirect(IndirectDispatch)'
retires when the 🟡 collapse to Call{CalleeRef::Port} lands.
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 6d7b4b01 · Trigger: schedule
  • Comparison: origin/main @ 9e696d3d ... review/pr-1264-6d7b4b01 @ 6d7b4b01
  • Thinking: 63s wall

Findings: None. The only change vs origin/main is adding docs/design-prereq-x-ho-field-call.md (717 lines). It is explicitly an AUDIT/deliverable (“does not modify the parser, lowerer, or emitter”), frames smoke results and proposed shapes as forward work, and ties 🟡/🔴 items to named dissolution triggers (e.g. operator → algebra witnesses, Callable/FieldCall/Indirect → unified Call + emitter split), which matches the scaffold / coproduct discipline in docs/modeling-discipline.md rather than contradicting it. Nothing in this diff touches executable substrate, Rust style (CODING.md), or test code (TESTING.md).

Verdict: APPROVE — Single scoped design doc; no rubric violations observed in the diff.

@briansrls
briansrls merged commit b9504ab into main Apr 30, 2026
4 checks passed
@briansrls
briansrls deleted the session/tidy-wolf-507 branch April 30, 2026 17:33
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: 6d7b4b01 · Trigger: manual
  • Conversation: View conversation

1. Story of the diff

This PR adds a single design audit, docs/design-prereq-x-ho-field-call.md, documenting why fold_lens<C> cannot currently be authored: lens consumers need to call Arrow-typed fields such as lens.read(...), but today the parser/lowerer only support narrower call positions. The audit records four failing smoke shapes, splits the prerequisite into X1 field-call dispatch, X2 Arrow-typed local/parameter calls, and X3 explicit block expressions, then proposes a substrate-facing refactor from TransformNode { target, inputs } to a single TransformDispatch authority. The load-bearing design choice is that higher-order calls need a typed runtime-callee path without losing dependency visibility, while preserving fail-closed parsing for block-vs-record ambiguity through an explicit do { ... } marker.

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Finding — BLOCKING, substrate design / single authority. The audit introduces FieldCall as a direct project-then-call substrate variant — docs/design-prereq-x-ho-field-call.md:248 says FieldCall (FieldCallDispatch), // project-then-call — new for X1, and docs/design-prereq-x-ho-field-call.md:267-274 defines it as { field_label, field_child, carrier, args }. But the sequencing section then says the runtime-sourced case for lens.read(...) lands with TransformDispatch::Indirect(...) and that “IndirectCall substrate extension is the actual unblocker” at docs/design-prereq-x-ho-field-call.md:556-571. Those are two different encodings for the same field-call surface unless the design explicitly chooses either direct FieldCall or normalized FieldProject → Indirect. As written, the implementation worker could create parallel substrate representations for lens.read(...), violating the layer-model single-authority bar before the substrate shape is implemented.

  1. INVARIANTS.md + modeling-discipline.md.

Finding — same blocking issue under Boundary Discipline / No Parallel Representations. The diff correctly identifies the anti-pattern at docs/design-prereq-x-ho-field-call.md:208-216 by rejecting a cross-field positional convention, but then reintroduces an authority split between FieldCall and Indirect for runtime field invocation. The fix should be mechanical in the doc: either make FieldCall the X1.b representation for parameter-carrier field calls and reserve Indirect for already-materialized Arrow ports such as g(x), or delete FieldCall and state that every field invocation lowers through a pure projection that yields an ArrowPortRef followed by Indirect.

  1. CODING.md.

Compliant. The proposed Rust shape uses typed carriers and explicit Result-returning construction surfaces, e.g. push_callable_transform(...) -> Result<NodeId, CallShapeError> and push_indirect_transform(...) -> Result<NodeId, CallShapeError> at docs/design-prereq-x-ho-field-call.md:317-335, plus resolve_arrow_port(...) -> Result<ArrowPortRef, NonArrowPortError> at docs/design-prereq-x-ho-field-call.md:349-359; that matches the “structured output / typed handle / fail-closed boundary” style.

  1. TESTING.md.

Compliant. This is explicitly an audit-only PR with no parser/lowerer changes, and it says so at docs/design-prereq-x-ho-field-call.md:692-699; adding runnable tests here would be premature. The future implementation acceptance matrix is behavior-driven and regression-shaped: T1.1–T1.4 cover field-call behavior and the non-Arrow diagnostic at docs/design-prereq-x-ho-field-call.md:573-577, T2.1–T2.2 cover Arrow-typed locals/parameters at docs/design-prereq-x-ho-field-call.md:593-595, and T3.1–T3.3 cover explicit block syntax and the fail-closed { ... } rejection at docs/design-prereq-x-ho-field-call.md:626-629.

  1. LOCKED DESIGN DECISIONS.

Compliant. The diff treats X3’s parser ambiguity as locked to explicit block syntax, not heuristic lookahead: docs/design-prereq-x-ho-field-call.md:604-624 records the Director-locked explicit-marker discipline and separates that from the exact token choice. The remaining open question is only whether X3 is required for this slice, which is explicitly named at docs/design-prereq-x-ho-field-call.md:631-638 and repeated in acceptance at docs/design-prereq-x-ho-field-call.md:696-699.

  1. TRACKED vs UNTRACKED DEBT.

Compliant. The audit does not leave an unbounded scaffold: it classifies the proposed coproduct variants in a dissolution ledger at docs/design-prereq-x-ho-field-call.md:436-488, names concrete triggers for Operator dissolution at docs/design-prereq-x-ho-field-call.md:453-459 and call-family dissolution at docs/design-prereq-x-ho-field-call.md:468-482, and separates the permanent HO dispatch capability from the transitional Indirect spelling at docs/design-prereq-x-ho-field-call.md:526-544.

3. Verdict

REQUEST_CHANGES

The audit is mostly careful and well-scoped, but the proposed substrate shape currently gives two incompatible representations for runtime field invocation: direct FieldCall and Indirect as the claimed lens.read(...) unblocker. Because this document locks the implementation direction for a substrate extension, that single-authority ambiguity should be resolved in the doc before it lands.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: 1cff516b · Trigger: manual
  • Conversation: View conversation

1. Story of the diff

This PR adds one new audit/design document, docs/design-prereq-x-ho-field-call.md, for the fold_lens<C> prerequisite gap. The document records four smoke failures showing that current v3 cannot call Arrow-valued fields or arbitrary Arrow expressions from .dag source: direct w.f(x) fails at parse time, (w.f)(x) cannot parse as a parenthesized callee, let g = wrap_double.f; g(5) does not lower cleanly, and { let ... } inside an expression body is parsed as a record literal rather than a block (docs/design-prereq-x-ho-field-call.md:24-143).

From that evidence, the audit splits the follow-up into X1 call-on-field/access generalization, X2 call-on-Arrow-typed local values, and X3 explicit block expressions. The load-bearing design move is the proposed substrate-side replacement of TransformNode.target + TransformNode.inputs with a typed TransformDispatch sum plus input_ports() as the single dependency walk (docs/design-prereq-x-ho-field-call.md:208-220, docs/design-prereq-x-ho-field-call.md:361-380). The document then maps those prerequisites back to fold_lens<C>, identifying lens.read, lens.sequential.op, lens.branch, lens.iterate, and lens.validate as the consumer calls that require the new surface (docs/design-prereq-x-ho-field-call.md:642-667).

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Finding — BLOCKING, substrate design ambiguity / single-authority. The audit clearly touches substrate shape: it proposes collapsing TransformNode.target and TransformNode.inputs into TransformDispatch (docs/design-prereq-x-ho-field-call.md:218-220). But the runtime field-call path is not single-authority yet. The document defines FieldCallDispatch as the representation for field invocation — “Field invocation: project an Arrow-typed field, then call it” with carrier and args fields (docs/design-prereq-x-ho-field-call.md:267-275) — while the sequencing section says the runtime-sourced case lands as TransformDispatch::Indirect(IndirectDispatch { callee: ArrowPortRef, args: Vec<PortId> }) (docs/design-prereq-x-ho-field-call.md:556-559) and then names lens.read(...) / lens.sequential.op(...) as L1.b sites whose actual unblocker is the IndirectCall substrate extension (docs/design-prereq-x-ho-field-call.md:566-571). That leaves the same source fact, lens.read(d, b) where lens is a runtime parameter, with two plausible encodings: one FieldCallDispatch, or a field projection producing an Arrow port followed by IndirectDispatch. Because this audit says it “locks the target shape” (docs/design-prereq-x-ho-field-call.md:500-501), it should explicitly choose one representation and state how the other is avoided.

  1. INVARIANTS.md + modeling-discipline.md.

Finding — Boundary Discipline / single-authority metadata. Same issue as above, stated at the invariant level: two consumers reading one resolved fact is fine, but two substrate representations for the same call shape are not. FieldCallDispatch is introduced as a distinct state family from plain projection (docs/design-prereq-x-ho-field-call.md:239-248), yet the fold-lens dependency text calls IndirectCall the unblocker for the same parameter-field call shape (docs/design-prereq-x-ho-field-call.md:566-571). The fix can be small: either say lens.read(...) lowers directly to FieldCallDispatch, with IndirectDispatch reserved for Arrow-valued variables/ports like g(x), or say field calls always lower as FieldProject + Indirect and remove/retire FieldCallDispatch from the target shape.

  1. CODING.md.

Compliant, modulo the finding above. The proposed Rust shape follows the local style: data carriers plus Dag construction functions, private payload fields, accessor methods, and a typed ArrowPortRef constructor path rather than a raw PortId convention (docs/design-prereq-x-ho-field-call.md:253-359). The omission of FieldCall from some constructor-proof prose is probably downstream of the same ambiguity, not a separate style finding.

  1. TESTING.md.

Compliant. This is an audit-only PR and explicitly states no parser/lowerer/emitter edits or test additions are part of the deliverable (docs/design-prereq-x-ho-field-call.md:675-699). For the future implementation PRs, the document records behavior-facing acceptance tests for field calls, parameter-field calls, nested field calls, non-Arrow callee diagnostics, Arrow-valued locals, and explicit block syntax (docs/design-prereq-x-ho-field-call.md:573-595, docs/design-prereq-x-ho-field-call.md:626-629).

  1. LOCKED DESIGN DECISIONS.

Compliant. The X3 syntax decision is explicitly framed as locked only at the discipline level — explicit block marker, not heuristic { ... } disambiguation — while leaving the concrete keyword open if Director chooses a different token (docs/design-prereq-x-ho-field-call.md:604-624). I do not see a divergence from a locked decision in the diff.

  1. TRACKED vs UNTRACKED DEBT.

Compliant, once the FieldCall/Indirect ambiguity is clarified. The coproduct ledger gives each proposed TransformDispatch family a keep/future-dissolve classification and names dissolution triggers for Operator and the future Callable / FieldCall / Indirect collapse (docs/design-prereq-x-ho-field-call.md:436-482). The HO dispatch capability versus the transitional Indirect spelling is also separated cleanly, with the capability marked permanent and the variant spelling marked transitional (docs/design-prereq-x-ho-field-call.md:526-544).

3. Verdict

REQUEST_CHANGES

The audit is valuable and mostly disciplined, but it currently leaves the central substrate representation for runtime field calls ambiguous between FieldCallDispatch and IndirectDispatch. Since this PR’s deliverable is the design document that future implementation will consume, that single-authority gap should be fixed before merging.

briansrls added a commit that referenced this pull request Apr 30, 2026
* test(v3): Prereq-X call-on-field-access blocker ratchet

Verifies the parser-grammar gap (call-on-field-access expression and
brace-block let inside = body) still blocks fold_lens<C> consumer wiring
(Prereq-3b dispatch). Pins the diagnostic shape of each gap so the
ratchet flips to red when the implementation lane (X1/X3 from #1264
audit) lands and the lane owner retires the fixtures.

No hand-Rust scaffolding for fold_lens<C>; the gap is structural in
the grammar and must be closed at the parser/lowerer, not bridged in
Rust.

* chore: apply cargo fmt

* test(v3): use real surface syntax in Prereq-X ratchet fixtures

Previous fixture used 'type Wrapper = Conj { f: Arrow(Int) -> Int }';
that risked pinning a parse error from the type-decl side rather than
the w.f(x) call-on-field-access gap. Switched both fixtures to
'type Wrapper { f: fn(Int) -> Int }' (matches Lens<C> field syntax in
src/v3/std/lens.dag) and added a control test asserting the type
declaration alone parses cleanly so X1/X3 isolate the call-site gap.

* chore: apply cargo fmt

* test(v3): register prereq_x ratchet path in EXPECTED_HAND_AUTHORED_TEST

SG-0 census ratchet failed at 8ea73f1 / 424ea69 because the new
prereq_x_call_on_field_access_ratchet_test.rs was a hand-authored .rs
not declared in the EXPECTED_HAND_AUTHORED_TEST list. Added it in
sorted ASCII-ascending position with a receipt comment naming the
dissolution trigger (lane owner retires at the same time as the X1/X3
parser/lowerer change).

* test(v3): switch .err().expect(...) to .expect_err(...) to satisfy clippy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant