From 1d70b89d541aa2fcff5c811192412d2798c49834 Mon Sep 17 00:00:00 2001 From: Srinivas Annam Date: Wed, 19 Aug 2026 08:33:37 -0700 Subject: [PATCH 1/2] fix(rewrite): reject/revert rewrites that drop or invent numbers (#778) --- src/components/features/ResumeRewrite.test.ts | 93 ++++ src/components/features/ResumeRewrite.tsx | 18 +- .../features/ResumeRewriteProposed.test.tsx | 158 +++++++ .../features/ResumeRewriteProposed.tsx | 56 ++- src/components/features/RewriteReviewList.tsx | 65 +-- .../features/SectionRewrite.test.ts | 71 +++ src/components/features/SectionRewrite.tsx | 111 ++++- src/design-system/shared/InlineDiff.test.tsx | 87 ++++ src/design-system/shared/InlineDiff.tsx | 40 +- src/lib/analytics.ts | 23 + src/lib/webllm/eval/README.md | 14 + src/lib/webllm/eval/report.test.ts | 30 ++ src/lib/webllm/eval/report.ts | 27 +- src/lib/webllm/eval/rubric.ts | 2 +- src/lib/webllm/eval/run-eval-browser.ts | 28 +- src/lib/webllm/eval/runner.test.ts | 117 +++++ src/lib/webllm/eval/runner.ts | 22 +- src/lib/webllm/eval/types.ts | 59 ++- src/lib/webllm/post-process.test.ts | 127 ++++- src/lib/webllm/post-process.ts | 98 +++- src/lib/webllm/preserve-numbers.test.ts | 445 +++++++++++++++++- src/lib/webllm/preserve-numbers.ts | 398 ++++++++++++---- src/lib/webllm/rewrite-guardrails.ts | 2 +- src/lib/webllm/rewrite-resume.test.ts | 90 +++- src/lib/webllm/rewrite-resume.ts | 36 +- src/lib/webllm/rewrite-section.test.ts | 95 +++- src/lib/webllm/rewrite-section.ts | 44 +- src/lib/webllm/rewrite-summary.test.ts | 27 +- src/lib/webllm/rewrite-summary.ts | 48 +- tests/fixtures/rewrite/reports/README.md | 68 +++ 30 files changed, 2277 insertions(+), 222 deletions(-) create mode 100644 src/design-system/shared/InlineDiff.test.tsx diff --git a/src/components/features/ResumeRewrite.test.ts b/src/components/features/ResumeRewrite.test.ts index 13d29390..f5e5ee25 100644 --- a/src/components/features/ResumeRewrite.test.ts +++ b/src/components/features/ResumeRewrite.test.ts @@ -45,6 +45,7 @@ const okResult: ResumeRewriteResult = { data: { text: "Senior engineer.", numbersPreserved: true, + reverted: false, droppedNumbers: [], addedNumbers: [], }, @@ -55,6 +56,7 @@ const okResult: ResumeRewriteResult = { data: { bullets: ["Shipped Foo to 10M users."], numbersPreserved: true, + reverted: false, droppedNumbers: [], addedNumbers: [], }, @@ -71,6 +73,7 @@ const driftResult: ResumeRewriteResult = { data: { bullets: ["Saved money."], numbersPreserved: false, + reverted: false, droppedNumbers: ["$5K"], addedNumbers: [], }, @@ -81,6 +84,7 @@ const driftResult: ResumeRewriteResult = { data: { text: "Senior engineer with 99.9% availability.", numbersPreserved: false, + reverted: false, droppedNumbers: [], addedNumbers: ["99.9%"], }, @@ -330,3 +334,92 @@ describe("ResumeRewritePanel", () => { expect(html).toContain("99.9%"); }); }); + +// ── #778: reverted sections must not read as clean passes ─────────────────── + +describe("reverted sections (#778)", () => { + /** One reverted experience section + one clean one. */ + const revertedResult: ResumeRewriteResult = { + allNumbersPreserved: true, + sections: [ + { + kind: "experience", + input: { + kind: "experience", + id: "experience:0", + label: "Engineer — Acme", + bullets: ["Grew ARR to $4.2M in FY24."], + }, + data: { + bullets: ["Grew ARR to $4.2M in FY24."], + numbersPreserved: true, + reverted: true, + droppedNumbers: ["$4.2M"], + addedNumbers: [], + }, + }, + { + kind: "experience", + input: { + kind: "experience", + id: "experience:1", + label: "Engineer — Beta", + bullets: ["Owned the write path."], + }, + data: { + bullets: ["Owned and hardened the write path."], + numbersPreserved: true, + reverted: false, + droppedNumbers: [], + addedNumbers: [], + }, + }, + ], + }; + + it("badges a reverted section in the in-flight list instead of showing a clean tick", () => { + // `numbersPreserved` is true on a reverted section by construction, so + // without an explicit branch the user would never learn their section + // went unrewritten. + const status: ResumeRewriteStatus = { + kind: "running", + progress: { + currentIndex: 2, + totalSections: 2, + currentLabel: null, + completed: revertedResult.sections, + }, + }; + const html = renderToStaticMarkup( + createElement(ResumeRewritePanel, { + status, + onDismiss: () => {}, + onApplied: () => {}, + onUndo: () => {}, + }), + ); + expect(html).toContain("kept original"); + // The clean section must not pick up the badge. + expect(html.match(/kept original/g)).toHaveLength(1); + expect(html).not.toContain("metric drift"); + }); + + it("captions the reverted section's empty diff in the proposed view", () => { + const status: ResumeRewriteStatus = { + kind: "proposed", + result: revertedResult, + snapshot: [], + }; + const html = renderToStaticMarkup( + createElement(ResumeRewritePanel, { + status, + onDismiss: () => {}, + onApplied: () => {}, + onUndo: () => {}, + }), + ); + expect(html).toContain("Kept unchanged"); + expect(html).toContain("$4.2M"); + }); +}); + diff --git a/src/components/features/ResumeRewrite.tsx b/src/components/features/ResumeRewrite.tsx index 6682e69b..59bf8131 100644 --- a/src/components/features/ResumeRewrite.tsx +++ b/src/components/features/ResumeRewrite.tsx @@ -425,13 +425,25 @@ function CompletedList({ ✓ {outcome.input.label} - {!outcome.data.numbersPreserved && ( + {/* #778: a reverted section preserves every number by construction, + so `numbersPreserved` alone would render it as a clean pass and + the user would never learn their section went unrewritten. */} + {outcome.data.reverted ? ( - metric drift + kept original + ) : ( + !outcome.data.numbersPreserved && ( + + metric drift + + ) )} ))} diff --git a/src/components/features/ResumeRewriteProposed.test.tsx b/src/components/features/ResumeRewriteProposed.test.tsx index 6244d83e..b17dedd9 100644 --- a/src/components/features/ResumeRewriteProposed.test.tsx +++ b/src/components/features/ResumeRewriteProposed.test.tsx @@ -43,6 +43,7 @@ const RESULT: ResumeRewriteResult = { data: { bullets: ["Led a team of 5 engineers", "Mentored two interns"], numbersPreserved: true, + reverted: false, droppedNumbers: [], addedNumbers: [], }, @@ -274,6 +275,7 @@ function summaryResult(proposed: string): ResumeRewriteResult { data: { text: proposed, numbersPreserved: true, + reverted: false, droppedNumbers: [], addedNumbers: [], }, @@ -392,3 +394,159 @@ describe("ProposedPanel — summary review + apply (issue 625)", () => { ).toHaveLength(0); }); }); + +describe("ProposedPanel — rejected rewrites (#778)", () => { + const REVERTED: ResumeRewriteResult = { + allNumbersPreserved: true, + sections: [ + { + kind: "experience", + input: { + kind: "experience", + id: "experience:0", + label: "Senior Engineer — Acme", + bullets: ["Grew ARR to $4.2M in FY24."], + }, + data: { + bullets: ["Grew ARR to $4.2M in FY24."], + numbersPreserved: true, + reverted: true, + droppedNumbers: ["$4.2M"], + addedNumbers: [], + }, + }, + ], + }; + + it("falls through to the read-only redline instead of offering accept rows", () => { + // The gate returned the input verbatim, so a review row would ask the user + // to "accept" their own bullet. + const handlers: SectionRewriteApply = { + obsIds: ["0|a"], + onReplace: vi.fn(), + onRemove: vi.fn(), + onAdd: vi.fn(), + }; + const el = render( + createElement(ProposedPanel, { + result: REVERTED, + onDismiss: vi.fn(), + onApplied: vi.fn(), + applyBySection: new Map([["experience:0", handlers]]), + }), + ); + const acceptButtons = [...el.querySelectorAll("button")].filter((b) => + b.getAttribute("aria-label")?.startsWith("Accept this"), + ); + expect(acceptButtons.length).toBe(0); + expect(el.textContent).toContain("Kept unchanged"); + expect(el.textContent).toContain("$4.2M"); + }); + + it("still shows the revert caption when the input carried a blank bullet", () => { + // The revert hands the input back verbatim, blanks included, while the + // original side has always filtered them. Filtering only one side made the + // two strings differ, so `InlineDiff` saw a change and suppressed the + // caption — leaving a revert looking like a silent no-op, the exact failure + // the caption exists to prevent. + const withBlank: ResumeRewriteResult = { + allNumbersPreserved: true, + sections: [ + { + kind: "experience", + input: { + kind: "experience", + id: "experience:0", + label: "Senior Engineer — Acme", + bullets: ["Grew ARR to $4.2M in FY24.", " "], + }, + data: { + bullets: ["Grew ARR to $4.2M in FY24.", " "], + numbersPreserved: true, + reverted: true, + droppedNumbers: ["$4.2M"], + addedNumbers: [], + }, + }, + ], + }; + const el = render( + createElement(ProposedPanel, { + result: withBlank, + onDismiss: vi.fn(), + onApplied: vi.fn(), + }), + ); + expect(el.textContent).toContain("Kept unchanged"); + }); + + it("does not dress a fully-reverted run as a clean success", () => { + // `allNumbersPreserved` is true here by construction; the tone has to come + // from `reverted` or the panel reads green on a run that changed nothing. + const el = render( + createElement(ProposedPanel, { + result: REVERTED, + onDismiss: vi.fn(), + onApplied: vi.fn(), + }), + ); + expect(el.innerHTML).toContain("border-feedback-warning-border"); + expect(el.innerHTML).not.toContain("border-feedback-success-border"); + }); + + it("explains the revert in the résumé-level alert, not just in the border colour", () => { + // The warning used to be guarded on `!allNumbersPreserved` alone, which is + // true-by-construction false here: a revert delivers the ORIGINAL bullets, + // and those preserve their own numbers. So the panel turned warning-toned + // and said nothing — no text, no `role="alert"` for a screen reader. Pinned + // by content, because a border class is not an explanation. + const el = render( + createElement(ProposedPanel, { + result: REVERTED, + onDismiss: vi.fn(), + onApplied: vi.fn(), + }), + ); + const alert = el.querySelector('[role="alert"]'); + expect(alert).not.toBeNull(); + // The reverted copy, not the "AI altered a metric — review before saving" + // variant: the delivered bullets are the user's own. + expect(alert!.textContent).toContain("Kept your original"); + expect(alert!.textContent).toContain("removed $4.2M"); + expect(alert!.textContent).not.toContain("Review before saving"); + }); + + it("names the invented figure when a section was reverted for inventing one", () => { + const invented: ResumeRewriteResult = { + allNumbersPreserved: true, + sections: [ + { + kind: "experience", + input: { + kind: "experience", + id: "experience:0", + label: "Senior Engineer — Acme", + bullets: ["Completed phase 5 of the migration."], + }, + data: { + bullets: ["Completed phase 5 of the migration."], + numbersPreserved: true, + reverted: true, + droppedNumbers: [], + addedNumbers: ["5"], + }, + }, + ], + }; + const el = render( + createElement(ProposedPanel, { + result: invented, + onDismiss: vi.fn(), + onApplied: vi.fn(), + }), + ); + const alert = el.querySelector('[role="alert"]'); + expect(alert?.textContent).toContain("invented 5"); + }); +}); + diff --git a/src/components/features/ResumeRewriteProposed.tsx b/src/components/features/ResumeRewriteProposed.tsx index 1445e6b2..a879ea8e 100644 --- a/src/components/features/ResumeRewriteProposed.tsx +++ b/src/components/features/ResumeRewriteProposed.tsx @@ -36,6 +36,7 @@ import { import { resolveSectionWrites } from "../../lib/rewrite-review/apply-accepted.ts"; import { useRewriteReview, type RewriteReview } from "../../hooks/useRewriteReview.ts"; import { + describeNumberDrift, NumberPreservationWarning, type SectionRewriteApply, } from "./SectionRewrite.tsx"; @@ -106,6 +107,10 @@ export function ProposedPanel({ for (const outcome of result.sections) { const apply = applyBySection?.get(outcome.input.id); if (!apply) continue; + // A rejected rewrite (#778) returns the input verbatim, so aligning it + // would offer the user an "accept" on their own text. Fall through to + // the read-only redline, which captions why there is nothing to see. + if (outcome.data.reverted) continue; const pairs = outcome.kind === "summary" ? summaryPairs( @@ -202,15 +207,24 @@ export function ProposedPanel({ const accepted = review.acceptedCount; + // #778: `allNumbersPreserved` is true when every section reverted — the + // originals preserve themselves — so it cannot carry the tone on its own, and + // it cannot decide whether the warning renders either. Guarding the alert on + // it alone turned the panel warning-coloured on a revert while saying nothing: + // a border colour, no text, no `role="alert"`. Same condition as + // `ProposedSection`'s in SectionRewrite.tsx, one level up. + const anyReverted = result.sections.some((o) => o.data.reverted); + return ( - {!result.allNumbersPreserved && ( + {(anyReverted || !result.allNumbersPreserved) && ( )}
    @@ -345,13 +359,20 @@ function SectionResult({ outcome }: { outcome: SectionOutcome }) { outcome.input.text, outcome.data.text || "", )} + noChangeLabel={ + outcome.data.reverted ? revertedLabel(outcome.data) : undefined + } /> ); } - const originalBullets = outcome.input.bullets.filter( - (b) => b.trim().length > 0, - ); + // Both sides get the SAME blank filter. A revert hands back the input verbatim + // (blanks included) while this side has always dropped them, so filtering only + // the original made a section with one blank bullet diff as "changed" — which + // suppressed the `noChangeLabel` that exists to stop a revert reading as a + // silent no-op (#778). + const originalBullets = withoutBlanks(outcome.input.bullets); + const proposedBullets = withoutBlanks(outcome.data.bullets); return (

    @@ -360,13 +381,36 @@ function SectionResult({ outcome }: { outcome: SectionOutcome }) { `• ${b}`).join("\n"), - outcome.data.bullets.map((b) => `• ${b}`).join("\n"), + proposedBullets.map((b) => `• ${b}`).join("\n"), )} + noChangeLabel={ + outcome.data.reverted ? revertedLabel(outcome.data) : undefined + } />

    ); } +function withoutBlanks(bullets: readonly string[]): string[] { + return bullets.filter((b) => b.trim().length > 0); +} + +/** + * Why a section in the whole-résumé review shows no redline (#778). Named per + * section rather than aggregated into the résumé-level warning because the + * chain rewrites each section independently — one reverting says nothing about + * the others, and a single banner would leave the reader guessing which. + */ +function revertedLabel(data: { + droppedNumbers: readonly string[]; + addedNumbers: readonly string[]; +}): string { + const detail = describeNumberDrift(data.droppedNumbers, data.addedNumbers); + return detail === "" + ? "Kept unchanged — the rewrite was rejected." + : `Kept unchanged — the rewrite ${detail}.`; +} + export interface AggregateDrift { dropped: string[]; added: string[]; diff --git a/src/components/features/RewriteReviewList.tsx b/src/components/features/RewriteReviewList.tsx index 0619c5a4..4ac12624 100644 --- a/src/components/features/RewriteReviewList.tsx +++ b/src/components/features/RewriteReviewList.tsx @@ -236,35 +236,44 @@ export function RewriteReviewList({
    {warning} -
    - - {total} change{total === 1 ? "" : "s"} proposed — review each below. - -
    - - -
    -
    + {/* Nothing to review renders no review chrome. Reachable since #778, + where a rejected rewrite deliberately yields zero pairs — "0 changes + proposed — review each below" over an empty list would be the + surface contradicting itself, with the `warning` above already + carrying the real outcome. */} + {total > 0 && ( + <> +
    + + {total} change{total === 1 ? "" : "s"} proposed — review each below. + +
    + + +
    +
    -
      - {pairs.map((pair) => ( - - ))} -
    +
      + {pairs.map((pair) => ( + + ))} +
    + + )}