Skip to content

refactor(#134, #135): contact-link ownership model + real profile-section boundary - #140

Merged
s-annam merged 1 commit into
mainfrom
gh-134-135-contact-ownership
Jun 21, 2026
Merged

refactor(#134, #135): contact-link ownership model + real profile-section boundary#140
s-annam merged 1 commit into
mainfrom
gh-134-135-contact-ownership

Conversation

@s-annam

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

Copy link
Copy Markdown
Contributor

Summary

Replaces two segmentation-imprecision workarounds in contact extraction with the proper signals from the typed SectionedResume (#138). Stacked on #138 — base is refactor-137-132-fallow-sectioned, not main.

  • Replace stripPromotedUrls phantom-entry scrub with a section-ownership/provenance model #134 — retire the stripPromotedUrls after-the-fact slug-subtraction scrub (isPromotedUrl, PROMOTED_LABEL_RE, urlSlug, the per-section strip driver) for a line-level ownership model: extractContact returns consumedLines: ReadonlySet<PdfLine>, and 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 entry.
  • Replace extractContact y-band header proxy with a real contact/links section boundary #135 — replace the geometric y-band header proxy (inHeaderRegion: ann.yTop < 280) with inProfileSection, a real boundary from the profile section's line extents; portfolio_url/website_url annotation lookups consult it. Document-wide LinkedIn/GitHub identity match (anywhereOnDoc) retained by design (spike §1.4).

Closes #134
Closes #135
Refs #109, #127

Test plan

Stacked-PR note

Base is the unmerged #138 branch. GitHub auto-retargets this PR to main once #138 merges; then rebase to drop #138's commits:
git rebase --onto main refactor-137-132-fallow-sectioned gh-134-135-contact-ownership

@rohithgollapalli

Copy link
Copy Markdown
Collaborator

Review — Approve ✅ (stacked on #138 — merge that first)

A clean, well-motivated refactor: it trades two segmentation-imprecision workarounds for the proper signals from SectionedResume. The after-the-fact stripPromotedUrls slug-scrub becomes a line-level ownership model (consumedLines), and the yTop < 280 magic-number band becomes a real inProfileSection boundary derived from profile line extents. Verified locally on gh-134-135-contact-ownership.

Verification performed

  • npm run typecheck / npm run lint clean
  • npm run test616/616 pass, 41 files; 24/24 corpus snapshots unchanged (no parsed-output drift from the rewrite)
  • ✅ No yTop < 280 band remains in contact extraction

Load-bearing invariants — both verified

  1. Coordinate-system consistency (the inProfileSection change). PdfLine.y and PdfLinkAnnotation.yTop are both top-origin (y grows downward) — documented at types.ts:60-61 and produced that way in pdf-extract.ts (yTop = viewport.height - … for both text items and annotation rects). So ann.yTop <= profileLineMaxY + slack is a valid comparison, and with top-origin Math.max(profile.y) is the bottom of the profile band — the correct boundary. The Infinity fallback for an empty profile (accept any page-1 annotation) is a sensible conservative default.
  2. Referential equality as the ownership key (the consumedLines change). findConsumedLines collects PdfLine objects from the document lines, and stripConsumedLines filters sections[].lines by consumed.has(l). I confirmed all three section-builders push the same line objects (no cloning): splitIntoSections (sections.ts:478), splitIntoSectionsWithMarkdown (:608), and sectionizeMarkdownLines (markdown-lines.ts:338). So identity-based ownership holds on every path.

Other things I checked

  • De-dup path is covered, despite zero test-file changes: the three parseHeuristicFromMarkdown — promoted-link de-duplication tests exercise the new ownership model end-to-end — phantom removal, deeper-repo-path preservation, and the longer-handle (?![\w./-]) boundary. The precision rule survived the move intact.
  • Regex statefulness: findConsumedLines reuses the g-flagged matchers across lines but resets lastIndex before both .test() and .replace(), so no cross-line skips. URLs are stripped before the label prefix, correctly preventing the label alternation (github/linkedin) from chewing the host out of the URL.
  • urlSlug / escapeRegex moved cleanly to contact.ts; no duplicate left in openresume.ts, import paths correct.

Notes (non-blocking)

  1. Whole-line ownership is narrower than the old substring scrub — by design, but two edge cases lose the old behavior and aren't tested:
    • A real bullet that mentions the exact identity link inline ("Built github.com/janesmith integration…") is no longer consumed (it has prose residue) and now keeps the URL inline, where stripPromotedUrls would have scrubbed the substring. Arguably better (it's genuine content), just a visible double-mention with the contact card.
    • The old isPromotedUrl cleared a structured project/achievement .url field equal to the promoted slug. That field-clearing is gone — a structured entry whose line carries prose and the bare identity URL as its .url could now render the link in both contact and the entry. Rare (project URLs are normally deeper paths, which the lookahead already excludes), and the corpus is unchanged, so no current fixture hits it. Worth a line in a follow-up test if you want to pin it.
  2. PROFILE_REGION_SLACK_PTS = 12 is a reasonable "one line-height" heuristic, but it's the one remaining tuned constant in the new boundary — fine, just flagging it replaced one magic number with a smaller, better-justified one rather than zero.

None block merge. Good structural cleanup with the risky invariants holding up under inspection.

🤖 Generated with Claude Code

@s-annam
s-annam deleted the branch main June 21, 2026 16:14
@s-annam s-annam closed this Jun 21, 2026
@s-annam s-annam reopened this Jun 21, 2026
@s-annam
s-annam changed the base branch from refactor-137-132-fallow-sectioned to main June 21, 2026 16:26
@s-annam
s-annam dismissed rohithgollapalli’s stale review June 21, 2026 16:26

The base branch was changed.

…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
@s-annam
s-annam force-pushed the gh-134-135-contact-ownership branch from 77e6d03 to 7176439 Compare June 21, 2026 16:28
@s-annam

s-annam commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main now that #138 is merged. Same as #139, this PR auto-closed when its stacked base (refactor-137-132-fallow-sectioned) was deleted on #138's squash-merge; reopened and retargeted to main. Dropped this branch's copy of the #137/#132 work via git rebase --onto origin/main d09fca2, leaving a single clean commit (7176439).

One conflict resolved in contact.ts: #138's nit fix had moved the extractContact doc-comment down onto the function, while this branch added the ContactScanResult type alias in the same region. Kept both — the type alias plus the doc-comment in its new on-function position (no duplicate).

Gates green on the rebased branch: npm run typecheck clean, npm run lint clean, npm run test 616/616 (24/24 corpus snapshots unchanged). Mergeable now clean (was CONFLICTING).

Both review notes are non-blocking observations (narrower whole-line ownership edge cases; the one remaining PROFILE_REGION_SLACK_PTS tuned constant) — flagging, not change requests — so leaving code as-is. The edge cases reviewer mentions are unhit by the corpus; deferring dedicated pins as out of scope for this refactor.

@s-annam
s-annam merged commit 3343d98 into main Jun 21, 2026
2 checks passed
@s-annam
s-annam deleted the gh-134-135-contact-ownership branch June 21, 2026 16:29
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