Repository navigation
CollectionOps consume Rust HO MethodTemplateContract refs - #1665
Conversation
…nostic-ordering-carrier
…nostic-ordering-carrier
…nostic-ordering-carrier
…nostic-ordering-carrier
…nostic-ordering-carrier
…nostic-ordering-carrier
…nostic-ordering-carrier
…nostic-ordering-carrier
…nostic-ordering-carrier
…nostic-ordering-carrier
…nostic-ordering-carrier
…nostic-ordering-carrier
…nostic-ordering-carrier
…ring-carrier' into r3-string-family-diagnostic-ordering-carrier
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
46c84cab· Trigger:schedule - Thinking:
285s wall
Non-blocking — Strengths
src/v3/std/emit_model.dagThe legacy CollectionOps literal-field scaffold now names the dissolution trigger required by INVARIANTS.md P5.
✅ No blocking concerns in the current diff.
|
Verified against current #1665 head The reference-row branch now resolves the referenced declaration, fetches the canonical Relevant current code is — sent from silent-hawk-421 |
|
Review metadata
Findings (if any)
No INVARIANTS / modeling-discipline / VerdictAPPROVE_WITH_COMMENTS — The main change is coherent and aligned with single-authority contracts and fail-closed parsing; only the test-helper messaging above is worth a quick follow-up (optional in this PR). Exploratory observations (optional)
|
|
Review metadata
FindingsNone. The substantive diff moves Rust VerdictAPPROVE — Scoped implementation-layer change: single-source contracts for Rust collection HO templates, explicit fail-closed checks, reference rows handled in projection, and a focused rustc roundtrip regression for non-copy |
|
Review metadata
FindingsNone. The diff moves Rust VerdictAPPROVE — Scoped, aligns with single-authority template data in Exploratory observations (optional)
|
…nostic-ordering-carrier # Conflicts: # src/v3/compiler/src/bootstrap_generated.rs # src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
022f63a3· Trigger:schedule - Thinking:
317s wall
BLOCKING (1)
Root Cause
src/v3/std/rust_method_template_contracts.dagthe HO filter template changed the receiver from method-call auto-borrow to a value let-binding → render the loop over ({iter}).iter() or add a typed borrowed-receiver placeholder before using MethodTemplateContract here.
| dag_method: { decl: filter_method }, | ||
| runtime_template: "{ let __filter_source = {iter}; let mut __result = Vec::new(); for {param} in __filter_source.iter() { if {body} { __result.push((*{param}).clone()); } } __result }", | ||
| emit_template: HigherOrderTemplates { | ||
| inline_template: "{ let __filter_source = {iter}; let mut __result = Vec::new(); for {param} in __filter_source.iter() { if {body} { __result.push((*{param}).clone()); } } __result }", |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Verified against current #1665 head The template still binds — sent from silent-hawk-421 |
…nostic-ordering-carrier
|
Review metadata
FindingsNone. The diff tightens single-authority for Rust VerdictAPPROVE — Substrate and Rust wiring look consistent with boundary discipline and fail-closed emission; no invariant or style violation surfaced in the reviewed diff. |
|
Review metadata
Findings: None. The diff tightens single authority for Rust Verdict: APPROVE — Scoped, consistent with INVARIANTS / modeling-discipline / CODING / TESTING for what this diff changes; no concrete rubric violations observed. |
|
Review metadata
FindingsNone. The diff tightens single authority for Rust higher-order collection templates by routing VerdictAPPROVE — Implementation-layer refactor aligned with P2 (one contract authority, consumers read through declared refs) and P3 (malformed contracts surface as typed errors, not silent defaults). No concrete rubric violations tied to specific diff lines. Exploratory observations (optional)
|
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
49c61def· Trigger:schedule - Thinking:
284s wall
Non-blocking — Strengths
src/v3/std/emit_model.dagThe legacy CollectionOps literal scaffold is documented, bounded to unmigrated render paths, and has a named deletion trigger, so it is tracked rather than accumulating debt.
✅ No blocking concerns; the PR resolves the prior fail-closed and reference-row issues while keeping the remaining literal bridge tracked.
|
Review metadata
1. Story of the diffThis PR moves Rust The supporting change is that method-template contract lists can now contain declaration references to named 2. Invariant categories
3. VerdictAPPROVE The PR moves Rust filter emission toward the declared contract authority without introducing a parallel template source, and the temporary leftover literal fields are explicitly bounded with a dissolution trigger. The fail-closed projection and Rust selection tests cover the risky seams I would expect for this change. |
…nostic-ordering-carrier
|
Review metadata
Findings: None. Nothing in the diff clearly breaks INVARIANTS.md, docs/modeling-discipline.md, CODING.md, or TESTING.md in a way that needs a blocking or must-fix callout. Rust filter emission now follows Verdict: APPROVE — Scope matches the title: CollectionOps and Rust emission consume HO contract refs; Go/Python get parallel Exploratory observations (optional): |
…nostic-ordering-carrier
…nostic-ordering-carrier # Conflicts: # src/v3/compiler/src/bootstrap_generated.rs # src/v3/compiler/tests/integration/parse_corpus_manifest.txt
1 participant
Summary
This PR completes the CollectionOps higher-order MethodTemplateContract consumption slice for Rust
filter/flat_map/any/all. The existingMethodEmitTemplatecoproduct remains the authority: Rust CollectionOps now reads contract references carried byCollectionOps.{filter,flat_map,any,all}_contract, validates the referenced method identity, requiresHigherOrderTemplates, and selects the inline template for current render paths. It fails closed on SingleTemplate, wrong method refs, malformed payloads, or missing fields.The HO Rust contract rows are now named declarations and are reused both by
rust_method_template_contractsand byrust_collection_ops, so Grounding can migrate the Rust HO quartet without copying literal templates into a third authority. Go/Python CollectionOps also receive matching contract refs becauseCollectionOpsnow structurally requires these fields; their legacy literal fields remain for current unmigrated render paths.This also addresses the blocking review on first-match list scanning: the Rust CollectionOps parser no longer scans
rust_method_template_contractsby list name. It consumes the explicit contract ref from the CollectionOps value instead.Validation
cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrapcargo fmt --all --checkcargo test -p v3-compiler rust_collection_ops_selects_higher_order_inline_templates --libcargo test -p v3-compiler rust_collection_ops_rejects_single_template_for_higher_order_selection --libAttempted but blocked by existing integration-test target setup before the selected test could run:
cargo test -p v3-compiler --test integration rustc_roundtrip_list_filter_then_fold_prints_sevenfailed compilingsrc/v3/compiler/tests/integration/p0_std_render_repeat_string_test.rswithcan't find crate for v2_compiler_tests.regen_bootstrap -- --verifywas also attempted after the successful regen, but Cargo repeatedly stuck on a stale artifact-directory lock with no compiler process; I did not treat that optional pass as blocking after the successful regen plus focused tests.