fix(population): reject int64 narrowing overflow - #1333
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change validates population labels before signed ChangesPopulation label boundary validation
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: 🔵 Low · up to The change correctly rejects overflowing population labels, but it may also reject valid int64 labels supplied with extended floating-point precision; merge is reasonable with owner awareness and follow-up coverage for that input type. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Exact-head Checks RCA\n\nThe current Python 3.12 failure (job
|
Pull request was closed
Defect
_compact_population_labels()historically validated external population labels through binary64 before signed-int64admission. That creates scientific/data-integrity risk before population compaction: values outside the signed-64 domain can saturate/wrap into a valid-looking group identity, while a valid wider floating-point value such as exactnp.longdouble(INT64_MAX)can be rounded upward to2**63and rejected even though it fits the Rust-owned signed-64 contract.Because multigroup calibration identifies compact group 0 as the reference population, label narrowing is part of the model-identification boundary rather than cosmetic input validation.
Test-first / review-driven lineage
597e9ecc998ab855587cfcb35789f32aae763406: reject unsigned labels aboveINT64_MAXand floating values at/above the signed-64 boundary before narrowing; preserve exact signed-int64upper-bound labels.8f548c8ec7a7b832fb59953fe13d05a7d9619b62: prove exactnp.longdouble(INT64_MAX)remains admissible when the platform provides precision beyond binary64; make the Python-sequence boundary regression independent of NumPy's version/platform-specific promotion choice.a7e560462263e8590d6ddab6dfe68c66173b1ed8: validate floating labels in their source dtype, compare2**63in that same dtype, and require the signed-int64result to round-trip in the original floating dtype. Unsigned labels retain the explicitINT64_MAXguard.6675615230389ffc7136be904dae8df50b87ec69: require ordinary finitefloat16labels to remain valid under warnings-as-errors; constructing a2**63sentinel directly infloat16otherwise emits an overflow warning even though no finitefloat16value can approach the signed-64 boundary.84e169e034b720cfda853cb970e2ebd468697a9a: construct the dtype-specific2**63boundary only for floating dtypes whose exponent range can reach it (np.finfo(dtype).maxexp > 63). Smaller floating dtypes rely on the existing finite/integral checks and lossless signed-int64round-trip.Preserved contract
Valid sparse/non-contiguous labels and exact signed-64 identities remain compacted deterministically. Binary64
2**63, unsigned values aboveINT64_MAX, non-finite, negative, fractional, or lossy labels fail closed. Wider floating dtypes may represent valid signed-64 integers without needless binary64 narrowing; smaller floating dtypes do not emit irrelevant boundary-construction warnings.Python changes validation/marshalling only. Multigroup/multilevel likelihood, population estimation, optimizer, uncertainty, recovery, and all result-affecting psychometric/statistical arithmetic remain unchanged and Rust-owned.
Exact integration boundary
Protected base immediately before this lifecycle write:
main@0827dfa634b0a54c6a330f858801a80537ba6951.Exact source/test head immediately before this write:
84e169e034b720cfda853cb970e2ebd468697a9a, open, non-Draft, mergeable.Fresh repository-local runs for this exact head are regenerating: CI is pending; CodeQL, Security Scan, and SAST Semgrep are queued. Formal reviews remain COMMENTED-only with no independent APPROVED review. The extended-precision correctness finding is resolved; predecessor-head reviews/checks remain historical.
Require exact-current-head repository CI/security/static-analysis plus every applicable protected central
coverage-evidence,opencode-review, Python/Rust/package/fuzz, dependency/SBOM/provenance context and effective independent approval before merge. No gate weakening, review dismissal, force update, or self-approval is used.