From 2e8600d69dfc65b51e5a1b1ad026f831296f5a67 Mon Sep 17 00:00:00 2001 From: Ayobami Adegoke Date: Mon, 21 Sep 2026 07:26:43 +0100 Subject: [PATCH] fix(rank): reject invalid score dimensions before saving results (#488) --- CHANGELOG.md | 7 +++++++ tests/test_rank_state.py | 34 ++++++++++++++++++++++++++++++++++ tools/rank_state.py | 4 +++- 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72ce6e8e..5ecee0e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,13 @@ per-file diff commands. ### Fixed +- **`/rank` rejects invalid score dimensions before updating an entry** + (`tools/rank_state.py`, `tests/test_rank_state.py`) - enforce the rubric's + inclusive 0-100 range and reject booleans, NaN, and infinities. Invalid results + now use the existing per-job error report, leaving the rejected entry intact + while valid results in the same batch are saved. CLI tests cover every score + dimension, oversized integers, boundary values, and fractional-score rounding. + - **Distinct non-Latin company names no longer share the unknown-company job key** (`tools/job_key.py`, `tests/test_job_key.py`) - when a non-empty company name has no ASCII slug, derive its fallback from a hash of the normalized name. diff --git a/tests/test_rank_state.py b/tests/test_rank_state.py index b5de98c2..9ccc8606 100644 --- a/tests/test_rank_state.py +++ b/tests/test_rank_state.py @@ -431,6 +431,40 @@ class Apply(RankStateCase): ) self.assertEqual(self.read_state()["a"]["status"], "new") + def test_invalid_scores_report_errors_without_changing_the_entry(self): + dimensions = ("technical", "experience", "behavioral", "career") + invalid = (-1, 101, True, False, float("nan"), float("inf"), -float("inf"), 10**400) + for dimension in dimensions: + for value in invalid: + with self.subTest(dimension=dimension, value=value): + original = entry(status="ranked", rank_score=70, strengths=["keep"]) + self.write_state({"a": original, "b": entry()}) + scores = dict.fromkeys(dimensions, 50) + scores[dimension] = value + out = self.run_tool( + "apply", "--results", self.results([ + {"key": "a", "status": "scored", "scores": scores}, + {"key": "b", "status": "scored", "scores": dict.fromkeys(dimensions, 80)}, + ]), expect=1, + ) + self.assertEqual(len(out["errors"]), 1) + self.assertEqual(out["errors"][0]["key"], "a") + self.assertIn(dimension, out["errors"][0]["error"]) + self.assertEqual(self.read_state()["a"], original) + self.assertEqual([row["key"] for row in out["ranked"]], ["b"]) + self.assertEqual(self.read_state()["b"]["rank_score"], 80) + + def test_score_boundaries_and_fractional_scores_remain_valid(self): + for value, expected in ((0, 0), (100, 100), (72.5, 73)): + with self.subTest(value=value): + self.write_state({"a": entry()}) + out = self.run_tool("apply", "--results", self.results([ + {"key": "a", "status": "scored", "scores": dict.fromkeys( + ("technical", "experience", "behavioral", "career"), value)}, + ])) + self.assertEqual(out["errors"], []) + self.assertEqual(self.read_state()["a"]["rank_score"], expected) + def test_re_scoring_an_already_ranked_job_is_idempotent(self): """Re-running /rank never re-scores an already-ranked job unless --all says so (Step 4), but if it does score one again, apply must produce diff --git a/tools/rank_state.py b/tools/rank_state.py index 86633232..a10f9c07 100644 --- a/tools/rank_state.py +++ b/tools/rank_state.py @@ -234,8 +234,10 @@ def overall_score(scores: dict) -> int: total = 0.0 for dim, weight in WEIGHTS.items(): value = scores.get(dim) - if not isinstance(value, (int, float)): + if isinstance(value, bool) or not isinstance(value, (int, float)): raise ValueError(f"missing or non-numeric score '{dim}'") + if not 0 <= value <= 100: + raise ValueError(f"score '{dim}' must be finite and between 0 and 100") total += float(value) * weight return int(total + 0.5)