[codex] Fix sidebar preview settings reset#2587
Merged
Merged
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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
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
This follow-up addresses two useful review findings left on the already-merged sidebar thread preview setting PR (#1856).
The visible-thread setting was included in the restore-defaults confirmation copy, but it was not part of the payload sent to
updateSettings. That meant a user could confirm a reset that explicitly listed "Visible threads" while the saved sidebar preview count stayed unchanged. The fix addssidebarThreadPreviewCountto the restore-defaults patch alongside the other client settings.The new setting was also present in
ClientSettingsSchemabut absent fromClientSettingsPatch. That left the patch contract inconsistent with the settings schema and would drop the field if the patch schema were used for decoding or validation. The fix adds the optional patch entry using the existingSidebarThreadPreviewCountschema.I checked the Vite-related review thread too. Current
mainno longer has the startup-crash fallback change, and the remaining Vite comment is about an unrelated build-contract decision, so this PR stays scoped to the two still-actionable settings issues.Validation
bun fmtbun lintbun typecheckbun run test --filter=@t3tools/contracts --filter=@t3tools/webnpx -y react-doctor@latest . --verbose --diffNote
Fix sidebar preview settings reset to exclude
sidebarThreadPreviewCountThe
useSettingsRestorehook was not resettingsidebarThreadPreviewCount(Visible threads) when restoring defaults. This fix adds the field to the reset payload and extends theClientSettingsPatchschema in settings.ts to accept it as an optional field.Macroscope summarized 8897a78.
Note
Low Risk
Low risk bugfix limited to client settings reset behavior and the settings patch contract; no auth/security or complex logic changes.
Overview
Fixes a mismatch where restoring default settings did not actually reset
sidebarThreadPreviewCount("Visible threads") despite being listed in the confirmation prompt.Also updates the contracts layer so
ClientSettingsPatchacceptssidebarThreadPreviewCount, keeping the patch schema consistent withClientSettingsSchemaand preventing the field from being dropped during validation/decoding.Reviewed by Cursor Bugbot for commit 8897a78. Bugbot is set up for automated code reviews on this repo. Configure here.