Skip to content

fix(compression): show a retry when settings fail to load - #15346

Merged
diegosouzapw merged 5 commits into
diegosouzapw:release/v3.8.52from
woodsonl:fix/compression-panel-load-failure
Oct 6, 2026
Merged

diegosouzapw merged 5 commits into
diegosouzapw:release/v3.8.52from
woodsonl:fix/compression-panel-load-failure

Conversation

@woodsonl

@woodsonl woodsonl commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • When GET /api/settings/compression fails (a 5xx, or a network error during a server restart), CompressionPanel (/dashboard/context/settings) and CompressionSettingsTab (embedded in /dashboard/context/caveman) now show "Prompt Compression: Failed To Load" and a Retry button in place of their controls. Retry shows the loading state, re-runs the component's loads, and the controls come back once a GET succeeds. Both load effects return a cleanup, so answers that arrive for a run a retry replaced are ignored.
  • Before this change, .catch(() => {}) swallowed the failure and .finally(() => setLoading(false)) rendered the editable controls on DEFAULT_CONFIG (engines: {}, outputStyles: [], the default contextBudget), and lastConfirmedRef kept those defaults. The output-style toggles and the context-budget dial are live at once, so the next click overwrote the stored outputStyles or contextBudget with the defaults plus one change. Turning on the master switch (shown off) unlocked the engine toggles. One engine toggle then PUT a one-entry engines map, which sanitizeEnginesForWrite (src/lib/db/compression.ts) stored with enginesExplicit: true, so dispatch followed that one-entry map. The tab's defaults have compression off, so it hid its editable sections and reported every layer as off.
  • The derived-pipeline text moves into a derivedPreviewText helper. That keeps CompressionPanel at the complexity cap of 15 that the if (loadFailed) return would otherwise exceed.
  • The message reuses settings.compressionTitle, common.failedToLoad and settings.retry, which all 67 locales already carry, so no catalog changes are needed. Its text uses text-red-600 dark:text-red-400, the repo's most common error-text pair. Computed from Tailwind's OKLCH values against the card surface, that is 4.8:1 in the light theme (#ffffff) and 6.0:1 in the dark theme (#161b22), above the WCAG AA minimum of 4.5:1. The previous text-red-500 measures 3.8:1 in the light theme.
  • CompressionSettingsTab already imported Button without using it. The retry button now uses it, so the tab's frozen @typescript-eslint/no-unused-vars entry comes out of config/quality/eslint-suppressions.json (lint-staged fails on an unused suppression).

Related Issues

  • No issue filed. Coordinates with the open branches fix/compression-panel-partial-save and fix/compression-engines-partial-write (see Reviewer Notes).

Validation

  • Change type: UI
  • Red first: commit 615f5f5 adds the first tests alone. On it the 3 new cases fail and the 11 existing cases in those files pass. With the settings GET answering 500, the switch sweep records 18 settings PUTs: { enabled: true }, engines maps that start one-entry ({ "session-dedup": { enabled: true } }), one-entry outputStyles lists, and liveZone. Commit 2104355 adds the stale-load cases. On it they fail (the panel's MCP toggle and the tab's rule list take the superseded load's values), and the 17 other cases pass.
  • Green: npx vitest run --config vitest.config.ts on the 9 files that render either component passes 50/50, including the caveman page that embeds the tab.
  • Mutation check: six mutants, each caught by exactly its intended test. In each component, they are: Retry clearing the error without showing loading, the thrown-fetch catch swallowing the error, and the effect without its cleanup.
  • npm run lint: ESLint on the 4 changed source and test files (--suppressions-location config/quality/eslint-suppressions.json --pass-on-unpruned-suppressions) is clean. The full lint runs in the CI lint job.
  • node scripts/check/check-complexity-ratchets.mjs --base-ref 4db3c54ec1: OK, 3 violations in the touched files against 3 at the base. npm run check:dashboard-typecheck: OK, within the frozen baseline. npx prettier --check on the changed files is clean.
  • Reconciled with the active release base: cut from release/v3.8.52 at 228d72d. git merge-tree against the current tip is clean, and none of the files changed here moved on the base.
  • Production-code changes include new automated tests in this PR.

Tests Added Or Updated

  • tests/unit/ui/compressionPanel.test.tsx, new describe("CompressionPanel when the settings GET fails"):
    • After a 500 and after a thrown fetch, pressing every switch the panel offers sends no settings PUT, no select or input renders, and a Retry is offered.
    • Retry with the retried GET held open shows loading and no controls. Once the GET answers, the stored rtk level loads, and the master switch saves { enabled: false }.
    • When the first load's mcp-accessibility GET answers after a retry finished, the toggle keeps the retried value.
  • tests/unit/ui/compression-settings-tab-partial-save.test.tsx, new describe("CompressionSettingsTab when the settings GET fails"):
    • After a 500 and after a thrown fetch, the tab shows the error and Retry in place of the all-off default summary.
    • Retry with the retried GET held open shows loading and no form, then the stored, enabled settings.
    • When the first load's rules GET answers after a retry finished, its rules stay out of the list.

Coverage Notes

  • The new cases cover every path added in both components: the 500 and thrown-fetch failures, the error card, Retry and its loading window, and the stale-answer cleanup.
  • These are .tsx vitest UI tests. ci.yml's test-vitest job runs them on this PR (npm run test:vitest:ui). In the CI run on 06fe786, both files ran and passed: compressionPanel.test.tsx (8 tests) and compression-settings-tab-partial-save.test.tsx (12 tests).

Reviewer Notes

  • ⚠️ base-red inherited: 🔴 Release branch not green: release/v3.8.52 #15306
  • CI failures on this PR that come from the base or the runner:
  • The complexity-ratchet failure from the first CI run came from this PR and is fixed in 06fe786.
  • Coordination: fix/compression-panel-partial-save rewrites save() and the refs above it, and fix/compression-engines-partial-write merges engines by id and changes setEngine. Both also edit CompressionPanel.tsx and compressionPanel.test.tsx. This PR leaves save(), those refs, setEngine and setupFetchMock untouched, and adds its tests in a separate describe. git merge-tree against the current heads (e3529ed and 42c7027) reports no conflicts. On each trial merge, the compression UI suites pass: 58/58 with the partial-save branch and 53/53 with the engines branch, including its compressionPanelEnginesPartialWrite.test.tsx.
  • The panel's Retry re-runs all three loads (settings, mcp-accessibility, combos), since a server restart fails them together.

Browser QA

A real-Chromium pass against a local dev server serving 06fe786, with GET /api/settings/compression patched to answer 500:

  • With an unpatched load first, the panel renders its controls. After arming the patch and remounting the panel through client-side navigation, the panel is replaced by a role="alert" card reading "Prompt Compression: Failed To Load" with exactly one Retry button.
  • The card text renders in the darker red on the light theme and the lighter red on the dark one; screenshots of both themes confirm the text is legible on the card.
  • Clearing the patch and pressing Retry re-runs the loads and restores the full control panel; the alert is gone.
  • The Caveman page, which does not read this endpoint, rendered normally during the pass.

When GET /api/settings/compression fails, CompressionPanel renders its
editable controls on DEFAULT_CONFIG. The first switch pressed then PUTs
those defaults over the stored row: a one-entry engines map, a one-entry
outputStyles list, a default liveZone. CompressionSettingsTab swallows
the same failure and reports every compression layer as off.

The new cases answer the first settings GET with a 500. They expect an
error with a retry in place of the controls, no settings PUT until a GET
succeeds, and the stored values once the retry loads. They fail here.
A failed GET /api/settings/compression (a 5xx, or a network error
during a server restart) was swallowed, and both compression views
rendered their editable controls on defaults. CompressionPanel's
confirmed-config ref kept those defaults, so the next output-style
toggle or context-budget change overwrote the stored outputStyles or
contextBudget, and an engine toggle stored a one-entry engines map that
dispatch then followed. CompressionSettingsTab reported every
compression layer as off.

Both now treat a non-OK response or a fetch error as a failed load and
render the error with a Retry button in place of the controls, so no
save can go out until a GET succeeds. Retry re-runs the component's
loads. The message reuses settings.compressionTitle, common.failedToLoad
and settings.retry, which every locale already carries.

The tab already imported Button without using it; the retry button uses
it now, so the tab's frozen no-unused-vars entry leaves
config/quality/eslint-suppressions.json.
@woodsonl
woodsonl requested a review from diegosouzapw as a code owner October 2, 2026 12:53
The load-failure cases now also run with a fetch that throws. The
Retry cases hold the retried GET open and check that loading shows and
no control renders until it answers.

Two new cases answer a request from the first load only after a retry
finished: the panel's mcp-accessibility toggle and the tab's rule list
must keep the retried load's values. They fail here, because the load
effects have no cleanup.
Both load effects now return a cleanup, so after a retry the answers
that arrive for the replaced run are ignored: the panel's
mcp-accessibility and combos loads, and the tab's rules load.

The derived-pipeline text moves into derivedPreviewText. That takes
the decision point the loadFailed return added back out of
CompressionPanel, so the complexity ratchet is at its base count again.

The load-error text uses text-red-600 dark:text-red-400, which meets
the WCAG AA contrast ratio on the card.
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