fix(salary): never take a qualified salary header as the company column (#700)

* fix(salary): never take a qualified salary header as the company column

The detection loop kept the LAST header matching a company pattern, so
"Company Name | City | Company Count | Company Index" made the index
column the company column and every converted row carried
"company": "108.5"; IDA's "Firmanavn og by | Antal | Firmaets indeks"
(#615) did the same. #571's exact-match preference only helped when a
header was exactly "Company" or "Firma".

Detection is now three tiers, each taking the first match: an exact
pattern header, then a pattern header that is not a count/index column,
then any pattern header; the city column can never alias the company
column. Four new cases; the two qualified-header ones fail on master.

* changelog: place the #700 entry under [Unreleased]

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Mads Lorentzen <madslorentzen_17@hotmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
authored and GitHub committed 2026-10-21 07:12:41 +02:00
1 parent 217206f9db
commit 58a4ec0468
3 files changed
+109 -13

No files matched your search

+13
View File
@@ -354,6 +354,19 @@ per-file diff commands.
present on the machine (bash, zsh, sh, dash), with a missing-folder case and a guard that no present on the machine (bash, zsh, sh, dash), with a missing-folder case and a guard that no
bashism returns. The zsh subtests fail on master. bashism returns. The zsh subtests fail on master.
- **`convert_salary_excel.py` no longer takes a qualified salary header as the company
column** (#700, `tools/convert_salary_excel.py`, `tests/test_convert_salary_excel.py`) - the
detection loop kept the *last* header matching a company pattern, so
`Company Name | City | Company Count | Company Index` made the index column the company
column and every converted row carried `"company": "108.5"`; a Danish header in the same
shape (`Firmanavn og by | Antal | Firmaets indeks`, constructed - the IDA workbook heads its
pairs `Antal` and `Indeks` and already converts) did the same. #571's exact-match preference
only helped when a header was exactly `Company` or `Firma`. Detection is now three tiers,
each taking the first match: an exact pattern header, then a pattern header that is not a
count/index column, then any pattern header; the city lookup skips the company column, so a
combined name-and-city header before a real city column no longer drops the city. Five new
cases; the two qualified-header ones fail on master.
### Security ### Security
- **CI no longer leaves the job token in `.git/config` or expands a PR-controlled name into a - **CI no longer leaves the job token in `.git/config` or expands a PR-controlled name into a
+75
View File
@@ -542,6 +542,81 @@ class BareCountIndexPairingTests(unittest.TestCase):
self.assertNotRegex(rendered, r"^\s*Count\s+500", ) self.assertNotRegex(rendered, r"^\s*Count\s+500", )
class QualifiedCompanyHeaderNextToQualifiedSalaryHeader(unittest.TestCase):
"""A name header must win over a salary header that also carries the word.
The detection loop kept the LAST header matching a company pattern, so
"Company Name | City | Company Count | Company Index" made the index
column the company column and every row's company read "108.5". #571
added an exact-match preference, which only helps when a header is
exactly "Company"; the qualified name header stayed exposed. The Danish
case below is a constructed header in the same shape, not the IDA
workbook, whose pairs are headed "Antal" and "Indeks" and already convert.
"""
def test_company_name_wins_over_company_index(self):
ws = FakeWorksheet([
("Company Name", "City", "Company Count", "Company Index"),
("Acme Corp", "Berlin", 25, 108.5),
])
entry = parse_sheet(ws)[0]
self.assertEqual(entry["company"], "Acme Corp")
self.assertEqual(entry["city"], "Berlin")
self.assertEqual(entry["categories"], {"company": {"count": 25, "index": 108.5}})
def test_danish_name_header_wins_over_qualified_index_header(self):
# The cell carries no comma so the case tests which column wins and
# does not depend on how a combined name-and-city cell is split (#660).
ws = FakeWorksheet([
("Firmanavn og by", "Antal", "Firmaets indeks"),
("Acme A/S", 25, 108.5),
])
entry = parse_sheet(ws)[0]
self.assertEqual(entry["company"], "Acme A/S")
self.assertEqual(sum(1 for c in entry["categories"].values() if c.get("index") == 108.5), 1)
self.assertEqual(sum(1 for c in entry["categories"].values() if c.get("count") == 25), 1)
def test_exact_header_still_wins_over_a_qualified_name_header(self):
# Tier 1 (#571) is unchanged: "Company" beats "Company Name" wherever it sits.
ws = FakeWorksheet([
("Company Name", "Company", "Salary Index"),
("Acme Corp (legal)", "Acme", 108.5),
])
self.assertEqual(parse_sheet(ws)[0]["company"], "Acme")
def test_city_column_never_aliases_the_company_column(self):
# "Firmanavn og by" carries both a company and a city pattern word.
ws = FakeWorksheet([
("Firmanavn og by", "Antal", "Indeks"),
("Acme A/S", 25, 108.5),
])
entry = parse_sheet(ws)[0]
self.assertEqual(entry["company"], "Acme A/S")
self.assertEqual(entry["city"], "")
self.assertEqual(entry["categories"], {"all_employees": {"count": 25, "index": 108.5}})
def test_combined_header_before_a_real_city_column_keeps_the_city(self):
# The city lookup must skip the company column: with the combined
# header first, the first city-pattern match used to land on it and
# was then cleared, dropping a real city column that followed.
for headers, row, city in (
(("Company and city", "City name", "Count", "Index"), ("Acme Corp", "Berlin", 25, 108.5), "Berlin"),
(("Firmanavn og by", "City name", "Antal", "Indeks"), ("Acme A/S", "Aarhus", 25, 108.5), "Aarhus"),
):
with self.subTest(headers=headers):
entry = parse_sheet(FakeWorksheet([headers, row]))[0]
self.assertEqual(entry["company"], row[0])
self.assertEqual(entry["city"], city)
self.assertEqual(entry["categories"], {"all_employees": {"count": 25, "index": 108.5}})
class CompoundCategoryPairingTests(unittest.TestCase): class CompoundCategoryPairingTests(unittest.TestCase):
def test_parse_sheet_pairs_danish_compound_index_with_count(self): def test_parse_sheet_pairs_danish_compound_index_with_count(self):
# "Lønindeks alle" is *detected* as an index column via # "Lønindeks alle" is *detected* as an index column via
+21 -13
View File
@@ -197,20 +197,28 @@ def parse_sheet(ws, sheet_label=None):
for cell in ws[header_row]: for cell in ws[header_row]:
headers.append(" ".join(str(cell.value).split()) if cell.value else "") headers.append(" ".join(str(cell.value).split()) if cell.value else "")
# Find company and city columns # Find company and city columns. Three tiers, each taking the FIRST match:
company_col = None # 1. an exact pattern header ("Company", "Firma", "By");
city_col = None # 2. a header that carries the pattern but is not a count/index column
for i, h in enumerate(headers): # ("Company Name", "Firmanavn og by");
if header_matches(h, COMPANY_PATTERNS): # 3. any header carrying the pattern.
company_col = i # The old loop kept the LAST pattern match, so "Company Name | ... |
elif header_matches(h, CITY_PATTERNS): # Company Index" made the salary index the company column: every row's
city_col = i # company read "108.5". #571 added tier 1; tiers 2-3 cover a qualified
# name header that sits next to a qualified salary header. The city
# lookup skips the company column: a combined "Company and city" header
# carries both words, and must not shadow a real city column after it.
def metadata_column(patterns, skip=None):
usable = [(i, h) for i, h in enumerate(headers) if i != skip]
exact = next((i for i, h in usable if h.lower().strip() in patterns), None)
if exact is not None:
return exact
candidates = [i for i, h in usable if header_matches(h, patterns)]
plain = [i for i in candidates if detect_column_type(headers[i]) is None]
return (plain or candidates or [None])[0]
# A qualified salary header such as "Company Index" may also contain a company_col = metadata_column(COMPANY_PATTERNS)
# metadata token. Prefer an unqualified metadata column when one exists, city_col = metadata_column(CITY_PATTERNS, skip=company_col)
# leaving the qualified column available as salary data.
company_col = next((i for i, h in enumerate(headers) if h.lower() in COMPANY_PATTERNS), company_col)
city_col = next((i for i, h in enumerate(headers) if h.lower() in CITY_PATTERNS), city_col)
if company_col is None: if company_col is None:
print(f"Warning: Could not find company column in sheet '{ws.title}'.", file=sys.stderr) print(f"Warning: Could not find company column in sheet '{ws.title}'.", file=sys.stderr)