Skip to content

fix(compression): drop the dead per-engine Preserve system prompt checkbox - #15605

Merged
diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.52from
woodsonl:fix/remove-engine-preserve-system-prompt
Oct 6, 2026
Merged

diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.52from
woodsonl:fix/remove-engine-preserve-system-prompt

Conversation

@woodsonl

@woodsonl woodsonl commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #15306 (release/v3.8.52 was red before this branch started; the recorded failures are inherited infra timeouts, none caused by this diff)

Summary

The Lite, Aggressive and Ultra engine pages (/dashboard/context/lite, /aggressive, /ultra) rendered a "Preserve system prompt" checkbox that never took effect:

  • Every runtime apply site reads the flag only from the global compression settings (options.config.preserveSystemPrompt), in cavemanAdapter.ts and strategySelector.ts.
  • The aggressive/ultra settings normalizers copy only named fields, so a stored per-engine value never survives a GET.
  • The Lite page never sent it: liteConfigSchema is strict and carries only compressToolResults and maxToolLength.

So unchecking the box saved without error, compression kept following the global setting, and the box snapped back to checked on the next load.

Changes:

  • Removed the preserveSystemPrompt field from LITE_SCHEMA, AGGRESSIVE_SCHEMA and ULTRA_SCHEMA so the engine config UI stops surfacing it. The global control in the compression panel remains the real switch.
  • Back-compat kept: the engine validateConfig checks and the strict aggressive/ultra Zod step-config schemas still accept the key, so stored combo step configs that carry it keep validating and PUT round-trips don't 400. Lite's strict liteConfigSchema never had a slot for the key (unchanged).
  • Dropped the unused ok() helper from cavemanAdapter.ts and removed its eslint-suppressions.json entry (the file's only suppression).
  • Removed the orphaned compressionEngineConfig.fields.preserveSystemPrompt label from all 67 locale files, verified per file by JSON parse and deep-compare against base minus exactly that key.

Related Issues

  • None on file (operator-reported dead control; no upstream issue number).

Validation

  • Change type: UI / i18n
  • Focused tests and category gates from the golden path
  • npm run lint (eslint with the suppressions file: clean on changed files)
  • Reconciled with the current active release base (merge-base equals the upstream tip 23a1148; base merge clean)
  • Production-code changes include a new or updated automated test in this PR
  • SonarQube: not a PR gate (opt-in while quota is unavailable)

Tests Added Or Updated

  • tests/unit/compression/engine-registry.test.ts: new regression test "does not expose a per-engine preserveSystemPrompt control". Verified red while the field was present (fails on lite, then aggressive/ultra) and green after the schema removal, per TDD. Also pins the retained validateConfig back-compat for aggressive/ultra (boolean accepted, non-boolean rejected) so a future cleanup cannot silently drop it, and scopes the rationale comment accurately (only the aggressive/ultra Zod schemas keep the optional slot).

Coverage Notes

  • This PR touches open-sse/; coverage of the changed paths: 8/8 tested (100%), 0 gaps. Dead-code deletions (the ok() helper, the locale labels) are not paths.
  • Coverage moved nowhere; the diff is net-negative (26 insertions, 233 deletions across 70 files).
COVERED PATHS — fix/remove-engine-preserve-system-prompt vs origin/release/v3.8.52

open-sse/services/compression/engines/cavemanAdapter.ts
├─ AGGRESSIVE_SCHEMA: "preserveSystemPrompt" field removed
│   └─ aggressiveEngine.getConfigSchema() → GET /api/compression/engines → /dashboard/context/aggressive
│       ├─ schema must NOT expose key ......................... [★★★ TESTED] engine-registry.test.ts:121-127
│       └─ checkbox absent on page (schema-driven render) ..... [★★★ TESTED] transitively + ui/engineConfigForm.test.tsx
├─ ULTRA_SCHEMA: field removed → /dashboard/context/ultra
│   └─ same regression loop ................................... [★★★ TESTED] engine-registry.test.ts:121-127
├─ LITE_SCHEMA: field removed → /dashboard/context/lite
│   └─ same regression loop ................................... [★★★ TESTED] engine-registry.test.ts:121-127
├─ ok() helper deleted (zero callers) ......................... [N/A not a path] eslint clean
├─ validateAggressiveConfig keeps legacy-key boolean check
│   ├─ {preserveSystemPrompt:true} → valid ................... [★★★ TESTED] engine-registry.test.ts:130
│   └─ {preserveSystemPrompt:"yes"} → invalid ................. [★★★ TESTED] engine-registry.test.ts:132
└─ validateUltraConfig keeps legacy-key boolean check
    ├─ {preserveSystemPrompt:true} → valid .................... [★★★ TESTED] engine-registry.test.ts:131
    └─ {preserveSystemPrompt:"yes"} → invalid ................. [★★★ TESTED] engine-registry.test.ts:133

config/quality/eslint-suppressions.json
└─ cavemanAdapter no-unused-vars entry removed (ok() gone) .... [N/A not a path] eslint → clean

src/i18n/messages/*.json (67 locales)
└─ compressionEngineConfig.fields.preserveSystemPrompt.label removed
    └─ en ↔ 66 locales key-identical, none extra ............... [★★ GATE-VERIFIED] scripts/i18n/check-key-completeness.mjs → OK

UNCHANGED (not charged to this diff)
├─ Global preserveSystemPrompt / preserveSystemPromptMode shim  [★★★ TESTED] ui/compression-settings-tab-consolidation.test.tsx
├─ validateLiteConfig legacy-key handling ..................... [★★★ TESTED] engine-registry.test.ts:92-99 (pre-existing)
└─ Zod aggressiveConfigSchema/ultraConfigSchema optional slot . [N/A] unchanged

COVERAGE: 8/8 paths tested (100%) — 0 gaps

Pre-Landing Review

  • /ponytail-review: clean, nothing to cut (diff already net-negative).
  • /gstack-review (testing + maintainability specialists, two in-host adversarial passes, exploratory QA; converged after one fix cycle): 0 unresolved defects, quality 10/10. Fixed during review: validator back-compat pins (operator-approved), stale test comment scoped accurately, orphaned i18n label deleted from 67 locales (operator-approved).
  • Findings skipped with reasons (all informational):
    • Installs that toggled the dead box carry the key inside raw DB rows; the read normalizers already whitelist it away and the next save on that engine page rewrites the row without it. No migration for invisible storage drift.
    • Pre-existing: aggressive/ultra apply sites clobber even an explicit step-level preserveSystemPrompt while omniglyph honors step-level values. Out of scope, unchanged by this branch.
    • Pre-existing: validateLiteConfig rejects maxToolLength: null while liteConfigSchema allows null. Unreachable today (no updateEngineConfig caller passes user input).
    • Test-strength nit: the schema-absence loop covers the three engines that ever carried the field, not the whole registry. Non-blocking.
  • Codex outside review unavailable (the machine's Codex model resolver is broken); both adversarial passes ran in-host.

Design Review

No frontend files changed (locale JSON is data; the UI renders from the schema). Design review skipped.

Eval Results

No prompt-related files changed. Evals skipped.

Scope Drift

Scope Check: CLEAN. The diff matches the stated intent: dead checkbox removal, test hardening, dead i18n label. No unrelated files touched.

Plan Completion

No plan file detected. Intent source: operator task description plus commit messages; every step delivered (consumer survey, schema removal, TDD red-then-green regression test, review fixes).

Verification Results

  • Functional QA (diff-aware, /gstack-qa): 3/3 contracts pass, verdict pass, 0 open items. Registry suite 5/5; legacy-config contract eval 8/8 checks true; UI engine-page suites 22/22.
  • Verification gate, four lanes all FRESH (exit 0) on the committed tree: tests-node-engine (registry + engine toggles + engines route + compression API + lite + aggressive + db), vitest-ui-engine, npm run typecheck:core, eslint with suppressions. The full tests/unit/compression/ directory also ran 1661/1661 during review.
  • npm run i18n:check-ui-coverage PASS (100% in all 66 locales after the key removal); tests/unit/dashboard-localization-contract.test.ts 14/14.
  • npm run i18n:check fails on docs/architecture/QUALITY_GATES.md drift: pre-existing on base (the file is not in this diff).

Documentation

Docs audit executed: no document describes the per-engine checkbox; the only preserveSystemPrompt mention in docs (COMPRESSION_ENGINES.md, omniglyph profile section) describes the global/per-step flag this PR does not touch. check:docs-all passes on identical doc bytes (the branch touches no doc files). No doc edits needed.

Reviewer Notes

  • Behavior change is UI-only: three checkboxes disappear. Runtime compression behavior is byte-identical before and after (both adversarial passes verified the apply sites already overrode any per-engine value with the global flag).
  • The one dangerous change (stripping the key from the strict Zod step-config schemas, which would 400 stored pipelines) was deliberately avoided; the schemas keep the optional slot.
  • Follow-up candidates for a future PR: prune the dead key from stored raw rows; align step-level preserveSystemPrompt semantics with omniglyph; widen the schema-absence test to the full engine registry.

…ckbox

The Lite, Aggressive and Ultra engine pages rendered a "Preserve system
prompt" checkbox that never took effect: every apply site reads the flag
from the global compression settings (options.config.preserveSystemPrompt),
the aggressive/ultra settings normalizers copy only named fields so a
stored per-engine value never round-trips, and the Lite page never sent it
(liteConfigSchema is strict and carries only compressToolResults and
maxToolLength). Unchecking the box saved without error, compression kept
following the global setting, and the checkbox snapped back to checked.

Remove the preserveSystemPrompt field from LITE_SCHEMA, AGGRESSIVE_SCHEMA
and ULTRA_SCHEMA so the engine config UI stops surfacing it; the global
"Preserve system prompt" control in the compression panel remains the
real switch. The engine validateConfig checks and the strict Zod
step-config schemas still accept the key so stored combo step configs
that carry it keep validating and PUT round-trips don't 400.

Also drop the unused ok() helper from cavemanAdapter.ts and its
no-unused-vars suppression entry (the file's only lint suppression).
Review-pass findings applied on top of the checkbox removal:

- The regression test now pins the retained validateConfig back-compat
  (aggressive/ultra accept a boolean preserveSystemPrompt and reject a
  non-boolean), so a future cleanup cannot silently drop that acceptance,
  and its comment now states accurately that only the aggressive/ultra Zod
  step-config schemas keep the optional slot (lite's strict liteConfigSchema
  never had one).
- Delete the orphaned compressionEngineConfig.fields.preserveSystemPrompt
  label from all 67 locale files. No schema exposes the field anymore, so
  the dynamic fields.<key> lookup in EngineConfigPage can never reach it.
  Removal is surgical text deletion verified per file by JSON parse and
  deep-compare against base minus exactly that key; i18n UI coverage and
  the localization contract suite stay green.
@woodsonl
woodsonl requested a review from diegosouzapw as a code owner October 5, 2026 23:31
@diegosouzapw
diegosouzapw merged commit 5a82a1c into diegosouzapw:release/v3.8.52 Oct 6, 2026
43 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