Fix system prompt clearing error by using delete API for empty prompts #272
Conversation
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
Resolves #270 where clearing the system prompt in User Preferences would fail. Now properly calls deleteInstruction() when prompt is empty instead of attempting to update with an empty string. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Deploying maple with
|
| Latest commit: |
6c88ad8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3e360403.maple-ca8.pages.dev |
| Branch Preview URL: | https://claude-issue-270-20250113-15.maple-ca8.pages.dev |
WalkthroughThe pull request makes a minor JSX tweak in ComparisonChart and updates PreferencesDialog save logic to handle empty prompts: delete existing instructions when cleared, prevent creating new instructions with empty prompts, and update or create instructions only when the prompt is non-empty. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant PD as PreferencesDialog
participant OS as os (instructions API)
U->>PD: Click Save
alt instructionId exists
alt prompt.trim() is empty
PD->>OS: deleteInstruction(instructionId)
OS-->>PD: OK
PD->>PD: Clear instructionId and prompt
else prompt.trim() non-empty
PD->>OS: updateInstruction(instructionId, prompt)
OS-->>PD: OK
end
else no instructionId
alt prompt.trim() non-empty
PD->>OS: createInstruction(prompt)
OS-->>PD: instructionId
PD->>PD: Set instructionId
else prompt.trim() empty
PD->>PD: No-op (no create)
end
end
PD-->>U: Save complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Greptile Overview
Summary
Fixed issue #270 where clearing the system prompt in User Preferences would fail with "Failed to save user preferences" error.
Key Changes:
- When an existing instruction exists and the prompt is cleared (empty string), now calls
deleteInstruction()instead of attemptingupdateInstruction()with empty prompt - Added validation to prevent creating new instructions with empty prompts
- Properly resets local state (
instructionIdandprompt) after deletion - Minor formatting cleanup in ComparisonChart component (non-functional)
The fix correctly handles all three scenarios: creating new prompts, updating existing prompts, and deleting prompts by clearing them.
Confidence Score: 5/5
- This PR is safe to merge with no risks - it's a clean, focused bug fix with proper state management
- The fix directly addresses the reported issue with a straightforward solution. The logic properly handles all three cases (create, update, delete) with appropriate validation using trim() to check for empty strings. State is correctly reset after deletion. The changes are minimal, well-contained, and follow React best practices.
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| frontend/src/components/PreferencesDialog.tsx | 5/5 | Fixed system prompt clearing by calling deleteInstruction() for empty prompts, preventing save failures |
| frontend/src/components/ComparisonChart.tsx | 5/5 | Minor formatting cleanup - removed empty lines and trailing spaces (non-functional change) |
Sequence Diagram
sequenceDiagram
participant User
participant PreferencesDialog
participant OpenSecretAPI
participant Database
User->>PreferencesDialog: Opens User Preferences
PreferencesDialog->>OpenSecretAPI: listInstructions()
OpenSecretAPI->>Database: Query instructions
Database-->>OpenSecretAPI: Return instructions
OpenSecretAPI-->>PreferencesDialog: Default instruction found
PreferencesDialog->>PreferencesDialog: Set instructionId & prompt
alt User clears system prompt (empty)
User->>PreferencesDialog: Clear text & Save
PreferencesDialog->>PreferencesDialog: Check if prompt.trim() === ""
PreferencesDialog->>OpenSecretAPI: deleteInstruction(instructionId)
OpenSecretAPI->>Database: Delete instruction
Database-->>OpenSecretAPI: Success
OpenSecretAPI-->>PreferencesDialog: Deleted
PreferencesDialog->>PreferencesDialog: setInstructionId(null), setPrompt("")
PreferencesDialog->>User: Success message
else User updates system prompt (non-empty)
User->>PreferencesDialog: Edit text & Save
PreferencesDialog->>OpenSecretAPI: updateInstruction(instructionId, {prompt})
OpenSecretAPI->>Database: Update instruction
Database-->>OpenSecretAPI: Success
OpenSecretAPI-->>PreferencesDialog: Updated
PreferencesDialog->>User: Success message
else User creates new prompt (no existing)
User->>PreferencesDialog: Enter text & Save
PreferencesDialog->>OpenSecretAPI: createInstruction({name, prompt, is_default})
OpenSecretAPI->>Database: Create instruction
Database-->>OpenSecretAPI: New instruction
OpenSecretAPI-->>PreferencesDialog: New instructionId
PreferencesDialog->>PreferencesDialog: setInstructionId(newId)
PreferencesDialog->>User: Success message
end
2 files reviewed, no comments
This was referenced Oct 14, 2025
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.
Resolves #270 where clearing the system prompt in User Preferences would fail. Now properly calls deleteInstruction() when prompt is empty instead of attempting to update with an empty string.
Summary by CodeRabbit
Bug Fixes
Refactor