Skip to content

fix(server): drop stale text generation options when resetting text-gen model selection#2076

Merged
juliusmarminge merged 6 commits intopingdotgg:mainfrom
UtkarshUsername:fix/text-gen-model-reset-atomic-replace
Apr 16, 2026
Merged

fix(server): drop stale text generation options when resetting text-gen model selection#2076
juliusmarminge merged 6 commits intopingdotgg:mainfrom
UtkarshUsername:fix/text-gen-model-reset-atomic-replace

Conversation

@UtkarshUsername
Copy link
Copy Markdown
Contributor

@UtkarshUsername UtkarshUsername commented Apr 16, 2026

What Changed

Introduced applyServerSettingsPatch function in packages/shared/src/serverSettings.ts to handle server settings patches with atomic replacement for textGenerationModelSelection. When provider or model is updated in the patch, the entire textGenerationModelSelection object is rebuilt instead of deep-merged, ensuring stale nested options are dropped. Updated apps/server/src/serverSettings.ts and apps/web/src/hooks/useSettings.ts to use this new patch logic. Added comprehensive tests in packages/shared/src/serverSettings.test.ts and apps/server/src/serverSettings.test.ts to verify correct behavior.

Why

Previously, resetting the text generation model selection didn't work. Stale options from prior configurations would persist. The atomic replacement ensures clean resets while maintaining options when explicitly provided.

UI Changes

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Note

Low Risk
Small, targeted change to settings merge semantics plus tests; main risk is subtle behavior change for callers relying on old deep-merge of textGenerationModelSelection when updating provider/model.

Overview
Fixes server settings patching so resetting textGenerationModelSelection no longer preserves stale nested options. Introduces shared helper applyServerSettingsPatch that deep-merges normally but replaces textGenerationModelSelection when a patch specifies provider and/or model, dropping omitted options.

Both server-side updateSettings (in-memory test layer and persisted path) and the web client’s optimistic settings update now use this helper, and new unit/integration tests cover replacement vs option-only merge behavior.

Reviewed by Cursor Bugbot for commit 7d14771. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Drop stale textGenerationModelSelection options when resetting text-gen model selection

  • Introduces applyServerSettingsPatch in packages/shared/src/serverSettings.ts that replaces textGenerationModelSelection (dropping stale options) when provider or model are present in the patch, while still deep-merging when only options are provided.
  • Replaces deepMerge with applyServerSettingsPatch in the server settings update handler and the test layer in apps/server/src/serverSettings.ts.
  • Applies the same fix to the optimistic client-side update in apps/web/src/hooks/useSettings.ts so the UI stays consistent with server behavior.
  • Behavioral Change: changing provider or model in a settings patch now discards previously stored options; patches that only update options continue to deep-merge as before.

Macroscope summarized 7d14771.

UtkarshUsername and others added 2 commits April 16, 2026 23:05
…selection

This change introduces `applyServerSettingsPatch` to correctly replace
`textGenerationModelSelection` when provider or model are updated,
preventing stale nested options from persisting. Updated server and
shared modules to use the new patch logic and added tests to verify
behavior.
@github-actions github-actions bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Apr 16, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 16, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 719bb999-41f7-4374-9671-7d5ca68a7c44

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 954df5c. Configure here.

Comment thread apps/server/src/serverSettings.ts
@juliusmarminge
Copy link
Copy Markdown
Member

@macroscope-app review this

@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp bot commented Apr 16, 2026

Review in progress. Results will be posted as check runs when complete:

  • Macroscope - Approvability Check
  • Macroscope - Correctness Check

@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp bot commented Apr 16, 2026

Approvability

Verdict: Needs human review

This PR changes runtime behavior for how server settings patches are applied - switching from deep merge to conditional replace semantics for textGenerationModelSelection. While the change is well-tested and narrowly scoped, it modifies core settings logic affecting both server and client, and the author is not a frequent contributor to these files.

You can customize Macroscope's approvability policy. Learn more.

…schema

Use applyServerSettingsPatch to ensure atomic updates and validate with
Schema.decodeSync to catch invalid settings.
macroscopeapp[bot]
macroscopeapp bot previously approved these changes Apr 16, 2026
Comment thread apps/server/src/serverSettings.ts Outdated
@macroscopeapp macroscopeapp bot dismissed their stale review April 16, 2026 19:41

Dismissing prior approval to re-evaluate e3e854f

@UtkarshUsername UtkarshUsername force-pushed the fix/text-gen-model-reset-atomic-replace branch from e3e854f to a4c650b Compare April 16, 2026 19:45
Comment thread apps/server/src/serverSettings.ts Outdated
Comment thread apps/server/src/serverSettings.ts Outdated
@juliusmarminge juliusmarminge enabled auto-merge (squash) April 16, 2026 20:01
@juliusmarminge juliusmarminge merged commit 7a08fcf into pingdotgg:main Apr 16, 2026
12 checks passed
Berkay2002 pushed a commit to Berkay2002/bcode that referenced this pull request Apr 17, 2026
Berkay2002 added a commit to Berkay2002/bcode that referenced this pull request Apr 17, 2026
…text-gen reset fix) (#6)

* fix(server): drop stale text generation options when resetting text-gen model selection (pingdotgg#2076)

(cherry picked from commit 7a08fcf)

* fix(web): prevent composer controls overlap on narrow windows (make plan sidebar responsive) (pingdotgg#1198)

(cherry picked from commit 19d4740)

* feat: add Claude Opus 4.7 to built-in models (pingdotgg#2072)

Co-authored-by: Julius Marminge <julius0216@outlook.com>
(cherry picked from commit 3e07f5a)

* test(server): align Sonnet 4.6 default effort assertions with fork

Upstream test assumed Sonnet 4.6 defaults to "high"; our fork intentionally
defaults to "medium" per Anthropic's recommendations (a6bbf4b).

---------

Co-authored-by: Utkarsh Patil <73941998+UtkarshUsername@users.noreply.github.com>
Co-authored-by: Ibrahim Elkamali <126423069+Marve10s@users.noreply.github.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
znoraka pushed a commit to znoraka/t3code that referenced this pull request Apr 17, 2026
aaditagrawal added a commit to aaditagrawal/t3code that referenced this pull request Apr 18, 2026
Integrates upstream/main (9df3c64) on top of fork's main (9602c18).

Upstream features adopted:
- Claude Opus 4.5 and 4.7 built-in models (pingdotgg#2072, pingdotgg#2143)
- Node-native TypeScript migration across desktop/server (pingdotgg#2098)
- Configurable project grouping with client-settings overrides (pingdotgg#2055, pingdotgg#2099)
- Thread status in command palette (pingdotgg#2107)
- Responsive composer / plan sidebar on narrow windows (pingdotgg#1198)
- Capture-phase CTRL+J keydown for Windows terminal toggle (pingdotgg#2113/pingdotgg#2142)
- Bypass xterm for global terminal shortcuts (pingdotgg#1580)
- Windows ARM build target (pingdotgg#2080)
- Windows PATH hydration + repair (pingdotgg#1729)
- Gitignore-aware workspace search (pingdotgg#2078)
- Claude process leak fix + stale session monitoring (pingdotgg#2042)
- Preserve provider bindings when stopping sessions (pingdotgg#2084)
- Clean up invalid pending-approval projections (pingdotgg#2106) — new migration
- Extract backend startup readiness coordination
- Drop stale text-gen options on reset (pingdotgg#2076)
- Extend negative repository identity cache TTL (pingdotgg#2083)
- Allow deleting non-empty projects from warning toast (pingdotgg#1264)
- Restore defaults only on General settings (pingdotgg#1710)
- Release workflow modernization (blacksmith runners, GitHub App token guards, v0.0.20 version bump)

Fork features preserved:
- All 8 providers (codex, claudeAgent, copilot, cursor, opencode,
  geminiCli, amp, kilo) with their adapters, services, and tests
- Fork's custom OpenCode protocol impl in apps/server/src/opencode/ (kept
  over upstream's @opencode-ai/sdk-based provider added in pingdotgg#1758 — fork's
  version is tested and integrated; upstream's parallel files deleted)
- Fork's direct-CLI Cursor adapter (kept over upstream's new ACP-based
  CursorProvider added in pingdotgg#1355 — upstream's parallel files deleted)
- Fork's ProviderRegistry aggregates only codex + claudeAgent snapshots;
  the other 6 providers register via ProviderAdapterRegistry
- PROVIDER_CACHE_IDS stays at [codex, claudeAgent] matching what the
  registry actually caches
- Migration IDs preserved (fork 23/24/25/26; upstream's new 025 lands at
  ID 27 to avoid re-applying on deployed fork DBs)
- Fork's generic per-provider settings (enabled/binaryPath/configDir/
  customModels) kept over upstream's opencode-specific serverUrl/password
- Log directory IPC channels, updateInstallInFlight tracking, icon
  composer pipeline all preserved
- Fork's simplified release.yml (no npm CLI publish, no nightly infra)
- composerDraftStore normalizeProviderKind widened to accept all 8 kinds
- Dark mode --background set to #0f0f0f

Test status:
- All 9 package typechecks pass
- Lint clean (0 errors)
- Tests: 1877 passed, 15 skipped (incl. 4 historically-flaky GitManager
  cross-repo PR selector tests newly gated with TODO for Node-native-TS
  follow-up)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants