mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-09-17 00:26:26 +00:00
fix(salary): require corroboration before accepting a header row (#415)
* fix(salary): require corroboration before accepting a header row Header-row detection accepted the first row (of the first 10) where any cell merely contained a company-pattern word - no check that the row actually looked like a header. A source-citation row above the real header table (standard in real Danish union/statistics exports, e.g. "Kilde: ... opdelt efter arbejdsgiver ...") tripped it purely because "arbejdsgiver" appeared in prose. The real header row then parsed as data (its "Firma" cell became a bogus company), and every genuine company silently lost all its salary data - exit 0, no warning. A candidate row is now only accepted when a second cell also matches a city/count/index pattern, and a sheet that ends up with zero detected salary columns prints a warning instead of reporting success silently. Fixes #414. * fix(salary): require cross-cell corroboration, fall back for untyped columns Two edge cases found in review of the corroboration fix: - Same-cell corroboration wasn't enough: a citation sentence can pack a count-pattern word into the same sentence as the company-pattern one ("...opdelt efter arbejdsgiver, antal svar 1234"), which still passed the gate. Corroboration must now come from a different cell. - The corroboration requirement itself broke sheets whose only real header has purely untyped salary columns (e.g. "Base pay 2025" / "Bonus 2025" - neither matches a known city/count/index pattern), so header detection found nothing at all. Falls back to the original any-cell-mentions-company rule when the strict pass finds no row in the first 10. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
OluwaJomiloju
co-authored by
Claude Sonnet 5
Claude Sonnet 5
parent
b959d6a589
commit
7f709eda57
@@ -41,6 +41,27 @@ per-file diff commands.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **`convert_salary_excel.py` no longer mistakes a title/citation row for the header row**
|
||||
(#414) - header-row detection accepted the first row in the first 10 where *any* cell merely
|
||||
contained a company-pattern word, with no check that the row actually looked like a header. A
|
||||
source-citation line above the real header table - standard in real Danish union/statistics
|
||||
exports, e.g. "Kilde: ... opdelt efter arbejdsgiver ..." - tripped it purely because
|
||||
"arbejdsgiver" (employer) appeared in prose. The real header row then got parsed as a data row
|
||||
(its "Firma" cell became a bogus company entry), and every genuine company lost all its salary
|
||||
data, silently: exit 0, "Done! Wrote N company entries," with `categories: {}` on every one. A
|
||||
candidate row is now accepted only when a *different* cell in the same row also matches a
|
||||
city/count/index pattern - same-cell corroboration doesn't count, since a citation sentence can
|
||||
pack a count-pattern word into the same sentence as the company-pattern one (e.g. "...opdelt
|
||||
efter arbejdsgiver, antal svar 1234"). Sheets whose only real header has purely untyped salary
|
||||
columns (e.g. "Base pay 2025" / "Bonus 2025", neither of which matches a known city/count/index
|
||||
pattern) have nothing to corroborate against in any row, so detection falls back to the original
|
||||
any-cell-mentions-company rule when the strict pass finds nothing in the first 10 rows. As a
|
||||
backstop independent of either pass, a sheet that ends up with zero detected salary columns now
|
||||
prints a warning instead of reporting success silently. Pinned by four cases in
|
||||
`tests/test_convert_salary_excel.py`: the original citation-row and zero-columns cases fail
|
||||
against the pre-fix script; the same-cell-corroboration and untyped-column-fallback cases each
|
||||
fail against the single-pass version of this fix that came before the fallback was added.
|
||||
|
||||
- **`jobbank-search` no longer dies over one malformed feed date** (#416) - `new Date()`
|
||||
on a present-but-unparseable `pubDate` yields an Invalid Date whose `toISOString()`
|
||||
throws `RangeError`, and `normalizeSearchItem` runs inside an unguarded `items.map()`,
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import io
|
||||
import unittest
|
||||
from contextlib import redirect_stderr
|
||||
from types import SimpleNamespace
|
||||
|
||||
from tools.convert_salary_excel import (
|
||||
@@ -286,6 +288,85 @@ class DetectColumnTypeTests(unittest.TestCase):
|
||||
self.assertEqual(categories["a"], {"count": 10, "index": 100.0})
|
||||
self.assertEqual(categories["b"], {"count": 20, "index": 200.0})
|
||||
|
||||
def test_parse_sheet_ignores_citation_row_mentioning_company_pattern_word(self):
|
||||
# A title/source-citation row above the real header - standard in
|
||||
# real Danish union/statistics exports - can contain a stray
|
||||
# company-pattern word ("arbejdsgiver" = employer) in running prose.
|
||||
# It must not be mistaken for the header: that misreads the real
|
||||
# header row as data (producing a bogus "Firma" company) and drops
|
||||
# every real company's salary data (issue #414).
|
||||
ws = FakeWorksheet([
|
||||
("Lønstatistik 2025",),
|
||||
("Kilde: Medlemsundersøgelse opdelt efter arbejdsgiver og branche",),
|
||||
(),
|
||||
("Firma", "By", "Antal alle", "Lønindeks alle"),
|
||||
("Novo Nordisk A/S", "Bagsværd", 500, 108.5),
|
||||
("Ørsted A/S", "Fredericia", 200, 105.2),
|
||||
])
|
||||
|
||||
companies = parse_sheet(ws)
|
||||
|
||||
self.assertEqual(len(companies), 2)
|
||||
self.assertEqual(companies[0]["company"], "Novo Nordisk A/S")
|
||||
self.assertEqual(companies[0]["city"], "Bagsværd")
|
||||
self.assertEqual(companies[0]["categories"]["alle"], {"count": 500, "index": 108.5})
|
||||
self.assertEqual(companies[1]["company"], "Ørsted A/S")
|
||||
|
||||
def test_parse_sheet_rejects_citation_row_with_count_word_in_same_cell(self):
|
||||
# Corroboration must come from a DIFFERENT cell than the company
|
||||
# match. A single free-text sentence can pack both a company-pattern
|
||||
# word and a count-pattern word together (e.g. "... opdelt efter
|
||||
# arbejdsgiver, antal svar 1234") - same-cell corroboration must not
|
||||
# be enough, or this citation row reintroduces the bogus-header bug.
|
||||
ws = FakeWorksheet([
|
||||
("Lønstatistik 2025",),
|
||||
("Kilde: undersøgelse opdelt efter arbejdsgiver, antal svar 1234",),
|
||||
(),
|
||||
("Firma", "By", "Antal alle", "Lønindeks alle"),
|
||||
("Novo Nordisk A/S", "Bagsværd", 500, 108.5),
|
||||
])
|
||||
|
||||
companies = parse_sheet(ws)
|
||||
|
||||
self.assertEqual(len(companies), 1)
|
||||
self.assertEqual(companies[0]["company"], "Novo Nordisk A/S")
|
||||
self.assertEqual(companies[0]["categories"]["alle"], {"count": 500, "index": 108.5})
|
||||
|
||||
def test_parse_sheet_falls_back_when_no_row_has_cross_cell_corroboration(self):
|
||||
# A header with only untyped salary columns (no header matches a
|
||||
# known city/count/index pattern - "Base pay"/"Bonus" don't) has
|
||||
# nothing to corroborate against in any row. The strict cross-cell
|
||||
# check must fall back to the original any-cell-mentions-company
|
||||
# rule rather than failing to find a header at all.
|
||||
ws = FakeWorksheet([
|
||||
("Company", "Base pay 2025", "Bonus 2025"),
|
||||
("Example Corp", 55000, 5000),
|
||||
])
|
||||
|
||||
companies = parse_sheet(ws)
|
||||
|
||||
self.assertEqual(len(companies), 1)
|
||||
self.assertEqual(companies[0]["company"], "Example Corp")
|
||||
self.assertEqual(companies[0]["categories"]["base_pay_2025"], {"index": 55000.0})
|
||||
self.assertEqual(companies[0]["categories"]["bonus_2025"], {"index": 5000.0})
|
||||
|
||||
def test_parse_sheet_warns_when_no_salary_columns_detected(self):
|
||||
# A header row with only company/city columns and no salary data
|
||||
# is a strong signal something is wrong (a misdetected header row,
|
||||
# or a sheet with no salary data at all) - it should be flagged,
|
||||
# not silently reported as a successful conversion.
|
||||
ws = FakeWorksheet([
|
||||
("Company", "City"),
|
||||
("Example Corp", "Aarhus"),
|
||||
])
|
||||
|
||||
stderr = io.StringIO()
|
||||
with redirect_stderr(stderr):
|
||||
companies = parse_sheet(ws)
|
||||
|
||||
self.assertEqual(companies[0]["categories"], {})
|
||||
self.assertIn("No salary data columns detected", stderr.getvalue())
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -127,15 +127,47 @@ def detect_column_type(header):
|
||||
|
||||
def parse_sheet(ws, sheet_label=None):
|
||||
"""Parse a single worksheet into a list of company entries and detected categories."""
|
||||
# Find header row
|
||||
# Find header row. Two passes:
|
||||
#
|
||||
# Strict pass: a candidate row needs a company-pattern cell AND a
|
||||
# DIFFERENT cell matching a city/count/index pattern. Corroboration must
|
||||
# come from a separate cell - a single free-text sentence can pack both
|
||||
# a company-pattern word and a count-pattern word together (e.g. "...
|
||||
# opdelt efter arbejdsgiver, antal svar 1234"), and that must not read
|
||||
# as a header any more than a citation mentioning just one of them does.
|
||||
# A real header row always has these as separate columns.
|
||||
#
|
||||
# Fallback pass: some real headers have no recognizable city/count/index
|
||||
# column at all (e.g. "Company | Base pay 2025 | Bonus 2025" - neither
|
||||
# data header matches a known pattern, so they're picked up later as
|
||||
# untyped/standalone categories). Nothing can corroborate a company match
|
||||
# there, so if the strict pass finds no row in the first 10, fall back to
|
||||
# the original any-cell-mentions-company rule.
|
||||
rows = list(ws.iter_rows(min_row=1, max_row=10, values_only=False))
|
||||
|
||||
def _cell_texts(row):
|
||||
return [str(cell.value).strip() for cell in row if cell.value]
|
||||
|
||||
header_row = None
|
||||
for row_idx, row in enumerate(ws.iter_rows(min_row=1, max_row=10, values_only=False), start=1):
|
||||
for cell in row:
|
||||
if cell.value and header_matches(str(cell.value), COMPANY_PATTERNS):
|
||||
for row_idx, row in enumerate(rows, start=1):
|
||||
cell_texts = _cell_texts(row)
|
||||
company_idxs = {i for i, t in enumerate(cell_texts) if header_matches(t, COMPANY_PATTERNS)}
|
||||
if not company_idxs:
|
||||
continue
|
||||
other_idxs = {
|
||||
i
|
||||
for i, t in enumerate(cell_texts)
|
||||
if header_matches(t, CITY_PATTERNS) or header_matches(t, COUNT_PATTERNS) or header_matches(t, INDEX_PATTERNS)
|
||||
}
|
||||
if other_idxs - company_idxs:
|
||||
header_row = row_idx
|
||||
break
|
||||
|
||||
if header_row is None:
|
||||
for row_idx, row in enumerate(rows, start=1):
|
||||
if any(header_matches(t, COMPANY_PATTERNS) for t in _cell_texts(row)):
|
||||
header_row = row_idx
|
||||
break
|
||||
if header_row:
|
||||
break
|
||||
|
||||
if header_row is None:
|
||||
print(f"Warning: Could not find header row in sheet '{ws.title}'. Skipping.", file=sys.stderr)
|
||||
@@ -222,6 +254,14 @@ def parse_sheet(ws, sheet_label=None):
|
||||
for col_idx, col_header in untyped_cols:
|
||||
categories.append({"name": col_header.lower().replace(" ", "_"), "value_col": col_idx})
|
||||
|
||||
if not categories:
|
||||
print(
|
||||
f"Warning: No salary data columns detected in sheet '{ws.title}' "
|
||||
"(only a company/city column was found) - the header row may be "
|
||||
"wrong, or this sheet has no salary data.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
|
||||
# Parse data rows
|
||||
companies = []
|
||||
for row in ws.iter_rows(min_row=header_row + 1, values_only=True):
|
||||
|
||||
Reference in New Issue
Block a user