From 14ffb56f739e18855ffa1600cad767135e967543 Mon Sep 17 00:00:00 2001 From: Vaishnavi Kale Date: Wed, 9 Sep 2026 15:47:00 -0700 Subject: [PATCH] test(edit): close two coverage gaps left by #682 (#817) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both gaps were green on main and both were one fixture away from mattering. 1. The corpus edit-leg absence check could be satisfied by role prose. `experienceFieldCheck`'s absence half searched `absenceText` — for the date call, `j(hit)`, the JSON of the whole edited role with `description` included — for `replaced`, which is the fixture's OWN prior `start_date` and frequently a bare "2019". Any future fixture whose role-0 prose merely mentions that year ("shipped the platform in 2019") would fail the gate for a reason unrelated to the slot under test. The title/company keys are safe on that wide slice by luck, not design: their literals are long and distinctive. A four-digit year is not. Fixed inside `experienceFieldCheck` rather than at the call site. Its presence half is already field-aware, so the absence half should be too, instead of every caller having to remember which slice is safe for which key. A date key now searches `editedRoleDateText` — the edited role's `start_date`/`end_date` slots and nothing else. The narrowing forfeits nothing: a replaced `start_date` can only reach `title`/`company` if `applyExperienceHeaderOverrides` writes a date into a header field, which is a different defect class with its own gate, and the one way the collapse CAN misplace the value — leaving it in `end_date` — is inside the narrowed slice and already asserted from the other side. The wide slice was only ever buying false failures. 2. `formatExperienceDateRange` had no direct test. It is exported and consumed on both the edit path (`ReconstructedRole`) and the EXPORT path (`ats-resume-model.ts`), and its `.trim()` calls are a documented, deliberate, reachable behaviour change on the latter. The #682 review found that removing both trims left `src/lib/edit/**` and two repro suites entirely green. Adds a `describe` block over the trim, the whitespace-only slot, the `is_current` → "Present" substitution (including that it out-ranks a populated `end_date`), the unanchored-`is_current` shape, the three fallthrough shapes and the empty case. 3. The module docblock was wrong about the function, and nothing caught it. Writing the test above falsified `experience-dates.ts`'s own prose: it claimed the two display-only formatters draw "Present" where `formatExperienceDateRange` draws "". All three draw "Present" — on any `is_current` row `end` is "Present" before either fallthrough is reached, so "" is unreachable. The real divergence is one row over and is a TRIM, not a collapse: on `{start_date: " ", is_current: true}` this module draws "Present" while both siblings read " " as a real anchor and draw " –Present". The docblock now says that, with the row tested beside it, and the stale sibling line references are corrected. That row is the intersection of the two behaviours this change pins — the trim and the substitution were covered separately, never together — and it is the one the module docblock names as the #672 hazard. Dropping both trims now turns 3 cases red rather than 2. Both groups are falsified against the pre-fix code: - Removing the two `.trim()` calls turns 3 of the 9 new cases red. - Restoring the wide-slice absence half turns exactly 1 of the 3 new cases red — the prose one — while the two controls stay green, so the new test discriminates rather than passing trivially. The only production change is comment-only: the corrected docblock. Full suite 6462 passed, up from 6450 by exactly the 12 cases added. --- src/lib/edit/experience-dates.test.ts | 88 +++++++++++++ src/lib/edit/experience-dates.ts | 35 +++-- .../heuristics/corpus-edit-roundtrip.test.ts | 123 ++++++++++++++++-- 3 files changed, 227 insertions(+), 19 deletions(-) diff --git a/src/lib/edit/experience-dates.test.ts b/src/lib/edit/experience-dates.test.ts index b5ea68a5..500affcc 100644 --- a/src/lib/edit/experience-dates.test.ts +++ b/src/lib/edit/experience-dates.test.ts @@ -15,6 +15,7 @@ import { describe, it, expect } from "vitest"; import { applyNormalizedDateOverrides, applyNormalizedExperienceDates, + formatExperienceDateRange, normalizeExperienceDates, relocatedEndAnchor, } from "./experience-dates.ts"; @@ -211,3 +212,90 @@ describe("relocatedEndAnchor", () => { expect(relocatedEndAnchor({ is_current: true })).toBeUndefined(); }); }); + +/** + * The formatter, which is exported, consumed on BOTH the edit path + * (`ReconstructedRole`) and the EXPORT path (`ats-resume-model.ts`'s + * `experienceDateRange`), and until #817 had no direct test at all. + * + * The `.trim()` calls are the part worth pinning. They are a deliberate + * behaviour change on the export path — the old `experienceDateRange` body + * tested truthiness without trimming, so an entry holding `start_date: " "` + * drew two spaces into the header slot and now draws nothing. That population is + * exactly the one this module is total over: raw entries that never passed + * through `applyOverrides`. Mutation-checked during the #682 review, removing + * both trims left `src/lib/edit/**` and two repro suites entirely green. + */ +describe("formatExperienceDateRange", () => { + it("joins both anchors with a spaced en-dash", () => { + expect(formatExperienceDateRange({ start_date: "2019", end_date: "2022" })).toBe( + "2019 \u2013 2022", + ); + }); + + it("trims each anchor before joining", () => { + // Red without the `.trim()` calls: yields " 2019 \u2013 2022 ". + expect( + formatExperienceDateRange({ start_date: " 2019 ", end_date: " 2022 " }), + ).toBe("2019 \u2013 2022"); + }); + + it("treats a whitespace-only anchor as absent, not as a value", () => { + // The reachable export-path case, and the one the trim exists for. Red + // without the trims: yields " \u2013 2022" and "2019 \u2013 ". + expect(formatExperienceDateRange({ start_date: " ", end_date: "2022" })).toBe( + "2022", + ); + expect(formatExperienceDateRange({ start_date: "2019", end_date: " " })).toBe( + "2019", + ); + expect(formatExperienceDateRange({ start_date: " ", end_date: "\t" })).toBe(""); + }); + + it("trims a whitespace start into the UNANCHORED is_current shape", () => { + // The one row where the two display-only formatters genuinely disagree with + // this one, and the intersection the rest of this block leaves uncovered: + // the trim and the `is_current` substitution are each pinned above, never + // together. `buildProjectDates` and `buildDateRange` read " " as a real + // anchor and draw " –Present"; this module trims it away and draws the + // unanchored "Present". Reachable from a raw export-path entry — exactly the + // population this module is total over (#817 review). + expect( + formatExperienceDateRange({ start_date: " ", is_current: true }), + ).toBe("Present"); + }); + + it("substitutes \"Present\" for the end anchor when is_current", () => { + expect( + formatExperienceDateRange({ start_date: "2019", is_current: true }), + ).toBe("2019 \u2013 Present"); + }); + + it("lets is_current win over a populated end_date", () => { + // `is_current` is read BEFORE `end_date`, so a stale end anchor cannot + // out-rank it — the shape `applyOverrides` produces for an ongoing role. + expect( + formatExperienceDateRange({ + start_date: "2019", + end_date: "2022", + is_current: true, + }), + ).toBe("2019 \u2013 Present"); + }); + + it("draws a lone \"Present\" when is_current has no start anchor", () => { + // Unanchored `is_current` — one of the two raw shapes the docblock says + // reach here from the export path but never from the edit card. + expect(formatExperienceDateRange({ is_current: true })).toBe("Present"); + }); + + it("falls through to whichever single anchor exists", () => { + expect(formatExperienceDateRange({ start_date: "2019" })).toBe("2019"); + expect(formatExperienceDateRange({ end_date: "2022" })).toBe("2022"); + }); + + it("returns the empty string when the entry carries no date at all", () => { + expect(formatExperienceDateRange({})).toBe(""); + expect(formatExperienceDateRange({ is_current: false })).toBe(""); + }); +}); diff --git a/src/lib/edit/experience-dates.ts b/src/lib/edit/experience-dates.ts index f7b1455e..d68040d1 100644 --- a/src/lib/edit/experience-dates.ts +++ b/src/lib/edit/experience-dates.ts @@ -75,16 +75,31 @@ * * WHAT THIS MODULE DOES NOT OWN. "The one rule" above is the rule for the EDIT * and EXPORT paths — the two that can corrupt a stored value. Two display-only - * formatters still hand-roll their own collapse and disagree with this one on the - * unanchored-`is_current` row: `buildDateRange` (`score/group-bullets.ts:328`, - * which feeds `formatExperienceHeader`) and `buildProjectDates` - * (`score/entry-dates.ts:16`) both draw a bare "Present" where - * {@link formatExperienceDateRange} draws "". They also use a tight "–" against - * this module's spaced " – ". Nothing is broken by that today: the shape is - * unreachable from the parser and, since #672, from the edit lane too, and - * neither formatter's output is ever re-parsed. Folding them in needs a separator - * parameter and a decision on that row, which is a change to display strings, not - * to #672's corruption — deliberately out of scope here. (`buildEducationDates` + * formatters still hand-roll their own collapse and disagree with this one: + * `buildDateRange` (`score/group-bullets.ts:366`, which feeds + * `formatExperienceHeader`) and `buildProjectDates` (`score/entry-dates.ts:20`). + * The disagreement is a TRIM, not a collapse, and it shows up on exactly one + * row — a whitespace-only start beside `is_current`, where this module trims the + * start away and draws the unanchored "Present" while both siblings read " " as + * a real anchor and draw " –Present": + * + * {is_current: true} → all three draw "Present" + * {start_date: " ", is_current: true} → "Present" here, " –Present" there + * + * They also use a tight "–" against this module's spaced " – ". An earlier + * revision of this paragraph claimed the siblings draw "Present" where this + * function draws "" — that is wrong for THIS function on any `is_current` row + * (`end` is "Present" before either fallthrough is reached, so "" is + * unreachable) and was only ever true of the composite + * `normalizeExperienceDates` → format that `ReconstructedRole` runs. Corrected + * in #817, which is also where that row first got a test. + * + * Nothing is broken by the divergence today: neither formatter's output is ever + * re-parsed, and the whitespace row that provokes it is reachable only from a + * raw export-path entry — never from the parser, nor, since #672, from the edit + * lane. Folding them in needs a separator parameter and a decision on that row, + * which is a change to display strings, not to #672's corruption — deliberately + * out of scope here. (`buildEducationDates` * in the same file is NOT a candidate: it reads `end_date` first on purpose, * because a lone education date is a graduation date, #97.) * diff --git a/src/lib/heuristics/corpus-edit-roundtrip.test.ts b/src/lib/heuristics/corpus-edit-roundtrip.test.ts index f6798d31..9ad10b27 100644 --- a/src/lib/heuristics/corpus-edit-roundtrip.test.ts +++ b/src/lib/heuristics/corpus-edit-roundtrip.test.ts @@ -365,26 +365,68 @@ function presenceCheck( } /** The ONE re-parsed role the experience edit landed on: the entry carrying our - * synthetic marker. `NEW_TITLE`/`NEW_COMPANY` are literals that appear nowhere - * in any fixture, so at most one entry matches. Returns the whole-experience - * JSON when no entry carries the marker — the presence half is already failing - * in that case and must not be masked by a silently-skipped absence half. */ -function editedRoleText(experience: readonly unknown[], fallback: string): string { - const hit = experience.find( + * synthetic marker, or `undefined` when none does. `NEW_TITLE`/`NEW_COMPANY` + * are literals that appear nowhere in any fixture, so at most one entry + * matches. Sole owner of the marker lookup — both slicers below derive from it + * so their fallbacks cannot drift apart independently (#817 review). */ +function markedRole(experience: readonly unknown[]): unknown | undefined { + return experience.find( (e) => j(e).includes(NEW_TITLE) || j(e).includes(NEW_COMPANY), ); +} + +/** The marked role as searchable text. Falls back to the whole-experience JSON + * when no entry carries the marker — the presence half is already failing in + * that case and must not be masked by a silently-skipped absence half. */ +function editedRoleText(experience: readonly unknown[], fallback: string): string { + const hit = markedRole(experience); return hit === undefined ? fallback : j(hit); } -/** One string field off a re-parsed experience entry, or `undefined` when the - * key is absent or non-string. */ type EditedRoleField = "title" | "company" | "start_date" | "end_date"; +/** One string field off a re-parsed experience entry, or `undefined` when the + * key is absent or non-string. */ function stringField(entry: unknown, key: EditedRoleField): string | undefined { const v = (entry as Record | null)?.[key]; return typeof v === "string" ? v : undefined; } +const DATE_KEYS: ReadonlySet = new Set(["start_date", "end_date"]); + +/** The DATE SLOTS of the ONE re-parsed role the experience edit landed on, as + * searchable text — `{"start_date":…,"end_date":…}` and nothing else. + * + * Why the absence half needs its own, narrower slice for a date key: `replaced` + * there is the fixture's OWN prior `start_date`, which is frequently a bare + * `"2019"`. The wide slice is `j(hit)` — the whole edited role, `description` + * included — so a fixture whose role-0 prose merely mentions that year ("shipped + * the platform in 2019", "since 2019") fails the gate for a reason that has + * nothing to do with the slot under test. The title/company keys are safe on the + * wide slice by luck, not by design: `NEW_TITLE`/`NEW_COMPANY` and the originals + * they replace are long, distinctive strings. A four-digit year is not. + * + * The narrowing forfeits nothing even in principle (#817 review): a replaced + * `start_date` can only reach `title`/`company` if `applyExperienceHeaderOverrides` + * writes a date into a header field, which is a different defect class with its + * own gate, and the one way the collapse CAN misplace the value — leaving it in + * `end_date` — is inside this slice, and checked again from the other side by + * the `end_date survived the lone-end-date collapse` assertion below. + * + * The no-marker fallback is every role's date slots. That is DELIBERATELY not + * the same "wide" as {@link editedRoleText}'s whole-experience JSON — wider in + * role count, narrower in field count — so the two can drift independently. + * Safe either way: the branch is only reached once the presence half has already + * failed, so the worst case is a second failure line, never a false green. */ +function editedRoleDateText(experience: readonly unknown[]): string { + const hit = markedRole(experience); + const slots = (e: unknown) => ({ + start_date: stringField(e, "start_date"), + end_date: stringField(e, "end_date"), + }); + return hit === undefined ? j(experience.map(slots)) : j(slots(hit)); +} + /** * The experience half of the gate, FIELD-AWARE — the presence claim is "the * `title` field of some role equals `NEW_TITLE`", not "`NEW_TITLE` occurs @@ -406,7 +448,12 @@ function experienceFieldCheck( ): void { if (!experience.some((e) => stringField(e, key) === present)) into.push(`${key} "${present}" missing on re-parse`); - if (replaced && absenceText.includes(replaced)) + if (!replaced) return; + // A date key searches its own slots; everything else searches the caller's + // slice. See {@link editedRoleDateText} for why the wide slice cannot be used + // for a value that may be a bare year. + const haystack = DATE_KEYS.has(key) ? editedRoleDateText(experience) : absenceText; + if (haystack.includes(replaced)) into.push(`replaced ${key} still present on re-parse`); } @@ -466,6 +513,9 @@ function computeEditFailures( // • this shape (START cleared, END supplied) → 45 of 60 FAIL // • the earlier shape (a new `start_date`) → 60/60 GREEN // The presence of a date field was never the thing under test; the SLOT is. + // `roleText` is INERT here: a date key routes its absence half through + // `editedRoleDateText` instead (#817). Passed anyway so the call reads + // uniformly with the title/company ones above, which do use it. experienceFieldCheck( f3.experience, "start_date", NEW_END_DATE, edits.replaced.start_date, roleText, fails.experience, @@ -549,6 +599,61 @@ async function editRoundtrip( } } +/** + * The absence half's scoping rule, asserted directly (#817). + * + * These run at module scope over synthetic roles rather than over the corpus, + * because the defect they pin is one NO fixture currently exhibits: it needs a + * role whose prose happens to repeat its own start year. The gate is green over + * the present fixtures and would go red on the next one added — which is exactly + * the class of failure a corpus gate cannot self-test. + */ +describe("experienceFieldCheck absence scoping (#817)", () => { + const role = (over: Record) => ({ + title: NEW_TITLE, + company: "Acme Corporation", + start_date: "2020", + end_date: "2022", + ...over, + }); + + // The `j(experience[0])` argument in the two date cases below is deliberately + // inert — a date key ignores `absenceText` — and is passed precisely to show + // that a wide slice containing the replaced value no longer fires the check. + it("does not fire when the replaced year merely recurs in role prose", () => { + const fails: string[] = []; + const experience = [ + role({ description: "Shipped the billing platform in 2019 and scaled it." }), + ]; + experienceFieldCheck( + experience, "start_date", "2020", "2019", j(experience[0]), fails, + ); + expect(fails).toEqual([]); + }); + + it("still fires when the replaced year survives in the date slot itself", () => { + const fails: string[] = []; + const experience = [role({ start_date: "2019" })]; + experienceFieldCheck( + experience, "start_date", "2020", "2019", j(experience[0]), fails, + ); + expect(fails).toContain("replaced start_date still present on re-parse"); + }); + + it("keeps searching the caller's wide slice for a non-date key", () => { + // The narrowing is date-only: a replaced TITLE that leaked into another + // field of the same role must still be caught. + const fails: string[] = []; + const experience = [ + role({ description: "Formerly Office manager, same team." }), + ]; + experienceFieldCheck( + experience, "title", NEW_TITLE, "Office manager", j(experience[0]), fails, + ); + expect(fails).toContain("replaced title still present on re-parse"); + }); +}); + describe("corpus edit-leg round-trip (#459)", { timeout: 20000 }, () => { const fixtures = walkPdfs(FIXTURE_ROOT);