Apply review feedback: detect service, detect seam, narrower retry

2026-09-03 05:10:16 +03:00 · 2026-08-28 14:36:36 +01:00
parent fcc3c911fe
commit 8de0e144e2
9 changed files with 223 additions and 160 deletions
@@ -28,14 +28,9 @@ import stirling.software.common.util.FormUtils;
 import stirling.software.common.util.FormUtils.NewFormFieldDefinition;
 import stirling.software.common.util.TempFileManager;
 import stirling.software.common.util.WebResponseUtils;
-import stirling.software.proprietary.formdetection.inference.OnnxFormDetector;
-import stirling.software.proprietary.formdetection.inference.RfDetr;
-import stirling.software.proprietary.formdetection.inference.Yolo;
 import stirling.software.proprietary.formdetection.model.DetectedField;
-import stirling.software.proprietary.formdetection.model.ModelCatalogEntry;
-import stirling.software.proprietary.formdetection.render.CoordinateMapper;
 import stirling.software.proprietary.formdetection.render.PageRasterizer;
-import stirling.software.proprietary.formdetection.service.FormDetectionModelManager;
+import stirling.software.proprietary.formdetection.service.FormDetectionService;

 /**
 * Detection endpoint, behind the {@code form-detection} key that is disabled until a model is
@@ -49,15 +44,7 @@ import stirling.software.proprietary.formdetection.service.FormDetectionModelMan
@Tag(name = "Auto Form Detection")
 public class FormDetectionController {

-    /** Hard bound on pages per request; inference is ~1s/page, so this caps worst-case work. */
-    static final int MAX_PAGES = 500;
-
-    /** Cap on total fields; past this an output PDF is unusable and NMS cost is O(n^2). */
-    static final int MAX_FIELDS = 2000;
-
-    private final FormDetectionModelManager manager;
-    private final OnnxFormDetector detector;
-    private final PageRasterizer rasterizer;
+    private final FormDetectionService detection;
    private final CustomPDFDocumentFactory pdfDocumentFactory;
    private final TempFileManager tempFileManager;

@@ -78,67 +65,13 @@ public class FormDetectionController {
            return ResponseEntity.badRequest()
                    .body(Map.of("reason", "INVALID_PDF", "message", "The uploaded file is empty"));
        }
-        if (!manager.isReady()) {
-            return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE)
-                    .body(
-                            Map.of(
-                                    "reason",
-                                    "DEPENDENCY",
-                                    "message",
-                                    "AI form-detection model is not installed"));
-        }
-        ModelCatalogEntry spec = manager.getActiveEntry().orElse(null);
-        if (spec == null) {
-            return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE)
-                    .body(
-                            Map.of(
-                                    "reason",
-                                    "DEPENDENCY",
-                                    "message",
-                                    "Active model spec unavailable"));
-        }
-        // An out-of-range or NaN threshold would keep essentially every anchor.
-        float score =
-                confThreshold != null && !confThreshold.isNaN()
-                        ? Math.clamp(confThreshold, 0f, 1f)
-                        : spec.getScoreThreshold();
-        byte[] pdfBytes = file.getBytes();

-        // Pages are consumed as they are rendered, so only one page of RGBA is ever live.
-        List<DetectedField> collected = new ArrayList<>();
        List<DetectedField> detections;
        try {
-            rasterizer.rasterize(
-                    pdfBytes,
-                    spec.getInputSize(),
-                    MAX_PAGES,
-                    page -> {
-                        Yolo.Preprocessed pre =
-                                Yolo.preprocess(page.rgba(), page.widthPx(), page.heightPx(), spec);
-                        Map<String, Yolo.RawOutput> out =
-                                detector.infer(pre.chw(), spec.getInputSize());
-                        for (Yolo.Detection d : decodeFor(spec, out, pre, score)) {
-                            DetectedField.RectPt rect = CoordinateMapper.toPdfPoints(d, page);
-                            if (rect.w() <= 0 || rect.h() <= 0) {
-                                continue;
-                            }
-                            collected.add(
-                                    new DetectedField(
-                                            fieldType(spec, d.classId()),
-                                            page.pageIndex(),
-                                            rect,
-                                            d.score()));
-                        }
-                    });
-            detections = collected;
-            if (collected.size() > MAX_FIELDS) {
-                log.info(
-                        "Capping {} detections to {} highest-confidence fields",
-                        collected.size(),
-                        MAX_FIELDS);
-                collected.sort((a, b) -> Double.compare(b.confidence(), a.confidence()));
-                detections = new ArrayList<>(collected.subList(0, MAX_FIELDS));
-            }
+            detections = detection.detect(file.getBytes(), confThreshold);
+        } catch (FormDetectionService.ModelUnavailableException e) {
+            return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE)
+                    .body(Map.of("reason", "DEPENDENCY", "message", e.getMessage()));
        } catch (PageRasterizer.PageLimitExceededException e) {
            return ResponseEntity.badRequest()
                    .body(Map.of("reason", "LIMIT", "message", e.getMessage()));
@@ -156,55 +89,31 @@ public class FormDetectionController {
                    .body(Map.of("reason", "DEPENDENCY", "message", e.getMessage()));
        }

-        if (applyToPdf) {
-            // PDFium is more forgiving than PDFBox, so a file can rasterize and still fail to load
-            // here; that is still the file's fault rather than the server's.
-            try (PDDocument document = pdfDocumentFactory.load(file)) {
-                FormUtils.repairMissingWidgetPageReferences(document);
-                List<NewFormFieldDefinition> defs = new ArrayList<>();
-                for (DetectedField f : detections) {
-                    defs.add(toDefinition(f));
-                }
-                FormUtils.addFields(document, defs);
-                return WebResponseUtils.pdfDocToWebResponse(
-                        document, baseName(file) + ".pdf", tempFileManager);
-            } catch (IOException e) {
-                log.debug("Auto Form Detection could not apply fields: {}", e.getMessage());
-                return ResponseEntity.badRequest()
-                        .body(
-                                Map.of(
-                                        "reason",
-                                        "INVALID_PDF",
-                                        "message",
-                                        "The PDF could not be opened for editing; it may be"
-                                                + " corrupt or password-protected"));
+        if (!applyToPdf) {
+            return ResponseEntity.ok(new DetectResponse(detections));
+        }
+        // PDFium is more forgiving than PDFBox, so a file can rasterize and still fail to load
+        // here; that is still the file's fault rather than the server's.
+        try (PDDocument document = pdfDocumentFactory.load(file)) {
+            FormUtils.repairMissingWidgetPageReferences(document);
+            List<NewFormFieldDefinition> defs = new ArrayList<>();
+            for (DetectedField f : detections) {
+                defs.add(toDefinition(f));
            }
+            FormUtils.addFields(document, defs);
+            return WebResponseUtils.pdfDocToWebResponse(
+                    document, baseName(file) + ".pdf", tempFileManager);
+        } catch (IOException e) {
+            log.debug("Auto Form Detection could not apply fields: {}", e.getMessage());
+            return ResponseEntity.badRequest()
+                    .body(
+                            Map.of(
+                                    "reason",
+                                    "INVALID_PDF",
+                                    "message",
+                                    "The PDF could not be opened for editing; it may be"
+                                            + " corrupt or password-protected"));
        }
-        return ResponseEntity.ok(new DetectResponse(detections));
-    }
-
-    /**
-     * Pick the decoder the model's head needs. Unknown values fall back to YOLO, which is what
-     * every catalogue entry was before a second head shape existed.
-     */
-    private static List<Yolo.Detection> decodeFor(
-            ModelCatalogEntry spec,
-            Map<String, Yolo.RawOutput> outputs,
-            Yolo.Preprocessed pre,
-            float score) {
-        if ("rfdetr".equalsIgnoreCase(spec.getDecoder())) {
-            return RfDetr.decode(outputs, spec, pre, score);
-        }
-        // Single-output head: take the sole tensor whatever the graph happens to call it.
-        return Yolo.decode(outputs.values().iterator().next(), spec, pre, score);
-    }
-
-    private static String fieldType(ModelCatalogEntry spec, int classId) {
-        List<String> types = spec.getClassFieldTypes();
-        if (types != null && classId >= 0 && classId < types.size()) {
-            return types.get(classId);
-        }
-        return "text";
    }

    private static NewFormFieldDefinition toDefinition(DetectedField f) {
@@ -33,7 +33,7 @@ import ai.onnxruntime.OrtSession;
@Service
@ConditionalOnClass(name = "ai.onnxruntime.OrtEnvironment")
@RequiredArgsConstructor
-public class OnnxFormDetector implements FormDetectionEngine {
+public class OnnxFormDetector implements UnloadableModel {

    private final FormDetectionModelManager manager;

@@ -4,7 +4,7 @@ package stirling.software.proprietary.formdetection.inference;
 * Lets the model manager drop an engine's loaded model. Must stay ONNX-free: Spring introspecting
 * {@code OnnxFormDetector} without onnxruntime would kill startup.
 */
-public interface FormDetectionEngine {
+public interface UnloadableModel {

    /** Discard any loaded model so the next inference reloads from disk. */
    void unload();
@@ -39,7 +39,7 @@ import stirling.software.common.configuration.RuntimePathConfig;
 import stirling.software.common.model.ApplicationProperties;
 import stirling.software.common.util.GeneralUtils;
 import stirling.software.proprietary.formdetection.catalog.ModelCatalogService;
-import stirling.software.proprietary.formdetection.inference.FormDetectionEngine;
+import stirling.software.proprietary.formdetection.inference.UnloadableModel;
 import stirling.software.proprietary.formdetection.model.FormDetectionStatus;
 import stirling.software.proprietary.formdetection.model.ModelCatalogEntry;
 import stirling.software.proprietary.formdetection.model.ModelStatusResponse;
@@ -94,7 +94,7 @@ public class FormDetectionModelManager {
     * Resolved lazily and by interface: the ONNX engine is absent from builds without onnxruntime,
     * and an eager or concrete-typed dependency would fail startup there.
     */
-    private final ObjectProvider<FormDetectionEngine> engineProvider;
+    private final ObjectProvider<UnloadableModel> engineProvider;

    private final AtomicBoolean installing = new AtomicBoolean(false);
    private volatile FormDetectionStatus state = FormDetectionStatus.NOT_INSTALLED;
@@ -397,7 +397,7 @@ public class FormDetectionModelManager {
     * unchanged, so without this the engine keeps serving the pre-swap session.
     */
    private void invalidateEngine() {
-        engineProvider.ifAvailable(FormDetectionEngine::unload);
+        engineProvider.ifAvailable(UnloadableModel::unload);
    }

    /** SHA-256 of an existing model file as lowercase hex, or {@code null} if it cannot be read. */
@@ -0,0 +1,131 @@
+package stirling.software.proprietary.formdetection.service;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
+import org.springframework.stereotype.Service;
+
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+
+import stirling.software.proprietary.formdetection.inference.OnnxFormDetector;
+import stirling.software.proprietary.formdetection.inference.RfDetr;
+import stirling.software.proprietary.formdetection.inference.Yolo;
+import stirling.software.proprietary.formdetection.model.DetectedField;
+import stirling.software.proprietary.formdetection.model.ModelCatalogEntry;
+import stirling.software.proprietary.formdetection.render.CoordinateMapper;
+import stirling.software.proprietary.formdetection.render.PageRasterizer;
+
+/**
+ * Runs the detection pipeline: rasterize, infer, decode, map to PDF points. Callers other than HTTP
+ * (a pipeline step, a scheduled job) go through here rather than back through the controller.
+ */
+@Slf4j
+@Service
+@ConditionalOnClass(name = "ai.onnxruntime.OrtEnvironment")
+@RequiredArgsConstructor
+public class FormDetectionService {
+
+    /** Hard bound on pages per request; inference is ~1s/page, so this caps worst-case work. */
+    public static final int MAX_PAGES = 500;
+
+    /** Cap on total fields; past this an output PDF is unusable and NMS cost is O(n^2). */
+    public static final int MAX_FIELDS = 2000;
+
+    private final FormDetectionModelManager manager;
+    private final OnnxFormDetector detector;
+    private final PageRasterizer rasterizer;
+
+    /** Thrown when no model is installed, or its catalogue spec has gone missing. */
+    public static class ModelUnavailableException extends RuntimeException {
+        public ModelUnavailableException(String message) {
+            super(message);
+        }
+    }
+
+    /**
+     * Detect fields across every page, in PDF points.
+     *
+     * @param confThreshold overrides the model's own score threshold; null uses the spec's
+     * @throws ModelUnavailableException no model is installed or active
+     * @throws PageRasterizer.PageLimitExceededException the document exceeds {@link #MAX_PAGES}
+     * @throws PageRasterizer.UnreadablePdfException the PDF is empty, corrupt or password-protected
+     * @throws IllegalStateException the ONNX native is missing for this OS/arch
+     */
+    public List<DetectedField> detect(byte[] pdfBytes, Float confThreshold) throws IOException {
+        if (!manager.isReady()) {
+            throw new ModelUnavailableException("AI form-detection model is not installed");
+        }
+        ModelCatalogEntry spec =
+                manager.getActiveEntry()
+                        .orElseThrow(
+                                () ->
+                                        new ModelUnavailableException(
+                                                "Active model spec unavailable"));
+
+        // An out-of-range or NaN threshold would keep essentially every anchor.
+        float score =
+                confThreshold != null && !confThreshold.isNaN()
+                        ? Math.clamp(confThreshold, 0f, 1f)
+                        : spec.getScoreThreshold();
+
+        // Pages are consumed as they are rendered, so only one page of RGBA is ever live.
+        List<DetectedField> collected = new ArrayList<>();
+        rasterizer.rasterize(
+                pdfBytes,
+                spec.getInputSize(),
+                MAX_PAGES,
+                page -> {
+                    Yolo.Preprocessed pre =
+                            Yolo.preprocess(page.rgba(), page.widthPx(), page.heightPx(), spec);
+                    Map<String, Yolo.RawOutput> out =
+                            detector.infer(pre.chw(), spec.getInputSize());
+                    for (Yolo.Detection d : decodeFor(spec, out, pre, score)) {
+                        DetectedField.RectPt rect = CoordinateMapper.toPdfPoints(d, page);
+                        if (rect.w() <= 0 || rect.h() <= 0) {
+                            continue;
+                        }
+                        collected.add(
+                                new DetectedField(
+                                        fieldType(spec, d.classId()),
+                                        page.pageIndex(),
+                                        rect,
+                                        d.score()));
+                    }
+                });
+
+        if (collected.size() <= MAX_FIELDS) {
+            return collected;
+        }
+        log.info(
+                "Capping {} detections to {} highest-confidence fields",
+                collected.size(),
+                MAX_FIELDS);
+        collected.sort((a, b) -> Double.compare(b.confidence(), a.confidence()));
+        return new ArrayList<>(collected.subList(0, MAX_FIELDS));
+    }
+
+    /** Pick the decoder the model's head needs; an unrecognised value falls back to YOLO. */
+    private static List<Yolo.Detection> decodeFor(
+            ModelCatalogEntry spec,
+            Map<String, Yolo.RawOutput> outputs,
+            Yolo.Preprocessed pre,
+            float score) {
+        if ("rfdetr".equalsIgnoreCase(spec.getDecoder())) {
+            return RfDetr.decode(outputs, spec, pre, score);
+        }
+        // Single-output head: take the sole tensor whatever the graph happens to call it.
+        return Yolo.decode(outputs.values().iterator().next(), spec, pre, score);
+    }
+
+    private static String fieldType(ModelCatalogEntry spec, int classId) {
+        List<String> types = spec.getClassFieldTypes();
+        if (types != null && classId >= 0 && classId < types.size()) {
+            return types.get(classId);
+        }
+        return "text";
+    }
+}
@@ -18,6 +18,7 @@ import stirling.software.proprietary.formdetection.inference.OnnxFormDetector;
 import stirling.software.proprietary.formdetection.model.ModelCatalogEntry;
 import stirling.software.proprietary.formdetection.render.PageRasterizer;
 import stirling.software.proprietary.formdetection.service.FormDetectionModelManager;
+import stirling.software.proprietary.formdetection.service.FormDetectionService;

 class FormDetectionControllerTest {

@@ -25,11 +26,11 @@ class FormDetectionControllerTest {
            FormDetectionModelManager manager,
            OnnxFormDetector detector,
            PageRasterizer rasterizer) {
+        // A real service over mocked collaborators, so the HTTP translation is exercised against
+        // the pipeline's actual exceptions rather than a stubbed stand-in.
        FormDetectionController controller =
                new FormDetectionController(
-                        manager,
-                        detector,
-                        rasterizer,
+                        new FormDetectionService(manager, detector, rasterizer),
                        Mockito.mock(CustomPDFDocumentFactory.class),
                        Mockito.mock(TempFileManager.class));
        return MockMvcBuilders.standaloneSetup(controller).build();
@@ -76,8 +77,7 @@ class FormDetectionControllerTest {
        PageRasterizer rasterizer = Mockito.mock(PageRasterizer.class);
        Mockito.doThrow(
                        new PageRasterizer.PageLimitExceededException(
-                                FormDetectionController.MAX_PAGES + 1,
-                                FormDetectionController.MAX_PAGES))
+                                FormDetectionService.MAX_PAGES + 1, FormDetectionService.MAX_PAGES))
                .when(rasterizer)
                .rasterize(Mockito.any(), Mockito.anyInt(), Mockito.anyInt(), Mockito.any());

@@ -99,7 +99,7 @@ class FormDetectionControllerTest {
                .rasterize(
                        Mockito.any(),
                        Mockito.anyInt(),
-                        Mockito.eq(FormDetectionController.MAX_PAGES),
+                        Mockito.eq(FormDetectionService.MAX_PAGES),
                        Mockito.any());
    }

@@ -33,7 +33,7 @@ import stirling.software.SPDF.config.EndpointConfiguration;
 import stirling.software.common.configuration.RuntimePathConfig;
 import stirling.software.common.model.ApplicationProperties;
 import stirling.software.proprietary.formdetection.catalog.ModelCatalogService;
-import stirling.software.proprietary.formdetection.inference.FormDetectionEngine;
+import stirling.software.proprietary.formdetection.inference.UnloadableModel;
 import stirling.software.proprietary.formdetection.model.ModelCatalogEntry;

 class FormDetectionModelManagerTest {
@@ -48,7 +48,7 @@ class FormDetectionModelManagerTest {

    /** Stands in for a build with no ONNX engine bean, which is the default packaging. */
    @SuppressWarnings("unchecked")
-    private static ObjectProvider<FormDetectionEngine> noEngine() {
+    private static ObjectProvider<UnloadableModel> noEngine() {
        return Mockito.mock(ObjectProvider.class);
    }

@@ -60,6 +60,16 @@ describe("processAutoFormDetection", () => {
    expect(body.get("confThreshold")).toBe("0.45");
  });

+  it("does not retry when the detect request itself fails", async () => {
+    (apiClient.post as Mock).mockRejectedValueOnce(new Error("503 no model"));
+
+    await expect(process(defaultParameters, [pdfFile()])).rejects.toThrow(
+      "503 no model",
+    );
+    expect(apiClient.post).toHaveBeenCalledTimes(1);
+    expect(applyFields).not.toHaveBeenCalled();
+  });
+
  it("re-requests a server-applied PDF when applying locally fails", async () => {
    expectConsole.warn(/applying fields locally failed/);
    (applyFields as Mock).mockRejectedValueOnce(new Error("bad xref"));
@@ -40,6 +40,24 @@ function asPdf(data: BlobPart, source: File): File {
  return new File([data], `${base}_form.pdf`, { type: "application/pdf" });
 }

+/**
+ * The one place detection is asked for. A detector running in the browser would substitute here
+ * without the caller changing, since the wire contract is plain geometry.
+ */
+export async function detectFields(
+  parameters: AutoFormDetectionParameters,
+  file: File,
+): Promise<DetectedField[]> {
+  // Ask for the field list rather than a finished PDF so the summary panel has counts to show;
+  // applying the fields here also spares the server a second parse of the same file.
+  const res = await apiClient.post(
+    DETECT_ENDPOINT,
+    buildAutoFormDetectionFormData(parameters, file, false),
+  );
+  return ((res.data as { detections?: DetectedField[] })?.detections ??
+    []) as DetectedField[];
+}
+
 async function processAutoFormDetection(
  parameters: AutoFormDetectionParameters,
  files: File[],
@@ -49,36 +67,31 @@ async function processAutoFormDetection(
  try {
    emitStage({ kind: "starting" });
    emitStage({ kind: "uploading" });
-
-    // Ask for the field list rather than a finished PDF so the summary panel has counts to
-    // show; applying the fields here also spares the server a second parse of the same file.
-    const res = await apiClient.post(
-      DETECT_ENDPOINT,
-      buildAutoFormDetectionFormData(parameters, file, false),
-    );
-    const fields = ((res.data as { detections?: DetectedField[] })
-      ?.detections ?? []) as DetectedField[];
+    const fields = await detectFields(parameters, file);

    emitStage({ kind: "applying" });
-    const { applyFields } =
-      await import("@app/services/formDetection/applyFields");
-    const bytes = await file.arrayBuffer();
-    const appliedPdf = await applyFields(bytes, fields);
+    try {
+      const { applyFields } =
+        await import("@app/services/formDetection/applyFields");
+      const bytes = await file.arrayBuffer();
+      const appliedPdf = await applyFields(bytes, fields);

-    emitSummary(summarizeFields(fields));
-    return { files: [asPdf(new Uint8Array(appliedPdf), file)] };
-  } catch (e) {
-    // pdf-lib rejects some documents PDFBox accepts; let the server write the fields instead.
-    console.warn(
-      "[AutoFormDetection] applying fields locally failed; asking the server to apply them",
-      e,
-    );
-    const res = await apiClient.post(
-      DETECT_ENDPOINT,
-      buildAutoFormDetectionFormData(parameters, file, true),
-      { responseType: "blob" },
-    );
-    return { files: [asPdf(res.data as Blob, file)] };
+      emitSummary(summarizeFields(fields));
+      return { files: [asPdf(new Uint8Array(appliedPdf), file)] };
+    } catch (e) {
+      // Guards the local apply only: pdf-lib rejects some documents PDFBox accepts. A failed
+      // detect must not land here, or every server error costs a second upload and inference.
+      console.warn(
+        "[AutoFormDetection] applying fields locally failed; asking the server to apply them",
+        e,
+      );
+      const res = await apiClient.post(
+        DETECT_ENDPOINT,
+        buildAutoFormDetectionFormData(parameters, file, true),
+        { responseType: "blob" },
+      );
+      return { files: [asPdf(res.data as Blob, file)] };
+    }
  } finally {
    emitStage({ kind: "done" });
  }