From 979b4047dc4ca7543168dafd896712dea4a579d5 Mon Sep 17 00:00:00 2001 From: Dennis Thiessen Date: Wed, 22 Jul 2026 20:41:18 +0200 Subject: [PATCH] =?UTF-8?q?fix(fundamentals):=20pure-core=20review=20?= =?UTF-8?q?=E2=80=94=20tie-aware=20percentile,=20null-safe=20reads?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Peer percentile is now a tie-aware rank against the OTHER issuers ((worse + 0.5*tied)/(peers-1)): an all-equal group maps to 50 (not 100), the median maps to 50, a unique best to 100, a unique worst to 0. 2. Deterministic reads use the consecutive non-null suffix ending at the latest point (>=3 values): a null latest or an internal gap yields no read, so a read never reflects a period displayed as n/a. 3. Peer filtering excludes non-finite (NaN/±inf) as well as null, including an invalid subject. Tests updated + added (all-equal, median rank, non-finite, latest-null history, internal gap). 15 passed. Co-Authored-By: Claude Opus 4.8 --- app/services/fundamentals_peers.py | 36 +++++++++++++++++++++------ app/services/fundamentals_reads.py | 23 ++++++++++++----- tests/unit/test_fundamentals_peers.py | 26 ++++++++++++------- tests/unit/test_fundamentals_reads.py | 10 ++++++++ 4 files changed, 73 insertions(+), 22 deletions(-) diff --git a/app/services/fundamentals_peers.py b/app/services/fundamentals_peers.py index c30c970..0695422 100644 --- a/app/services/fundamentals_peers.py +++ b/app/services/fundamentals_peers.py @@ -14,11 +14,18 @@ a peer percentile** — leverage is compared via net_debt_to_ebitda. from __future__ import annotations +import math import statistics from dataclasses import dataclass +from typing import Any MIN_PEERS = 5 + +def _finite(v: Any) -> bool: + """True for a finite number — excludes None, bool, NaN, ±inf (plan: null/invalid).""" + return isinstance(v, (int, float)) and not isinstance(v, bool) and math.isfinite(v) + # Metric -> is a higher value more favorable? (Peer-eligible metrics only; # absolute net_debt is intentionally absent — size-dependent.) HIGHER_IS_BETTER: dict[str, bool] = { @@ -50,18 +57,33 @@ def peer_stat( """Median + favorable percentile for ``subject`` within its group. ``group_values`` is every issuer's value for the metric (including the - subject), CIK-deduplicated by the caller. Nulls are excluded. Returns None - when fewer than ``min_peers`` valid values exist, or the subject is null. + subject), CIK-deduplicated by the caller. Null/invalid (non-finite) values are + excluded. Returns None when fewer than ``min_peers`` valid values exist, or + the subject is null/invalid. + + The percentile is a **tie-aware rank against the other issuers** — + ``(worse + 0.5·tied) / (peers − 1)`` — so a whole group of equal values maps + to 50, not 100, and the median maps to 50. """ - valid = [v for v in group_values if v is not None] - if subject is None or len(valid) < min_peers: + valid = [v for v in group_values if _finite(v)] + if not _finite(subject) or len(valid) < min_peers: return None median = statistics.median(valid) + + others = valid.copy() + try: + others.remove(subject) # rank the subject against the OTHER issuers + except ValueError: + pass + denom = len(others) + if denom == 0: + return None if higher_is_better: - favorable = sum(1 for v in valid if v <= subject) + worse = sum(1 for v in others if v < subject) else: - favorable = sum(1 for v in valid if v >= subject) - percentile = round(favorable / len(valid) * 100) + worse = sum(1 for v in others if v > subject) + tied = sum(1 for v in others if v == subject) + percentile = round((worse + 0.5 * tied) / denom * 100) return PeerStat(median=median, favorable_percentile=percentile, peer_count=len(valid)) diff --git a/app/services/fundamentals_reads.py b/app/services/fundamentals_reads.py index 60d80bb..4bb8d18 100644 --- a/app/services/fundamentals_reads.py +++ b/app/services/fundamentals_reads.py @@ -22,13 +22,23 @@ PEER_FAVORABLE = 60 PEER_ADVERSE = 40 -def _history_values(history: list[Any]) -> list[float]: - return [p.value for p in history if p.value is not None] +def _latest_run(history: list[Any]) -> list[float]: + """The consecutive non-null values ending at the latest point (oldest->newest). + A null latest, or an internal gap, truncates the run — so a read never reflects + a period whose displayed value is n/a.""" + run: list[float] = [] + for p in reversed(history): + if p.value is None: + break + run.append(p.value) + run.reverse() + return run def growth_read(history: list[Any]) -> str | None: - """Change in a YoY-growth series: latest − prior. Needs >= 3 periods.""" - vals = _history_values(history) + """Change in a YoY-growth series: latest − prior. Needs >= 3 consecutive + non-null values ending at the latest point.""" + vals = _latest_run(history) if len(vals) < MIN_PERIODS: return None delta = vals[-1] - vals[-2] @@ -40,8 +50,9 @@ def growth_read(history: list[Any]) -> str | None: def margin_read(history: list[Any]) -> str | None: - """Latest margin vs the mean of prior periods (pp). Needs >= 3 periods.""" - vals = _history_values(history) + """Latest margin vs the mean of prior periods (pp). Needs >= 3 consecutive + non-null values ending at the latest point.""" + vals = _latest_run(history) if len(vals) < MIN_PERIODS: return None delta = vals[-1] - mean(vals[:-1]) diff --git a/tests/unit/test_fundamentals_peers.py b/tests/unit/test_fundamentals_peers.py index 6702d02..e944efa 100644 --- a/tests/unit/test_fundamentals_peers.py +++ b/tests/unit/test_fundamentals_peers.py @@ -2,24 +2,30 @@ from __future__ import annotations +import math + from app.services import fundamentals_peers as pr -def test_peer_stat_higher_is_better_percentile_and_median(): +def test_median_ranks_at_50_tie_aware(): s = pr.peer_stat(3, [1, 2, 3, 4, 5], higher_is_better=True) assert s.median == 3 - assert s.favorable_percentile == 60 # beats/ties 3 of 5 + assert s.favorable_percentile == 50 # tie-aware rank of the median assert s.peer_count == 5 +def test_all_equal_peers_rank_at_50(): + s = pr.peer_stat(3, [3, 3, 3, 3, 3], higher_is_better=True) + assert s.favorable_percentile == 50 # not 100 — ties don't get full credit + + def test_peer_stat_lower_is_better_flips_direction(): - s = pr.peer_stat(3, [1, 2, 3, 4, 5], higher_is_better=False) - assert s.favorable_percentile == 60 # 3 of 5 are >= 3 + assert pr.peer_stat(3, [1, 2, 3, 4, 5], higher_is_better=False).favorable_percentile == 50 -def test_peer_stat_top_and_bottom(): +def test_peer_stat_unique_top_and_bottom(): assert pr.peer_stat(5, [1, 2, 3, 4, 5], higher_is_better=True).favorable_percentile == 100 - assert pr.peer_stat(1, [1, 2, 3, 4, 5], higher_is_better=True).favorable_percentile == 20 + assert pr.peer_stat(1, [1, 2, 3, 4, 5], higher_is_better=True).favorable_percentile == 0 def test_peer_stat_requires_min_valid_peers(): @@ -27,9 +33,11 @@ def test_peer_stat_requires_min_valid_peers(): assert pr.peer_stat(None, [1, 2, 3, 4, 5], higher_is_better=True) is None # null subject -def test_peer_stat_excludes_nulls_from_group(): - s = pr.peer_stat(3, [1, 2, 3, 4, 5, None, None], higher_is_better=True) - assert s.peer_count == 5 # nulls dropped +def test_peer_stat_excludes_null_and_non_finite(): + s = pr.peer_stat(3, [1, 2, 3, 4, 5, None, math.nan, math.inf, -math.inf], higher_is_better=True) + assert s.peer_count == 5 # nulls + NaN/inf dropped + # a non-finite subject is invalid + assert pr.peer_stat(math.nan, [1, 2, 3, 4, 5], higher_is_better=True) is None def test_net_debt_is_not_peer_eligible(): diff --git a/tests/unit/test_fundamentals_reads.py b/tests/unit/test_fundamentals_reads.py index 23da0e5..ccb820c 100644 --- a/tests/unit/test_fundamentals_reads.py +++ b/tests/unit/test_fundamentals_reads.py @@ -18,6 +18,16 @@ def test_growth_read_boundaries(): assert rd.growth_read(_hist(5, 6)) is None # < 3 periods +def test_reads_use_latest_nonnull_suffix(): + # latest displayed value is n/a -> no read (never reflect a null latest) + assert rd.growth_read(_hist(5, 6, 8, None)) is None + assert rd.margin_read(_hist(19, 20, 22, None)) is None + # an internal gap truncates the run -> fewer than 3 consecutive -> no read + assert rd.growth_read(_hist(5, 6, None, 8)) is None + # a clean 3-run after an older gap still reads + assert rd.growth_read(_hist(None, 5, 6, 8)) == "accelerating" + + def test_margin_read_vs_mean_of_prior(): # prior mean = (19+20)/2 = 19.5; latest 21 -> +1.5 -> improving assert rd.margin_read(_hist(19, 20, 21)) == "improving"