From 6094040e0afcd6e1843c7780d9ddbc822ac73dbc Mon Sep 17 00:00:00 2001 From: ConnorYoh <40631091+ConnorYoh@users.noreply.github.com> Date: Fri, 31 Jul 2026 14:52:44 +0100 Subject: [PATCH] Make Auto Rotate's settings editable in the pipeline composer (#7222) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #7152. ## Why Auto Rotate was already selectable as a pipeline step — `getExecutableTools` lists any registry tool with an `operationConfig` and an endpoint resolvable from defaults, which it has. But once added, the settings pane showed *"Displaying these tool params for editing is not supported yet"* and the step card read *"Runs with default settings"*. Both come from the same cause. `classifyToolStepSupport` keys off the operation config's mappers: ```js const hasMappers = Boolean(config?.toApiParams && config?.fromApiParams); if (!hasMappers) return "unsupported"; return entry.automationSettings ? "editable" : "noSettings"; ``` Auto Rotate had `automationSettings` but no mappers, because its custom processor builds its own FormData and never needed them — so it classified as `unsupported`. The more consequential half is `serializeToolStep`, which emits `parameters: {}` for a mapper-less tool. A composed pipeline therefore ran Auto Rotate on server defaults with no way to change that. "Runs with default settings" was literal. ## What Declares `toApiParams` / `fromApiParams` on `autoRotateOperationConfig`, mapping `detectionMode`, `confidenceThreshold` and `inferUndetected`. That flips the step to `editable`, so the composer reuses the existing `AutoRotateAutomationSettings` panel, and the chosen settings now reach the backend. **No backend change.** Policy steps already serialise `parameters` as form fields and the endpoint already accepts these three — verified against a running server while working on #7152. ## Testing - New round-trip test in `toolAutomation.test.ts`: the step serialises to `{detectionMode, confidenceThreshold, inferUndetected}` and deserialises back with `support: "editable"`, using the real operation config. - `core/hooks/tools/shared` + `portal/components/pipelines` suites: 63 tests pass. - Typecheck across build variants, ESLint, Prettier. The composer UI itself was not clicked: the portal is not served by the editor dev server locally, so the verification is the unit test plus the render path (`PipelineStepSettings` renders `entry.automationSettings` when support is `editable`, and the registry entry supplies it). Worth a click-through on the preview deploy. ## Note for maintainers While tracing this I found `PIPELINE_OPERATIONS` in `portal/components/pipelines/pipelineOperations.ts` is exported but never imported — the composer builds its list from the tool registry instead. `humanizeOperation` in the same file *is* still used as a label fallback. Left alone here as it is out of scope, but it looks like dead code worth deleting separately. Generated with [Claude Code](https://claude.com/claude-code) --- .../autoRotate/useAutoRotateOperation.ts | 35 +++++++++++++++- .../hooks/tools/shared/toolAutomation.test.ts | 41 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/frontend/editor/src/core/hooks/tools/autoRotate/useAutoRotateOperation.ts b/frontend/editor/src/core/hooks/tools/autoRotate/useAutoRotateOperation.ts index 6460a66cfa..488a15a8e2 100644 --- a/frontend/editor/src/core/hooks/tools/autoRotate/useAutoRotateOperation.ts +++ b/frontend/editor/src/core/hooks/tools/autoRotate/useAutoRotateOperation.ts @@ -16,12 +16,43 @@ import { ToolOperationHook, } from "@app/hooks/tools/shared/useToolOperation"; import { createStandardErrorHandler } from "@app/utils/toolErrorHandler"; +import type { + ToolApiParams, + ToolEndpoint, +} from "@app/hooks/tools/shared/toolApiMapping"; import { AutoRotateParameters, + AutoRotateDetectionMode, defaultParameters, } from "@app/hooks/tools/autoRotate/useAutoRotateParameters"; -export const AUTO_ROTATE_ENDPOINT = "/api/v1/misc/auto-rotate-pdf"; +export const AUTO_ROTATE_ENDPOINT = + "/api/v1/misc/auto-rotate-pdf" satisfies ToolEndpoint; + +type AutoRotateApiParams = ToolApiParams[typeof AUTO_ROTATE_ENDPOINT]; + +/** + * Frontend params -> the endpoint's request model. Declaring these is what lets a + * backend pipeline step carry this tool's settings: serializeToolStep sends `{}` + * for a tool without mappers, and the pipeline composer can only offer a settings + * panel for a tool that can map its parameters both ways. + */ +export const autoRotateToApiParams = ( + parameters: AutoRotateParameters, +): AutoRotateApiParams => ({ + detectionMode: parameters.detectionMode, + confidenceThreshold: parameters.confidenceThreshold, + inferUndetected: parameters.inferUndetected, +}); + +/** Rehydrate this tool's settings from a stored step's request body. */ +export const autoRotateFromApiParams = ( + apiParams: AutoRotateApiParams, +): Partial => ({ + detectionMode: apiParams.detectionMode as AutoRotateDetectionMode, + confidenceThreshold: apiParams.confidenceThreshold, + inferUndetected: apiParams.inferUndetected, +}); export type AutoRotateMethod = "text" | "osd" | "inferred" | "none"; @@ -135,6 +166,8 @@ export const autoRotateOperationConfig = defineCustomTool( operationType: "autoRotate", endpoint: AUTO_ROTATE_ENDPOINT, customProcessor: createAutoRotateProcessor(), + toApiParams: autoRotateToApiParams, + fromApiParams: autoRotateFromApiParams, defaultParameters, }, ); diff --git a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts index 03b85798d8..f4cb974692 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts @@ -22,6 +22,8 @@ import { defaultParameters as compressDefaults } from "@app/hooks/tools/compress import { splitOperationConfig } from "@app/hooks/tools/split/useSplitOperation"; import { SPLIT_METHODS } from "@app/constants/splitConstants"; import { redactOperationConfig } from "@app/hooks/tools/redact/useRedactOperation"; +import { autoRotateOperationConfig } from "@app/hooks/tools/autoRotate/useAutoRotateOperation"; +import { defaultParameters as autoRotateDefaults } from "@app/hooks/tools/autoRotate/useAutoRotateParameters"; function entry(over: Partial): ToolRegistryEntry { return { @@ -92,6 +94,11 @@ const dynamicRegistry: Partial = { automationSettings: NoopSettings, operationConfig: asRegistryConfig(redactOperationConfig), }), + autoRotate: entry({ + name: "Auto Rotate", + automationSettings: NoopSettings, + operationConfig: asRegistryConfig(autoRotateOperationConfig), + }), }; describe("getExecutableTools", () => { @@ -157,6 +164,40 @@ describe("serialize/deserialize round-trip", () => { }); }); + test("auto rotate carries its detection settings into the backend step", () => { + // A custom-processor tool: the browser applies the rotations, but a backend + // pipeline still has to receive the detection settings, which only happens + // because the config declares mappers. + const step: WorkingToolStep = { + toolId: "autoRotate" as ToolId, + operation: "/api/v1/misc/auto-rotate-pdf", + params: { + ...autoRotateDefaults, + detectionMode: "osd", + confidenceThreshold: 20, + inferUndetected: false, + }, + support: "editable", + }; + + const api = serializeToolStep(step, dynamicRegistry); + expect(api.operation).toBe("/api/v1/misc/auto-rotate-pdf"); + expect(api.parameters).toEqual({ + detectionMode: "osd", + confidenceThreshold: 20, + inferUndetected: false, + }); + + const back = deserializeToolStep(api, dynamicRegistry); + expect(back.toolId).toBe("autoRotate"); + expect(back.support).toBe("editable"); + expect(back.params).toMatchObject({ + detectionMode: "osd", + confidenceThreshold: 20, + inferUndetected: false, + }); + }); + test("a dynamic-endpoint tool (split by chapters) round-trips as an editable step", () => { const step: WorkingToolStep = { toolId: "split" as ToolId,