test: mutation-harden LibFixedPointDecimalParse coverage - #22
test: mutation-harden LibFixedPointDecimalParse coverage#22thedavidmeister wants to merge 4 commits into
Conversation
Adversarial mutation pass over decimalStringTofixedPoint. Existing tests
already kill most mutants; two genuine surviving mutants were found and
killed with new discriminating tests:
- Fractional-part overflow propagation (line 71, previously uncovered): a
>77-digit fraction overflows uint256 in the inner integer parse and the
resulting ParseDecimalOverflow selector is returned from the fractional
error branch. Pinned with a 78-nine fraction.
- Decimal-point gate (line 44): a non-point character directly after the
integer with no further input ("1x") must be rejected as an invalid
string. Without the gate the parse wrongly succeeds as 1e18. The
pre-existing corrupt-integer tests only reached this path via inputs whose
garbage is caught by a later gate, so they did not discriminate it.
Two remaining survivors are equivalent mutants (the strip-loop lower bound
>= -> > at fracStart, and the frac-add overflow guard < -> <=) that cannot
change any output; documented as such, no test added.
Tests only: src/ is unchanged. Adds an audit/mutation-test-scans.json scan
record for org-wide health tracking.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WalkthroughThe decimal parser now rejects inputs such as ChangesDecimal parser validation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Rework note (human reject, 2026-07-04): design ruling — an EMPTY FRACTION IS INVALID. The |
…tring parse An empty fraction is invalid per the org design ruling in #24: the decimal point MUST be followed by at least one digit, so "1." returns ParseDecimalInvalidString instead of parsing as 1e18. Pins the revert on the exact selector for the whole category and pins adjacent valid forms unchanged. Closes #24 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thedavidmeister
added
the
human:needs-work
|
🤖 ai:producer |
thedavidmeister
added
ai:needs-work
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@audit/mutation-test-scans.json`:
- Around line 3-6: Update the mutation-scan audit record for publishedTag v0.2.0
by rerunning the scan at the current HEAD, replacing the stale commit and
commitsAheadOfTag values with the resulting revision and count, and refreshing
the timestamp as appropriate. Preserve the existing audit record format and
scoped-file coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 311d0caa-71cc-41fc-b5f0-773576e2c1da
📒 Files selected for processing (3)
audit/mutation-test-scans.jsonsrc/lib/parse/LibFixedPointDecimalParse.soltest/src/lib/parse/LibFixedPointDecimalParse.decimalStringToFixedPoint.t.sol
| "timestamp": "2026-06-15T00:00:00Z", | ||
| "commit": "dc85846152e2fd42f8f2c07feb72cc1abc708674", | ||
| "publishedTag": "v0.2.0", | ||
| "commitsAheadOfTag": 0, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
recorded_commit="$(jq -r '.[0].commit' audit/mutation-test-scans.json)"
published_tag="$(jq -r '.[0].publishedTag' audit/mutation-test-scans.json)"
expected_ahead="$(jq -r '.[0].commitsAheadOfTag' audit/mutation-test-scans.json)"
tag_commit="$(git rev-parse "${published_tag}^{commit}")"
actual_ahead="$(git rev-list --count "${tag_commit}..${recorded_commit}")"
test "${tag_commit}" = "${recorded_commit}"
test "${actual_ahead}" -eq "${expected_ahead}"
git diff --quiet "${recorded_commit}" HEAD -- \
src/lib/parse/LibFixedPointDecimalParse.sol \
test/src/lib/parse/LibFixedPointDecimalParse.decimalStringToFixedPoint.t.solRepository: rainlanguage/rain.math.fixedpoint
Length of output: 371
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- record ---'
cat -n audit/mutation-test-scans.json
printf '%s\n' '--- local refs and objects ---'
git show-ref --tags | tail -20 || true
git rev-parse --verify dc85846152e2fd42f8f2c07feb72cc1abc708674^{commit} 2>&1 || true
git rev-parse --verify HEAD
git diff --stat dc85846152e2fd42f8f2c07feb72cc1abc708674 HEAD -- \
src/lib/parse/LibFixedPointDecimalParse.sol \
test/src/lib/parse/LibFixedPointDecimalParse.decimalStringToFixedPoint.t.sol 2>&1 || true
printf '%s\n' '--- remote metadata ---'
git remote -v || trueRepository: rainlanguage/rain.math.fixedpoint
Length of output: 1291
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
repo='rainlanguage/rain.math.fixedpoint'
tag='v0.2.0'
sha='dc85846152e2fd42f8f2c07feb72cc1abc708674'
api() {
curl --fail --silent --show-error \
-H 'Accept: application/vnd.github+json' \
"https://api.github.com/$1"
}
printf '%s\n' '--- tag ref ---'
api "repos/${repo}/git/ref/tags/${tag}" | jq '{ref, object}'
printf '%s\n' '--- recorded commit ---'
api "repos/${repo}/commits/${sha}" | jq '{sha, message: .commit.message, date: .commit.committer.date, parents: [.parents[].sha]}'
printf '%s\n' '--- tag commit list ---'
api "repos/${repo}/commits?sha=${tag}&per_page=100" |
jq --arg sha "$sha" '[.[] | {sha, date: .commit.committer.date}] | {count: length, recorded: map(select(.sha == $sha))}'
printf '%s\n' '--- repository contents at recorded commit ---'
for path in \
src/lib/parse/LibFixedPointDecimalParse.sol \
test/src/lib/parse/LibFixedPointDecimalParse.decimalStringToFixedPoint.t.sol
do
printf '%s\n' "PATH: $path"
api "repos/${repo}/contents/${path}?ref=${sha}" |
jq '{path, sha, size}'
doneRepository: rainlanguage/rain.math.fixedpoint
Length of output: 238
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sha='dc85846152e2fd42f8f2c07feb72cc1abc708674'
tag='v0.2.0'
printf '%s\n' '--- all local tag refs ---'
git show-ref --tags --dereference || true
printf '%s\n' '--- recorded commit ---'
git show -s --format='commit=%H%nparents=%P%nauthor=%aI%ncommitter=%cI%ntitle=%s' "$sha"
printf '%s\n' '--- current commit ---'
git show -s --format='commit=%H%nparents=%P%nauthor=%aI%ncommitter=%cI%ntitle=%s' HEAD
printf '%s\n' '--- remote tag refs ---'
git ls-remote --tags origin "refs/tags/${tag}" "refs/tags/${tag}^{}" || true
printf '%s\n' '--- ancestry and distance ---'
if git cat-file -e "${sha}^{commit}" 2>/dev/null; then
printf 'recorded_is_ancestor_of_HEAD=%s\n' \
"$(git merge-base --is-ancestor "$sha" HEAD; printf '%s' "$?")"
git rev-list --count "${sha}..HEAD"
fi
printf '%s\n' '--- scoped file blob IDs at scan and HEAD ---'
for path in \
src/lib/parse/LibFixedPointDecimalParse.sol \
test/src/lib/parse/LibFixedPointDecimalParse.decimalStringToFixedPoint.t.sol
do
printf 'PATH %s\n' "$path"
printf 'scan='
git rev-parse "${sha}:${path}" 2>&1 || true
printf 'head='
git rev-parse "HEAD:${path}" 2>&1 || true
git diff --quiet "$sha" HEAD -- "$path"; printf 'unchanged=%s\n' "$?"
doneRepository: rainlanguage/rain.math.fixedpoint
Length of output: 1198
🏁 Script executed:
#!/usr/bin/env bash
set -u
sha='dc85846152e2fd42f8f2c07feb72cc1abc708674'
tag_commit='a535add9a2d76dfc0464af0177dbe53ef13ed600'
printf '%s\n' '--- tag relation ---'
printf 'tag_commit=%s\n' "$tag_commit"
printf 'recorded_commit=%s\n' "$sha"
printf 'recorded_equals_tag=%s\n' "$([ "$tag_commit" = "$sha" ] && echo true || echo false)"
printf 'commits_ahead=%s\n' "$(git rev-list --count "${tag_commit}..${sha}")"
printf '%s\n' '--- scoped file comparison ---'
for path in \
src/lib/parse/LibFixedPointDecimalParse.sol \
test/src/lib/parse/LibFixedPointDecimalParse.decimalStringToFixedPoint.t.sol
do
printf 'PATH %s\n' "$path"
printf 'scan_blob='
git rev-parse "${sha}:${path}" 2>&1 || true
printf 'head_blob='
git rev-parse "HEAD:${path}" 2>&1 || true
if git diff --quiet "$sha" HEAD -- "$path"; then
printf 'unchanged=true\n'
else
printf 'unchanged=false\n'
fi
done
printf '%s\n' '--- scoped changes after recorded commit ---'
git diff --name-status "$sha" HEAD -- \
src/lib/parse/LibFixedPointDecimalParse.sol \
test/src/lib/parse/LibFixedPointDecimalParse.decimalStringToFixedPoint.t.sol
printf '%s\n' '--- remote tag resolution ---'
git ls-remote --tags origin 'refs/tags/v0.2.0' 'refs/tags/v0.2.0^{}'Repository: rainlanguage/rain.math.fixedpoint
Length of output: 1060
Correct the mutation-scan audit record.
v0.2.0 resolves to a535add9a2d76dfc0464af0177dbe53ef13ed600. The recorded commit is two commits ahead, not zero. Both scoped files changed between the recorded commit and HEAD. Rerun the scan at the current revision and update the audit record.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@audit/mutation-test-scans.json` around lines 3 - 6, Update the mutation-scan
audit record for publishedTag v0.2.0 by rerunning the scan at the current HEAD,
replacing the stale commit and commitsAheadOfTag values with the resulting
revision and count, and refreshing the timestamp as appropriate. Preserve the
existing audit record format and scoped-file coverage.
1 participant
What
Scoped adversarial-mutation-test coverage pass over the core string→fixed-point
parser
LibFixedPointDecimalParse.decimalStringTofixedPoint. This was the onlymodule with a real coverage gap (97.5% lines / 88.9% branches; every other module
is at 100%).
Update (2026-07-04 rework note): per the human design ruling recorded in #24
— an EMPTY FRACTION IS INVALID — this PR no longer pins
"1."as a valid emptyfraction. The parser fix for #24 (branch
fix-24-trailing-decimal-revert,PR #25) is merged into this branch so the parser fix and the hardened suite land
together: the decimal-point gate in
src/lib/parse/LibFixedPointDecimalParse.solnow requires at least one digit after the point, returning
ParseDecimalInvalidString.selectorfor a bare trailing point, and the"1."acceptance pin is replaced with a revert pin on that exact selector. The two
fractional-overflow tests and the
"1x"/"10x"decimal-point-gate tests arekept as-is.
The discipline: for each behaviour, break the exact line and run the whole suite.
Mutants the existing tests already kill are credited (no new test). Only mutants
that survive the existing suite (real gaps) get a new discriminating test that
passes clean and fails under the mutation.
Group hardened
decimalStringTofixedPoint— 15 behaviours probed. The existing example + round-triptests already kill most mutants. Two genuine survivors were found and killed.
Mutation matrix (behaviour → mutation → killer)
!=→==== 0→falsetestDecimalStringToFixedPointInvalidAfterIntegercursor<end→false>=→>fracStart'0'at fracStart contributes 0 whether stripped or parsed== 1→== 0+1→+0cursor-1→cursor-0--→++cursor > fracStartgatetrue(errorSelector,0)→(0,0)testDecimalStringToFixedPointFailureFractionalPartOverflow>18→>19>18→>=18ooms = 18 - digits17 - digits<→<=scaledFracis always strictly > 0 (frac > 0 after strip,frac*10**ooms < 2^256), sovalue == preValueis impossibleif (value<preValue)→if (false)The two gaps filled
fraction with more than 77 digits overflows
uint256inside the innerunsafeDecimalStringToInt, before thedigits > 18precision-loss check isreached, and the resulting
ParseDecimalOverflowselector is returned from thefractional error branch. No existing test passed an overflowing fraction. New
test pins a 78-nine fraction →
ParseDecimalOverflow.integer with nothing after it (
"1x") must be rejected as an invalid string.Bypassing the gate makes the parse wrongly succeed as
1e18. The pre-existingcorrupt-integer tests only reached this code via inputs (
"1a1.1") whose garbageis caught by a later gate, so they did not discriminate the decimal-point gate
itself. New test pins
"1x"/"10x"→ParseDecimalInvalidString, plus"1."→
ParseDecimalInvalidString(a bare trailing point is a malformed emptyfraction per the ruling in Parser accepts a bare trailing decimal point ('1.') as a valid empty fraction — ruled invalid #24; this pin was originally acceptance →
1e18andwas replaced per the rework note).
Parser fix carried from #24 (PR #25 merged in)
Rejected now (
ParseDecimalInvalidString, selector0x3e8e62d6):"1.","0.","123.","00.","115792089237316195423570985008687907853269984665640564039457."— the whole category: any integer digits followed by a bare trailing point"1.e5"— scientific notation is unsupported here, so nothing after the bare point can rescue the empty fraction"."appendedUnchanged:
"1","1.0","0","0.0","123","123.0", and theuint256-max string still parse to the same values as before (pinned)"."and".5"still reject withParseEmptyDecimalString(empty integer part, rejected before the fraction is considered; their status is pinned, not changed)"1."pin in this suite, replaced here.LibFixedPointDecimalFormat.fixedPointToDecimalStringonly emits a.when the fraction is nonzero with at least one nonzero digit after it, so thetestStringRoundTripFuzzround-trip is unaffected.This repo has no generated pointer/deploy-constant files and nothing deployed
(pure
internallibrary), so no pin regeneration and no redeploy is required.Remaining-gaps checklist
decimalStringTofixedPointprobed.>=/>, frac-add guard</<=) — no behaviour can distinguish them, no test added.src/change is exactly the single empty-fraction guard from the Parser accepts a bare trailing decimal point ('1.') as a valid empty fraction — ruled invalid #24 fix; full suite green (83 tests, was 78);forge fmt --checkandforge lintclean.LibFixedPointDecimalScale,LibWillOverflow,LibFixedPointDecimalArithmeticOpenZeppelin,LibFixedPointDecimalFormat) are at 100% line/branch and heavily fuzzed against a slow reference implementation; not re-probed in this scoped pass (candidates for a follow-up scoped pass if desired).Triage note (not a bug, not in this PR's scope to change)
A fraction of >77 digits reports
ParseDecimalOverflowrather thanParseDecimalPrecisionLoss, because the inner-parse overflow check runs before thedigits > 18check. Both outcomes reject the (invalid) string; only the selectordiffers from a "precision-loss-first" reading. The new test pins the current
(Overflow) behaviour.
Closes #24
QA
testDecimalStringToFixedPointEmptyFraction,testDecimalStringToFixedPointEmptyFractionFuzz(uint256),testDecimalStringToFixedPointInvalidAfterInteger(carries the replaced"1."revert pin) — each fails on base (verified by deleting the empty-fraction guard fromdecimalStringTofixedPoint, i.e. restoring the pre-Parser accepts a bare trailing decimal point ('1.') as a valid empty fraction — ruled invalid #24-fix parser, and running the parse suite: exactly these 3 failed with0x00000000… != 0x3e8e62d6…— success whereParseDecimalInvalidStringis required — while both fractional-overflow tests and the pre-existing tests stayed green; guard restored, working tree byte-identical to the pushed commit, full suite re-run green 83/83)src/lib/parse/LibFixedPointDecimalParse.solempty-fraction guardif (cursor == fracStart) { return (ParseDecimalInvalidString.selector, 0); }→ deleted (re-allow the empty fraction) → killed by all three discriminating tests above; the test-suite side is the 15-behaviour matrix in this body1e18,123e18,type(uint256).max) derived from decimal semantics, never recomputed through the parser; the expected selector is the pre-existingParseDecimalInvalidStringerror named by the ruling in Parser accepts a bare trailing decimal point ('1.') as a valid empty fraction — ruled invalid #24"0.","1.","123.","00.", 60-digit integer +".","1.e5", and a fuzz over all non-overflowing integers +"."), pinned selector0x3e8e62d6, sweep found only this suite's former"1."pin (replaced). Parser accepts a bare trailing decimal point ('1.') as a valid empty fraction — ruled invalid #24's category note about sibling parsers in OTHER repos (rain.math.float, rainlang literal parsing) is outside this repo and stays with Parser accepts a bare trailing decimal point ('1.') as a valid empty fraction — ruled invalid #24's note; this PR closes theLibFixedPointDecimalParsefix it names.Test summary: merged head 83 passed / 0 failed;
forge fmt --checkandforge lintclean.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
1..Tests