Skip to content

feat(r3): add MethodTemplateContract projection oracle for row 85 - #1568

Merged
briansrls merged 25 commits into
mainfrom
session/silent-boar-29-pb-method-template-projection
May 3, 2026
Merged

briansrls merged 25 commits into
mainfrom
session/silent-boar-29-pb-method-template-projection

Conversation

@briansrls

@briansrls briansrls commented May 3, 2026 •

Copy link
Copy Markdown
Contributor

Scope (post-rescope)

This PR lands a Rust projection / test-oracle helper over the canonical bootstrap Dag for MethodTemplateContract rows. Per Director / Grounding / PB / Substrate disposition (inbox #1134 traffic culminating in #issuecomment-4365827471):

  • Does not close Gap 4 for R3 row 85 / PB docs(r3): R3 row-85 PB-Zero method-template projection packet (decision-gated) #1560.
  • Build-step surface parked on missing ephemeral generated-source-root support: Substrate confirmed that no repo pipeline exists today to generate a .dag into OUT_DIR/equivalent and thread that generated root through v2 self-compile / stage0 paths as a stable dependency root. Gap 4 build-step waits for either Gap 5 (which may avoid the dependency) or ci(v3): assess and gate bootstrap snapshot freshness #1558-style build-pipeline work that provides generated-source-root / import support.
  • Not final API for Grounding leaf migration. calm-tern-200 / Phase-2 leaf consumers must not code against this hook as the migration target. Routing to Grounding remains parked.

The fail-closed shape and tests below are valid for the oracle/helper role and should survive whichever future architecture lands.

Module: v3_compiler::pb_method_template_projection

pub enum MethodTemplateTarget { Rust, Python, Go }

pub enum MethodEmitTemplateProjection {
    Single { template: String },
    HigherOrder { inline_template: String, fn_ref_template: String },
}

pub enum PlaceholderConventionProjection { IndexedArgs, NamedArg }

pub struct MethodTemplateContractRow {
    pub dag_method: DeclarationId,            // MethodRef.decl, verified to instantiate MethodDeclaration
    pub runtime_template: String,
    pub emit_template: MethodEmitTemplateProjection,
    pub wraps_result: bool,
    pub placeholder_convention: PlaceholderConventionProjection,
}

pub enum MethodTemplateProjectionError { /* 17 typed variants, fail-closed; see source */ }

pub fn method_template_contract_rows(
    dag: &Dag,
    target: MethodTemplateTarget,
) -> Result<Vec<MethodTemplateContractRow>, MethodTemplateProjectionError>;

pub fn method_template_contract_row(
    dag: &Dag,
    target: MethodTemplateTarget,
    dag_method: DeclarationId,
) -> Result<Option<MethodTemplateContractRow>, MethodTemplateProjectionError>;

All five MethodTemplateContract fields preserved by typed Rust mirrors. Sum carriers (MethodEmitTemplate, PlaceholderConvention) stay constructor-tagged enums — substrate identity preserved, not flattened to strings.

Fail-closed boundaries

  • Closed-record schema on both MethodTemplateContract (5 fields) and inner MethodRef (1 field): duplicate or unknown labels surface as RowDuplicateField / RowUnknownField.
  • MethodRef.decl instantiation gate: decl must reference a declaration whose TypeConnective::Instantiation { template, .. } matches MethodDeclaration's id. Mirrors the boundary contract src/v3/std/methods.dag calls out until DeclarationRef<MethodDeclaration> refinement lands. Pattern matches the existing Stratum A registry gate in method_registry_test.rs::method_registry_covers_*.
  • Per-target uniqueness: enforced at the canonical row-list API. DuplicateMethodTemplateRow surfaces on the first collision; the lookup helper inherits the fail-closed shape.
  • Sum-payload validation: PlaceholderConvention is nullary — non-empty payloads surface as PlaceholderConventionPayloadNotEmpty. MethodEmitTemplate payload shapes (1 string for SingleTemplate; 2 strings for HigherOrderTemplates) validate exactly.

Tests

7 in-module unit tests + 4 integration tests — all passing.

Unit tests (mutate the in-memory bootstrap Dag via crate-private Dag::declaration_mut):

  • duplicate_field_on_row_surfaces_typed_error
  • unknown_field_on_row_surfaces_typed_error
  • duplicate_field_on_method_ref_surfaces_typed_error
  • unknown_field_on_method_ref_surfaces_typed_error
  • method_ref_decl_not_method_declaration_surfaces_typed_error
  • duplicate_row_surfaces_typed_error_at_canonical_row_list_api
  • malformed_placeholder_payload_surfaces_typed_error

Integration tests (over generated_full_bootstrap_dag()):

  • projects_every_target_with_all_five_fields
  • projection_preserves_per_target_dag_method_uniqueness
  • rust_count_row_anchors_runtime_emit_drift
  • lookup_helper_finds_known_row_and_returns_none_for_unknown_method

SG-0 census

Two new hand-Rust files admitted to EXPECTED_HAND_AUTHORED_NON_TEST and EXPECTED_HAND_AUTHORED_TEST with director-sign-off comments anchoring them to R3 row 85. The integration-test entry names its dissolution trigger (testgen path covering reflected-Dag structural assertions over std/ row authorities).

Authority cross-reads

  • docs/decisions/r3-row85-method-template-read-surface.md — read-surface decision.
  • docs/briefs/r3-row85-pb-zero-method-template-projection-packet.md §5 — Gap 4 worker shape.
  • src/v3/std/emit_model.dag:506-512 — MethodTemplateContract substrate carrier.
  • src/v3/std/methods.dag — MethodRef boundary contract.
  • INVARIANTS.md C-8 (fail-closed) + P2 (single-authority).

Test plan

  • cargo check -p v3-compiler — clean.
  • cargo test -p v3-compiler --lib pb_method_template_projection — 7/7 pass.
  • cargo test -p v3-compiler --test integration pb_method_template_projection — 4/4 pass.
  • cargo clippy -p v3-compiler --tests -- -D warnings — clean.
  • cargo fmt --all --check — clean.
  • cargo test -p v3-compiler --test integration sg0_census — 12/12 pass (SG-0 ratchet admits new files).

🤖 Generated with Claude Code

…bootstrap Dag (#1560 Gap 4)

Implements the PB-Bootstrap-Process consumer hook for R3 row 85 / PB #1560
Gap 4 per the canonical decision in
`docs/decisions/r3-row85-method-template-read-surface.md`.

The single row authority remains
`src/v3/std/{rust,python,go}_method_template_contracts.dag`. This module
adds a target-keyed structural projection over the lowered bootstrap `Dag`
so PB-zero / v2-retirement consumers can select rows by
target + `MethodRef.decl` without:

- importing `v3.std.*` into v2 crates (Gap 4 acceptance A1),
- introducing a second `Map<String, String>` template-text authority (A2),
- copying any template strings into v2 sources (A3).

All five `MethodTemplateContract` fields are preserved by typed Rust
mirrors (A4): `dag_method` (DeclarationId via MethodRef.decl),
`runtime_template` (String), `emit_template` (sum: Single |
HigherOrder), `wraps_result` (bool), `placeholder_convention` (sum:
IndexedArgs | NamedArg). The two sum carriers stay constructor-tagged
enums — substrate identity is preserved, not flattened to strings.

API:

  pub fn method_template_contract_rows(
      dag: &Dag,
      target: MethodTemplateTarget,
  ) -> Result<Vec<MethodTemplateContractRow>, MethodTemplateProjectionError>

Errors are typed and fail-closed (per INVARIANTS.md C-8): every observable
mismatch between the bootstrap Dag shape and the substrate carrier
surfaces as a typed value rather than a panic.

Tests (`tests/integration/pb_method_template_projection_test.rs`):

1. `projects_every_target_with_all_five_fields` — projects Rust/Python/Go
   from `generated_full_bootstrap_dag()`; asserts non-empty rows and the
   five-field receipt for each row.
2. `projection_preserves_per_target_dag_method_uniqueness` — mirrors the
   substrate-side `method_template_contract_per_target_dag_method_unique`
   claim through the public projection API; if the projection drifts
   from the substrate carrier, this fails before any v2 consumer would.
3. `rust_count_row_anchors_runtime_emit_drift` — spot-checks the
   documented `count` runtime/emit template drift
   (`{recv}.len()` vs `({recv}.len() as i64)`) against real row text;
   anchors the projection without reproducing template strings as
   fixtures.

Out of scope (per dispatch / packet §5.3):

- `LanguageSpec.method_templates` rewrite (Gap 5).
- v2 leaf-emit migration.
- Row population (`string_contains` Python/Go, Go `chars` parity gaps).
- `LEGACY_METHOD_TEMPLATE_AUTHORITIES` ratchet stays live until Gap 5
  and leaf-emit migrations land (A6).

R3 debt receipt: Debt found + routed / partial implementation against
ROADMAP.md:512 and ledger row 85; closes Gap 4 only. Gap 5 sequenced
strictly after this PR merges.

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

Copy link
Copy Markdown
Contributor Author

Reviewed current head 424b7ec for Gap 4 scope. The PR is in the right shape overall: public v3_compiler::pb_method_template_projection module, no v2 migration, no row population, no copied template map, and tests over generated_full_bootstrap_dag().

One blocking fix before I clear it:

src/v3/compiler/src/pb_method_template_projection.rs:386 destructures FieldValue::Variant { constructor, .. } for placeholder_convention and then returns IndexedArgs / NamedArg without checking that the payload is empty. PlaceholderConvention is a nullary sum carrier (IndexedArgs | NamedArg), and the PR body promises every observable mismatch between the bootstrap Dag shape and the substrate carrier is surfaced as a typed error. As written, a malformed bootstrap row like IndexedArgs(false) would be accepted and the payload silently discarded.

Please fail-close on non-empty placeholder payloads, with a typed error and focused test. There is already precedent in src/v3/grounding_tests/src/stratum_a.rs around placeholder_convention_canonical_rejects_payload_for_nullary_indexed_args().

Optional but useful for the Grounding handshake: add a small direct lookup helper by (target, dag_method) if it fits cleanly. The row-list API is usable, but a lookup helper would keep Gap 5/leaf migration from reimplementing selection logic. I do not consider that part blocking if you want to keep this PR as rows-only.

— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 424b7ec3 · Trigger: schedule
  • Comparison: origin/main @ 9dee7451 ... review/pr-1568-424b7ec3 @ 424b7ec3
  • Thinking: 66s wall

Findings: None. The hook returns Result with a typed MethodTemplateProjectionError (no silent None-on-failure for real errors), avoids a second template authority, and reads the lowered bootstrap shape through FieldValue / declaration_by_name rather than inventing a parallel lookup. That matches P3 (fail-closed) and P2 (single authority / typed boundary) as applied to this implementation-layer consumer, not new Dag substrate.

Verdict: APPROVE — The change is small, documented against the row-85 decision, and the integration tests target the declared contract (generated_full_bootstrap_dag() + projection). Nothing in the diff clearly breaks INVARIANTS.md, CODING.md, or TESTING.md for new code (library code avoids unwrap; tests may expect/panic, which is acceptable per CODING.md).

Exploratory (optional): docs/modeling-discipline.md Practice 4 asks for inline 🟢/🟡/🔴 classification on new multi-variant Rust enums. MethodTemplateTarget, MethodEmitTemplateProjection, and PlaceholderConventionProjection have substrate-anchoring rustdoc but not those tags (pb_method_template_projection.rs around lines 29, 52, and 65). If your review bar treats that as mechanical compliance, a one-line comment per enum is an easy follow-up; it is not tied to a correctness bug in this diff.

briansrls and others added 5 commits May 3, 2026 04:06
…ored census

Per CI failure on 424b7ec: adding the new hand-Rust files
`src/v3/compiler/src/pb_method_template_projection.rs` and
`src/v3/compiler/tests/integration/pb_method_template_projection_test.rs`
drifts the SG-0 ratchet at
`src/v3/compiler/tests/integration/sg0_census_test.rs`. Add both files
to `EXPECTED_HAND_AUTHORED_NON_TEST` and `EXPECTED_HAND_AUTHORED_TEST`
respectively, with director-sign-off comments anchoring them to the
R3 row 85 / PB #1560 Gap 4 dispatch (decision doc
`docs/decisions/r3-row85-method-template-read-surface.md`).

The new module + focused test are the canonical PB consumer hook + its
acceptance receipt; both stay hand-Rust until the testgen path covers
reflected-Dag structural assertions over std/ row authorities, same
discipline as `method_template_contract_test.rs`.

Validation:
- cargo test -p v3-compiler --test integration sg0_census — 12/12 pass
- cargo test -p v3-compiler --test integration pb_method_template_projection — 4/4 pass
- cargo test -p v3-compiler --lib pb_method_template_projection — 1/1 pass

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per cursor exploratory note on PR #1568 head 424b7ec (non-blocking,
explicitly optional): docs/modeling-discipline.md Practice 4 asks for
inline 🟢 / 🟡 / 🔴 classification on new multi-variant Rust enums.

Add 🟢 TERMINAL tags to all four enums in pb_method_template_projection
with anchoring rationale:

- MethodTemplateTarget — terminal at per-target row-list scope; mirrors
  the three closed list-declaration authorities.
- MethodEmitTemplateProjection — terminal at target method-emission-
  template scope, inheriting from substrate carrier MethodEmitTemplate
  (already marked 🟢 TERMINAL at emit_model.dag:453).
- PlaceholderConventionProjection — terminal nullary sum mirroring the
  closed substrate carrier.
- MethodTemplateProjectionError — terminal at bootstrap-mismatch scope;
  closed substrate-mismatch enumeration, not a transitional grab-bag.

No code change.

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

Copy link
Copy Markdown
Contributor Author

Reviewed latest head ffd1fd2. The earlier fail-closed issue is fixed: PlaceholderConventionPayloadNotEmpty plus the malformed-payload test cover the nullary sum payload case, and the additive method_template_contract_row helper is a useful selection surface.

However, I am not clearing #1568 yet because the larger Grounding STOP remains unresolved: this PR still exposes a Rust v3_compiler API, while the actual v2 leaf consumers named by Grounding are .dag source call sites (src/v2/05_emit.dag, src/v2/05_emit_rust.dag). There is still no demonstrated legal .dag → v3_compiler::pb_method_template_projection consumption path, no non-duplicating .dag/generated projection surface, and no Director-approved shift to migrating generated stage0 Rust instead of .dag source.

Please either:

  1. update feat(r3): add MethodTemplateContract projection oracle for row 85 #1568 with the actual consumer surface that .dag migration can use, with a clear explanation of why it is a projection and not a second authority;
  2. cite an existing extdep/interop precedent and adapt the PR to it; or
  3. STOP+PING and reclassify feat(r3): add MethodTemplateContract projection oracle for row 85 #1568 as a Rust projection/oracle helper that does not close Gap 4 until a follow-on .dag-consumable surface lands.

Until one of those is true, please do not route this to Grounding as final API for calm-tern-200.

— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Acknowledged on PR side — disposition already aligned with your option 3 in the parent inbox cross-reply at #1134 (comment).

Position for the PR record:

  • feat(r3): add MethodTemplateContract projection oracle for row 85 #1568 reclassifies as the Rust projection / test-oracle surface only (the role named in docs/decisions/r3-row85-method-template-read-surface.md §"PB Surface Classification"). It does not close Gap 4 on its own; the build-step / .dag-consumable surface is a follow-on.
  • The blocking PlaceholderConventionPayloadNotEmpty fail-closed fix and the additive method_template_contract_row helper stay regardless of disposition; both are independent of the consumer-hook architecture.
  • The Rust hook is reusable input for whichever build-step surface the rescope chooses (option 1 generated-.dag projection or option 2 direct-.dag consumption per the inbox reply). Same projection logic + ratchets per the decision doc's "Both uses must share the same substrate projection."

Not routing to Grounding as final API for calm-tern-200. Holding all further implementation on this branch pending Director / Grounding / PB acceptance of the rescoped Gap 4 architecture.

If you want #1568 merged as the Rust oracle surface (with Gap 4 closure deferred to a follow-on), that needs explicit acceptance — otherwise happy to leave it open until the boundary is decided.

— sent from silent-boar-29

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: ffd1fd20 · Trigger: schedule
  • Comparison: origin/main @ 9dee7451 ... review/pr-1568-ffd1fd20 @ ffd1fd20
  • Thinking: 40s wall

Verdict: APPROVE — The diff adds an implementation-layer read hook (pb_method_template_projection.rs) that projects already-authored row authorities from the bootstrap Dag, returns Result<_, MethodTemplateProjectionError> on shape mismatch (no silent defaults), and uses Ok(None) only for “no row for this DeclarationId,” which matches fail-closed / Option semantics in docs/modeling-discipline.md Practice 1. New multi-variant Rust enums include explicit 🟢 TERMINAL notes per Practice 4. The module stays data + free functions and typed carriers, consistent with CODING.md. SG-0 census entries document the new hand-Rust surface. Tests target the declared boundary (generated_full_bootstrap_dag()), which is appropriate for this hook; nothing in the diff contradicts INVARIANTS.md in a way that needs a cited blocking line.

Exploratory (optional): projects_every_target_with_all_five_fields in pb_method_template_projection_test.rs loops over every row with several assertions; TESTING.md prefers one structural claim per test where claims are unrelated — here they are all in service of Gap 4 A4, so this is a mild style tradeoff, not a violation.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: cc2abdcc · Trigger: manual
  • Comparison: main @ 9dee7451 ... session/silent-boar-29-pb-method-template-projection @ ffd1fd20
  • Conversation: View conversation

1. Story of the diff

This PR adds a PB bootstrap consumer hook that reads the canonical MethodTemplateContract row authorities out of the full bootstrap Dag and exposes them as typed Rust projection rows, rather than introducing a parallel template table. The load-bearing module is src/v3/compiler/src/pb_method_template_projection.rs: MethodTemplateTarget selects the per-target list declaration, method_template_contract_rows projects every row with typed errors for shape mismatches, and the emit_template / placeholder_convention substrate sums are preserved as Rust enums instead of flattened strings. The integration test then exercises the public projection over generated_full_bootstrap_dag() for Rust/Python/Go, checks per-target method uniqueness, anchors one known Rust count_method row, and covers the lookup helper; a unit test mutates an in-memory Dag to ensure malformed nullary payloads fail closed. The SG-0 census is updated to admit the new hand-authored source and test file.

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Compliant — this is a substrate-consuming implementation hook, not a new Dag substrate shape: src/v3/compiler/src/pb_method_template_projection.rs:194: pub fn method_template_contract_rows( takes &Dag and projects into Rust carriers, while the row authority is explicitly kept in the .dag lists at src/v3/compiler/src/pb_method_template_projection.rs:6: //! row authority remains \src/v3/std/{rust,python,go}_method_template_contracts.dag;.

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

Finding — BLOCKING, P2 Boundary Discipline / P3 Fail-Closed / API-level enforcement.

src/v3/compiler/src/pb_method_template_projection.rs:233: Ok(rows.into_iter().find(|row| row.dag_method == dag_method)) silently selects the first matching row if the list ever contains duplicate (target, dag_method) entries. The PR itself treats per-target method uniqueness as a substrate claim — src/v3/compiler/tests/integration/pb_method_template_projection_test.rs:126: seen.insert(row.dag_method), — but the public lookup helper, which Gap 5 / leaf-emit consumers are expected to use, does not preserve that claim fail-closed. If two rows exist, that is two authorities for the same method template; the helper should return a typed duplicate-key projection error instead of fabricating “the” answer by iteration order.

  1. CODING.md.

Compliant — the implementation follows data + free functions and typed result shapes: src/v3/compiler/src/pb_method_template_projection.rs:89: pub enum MethodTemplateProjectionError { defines structured failures, and src/v3/compiler/src/pb_method_template_projection.rs:236: fn project_row( keeps projection logic as small free helpers rather than adding behavior onto Dag.

  1. TESTING.md.

Compliant — the tests are at the right level for this hook: the public contract is “project from the full bootstrap Dag,” so src/v3/compiler/tests/integration/pb_method_template_projection_test.rs:38: let dag = generated_full_bootstrap_dag(); is appropriate integration coverage, while the malformed-payload case that needs crate-private Dag mutation is kept as a module unit test at src/v3/compiler/src/pb_method_template_projection.rs:503: fn malformed_placeholder_payload_surfaces_typed_error() {.

  1. LOCKED DESIGN DECISIONS.

N/A — the diff references the row-85 decision and stays within the named Gap 4 scope; it does not alter a locked thesis/design rule. The scope boundary is explicit at src/v3/compiler/src/pb_method_template_projection.rs:21: //! Gap 4 only — \LanguageSpec.method_templates rewrite (Gap 5) and v2 leaf.

  1. TRACKED vs UNTRACKED DEBT.

Compliant — the new hand-authored files are admitted through the SG-0 census with scoped comments: production hook at src/v3/compiler/tests/integration/sg0_census_test.rs:228: "src/v3/compiler/src/pb_method_template_projection.rs", and test hook at src/v3/compiler/tests/integration/sg0_census_test.rs:376: "src/v3/compiler/tests/integration/pb_method_template_projection_test.rs",. The test-file comment also names the dissolution trigger: src/v3/compiler/tests/integration/sg0_census_test.rs:374: // alongside \method_template_contract_test.rs until testgen covers.

3. Verdict

REQUEST_CHANGES. The projection shape is generally disciplined and well-tested, but the public direct lookup helper currently turns duplicate row authority into first-match behavior. Since this helper is explicitly positioned as the consumer contract for later migration, it should fail closed on duplicate (target, dag_method) rows before this lands.

@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: ffd1fd20 · Trigger: schedule
  • Thinking: 150s wall

BLOCKING (2)

Root Cause

  • src/v3/compiler/src/pb_method_template_projection.rs The new consumer hook reimplements only presence/type extraction instead of the existing closed-schema plus MethodDeclaration registry gate used by the MethodTemplateContract Stratum A projection → add typed projection errors for row/MethodRef schema mismatch and non-MethodDeclaration decl targets before returning MethodTemplateContractRow.

⚠️ The hook is close, but it should reject malformed row authority at this public projection boundary before consumers depend on the weaker shape.

fields: &'a [(String, FieldValue)],
field: &'static str,
) -> Result<&'a FieldValue, MethodTemplateProjectionError> {
fields

This comment was marked as resolved.

.map(|(_, v)| v)
.ok_or(MethodTemplateProjectionError::DagMethodNotMethodRefRecord { list, row_index })?;
match decl_value {
FieldValue::Reference(decl_id) => Ok(*decl_id),

This comment was marked as resolved.

@briansrls

Copy link
Copy Markdown
Contributor Author

Pushed 1e5c21f0a addressing the BLOCKING fail-closed finding from openai-pro / gpt-5-5-pro review on ffd1fd20:

  • New typed error MethodTemplateProjectionError::DuplicateMethodTemplateRow { list, first_row_index, duplicate_row_index }.
  • method_template_contract_row rewritten to scan all matches and surface the typed error on the second hit instead of silently returning the first match.
  • New unit test duplicate_row_in_lookup_helper_surfaces_typed_error injects a duplicate via crate-private Dag::declaration_mut and asserts the typed error.

This is a correctness fix to code already on the branch, not an architectural advance. #1568 is still held open per parent direction; the disposition (rescope as oracle/helper vs expand build-step surface) remains pending Director / Grounding / PB acceptance. The fail-closed gap was a real bug regardless of which architecture lands; sitting on it was not the right call.

2 unit + 4 integration tests pass; clippy + fmt clean.

— sent from silent-boar-29

briansrls and others added 3 commits May 3, 2026 04:27
Per inline review at src/v3/compiler/src/pb_method_template_projection.rs:310:

`field_by_label` used `.find()` over the row fields, accepting the first
matching field and silently ignoring duplicates. `project_row` had no
closed-schema check, so a row with an extra unknown field
(`{ dag_method: ..., renamed_field: ... }`) would project successfully
with the unknown silently discarded. Same gap on `MethodRef` (the inner
`{ decl: ... }` record).

P2 single-authority + C-8 fail-closed both want every observable
substrate-shape mismatch to surface as a typed error.

Add typed errors:
- `RowDuplicateField { list, row_index, record, field, first_field_index, duplicate_field_index }`
- `RowUnknownField { list, row_index, record, field, field_index }`

Replace `field_by_label` with `closed_record_lookup(list, row_index, record, fields, expected)`
that walks fields once, fails-closed on duplicate or unknown labels, and
returns a `HashMap<&str, &FieldValue>`. `lookup_required` then surfaces
`RowMissingField` per-field. Apply to both `MethodTemplateContract`
(five expected labels) and `MethodRef` (single `decl` label).

Four new in-module unit tests cover the failure modes:
- duplicate_field_on_row_surfaces_typed_error
- unknown_field_on_row_surfaces_typed_error
- duplicate_field_on_method_ref_surfaces_typed_error
- unknown_field_on_method_ref_surfaces_typed_error

Validation:
- cargo test -p v3-compiler --lib pb_method_template_projection — 6/6 pass
- cargo test -p v3-compiler --test integration pb_method_template_projection — 4/4 pass
- cargo clippy -p v3-compiler --tests -- -D warnings — clean
- cargo fmt --all --check — clean

Patch is correctness-only; #1568 still held open as oracle/helper
pending Director / Grounding / PB rescope.

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

Per inline review at src/v3/compiler/src/pb_method_template_projection.rs:335:

`project_dag_method` returned any `DeclarationId` from `MethodRef.decl`
without verifying it instantiates `MethodDeclaration`. `src/v3/std/methods.dag`
explicitly calls out the boundary contract: today's substrate grammar
can't express `DeclarationRef<MethodDeclaration>` (refinement-typing-on-
DeclarationRef gap), so the projection must enforce fail-closed at the
boundary that `decl` references a `MethodDeclaration`-instantiating
data binding.

Pattern mirrors `method_registry_test.rs::method_registry_covers_*`:
look up `MethodDeclaration`'s declaration ID, walk the referenced
`Declaration`'s `connective`, require
`TypeConnective::Instantiation { template, .. }` where
`template == method_declaration_id`.

Add typed errors:
- `MethodRefDeclNotMethodDeclaration { list, row_index, decl_id }`
- `MethodDeclarationCarrierMissing` (substrate degenerate case)

Add unit test
`method_ref_decl_not_method_declaration_surfaces_typed_error` that
mutates the first row's `MethodRef.decl` to point at
`MethodTemplateContract` (a type declaration, not a `MethodDeclaration`
instance) via crate-private `Dag::declaration_mut`, and asserts the
typed error.

Existing integration test
`lookup_helper_finds_known_row_and_returns_none_for_unknown_method`
remains green: it passes a non-`MethodDeclaration` ID into the lookup;
since every projected `row.dag_method` is now verified to be a
`MethodDeclaration`, the comparison never matches and the helper
correctly returns `None` (not "fabricate a row").

Validation:
- cargo test -p v3-compiler --lib pb_method_template_projection — 7/7 pass
- cargo test -p v3-compiler --test integration pb_method_template_projection — 4/4 pass
- cargo clippy -p v3-compiler --tests -- -D warnings — clean
- cargo fmt --all --check — clean

Patch is correctness-only; #1568 still held open as oracle/helper
pending Director / Grounding / PB rescope.

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

Copy link
Copy Markdown
Contributor Author

Already addressed in commits landed after the bot's ffd1fd20 snapshot:

  • c89d128d2 — closed_record_lookup enforces the closed schema on both MethodTemplateContract row (5 fields) and inner MethodRef (1 field). Typed errors RowDuplicateField { record, field, ... } and RowUnknownField { record, field, ... } surface duplicate / unknown labels fail-closed.
  • aab7e6246 — project_dag_method now requires MethodRef.decl to point at a Declaration whose TypeConnective::Instantiation { template, .. } matches MethodDeclaration's ID (the boundary contract src/v3/std/methods.dag calls out until DeclarationRef<MethodDeclaration> refinement lands). New typed errors MethodRefDeclNotMethodDeclaration { list, row_index, decl_id } + MethodDeclarationCarrierMissing.

Pattern mirrors method_registry_test.rs::method_registry_covers_* (the existing Stratum A registry gate the bot cites). Five new in-module unit tests cover all four mismatch classes + the lookup-helper duplicate-row case:

  • duplicate_field_on_row_surfaces_typed_error
  • unknown_field_on_row_surfaces_typed_error
  • duplicate_field_on_method_ref_surfaces_typed_error
  • unknown_field_on_method_ref_surfaces_typed_error
  • method_ref_decl_not_method_declaration_surfaces_typed_error
  • duplicate_row_in_lookup_helper_surfaces_typed_error
  • malformed_placeholder_payload_surfaces_typed_error

Current head: aab7e6246 (7 unit + 4 integration tests pass; clippy + fmt clean).

Disposition unchanged: #1568 still held open as oracle/helper pending Director / Grounding / PB rescope per parent direction. The fail-closed correctness fixes are independent of architecture choice.

— sent from silent-boar-29

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: aab7e624 · Trigger: schedule
  • Comparison: origin/main @ 9dee7451 ... review/pr-1568-aab7e624 @ aab7e624
  • Thinking: 32s wall

APPROVE — Diff is large but narrowly scoped: a new fail-closed projection module + integration tests + census admission. The module is implementation-layer (Rust-internal projection over the bootstrap Dag, not new substrate), but it correctly treats the substrate carriers (MethodEmitTemplate, PlaceholderConvention) as closed sums, enforces closed-record schemas on both MethodTemplateContract and MethodRef, fail-closes on the missing DeclarationRef<MethodDeclaration> refinement at the API boundary, and surfaces per-target dag_method uniqueness as a typed error rather than silently picking the first row. Sum identity is preserved (no Map<String,String> flattening), template text is read straight from the row authorities (single-authority preserved). Errors are typed values, not panics — consistent with C-8. Tests exercise both the green path and the structural mismatch variants via crate-private Dag mutation; integration tests verify the public API and per-target uniqueness through the projection rather than re-walking the Dag. Census files updated. No invariant violations observed in the diff.

Exploratory observations (non-blocking):

  • pb_method_template_projection.rs:493 uses expect("expected label must match a known constant") to re-borrow a &'static str from the expected slice. This is fine, but the lookup could just stash the matched &'static str from expected at insertion time to avoid the re-scan on the duplicate path.
  • method_template_contract_row re-projects all rows then linear-scans; for Gap-5 hot consumers a HashMap<DeclarationId, Row> keyed cache may be worth introducing later — explicitly out of scope here.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: a56cdebf · Trigger: manual
  • Comparison: main @ 9dee7451 ... session/silent-boar-29-pb-method-template-projection @ aab7e624
  • Conversation: View conversation

1. Story of the diff

This PR adds a new PB bootstrap consumer hook, exported from src/v3/compiler/src/lib.rs:20, that projects MethodTemplateContract rows out of the full bootstrap Dag instead of copying method-template text into a parallel v2-side table. The new pb_method_template_projection module defines target-specific row selection (MethodTemplateTarget), typed projections for the two substrate sum carriers (MethodEmitTemplateProjection, PlaceholderConventionProjection), a five-field MethodTemplateContractRow, and a typed error enum for malformed bootstrap shapes. The public surface is method_template_contract_rows(...) for all rows by target and method_template_contract_row(...) for direct (target, dag_method) lookup; integration tests wire the module into the suite and check all three targets, uniqueness, the Rust count_method row, and missing-row behavior. The SG-0 census is updated to admit both the new hand-authored Rust module and its integration test as known hand-authored files.

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation). — Finding, BLOCKING.

This is an implementation module, but it is a substrate-boundary consumer: it projects MethodTemplateContract row facts from the bootstrap Dag into the public PB/v2-retirement hook. The projection currently accepts a malformed record with duplicate or unknown fields because field_by_label just takes the first matching label: src/v3/compiler/src/pb_method_template_projection.rs:365-368 uses fields.iter().find(|(label, _)| label == field).map(|(_, value)| value). The same issue exists for nested MethodRef: src/v3/compiler/src/pb_method_template_projection.rs:384-388 uses .find(|(label, _)| label == "decl") and never validates that decl is the only field. That means the boundary can silently discard extra substrate coordinates or choose one of two authorities for the same coordinate.

  1. INVARIANTS.md + modeling-discipline.md. — Finding, BLOCKING.

This violates P2 Boundary Discipline / single-authority metadata and P3 Fail-Closed. The diff itself declares the intended fail-closed behavior: src/v3/compiler/src/pb_method_template_projection.rs:136-139 says duplicate fields are two authorities for the same row coordinate, and src/v3/compiler/src/pb_method_template_projection.rs:148-152 says unknown labels should surface rather than be silently ignored. But the implementation never performs whole-record schema validation; it only asks for each expected field by first match. If MethodTemplateContract grows a sixth field or a row accidentally contains two runtime_template entries, method_template_contract_rows still returns Ok(...), losing the fact that the substrate shape no longer matches the projected carrier.

  1. CODING.md. — Compliant.

The public API is data + free functions rather than a method-heavy object surface: src/v3/compiler/src/pb_method_template_projection.rs:252-255 and src/v3/compiler/src/pb_method_template_projection.rs:290-294 expose pure Dag readers returning structured Result<...> carriers, not hidden state or side effects.

  1. TESTING.md. — Finding, tied to the blocking schema-validation issue.

The added tests cover the green bootstrap path and two malformed cases, but they do not exercise the declared RowDuplicateField / RowUnknownField fail-closed paths added at src/v3/compiler/src/pb_method_template_projection.rs:140-153. Because those variants are unreachable in the current implementation, a test that mutates a row to add a duplicate runtime_template or an unknown field would currently expose the bug. The right level is the existing unit-test style in src/v3/compiler/src/pb_method_template_projection.rs:533-668, where the PR already mutates the in-memory bootstrap Dag for malformed-shape cases.

  1. LOCKED DESIGN DECISIONS. — N/A.

The diff references the row-85 decision doc, but it does not modify a locked design decision or introduce a visible divergence from one in the changed files.

  1. TRACKED vs UNTRACKED DEBT. — Compliant.

The new hand-authored files are admitted to the SG-0 census with bounded rationale: the implementation entry is documented at src/v3/compiler/tests/integration/sg0_census_test.rs:224-228, and the test entry names its temporary condition, “until testgen covers reflected-Dag structural assertions,” at src/v3/compiler/tests/integration/sg0_census_test.rs:372-376.

3. Verdict

REQUEST_CHANGES

The shape of the hook is aligned with the intended single-authority projection, but the row-schema validation gap is load-bearing: the public substrate consumer can silently ignore unknown fields or choose the first duplicate field despite declaring typed errors for exactly those cases. Fixing that in this PR should be small: validate allowed labels and duplicate labels for both MethodTemplateContract and nested MethodRef, then add the corresponding malformed-Dag unit tests.

@briansrls

Copy link
Copy Markdown
Contributor Author

Already addressed in commits landed after the bot's a56cdebf snapshot. The exact concerns cited (closed-schema validation on row + nested MethodRef, plus unit tests for RowDuplicateField / RowUnknownField) are present at HEAD aab7e6246:

Closed-schema validation (commit c89d128d2):

field_by_label was deleted; replaced with closed_record_lookup(list, row_index, record, fields, expected) that walks every field once, fails-closed on RowDuplicateField and RowUnknownField, and returns a HashMap<&str, &FieldValue>. Applied to:

  • MethodTemplateContract row, expected = ["dag_method", "runtime_template", "emit_template", "wraps_result", "placeholder_convention"] (pb_method_template_projection.rs ~line 295)
  • MethodRef inner record, expected = ["decl"] (pb_method_template_projection.rs ~line 466)

MethodDeclaration boundary check (commit aab7e6246):

Adds MethodRefDeclNotMethodDeclaration and MethodDeclarationCarrierMissing. project_dag_method walks dag.declaration(decl_id).connective, requires TypeConnective::Instantiation { template, .. } matching MethodDeclaration's id (boundary contract per src/v3/std/methods.dag until DeclarationRef<MethodDeclaration> refinement lands). Pattern mirrors method_registry_test.rs::method_registry_covers_*.

Unit tests (5 new, all in-module):

  • duplicate_field_on_row_surfaces_typed_error
  • unknown_field_on_row_surfaces_typed_error
  • duplicate_field_on_method_ref_surfaces_typed_error
  • unknown_field_on_method_ref_surfaces_typed_error
  • method_ref_decl_not_method_declaration_surfaces_typed_error

Plus the previously landed duplicate_row_in_lookup_helper_surfaces_typed_error and malformed_placeholder_payload_surfaces_typed_error.

Test count at HEAD aab7e6246: 7 unit + 4 integration tests pass; cargo clippy --tests -- -D warnings clean; cargo fmt --all --check clean.

Disposition unchanged: #1568 still held open as oracle/helper pending Director / Grounding / PB rescope per parent direction. The fail-closed correctness fixes are independent of architecture choice.

— sent from silent-boar-29

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: aab7e624 · Trigger: manual
  • Comparison: main @ 9dee7451 ... session/silent-boar-29-pb-method-template-projection @ aab7e624
  • Conversation: View conversation

1. Story of the diff

This PR adds a new public Rust projection layer, pb_method_template_projection, that reads the existing per-target MethodTemplateContract list declarations out of the full bootstrap Dag rather than introducing a second template-text source. The module keeps the row authority in the .dag row lists, exposes target selection through MethodTemplateTarget, and preserves all five row fields as typed Rust carriers, including constructor-tagged enums for MethodEmitTemplate and PlaceholderConvention instead of flattening them into strings (src/v3/compiler/src/pb_method_template_projection.rs:1-23, :67-101). The projection is fail-closed for malformed row shape, missing fields, duplicate/unknown record fields, non-MethodDeclaration MethodRef.decl targets, malformed sum payloads, and duplicate lookup matches (src/v3/compiler/src/pb_method_template_projection.rs:104-254, :386-660). The PR also wires integration coverage for the green bootstrap snapshot, unit-style malformed-Dag tests inside the module, and admits both new hand-authored files into the SG-0 census (src/v3/compiler/tests/integration.rs:138-139, src/v3/compiler/tests/integration/pb_method_template_projection_test.rs:36-196, src/v3/compiler/tests/integration/sg0_census_test.rs:224-228, :372-376).

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Compliant — this is implementation-side Rust projection over existing substrate rows, not a new Dag substrate type or dag.rs mutation; the load-bearing modeling choice is that substrate sum identity is preserved in Rust mirrors (MethodEmitTemplateProjection at src/v3/compiler/src/pb_method_template_projection.rs:67-75 and PlaceholderConventionProjection at :83-87) rather than re-modeled as string cases.

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

Finding — NON-BLOCKING, P2 single-authority / API-level enforcement. The public row-list API is documented as the canonical PB-zero consumer hook, and specifically says v2 migrations consume Vec<MethodTemplateContractRow> rather than parallel Map<String, String> tables (src/v3/compiler/src/pb_method_template_projection.rs:266-269). But method_template_contract_rows currently only maps and collects projected rows (src/v3/compiler/src/pb_method_template_projection.rs:288-291), while duplicate (target, dag_method) detection is enforced only in the narrower direct lookup helper (src/v3/compiler/src/pb_method_template_projection.rs:300-307, :320-325). That means the main public list API can still return Ok(Vec<_>) containing two authorities for the same method if a malformed or future bootstrap list duplicates a row, leaving consumers that build their own table to choose a winner. I would move per-target uniqueness validation into method_template_contract_rows itself, or return a keyed uniqueness-checked carrier, and let method_template_contract_row delegate to that. This is the API-level enforcement/single-authority concern described by the supplied rubric. chatgpt-review-833b738d-7afe-40…

chatgpt-review-9c2de0a5-b08b-47…

  1. CODING.md.

Compliant — the production API uses free functions with explicit dependencies and structured results: both public entry points take &Dag plus precise selector inputs and return Result<..., MethodTemplateProjectionError> (src/v3/compiler/src/pb_method_template_projection.rs:270-273, :308-312). The helper split also keeps the projection steps readable: closed-record lookup, required-field lookup, MethodRef projection, emit-template projection, and placeholder projection are separate functions (src/v3/compiler/src/pb_method_template_projection.rs:395-660). chatgpt-review-bc8ddb6d-a42f-4a…

  1. TESTING.md.

Compliant — the PR adds focused integration tests over the intended generated_full_bootstrap_dag() surface, including all-target projection, per-target uniqueness, a real Rust count_method spot check, and direct lookup behavior (src/v3/compiler/tests/integration/pb_method_template_projection_test.rs:36-196). It also adds malformed-Dag unit tests at the right level for crate-private mutations that cannot be produced from the on-disk authorities: duplicate row/field, unknown field, invalid MethodRef.decl, and non-empty nullary payload (src/v3/compiler/src/pb_method_template_projection.rs:779-1034). The one gap is the finding above: duplicate enforcement is tested for the lookup helper, not made structural on the row-list API. chatgpt-review-9ecb3b58-ab2d-4c…

  1. LOCKED DESIGN DECISIONS.

N/A — the diff references the row-85 decision and explicitly scopes itself to Gap 4, with Gap 5 and v2 leaf-emit migration sequenced later (src/v3/compiler/src/pb_method_template_projection.rs:4-23), but it does not alter a locked substrate/design decision in the diff.

  1. TRACKED vs UNTRACKED DEBT.

Compliant — the new hand-authored source and test files are admitted to the SG-0 census with specific R3 row 85 / PB #1560 Gap 4 comments (src/v3/compiler/tests/integration/sg0_census_test.rs:224-228, :372-376). The integration-test census entry also names its dissolution trigger: it stays hand-Rust until testgen covers reflected-Dag structural assertions over std row authorities (src/v3/compiler/tests/integration/sg0_census_test.rs:372-375).

3. Verdict

APPROVE_WITH_COMMENTS

The PR is broadly well-shaped: it preserves the existing row authority, uses typed carriers, and adds meaningful fail-closed tests. The main thing I would tighten before the next consumer lands is making duplicate (target, dag_method) rejection part of the canonical row-list API, not only the convenience lookup helper.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 02521317 · Trigger: schedule
  • Comparison: origin/main @ 9dee7451 ... review/pr-1568-02521317 @ 02521317
  • Thinking: 44s wall

Findings

  • NON-BLOCKING — CODING.md (small/composable modules): New compiler module src/v3/compiler/src/pb_method_template_projection.rs is ~1155 lines, well above the doc’s “~500 lines is a smell” bar for modules and the stated enforcement stance for new code. Example anchor (module starts here in the diff):
+//! PB-Bootstrap-Process consumer hook: project `MethodTemplateContract` rows
+//! from the full bootstrap `Dag`.
+//!
+//! Implements **R3 row 85 / PB #1560 Gap 4** per the decision in
+//! `docs/decisions/r3-row85-method-template-read-surface.md`. The single
+//! row authority remains `src/v3/std/{rust,python,go}_method_template_contracts.dag`;
+//! this module exposes a target-keyed structural projection of those rows
+//! over the lowered bootstrap `Dag` so PB-zero / v2-retirement consumers can
+//! select rows by target + `MethodRef` without:
+//!
+//! - importing `v3.std.*` into v2 crates (Gap 4 acceptance A1),
+//! - introducing a second `Map<String, String>` template-text authority (A2),
+//! - copying any template strings into v2 sources (A3).
+//!
+//! All five `MethodTemplateContract` fields (`dag_method`, `runtime_template`,
+//! `emit_template`, `wraps_result`, `placeholder_convention`) are preserved
+//! by typed Rust mirrors below (A4). `emit_template` and
+//! `placeholder_convention` are sum carriers; their substrate identity is
+//! preserved as constructor-tagged enums, not flattened to strings.
+//!
+//! Gap 4 only — `LanguageSpec.method_templates` rewrite (Gap 5) and v2 leaf
+//! emit migration are sequenced strictly after this hook lands. No v2
+//! consumer is migrated by this module.

Splitting later (e.g. error type + row projection vs. tests) would align with that guideline without changing behavior.

Nothing in the diff suggests violations of P3 fail-closed (typed Result / MethodTemplateProjectionError), P2 single-authority for template rows (read from bootstrap Dag, no parallel string table in the API), or docs/modeling-discipline.md enum practice (🟢 TERMINAL annotations on the new multi-variant enums in this file). TESTING.md: integration tests target the declared hook + generated_full_bootstrap_dag(); #[cfg(test)] uses panics only in harness setup, which matches the doc’s carve-out for test code.

Verdict

APPROVE_WITH_COMMENTS — Behavior and boundaries in the diff match the rubric (fail-closed projection, closed schema checks, uniqueness at the row-list API, census registration with decision-doc context). The only clear doc tension is the new hand-Rust module size versus CODING.md’s module-length guidance; worth a follow-up split if this file will keep growing.

briansrls and others added 3 commits May 3, 2026 06:30
… (BLOCKING)

Per parent direction on PR #1568 (inbox #1134): align the projection-side
MethodDeclaration registry boundary with the Stratum A gate at
`src/v3/grounding_tests/src/stratum_a.rs::method_registry_name`.
Previously the helper validated only "Instantiation { template =
MethodDeclaration } + value_body.is_some()". The substrate-side contract
is stricter:

  data <name>_method: MethodDeclaration = { name: "..." }

…which means the data binding must also carry a `Structural` value body
with a closed schema of exactly one `name` field whose value is a string
literal. A binding with extra fields, wrong-shaped name, or non-Structural
body would have passed the previous check.

Refactor the shared helper to enforce the four-clause contract:

  fn validate_method_declaration_data_binding(dag, decl_id, method_decl_id)
      -> Result<String /* method name */, MethodDeclarationBindingViolation>

  1. Connective is Instantiation.
  2. Instantiation.template == MethodDeclaration.
  3. value_body is Some(ValueBody::Structural { fields }).
  4. fields are exactly `["name"]` with FieldValue::Literal(LiteralBits::String).

New typed reason enum `MethodDeclarationBindingViolation` (six variants:
ConnectiveNotInstantiation / InstantiationTemplateNotMethodDeclaration /
ValueBodyMissing / ValueBodyNotStructural /
StructuralFieldsNotClosedNameOnly { observed_labels } /
NameNotStringLiteral). Both umbrella errors (`MethodRefDeclNotMethodDeclaration`,
`LookupKeyNotMethodDeclaration`) carry a `reason` field naming which
sub-check failed.

Two new in-module unit tests cover the new sub-checks:

- `method_ref_decl_extra_field_surfaces_closed_schema_violation` —
  injects an `alias` field beyond the closed `[name]` schema.
- `method_ref_decl_name_not_string_literal_surfaces_typed_error` —
  replaces `name`'s value with a Bool literal.

Existing tests updated to assert the granular reason where appropriate
(`ValueBodyMissing` for the alias-shape mutation,
`ConnectiveNotInstantiation` for the type-declaration target).

Validation:
- cargo test -p v3-compiler --lib pb_method_template_projection — 10/10 pass
- cargo test -p v3-compiler --test integration pb_method_template_projection — 4/4 pass
- cargo clippy -p v3-compiler --tests -- -D warnings — clean
- cargo fmt --all --check — clean

Disposition unchanged: oracle-only; does not close Gap 4.

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

Copy link
Copy Markdown
Contributor Author

Already addressed at current HEAD 8a8b0dd47. The reviewer's snapshot is c4445e98, several commits behind. Each requested item now lives at HEAD:

  • Shared helper used by both row projection and direct lookup. validate_method_declaration_data_binding(dag, decl_id, method_declaration_id) -> Result<String /* method name */, MethodDeclarationBindingViolation> in pb_method_template_projection.rs. Consumed by project_dag_method (per-row) and method_template_contract_row (lookup helper) — single source of truth, mirrors method_registry_name in Stratum A.

  • Four-clause contract. Helper enforces all four:

    1. Connective is Instantiation.
    2. Instantiation.template == MethodDeclaration.
    3. value_body is Some(ValueBody::Structural { fields }).
    4. fields are exactly ["name"] with FieldValue::Literal(LiteralBits::String(_)).
  • Typed errors with reason context. New enum MethodDeclarationBindingViolation (6 variants: ConnectiveNotInstantiation, InstantiationTemplateNotMethodDeclaration, ValueBodyMissing, ValueBodyNotStructural, StructuralFieldsNotClosedNameOnly { observed_labels }, NameNotStringLiteral). Both umbrella errors (MethodRefDeclNotMethodDeclaration per-row, LookupKeyNotMethodDeclaration lookup) carry a reason: MethodDeclarationBindingViolation field naming which sub-check failed.

  • In-module malformed-Dag unit tests covering each new sub-check:

    • method_ref_decl_alias_without_value_body_surfaces_typed_error (ValueBodyMissing reason — strips value_body).
    • method_ref_decl_extra_field_surfaces_closed_schema_violation (StructuralFieldsNotClosedNameOnly reason — pushes an alias field onto a real binding's body).
    • method_ref_decl_name_not_string_literal_surfaces_typed_error (NameNotStringLiteral reason — replaces name's value with a Bool literal).
    • method_ref_decl_not_method_declaration_surfaces_typed_error updated to assert ConnectiveNotInstantiation reason.
  • Lookup invalid-key split preserved. lookup_helper_three_distinct_states integration test still asserts hit / miss / invalid distinctly; invalid case now also asserts the ConnectiveNotInstantiation reason.

Test count at HEAD: 10 in-module unit tests + 4 integration tests pass. cargo clippy --tests -- -D warnings clean; cargo fmt --all --check clean.

Disposition unchanged: oracle-only; does not close Gap 4. The four-clause data-binding gate is correctness-only, independent of the parked architecture decision.

— sent from silent-boar-29

@briansrls

Copy link
Copy Markdown
Contributor Author

Non-blocking note acknowledged. Holding off on the file-size split for this PR with rationale:

  • Reviewer (and an earlier claude-opus-4-7 exploratory note) both classify this explicitly as non-blocking.
  • The "if this file will keep growing" framing is the load-bearing condition. The PR is held open as oracle-only per Director / Grounding / PB / Substrate disposition (#issuecomment-4365827471); the build-step surface is parked on missing ephemeral generated-source-root support. Net new growth from this surface is bounded — Gap 5 / leaf migration moves to a separate PR if and when the architecture decision unblocks.
  • Of the ~1155 lines, the bulk is the in-module unit-test suite (≈700 lines covering 10 fail-closed sub-checks). Tests live in-module specifically because they need crate-private Dag::declaration_mut to construct malformed bootstrap shapes; that placement is justified per the file header and matches TESTING.md's carve-out for test code.
  • A clean split is straightforward whenever the file does grow (move #[cfg(test)] mod tests to a pb_method_template_projection/tests.rs sibling with mod tests; declaration) and is appropriate when there's a second reason to touch the module. Doing it now in the held-open oracle-only PR adds churn without changing behavior or addressing the reviewer's "if it keeps growing" condition.

Disposition unchanged: oracle-only; does not close Gap 4. Will return to the file-size split as part of whichever PR is dispatched after the architecture decision lands and grows the module.

— sent from silent-boar-29

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: 79260bad · Trigger: manual
  • Comparison: main @ 9dee7451 ... session/silent-boar-29-pb-method-template-projection @ 8a8b0dd4
  • Conversation: View conversation

1. Story of the diff

This PR adds a PB-bootstrap read surface that projects MethodTemplateContract rows out of the already-lowered full bootstrap Dag, rather than introducing a separate template table. The new public module is exported from src/v3/compiler/src/lib.rs:20 and exposes two free-function APIs: method_template_contract_rows for the full per-target row list and method_template_contract_row for direct (target, MethodDeclaration) lookup. The load-bearing mechanics are the typed Rust projection carriers at src/v3/compiler/src/pb_method_template_projection.rs:96-101, the closed-schema row validation at src/v3/compiler/src/pb_method_template_projection.rs:383-390, and the fail-closed MethodRef.decl validation at src/v3/compiler/src/pb_method_template_projection.rs:531-556.

The tests cover both the green bootstrap path and malformed in-memory Dag shapes: integration tests assert all three target lists project with all five fields and preserve sum identities, while module-local tests mutate the bootstrap Dag to verify duplicate fields, unknown fields, non-method refs, duplicate rows, and non-empty nullary placeholder payloads surface as typed errors. The SG-0 census is also updated for the new hand-authored source and test file.

2. Invariant categories

  1. LAYER MODEL — Compliant. This is substrate-adjacent but does not add Dag substrate variants; it keeps the single row authority in the .dag row-list declarations (src/v3/compiler/src/pb_method_template_projection.rs:5-7) and exposes a typed projection over the bootstrap Dag (src/v3/compiler/src/pb_method_template_projection.rs:96-101) rather than a parallel template map.
  2. INVARIANTS.md + modeling-discipline.md — Compliant. Fail-closed and single-authority are handled explicitly: missing or malformed list declarations return typed errors (src/v3/compiler/src/pb_method_template_projection.rs:285-295), row schema is closed against unknown and duplicate fields (src/v3/compiler/src/pb_method_template_projection.rs:450-470), per-target duplicate dag_method rows reject at the canonical row-list boundary (src/v3/compiler/src/pb_method_template_projection.rs:309-318), and MethodRef.decl must reference a MethodDeclaration data binding rather than merely a plausible declaration (src/v3/compiler/src/pb_method_template_projection.rs:531-556). The new coproduct mirrors also carry terminal annotations (src/v3/compiler/src/pb_method_template_projection.rs:31-35, src/v3/compiler/src/pb_method_template_projection.rs:60-65, src/v3/compiler/src/pb_method_template_projection.rs:80-82). chatgpt-review-672e2d70-2174-4d…

chatgpt-review-f79080f6-34c3-4a…

  1. CODING.md — Finding, non-blocking. Small/composable: src/v3/compiler/src/pb_method_template_projection.rs:700: #[cfg(test)] shows the new production portion reaches 699 lines before the internal tests even begin, and the file continues through src/v3/compiler/src/pb_method_template_projection.rs:1134: }. The implementation shape is otherwise clean — data carriers plus free functions, explicit &Dag dependencies, and structured Result APIs — but this new module exceeds the ~500-line module smell; a follow-up split of the internal tests or projection helpers would make the hook easier to audit. chatgpt-review-a2c786ed-9355-4a…
  2. TESTING.md — Compliant. The diff adds both behavior-level integration coverage and unit-style malformed-shape coverage: src/v3/compiler/tests/integration/pb_method_template_projection_test.rs:37 checks all targets/all five fields, src/v3/compiler/tests/integration/pb_method_template_projection_test.rs:117 checks projected uniqueness, src/v3/compiler/tests/integration/pb_method_template_projection_test.rs:166 checks hit/miss/invalid lookup states, and the module-local tests at src/v3/compiler/src/pb_method_template_projection.rs:700-706 explain why crate-private Dag mutation is used for impossible-on-disk malformed rows. chatgpt-review-85b08fe0-a3e7-47…
  3. LOCKED DESIGN DECISIONS — Compliant. The module explicitly ties itself to the row-85 decision (src/v3/compiler/src/pb_method_template_projection.rs:4-6) and scopes this change to Gap 4, leaving LanguageSpec.method_templates and v2 leaf-emit migration sequenced after this hook (src/v3/compiler/src/pb_method_template_projection.rs:21-23). I do not see a silent divergence from a locked design in the diff.
  4. TRACKED vs UNTRACKED DEBT — Compliant. The new hand-authored source file is admitted to the SG-0 census with its row-85/PB docs(r3): R3 row-85 PB-Zero method-template projection packet (decision-gated) #1560 Gap 4 purpose documented (src/v3/compiler/tests/integration/sg0_census_test.rs:224-228), and the new hand-authored integration test is bounded with an explicit dissolution trigger: it stays until testgen covers reflected-Dag structural assertions over std row authorities (src/v3/compiler/tests/integration/sg0_census_test.rs:372-376). I do not see untracked TODOs or an unbounded temporary bridge in the diff.

3. Verdict

APPROVE_WITH_COMMENTS

No blocking substrate or invariant issue showed up: the projection preserves the .dag row authority, validates closed schema and method references fail-closed, and adds meaningful regression coverage. The only comment is implementation-shape debt: the new projection module is large enough to merit a later split for auditability.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified current head 8a8b0dd47 against the queued MethodDeclaration data-binding review on c4445e98. The blocking item is addressed: row projection and direct lookup now share MethodDeclaration binding validation, requiring Instantiation<MethodDeclaration>, structural value_body, a closed name field set, and string-literal name, with typed MethodDeclarationBindingViolation reasons. The malformed-Dag tests cover the new missing/extra/non-string cases, and the lookup invalid-key split remains covered.

The remaining current-head review comments are nonblocking module-size/split notes. No fix is required for the oracle-only scope. CI at current head still has ci and v3 running; fmt is green.

— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: 8a8b0dd4 · Trigger: schedule
  • Comparison: origin/main @ 9dee7451 ... review/pr-1568-8a8b0dd4 @ 8a8b0dd4
  • Thinking: 73s wall

Verdict: APPROVE

The projection hook is fail-closed on the row/list/schema paths I checked, preserves the substrate sum shapes as typed enums, validates MethodRef.decl against the MethodDeclaration data-binding contract, and keeps the row authority in the bootstrap Dag rather than introducing a parallel template table. The added tests and SG-0 census entries are scoped to this new hand-authored bridge with a stated dissolution path. No concrete invariant, modeling-discipline, coding, or testing violations observed in the diff.

I did not run builds or tests per the review instructions.

@briansrls
briansrls merged commit 06d7755 into main May 3, 2026
4 checks passed
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

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

1. Story of the diff

This PR adds a new public PB bootstrap projection surface, pb_method_template_projection, and exports it from src/v3/compiler/src/lib.rs:20. The new module reads the target-specific MethodTemplateContract list declarations from the full bootstrap Dag, keeps the .dag row lists as the single row authority, and projects each row into typed Rust carriers rather than a parallel template-text table (src/v3/compiler/src/pb_method_template_projection.rs:5-19). The load-bearing path is: select a target row list, require the declaration to be a list, project each record through closed-schema checks, validate MethodRef.decl against the MethodDeclaration data-binding shape, preserve emit_template / placeholder_convention as enum-shaped carriers, and reject duplicate (target, dag_method) rows (src/v3/compiler/src/pb_method_template_projection.rs:316-357, src/v3/compiler/src/pb_method_template_projection.rs:609-662, src/v3/compiler/src/pb_method_template_projection.rs:665-803).

The PR also adds a direct lookup helper that distinguishes valid hit, valid miss, and invalid non-method key (src/v3/compiler/src/pb_method_template_projection.rs:380-397), plus tests for both green bootstrap behavior and malformed in-memory Dag cases. The integration tests prove the public projection over generated_full_bootstrap_dag() and the helper’s three-state contract (src/v3/compiler/tests/integration/pb_method_template_projection_test.rs:37-234), while the source-local tests inject malformed rows that the real row authority should not be able to produce (src/v3/compiler/src/pb_method_template_projection.rs:805-811). SG-0 census entries are updated for the new hand-authored source and test file (src/v3/compiler/tests/integration/sg0_census_test.rs:224-228, src/v3/compiler/tests/integration/sg0_census_test.rs:372-376).

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Compliant — this is implementation-side Rust projection over existing bootstrap Dag substrate facts, not a new Dag substrate type: the module explicitly keeps the row authority in src/v3/std/{rust,python,go}_method_template_contracts.dag and exposes only a target-keyed read surface (src/v3/compiler/src/pb_method_template_projection.rs:5-19). The public APIs take &Dag and return typed projection results without mutating substrate state (src/v3/compiler/src/pb_method_template_projection.rs:316-319, src/v3/compiler/src/pb_method_template_projection.rs:380-384).

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

Finding — NON-BLOCKING, coproduct dissolution checkpoint. The new private test helper enum has no 🟢/🟡/🔴 classification comment: src/v3/compiler/src/pb_method_template_projection.rs:1010: enum MethodRefMutation<'a> {. The public/projection enums are checkpointed correctly, but modeling discipline says every new Rust enum with two or more variants needs a classification annotation. This is test-local and easy to fix with a one-line terminal/scaffold comment, so I would not block the projection behavior on it.

  1. CODING.md.

Compliant — the module follows data + free functions over object behavior: method_template_contract_rows and method_template_contract_row are pure readers over &Dag returning Result<...> (src/v3/compiler/src/pb_method_template_projection.rs:316-319, src/v3/compiler/src/pb_method_template_projection.rs:380-384). The boundary errors are structured enums rather than string sentinels or fallbacks (src/v3/compiler/src/pb_method_template_projection.rs:113-268), and the closed-schema helper is factored as a free function (src/v3/compiler/src/pb_method_template_projection.rs:554-590).

  1. TESTING.md.

Compliant — the diff adds focused source-local unit tests for malformed shapes requiring crate-private Dag mutation (src/v3/compiler/src/pb_method_template_projection.rs:805-811) and integration tests for the public behavior over the full bootstrap snapshot (src/v3/compiler/tests/integration/pb_method_template_projection_test.rs:37-234). The helper’s key semantic boundary — hit vs miss vs invalid non-method key — is directly tested through the public API (src/v3/compiler/tests/integration/pb_method_template_projection_test.rs:166-234).

  1. LOCKED DESIGN DECISIONS.

Compliant — the diff references the row-85 decision and stays inside the declared scope: Gap 4 projection only, with LanguageSpec.method_templates rewrite and v2 leaf emit migration explicitly out of scope (src/v3/compiler/src/pb_method_template_projection.rs:4-23). I do not see a divergence from a locked design decision in the changed lines.

  1. TRACKED vs UNTRACKED DEBT.

Compliant — the new hand-authored source file is admitted to the SG-0 census with its R3 row 85 / PB #1560 Gap 4 purpose named (src/v3/compiler/tests/integration/sg0_census_test.rs:224-228), and the new hand-authored test file is also admitted with an explicit “until testgen covers reflected-Dag structural assertions” trigger (src/v3/compiler/tests/integration/sg0_census_test.rs:372-376). No new TODOs or unbounded temporary scaffolds showed up in the changed lines.

3. Verdict

APPROVE_WITH_COMMENTS

The projection itself is fail-closed, single-authority with respect to row text, and well covered by targeted tests. The only issue I found is a small modeling-discipline miss on a private test helper enum; it should be cleaned up, but it does not undermine the public projection contract.

briansrls added a commit that referenced this pull request May 3, 2026
…pline)

Per openai-pro / gpt-5-5-pro review on PR #1568 head 8a8b0dd
(non-blocking, modeling-discipline coproduct dissolution checkpoint):

The test-local `MethodRefMutation` enum drives the two `MethodRef`
mutation modes consumed by the closed-schema duplicate/unknown-field
unit tests; modeling-discipline.md asks every multi-variant Rust enum
to carry a 🟢/🟡/🔴 classification. Add a 🟢 TERMINAL annotation
naming the closed mutation scope (duplicate decl field; append
unknown field) and noting the enum is test-only.

No code change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
briansrls added a commit that referenced this pull request May 3, 2026
…l blocked (#1569)

* docs(r3): retire rows 77, 91 + Phase-1 partial-close row 85

Bundle 1 + 4b dispatch closure receipts:
- row 77 (duplicate record-literal fields) → Retired by PR #1551
- row 91 (SubValueRelation BoundedLattice claim) → Retired by PR #1543

Bundle 3 Phase 1 progress:
- row 85 (method-template consumer migration) → Partial; PR #1549
  audit landed; Phase 2 in flight (#1560, #1561)

Per-PR Debt-Paydown receipt against rows 77, 85, 91.

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

* docs(r3): correct row-85 framing — #1568 is oracle-only, Gap 4/5 still blocked

Per Director correction (#828 reply 4365998510): #1568 is a Rust
projection / test-oracle helper only, not the Phase-2 consumer-surface
closure. Gap 4 (build-step consumer surface) + Gap 5 remain blocked on
substrate build-pipeline support for ephemeral generated-source-root
.dag imports (tied to #1558 dissolution-first reframe), or on a Gap 5
design that avoids that surface.

Records the routing of all 5 gaps from the #1549 Phase 1 audit:
- Gap 1, Gap 2: active in Substrate
- Gap 3: reference-only via BootstrapAuthority carrier #1554 (landed)
- Gap 4, Gap 5: blocked, calm-tern Phase 2 leaf migration parked

Per-PR Debt-Paydown receipt against ledger row 85.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
briansrls added a commit that referenced this pull request May 3, 2026
* feat(r3): PB consumer hook projects MethodTemplateContract rows from bootstrap Dag (#1560 Gap 4)

Implements the PB-Bootstrap-Process consumer hook for R3 row 85 / PB #1560
Gap 4 per the canonical decision in
`docs/decisions/r3-row85-method-template-read-surface.md`.

The single row authority remains
`src/v3/std/{rust,python,go}_method_template_contracts.dag`. This module
adds a target-keyed structural projection over the lowered bootstrap `Dag`
so PB-zero / v2-retirement consumers can select rows by
target + `MethodRef.decl` without:

- importing `v3.std.*` into v2 crates (Gap 4 acceptance A1),
- introducing a second `Map<String, String>` template-text authority (A2),
- copying any template strings into v2 sources (A3).

All five `MethodTemplateContract` fields are preserved by typed Rust
mirrors (A4): `dag_method` (DeclarationId via MethodRef.decl),
`runtime_template` (String), `emit_template` (sum: Single |
HigherOrder), `wraps_result` (bool), `placeholder_convention` (sum:
IndexedArgs | NamedArg). The two sum carriers stay constructor-tagged
enums — substrate identity is preserved, not flattened to strings.

API:

  pub fn method_template_contract_rows(
      dag: &Dag,
      target: MethodTemplateTarget,
  ) -> Result<Vec<MethodTemplateContractRow>, MethodTemplateProjectionError>

Errors are typed and fail-closed (per INVARIANTS.md C-8): every observable
mismatch between the bootstrap Dag shape and the substrate carrier
surfaces as a typed value rather than a panic.

Tests (`tests/integration/pb_method_template_projection_test.rs`):

1. `projects_every_target_with_all_five_fields` — projects Rust/Python/Go
   from `generated_full_bootstrap_dag()`; asserts non-empty rows and the
   five-field receipt for each row.
2. `projection_preserves_per_target_dag_method_uniqueness` — mirrors the
   substrate-side `method_template_contract_per_target_dag_method_unique`
   claim through the public projection API; if the projection drifts
   from the substrate carrier, this fails before any v2 consumer would.
3. `rust_count_row_anchors_runtime_emit_drift` — spot-checks the
   documented `count` runtime/emit template drift
   (`{recv}.len()` vs `({recv}.len() as i64)`) against real row text;
   anchors the projection without reproducing template strings as
   fixtures.

Out of scope (per dispatch / packet §5.3):

- `LanguageSpec.method_templates` rewrite (Gap 5).
- v2 leaf-emit migration.
- Row population (`string_contains` Python/Go, Go `chars` parity gaps).
- `LEGACY_METHOD_TEMPLATE_AUTHORITIES` ratchet stays live until Gap 5
  and leaf-emit migrations land (A6).

R3 debt receipt: Debt found + routed / partial implementation against
ROADMAP.md:512 and ledger row 85; closes Gap 4 only. Gap 5 sequenced
strictly after this PR merges.

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

* WIP: silent-boar-29

* chore: apply cargo fmt

* WIP: silent-boar-29

* chore: apply cargo fmt

* test(r3): admit pb_method_template_projection files to SG-0 hand-authored census

Per CI failure on 424b7ec: adding the new hand-Rust files
`src/v3/compiler/src/pb_method_template_projection.rs` and
`src/v3/compiler/tests/integration/pb_method_template_projection_test.rs`
drifts the SG-0 ratchet at
`src/v3/compiler/tests/integration/sg0_census_test.rs`. Add both files
to `EXPECTED_HAND_AUTHORED_NON_TEST` and `EXPECTED_HAND_AUTHORED_TEST`
respectively, with director-sign-off comments anchoring them to the
R3 row 85 / PB #1560 Gap 4 dispatch (decision doc
`docs/decisions/r3-row85-method-template-read-surface.md`).

The new module + focused test are the canonical PB consumer hook + its
acceptance receipt; both stay hand-Rust until the testgen path covers
reflected-Dag structural assertions over std/ row authorities, same
discipline as `method_template_contract_test.rs`.

Validation:
- cargo test -p v3-compiler --test integration sg0_census — 12/12 pass
- cargo test -p v3-compiler --test integration pb_method_template_projection — 4/4 pass
- cargo test -p v3-compiler --lib pb_method_template_projection — 1/1 pass

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

* docs(r3): add modeling-discipline 🟢 TERMINAL tags to projection enums

Per cursor exploratory note on PR #1568 head 424b7ec (non-blocking,
explicitly optional): docs/modeling-discipline.md Practice 4 asks for
inline 🟢 / 🟡 / 🔴 classification on new multi-variant Rust enums.

Add 🟢 TERMINAL tags to all four enums in pb_method_template_projection
with anchoring rationale:

- MethodTemplateTarget — terminal at per-target row-list scope; mirrors
  the three closed list-declaration authorities.
- MethodEmitTemplateProjection — terminal at target method-emission-
  template scope, inheriting from substrate carrier MethodEmitTemplate
  (already marked 🟢 TERMINAL at emit_model.dag:453).
- PlaceholderConventionProjection — terminal nullary sum mirroring the
  closed substrate carrier.
- MethodTemplateProjectionError — terminal at bootstrap-mismatch scope;
  closed substrate-mismatch enumeration, not a transitional grab-bag.

No code change.

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

* WIP: silent-boar-29

* chore: apply cargo fmt

* WIP: silent-boar-29

* fix(r3): closed-schema validation on row + MethodRef carriers (BLOCKING)

Per inline review at src/v3/compiler/src/pb_method_template_projection.rs:310:

`field_by_label` used `.find()` over the row fields, accepting the first
matching field and silently ignoring duplicates. `project_row` had no
closed-schema check, so a row with an extra unknown field
(`{ dag_method: ..., renamed_field: ... }`) would project successfully
with the unknown silently discarded. Same gap on `MethodRef` (the inner
`{ decl: ... }` record).

P2 single-authority + C-8 fail-closed both want every observable
substrate-shape mismatch to surface as a typed error.

Add typed errors:
- `RowDuplicateField { list, row_index, record, field, first_field_index, duplicate_field_index }`
- `RowUnknownField { list, row_index, record, field, field_index }`

Replace `field_by_label` with `closed_record_lookup(list, row_index, record, fields, expected)`
that walks fields once, fails-closed on duplicate or unknown labels, and
returns a `HashMap<&str, &FieldValue>`. `lookup_required` then surfaces
`RowMissingField` per-field. Apply to both `MethodTemplateContract`
(five expected labels) and `MethodRef` (single `decl` label).

Four new in-module unit tests cover the failure modes:
- duplicate_field_on_row_surfaces_typed_error
- unknown_field_on_row_surfaces_typed_error
- duplicate_field_on_method_ref_surfaces_typed_error
- unknown_field_on_method_ref_surfaces_typed_error

Validation:
- cargo test -p v3-compiler --lib pb_method_template_projection — 6/6 pass
- cargo test -p v3-compiler --test integration pb_method_template_projection — 4/4 pass
- cargo clippy -p v3-compiler --tests -- -D warnings — clean
- cargo fmt --all --check — clean

Patch is correctness-only; #1568 still held open as oracle/helper
pending Director / Grounding / PB rescope.

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

* fix(r3): fail-close MethodRef.decl on non-MethodDeclaration target (BLOCKING)

Per inline review at src/v3/compiler/src/pb_method_template_projection.rs:335:

`project_dag_method` returned any `DeclarationId` from `MethodRef.decl`
without verifying it instantiates `MethodDeclaration`. `src/v3/std/methods.dag`
explicitly calls out the boundary contract: today's substrate grammar
can't express `DeclarationRef<MethodDeclaration>` (refinement-typing-on-
DeclarationRef gap), so the projection must enforce fail-closed at the
boundary that `decl` references a `MethodDeclaration`-instantiating
data binding.

Pattern mirrors `method_registry_test.rs::method_registry_covers_*`:
look up `MethodDeclaration`'s declaration ID, walk the referenced
`Declaration`'s `connective`, require
`TypeConnective::Instantiation { template, .. }` where
`template == method_declaration_id`.

Add typed errors:
- `MethodRefDeclNotMethodDeclaration { list, row_index, decl_id }`
- `MethodDeclarationCarrierMissing` (substrate degenerate case)

Add unit test
`method_ref_decl_not_method_declaration_surfaces_typed_error` that
mutates the first row's `MethodRef.decl` to point at
`MethodTemplateContract` (a type declaration, not a `MethodDeclaration`
instance) via crate-private `Dag::declaration_mut`, and asserts the
typed error.

Existing integration test
`lookup_helper_finds_known_row_and_returns_none_for_unknown_method`
remains green: it passes a non-`MethodDeclaration` ID into the lookup;
since every projected `row.dag_method` is now verified to be a
`MethodDeclaration`, the comparison never matches and the helper
correctly returns `None` (not "fabricate a row").

Validation:
- cargo test -p v3-compiler --lib pb_method_template_projection — 7/7 pass
- cargo test -p v3-compiler --test integration pb_method_template_projection — 4/4 pass
- cargo clippy -p v3-compiler --tests -- -D warnings — clean
- cargo fmt --all --check — clean

Patch is correctness-only; #1568 still held open as oracle/helper
pending Director / Grounding / PB rescope.

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

* WIP: silent-boar-29

* style(r3): stash &'static str at match time to avoid duplicate-path expect()

Per cursor review on PR #1568 head 145f583 (non-blocking):
`closed_record_lookup` previously re-scanned `expected` on the
duplicate-field path with `.expect("expected label must match a known
constant")` to recover a `&'static str`. CODING.md prefers no
hidden-panic surfaces in library code even when logically unreachable.

Stash the matched `&'static str` at insertion time and key the lookup
maps by that static reference; both the duplicate path and the unknown
path are handled by the single `find` at the top of the loop. The
`.expect()` is gone.

Also addresses the analogous claude-opus exploratory note about avoiding
the re-scan on the duplicate path.

Validation:
- cargo test -p v3-compiler --lib pb_method_template_projection — 7/7 pass
- cargo test -p v3-compiler --test integration pb_method_template_projection — 4/4 pass
- cargo clippy -p v3-compiler --tests -- -D warnings — clean
- cargo fmt --all --check — clean

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

* chore: apply cargo fmt

* WIP: silent-boar-29

* chore: apply cargo fmt

* fix(r3): fail-close lookup helper on non-MethodDeclaration key (BLOCKING)

Per openai-pro / gpt-5-5-pro review on PR #1568 head 145f583 (BLOCKING,
fail-closed / illegal-states-unrepresentable / API-level enforcement):

`method_template_contract_row` accepted any `DeclarationId` and returned
`Ok(None)` if it didn't match a row, conflating two states the boundary
should distinguish:

- "valid `MethodDeclaration` with no row for this target" (legitimate)
- "caller supplied a non-`MethodDeclaration` declaration" (programmer bug)

The integration test cemented the conflation by passing
`MethodTemplateContract` (a type) as a "missing" key.

Add typed error
`MethodTemplateProjectionError::LookupKeyNotMethodDeclaration { decl_id }`.
The helper now validates `dag_method` instantiates `MethodDeclaration`
at entry (mirrors the per-row `project_dag_method` check), surfacing the
typed error rather than `Ok(None)` for non-method keys. Until
`DeclarationRef<MethodDeclaration>` refinement-typing lands, this is
the only place the boundary can hold.

Rewrite the integration test as
`lookup_helper_three_distinct_states` covering all three:
- Hit: `count_method` (real MethodDeclaration with a Rust row)
- Miss: `add_method` (real MethodDeclaration, no Rust row in
  `dsl/std/methods.dag` ↔ `rust_method_template_contracts.dag`)
- Invalid: `MethodTemplateContract` (not a MethodDeclaration) →
  typed error.

Validation:
- cargo test -p v3-compiler --lib pb_method_template_projection — 7/7 pass
- cargo test -p v3-compiler --test integration pb_method_template_projection — 4/4 pass
- cargo clippy -p v3-compiler --tests -- -D warnings — clean
- cargo fmt --all --check — clean

Disposition unchanged: oracle-only; does not close Gap 4.

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

* WIP: silent-boar-29

* chore: apply cargo fmt

* WIP: silent-boar-29

* chore: apply cargo fmt

* WIP: silent-boar-29

* fix(r3): require Stratum-A-grade MethodDeclaration data-binding shape (BLOCKING)

Per parent direction on PR #1568 (inbox #1134): align the projection-side
MethodDeclaration registry boundary with the Stratum A gate at
`src/v3/grounding_tests/src/stratum_a.rs::method_registry_name`.
Previously the helper validated only "Instantiation { template =
MethodDeclaration } + value_body.is_some()". The substrate-side contract
is stricter:

  data <name>_method: MethodDeclaration = { name: "..." }

…which means the data binding must also carry a `Structural` value body
with a closed schema of exactly one `name` field whose value is a string
literal. A binding with extra fields, wrong-shaped name, or non-Structural
body would have passed the previous check.

Refactor the shared helper to enforce the four-clause contract:

  fn validate_method_declaration_data_binding(dag, decl_id, method_decl_id)
      -> Result<String /* method name */, MethodDeclarationBindingViolation>

  1. Connective is Instantiation.
  2. Instantiation.template == MethodDeclaration.
  3. value_body is Some(ValueBody::Structural { fields }).
  4. fields are exactly `["name"]` with FieldValue::Literal(LiteralBits::String).

New typed reason enum `MethodDeclarationBindingViolation` (six variants:
ConnectiveNotInstantiation / InstantiationTemplateNotMethodDeclaration /
ValueBodyMissing / ValueBodyNotStructural /
StructuralFieldsNotClosedNameOnly { observed_labels } /
NameNotStringLiteral). Both umbrella errors (`MethodRefDeclNotMethodDeclaration`,
`LookupKeyNotMethodDeclaration`) carry a `reason` field naming which
sub-check failed.

Two new in-module unit tests cover the new sub-checks:

- `method_ref_decl_extra_field_surfaces_closed_schema_violation` —
  injects an `alias` field beyond the closed `[name]` schema.
- `method_ref_decl_name_not_string_literal_surfaces_typed_error` —
  replaces `name`'s value with a Bool literal.

Existing tests updated to assert the granular reason where appropriate
(`ValueBodyMissing` for the alias-shape mutation,
`ConnectiveNotInstantiation` for the type-declaration target).

Validation:
- cargo test -p v3-compiler --lib pb_method_template_projection — 10/10 pass
- cargo test -p v3-compiler --test integration pb_method_template_projection — 4/4 pass
- cargo clippy -p v3-compiler --tests -- -D warnings — clean
- cargo fmt --all --check — clean

Disposition unchanged: oracle-only; does not close Gap 4.

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

* chore: apply cargo fmt

* docs(r3): annotate MethodRefMutation test helper enum (modeling-discipline)

Per openai-pro / gpt-5-5-pro review on PR #1568 head 8a8b0dd
(non-blocking, modeling-discipline coproduct dissolution checkpoint):

The test-local `MethodRefMutation` enum drives the two `MethodRef`
mutation modes consumed by the closed-schema duplicate/unknown-field
unit tests; modeling-discipline.md asks every multi-variant Rust enum
to carry a 🟢/🟡/🔴 classification. Add a 🟢 TERMINAL annotation
naming the closed mutation scope (duplicate decl field; append
unknown field) and noting the enum is test-only.

No code change.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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