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);