Skip to content

Commit e50c3de

Browse files
Frooodlejbrunton96
andauthored
Run classification locally first and only escalate an unsure verdict to the AI (#7580)
Split out of #7574 — this is the classification half, which is independent of the editor-source work and can land on its own. ## What this does - **Runs the local heuristic first and only escalates an unsure verdict to the AI.** A high-confidence local answer stands; anything less (or a file the heuristic hasn't reached yet) goes to the engine. A wrong label costs more than an engine call, so the bar is deliberately strict. - **Makes `classify` an authorable pipeline task**, so it can be used as a step like any other tool, and skips files that are already classified. - **Leaves the seeded Classification policy unowned** rather than naming a `system` placeholder that was never a real user; existing seeds are repaired on boot. ## Review feedback applied From @jbrunton96 on #7574: - **The generic runner no longer names classification.** Everything classification-specific moved into `proprietary/data/classificationPolicy.ts`, and `usePolicyAutoRun` now asks capability questions instead: `policyRewritesDocument`, `policyDeliversOutputFiles`, `policyRequiresAiEngine`, `shouldDispatchToAi`. There is no `id === "classification"` left in the runner. - **Ordering is no longer a name in the runner.** `pinClassificationLast` is gone; the runner sorts annotating policies after rewriting ones. The constraint is real: an annotating policy is non-blocking, so a rewriting one running after it forks from the pre-annotation version and drops the labels. To be straight about what this is and isn't - see "Still open" below - `policyRewritesDocument` is still keyed on the category id, not on a property each policy declares. The check moved out of the runner; it did not stop being a check on one id. - **Confidence is typed.** New `ClassificationConfidence` union in `core/types/fileContext.ts`, reused by `fileStorage`, `HeuristicConfidence`, and the trusted-verdict constant instead of being respelled at each site. - **Comments trimmed** to the repo's 2-line guideline, and a stale seeder javadoc that still claimed an internal-user owner was corrected. ## Still open, deliberately `classificationPolicy.ts` answers its capability questions with `categoryId === "classification"`. That is the same check relocated, not removed, and the module doc now says so outright. Deliberate, for two reasons: - **The concept it would be declared against is going away.** Policies are becoming pipelines with labels behind a separate enforcement layer, which removes the category the flag would live on. A capability system built on `categoryId` today gets migrated twice. - **Classification is genuinely privileged, not accidentally special.** It is the only policy with a browser-side implementation, so it can answer without the server. That is a product decision, and a local-only mode for set scenarios is planned - the flag for it should be designed with that feature, not guessed at now. The end state for the rest: an in-place output mode retires the ordering rule and `policyDeliversOutputFiles`, and a run result that can carry findings as well as files retires the remainder. Both touch the import path, which is the most delicate code in `usePolicyAutoRun` - not something to bolt on to a PR that has already been split once. Nothing is broken by leaving it. A user-built classify pipeline still gets its labels: the generic import path reads them off the returned PDF. It versions the file instead of labelling in place, and it misses the local-heuristic shortcut, so it always bills the engine. ## Testing - `classificationPolicy.test.ts` — 12 cases covering each capability and the escalation rule - Full frontend `proprietary` project: 39 files / 442 tests - `:proprietary:test` for `DefaultClassificationPolicySeederTest` + `ClassifyLabelControllerTest` - `tsc --noEmit` on core, proprietary, portal, saas, desktop, cloud --------- Co-authored-by: James Brunton <jbrunton96@gmail.com>
1 parent 8e7501a commit e50c3de

31 files changed

Lines changed: 707 additions & 289 deletions

app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ClassifyLabelController.java

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.util.Set;
1010

1111
import org.apache.pdfbox.pdmodel.PDDocument;
12+
import org.apache.pdfbox.pdmodel.PDDocumentInformation;
1213
import org.springframework.beans.factory.annotation.Autowired;
1314
import org.springframework.core.io.Resource;
1415
import org.springframework.http.MediaType;
@@ -20,12 +21,13 @@
2021
import org.springframework.web.multipart.MultipartFile;
2122

2223
import io.github.pixee.security.Filenames;
23-
import io.swagger.v3.oas.annotations.Hidden;
2424
import io.swagger.v3.oas.annotations.Operation;
2525
import io.swagger.v3.oas.annotations.tags.Tag;
2626

2727
import lombok.extern.slf4j.Slf4j;
2828

29+
import stirling.software.common.model.tool.ToolFormat;
30+
import stirling.software.common.model.tool.ToolIO;
2931
import stirling.software.common.service.CustomPDFDocumentFactory;
3032
import stirling.software.common.service.PdfMetadataService;
3133
import stirling.software.common.service.UserServiceInterface;
@@ -48,11 +50,13 @@
4850
* <p>Runs as a Classification-policy pipeline step: it reads a bounded page window, asks the AI
4951
* engine to classify the document against the built-in label set, and stores the engine's JSON
5052
* answer — minus the transport-only {@code outcome} field — in the custom Info-dictionary key
51-
* {@link PdfMetadataService#CLASSIFICATION_KEY}. Returns the labelled PDF. Not intended for direct
52-
* client use.
53+
* {@link PdfMetadataService#CLASSIFICATION_KEY}. Returns the labelled PDF.
54+
*
55+
* <p>Published in the API spec rather than hidden, so the tool-model generator emits it and a
56+
* pipeline can name it as a step like any other tool. Classification is a thing a pipeline does,
57+
* not a thing only the Classification policy may do.
5358
*/
5459
@Slf4j
55-
@Hidden
5660
@RestController
5761
@RequestMapping("/api/v1/ai/tools")
5862
@Tag(name = "AI Tools", description = "Dispatchable AI-backed tools.")
@@ -99,19 +103,31 @@ public ClassifyLabelController(
99103
}
100104

101105
@PostMapping(value = "/classify-and-label", consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
106+
// PDF in, the same PDF out with a verdict on it, so a chain can be checked across this step.
107+
@ToolIO(accepts = ToolFormat.PDF, produces = ToolFormat.PDF)
102108
@Operation(
103109
summary = "Classify a PDF and label its metadata",
104110
description =
105111
"Reads the first two and last two pages, classifies the document via the AI"
106112
+ " engine, and stores the result in the StirlingPDFClassification"
107-
+ " metadata field. Dispatched by the Classification policy; not"
108-
+ " intended for direct client use.")
113+
+ " metadata field. A document that already carries a verdict is"
114+
+ " passed through untouched unless reclassify=true.")
109115
public ResponseEntity<Resource> classifyAndLabel(
110-
@RequestParam("fileInput") MultipartFile fileInput) throws IOException {
116+
@RequestParam("fileInput") MultipartFile fileInput,
117+
@RequestParam(value = "reclassify", defaultValue = "false") boolean reclassify)
118+
throws IOException {
111119
aiFeatureGate.requireClassify();
112120
try (PDDocument document = pdfDocumentFactory.load(fileInput, true)) {
113121
String fileName = safeFileName(fileInput.getOriginalFilename());
114122

123+
if (!reclassify && isClassified(document)) {
124+
// Classifying twice costs a second engine call and charges for it, and a document
125+
// that already carries a verdict has nothing new to learn. A pipeline can run this
126+
// step over a mixed batch without paying for the ones already done.
127+
log.debug("[classify-and-label] {} already classified; passing through", fileName);
128+
return WebResponseUtils.pdfDocToWebResponse(document, fileName, tempFileManager);
129+
}
130+
115131
List<EngineLabel> allowed = resolveAllowedLabels();
116132
if (allowed.isEmpty()) {
117133
// No vocabulary to classify against: pass the file through unlabelled rather than
@@ -135,6 +151,25 @@ public ResponseEntity<Resource> classifyAndLabel(
135151
}
136152
}
137153

154+
/**
155+
* Whether a verdict is already on the document.
156+
*
157+
* <p>This only reads back what a previous run of this step wrote. It is not a statement that
158+
* the verdict is trustworthy: the key is ordinary PDF metadata that whoever supplied the file
159+
* can set. Skipping the engine on the strength of it is safe because the cost of being wrong is
160+
* a missing re-classification, not a wrong decision. Anything that makes a SECURITY decision
161+
* from this field - routing a document somewhere on the strength of its label, say - must
162+
* classify with {@code reclassify=true} rather than trust what arrived.
163+
*/
164+
private static boolean isClassified(PDDocument document) {
165+
PDDocumentInformation info = document.getDocumentInformation();
166+
if (info == null) {
167+
return false;
168+
}
169+
String existing = info.getCustomMetadataValue(PdfMetadataService.CLASSIFICATION_KEY);
170+
return existing != null && !existing.isBlank();
171+
}
172+
138173
private List<AiPageText> extractWindow(PDDocument document) throws IOException {
139174
List<AiPageText> pages = new ArrayList<>();
140175
for (int pageNumber : windowPageNumbers(document.getNumberOfPages(), WINDOW_PAGES)) {

app/proprietary/src/main/java/stirling/software/proprietary/policy/model/Policy.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ public Policy withOutput(OutputSpec resolved) {
8585
return new Policy(id, name, owner, enabled, inputs, steps, resolved, outputIds, teamId);
8686
}
8787

88+
/** A copy under a different owner (e.g. moving a seed off a placeholder name). */
89+
public Policy withOwner(String newOwner) {
90+
return new Policy(id, name, newOwner, enabled, inputs, steps, output, outputIds, teamId);
91+
}
92+
8893
/** A copy referencing the given saved output destinations. */
8994
public Policy withOutputIds(List<String> newOutputIds) {
9095
return new Policy(id, name, owner, enabled, inputs, steps, output, newOutputIds, teamId);

app/proprietary/src/main/java/stirling/software/proprietary/policy/seed/DefaultClassificationPolicySeeder.java

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
import stirling.software.proprietary.security.service.TeamService;
2323

2424
/**
25-
* Seeds an enabled Classification policy per team so classification is on by default. Idempotent;
26-
* skips the internal team.
25+
* Seeds an enabled Classification policy per team; idempotent, skips the internal team. Left
26+
* unowned: nobody created it, and an owner here would have to name a real user.
2727
*/
2828
@Slf4j
2929
@Component
@@ -34,6 +34,11 @@ public class DefaultClassificationPolicySeeder {
3434
private static final String CLASSIFY_ENDPOINT = "/api/v1/ai/tools/classify-and-label";
3535
private static final String POLICY_NAME = "Classification Policy";
3636

37+
/**
38+
* Pre-existing seeds used this placeholder, which was never a user; see {@link #repairOwner}.
39+
*/
40+
private static final String LEGACY_OWNER = "system";
41+
3742
private final PolicyStore policyStore;
3843
private final TeamRepository teamRepository;
3944

@@ -46,9 +51,8 @@ public void seedDefaultTeamOnStartup() {
4651
.ifPresent(team -> seedIfMissing(team.getId(), team.getName()));
4752
}
4853

49-
// Any team created at runtime (admin-created, SaaS sign-ups). Seeds inside the team's own
50-
// transaction: rollback still leaves no policy behind, and the store's pessimistic lock needs a
51-
// live transaction, which AFTER_COMMIT cannot offer.
54+
// Seeds inside the new team's own transaction: rollback leaves no policy behind, and the
55+
// store's pessimistic lock needs a live transaction, which AFTER_COMMIT cannot offer.
5256
@TransactionalEventListener(phase = TransactionPhase.BEFORE_COMMIT)
5357
public void onTeamCreated(TeamCreatedEvent event) {
5458
seedIfMissing(event.teamId(), event.teamName());
@@ -58,16 +62,33 @@ private void seedIfMissing(Long teamId, String teamName) {
5862
if (teamId == null || TeamService.INTERNAL_TEAM_NAME.equals(teamName)) {
5963
return;
6064
}
61-
boolean alreadySeeded =
65+
Policy existing =
6266
policyStore.findByTeam(teamId).stream()
63-
.anyMatch(DefaultClassificationPolicySeeder::isClassification);
64-
if (alreadySeeded) {
67+
.filter(DefaultClassificationPolicySeeder::isClassification)
68+
.findFirst()
69+
.orElse(null);
70+
if (existing != null) {
71+
repairOwner(existing);
6572
return;
6673
}
6774
policyStore.save(defaultPolicy(teamId));
6875
log.info("Seeded default Classification policy for team {}", teamId);
6976
}
7077

78+
/**
79+
* Clear an owner seeded as a placeholder name. An owner someone deliberately set is left alone.
80+
*/
81+
private void repairOwner(Policy policy) {
82+
if (!LEGACY_OWNER.equals(policy.owner())) {
83+
return;
84+
}
85+
policyStore.save(policy.withOwner(null));
86+
log.info(
87+
"Cleared placeholder owner '{}' on Classification policy {}",
88+
LEGACY_OWNER,
89+
policy.id());
90+
}
91+
7192
private static boolean isClassification(Policy policy) {
7293
return policy.output() != null
7394
&& CATEGORY.equals(policy.output().options().get("categoryId"));
@@ -85,7 +106,9 @@ static Policy defaultPolicy(Long teamId) {
85106
return new Policy(
86107
null,
87108
POLICY_NAME,
88-
"system",
109+
// Nobody created this - it is seeded. A name here would have to be a real user, and
110+
// every consumer of owner already handles its absence.
111+
null,
89112
true,
90113
List.of(),
91114
List.of(new PipelineStep(CLASSIFY_ENDPOINT, Map.of())),

app/proprietary/src/test/java/stirling/software/proprietary/controller/api/ClassifyLabelControllerTest.java

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.util.List;
1515

1616
import org.apache.pdfbox.pdmodel.PDDocument;
17+
import org.apache.pdfbox.pdmodel.PDDocumentInformation;
1718
import org.junit.jupiter.api.Test;
1819
import org.junit.jupiter.api.extension.ExtendWith;
1920
import org.mockito.ArgumentCaptor;
@@ -76,7 +77,7 @@ private void stubSinglePageDocument() throws Exception {
7677
.thenReturn("{\"outcome\":\"classification\",\"labels\":[\"invoice\"]}");
7778

7879
try {
79-
controller.classifyAndLabel(file);
80+
controller.classifyAndLabel(file, false);
8081
} catch (Exception ignored) {
8182
// WebResponseUtils.pdfDocToWebResponse needs a real temp file; the engine call and
8283
// metadata write we assert on have already happened by the time it runs.
@@ -89,6 +90,52 @@ private JsonNode sentEngineRequest() throws Exception {
8990
return objectMapper.readTree(body.getValue());
9091
}
9192

93+
/** Stubs a document that already carries a verdict, as a second run over a batch would see. */
94+
private MultipartFile alreadyClassifiedDocument() throws Exception {
95+
PDDocument document = mock(PDDocument.class);
96+
PDDocumentInformation info = mock(PDDocumentInformation.class);
97+
when(document.getDocumentInformation()).thenReturn(info);
98+
when(info.getCustomMetadataValue(PdfMetadataService.CLASSIFICATION_KEY))
99+
.thenReturn("{\"labels\":[\"invoice\"]}");
100+
MultipartFile file = mock(MultipartFile.class);
101+
when(file.getOriginalFilename()).thenReturn("invoice.pdf");
102+
when(pdfDocumentFactory.load(any(MultipartFile.class), eq(true))).thenReturn(document);
103+
return file;
104+
}
105+
106+
@Test
107+
void classifyAndLabel_skipsADocumentThatAlreadyCarriesAVerdict() throws Exception {
108+
withLabels(List.of(new ClassificationLabel("invoice", "Invoice", null)));
109+
MultipartFile file = alreadyClassifiedDocument();
110+
111+
try {
112+
controller.classifyAndLabel(file, false);
113+
} catch (Exception ignored) {
114+
// The response needs a real temp file; the decision under test happens before it.
115+
}
116+
117+
// No second engine call, and no charge for one: re-classifying buys the same answer twice.
118+
verify(aiEngineClient, never()).post(anyString(), anyString(), any());
119+
verify(pdfMetadataService, never()).setClassificationMetadata(any(), anyString());
120+
}
121+
122+
@Test
123+
void classifyAndLabel_reclassifiesWhenAskedTo() throws Exception {
124+
withLabels(List.of(new ClassificationLabel("invoice", "Invoice", null)));
125+
MultipartFile file = alreadyClassifiedDocument();
126+
when(pdfContentExtractor.extractPageTextRaw(any(), eq(1))).thenReturn("Invoice total");
127+
when(aiEngineClient.post(eq("/api/v1/documents/classify"), anyString(), isNull()))
128+
.thenReturn("{\"outcome\":\"classification\",\"labels\":[\"receipt\"]}");
129+
130+
try {
131+
controller.classifyAndLabel(file, true);
132+
} catch (Exception ignored) {
133+
// As above.
134+
}
135+
136+
verify(aiEngineClient).post(eq("/api/v1/documents/classify"), anyString(), isNull());
137+
}
138+
92139
@Test
93140
void classifyAndLabel_writesClassificationWithoutOutcome() throws Exception {
94141
withLabels(List.of(new ClassificationLabel("invoice", "Invoice", null)));

app/proprietary/src/test/java/stirling/software/proprietary/policy/seed/DefaultClassificationPolicySeederTest.java

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,14 @@ private DefaultClassificationPolicySeeder seeder() {
3636
}
3737

3838
private static Policy classificationPolicy(Long teamId) {
39+
return classificationPolicy(teamId, null);
40+
}
41+
42+
private static Policy classificationPolicy(Long teamId, String owner) {
3943
return new Policy(
4044
"p1",
4145
"Classification Policy",
42-
"system",
46+
owner,
4347
true,
4448
List.of(),
4549
List.of(),
@@ -77,6 +81,29 @@ void doesNotSeedWhenAClassificationPolicyAlreadyExists() {
7781
verify(policyStore, never()).save(any());
7882
}
7983

84+
@Test
85+
void clearsAPlaceholderOwnerSeededBeforeOwnersHadToBeReal() {
86+
when(policyStore.findByTeam(7L)).thenReturn(List.of(classificationPolicy(7L, "system")));
87+
88+
seeder().onTeamCreated(new TeamCreatedEvent(7L, "Acme"));
89+
90+
// "system" was never a user row, and a step dispatch authenticates as the owner. Absence
91+
// is handled everywhere; a placeholder name is not.
92+
ArgumentCaptor<Policy> saved = ArgumentCaptor.forClass(Policy.class);
93+
verify(policyStore).save(saved.capture());
94+
assertThat(saved.getValue().owner()).isNull();
95+
assertThat(saved.getValue().id()).isEqualTo("p1");
96+
}
97+
98+
@Test
99+
void leavesADeliberatelyChosenOwnerAlone() {
100+
when(policyStore.findByTeam(7L)).thenReturn(List.of(classificationPolicy(7L, "alice")));
101+
102+
seeder().onTeamCreated(new TeamCreatedEvent(7L, "Acme"));
103+
104+
verify(policyStore, never()).save(any());
105+
}
106+
80107
@Test
81108
void doesNotSeedForTheInternalTeam() {
82109
seeder().onTeamCreated(new TeamCreatedEvent(2L, "Internal"));

frontend/editor/public/locales/en-US/translation.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4592,6 +4592,10 @@ desc = "Change document restrictions and permissions"
45924592
tags = "permissions,restrictions,rights,access control,allow,deny,printing,copying,editing,modify permissions,security settings,user rights"
45934593
title = "Change Permissions"
45944594

4595+
[home.classify]
4596+
desc = "Identify what kind of document this is and tag it."
4597+
title = "Classify"
4598+
45954599
[home.compare]
45964600
desc = "Compares and shows the differences between 2 PDF Documents"
45974601
tags = "difference,compare,diff,compare PDFs,compare documents,find differences,show differences,changes,what changed,track changes,revisions,version compare,side by side,contrast,delta"
@@ -10841,6 +10845,7 @@ searchPlaceholder = "Search tools..."
1084110845

1084210846
[toolPicker.subcategories]
1084310847
advancedFormatting = "Advanced Formatting"
10848+
ai = "AI"
1084410849
automation = "Automation"
1084510850
developerTools = "Developer Tools"
1084610851
documentReview = "Document Review"

frontend/editor/public/og-metadata.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,11 @@
195195
"title": "Compress - Stirling PDF",
196196
"description": "Compress PDFs to reduce their file size."
197197
},
198+
"classify": {
199+
"image": "/og_images/home.png",
200+
"title": "Classify - Stirling PDF",
201+
"description": "Identify what kind of document this is and tag it."
202+
},
198203
"extractPages": {
199204
"image": "/og_images/extract-pages.png",
200205
"title": "Extract Pages - Stirling PDF",
@@ -575,6 +580,7 @@
575580
"/remove-cert-sign": "removeCertSign",
576581
"/unlock-p-d-f-forms": "unlockPDFForms",
577582
"/compress": "compress",
583+
"/classify": "classify",
578584
"/extract-pages": "extractPages",
579585
"/reorganize-pages": "reorganizePages",
580586
"/extract-images": "extractImages",

frontend/editor/public/og-metadata.saas.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,11 @@
196196
"title": "Compress - Stirling PDF",
197197
"description": "Compress PDFs to reduce their file size."
198198
},
199+
"classify": {
200+
"image": "/og_images/home.png",
201+
"title": "Classify - Stirling PDF",
202+
"description": "Identify what kind of document this is and tag it."
203+
},
199204
"extractPages": {
200205
"image": "/og_images/extract-pages.png",
201206
"title": "Extract Pages - Stirling PDF",
@@ -588,6 +593,7 @@
588593
"/remove-cert-sign": "removeCertSign",
589594
"/unlock-p-d-f-forms": "unlockPDFForms",
590595
"/compress": "compress",
596+
"/classify": "classify",
591597
"/extract-pages": "extractPages",
592598
"/reorganize-pages": "reorganizePages",
593599
"/extract-images": "extractImages",

frontend/editor/scripts/generate-tool-api-types.mts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,16 @@ import { dirname, resolve } from "node:path";
1111
import { parseArgs } from "node:util";
1212
import { compile, type JSONSchema } from "json-schema-to-typescript";
1313

14-
// The API namespaces whose endpoints a pipeline can reference. `/api/v1/ai/tools/`
15-
// is absent from the spec, so it cannot appear here. Extend this list when other
16-
// namespaces become tools.
17-
//
18-
// `/api/v1/filter/` and `/api/v1/integration/` are included even though neither is a
19-
// user-facing tool: a stored pipeline can contain one, and ToolEndpoint keys the I/O
20-
// table, so leaving them out would stop a chain being checked past such a step.
14+
// Endpoints a pipeline can reference. filter/integration are not user-facing tools but a stored
15+
// pipeline can contain one; the AI namespace is admitted one endpoint at a time, not wholesale.
2116
const ALLOWED_PATH_PREFIXES = [
2217
"/api/v1/general/",
2318
"/api/v1/misc/",
2419
"/api/v1/security/",
2520
"/api/v1/convert/",
2621
"/api/v1/filter/",
2722
"/api/v1/integration/",
23+
"/api/v1/ai/tools/classify-and-label",
2824
];
2925

3026
// File plumbing, not user parameters: `fileInput` and `file` are the uploaded primary document

0 commit comments

Comments
 (0)