fix(skills): author display labels for all unlabelled dictionary entries (#681)

[shubhransh-gupta]

Summary

• Authors branded label values for all 139 previously-unlabelled entries in jd-match/skills.ts, so recognized skills render as JavaScript, PostgreSQL, iOS, etc. instead of lowercase kebab ids.
• Bumps SKILLS_DICTIONARY_VERSION to 1.2 and tightens skills.test.ts: explicit lowercase-brand allowlist (iOS, dbt, jQuery, gRPC, scikit-learn, pandas, pytest) plus a new invariant that every entry has an authored label.
• Updates display-string expectations in affected tests (query-builder, extract-jd-terms, SkillTermGuidance). Matching, aliases, and id-based assertions are unchanged.

Egress note (from #681)

query.skills entries are sent verbatim to job feeds (tag=, search=). Casing changes from python → Python; matching remains case-insensitive on our side.

Test plan

• npm run verify passes locally (5875 tests)
• /jobs/ skills row shows branded labels beside free-text chips
• SkillTermGuidance "Already in your résumé" line uses labels, not ids
• Job search feeds still return results for canonical skills after casing change

Fixes #681

[shubhransh-gupta]

@s-annam — review ping for #681 👋

CI is green (verify + fallow succeeded). This is data-only in jd-match/skills.ts:

• Authored label for all 139 previously-unlabelled entries (brand casing: JavaScript, PostgreSQL, iOS, …)
• SKILLS_DICTIONARY_VERSION → 1.2
• Test updates for display-string expectations; matching/aliases unchanged

Same pattern as the #607/#594 label work you merged before — happy to adjust any label spellings. Thanks!

[s-annam]

Approving. The data change is clean and I verified it structurally rather than by reading 139 lines: parsing both sides of the diff, the dictionary is 176 entries before and after, with 0 ids added or removed, 0 alias arrays changed, 0 pre-existing labels altered, 0 duplicate labels, and unlabelled 139 → 0. That discharges #681's AC1/AC3/AC6 more convincingly than a spot-check would, and the brand casing itself is good — I read all 139 and found one debatable call (below). npm run verify is green on 2f3d3cc on my machine.

Three Secondary findings and four Nits, none merge-blocking. I could not push any of them — this is a fork PR, so the reviewer auto-fix path is closed by design; everything is prose or a one-click suggestion below.

Overlap note, for the maintainer only: #854 touches the identical five files for the same issue. I reviewed this PR strictly on its own merits and formed no view on which should land; flagging it only so the two are not merged independently.

Secondary

1. Six labels change the outbound search token, not just its case — the body's egress note under-describes it. (inline)

The body says "Casing changes from python → Python; matching remains case-insensitive on our side." That is true for 133 of the 139. For six, the label is not a case-variant of the id:

id	new label	outbound
cpp	C++	C%2B%2B
csharp	C#	C%23
tailwind	Tailwind CSS	Tailwind%20CSS
d3	D3.js	D3.js
huggingface	Hugging Face	Hugging%20Face
soc2	SOC 2	SOC%202

Case-insensitivity does not cover a token whose content changed. query.skills is also consumed verbatim by the local admission filter — buildQueryTermPatterns (src/lib/job-search/search.ts:161-166) lowercases each skill and compiles it into (?<![a-z0-9])<term>(?![a-z0-9]) against the posting text — so these six change what postings are admitted, which is the one thing #681's Scope section ruled out ("no matching change").

I traced each: C++, C#, D3.js (d3 was 2 chars and failed isSignificantSkillTerm, so it admitted nothing before), Hugging Face and SOC 2 all get better. The one plausible narrowing is Tailwind CSS — a posting saying only "Tailwind" no longer matches that term. Bounded, since matchesQuery is .some() across every query term.

Not blocking: mixed direction, mostly an improvement, and no user-visible break. But it is a real behaviour change outside the stated scope, described in the body as casing, and pinned by no test. Worth a line in the body and, ideally, a test on Tailwind CSS.

2. Two of #681's acceptance criteria are unimplemented. (inline)

• AC4 — "a résumé with ["SQL","AWS","iOS","machine learning"] produces chips SQL, AWS, iOS, Machine Learning. Asserted in query-builder.test.ts." No such test was added; iOS, the issue's own sharpest case, is asserted nowhere.
• AC5 — "missingTermLabel renders CI/CD/SQL, asserted." Not added. The existing JobQueryEditor.test.tsx calls (:664, :782) use missingTermLabel(...) as their own expected value, so they pin no literal.

I am not calling these blocking, and the reasoning should be on the record so you can overrule it knowingly: the new SKILLS.filter(s => !s.label) invariant is strictly stronger than either example. It proves the property for all 176 entries, and every render site named in the issue (deriveSkills, missingTermLabel at TermQualityAdvisory.tsx:83, extractSkillPass) reads the same idToLabel map, so the class cannot regress while that test is green. What is lost is a readable example at the render site, not coverage.

3. Two docblocks now assert an invariant this PR falsified. (body-only — both sit on lines the diff does not touch, so there is nothing to anchor to.)

• src/lib/jd-match/skills.ts:40 — "MUST START WITH A CAPITAL (or a digit) — skills.test.ts enforces it." Seven labels now do not, and the test carries an allowlist. This is the authoring contract for the very file this PR rewrote, and the issue explicitly wanted the next contributor to "meet this test before they meet this issue" — right now they meet a docblock that contradicts it. It should also record the new mandatory-label invariant (the ? on label?: string now survives only as getSkillIndex's total-function guard).
• src/components/features/TermQualityAdvisory.tsx:78-80 — "That only holds because every authored label is title-cased (#607); a lowercase one would render + people management." + iOS, + dbt, + pandas, + pytest, + gRPC, + jQuery, + scikit-learn now render. The behaviour is correct; the stated rationale is not.

Nits

4. Changelog entry is out of order — inserted between 1.0 and 1.1, so the list reads 1.0, 1.2, 1.1. (inline)

5. The stable-sort test's input changed, and the reason deserves a line. (inline, with a suggestion) The body says it "updates display-string expectations", but this hunk swapped ["go","rust"] for ["python","java"]. I checked whether that masked something, and it does the opposite — it repairs a previously-vacuous assertion. compileIndex (skills.ts:335-337) registers only aliases, never the id, and go's aliases are ["golang","go lang"] — so the literal "go" was never recognized. On main it fell through to titleCase → "Go", making indexOf("go") equal -1, and -1 < 0 passed for the wrong reason. Good change; it just reads like an unexplained edit.

6. Webpack — webpack's own docs deliberately style it lowercase, like dbt/pandas/pytest already in your allowlist. Author's call; it needs the allowlist entry too, so I have not made it a one-click suggestion. (inline)

7. The unticked egress box — I discharged it for you. #681's last AC wants the body to confirm a feed still returns results under the new casing. I measured all three live:

feed	lowercase	new casing
Jobicy tag=	python → 100	Python → 100
Remotive search=	postgresql → 14	PostgreSQL → 14
Arbeitnow search=	nodejs → 175	NodeJS → 175

All three are case-insensitive, so the casing half of the egress change is safe. Note this does not cover Secondary 1's six token changes, which are a different question.

Gates

verify green (fallow clean on the diff). Fixture-PII (3a), design-system/reuse (3b), style tokens (3c) and skill/script command review (3e) do not apply — no fixtures, no scripts/, no .claude/, and the only component file touched is a test.

---

Reviewed by: Claude Opus 5 (high)

[s-annam]

Secondary 1 — this is one of six labels that changes the search token, not its case.

query.skills is not only display text: buildQueryTermPatterns (src/lib/job-search/search.ts:161-166) lowercases each entry and compiles it into (?<![a-z0-9])<term>(?![a-z0-9]) against the posting title + description. So tailwind → Tailwind CSS changes the admission pattern from the token tailwind to the phrase tailwind css, and a posting that says only "Tailwind" no longer matches on this term.

The other five (cpp→C++, csharp→C#, d3→D3.js, huggingface→Hugging Face, soc2→SOC 2) all move the other way and are improvements — d3 in particular was 2 characters and failed isSignificantSkillTerm, so it admitted nothing at all before.

The label itself is right; Tailwind CSS is the brand. The ask is just that the PR body say the egress change is not only casing, since #681's Scope said "no matching change". A test on this specific entry would be the durable fix.

[s-annam]

Secondary 2 — this invariant is why I am not blocking on #681's AC4/AC5, and it is the strongest thing in the PR.

It is strictly stronger than the two example assertions the issue asked for: it proves the property for all 176 entries rather than the 4 named, and since deriveSkills, missingTermLabel (TermQualityAdvisory.tsx:83) and extractSkillPass all read the same idToLabel map, the class cannot regress while this is green.

What is still missing is a readable example at a render site — iOS, the issue's own headline case, is asserted nowhere, and the missingTermLabel calls in JobQueryEditor.test.tsx:664,782 use the helper as their own expected value, so they pin no literal. Worth adding; not worth a round-trip.

[s-annam]

Nit — out of order. The changelog above reads oldest-first (1.0, then 1.1), so inserting 1.2 here makes it 1.0, 1.2, 1.1. It belongs below the 1.1 block.

[s-annam]

Nit — webpack styles itself lowercase. webpack's own site and docs use lowercase deliberately, the same reason dbt, pandas, pytest and scikit-learn are already in your LOWERCASE_BRAND_LABELS allowlist.

Your call — it is defensible to sentence-case it — but if you take it, it needs the allowlist entry in skills.test.ts too, which is why I have not offered it as a one-click suggestion.

[s-annam]

Nit — worth a line of intent; this hunk is doing more than the body claims.

The body describes the test changes as "display-string expectations", but this one swapped the input from ["go", "rust", ...]. I checked whether that hid a failure — it does the reverse. compileIndex (skills.ts:335-337) registers only aliases, never the id, and go's aliases are ["golang", "go lang"], so the literal "go" was never recognized: on main it fell through to titleCase as "Go", indexOf("go") was -1, and -1 < 0 passed for entirely the wrong reason. The swap makes the assertion real.

That is good, and a reader six months from now should not have to re-derive it:

Suggested change

	// Canonical tier keeps its own relative order (Python before Java)...
	// Both inputs must be RECOGNIZED for this to test anything: `go`'s aliases
	// are ["golang", "go lang"] and `compileIndex` never registers the id, so
	// the literal "go" fell through to `titleCase` and `indexOf` returned -1 —
	// the old assertion passed vacuously. Canonical tier keeps résumé order...

[s-annam]

Approved — thanks, this is careful work. The dictionary audit holds up: 176 entries before and after, no id added or removed, no alias array touched, no pre-existing label altered, unlabelled 139 → 0.

Two things before I merge, both small:

1. Changelog ordering. The new 1.2 entry sits above 1.1 in the block at skills.ts. Move it below so the list stays chronological.

2. The body describes the egress change as casing, and it isn't. Six of the new labels are not case-variants of their id — cpp → C++, csharp → C#, tailwind → Tailwind CSS, d3 → D3.js, huggingface → Hugging Face, soc2 → SOC 2. That matters because query.skills is not display-only: searchPhrase() and primaryKeyword() in src/lib/job-search/providers/keywords.ts:27,37 build the outbound keyword string from it, so the label is the token we send to Jobicy/Remotive/Arbeitnow.

I traced each of the six and I'm satisfied the change is net-positive — five widen or fix (d3 previously failed isSignificantSkillTerm and admitted nothing), and only Tailwind CSS plausibly narrows. The three feeds also measured case-insensitive, so the casing-only labels are inert. So this is a description fix, not a code fix: please state the six non-casing labels and the egress effect in the PR body, which is what #681's AC 10 asked for.

One thing you should know, because it's a real find and it wasn't in your description: the ["go","rust"] → ["python","java"] swap at query-builder.test.ts:416 looks like it masks a failure and is the opposite — compileIndex never registers ids as aliases, and go's aliases are ["golang","go lang"], so indexOf("go") returned -1 on main and the ordering assertion was passing vacuously. Your swap makes it a real assertion. I've filed the underlying gap separately; no action needed from you here beyond a line in the body if you want the credit recorded.

Push those two and I'll merge. Tag @s-annam if you disagree with any of it — the egress reading in particular is worth pushing back on if you read it differently.