diff --git a/changelog.d/fixes/6792-compression-put-all-catalog-engines.md b/changelog.d/fixes/6792-compression-put-all-catalog-engines.md new file mode 100644 index 0000000000..813620e098 --- /dev/null +++ b/changelog.d/fixes/6792-compression-put-all-catalog-engines.md @@ -0,0 +1 @@ +- **fix(api):** the compression config `PUT` schema (`stackedPipelineStepSchema`) now accepts every `ENGINE_CATALOG` id — the structural engines `session-dedup`/`ccr`/`headroom`/`relevance`/`llmlingua`/`omniglyph` and the `aggressive` `ultra` intensity — so a `GET`→`PUT` round-trip of a stacked pipeline no longer 400s on a valid engine the discriminated union had omitted (#6747 — thanks @Pitchfork-and-Torch). diff --git a/open-sse/services/compression/types.ts b/open-sse/services/compression/types.ts index fa973fd44e..9bba7973b2 100644 --- a/open-sse/services/compression/types.ts +++ b/open-sse/services/compression/types.ts @@ -39,6 +39,7 @@ export type CompressionEngineId = | "headroom" | "ccr" | "llmlingua" + | "relevance" | "omniglyph"; export interface CavemanRule { diff --git a/src/lib/db/compression.ts b/src/lib/db/compression.ts index e5c8372992..c1e92101fe 100644 --- a/src/lib/db/compression.ts +++ b/src/lib/db/compression.ts @@ -260,8 +260,9 @@ function normalizeContextEditingConfig(value: unknown): ContextEditingConfig { } // Engines allowed in the global stackedPipeline setting. MUST stay in sync with the -// compression-combo KNOWN_ENGINE_IDS (src/lib/db/compressionCombos.ts) — otherwise the -// global setting silently strips engines the combo path accepts (B-PIPELINE-DIVERGENCE). +// compression-combo KNOWN_ENGINE_IDS (src/lib/db/compressionCombos.ts) and with +// stackedPipelineStepSchema / ENGINE_CATALOG — otherwise the global setting silently +// strips engines the combo path accepts (B-PIPELINE-DIVERGENCE / #6747). const STACKED_PIPELINE_ENGINE_IDS = new Set([ "lite", "caveman", @@ -272,6 +273,7 @@ const STACKED_PIPELINE_ENGINE_IDS = new Set([ "session-dedup", "ccr", "llmlingua", + "relevance", "omniglyph", ]); @@ -657,8 +659,7 @@ export async function getCompressionSettings(): Promise { storedEngines = parseStoredEnginesMap(parsed); break; case "activeComboId": - config.activeComboId = - typeof parsed === "string" && parsed.trim() ? parsed.trim() : null; + config.activeComboId = typeof parsed === "string" && parsed.trim() ? parsed.trim() : null; break; case "ultraEngine": // Phase 4 (B): SLM tier selector. Only the two known values; anything else diff --git a/src/lib/db/compressionCombos.ts b/src/lib/db/compressionCombos.ts index e7f3dcb888..e4333509bd 100644 --- a/src/lib/db/compressionCombos.ts +++ b/src/lib/db/compressionCombos.ts @@ -56,6 +56,7 @@ function parseJsonArray(value: unknown, fallback: T[]): T[] { } } +// Keep in sync with stackedPipelineStepSchema + ENGINE_CATALOG (#6747). const KNOWN_ENGINE_IDS = [ "lite", "caveman", @@ -66,6 +67,7 @@ const KNOWN_ENGINE_IDS = [ "session-dedup", "ccr", "llmlingua", + "relevance", ]; function normalizePipeline(value: unknown): CompressionPipelineStep[] { @@ -92,8 +94,7 @@ function upgradeLegacySeededDefaultCompressionCombo(): void { const row = db .prepare("SELECT name, description, pipeline FROM compression_combos WHERE id = ?") .get(DEFAULT_COMPRESSION_COMBO_ID) as - | { name?: string; description?: string; pipeline?: string } - | undefined; + { name?: string; description?: string; pipeline?: string } | undefined; if (!row) return; diff --git a/src/shared/validation/compressionConfigSchemas.ts b/src/shared/validation/compressionConfigSchemas.ts index bdebd8e63a..c57be7dd7d 100644 --- a/src/shared/validation/compressionConfigSchemas.ts +++ b/src/shared/validation/compressionConfigSchemas.ts @@ -141,6 +141,19 @@ export const ultraConfigSchema = z const noConfigSchema = z.object({}).strict(); +// Structural engines (session-dedup / ccr / headroom / relevance / llmlingua) do not +// expose a fixed intensity enum in ENGINE_CATALOG — accept optional free-form intensity +// and a loose config bag so GET→PUT round-trips of stackedPipeline succeed (#6747). +const structuralStepConfigSchema = z.record(z.string(), z.unknown()).optional(); + +/** + * Writable stacked-pipeline step shape for PUT /api/settings/compression and + * PUT /api/context/combos/[id]. MUST accept every engine id in ENGINE_CATALOG / + * GET /api/compression/engines and every engine the DB normalizer keeps + * (src/lib/db/compression.ts STACKED_PIPELINE_ENGINE_IDS). Issue #6747: a 5-engine + * discriminator rejected session-dedup/ccr/headroom/relevance/llmlingua on write even + * though GET returned them. + */ export const stackedPipelineStepSchema = z.discriminatedUnion("engine", [ z .object({ @@ -159,7 +172,8 @@ export const stackedPipelineStepSchema = z.discriminatedUnion("engine", [ z .object({ engine: z.literal("aggressive"), - intensity: z.literal("standard").optional(), + // #6747: previously only "standard"; GET / engines.level may echo "ultra" + intensity: z.enum(["standard", "ultra"]).optional(), config: aggressiveConfigSchema.optional(), }) .strict(), @@ -177,6 +191,48 @@ export const stackedPipelineStepSchema = z.discriminatedUnion("engine", [ config: rtkConfigSchema.optional(), }) .strict(), + z + .object({ + engine: z.literal("session-dedup"), + intensity: z.string().optional(), + config: structuralStepConfigSchema, + }) + .strict(), + z + .object({ + engine: z.literal("ccr"), + intensity: z.string().optional(), + config: structuralStepConfigSchema, + }) + .strict(), + z + .object({ + engine: z.literal("headroom"), + intensity: z.string().optional(), + config: structuralStepConfigSchema, + }) + .strict(), + z + .object({ + engine: z.literal("relevance"), + intensity: z.string().optional(), + config: structuralStepConfigSchema, + }) + .strict(), + z + .object({ + engine: z.literal("llmlingua"), + intensity: z.string().optional(), + config: structuralStepConfigSchema, + }) + .strict(), + z + .object({ + engine: z.literal("omniglyph"), + intensity: z.string().optional(), + config: structuralStepConfigSchema, + }) + .strict(), ]); /** @@ -185,17 +241,23 @@ export const stackedPipelineStepSchema = z.discriminatedUnion("engine", [ * dropdowns and `stackedPipelineStepSchema`: every engine/intensity offered here is, * by construction, accepted by the API update schema. * - * Do NOT add an engine here that is not a branch of `stackedPipelineStepSchema` — the - * `PUT /api/context/combos/[id]` route validates against that discriminated union and - * would reject the payload with HTTP 400 (#4955: the UI previously offered `headroom`, - * `session-dedup`, `ccr`, `llmlingua`, none of which the union accepts, so selecting - * one silently failed the save). The parity is guarded by a unit test. + * Every ENGINE_CATALOG id must appear here (empty intensity list = no level selector). + * Parity with `stackedPipelineStepSchema` is guarded by unit tests (#4955 / #6747). + * #4955 fixed a UI/schema drift by shrinking the UI; #6747 expands the schema so the + * full catalog (and GET stackedPipeline) can round-trip on PUT. */ export const STACKED_PIPELINE_ENGINE_INTENSITIES: Record = { - rtk: ["minimal", "standard", "aggressive"], - caveman: ["lite", "full", "ultra"], + // Order matches ENGINE_CATALOG stackPriority for readability + "session-dedup": [], + ccr: [], lite: ["lite"], - aggressive: ["standard"], + rtk: ["minimal", "standard", "aggressive"], + headroom: [], + relevance: [], + caveman: ["lite", "full", "ultra"], + aggressive: ["standard", "ultra"], + llmlingua: [], + omniglyph: [], ultra: ["ultra"], }; diff --git a/tests/unit/compression/combos-engine-ui-schema-parity-4955.test.ts b/tests/unit/compression/combos-engine-ui-schema-parity-4955.test.ts index 2162dd765d..4855faa60b 100644 --- a/tests/unit/compression/combos-engine-ui-schema-parity-4955.test.ts +++ b/tests/unit/compression/combos-engine-ui-schema-parity-4955.test.ts @@ -3,15 +3,17 @@ import assert from "node:assert/strict"; import { STACKED_PIPELINE_ENGINE_INTENSITIES, + compressionSettingsUpdateSchema, stackedPipelineStepSchema, } from "../../../src/shared/validation/compressionConfigSchemas.ts"; +import { ENGINE_IDS } from "../../../open-sse/services/compression/engineCatalog.ts"; -// Regression guard for #4955: the Engine Combos pipeline editor used to offer engines -// (headroom, session-dedup, ccr, llmlingua) that `stackedPipelineStepSchema` rejects, so -// selecting one made `PUT /api/context/combos/[id]` fail with HTTP 400 and the UI swallowed -// it. The fix routes the dropdown through STACKED_PIPELINE_ENGINE_INTENSITIES, which MUST stay -// in lockstep with the discriminated union below. -describe("Engine Combos UI ↔ stackedPipelineStepSchema parity (#4955)", () => { +// Regression guard for #4955 / #6747: +// - #4955: UI and API schema must stay in lockstep (no engines the UI offers that PUT rejects). +// - #6747: PUT must accept every ENGINE_CATALOG / GET stackedPipeline engine so GET→PUT +// round-trips of compression settings succeed (session-dedup, ccr, headroom, relevance, +// llmlingua were previously rejected by a 5-engine discriminator). +describe("Engine Combos UI ↔ stackedPipelineStepSchema parity (#4955 / #6747)", () => { const unionEngines = stackedPipelineStepSchema.options .map((option: { shape: { engine: { value: string } } }) => option.shape.engine.value) .sort(); @@ -21,8 +23,18 @@ describe("Engine Combos UI ↔ stackedPipelineStepSchema parity (#4955)", () => assert.deepEqual(uiEngines, unionEngines); }); + it("covers every ENGINE_CATALOG id (GET /api/compression/engines parity)", () => { + assert.deepEqual([...ENGINE_IDS].sort(), unionEngines); + }); + it("every (engine, intensity) the UI can emit is accepted by the schema", () => { for (const [engine, intensities] of Object.entries(STACKED_PIPELINE_ENGINE_INTENSITIES)) { + // Engines with no level selector still need bare { engine } accepted + assert.equal( + stackedPipelineStepSchema.safeParse({ engine }).success, + true, + `expected bare { engine: "${engine}" } to be accepted` + ); for (const intensity of intensities) { const result = stackedPipelineStepSchema.safeParse({ engine, intensity }); assert.equal( @@ -34,18 +46,37 @@ describe("Engine Combos UI ↔ stackedPipelineStepSchema parity (#4955)", () => } }); - it("the engines removed from the UI in #4955 are indeed rejected by the schema", () => { - for (const engine of ["headroom", "session-dedup", "ccr", "llmlingua"]) { + it("accepts structural catalog engines that #4955 had temporarily dropped from the UI (#6747)", () => { + for (const engine of ["headroom", "session-dedup", "ccr", "llmlingua", "relevance"]) { assert.equal( - stackedPipelineStepSchema.safeParse({ engine, intensity: "standard" }).success, - false, - `engine "${engine}" must not be a valid stacked-pipeline step` + stackedPipelineStepSchema.safeParse({ engine }).success, + true, + `engine "${engine}" must be a valid stacked-pipeline step` ); - assert.equal( - STACKED_PIPELINE_ENGINE_INTENSITIES[engine], - undefined, - `engine "${engine}" must not be offered by the combos UI` + assert.ok( + Object.prototype.hasOwnProperty.call(STACKED_PIPELINE_ENGINE_INTENSITIES, engine), + `engine "${engine}" must be offered by the combos UI` ); } }); + + it("accepts a full GET-shaped stackedPipeline on settings update (#6747)", () => { + const result = compressionSettingsUpdateSchema.safeParse({ + stackedPipeline: [ + { engine: "session-dedup" }, + { engine: "ccr" }, + { engine: "lite", intensity: "lite" }, + { engine: "rtk", intensity: "standard" }, + { engine: "headroom" }, + { engine: "relevance" }, + { engine: "caveman", intensity: "full" }, + { engine: "aggressive", intensity: "ultra" }, + { engine: "llmlingua" }, + { engine: "ultra", intensity: "ultra" }, + ], + }); + assert.equal(result.success, true, () => + result.success ? "" : JSON.stringify(result.error.issues) + ); + }); });