feat(opy-compiler): lower control flow into canonical WIR

[Teakowa]

Summary

• Rebaseline the opy compiler integration on the released workshop-rs v0.1.11 contract.
• Remove the local action-layout model and query canonical emitted widths through workshop_rs::emitter::action_width, with stable source-attributed layout diagnostics.
• Add a pinned OverPy 9.7.10 switch case whose earlier arm contains nested if/while structure and whose later case/default targets cross that structure.
• Migrate the ordered SwitchArm HIR wire grammar to explicit wright/opy-hir@2.0.0; v1 payloads are rejected at the envelope before body inspection.

Multiple switch breaks with later reachable actions remain an explicit unsupported-integration-surface diagnostic because the released canonical WIR still has no lossless multi-target switch carrier. No source arm or action is silently dropped.

The HIR major migration is producer/consumer contract-correct in opy-rs and documented in docs/hir/opy-hir-v2.md. Existing external consumers such as wright require their owning-repository migration; this PR does not modify that repository opportunistically.

Verification

• cargo metadata --locked --no-deps
• cargo fmt --all -- --check
• cargo clippy --workspace --all-targets --all-features -- -D warnings
• cargo test --workspace --all-targets --all-features
• python3 -m unittest discover -s compatibility/tests
• Frontend differential corpus, including the new structured-switch fixture.
• 8 issue Lower OPY control flow into canonical Workshop WIR with oracle evidence #47 compiler tests using native/oracle reparsing through workshop-rs and roundtrip::equivalent.

Closes #47

[Teakowa]

Review verdict: changes required

I reviewed the full #47 scope against the current PR head, the pinned OverPy 9.7.10 behavior, the existing HIR contract, the new fixtures, and the current workshop-rs WIR/emitter. CI is green, but the current fixture set misses several control-flow shapes where the lowering changes observable semantics. These are merge blockers for a PR that closes #47.

Blockers

1. No-break switch + default does not preserve fallthrough semantics, and the current HIR loses default source order.

   Pinned OverPy flattens every case/default in source order and only introduces Else through a source break. In upstream src/tests/switches.opy, the second switch has no break, places default before a later case Hero.BRIGITTE, and intentionally falls through into the later body. Its expected Workshop has no Else at the default boundary.

   The current HIR represents cases and default separately, so the relative position of default is already lost. lower_switch() then always appends the default as else_body when no case break occurred. That changes both unmatched-default entry and case-to-default fallthrough behavior. This cannot be treated as output-shape variance; it changes runtime control flow.

   Preserve enough OPY semantic ordering in the opy-rs-owned HIR (or otherwise establish a lossless OPY-owned contract) and lower the pinned no-break/default-before-case shapes with oracle evidence. Do not solve this by adding OPY-specific nodes to workshop-rs.

2. A second direct switch break silently drops later cases.

   lower_switch() stops iterating lowered cases after the second direct break (if breaks { break; }) with the assumption that later boundaries are unreachable. That assumption is false: the selector can jump directly to every case label. Pinned src/tests/switches.opy has an outer nested switch where case 1 breaks, case 2 breaks, and case 3 remains directly reachable and contains another switch; the pinned output retains case 3 after both Else boundaries.

   This is specifically prohibited by #47's no-silent-dropping acceptance criterion. Add a source-level oracle case with two breaks followed by a non-empty later case and preserve all directly reachable cases.

3. Switch jump offsets are computed from WIR container counts rather than emitted Workshop action distances.

   The current offset calculation uses body.len() + usize::from(breaks). A structured WIR action such as If, While, ForGlobalVariable, or a nested switch occupies one ActionId in that vector but emits multiple Workshop control-flow actions/markers. Pinned OverPy computes switch offsets with __distanceTo__ against actual labels after flattening; its nested switch fixtures demonstrate offsets that account for nested structure.

   As a result, a switch with a structured action in an earlier case can jump into the wrong place for a later case/default. Do not derive Workshop Skip distances from arena/vector length. Use a representation/expansion step that can compute the real canonical target distance, or reject shapes that cannot yet be represented safely. Add oracle-backed cases where an earlier case contains nested structured control flow and a later case/default is selected.

4. Do-while break distance has the same structural-count bug.

   lower_do_while_body() uses tail.len() + 1 for the generated skip/skipIf. Pinned OverPy places a label after the entire enclosing do-while and resolves break through __distanceTo__. If the tail contains a structured if/while/for/switch, tail.len() undercounts the flattened Workshop actions before the loop action/target label.

   The current positive fixture only has one simple assignment after the conditional break, so it cannot catch this. Add direct/conditional do-while break evidence with a structured tail and ensure the jump targets the post-loop location semantically, not by WIR node count.

5. do ... while placement rules from the pinned OPY language are not enforced, so #61 promotes invalid upstream source into a WrightKit-only accepted form.

   Pinned OverPy __doWhile__.ts requires do-while to be at the beginning of a rule/def/do-while context (preceding siblings may only be pass). The current parser accepts do as a generic statement, HIR lowering carries it anywhere, and #61 now compiles any Stmt::DoWhile by flattening body + Loop If.

   A source such as an ordinary assignment followed by do: ... while ... is rejected by the pinned language but would compile natively; worse, Loop If loops back across preceding actions, so this can also produce incorrect runtime behavior. Enforce this OPY-owned source-semantic restriction at the frontend/HIR boundary with a source-attributed diagnostic and a pinned negative probe.

Major findings

6. The claimed direct/nested do-while-break support needs executable native-oracle coverage. The pinned upstream loop corpus includes direct do-while break and nested do-while break, but the new positive fixture exercises only a single conditional break with a flat tail. Once distance lowering is fixed, add minimized direct and nested cases so the support-matrix claim is backed by executable evidence rather than implementation inference.

7. supported range-for should state the actual binder boundary. The current compiler accepts only a global-variable binder. Pinned OverPy also has for hostPlayer.I in range(3) and emits For Player Variable(...). It is acceptable to keep that outside this bounded #47 slice if that is the intended contract, but the machine/human support claims should say "global-binder range-for" (or point to a follow-up) rather than leave supported range-for undefined.

Minor / metadata

8. compatibility/support-matrix.json still has the pre-#61 snapshot provenance (asOfCommit points to the #54-era commit and the snapshot note only describes the older integration slices). Refresh it when the corrected #47 evidence is final.

9. The PR body ends with Closes #47,; remove the trailing comma. Also update the human evidence-table wording that still labels the compiler structural tests as #40/#46 only once #47 tests are final.

Verified good

• if/elif/else and while use canonical structured WIR rather than OPY-shaped backend nodes;
• global-binder range(...) lowering uses canonical ForGlobalVariable and preserves binder provenance;
• direct loop break remains a canonical Workshop action;
• unsupported nested conditional switch-break fails with a stable source-attributed diagnostic instead of being guessed;
• the new positive path genuinely reparses native and pinned Workshop through workshop-rs and compares structural equivalence;
• no Workshop catalog/control-flow semantics were copied into opy-frontend;
• current CI run #116 is green on the PR head.

After the switch ordering/multi-break/distance semantics and do-while placement/distance issues are repaired with minimized pinned-oracle coverage, re-audit the #47 support row before merging.

[Teakowa]

Review verdict: changes still required

I re-audited the full #47 scope at 826cf605 after the previous review fixes. The original observable-semantic blockers are substantially improved: switch arm source order is preserved, later multi-break cases are no longer silently dropped (unsupported shapes fail explicitly), do-while placement is enforced at the OPY boundary, direct/conditional/nested do-while fixtures exist, support claims are narrowed to global-binder range-for, and CI #118 is green.

Two merge blockers remain.

Blocker 1 — the PR makes a breaking Opy HIR wire change while still emitting protocol 1.1.0

Stmt::Switch changed from the existing cases + default wire grammar to a required ordered arms grammar. That is the correct semantic model internally, but it is not an additive v1 change.

The normative Opy HIR v1 spec explicitly says that removing/renaming a node/field or changing requiredness is a major-version change, and that minor/patch evolution does not change node grammar. The producer still emits wright/opy-hir@1.1.0.

This means an existing v1.1 payload containing cases/default is now accepted at the envelope boundary and only later fails/degenerates against the new body grammar, while an existing v1.1 consumer cannot understand the new arms payload. That violates the protocol's own compatibility contract.

Fix this at the wire boundary. Either:

• make the ordered-arm evolution backward-compatible/additive for v1 while keeping one canonical internal representation, or
• perform an explicit major-version transition and update/verify every consumer.

Do not silently redefine the accepted v1.1 grammar in place.

Blocker 2 — opy-compiler now duplicates Workshop emitter layout semantics to compute Skip distances

normalized_action_width() recursively hard-codes how every canonical WIR action contributes emitted Workshop action lines (If branch headers/Else/End, While/For terminators, leaf action widths), then switch/do-while lowering derives Skip offsets from that local model.

This fixes the old Vec<ActionId>::len() bug for the current fixtures, but it creates a second implementation of Workshop emission/control-flow layout inside opy-rs. That is the wrong ownership boundary: WIR shape, canonical emission, and the emitted action distance required by native Skip semantics are Workshop-owned contracts.

The PR itself already establishes that workshop-rs v0.1.8 lacks a lossless target carrier for some switch shapes. If safe relative-target/width resolution is required for OPY lowering, route the generic capability to workshop-rs, publish the authoritative contract, then consume it here. Do not keep an opy-rs mirror of emitter internals as the durable solution.

Keeping evidence-insufficient multi-target switch forms as explicit unsupported-integration-surface is fine until that canonical capability exists; silently duplicating emitter semantics is not.

Major — switch structured-width evidence is still missing

The new do-while fixture correctly includes a structured if after a break, but the new issue-47-switch-order fixture contains only flat assignments. The previous review specifically required a supported switch where an earlier arm contains nested structured control flow and a later arm/default is selected, because that is the shape that broke the old offset logic.

Add a minimized oracle-backed switch case that actually exercises the new target/width contract across nested if/while/for (one representative structured form is sufficient once the authoritative backend contract owns the measurement). issue-33-switch-break does not prove this supported path because its relevant multi-break/nested shapes exercise different boundaries.

Major / follow-up bookkeeping — player-variable range binder is narrowed but not tracked

The support row now correctly says global-binder range-for, so this does not block #47. However the requested follow-up for pinned forms such as for hostPlayer.I in range(...) does not appear to exist yet. Deduplicate and create a focused opy-rs issue so the known bounded gap is not lost.

Verified good in this revision

• source-ordered SwitchArm::{Case, Default} preserves default-before-case fallthrough internally;
• duplicate defaults are validated;
• multi-break switches with later reachable actions fail explicitly instead of truncating cases;
• do-while placement now matches the pinned beginning-of-rule/def/do-while restriction with source attribution;
• direct, conditional, nested, and structured-tail do-while cases are oracle-backed;
• native/oracle Workshop is reparsed through workshop-rs and compared structurally;
• the support row explicitly documents the multi-break/nested conditional switch limitations and global-binder range-for boundary;
• CI run #118 is green on the current head.

Once the HIR wire-version contract and Workshop-owned target/width boundary are corrected, add the missing structured-switch oracle probe and re-run the full #47 evidence suite. At that point #61 should be close to merge-ready.

[Teakowa]

Follow-up tracking created during review:

• wrightkit/workshop-rs#102 — canonical Workshop-owned control-flow target/distance contract; this is the owning-repo prerequisite for removing the local emitter-width model.
• Support player-variable range-for binders #65 — player-variable range(...) binders; explicitly non-gating for Lower OPY control flow into canonical Workshop WIR with oracle evidence #47.

The remaining PR verdict is unchanged: the HIR wire-version break and the local Workshop emission-width duplication are merge blockers; #65 is follow-up only.

[Teakowa]

Review verdict: implementation blockers resolved; one evidence-metadata fix remains

I re-audited the full #47 scope at 9faafff, including the two commits since 826cf605, the released workshop-rs v0.1.11 contract, HIR wire migration, new structured-switch oracle fixture, support claims, and hosted CI.

Previous merge blockers — resolved

1. Workshop action-layout ownership is fixed. The workspace and compiler now pin released workshop-rs = 0.1.11; switch/do-while distance calculation calls workshop_rs::emitter::action_width(...). The local normalized_action_width() / emitter-expansion table is gone. The v0.1.11 tag contains the exact public API consumed here, so opy-rs no longer mirrors Workshop emitter layout semantics.

2. HIR wire compatibility is fixed correctly at a major boundary. Ordered switch arms now use explicit wright/opy-hir@2.0.0; the producer emits v2, the v2 parser checks the protocol major before body inspection, and v1 is explicitly rejected. docs/hir/opy-hir-v2.md records the breaking cases/default -> arms migration rather than silently redefining v1.1.0.

3. Structured switch-target evidence now catches the old width bug. issue-47-switch-structured-target puts nested if + while structure in an earlier arm and constrains later case/default targets. The pinned oracle offsets ([7, 0, 6]) depend on emitted action width rather than WIR node count, and the dedicated compiler test reparses native/oracle Workshop through workshop-rs and requires roundtrip::equivalent.

4. The earlier switch source-order, explicit multi-break rejection, do-while placement, direct/conditional/nested/structured-tail do-while evidence, global-binder range boundary, provenance, and unsupported-shape diagnostics remain intact.

5. Hosted CI #121 is green on this exact head across Rust stable, Rust 1.85.0, macOS JS runtime, and Windows JS runtime.

Major — fix before merge: corpus-count evidence is internally inconsistent

This PR starts from the 44-fixture / 30-synthetic baseline and adds seven #47 synthetic fixtures:

• control-flow
• unsupported
• switch-order
• switch-multiple-break
• do-while-shapes
• do-while-invalid-placement
• switch-structured-target

So the PR-head corpus is 51 total = 37 synthetic + 1 census + 13 real-world.

Current docs disagree:

• compatibility/README.md says 50 / 36 synthetic;
• compatibility/fixtures/README.md says 46 snapshots / 32 synthetic.

Those statements are part of the evidence/provenance surface and should not ship as false current counts. Reconcile them to the actual PR-head corpus, while keeping dated historical verification statements explicitly dated rather than implying a fresh oracle rerun that did not occur.

Minor

crates/opy-frontend/tests/differential.rs::run_native still says native HIR must satisfy the v1 invariants in an assertion message after the v2 migration. Change it to v2 or version-neutral wording.

Cross-repo note — non-gating for #61

wright main still has its own wright_core::hir major-v1 contract today. That does not make this owner-side v2 change invalid: the protocol major is explicit and Wright is not currently consuming this opy-rs crate directly. The consumer-side cutover is already represented by wright#227 / the broader ownership migration; when that proceeds it must consume the v2 contract rather than carrying the old v1 copy forward.

Verdict

No remaining #47 correctness, Workshop-ownership, HIR-versioning, provenance-attribution, or CI blocker was found. Fix the corpus-count metadata (and preferably the stale assertion wording), rerun the lightweight metadata/tests gate, and this PR is merge-ready.