Repository navigation
fix(dashboard): treat an emptied engine number field as unset - #15612
Merged
diegosouzapw merged 19 commits intoOct 6, 2026
Merged
diegosouzapw merged 19 commits into
diegosouzapw merged 19 commits into
Conversation
EngineConfigPage seeds its form once at load and PUTs the whole engine sub-object from that copy. The server replaces the sub-object row, so a save writes back every field the page loaded, including ones it does not show. The new tests reproduce four results of that: - Aggressive: thresholds, tool strategies, and the summarizer switch that the settings tab saved after the page loaded go back to the page's copy. - Ultra, default install: every save returns 400. The form sends modelPath "" and the route requires a non-empty string. - Ultra: a save drops ultra.enabled, because the page strips it from the body and the server replaces the row. - Ultra: emptying the model path returns 400; it should clear the path. Two more cases pin the fixed behavior: a save whose fresh read fails writes nothing, and a second save does not resend the first one's field. The UI test runs the page against a fake route that applies the real update schema. The node test runs the same flows through the real settings module and imports the save helper the fix adds, so it fails on this commit with a missing-module error.
The Ultra page's Preview button sends the whole form as config.ultra, including the empty model path the form shows by default. The preview route checks that config against the same strict schema as a save, so the preview returns 400 and the page shows "Preview failed." on every install that has no model path set. The UI test clicks Preview on a default install against a fake route that applies the preview schema. The node test checks a preview config built from the seeded form with that schema; it needs the helper the fix adds.
EngineConfigPage read the settings once at load and PUT the whole engine sub-object from that copy on every save. The server replaces the row, so a save wrote back every field the page loaded: - On the Aggressive page, thresholds, tool strategies, and the summarizer switch saved from the settings tab after the page loaded went back to the page's copy. - On the Ultra page, the form's empty model path failed the route's non-empty check, so every save on a default install returned 400. Once a path was set, a save dropped ultra.enabled, which the page strips from the body. A save now reads the stored sub-object again and applies only the fields edited since the page loaded or last saved. enabled is never taken from the form, and an emptied text field removes its key, so clearing the model path clears it. If that read fails, the save fails and writes nothing. Lite keeps its own body, since its server write already merges with the stored row. Preview had the same empty model path problem: the preview route checks config against the same schema, so an Ultra preview returned 400 on a default install. Preview now drops emptied text fields. The seeding and body logic moves to engineConfigSave.ts so node:test can cover it next to the UI test.
Build the form defaults with Object.fromEntries, and use configState directly in handleSave in place of a local alias.
After a save, the engine page kept showing the values it loaded, so a field another page changed could not be set back to its old value. A save whose response was lost left the baseline unchanged, so rolling that field back was never sent. The lite branch wrote back its load-time switch. A failed settings read at load left the form showing defaults as if they were saved. The UI tests reproduce each case through the page. The node tests drive the save helpers against the real settings store, so release PRs gate on them.
After a successful save the page re-seeds the form and its baseline from the PUT response, keeping any edit typed while the request was out. After a failed save the baseline drops the fields that save carried, since the server may have applied it, so the next save sends them again. Lite builds its body the same way as the other engines: the row stored at save time plus the fields edited since the last save. The cap is checked against its range before anything is sent, and a cleared cap sends null. When the settings read fails at load, the page shows the load error and keeps Save off, because the form would otherwise show defaults as saved values.
…ne-config-page-partial-save
A rejected PUT must keep the edit for the next save, a PUT the server applied before answering 500 must be sent again, a settings read that fails after a reload must not disable the reloaded page, and a keystroke landing as a save finishes must not revert the other fields. The form tests now expect per-field change events, and the node suite round-trips every engine sub-object through the update schema.
The form used to hand the page a whole rebuilt object spread from its props, so a keystroke landing while a save finished could revert fields to load-time values and the next save wrote them back. It now reports single field changes and the page applies them to its latest state. A failed read of the stored settings now gets its own message instead of the engine-information one, is set inside the load's cancelled guard so a superseded load cannot disable Save, and the save-time re-read tolerates a rejected GET like the load-time one already did.
…-field placeholder
3 tasks done
diegosouzapw
merged commit Oct 6, 2026
6a7361a
into
diegosouzapw:release/v3.8.52
37 of 51 checks passed
This was referenced Oct 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Engine config pages turned an emptied number input into
0(Number("")), andvalue[f.key] ?? f.defaultValuewrote that 0 back into the field. For fields whose schema accepts 0 the save silently changed compression:aggressive.minSavingsThreshold: 0means the downgrade almost never triggers,ultra.compressionRate: 0prunes every low-score token,ultra.minScoreThreshold: 0prunes nothing. For fields with a floor (aggressive.maxTokensPerMessagemin 256,headroom.minRowsmin 2,sessionDedup.minBlockCharsmin 1,ccr.minCharsmin 100,ccr.retrievalRampFactormin 1) the whole PUT returned 400 and the page showed a generic save error; the preview request failed the same way.Now an emptied number input means "not set" on every field:
NaNand shows an empty input, with the schema default as placeholder so the blank state reads as "default applies".buildEngineDetailUpdateleavesNaNout of the PUT body, so whole-row engines drop the stored value; lite keeps its NaN-to-null path so a cleared cap still clears the stored cap.withoutEmptyTextdropsNaNfrom preview configs.NaNis the unset sentinel. Overflow input (1e999becomesInfinity) stays in the body and preview config so the settings schema rejects it visibly, and lite's cap range guard skips onlyNaN, so an overflow cap fails the range check instead of silently clearing the stored cap.badInputentry (1e, or1,5in a comma-decimal locale) arrives as an empty value with the validity flag set; the form keeps the last valid value instead of mapping it to the unset sentinel.Number.isNaNinstead ofNumber.isFinite, so overflowminRowsreaches the preview schema and fails visibly, matching the save path.Coordination
Built on #15595 (
662a92eefa), which shipsengineConfigSave.tsand the per-fieldonChangethis fix extends. Rebase ontorelease/v3.8.52once #15595 merges; until then the GitHub diff includes #15595's commits, and the real delta is662a92eefa..HEAD.Related Issues
Validation
04c3f0d7ef: 4 failed / 2 guard-passed (vitest) and 3 failed (node:test).eb9bca688c: 3 + 2 red before the NaN-only fix.7010d5f957: badInput test red before the guard.npm run typecheck:coreclean,eslintexit 0 on changed pathsclears a cap emptied while a save was in flightpins the lost-update invariant againstformAfterSave; the overflow tests flip if the sentinel widens back to!Number.isFiniteTests Added Or Updated
tests/unit/ui/engine-config-page-empty-number.test.tsx(12 tests): emptied field saves unset and stays empty; zero-rejecting field saves 200 instead of 400; headroom preview both arms; lite cap clear; typed 0 survives; overflow cap rejected with no PUT; failed-save retry; post-save display pin; badInput keeps stored value; cap cleared during an in-flight savetests/unit/compression/engine-config-empty-number.test.ts(8 tests): NaN omitted from the body; stored value dropped; typed 0 kept; lite NaN-to-null; overflow kept in body, lite body and preview config; emptied numbers dropped from previewCoverage Notes
New branches in
engineConfigSave.ts(NaN loop arm, lite cap mapping,withoutEmptyText),EngineConfigForm(badInput guard, placeholder, render) andEngineConfigPage(cap range guard, headroom preview branch) are all exercised by the suites above. The 60% aggregate gate runs in CI.Reviewer Notes
release/v3.8.52is red; the inherited failures are tracked there and are not caused by this branch.Infinity-as-unset conflation (typing1e999silently deleted stored config), untested preview/save branches, an unparseable-entry unset. Declined with reasons: shared-predicate extraction (net 0 lines),withoutEmptyTextrename (churns fix(dashboard): engine pages save only the fields that changed #15595's files while this branch must rebase onto it)..gstack/qa-reports/run-20261006T004008Z).compressionPipelineModel.tsshows a prettier-only reflow with no semantic change; a directory-scoped prettier run reformatted it and lint-staged re-applies the same formatting, so a revert will not stick.