Skip to content

fix(dashboard): preserve malformed legacy reasoning_effort on unrelated edits - #79486

Open
dbc-hbin wants to merge 2 commits into
NousResearch:mainfrom
dbc-hbin:fix/cron-preserve-malformed-reasoning
Open

fix(dashboard): preserve malformed legacy reasoning_effort on unrelated edits#79486
dbc-hbin wants to merge 2 commits into
NousResearch:mainfrom
dbc-hbin:fix/cron-preserve-malformed-reasoning

Conversation

@dbc-hbin

@dbc-hbin dbc-hbin commented Aug 5, 2026

Copy link
Copy Markdown

Summary

Fixes the dashboard data-loss bug flagged in the #69911 review: a malformed legacy reasoning_effort stored on a cron job was collapsed to null on hydration, and buildCronJobPayload always emitted the key on update — so any unrelated dashboard edit (for example renaming a job) cleared the stored value.

This branch is #69911's feat/cron-reasoning-effort-consolidation head plus one follow-up fix commit, rebased onto the same main.

Problem

cron/jobs.py::update_job treats reasoning_effort as presence-sensitive: an omitted key leaves the stored override unchanged, while null removes it. The web editor always sent reasoning_effort: null for a malformed legacy value (e.g. " turbo "), so a name-only edit destructively removed the stored value — contradicting the read/preserve compatibility contract.

Fix

  • web/src/lib/cron-job.ts
    • Hydration now keeps a malformed legacy value as distinct editor state: the raw trimmed string stays in reasoning_effort and preserve_reasoning_on_save is set.
    • buildCronJobPayload omits reasoning_effort from the update payload when the selector was not changed on a malformed legacy value, so unrelated edits preserve it.
    • A valid effort is still always emitted; an explicit selector change still emits null (clear) or the chosen value.
  • web/src/pages/CronPage.tsx
    • Changing the reasoning selector clears preserve_reasoning_on_save, so picking "Inherit / default" on a malformed job explicitly clears it.
  • web/src/lib/cron-job.test.ts
    • Replaced the test that codified the destructive null rewrite with a name-only edit regression test asserting reasoning_effort is omitted from the payload, plus a test that an explicit selector change still clears the value.

This mirrors the desktop editor's existing reasoningPreserveOnSave semantics in apps/desktop/src/app/cron/cron-job-model.ts.

Verification

  • Web Vitest: 102 passed (cron-job suite now 12)
  • Web typecheck (tsc -p . --noEmit): passed
  • ESLint on changed files: passed
  • Web production build: passed

dbc-hbin added 2 commits July 23, 2026 14:54
…ed edits

Hydration previously collapsed a malformed legacy reasoning_effort to null,
and buildCronJobPayload always emitted the key, so any unrelated dashboard
edit (e.g. renaming a job) cleared the stored value via the presence-sensitive
backend update.

- keep the raw value as distinct editor state (preserve_reasoning_on_save)
- omit reasoning_effort from update payloads unless the selector changed
- explicit selector changes still emit null (clear) or the valid effort
- replace the destructive-rewrite test with a name-only edit regression test

Mirrors the desktop editor's reasoningPreserveOnSave semantics.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets comp/desktop Electron desktop app (apps/desktop/*) comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 5, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Same note as #69911 (this branch is its head + a fix): the core cron reasoning-effort half landed on main via #91244 (43c6dac) — the dashboard/desktop/web surfacing plus your malformed-legacy-value preservation fix remain unlanded and still wanted. A rebase onto current main scoped to the UI half + the presence-sensitive update fix would be very welcome; build on _normalize_reasoning_effort/_resolve_job_reasoning_config and keep the field off the model-facing tool schema (policy-pinning test exists).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard comp/cron Cron scheduler and job management comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/desktop Electron desktop app (apps/desktop/*) comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants