Skip to content

refactor: extract the edit, résumé-format, and suffix-lexicon contracts - #922

Open
s-annam wants to merge 1 commit into
mainfrom
gh-batch-652-649-917-extract-shared-contracts
Open

refactor: extract the edit, résumé-format, and suffix-lexicon contracts#922
s-annam wants to merge 1 commit into
mainfrom
gh-batch-652-649-917-extract-shared-contracts

Conversation

@s-annam

@s-annam s-annam commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Three seams that were coupled only by prose become named, tested contracts. No behaviour
changes — corpus snapshots are byte-identical and the exporter draws the same bytes.

Closes #652
Closes #649
Closes #917

Review focus

  • src/lib/edit/apply-overrides.tsPartial<EditSnapshot> makes an omitted channel a
    silent no-op. Is there a call site that the old positional signature forced to pass a
    channel, which can now quietly skip it?
  • src/hooks/useAnalyzedResume.ts — the score memo's dep list is byte-identical, but the
    fold memo now takes snapshot wholesale. Does that make it easier for a future channel to
    reach the fold without ever reaching the score? (One already does — see the
    descriptionOverrides note below.)
  • src/lib/heuristics/extract/corporate-suffix.ts — the four sets now select tokens by key
    from a shared base. Do those key lists still express the same per-set human judgement the
    old free-text lists did, particularly where the broad set must stay broader?
  • src/lib/pdf/role-header-production-domain.test.ts:96 — the gate normalises "" to
    absent, so its claim is "recovers exactly modulo ''undefined". Does that hide a real
    divergence on the empty-company dialect?

Test plan

  • npm run typecheck clean
  • npm run lint clean
  • npm run verify exits 0 (full suite, build, packaging + fixture + baseline gates)
  • Full npx vitest run — 387 files / 6539 tests pass
  • Corpus snapshots byte-identical; no UPDATE_FIXTURES used
  • No fixture binary added or changed by this PR

Adversarial review

Two rounds, pre-commit, on the accumulated tree. Round 1 split across two independent
reviewers on disjoint surfaces (React/hooks; parser/export lib); round 2 audited the fix.

Round 1 — 2 blocking, both fixed

1. splitRoleHeader documented an invertible domain that production contradicts.
The module shipped with no production consumer, justified as "the executable spec the
heuristics approximate". Running the real leg (buildAtsResumeModelrenderAtsResumePdf
runCascade) over all 13 declared-invertible cases, 3 disagreed:

case spec said production actually returns
title with a comma ("Director, Marketing" shape) {title:"Kilo Engineer, Sr.", company:"Globex"} {title:"Kilo Engineer", company:"Sr.", team:"Globex"}
team with a middot {team:"Payments · Risk"} {team:"Payments"}"· Risk" dropped
untrimmed company padding " Acme " verbatim "Acme"

Worse than dead code: the docblock and its test certified a live round-trip defect as safe,
and comma-bearing titles are a common real shape. Fixed by restating the domain against
measured behaviour (4 clauses → 6), moving the 3 cases into the "shapes the format loses"
block with production's real answers pinned as measured data, and adding
src/lib/pdf/role-header-production-domain.test.ts as a drift gate that runs the real
production leg. Round 2 confirmed the gate is not vacuous: all 13 rows assert (26 tests),
and under mutation it fails (separator change → 11/26 fail; flipped pin → 2/26).

2. #917 AC1 was half-unmet — the composer shipped without a token base.
AC1 asks for "one token base + composer; the four vocabularies generated from it", but only
composeSuffixRegex existed. Each call site still hand-typed free strings, so the same
concept was already spelled two ways across sets ("l.l.c" vs "L.L.C") with nothing to
catch a typo — the exact class of drift the issue exists to end. Fixed with
SUFFIX_TOKENS (39 canonical entries) + SuffixTokenKey + selectSuffixTokens; all four
sets now select by key, so a typo is a compile error. Memberships are unchanged — the broad
tail-deferral set keeps Media/Partners/Ventures/Bank and the strict set still
rejects them, so "Media Director" still parses as a title.

Verification of the behaviour-preserving claims

  • Byte-identical regexes — the four composed .source/.flags values were recomputed
    independently from the current call sites and compared against literals from
    git show HEAD:, without reading the test's own goldens (a golden copied from new output
    would prove nothing). 4/4 match. Goldens were not edited.
  • Download-export follow-up (#421 Secondary #11) #428 score-reference identity — verified empirically, not by reading: six transitions
    driven through a probe component. Non-scoring extras keep the score object reference; a
    back-fill into an empty slot, a certification edit, and a link clear each mint a new one.
  • score memo dep array — byte-identical to HEAD; the round-1 fix was comment-only.
  • Codemod correctness — a positional→named comparator over all ~20 rewritten call sites
    (55 in apply-overrides.test.ts alone) found no argument in a wrong field.

Round 2 — clean, no blocking findings

Surviving nits (not acted on)

  • experience-disambiguate.ts:29 — nothing pins HEADER_DELIM_SPLIT_RE's spliced .source.
    Latent only; pinning it would mean exporting a module-private constant purely for
    testability. (Note the flag risk is narrower than it first looks: new RegExp(src) with no
    flags argument never inherits flags, so it cannot acquire g/lastIndex state.)
  • role-header-production-domain.test.ts:96headerFieldsOf normalises "" to absent, so
    the row-level claim is "recovers exactly modulo ''undefined". Documented in-file.

Pre-existing bugs found and deliberately NOT fixed (out of scope)

  • descriptionOverrides never re-grades. It is not a score memo dep but writes
    experience[].description, which score.ts uses as the bullet-pool fallback for
    glyph-less résumés. Repro: edit a description → score object identical, overall stays 24;
    then touch any unrelated scoring channel → jumps to 94. Two further symptoms: score.bullets
    keeps pre-edit text, and groupBulletsByExperience on that stale pool returns
    experienceIndex: null, so the role's editable rows detach into "Other". Export is
    unaffected. Pre-existing and unchanged in behaviour by this PR, but less detectable
    after it — the two dep lists used to sit adjacent and diffable, so a missing channel was
    visible; now a new channel joins the fold automatically and the score only by hand. A
    comment in the score memo names the knowingly-absent channels to restore that signal.
  • Five test harnesses hand-build an edited grade without claimedBulletKeys, bypassing
    scoreEditedResume whose point is that the key is required — so they simulate production
    with the Stable bullet identity through parse → edit → export #648 defect present. Pre-existing, untouched by the codemod.

Three seams that were coupled only by prose become named, tested contracts.
No behaviour changes: corpus snapshots are byte-identical and the exporter
draws the same bytes it drew before.

`applyOverrides` took 19 positional parameters and had drifted four times
(#652). It now takes `(base: EditBase, snapshot: EditOverrides)`, where
`EditOverrides = Partial<EditSnapshot>` — so the channel set is derived from
the persisted snapshot type and cannot drift from it again. The fold/re-score
pipeline moves out of `useAnalyzedResume` into `src/lib/edit/edit-pipeline.ts`,
and four hand-built `AnonymousAtsScoreInput` constructions collapse onto
`scoreParsedResume`. `EditSnapshot`'s persisted shape is unchanged, so stored
drafts and library records still replay.

The parser hard-coded what the exporter draws at sites in six modules (#649).
The separator contract moves into a zero-dep `src/lib/resume-format/`:
`MIDDOT`, `MIDDOT_JOIN`, `MIDDOT_SPLIT_RE`, `HEADER_DATE_GAP`,
`HEADER_WRAP_INDENT`, and a `composeRoleHeader`/`splitRoleHeader` pair whose
invertible domain is stated against measured production behaviour and gated by
a test that runs the real `buildAtsResumeModel → renderAtsResumePdf →
runCascade` leg. The date-range dialect is deliberately left alone; unifying it
changes rendered bytes and needs its own snapshot sweep.

Corporate-suffix vocabulary existed as four separate sets, which is why the
#641 trailing-period fix had to be diagnosed and applied per copy (#917).
`extract/corporate-suffix.ts` now holds one canonical token base plus a
composer, with `allowTrailingDot` expressing #641 once. The four sets select
from the base by key — a typo is a compile error — but keep their own
memberships: the tail-deferral set stays deliberately broader than the strict
one, so a real title like "Media Director" still parses as a title. Each
generated regex is pinned byte-identical in `.source` and `.flags`.
`title-shape.ts` keeps its hand-written set to preserve its import-leaf
contract from #605.

Closes #652
Closes #649
Closes #917
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying offlinecv with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7afc414
Status: ✅  Deploy successful!
Preview URL: https://aceca8ed.offlinecv.pages.dev
Branch Preview URL: https://gh-batch-652-649-917-extract.offlinecv.pages.dev

View logs

} from "./separators.ts";

export { composeRoleHeader, splitRoleHeader } from "./role-header.ts";
export type { RoleHeaderFields, ComposedRoleHeader } from "./role-header.ts";
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