feat: JD matching v1 (deterministic skill + phrase coverage) - #41
Conversation
Adds a paste-a-JD textarea below the parser surface and a diagnostic JD-coverage panel: which terms from the JD show up in the resume text, which don't, with a weighted score. All in-browser, no network call. - src/lib/jd-match/skills.ts — ~120-entry curated skill dictionary with aliases, compiled into a single longest-first regex. - src/lib/jd-match/extract-jd-terms.ts — two-pass extractor (skill + noun-phrase) with boilerplate (EEO / benefits / pay) section stripping and per-term snippets for hover context. - src/lib/jd-match/coverage.ts — flat lowercased resume corpus, aliase- aware coverage check, weighted score (skill 1.0, noun 0.5). - src/components/features/JdMatch.tsx — covered/missing columns, N-of-M headline, snippet on hover. Copy stays diagnostic, not prescriptive. - src/App.tsx — textarea below Result; panel renders only when both a parsed resume and a non-empty JD are present. Tests: skills + extract + coverage + JdMatch — 25 new, suite green at 219/219. Typecheck + production build clean. Refs #2 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…score Two findings from re-reading issue #2 against my first pass: - The issue's Goal says "JD input alongside the existing resume dropzone". My first pass gated the textarea on `state.phase === "done"`, so users couldn't paste a JD until parsing finished. Move the textarea out of the gate so it's visible from the start; the match panel still waits for both inputs. Add a small hint when the user has typed a JD but hasn't dropped a resume yet. - The issue's Part 4 says "Headline number (the score) with the disclaimer 'diagnostic, not a verdict'". My first pass only showed N-of-M; the weighted score never surfaced. Add a one-line weighted-coverage readout (X/100, with per-source weights) under the headline — keeps the N-of-M copy primary, satisfies "headline number", and stays inside the no-"X% match" copy rule. Refs #2 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
s-annam
left a comment
There was a problem hiding this comment.
PR Review: feat: JD matching v1 (deterministic skill + phrase coverage)
Summary
Clean, well-tested v1 of deterministic JD matching. All 9 acceptance criteria from #2 met, full suite green (219 tests), typecheck + build clean. No blocking issues — three non-blocking refinements inline.
Spec Alignment (#2)
| Requirement | Status | Notes |
|---|---|---|
| JD textarea in App.tsx | ✅ | semantic tokens, JD-stays-in-browser copy |
| extract-jd-terms.ts: skill dict + noun pass | ✅ | two-pass, aliases, dedupe to canonical IDs |
| Boilerplate exclusion (EEO/benefits/pay) | ✅ | ~30 anchors, tested |
| coverage.ts: corpus + weighted score (skill 1.0 / noun 0.5) | ✅ | flat lowercased corpus, divide-by-zero guarded |
| JdMatch.tsx: score + diagnostic framing, 2 columns, hover snippet | ✅ | correctly placed in features/, no "add this" CTA |
| ≥100 skills w/ aliases | ✅ | ~120 entries, test asserts ≥100 |
| Copy discipline (no %match / no ATS verdict) | ✅ | N-of-M headline, alpha label |
| Unit tests per file + existing pass | ✅ | 4+9+5+~7 new; full suite 219 green |
| Deterministic in-browser | ✅ | useMemo, no network |
Highlights
- Boilerplate anchor list documented with sourcing rationale (
extract-jd-terms.ts:26-41) — the "About us NOT excluded" carve-out shows real thought. useMemogating is correct: returnsnullunless JD non-empty AND parse done AND terms extracted — no half-states.- Copy discipline nailed: weighted score shown as "Weighted coverage: X/100" below the N-of-M headline, never as "% match."
Key Findings
- [Suggestion]
escapeRegex+BOUNDARY/BOUNDARY_ENDduplicated betweencoverage.ts:115-119andskills.ts:226-229. Extract to a sharedjd-match/regex-utils.ts. - [Suggestion] Noun-pass
.slice(0, 25)cap (extract-jd-terms.ts:299) is documented but invisible to the user — a JD with >25 noun phrases silently drops some. Consider a "+N more" note or confirm 25 is comfortably above typical counts. - [Nit]
mentions()recompiles aRegExpper alias per call — fine now, worth a prebuilt matcher if skill count grows.
Verdict
Action: APPROVE
Rationale: All acceptance criteria met, tests green, no blocking issues. The three items are optional refinements, not merge gates.
…prebuilt skill matcher) - Extract shared boundary lookarounds + `escapeRegex` into `src/lib/jd-match/regex-utils.ts`. Removes the duplicated copies in `coverage.ts` and `skills.ts` (review suggestion #1). - Prebuild a per-canonical-ID `mentionPatterns` map in `skills.ts` so `coverage.corpusMentionsSkill` is a single `re.test()` per term instead of recompiling a regex per alias per call (review nit #3). - Move the noun-pass cap up to the `extractJdTerms` caller and surface the dropped overflow on `ExtractJdTermsResult.nounsDropped` + a "+N more capitalized phrases" footnote in `JdMatch`. The user now knows when the panel isn't exhaustive (review suggestion #2). Cap remains 25 with a comment explaining why. Tests: +3 new (noun-cap fixture in extract-jd-terms, surfacing + omission in JdMatch). Suite 222/222 green, typecheck + production build clean. Refs #2 #41 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
s-annam
left a comment
There was a problem hiding this comment.
PR Review: feat: JD matching v1 (deterministic skill + phrase coverage)
Summary
Re-review after the refactor commit — all three prior findings (shared regex-utils, surfaced noun cap, prebuilt skill matcher) are resolved. Full suite green (222), typecheck + build clean, CI verify passed. No blocking issues. One copy-precision suggestion + two nits below.
Spec Alignment (#2)
| Requirement | Status | Notes |
|---|---|---|
| JD textarea in App.tsx | ✅ | semantic tokens, "JD stays in browser tab" copy |
| extract-jd-terms.ts: skill + noun pass | ✅ | two-pass, alias dedupe to canonical IDs, snippet anchoring |
| Boilerplate exclusion (EEO/benefits/pay) | ✅ | ~30 anchors, line-block strip, tested |
| coverage.ts: corpus + weighted score (skill 1.0 / noun 0.5) | ✅ | corpus covers summary/skills/experience/education — matches what the heuristic path actually fills; divide-by-zero guarded |
| JdMatch.tsx: score + diagnostic framing, 2 cols, hover snippet | ✅ | features/, no "add this" CTA, title= tooltip |
| ≥100 skills w/ aliases | ✅ | ~120 entries; test asserts ≥100 |
| Copy discipline (no %match / no ATS verdict) | ✅ | N-of-M headline, alpha label, "diagnostic not a verdict" |
| Unit tests per file + existing pass | ✅ | 38 new jd-match/JdMatch tests; suite 222 green |
| Deterministic in-browser | ✅ | useMemo, no network |
| CI green | ✅ | verify pass |
Highlights
- Prior-review turnaround is clean:
regex-utils.tsnow ownsALIAS_BOUNDARY_*+escapeRegexfor both the skill compiler and the corpus probe — one boundary definition, two call sites. - Precision-over-recall dictionary choices are deliberate and correct:
goaliases togolang/go lang(never bare "go"),c→"c language",r-lang→"r language" — avoids the English-word false positives the docstring calls out. buildCorpusreads exactly the fields the heuristic cascade populates (skillsstring[], not the emptyskills_explicit) — verified againstopenresume.ts.- Noun/skill dedup is airtight: "Apache Kafka" noun-hit is dropped because
apache kafkais a kafka alias.
Key Findings
- [Suggestion] The "+N more" footnote says the noun pass "caps at the most informative ones," but
extractJdTermsdoesallNouns.slice(0, NOUN_PASS_CAP)— first-N in document order, not ranked by informativeness. False precision; either reword to "caps at the first N we find" or actually rank before slicing. - [Nit]
stripBoilerplateis line-granular, so an anchor sharing a line with real skill copy over-strips — e.g.Salary range: $100k. We use Rust and Go.drops Rust/Go. The docstring claims an under-strip bias; this case is the opposite. Rare in practice; fine for v1, worth a comment correction. - [Nit]
stripBoilerplateblank-lineif (skipping) {…} else {…}branches are identical except the flag reset — collapse toif (skipping) skipping = false; kept.push("");.
Verdict
Action: APPROVE
Rationale: All 9 acceptance criteria met, prior findings resolved, suite + CI green. The three items are optional polish, not merge gates.
…ormative") Reviewer flagged that JdMatch's footnote said the noun-pass "caps at the most informative ones" while the code does `allNouns.slice(0, NOUN_PASS_CAP)` — first-N in document order, no ranking. Per the copy-claims-round-trip- to-code rule, reword the footnote and the NOUN_PASS_CAP docstring so both describe the actual slice. v1 stays first-N; a ranked version is a follow-up if needed. Refs #41 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… correct docstring
Two review nits on the same function:
- The if/else in the blank-line branch had identical sides except for the
`skipping = false` reset; collapse to one path so the structure matches
intent.
- The docstring claimed an under-strip bias. Line-granular matching can
actually over-strip a line that mixes an anchor with skill copy
("Salary range: $100k. We use Rust and Go." drops Rust/Go). Rewrite the
docstring to call this out as the real tradeoff, with a sentence-
granular pass as the follow-up if it shows up in real JDs.
Refs #41
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
s-annam
left a comment
There was a problem hiding this comment.
PR Review: feat: JD matching v1 (deterministic skill + phrase coverage)
Summary
Clean v1. All 9 acceptance criteria from #2 met; suite green on the checked-out branch (222 tests), typecheck + build clean. The two prior reviews' findings (shared regex-utils, surfaced noun cap, prebuilt matcher, copy-roundtrip "first-N" wording, blank-branch collapse) are all resolved in ba2be56 / 666c1f2. No blocking issues — four non-blocking refinements.
Spec Alignment (#2)
| Requirement | Status | Notes |
|---|---|---|
| Paste-a-JD textarea | ✅ | semantic tokens, "JD stays in browser tab" |
| extract-jd-terms.ts skill + noun pass | ✅ | two-pass, alias dedupe to canonical IDs, snippet-anchored |
| Boilerplate exclusion (EEO/benefits/pay/visa) | ✅ | ~30 anchors, line-block strip, tested |
| coverage.ts corpus + weighted (skill 1.0 / noun 0.5) | ✅ | corpus = summary/skills/exp/edu; div-by-zero guarded |
| JdMatch.tsx score + diagnostic, 2 cols, hover snippet | ✅ | features/, no "add this" CTA, title= tooltip |
| ≥100 skills w/ aliases | ✅ | ~120; test asserts ≥100 |
| Copy discipline (no %match / no ATS verdict) | ✅ | N-of-M headline, alpha label, "diagnostic not a verdict" |
| Unit/component tests per file + existing pass | ✅ | 38 new; suite 222 green |
| Deterministic in-browser, no network | ✅ | useMemo, no fetch |
| CI green | ✅ | verify |
Highlights
- Regex-state correctness:
extractSkillPassclones the cachedg-flagged pattern (new RegExp(source, flags)) so per-calllastIndexnever leaks across JDs. Boundary lookarounds are consuming-prefix + lookahead-suffix, which correctly handles adjacent aliases (react vue,react,vue). - Precision-over-recall dictionary is deliberate:
go→golangonly,c→c language,r-lang→r language— dodges English-word false positives. - Noun/skill dedup is airtight: an "Apache Kafka" noun hit is dropped via
aliasToId.has("apache kafka"). - Longest-first alias sort means
ruby on rails/react.jswin over their prefixes in a single pass.
Key Findings (all non-blocking)
- [Suggestion] Covered/Missing rows render
term.display, which for skills is the canonical kebab ID — a JD asking "A/B testing" surfaces asa-b-testing; same forr-lang,machine-learning,ci-cd. Consider an optionallabelonSkillEntryfor human display, falling back toid. - [Suggestion] The card chrome
rounded-xl border border-border-light bg-surface-card p-5 shadow-smis now duplicated 5× (Result ×2, ContactCard, JdMatch, this new App section). Noshared/dir exists yet — pre-existing debt, but this PR adds two copies; ashared/Cardwould centralize it per the 3-tier architecture in CLAUDE.md. - [Nit] The textarea has a placeholder and a sibling
<h2>but no associated<label>/aria-label— screen readers announce only the placeholder. - [Nit]
extractJdTerms+computeCoveragere-run on every textarea keystroke (useMemo depjdText) once a PDF is loaded. Fine at JD sizes; debounce if JDs ever grow large.
Verdict
Action: APPROVE
Rationale: All acceptance criteria met, prior findings resolved, suite + CI green, no correctness/security/reuse blockers. The four items are optional polish, not merge gates.
|
|
||
| export interface SkillEntry { | ||
| /** Canonical skill ID — what the UI renders. */ | ||
| readonly id: string; |
There was a problem hiding this comment.
[Suggestion]: id doubles as the UI display string (JdMatch renders term.display, which for skills is the canonical id). So a JD asking "A/B testing" shows up in the Covered/Missing list as a-b-testing; likewise r-lang, machine-learning, ci-cd, data-warehouse. Consider an optional label?: string here for human display, falling back to id. Non-blocking polish.
| /> | ||
| )} | ||
|
|
||
| <section className="flex flex-col gap-3 rounded-xl border border-border-light bg-surface-card p-5 shadow-sm"> |
There was a problem hiding this comment.
[Suggestion]: This card chrome (rounded-xl border border-border-light bg-surface-card p-5 shadow-sm) is now duplicated 5× across Result.tsx (×2), ContactCard.tsx, JdMatch.tsx, and here. There's no src/components/shared/ yet, so this matches existing convention rather than introducing new drift — but per the 3-tier arch in CLAUDE.md a shared/Card would let all five share one definition. Pre-existing debt; flagging since this PR adds two more copies.
| tab. | ||
| </p> | ||
| </div> | ||
| <textarea |
There was a problem hiding this comment.
[Nit]: The textarea has a placeholder and a sibling <h2>, but no programmatic label — a screen reader announces only the placeholder text. Consider aria-label="Job description" (or wire the <h2> via id + aria-labelledby).
Apply the non-blocking findings from the JD-match v1 review: - Extract src/components/shared/Card.tsx and migrate all five card sites (Result's parsed + limited panels, ContactCard, JdMatch, and the App JD section) onto it. The rounded-xl / border-border-light / bg-surface-card / p-5 chrome lived in five hand-rolled copies; Card now owns it and callers keep their own flex/gap/shadow/scroll-mt via className. Behavior-preserving (identical class set per site). - Add an optional `label` to SkillEntry and render it instead of the kebab canonical id, so a JD asking for "A/B testing" surfaces as "A/B testing" rather than "a-b-testing" (also CI/CD, React Native, Power BI, GitHub Actions, …). display falls back to id when unset. - Associate the JD textarea with its heading via aria-labelledby so a screen reader announces more than the placeholder. The fourth finding (per-keystroke re-extraction) is left as-is — it's within v1 JD sizes and debouncing adds complexity for no real gain. Co-Authored-By: Samhit21 <37660029+Samhit21@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
s-annam
left a comment
There was a problem hiding this comment.
Re-approving after pushing the review-fix commit (acf3754): shared Card primitive across all five card sites, optional label on SkillEntry so skills render human names instead of kebab ids, and aria-labelledby on the JD textarea. Suite 224 green, typecheck + build clean locally. Per-keystroke re-extraction left as-is for v1.
JD matching v1: paste-a-JD textarea + deterministic skill/phrase coverage panel. Curated ~120-skill dictionary with aliases, two-pass extraction (skill + noun-phrase) with boilerplate stripping, weighted coverage score (skill 1.0 / noun 0.5), diagnostic copy (N-of-M, never "% match"). All in-browser, no network. Plus review fixes: shared Card primitive across all card sites, human skill labels, JD textarea a11y. Resolves #2 Co-Authored-By: Samhit21 <37660029+Samhit21@users.noreply.github.com>
JD matching v1: paste-a-JD textarea + deterministic skill/phrase coverage panel. Curated ~120-skill dictionary with aliases, two-pass extraction (skill + noun-phrase) with boilerplate stripping, weighted coverage score (skill 1.0 / noun 0.5), diagnostic copy (N-of-M, never "% match"). All in-browser, no network. Plus review fixes: shared Card primitive across all card sites, human skill labels, JD textarea a11y. Resolves #2 Co-Authored-By: Samhit21 <37660029+Samhit21@users.noreply.github.com>
JD matching v1: paste-a-JD textarea + deterministic skill/phrase coverage panel. Curated ~120-skill dictionary with aliases, two-pass extraction (skill + noun-phrase) with boilerplate stripping, weighted coverage score (skill 1.0 / noun 0.5), diagnostic copy (N-of-M, never "% match"). All in-browser, no network. Plus review fixes: shared Card primitive across all card sites, human skill labels, JD textarea a11y. Resolves #2 Co-Authored-By: Samhit21 <37660029+Samhit21@users.noreply.github.com>
Summary
src/lib/jd-match/skills.tsships ~120 canonical skills with aliases (react.js/reactjs/react,k8s/kubernetes, …). Compiled into one longest-first regex so multi-word aliases (ruby on rails) win against their prefixes.Files
src/lib/jd-match/skills.ts— dictionary + compiled alias regexsrc/lib/jd-match/extract-jd-terms.ts— skill + noun-phrase passes with boilerplate strippingsrc/lib/jd-match/coverage.ts— corpus build + weighted coverage scoresrc/lib/jd-match/index.ts— barrel exportsrc/components/features/JdMatch.tsx— covered/missing columnssrc/App.tsx— textarea + panel wiring (renders only when both PDF and JD are present)*.test.tsnext to each new source file (25 new tests)Test plan
npm run test— 219/219 green (25 new)npm run typecheck— cleannpm run build— cleanOut of scope (per the issue)
Refs #2
🤖 Generated with Claude Code