feat: enhance error dialogs with URL detection and copy functionality#900
feat: enhance error dialogs with URL detection and copy functionality#900
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances error handling throughout the application by replacing standard wx.MessageBox error dialogs with a new enhanced error dialog that detects URLs in error messages and provides additional functionality.
- Introduces
EnhancedErrorDialogwith URL detection, "Open in Browser" button, and "Copy Error" functionality - Replaces all
wx.MessageBoxerror displays with the enhanced dialog across multiple components - Provides context-aware messaging for completion errors vs. other error types
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| basilisk/gui/enhanced_error_dialog.py | New module implementing the enhanced error dialog with URL detection and copy functionality |
| basilisk/gui/ocr_handler.py | Replaces wx.MessageBox with enhanced error dialog for OCR errors |
| basilisk/gui/conversation_tab.py | Updates transcription, title generation, save, and completion error handling to use enhanced dialog |
| basilisk/completion_handler.py | Replaces fallback error dialog with enhanced version |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds a new enhanced error dialog with URL detection, copy and open-in-browser actions, and replaces several wx.MessageBox error calls in completion_handler, conversation_tab, and ocr_handler to use the new dialog while preserving existing control flow and callbacks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant C as Component
participant D as EnhancedErrorDialog
participant B as System Browser
participant CL as Clipboard
U->>C: Trigger operation (completion/ocr/save/etc.)
C->>C: Error occurs
C->>D: show_enhanced_error_dialog(parent, message, title, is_completion_error)
activate D
Note over D: Render message, extract URLs, provide Copy/Open actions
D-->>C: Modal result (OK/closed)
deactivate D
opt User clicks "Copy Error"
D->>CL: Copy error text
CL-->>D: Success/Failure
end
opt User selects URL and clicks "Open in Browser"
D->>B: Open selected URL
B-->>D: Success/Failure
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
basilisk/completion_handler.py(2 hunks)basilisk/gui/conversation_tab.py(5 hunks)basilisk/gui/enhanced_error_dialog.py(1 hunks)basilisk/gui/ocr_handler.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Use tabs (not spaces) for indentation in Python code
Use snake_case for variables/functions, PascalCase for classes, and UPPER_SNAKE_CASE for constants
Write Google-style docstrings with type hints for all public APIs
Use double quotes for strings; wrap translatable text with _("...")
Do not import the translation function _; it is provided as a builtin
Group imports as standard library, third-party, then local; sort each group alphabetically
Mark translatable strings with _("...") and provide preceding translator context comments using '# Translators:' or '# translators:' when helpful
Use provider-specific streaming APIs to display messages in real time
Files:
basilisk/completion_handler.pybasilisk/gui/ocr_handler.pybasilisk/gui/enhanced_error_dialog.pybasilisk/gui/conversation_tab.py
⚙️ CodeRabbit configuration file
**/*.py: ## Indentation
We use tabs, not spaces.naming conventions
- snake_case for variables and functions
- PascalCase for classes
- UPPER_SNAKE_CASE for constants
- snake_case for files and directories
- Use whole words in names when possible
documentation
- Use docstrings for all public classes and functions
- Use type hints for all public classes and functions
- Use google style for docstrings
Strings
- Always use double quotes for strings
- Use percent formatting for string
- Use _("string") for translatable strings
- Use **# translator: ** with context for translatable strings
Files:
basilisk/completion_handler.pybasilisk/gui/ocr_handler.pybasilisk/gui/enhanced_error_dialog.pybasilisk/gui/conversation_tab.py
basilisk/gui/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
basilisk/gui/**/*.py: In wxPython UI code, bind events using self.Bind(wx.EVT_, self.on_) consistently
After any dialog.ShowModal(), always call dialog.Destroy()
Use BoxSizer-based layouts; avoid fixed positioning for responsiveness
Ensure accessibility: provide labels/roles for screen readers and maintain NVDA integration hooks
Files:
basilisk/gui/ocr_handler.pybasilisk/gui/enhanced_error_dialog.pybasilisk/gui/conversation_tab.py
🧬 Code graph analysis (4)
basilisk/completion_handler.py (2)
basilisk/gui/enhanced_error_dialog.py (1)
show_enhanced_error_dialog(38-65)__builtins__.pyi (1)
_(1-1)
basilisk/gui/ocr_handler.py (2)
basilisk/gui/enhanced_error_dialog.py (1)
show_enhanced_error_dialog(38-65)__builtins__.pyi (1)
_(1-1)
basilisk/gui/enhanced_error_dialog.py (1)
__builtins__.pyi (1)
_(1-1)
basilisk/gui/conversation_tab.py (2)
basilisk/gui/enhanced_error_dialog.py (1)
show_enhanced_error_dialog(38-65)__builtins__.pyi (1)
_(1-1)
⏰ 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). (2)
- GitHub Check: build_app / build_windows (x86)
- GitHub Check: build_app / build_windows (x86_64)
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
basilisk/gui/conversation_tab.py (4)
463-468: Replace remaining wx.MessageBox error dialogs with enhanced dialogThese instances violate the PR objective to standardize error dialogs.
@@ - wx.MessageBox( - _("The selected provider does not support speech-to-text"), - _("Error"), - wx.OK | wx.ICON_ERROR, - ) + show_enhanced_error_dialog( + parent=self, + message=_("The selected provider does not support speech-to-text"), + title=_("Error"), + is_completion_error=False, + ) return @@ - wx.MessageBox( - _("The selected provider does not support speech-to-text"), - _("Error"), - wx.OK | wx.ICON_ERROR, - ) + show_enhanced_error_dialog( + parent=self, + message=_("The selected provider does not support speech-to-text"), + title=_("Error"), + is_completion_error=False, + ) return @@ - wx.MessageBox( - _( - "A completion is already in progress. Please wait until it finishes." - ), - _("Error"), - wx.OK | wx.ICON_ERROR, - ) + show_enhanced_error_dialog( + parent=self, + message=_( + "A completion is already in progress. Please wait until it finishes." + ), + title=_("Error"), + is_completion_error=False, + ) returnAs per coding guidelines.
Also applies to: 547-552, 694-700
308-311: Fix default key handling: wx.KeyEvent.Skip is an instance methodCurrent code attempts to call a class method reference; it won’t skip properly.
- shortcut = (event.GetModifiers(), event.GetKeyCode()) - actions = {(wx.MOD_CONTROL, ord('P')): self.on_choose_profile} - action = actions.get(shortcut, wx.KeyEvent.Skip) - action(event) + shortcut = (event.GetModifiers(), event.GetKeyCode()) + actions = {(wx.MOD_CONTROL, ord("P")): self.on_choose_profile} + action = actions.get(shortcut) + if action: + action(event) + else: + event.Skip()
379-388: Do not destroy a menu you didn’t createadd_standard_context_menu_items() destroys the passed-in menu, which is unexpected and can crash callers.
- menu.Destroy() + # Caller is responsible for menu lifetime.
235-267: House style: prefer self.Bind(...) for event hookupsMigrate per-control .Bind calls to self.Bind with source for consistency across basilisk/gui.
- self.submit_btn.Bind(wx.EVT_BUTTON, self.on_submit) + self.Bind(wx.EVT_BUTTON, self.on_submit, self.submit_btn) @@ - self.stop_completion_btn.Bind(wx.EVT_BUTTON, self.on_stop_completion) + self.Bind(wx.EVT_BUTTON, self.on_stop_completion, self.stop_completion_btn) @@ - self.toggle_record_btn.Bind(wx.EVT_BUTTON, self.toggle_recording) + self.Bind(wx.EVT_BUTTON, self.toggle_recording, self.toggle_record_btn) @@ - self.apply_profile_btn.Bind(wx.EVT_BUTTON, self.on_choose_profile) + self.Bind(wx.EVT_BUTTON, self.on_choose_profile, self.apply_profile_btn)Based on learnings.
Also applies to: 240-241, 249-251, 259-260, 266-267
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
basilisk/gui/conversation_tab.py(5 hunks)basilisk/gui/enhanced_error_dialog.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
basilisk/gui/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
basilisk/gui/**/*.py: In wxPython UI code, bind events using self.Bind(wx.EVT_, self.on_) consistently
After any dialog.ShowModal(), always call dialog.Destroy()
Use BoxSizer-based layouts; avoid fixed positioning for responsiveness
Ensure accessibility: provide labels/roles for screen readers and maintain NVDA integration hooks
Files:
basilisk/gui/conversation_tab.pybasilisk/gui/enhanced_error_dialog.py
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Use tabs (not spaces) for indentation in Python code
Use snake_case for variables/functions, PascalCase for classes, and UPPER_SNAKE_CASE for constants
Write Google-style docstrings with type hints for all public APIs
Use double quotes for strings; wrap translatable text with _("...")
Do not import the translation function _; it is provided as a builtin
Group imports as standard library, third-party, then local; sort each group alphabetically
Mark translatable strings with _("...") and provide preceding translator context comments using '# Translators:' or '# translators:' when helpful
Use provider-specific streaming APIs to display messages in real time
Files:
basilisk/gui/conversation_tab.pybasilisk/gui/enhanced_error_dialog.py
⚙️ CodeRabbit configuration file
**/*.py: ## Indentation
We use tabs, not spaces.naming conventions
- snake_case for variables and functions
- PascalCase for classes
- UPPER_SNAKE_CASE for constants
- snake_case for files and directories
- Use whole words in names when possible
documentation
- Use docstrings for all public classes and functions
- Use type hints for all public classes and functions
- Use google style for docstrings
Strings
- Always use double quotes for strings
- Use percent formatting for string
- Use _("string") for translatable strings
- Use **# translator: ** with context for translatable strings
Files:
basilisk/gui/conversation_tab.pybasilisk/gui/enhanced_error_dialog.py
🧠 Learnings (2)
📚 Learning: 2025-09-28T08:26:32.431Z
Learnt from: CR
PR: SigmaNight/basiliskLLM#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-28T08:26:32.431Z
Learning: Applies to basilisk/gui/**/*.py : In wxPython UI code, bind events using self.Bind(wx.EVT_*, self.on_*) consistently
Applied to files:
basilisk/gui/enhanced_error_dialog.py
📚 Learning: 2025-09-28T08:26:32.432Z
Learnt from: CR
PR: SigmaNight/basiliskLLM#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-28T08:26:32.432Z
Learning: Applies to **/*.py : Use double quotes for strings; wrap translatable text with _("...")
Applied to files:
basilisk/gui/enhanced_error_dialog.py
🧬 Code graph analysis (2)
basilisk/gui/conversation_tab.py (1)
basilisk/gui/enhanced_error_dialog.py (1)
show_enhanced_error_dialog(38-65)
basilisk/gui/enhanced_error_dialog.py (2)
__builtins__.pyi (1)
_(1-1)basilisk/conversation/attached_file.py (1)
url(466-475)
🔇 Additional comments (10)
basilisk/gui/enhanced_error_dialog.py (7)
58-65: Good: dialog lifecycle handled correctlyShowModal followed by Destroy satisfies our wx dialog lifecycle rule.
Based on learnings.
102-124: Layout follows sizers; accessible text presentBoxSizer usage and wrapped StaticText look good. Keep it consistent elsewhere.
As per coding guidelines.
20-23: Use double quotes for regex literal; consider a simpler, more robust URL patternSwitch to double quotes to match project string rules. Also consider a simpler, battle‑tested pattern to reduce false positives.
Apply this minimal style fix:
URL_PATTERN = re.compile( - r'https?://(?:[-\w.])+(?:\:[0-9]+)?(?:/(?:[\w/_.])*(?:\?(?:[\w&=%.])*)?(?:\#(?:[\w.])*)?)?', + r"https?://(?:[-\w.])+(?::[0-9]+)?(?:/(?:[\w/_.])*(?:\?(?:[\w&=%.])*)?(?:#(?:[\w.])*)?)?", re.IGNORECASE, )Optional: replace with a simpler pattern if acceptable:
- r"https?://[^\s/$.?#].[^\s]*"
As per coding guidelines.
38-44: Type hint fix: title should be Optional[str]Current annotation allows None but types say str. Align with typing and mypy.
def show_enhanced_error_dialog( parent: Optional[wx.Window], message: str, - title: str = None, + title: Optional[str] = None, is_completion_error: bool = False, ) -> int:As per coding guidelines.
186-189: Bind on the dialog, not per‑control; fix quotingUse self.Bind(...) consistently and double quotes.
- if hasattr(self, 'open_url_btn'): - self.open_url_btn.Bind(wx.EVT_BUTTON, self.on_open_url) - self.copy_btn.Bind(wx.EVT_BUTTON, self.on_copy_error) + if hasattr(self, "open_url_btn"): + self.Bind(wx.EVT_BUTTON, self.on_open_url, self.open_url_btn) + self.Bind(wx.EVT_BUTTON, self.on_copy_error, self.copy_btn)Based on learnings.
196-215: Avoid wx.MessageBox inside the enhanced dialog; ensure clipboard is always closed
- Don’t invoke wx.MessageBox from within this dialog; it undermines the unified error UX and can recurse.
- Guarantee wx.TheClipboard.Close() via finally to avoid locking the clipboard on exceptions.
def on_copy_error(self, event: wx.CommandEvent): @@ - try: - if wx.TheClipboard.Open(): - wx.TheClipboard.SetData(wx.TextDataObject(self.message)) - wx.TheClipboard.Close() - - # Change button label to show confirmation - original_label = self.copy_btn.GetLabel() - self.copy_btn.SetLabel(_("Copied!")) - self.copy_btn.Disable() - - # Reset button label after 2 seconds - wx.CallLater(2000, self._reset_copy_button, original_label) - else: - wx.MessageBox( - _("Failed to access clipboard"), - _("Error"), - wx.OK | wx.ICON_ERROR, - self, - ) - except Exception as e: - logger.error("Failed to copy to clipboard: %s", e) - wx.MessageBox( - _("Failed to copy to clipboard: %s") % str(e), - _("Error"), - wx.OK | wx.ICON_ERROR, - self, - ) + try: + opened = wx.TheClipboard.Open() + try: + if opened: + if not wx.TheClipboard.SetData(wx.TextDataObject(self.message)): + raise RuntimeError("SetData returned False") + else: + # Inline, non-modal feedback instead of MessageBox + self._flash_copy_button(_("Failed to access clipboard")) + return + finally: + if opened: + wx.TheClipboard.Close() + + # Success feedback + original_label = self.copy_btn.GetLabel() + self.copy_btn.SetLabel(_("Copied!")) + self.copy_btn.Disable() + wx.CallLater(2000, self._reset_copy_button, original_label) + except Exception as e: + logger.error("Failed to copy to clipboard: %s", e) + self._flash_copy_button(_("Failed to copy to clipboard: %s") % e)Add this helper (place near other privates):
+ def _flash_copy_button(self, text: str): + original_label = self.copy_btn.GetLabel() + self.copy_btn.SetLabel(text) + self.copy_btn.Enable() + wx.CallLater(2000, self._reset_copy_button, original_label)As per coding guidelines.
240-258: Avoid MessageBox on URL open failure; provide inline feedback and fix quoting
- Do not open MessageBox from inside the enhanced dialog.
- Provide inline button feedback and ensure double quotes.
- if not hasattr(self, 'url_choice'): + if not hasattr(self, "url_choice"): return @@ try: logger.info("Opening URL in browser: %s", url) webbrowser.open(url) except Exception as e: logger.error("Failed to open URL %s: %s", url, e) - wx.MessageBox( - _("Failed to open URL in browser: %s") % str(e), - _("Error"), - wx.OK | wx.ICON_ERROR, - self, - ) + # Inline, non-modal feedback on the button + if hasattr(self, "open_url_btn"): + orig = self.open_url_btn.GetLabel() + self.open_url_btn.SetLabel(_("Open failed")) + wx.CallLater(2000, self.open_url_btn.SetLabel, orig)As per coding guidelines.
basilisk/gui/conversation_tab.py (3)
42-42: LGTM: using enhanced error dialogThe import is correct and matches project direction.
733-738: LGTM: title generation errors use enhanced dialog and percent formattingConforms to translation and formatting rules.
887-893: LGTM: completion errors migrated to enhanced dialogMessage and title are localized; flag set appropriately.
AAClause
force-pushed
the
errorMsgURL
branch
2 times, most recently
from
ef65e59 to
2a051e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
basilisk/gui/ocr_handler.py (7)
93-101: Replace MessageBox with enhanced dialog for consistencyAligns with PR objective to unify error dialogs.
Apply:
- wx.MessageBox( - _( - "An error occurred while processing OCR results. Details: \n%s" - ) - % e, - _("Error"), - wx.OK | wx.ICON_ERROR, - ) + show_enhanced_error_dialog( + parent=self.parent, + message=_("An error occurred while processing OCR results. Details: \n%s") % e, + title=_("OCR Error"), + is_completion_error=False, + )
277-282: Unify provider‑capability error to enhanced dialogKeep error UX consistent.
Apply:
- wx.MessageBox( - # Translators: This message is displayed when the current provider does not support OCR. - _("The selected provider does not support OCR."), - _("Error"), - wx.OK | wx.ICON_ERROR, - ) + show_enhanced_error_dialog( + parent=self.parent, + # Translators: This message is displayed when the current provider does not support OCR. + message=_("The selected provider does not support OCR."), + title=_("Error"), + is_completion_error=False, + )
286-291: Unify “no attachments” error to enhanced dialogSame migration as above.
Apply:
- wx.MessageBox( - # Translators: This message is displayed when there are no attachments to perform OCR on. - _("No attachments to perform OCR on."), - _("Error"), - wx.OK | wx.ICON_ERROR, - ) + show_enhanced_error_dialog( + parent=self.parent, + # Translators: This message is displayed when there are no attachments to perform OCR on. + message=_("No attachments to perform OCR on."), + title=_("Error"), + is_completion_error=False, + )
299-304: Unify “no client” error to enhanced dialogComplete the migration.
Apply:
- wx.MessageBox( - # Translators: This message is displayed when the current provider does not have a client. - _("The selected provider does not have a client."), - _("Error"), - wx.OK | wx.ICON_ERROR, - ) + show_enhanced_error_dialog( + parent=self.parent, + # Translators: This message is displayed when the current provider does not have a client. + message=_("The selected provider does not have a client."), + title=_("Error"), + is_completion_error=False, + )
226-226: Logging format bug: missing %s placeholderThis will raise during logging formatting.
Apply:
- log.error("Error terminating process: %", e, exc_info=True) + log.error("Error terminating process: %s", e, exc_info=True)
167-175: Optional: migrate info popups to non‑blocking UXThe YES/NO MessageBox here is informational and not an error; optional to keep. If you want full consistency, consider a non‑modal notice or in‑panel banner.
If you decide to migrate, confirm whether EnhancedErrorDialog should support YES/NO; if not, keep as is. As per coding guidelines.
267-345: Centralize error MessageBoxes across GUI
Multiple standalonewx.MessageBox(..., wx.OK | wx.ICON_ERROR)calls remain in basilisk/gui:
- prompt_attachments_panel.py (lines 423, 692, 741)
- html_view_window.py (line 113)
- main_frame.py (lines 341, 554, 650, 670, 847, 1039, 1061)
- account_dialog.py (lines 139, 144, 149, 1071, 1121)
Extract these into a shared error-display helper and replace direct calls with that centralized method.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
basilisk/completion_handler.py(2 hunks)basilisk/gui/conversation_tab.py(5 hunks)basilisk/gui/enhanced_error_dialog.py(1 hunks)basilisk/gui/ocr_handler.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Use tabs (not spaces) for indentation in Python code
Use snake_case for variables/functions, PascalCase for classes, and UPPER_SNAKE_CASE for constants
Write Google-style docstrings with type hints for all public APIs
Use double quotes for strings; wrap translatable text with _("...")
Do not import the translation function _; it is provided as a builtin
Group imports as standard library, third-party, then local; sort each group alphabetically
Mark translatable strings with _("...") and provide preceding translator context comments using '# Translators:' or '# translators:' when helpful
Use provider-specific streaming APIs to display messages in real time
Files:
basilisk/completion_handler.pybasilisk/gui/conversation_tab.pybasilisk/gui/ocr_handler.pybasilisk/gui/enhanced_error_dialog.py
⚙️ CodeRabbit configuration file
**/*.py: ## Indentation
We use tabs, not spaces.naming conventions
- snake_case for variables and functions
- PascalCase for classes
- UPPER_SNAKE_CASE for constants
- snake_case for files and directories
- Use whole words in names when possible
documentation
- Use docstrings for all public classes and functions
- Use type hints for all public classes and functions
- Use google style for docstrings
Strings
- Always use double quotes for strings
- Use percent formatting for string
- Use _("string") for translatable strings
- Use **# translator: ** with context for translatable strings
Files:
basilisk/completion_handler.pybasilisk/gui/conversation_tab.pybasilisk/gui/ocr_handler.pybasilisk/gui/enhanced_error_dialog.py
basilisk/gui/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
basilisk/gui/**/*.py: In wxPython UI code, bind events using self.Bind(wx.EVT_, self.on_) consistently
After any dialog.ShowModal(), always call dialog.Destroy()
Use BoxSizer-based layouts; avoid fixed positioning for responsiveness
Ensure accessibility: provide labels/roles for screen readers and maintain NVDA integration hooks
Files:
basilisk/gui/conversation_tab.pybasilisk/gui/ocr_handler.pybasilisk/gui/enhanced_error_dialog.py
🧠 Learnings (2)
📚 Learning: 2025-09-28T08:26:32.432Z
Learnt from: CR
PR: SigmaNight/basiliskLLM#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-28T08:26:32.432Z
Learning: Applies to **/*.py : Use double quotes for strings; wrap translatable text with _("...")
Applied to files:
basilisk/completion_handler.pybasilisk/gui/enhanced_error_dialog.py
📚 Learning: 2025-09-28T08:26:32.431Z
Learnt from: CR
PR: SigmaNight/basiliskLLM#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-28T08:26:32.431Z
Learning: Applies to basilisk/gui/**/*.py : In wxPython UI code, bind events using self.Bind(wx.EVT_*, self.on_*) consistently
Applied to files:
basilisk/gui/enhanced_error_dialog.py
🧬 Code graph analysis (4)
basilisk/completion_handler.py (2)
basilisk/gui/enhanced_error_dialog.py (1)
show_enhanced_error_dialog(38-65)__builtins__.pyi (1)
_(1-1)
basilisk/gui/conversation_tab.py (1)
basilisk/gui/enhanced_error_dialog.py (1)
show_enhanced_error_dialog(38-65)
basilisk/gui/ocr_handler.py (2)
basilisk/gui/enhanced_error_dialog.py (1)
show_enhanced_error_dialog(38-65)__builtins__.pyi (1)
_(1-1)
basilisk/gui/enhanced_error_dialog.py (1)
__builtins__.pyi (1)
_(1-1)
⏰ 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). (2)
- GitHub Check: build_app / build_windows (x86_64)
- GitHub Check: build_app / build_windows (x86)
🔇 Additional comments (11)
basilisk/completion_handler.py (1)
27-27: Import looks goodCentralizing error UI via show_enhanced_error_dialog aligns with the PR intent.
basilisk/gui/ocr_handler.py (2)
21-21: Import of enhanced dialog – OKMatches usage below; consistent with other modules.
147-152: Good switch to enhanced dialog for OCR errorsCorrect parent and is_completion_error=False.
basilisk/gui/enhanced_error_dialog.py (7)
41-43: Type hint: title should be Optional[str]Annotate the nullable title correctly to avoid mypy warnings and clarify API.
Apply:
- title: str = None, + title: Optional[str] = None,As per coding guidelines.
20-23: URL regex is brittle; broaden allowed charactersCurrent pattern misses many valid URLs (queries/fragments, UTF‑8, parentheses) and over‑constrains allowed chars. Prefer a simpler, more permissive detector.
Apply:
-URL_PATTERN = re.compile( - r"https?://(?:[-\w.])+(?:\:[0-9]+)?(?:/(?:[\w/_.])*(?:\?(?:[\w&=%.])*)?(?:\#(?:[\w.])*)?)?", - re.IGNORECASE, -) +URL_PATTERN = re.compile( + r"https?://[^\s<>()\"']+", + re.IGNORECASE, +)This matches typical http/https URLs (incl. ports/paths/queries/fragments) while avoiding trailing punctuation. As per coding guidelines.
184-189: Use dialog-level event binding (self.Bind) and double quotesBind on the dialog per house style; also fix quoting.
Apply:
- if hasattr(self, 'open_url_btn'): - self.open_url_btn.Bind(wx.EVT_BUTTON, self.on_open_url) - self.copy_btn.Bind(wx.EVT_BUTTON, self.on_copy_error) + if hasattr(self, "open_url_btn"): + self.Bind(wx.EVT_BUTTON, self.on_open_url, self.open_url_btn) + self.Bind(wx.EVT_BUTTON, self.on_copy_error, self.copy_btn)Based on learnings.
253-258: Avoid MessageBox on URL open failure; log and non-intrusive feedbackDon’t pop a second dialog from within the error dialog. Beep and consider adding a status label if desired.
Apply:
- except Exception as e: - logger.error("Failed to open URL %s: %s", url, e) - wx.MessageBox( - _("Failed to open URL in browser: %s") % str(e), - _("Error"), - wx.OK | wx.ICON_ERROR, - self, - ) + except Exception as e: + logger.error("Failed to open URL %s: %s", url, e) + wx.Bell()
215-222: Same: replace MessageBox on copy exception with inline feedbackKeep UX consistent; avoid recursive dialogs.
Apply:
- except Exception as e: - logger.error("Failed to copy to clipboard: %s", e) - wx.MessageBox( - _("Failed to copy to clipboard: %s") % str(e), - _("Error"), - wx.OK | wx.ICON_ERROR, - self, - ) + except Exception as e: + logger.error("Failed to copy to clipboard: %s", e) + wx.Bell() + original_label = self.copy_btn.GetLabel() + self.copy_btn.SetLabel(_("Copy failed: %s") % e) + self.copy_btn.Disable() + wx.CallLater(2000, self._reset_copy_button, original_label)
206-215: Avoid MessageBox inside the enhanced dialog; give inline feedback insteadShowing wx.MessageBox here creates nested error popups from within the error dialog. Provide subtle inline feedback and log/beep.
Apply:
- else: - wx.MessageBox( - _("Failed to access clipboard"), - _("Error"), - wx.OK | wx.ICON_ERROR, - self, - ) + else: + logger.warning("Failed to access clipboard") + wx.Bell() + original_label = self.copy_btn.GetLabel() + self.copy_btn.SetLabel(_("Copy failed")) + self.copy_btn.Disable() + wx.CallLater(2000, self._reset_copy_button, original_label)As per coding guidelines.
240-241: Consistent quotingUse double quotes per project standard.
Apply:
- if not hasattr(self, 'url_choice'): + if not hasattr(self, "url_choice"):As per coding guidelines.
basilisk/gui/conversation_tab.py (1)
524-528: Drop redundant str() when using %s formatting
%already callsstr()on its operand, so the explicit wrap is unnecessary. Removing it keeps the formatting idiomatic.- message=_("An error occurred during transcription: %s") - % str(error), + message=_("An error occurred during transcription: %s") % error,As per coding guidelines.
- Add enhanced error dialog that detects URLs in error messages - Provide "Open in Browser" button for URLs in error messages - Add "Copy Error" button to copy error messages to clipboard - Replace standard wx.MessageBox with enhanced dialog for all errors - Apply enhanced dialogs to completion, transcription, OCR, and save errors
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…le error messages
… error dialog call
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
for more information, see https://pre-commit.ci
- Fix type hints, event binding, and clipboard cleanup in enhanced dialog - Replace MessageBox calls inside dialog with inline feedback - Fix key handling, menu lifetime, and logging format issues - Focus OK button instead of Copy Error button - Use enhanced dialog only for dynamic technical errors, revert simple messages to wx.MessageBox
Summary by CodeRabbit
New Features
Refactor