Skip to content

fix(settings): write numbers with JavaScript String(value) semantics - #507

Merged
e54-bot merged 3 commits into
mainfrom
wt-496-js-settings-numbers
Oct 9, 2026
Merged

e54-bot merged 3 commits into
mainfrom
wt-496-js-settings-numbers

Conversation

@e54-bot

@e54-bot e54-bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #496 — the settings-number half of the pinned-oracle divergence.

OverPy 9.7.10 renders numeric settings values with JavaScript
String(value) semantics (1e21 → 1e+21, 0.0000001 → 1e-7,
123456789012345678901234 → 1.2345678901234569e+23) and reads decimal
exponent forms plus 0x/0b/0o integer literals. opy-rs emitted Rust
Display text and could not read exponent or binary/octal literals at
all: the settings block is re-lexed for #!define expansion and the
lexer split 0b101 into 0 + b101, which the re-render joined as
0 101.

  • lexer: keep 0b/0o literals as one number token like 0x
    (also makes them valid in ordinary expressions, matching upstream's
    JS tokenizer — 0b101 + 0o17 folds to 20)
  • parser: evaluate radix number tokens in f64 like JavaScript
    Number (no u64 overflow cutoff for long digit runs)
  • settings: accept exponent and radix literals in the JSONC cursor
    parser and render scalars plus bare list elements through the existing
    compiler::number_format::javascript_text (now pub(crate), with
    JavaScript spellings for non-finite values)

Verified byte-for-byte against the oracle: all 7 scalar probes and all
5 list elements emit identical text.

Remaining scope of #496

Catalogued settings keys whose authored value is an object/block still
fail (settings key 'mapRotation' does not match its table kind).
That is owned by workshop-rs (canonical settings acceptance) and
lands via wrightkit/workshop-rs#413 plus a released workshop-rs
version bump here.

Test plan

  • cargo fmt --all -- --check
  • cargo clippy -p opy-rs --all-targets -- -D warnings
  • cargo test -p opy-rs --lib (523), --test tooling (13, incl.
    new settings_numbers_render_like_the_pinned_oracle)
  • cargo test -p opy-rs --test differential — oracle corpus agrees
  • python3 -m unittest discover -s tools/overpy/tests (40)
  • Oracle probes: 0b101→5, 0o17→15, 0x1F→31, 1e21→1e+21,
    0.0000001→1e-7, 123456789012345678901234→1.2345678901234569e+23,
    0.5e3→500, 12e2→1200, 1.0→1, 1e20→100000000000000000000
    — all match

The pinned OverPy 9.7.10 renders numeric settings values with JavaScript
Number.prototype.toString semantics and reads decimal exponent forms plus
0x/0b/0o integer literals. opy-rs emitted Rust Display text and could not
read exponent or binary/octal literals at all: the settings block is
re-lexed for macro expansion and the lexer split 0b101 into 0 + b101.

- lexer: keep 0b and 0o literals as one number token like 0x
- parser: read radix number tokens in f64 like JavaScript Number
- settings: accept exponent and radix literals and render scalars and
  bare list elements through javascript_text

Refs #496

@e54-bot e54-bot 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.

Reviewed against the issue and re-probed the pinned 9.7.10 oracle independently. The core claims hold: 0b101→5, 0o17→15, 0x1F→31, 1e21→1e+21, 0.0000001→1e-7, 123456789012345678901234→1.2345678901234569e+23, 1e20→100000000000000000000 all emit byte-identical text to the oracle in scalar and bare-list positions, including catalogued keys (ffaSlots: 1e+21, respawnTime%: 1e+21), -0x1F→-31, 0x10+2/0x10-2/0x10*2→18/14/32 via the expression-continuation rewind, 1e999→Infinity, and #!define-expanded radix values. Refs #496 is correct — the remaining enum-key/object-value scope is genuinely workshop-rs#413. All four new tests fail on the pre-change code (verified on a 12f3b5d worktree).

Three findings:

  1. [low] Stale support claim — docs/language-support/syntax-and-projects.md:21 still lists only 0x/0X hexadecimal literals as supported. This change adds 0b/0B/0o/0O acceptance in expressions and settings, and exponent/radix forms in the settings block. The repo's own rule is to keep compatibility/support claims synchronized with implementation; update the row (e.g. binary/octal radix spellings, and the settings-block number surface) in this PR.

  2. [medium — needs a recorded decision] Deliberate acceptance superset for signed exponents — the pinned oracle rejects 1e-7, 1e+7, 2.5e-3, -2.5e-3 in settings (Error: Unhandled operator '-' or '-=' in custom game settings); this PR accepts them and emits 1e-7/10000000/0.0025/-0.0025. The parse_number doc comment documents the choice honestly and parse_block_reads_javascript_number_literals pins -2.5e-3. Accepting is arguably the correct JS semantics and is diagnostics-parity rather than a structural output difference — but #496's acceptance criterion says remaining differences should each be a recorded exception. An inline comment is a record of engineering intent, not an approving decision: please either point to the approving decision/issue or record it explicitly (and if the intended contract is "match the reference", the alternative is to reject the signed exponent like the tokenizer does).

  3. [low — remaining difference in the touched surface] Bare list elements that are not pure literals still emit verbatim — [1+2] emits 1+2 where the reference folds to 3; [0b101*2] emits 0b101*2 where the reference writes 10; [NaN]/[inf] emit their text where the reference errors (Unknown function name). This is pre-existing (elements were always verbatim strings) and folding would need the expression path for list elements — likely a follow-up rather than this PR — but per the issue's "list shapes" acceptance criterion it is an unrecorded remaining difference and should be tracked explicitly.

For the record, the new rejections are the right side of the contract: 0x, 0b, 0o, 0b2, 0b102, 0x1G all produce garbage in the reference (a: NaN, a: 1 via parseInt/Number quirks) — rejecting with lex-error/settings-invalid matches the established "reject what cannot be canonical" exception pattern.

Review evidence shows the reference's settings tokenizer splits an
exponent sign out as an operator and errors, so 1e-7, 1e+21 and 2.5e-3
are rejected there while 1e21, -0.5 and expression forms such as 1+2
are accepted. Read only unsigned exponents in the settings literal
parser so scalar acceptance converges exactly, and update the integer
literal row of the language-support table for the 0b/0o spellings.

Refs #496
@e54-bot

e54-bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

All three findings are addressed in head 567ba1e (this review evaluated 7752942; the follow-up commit landed after it):

  1. Stale support claim — docs/language-support/syntax-and-projects.md:21 now lists 0x/0X, 0b/0B, 0o/0O with the same numeric meaning as the pinned tokenizer.
  2. Signed exponents — converged rather than recorded: parse_number now reads only unsigned exponents and rejects 1e-7/1e+21/-2.5e-3/0.5e+3 like the reference (parse_block_rejects_signed_exponents_like_the_pinned_reference). Corrected probe evidence posted on Unknown-key settings values and enum-key lists are written differently from the pinned reference #496 (issuecomment-6088127158).
  3. Bare-list verbatim — recorded as a remaining difference on Unknown-key settings values and enum-key lists are written differently from the pinned reference #496 (issuecomment-6088131073): non-literal list elements stay verbatim where the reference folds or errors. Agreed it is a follow-up, not this PR.

@e54-bot e54-bot 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.

Re-reviewed 12f3b5d..567ba1e against #496 and re-probed the pinned 9.7.10 oracle independently. All three prior findings are resolved:

  1. Docs — syntax-and-projects.md:21 now lists 0x/0X, 0b/0B, 0o/0O. Resolved.
  2. Signed exponents — resolved by convergence: parse_number reads unsigned exponents only. Verified on both sides: 1e-7, 1e+21, 2.5e-3, -2.5e-3, 1E-5, 1e+5, -1e-7 all reject (oracle Unhandled operator / ours settings-invalid); 1e21→1e+21, 1e7→10000000, 1E5→100000, 9e99→9e+99, 12e2→1200, -1e21→-1e+21, -0.5, - 5→-5, 0x1e5→485, -0x1F→-31, 1e999→Infinity, +0b101/[+5] in lists all emit byte-identical value text. 0x/0b/0o radix work in settings relies on the new lexer tokens round-tripping expand_settings's re-lex — verified (0b101→5 scalar, #!define RAD 0b101→5, 0b101 + 0o17 + 0x1e5→505 in rule context).
  3. Bare list elements — recorded on the issue as claimed; verified [1+2]→1+2 vs oracle 3, [0b101*2]→0b101*2 vs 10, [NaN]/[inf]/[1e-7]/[e5] emit vs oracle error.

Validation: cargo test -p opy-rs --lib (524 pass), --test tooling (13 pass), fmt/clippy/git diff --check clean. New signed-exponent test discriminates against the intermediate implementation by construction (at 7752942 the signed branch parsed 1e-7 → unwrap_err fails).

New findings (all re-probed, not trusted from the issue record):

  1. [medium] Catalogued number/percent members still emit Rust Display, not JS String(value) — the prior round's claim that ffaSlots: 1e+21 and respawnTime%: 1e+21 verified byte-identical is not reproducible. ffaSlots: 1e21 is range-rejected by the oracle (cannot be over 12), so it could never have verified; and respawnTime%: 1e21 measurably differs: oracle Respawn Time Scalar: 1e+21%, ours 1000000000000000000000%. Also respawnTime%: 0.0000001 and heroes.allTeams.mei.ability1Cooldown%: 0.0000001 → oracle 1e-7% vs ours 0.0000001%; respawnTime%: 1e999 → Infinity% vs inf%; 123456789012345678901234 → 1.2345678901234569e+23% vs 123456789012345690000000%. Cause: convert_settings_node keeps SourceNode::Number typed → Member::Number/Member::Percent → workshop-rs format_setting_number (Rust Display, never exponent form). The JS treatment reached scalar_text (unknown keys — verified Hero Limit: 1e+21), convert_workshop_node (settings.workshop — verified 1e+21/1e-7/Infinity), display_value, and list elements — but not the typed member path, which this PR's acceptance expansion newly reaches (1e21 spellings rejected before, now emit Display text). The formatter is workshop-rs-owned, so correction is either a workshop-rs-side change or a recorded remaining divergence on #496 like the object-value case — plus the verification claim needs correcting.

  2. [low] 1.e5/0.e5 newly accepted where the reference rejects — the unsigned-exponent branch accepts digits '.' <zero digits> 'e' digits: ours emits 100000/0, oracle errors Expected a number after '.' but got 'e5'. Same in lists ([1.e5]→100000 vs error). Introduced by this PR's exponent support (base rejected it); unrecorded accept-superset. Either require a fraction digit before the exponent or record it. Related unrecorded siblings in the same surface (pre-existing, unchanged here): 5.→5 vs oracle Expected tokens after '.', -.5→-0.5 vs oracle Unknown member '5'.

  3. [low — record correction] The issue comment recording "k": 1-2 as divergent is wrong: the oracle emits k: -1 — converged, as are 1%2→1, 1 - 2→-1, 2-1→1. The settings evaluator accepts + - * / % and ==; only = errors (ours rejects it too). Genuine remaining expression-path divergences run the other way (reject-supersets, all pre-existing): --5→oracle 5, 1<2/1>2→true/false, 1 and 2→2, 1 or 2→1, Infinity→Infinity — ours rejects all of these.

  4. [note] Bare 0b/0o/0X literals in expression position emit the evaluated value vs the oracle's verbatim spelling: g = 0b101→5 vs 0b101, g = 0o17→15 vs 0o17, g = 0X1F→31 vs 0X1F (the oracle canonicalizes only lowercase 0x; folded positions converge — 0b101+0→5 both). Newly-accepted surface; our output is the canonically parseable one, so this is defensible like the 0x→reject-vs-NaN cases, but it is an unrecorded emitted-text difference. Related pre-existing: g = 1e21→1000000000000000000000 vs oracle 1e21, g = 1e-7→0.0000001 vs oracle Subtract(1e, 7) — the reference never reads signed exponents anywhere, which is consistent with this PR's settings-side choice.

Verdict: prior findings resolved, but finding 1 is a real remaining output divergence inside this PR's stated scope that was mis-verified last round — it needs a fix, an owner-directed follow-up, or a recorded exception before merge.

The pinned settings tokenizer rejects 1.e5-style spellings ("Expected a number after '.'"). Scalar members now error the same way; the spelling is also gated out of js_number_literal so list and resolved-constant positions keep the authored text instead of emitting a fabricated value.

Refs #496
@e54-bot

e54-bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the re-review findings in a7f37d8:

  1. [medium] Catalogued number/percent formatting — confirmed; the typed Member::Number/Percent path still goes through format_setting_number (Rust Display). That formatter is the canonical Workshop emitter owned by workshop-rs, and switching it to JS semantics has round-trip consequences (the canonical lexer cannot reparse 1e+21), so this is routed to workshop-rs#415 for an owner decision and recorded on Unknown-key settings values and enum-key lists are written differently from the pinned reference #496 as a remaining divergence. Also correct on the mis-verification: ffaSlots: 1e21 is range-rejected upstream — the earlier "byte-identical" claim for that pair was wrong.
  2. [low] 1.e5/0.e5 — fixed in a7f37d8: scalar members now reject like the reference; the spelling is gated out of js_number_literal too, so [1.e5] keeps authored text rather than emitting 100000 (accept-superset vs the reference's error, recorded).
  3. [low] record correction — posted on Unknown-key settings values and enum-key lists are written differently from the pinned reference #496: 1-2→-1 converges; the evaluator accepts + - * / %/==.
  4. [note] radix spellings in expression position — recorded on Unknown-key settings values and enum-key lists are written differently from the pinned reference #496 as an emitted-text divergence alongside the pre-existing g = 1e21 sibling.

cargo fmt/clippy/tests re-verified on a7f37d8 (settings lib tests 47/47; k: 1.e5 → settings-invalid, [1.e5] → verbatim 1.e5, [1.5e3] → 1500, [5.] → 5 unchanged).

@e54-bot e54-bot 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.

Re-review of a7f37d8 — verified against the pinned oracle (overpy 9.7.10 via tools/overpy/probe_batch.cjs) and the local suite.

Fix verified:

  • parse_number's fraction == 0 && peek ∈ {e,E} guard rejects 1.e5, 0.e5, 1.E5, -1.e5, 1.e, 1.e+5, 1.ee in scalar position (settings-invalid), matching the oracle's Expected a number after '.' rejection on every spelling probed. 1e5, 1E5, 1.5e3, 0.5, 0x1F still emit correctly (100000%, 1500%, 0.5%, 31%).
  • js_number_literal's .e/".E" substring gate: no valid literal is wrongly rejected (1.5e3, 1E5, radix spellings still parse; the only f64-parseable strings containing .e/".E" are the gated spellings themselves). 1.e5, 1.e+5, 1.ee5, 0x1.e, -.e5 now keep authored text.
  • [1.e5] keeps authored 1.e5 instead of fabricating 100000; catalogued lists still reject it downstream (unknown map '1.e5' — the same class the oracle reports for evaluated spellings, e.g. [1e5] → Unknown map '100000'), and non-catalogued keys error at emission. I found no silent-passthrough path.
  • Finding-1 routing is honest: respawnTime%: 1e21 still emits 1000000000000000000000% where the oracle emits 1e+21%; the catalog-member JS-String divergence is unchanged pending workshop-rs#415.
  • cargo test -p opy-rs (525 unit + corpus differential + integration): all pass. cargo fmt --all -- --check and cargo clippy -p opy-rs --all-targets: clean.

Residual divergences noticed while probing (all pre-date a7f37d8; none block):

  • 5., 12., -.5, .5 remain accepted where the oracle errors (Expected tokens after '.' / Unknown member '5'). Base parse_number consumed . unconditionally before this PR, so this is a pre-existing gap in the same family as 1.e5 — worth recording on #496 as a known divergence or a small follow-up.
  • 1.5e (bare trailing exponent): we error expected ',' or '}'; the oracle emits NaN%. Rejecting is arguably the better behavior; noting for completeness.
  • 1e999 emits inf% vs the oracle's Infinity% — same Rust-Display-vs-JS-String root as finding 1, covered by the workshop-rs#415 routing.
  • Nit: the new error reports invalid number '1.' (the consumed prefix) where the reference names the offender (Expected a number after '.' but got 'e5'); accurate but less descriptive.

The round-2 1.e5/0.e5 regression is closed; no new findings against this commit.

@e54-bot
e54-bot merged commit b19eeb2 into main Oct 9, 2026
6 checks passed
@e54-bot
e54-bot deleted the wt-496-js-settings-numbers branch October 9, 2026 21:29
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.

2 participants