fix(salary): validate metadata before rendering lookups (#575)

* fix(salary): validate metadata before rendering lookups

* docs(changelog): record fix for #575
This commit is contained in:
Rudy Mizrahi Celekli authored and GitHub committed 2026-10-18 07:05:10 +02:00
1 parent 060f2dbe78
commit 0b22e55e8f
3 files changed
+73

No files matched your search

+2
View File
@@ -269,6 +269,8 @@ per-file diff commands.
comma-separated value list containing only whitespace now returns `BAD_ARG` and
exits 1 instead of silently dropping the filter and running a broader search.
- **Salary lookups validate metadata before rendering** (#575) - malformed metadata produces an actionable error instead of a rendering traceback.
- **The `/apply` host-check test derives its installed-portal hosts from the skills tree
instead of a hand-copied set** (#497, `tests/test_apply_host_check.py`) - `SHIPPED_PORTAL_HOSTS`
was six names typed into the test and nothing tied it to anything: a portal added by
+11
View File
@@ -18,6 +18,7 @@ Usage:
"""
import json
import math
import sys
import re
import argparse
@@ -69,6 +70,16 @@ def collect_validation_issues(data):
metadata = data.get("metadata", {})
if metadata is not None and not isinstance(metadata, dict):
errors.append("'metadata' must be an object when provided")
elif isinstance(metadata, dict):
baseline = metadata.get("index_baseline", 100)
try:
finite_baseline = math.isfinite(baseline)
except (TypeError, OverflowError):
finite_baseline = False
if isinstance(baseline, bool) or not finite_baseline:
errors.append("'metadata.index_baseline' must be a finite number")
if not isinstance(metadata.get("index_label", "Index"), str):
errors.append("'metadata.index_label' must be a string")
companies = data.get("companies")
if not isinstance(companies, list):
+60
View File
@@ -0,0 +1,60 @@
"""The documented pre-flight must reject metadata that breaks table lookups."""
import json
import shutil
import subprocess
import sys
import unittest
from pathlib import Path
from tempfile import TemporaryDirectory
TOOL = Path(__file__).resolve().parent.parent / "salary_lookup.py"
class SalaryMetadataCLI(unittest.TestCase):
def run_lookup(self, metadata, *args):
with TemporaryDirectory() as directory:
root = Path(directory)
shutil.copyfile(TOOL, root / TOOL.name)
(root / "salary_data.json").write_text(json.dumps({
"metadata": metadata,
"companies": [{"company": "Acme", "categories": {"all_employees": {"index": 165}}}],
}), encoding="utf-8")
return subprocess.run(
[sys.executable, str(root / TOOL.name), *args],
capture_output=True, text=True, encoding="utf-8",
)
def test_preflight_rejects_a_text_baseline(self):
result = self.run_lookup({"index_baseline": "150"}, "--validate")
self.assertEqual(result.returncode, 1, result.stdout + result.stderr)
self.assertIn("index_baseline", result.stdout + result.stderr)
def test_lookup_rejects_a_text_baseline_without_a_traceback(self):
result = self.run_lookup({"index_baseline": "150"}, "Acme")
self.assertEqual(result.returncode, 1)
self.assertIn("index_baseline", result.stderr)
self.assertNotIn("Traceback", result.stderr)
def test_preflight_rejects_a_non_text_index_label(self):
result = self.run_lookup({"index_label": ["Salary"]}, "--validate")
self.assertEqual(result.returncode, 1, result.stdout + result.stderr)
self.assertIn("index_label", result.stdout + result.stderr)
def test_preflight_rejects_unusable_numeric_baselines(self):
for baseline in [True, None, float("inf"), float("nan"), 10 ** 400]:
with self.subTest(baseline=baseline):
result = self.run_lookup({"index_baseline": baseline}, "--validate")
self.assertEqual(result.returncode, 1, result.stdout + result.stderr)
self.assertIn("index_baseline", result.stdout + result.stderr)
self.assertNotIn("Traceback", result.stderr)
def test_numeric_baseline_and_optional_metadata_controls(self):
for metadata, percentage in [({"index_baseline": 150, "index_label": "Salary"}, "+10.0%"),
({"index_baseline": 0}, None), ({}, "+65.0%"), (None, "+65.0%")]:
with self.subTest(metadata=metadata):
result = self.run_lookup(metadata, "Acme")
self.assertEqual(result.returncode, 0, result.stderr)
if percentage:
self.assertIn(percentage, result.stdout)
else:
self.assertNotIn("%", result.stdout)