Skip to content

test: check formatting against an independent rendering - #398

Merged
thedavidmeister merged 6 commits into
mainfrom
2026-10-09-issue-372-format-oracle
Oct 10, 2026
Merged

thedavidmeister merged 6 commits into
mainfrom
2026-10-09-issue-372-format-oracle

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Closes #372

Formatting tests checked format against parse (round trip plus format(parse(s)) == s), against itself ("-" + format(x) == format(-x), two representations agree), or only checked length and some characters. A formatter that is wrong in the same way in every place passes all of these.

expectedFormat(c, e, scientific) builds the expected string from the OZ Strings.toString(|c|) digits. It does not call the formatter:

  • plain: trailing zeros are stripped, then the decimal point goes e places from the right.
  • scientific: d.ddd, followed by e and the leading digit's exponent (e + digits - 1). The e part is left out when that exponent is 0. _toScientific's NatSpec now states that rule.

testExpectedFormatOracle checks the oracle against hand-written strings.

Per the #376 ruling (b), the formatter's int32 and int224 refusal guards are to be dropped, so this PR asserts no guard revert. Only the documented ±1000 plain-mode cap is asserted.

Changes:

  • Every test test: formatting is only checked against parse, never against an independent rendering #372 lists now asserts exact equality with the oracle. That includes the round trips (also for the re-parsed Float's own representation), the negative-sign test, the canonical-across-representations test, the length-only tests, the negative cap ("0." + 999 zeros + "1") and 1e67 ("1" + 67 zeros).
  • testFormatNonScientificOutputShape is removed. Its shape checks are implied by exact equality, and testFormatNonScientificSucceedsForNonPositiveExponents now asserts the oracle over the same domain.
  • New testFormatScientificFullExponentDomain covers every int224 coefficient and int32 exponent and asserts the oracle. Inputs whose leading digit exponent is above int32.max are skipped while src still refuses them.
  • New testFormatNonScientificPositiveExponentBoundary covers e = 1..67, both signs: floor(int224.max / 10^e) must render as the oracle.
  • _toScientific NatSpec: the eN part is omitted when N is zero.

The pre-existing guard-revert tests on main (testFormatScientificDisplayExponentOverflowReverts, testFormatScientificRevertsNearPositiveInt32Limit, testFormatNonScientificRevertsOnLongPositiveExp, the revert branch of testFormatParseRoundTripNonScientificSafePosExp, testFormatNonScientificLargePositiveExponent, testFormatNonScientificLargeCoefficientLargeExponent) are left for the #376 change that drops the guards.

QA

  • Verified with mutation-probe (adversarial-mutation-test skill): one 39-mutant file over src/lib/format/LibFormatDecimalFloat.sol, run against a fresh clone of main 51a3451 and against this branch 02e2156. Suite: forge test --match-path test/src/lib/format/*, FOUNDRY_FUZZ_SEED=0x1. Main kills 34/39, this branch kills 36/39.
  • Killed on this branch and survived on main: "e" -> "e+" for 1000 < displayExponent < 1e9 (killed by the oracle-equality fuzzes; main survives because the string still parses to the same value), and the int32 lower guard tightened by 100.
  • Survive on both: exponent > MAX_NON_SCIENTIFIC_EXPONENT -> > MAX + 1 (equivalent: any exponent >= 68 already reverts with the same UnformatableExponent(exponent)), uExp >= 68 -> >= 69, and limit /= 10 -> /= 9. The last two only loosen the positive-exponent refusal guard that test: format refusal rules are copied from undocumented code guards #376 drops. They no longer fail here because the revert assertions were removed.
  • The other 34 mutants (zero, mode switch, scale and exponent off-by-ones, sign handling, fraction leading zeros, scientific trailing-zero strip including keeping a 0 above 1e40, point characters, e0 rule, plain trailing-zero strip, point placement, leading zeros, cap lower bound) are killed on both.
  • Oracle: expectedFormat takes the Strings.toString(|c|) digits and places them by the documented plain and scientific rules. It never calls the formatter, parse or maximizeFull, and is pinned by testExpectedFormatOracle against hand-written strings. The fuzz found no disagreement between the formatter and the oracle.
  • Category check: test: formatting is only checked against parse, never against an independent rendering #372 asks for (a) round trips and canonical checks, (b) metamorphic-only checks and (c) no-value or partial-value checks to get an independent oracle. All three are covered. (a) RoundTripNonNegative, RoundTripNegative, ScientificFullDomain, NonScientificNegExpFullDomain, the SafePosExp success branch and AtIntDigitBoundary now assert the oracle. (b) CanonicalAcrossRepresentations and the - check now assert the oracle; OutputShape is removed because the oracle assertion covers it. (c) SucceedsForNonPositiveExponents and ScientificNegativeBoundaryDoesNotRevert now assert the oracle, and ExponentAtNegativeCap asserts the full string.

🤖 Generated with Claude Code

baku-ccron and others added 3 commits October 9, 2026 13:31
Closes #372

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 37 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 4803f5ca-2519-43f2-88fc-62710d30db74

📥 Commits

Reviewing files that changed from the base of the PR and between d5605ae and 9ae345d.


📒 Files selected for processing (2)
  • src/lib/format/LibFormatDecimalFloat.sol
  • test/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.sol

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thedavidmeister
thedavidmeister merged commit 17b5b74 into main Oct 10, 2026
7 of 8 checks passed
@github-actions

Copy link
Copy Markdown

@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment:

S/M/L PR Classification Guidelines:

This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed.

Small (S)

Characteristics:

  • Simple bug fixes, typos, or minor refactoring
  • Single-purpose changes affecting 1-2 files
  • Documentation updates
  • Configuration tweaks
  • Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

  • Fix typo in variable name
  • Update README with new instructions
  • Adjust configuration values
  • Simple one-line bug fixes
  • Import statement cleanup

Medium (M)

Characteristics:

  • Feature additions or enhancements
  • Refactoring that touches multiple files but maintains existing behavior
  • Breaking changes with backward compatibility
  • Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

  • Add new feature or component
  • Refactor common utility functions
  • Update dependencies with minor breaking changes
  • Add new component with tests
  • Performance optimizations
  • More complex bug fixes

Large (L)

Characteristics:

  • Major feature implementations
  • Breaking changes or API redesigns
  • Complex refactoring across multiple modules
  • New architectural patterns or significant design changes
  • Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

  • Complete new feature with frontend/backend changes
  • Protocol upgrades or breaking changes
  • Major architectural refactoring
  • Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

  • Test coverage impact: More comprehensive test changes lean toward larger classification
  • Risk level: Changes to critical systems bump up a size category
  • Team familiarity: Novel patterns or technologies increase complexity

Notes:

  • the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
  • the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
  • do not include any additional text, only the size classification
  • your assessment comment must not include tips or additional sections
  • do NOT tag me or anyone else on your comment

@linear

linear Bot commented Oct 10, 2026

Copy link
Copy Markdown

RAI-3156

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.

test: formatting is only checked against parse, never against an independent rendering

1 participant