Skip to content

fix(compression): apply the lossy policy to the default-combo fallback - #15609

Merged
diegosouzapw merged 4 commits into
diegosouzapw:release/v3.8.52from
woodsonl:fix/compression-default-combo-lossy
Oct 6, 2026
Merged

diegosouzapw merged 4 commits into
diegosouzapw:release/v3.8.52from
woodsonl:fix/compression-default-combo-lossy

Conversation

@woodsonl

@woodsonl woodsonl commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • handleChatCore's legacy default-combo fallback now respects the x-omniroute-compression header and the fix(compression): keep lossy engines off the default path #14529 lossy policy: a plan the header chose (named combo, engine:, default, off) always wins, and on every other request the default combo runs only its safe steps. Lossy engines such as rtk stay off unless the request sends allow-lossy.
  • planFromHeader now requires a non-empty array pipeline for a named-combo match. An empty-pipeline combo (reachable from the per-engine config page) used to win header precedence and let the builtin rtk/caveman pipeline run lossy on requests that never opted in. The same lookup resolved prototype keys (constructor, toString) to truthy garbage.
  • The fallback logs a debug line when it skips for a request carrying a compression header, so operators can see why an edited default combo did not apply.

Related Issues

Validation

  • Change type: routing
  • Focused tests and category gates from the golden path
  • npm run lint (pre-commit: lint-staged, docs-sync, any-budget, tracked-artifacts all green)
  • Reconciled with the current active release base; focused checks rerun afterward (upstream/release/v3.8.52 merged at 54e865d; 47/47 focused tests and typecheck:core green on the merged tree)
  • Production-code changes include a new or updated automated test in this PR

⚠️ base-red inherited: #15306 (release/v3.8.52 tip was red when this PR opened; the failures are not from this branch)

Tests Added Or Updated

  • tests/unit/compression/default-combo-lossy-policy.test.ts (new): the default combo's lossy steps are dropped without opt-in, kept under allow-lossy, and the header wins.
  • tests/unit/chatcore-compression-combo-predicates.test.ts: eight new cases (yield-to-header, allow-lossy, empty pipeline, single lossy engine swapped for the safe pair, single safe engine kept, resolvable engine: header yields).
  • tests/unit/compression/compression-header-dispatch.test.ts: two new cases (empty-pipeline combo is not a chosen plan, prototype-key header values resolve to null).
  • tests/integration/chatcore-compression-integration.test.ts: removed the obsolete skipped case that expected rtk to run on the seeded default combo.

Coverage Notes

open-sse/ changes are covered by the three unit files above plus the integration file. The full compression aggregate (1682 tests) and typecheck:core pass on the merged tree. Evidence: _artifacts/suite-final.log and the QA report (verdict pass, 4/4 contracts) in .gstack/qa-reports/qa-report-compression-default-combo-2026-10-05.md.

Reviewer Notes

  • Behavior change 1: a request with header "default" now gets the panel-derived default profile and no longer activates the DB default combo's outputMode or language packs. A client relying on that loses it silently; the response header still reports source=default.
  • Behavior change 2: combos with an empty pipeline no longer win header precedence, so the builtin stacked pipeline no longer runs unrequested through that path.
  • Known adjacent gaps left for follow-up on purpose (each needs its own TDD loop): adaptive floor-mode escalation can re-inject lossy engines after the policy (strategySelector.ts); the compressionPlan hand-off drops adaptive escalation when the default combo applies; routing-assigned compression combos still execute their raw pipeline while plan metadata reports the stripped one; the downgrade bypasses result memoization because session-dedup is not deterministic-eligible.

handleChatCore's legacy default-combo fallback installs the default
compression combo's pipeline after resolveBasePlan has already applied
the lossy-request policy. So an operator-edited default combo, for
example one with rtk added, ran its lossy steps on requests that sent no
x-omniroute-compression opt-in. The fallback also replaced a plan the
request header had chosen, such as a named combo.

defaultComboForRequest now yields to a plan the header chose and runs
the default combo's pipeline through applyLossyRequestPolicy. With
allow-lossy the full combo runs; every other request gets only its safe
steps. Unit tests replace the skipped integration test that expected rtk
to run on the seeded default combo.
planFromHeader treated any truthy combos-map value as a plan the header
chose. A combo with an empty pipeline (reachable via setEngineInDefaultCombo)
then won the precedence race, made the default-combo fallback yield, and let
the builtin rtk/caveman pipeline run lossy on a request that never opted in:
the exact outcome the lossy policy exists to prevent. The unguarded plain-object
lookup also resolved prototype keys (constructor, toString) to truthy garbage.

The named-combo branch now requires Array.isArray with length > 0. The
default-combo fallback logs when it skips for a request carrying a compression
header, the predicates module comment matches its three exports, and unit tests
pin the empty-pipeline, single-engine, resolvable-engine-header, and
prototype-key cases.
@woodsonl
woodsonl requested a review from diegosouzapw as a code owner October 5, 2026 23:43
@diegosouzapw
diegosouzapw merged commit 9d6d1ad into diegosouzapw:release/v3.8.52 Oct 6, 2026
41 of 51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants