Skip to content

Commit 92b73a2

Browse files
authored
feat(export): pre-download validation pass for glyph loss and bullet-page splits (#621) (#872)
Adds a pre-download validation pass that surfaces two known render defects instead of failing silently: characters the export font has no glyph for, and bullets a page break falls inside. Glyph-loss findings name the character as authored (not the cased probe the renderer's heading uppercase transform produces), so the user can search their résumé for the character actually reported. Closes #621
1 parent 9a8ee2f commit 92b73a2

31 files changed

Lines changed: 1147 additions & 108 deletions

src/components/features/ExportDialog.test.tsx

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { createRoot, type Root } from "react-dom/client";
2525
import type { CascadeResult } from "../../lib/heuristics/types.ts";
2626
import type { AnonymousAtsScore } from "../../lib/score/score.ts";
2727
import type { ContactOverrides } from "../../hooks/useEditableParse.ts";
28+
import type { RenderFinding } from "../../lib/pdf/render-findings.ts";
2829

2930
(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT =
3031
true;
@@ -33,6 +34,9 @@ const pdfDownload = vi.fn();
3334
const markdownDownload = vi.fn();
3435
const reportDownload = vi.fn(() => Promise.resolve(true));
3536
let pdfError: string | null = null;
37+
/** #621 export findings the PDF row surfaces — empty for a clean export, which
38+
* is what every case here renders unless it says otherwise. */
39+
let pdfFindings: RenderFinding[] = [];
3640

3741
/** What each hook was handed as its journey Download-stage mark site (#826).
3842
* The success point lives inside the hooks, so what this component owns — and
@@ -46,7 +50,12 @@ vi.mock("../../hooks/useDownloadPdf.ts", () => ({
4650
onDownloaded?: () => void,
4751
) => {
4852
captured.pdf = onDownloaded;
49-
return { download: pdfDownload, isGenerating: false, error: pdfError };
53+
return {
54+
download: pdfDownload,
55+
isGenerating: false,
56+
error: pdfError,
57+
findings: pdfFindings,
58+
};
5059
},
5160
}));
5261
vi.mock("../../hooks/useDownloadMarkdown.ts", () => ({
@@ -154,6 +163,7 @@ beforeEach(() => {
154163
markdownDownload.mockClear();
155164
reportDownload.mockClear();
156165
pdfError = null;
166+
pdfFindings = [];
157167
// jsdom does not implement modal dialogs in every version, and the primitive
158168
// calls `showModal()` from an effect. Stubbed to a plain open so the tests
159169
// exercise the dialog's CONTENT rather than the UA's modality.
@@ -421,4 +431,46 @@ describe("ExportDialog", () => {
421431
}
422432
expect(onExported).toHaveBeenCalledTimes(3);
423433
});
434+
435+
// #621 — the export reports what it could not render cleanly, on the row that
436+
// produced the file. Advisory: the user already has the PDF.
437+
describe("export findings", () => {
438+
it("renders NO warning chrome when the export was clean", () => {
439+
// The common case. A permanent "0 issues" strip on the download row would
440+
// train every user to stop reading it.
441+
const el = render(exportable());
442+
expect(text(el)).not.toContain("Check the export");
443+
expect(el.querySelector("[aria-live]:not([aria-live=\"off\"]) ul")).toBeNull();
444+
});
445+
446+
it("names the field and states what happened, never colour alone", () => {
447+
pdfFindings = [
448+
{
449+
kind: "glyph-degraded",
450+
severity: "warning",
451+
sourceField: "Experience \u2192 Staff Engineer \u00b7 Acme \u2192 bullet 3",
452+
detail: 'The export font has no glyph for "\u2605", so it was drawn as "?".',
453+
},
454+
];
455+
const el = render(exportable());
456+
// The badge carries a WORD, not just a tone.
457+
expect(text(el)).toContain("Check the export");
458+
expect(text(el)).toContain("Experience \u2192 Staff Engineer \u00b7 Acme \u2192 bullet 3");
459+
expect(text(el)).toContain("\u2605");
460+
// And it says the file arrived — a finding is not a refusal.
461+
expect(text(el)).toContain("Your PDF downloaded");
462+
});
463+
464+
it("counts the overflow instead of listing forty rows", () => {
465+
pdfFindings = Array.from({ length: 8 }, (_, i) => ({
466+
kind: "glyph-degraded" as const,
467+
severity: "info" as const,
468+
sourceField: `Experience \u2192 Role ${i + 1}`,
469+
detail: 'The export font has no glyph for "\u2192", so it was drawn as "->".',
470+
}));
471+
const el = render(exportable());
472+
expect(el.querySelectorAll("li")).toHaveLength(5);
473+
expect(text(el)).toContain("and 3 more");
474+
});
475+
});
424476
});

src/components/features/ExportDialog.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import {
7373
} from "../../lib/contact.ts";
7474
import { ExportGateBody, fixFirstGap } from "./ExportGateBody.tsx";
7575
import { ExportRow, ExportReportRow } from "./ExportRows.tsx";
76+
import { ExportFindings } from "./ExportFindings.tsx";
7677
import { useDownloadPdf } from "../../hooks/useDownloadPdf.ts";
7778
import { useDownloadMarkdown } from "../../hooks/useDownloadMarkdown.ts";
7879
import { useDownloadReport } from "../../hooks/useDownloadReport.ts";
@@ -198,6 +199,11 @@ export function ExportDialog({
198199
>
199200
{pdf.isGenerating ? "Generating…" : "Download PDF"}
200201
</Button>
202+
{/* What the export could not draw cleanly (#621) — advisory, and
203+
renders NOTHING for the clean résumé that is the common case.
204+
It sits on the row that produced the file, beside the row's own
205+
`ErrorState`, because that is the surface already mounted. */}
206+
<ExportFindings findings={pdf.findings} />
201207
</ExportRow>
202208

203209
<ExportRow
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// Copyright 2026 The offlinecv Authors
3+
4+
/**
5+
* ExportFindings — what the exporter could not render cleanly, shown on the row
6+
* that produced the file (#621).
7+
*
8+
* Reuse analysis (CLAUDE.md Golden Rule). This is NOT a new banner primitive.
9+
* The design-system barrel deliberately ships no Toast/Snackbar, and its house
10+
* pattern for "an action finished and has something to say" is to confirm IN the
11+
* surface already mounted rather than open a new one — so this swaps content
12+
* into `ExportRow`, beside the `ErrorState` that already reports a failed
13+
* export, and carries its status on the shared `StatusBadge`. Nothing new is
14+
* added to the design system.
15+
*
16+
* Three rules it exists to keep:
17+
*
18+
* 1. **Zero findings renders NOTHING.** Not an empty "0 issues" panel, not a
19+
* green all-clear. Most résumés are clean, and a permanent status strip on
20+
* the download row would teach every user to stop reading it.
21+
* 2. **Never colour alone.** The badge carries the WORD, the sentence states
22+
* the count, and every finding names its own field — the tone is
23+
* reinforcement.
24+
* 3. **Advisory, never a blocker.** The user already has their PDF by the time
25+
* this renders; the copy says so. Refusing a download is `useDownloadPdf`'s
26+
* #664 font gate and stays there.
27+
*
28+
* `aria-live="polite"` announces the swap. The parent format list is itself a
29+
* polite live region, so the announcement would happen regardless; declaring it
30+
* here keeps the guarantee attached to the component that needs it rather than
31+
* to a container that could be restructured.
32+
*/
33+
34+
import { StatusBadge } from "@design-system";
35+
import type { RenderFinding } from "../../lib/pdf/render-findings.ts";
36+
37+
/**
38+
* How many findings are listed before the rest are counted. A résumé pasted from
39+
* a source full of arrows can produce one finding per bullet; forty rows in a
40+
* dialog is a wall, and the first few already say what kind of thing is wrong
41+
* and where to start.
42+
*/
43+
const MAX_LISTED = 5;
44+
45+
export function ExportFindings({
46+
findings,
47+
}: {
48+
findings: readonly RenderFinding[];
49+
}) {
50+
if (findings.length === 0) return null;
51+
const listed = findings.slice(0, MAX_LISTED);
52+
const rest = findings.length - listed.length;
53+
// `warning` when anything was actually destroyed; `info` when every finding is
54+
// a substitution that still reads correctly (an arrow drawn as "->").
55+
const tone = findings.some((f) => f.severity === "warning") ? "warning" : "info";
56+
57+
return (
58+
<div aria-live="polite" className="flex flex-col gap-2">
59+
<div className="flex flex-wrap items-center gap-2">
60+
<StatusBadge tone={tone}>Check the export</StatusBadge>
61+
<p className="text-sm text-content-secondary">
62+
Your PDF downloaded, but{" "}
63+
{findings.length === 1
64+
? "one thing"
65+
: `${findings.length} things`}{" "}
66+
did not come out as written.
67+
</p>
68+
</div>
69+
<ul className="flex flex-col gap-1">
70+
{listed.map((finding, idx) => (
71+
<li
72+
key={`${finding.kind}:${finding.sourceField}:${finding.detail}:${idx}`}
73+
className="text-sm text-content-tertiary"
74+
>
75+
<span className="font-medium text-content-secondary">
76+
{finding.sourceField}
77+
</span>{" "}
78+
{finding.detail}
79+
</li>
80+
))}
81+
</ul>
82+
{rest > 0 && (
83+
<p className="text-sm text-content-tertiary">
84+
…and {rest} more like {rest === 1 ? "this" : "these"}.
85+
</p>
86+
)}
87+
</div>
88+
);
89+
}

src/hooks/useDownloadPdf.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@
1717
* résumé bytes leave the browser, which is the actual guarantee. Say custody,
1818
* not runtime.
1919
*
20+
* It also carries the export's own findings (#621) — what the renderer could
21+
* not draw cleanly — back to the surface. Those are ADVISORY and arrive only
22+
* after the bytes have reached the user: the refusal below is the one thing that
23+
* stops a download, and reporting must never grow into a second one.
24+
*
2025
* This hook owns the refusal for #664. When the Poppins fetch fails, the
2126
* renderer falls back to Helvetica, whose WinAnsi codec replaces anything
2227
* outside it with `?` — including a candidate's own name. Rather than hand back
@@ -37,6 +42,7 @@ import {
3742
renderAtsResumePdf,
3843
type ExportGlyphLoss,
3944
} from "../lib/pdf/render-ats-pdf.ts";
45+
import type { RenderFinding } from "../lib/pdf/render-findings.ts";
4046
import { slugifyName, triggerBlobDownload } from "../lib/download/blob-download.ts";
4147
import { trackDownloadCompleted, type DownloadSource } from "../lib/analytics.ts";
4248
import { clearBlankDraft } from "./useResumeAnalysis.ts";
@@ -45,6 +51,13 @@ export interface UseDownloadPdf {
4551
download: () => Promise<void>;
4652
isGenerating: boolean;
4753
error: string | null;
54+
/**
55+
* What the LAST completed render could not draw cleanly (#621) — empty until a
56+
* download has run, and empty again the moment the next one starts, so the
57+
* surface can never show findings that belong to a résumé the user has since
58+
* edited. Advisory only: the download already happened.
59+
*/
60+
findings: RenderFinding[];
4861
}
4962

5063
/** Turn a candidate name into a safe, lower-kebab PDF filename. */
@@ -87,10 +100,14 @@ export function useDownloadPdf(
87100
): UseDownloadPdf {
88101
const [isGenerating, setIsGenerating] = useState(false);
89102
const [error, setError] = useState<string | null>(null);
103+
const [findings, setFindings] = useState<RenderFinding[]>([]);
90104

91105
const download = useCallback(async () => {
92106
setIsGenerating(true);
93107
setError(null);
108+
// Cleared up front, alongside `error`, for the same reason: a stale report
109+
// about the PREVIOUS export would read as a verdict on this one.
110+
setFindings([]);
94111
try {
95112
const model = buildAtsResumeModel(result, score);
96113

@@ -105,14 +122,17 @@ export function useDownloadPdf(
105122
return;
106123
}
107124

108-
const bytes = await renderAtsResumePdf(model);
125+
const { bytes, findings: rendered } = await renderAtsResumePdf(model);
109126
// `bytes.slice()` copies into a fresh ArrayBuffer-backed view so Blob gets
110127
// a clean buffer.
111128
triggerBlobDownload(
112129
bytes.slice(),
113130
"application/pdf",
114131
filenameFromName(model.contact.name),
115132
);
133+
// AFTER the download, never before: a finding is a report on the file the
134+
// user now has, not a gate in front of it (#621).
135+
setFindings(rendered);
116136

117137
// Distinguish a from-scratch authored download from an uploaded one
118138
// (#313). `tiers` is empty ONLY for `buildBlankResult()`'s output —
@@ -137,5 +157,5 @@ export function useDownloadPdf(
137157
// is keyed to the résumé that was on screen then.
138158
}, [result, score, onDownloaded]);
139159

140-
return { download, isGenerating, error };
160+
return { download, isGenerating, error, findings };
141161
}

src/lib/edit/description-override-roundtrip.repro.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ describe("descriptionOverrides edit-leg round-trip (#489)", { timeout: 20000 },
114114
display,
115115
scoreEditedResume(applied, p1.triggers, []),
116116
);
117-
const p3 = await runCascade(await renderAtsResumePdf(model));
117+
const p3 = await runCascade((await renderAtsResumePdf(model)).bytes);
118118

119119
const p3Text = JSON.stringify(p3.canonical.fields);
120120
expect(p3Text.includes(NEW_DESCRIPTION)).toBe(true);

src/lib/heuristics/corpus-edit-roundtrip.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ async function editRoundtrip(
543543
display,
544544
scoreEditedResume(applied, p1.triggers, Object.keys(edits.bullets)),
545545
);
546-
return { p3: await runCascade(await renderAtsResumePdf(model)) };
546+
return { p3: await runCascade((await renderAtsResumePdf(model)).bytes) };
547547
} catch (err) {
548548
return { renderError: `export/re-parse threw: ${(err as Error).message}` };
549549
}

src/lib/heuristics/multi-experience-roundtrip.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ describe("#311 multiple experience sections — parse + round-trip", { timeout:
9797
const bytes = await fsp.readFile(FIXTURE);
9898
const parse1 = await runCascade(new Uint8Array(bytes));
9999
const model = buildAtsResumeModel(parse1, scoreOf(parse1));
100-
const exportedBytes = await renderAtsResumePdf(model);
100+
const { bytes: exportedBytes } = await renderAtsResumePdf(model);
101101
const parse3 = await runCascade(new Uint8Array(exportedBytes));
102102

103103
// Two distinct experience-category groups on the way IN. (The export

src/lib/heuristics/roundtrip-hop.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export async function runRoundtripHop(
7878
layer = HOP_LAYERS[1];
7979
const model = buildAtsResumeModel(before, score);
8080
layer = HOP_LAYERS[2];
81-
const bytes = await renderAtsResumePdf(model);
81+
const { bytes } = await renderAtsResumePdf(model);
8282
layer = HOP_LAYERS[3];
8383
return { after: await runCascade(bytes) };
8484
} catch (err) {

src/lib/pdf/export-layout-contract.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ async function drawnLines(
185185
build: ModelBuilder,
186186
filler: number,
187187
): Promise<PdfDrawnLine[]> {
188-
return extractPdfDrawnLines(await renderAtsResumePdf(build(filler)));
188+
return extractPdfDrawnLines((await renderAtsResumePdf(build(filler))).bytes);
189189
}
190190

191191
/** Index of the single drawn line containing `token` (fails if 0 or 2+ match). */
@@ -766,7 +766,7 @@ describe("export layout contract — keep-with-next pagination (#629)", () => {
766766
},
767767
],
768768
};
769-
const lines = await extractPdfDrawnLines(await renderAtsResumePdf(model));
769+
const lines = await extractPdfDrawnLines((await renderAtsResumePdf(model)).bytes);
770770
const pages = Math.max(...lines.map((l) => l.page));
771771
const densest = Math.max(
772772
...Array.from({ length: pages }, (_, i) => linesOnPage(lines, i + 1)),
@@ -827,7 +827,7 @@ describe("export layout contract — the Summary body honours widow control", ()
827827
* {@link bulletLinesPerPage} returns, so a `1` is the widow. */
828828
async function summaryLinesPerPage(words: number): Promise<number[]> {
829829
const lines = await extractPdfDrawnLines(
830-
await renderAtsResumePdf(summaryModel(words)),
830+
(await renderAtsResumePdf(summaryModel(words))).bytes,
831831
);
832832
const perPage = new Map<number, number>();
833833
for (const line of lines) {

src/lib/pdf/render-ats-pdf.flush-right-links.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ const MODEL: AtsResumeModel = {
9090

9191
describe("renderAtsResumePdf — flush-right dates + link annotations (#425)", () => {
9292
it("draws the entry date flush-right against the content margin", async () => {
93-
const { items } = await inspect(await renderAtsResumePdf(MODEL));
93+
const { items } = await inspect((await renderAtsResumePdf(MODEL)).bytes);
9494
// A year token from the date range, on the right side of the page.
9595
const dateItems = items.filter((i) => /20(20|23)/.test(i.str) && i.x > 300);
9696
expect(dateItems.length).toBeGreaterThan(0);
@@ -105,7 +105,7 @@ describe("renderAtsResumePdf — flush-right dates + link annotations (#425)", (
105105
});
106106

107107
it("registers clickable URI link annotations for the contact links", async () => {
108-
const { links } = await inspect(await renderAtsResumePdf(MODEL));
108+
const { links } = await inspect((await renderAtsResumePdf(MODEL)).bytes);
109109
expect(links).toContain("https://linkedin.com/in/jane");
110110
expect(links).toContain("https://github.com/jane");
111111
expect(links).toContain("mailto:jane@example.com");
@@ -125,7 +125,7 @@ describe("renderAtsResumePdf — flush-right dates + link annotations (#425)", (
125125
},
126126
sections: [],
127127
};
128-
const { annots } = await inspect(await renderAtsResumePdf(model));
128+
const { annots } = await inspect((await renderAtsResumePdf(model)).bytes);
129129
const email = annots.find((a) => a.url === "mailto:jane@example.com");
130130
const site = annots.find((a) => a.url.startsWith("https://example.com"));
131131
expect(email).toBeDefined();
@@ -148,7 +148,7 @@ describe("renderAtsResumePdf — flush-right dates + link annotations (#425)", (
148148
};
149149
// pdfjs may normalize a bare-host URL with a trailing slash, so match on the
150150
// scheme+host rather than an exact string.
151-
const { links } = await inspect(await renderAtsResumePdf(model));
151+
const { links } = await inspect((await renderAtsResumePdf(model)).bytes);
152152
expect(links.some((u) => u.startsWith("https://www.jane.dev"))).toBe(true);
153153
expect(links.some((u) => u.startsWith("http://portfolio.example"))).toBe(true);
154154
// The naive display-rebuilt targets (www dropped, forced https) must NOT appear.

0 commit comments

Comments
 (0)