refactor(analysis): consolidate null summary + type asymmetry boundary#32
Merged
project-navi-bot merged 3 commits intomainfrom Mar 29, 2026
Merged
Conversation
Extract _compute_distribution_stats() -> NullDistributionSummary as single shared implementation for null distribution summaries. Replaces both the inline stats in compute_null_result() and the separate _null_summary() function, eliminating duplicated logic that could silently diverge. - Add NullDistributionSummary frozen dataclass (mean, std ddof=0, min_val, max_val, nearest-rank percentiles, n) - AsymmetryNullResult.null_signed_excess_summary typed from dict[str, float] to NullDistributionSummary - PermutationNullResult keeps flat public shape; populated from shared summary computation internally - Unify CANONICAL_LABELS: single definition in analysis/types.py, imported by recurrence.py and eligibility.py - Golden-value regression tests prove exact numeric identity on pinned seed (captured pre-refactor on main at 8a24c77) Tests: 401 -> 410 (+9). Lint, format, mypy clean.
Nelson Spence (Fieldnote-Echo)
requested a review
from project-navi-bot
as a code owner
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The std computation produces platform-dependent last-digit rounding (0.9707728879609279 local vs 0.9707728879609274 CI). Use pytest.approx with rel=1e-12 tolerance — tight enough to catch real divergence, loose enough for IEEE 754 platform variance.
F-01: AsymmetryNullResult.to_dict() now flattens
NullDistributionSummary back to the legacy {mean, std, min, max,
p5, p25, ...} shape for JSON artifact compatibility. Internal type
is NullDistributionSummary; serialized shape is unchanged.
F-02: Golden regression test now asserts full summary contract
(percentiles + n), plus a to_dict() test proving the legacy flat
shape is preserved (no nested "percentiles" key, no "min_val").
project-navi-bot
approved these changes
Mar 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 1A from the debt reduction plan. Eliminates duplicated null-summary logic and fixes an untyped boundary.
_compute_distribution_stats()— single implementation replacing bothcompute_null_result()inline stats and the separate_null_summary()function. Population std (ddof=0), nearest-rank percentiles. Cannot silently diverge anymore.NullDistributionSummaryfrozen dataclass — replaces the untypeddict[str, float]inAsymmetryNullResult.null_signed_excess_summary. Fixed schema withto_dict()serialization.PermutationNullResultunchanged — flat public fields populated from shared computation internally. No report/serialization churn.CANONICAL_LABELSunified — single definition inanalysis/types.py, imported byrecurrence.pyandeligibility.py. Pilot schema's broaderVALID_LABELSstays separate.Verification
Golden values captured pre-refactor on main (
8a24c77), asserted post-refactor:compute_null_result()p-values, mean, std, min, max, percentiles — exact match across all three tail modesrun_asymmetry_null()p-values, signed excess, null summary — exact match on pinned seed/fixtureTest plan
🤖 Generated with Claude Code