fix(#689): provider cache reset after settings import#726
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesImported settings replay handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
f8ac8ef to
26dea4e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
edelauna
left a comment
There was a problem hiding this comment.
Thanks for you contribution just had some additional testing to add.
|
/review |
|
@edelauna Thank you for review. I checked both points and pushed a follow-up commit with the requested updates. The test now waits for the post-rerender effect before asserting the replayed timestamp case, and I added an additional case for a fresh |
edelauna
left a comment
There was a problem hiding this comment.
Look good - thanks for your contribution
SettingsView keeps a local cachedState buffer so settings edits do not write through to the live extension state before Save. After importing settings, the import timestamp was used to bust that cache, but the effect could run again for the same import timestamp whenever extensionState changed identity. That allowed a later live state update to overwrite an in-progress provider edit with the previously saved provider, causing the provider UI to briefly switch and then revert. Track the handled settingsImportedAt value and only reset cachedState once per import event. Add a regression test for the imported-settings flow where Baseten is saved with an API key, the same import timestamp is replayed, and a subsequent DeepSeek provider edit is preserved and saved.
Strengthen SettingsView regression coverage around settingsImportedAt. Verify that replaying the same import timestamp does not let the cache-busting effect overwrite an in-progress provider edit, and add the inverse case to prove a fresh import timestamp still refreshes cachedState from the latest extensionState apiConfiguration. Also assert that a fresh import clears the dirty state by disabling the Save button after pending edits are replaced by imported settings.
ac5d042 to
9198817
Compare
…o-Code-Org#726) [imported from upstream 63dec51]
Related GitHub Issue
Closes: #689
Description
SettingsView uses a local cachedState buffer so settings edits do not write through to live extension state before Save. After importing settings, the cache-busting effect could run multiple times for the same settingsImportedAt value whenever extensionState changed identity.
That could overwrite an in-progress provider edit with the previously saved provider, making the UI briefly switch and then revert.
This change tracks the handled settingsImportedAt value and resets cachedState only once per import event. It also adds a regression test covering the imported-settings flow where Baseten is saved with an API key, the same import timestamp replays, and a later DeepSeek provider edit is preserved and saved.
Test Procedure
Ran the focused webview regression test: - all 3 tests passed.
pnpm --dir webview-ui exec vitest run src/components/settings/__tests__/SettingsView.change-detection.spec.tsxVerified the new regression coverage for the imported-settings flow:
This verifies that SettingsView only busts cachedState once per settingsImportedAt value, so a replayed imported state does not overwrite an in-progress Provider edit.
Pre-Submission Checklist
Screenshots / Videos
Documentation Updates
Additional Notes
Get in Touch
Summary by CodeRabbit
Bug Fixes
Tests
settingsImportedAtand verifying correct provider switching (e.g., Baseten to DeepSeek) and UI reset/sync behavior.