Conversation
Contributor
📝 WalkthroughWalkthroughThis PR fixes a bug preventing auto-compaction settings from being saved by introducing proper numeric input normalization with min/max bounds, type changes, and enhanced UI with a sticky header component. Test coverage for the new normalization logic and header behavior is included. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/renderer/settings/components/DeepChatAgentsSettings.vue (1)
1087-1092: Normalize the loaded compaction values with the same helpers.
fromAgentstill passes these fields throughnumText, so persisted out-of-range values render raw and stringified numbers render blank, then get reset on the next save. Reusing the save-path normalizers here keeps load/save behavior symmetric.♻️ Proposed fix
- autoCompactionTriggerThreshold: numText( - config.autoCompactionTriggerThreshold ?? AUTO_COMPACTION_TRIGGER_THRESHOLD_DEFAULT - ), - autoCompactionRetainRecentPairs: numText( - config.autoCompactionRetainRecentPairs ?? AUTO_COMPACTION_RETAIN_RECENT_PAIRS_DEFAULT - ) + autoCompactionTriggerThreshold: String( + normalizeAutoCompactionTriggerThreshold(config.autoCompactionTriggerThreshold) + ), + autoCompactionRetainRecentPairs: String( + normalizeAutoCompactionRetainRecentPairs(config.autoCompactionRetainRecentPairs) + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/settings/components/DeepChatAgentsSettings.vue` around lines 1087 - 1092, The loaded compaction fields are being converted with numText which differs from the save-path normalizers and causes persisted out-of-range or stringified numbers to render incorrectly; update the initialization of autoCompactionTriggerThreshold and autoCompactionRetainRecentPairs to use the same helper functions used when saving (the save-path normalizers) instead of numText so load/save behavior is symmetric — locate where fromAgent and the save-path normalizers are defined and replace the numText conversion for these two fields with those exact normalize functions that the save path uses.test/renderer/components/DeepChatAgentsSettings.test.ts (1)
357-483: Please pin the actual#1425regression path in the test suite.These cases validate coercion, but they never flip
autoCompactionEnabledbefore saving, and they also skip the new clamp/round branches. One test that toggles the switch and saves, plus one out-of-range/decimal input case, would cover the paths most likely to regress.Also applies to: 485-611
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/renderer/components/DeepChatAgentsSettings.test.ts` around lines 357 - 483, Add two tests in DeepChatAgentsSettings.test.ts to cover the regression: one that flips the autoCompactionEnabled switch then saves to ensure updateDeepChatAgent is called with config.autoCompactionEnabled toggled (use the component DeepChatAgentsSettings and the switch selector '[data-testid="auto-compaction-enabled-switch"]'), and one that inputs out-of-range/decimal values into '[data-testid="auto-compaction-trigger-threshold-input"]' and '[data-testid="auto-compaction-retain-recent-pairs-input"]' (e.g., negative or >max and a decimal) then saves and assert config.autoCompactionTriggerThreshold is clamped/rounded and config.autoCompactionRetainRecentPairs is clamped/rounded in the payload passed to configPresenter.updateDeepChatAgent; reuse the existing mocking pattern for configPresenter and assert updateDeepChatAgent mock.calls payloads accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/settings/components/DeepChatAgentsSettings.vue`:
- Around line 1087-1092: The loaded compaction fields are being converted with
numText which differs from the save-path normalizers and causes persisted
out-of-range or stringified numbers to render incorrectly; update the
initialization of autoCompactionTriggerThreshold and
autoCompactionRetainRecentPairs to use the same helper functions used when
saving (the save-path normalizers) instead of numText so load/save behavior is
symmetric — locate where fromAgent and the save-path normalizers are defined and
replace the numText conversion for these two fields with those exact normalize
functions that the save path uses.
In `@test/renderer/components/DeepChatAgentsSettings.test.ts`:
- Around line 357-483: Add two tests in DeepChatAgentsSettings.test.ts to cover
the regression: one that flips the autoCompactionEnabled switch then saves to
ensure updateDeepChatAgent is called with config.autoCompactionEnabled toggled
(use the component DeepChatAgentsSettings and the switch selector
'[data-testid="auto-compaction-enabled-switch"]'), and one that inputs
out-of-range/decimal values into
'[data-testid="auto-compaction-trigger-threshold-input"]' and
'[data-testid="auto-compaction-retain-recent-pairs-input"]' (e.g., negative or
>max and a decimal) then saves and assert config.autoCompactionTriggerThreshold
is clamped/rounded and config.autoCompactionRetainRecentPairs is clamped/rounded
in the payload passed to configPresenter.updateDeepChatAgent; reuse the existing
mocking pattern for configPresenter and assert updateDeepChatAgent mock.calls
payloads accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1dbf2000-d39e-4e13-bbd7-a1528be192e1
📒 Files selected for processing (2)
src/renderer/settings/components/DeepChatAgentsSettings.vuetest/renderer/components/DeepChatAgentsSettings.test.ts
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
1 participant
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.
fixed #1425
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes