Repository navigation
fix: bound exact numeric literals before conversion - #288
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review Consolidated update in 88f48f3 addresses all seven verified Sonar findings: split bounded Decimal-reference validation to reduce cognitive complexity and separate test assertions/exception setup. The full suite was repeated after these changes: 1,971 passed, 95.27% combined coverage. Lint, format, pyright/ty, Pylint, docstrings, structural checks, strict docs, builds and diff checks pass. The full main-to-HEAD diff was reviewed again with OpenCodeReview 1.12.13 local delegation and applicable rules; all 10 files covered, including the four manual exclusions. Please review the current head. |
|
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |



Problem and result
Compact Decimal inputs could bypass declared fractional precision through Decimal-context normalization (for example
1e-999999999), then amplify fixed-point serialization or rational conversion. Numeric JSON literals could also underflow through binary64 before exact validation.This patch rejects out-of-contract nonzero quantities before expansion, independently of the caller’s Decimal precision/traps. It covers domain quantities, signed payoffs, exact moments, probabilities and application budget overrides. Versioned
from_jsonreaders preserve Decimal literals until typed validation; explicit binary64 fields retain their semantics. Independent benchmark rational references now have documented resource ceilings before conversion and after parsing.Refs #251, #255, #261. Does not close the corpus research issue.
Architecture and compatibility
The shared typed contracts enforce effective fractional places without normalization or powers. A local validation context protects Pydantic constraints without mutating caller state. Twelve-place inputs and 48-place outputs keep their existing limits; valid negative signed results and genuine zero remain valid. No transport dependency is added.
Canonical v1 decimal strings, identities, digests and mathematical golden fixtures retain their meaning. The legacy domain-record JSON decoder keeps binary64 behavior. No schema migration or automatic artifact rewrite occurs. Previously accepted overprecision/oversized references now reject. Already serialized zero cannot reveal prior underflow: reproduce from original inputs if their encoding is uncertain.
Usage
Use documented artifact
from_jsonreaders and decimal strings for exact JSON quantities. Generic Pydantic JSON helpers are not the canonical duplicate-key/precision-preserving reader.Verification
pytest -n4 --cov --cov-branchwith Syft available.make lint format-check typecheck interrogate cost-structural docs build,pylint -E coleman,git diff --check main...HEAD: passed. The full suite and all quality checks were repeated after the consolidated Sonar corrections (reference-parser complexity and regression assertion clarity).--from main --to HEADpreview and applicable rules inspected. 10/10 files reviewed: six selected Python files plus all four excluded tests/Markdown/generated-reference files manually reviewed. No model/review endpoint used; no confirmed critical/high/medium findings remain.Limits and deferred work
Rational reference ceilings: 4096 text characters, 512 coefficient digits/absolute Decimal exponent, and 4096 numerator/denominator bits. This bounds parsing resources, not scientific validity. Public RTPTorrent source assessment and temporal-evidence gaps remain separate work under #261. Reviewed en-US release notes are included for automatic 4.4.1 patch publication; version bump remains owned by the release workflow.