diff --git a/.claude/skills/job-scraper/SKILL.md b/.claude/skills/job-scraper/SKILL.md index cd638396..5a079435 100644 --- a/.claude/skills/job-scraper/SKILL.md +++ b/.claude/skills/job-scraper/SKILL.md @@ -157,7 +157,7 @@ For each new job, do a rapid fit check (NOT the full evaluation from `04-job-eva python3 tools/job_key.py --company "" --title "" --url "<url>" ``` -It prints one line: the canonical key for that posting. The key must be a pure function of the posting, because two runs that slugify differently store the same job twice and defeat the dedup this step exists to provide. The helper also length-caps long titles and disambiguates the cap with a hash of the full slug, so a truncated title is stable across runs and two different long titles never collide. `python3 tools/job_key.py --audit` reports entries in an existing state file that predate this rule; it only reports, and never rewrites keys, since a rewritten key breaks the tracker's own company+role matching. +It prints one line: the canonical key for that posting. The key must be a pure function of the posting, because two runs that slugify differently store the same job twice and defeat the dedup this step exists to provide. The helper also length-caps long titles and disambiguates the cap with a hash of the full slug, so a truncated title is stable across runs and two different long titles never collide. `python3 tools/job_key.py --audit` reports entries in an existing state file that predate this rule; it only reports. `python3 tools/job_key.py --audit --migrate` rewrites the keys that no longer match the rule (backup first, old->new reported, an existing key is never overwritten, a second run changes nothing). 2. Add ALL fetched jobs (new and skipped) to `job_scraper/seen_jobs.json` through the tool below. Each stored entry has this structure: ```json diff --git a/.gitignore b/.gitignore index b3473abe..c4926fad 100644 --- a/.gitignore +++ b/.gitignore @@ -25,6 +25,8 @@ salary_data.json # its own directory, so these land at .claude/skills/job-scraper/job_scraper/*. # A rooted `job_scraper/...` pattern silently fails to match them. **/job_scraper/seen_jobs.json +# Backups written by `tools/job_key.py --audit --migrate` hold the same history. +**/job_scraper/seen_jobs.json.bak-* **/job_scraper/notion_sync.json **/job_scraper/*.md *_BehavioralReport.pdf diff --git a/tests/test_job_key.py b/tests/test_job_key.py index e7f82b0d..723edf6c 100644 --- a/tests/test_job_key.py +++ b/tests/test_job_key.py @@ -14,7 +14,7 @@ import unittest from pathlib import Path sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "tools")) -from job_key import is_canonical, is_legacy_shape, make_key, slugify # noqa: E402 +from job_key import _LATIN_FOLD, is_canonical, is_legacy_shape, make_key, slugify # noqa: E402 REPO = Path(__file__).resolve().parent.parent TOOL = REPO / "tools" / "job_key.py" @@ -159,12 +159,49 @@ class MixedScriptNamesKeepTheirIdentity(unittest.TestCase): # NFKD folds these without dropping a letter, so existing keys stay put. self.assertEqual(make_key("Zürich Versicherung", "Ingénieur finance"), "zurich-versicherung_ingenieur-finance") - def test_latin_letters_without_a_decomposition_do_not_re_key(self): - # "ø" and "æ" have no NFKD decomposition and are dropped by the fold, - # but they are Latin letters: "Ørsted" has keyed as "rsted" since the - # rule existed, and a live Danish seen_jobs.json must not re-key. - self.assertEqual(make_key("Ørsted A/S", "Senior Engineer"), "rsted-a-s_senior-engineer") - self.assertEqual(make_key("Mærsk", "Søfarende"), "mrsk_sfarende") + def test_latin_letters_without_a_decomposition_are_folded(self): + # "ø" and "æ" have no NFKD decomposition, so the ASCII fold used to drop + # them ("Mærsk" -> "mrsk"). #537 folds them through a table instead; a + # live seen_jobs.json is moved by `--audit --migrate`. + self.assertEqual(make_key("Ørsted A/S", "Senior Engineer"), "orsted-a-s_senior-engineer") + self.assertEqual(make_key("Mærsk", "Søfarende"), "maersk_sofarende") # was mrsk_sfarende before #537 + + def test_ao_and_ro_no_longer_collide(self): + self.assertNotEqual(make_key("Ærø", "Pilot"), make_key("Rø", "Pilot")) + self.assertEqual(slugify("Ærø"), "aero") + + def test_unchanged_letters_keep_their_keys(self): + self.assertEqual(slugify("Århus"), "arhus") # å stays a, not aa + self.assertEqual(slugify("é"), "e") + + def test_every_table_character_folds_including_capitals(self): + for ch, folded in _LATIN_FOLD.items(): + with self.subTest(ch=ch): + self.assertEqual(slugify(f"x{ch}y"), f"x{folded.lower()}y") + + def test_latin_extended_a_letters_fold_including_capitals(self): + cases = { + "ı": "i", + "ħ": "h", "Ħ": "h", + "ĸ": "k", + "ŋ": "ng", "Ŋ": "ng", + "ŧ": "t", "Ŧ": "t", + } + for ch, folded in cases.items(): + with self.subTest(ch=ch): + self.assertEqual(slugify(f"x{ch}y"), f"x{folded}y") + + def test_turkish_dotless_i_keeps_its_letters(self): + self.assertEqual(slugify("Kılıç"), "kilic") + + def test_fold_applies_to_decomposed_input_too(self): + # "Å" typed as A + combining ring: NFKD keeps the A and the mark is dropped. + self.assertEqual(slugify("Århus"), "arhus") + + def test_letter_that_decomposes_into_a_table_letter_is_folded(self): + # "ǿ" (U+01FF) is NFKD "ø" + acute, so the fold must run after NFKD. + self.assertEqual(slugify("ǿ"), "o") + self.assertEqual(slugify("Ǿ"), "o") class CompanyFallbackCLI(unittest.TestCase): @@ -287,5 +324,128 @@ class AuditCLI(unittest.TestCase): self.assertNotIn("Traceback", proc.stderr) +class MigrateCLI(unittest.TestCase): + """`--audit --migrate` rewrites drifted keys in place, safely.""" + + OLD = "mrsk_sfarende" + NEW = "maersk_sofarende" + + def setUp(self): + import tempfile + + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.path = Path(self._tmp.name) / "seen_jobs.json" + + def write(self, seen): + self.path.write_text(json.dumps({"seen": seen}, ensure_ascii=False), encoding="utf-8") + + def migrate(self): + proc = subprocess.run( + [sys.executable, str(TOOL), "--audit", str(self.path), "--migrate"], + capture_output=True, text=True, encoding="utf-8", + ) + return json.loads(proc.stdout), proc + + def seen(self): + return json.loads(self.path.read_text(encoding="utf-8"))["seen"] + + def backups(self): + return list(self.path.parent.glob("seen_jobs.json.bak-*")) + + def test_rewrites_the_key_and_keeps_the_entry(self): + entry = {"company": "Mærsk", "title": "Søfarende", "status": "ranked"} + self.write({self.OLD: entry}) + report, proc = self.migrate() + self.assertEqual(proc.returncode, 0) + self.assertEqual(self.seen(), {self.NEW: entry}) + self.assertEqual(report["migrated"], [{"old": self.OLD, "new": self.NEW}]) + self.assertEqual(report["keys_not_matching_current_rule"], 0) + + def test_backup_holds_the_original_bytes(self): + self.write({self.OLD: {"company": "Mærsk", "title": "Søfarende"}}) + original = self.path.read_bytes() + report, _ = self.migrate() + backups = self.backups() + self.assertEqual(len(backups), 1) + self.assertEqual(backups[0].read_bytes(), original) + self.assertEqual(report["backup"], str(backups[0])) + + def test_existing_new_key_is_reported_not_overwritten(self): + existing = {"company": "Mærsk", "title": "Søfarende", "status": "applied"} + stale = {"company": "Mærsk", "title": "Søfarende", "status": "new"} + self.write({self.NEW: existing, self.OLD: stale}) + report, _ = self.migrate() + self.assertEqual(self.seen(), {self.NEW: existing, self.OLD: stale}) + self.assertEqual(report["migrate_collisions"], [{"old": self.OLD, "new": self.NEW}]) + self.assertEqual(report["migrated"], []) + self.assertEqual(self.backups(), []) + + def test_chain_moves_in_one_run_and_the_second_run_is_byte_identical(self): + # A is keyed "wrong", and today's key for A is x. B sits on x and today + # belongs on y. Moving B first frees x for A. + a = {"company": "Mærsk", "title": "Søfarende", "status": "new"} + b = {"company": "Ørsted", "title": "Engineer", "status": "applied"} + x = self.NEW + y = make_key("Ørsted", "Engineer") + self.write({"stale-a_x": a, x: b}) # file order: A first, so B must not block it + self.assertEqual(make_key("Mærsk", "Søfarende"), x) + report, _ = self.migrate() + self.assertEqual(self.seen(), {x: a, y: b}) + self.assertEqual({m["old"]: m["new"] for m in report["migrated"]}, {"stale-a_x": x, x: y}) + self.assertEqual(report["migrate_collisions"], []) + after = self.path.read_bytes() + again, _ = self.migrate() + self.assertEqual(again["migrated"], []) + self.assertEqual(self.path.read_bytes(), after) + self.assertEqual(len(self.backups()), 1) + + def test_migrating_one_key_changes_only_the_lines_that_carry_it(self): + # Written the way tools/rank_state.save_state writes it. + seen = { + self.OLD: {"company": "Mærsk", "title": "Søfarende", "tags": ["a", "b"]}, + "acme_soc": {"company": "Acme", "title": "SOC", "url": "https://x/1"}, + } + text = json.dumps({"seen": seen}, indent=2, ensure_ascii=False) + chr(10) + self.path.write_text(text, encoding="utf-8") + self.migrate() + before, after = text.splitlines(), self.path.read_text(encoding="utf-8").splitlines() + self.assertEqual(len(before), len(after)) + changed = [(o, n) for o, n in zip(before, after) if o != n] + self.assertEqual(len(changed), 1) + self.assertIn(self.OLD, changed[0][0]) + self.assertIn(self.NEW, changed[0][1]) + + def test_second_run_changes_nothing(self): + self.write({self.OLD: {"company": "Mærsk", "title": "Søfarende"}, "acme_soc": {"company": "Acme", "title": "SOC"}}) + self.migrate() + after_first = self.path.read_bytes() + report, _ = self.migrate() + self.assertEqual(report["migrated"], []) + self.assertEqual(self.path.read_bytes(), after_first) + self.assertEqual(len(self.backups()), 1) + + def test_no_drift_writes_no_backup(self): + self.write({"acme_soc": {"company": "Acme", "title": "SOC"}}) + before = self.path.read_bytes() + report, _ = self.migrate() + self.assertEqual(report["migrated"], []) + self.assertEqual(self.path.read_bytes(), before) + self.assertEqual(self.backups(), []) + + def test_audit_alone_never_rewrites(self): + self.write({self.OLD: {"company": "Mærsk", "title": "Søfarende"}}) + before = self.path.read_bytes() + proc = subprocess.run( + [sys.executable, str(TOOL), "--audit", str(self.path)], capture_output=True, text=True, encoding="utf-8" + ) + self.assertEqual(json.loads(proc.stdout)["keys_not_matching_current_rule"], 1) + self.assertEqual(self.path.read_bytes(), before) + + def test_migrate_without_audit_is_a_usage_error(self): + proc = subprocess.run([sys.executable, str(TOOL), "--migrate"], capture_output=True, text=True, encoding="utf-8") + self.assertEqual(proc.returncode, 2) + + if __name__ == "__main__": unittest.main() diff --git a/tools/job_key.py b/tools/job_key.py index babc887b..8e17ad1f 100644 --- a/tools/job_key.py +++ b/tools/job_key.py @@ -36,6 +36,11 @@ readable prefix and the identity comes from the same fallback. Usage: python3 tools/job_key.py --company "Acme Corp" --title "SOC Analyst (L2)" python3 tools/job_key.py --audit job_scraper/seen_jobs.json + python3 tools/job_key.py --audit --migrate + +`--audit --migrate` rewrites keys that no longer match the current rule, in +place: backup first, old->new reported, a new key that already exists is +reported and left alone, and a second run changes nothing. Exit 0 when a key is produced, or when an audit finds nothing. Exit 1 when an audit finds violations. @@ -44,9 +49,13 @@ audit finds violations. import argparse import hashlib import json +import os import re +import shutil import sys +import tempfile import unicodedata +from datetime import datetime from pathlib import Path ROOT = Path(__file__).resolve().parent.parent @@ -63,12 +72,39 @@ _NON_SLUG = re.compile(r"[^a-z0-9]+") _JOB_ID = re.compile(r"(\d{6,})") +# Latin letters with no NFKD decomposition, which the ASCII fold would otherwise +# drop ("Ørsted" keyed as "rsted", "Mærsk" as "mrsk", "Kılıç" as "klc"). Values +# follow Unidecode 1.4.0; ICU's Latin-ASCII differs only on "ĸ" (q) and "ŋ" (n). +# "ĸ" and "ı" have no table capital: "ĸ" has none and "ı"'s is plain "I". +# "å" is deliberately absent: it decomposes to "a" plus a ring without losing a +# letter, and "å"->"aa" would move keys that work today for no gain (#537). +_LATIN_FOLD = { + "ø": "o", "Ø": "O", + "æ": "ae", "Æ": "AE", + "œ": "oe", "Œ": "OE", + "ß": "ss", "ẞ": "SS", + "þ": "th", "Þ": "TH", + "ð": "d", "Ð": "D", + "đ": "d", "Đ": "D", + "ł": "l", "Ł": "L", + "ı": "i", + "ħ": "h", "Ħ": "H", + "ĸ": "k", + "ŋ": "ng", "Ŋ": "NG", + "ŧ": "t", "Ŧ": "T", +} +_LATIN_FOLD_TABLE = str.maketrans(_LATIN_FOLD) + + def slugify(text: str) -> str: """Lowercase ASCII slug. Non-Latin scripts legitimately reduce to ''.""" if not text: return "" + # Fold after NFKD so a letter that decomposes into a fold-table letter plus + # a mark ("ǿ" -> "ø" + acute) is folded too. decomposed = unicodedata.normalize("NFKD", str(text)) - ascii_only = decomposed.encode("ascii", "ignore").decode("ascii") + folded = decomposed.translate(_LATIN_FOLD_TABLE) + ascii_only = folded.encode("ascii", "ignore").decode("ascii") return _NON_SLUG.sub("-", ascii_only.lower()).strip("-") @@ -81,9 +117,8 @@ def _lost_letters(text: str) -> bool: its Latin or digit fragment, and two different names can share it. Latin letters that also lack a decomposition ("ø", "æ", "ß", "ł") are - deliberately NOT counted: "Ørsted" has keyed as "rsted" since the rule - existed, and treating it as lossy would re-key every Danish company in a - live seen_jobs.json for a collision that does not happen in practice. + not counted: slugify folds them through _LATIN_FOLD ("ø" -> "o"), so they + keep their letter and are not lossy. """ if not text: return False @@ -177,24 +212,125 @@ def is_legacy_shape(key: str) -> bool: ) -def audit(path: Path) -> int: +def _load_seen(path: Path): + """Return (document, seen) or print the reason and return None.""" try: doc = json.loads(path.read_text(encoding="utf-8")) except (OSError, json.JSONDecodeError) as exc: print(f"cannot read {path}: {exc}", file=sys.stderr) - return 1 + return None seen = doc.get("seen", doc) if isinstance(doc, dict) else doc if not isinstance(seen, dict): print(f"{path}: expected an object of job entries", file=sys.stderr) - return 1 + return None not_objects = [k for k, v in seen.items() if not isinstance(v, dict)] if not_objects: print(f"{path}: entry is not an object: {not_objects[0]}", file=sys.stderr) - return 1 + return None bad_url = [k for k, v in seen.items() if v.get("url") is not None and not isinstance(v["url"], str)] if bad_url: print(f"{path}: entry url is not a string: {bad_url[0]}", file=sys.stderr) + return None + return doc, seen + + +def _drifted(seen: dict) -> list[str]: + """Canonical keys that differ from what make_key would produce today.""" + return [ + k for k, v in seen.items() + if is_canonical(k) and k != make_key(v.get("company", ""), v.get("title", ""), v.get("url", "")) + ] + + +def _rename_pass(seen: dict) -> tuple[dict[str, str], list[dict]]: + """One pass: which drifted keys can move, and which are blocked. + + A key is blocked when its new key is held by an entry that does not itself + move in this pass, or when an earlier entry (file order) claims the same + new key. A blocked entry stays put, which can in turn block another, so + the movers are narrowed until the set is stable. An entry whose new key is + held by a mover is fine: the chain A -> x, B(on x) -> y moves together. + """ + target = { + k: make_key(seen[k].get("company", ""), seen[k].get("title", ""), seen[k].get("url", "")) + for k in _drifted(seen) + } + movers = dict(target) + blocked: dict[str, str] = {} + changed = True + while changed: + changed = False + claimed: set[str] = set() + for old, new in list(movers.items()): + if (new in seen and new not in movers) or new in claimed: + blocked[old] = new + del movers[old] + changed = True + else: + claimed.add(new) + return movers, [{"old": o, "new": n} for o, n in blocked.items()] + + +def migrate(path: Path, doc, seen: dict) -> dict: + """Rewrite drifted keys in place; return the report. + + Runs passes until nothing renames. Deterministic (file order decides which + of two entries claims a new key), never overwrites an existing key, and + idempotent: once every rewritable key is current, a second run finds + nothing to change and writes nothing. Collisions in the report are the + final ones only. + """ + current = dict(seen) + migrated: list[dict] = [] + collisions: list[dict] = [] + for _ in range(len(seen) + 1): + renames, collisions = _rename_pass(current) + if not renames: + break + migrated.extend({"old": o, "new": n} for o, n in renames.items()) + current = {renames.get(k, k): v for k, v in current.items()} + report = { + "migrated": migrated, + "migrate_collisions": collisions, + "backup": None, + } + if not migrated: + return report + + backup = path.with_name(f"{path.name}.bak-{datetime.now():%Y%m%d-%H%M%S}") + shutil.copy2(path, backup) + report["backup"] = str(backup) + + out = dict(doc, seen=current) if isinstance(doc, dict) and "seen" in doc else current + # Same dump settings as tools/rank_state.save_state, so a migration changes + # only the lines that carry a renamed key. Atomic replace: a half-written + # seen_jobs.json loses the scrape history. + fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix=f".{path.name}.", suffix=".tmp") + try: + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump(out, fh, indent=2, ensure_ascii=False) + fh.write("\n") + os.replace(tmp, path) + except BaseException: + Path(tmp).unlink(missing_ok=True) + raise + return report + + +def audit(path: Path, do_migrate: bool = False) -> int: + loaded = _load_seen(path) + if loaded is None: return 1 + doc, seen = loaded + + migration = {} + if do_migrate: + migration = migrate(path, doc, seen) + if migration["migrated"]: + loaded = _load_seen(path) + if loaded is None: + return 1 + doc, seen = loaded malformed = [k for k in seen if not is_canonical(k) and not is_legacy_shape(k)] legacy = [k for k in seen if is_legacy_shape(k)] @@ -205,11 +341,8 @@ def audit(path: Path) -> int: by_url.setdefault(url, []).append(key) duplicates = {u: ks for u, ks in by_url.items() if len(ks) > 1} # A key that does not match what make_key would produce today is drift, not - # damage: reported separately so a rename is a choice, never automatic. - drift = [ - k for k, v in seen.items() - if is_canonical(k) and k != make_key(v.get("company", ""), v.get("title", ""), v.get("url", "")) - ] + # damage: reported separately so a rename is a choice (--migrate), never automatic. + drift = _drifted(seen) print(json.dumps({ "entries": len(seen), @@ -217,6 +350,7 @@ def audit(path: Path) -> int: "legacy_three_part_keys": legacy, "duplicate_urls": duplicates, "keys_not_matching_current_rule": len(drift), + **migration, }, indent=2, ensure_ascii=False)) return 1 if (malformed or duplicates) else 0 @@ -241,10 +375,13 @@ def main() -> int: ap.add_argument("--title") ap.add_argument("--url", default="") ap.add_argument("--audit", nargs="?", const=str(STATE), metavar="STATE_JSON") + ap.add_argument("--migrate", action="store_true", help="with --audit: rewrite drifted keys in place (backup first)") args = ap.parse_args() + if args.migrate and not args.audit: + ap.error("--migrate needs --audit") if args.audit: - return audit(Path(args.audit)) + return audit(Path(args.audit), args.migrate) if args.company is None or args.title is None: ap.error("give --company and --title, or --audit") print(make_key(args.company, args.title, args.url)) diff --git a/tools/security_guards.py b/tools/security_guards.py index 96101544..ebdc7859 100644 --- a/tools/security_guards.py +++ b/tools/security_guards.py @@ -168,6 +168,7 @@ REQUIRED_IGNORE_RULES = [ # to its own directory, so the state file lands under .claude/skills/... and # a repo-rooted rule silently fails to match it. "**/job_scraper/seen_jobs.json", + "**/job_scraper/seen_jobs.json.bak-*", "**/job_scraper/notion_sync.json", "**/job_scraper/*.md", "*_BehavioralReport.pdf",