Skip to content

fix(skills): allow single-letter languages C, R, and D (#832) #840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shubhransh-gupta wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(skills): allow single-letter languages C, R, and D (#832) #840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/lib/heuristics/corpus.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ const TRUTH_ANNOTATED_FIELD_FLOOR = 150;
* `npm run check:baselines` on every run, and bounded here — undescribed debt may
* not GROW. File the issue and flip the entry to `open`; then lower this.
*/
const UNFILED_TRUTH_CEILING = 9;
const UNFILED_TRUTH_CEILING = 7;

/** Generator category = the fixture root's immediate subdirectory. */
function categoryOf(repoRelPdfPath: string): string {
Expand Down
21 changes: 19 additions & 2 deletions src/lib/heuristics/extract/skills.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,8 @@ describe("tokenizeSkillLine — issue #221 non-skill sub-labels", () => {
});

it("keeps skill sub-labels (Languages/Technologies/Tools/Frameworks)", () => {
expect(tokenizeSkillLine("Languages: Python, Go, C++, Java")).toEqual(
expect.arrayContaining(["Python", "Go", "C++", "Java"]),
expect(tokenizeSkillLine("Languages: Python, Go, C++, Java, C")).toEqual(
expect.arrayContaining(["Python", "Go", "C++", "Java", "C"]),
Comment on lines +405 to +406

@s-annam s-annam Aug 16, 2026

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 (non-blocking). This is the #221 sub-label test, widened to also carry a #832 case. Now that #832 has its own describe block immediately below, reverting this line keeps each test attributable to the issue it guards — useful later when someone bisects one of the two and wants to know which assertions are load-bearing for which fix.

Both forms pass (["Python", "Go", "C++", "Java", "C"] is the actual return either way), so this is purely about attribution.

Suggested change
expect(tokenizeSkillLine("Languages: Python, Go, C++, Java, C")).toEqual(
expect.arrayContaining(["Python", "Go", "C++", "Java", "C"]),
expect(tokenizeSkillLine("Languages: Python, Go, C++, Java")).toEqual(
expect.arrayContaining(["Python", "Go", "C++", "Java"]),

);
expect(tokenizeSkillLine("Technologies: Linux, AWS, Docker, iOS")).toEqual(
expect.arrayContaining(["Linux", "AWS", "Docker", "iOS"]),
Expand All @@ -426,6 +426,23 @@ describe("tokenizeSkillLine — issue #221 non-skill sub-labels", () => {
});
});

describe("tokenizeSkillLine — issue #832 single-letter languages", () => {
it("keeps C, R, and D as real programming languages", () => {
expect(tokenizeSkillLine("Languages: C, R, D")).toEqual(
expect.arrayContaining(["C", "R", "D"]),
);
Comment on lines +431 to +433

@s-annam s-annam Aug 16, 2026

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 (non-blocking). expect.arrayContaining passes even if the tokenizer returns extra tokens alongside these three, so it under-specifies what the fix guarantees. The exact return on this branch is ["C", "R", "D"] — I ran it — so a strict toEqual costs nothing and pins the behaviour precisely.

Suggested change
expect(tokenizeSkillLine("Languages: C, R, D")).toEqual(
expect.arrayContaining(["C", "R", "D"]),
);
expect(tokenizeSkillLine("Languages: C, R, D")).toEqual(["C", "R", "D"]);

});

it("still rejects other single-character noise tokens", () => {
expect(tokenizeSkillLine("Skills: x, J, •, (")).toEqual([]);

@s-annam s-annam Aug 15, 2026

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 (non-blocking): every token on this line is expected to be rejected, so toEqual([]) also passes in the world where the Skills: sub-label path drops the whole cell — it cannot tell "rejected the noise" from "rejected everything".

A positive control in the same line pins it:

expect(tokenizeSkillLine("Skills: Python, x, J, C")).toEqual(["Python", "C"]);

I ran that on this branch — it returns exactly ["Python", "C"], so it asserts the allowlist admits C, rejects x and J, and that the surrounding cell path is live, all at once. Keep the •/( case too if you like; just don't let it be the only assertion.

});

it("allows allowlisted single letters alongside real skills on the same line", () => {
// Positive control: distinguishes rejecting noise from rejecting everything.
expect(tokenizeSkillLine("Skills: Python, x, J, C")).toEqual(["Python", "C"]);
});
});

describe("parseHeuristic — issue #221 Interests sub-label in SKILLS section", () => {
it("excludes Interests items while keeping Languages/Technologies skills", () => {
// Repro from the issue: a Technical Skills section internally sub-labeled
Expand Down
7 changes: 7 additions & 0 deletions src/lib/heuristics/extract/skills.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,14 @@ function looksLikeContactLink(tok: string): boolean {
return PROFILE_LABEL_RE.test(t) || PROFILE_HOST_RE.test(t) || URLISH_RE.test(t);
}

/** One-character tokens that are real, commonly-listed languages. The
* `tok.length < 2` floor in `isSkillToken` is a noise guard against stray
* glyphs left by column splitting; these are the only single characters that
* are not noise, so they are allowlisted rather than lowering the floor. */
const SINGLE_LETTER_SKILLS = new Set(["c", "r", "d"]);
Comment on lines +141 to +145

@s-annam s-annam Aug 16, 2026

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 — the docblock says what is in the set, not what was deliberately kept out.

#832 makes the exclusions the interesting decision: J, K, Q and F are all real language names and were left out on purpose, because each is also a plausible stray glyph and admitting them "trades a rare recall win for a common precision loss." That reasoning exists only in the issue. A year from now someone reads these four lines, notices J is missing, and re-litigates it from scratch — or just adds it.

One sentence fixes it. Non-blocking, and fine to skip. If you do want it, fold it into the existing commit with git commit --amend rather than adding a second commit — the review body explains why I am not offering this as a one-click suggestion:

/** One-character tokens that are real, commonly-listed languages. The
 *  `tok.length < 2` floor in `isSkillToken` is a noise guard against stray
 *  glyphs left by column splitting; these are the only single characters that
 *  are not noise, so they are allowlisted rather than lowering the floor.
 *  Deliberately excludes J/K/Q/F (#832): each is a real language name, but
 *  each is also a plausible stray glyph, so admitting them trades a rare
 *  recall win for a common precision loss. */


function isSkillToken(tok: string): boolean {
if (tok.length === 1 && SINGLE_LETTER_SKILLS.has(tok.toLowerCase())) return true;

@s-annam s-annam Aug 15, 2026

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.

This line is correct and matches #832's plan exactly — including the early return, which is what lets an allowlisted token skip the numeric / URL / date-range / word-count checks below.

What it also does, though, is move four baked corpus snapshots and clear one knownWrong exemption, and those files have to move in the same commit or CI is red. See Blocking 1 and 2 in the review body — npm run bake-fixtures, delete the multi-degree-coursework skills exemption, flip the skia-proxy one to open/#833, and lower UNFILED_TRUTH_CEILING 9 → 7.

if (tok.length < 2 || tok.length > 40) return false;
if (/^\d+$/.test(tok)) return false;
// A professional-profile link (or its bare "GitHub" / "LinkedIn" heading) is
Expand Down
6 changes: 3 additions & 3 deletions tests/fixtures/pdfs/google-docs/google-docs-skia-proxy-role-first-experience.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"phoneIsValid",
"skills"
],
"skillsCount": 11,
"skillsCount": 12,
"experienceCount": 2,
"educationCount": 1,
"projectsCount": 0,
Expand Down Expand Up @@ -74,7 +74,7 @@
"sectionSource": "regex",
"pageCount": 1,
"rawCharCount": 1428,
"extractedCharCount": 1146,
"extractedCharCount": 1147,
"sections": [
{
"name": "profile",
Expand All @@ -101,7 +101,7 @@
"hasSummary": false,
"experienceCount": 2,
"educationCount": 1,
"skillsCount": 11
"skillsCount": 12
},
"linkAnnotationCount": 0,
"disagreements": []
Expand Down
6 changes: 3 additions & 3 deletions tests/fixtures/pdfs/google-docs/google-docs-skia-proxy-role-first-experience.truth.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@
"note": "Role 2's employer line reads “Multicultural Engineering Program – State Polytechnic University”; `company` comes back as just the university — the program half is not lost, the parser puts it on `team`, but `experience.company` scores the `company` field alone. The identical shape is measured on unknown/single-column-title-below-anchor. Possibly a defensible org/team split rather than a defect — recorded rather than assumed, because ground truth's job is to state what the page says and let a human adjudicate."
},
"skills": {
"issue": null,
"status": "unfiled",
"note": "Two independent disagreements on one field: the single-letter token “C” is DROPPED from the Programming Languages row (the identical drop is measured on latex/multi-degree-coursework, so it is not fixture-specific), and “Fluent in Spanish” is admitted as a skill from the “Language:” row."
"issue": 833,
"status": "open",
"note": "“Fluent in Spanish” is admitted as a skill from the “Language:” row; the Programming Languages row is now correct after #832."
}
}
}
6 changes: 3 additions & 3 deletions tests/fixtures/pdfs/latex/deedy-resume-macfonts.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"skills",
"website_url"
],
"skillsCount": 22,
"skillsCount": 23,
"experienceCount": 6,
"educationCount": 3,
"projectsCount": 0,
Expand Down Expand Up @@ -83,7 +83,7 @@
"sectionSource": "markdown",
"pageCount": 1,
"rawCharCount": 3205,
"extractedCharCount": 2156,
"extractedCharCount": 2157,
"sections": [
{
"name": "profile",
Expand Down Expand Up @@ -114,7 +114,7 @@
"hasSummary": false,
"experienceCount": 6,
"educationCount": 3,
"skillsCount": 22
"skillsCount": 23
},
"linkAnnotationCount": 9,
"disagreements": []
Expand Down
6 changes: 3 additions & 3 deletions tests/fixtures/pdfs/latex/deedy-resume-openfonts.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"skills",
"website_url"
],
"skillsCount": 22,
"skillsCount": 23,
"experienceCount": 6,
"educationCount": 3,
"projectsCount": 0,
Expand Down Expand Up @@ -83,7 +83,7 @@
"sectionSource": "markdown",
"pageCount": 1,
"rawCharCount": 3207,
"extractedCharCount": 2158,
"extractedCharCount": 2159,
"sections": [
{
"name": "profile",
Expand Down Expand Up @@ -114,7 +114,7 @@
"hasSummary": false,
"experienceCount": 6,
"educationCount": 3,
"skillsCount": 22
"skillsCount": 23
},
"linkAnnotationCount": 9,
"disagreements": []
Expand Down
6 changes: 3 additions & 3 deletions tests/fixtures/pdfs/latex/multi-degree-coursework.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"website_url",
"work_authorization"
],
"skillsCount": 17,
"skillsCount": 18,
"experienceCount": 4,
"educationCount": 2,
"projectsCount": 3,
Expand Down Expand Up @@ -77,7 +77,7 @@
"sectionSource": "markdown",
"pageCount": 1,
"rawCharCount": 3428,
"extractedCharCount": 2625,
"extractedCharCount": 2626,
"sections": [
{
"name": "profile",
Expand Down Expand Up @@ -108,7 +108,7 @@
"hasSummary": false,
"experienceCount": 4,
"educationCount": 2,
"skillsCount": 17
"skillsCount": 18
},
"linkAnnotationCount": 6,
"disagreements": []
Expand Down
9 changes: 1 addition & 8 deletions tests/fixtures/pdfs/latex/multi-degree-coursework.truth.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,5 @@
"Docker",
"Raspberry Pi",
"iOS"
],
"knownWrong": {
"skills": {
"issue": null,
"status": "unfiled",
"note": "The single-letter token “C” is DROPPED from the Languages row while “C++” survives. Second independent measurement of the same drop (see google-docs/google-docs-skia-proxy-role-first-experience)."
}
}
]
}
6 changes: 3 additions & 3 deletions tests/fixtures/pdfs/unknown/pdflib-leading-glyph-skills-header.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"phoneIsValid",
"skills"
],
"skillsCount": 8,
"skillsCount": 9,
"experienceCount": 1,
"educationCount": 1,
"projectsCount": 0,
Expand Down Expand Up @@ -74,7 +74,7 @@
"sectionSource": "markdown",
"pageCount": 1,
"rawCharCount": 462,
"extractedCharCount": 289,
"extractedCharCount": 290,
"sections": [
{
"name": "profile",
Expand All @@ -101,7 +101,7 @@
"hasSummary": false,
"experienceCount": 1,
"educationCount": 1,
"skillsCount": 8
"skillsCount": 9
},
"linkAnnotationCount": 0,
"disagreements": []
Expand Down
Loading