Skip to content

R3 gate #1: tier3 termination mirror dissolved - #2677

Closed
briansrls wants to merge 5 commits into
mainfrom
session/royal-tern-883
Closed

briansrls wants to merge 5 commits into
mainfrom
session/royal-tern-883

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session royal-tern-883.
Pushing to session/royal-tern-883 advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

  • TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

@briansrls
briansrls marked this pull request as ready for review May 11, 2026 04:45
@briansrls

Copy link
Copy Markdown
Contributor Author

Verified the dashboard-only review against current HEAD c95b54e. The six termination lattice helper bodies are absent from src/v3/compiler/src/dag.rs, and the remaining declarations resolve through src/v3/std/termination.dag. The noted include_str!/pub fn tripwire limitation was explicitly non-blocking; I also checked for pub(...) fn reintroductions locally and found none, so there is no code change to make for this review. — sent from royal-tern-883

@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: 101a1d02 · Trigger: schedule
  • Thinking: 210s wall

BLOCKING (1)

Root Cause

  • docs/briefs/r3-pb-tier3-perf-budget-worker.md Phase-1 baseline capture is specified as a prerequisite but no baseline exists in the base tree or PR diff → land the frozen termination baseline first or keep the termination bench until capture completes.

Non-blocking — Strengths

  • src/v3/compiler/src/dag.rs The lattice operation mirror deletion correctly moves operation authority back to std.termination and reduces duplicate hand-Rust state under P5.

ROADMAP — Incomplete

  • Tier-3 perf-budget Phase-1 hand-Rust mirror benches: The termination bench retirement is in the right dissolution lane, but the required frozen baseline data is not present before the bench disappears.

⚠️ One ordering issue blocks this: capture or commit the termination perf baseline before deleting the only hand-Rust bench that can produce it.

@briansrls

Copy link
Copy Markdown
Contributor Author

Violations (could not place on specific lines):

  • src/v3/compiler/benches/tier3_fixtures/README.md:5 BLOCKING: Retiring the termination Phase 1 bench before a committed tier3_baseline.json row violates the R3 perf-budget ordering in docs/briefs/r3-pb-tier3-perf-budget-worker.md and makes the hand-Rust termination baseline unrecoverable after the mirror is deleted.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-thinking
  • Commit: 0406d174 · Trigger: manual
  • Comparison: main @ 5065653a ... session/royal-tern-883 @ 0406d174
  • Conversation: View conversation

1. Story of the diff

This PR removes the remaining hand-Rust executable mirror for the termination lattice from dag.rs and narrows dag.rs back to carrier-only responsibility. The load-bearing change is the comment shift at src/v3/compiler/src/dag.rs:890-893: .dag remains the carrier and lattice-operation authority, while Rust keeps only carrier shapes needed by current consumers, and executable operations like merge_evidence / join_evidence must stay in std.termination. The supporting changes retire the corresponding Criterion benchmark from tier3_mirror_perf.rs and update the tier-3 fixture README to say the termination bench retired with the mirror (src/v3/compiler/benches/tier3_fixtures/README.md:5-6). The test previously asserting Rust mirror behavior is converted into a dissolution guard: it checks the Rust functions do not reappear in dag.rs and that the named declarations still resolve to src/v3/std/termination.dag (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:495-527).

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Compliant — this touches substrate-adjacent Rust carriers, but the decision is to reduce implementation authority: src/v3/compiler/src/dag.rs:890-893 explicitly assigns carrier/lattice authority to .dag and keeps Rust to carrier shapes only.

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

Compliant — single authority / progress is dissolution / coprod mirror dissolution are honored by deleting the executable Rust mirror and adding a reintroduction guard. The test forbids the Rust mirror functions from reappearing at src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:498-508, then checks those operation names resolve to std.termination at src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:512-526.

  1. CODING.md.

Compliant — the diff removes free-function implementation code from the broad dag.rs substrate module instead of adding methods, hidden state, or a new helper object. The remaining module comment at src/v3/compiler/src/dag.rs:891-893 defines a narrow interface boundary: Rust carrier shapes only; executable lattice operations stay in the .dag authority.

  1. TESTING.md.

Compliant — the test is behaviorally aligned with the change’s purpose: the contract is no longer “Rust mirror returns the same lattice values,” but “Rust mirror is dissolved and std.termination remains the source.” The first half asserts non-reintroduction via include_str! at src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:496-508; the second half asserts source authority via Dag::new() and declaration spans at src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:512-526.

  1. LOCKED DESIGN DECISIONS.

Compliant — this does not alter a locked substrate shape or introduce a new target, connective, behavior, or bootstrap exception. It moves toward the locked zero-floor / generated-authority direction by retiring a hand-Rust mirror rather than adding one, as reflected by src/v3/compiler/src/dag.rs:890-893.

  1. TRACKED vs UNTRACKED DEBT.

Compliant — no new scaffold, TODO, compatibility bridge, or temporary representation is introduced. The prior scaffold language is replaced with a steady-state boundary statement at src/v3/compiler/src/dag.rs:888-893, and the test names the dissolution guard directly at src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:495-508.

2.5. Top-down PM intent review

Compliant — the highest-level intent here is dissolution of a hand-Rust termination mirror, not preservation of its benchmark or duplicate executable behavior. The diff preserves that intent directly: dag.rs says executable lattice operations must stay in std.termination (src/v3/compiler/src/dag.rs:890-893), the bench no longer imports or runs merge_evidence (src/v3/compiler/benches/tier3_mirror_perf.rs:15-16), and the test asserts the removed functions must not reappear in dag.rs (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:498-508). I do not see semantic dilution from “dissolve the mirror” into permanent scaffolding.

3. Verdict

APPROVE — the PR removes duplicate Rust execution authority for termination lattice operations, updates the perf fixture story accordingly, and adds a focused regression guard against reintroducing the mirror. I found no diff-cited invariant violation.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head 8aad1c7: this ordering finding is valid. There is no committed src/v3/compiler/benches/tier3_baseline.json, and this PR removes the only live termination Phase-1 bench row. I cannot safely fix this inside the dissolution PR by committing a placeholder/local baseline because docs/audit/c1-tier3-baseline-capture-procedure.md §6 explicitly forbids non-canonical or placeholder tier3_baseline.json data. I also cannot keep the termination bench without reintroducing the hand-Rust termination lattice mirror that this gate is assigned to dissolve. This is the documented C1/R3 sequencing STOP condition, so I escalated to the PB manager for a sequencing/waiver decision rather than papering over it in code. — sent from royal-tern-883

@briansrls

Copy link
Copy Markdown
Contributor Author

Closing this PR as blocked by the C1 Phase-1 baseline prerequisite. PB Manager confirmed R-7 should be a hard prerequisite: src/v3/compiler/benches/tier3_baseline.json must land on main before any T-Tier3 mirror dissolution PR deletes the corresponding Phase-1 hand-Rust bench. Keeping this PR open as mergeable would permanently lose the termination baseline capture path, while committing placeholder/local baseline data is forbidden by docs/audit/c1-tier3-baseline-capture-procedure.md §6. This work should be respawned after C1 Phase-1 baseline capture lands. — sent from royal-tern-883

@briansrls

Copy link
Copy Markdown
Contributor Author

Closed as blocked on C1 Phase-1 baseline capture prerequisite. See prior comment for sequencing details.

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