From b2545d51212c3ca6278304ddb0c0504611f2a7f1 Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 19:44:36 +0200 Subject: [PATCH 01/25] fix(latex): brace bracket-leading bullets, document escapes, pin pdftotext encoding Three findings from the 2026-08-19 review (F9, F31, F34): - F9: every placeholder bullet written as \item [text] let LaTeX parse the bracketed text as the item's optional label, rendering it clipped off the left page edge and absent from the PDF text layer ("Achievement" appeared 9 times in cv/main_example.tex and 0 times in the extraction, with a clean compile and green CI). Bullets are now braced as \item {[text]} in the example CV and in the template 06-cover-letter-templates.md teaches, and CI's stock PDF assertions additionally require "Achievement" to survive pdftotext. - F31: 05-cv-templates.md gains a "LaTeX Special Characters" section and 06's is completed beyond \_ and \&. The load-bearing case is an unescaped % in a quantified achievement bullet: it starts a LaTeX comment and silently deletes the rest of the line from the PDF. - F34: the documented ATS extraction commands (apply.md, 05-cv-templates.md, CLAUDE.md) now carry -enc UTF-8. Xpdf-based pdftotext builds default to Latin-1 output, so a correct non-ASCII CV failed the replacement-character parseability check. framework_version: 05-cv-templates.md 1.4.1 -> 1.4.2, 06-cover-letter-templates.md 1.0.1 -> 1.0.2. All three pinned by the new tests/test_latex_guidance.py (9 tests; suite now 261). Co-Authored-By: Claude Opus 5 (1M context) --- .claude/commands/apply.md | 4 +- .../05-cv-templates.md | 27 +++- .../06-cover-letter-templates.md | 16 +- .github/workflows/ci.yml | 3 +- CHANGELOG.md | 27 ++++ CLAUDE.md | 2 +- cv/main_example.tex | 22 +-- tests/test_latex_guidance.py | 140 ++++++++++++++++++ 8 files changed, 217 insertions(+), 24 deletions(-) create mode 100644 tests/test_latex_guidance.py diff --git a/.claude/commands/apply.md b/.claude/commands/apply.md index 893da9a..f9f5b0e 100644 --- a/.claude/commands/apply.md +++ b/.claude/commands/apply.md @@ -254,12 +254,12 @@ Do not proceed to Step 6 until both PDFs pass inspection. An ATS parser reads the PDF's embedded **text layer**, not the rendered page — a CV that passed visual inspection can still extract as garbage (icon glyphs where the contact details should be, scrambled reading order in multi-column layouts). This step verifies what a parser actually sees. It applies to the **CV only**; cover letters rarely go through keyword screening. -**Availability check:** run `pdftotext -v`. `pdftotext` (poppler) is an optional dependency, not part of TeX distributions. If it is missing, print a one-line warning that the mechanical parse check is skipped, do the keyword-coverage check (item 3 below) against your visual Read of the PDF instead, and note the degraded mode in the Step 6 report. Same graceful-skip pattern as the salary lookup. +**Availability check:** run `pdftotext -v`. `pdftotext` (poppler) is an optional dependency, not part of TeX distributions. If it is missing, print a one-line warning that the mechanical parse check is skipped, do the keyword-coverage check (item 3 below) against your visual Read of the PDF instead, and note the degraded mode in the Step 6 report. Same graceful-skip pattern as the salary lookup. Keep the `-enc UTF-8` flag: Xpdf-based builds default to Latin-1 output, and without it a correct non-ASCII CV fails the replacement-character check below. **1. Extract the text layer:** ```bash -cd cv && pdftotext -layout main__.pdf main__.txt +cd cv && pdftotext -layout -enc UTF-8 main__.pdf main__.txt ``` Read the `.txt` file. diff --git a/.claude/skills/job-application-assistant/05-cv-templates.md b/.claude/skills/job-application-assistant/05-cv-templates.md index c046aa4..220c6ca 100644 --- a/.claude/skills/job-application-assistant/05-cv-templates.md +++ b/.claude/skills/job-application-assistant/05-cv-templates.md @@ -1,5 +1,5 @@ --- -framework_version: 1.4.1 +framework_version: 1.4.2 --- # CV Templates and Tailoring Guide @@ -211,6 +211,27 @@ Wherever the CV names a verifiable artifact - a public project, a hackathon entr - End with: "More references are available upon request." - **Do not attach reference letters** - employers typically contact references directly +### LaTeX Special Characters (important) + +Postings and profile data arrive as plain text; the CV is LaTeX. Escape these wherever they land in body text - company names, achievement bullets, skill lists: + +| Character | Write | Typical trigger | +|---|---|---| +| `&` | `\&` | company names: Bang \& Olufsen, Brüel \& Kjær, H\&M | +| `%` | `\%` | quantified achievements: "cut latency by 40\%" | +| `$` | `\$` | salary and cost figures | +| `#` | `\#` | "ranked \#1", C\# | +| `_` | `\_` | file names, code identifiers | +| `~` | `\textasciitilde{}` | URLs, "approx. 5 years" tildes | +| `^` | `\textasciicircum{}` | version strings, math | + +Two failure modes deserve special care: + +- **`%` fails silently.** An unescaped `%` starts a LaTeX comment: the compile succeeds with zero errors, and everything after the `%` on that line vanishes from the PDF. `Cut inference latency by 40% and saved DKK 2M annually` renders as "Cut inference latency by 40" - the bullet keeps its impressive-looking fragment and loses the actual result. Quantified achievement bullets are exactly where the guidance steers you ("use numbers where possible"), so check every `%` in every bullet before compiling. +- **`&` fails loudly** inside `\cventry` (alignment-tab errors, `Missing } inserted`). The compile loop catches it, but escape employer names up front rather than debugging the compile. + +Related trap: a bullet whose text begins with a literal `[` must be braced - `\item {[text]}` - or LaTeX parses the bracketed text as `\item`'s optional label and renders it clipped off the left page edge with a clean compile. The example CV's placeholder bullets are braced for exactly this reason. + ## Compile-and-Inspect Loop (MANDATORY) After writing the CV and before presenting to the user, always compile and visually inspect the PDF. Iterate until the layout is clean. Workflow: @@ -246,10 +267,10 @@ Restore the highest-relevance item that was previously cut — a CV that ends mi Most employers run CVs through an ATS before a human sees them, and the ATS reads the PDF's embedded **text layer**, not the rendered page. A CV can pass visual inspection and still extract as garbage. After the layout passes the compile-and-inspect loop, verify the text layer: ```bash -cd cv && pdftotext -layout main__.pdf main__.txt +cd cv && pdftotext -layout -enc UTF-8 main__.pdf main__.txt ``` -`pdftotext` comes from [poppler](https://poppler.freedesktop.org/), not the TeX distribution - it is an **optional** dependency. If it is not installed, skip the mechanical check with a warning and rely on the visual PDF read for keyword coverage. +`pdftotext` comes from [poppler](https://poppler.freedesktop.org/), not the TeX distribution - it is an **optional** dependency. The `-enc UTF-8` flag is not optional: Xpdf-based `pdftotext` builds default to Latin-1 output, which makes every non-ASCII character in a perfectly good CV read back as a replacement character and fail the parseability check below for no real reason. If it is not installed, skip the mechanical check with a warning and rely on the visual PDF read for keyword coverage. What to check in the extraction: diff --git a/.claude/skills/job-application-assistant/06-cover-letter-templates.md b/.claude/skills/job-application-assistant/06-cover-letter-templates.md index 1d1f8c2..c5e4a12 100644 --- a/.claude/skills/job-application-assistant/06-cover-letter-templates.md +++ b/.claude/skills/job-application-assistant/06-cover-letter-templates.md @@ -1,5 +1,5 @@ --- -framework_version: 1.0.1 +framework_version: 1.0.2 --- # Cover Letter Templates and Tailoring Guide @@ -92,9 +92,9 @@ The font wrapper is mandatory — if you just move `\begin{itemize}` outside `\l {\raggedright\fontspec[Path = OpenFonts/fonts/raleway/]{Raleway-Medium}\fontsize{11pt}{13pt}\selectfont \begin{itemize} - \item [Concrete achievement/skill 1] - \item [Concrete achievement/skill 2] - \item [Concrete achievement/skill 3] + \item {[Concrete achievement/skill 1]} + \item {[Concrete achievement/skill 2]} + \item {[Concrete achievement/skill 3]} \end{itemize}\par} \lettercontent{[Connection to company - why this role, why this company specifically]} @@ -146,10 +146,14 @@ The font wrapper is mandatory — if you just move `\begin{itemize}` outside `\l - 3-5 bullets is ideal - Start each bullet with bold label or action verb - Use `\textbf{Label:}` for category-style bullets +- A bullet whose text begins with a literal `[` must be braced: `\item {[text]}`. Unbraced, LaTeX parses `[text]` as `\item`'s optional label and renders it off the left page edge, missing from the PDF text layer entirely ### LaTeX Special Characters -- Underscore: `\_` -- Ampersand: `\&` +Escape these wherever they appear in body text: +- Ampersand: `\&` (company names: Brüel \& Kjær, H\&M) - unescaped, the compile fails loudly +- Percent: `\%` ("grew revenue 30\%") - unescaped, it does **not** fail: everything after the `%` on that line is silently eaten as a LaTeX comment +- Dollar: `\$`, hash: `\#`, underscore: `\_` +- Tilde: `\textasciitilde{}`, caret: `\textasciicircum{}`, backslash: `\textbackslash{}` ### Non-English Cover Letters - Same template structure, just write content in the posting's language diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d496020..7156572 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -144,7 +144,8 @@ jobs: python3 tools/verify_pdf.py cv/main_example.pdf \ --pages 2 \ --contains '[your.email@example.com]' \ - --contains 'Professional Experience' + --contains 'Professional Experience' \ + --contains 'Achievement' python3 tools/verify_pdf.py cover_letters/cover_example.pdf \ --pages 1 \ --contains 'your.email@example.com' \ diff --git a/CHANGELOG.md b/CHANGELOG.md index 8449f29..20901ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,16 @@ per-file diff commands. ### Added +- **LaTeX special-character guidance for CVs** (`framework_version` 1.4.1 -> 1.4.2 in + `05-cv-templates.md`, 1.0.1 -> 1.0.2 in `06-cover-letter-templates.md`) - `05` gains a + "LaTeX Special Characters" section and `06`'s existing one is completed beyond `\_`/`\&`. + The load-bearing case is an unescaped `%` in a quantified achievement bullet: it starts a + LaTeX comment, so "cut latency by 40% and saved DKK 2M" compiles with zero errors and + renders as "cut latency by 40" - silent content loss in the deliverable, on exactly the + content the guidance steers users to write. `&` in employer names (Bang & Olufsen, H&M) + fails loudly at compile time and is now documented alongside. Pinned by + `tests/test_latex_guidance.py`. + - **`seen_jobs.json` entries record which mechanism produced them** - a new additive `source` field (`cli` for Step 1b portal-CLI output, `websearch` for the Step 1c fallback), a Step 1c rule tagging fallback results at collection time, and a `fallback (websearch):` line in the @@ -46,6 +56,23 @@ per-file diff commands. ### Fixed +- **Example-CV bullets no longer swallowed as LaTeX optional labels** - every placeholder + bullet written as `\item [text]` (11 in `cv/main_example.tex`, 3 in + `06-cover-letter-templates.md`'s taught template) let LaTeX parse the bracketed text as + `\item`'s optional argument: the shipped example CV rendered all Professional Experience + bullets clipped off the left page edge, with the word "Achievement" appearing 9 times in + the source and 0 times in the PDF text layer - a clean compile, green CI. Bullets are now + braced (`\item {[text]}`), the cover-letter guide teaches the braced form, and CI's stock + PDF assertions additionally require `Achievement` to survive `pdftotext`. Pinned by + `tests/test_latex_guidance.py`. +- **Documented ATS extraction commands pin `-enc UTF-8`** - `pdftotext -layout` without an + encoding flag emits Latin-1 on Xpdf builds, so every non-ASCII character in a correct CV + (Rambøll, Ingeniør, København) read back as a replacement character and failed the + parseability checklist, steering the agent to "fix" a healthy document. The commands in + `apply.md`, `05-cv-templates.md`, and `CLAUDE.md`'s verification checklist now carry + `-enc UTF-8`, which is deterministic on both poppler and Xpdf. Pinned by + `tests/test_latex_guidance.py`. + - **`jobbank-search` search output now carries the `/scrape` contract's `date` field** (#342) - the CLI emitted `posted` (full ISO 8601) but not the cross-portal `date` key, the one Step 2 contract field it was missing. Search results now additively emit `date` as `YYYY-MM-DD` diff --git a/CLAUDE.md b/CLAUDE.md index c1c20b7..608cd1c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -140,7 +140,7 @@ Both documents MUST be compiled and visually inspected via the Read tool on the - [ ] **Cover letter bullet font matches body font** - `\lettercontent{}` must not wrap `\begin{itemize}...\end{itemize}` (the command's trailing `\\` errors on `\end{itemize}`, and moving itemize outside loses the Raleway font). Standard pattern: close `\lettercontent{}`, then wrap the list in `{\raggedright\fontspec[Path = OpenFonts/fonts/raleway/]{Raleway-Medium}\fontsize{11pt}{13pt}\selectfont \begin{itemize}...\end{itemize}\par}` ### ATS & keyword verification (CV) -ATS parsers read the PDF's embedded text layer, not the rendered page. Extract it with `pdftotext -layout` and verify what a parser sees. `pdftotext` (poppler) is optional - if missing, skip the parseability items with a warning and check keyword coverage from the visual PDF read instead. +ATS parsers read the PDF's embedded text layer, not the rendered page. Extract it with `pdftotext -layout -enc UTF-8` and verify what a parser sees. `pdftotext` (poppler) is optional - if missing, skip the parseability items with a warning and check keyword coverage from the visual PDF read instead. - [ ] CV text layer extracts cleanly - no `(cid:*)` markers, `�` replacement characters, or text visible in the PDF but absent from the extraction - [ ] Email and phone appear as **literal text** in the extraction (icon-glyph noise like `MOBILE-ALT`/`Envelope` is harmless, but a contact detail carried only by an icon or hyperlink is invisible to ATS) - [ ] Reading order of the extracted text matches the visual order (single-column stock template is safe; multi-column custom templates are where this breaks) diff --git a/cv/main_example.tex b/cv/main_example.tex index 7070979..59bb7f3 100644 --- a/cv/main_example.tex +++ b/cv/main_example.tex @@ -93,10 +93,10 @@ % --- Most Recent Role --- \item{\cventry{[YYYY-Present]}{[Job Title]}{[Company]}{[City, Country]}{}{\vspace{1pt} \begin{itemize} - \item [Achievement or responsibility 1 - be specific, use numbers where possible] - \item [Achievement or responsibility 2] - \item [Achievement or responsibility 3] - \item [Achievement or responsibility 4] + \item {[Achievement or responsibility 1 - be specific, use numbers where possible]} + \item {[Achievement or responsibility 2]} + \item {[Achievement or responsibility 3]} + \item {[Achievement or responsibility 4]} \end{itemize}}} \vspace{3pt} @@ -104,9 +104,9 @@ % --- Previous Role --- \item{\cventry{[YYYY-YYYY]}{[Job Title]}{[Company]}{[City, Country]}{}{\vspace{1pt} \begin{itemize} - \item [Achievement or responsibility 1] - \item [Achievement or responsibility 2] - \item [Achievement or responsibility 3] + \item {[Achievement or responsibility 1]} + \item {[Achievement or responsibility 2]} + \item {[Achievement or responsibility 3]} \end{itemize}}} \vspace{3pt} @@ -114,8 +114,8 @@ % --- Earlier Role --- \item{\cventry{[YYYY-YYYY]}{[Job Title]}{[Company]}{[City, Country]}{}{\vspace{1pt} \begin{itemize} - \item [Achievement or responsibility 1] - \item [Achievement or responsibility 2] + \item {[Achievement or responsibility 1]} + \item {[Achievement or responsibility 2]} \end{itemize}}} \end{itemize} @@ -147,7 +147,7 @@ Thesis: ``[Thesis Title].'' [Brief description of research focus.] \section{Languages} \vspace{1pt} \begin{itemize} -\item [Language 1] (native), [Language 2] (fluent), [Language 3] (intermediate). +\item {[Language 1] (native), [Language 2] (fluent), [Language 3] (intermediate).} \end{itemize} % ============================================================ @@ -157,7 +157,7 @@ Thesis: ``[Thesis Title].'' [Brief description of research focus.] \section{Publications} \vspace{1pt} \begin{itemize} -\item [Author(s)] ([Year]). [Title]. [Journal/Conference]. \href{[DOI_URL]}{DOI link} +\item {[Author(s)] ([Year]). [Title]. [Journal/Conference]. \href{[DOI_URL]}{DOI link}} \end{itemize} % ============================================================ diff --git a/tests/test_latex_guidance.py b/tests/test_latex_guidance.py new file mode 100644 index 0000000..ca8539a --- /dev/null +++ b/tests/test_latex_guidance.py @@ -0,0 +1,140 @@ +"""Guards for the LaTeX authoring guidance and the example documents. + +Three silent-failure modes live here, all found by the 2026-08-19 review +(F9, F31, F34). Each one produces a clean compile and a green CI run +while the rendered document or its ATS extraction is wrong, so the spec +files and the example sources are the only place a test can catch them: + +- F9: a bullet written as `\\item [text]` is parsed as moderncv's + optional label, rendered off the left page edge, and dropped from the + PDF text layer. The example CV shipped that way for months. +- F31: an unescaped `%` in body text silently truncates the rest of the + line (`&` at least fails loudly). The guidance must name the escapes. +- F34: `pdftotext` without `-enc UTF-8` emits Latin-1 on Xpdf builds, + so a correct Danish CV fails the documented "no replacement + characters" check and the agent is sent to "fix" a healthy document. +""" +import re +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +SKILL_DIR = REPO / ".claude" / "skills" / "job-application-assistant" +CV_TEMPLATES = SKILL_DIR / "05-cv-templates.md" +COVER_TEMPLATES = SKILL_DIR / "06-cover-letter-templates.md" +APPLY = REPO / ".claude" / "commands" / "apply.md" +EXAMPLE_CV = REPO / "cv" / "main_example.tex" +EXAMPLE_COVER = REPO / "cover_letters" / "cover_example.tex" + +# \item whose body starts with [ - with or without whitespace between. +# LaTeX skips spaces while scanning for the optional argument, so +# `\item [text]` and `\item[text]` both swallow the text as a label. +# The safe spelling `\item {[text]}` does not match. +UNBRACED_BRACKET_ITEM = re.compile(r"\\item\s*\[") + +# The escapes both guidance files must document. `%` is the load-bearing +# one: it truncates silently. The others fail loudly or corrupt spacing. +REQUIRED_ESCAPES = ["\\&", "\\%", "\\$", "\\#", "\\_"] + + +def section(text, heading): + """Return the body of a markdown section up to the next heading.""" + pattern = re.compile( + rf"^#+ {re.escape(heading)}[^\n]*\n(.*?)(?=^#+ |\Z)", + re.MULTILINE | re.DOTALL, + ) + match = pattern.search(text) + return match.group(1) if match else None + + +class TestBulletBracketTrap(unittest.TestCase): + """F9: no document or template doc may teach `\\item [text]`.""" + + def assert_no_unbraced_bracket_items(self, path): + offending = [ + f"{path.name}:{lineno}: {line.strip()}" + for lineno, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1) + if UNBRACED_BRACKET_ITEM.search(line) + ] + self.assertEqual( + offending, + [], + "\\item followed by [ is parsed as an optional label and the " + "text is clipped off the page; write \\item {[...]} instead:\n" + + "\n".join(offending), + ) + + def test_example_cv_has_no_bracket_labelled_bullets(self): + self.assert_no_unbraced_bracket_items(EXAMPLE_CV) + + def test_example_cover_letter_has_no_bracket_labelled_bullets(self): + self.assert_no_unbraced_bracket_items(EXAMPLE_COVER) + + def test_cover_letter_guide_does_not_teach_the_broken_pattern(self): + self.assert_no_unbraced_bracket_items(COVER_TEMPLATES) + + def test_cv_guide_does_not_teach_the_broken_pattern(self): + self.assert_no_unbraced_bracket_items(CV_TEMPLATES) + + +class TestSpecialCharacterGuidance(unittest.TestCase): + """F31: both template guides must document the LaTeX escapes.""" + + def assert_escapes_documented(self, path): + body = section(path.read_text(encoding="utf-8"), "LaTeX Special Characters") + self.assertIsNotNone( + body, f"{path.name} has no 'LaTeX Special Characters' section" + ) + missing = [esc for esc in REQUIRED_ESCAPES if esc not in body] + self.assertEqual( + missing, + [], + f"{path.name}'s special-characters section is missing: {missing}", + ) + + def test_cv_guide_documents_the_escapes(self): + self.assert_escapes_documented(CV_TEMPLATES) + + def test_cover_letter_guide_documents_the_escapes(self): + self.assert_escapes_documented(COVER_TEMPLATES) + + def test_cv_guide_warns_that_percent_truncates_silently(self): + body = section( + CV_TEMPLATES.read_text(encoding="utf-8"), "LaTeX Special Characters" + ) + self.assertIsNotNone(body) + self.assertRegex( + body, + re.compile(r"silent", re.IGNORECASE), + "the % failure mode must be called out as silent - it is the " + "reason this section exists (a clean compile with the rest of " + "the bullet gone)", + ) + + +class TestAtsExtractionEncoding(unittest.TestCase): + """F34: every documented extraction command must pin the encoding.""" + + def assert_pdftotext_commands_pin_utf8(self, path): + offending = [ + f"{path.name}:{lineno}: {line.strip()}" + for lineno, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1) + if "pdftotext" in line and "-layout" in line and "-enc UTF-8" not in line + ] + self.assertEqual( + offending, + [], + "pdftotext without -enc UTF-8 emits Latin-1 on Xpdf builds, so " + "the ATS check reports phantom replacement characters on any " + "non-ASCII CV; add -enc UTF-8:\n" + "\n".join(offending), + ) + + def test_apply_extraction_command_pins_utf8(self): + self.assert_pdftotext_commands_pin_utf8(APPLY) + + def test_cv_guide_extraction_command_pins_utf8(self): + self.assert_pdftotext_commands_pin_utf8(CV_TEMPLATES) + + +if __name__ == "__main__": + unittest.main() From 7aba0b4a9df9f03cda4bbcdc9c0c2ccb3edee432 Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 19:46:34 +0200 Subject: [PATCH 02/25] fix(jobdanmark-search): accept a comma after the postcode in location extraction The location regex required whitespace after the 4-digit postcode, but live companyAddress values frequently read "2670, Greve" - those results emitted location: null (7/30 in the review's live sample; 1/30 after this fix), leaving /scrape's geography filter nothing to act on. Extraction is now a helper with a comma fallback that requires a non-digit city start, so a 4-digit street number never wins over the real postcode, and the captured city is trimmed. Review finding F2 (2026-08-19). Co-Authored-By: Claude Opus 5 (1M context) --- .../cli/src/commands/search.ts | 14 +++++++++- .../cli/tests/search-normalization.test.ts | 27 +++++++++++++++++++ CHANGELOG.md | 7 +++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/.agents/skills/jobdanmark-search/cli/src/commands/search.ts b/.agents/skills/jobdanmark-search/cli/src/commands/search.ts index f0faee8..e311c09 100644 --- a/.agents/skills/jobdanmark-search/cli/src/commands/search.ts +++ b/.agents/skills/jobdanmark-search/cli/src/commands/search.ts @@ -39,6 +39,18 @@ function toContractDate(value: string | null): string | null { return match ? `${match[3]}-${match[2]}-${match[1]}` : (value ?? null) } +// Live companyAddress values put the city after the postcode either as +// "Lautruphoej 2, 2750 Ballerup" or "2670, Greve". The comma fallback +// requires a non-digit after the comma so a 4-digit street number +// ("Vejlevej 1234, 7100 Vejle") never wins over the real postcode. +function extractCity(address: string | null): string | null { + if (!address) return null + const city = + address.match(/\d{4}\s+(.+)$/)?.[1] ?? address.match(/\d{4}\s*,\s*([^\d,].*)$/)?.[1] + const trimmed = city?.trim() + return trimmed ? trimmed : null +} + export function normalizeItem(item: ApiSearchItem): Record { const relativeUrl = item.url const fullUrl = relativeUrl.startsWith("http") @@ -83,7 +95,7 @@ export function normalizeItem(item: ApiSearchItem): Record { coverImage, silhouetteLogo: item.silhouetteLogo, company: item.companyName, - location: item.companyAddress?.match(/\d{4}\s+(.+)$/)?.[1] ?? null, + location: extractCity(item.companyAddress), date: toContractDate(item.publishedDate), deadline: toContractDate(item.applicationDeadline), } diff --git a/.agents/skills/jobdanmark-search/cli/tests/search-normalization.test.ts b/.agents/skills/jobdanmark-search/cli/tests/search-normalization.test.ts index 6a63111..1c20287 100644 --- a/.agents/skills/jobdanmark-search/cli/tests/search-normalization.test.ts +++ b/.agents/skills/jobdanmark-search/cli/tests/search-normalization.test.ts @@ -44,6 +44,33 @@ describe("Jobdanmark search normalization", () => { expect(result.company).toBe("Statens It"); }); + test("extracts the city when a comma follows the postcode (live jobdanmark shape)", () => { + const result = normalizeItem({ + ...item(), + companyAddress: "2670, Greve", + }); + + expect(result.location).toBe("Greve"); + }); + + test("trims trailing whitespace from the extracted city", () => { + const result = normalizeItem({ + ...item(), + companyAddress: "7100, Vejle ", + }); + + expect(result.location).toBe("Vejle"); + }); + + test("does not mistake a 4-digit street number for the postcode", () => { + const result = normalizeItem({ + ...item(), + companyAddress: "Vejlevej 1234, 7100 Vejle", + }); + + expect(result.location).toBe("Vejle"); + }); + test("survives a null companyAddress from the API", () => { const result = normalizeItem({ ...item(), diff --git a/CHANGELOG.md b/CHANGELOG.md index 20901ba..1887902 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,13 @@ per-file diff commands. ### Fixed +- **`jobdanmark-search` extracts the city when a comma follows the postcode** - the + `location` regex required whitespace after the 4-digit postcode, but live + `companyAddress` values frequently read `"2670, Greve"`; those results emitted + `location: null` (7 of 30 in a live sample), so `/scrape`'s geography/commute filter + (Rule 3) had nothing to act on. The extraction now accepts an optional comma, trims the + captured city, and still refuses to mistake a 4-digit street number for the postcode. + Pinned by three new cases in `tests/search-normalization.test.ts`. - **Example-CV bullets no longer swallowed as LaTeX optional labels** - every placeholder bullet written as `\item [text]` (11 in `cv/main_example.tex`, 3 in `06-cover-letter-templates.md`'s taught template) let LaTeX parse the bracketed text as From 1c19f6c45f133240551c104766807a716caf4d0e Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 19:48:50 +0200 Subject: [PATCH 03/25] fix(salary-tools): decide number locale by last separator, pair compound headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings F7 and F8 (2026-08-19): - parse_numeric_cell's both-separators branch always assumed European locale, silently turning a US "1,234.56" into 1.23456 - a 1000x corruption written to salary_data.json with no warning. The separator that appears last is now treated as the decimal separator, which also makes multi-group values ("1,234,567.89") parse instead of raising a raw float error. Single-separator ambiguity guards are unchanged. - strip_type_patterns stripped only whole tokens, so the compound header "Lønindeks alle" kept its type word and could never pair with "Antal alle" - failing exactly for the compound-word locale COMPOUND_PATTERNS exists to support. It now also strips compound patterns as substrings, mirroring header_matches. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 10 ++++++ tests/test_convert_salary_excel.py | 52 ++++++++++++++++++++++++++++++ tools/convert_salary_excel.py | 18 +++++++++-- 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1887902..c067eb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,16 @@ per-file diff commands. ### Fixed +- **`convert_salary_excel.py` no longer corrupts US/UK-formatted numbers 1000x** - the + both-separators branch always assumed European locale, so a `"1,234.56"` cell was + silently converted to `1.23456` and written into `salary_data.json`. The rule is now + "the separator that appears last is the decimal separator", which also makes + multi-group values (`"1,234,567.89"`, `"1.234.567,89"`) parse instead of raising. And + `strip_type_patterns` now strips `COMPOUND_PATTERNS` words as substrings, mirroring + `header_matches`, so a Danish compound header pair ("Antal alle" / "Lønindeks alle") + pairs into one category instead of two unpaired standalones - the exact locale the + compound support was added for. Pinned by six new cases in + `tests/test_convert_salary_excel.py`. - **`jobdanmark-search` extracts the city when a comma follows the postcode** - the `location` regex required whitespace after the 4-digit postcode, but live `companyAddress` values frequently read `"2670, Greve"`; those results emitted diff --git a/tests/test_convert_salary_excel.py b/tests/test_convert_salary_excel.py index e1ede29..665825e 100644 --- a/tests/test_convert_salary_excel.py +++ b/tests/test_convert_salary_excel.py @@ -5,6 +5,7 @@ from tools.convert_salary_excel import ( INDEX_PATTERNS, detect_column_type, header_matches, + parse_numeric_cell, parse_sheet, ) @@ -288,3 +289,54 @@ class DetectColumnTypeTests(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +class ParseNumericCellLocaleTests(unittest.TestCase): + # The separator that appears LAST is the decimal separator. Assuming + # European ("." thousands, "," decimal) for every both-separator string + # turned a US "1,234.56" into 1.23456 - a silent 1000x corruption that + # flowed into salary_data.json and negotiation advice. + + def test_us_thousands_and_decimal_string(self): + self.assertEqual(parse_numeric_cell("1,234.56"), 1234.56) + + def test_us_multiple_thousands_groups(self): + self.assertEqual(parse_numeric_cell("1,234,567.89"), 1234567.89) + + def test_european_thousands_and_decimal_string(self): + self.assertEqual(parse_numeric_cell("1.234,56"), 1234.56) + + def test_european_multiple_thousands_groups(self): + self.assertEqual(parse_numeric_cell("1.234.567,89"), 1234567.89) + + +class CompoundCategoryPairingTests(unittest.TestCase): + def test_parse_sheet_pairs_danish_compound_index_with_count(self): + # "Lønindeks alle" is *detected* as an index column via + # COMPOUND_PATTERNS, but the derived category name must also lose the + # compound word or it can never pair with "Antal alle" ("alle" vs + # "lønindeks alle") - exactly the locale the compound support exists for. + ws = FakeWorksheet([ + ("Firma", "Antal alle", "Lønindeks alle"), + ("Example Corp", 12, 118.0), + ]) + + companies = parse_sheet(ws) + + self.assertEqual( + companies[0]["categories"]["alle"], + {"count": 12, "index": 118.0}, + ) + + def test_parse_sheet_sheet_level_us_locale_value(self): + ws = FakeWorksheet([ + ("Company", "Salary Index"), + ("Example Corp", "1,234.56"), + ]) + + companies = parse_sheet(ws) + + self.assertEqual( + companies[0]["categories"]["salary_index"], + {"index": 1234.56}, + ) diff --git a/tools/convert_salary_excel.py b/tools/convert_salary_excel.py index 685fdfc..2e8b6bb 100644 --- a/tools/convert_salary_excel.py +++ b/tools/convert_salary_excel.py @@ -65,7 +65,13 @@ def parse_numeric_cell(value): if not text: raise ValueError("not numeric") if "," in text and "." in text: - text = text.replace(".", "").replace(",", ".") + # The separator that appears last is the decimal separator: European + # "1.234,56" and US "1,234.56" are both unambiguous here, unlike the + # single-separator cases below. + if text.rfind(",") > text.rfind("."): + text = text.replace(".", "").replace(",", ".") + else: + text = text.replace(",", "") elif "," in text: if re.fullmatch(r"[+-]?\d+,\d{3}", text): raise ValueError("ambiguous comma separator") @@ -95,10 +101,18 @@ def header_matches(header, patterns): def strip_type_patterns(header, patterns): - """Remove count/index words from a header to derive a category name.""" + """Remove count/index words from a header to derive a category name. + + Mirrors ``header_matches``: patterns strip as whole tokens, and any + pattern also listed in ``COMPOUND_PATTERNS`` additionally strips as a + substring - otherwise a compound header like "Lønindeks alle" keeps the + type word in its category name and can never pair with "Antal alle". + """ name = header.lower() for p in patterns: name = re.sub(rf"(? Date: Wed, 19 Aug 2026 19:50:11 +0200 Subject: [PATCH 04/25] fix(reset): include documents/postings/ in the documents scope /reset's preview, delete block, and scope description all skipped documents/postings/ - the drop folder for hand-pasted posting text, documented in documents/README.md and protected as personal data by security_guards.py - and then asserted "The documents/ folder is now empty." The new test derives the folder list from the git tree, so any future drop folder fails it until /reset covers it. Review finding F26 (2026-08-19). Co-Authored-By: Claude Opus 5 (1M context) --- .claude/commands/reset.md | 8 +++-- CHANGELOG.md | 7 ++++ tests/test_reset_command.py | 72 +++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 tests/test_reset_command.py diff --git a/.claude/commands/reset.md b/.claude/commands/reset.md index ef62b97..a9df6b2 100644 --- a/.claude/commands/reset.md +++ b/.claude/commands/reset.md @@ -20,7 +20,7 @@ If `$ARGUMENTS` is empty or does not contain a recognized scope keyword, ask: > > - **`profile`** — Clears candidate data from the skill files (profile, behavioral, STAR examples, profile statements). The framework structure and writing rules are preserved. Use this to re-run `/setup` from scratch. > -> - **`documents`** — Deletes all files you've placed in the `documents/` folder (CV PDFs, LinkedIn export, diplomas, references, past applications). The folder structure and `README.md` are preserved. +> - **`documents`** — Deletes all files you've placed in the `documents/` folder (CV PDFs, LinkedIn export, diplomas, references, pasted job postings, past applications). The folder structure and `README.md` are preserved. > > - **`all`** — Both of the above. > @@ -68,7 +68,7 @@ The following files are NOT touched (they contain framework rules, not candidate ### If scope includes `documents`: -Use Glob to list all files present in `documents/cv/`, `documents/linkedin/`, `documents/diplomas/`, `documents/references/`, and `documents/applications/`. Present as: +Use Glob to list all files present in `documents/cv/`, `documents/linkedin/`, `documents/diplomas/`, `documents/references/`, `documents/postings/`, and `documents/applications/`. Present as: ``` ## Documents reset will delete: @@ -85,6 +85,9 @@ documents/diplomas/ documents/references/ - [filename] or "(empty)" +documents/postings/ + - [filename] or "(empty)" + documents/applications/ - [subfolder/filename] or "(empty)" @@ -193,6 +196,7 @@ rm -f documents/cv/* rm -f documents/linkedin/* rm -f documents/diplomas/* rm -f documents/references/* +rm -f documents/postings/* rm -rf documents/applications/*/ ``` diff --git a/CHANGELOG.md b/CHANGELOG.md index c067eb1..13e077b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,13 @@ per-file diff commands. ### Fixed +- **`/reset documents` now clears `documents/postings/`** - the drop folder for + hand-pasted job posting text was absent from the preview, the delete block, and the + user-facing scope description, after which the command told the user "The `documents/` + folder is now empty" - false whenever postings were present, and they are exactly the + personal residue a reset exists to clear. A new `tests/test_reset_command.py` derives + the folder list from the git tree, so any future drop folder fails the test until + `/reset` covers it. - **`convert_salary_excel.py` no longer corrupts US/UK-formatted numbers 1000x** - the both-separators branch always assumed European locale, so a `"1,234.56"` cell was silently converted to `1.23456` and written into `salary_data.json`. The rule is now diff --git a/tests/test_reset_command.py b/tests/test_reset_command.py new file mode 100644 index 0000000..e8919ac --- /dev/null +++ b/tests/test_reset_command.py @@ -0,0 +1,72 @@ +"""Guards for /reset's documents scope. + +/reset ends its documents pass by telling the user "The `documents/` +folder is now empty." That statement is only true if every personal-data +drop folder is actually covered by both the Step 1 preview and the +Step 3 delete block. `documents/postings/` was missing from both while +being documented in documents/README.md and protected as personal data +by tools/security_guards.py (review finding F26, 2026-08-19), so a reset +silently kept the user's hand-pasted job postings. + +The folder list is derived from the repository tree, so adding a new +drop folder under documents/ fails this test until /reset covers it. +""" +import re +import subprocess +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +RESET = REPO / ".claude" / "commands" / "reset.md" + + +def tracked_document_subfolders(): + """Names of documents/ subfolders tracked in git (ignores local noise).""" + out = subprocess.run( + ["git", "ls-files", "documents/"], + cwd=REPO, + capture_output=True, + text=True, + check=True, + ).stdout + folders = set() + for line in out.splitlines(): + parts = line.split("/") + if len(parts) >= 3: # documents// + folders.add(parts[1]) + return folders + + +class TestResetCoversEveryDocumentsSubfolder(unittest.TestCase): + def setUp(self): + self.text = RESET.read_text(encoding="utf-8") + self.folders = tracked_document_subfolders() + # The tree must actually contain the folders this test is about, + # or the assertions below would pass vacuously. + self.assertGreaterEqual(len(self.folders), 5, self.folders) + + def test_preview_lists_every_subfolder(self): + missing = [ + f for f in sorted(self.folders) if f"documents/{f}/" not in self.text + ] + self.assertEqual( + missing, + [], + "reset.md's preview never mentions these documents/ subfolders, " + f"so the user confirms a deletion list that omits them: {missing}", + ) + + def test_delete_block_removes_every_subfolder(self): + deleted = set(re.findall(r"rm -r?f documents/(\w+)/", self.text)) + missing = sorted(self.folders - deleted) + self.assertEqual( + missing, + [], + "reset.md's delete block has no rm line for these documents/ " + 'subfolders, yet the command then claims "The `documents/` ' + f'folder is now empty.": {missing}', + ) + + +if __name__ == "__main__": + unittest.main() From 9ab697de6412806963ce0c9d7f93f2c0356d3d61 Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 19:52:21 +0200 Subject: [PATCH 05/25] fix(evaluation): update stale Language Gate preamble to reflect tracking 04-job-evaluation.md still said the gate result "is not a field /scrape or /rank track" - true when the gate was introduced, false since /rank began persisting language_gate/language_note as shortlist veto fields and /scrape began surfacing the flag. The authoritative framework file taught agents the opposite of rank.md's own persistence rule. New coupling test pins that the section names the tracked fields and never reverts to the untracked claim. framework_version 1.2.3 -> 1.2.4. Review finding F24 (2026-08-19). Co-Authored-By: Claude Opus 5 (1M context) --- .../04-job-evaluation.md | 4 +-- CHANGELOG.md | 8 +++++ tests/test_rank_command.py | 30 +++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/.claude/skills/job-application-assistant/04-job-evaluation.md b/.claude/skills/job-application-assistant/04-job-evaluation.md index b11eb07..376ced9 100644 --- a/.claude/skills/job-application-assistant/04-job-evaluation.md +++ b/.claude/skills/job-application-assistant/04-job-evaluation.md @@ -1,5 +1,5 @@ --- -framework_version: 1.2.3 +framework_version: 1.2.4 --- # Job Evaluation Framework @@ -32,7 +32,7 @@ A role that fails this gate is not scored and not drafted. Everything below appl ## Language Gate — run before scoring -No dimension or gate anywhere in this framework currently checks a posting's language requirements against what the candidate actually speaks - it is not one of the five Scoring Dimensions below, not a field `/scrape` or `/rank` track, and not something `/apply`'s language detection (Step 1, which already extracts a posting's required language generically) has anywhere to report to. This gate adds that check, structured the same way as the Eligibility Gate above: read the posting, classify against profile data, and treat a hard mismatch as FAIL before scoring. +This gate checks a posting's language requirements against what the candidate actually speaks. It is not one of the five Scoring Dimensions below - it runs before them, structured the same way as the Eligibility Gate above: read the posting, classify against profile data, and treat a hard mismatch as FAIL before scoring. Its verdict is tracked downstream: `/rank` records the result as `language_gate` (PASS/FAIL/FLAG) with a supporting `language_note`, persists both into `seen_jobs.json`, and treats a FAIL as a shortlist veto; `/scrape` surfaces the flag in its results table and carries a language-override rule for postings whose ad language differs from the role's working language. `/apply`'s language detection (Step 1, which extracts a posting's required language generically) feeds this same check. Read the posting's language requirements as stated for **the role itself** — not the language the ad happens to be written in. A posting written in a language you don't work in, for a role that only needs languages you do work in on the job, passes fine; only an explicit job-condition requirement ("fluent X required," "must communicate with the Y team in Z") triggers this check. For each language the posting requires as a job condition, compare it against your Languages table in CLAUDE.md / `01-candidate-profile.md`: diff --git a/CHANGELOG.md b/CHANGELOG.md index 13e077b..acf4217 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,14 @@ per-file diff commands. ### Fixed +- **Language Gate preamble no longer claims the gate is untracked** (`framework_version` + 1.2.3 -> 1.2.4 in `04-job-evaluation.md`) - the paragraph still said the result "is not + a field `/scrape` or `/rank` track", written before the gate was wired into both + consumers. An agent reading the authoritative framework file learned the opposite of + what `rank.md` itself insists on ("These veto fields are as important to persist as + the score itself"). The preamble now names `language_gate`/`language_note` and how each + consumer uses them; a coupling test in `tests/test_rank_command.py` keeps the framework + text honest about the tracking. - **`/reset documents` now clears `documents/postings/`** - the drop folder for hand-pasted job posting text was absent from the preview, the delete block, and the user-facing scope description, after which the command told the user "The `documents/` diff --git a/tests/test_rank_command.py b/tests/test_rank_command.py index bc20976..dbb8d06 100644 --- a/tests/test_rank_command.py +++ b/tests/test_rank_command.py @@ -20,6 +20,9 @@ except ImportError: REPO = Path(__file__).resolve().parent.parent COMMAND = REPO / ".claude" / "commands" / "rank.md" SCRAPER_SKILL = REPO / ".claude" / "skills" / "job-scraper" / "SKILL.md" +EVALUATION = ( + REPO / ".claude" / "skills" / "job-application-assistant" / "04-job-evaluation.md" +) def _sections(text: str) -> dict[str, str]: @@ -102,6 +105,33 @@ class RankCommandSpec(unittest.TestCase): "not only when /rank re-scores it", ) + def test_evaluation_framework_acknowledges_language_gate_tracking(self): + """04-job-evaluation.md is the authoritative file /rank tells its agents + to read. Its Language Gate preamble once said the gate result "is not a + field /scrape or /rank track" - written before the gate was wired into + both consumers, and never updated. An agent reading that learns the + opposite of what rank.md itself insists on ("These veto fields are as + important to persist as the score itself"). The framework text must name + the tracked fields and must not claim they are untracked.""" + text = EVALUATION.read_text(encoding="utf-8") + gate = text.partition("## Language Gate")[2].partition("\n## ")[0] + self.assertTrue(gate, "04-job-evaluation.md has no Language Gate section") + self.assertIn( + "language_gate", + gate, + "the Language Gate section must name the language_gate field /rank persists", + ) + self.assertIn( + "language_note", + gate, + "the Language Gate section must name the language_note field /rank persists", + ) + self.assertNotIn( + "not a field", + gate, + "stale claim: the gate result IS tracked by /scrape and /rank now", + ) + def test_step2_schema_includes_language_gate_fields(self): sections = _sections(COMMAND.read_text(encoding="utf-8")) step2 = sections.get("Step 2: Batch-Fetch and Score", "") From 57e82d2b59ad39176d619c20672802474fe63934 Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 19:53:29 +0200 Subject: [PATCH 06/25] fix(rank): treat non-ISO stored deadlines as absent in urgency and the sweep Rule 6's expiry sweep mutates status automatically from stored deadline values, yet had no rule for the non-ISO shapes portals have shipped into seen_jobs.json ("ASAP", DD.MM.YYYY, free text) - "ASAP" is incomparable and "01.09.2026" is ambiguous between 1 Sep and 9 Jan. /outcome, which merely displays dates, already carried the defensive-parse rule. A non-YYYY-MM-DD stored value is now handled like an absent one (left alone, never compared, never guessed at) and reported once with its portal. Includes the F24-style coupling test. Review finding F17 (2026-08-19). Co-Authored-By: Claude Opus 5 (1M context) --- .claude/commands/rank.md | 4 ++-- CHANGELOG.md | 7 +++++++ tests/test_rank_command.py | 19 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/.claude/commands/rank.md b/.claude/commands/rank.md index e7d6e85..caafce5 100644 --- a/.claude/commands/rank.md +++ b/.claude/commands/rank.md @@ -73,8 +73,8 @@ Back in the main context, for each scored job: 2. Map to the framework's verdict bands (Strong Fit 75+, Good Fit 60-74, Moderate Fit 45-59, Weak Fit 30-44, Poor Fit <30). 3. **Location veto:** `FAIL` (e.g. requires relocation) excludes the job from the shortlist no matter the score - list it separately with the reason. `FLAG` (e.g. heavy travel) stays in the ranking but carries a visible ⚠ marker for the user to judge. 4. **Language veto:** `language_gate: FAIL` (posting requires a language the candidate hasn't declared at all) excludes the job from the shortlist, same as a location FAIL - list it under "Excluded" with the quoted requirement from `language_note`. `language_gate: FLAG` (declared language, requirement reads above the declared level) stays in the ranking with a visible ⚠ marker and `language_note` shown alongside the score, same treatment as a location FLAG. -5. **Deadline urgency:** a deadline within 7 days gets a 🔥 marker and wins ties. A deadline that has already passed moves the job to `expired`. Take the deadline from the scoring agent's Step 2 JSON for a job scored in this run, and from the stored `deadline` in `seen_jobs.json` for one that already carries it - a stored value costs no fetch, so urgency is re-derived on every run without re-reading the posting. When both exist and disagree, the freshly scored value wins and replaces the stored one. -6. **Expiry sweep over already-ranked entries.** Before presenting, check the stored `deadline` of every `ranked` entry this run did not re-score. Any whose deadline has passed becomes `expired`; any within 7 days is listed under a short **Closing soon** heading in Step 5 with its 🔥 marker. This needs no fetch and no agent - it is a date comparison against values already on disk, and it is what finally enforces `/scrape`'s "only open positions" rule beyond the moment of fetching. **An entry with no stored `deadline` is left alone, never guessed at** - most entries predate the column, and inferring a deadline from `first_seen` would retire jobs on a date nobody set. `--all` re-scores entries of any status including `expired`, so a job the sweep retired can still be revived by a later `--all` that re-fetches it and finds the posting live: the sweep is reversible, which is what makes an automated status change acceptable here at all. +5. **Deadline urgency:** a deadline within 7 days gets a 🔥 marker and wins ties. A deadline that has already passed moves the job to `expired`. Take the deadline from the scoring agent's Step 2 JSON for a job scored in this run, and from the stored `deadline` in `seen_jobs.json` for one that already carries it - a stored value costs no fetch, so urgency is re-derived on every run without re-reading the posting. When both exist and disagree, the freshly scored value wins and replaces the stored one. A stored value that does not parse as `YYYY-MM-DD` is skipped for urgency as well - rule 6's defensive-parse rule applies wherever a stored deadline is compared. +6. **Expiry sweep over already-ranked entries.** Before presenting, check the stored `deadline` of every `ranked` entry this run did not re-score. Any whose deadline has passed becomes `expired`; any within 7 days is listed under a short **Closing soon** heading in Step 5 with its 🔥 marker. This needs no fetch and no agent - it is a date comparison against values already on disk, and it is what finally enforces `/scrape`'s "only open positions" rule beyond the moment of fetching. **An entry with no stored `deadline` is left alone, never guessed at** - most entries predate the column, and inferring a deadline from `first_seen` would retire jobs on a date nobody set. **Parse stored deadlines defensively:** a stored value that is not a `YYYY-MM-DD` date is treated exactly like an absent one - left alone, never compared, never guessed at - and reported once in the Step 5 summary with its portal, so the bad value gets traced to its source instead of silently steering the sweep (portals have shipped `"ASAP"`, `DD.MM.YYYY`, and free-text deadline shapes into stored data). `--all` re-scores entries of any status including `expired`, so a job the sweep retired can still be revived by a later `--all` that re-fetches it and finds the posting live: the sweep is reversible, which is what makes an automated status change acceptable here at all. Sort by overall score (descending), urgency as tiebreaker. diff --git a/CHANGELOG.md b/CHANGELOG.md index acf4217..565ca4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,13 @@ per-file diff commands. ### Fixed +- **`/rank`'s expiry sweep parses stored deadlines defensively** - the sweep changes + status automatically from a date comparison against values on disk, but portals have + shipped non-ISO shapes into `seen_jobs.json` (`"ASAP"`, `DD.MM.YYYY`, free text), and + `/rank` had no rule for them while the display-only `/outcome` already did. A stored + deadline that is not `YYYY-MM-DD` is now treated exactly like an absent one wherever a + stored deadline is compared (urgency and sweep), and reported once with its portal. + Pinned by `tests/test_rank_command.py`. - **Language Gate preamble no longer claims the gate is untracked** (`framework_version` 1.2.3 -> 1.2.4 in `04-job-evaluation.md`) - the paragraph still said the result "is not a field `/scrape` or `/rank` track", written before the gate was wired into both diff --git a/tests/test_rank_command.py b/tests/test_rank_command.py index dbb8d06..356e805 100644 --- a/tests/test_rank_command.py +++ b/tests/test_rank_command.py @@ -132,6 +132,25 @@ class RankCommandSpec(unittest.TestCase): "stale claim: the gate result IS tracked by /scrape and /rank now", ) + def test_sweep_parses_stored_deadlines_defensively(self): + """Rule 6's expiry sweep mutates status automatically from stored + deadline values, and portals have shipped non-ISO shapes into + seen_jobs.json ("ASAP" from jobindex, DD.MM.YYYY from jobbank, + free text from jobdanmark's detail fallback). /outcome carries a + defensive date-parse rule for mere display; the command that + silently changes state needs one at least as much.""" + text = COMMAND.read_text(encoding="utf-8") + self.assertIn( + "Parse stored deadlines defensively", + text, + "rule 6's sweep must state the defensive-parse rule", + ) + self.assertRegex( + text, + r"not a `YYYY-MM-DD` date[^.]*treated exactly like an absent one", + "a non-ISO stored deadline must be handled as absent, not compared or guessed at", + ) + def test_step2_schema_includes_language_gate_fields(self): sections = _sections(COMMAND.read_text(encoding="utf-8")) step2 = sections.get("Step 2: Batch-Fetch and Score", "") From 0e054f16e770a7fc4deca4824361f4de883a96ab Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 19:54:32 +0200 Subject: [PATCH 07/25] fix(upskill): give Step 3.3 a rule for blank fit_rating rows /outcome-created tracker rows (applications made outside the workflow) never got a fit evaluation, so fit_rating is blank - and Step 3.3's weight formula divides by it with no stated rule. Blank read as 0 means weight 1.0, the maximum: the job the framework knows least about would dominate the heatmap and the learning plan. Blank now falls back to a matched ranked entry's rank_score, else skip+count+report once - the same pattern the skill already applies to missing gaps. Review finding F29 (2026-08-19). Co-Authored-By: Claude Opus 5 (1M context) --- .claude/skills/upskill/SKILL.md | 2 +- CHANGELOG.md | 8 ++++++++ tests/test_upskill_skill.py | 20 ++++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/.claude/skills/upskill/SKILL.md b/.claude/skills/upskill/SKILL.md index 5859abb..57b5822 100644 --- a/.claude/skills/upskill/SKILL.md +++ b/.claude/skills/upskill/SKILL.md @@ -56,7 +56,7 @@ This mode now merges two sources — tracker rows (Step 2.1) and ranked postings 1. **Dedupe.** Match tracker rows against ranked entries on case-insensitive company + role (casefold + strip on both fields) — the same match `/notion-sync`'s Step 2 describes. A job present in both counts once. 2. **Recorded gaps beat inferred skills.** For any job that has a recorded `gaps` array (from a ranked entry, or from a tracker row that matched one), use those gap bullets directly as the skill list for that job instead of inferring from `role`/`sector`/`notes`. For a ranked-only job with no `gaps` (already skipped and counted in Step 2.3) or a tracker-only row, fall back to inferring likely required skills from `role`, `sector`, and `notes` — optionally WebFetch the row's `source` URL for more detail, but skip if the URL is missing or dead. -3. **One weight per job**, both 0–100 on the same scale: `(100 - fit_rating) / 100` for tracker rows, `(100 - rank_score) / 100` for ranked-only rows. If a job is in both (Step 3.1 matched it), prefer the tracker's numeric `fit_rating` for the weight. +3. **One weight per job**, both 0–100 on the same scale: `(100 - fit_rating) / 100` for tracker rows, `(100 - rank_score) / 100` for ranked-only rows. If a job is in both (Step 3.1 matched it), prefer the tracker's numeric `fit_rating` for the weight. A **blank or non-numeric `fit_rating`** (rows `/outcome` creates for applications made outside the workflow never got a fit evaluation) contributes no weight: fall back to a matched ranked entry's `rank_score` when Step 3.1 found one, otherwise skip the row, count it, and report the count once in the terminal — the same treatment Step 2.3 gives a missing `gaps` field, and for the same reason. Never treat a blank as 0: that reads as weight 1.0, the maximum, and lets the one job the framework knows nothing about dominate the heatmap. 4. **Score.** Build a **skill frequency map**: for each extracted skill (recorded gap bullet or inferred skill), count how many jobs mention it, then multiply each job's contribution by its weight from Step 3.3. Track whether each contribution came from a recorded gap or an inferred one, for Step 5's provenance column. Final score for each skill: `sum of (weight × occurrence)` across all jobs. diff --git a/CHANGELOG.md b/CHANGELOG.md index 565ca4a..3c96fb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,14 @@ per-file diff commands. ### Fixed +- **`/upskill` no longer divides by a blank `fit_rating`** - `/outcome` creates tracker + rows for applications made outside the workflow with no fit evaluation, so their + `fit_rating` is blank, and Step 3.3's `(100 - fit_rating) / 100` had no rule for that. + The naive blank-as-0 reading yields weight 1.0 (the maximum), letting the one job the + framework knows nothing about dominate the skill-gap heatmap. A blank or non-numeric + `fit_rating` now falls back to a matched ranked entry's `rank_score`, else the row is + skipped, counted, and reported once - mirroring the skill's own missing-`gaps` + handling. Pinned by `tests/test_upskill_skill.py`. - **`/rank`'s expiry sweep parses stored deadlines defensively** - the sweep changes status automatically from a date comparison against values on disk, but portals have shipped non-ISO shapes into `seen_jobs.json` (`"ASAP"`, `DD.MM.YYYY`, free text), and diff --git a/tests/test_upskill_skill.py b/tests/test_upskill_skill.py index 7327fd2..62eaa46 100644 --- a/tests/test_upskill_skill.py +++ b/tests/test_upskill_skill.py @@ -82,6 +82,26 @@ class UpskillSkillSpec(unittest.TestCase): self.assertIn("(100 - fit_rating) / 100", step3) self.assertIn("(100 - rank_score) / 100", step3) + def test_step3_handles_blank_fit_rating(self): + """/outcome creates tracker rows for applications made outside the + workflow, and no rule anywhere fills fit_rating on that path - yet + Step 3.3 divides by it. A naive read of blank as 0 yields weight 1.0 + (the maximum), making the one job the framework knows nothing about + dominate the heatmap. The skill already handles missing gaps with + skip+count+report; the same pattern must cover fit_rating.""" + sections = _sections(SKILL.read_text(encoding="utf-8")) + step3 = sections.get("Step 3: Pass 1 — Hard Skill Diff", "") + self.assertIn( + "blank or non-numeric `fit_rating`", + step3, + "Step 3.3 must state what happens to a row whose fit_rating is blank", + ) + self.assertIn( + "Never treat a blank as 0", + step3, + "the blank-as-0 reading (weight 1.0, maximum) is the failure mode and must be forbidden explicitly", + ) + def test_step5_heatmap_shows_gap_provenance(self): sections = _sections(SKILL.read_text(encoding="utf-8")) step5 = sections.get("Step 5: Build Gap Heatmap", "") From 4ed5fee221fb1081a78fd8a8be0fef50d2068cb2 Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 19:55:48 +0200 Subject: [PATCH 08/25] fix(gmail-sync): replace in:inbox with -in:sent -in:drafts in:inbox matches only messages currently in the Inbox, so it silently excluded archived mail and everything routed past the inbox by a label-and-archive filter - exactly the mail matched by the job-search label Step 3.1 hunts for. The stated intent ("skip sent/drafts") is what the negative operators express. Failure mode was silent under-detection that read as "no updates" and left the tracker stale. Review finding F18 (2026-08-19). Co-Authored-By: Claude Opus 5 (1M context) --- .claude/commands/gmail-sync.md | 4 +-- CHANGELOG.md | 7 ++++++ tests/test_gmail_sync_command.py | 43 ++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 tests/test_gmail_sync_command.py diff --git a/.claude/commands/gmail-sync.md b/.claude/commands/gmail-sync.md index f2a56fb..8b6be57 100644 --- a/.claude/commands/gmail-sync.md +++ b/.claude/commands/gmail-sync.md @@ -46,9 +46,9 @@ Lookback window: `since ` argument if given, else `state.last_sync` if set - A quoted-name OR-group of the open applications' company names, e.g. `{"Acme Corp" "BigCo"}` - A sender-domain OR-group of common ATS platforms: `{from:greenhouse.io from:lever.co from:myworkday.com from:ashbyhq.com from:smartrecruiters.com from:icims.com from:bamboohr.com}` - The lookback bound, e.g. `newer_than:30d` or `after:2026/06/15` - - `in:inbox` (skip sent/drafts - status signals come from what employers send you, not what you sent them) + - `-in:sent -in:drafts` (status signals come from what employers send you, not what you sent them; the negative operators keep **archived** mail and label-filtered mail in scope - restricting to the Inbox instead would silently drop both, including exactly the mail matched by the job-search label from step 1, since the standard filter that applies such a label also archives it) -Example: `newer_than:30d in:inbox ({"Acme Corp" "BigCo"} OR {from:greenhouse.io from:lever.co from:myworkday.com from:ashbyhq.com})` +Example: `newer_than:30d -in:sent -in:drafts ({"Acme Corp" "BigCo"} OR {from:greenhouse.io from:lever.co from:myworkday.com from:ashbyhq.com})` 4. Call `search_threads` with `view: THREAD_VIEW_MINIMAL`, `pageSize: 50`, paginating via `pageToken` until exhausted or results are clearly outside the relevant window. diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c96fb3..5f4416e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,13 @@ per-file diff commands. ### Fixed +- **`/gmail-sync` no longer restricts its search to the Inbox** - the query used + `in:inbox` to "skip sent/drafts", but that operator also excludes every archived + message, and self-defeatingly the mail matched by the very job-search label Step 3.1 + hunts for (the standard filter that applies such a label also archives). The query now + uses `-in:sent -in:drafts`, which matches the stated intent exactly. The failure mode + was silent under-detection: a missed rejection or interview invite read as "no + updates". Pinned by the new `tests/test_gmail_sync_command.py`. - **`/upskill` no longer divides by a blank `fit_rating`** - `/outcome` creates tracker rows for applications made outside the workflow with no fit evaluation, so their `fit_rating` is blank, and Step 3.3's `(100 - fit_rating) / 100` had no rule for that. diff --git a/tests/test_gmail_sync_command.py b/tests/test_gmail_sync_command.py new file mode 100644 index 0000000..deb9b49 --- /dev/null +++ b/tests/test_gmail_sync_command.py @@ -0,0 +1,43 @@ +"""Guards for /gmail-sync's Gmail query semantics. + +The command's stated intent is "skip sent/drafts - status signals come +from what employers send you". `in:inbox` does not mean that: it matches +only messages currently IN the inbox, so it also excludes every archived +message - and, self-defeatingly, the mail matched by the very +job-search label Step 3.1 hunts for, because the standard filter that +applies such a label also archives ("skip the inbox"). The correct +operators for the stated intent are `-in:sent -in:drafts` (review +finding F18, 2026-08-19). The failure mode is silent under-detection: a +missed rejection or interview invite just looks like "no updates". +""" +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +GMAIL_SYNC = REPO / ".claude" / "commands" / "gmail-sync.md" + + +class TestGmailQueryOperators(unittest.TestCase): + def setUp(self): + self.text = GMAIL_SYNC.read_text(encoding="utf-8") + + def test_query_excludes_sent_and_drafts_explicitly(self): + self.assertIn( + "-in:sent -in:drafts", + self.text, + "the query must exclude sent/drafts with negative operators, " + "which keep archived and label-filtered mail in scope", + ) + + def test_query_never_restricts_to_the_inbox(self): + self.assertNotIn( + "in:inbox", + self.text.replace("-in:sent", "").replace("-in:drafts", ""), + "in:inbox silently drops archived mail and everything a " + "label-and-archive filter routed past the inbox - exactly the " + "mail the label search in Step 3.1 exists to find", + ) + + +if __name__ == "__main__": + unittest.main() From c20458d768677cae22694ae6c24c0bf24b6b93fb Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 19:57:46 +0200 Subject: [PATCH 09/25] test(robots-check): pin the tie-break clause and the browser-UA fallback The tie-break test listed Disallow first - the one ordering where deleting the clause changes nothing - and gate()'s read-the-policy-as-a- browser recovery (the Barclays-class case 09-web-research.md documents as covered) had no test. Both gaps are guard code whose breakage is silent by construction. Mutation-verified: the tie-break deletion and the UA-loop reduction each now fail exactly the new tests. Review findings F21 and F30 (2026-08-19). Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 7 ++++++ tests/test_robots_check.py | 50 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f4416e..99aa0fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,13 @@ per-file diff commands. ### Added +- **Discriminating tests for `robots_check`'s tie-break and browser-UA fallback** - the + existing tie test put Disallow first, the one ordering that cannot detect deletion of + the tie-break clause; and the browser-readback recovery that `09-web-research.md` + claims is covered had no test at all. Three new tests in `tests/test_robots_check.py` + pin the Allow-first tie, the 403-to-honest/200-to-browser recovery, and that a + browser-fetched policy is still obeyed strictly. Each was mutation-verified: deleting + the tie-break clause or the UA fallback now fails the suite. - **LaTeX special-character guidance for CVs** (`framework_version` 1.4.1 -> 1.4.2 in `05-cv-templates.md`, 1.0.1 -> 1.0.2 in `06-cover-letter-templates.md`) - `05` gains a "LaTeX Special Characters" section and `06`'s existing one is completed beyond `\_`/`\&`. diff --git a/tests/test_robots_check.py b/tests/test_robots_check.py index 08c7860..beb9286 100644 --- a/tests/test_robots_check.py +++ b/tests/test_robots_check.py @@ -52,6 +52,13 @@ class TestPathRules(unittest.TestCase): """Cautious tie-break: Google resolves ties to Allow, we do not.""" self.assertFalse(allowed("User-agent: *\nDisallow: /a\nAllow: /a\n", "*", "/a")) + def test_equal_specificity_tie_goes_to_disallow_when_allow_listed_first(self): + """The only ordering that exercises the tie-break clause: with Allow + first, deleting the clause makes the first rule at a given length win + and Allow would leak through. The Disallow-first sibling above cannot + detect that mutation (review finding F21, 2026-08-19).""" + self.assertFalse(allowed("User-agent: *\nAllow: /a\nDisallow: /a\n", "*", "/a")) + def test_api_block_and_sibling_path(self): self.assertFalse(allowed(JOBUP, "*", "/api/v1/public/search")) self.assertTrue(allowed(JOBUP, "*", "/en/jobs/")) @@ -130,6 +137,49 @@ class TestSoftTwoHundred(unittest.TestCase): self.assertEqual(rc, 1) self.assertIn("not a robots.txt", msg) + def test_gate_reads_policy_as_browser_when_honest_request_is_refused(self): + """09-web-research.md's Barclays-class recovery: the policy file itself + returns 403 to Claude-User and 200 to a browser, and the checker must + then read it as a browser and obey it strictly. This is gate()'s UA + fallback loop, previously untested despite the doc's coverage claim + (review finding F30, 2026-08-19).""" + import robots_check + + original = robots_check._fetch + + def waf(url, ua): + if ua == robots_check.BROWSER: + return ("User-agent: *\nAllow: /\n", 200) + return ("403 Forbidden", 403) + + robots_check._fetch = waf + try: + rc, msg = robots_check.gate("https://waf.example/jobs") + finally: + robots_check._fetch = original + self.assertEqual(rc, 0) + self.assertIn("ALLOWED", msg) + + def test_gate_obeys_a_browser_fetched_policy_strictly(self): + """The fallback must not fail open: a policy readable only as a browser + still disallows what it disallows.""" + import robots_check + + original = robots_check._fetch + + def waf(url, ua): + if ua == robots_check.BROWSER: + return ("User-agent: *\nDisallow: /jobs\n", 200) + return ("403 Forbidden", 403) + + robots_check._fetch = waf + try: + rc, msg = robots_check.gate("https://waf.example/jobs") + finally: + robots_check._fetch = original + self.assertEqual(rc, 1) + self.assertIn("DISALLOWED", msg) + def test_a_genuinely_empty_robots_is_still_allow_all(self): """RFC 9309: an empty file permits everything. Do not over-correct.""" self.assertTrue(is_robots_body("")) From 9a074b262d71f90ae2a66408ec9485fdc4841df2 Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 19:59:22 +0200 Subject: [PATCH 10/25] test(lint-skills): cover check_skill and check_command, not just settings The linter's main job - frontmatter keys, allowed-tools targets, the command title rule - had zero assertions; deleting the missing- allowed-tools error left the suite green. The fixture's yaml stub now parses the flat frontmatter the fixtures write instead of returning a canned mapping, and four new cases pin both check functions. Mutation-verified against the real linter. Review finding F23 (2026-08-19). Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 6 ++++ tests/test_lint_skills.py | 70 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99aa0fd..7099c93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ per-file diff commands. ### Added +- **Tests for `lint_skills.py`'s skill and command checks** - only `check_settings()` + had coverage; the linter's main job (frontmatter keys, `allowed-tools` targets + existing, the `# /` command title rule) was unasserted, so deleting the + missing-allowed-tools error survived the whole suite. Four new cases in + `tests/test_lint_skills.py`, with the fixture's yaml stub upgraded to parse the real + frontmatter. Mutation-verified. - **Discriminating tests for `robots_check`'s tie-break and browser-UA fallback** - the existing tie test put Disallow first, the one ordering that cannot detect deletion of the tie-break clause; and the browser-readback recovery that `09-web-research.md` diff --git a/tests/test_lint_skills.py b/tests/test_lint_skills.py index 640e4aa..a7f57b3 100644 --- a/tests/test_lint_skills.py +++ b/tests/test_lint_skills.py @@ -29,11 +29,19 @@ class LinterRepoFixture(unittest.TestCase): shutil.copy(LINTER_SCRIPT, tools / "lint_skills.py") # The Python-test CI job does not install PyYAML; the separate lint job # does. These settings-focused tests only need a valid frontmatter map. + # The stub parses simple "key: value" lines, enough for the flat + # frontmatter these fixtures write, so the checks under test see the + # actual file content instead of a canned mapping. (tools / "yaml.py").write_text( "class YAMLError(Exception):\n" " pass\n\n" - "def safe_load(_text):\n" - " return {'name': 'example', 'description': 'Example skill'}\n", + "def safe_load(text):\n" + " result = {}\n" + " for line in (text or '').splitlines():\n" + " if ':' in line:\n" + " key, _, value = line.partition(':')\n" + " result[key.strip()] = value.strip()\n" + " return result\n", encoding="utf-8", ) @@ -103,5 +111,63 @@ class SettingsShapeTests(LinterRepoFixture): self.assertEqual(result.returncode, 1) self.assertIn("expected permissions.allow to be a list", result.stdout) self.assertNotIn("Traceback", result.stderr) +class SkillAndCommandCheckTests(LinterRepoFixture): + """check_skill()/check_command() are the linter's main job and were + previously untested - only check_settings() had coverage, so deleting + e.g. the missing-allowed-tools error survived the whole suite (review + finding F23, 2026-08-19).""" + + def write_skill(self, frontmatter: str): + skill = self.root / ".claude" / "skills" / "example" / "SKILL.md" + skill.write_text(frontmatter, encoding="utf-8") + + def test_allowed_tools_referencing_a_missing_file_fails(self): + self.write_skill( + "---\n" + "name: example\n" + "description: Example skill\n" + "allowed-tools: Bash(bun run .claude/skills/example/DOES_NOT_EXIST.ts *)\n" + "---\n" + ) + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("allowed-tools references a missing file", result.stdout) + self.assertIn("DOES_NOT_EXIST.ts", result.stdout) + + def test_allowed_tools_referencing_an_existing_file_passes(self): + target = self.root / ".claude" / "skills" / "example" / "cli.ts" + target.write_text("// present\n", encoding="utf-8") + self.write_skill( + "---\n" + "name: example\n" + "description: Example skill\n" + "allowed-tools: Bash(bun run .claude/skills/example/cli.ts *)\n" + "---\n" + ) + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_frontmatter_missing_description_fails(self): + self.write_skill("---\nname: example\ndescription:\n---\n") + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1) + self.assertIn("missing required key 'description'", result.stdout) + + def test_command_without_slash_title_fails(self): + command = self.root / ".claude" / "commands" / "setup.md" + command.write_text("# setup - missing the slash\n", encoding="utf-8") + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1) + self.assertIn("must start with a '# /' title", result.stdout) + + if __name__ == "__main__": unittest.main() From 65fbe8b8a4d0448acadfd16f3f2091d9d4a6c206 Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 20:00:25 +0200 Subject: [PATCH 11/25] test(framework-version): cover the CI gate that had zero tests check_framework_version.py guards fork-rebase safety (Gate E) and could be neutralised by a one-line change that reads as a refactor, with nothing in the repo noticing - a broken guard is silent by construction. Four new tests run the real script inside an isolated git repo: clean tree passes, unbumped edit fails, bumped edit passes, missing marker fails. Mutation-verified against the exact return-False disable the review demonstrated. Review finding F22 (2026-08-19). Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 6 ++ tests/test_check_framework_version.py | 110 ++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 tests/test_check_framework_version.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7099c93..41b6d3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ per-file diff commands. ### Added +- **Tests for `check_framework_version.py`** - the CI gate that stops a framework file + from being edited without a `framework_version` bump had zero tests, so the one-line + mutation `return meaningful_changes > 0` -> `return False` disabled it while the suite + stayed green. Four cases in the new `tests/test_check_framework_version.py` (clean + tree, unbumped edit, bumped edit, missing marker), each running the real script inside + an isolated git repo. Mutation-verified against that exact disable. - **Tests for `lint_skills.py`'s skill and command checks** - only `check_settings()` had coverage; the linter's main job (frontmatter keys, `allowed-tools` targets existing, the `# /` command title rule) was unasserted, so deleting the diff --git a/tests/test_check_framework_version.py b/tests/test_check_framework_version.py new file mode 100644 index 0000000..9562233 --- /dev/null +++ b/tests/test_check_framework_version.py @@ -0,0 +1,110 @@ +"""Guards for tools/check_framework_version.py - the CI gate itself. + +This gate is what stops a PR from editing a profile-bearing framework +file without bumping `framework_version` (the fork-rebase safety marker). +It ran in CI with zero tests, so a one-line mutation +(`return meaningful_changes > 0` -> `return False`) disabled it while +the whole suite stayed green (review finding F22, 2026-08-19). A broken +guard is silent by construction: nothing fails, it just stops catching. + +Each test builds an isolated git repo with the script copied inside it +(the script resolves ROOT from __file__), so the real repo is never read +or written. +""" +import os +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +SCRIPT = REPO_ROOT / "tools" / "check_framework_version.py" + +FRONTMATTER = "---\nframework_version: 1.0.0\n---\n" +BODY = "# Test framework file\n\nOriginal guidance sentence.\n" + + +class CheckerRepoFixture(unittest.TestCase): + def setUp(self): + self.root = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.root, ignore_errors=True) + + tools = self.root / "tools" + tools.mkdir() + shutil.copy(SCRIPT, tools / "check_framework_version.py") + + self.skill_dir = self.root / ".claude" / "skills" / "job-application-assistant" + self.skill_dir.mkdir(parents=True) + self.framework_file = self.skill_dir / "01-test-profile.md" + self.framework_file.write_text(FRONTMATTER + BODY, encoding="utf-8") + + self.git("init", "-q") + self.git("add", "-A") + self.git("commit", "-q", "-m", "base") + + def git(self, *args): + subprocess.run( + ["git", "-c", "user.name=test", "-c", "user.email=test@example.com", *args], + cwd=self.root, + check=True, + capture_output=True, + text=True, + ) + + def run_checker(self): + # Strip GitHub Actions variables so get_base_commit() takes the + # local path (uncommitted changes vs HEAD) regardless of where the + # test suite itself runs. + env = {k: v for k, v in os.environ.items() if not k.startswith("GITHUB_")} + return subprocess.run( + [sys.executable, str(self.root / "tools" / "check_framework_version.py")], + capture_output=True, + text=True, + env=env, + ) + + +class FrameworkVersionGateTests(CheckerRepoFixture): + def test_clean_tree_passes(self): + result = self.run_checker() + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("Framework Version Check: OK", result.stdout) + + def test_unbumped_edit_fails(self): + self.framework_file.write_text( + FRONTMATTER + BODY + "\nA new sentence without a version bump.\n", + encoding="utf-8", + ) + + result = self.run_checker() + + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("modified without bumping 'framework_version'", result.stdout) + + def test_bumped_edit_passes(self): + bumped = FRONTMATTER.replace("1.0.0", "1.0.1") + self.framework_file.write_text( + bumped + BODY + "\nA new sentence with a version bump.\n", + encoding="utf-8", + ) + + result = self.run_checker() + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_file_without_version_marker_fails(self): + (self.skill_dir / "02-unmarked.md").write_text( + "# No frontmatter at all\n", encoding="utf-8" + ) + + result = self.run_checker() + + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("missing 'framework_version'", result.stdout) + + +if __name__ == "__main__": + unittest.main() From 2edf8c41f159cbe6b3c1a5f8a7e803eec2cce5e7 Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 20:03:00 +0200 Subject: [PATCH 12/25] test(portals): cover linkedin card date/location and jobindex parseSearchPage The linkedin fixture was purpose-built for entity decoding and had no