Skip to content

refactor: decompose scan/extractSkills + typed SectionedResume (#137, #132) - #138

Merged
s-annam merged 2 commits into
mainfrom
refactor-137-132-fallow-sectioned
Jun 21, 2026
Merged

refactor: decompose scan/extractSkills + typed SectionedResume (#137, #132)#138
s-annam merged 2 commits into
mainfrom
refactor-137-132-fallow-sectioned

Conversation

@s-annam

@s-annam s-annam commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Two behavior-preserving refactors bundled onto one branch (epic run, disjoint blast radii):

Closes #137
Closes #132

Test plan

  • npm run typecheck clean
  • npm run test green (616/616, 41 files)
  • npm run lint clean
  • Zero *.expected.json churn (goldens NOT regenerated — clean diff is the pass condition)
  • Manually verified in npm run dev / npm run preview

…dResume

- #137: split scan() (extract/contact.ts) and extractSkills()
  (extract/skills.ts) into smaller helpers to clear fallow
  cognitive-complexity >15; behavior-preserving, byte-identical snapshots
- #132: promote typed SectionedResume onto CascadeResult and retire the
  skillsSectionText side-channel end-to-end; scorer now derives
  skills-exclusion from sections.byName.get("skills"); corpus goldens
  unchanged

Resolves #137
Resolves #132

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GstfZc8CpvugDY85bS541Y
@rohithgollapalli

Copy link
Copy Markdown
Collaborator

Review — Approve ✅

Two clean, behavior-preserving refactors with disjoint blast radii. All gates pass locally.

Verification performed

  • npm run typecheck clean
  • npm run lint clean
  • npm run test616/616 pass, 41 files
  • ✅ Zero *.expected.json churn (git diff --stat origin/main...HEAD on fixtures is empty — the stated pass condition)
  • ✅ No lingering code references to skillsSectionText/skillsSectionLines — remaining hits are all doc comments / docs/segmentation-spike.md

#137 — decompose scan / extractSkills

  • contact.ts: extractLocation, extractOtherUrls, scan lifted out of the extractContact closure verbatim. Diffed line-by-line — location precedence (US→intl, @ guard), phone-region derivation order, LinkedIn fallback chain, portfolio/website bucketing all identical.
  • skills.ts: tokenizeCell is a faithful extraction of the inner loop, preserving the critical ordering (drop contact-link cells before SKILL_SPLIT_RE shreds the URL).

#132 — typed SectionedResume, retire skillsSectionText

Behavior preservation hinges on byName.get("skills") being byte-identical to the old skillsSectionLines, and it holds:

  • Old: findSection(sections, "skills") flattens all matching sections' lines in document order, then .map(trim).filter(nonEmpty). New: toSectionedResume iterates sections in document order and accumulates per-section trimmed/filtered lines. Per-line trim/filter ⇒ same array; accumulate-don't-overwrite correctly mirrors findSection's multi-match flatten.
  • Scorer guard is equivalent: old if (!skillsSectionText) vs new if (!lines || lines.length === 0) — both collapse empty/absent to undefined. The split(/\r?\n/) round-trip is removed without changing the resulting set.
  • The two no-Tier-1 defaults (runCascadeFromMarkdown no-markdown path, buildScannedResult) correctly yield byName.get("skills") === undefined, matching the previously-absent skillsSectionText.

Minor (non-blocking)

  1. accomplishmentSections default inconsistencytoSectionedResume sets the canonical 3-name list, but the fallback defaults (cascade scanned / no-markdown paths and test helpers) set []. Inert today (documented as "not yet consumed"), but a future consumer reading it off a scanned result would get [] rather than the canonical list. Consider reusing ACCOMPLISHMENT_SECTION_NAMES in those defaults.
  2. CosmeticextractContact now opens with a stray blank line (contact.ts:169) where the scan closure used to sit.
  3. The extractContact doc-comment (contact.ts:39–55) is now ~125 lines above the function it documents (the refactor widened a pre-existing gap); optionally worth moving down onto the function.

None block merge.

🤖 Generated with Claude Code

- Export ACCOMPLISHMENT_SECTION_NAMES and reuse it in the cascade
  no-markdown / scanned-abandon fallback defaults instead of [], so a
  future consumer reading accomplishmentSections off a scanned result
  gets the canonical list, matching toSectionedResume.
- Move the extractContact doc-comment back down onto the function (the
  scan/extractLocation/extractOtherUrls extraction had widened the gap).
- Drop the stray leading blank line in extractContact.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GstfZc8CpvugDY85bS541Y
@s-annam

s-annam commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all three minor nits from the review in de869c0:

  1. accomplishmentSections default inconsistency — exported ACCOMPLISHMENT_SECTION_NAMES from sections.ts and reused it in both cascade fallback defaults (no-markdown path cascade.ts:357, scanned-abandon cascade.ts:476) in place of []. A future consumer reading accomplishmentSections off a scanned result now gets the canonical list, matching toSectionedResume.
  2. Stray blank line — removed the leading blank line in extractContact.
  3. Orphaned doc-comment — moved the extractContact doc-comment back down onto the function (the scan/extractLocation/extractOtherUrls extraction had widened the gap).

Gates green on the branch: npm run typecheck clean, npm run lint clean, npm run test 616/616. Zero *.expected.json churn. Re-requesting review since the push dismissed the prior approval.

@s-annam
s-annam requested a review from rohithgollapalli June 21, 2026 16:13
@s-annam
s-annam merged commit 12fe7b8 into main Jun 21, 2026
2 checks passed
@s-annam
s-annam deleted the refactor-137-132-fallow-sectioned branch June 21, 2026 16:14
@s-annam
s-annam restored the refactor-137-132-fallow-sectioned branch June 21, 2026 16:18
@s-annam
s-annam deleted the refactor-137-132-fallow-sectioned branch June 21, 2026 16:18
@s-annam
s-annam restored the refactor-137-132-fallow-sectioned branch June 21, 2026 16:26
@s-annam
s-annam deleted the refactor-137-132-fallow-sectioned branch June 21, 2026 16:26
s-annam added a commit that referenced this pull request Jun 21, 2026
…tion boundary

Replace two segmentation-imprecision workarounds in contact extraction with
the proper signals now available from the typed SectionedResume (#138).

- #134: retire the stripPromotedUrls after-the-fact slug-subtraction scrub
  (isPromotedUrl, PROMOTED_LABEL_RE, urlSlug, per-section strip driver) in
  favor of a line-level ownership model. extractContact now returns
  consumedLines: ReadonlySet<PdfLine>; buildHeuristicResult strips owned
  lines from body pools before extraction, so a promoted identity link is
  claimed by contact and never re-renders as a phantom project/achievement.
  #125 identity-link fixtures pass; zero corpus-golden movement.

- #135: replace extractContact's geometric y-band header proxy
  (inHeaderRegion: ann.yTop < 280) with inProfileSection, a real boundary
  derived from the profile section's line extents. portfolio_url/website_url
  annotation lookups consult it; the document-wide LinkedIn/GitHub identity
  match (anywhereOnDoc) is retained by design (spike §1.4). No PDF-points
  magic number remains; contact-field counts unchanged on all fixtures.

Verified: typecheck clean, 616/616 tests pass, 24/24 corpus snapshots
unchanged. Refs #109, #127.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GstfZc8CpvugDY85bS541Y
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants