From 6293aaddaa61946faea42be9241ca51d9ff3faef Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Mon, 6 Jul 2026 16:59:37 -0400 Subject: [PATCH 1/2] fix(risk-treatment): rank tasks by exact in-process cosine, not filtered ANN (#3359) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Draft treatment plans returned "0 tasks and 0 controls" because findSimilarTasks issued a metadata-filtered ANN query (organizationId AND sourceType = "task") against a single 180k+ vector shared-namespace index. Upstash applies metadata filters during approximate HNSW traversal, so a highly selective per-org filter makes the traversal exhaust its candidate budget on nearer, non-matching vectors before reaching the org's tasks — returning 0 candidates even at topK=1000 when relevant tasks exist. It gets worse as the shared index grows. An org holds at most low-hundreds of tasks, so enumerate the org's task vectors by their `task_${org}_` id prefix and score them exactly in-process. This is the right-sized tool at this scale: no recall loss, no data migration. Controls are derived from the suggested tasks, so restoring task recall restores both. Scores use `(1 + cos) / 2` to match Upstash's COSINE score scale, so the downstream department boost / threshold in linkSuggestions and the reranker's cosineScore hint are unchanged. Verified end-to-end against production data. Isolated to findSimilarTasks; the orphan prune and in-scope filtering in runLinkage are unchanged and still compose correctly. Fixes CS-681 Claude-Session: https://claude.ai/code/session_014LMSrxyX5U6gz8QZqJAGhc Co-authored-by: Claude Opus 4.8 (1M context) --- apps/app/src/lib/embedding/embedding.spec.ts | 117 ++++++++++++++--- apps/app/src/lib/embedding/index.ts | 128 +++++++++++++++---- 2 files changed, 201 insertions(+), 44 deletions(-) diff --git a/apps/app/src/lib/embedding/embedding.spec.ts b/apps/app/src/lib/embedding/embedding.spec.ts index 3c6d6a20b1..9cf759e422 100644 --- a/apps/app/src/lib/embedding/embedding.spec.ts +++ b/apps/app/src/lib/embedding/embedding.spec.ts @@ -29,9 +29,11 @@ vi.mock('ai', () => ({ })), })); +import { embedMany } from 'ai'; import { upsertEntityEmbeddings, findSimilarTasks, + cosineToUnitScore, waitForIndexed, pruneOrphanTaskVectors, } from './index'; @@ -168,11 +170,42 @@ describe('upsertEntityEmbeddings', () => { }); }); +describe('cosineToUnitScore', () => { + it('maps identical vectors to 1, opposite to 0, orthogonal to 0.5', () => { + expect(cosineToUnitScore([1, 0], [1, 0])).toBeCloseTo(1, 10); + expect(cosineToUnitScore([1, 0], [-1, 0])).toBeCloseTo(0, 10); + expect(cosineToUnitScore([1, 0], [0, 1])).toBeCloseTo(0.5, 10); + }); + + it('is magnitude-invariant (pure cosine) and matches Upstash (1+cos)/2', () => { + // Same direction, different magnitudes → still 1. + expect(cosineToUnitScore([2, 0], [5, 0])).toBeCloseTo(1, 10); + // cos = 0.6 → (1 + 0.6) / 2 = 0.8 + expect(cosineToUnitScore([3, 4], [3, 0])).toBeCloseTo(0.8, 10); + }); + + it('returns 0 for a zero vector instead of NaN', () => { + expect(cosineToUnitScore([0, 0], [1, 1])).toBe(0); + expect(cosineToUnitScore([1, 1], [0, 0])).toBe(0); + }); +}); + describe('findSimilarTasks', () => { - it('queries with org + sourceType filter and returns id+score+department', async () => { - queryMock.mockResolvedValueOnce([ - { id: 'task_org_1_tsk_a', score: 0.82, metadata: { sourceId: 'tsk_a', department: 'hr' } }, - { id: 'task_org_1_tsk_b', score: 0.71, metadata: { sourceId: 'tsk_b', department: 'none' } }, + // Range page helper — `nextCursor: ''` signals the enumeration is drained. + function taskPage( + vectors: Array<{ id: string; vector: number[]; metadata?: Record }>, + ) { + rangeMock.mockResolvedValueOnce({ nextCursor: '', vectors }); + } + + it('enumerates the org task vectors by prefix and ranks them in-process by cosine', async () => { + // Query points along [1,0]; task vectors are chosen so the cosine ordering + // is deterministic: tsk_a (identical) > tsk_b (orthogonal) > tsk_c (opposite). + vi.mocked(embedMany).mockResolvedValueOnce({ embeddings: [[1, 0]] } as never); + taskPage([ + { id: 'task_org_1_tsk_b', vector: [0, 1], metadata: { sourceId: 'tsk_b', department: 'none' } }, + { id: 'task_org_1_tsk_a', vector: [1, 0], metadata: { sourceId: 'tsk_a', department: 'hr' } }, + { id: 'task_org_1_tsk_c', vector: [-1, 0], metadata: { sourceId: 'tsk_c' } }, ]); const results = await findSimilarTasks({ @@ -181,42 +214,88 @@ describe('findSimilarTasks', () => { topK: 10, }); - expect(queryMock).toHaveBeenCalledWith( + // Enumerates via prefix range WITH the stored vectors — never an ANN query. + expect(rangeMock).toHaveBeenCalledWith( expect.objectContaining({ - topK: 10, + cursor: '0', + prefix: 'task_org_1_', + includeVectors: true, includeMetadata: true, - filter: 'organizationId = "org_1" AND sourceType = "task"', }), ); + expect(queryMock).not.toHaveBeenCalled(); + // Sorted by score desc, scores on Upstash's (1+cos)/2 scale. expect(results).toEqual([ - { id: 'tsk_a', score: 0.82, department: 'hr' }, - { id: 'tsk_b', score: 0.71, department: 'none' }, + { id: 'tsk_a', score: 1, department: 'hr' }, + { id: 'tsk_b', score: 0.5, department: 'none' }, + { id: 'tsk_c', score: 0, department: undefined }, ]); }); - it('returns empty when query text is empty', async () => { + it('applies the topK cap after ranking', async () => { + vi.mocked(embedMany).mockResolvedValueOnce({ embeddings: [[1, 0]] } as never); + taskPage([ + { id: 'task_org_1_tsk_a', vector: [1, 0], metadata: { sourceId: 'tsk_a' } }, + { id: 'task_org_1_tsk_b', vector: [0.9, 0.1], metadata: { sourceId: 'tsk_b' } }, + { id: 'task_org_1_tsk_c', vector: [-1, 0], metadata: { sourceId: 'tsk_c' } }, + ]); + const results = await findSimilarTasks({ organizationId: 'org_1', - queryText: ' ', + queryText: 'phishing risk', + topK: 2, }); + + expect(results).toHaveLength(2); + expect(results.map((r) => r.id)).toEqual(['tsk_a', 'tsk_b']); + }); + + it('paginates the enumeration across cursor pages', async () => { + vi.mocked(embedMany).mockResolvedValueOnce({ embeddings: [[1, 0]] } as never); + rangeMock + .mockResolvedValueOnce({ + nextCursor: 'cursor_2', + vectors: [{ id: 'task_org_1_tsk_p1', vector: [1, 0], metadata: { sourceId: 'tsk_p1' } }], + }) + .mockResolvedValueOnce({ + nextCursor: '', + vectors: [{ id: 'task_org_1_tsk_p2', vector: [0, 1], metadata: { sourceId: 'tsk_p2' } }], + }); + + const results = await findSimilarTasks({ organizationId: 'org_1', queryText: 'x' }); + + expect(rangeMock).toHaveBeenCalledTimes(2); + expect(rangeMock.mock.calls[1][0]).toEqual( + expect.objectContaining({ cursor: 'cursor_2', prefix: 'task_org_1_' }), + ); + // A task from page 2 is included in the ranking. + expect(results.map((r) => r.id).sort()).toEqual(['tsk_p1', 'tsk_p2']); + }); + + it('returns empty when query text is empty (no embed, no range)', async () => { + const results = await findSimilarTasks({ organizationId: 'org_1', queryText: ' ' }); expect(results).toEqual([]); + expect(rangeMock).not.toHaveBeenCalled(); expect(queryMock).not.toHaveBeenCalled(); }); + it('returns empty when the org has no task vectors', async () => { + vi.mocked(embedMany).mockResolvedValueOnce({ embeddings: [[1, 0]] } as never); + taskPage([]); + const results = await findSimilarTasks({ organizationId: 'org_1', queryText: 'x' }); + expect(results).toEqual([]); + }); + it('recovers the raw task id from the prefix when metadata.sourceId is missing', async () => { // Legacy vector with no sourceId metadata. Returning the prefixed embedding // id would make runLinkage drop it as "not in live scope" (taskById is keyed // by raw ids), silently starving suggestions. (cubic P1) - queryMock.mockResolvedValueOnce([ - { id: 'task_org_1_tsk_legacy', score: 0.8, metadata: {} }, - ]); + vi.mocked(embedMany).mockResolvedValueOnce({ embeddings: [[1, 0]] } as never); + taskPage([{ id: 'task_org_1_tsk_legacy', vector: [1, 0], metadata: {} }]); - const results = await findSimilarTasks({ - organizationId: 'org_1', - queryText: 'phishing risk', - }); + const results = await findSimilarTasks({ organizationId: 'org_1', queryText: 'phishing risk' }); - expect(results).toEqual([{ id: 'tsk_legacy', score: 0.8, department: undefined }]); + expect(results).toEqual([{ id: 'tsk_legacy', score: 1, department: undefined }]); }); }); diff --git a/apps/app/src/lib/embedding/index.ts b/apps/app/src/lib/embedding/index.ts index 7d8b4765ef..5fc79eab3d 100644 --- a/apps/app/src/lib/embedding/index.ts +++ b/apps/app/src/lib/embedding/index.ts @@ -57,13 +57,14 @@ export interface SimilarTaskResult { const EMBEDDING_MODEL = 'text-embedding-3-large'; const EMBEDDING_DIMENSIONS = 1536; const DEFAULT_TOP_K = 25; -// Page size for the orphan-vector sweep. `pruneOrphanTaskVectors` paginates an -// org's task vectors via `range` over their shared id prefix, so there is no -// top-K ceiling — this only controls how many vectors come back per round trip. -const ORPHAN_SCAN_PAGE_SIZE = 1000; +// Pagination for enumerating an org's task vectors by their shared id prefix — +// used by both `findSimilarTasks` (exact in-process ranking) and +// `pruneOrphanTaskVectors` (orphan sweep). Prefix enumeration has no top-K +// ceiling; page size only controls how many vectors come back per round trip. +const TASK_VECTOR_PAGE_SIZE = 1000; // Backstop so a misbehaving cursor can't loop forever. 500 pages × 1000 = 500k // task vectors, far beyond any real org — hitting it signals a bug, not scale. -const ORPHAN_SCAN_MAX_PAGES = 500; +const TASK_VECTOR_MAX_PAGES = 500; let cachedIndex: Index | null = null; @@ -187,9 +188,90 @@ export async function upsertEntityEmbeddings({ }; } +interface OrgTaskVector { + /** raw task id (sourceId), not the prefixed embedding id */ + sourceId: string; + vector: number[]; + department?: string; +} + +/** + * Enumerate an org's task vectors (with their embeddings) by their shared id + * prefix. Every task vector is keyed `task_${organizationId}_${sourceId}` (see + * `embeddingId`), so a cursor-paginated `range` returns exactly this org's task + * vectors — no metadata filter, no top-K ceiling. + */ +async function fetchOrgTaskVectors(organizationId: string): Promise { + const index = getIndex(); + const prefix = embeddingIdPrefix('task', organizationId); + + const out: OrgTaskVector[] = []; + let cursor: string | number = '0'; + let enumeratedFully = false; + for (let page = 0; page < TASK_VECTOR_MAX_PAGES; page++) { + const { vectors, nextCursor }: RangeResult = await index.range({ + cursor, + limit: TASK_VECTOR_PAGE_SIZE, + prefix, + includeVectors: true, + includeMetadata: true, + }); + for (const r of vectors) { + if (!r.vector) continue; // defensive: skip any vector row missing embeddings + const meta = (r.metadata ?? {}) as { sourceId?: string; department?: string }; + out.push({ + sourceId: meta.sourceId ?? sourceIdFromEmbeddingId('task', organizationId, String(r.id)), + vector: r.vector as number[], + department: meta.department ?? undefined, + }); + } + if (!nextCursor) { + enumeratedFully = true; + break; + } + cursor = nextCursor; + } + if (!enumeratedFully) { + console.warn( + `[embedding] task-vector enumeration hit the ${TASK_VECTOR_MAX_PAGES}-page cap for org ${organizationId} after ${out.length} vector(s); ranking may be incomplete`, + ); + } + return out; +} + +/** + * Cosine similarity mapped to Upstash's COSINE score scale, `(1 + cos) / 2`, so + * scores stay in [0, 1] and are drop-in compatible with the department boost / + * threshold in `linkSuggestions` and the reranker's `cosineScore` hint. + */ +export function cosineToUnitScore(a: number[], b: number[]): number { + const len = Math.min(a.length, b.length); + let dot = 0; + let normA = 0; + let normB = 0; + for (let i = 0; i < len; i++) { + dot += a[i] * b[i]; + normA += a[i] * a[i]; + normB += b[i] * b[i]; + } + if (normA === 0 || normB === 0) return 0; + const cos = dot / (Math.sqrt(normA) * Math.sqrt(normB)); + return (1 + cos) / 2; +} + /** * Find the top-K most similar Tasks in an org for a given query string. * Returns raw task ids (not prefixed embedding ids) along with score + department. + * + * Retrieval enumerates the org's task vectors by id prefix and scores them + * exactly in-process, rather than issuing a metadata-filtered ANN query. A + * filtered `query` (`organizationId AND sourceType`) collapses to near-zero + * recall here: one org is a tiny slice of a 180k+ vector shared-namespace index, + * so Upstash's approximate traversal exhausts its candidate budget on nearer, + * non-matching vectors before reaching this org's tasks — returning 0 candidates + * even when relevant tasks exist, which starved treatment plans of every + * suggestion (CS-681). An org holds at most low-hundreds of tasks, so exact + * scoring over the full set is both correct (no recall loss) and cheap. */ export async function findSimilarTasks({ organizationId, @@ -203,23 +285,19 @@ export async function findSimilarTasks({ values: [queryText], providerOptions: { openai: { dimensions: EMBEDDING_DIMENSIONS } }, }); - - const index = getIndex(); - const results = await index.query({ - vector: embeddings[0], - topK, - includeMetadata: true, - filter: `organizationId = "${organizationId}" AND sourceType = "task"`, - }); - - return results.map((r) => { - const meta = (r.metadata ?? {}) as { sourceId?: string; department?: string }; - return { - id: meta.sourceId ?? sourceIdFromEmbeddingId('task', organizationId, String(r.id)), - score: r.score, - department: meta.department ?? undefined, - }; - }); + const queryVector = embeddings[0]; + + const taskVectors = await fetchOrgTaskVectors(organizationId); + if (taskVectors.length === 0) return []; + + return taskVectors + .map((t) => ({ + id: t.sourceId, + score: cosineToUnitScore(queryVector, t.vector), + department: t.department, + })) + .sort((a, b) => b.score - a.score) + .slice(0, topK); } export interface PruneOrphanTaskVectorsResult { @@ -267,12 +345,12 @@ export async function pruneOrphanTaskVectors({ let scanned = 0; let cursor: string | number = '0'; let enumeratedFully = false; - for (let page = 0; page < ORPHAN_SCAN_MAX_PAGES; page++) { + for (let page = 0; page < TASK_VECTOR_MAX_PAGES; page++) { // Annotate the result explicitly: `cursor = nextCursor` would otherwise make // TS infer `nextCursor`'s type from a binding that depends on it (TS7022). const { vectors, nextCursor }: RangeResult = await index.range({ cursor, - limit: ORPHAN_SCAN_PAGE_SIZE, + limit: TASK_VECTOR_PAGE_SIZE, prefix, includeMetadata: true, }); @@ -292,7 +370,7 @@ export async function pruneOrphanTaskVectors({ } if (!enumeratedFully) { console.warn( - `[embedding] orphan sweep hit the ${ORPHAN_SCAN_MAX_PAGES}-page cap for org ${organizationId} after scanning ${scanned} vector(s); enumeration may be incomplete`, + `[embedding] orphan sweep hit the ${TASK_VECTOR_MAX_PAGES}-page cap for org ${organizationId} after scanning ${scanned} vector(s); enumeration may be incomplete`, ); } From 5bac5c9bb23cc15f61d3912a6bb457c57a026083 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov <72318342+tofikwest@users.noreply.github.com> Date: Mon, 6 Jul 2026 17:16:27 -0400 Subject: [PATCH 2/2] fix(policies): keep PDF headings with their section (CS-704) (#3358) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(policies): keep PDF headings with their section (CS-704) Policy PDF exports orphaned section headings at the bottom of a page while the body flowed to the next page. Both shared generators only checked that the heading line itself fit before rendering it — there was no keep-together logic, so a heading could land on the last line of a page. Before rendering a heading, reserve room for the heading PLUS the first few lines of the following section (HEADING_KEEP_WITH_LINES); if that doesn't fit, push the whole heading to the next page. The reserve accounts for each generator's actual cursor advancement (the app's per-line lineHeight checks vs. the API renderer's DEFAULT_BREAK_SPACE look-ahead) and only applies when content actually follows the heading, so a trailing heading isn't bumped to a page of its own. Heading lines now also render without per-line page-break checks, so a multi-line heading can no longer be split across pages. Applied to both: - apps/app/src/lib/pdf-generator.ts (single-policy in-app export) - apps/api/src/trust-portal/policy-pdf-renderer.service.ts (full policy-pack export in-app and via the Trust Portal) Adds a regression test that sweeps headings across page boundaries and asserts none are separated from the first line of their section. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_013q6YNeuzKbUdX4yZ3yTnZk * fix(policies): address cubic review on heading keep-together Two issues raised by cubic on the CS-704 keep-together change: 1. hasFollowingContent only checked array position, so a heading followed only by empty paragraphs or hard breaks (common trailing nodes in TipTap docs) still reserved keep-together space and could be bumped to a lonely page — defeating the trailing-heading guard. Now checks whether any following sibling renders visible text (hasRenderableTextAfter). Applied to both the app and API generators. 2. The API heading render loop dropped per-line page-break checks, so a pathological heading wrapping past a full page would overflow the bottom margin instead of paginating. Restored a per-line check as a safety net; the up-front keep-together reserve means normal headings never trip it, so they still can't be split across pages. Adds regression tests for both (verified failing without the fixes). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_013q6YNeuzKbUdX4yZ3yTnZk * fix(policies): cover tables + oversized headings in keep-together (cubic) Second cubic pass on the CS-704 change raised two more edge cases: 1. The keep-together reserve was a fixed text-only slab, so a heading immediately followed by a table with a tall first row (or by leading hard breaks) could still be orphaned — the table's own per-row page-break fires after the heading already committed. Replaced the position/text check with sectionLeadHeight(), which measures what the following section actually needs: a few text lines, a table's first-row height, or 0 for a trailing heading (skipping empty paragraphs and hard breaks). 2. An oversized heading (wrapping past a full page) could emit a blank leading page: requiredHeight exceeded the usable page height, so the up-front checkPageBreak added a page while the current one was still empty. Capped requiredHeight to the usable page height; the per-line loop paginates the heading across pages instead. Applied to both generators (app + API). Adds regression tests for a heading-before-tall-table and the blank-page guard (both verified failing without the fixes). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_013q6YNeuzKbUdX4yZ3yTnZk --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../policy-pdf-renderer.service.spec.ts | 287 ++++++++++++++++++ .../policy-pdf-renderer.service.ts | 146 +++++++-- apps/app/src/lib/pdf-generator.ts | 115 ++++++- 3 files changed, 521 insertions(+), 27 deletions(-) diff --git a/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts b/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts index 926ed3f87d..6434efcbfd 100644 --- a/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts +++ b/apps/api/src/trust-portal/policy-pdf-renderer.service.spec.ts @@ -937,6 +937,293 @@ describe('PolicyPdfRendererService', () => { expect(result.length).toBeGreaterThan(0); }); + // --- CS-704: heading keep-together (no orphaned headings) --------------- + + // Split an (uncompressed) jsPDF buffer into per-page visible text. jsPDF + // writes one content stream per page, so the concatenated (text)Tj tokens + // between two `endstream` markers are exactly one page's text. This lets + // us assert which page a given string lands on. + const pageTextsFrom = (buf: Buffer): string[] => + buf + .toString('latin1') + .split('endstream') + .map((segment) => { + const start = segment.lastIndexOf('stream'); + if (start === -1) return ''; + const body = segment.slice(start + 'stream'.length); + const re = /\((.*?)\)\s*Tj/g; + let text = ''; + let m: RegExpExecArray | null; + while ((m = re.exec(body)) !== null) text += m[1]; + return text; + }) + .filter((t) => t.length > 0); + + const pageIndexContaining = (pages: string[], needle: string): number => + pages.findIndex((t) => t.includes(needle)); + + // Like pageTextsFrom but keeps blank pages (in order), so a blank leading + // page shifts the index of later content. Drops the trailing xref/trailer + // segment that follows the final content stream. + const orderedPageTexts = (buf: Buffer): string[] => { + const segments = buf.toString('latin1').split('endstream'); + return segments.slice(0, -1).map((segment) => { + const start = segment.lastIndexOf('stream'); + if (start === -1) return ''; + const body = segment.slice(start + 'stream'.length); + const re = /\((.*?)\)\s*Tj/g; + let text = ''; + let m: RegExpExecArray | null; + while ((m = re.exec(body)) !== null) text += m[1]; + return text; + }); + }; + + it('does not orphan a heading at the bottom of a page (CS-704)', () => { + // Sweep headings across many vertical offsets by growing the amount of + // filler before each one. Without keep-together logic at least one + // heading lands at a page bottom with its body flowing to the next page. + // With the fix, every heading must share a page with the first line of + // its section. + const SECTIONS = 30; + const nodes: Array> = []; + for (let i = 0; i < SECTIONS; i++) { + for (let f = 0; f < i; f++) { + nodes.push({ + type: 'paragraph', + content: [ + { + type: 'text', + text: `filler ${i}-${f} advancing the cursor down the page`, + }, + ], + }); + } + nodes.push({ + type: 'heading', + attrs: { level: 2 }, + content: [{ type: 'text', text: `HEADINGMARKER${i} Section ${i}` }], + }); + nodes.push({ + type: 'paragraph', + content: [ + { + type: 'text', + text: `BODYMARKER${i} first line of the section body content`, + }, + ], + }); + } + + const result = service.renderPoliciesPdfBuffer([ + { name: 'Keep Together Policy', content: { type: 'doc', content: nodes } }, + ]); + + const pages = pageTextsFrom(result); + expect(pages.length).toBeGreaterThan(1); // multi-page, or the test is moot + + const orphaned: number[] = []; + for (let i = 0; i < SECTIONS; i++) { + const headingPage = pageIndexContaining(pages, `HEADINGMARKER${i}`); + const bodyPage = pageIndexContaining(pages, `BODYMARKER${i}`); + expect(headingPage).toBeGreaterThanOrEqual(0); + expect(bodyPage).toBeGreaterThanOrEqual(0); + if (headingPage !== bodyPage) orphaned.push(i); + } + + expect(orphaned).toEqual([]); + }); + + it('does not push a trailing heading (no following content) to its own page', () => { + // The keep-together reserve must only apply when a section actually + // follows the heading — a heading that is the last node stays on the + // current page rather than being bumped to a fresh one. + const result = service.renderPoliciesPdfBuffer([ + { + name: 'Trailing Heading Policy', + content: { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'INTROBODY opening paragraph' }], + }, + { + type: 'heading', + attrs: { level: 2 }, + content: [{ type: 'text', text: 'TRAILINGHEADING appendix' }], + }, + ], + }, + }, + ]); + + const pages = pageTextsFrom(result); + const introPage = pageIndexContaining(pages, 'INTROBODY'); + const headingPage = pageIndexContaining(pages, 'TRAILINGHEADING'); + expect(introPage).toBe(0); + expect(headingPage).toBe(0); + }); + + it('does not bump a heading when only empty nodes follow it', () => { + // A heading near the page bottom followed ONLY by empty paragraphs / hard + // breaks (common trailing nodes in TipTap docs) must be treated as a + // trailing heading — reserving keep-together space for a non-existent + // section would bump it to a lonely page. + const nodes: Array> = []; + for (let f = 0; f < 23; f++) { + nodes.push({ + type: 'paragraph', + content: [ + { type: 'text', text: `filler line ${f} to consume vertical space` }, + ], + }); + } + nodes.push({ + type: 'heading', + attrs: { level: 2 }, + content: [{ type: 'text', text: 'EMPTYTRAILHEADING near the bottom' }], + }); + // Empty trailing content that renders nothing visible. + nodes.push({ type: 'paragraph' }); + nodes.push({ type: 'paragraph', content: [{ type: 'hardBreak' }] }); + + const result = service.renderPoliciesPdfBuffer([ + { name: 'Empty Trailing Policy', content: { type: 'doc', content: nodes } }, + ]); + + const pages = pageTextsFrom(result); + // The heading shares the page with the filler above it, rather than being + // pushed onto a page of its own. + expect(pageIndexContaining(pages, 'filler line 22')).toBe(0); + expect(pageIndexContaining(pages, 'EMPTYTRAILHEADING')).toBe(0); + }); + + it('keeps a heading with a following table that has a tall first row', () => { + // A heading immediately followed by a table with a tall first row must + // not be orphaned: the reserve has to account for the table's first-row + // height, not just a few plain text lines. Sweep offsets so at least one + // heading lands where the fixed text-only reserve would have orphaned it. + const tallCell = (label: string) => ({ + type: 'tableCell', + content: [ + { + type: 'paragraph', + content: [ + { + type: 'text', + text: `${label} line one line two line three line four line five line six`, + }, + ], + }, + ], + }); + + const SECTIONS = 24; + const nodes: Array> = []; + for (let i = 0; i < SECTIONS; i++) { + for (let f = 0; f < i; f++) { + nodes.push({ + type: 'paragraph', + content: [ + { type: 'text', text: `filler ${i}-${f} advancing the cursor` }, + ], + }); + } + nodes.push({ + type: 'heading', + attrs: { level: 2 }, + content: [{ type: 'text', text: `TABLEHEADING${i} Section ${i}` }], + }); + nodes.push({ + type: 'table', + content: [ + { + type: 'tableRow', + content: [tallCell(`TABLECELL${i}`), tallCell(`meta${i}`)], + }, + ], + }); + } + + const result = service.renderPoliciesPdfBuffer([ + { name: 'Heading Table Policy', content: { type: 'doc', content: nodes } }, + ]); + + const pages = pageTextsFrom(result); + const orphaned: number[] = []; + for (let i = 0; i < SECTIONS; i++) { + const headingPage = pageIndexContaining(pages, `TABLEHEADING${i}`); + const tablePage = pageIndexContaining(pages, `TABLECELL${i}`); + expect(headingPage).toBeGreaterThanOrEqual(0); + expect(tablePage).toBeGreaterThanOrEqual(0); + if (headingPage !== tablePage) orphaned.push(i); + } + expect(orphaned).toEqual([]); + }); + + it('does not emit a blank leading page for an oversized heading', () => { + // A nameless policy whose first (and only) node is a heading taller than + // the page must start rendering on page 1, not after an empty page. The + // reserve is capped at the usable page height so the up-front check can't + // add a page while the current one is still empty. + const longHeadingText = Array.from( + { length: 200 }, + () => 'BLANKGUARDWORD', + ).join(' '); + + const result = service.renderPoliciesPdfBuffer([ + { + name: '', + content: { + type: 'doc', + content: [ + { + type: 'heading', + attrs: { level: 1 }, + content: [{ type: 'text', text: longHeadingText }], + }, + ], + }, + }, + ]); + + const orderedPages = orderedPageTexts(result); + // The very first page carries heading text — no blank page precedes it. + expect(orderedPages[0]).toContain('BLANKGUARDWORD'); + }); + + it('paginates a heading longer than a page instead of overflowing', () => { + // Pathological guard: a heading that wraps to more lines than fit on one + // page must span multiple pages rather than run off the bottom margin. + const longHeadingText = Array.from( + { length: 200 }, + () => 'OVERFLOWWORD', + ).join(' '); + + const result = service.renderPoliciesPdfBuffer([ + { + name: 'Giant Heading Policy', + content: { + type: 'doc', + content: [ + { + type: 'heading', + attrs: { level: 1 }, + content: [{ type: 'text', text: longHeadingText }], + }, + ], + }, + }, + ]); + + const pages = pageTextsFrom(result); + const pagesWithHeading = pages.filter((t) => + t.includes('OVERFLOWWORD'), + ).length; + expect(pagesWithHeading).toBeGreaterThan(1); + }); + it('applies custom primary color', () => { const result = service.renderPoliciesPdfBuffer( [ diff --git a/apps/api/src/trust-portal/policy-pdf-renderer.service.ts b/apps/api/src/trust-portal/policy-pdf-renderer.service.ts index 11c3c61c7c..12bbb6396a 100644 --- a/apps/api/src/trust-portal/policy-pdf-renderer.service.ts +++ b/apps/api/src/trust-portal/policy-pdf-renderer.service.ts @@ -25,6 +25,19 @@ interface PolicyForPDF { content: any; } +// Keep-together: minimum number of body lines that must fit on the same page +// as a heading. If the heading plus this many lines of the following section +// don't fit, the heading is pushed to the next page so it isn't orphaned at +// the bottom of a page (CS-704). +// NOTE: Keep in sync with apps/app/src/lib/pdf-generator.ts HEADING_KEEP_WITH_LINES +const HEADING_KEEP_WITH_LINES = 3; + +// Default vertical room (mm) checkPageBreak requires below the cursor before +// committing content to the current page. Body lines are laid out one at a +// time and each one demands this much space, so the heading keep-together +// reserve must include one such look-ahead for the following section. +const DEFAULT_BREAK_SPACE = 20; + @Injectable() export class PolicyPdfRendererService { /** @@ -222,7 +235,80 @@ export class PolicyPdfRendererService { return text; } - private checkPageBreak(config: PDFConfig, requiredSpace: number = 20): void { + // First-row height of a table, mirroring the row-height math in renderTable + // so the heading keep-together reserve can size itself against a table that + // follows a heading (not just plain paragraphs). + // NOTE: keep in sync with renderTable's rowHeight calculation. + private measureTableFirstRowHeight( + tableNode: JSONContent, + config: PDFConfig, + ): number { + const firstRow = tableNode.content?.[0]; + if (!firstRow?.content?.length) return 0; + + let columnCount = 0; + for (const cell of firstRow.content) { + columnCount += cell.attrs?.colspan ?? 1; + } + if (columnCount === 0) return 0; + + const colWidth = config.contentWidth / columnCount; + const cellPadding = 2; + let rowHeight = config.lineHeight + cellPadding * 2; + for (const cell of firstRow.content) { + if (cell.type !== 'tableCell' && cell.type !== 'tableHeader') continue; + const width = colWidth * (cell.attrs?.colspan ?? 1); + const text = this.cleanTextForPDF(this.extractCellText(cell.content ?? [])); + const lines = config.doc.splitTextToSize( + text || ' ', + width - cellPadding * 2, + ) as string[]; + rowHeight = Math.max( + rowHeight, + lines.length * config.lineHeight + cellPadding * 2, + ); + } + return rowHeight; + } + + // Extra height (beyond the heading's own height) needed to keep a heading + // with the start of the section that follows it. Returns 0 when nothing + // visible follows (a trailing heading). Handles tables (first-row height) and + // leading hard breaks, so keep-together isn't limited to plain paragraphs. + private sectionLeadHeight( + content: JSONContent[], + index: number, + config: PDFConfig, + ): number { + let lead = 0; + for (let i = index + 1; i < content.length; i++) { + const node = content[i]; + if (node.type === 'hardBreak') { + lead += config.lineHeight; + continue; + } + if (node.type === 'table' && node.content?.length) { + // Heading trailing gap + the table's first row must fit together. + return ( + lead + config.lineHeight + this.measureTableFirstRowHeight(node, config) + ); + } + if (this.extractTextFromContent([node]).trim().length === 0) { + // Empty paragraph or similar: advances the cursor slightly, keep scanning. + if (node.type === 'paragraph') lead += config.lineHeight * 0.5; + continue; + } + // First text-bearing block: reserve the heading's first few section lines + // (HEADING_KEEP_WITH_LINES advances, the last still needs the look-ahead). + return lead + HEADING_KEEP_WITH_LINES * config.lineHeight + DEFAULT_BREAK_SPACE; + } + return 0; + } + + private checkPageBreak( + config: PDFConfig, + requiredSpace: number = DEFAULT_BREAK_SPACE, + ): void { if (config.yPosition + requiredSpace > config.pageHeight - config.margin) { config.doc.addPage(); config.yPosition = config.margin; @@ -256,10 +342,9 @@ export class PolicyPdfRendererService { } private processContent(config: PDFConfig, content: JSONContent[]): void { - for (const node of content) { + for (const [nodeIndex, node] of content.entries()) { switch (node.type) { - case 'heading': - this.checkPageBreak(config, 30); + case 'heading': { const level = node.attrs?.level || 1; const headingSizes: { [key: number]: number } = { 1: 16, @@ -273,25 +358,52 @@ export class PolicyPdfRendererService { config.doc.setFont('helvetica', 'bold'); config.doc.setTextColor(0, 0, 0); - if (node.content) { - const headingText = this.cleanTextForPDF( - this.extractTextFromContent(node.content), - ); - const lines = config.doc.splitTextToSize( - headingText, - config.contentWidth, - ); - for (const line of lines) { - this.checkPageBreak(config); - config.doc.text(line, config.margin, config.yPosition); - config.yPosition += config.lineHeight * 1.2; - } + // Measure the heading against the heading font BEFORE deciding + // whether it fits, so multi-line headings are counted correctly. + const headingText = node.content + ? this.cleanTextForPDF(this.extractTextFromContent(node.content)) + : ''; + const headingLines = headingText + ? (config.doc.splitTextToSize( + headingText, + config.contentWidth, + ) as string[]) + : []; + + // Keep-together: require room for the heading itself PLUS the first + // few lines of the section that follows it; otherwise push the whole + // heading to the next page so it isn't stranded at the page bottom. + // sectionLeadHeight measures what the following section needs (a few + // text lines, or a table's first row, or 0 for a trailing heading), + // and is added to the heading's own height. The reserve is capped at + // the usable page height so an oversized heading can't force an empty + // leading page — the per-line loop below paginates it instead. + const leadHeight = this.sectionLeadHeight(content, nodeIndex, config); + const headingHeight = + Math.max(headingLines.length, 1) * config.lineHeight * 1.2; + const usableHeight = config.pageHeight - config.margin * 2; + const requiredHeight = Math.min( + leadHeight > 0 ? headingHeight + leadHeight : headingHeight, + usableHeight, + ); + this.checkPageBreak(config, requiredHeight); + + // For a normal heading the up-front check already reserved room, so + // these per-line checks never fire (the heading stays intact). They + // only act as a safety net for a pathological heading that wraps to + // more lines than fit on a page, paginating it instead of letting it + // overflow past the bottom margin. + for (const line of headingLines) { + this.checkPageBreak(config, config.lineHeight * 1.2); + config.doc.text(line, config.margin, config.yPosition); + config.yPosition += config.lineHeight * 1.2; } config.doc.setFontSize(config.defaultFontSize); config.doc.setFont('helvetica', 'normal'); config.yPosition += config.lineHeight; break; + } case 'paragraph': this.checkPageBreak(config); diff --git a/apps/app/src/lib/pdf-generator.ts b/apps/app/src/lib/pdf-generator.ts index 662bed434d..9257688306 100644 --- a/apps/app/src/lib/pdf-generator.ts +++ b/apps/app/src/lib/pdf-generator.ts @@ -101,6 +101,13 @@ const convertToInternalFormat = (content: TipTapJSONContent[]): JSONContent[] => })); }; +// Keep-together: minimum number of body lines that must fit on the same page +// as a heading. If the heading plus this many lines of the following section +// don't fit, the heading is pushed to the next page so it isn't orphaned at +// the bottom of a page (CS-704). +// NOTE: Keep in sync with apps/api/src/trust-portal/policy-pdf-renderer.service.ts HEADING_KEEP_WITH_LINES +const HEADING_KEEP_WITH_LINES = 3; + // Helper function to check for page breaks const checkPageBreak = (config: PDFConfig, requiredHeight: number = config.lineHeight) => { if (config.yPosition + requiredHeight > config.pageHeight - config.margin) { @@ -148,6 +155,60 @@ const extractTextFromContent = (content: JSONContent[]): string => { }).join(''); }; +// First-row height of a table, mirroring renderTable's row-height math so the +// heading keep-together reserve can size itself against a table that follows a +// heading (not just plain paragraphs). Keep in sync with renderTable below. +const measureTableFirstRowHeight = ( + tableNode: JSONContent, + config: PDFConfig, +): number => { + const firstRow = tableNode.content?.[0]; + if (!firstRow?.content?.length) return 0; + + let columnCount = 0; + for (const cell of firstRow.content) { + columnCount += cell.attrs?.colspan ?? 1; + } + if (columnCount === 0) return 0; + + const colWidth = config.contentWidth / columnCount; + const cellPadding = 2; + let rowHeight = config.lineHeight + cellPadding * 2; + for (const cell of firstRow.content) { + if (cell.type !== 'tableCell' && cell.type !== 'tableHeader') continue; + const width = colWidth * (cell.attrs?.colspan ?? 1); + const text = cleanTextForPDF(extractCellText(cell.content ?? [])); + const lines = config.doc.splitTextToSize(text || ' ', width - cellPadding * 2); + rowHeight = Math.max( + rowHeight, + lines.length * config.lineHeight + cellPadding * 2, + ); + } + return rowHeight; +}; + +// Content height (beyond the heading's own height plus spacingAfter) needed to +// keep a heading with the start of the section that follows it. Returns 0 when +// nothing visible follows (a trailing heading). Handles tables (first-row +// height) so keep-together isn't limited to plain paragraphs. Empty paragraphs +// and hard breaks render nothing here, so they're skipped. +const sectionLeadHeight = ( + content: JSONContent[], + index: number, + config: PDFConfig, + spacingAfter: number, +): number => { + for (let i = index + 1; i < content.length; i++) { + const node = content[i]; + if (node.type === 'table' && node.content?.length) { + return spacingAfter + measureTableFirstRowHeight(node, config); + } + if (extractTextFromContent([node]).trim().length === 0) continue; + return spacingAfter + HEADING_KEEP_WITH_LINES * config.lineHeight; + } + return 0; +}; + // Enhanced helper function that renders text with proper formatting const renderFormattedContent = ( config: PDFConfig, @@ -178,14 +239,14 @@ const renderFormattedContent = ( // Process JSON content recursively const processContent = (config: PDFConfig, content: JSONContent[], level: number = 0) => { - for (const item of content) { + for (const [nodeIndex, item] of content.entries()) { switch (item.type) { - case 'heading': + case 'heading': { const headingLevel = item.attrs?.level || 1; let fontSize: number; let spacingBefore: number; let spacingAfter: number; - + switch (headingLevel) { case 1: fontSize = 14; @@ -207,18 +268,52 @@ const processContent = (config: PDFConfig, content: JSONContent[], level: number spacingBefore = config.lineHeight; spacingAfter = config.lineHeight * 0.5; } - + config.yPosition += spacingBefore; - checkPageBreak(config); - - if (item.content) { - const headingText = extractTextFromContent(item.content); + + // Keep-together: require room for the heading itself PLUS the first + // few lines of the section that follows it; otherwise push the whole + // heading to the next page so it isn't stranded at the page bottom + // (CS-704). Only reserve the following-section space when content + // actually follows this heading. + const headingText = item.content + ? extractTextFromContent(item.content) + : ''; + config.doc.setFontSize(fontSize); + config.doc.setFont('helvetica', 'bold'); + const headingLineCount = headingText + ? config.doc.splitTextToSize( + cleanTextForPDF(headingText), + config.contentWidth, + ).length + : 0; + // sectionLeadHeight measures what the following section needs (a few + // text lines, or a table's first row, or 0 for a trailing heading), + // added to the heading's own height. The reserve is capped at the + // usable page height so an oversized heading can't force an empty + // leading page — addTextWithWrapping's per-line checks paginate it. + const headingHeight = Math.max(headingLineCount, 1) * config.lineHeight; + const leadHeight = sectionLeadHeight( + content, + nodeIndex, + config, + spacingAfter, + ); + const usableHeight = config.pageHeight - config.margin * 2; + const requiredHeight = Math.min( + leadHeight > 0 ? headingHeight + leadHeight : headingHeight, + usableHeight, + ); + checkPageBreak(config, requiredHeight); + + if (headingText) { addTextWithWrapping(config, headingText, fontSize, true); } - + config.yPosition += spacingAfter; break; - + } + case 'paragraph': if (item.content) { const paragraphText = extractTextFromContent(item.content);