fix(settings): apply hero settings keys per the pinned per-hero schema

[e54-bot]

Summary

Fixes #495.

The pinned OverPy compiler merges __generalAndEachHero__ plus each admissible __eachHero__ key into every hero's settings set; authored keys missing from the merged set are written back verbatim. opy-rs translated through a hero-agnostic table, so catalogued-but-inapplicable keys emitted canonical names and percent formatting (e.g. ana.ability3Cooldown% → 50%, reinhardt.ammoClipSize% → Ammunition Clip Size Scalar).

• Adds crates/opy-rs/src/compiler/data/hero_applicability.json, generated by tools/overpy/gen_hero_applicability.cjs from the pinned compiler's own post-load schema merge (OverPy 9.7.10; issue comments confirm identical in 9.7.17): 19 universal keys, 75 include-form, 8 exclude-form across 53 heroes.
• pass_through_members now gates workshop_rs::settings::definition on that set; inapplicable members take a new verbatim_form (Raw for scalars) so the authored key spelling survives — RawValue would still resolve the canonical name at a catalogued path.
• Converges the pinned compiler's adjacent rewrites: mccree→cassidy / hammond→wreckingBall in hero groups and roster lists, ability1KB%→ability1Kb% member keys, and enabledHeroes/disabledHeroes emitted after hero groups.

Differential (53 heroes × 102 authored keys vs oracle)

All applicability divergences resolved; both issue examples match. Remaining divergences are workshop-rs name-data gaps, recorded on #495 and tracked in wrightkit/workshop-rs#414:

1. enableAutomaticFire/enableScoping hard-error on ana/ashe/widowmaker (missing primaryFire slot mapping).
2. Generic Secondary Fire literal for ~14 heroes (blank export label).
3. Roadhog multi-slot secondary-fire naming (Pig Pen/Take a Breather/Secondary Fire).

Verification

• cargo test --workspace --all-targets --all-features — all green (incl. 4 new regression tests through compiled_lines)
• cargo fmt --all -- --check, cargo clippy --workspace --all-targets --all-features -- -D warnings, git diff --check — clean
• python3 -m unittest discover -s tools/overpy/tests — 40 passed
• Oracle differential per above

[e54-bot]

Independent review: verified the diff against the pinned oracle (vendored tools/overpy/oracle, overpy 9.7.10) with ~27 adversarial probe compiles diffed against opy-cli output.

Verified correct: the regenerated hero_applicability.json is byte-identical (node tools/overpy/gen_hero_applicability.cjs → no diff; 19/75/8 split confirmed); hero_setting_applies membership semantics match compileCustomGameSettingsDict's merged-set lookup including the enableGenericSecondaryFire→enableSecondaryFire dest-key rename; verbatim_form/written_form split is right (Raw bypasses name resolution, RawValue resolves canonically — needed at catalogued paths); canonicalization order (group rename → ability1KB% rename → gate on canonical name) is correct; roster lists emit last with canonicalized elements; main/lobby/gamemodes paths untouched by hero logic; unknown heroes still fail downstream; the 4 tests discriminate (they fail without the gate). fmt/clippy/tests all green.

Findings (none block the issue's stated scope, ranked):

1. Medium — general-flattened members get reprocessed as heroes. In pass_through_unknown_members, general's children are spliced into the team children before the second for hero in children loop, so a Group authored inside general is then canonicalized, ability1KB%-renamed, and hero-gated; a general-nested List named enabledHeroes gets its elements canonicalized. Probes:
   • {"general": {"mccree": {"health%": 50}}} → oracle emits verbatim mccree { health%: 50 }; opy-rs emits Cassidy { Health: 50 } (renamed group + translated label, verbatim value without %).
   • {"general": {"enabledHeroes": ["mccree"]}} → oracle enabledHeroes { mccree }; opy-rs enabled heroes { Cassidy }.
     Upstream compiles general's dict strictly against general.values — nested members write verbatim with authored names. Fix is cheap: run the hero pass only over rest/hero_lists, or record the general-member count and skip() it in the second loop.
2. Medium-low — ability1KB% rename keeps authored position; upstream moves it to the end. Upstream does d["ability1Kb%"] = d["ability1KB%"]; delete d["ability1KB%"] — JS insertion order places the renamed key last. Probe dva: {"health%":40, "ability1KB%":50, "ultGen%":60} → oracle Health / Ultimate Generation… / Boosters Knockback; opy-rs Health / Boosters Knockback / Ultimate Generation…. Also {"ability1Kb%":10, "ability1KB%":50} → oracle collapses to one Boosters …: 50% (overwrite); opy-rs emits both lines. The new tests only use single-member hero dicts so they can't see this.
3. Low — both enabledHeroes and disabledHeroes authored. Oracle hard-errors Cannot have both 'enabledHeroes' and 'disabledHeroes' in team 'allTeams'; opy-rs emits both in authored order. Pre-existing leniency, but the reorder makes the divergence visible — either reproduce the error or record the exception.
4. Low — inapplicable catalogued key with non-scalar value errors instead of passing through. verbatim_form falls back to written_form for List/Group, producing a Group at a catalogued path that member::accept then rejects: ana: {"ability3Cooldown%": [11,22]} → settings key 'ability3Cooldown%' does not match its table kind; oracle emits a verbatim ability3Cooldown% { 11 / 22 } block. Pathological input and consistent with pre-existing catalogued-kind strictness, but it holes the "fully verbatim" claim for non-scalars.
5. Low — non-Group general ({"general": 5}): opy-rs errors "outside the emission table"; oracle silently drops it. Arguably better, noting for completeness.

Notes: inapplicable keys warn as unknown settings key 'ability3Cooldown%' — technically catalogued-but-inapplicable; cosmetic. Serde ignores the audit fields (schemaVersion/pinned/derivation/generalKeys) — intentional, though deny_unknown_fields would catch generator drift. genji.enableSecondaryFire → enableSecondaryFire: Off vs oracle Secondary Fire: Off reproduces the recorded wk#414 generic-label gap, as documented.

Core fix is correct and matches the oracle on the issue's examples; findings 1–2 are real upstream divergences on accepted input worth fixing or tracking.

[e54-bot]

Re-review findings addressed in 39e7f96 — all verified against the pinned oracle before and after:

Fixed

• Medium — general children reprocessed: the flattened general members no longer take the hero-name or applicability passes; only authored hero groups and roster lists do. dva nested under general now keeps ability1KB%: 50 verbatim instead of renaming to Boosters Knockback Scalar. (Residual verbatim-emission gap noted below.)
• Medium-low — ability1KB% ordering: implemented upstream's assign+delete semantics — the renamed member moves to the end ({ability1KB%:20, health%:150} → Health then Boosters Knockback Scalar: 20%), or collapses onto an authored ability1Kb% position with the source value winning ({ability1Kb%:10, …, ability1KB%:20} → Boosters Knockback Scalar: 20% at the dest position). Both probe shapes match the oracle byte-for-byte.
• Low — both rosters in one team: now errors settings-hero-lists: Cannot have both 'enabledHeroes' and 'disabledHeroes' in team 'allTeams' — same text as the oracle.
• Low — non-dict general: dropped silently like upstream (was an emission error).

Routed to wrightkit/workshop-rs#417 (canonical emitter gap, needs a pass-through escape for non-scalar nodes at catalogued paths):

• Inapplicable catalogued key with a non-scalar value (ana.ability3Cooldown%: [11,22]): oracle writes the block verbatim; the written form is a Group of Raw children but member::accept kind-checks the catalogued scalar path.
• general-nested groups/roster lists: pass-through marks them correctly (unknown-setting warnings fire), but emission still resolves group names as heroes and list elements as ListHero — verbatim text needs the same emitter escape.

New tests cover the KB ordering/collapse, general scoping, non-dict general drop, and the both-rosters error. cargo test -p opy-rs green, fmt/clippy clean.

[e54-bot]

Follow-up verification of 39e7f96 against the pinned oracle (50 probes, whitespace-normalized content comparison; oracle emits settings\n{\n\t… so raw bytes always differ — semantics/order compared token-for-token).

Claimed fixes — verified

• Flattened general children: hero passes no longer reach them. general: {"dva": {"ability1KB%": 50}, "damageDealt%": 50} keeps ability1KB% verbatim (previously Boosters Knockback Scalar) and translates Damage Dealt: 50%. The residual dva→D.Va group-name resolution is emission-side — routed gap, below.
• ability1KB% assign+delete order: all five order-sensitive probes match the oracle — move-to-end ({ability1KB%, health%} → Health first, Boosters Knockback Scalar last) and collapse onto an authored ability1Kb% at the destination position with the source value, in both authored orders.
• Both-lists error: verbatim upstream message Cannot have both 'enabledHeroes' and 'disabledHeroes' in team '<authored>' for allTeams, team1, team2, and when the offending team is not the first authored. An empty enabledHeroes: [] still counts, matching upstream key-presence.
• Non-dict general: number/boolean dropped cleanly, matching oracle.
• Error path: workshop_settings → Result is handled at both call sites (compiler/lowering.rs:293 ?, tooling.rs:208 match → Diagnostic::from_integration_error as Error severity). opy compile and opy check both surface error[settings-hero-lists] and fail. No other callers.
• Regression: general scalars translate, general members emit before hero groups, roster lists emit last, roster elements canonicalize (mccree/hammond in lists → Cassidy/Wrecking Ball) — all match oracle.
• Gates: cargo test -p opy-rs (all pass), fmt --check, clippy --all-targets clean.

Routed residuals — confirmed in scope, nothing more

• ana.ability3Cooldown%: [11,22] / {x:1} → oracle emits the verbatim block; native error[workshop-emission]: settings key 'ability3Cooldown%' does not match its table kind.
• General-nested groups/lists: oracle emits someGroup{a:1}, someList{x,y}, enabledHeroes{mccree} verbatim; native resolves names (dva→D.Va, enabledHeroes→enabled heroes/Ana) or errors unknown hero 'mccree'/'someGroup'/'someList'. Exactly the canonical-emitter verbatim escape tracked in wrightkit/workshop-rs#417.

New findings

Medium — hero-group name canonicalization still uses in-place rename (settings.rs:199-203). Upstream canonicalizes mccree/hammond team members by assign+delete (overpy.js:71287-71294) — the same mechanism this commit fixed for ability1KB%, but the group-name rename was left in place. Verified divergences:

• {"mccree": {health%: 50}, "ana": {health%: 60}} → oracle emits Ana then Cassidy (destination appended at end); native emits Cassidy then Ana (order flipped). Same for hammond→Wrecking Ball.
• {"cassidy": {health%: 10}, "mccree": {health%: 50}} (and the reverse order) → oracle collapses to a single Cassidy { Health: 50% } (source value wins at the destination position); native emits two Cassidy blocks (Health: 10% and Health: 50%).

The same position/remove/find/push treatment used for ability1KB% is needed on the group name: remove mccree/hammond, then overwrite an authored cassidy/wreckingBall group in place or append at the end.

Low — non-dict general drop is broader than upstream (settings.rs:162). Upstream runs compileCustomGameSettingsDict over general and Object.keys of a string or list yields index keys: general: "ab" emits 0: a, 1: b; general: [10, 20] emits 0: 10, 1: 20. Only numbers/booleans produce zero keys and are truly dropped (and null is rejected earlier at settings-parse, which native also rejects). Native drops the string/list forms. Garbage-tier input, but observable.

Low — both-lists check is List-scoped; upstream checks key presence (settings.rs:168-184). "enabledHeroes" in team is true for any value kind: {"enabledHeroes": 5, "disabledHeroes": ["genji"]} or a dict-valued enabledHeroes still produces Cannot have both … upstream. Native misses the presence check for non-list members and instead errors later at emission ('enabledHeroes' does not match its table kind / unknown hero 'enabledHeroes'). Both reject the program — diagnostic parity only.

Informational (pre-existing, not introduced here)

• {"ana": [1, 2]} at team level → oracle emits Ana { 0: 1; 1: 2 }; native errors outside the emission table. heroes: 5 → oracle emits empty heroes {}; native errors settings block children must be groups. Same verbatim-emission class as workshop-rs#417; the _ => {} arms predate this commit.
• {"ana": 5}, allTeams: 5, non-hero team members (foo: 5, foo: {}, foo: [1,2]), duplicate keys, and bad roster elements all error on both sides (upstream via tows/TypeError, native via emission/parse errors) — parity holds.
• Nit: the stored commit message lost every backticked identifier ("The flattened team members", "renamed in place", "both and ", "A non-dict under a team") — likely stripped during message authoring.

Verdict

The four claimed fixes are real and verified; the two routed residuals reproduce exactly as described and nothing beyond their scope was found. One Medium finding remains: the identical assign+delete semantics for mccree/hammond group names (duplicate Cassidy blocks + ordering) should get the same treatment before merge, or be explicitly deferred with a routed issue.

[e54-bot]

Second re-review round addressed in de5f20e:

• Hero-group aliases (mccree/hammond): now renamed with the same assign+delete ordering as member keys — the moved group lands at the end unless a canonical-dest group exists, in which case the source collapses onto the destination's position and its members replace the destination. Verified against the oracle for order and single-collapse on both authored orders.
• Non-dict general: string and list values emit index-keyed members (e.g. "0: a"), numbers/booleans drop — matching upstream's Object.keys iteration.
• Roster conflict: both enabledHeroes/disabledHeroes detected by key presence regardless of node kind.
• Flattened general members are excluded from the hero pass by construction (the pass iterates rest + rosters only).

All 6 cases probe-verified against the pinned oracle; 21 tooling tests, fmt, clippy green.

[e54-bot]

Third re-review round verified de5f20e against the pinned oracle (58 probe compiles, token-level comparison; probes kept under target/review-509-probes/).

Claimed fixes — verified

• Group rename = assign+delete: all single-alias shapes match the oracle exactly — move-to-end ({"mccree":{"health%":50},"ana":{"health%":60}} → Ana then Cassidy), collapse onto the canonical dest in both authored orders (cassidy+mccree and reverse → single Cassidy { Health: 50% } at the dest position, source members wholesale replacing dest members), mid-list dest (ana, mccree, cassidy → Ana then Cassidy at dest index), empty-group collapse, non-Group dests ({"cassidy":[1,2],"mccree":{…}} → single Cassidy group), and ability1KB% inside a renamed group ({"mccree":{"ability1KB%":30}} → Cassidy { ability1Kb%: 30 } verbatim, correctly gated against cassidy's set).
• Non-dict general: {"general":"ab"} → 0: a, 1: b; {"general":[10,20]} → 0: 10, 1: 20; number/bool/empty-string drop — all match, including a hero-named string ("dva" → 0: d, 1: v, 2: a, not a D.Va group, confirming flattened members skip the hero pass).
• Roster conflict by key presence: enabledHeroes as number/dict/string + disabledHeroes list, both-numbers, empty-list, and team2 — all produce the verbatim upstream message Cannot have both 'enabledHeroes' and 'disabledHeroes' in team '<name>'.
• Parity holds on rejects: scalar/unknown hero members, duplicate keys, null, and non-list enabledHeroes alone (upstream TypeError vs native table-kind error — both reject).

Finding

Medium — the two alias renames must run in upstream's fixed order, not authored order (settings.rs:218-237). Upstream performs mccree→cassidy then hammond→wreckingBall as two sequential steps (overpy.js:71286-71294), so when both aliases append they always land cassidy-before-wreckingBall. The single left-to-right pass appends whichever alias is authored first:

• {"hammond":{"health%":50},"mccree":{"health%":60}} → oracle Cassidy{60} then Wrecking Ball{50}; native Wrecking Ball{50} then Cassidy{60}.
• {"hammond":{…},"genji":{…},"mccree":{…}} → oracle Genji, Cassidy, Wrecking Ball; native Genji, Wrecking Ball, Cassidy.
• {"disabledHeroes":["mccree"],"hammond":{…},"mccree":{…}} → oracle Cassidy, Wrecking Ball, disabled heroes; native Wrecking Ball, Cassidy, disabled heroes.

Trigger: hammond authored before mccree with no wreckingBall group present (a wreckingBall dest collapses in place and hides it; cassidy dests don't matter). Alphabetical OW1-era member lists hit this since hammond < mccree. Fix matches upstream structure: run the remove→rename→collapse/append once for mccree, then again for hammond, instead of one generic pass.

Low / route-with-#417

• general list with non-scalar elements: {"general":[[1,2]]} → oracle emits a 0 { 1 / 2 } block; native flattens to 0: [1,2]. {"general":[{"a":1},"x"]} → oracle 0 { a: 1 } + 1: x; native rejects at parse (expected ',' or ']'). The index-keyed flattening assumes scalar element.value; the structured cases need the same verbatim-emission escape as wrightkit/workshop-rs#417 (the dict-element one additionally needs list-element grammar support).
• Alias rename is Group-scoped; upstream's truthiness check covers any value kind: {"mccree":[1,2]} → oracle Cassidy { 0: 1 / 1: 2 }; {"mccree":[1,2],"cassidy":{…}} → oracle replaces cassidy with the list. Native leaves the list named mccree and errors at emission — inside the #417 gap today, but when that escape lands this path would still emit mccree instead of Cassidy. Worth capturing in the routed issue so the rename isn't silently dict-only.
• Astral chars in general strings: {"general":"a😀"} → oracle iterates UTF-16 units (1: \ud83d, 2: \ude00); chars() iterates scalars (1: 😀). Upstream's lone-surrogate output is arguably its own bug — pathological, noting for routing completeness.
• mccree/hammond scalar values (5, 0, false, "x") reject on both sides — upstream via TypeError ("ability1KB%" in 5) or tows, native via emission kind-check. Parity.

Gates

cargo test -p opy-rs --test tooling — 21/21 pass (incl. the new group-rename, non-dict-general, and both-rosters tests). The tests cover single-alias shapes; none exercise the both-alias ordering above, which is how the Medium slipped through.

[e54-bot]

The both-aliases ordering medium is fixed in 67e8925: the rename now runs one pass per entry of HERO_NAME_ALIASES (mccree, then hammond), so appended groups follow alias order rather than authored order. Verified against the oracle for hammond-before-mccree, mccree-before-hammond, and both aliases with canonical destinations authored first. The alias list is now a const table shared by canonical_hero_name so the pass order cannot drift from the map.

The three lows are recorded: general list/dict elements and non-group alias values stay routed with wrightkit/workshop-rs#417 (the verbatim-emission wall), and the astral-char case is noted there as pathological UTF-16-vs-chars divergence.

[e54-bot]

Follow-up review at 67e8925. Verified the per-alias pass against the pinned oracle (overpy 9.7.10) and opy-cli compile.

Fix verified. HERO_NAME_ALIASES iterates one full pass per alias in upstream's order (overpy.js:71286-71294), and the while scan correctly leaves index unincremented after a remove so consecutive same-alias groups are all handled. Oracle probes — both aliases in both authored orders, aliases interleaved with canonical groups and enabledHeroes, alias+canonical-dest collapse in all four orders, all four names in one team, canonical dest authored as a scalar, ability1KB% inside renamed groups, both teams, empty alias group — are token-identical between the oracle and our output, including the prior Medium shape (hammond before mccree → Cassidy emitted first). Regressions re-checked: ability1KB% assign+delete, general flattening, roster conflict detection all still match; cargo test -p opy-rs --test tooling, fmt, clippy clean.

Non-blocking observations (all predate this commit — the diff touches only the alias loop — but sit in this PR's feature area, flagging for follow-up triage):

• general members that are group/list-valued with a non-hero, non-settings name (e.g. general: {"foo": {"x": 1}} or {"mccree": {...}}) hard-error unknown hero at emission; upstream writes them verbatim inside General (foo { x: 1 }). A group named like a hero (general: {"dva": {"health%": 50}}) resolves and translates (D.Va { Health: 50 }) where upstream stays fully verbatim (dva { health%: 50 }). Upstream compiles general's dict against general.values only; routing general children through the heroes.<team> path makes hero resolution apply.
• List-valued team members (ana: [1,2], mccree: [1,2]) error outside the emission table; upstream emits an index-keyed block (Ana { 0: 1; 1: 2 }). Scalar/string/falsy alias values already fail upstream (TypeError or "No match found for keyword 'mccree'"), so our structured error there is parity-or-better.

Neither blocks this fix; both are narrow edge cases on upstream-accepted input. Consider a follow-up issue for verbatim group emission under heroes.<team> and non-dict team members.