Claude task no tuning #1884
Claude task no tuning #1884
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements significant changes including configuration updates, linting tool configuration, model validation improvements, and introduces wallet functionality for currency exchange operations.
- Adds comprehensive flake8 and Ruff configuration with strict linting rules
- Implements wallet and transaction entities with currency conversion and fee handling
- Improves code quality with docstrings, type hints, and better error handling
Reviewed Changes
Copilot reviewed 42 out of 46 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.cfg | Adds flake8 configuration with code style ignores |
| pyproject.toml | Adds comprehensive mypy and Ruff linting configuration |
| backend/pyproject.toml | Removes old tool configurations |
| backend/app/models.py | Adds wallet/transaction models and improves existing models |
| backend/app/crud.py | Implements wallet/transaction CRUD operations |
| backend/app/api/routes/wallets.py | New wallet management API endpoints |
| backend/app/constants.py | Adds application constants |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| password: str | None = Field(default=None, min_length=8, max_length=40) | ||
| """User update model.""" | ||
|
|
||
| email: EmailStr | None = Field(default=None, max_length=STRING_MAX_LENGTH) # type: ignore[assignment] |
There was a problem hiding this comment.
The max_length parameter should use EMAIL_MAX_LENGTH constant instead of STRING_MAX_LENGTH for email fields to maintain consistency with other email field definitions.
Copilot uses AI. Check for mistakes.
|
|
||
| user = crud.create_user(session=session, user_create=user_in) | ||
| if settings.emails_enabled and user_in.email: | ||
| if not settings.emails_enabled and user_in.email: |
There was a problem hiding this comment.
The condition should be if settings.emails_enabled and user_in.email: to send emails when enabled, not when disabled.
| if not settings.emails_enabled and user_in.email: | |
| if settings.emails_enabled and user_in.email: |
Copilot uses AI. Check for mistakes.
| raise HTTPException( | ||
| status_code=409, detail="User with this email already exists" | ||
| status_code=CONFLICT_CODE, | ||
| detail="models.User with this email already exists", |
There was a problem hiding this comment.
The error message should say 'User with this email already exists' instead of 'models.User' to maintain consistency with user-facing messages.
| detail="models.User with this email already exists", | |
| detail="User with this email already exists", |
Copilot uses AI. Check for mistakes.
| password: str, | ||
| ) -> dict[str, str]: | ||
| """Authenticate a user and return headers.""" | ||
| login_data = {"username": username, USER_PASSWORD_KEY: password} |
There was a problem hiding this comment.
The key should be 'password' instead of USER_PASSWORD_KEY constant which evaluates to 'password'. Using the constant here creates inconsistent dictionary key usage.
| login_data = {"username": username, USER_PASSWORD_KEY: password} | |
| login_data = {"username": username, "password": password} |
Copilot uses AI. Check for mistakes.
Implementation time: 5.47