fix(heuristics): Word-template parsing + identity-link, education & experience extraction

[rohithgollapalli]

Summary

Branch of contact / education / experience parsing fixes surfaced from
real-resume bug reports, on top of the earlier Word-template work (#29) and
scoring fixes (#30, #31). Extraction-layer only (src/lib); the sole UI
change is the contact card adapting its row count.

Contact / identity links

• LinkedIn & GitHub recovered document-wide (footer / "Links" blocks),
  promoted into the contact card, de-duplicated out of the rendered body, and
  kept out of Skills (isSkillToken rejects profile URLs + bare labels).
• LinkedIn matching now symmetric with GitHub: any linkedin.com/<handle>
  profile (incl. bare vanity URLs) resolves; /company, /jobs, etc. excluded.
• GitHub is an optional contact row — absent → no "not detected" gap, no
  penalty to the detected/total ratio.
• DOCX header/footer hyperlinks extracted. mammoth only converts the body,
  so a header-placed "LinkedIn | GitHub" row was lost; we read the
  word/header*.xml / footer*.xml parts from the DOCX zip (jszip, added as a
  direct dep) and resolve their relationship-based targets.

Education — multi-qualification sections now extract every degree (was:
only the first, often corrupted). One chunk per qualification via a dual
boundary; handles degree-first / institution-first orders and acronym schools
(MIT, UC Berkeley).

Experience

• Company vs. designation: a university/college (incl. plural "Schools") counts
  as the company; expanded title keywords (Assistant, Intern, …) so designations
  are recognized.
• Boundary integrity: a wrapped bullet's marker-less tail (indented past the
  bullet marker) no longer leaks into the next entry's header.

Snapshots re-baked where wrapped-bullet / doc-wide-link changes shift counts.

Closes #29
Closes #30
Closes #31

Test plan

• npm run typecheck clean
• npm run test green (494 tests)
• npm run lint clean
• Manually verified in npm run dev (contact card, projects, skills, DOCX header links)
• Fixture personas verified synthetic — no real PII (Step 3.5): Chanchal Sharma, @example.com, (718) 555-0100

Known follow-ups (not in this PR)

• Per-entry experience location; is_current on en-dash "Present"
• Merging wrapped-bullet tails back into responsibility text (needs a corpus re-bake)

[rohithgollapalli]

Deferred — extractBulletsFromText is pre-existing code from main (#121), not introduced by this PR, and fallow runs report-only so it won't fail verify. Leaving this open; the lone-bullet-merge extraction is a sensible follow-up cleanup but out of scope for this review pass.

[s-annam]

PR Review: Word-template parsing + identity-link, education & experience extraction

Gates (run on checked-out branch): typecheck ✅ · test ✅ 575 passed (40 files) · lint ✅ · fixture PII ✅ (synthetic: Chanchal Sharma, chanchals@example.com, (718) 555-0100; no Info-dict/author leak).

Summary

Strong, well-commented extraction branch. Logic is sound, the cascade→scorer skillsSectionText wiring is fully threaded, contact-row optionality is a clean refactor, snapshots re-baked. One trivial-but-real blocker (a stray NUL byte); otherwise mergeable.

Highlights

• skillsSectionLines → skillsSectionText correctly threaded openresume → cascade (both PDF + markdown paths) → useResumeAnalysis → scorer buildSkillsExclusion; the pure scorer never re-derives sections.
• splitColumnCells reuses the now-exported mergeItemText rather than a blind \s+ split — multi-word skills survive.
• URLISH_RE's path-slash requirement deliberately spares dotted skills (Node.js, ASP.NET).
• phone en-dash fold is pre-gate-only; findPhoneNumbersInText still parses Unicode natively — matches the (718) 555–0100 fixture end-to-end.
• Education multi-degree chunking + educationFromChunk cleanly replaces the first-degree-only loop.

Key Findings

1. [Blocking] — src/lib/ingest/docx.ts:129 contains a raw NUL byte (U+0000) in source.
The dedup key is `${text}\x00${url}` with a literal NUL, not an escape. Consequence: git classifies the file as binary (the PR shows Bin 2468 → 7846 bytes), so the entire ~5KB of new DOCX header/footer logic is invisible in the PR diff and to code-scanning. NUL bytes in source are also an editor/formatter footgun. It works at runtime and tests pass — this is a reviewability/tooling blocker, not a correctness one. Trivial fix, identical runtime string:

const key = `${text}\u0000${url}`;   // keeps the file UTF-8 text + diffable

Confirmed three ways: python3 byte dump, file reporting data, and the binary diff. Introduced by this PR (base was plain text).

2. [Suggestion] — extractBulletsFromText cognitive complexity 20 (>15). Already surfaced by the github-advanced-security bot on score.ts:612. CI's fallow step is report-only (non-blocking), so this won't fail verify — but lifting the lone-bullet merge into a small helper would clear it.

3. [Nit] — entry-blocks.ts:140 — local let bulletMarkerX shadows the module-level bulletMarkerX(lines) (l.163) and re-inlines the identical computation; call the helper. (Inline below.)

Verdict

Action: REQUEST_CHANGES
Rationale: Single blocking item (#1) — fix the NUL byte so docx.ts stays diffable in main's history. Trivial change. #2/#3 optional. Everything else is solid.

[s-annam]

Two follow-ups from the architecture pass — neither blocks this PR:

• Refactor: split extract-fields.ts god-module (1060 LOC) into per-field modules #126 — split extract-fields.ts (now 1060 LOC) into per-field modules. Behavior-preserving refactor; this PR's +400 lines are fine where they are.
• Centralize section segmentation to retire downstream field-extractor special-cases #127 — centralize section segmentation so the scorer/extractors stop needing per-template side-channels (the skillsSectionText thread added here is the example). Relates to Generalize resume section recognition (layered recognizer) #109. This is the leverage item — better segmentation deletes whole classes of the downstream patches this branch adds one at a time.

The heuristic accretion in this PR is disciplined (pure named predicates + "non-trigger byte-identical" + fixtures), so this is about the next refactor, not this diff.

One note tied to the blocking NUL-byte item: because the literal NUL character (U+0000) makes git treat docx.ts as binary, the ~5KB of new DOCX header/footer logic never showed in the PR diff (I read the full file directly — logic is sound). Once the separator is written as the JavaScript escape \u0000 (backslash-u-0000) instead of a raw NUL byte, the file becomes UTF-8 text and diffable; worth a normal diff pass on it then.

[rohithgollapalli]

Code Review

Self-review pass by the branch author — still needs a second reviewer's approval to merge.

Overview

Extraction-layer fixes addressing a series of real-résumé bug reports: document-wide LinkedIn/GitHub recovery + DOCX header/footer hyperlink extraction (new jszip dep), multi-degree education extraction, experience company-vs-designation disambiguation, and wrapped-bullet-tail boundary integrity. Bundles the prior #29/#30/#31 commits and a merge of main (#119/#121/#123) with re-baked snapshots + algo bump to 1.2.

Correctness

• 🔴 Found & fixed during review (6ec3d8f): a stray NUL byte (U+0000) in docx.ts where a space belonged (a dedup-key separator). Behavior-equivalent so tests passed, but it made git classify the file binary → its diff was invisible in this PR. Now textual and reviewable.
• ✅ new RegExp from a URL slug in openresume.ts uses escapeRegex — no injection/ReDoS.
• ✅ Scanned all touched source: no other binary/NUL files, no console.log/TODO/.only/debugger leftovers.
• ✅ Merge resolution sound: score.ts keeps main's unexported const (dead-code gate), merges both changelogs, bumps 1.1→1.2; re-baked snapshots match merged behavior (awesome-cv-cv verified equal to main's #119 output).

Code quality & conventions

• Adheres to repo norms: SPDX headers, @design-system imports, domain logic in src/lib, dynamic imports for jszip/mammoth/turndown to keep the entry chunk lean, hand-rolled minimal interfaces instead of @types/jszip.
• Helpers are small, well-named, and commented with the why (looksLikeCompany, isWrappedContinuation, isLinkedinProfileUrl).
• Minor: anywhereOnDoc = () => true is a no-op band predicate — reads slightly oddly but clear in context.

Test coverage — strong

New unit tests for every behavior (contact rows, skills exclusion + dotted-skill guard, vanity LinkedIn, multi-degree education across both orderings + acronym schools, title/company disambiguation, x-positioned wrapped-bullet boundary, end-to-end DOCX-header zip round-trip). Full suite 575 green, typecheck + lint clean.

Risks / observations (non-blocking)

• ⚠️ PR size: 51 files bundling ~6 distinct fixes + 4 prior commits + a merge is a lot for one reviewer. Suggest a file-by-file skim.
• XML-via-regex: parseHeaderFooterHyperlinks parses DOCX header XML with regex, not a real parser — pragmatic and tested, fragile against unusual <w:hyperlink> nestings.
• Heuristic constants: splitColumnCells spacer width and isWrappedContinuation markerX + 2 — commented, low risk.
• Documented follow-ups (not in PR): per-entry experience location, is_current on en-dash "Present", and merging wrapped-bullet tails back into responsibility text.

Security

No secrets, no new network calls (jszip operates on in-browser DOCX bytes; everything stays client-side). Fixture PII preflight passed — synthetic personas.

Verdict

Approve-worthy once a second reviewer signs off. One real defect (NUL byte) surfaced and fixed in this pass; the rest is well-tested and convention-clean. Recommend a reviewer skim the now-visible docx.ts and the extract-fields.ts education/disambiguation hunks specifically.

[Samhit21]

PR Review: fix(heuristics): Word-template parsing + identity-link, education & experience extraction

Summary

Re-read post the NUL-byte fix (6ec3d8f) so docx.ts is diffable. The architecture is clean — promoted-link dedup is well-isolated in openresume.ts, the optional-row pattern in contact.ts carries
its own docstring, and splitColumnCells reusing mergeItemText is the right call. s-annam's review covered the load-bearing items (NUL byte, extractBulletsFromText complexity, bulletMarkerX
shadowing) and split out the right follow-up architecture issues (#126 / #127). Two net-new suggestions below — both real, neither blocking.

Findings

1. [Suggestion] decodeXml at docx.ts:88 misses numeric XML entity references (&,  ). Same gap that #117 closed in fetch-jd.ts; cheap fix is two extra .replace() calls before the
   named-entity pass. Hyperlink URLs and visible text from some Word save paths round-trip corrupted today.
2. [Suggestion] stripPromotedUrls' slug regex at openresume.ts:144 uses lookahead (?![\w/]), which doesn't exclude - or . — a user whose GitHub slug is a prefix of another handle in the
   body (e.g. foo vs foo-bar) gets chopped mid-token. Same regex also lacks /g, so a line that mentions the same identity link twice keeps the second copy. Hoist+widen the lookahead and add /g.

Verdict

Action: COMMENT — neither finding is merge-blocking. #1 is the higher-value catch (closer parallel to a recently-fixed bug elsewhere in the lib); #2 is the more interesting reasoning bug but
probability of triggering in the wild is low.

[rohithgollapalli]

Revision pushed: 1ccef64

Addressed the review feedback:

Thread	Resolution
NUL byte in docx.ts (s-annam, blocking)	✅ Fixed earlier in 6ec3d8f — file is UTF-8 text + diffable now (Samhit21 confirmed)
bulletMarkerX shadow (s-annam, nit)	✅ Fixed — calls the module-level helper
decodeXml numeric entities (Samhit21)	✅ Fixed + test (decimal & hex refs)
stripPromotedUrls slug lookahead (Samhit21)	✅ Fixed + test (longer prefix-sharing handle preserved); regex hoisted
extractBulletsFromText complexity (bot)	↪️ Deferred — pre-existing from #121, fallow is report-only; left open

Gates green on the branch: typecheck ✅ · test ✅ 577 passed · lint ✅. Re-requesting review — thanks @s-annam @Samhit21.

[s-annam]

@rohithgollapalli — re-reviewed the revision (1ccef64). Nice work clearing the four prior threads (NUL byte, bulletMarkerX shadow, decodeXml numeric entities, stripPromotedUrls lookahead) — all addressed and tested. Gates green locally: typecheck ✅ · test 616 passed ✅ · lint ✅. Browser smoke on a Word fixture parses end-to-end (contact, experience, education, skills all populate, no crash).

Holding approval on one thing — the corpus regression surface. This PR regenerates ~25 *.expected.json snapshots, so a green corpus run proves "output matches the new golden," not "no regression vs main." I diffed every changed snapshot against origin/main. Most deltas check out:

• bulletCount/totalBullets ↓ → wrapped-bullet-tail merge (expected from this PR)
• student-projects projectsCount 4→2 → verified correct (PDF has exactly 2 real projects; 4 was an over-count)
• skillsCount ↑, github_url recovered → improvements
• algoVersion 1.1→1.2 → intended ATS_SCORE_ALGO_VERSION bump

One delta is a real field loss I want you to confirm:

• unknown/weasyprint-cairo-two-column: achievementsCount 1→0, heuristic_achievements dropped from fieldsPopulated. I checked it against the PDF (it has an AWARDS section) and ran the cascade on both branches: main extracts it as [{title:"", year:"2021", description:"Acme Innovation Prize, 2023"}] — mangled by the two-column layout, but present; this branch drops it entirely (text stays in rawText, no structured field).

Four questions before I approve:

1. What corpus testing did you do beyond re-baking? Did you diff old vs new *.expected.json and eyeball each delta, or regenerate and confirm green?
2. weasyprint-cairo-two-column achievements drop — intended? If yes, what's the rationale; if no, let's fix it or file a follow-up.
3. The other projectsCount 2→1 decreases (google-docs-skia-proxy-nonstandard, weasyprint-cairo-nonstandard, google-docs-skia-proxy-two-column) — please confirm each is a corrected over-count, not a dropped real section.
4. With 51 files / ~6 fixes in one PR, can you give a per-fix → affected-fixtures map? That lets us verify blast radius per change rather than as one blob.

Everything else looks solid — once these are answered I'll approve.

[rohithgollapalli]

@s-annam — thanks for the careful diff; you found the one real problem. Answers below.

1. Corpus testing beyond re-baking. During development I diffed each changed snapshot against my pre-change HEAD and eyeballed the deltas, and live-verified the headline fixtures (e.g. dumped parsed projects for student-projects 4→2). What I did not do rigorously was diff the final merged state field-by-field against origin/main — which is exactly where the weasyprint-cairo-two-column achievement slipped through. Fair catch.

2. weasyprint-cairo-two-column achievements drop — not intended; filed as #131. Root cause: my new collectAnchors rule rejects first-line anchors at x > bulletMarkerX. This fixture is a two-column flatten with two bullet margins (awards at x≈48, experience at x≈249); the global bulletMarkerX=48 makes the award's "2021" line (x=54) and the whole right column read as wrapped-bullet tails → the section yields 0 blocks. On main the same section extracts {title:"", year:"2021", description:"Acme Innovation Prize, 2023"} — an empty-title entry that mis-merges two distinct awards (the 2021 is "Globex…", the description is "Acme…"). So it's mangled-noise → nothing, not usable → lost. The principled fix is column-aware segmentation (your #127), which makes the section parse correctly rather than either count being right. I filed #131 (linked to #127/#118) rather than bolt a fragile single-column special-case onto this PR. If you'd prefer I preserve the count now I can add a fallback (first_line yields 0 anchors → revert to the prevIsBullet rule), but that just restores the empty-title noise — I lean toward the follow-up.

3. The other projectsCount 2→1 drops — confirmed corrected over-counts (dumped the raw project sections):

• google-docs-skia-proxy-nonstandard & weasyprint-cairo-nonstandard: one real tinydb project; the dropped 2nd was the wrapped bullet tail "hardware." (x=65, past marker 59).
• google-docs-skia-proxy-two-column: one real opentrace project; dropped 2nd was the tail "OpenTelemetry export." (x=254).

All single-column wrapped tails that main mis-promoted to phantom headers. The surviving project per fixture is the real, clean one.

4. Per-fix → affected-fixtures map:

Fix	Snapshot effect	Fixtures
#29 column-skills split	skillsCount ↑	awesome-cv-cv (27→32), chromium-two-column-sidebar (10→18), two-column-achievements-sidebar (8→18); + new chanchal-sharma×2
Wrapped-bullet phantom removal	projectsCount ↓ (corrected over-counts)	google-docs-skia-proxy-nonstandard, google-docs-skia-proxy-two-column, weasyprint-cairo-nonstandard (each 2→1), student-projects (4→2)
Boundary header-recovery (flip side)	projectsCount ↑	chromium-two-column-sidebar (2→3) — real header after a wrapped bullet now opens an entry
Same rule on a 2-col flatten (regression → #131)	achievementsCount 1→0	weasyprint-cairo-two-column
#30 / #31 scoring	specificity / structure / completeness	bulleted-skills + redacted-date fixtures
algoVersion bump	algoVersion 1.1→1.2	all snapshots
doc-wide identity links	github_url in fieldsPopulated	deedy-resume-macfonts/openfonts

Net: every count delta is an intended improvement or a corrected over-count, except the #131 two-column-achievements regression — which I'd like to take as a follow-up since the principled fix is segmentation (#127), not a per-section x-tolerance patch. Happy to switch to the fallback instead if you'd rather not reduce the count in this PR.

[s-annam]

APPROVE

Re-reviewed the revision (1ccef64 / merged de04de6). All four held questions answered, and I verified the load-bearing ones against the code:

• weasyprint-cairo-two-column achievements 1→0 — not a real loss. collectAnchors (entry-blocks.ts:146, if (lines[i].x > markerX) continue;) uses the global min bulletMarkerX, which can't represent a two-column flatten with two bullet margins — confirmed. main's output there was mangled empty-title noise ({title:"", year:"2021", description:"Acme Innovation Prize, 2023"}, mis-merging two awards), so 0 ≥ noise. Correctly filed as #131; right fix is column-aware segmentation (#127, now wired as blocked_by). Accepting the deferral — a single-column fallback would just restore the noise.
• Other projectsCount 2→1 drops — confirmed corrected over-counts (wrapped-bullet tails main mis-promoted to phantom headers).
• Per-fix → fixture map — delivered; every count delta is an intended improvement or corrected over-count.

Gates green on the branch: typecheck ✅ · test 616 passed ✅ · lint ✅ · fixture PII ✅ · verify + fallow CI ✅. Browser smoke on a Word fixture parses end-to-end.

One process note, not blocking: the per-fix → fixture blast-radius map should ship in the PR body next time, not after a reviewer digs for it — that's the cost of a 51-file / 6-fix bundle. Ties into the #126 split. Nice work clearing the threads.