Skip to content

fix(parser): ignore language proficiency rows - #865

Open
qtjg wants to merge 4 commits into
offlinecv:mainfrom
qtjg:mb/issue-833-language-proficiency
Open

fix(parser): ignore language proficiency rows#865
qtjg wants to merge 4 commits into
offlinecv:mainfrom
qtjg:mb/issue-833-language-proficiency

Conversation

@qtjg

@qtjg qtjg commented Aug 18, 2026

Copy link
Copy Markdown

Resolves #833

Summary

Ignore spoken-language proficiency rows such as Language: Fluent in Spanish when extracting résumé skills, while preserving programming-language rows and bare spoken-language lists. The label and proficiency wording are checked together, so Languages: Python, Go, TypeScript remains unchanged and non-language labels such as Certifications: Certified in AWS Solutions Architecture are unaffected.

The change adds focused regression coverage, removes the resolved #833 truth exception, and updates the affected Google Docs and Word corpus snapshots.

Verification

  • npx vitest run src/lib/heuristics/extract/skills.test.ts — 49 passed
  • npm run typecheck — passed
  • npx vitest run src/lib/heuristics/corpus.test.ts src/lib/heuristics/corpus-roundtrip.test.ts — passed
  • npm run lint — passed
  • npm run check:fixtures — passed
  • npm run check:baselines — passed
  • npm run check:core — passed
  • npm run build — passed

The full repository verification also reproduces an unrelated existing BroadcastChannel/MessageEvent failure in src/hooks/useLibraryChanges.test.tsx; 358 test files passed and 1 failed.

qtjg added 4 commits August 18, 2026 07:00
Resolves offlinecv#833

Ignore spoken-language proficiency rows such as `Language: Fluent in Spanish` while preserving programming-language and bare spoken-language lists. The change includes regression coverage and updates the affected corpus snapshots.

Verified locally with focused parser tests, corpus and round-trip tests, typecheck, lint, fixture checks, baseline checks, core-package checks, and production build. The full suite also reproduces an unrelated existing BroadcastChannel/MessageEvent failure in src/hooks/useLibraryChanges.test.tsx.

@s-annam s-annam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The precision goal is right and the corpus moves the right way — Fluent in Spanish is gone from both fixtures and the #833 truth exception clears cleanly against an exactly-matching 11-entry truth. But the discriminator is a bare substring test over the whole body, and it takes the whole cell with it. Two confirmed regressions follow, and neither is visible to verify because no fixture happens to pair the shapes.

Verdict rule: ≥1 Blocking → REQUEST_CHANGES. 2 Blocking, 2 Secondary, 1 Nit. Nothing was committed or pushed.

Every claim below was measured on this branch against origin/main with throwaway probes over the real extractSkills / tokenizeSkillLine (probe files removed; tree left clean).

Blocking

1. LANGUAGE_PROFICIENCY_BODY_RE matches inside real programming-language names, and the drop is whole-cell — an entire skills row vanishes

\bbasic\b fires on Visual Basic, \bnative\b on React Native, and \bproficient\b on the ubiquitous Proficient in <list> phrasing. Because the drop happens in tokenizeCell before the split, every sibling token on the row dies with the offender:

Input origin/main this branch
Languages: C, C++, Visual Basic ["C","C++","Visual Basic"] []
Languages: Kotlin, Swift, React Native 3 tokens []
Languages: Proficient in Java, Python, Go ["Proficient in Java","Python","Go"] []
Languages: Java, Python (proficient), Go ["Java","Python (proficient)","Go"] []

The label conjunct is Languages in all four, so the label is effectively deciding — the exact failure #833's root-cause section ruled out: "denying the label would delete the single most valuable row on the page." This is a strictly larger loss than the one token the fix targets.

Not hypothetical on this corpus. openresume-laverne-word-quartz.pdf — the fixture this PR re-snapshots — draws Computer: Proficient in Windows and Mac OS, Microsoft Word, PowerPoint, and Excel, spared only by its label. multi-degree-coursework.pdf draws Languages: Python, Go, C, C++, Java, JavaScript, Swift, PHP, HTML, CSS — one Visual Basic away from returning nothing.

Why this reads as under-implementing the plan, not a judgement call: #833 step 1 specifies the body conjunct as "whose body reads as a proficiency predication rather than a delimited list". Only the proficiency-word half shipped; the delimited-list half is missing. The regexes in the issue illustrate that two-part rule rather than replacing it.

One shape that satisfies every AC — drop only when every delimited fragment is a proficiency predication:

const fragments = debulleted
  .slice(match[0].length)
  .split(SKILL_SPLIT_RE)
  .map((f) => f.trim())
  .filter((f) => f !== "");
// A list whose fragments are mostly plain tokens is a programming-language row
// that happens to contain "Basic"/"React Native"/"(proficient)"; only an
// all-proficiency body is a spoken-language proficiency statement.
return fragments.length > 0 && fragments.every((f) => LANGUAGE_PROFICIENCY_BODY_RE.test(f));

Against the ACs: Fluent in Spanish 1/1 → dropped. Python, Go, TypeScript 0/3 → kept. Spanish, French, Mandarin 0/3 → kept (the stated limit, preserved). C, C++, Visual Basic 1/3 → kept. Proficient in Java, Python, Go 1/3 → kept. Note splitRespectingParens is what handles commas — reuse the existing splitter rather than a new one.

Whatever shape you land on, please pin the four rows above as unit assertions. The new test at skills.test.ts:838 covers only clean rows, so nothing currently guards this.

2. Dropping the cell destroys the category anchor, so a soft-wrapped proficiency row misfiles its tail into the previous category

Three lines — Frameworks: React, Vue / Languages: English (native), Spanish (fluent), / French:

value categories
origin/main ["React","Vue","English (native)","Spanish (fluent)","French"] Frameworks:[React,Vue], Languages:[English (native),Spanish (fluent),French]
this branch ["React","Vue","French"] Frameworks:[React,Vue,French]

French is now a Framework. The mechanism: the language cell yields zero tokens, so extractSkills continues at skills.ts:680 before matchCellLabel ever runs, no Languages category is opened, and the wrapped continuation falls into the "bare cell extends the last category" branch at skills.ts:691. Any wrap isSoftWrapContinuation declines reproduces it.

This is worse than the loss in #1 — it is silently wrong data rather than missing data, and skills feeds the JD-match and job-search keyword surfaces the issue itself calls load-bearing.

Fixing #1 does not fix this. Under the all-fragments rule above, English (native), Spanish (fluent), is still legitimately dropped, so the leak survives. It needs its own answer — either register the category label before the token-count continue, or drop the offending fragments rather than the cell — which is also the cleanest way to make #1 and #2 fall out of one change.

Secondary

3. LANGUAGE_LABEL_RE is stricter than its sibling, so the targeted defect survives on common label variants

/^languages?$/i tested against the untrimmed capture. Measured, all still admitted on this branch:

  • Foreign Languages: Fluent in Spanish["Fluent in Spanish"]
  • Spoken Languages: Native German, conversational French["Native German","conversational French"]
  • Languages : Fluent in Spanish (space before colon) → ["Fluent in Spanish"]

The first two are #833's defect, unfixed. The third is a capture artifact: SUBLABEL_BODY ([A-Z][A-Za-z &/]+) admits a space, so the capture is "Languages ". NON_SKILL_SUBLABEL_RE carries a deliberate \s*$ for exactly this and matchCellLabel .trim()s — this regex does neither, so Interests : Tennis is correctly dropped while Languages : Fluent in Spanish escapes. /^(?:foreign\s+|spoken\s+|other\s+)?languages?\s*$/i, mirroring the sibling's leading-qualifier tolerance, covers all three.

4. The proficiency vocabulary misses the standard scale wordings — including one already in the corpus

\bproficient\b does not match the noun proficiency, and only working\s+proficiency is enumerated. Still admitted:

  • Languages: Full professional proficiency in German
  • Languages: Elementary proficiency in French
  • Languages: JLPT N2 Japanese Proficiency

That last one is drawn by two committed fixturesgoogle-docs/google-docs-skia-proxy-multiline-bullets-coursework.pdf and unknown/student-projects-activities-singlecol.pdf (verified with pdftotext) — and both baselines are untouched by this PR, confirming the row still lands in skills. proficienc(y|ies), plus elementary / limited, closes it. Worth doing here rather than as a follow-up, since the fixtures are already in the corpus.

Nits

Non-blocking; neither changes the verdict.

  1. The isLanguageProficiencyCell(cell) guard in matchCellLabel is unreachable, and the docblock above it no longer describes the guard set. extractSkills:680 continues on a zero-token cell and tokenizeCell already returned early for exactly these, so matchCellLabel — which has no other callers — can never see one. Harmless as defensive alignment with the existing NON_SKILL_SUBLABEL_RE mirror, and worth keeping for that reason, but the docblock at skills.ts:632 explains only the Interests/Hobbies mirror and should name this one too. Related: this call site passes the raw cell while skills.ts:341 passes the already-stripBulleted text; stripBullet is idempotent so it is correct either way, but taking the captured label and body as parameters would make the "two decisions stay aligned" contract structural instead of by-convention.

Acceptance criteria — #833

AC Result
google-docs-skia-proxy-role-first-experience.pdf — no Fluent in Spanish, no Language category skillsCount 12 → 11, matching the 11-entry truth skills exactly
Languages: Python, Go, TypeScript → 3 skills under Languages. Unit test ✅ covered
Languages: Spanish, French, Mandarin still admitted (stated limit) ✅ covered
Certified in AWS Solutions Architecture under a non-language label unaffected ✅ covered
multi-degree-coursework.pdf Languages row unchanged expected.json untouched; that row carries no proficiency word
corpus.test.ts passes; precision up on target, down on none ✅ on the corpus — but see Blocking 1/2, which the corpus does not exercise
corpus-roundtrip.test.ts passes, no new KNOWN_FAILURES rows ✅ no baseline file in the diff
npm run verify passes ✅ reproduced locally, green through build

Plan step 3 also holds: no languages field exists in docs/canonical-resume-model.md or the parsed types, so dropping is the right answer and none was added. Step 5 holds too — UNFILED_TRUTH_CEILING is untouched at 7, correct since the cleared entry was status: "open", never unfiled.

Description accuracy (gate 3f)

## Summary claims the change preserves programming-language rows. Blocking 1 shows it does not, for Visual Basic / React Native / Proficient in … / X (basic). Same defect, so not counted twice — but once the predicate is narrowed, the summary needs a line naming which language rows are still dropped, so the limit is stated rather than discovered.

The ## Verification list is otherwise accurate and reproduced here.

Gates

  • 3a fixture PII — no binary added or changed; only .expected.json / .truth.json. npm run check:fixtures green (it sweeps truth sidecars since #654).
  • 3b design system / 3c style tokens — n/a, nothing under src/components/.
  • 3d fallow — 2 complexity findings (isSoftWrapContinuation, extractSkills) + 1 clone group at skills.test.ts:589-664, all pre-existing and outside this diff. Report-only.
  • 3e skill/script command bugs — n/a, no bash/gh/script files in the diff.
  • npm run verify — green end to end here, full suite included. The useLibraryChanges.test.tsx BroadcastChannel/MessageEvent failure noted in the PR body did not reproduce; it looks environment-dependent, and nothing in this diff touches it.

Branch hygiene

The branch carries 4 commits (1b083e3, 616c09c, b20c622, 8eaae56). main merges through a merge queue that derives the squash message from the branch, so as-is this lands test(fixtures): refresh Word skill snapshot in main permanently. Collapse to one before enqueueing — /collapse-pr does it. Not counted as a finding, and this review pushed nothing (≥1 Blocking).


Reviewed by: Claude Opus 5 (high)

const debulleted = stripBullet(cell);
const match = debulleted.match(SUBLABEL_PREFIX_RE);
if (!match || !LANGUAGE_LABEL_RE.test(match[1])) return false;
return LANGUAGE_PROFICIENCY_BODY_RE.test(debulleted.slice(match[0].length));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking. This tests the proficiency vocabulary against the whole body, so a delimited programming-language row containing one matching word is dropped entirely — and tokenizeCell drops the cell before the split, so every sibling token on the row dies too.

Measured on this branch vs origin/main:

Input origin/main this branch
Languages: C, C++, Visual Basic ["C","C++","Visual Basic"] []
Languages: Kotlin, Swift, React Native 3 tokens []
Languages: Proficient in Java, Python, Go ["Proficient in Java","Python","Go"] []
Languages: Java, Python (proficient), Go ["Java","Python (proficient)","Go"] []

\bbasic\b catches Visual Basic, \bnative\b catches React Native, \bproficient\b catches the common Proficient in <list> phrasing. The label is effectively deciding, which is what #833 ruled out.

#833 step 1 specifies the body conjunct as "a proficiency predication rather than a delimited list" — the delimited-list half is missing. Requiring every delimited fragment to be a proficiency predication satisfies all four ACs and rejects all four rows above:

const fragments = debulleted
  .slice(match[0].length)
  .split(SKILL_SPLIT_RE)
  .map((f) => f.trim())
  .filter((f) => f !== "");
return fragments.length > 0 && fragments.every((f) => LANGUAGE_PROFICIENCY_BODY_RE.test(f));

(splitRespectingParens is what handles commas — worth reusing the existing splitter.) Please pin the four rows as unit assertions; the new test covers only clean rows.

if (labelMatch && NON_SKILL_SUBLABEL_RE.test(labelMatch[1])) return;
if (
labelMatch &&
(NON_SKILL_SUBLABEL_RE.test(labelMatch[1]) || isLanguageProficiencyCell(debulleted))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking. Returning here yields zero tokens, so extractSkills continues at skills.ts:680 before matchCellLabel runs and no Languages category is ever opened. A soft-wrapped continuation then falls into the "bare cell extends the last category" branch at skills.ts:691 and is filed under the previous label.

Frameworks: React, Vue / Languages: English (native), Spanish (fluent), / French:

  • origin/mainFrameworks:[React,Vue], Languages:[English (native),Spanish (fluent),French]
  • this branch → Frameworks:[React,Vue,French]

French becomes a Framework — silently wrong data rather than missing data, on a surface that feeds JD-match and job-search keywords.

Note the all-fragments fix for the other blocker does not resolve this: English (native), Spanish (fluent), is still legitimately dropped. Either register the category label before the zero-token continue, or drop the offending fragments rather than the whole cell — the latter makes both blockers fall out of one change.

* added to NON_SKILL_SUBLABEL_RE: on most engineering résumés `Languages:`
* heads the programming-language row, so the label is ambiguous and the body
* must decide whether this is a proficiency statement. */
const LANGUAGE_LABEL_RE = /^languages?$/i;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Secondary. Stricter than its sibling NON_SKILL_SUBLABEL_RE, so #833's defect survives on common variants. All still admitted on this branch:

  • Foreign Languages: Fluent in Spanish["Fluent in Spanish"]
  • Spoken Languages: Native German, conversational French["Native German","conversational French"]
  • Languages : Fluent in Spanish["Fluent in Spanish"]

The third is a capture artifact: SUBLABEL_BODY ([A-Z][A-Za-z &/]+) admits a space, so the capture is "Languages ". NON_SKILL_SUBLABEL_RE carries a deliberate \s*$ for exactly this and matchCellLabel .trim()s the capture — this regex does neither, so Interests : Tennis is dropped while Languages : Fluent in Spanish escapes.

/^(?:foreign\s+|spoken\s+|other\s+)?languages?\s*$/i mirrors the sibling's leading-qualifier tolerance and covers all three.

* programming-language list by its proficiency wording rather than by the
* label. */
const LANGUAGE_PROFICIENCY_BODY_RE =
/\b(fluent|native|bilingual|conversational|proficient|intermediate|beginner|basic|working\s+proficiency|mother\s+tongue)\b/i;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Secondary. The vocabulary misses the standard scale wordings: \bproficient\b does not match the noun proficiency, and only working\s+proficiency is enumerated. Still admitted:

  • Languages: Full professional proficiency in German
  • Languages: Elementary proficiency in French
  • Languages: JLPT N2 Japanese Proficiency

The last is drawn by two committed fixtures — google-docs/google-docs-skia-proxy-multiline-bullets-coursework.pdf and unknown/student-projects-activities-singlecol.pdf (verified with pdftotext) — and both baselines are untouched by this PR, confirming the row still reaches skills.

Adding proficienc(y|ies) plus elementary / limited closes it. Worth doing here rather than as a follow-up, since the fixtures already exist.

if (
!m ||
NON_SKILL_SUBLABEL_RE.test(m[1]) ||
isLanguageProficiencyCell(cell)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit. Unreachable: extractSkills:680 continues on a zero-token cell and tokenizeCell already returned early for exactly these, so matchCellLabel — which has no other callers — can never be handed one. Fine to keep as defensive alignment with the NON_SKILL_SUBLABEL_RE mirror, but the docblock just above (skills.ts:632) explains only the Interests/Hobbies mirror and should name this guard too.

Related: this passes the raw cell while skills.ts:341 passes the already-stripBulleted text. Correct either way since stripBullet is idempotent, but taking the captured label and body as parameters would make the "two decisions stay aligned" contract structural rather than by-convention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[parser] skills — a "Language: Fluent in Spanish" proficiency row is admitted as a skill token

2 participants