Conversation
Plumb libphonenumber isValid() from extraction to the anonymous scorer so a parsed-but-invalid phone earns half completeness credit instead of full. Plumbing changes (score.ts cannot import phone.ts — entry-chunk budget): - contact.ts: add phoneIsValid? to ContactExtractionResult and ContactScanResult; scan() sets it from phoneResult.isValid; extractContact() carries it from whichever scan (primary/fallback) supplied the phone. - types.ts: add phoneIsValid? to HeuristicParsedResume with doc comment. - openresume.ts: spread phoneIsValid into parsed when phone is present. - regex-fallback.ts: set parsed.phoneIsValid = phoneResult.isValid in the Tier 1.5 phone block. Scoring changes (score.ts): - Add phoneIsValid? to AnonymousAtsScoreInput.parsed. - Add PHONE_INVALID_CREDIT = 0.5 constant. - Split phone out of the generic contact-field loop: valid/unknown → full credit; phoneIsValid===false → passed:false, credit:0.5 (still surfaces in missing); absent/below floor → zero credit. - Bump ATS_SCORE_ALGO_VERSION 1.3 → 1.4; add changelog entry. Tests: - Four new score.test.ts cases covering all three tiers (valid, invalid half-credit, absent, below-confidence-floor). - 25 corpus expected.json re-baked: only algoVersion and the new phoneIsValid key in parsedKeys changed; no completeness scores shifted (all fixtures use valid 555-0100..0199 numbers per PII policy). Deferred edge: stale phoneIsValid when user edits phone inline (accepted; authed scorer stays presence-only, unaffected). Closes #70
rohithgollapalli
left a comment
There was a problem hiding this comment.
Review
Plumbing is clean and the scorer change is correct. The 25-fixture rebake touching only algoVersion + the new key confirms no score shifts. Two findings:
1. src/lib/edit/apply-overrides.ts:204-214 — editing the phone carries a stale phoneIsValid
nextParsed is built with { ...parsed } (line 196), copying the original phoneIsValid. The contact-override loop overwrites nextParsed.phone (line 212) but never refreshes phoneIsValid.
Scenario: a resume parses with an invalid phone (phoneIsValid: false → half credit, "phone" in missing). The user fixes the number in the editor → App.tsx re-grades with the override output → phone is the new valid string but phoneIsValid is still false, so the scorer keeps awarding half credit and keeps listing phone as missing. The correction has no effect on the score.
Fix: when phone is overridden, delete nextParsed.phoneIsValid (falls back to backward-compatible full credit) — mirroring the empty-string clear path.
2. src/lib/score/score.test.ts:722 — half-credit test's upper bound is too weak
expect(invalidResult.completeness.score).toBeLessThanOrEqual(validResult.completeness.score) passes even if invalid phones were granted full credit — i.e. the test stays green if the feature breaks in exactly its core direction. Only the > absentResult assertion has teeth.
Fix: use toBeLessThan(...) (here 29 < 30, so it holds strictly).
Finding #1 is the substantive one — a real gap in the edit re-grade path this PR introduces.
🤖 Generated with Claude Code
Address PR #157 review (rohithgollapalli): - applyOverrides now drops the stale `phoneIsValid` flag whenever the phone field is overridden or cleared. Without this, a user who fixed an invalid phone in the editor kept the old `false`, so the re-grade went on awarding half completeness credit and listing phone as missing — the correction had no effect. Mirrors the empty-string clear path. - score.test.ts half-credit case: toBeLessThanOrEqual -> toBeLessThan so the assertion fails if invalid phones are ever granted full credit (the exact direction the feature would break). - Add apply-overrides regression coverage for both clear paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018GAj62FwHV2WHunBTGCR9P
|
Both review findings addressed in 1c7f5c8:
Typecheck clean, 724/724 green. |
# Conflicts: # tests/fixtures/pdfs/word/chanchal-sharma-bulleted-skills.expected.json # tests/fixtures/pdfs/word/chanchal-sharma-sample.expected.json
Plumbs libphonenumber isValid() to the anonymous scorer: a parsed-but-invalid phone earns half completeness credit; valid/absent phones unchanged. Edit re-grade clears stale phoneIsValid on phone override/clear. Resolves #70.
Plumbs libphonenumber isValid() to the anonymous scorer: a parsed-but-invalid phone earns half completeness credit; valid/absent phones unchanged. Edit re-grade clears stale phoneIsValid on phone override/clear. Resolves #70.
Summary
Plumbs `libphonenumber` `isValid()` from the extraction layer all the way to the anonymous scorer so a parsed-but-invalid phone earns half completeness credit instead of full. Valid phones and absent phones are unchanged.
Three-tier scoring: valid/unknown validity → full credit;
phoneIsValid===false→passed: false, credit: 0.5(still surfaces in `missing`); absent/below confidence floor → zero credit. Mirrors the redacted-dates half-credit pattern.Closes #70
What changed
Plumbing (scorer cannot import
phone.ts— entry-chunk budget):contact.ts: addphoneIsValid?toContactExtractionResult;scan()sets it fromphoneResult.isValid;extractContact()carries it from whichever scan supplied the phone.types.ts: addphoneIsValid?toHeuristicParsedResume.openresume.ts: spreadphoneIsValidintoparsedwhen phone is present.regex-fallback.ts: setparsed.phoneIsValidin the Tier 1.5 phone block.Scoring (
score.ts):phoneIsValid?toAnonymousAtsScoreInput.parsed.PHONE_INVALID_CREDIT = 0.5constant.ATS_SCORE_ALGO_VERSION1.3 → 1.4 + changelog entry.Tests: 4 new
score.test.tscases; 25 corpusexpected.jsonre-baked (onlyalgoVersionand newphoneIsValidkey changed — no completeness scores shifted, confirming all fixtures use valid 555-0100..0199 numbers per PII policy).Test plan
npm run typecheckcleannpm run testgreen (722/722)npm run dev/npm run preview