Skip to content

feat(compiler): close catalog-backed lowering - #78

Merged
Teakowa merged 3 commits into
mainfrom
codex/issue-42-catalog-lowering
Aug 28, 2026
Merged

feat(compiler): close catalog-backed lowering#78
Teakowa merged 3 commits into
mainfrom
codex/issue-42-catalog-lowering

Conversation

@Teakowa

@Teakowa Teakowa commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • lower canonical builtin actions/values through manifest catalogId links
  • lower receiver member actions/values, append, and bare member access into canonical WIR
  • validate enum/domain members through the workshop-rs catalog
  • preserve explicit diagnostics for catalog gaps and unknown enum members
  • add issue Close catalog-backed OPY lowering and corpus evidence #42 integration evidence and update the support matrix with bounded coverage

Verification

  • 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

Fixes #42

Implement canonical builtin, receiver, enum, and contextual lowering for the evidenced OPY slice, with explicit catalog-gap diagnostics and issue #42 integration coverage.

Fixes #42

@Teakowa Teakowa 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.

Blocking review — one-pass audit against #42 / #36. The implementation direction is sound, CI is green, and the patch preserves the ownership boundary, but this PR cannot close #42 yet.

Blockers

1. The chase evidence currently locks an unverified Workshop operand form instead of matching the pinned semantic evidence

catalog_backed_contextual_chase_calls_match_the_pinned_oracle asserts:

Chase Global Variable Over Time(Global.Round_Attack_Time, 0, 30, None);

but the pinned OverPy oracle for the same chase-condition-agentlab fixture emits:

Chase Global Variable Over Time(Round_Attack_Time, 0, 30, None);

This is not merely an output-format preference: the first parameter is the global-variable selector, while Global.Round_Attack_Time is the normal value-form spelling of a global-variable read. The current workshop-rs emitter special-cases player chase operands but passes the global operand through ordinary value emission, and its own round-trip tests currently pin that Global.g form. Structural WIR validation and canonical-id validation therefore do not prove the emitted operand role is legal/correct.

Because #42 requires no unexpected semantic divergence, do not encode the consumer-side difference as success. Resolve this at the owning boundary: verify/fix the chase variable-operand emission contract in workshop-rs, release a consumable contract, then update opy-rs and make this fixture use the same semantic oracle-equivalence path as the receiver fixture. If reproducible Workshop runtime evidence proves both spellings are legal and observably equivalent, record that evidence explicitly instead; the current repo evidence does not establish it.

2. The explicit real-world corpus acceptance criterion is not exercised by the native compiler

#42 (and parent #36) requires representative real-world corpus cases to reach deterministic Workshop emission with no unexpected divergence in the supported slice. The new opy-compiler tests only load compatibility/fixtures/synthetic/*. receiver-calls and chase-condition-agentlab have useful provenance links to real discoveries/projects, but both are intentionally classified as synthetic-original fixtures.

The Python compatibility tests in the PR verification validate fixture/oracle metadata and the diff machinery; they do not run real-world/* through opy-compiler. Therefore the green CI does not supply the real-world native-compilation evidence required to auto-close #42.

Add durable executable evidence for at least one representative provenance-linked real-world case/slice through opy-compiler (deterministic emission plus the appropriate semantic/oracle comparison). If a whole imported project is blocked by unrelated non-goal features, a minimized regression with project/revision/path provenance is appropriate; do not silently substitute a synthetic-only gate for the issue AC.

Major

3. The support-matrix note assigns OPY receiver/member surface ownership to the wrong repo

The updated semantics/receiver-members note says that the “full receiver member lists” remain Workshop-owned. That conflicts with the manifest's existing ownership contract: member functions, aliases, receiver categories, signatures, and source names are OPY source-language API metadata owned by opy-rs; workshop-rs owns the canonical Workshop target identities/catalog, content domains, localization, validation, and emission.

Please reword the matrix note so future work is not routed to the wrong owner. Keeping Workshop catalog/member target semantics lowering-dependent is correct; the OPY member list itself is not Workshop-owned.

Minor

No additional merge-blocking minor findings. Once the blockers are addressed, chase-enums would also benefit from reusing assert_matches_oracle(...) instead of several substring assertions, since the helper and pinned oracle already exist and that avoids accidentally proving only selected lines.

Verified good

  • #41 / #52 prerequisites are complete.
  • HIR remains Workshop-independent; catalog binding stays in opy-compiler.
  • Manifest catalogId links are consumed without copying canonical Workshop tables.
  • Receiver/catalog gaps and unknown enum members remain explicit source-attributed failures.
  • memberAccess is a documented canonical WIR helper in workshop-rs, so that path is not an ownership violation.
  • compile_hir runs both Program::validate and canonical-id validation before emission.
  • Support-matrix states remain conservatively lowering-dependent rather than claiming full support.
  • The PR is narrowly scoped to the compiler, focused evidence, and synchronized compatibility metadata.
  • Current CI for d18fc2bd3805e2061f18039d1ae640a21ec63f8c is green.

@Teakowa Teakowa 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.

Correction to my previous blocking review: retract the chaseOverTime global-variable operand finding. The distinction I conflated was OPY source syntax vs emitted Workshop syntax. In OPY, chaseOverTime(Round_Attack_Time, ...) correctly refers to the global variable without a Global. qualifier; lowering/emission may legitimately produce Chase Global Variable Over Time(Global.Round_Attack_Time, ...) in Workshop text. That difference is not by itself a semantic divergence and should not block this PR.

The remaining blocking concern is the #42 acceptance evidence: the new native compiler tests exercise synthetic fixtures only, while #42/#36 explicitly require representative real-world corpus cases to reach deterministic Workshop emission. The support-matrix ownership wording concern also remains.

Exercise the provenance-recorded overpy-cake project through native OPY HIR lowering, canonical WIR validation, and deterministic Workshop emission. Clarify OPY versus Workshop ownership in the support matrix.

Fixes #42
@Teakowa

Teakowa commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Review findings addressed in c2877ae.

  1. Real-world acceptance evidence: compatibility/fixtures/real-world/overpy-cake, sourced from Zezombye/overpy examples/cake.opy at immutable revision eea67adbcf6926c4004e35e25ab4be072624a44e; the fixture records the source URL, revision, path, and GPL-3.0 provenance.
  2. Native path: the test reads that project source, runs OPY preprocessing/parsing/semantic HIR, invokes opy-compiler::Compiler::compile_hir, and therefore runs structural WIR plus canonical catalog-id validation before deterministic Workshop emission. It compiles twice and checks identical emission, then checks the emitted WIR contains the exercised catalog identities (createBeamEffect, playEffect, randomReal, and randomValueInArray).
  3. Remaining explicit gaps: full Workshop catalog/member/domain breadth, settings/locales/hooks, and other unsupported OPY constructs remain lowering-dependent; catalog-gap members and unknown enum members remain source-attributed diagnostics. The pinned cake Workshop text has an existing ambiguous Visible To parser limitation, so this real-world gate does not claim parser text round-trip equivalence.
  4. Ownership wording now states that OPY receiver/member source metadata belongs to opy-rs, while workshop-rs owns canonical Workshop target identities/semantics, content domains, localization, validation, and emission.

Verification on the PR branch:

  • 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
  • git diff --check

@Teakowa Teakowa 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.

Follow-up one-pass audit at c2877aed. The two substantive findings from the previous review are now addressed: the PR executes the actual provenance-recorded real-world/overpy-cake source through OPY HIR → opy-compiler → validated canonical WIR → deterministic Workshop emission, and the support-matrix receiver/member ownership wording now correctly keeps the OPY source surface in opy-rs while Workshop target semantics remain workshop-rs-owned.

Blocker

Current CI is red because the unknown-enum negative test no longer reaches the integration boundary

Both Rust quality jobs fail in unknown_catalog_enum_member_is_a_source_attributed_gap. On the current PR merge result, opy_rs::compile(...) rejects ChaseTimeReeval.NOPE first with the source-layer unknown-enum-member diagnostic, so the test panics at expect("frontend resolves") before opy-compiler is exercised.

Do not weaken the OPY semantic diagnostic to make this test pass. The source layer is correctly rejecting an OPY-known invalid enum member. If #42 still needs an integration-boundary negative test for a stale/unknown Workshop catalog member, construct or otherwise reach a valid HIR identity that the OPY layer accepts but the Workshop catalog rejects; otherwise remove this unreachable duplicate test if the existing source diagnostic plus canonical-id validation already cover the contract.

Current CI run 33154371356: formatting and Clippy pass; both stable and 1.85 Rust jobs fail only in the test step on this case. real_world_cake_exercises_catalog_lowering_end_to_end itself passes.

Major / Minor

No additional Major or Minor findings from the updated diff. The real-world test deliberately avoids whole-output oracle reparsing because the pinned cake output contains the known ambiguous Visible To spelling; for #42's bounded catalog slice, deterministic emission plus canonical WIR/catalog validation and the existing focused oracle-backed synthetic cases are sufficient here without imposing output-text identity.

Once the failing negative test is corrected and the new head is green, I do not see another objective blocker to closing #42 / #36 from this PR.

Keep the source-layer unknown-enum-member diagnostic authoritative while retaining the reachable catalog-gap integration negative test.

Refs #42
@Teakowa

Teakowa commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Review 5049634081 follow-up is fixed in commit 7eb81ed.

  • Removed the unreachable unknown_catalog_enum_member_is_a_source_attributed_gap integration test. ChaseTimeReeval.NOPE is correctly rejected by the OPY frontend with unknown-enum-member, so the test could not validly exercise opy-compiler and was causing the PR check failure.
  • Kept the reachable eventPlayer.getHero() catalog-gap negative test at the compiler integration boundary.
  • No production code, diagnostics, catalog ownership, or Global.Round_Attack_Time emission behavior changed.
  • Existing acceptance evidence remains: provenance-recorded compatibility/fixtures/real-world/overpy-cake, native OPY parse/semantic HIR -> opy-compiler -> validated canonical WIR -> deterministic Workshop emission, with structural catalog-call assertions.

Local verification passed:

  • cargo test -p opy-compiler --test issue_42_oracle (6 passed)
  • python3 -m unittest discover -s compatibility/tests (21 passed)
  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo test --workspace --all-targets --all-features
  • git diff --check

The branch is pushed; fresh hosted checks are now running.

@Teakowa

Teakowa commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Fresh workflow is fully green on : Rust 1.85.0, stable, macOS JS runtime, and Windows JS runtime all passed. PR merge state is .

@Teakowa

Teakowa commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Correction: fresh workflow 33158708355 is fully green on commit 7eb81ed: Rust 1.85.0, stable, macOS JS runtime, and Windows JS runtime all passed. PR merge state is CLEAN.

@Teakowa Teakowa 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.

Follow-up one-pass audit on 7eb81ed8c4e95a4fd255cc164b57cb5ca66e613e: no remaining Blocker / Major / Minor findings.

The previous CI blocker is correctly resolved by removing the unreachable integration test rather than weakening the authoritative OPY unknown-enum-member diagnostic. The reachable catalog-gap integration negative test remains.

Revalidated against #42 / #36:

  • catalog-backed builtin/member/enum lowering remains bounded and canonical through workshop-rs;
  • structural WIR and canonical-id validation remain in the native compiler path;
  • real-world/overpy-cake now provides provenance-recorded native end-to-end evidence with deterministic Workshop emission and representative catalog-backed action/value calls;
  • support-matrix ownership wording correctly keeps the OPY receiver/member source surface in opy-rs and Workshop target semantics/catalog validation in workshop-rs;
  • unsupported/gap states remain explicit rather than being counted as support;
  • the latest CI run is green on stable and Rust 1.85.0, with the JS runtime matrix also green.

From review evidence, PR #78 can satisfy and close #42 when merged. With #42 merged/closed, #36's current active edge is also objectively cleared; #36 should be rechecked against the merged main state before closing.

@Teakowa
Teakowa merged commit 1a434e4 into main Aug 28, 2026
4 checks passed
@Teakowa
Teakowa deleted the codex/issue-42-catalog-lowering branch August 28, 2026 09:55
@e54-bot e54-bot mentioned this pull request Aug 28, 2026
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.

Close catalog-backed OPY lowering and corpus evidence

1 participant