Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion scripts/run-live-tests.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,19 @@
import { spawnSync } from "node:child_process";
import path from "node:path";
import { fileURLToPath } from "node:url";
import nextEnv from "@next/env";
import { childProcessExitCode } from "./child-process-result.mjs";
import { requireProviderTestPermission } from "./test-environment.mjs";
import { acquireHeavyRunLock } from "./test-run-lock.mjs";

const projectRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
const vitestBin = path.join(projectRoot, "node_modules", "vitest", "vitest.mjs");
const { loadEnvConfig } = nextEnv;
const providerTestPermission = process.env.ALLOW_PROVIDER_TESTS;
loadEnvConfig(projectRoot);
Comment thread
BigSimmo marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Set test mode before loading live-test env files

When ALLOW_PROVIDER_TESTS=true npm run test:live is run from a normal shell, this call happens before the script sets NODE_ENV: "test" for Vitest, so Next's env loader resolves the non-test mode and can load .env.production.local/.env.production values into lock.environment before passing them to the test child. In a checkout that keeps production Supabase/OpenAI credentials in those files, the explicit live-test approval can therefore run the provider suite against production-shaped configuration instead of the intended test configuration; set the intended mode before loadEnvConfig (or pass the appropriate dev/test loading mode) so the env files match the test process.

Useful? React with 👍 / 👎.


try {
requireProviderTestPermission();
requireProviderTestPermission({ ALLOW_PROVIDER_TESTS: providerTestPermission });
} catch (error) {
console.error(error instanceof Error ? error.message : String(error));
process.exit(1);
Expand Down
56 changes: 44 additions & 12 deletions src/lib/extractors/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,27 @@ export function parseExtractedDocumentPayload(raw: string): ExtractedDocument {
return extractedDocumentSchema.parse(JSON.parse(raw));
}

export function isUsableFallbackPdfImage(image: {
data?: Uint8Array | null;
width?: number | null;
height?: number | null;
}) {
return (
Boolean(image.data?.byteLength) &&
typeof image.width === "number" &&
Number.isInteger(image.width) &&
image.width > 0 &&
typeof image.height === "number" &&
Number.isInteger(image.height) &&
image.height > 0
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

function isRecoverableFallbackPdfImageError(error: unknown) {
if (!(error instanceof Error)) return false;
return /^Image object .+(?:: (?:data field is empty or invalid|data buffer is empty)| not found)/.test(error.message);
}

export async function terminateProcessTree(child: ChildProcess) {
const pid = child.pid;
if (!pid) return;
Expand Down Expand Up @@ -307,17 +328,29 @@ export async function extractPdf(
for (const page of rawPages) budget.addPage(page.text);

const images: ExtractedDocument["images"] = [];
const imageCountByPage = new Map<number, number>();
const malformedImagePages = new Set<number>();
// Extract one page at a time so aggregate limits can stop subsequent decoding, and
// request only binary data to avoid holding a duplicate base64 representation.
for (const rawPage of rawPages) {
const imageResult = await imageParser.getImage({
partial: [rawPage.pageNumber],
imageBuffer: true,
imageDataUrl: false,
imageThreshold: 20,
});
let imageResult: Awaited<ReturnType<PDFParse["getImage"]>>;
try {
imageResult = await imageParser.getImage({
partial: [rawPage.pageNumber],
imageBuffer: true,
imageDataUrl: false,
imageThreshold: 20,
});
} catch (imageError) {
if (!isRecoverableFallbackPdfImageError(imageError)) throw imageError;
imageCountByPage.set(rawPage.pageNumber, (imageCountByPage.get(rawPage.pageNumber) ?? 0) + 1);
malformedImagePages.add(rawPage.pageNumber);
continue;
}
for (const page of imageResult.pages) {
imageCountByPage.set(page.pageNumber, (imageCountByPage.get(page.pageNumber) ?? 0) + page.images.length);
for (const [index, image] of page.images.entries()) {
if (!isUsableFallbackPdfImage(image)) continue;
Comment thread
BigSimmo marked this conversation as resolved.
Comment thread
BigSimmo marked this conversation as resolved.
Comment thread
BigSimmo marked this conversation as resolved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
budget.assertRenderDimensions(image.width, image.height);
budget.assertArtifact(image.data.byteLength);
const mimeType = "image/png";
Expand Down Expand Up @@ -348,12 +381,6 @@ export async function extractPdf(
// below-threshold text as needsOcr so index_quality surfaces it (and the worker refuses
// to mark an image-only PDF as fully indexed).
const JS_FALLBACK_MIN_TEXT_CHARS = 40;
const imageCountByPage = new Map<number, number>();
for (const image of images) {
if (image.pageNumber === null) continue;
imageCountByPage.set(image.pageNumber, (imageCountByPage.get(image.pageNumber) ?? 0) + 1);
}

const pages: ExtractedPage[] = rawPages.map((page) => {
const textLength = page.text.trim().length;
const hasImages = (imageCountByPage.get(page.pageNumber) ?? 0) > 0;
Expand All @@ -362,6 +389,11 @@ export async function extractPdf(
});

const warnings = ["Used JavaScript PDF fallback; install Python PDF/OCR prerequisites for scanned PDFs."];
if (malformedImagePages.size > 0) {
warnings.push(
`malformed_fallback_images: skipped unusable image data on ${malformedImagePages.size} page(s); review or OCR sparse pages.`,
);
}
const ocrNeededPages = pages.filter((page) => page.needsOcr).length;
if (ocrNeededPages > 0) {
warnings.push(`needs_ocr: ${ocrNeededPages} page(s) appear image-only and were not OCR'd by the JS fallback.`);
Expand Down
60 changes: 60 additions & 0 deletions tests/pdf-extraction-budget.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,4 +241,64 @@ describe("PDF extraction budgets", () => {
"This extracted text is deliberately longer than one byte.",
);
});

it("skips malformed image entries returned by the JavaScript fallback", async () => {
const root = await mkdtemp(path.join(tmpdir(), "clinical-kb-pdf-malformed-image-test-"));
roots.push(root);
const fakeExtractor = path.join(root, "missing-dependency.py");
await writeFile(
fakeExtractor,
"import sys\nprint('PyMuPDF unavailable', file=sys.stderr)\nraise SystemExit(2)\n",
"utf8",
);
vi.spyOn(PDFParse.prototype, "getText").mockResolvedValue({
pages: [{ num: 1, text: "Short fallback text." }],
text: "Short fallback text.",
} as never);
vi.spyOn(PDFParse.prototype, "getImage").mockResolvedValue({
pages: [
{
pageNumber: 1,
images: [
{ data: undefined, width: 10, height: 20 },
{ data: new Uint8Array([1]), width: 10.5, height: 20 },
],
},
],
} as never);
vi.spyOn(PDFParse.prototype, "destroy").mockResolvedValue(undefined);

const extracted = await extractPdf(await createTextPdf(), { scriptPathOverride: fakeExtractor });
roots.push(...(extracted.temporaryPaths ?? []));

expect(extracted.images).toEqual([]);
expect(extracted.pages[0]?.text).toContain("Short fallback text");
expect(extracted.pages[0]?.needsOcr).toBe(true);
});

it("preserves text and OCR evidence when pdf-parse rejects malformed image data", async () => {
const root = await mkdtemp(path.join(tmpdir(), "clinical-kb-pdf-rejected-image-test-"));
roots.push(root);
const fakeExtractor = path.join(root, "missing-dependency.py");
await writeFile(
fakeExtractor,
"import sys\nprint('PyMuPDF unavailable', file=sys.stderr)\nraise SystemExit(2)\n",
"utf8",
);
vi.spyOn(PDFParse.prototype, "getText").mockResolvedValue({
pages: [{ num: 1, text: "Short fallback text." }],
text: "Short fallback text.",
} as never);
vi.spyOn(PDFParse.prototype, "getImage").mockRejectedValue(
new Error("Image object img_p0_1: data field is empty or invalid. Available fields: width, height"),
);
vi.spyOn(PDFParse.prototype, "destroy").mockResolvedValue(undefined);

const extracted = await extractPdf(await createTextPdf(), { scriptPathOverride: fakeExtractor });
roots.push(...(extracted.temporaryPaths ?? []));

expect(extracted.images).toEqual([]);
expect(extracted.pages[0]).toMatchObject({ text: "Short fallback text.", needsOcr: true });
expect(extracted.warnings).toEqual(expect.arrayContaining([expect.stringMatching(/^malformed_fallback_images:/)]));
});
});
11 changes: 11 additions & 0 deletions tests/test-runner-safety.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,17 @@ describe("provider-safe test environment", () => {
expect(`${result.stdout}${result.stderr}`).toContain("Live provider tests are disabled");
});

it("loads credentials from Next environment files without accepting persisted permission", () => {
const runner = readFileSync(new URL("../scripts/run-live-tests.mjs", import.meta.url), "utf8");
const permissionSnapshot = "const providerTestPermission = process.env.ALLOW_PROVIDER_TESTS;";
const permissionCheck = "requireProviderTestPermission({ ALLOW_PROVIDER_TESTS: providerTestPermission });";
expect(runner).toContain('import nextEnv from "@next/env";');
expect(runner).toContain("const { loadEnvConfig } = nextEnv;");
expect(runner.indexOf(permissionSnapshot)).toBeGreaterThanOrEqual(0);
expect(runner.indexOf(permissionSnapshot)).toBeLessThan(runner.indexOf("loadEnvConfig(projectRoot);"));
expect(runner.indexOf("loadEnvConfig(projectRoot);")).toBeLessThan(runner.indexOf(permissionCheck));
});

it("fails focused selection closed for a deleted or missing explicit source path", () => {
const result = spawnSync(process.execPath, ["scripts/test-focused.mjs", "--files", "src/missing-source.ts"], {
cwd: path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."),
Expand Down