Skip to content

fix(compiler): apply OverPy's per-parameter False/True substitutions under optimizeForSize - #398

Merged
Teakowa merged 2 commits into
mainfrom
feat/388-false-true-substitutions
Sep 26, 2026
Merged

Teakowa merged 2 commits into
mainfrom
feat/388-false-true-substitutions

Conversation

@e54-bot

@e54-bot e54-bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #388

Summary

  • Adopt workshop-rs 0.8.0, which stops normalizing contextual literals (fix(parser)!: preserve contextual literal substitutions workshop-rs#300).
  • Under #!optimizeForSize, spell 0/1/Null/zero vectors as OverPy 9.7.10 does per parameter, from a table generated by tools/overpy/gen_literal_flags.cjs (OverPy's canReplace0ByFalse, canReplace1ByTrue, canReplace0ByNull, canReplaceNullVectorByNull). Policy stays in opy-rs and no longer reads catalog paramCoercions.
  • Remove the unconditional lowering normalization (0→Null, False→0, empty array→"") that only converged because the old parser did the same. Unoptimized output keeps the authored spelling, as the pinned reference does (checked by compiling probes with pinned OverPy).
  • Update tests that asserted the old normalized text to the oracle's output.

Verification

  • cargo test --workspace, cargo clippy --workspace --all-targets, cargo fmt --check, tools/overpy/tests: pass.
  • tools/overpy/probe_builtins.py: 4064 probes, unexplained: 0 (two approved gaps unchanged).
  • tools/overpy/structural_gate.py: Bastion main.opy and externalMain.opy report 0 differences against pinned OverPy.
  • Wright canonical diff (issue Apply OverPy's per-parameter False/True substitutions under optimizeForSize #388 AC): a clean Wright main worktree with only workshop-rs bumped to =0.8.0 (its main pins 0.7.1, which predates fix(parser)!: preserve contextual literal substitutions workshop-rs#300), compiling Bastion (f693704) through this branch's opy-provider, compared with workshop_rs::roundtrip::equivalent against pinned OverPy 9.7.10 output. main.opy (zh-CN): equivalent. externalMain.opy (en-US): equivalent, also via wright semantic-compare. wright semantic-compare only parses en-US, so the zh-CN comparison used workshop-rs directly with the zh-CN locale.

…under optimizeForSize

Replace the hand-written literal special cases with a table of OverPy
9.7.10's canReplace0ByFalse, canReplace1ByTrue, canReplace0ByNull and
canReplaceNullVectorByNull flags, generated by
tools/overpy/gen_literal_flags.cjs and applied to every call argument,
array element and vector component. Replacement policy stays in opy-rs
and no longer reads the catalog's broader acceptance coercions.

Stop normalizing authored literals while lowering (0 to Null, False to
0, empty array to empty string), which only converged because the
workshop-rs parser did the same. Unoptimized output now keeps the
authored spelling, as the pinned reference does.

Adopt workshop-rs 0.8.0, which preserves contextual literals.

Fixes #388

@Teakowa Teakowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three blocking findings:

  1. crates/opy-rs/src/compiler/size_optimization/literal_flags.rs / tools/overpy/gen_literal_flags.cjs: the generated upstream canReplace* table is interpreted directly by generic Rust to choose observable OverPy lowering. That conflicts with the current language-core.md / AGENTS.md contract: source-language behavior and coercion/lowering policy belong in discoverable typed Rust; generated/declarative data must not become a semantic control layer, and upstream implementation data should not be mechanically imported into the core. Encode this policy directly in opy-rs typed code and use the upstream table/probe for completeness verification, or obtain an explicit owner architecture decision changing that contract.

  2. Required verification is missing. AGENTS.md says lowering/optimization/builtin-argument changes must run tools/overpy/probe_builtins.py. The PR verification does not report it, and the current CI run does not execute that probe. Run the full builtin-call probe and resolve any unexplained differences.

  3. Issue #388 explicitly requires Wright's canonical diff for Bastion main.opy and externalMain.opy; the PR says it was not run. The opy-rs structural gate is green for those same pinned entries, but review should not silently substitute a different check for an explicit AC. Run the Wright check, or have the owner update the AC if the structural gate is now the intended replacement.

Address review: the generated canReplace table drove lowering policy
directly. Write the per-parameter policy as typed Rust in literal_slots.rs
and keep the upstream flags only as verification data
(tools/overpy/upstream-literal-flags.json), checked by
slots_cover_the_upstream_flags in both directions.

Verified with tools/overpy/probe_builtins.py (unexplained: 0) and the
Bastion structural gate (0 differences).

@Teakowa Teakowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings 1 and 2 are resolved. The generated upstream flags are now verification data only, while the policy is encoded in typed Rust, and the follow-up commit records the builtin probe with 0 unexplained differences.

One blocker remains: Issue #388 still explicitly requires Wright's canonical diff for Bastion main.opy and externalMain.opy, while the PR still says that check was not run. Run that acceptance check, or update the Issue contract if the opy-rs structural gate is now the intended replacement. No new code-level findings in the follow-up patch.

@e54-bot

e54-bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Thanks. All three findings are addressed in 5eb8b5c and the PR description.

  1. The policy is typed Rust in literal_slots.rs. The upstream flags are verification data only (tools/overpy/upstream-literal-flags.json), checked in both directions by slots_cover_the_upstream_flags.
  2. tools/overpy/probe_builtins.py: 4064 probes, unexplained: 0.
  3. Wright canonical diff, run on a clean Wright main with only workshop-rs bumped to =0.8.0 (its main pins 0.7.1, which predates fix(parser)!: preserve contextual literal substitutions workshop-rs#300), through this branch's opy-provider: Bastion main.opy (zh-CN) and externalMain.opy (en-US) are both equivalent to pinned OverPy 9.7.10. Details are in the PR description. The Wright workshop-rs bump is being opened as a separate PR.

Requesting re-review.

@e54-bot
e54-bot requested a review from Teakowa September 26, 2026 08:38

@Teakowa Teakowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The three prior review findings are resolved, the Wright acceptance check is now documented for both Bastion entries, and the current CI run is fully green.

@Teakowa
Teakowa merged commit 4bf6be7 into main Sep 26, 2026
5 checks passed
@Teakowa
Teakowa deleted the feat/388-false-true-substitutions branch September 26, 2026 08:41
@e54-bot e54-bot mentioned this pull request Sep 26, 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.

Apply OverPy's per-parameter False/True substitutions under optimizeForSize

2 participants