mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-09-17 00:26:26 +00:00
fix(salary): treat null metadata/categories as absent instead of crashing (#413)
--validate treats an explicit "metadata": null / "categories": null the same
as an omitted key ("...must be an object when provided", None is skipped), but
format_entry read both through dict.get(key, {}), which only substitutes the
default for an *absent* key - a present-but-null value passed through. The
renderer then hit None.get("index_label", ...) (AttributeError) or, via the
numeric-field fallback, None[key] = value (TypeError), so a hand-maintained
salary_data.json using null for "no value" died with an uncaught traceback
right after printing "Found 1 match(es)".
format_entry now coerces both to {} up front, honouring the validator's
existing "when provided" contract at the single consumer that broke it.
Tests (all verified to fail on the unfixed renderer):
- two unit cases calling format_entry with null metadata / null categories
- two end-to-end cases running main() --validate (blesses the file) then the
lookup path (renders it), one per null shape
Plus an [Unreleased] CHANGELOG entry.
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
6ef295bf7b
commit
9833a5dcb7
@@ -61,6 +61,19 @@ per-file diff commands.
|
||||
Pinned by three new cases in `test_scrape_contract.py`, each verified to fail on the unfixed
|
||||
spec. Reported and diagnosed from a real run by @sandunwijerathne.
|
||||
|
||||
- **`salary_lookup.py` no longer crashes on a `null` `metadata` or `categories`** - `--validate`
|
||||
treats an explicit `"metadata": null` / `"categories": null` the same as an omitted key (the
|
||||
shape checks are "...must be an object *when provided*" and skip `None`), but the renderer read
|
||||
both through `dict.get(key, {})`, which only substitutes the default for an *absent* key - a
|
||||
present-but-null value passed straight through. `format_entry` then hit `None.get("index_label",
|
||||
...)` (`AttributeError`) or, via the numeric-field fallback, `None[key] = value` (`TypeError`),
|
||||
so a hand-maintained `salary_data.json` using `null` for "no value here" died with an uncaught
|
||||
traceback right after printing `Found 1 match(es)`. `format_entry` now coerces both to `{}` up
|
||||
front, so `null`, absent, and `{}` behave identically. Pinned by four cases in
|
||||
`test_salary_lookup.py` - two unit calls into `format_entry` and two end-to-end (`main()
|
||||
--validate` blesses the file, then the lookup path renders it), one per null shape, all verified
|
||||
to fail on the unfixed renderer.
|
||||
|
||||
## [1.7.0] - 2026-08-29
|
||||
|
||||
### Fixed
|
||||
|
||||
+7
-2
@@ -291,6 +291,11 @@ def search_company(data, query, city=None):
|
||||
|
||||
def format_entry(entry, metadata):
|
||||
"""Format a single company entry for display."""
|
||||
# `metadata` and `entry["categories"]` may be an explicit null: --validate
|
||||
# treats a null the same as an omitted key ("...must be an object when
|
||||
# provided"), but dict.get(key, default) only substitutes the default for an
|
||||
# absent key, so a null reached `.get()`/`[]` here and crashed the lookup.
|
||||
metadata = metadata or {}
|
||||
lines = []
|
||||
lines.append(f"\n{'='*60}")
|
||||
lines.append(f" {entry['company']}")
|
||||
@@ -298,8 +303,8 @@ def format_entry(entry, metadata):
|
||||
lines.append(f" Location: {entry['city']}")
|
||||
lines.append(f"{'='*60}")
|
||||
|
||||
# Get category data (everything except company/city fields)
|
||||
categories = entry.get("categories", {})
|
||||
# Get category data (everything except company/city fields).
|
||||
categories = entry.get("categories") or {}
|
||||
if not categories:
|
||||
# Fallback: treat any numeric fields as categories
|
||||
skip_keys = {"company", "city", "categories"}
|
||||
|
||||
@@ -87,6 +87,34 @@ class FormatEntryTests(unittest.TestCase):
|
||||
self.assertIn("45000.0", rendered)
|
||||
self.assertIn("+12.5%", rendered)
|
||||
|
||||
def test_null_categories_with_sibling_dict_does_not_crash(self):
|
||||
# --validate accepts "categories": null, so format_entry must not crash
|
||||
# on it. entry.get("categories", {}) returns None (not {}) for an
|
||||
# explicit null, and the numeric-field fallback then did None[key] = ....
|
||||
entry = {
|
||||
"company": "Example Corp",
|
||||
"city": "",
|
||||
"categories": None,
|
||||
"engineering": {"count": 10, "index": 105.0},
|
||||
}
|
||||
|
||||
rendered = format_entry(entry, {"index_baseline": 100, "index_label": "Index"})
|
||||
|
||||
self.assertRegex(rendered, r"Engineering\s+10\s+105\.0")
|
||||
|
||||
def test_null_metadata_does_not_crash(self):
|
||||
# --validate accepts "metadata": null the same way; format_entry then did
|
||||
# None.get("index_label", ...) -> AttributeError.
|
||||
entry = {
|
||||
"company": "Example Corp",
|
||||
"city": "",
|
||||
"categories": {"eng": {"count": 5, "index": 108.0}},
|
||||
}
|
||||
|
||||
rendered = format_entry(entry, None)
|
||||
|
||||
self.assertRegex(rendered, r"Eng\s+5\s+108\.0")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# match_score tests (from #106)
|
||||
@@ -331,6 +359,70 @@ class ValidateFlagTests(unittest.TestCase):
|
||||
self.assertIn("Duplicate company name", out)
|
||||
|
||||
|
||||
class NullShapeEndToEndTests(unittest.TestCase):
|
||||
"""The disagreement in full: --validate blesses a file with a null
|
||||
metadata/categories, then the lookup path must render it, not crash.
|
||||
|
||||
Both payloads pass --validate on master; the second command then dies
|
||||
(TypeError in the categories fallback, AttributeError on metadata.get).
|
||||
"""
|
||||
|
||||
def _run_main(self, payload, *argv_tail):
|
||||
"""Run main() against `payload` with the given argv. Returns
|
||||
(exit_code_or_None, stdout). main() returns normally on a successful
|
||||
render, so a missing SystemExit is success, not an error."""
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
data_file = Path(tmpdir) / "salary_data.json"
|
||||
data_file.write_text(payload, encoding="utf-8")
|
||||
original_data_file = salary_lookup.DATA_FILE
|
||||
salary_lookup.DATA_FILE = data_file
|
||||
argv_patch = mock.patch("sys.argv", ["salary_lookup.py", *argv_tail])
|
||||
argv_patch.start()
|
||||
try:
|
||||
stdout = io.StringIO()
|
||||
try:
|
||||
with redirect_stdout(stdout):
|
||||
salary_lookup.main()
|
||||
return None, stdout.getvalue()
|
||||
except SystemExit as exc:
|
||||
return exc.code, stdout.getvalue()
|
||||
finally:
|
||||
argv_patch.stop()
|
||||
salary_lookup.DATA_FILE = original_data_file
|
||||
|
||||
def test_null_categories_passes_validate_then_renders(self):
|
||||
payload = (
|
||||
'{"metadata": {"index_label": "Index", "index_baseline": 100},'
|
||||
' "companies": [{"company": "Foo A/S", "city": "Aarhus",'
|
||||
' "categories": null,'
|
||||
' "engineering": {"count": 10, "index": 105}}]}'
|
||||
)
|
||||
|
||||
code, out = self._run_main(payload, "--validate")
|
||||
self.assertEqual(code, 0)
|
||||
self.assertIn("OK", out)
|
||||
|
||||
code, out = self._run_main(payload, "Foo")
|
||||
self.assertIsNone(code)
|
||||
self.assertIn("Foo A/S", out)
|
||||
self.assertRegex(out, r"Engineering\s+10\s+105")
|
||||
|
||||
def test_null_metadata_passes_validate_then_renders(self):
|
||||
payload = (
|
||||
'{"metadata": null,'
|
||||
' "companies": [{"company": "Foo A/S", "city": "Aarhus",'
|
||||
' "categories": {"engineering": {"count": 10, "index": 105}}}]}'
|
||||
)
|
||||
|
||||
code, out = self._run_main(payload, "--validate")
|
||||
self.assertEqual(code, 0)
|
||||
self.assertIn("OK", out)
|
||||
|
||||
code, out = self._run_main(payload, "Foo")
|
||||
self.assertIsNone(code)
|
||||
self.assertRegex(out, r"Engineering\s+10\s+105")
|
||||
|
||||
|
||||
class UtilityTests(unittest.TestCase):
|
||||
def test_normalize_strips_suffix_and_noise(self):
|
||||
self.assertEqual(normalize("Novo Nordisk A/S"), "novonordisk")
|
||||
|
||||
Reference in New Issue
Block a user