From 0b22e55e8f2bf7ea508955bb6fe4b01fcfb080a9 Mon Sep 17 00:00:00 2001
From: Rudy Mizrahi Celekli <47457359+rudycelekli@users.noreply.github.com>
Date: Sun, 18 Oct 2026 01:05:10 -0400
Subject: [PATCH] fix(salary): validate metadata before rendering lookups
 (#575)

* fix(salary): validate metadata before rendering lookups

* docs(changelog): record fix for #575
---
 CHANGELOG.md                      |  2 ++
 salary_lookup.py                  | 11 ++++++
 tests/test_salary_metadata_cli.py | 60 +++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+)
 create mode 100644 tests/test_salary_metadata_cli.py

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7614ffe1..ed322096 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
diff --git a/salary_lookup.py b/salary_lookup.py
index 50403700..270bc6d8 100644
--- a/salary_lookup.py
+++ b/salary_lookup.py
@@ -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):
diff --git a/tests/test_salary_metadata_cli.py b/tests/test_salary_metadata_cli.py
new file mode 100644
index 00000000..82cf031c
--- /dev/null
+++ b/tests/test_salary_metadata_cli.py
@@ -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)