Merged
Conversation
- Remove incorrect await on SQLAlchemy delete() (repositories.py) - Extract duplicate workflow prompts to dedicated files - Add ErrorBoundary component to React frontend - Fix test warnings for unawaited coroutines - Delete deprecated prompt files Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Enhance project documentation for better understanding - Introduce ErrorBoundary component for error handling in React - Refactor repository methods to remove unnecessary await - Update test cases to improve clarity and maintainability
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves code quality across multiple areas: fixes incorrect async/await usage in SQLAlchemy repositories, eliminates test warnings, adds error handling to the React frontend, and reduces duplication in GitHub workflows.
Changes:
- Fixed bug where SQLAlchemy's synchronous
delete()method was incorrectly awaited - Added proper coroutine cleanup in unit tests to eliminate pytest warnings
- Introduced React ErrorBoundary component for graceful error handling
- Refactored GitHub workflows to use centralized prompt files, eliminating ~200 lines of duplication
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| core/database/repositories.py | Removed incorrect await from synchronous session.delete() calls in three repository classes |
| tests/unit/api/test_analytics.py | Added mock_create_task fixture that properly closes coroutines to fix pytest warnings |
| app/src/components/ErrorBoundary.tsx | New React component for catching rendering errors with user-friendly recovery UI |
| app/src/router.tsx | Wrapped application with ErrorBoundary for top-level error handling |
| .github/workflows/impl-generate.yml | Replaced ~70 lines of inline prompt with reference to external file |
| .github/workflows/impl-repair.yml | Replaced ~70 lines of inline prompt with reference to external file |
| prompts/workflow-prompts/impl-generate-claude.md | New centralized prompt file for implementation generation |
| prompts/workflow-prompts/impl-repair-claude.md | New centralized prompt file for implementation repair |
| prompts/workflow-prompts/improve-from-feedback.md | Removed deprecated prompt file |
| prompts/workflow-prompts/generate-implementation.md | Removed deprecated prompt file |
Comment on lines
+42
to
+44
| handleRetry = (): void => { | ||
| this.setState({ hasError: false, error: null }); | ||
| }; |
There was a problem hiding this comment.
The handleRetry method is defined but never used. It should either be connected to a button in the UI or removed to avoid dead code.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add "Try Again" button that calls handleRetry to reset error state without a full page reload. Addresses Copilot review comment about unused method. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Fixes 3 security vulnerabilities: - CVE: XSS via Open Redirects (high) - CVE: SSR XSS in ScrollRestoration (high) - CVE: CSRF in Action/Server Action Request Processing (medium) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| @@ -0,0 +1,131 @@ | |||
| import { Component, ReactNode } from 'react'; | |||
There was a problem hiding this comment.
Missing React import. The code uses React.ErrorInfo on line 31 but only imports Component and ReactNode from 'react'. Add import React or change line 31 to use a direct import of ErrorInfo type.
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
awaiton SQLAlchemydelete()callsprompts/workflow-prompts/Changes
core/database/repositories.py- Remove await from sync delete()app/src/components/ErrorBoundary.tsx- New componentapp/src/router.tsx- Wrap with ErrorBoundary.github/workflows/impl-generate.yml- Use external prompt file.github/workflows/impl-repair.yml- Use external prompt fileprompts/workflow-prompts/impl-generate-claude.md- Newprompts/workflow-prompts/impl-repair-claude.md- Newtests/unit/api/test_analytics.py- Fix coroutine warningsTest plan
🤖 Generated with Claude Code