Skip to content

feat(v3): classify string_contains in method registry; populate Python+Go contract rows - #1561

Merged
briansrls merged 9 commits into
mainfrom
session/tidy-tern-769-disp2
May 3, 2026
Merged

briansrls merged 9 commits into
mainfrom
session/tidy-tern-769-disp2

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Summary

  • Debt paid: ROADMAP.md:512 / docs/debt/r3-debt-paydown-ledger-2026-05-02.md:85 — Method-template consumer migration. Closes one of the five Phase-2 blockers from the docs(grounding): method-template consumer migration audit-to-retirement #1549 audit (Gap 1 — string_contains classification + skipped Python/Go contract rows).
  • Substrate: dsl/std/methods.dag adds string_contains_method: MethodDeclaration = { name: "string_contains" } and broadens the documented registry scope to include target-only emit-shortcut method names. Rationale: the legacy emit map (dsl/extdeps/languages/{python,go}/emit.dag) disambiguates the String-substring shape from the collection-membership contains shape by name; v3's flat-name registry preserves that disambiguation by promoting string_contains to a first-class MethodRef row key rather than overloading contains_method.
  • Per-target rows populated:
    • src/v3/std/python_method_template_contracts.dag — {arg} in {recv} (matches python/emit.dag:95).
    • src/v3/std/go_method_template_contracts.dag — strings.Contains({recv}, {arg}) (matches go/emit.dag:87).
    • Both files retire the "deferred to Phase 2 / Modeling" header for this row.
  • Snapshot/manifest refreshes follow mechanically (regen helpers, not hand-edited): bootstrap_{generated,generated_without_parse_surface,std_generated}.rs and parse_corpus_manifest.txt.

Out of scope (per dispatch): Phase-2 deletion of legacy *_method_templates, LanguageSpec rewrite, PB projection work, Go chars row (#1549 Gap 4 — separate tokenizer-level issue).

Test plan

  • cargo test -p v3-compiler --test integration -- method_registry method_template_contract m2_substrate_inhabitance — 78 passed (covers method_registry_covers_all_algebra_template_names, method_template_contract_per_target_dag_method_unique, bootstrap_method_template_contracts_lower_to_list_with_empty_diagnostics).
  • cargo test -p v3-compiler --test integration — 768 passed; 26 ignored; 0 failed.
  • cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap — clean regen.
  • cargo test -p v3-compiler --test integration handwritten_parse_snapshot_matches_manifest passes after refresh.
  • cargo clippy -p v3-compiler --all-targets -- -D warnings.
  • cargo fmt --all --check (pre-push hook).

🤖 Generated with Claude Code

…n+Go contract rows

Closes #1549 audit Gap 1 (Phase-2 blocker for full
`{python,go}_method_template_contracts` parity). Receipt against
ROADMAP.md:512 / docs/debt/r3-debt-paydown-ledger-2026-05-02.md:85
("Method-template consumer migration").

Substrate change in `dsl/std/methods.dag`: adds
`data string_contains_method: MethodDeclaration = { name: "string_contains" }`
and broadens the registry's documented scope to include target-only
emit-shortcut method names (the legacy emit map disambiguates the
substring shape from collection-membership `contains` by name —
v3's flat-name registry preserves that disambiguation by promoting
`string_contains` to a first-class `MethodRef` row key).

Per-target row population:
- `src/v3/std/python_method_template_contracts.dag` adds
  `string_contains_method` to imports and a row carrying Python's
  `{arg} in {recv}` template (matches `dsl/extdeps/languages/python/emit.dag:95`).
- `src/v3/std/go_method_template_contracts.dag` adds the same import and
  a row carrying Go's `strings.Contains({recv}, {arg})` template
  (matches `dsl/extdeps/languages/go/emit.dag:87`).
- Both files retire their "deferred to Phase 2 / Modeling" headers for
  this row; `chars` (Go-only, escaped-empty-string tokenizer issue)
  remains skipped per #1549 Gap 4 and is out of scope here.

Snapshot/manifest refreshes mechanically follow:
- `bootstrap_{generated,generated_without_parse_surface,std_generated}.rs`
  via `cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap`.
- `parse_corpus_manifest.txt` via
  `cargo test -p v3-compiler --test integration refresh_handwritten_parse_snapshot_manifest -- --ignored`.

Out of scope (per dispatch): Phase-2 deletion of legacy
`*_method_templates`, `LanguageSpec` rewrite, PB projection work, Go
`chars` row.

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

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 918f475e · Trigger: schedule
  • Comparison: origin/main @ 94c479cf ... review/pr-1561-918f475e @ 918f475e
  • Thinking: 44s wall

Findings: None. The substantive change is registry plus contract data: string_contains_method in dsl/std/methods.dag with comments that explain target-only vs algebra contains and point at the locked inbox work; matching MethodTemplateContract rows in python_method_template_contracts.dag and go_method_template_contracts.dag using dag_method: { decl: string_contains_method }; parse_corpus_manifest.txt updated for the touched .dag files; bootstrap Rust regenerated. That aligns with P2 (single MethodRef authority instead of a bare-name gap), P1/P5 (closes the previously deferred classification with an explicit rationale and named receipts), and does not introduce fail-open paths, new enums without classification, or test-style issues in the diff itself.

Verdict: APPROVE — Small, coherent closure of the documented string_contains registry/row gap; nothing in the reviewed diff contradicts INVARIANTS, modeling discipline, CODING, or TESTING in a way that warrants a change request.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review pass on #1561: the implementation matches the Gap 1 dispatch.

Checked the diff against current main: dsl/std/methods.dag adds string_contains_method as a target-only emit-shortcut MethodDeclaration and documents why it is distinct from algebra contains; Python and Go contract files import that key and add rows matching the legacy emit templates ({arg} in {recv} and strings.Contains({recv}, {arg})). The previous deferral comments are retired, and the PR keeps Phase-2 deletion / LanguageSpec rewrite / PB projection / Go chars out of scope.

Generated churn looks proportionate: bootstrap_std_generated.rs, full bootstrap snapshots, and parse_corpus_manifest.txt move with the new std method declaration and row changes. I did not find a code blocker in the changed hunks.

I did not rerun the full local validation here; the PR body already reports full v3, targeted method registry/template/substrate tests, clippy, fmt, bootstrap regen, and manifest verification.

— sent from jolly-ram-908 (inbox #1130); reply at #1130

…-disp2

# Conflicts:
#	src/v3/compiler/src/bootstrap_generated.rs
#	src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: 71061a1b · Trigger: schedule
  • Comparison: origin/main @ 0427f96f ... review/pr-1561-71061a1b @ 71061a1b
  • Thinking: 14s wall

APPROVE — Diff is narrowly scoped and clean: adds string_contains_method to the registry (with comment justifying target-only emit-shortcut classification distinct from algebra contains), adds matching rows in Python and Go contract lists referencing the MethodRef, and updates the parse-corpus manifest hashes for the two changed .dag files. The change resolves a previously-flagged "open classification" comment and replaces the bare-string TODO with a real MethodDeclaration authority — directly aligned with single-authority / MethodRef-over-string discipline. No invariant violations observed.

…-disp2

# Conflicts:
#	src/v3/compiler/src/bootstrap_generated.rs
#	src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs

@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: 71061a1b · Trigger: schedule
  • Thinking: 399s wall

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

  • dsl/std/methods.dag Because dsl/std/algebra.dag already has FreeMonoid scalar contains, the new target-only string_contains entry should explicitly say it is a legacy emit alias and name its dissolution trigger under #1549 Gap 1/R3 if deferred.

✅ No blocking concerns; the Python and Go contract rows preserve MethodRef authority and the target template shapes look consistent.

Per #1561 review feedback (codex): the previous comment risked reading
as if algebra `contains` was collection-only. `dsl/std/algebra.dag`
already declares `contains` once on `free_monoid_scalar_templates`
(String substring) and once on collection profiles (element/key
membership) under the same method name; the algebra-profile coordinate
carries the disambiguation. The `string_contains` registry entry is
specifically a *legacy v2 emit-map alias* that exists only because v2's
emit pipeline keys by bare method-name string and disambiguates by
minting a separate name. Naming the dissolution trigger explicitly
(v2 emit migrating to algebra-profile-keyed dispatch — #1549 audit
Recommendations §3-§4 / R3 Method-template-consumer-migration debt
row) closes the loop.

Comment-only edit; bootstrap snapshots regenerated mechanically because
include_str! captures source bytes.

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

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: b343c797 · Trigger: schedule
  • Comparison: origin/main @ 7cd1cecd ... review/pr-1561-b343c797 @ b343c797
  • Thinking: 59s wall

Findings: None. The substantive diff adds a documented legacy-alias MethodDeclaration in dsl/std/methods.dag with an explicit dissolution trigger (#1549 / algebra-profile-keyed dispatch), wires Python and Go MethodTemplateContract rows to string_contains_method (single registry authority, not a second template source), and refreshes parse_corpus_manifest.txt for the edited contract DAGs. That matches P2 / single-authority and the “tracked scaffold” expectations in INVARIANTS.md and docs/modeling-discipline.md rather than violating them. Large bootstrap_*_generated*.rs churn is mechanical fallout from the registry change, not a new Rust design surface; nothing in the diff suggests new fail-open paths or duplicate competing authorities for the same render fact.

Verdict: APPROVE — Narrowly scoped, consistent with the rubric, and the legacy-alias story is explicitly bounded and dissolvable.

Exploratory observations (optional): src/v3/std/methods.dag (not in this diff) still describes the dsl registry as “the 63 per-method data bindings”; after this PR the dsl file adds at least one extra binding beyond the algebra union, so that companion comment may deserve a tiny follow-up alignment later if you care about comment-level precision.

Per #1561 reviewer's optional follow-up: the companion typed-MethodRef
file referenced `dsl/std/methods.dag` as carrying "the 63 per-method
data bindings", which silently went stale once the dsl file gained the
`string_contains_method` legacy emit alias. Acknowledge the alias
convention and point at the dsl file's dissolution trigger so the two
headers stay in sync.

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

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 72bd2880 · Trigger: schedule
  • Comparison: origin/main @ 98caedb8 ... review/pr-1561-72bd2880 @ 72bd2880
  • Thinking: 33s wall

Findings

  • dsl/std/methods.dag:39–40 (comment prose) — Minor CODING.md / readability nit only: the header comment contains literal backslash-escaped quotes (\"Method-template / migration\") where normal quotation marks (or no nested quotes) would read cleanly. Not an invariant violation; optional cleanup.

Verdict

APPROVE — The diff wires string_contains as an explicit legacy alias in dsl/std/methods.dag, documents why it exists separately from algebra contains, and states a named dissolution trigger tied to #1549 / profile-keyed dispatch, which matches P5 tracked-bridge expectations in INVARIANTS.md. Python and Go MethodTemplateContract rows point at string_contains_method with templates aligned to existing v2 emit maps; src/v3/std/methods.dag companion comment stays consistent. Bootstrap/regenerated Rust churn follows from the new declaration. Nothing in the diff suggests P2 parallel semantic authority: the text is explicit that algebra does not split and the alias is for bare-string / MethodRef keying.

Exploratory observations (optional)

  • The large bootstrap_generated*.rs diffs are mechanical IDshift from one new MethodDeclaration; expected for this kind of change in this repo.

briansrls and others added 2 commits May 3, 2026 04:25
…-disp2

# Conflicts:
#	src/v3/compiler/src/bootstrap_generated.rs
#	src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
Per #1561 reviewer nit. Cosmetic prose cleanup; bootstrap snapshots
regenerated mechanically.

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

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 0acc4528 · Trigger: schedule
  • Comparison: origin/main @ 98caedb8 ... review/pr-1561-0acc4528 @ 0acc4528
  • Thinking: 24s wall

Reviewed the rubric excerpts and the full substantive diff (dsl/std/methods.dag, src/v3/std/*.dag, manifest). The remainder of the change is regenerated bootstrap (bootstrap_generated*.rs, bootstrap_std_generated.rs) driven by the DAG edits—mechanical ID/layout churn, not hand-authored logic.

Findings: None. The legacy string_contains entry is documented with an explicit dissolution trigger and scope (#1549 / v2 emit migration), satisfying the “tracked scaffold” pattern from the modeling discipline lens. Contract rows wire string_contains_method to the intended Python and Go templates and update imports consistently. parse_corpus_manifest.txt checksum updates match the touched .dag files per integration-test expectations (TESTING.md: corpus pins).

Verdict: APPROVE — Narrowly scoped registry + contract alignment; no concrete violations of INVARIANTS.md, docs/modeling-discipline.md, CODING.md, or TESTING.md visible in the authored diff.

@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: 0acc4528 · Trigger: schedule
  • Thinking: 245s wall

Non-blocking — Strengths

  • dsl/std/methods.dag The alias classification is tracked bridge debt: documented, bounded to v2 bare-string emit dispatch, and tied to algebra-profile-keyed dispatch as the dissolution path.
  • src/v3/std/python_method_template_contracts.dag The new row preserves MethodRef authority and matches the legacy Python substring render shape.
  • src/v3/std/go_method_template_contracts.dag The new row preserves MethodRef authority and matches the legacy Go strings.Contains render shape.

✅ No blocking concerns; the prior classification gap is resolved cleanly.

…-disp2

# Conflicts:
#	src/v3/compiler/src/bootstrap_generated.rs
#	src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: 3cf8cc59 · Trigger: schedule
  • Comparison: origin/main @ 5ef6530c ... review/pr-1561-3cf8cc59 @ 3cf8cc59
  • Thinking: 48s wall

Verdict: APPROVE

The diff is narrowly scoped and the string_contains legacy alias is documented as bounded transitional debt with a clear dissolution trigger. Python/Go rows point at the new MethodRef entry and match the existing legacy emit templates. No concrete violations of the pinned modeling, coding, or testing guidance observed. Tests not run per instruction.

…-disp2

# Conflicts:
#	src/v3/compiler/src/bootstrap_generated.rs
#	src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: f7284f55 · Trigger: schedule
  • Comparison: origin/main @ d4056ec8 ... review/pr-1561-f7284f55 @ f7284f55
  • Thinking: 11s wall

APPROVE — narrow, well-documented change. Adds string_contains_method to the registry with a clear header explaining the legacy-alias convention and a named dissolution trigger (v2 emit migrating to algebra-profile-keyed dispatch, #1549 R3), then wires Python and Go contract rows referencing it. Resolves the previously-deferred Phase 1 gap; comments in the contract files were updated in lockstep so the rationale stays consistent across the three sources. Generated bootstrap files and the parse-corpus manifest hashes regenerate as expected.

No findings.

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

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

Non-blocking — Strengths

  • dsl/std/methods.dag The string_contains alias is documented, bounded to legacy v2 bare-string emit dispatch, and has a named dissolution trigger into algebra-profile-keyed contains dispatch.
  • src/v3/std/go_method_template_contracts.dag The Go row preserves MethodRef authority and matches the legacy strings.Contains({recv}, {arg}) render shape for substring contains.
  • src/v3/std/python_method_template_contracts.dag The Python row preserves MethodRef authority and matches the legacy {arg} in {recv} render shape for substring contains.

✅ No blocking concerns.

briansrls added a commit that referenced this pull request May 3, 2026
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>
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
briansrls deleted the session/tidy-tern-769-disp2 branch June 1, 2026 18:43
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