refactor: extract the edit, résumé-format, and suffix-lexicon contracts - #922
Open
s-annam wants to merge 1 commit into
Open
refactor: extract the edit, résumé-format, and suffix-lexicon contracts#922s-annam wants to merge 1 commit into
s-annam wants to merge 1 commit into
Conversation
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
Deploying offlinecv with
|
| 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 |
| } from "./separators.ts"; | ||
|
|
||
| export { composeRoleHeader, splitRoleHeader } from "./role-header.ts"; | ||
| export type { RoleHeaderFields, ComposedRoleHeader } from "./role-header.ts"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
applyOverridestook 19 positional parameters and had drifted four times.It now takes
(base: EditBase, snapshot: EditOverrides)withEditOverrides = Partial<EditSnapshot>, so the channel set derives from the persistedsnapshot type and cannot drift from it again. The fold/re-score pipeline moves out of
useAnalyzedResumeintosrc/lib/edit/edit-pipeline.ts, and four hand-builtAnonymousAtsScoreInputconstructions collapse ontoscoreParsedResume.EditSnapshot's persisted shape is unchanged, so stored drafts and library recordsstill replay.
src/lib/resume-format/, with acomposeRoleHeader/splitRoleHeaderpair whoseinvertible domain is stated against measured production behaviour and gated by a test
that runs the real
buildAtsResumeModel → renderAtsResumePdf → runCascadeleg. Thedate-range dialect is deliberately untouched — unifying it changes rendered bytes and
needs its own snapshot sweep.
[parser] Pass B single-token city rule cleaves a company whose last word precedes a state tail (Palo Alto Networks, CA) #641 trailing-period fix had to be diagnosed and applied per copy. One canonical token
base plus a composer now generates all four, with
allowTrailingDotexpressing [parser] Pass B single-token city rule cleaves a company whose last word precedes a state tail (Palo Alto Networks, CA) #641once. The sets keep their own memberships: the tail-deferral set stays deliberately
broader than the strict one, so "Media Director" still parses as a title.
Closes #652
Closes #649
Closes #917
Review focus
src/lib/edit/apply-overrides.ts—Partial<EditSnapshot>makes an omitted channel asilent 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— thescorememo's dep list is byte-identical, but thefold memo now takes
snapshotwholesale. Does that make it easier for a future channel toreach the fold without ever reaching the score? (One already does — see the
descriptionOverridesnote below.)src/lib/heuristics/extract/corporate-suffix.ts— the four sets now select tokens by keyfrom 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""toabsent, so its claim is "recovers exactly modulo
''→undefined". Does that hide a realdivergence on the empty-company dialect?
Test plan
npm run typecheckcleannpm run lintcleannpm run verifyexits 0 (full suite, build, packaging + fixture + baseline gates)npx vitest run— 387 files / 6539 tests passUPDATE_FIXTURESusedAdversarial 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.
splitRoleHeaderdocumented 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 (
buildAtsResumeModel→renderAtsResumePdf→
runCascade) over all 13 declared-invertible cases, 3 disagreed:"Director, Marketing"shape){title:"Kilo Engineer, Sr.", company:"Globex"}{title:"Kilo Engineer", company:"Sr.", team:"Globex"}{team:"Payments · Risk"}{team:"Payments"}—"· Risk"dropped" 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.tsas a drift gate that runs the realproduction 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
composeSuffixRegexexisted. Each call site still hand-typed free strings, so the sameconcept was already spelled two ways across sets (
"l.l.c"vs"L.L.C") with nothing tocatch a typo — the exact class of drift the issue exists to end. Fixed with
SUFFIX_TOKENS(39 canonical entries) +SuffixTokenKey+selectSuffixTokens; all foursets now select by key, so a typo is a compile error. Memberships are unchanged — the broad
tail-deferral set keeps
Media/Partners/Ventures/Bankand the strict set stillrejects them, so "Media Director" still parses as a title.
Verification of the behaviour-preserving claims
.source/.flagsvalues were recomputedindependently 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 outputwould prove nothing). 4/4 match. Goldens were not edited.
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.
scorememo dep array — byte-identical toHEAD; the round-1 fix was comment-only.(55 in
apply-overrides.test.tsalone) found no argument in a wrong field.Round 2 — clean, no blocking findings
Surviving nits (not acted on)
experience-disambiguate.ts:29— nothing pinsHEADER_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 noflags argument never inherits flags, so it cannot acquire
g/lastIndexstate.)role-header-production-domain.test.ts:96—headerFieldsOfnormalises""to absent, sothe row-level claim is "recovers exactly modulo
''→undefined". Documented in-file.Pre-existing bugs found and deliberately NOT fixed (out of scope)
descriptionOverridesnever re-grades. It is not ascorememo dep but writesexperience[].description, whichscore.tsuses as the bullet-pool fallback forglyph-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.bulletskeeps pre-edit text, and
groupBulletsByExperienceon that stale pool returnsexperienceIndex: null, so the role's editable rows detach into "Other". Export isunaffected. 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
scorememo names the knowingly-absent channels to restore that signal.claimedBulletKeys, bypassingscoreEditedResumewhose point is that the key is required — so they simulate productionwith the Stable bullet identity through parse → edit → export #648 defect present. Pre-existing, untouched by the codemod.