Skip to content

chore: add lint-fix script; minor code quality improvements in db check, rate limiter, testing config; update changelog - #85

Merged
d-ulker merged 18 commits into
mainfrom
fix/code-quality-only
Aug 16, 2025
Merged

chore: add lint-fix script; minor code quality improvements in db check, rate limiter, testing config; update changelog#85
d-ulker merged 18 commits into
mainfrom
fix/code-quality-only

Conversation

@d-ulker

@d-ulker d-ulker commented Aug 16, 2025

Copy link
Copy Markdown
Owner

Summary by Sourcery

Add a lint-fix automation script and apply minor code quality improvements across several modules, including variable renaming, whitespace cleanup, and formatting tweaks.

Enhancements:

  • Introduce scripts/fix_linting_issues.py to automate removal of trailing whitespace, fix indentation, and clean blank lines
  • Refactor variable naming and indentation in scripts/database/check_pgvector.py and scripts/testing/config.py for clarity
  • Remove extraneous trailing whitespace and tidy blank lines in API rate limiter and debug import modules

Documentation:

  • Update CHANGELOG.md to document code quality improvements

Summary by CodeRabbit

  • New Features

    • Added a CLI tool to automatically fix common Python linting issues across the repository.
  • Documentation

    • Updated changelog with a Code Quality Improvements subsection.
  • Style

    • Formatting and readability improvements across rate limiting, testing configuration, and debugging utilities.
  • Chores

    • Improved logging, clearer step-by-step guidance when a required database extension is missing, and minor robustness tweaks.
  • Behavior

    • Rate-limiting defaults adjusted to prefer blacklist-based IP handling.

…ck, rate limiter, testing config; update changelog
@d-ulker
d-ulker requested a review from Copilot August 16, 2025 10:45
@d-ulker d-ulker self-assigned this Aug 16, 2025
@sourcery-ai

sourcery-ai Bot commented Aug 16, 2025

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This PR adds an automated lint-fixing script and applies targeted code-quality cleanups—removing extraneous whitespace, refining variable names and indentation—across the API rate limiter, database extension checker, testing config, and debug import script, plus updates the changelog to document these improvements.

Class diagram for the new lint-fixing script

classDiagram
    class LintFixer {
        +find_python_files(project_root: Path) List[Path]
        +fix_trailing_whitespace(file_path: Path) Tuple[bool, List[str]]
        +fix_indentation_issues(file_path: Path) Tuple[bool, List[str]]
        +fix_blank_lines_with_whitespace(file_path: Path) Tuple[bool, List[str]]
        +main()
    }
    LintFixer <|.. main
Loading

Class diagram for updated TokenBucketRateLimiter

classDiagram
    class TokenBucketRateLimiter {
        -config: RateLimitConfig
        -buckets: Dict[str, float]
        -last_refill: Dict[str, float]
        -blocked_clients: Dict[str, float]
        -concurrent_requests: Dict[str, int]
        -request_history: Dict[str, Deque]
        -lock: threading.RLock
        +__init__(config: RateLimitConfig)
        +_get_client_key(client_ip: str, user_agent: str = "") str
        +_is_ip_allowed(client_ip: str) bool
        +_is_client_blocked(client_key: str) bool
        +_analyze_user_agent(user_agent: str) int
        +_analyze_request_patterns(client_key: str, client_ip: str) int
        +_detect_abuse(client_key: str, client_ip: str, user_agent: str = "") bool
        +_refill_bucket(client_key: str)
        +allow_request(client_ip: str, user_agent: str = "") Tuple[bool, str, Dict]
        +release_request(client_ip: str, user_agent: str = "")
        +get_stats() Dict
        +add_to_blacklist(ip: str)
        +remove_from_blacklist(ip: str)
        +add_to_whitelist(ip: str)
        +remove_from_whitelist(ip: str)
        +reset_state()
    }
Loading

Class diagram for updated check_pgvector function

classDiagram
    class check_pgvector {
        +check_pgvector()
        -extension_installed: bool
    }
Loading

Class diagram for updated testing config

classDiagram
    class TestingConfig {
        +_get_base_url() str
        +test_batch_prediction(texts: list) dict
    }
Loading

File-Level Changes

Change Details Files
Introduce automated lint-fix script
  • Add scripts/fix_linting_issues.py with functions to find Python files and fix trailing whitespace, indentation, and blank-line issues
  • Implement a CLI-style main entry that processes all files and reports fixes
  • Include shebang, module docstring, and fix-summary printing
scripts/fix_linting_issues.py
Refactor pgvector extension checker for clarity
  • Rename is_installed to extension_installed for consistency
  • Adjust comment indentation levels
  • Update return statement to use the renamed variable
scripts/database/check_pgvector.py
Clean up testing config formatting
  • Align multi-line environment-variable lookup indentation
  • Remove trailing whitespace in error dict literal
  • Ensure consistent line endings
scripts/testing/config.py
Remove extraneous whitespace in rate limiter module
  • Strip trailing blank spaces between method definitions
  • Standardize blank-line placement without altering logic
  • Retain original behavior while improving readability
src/api_rate_limiter.py
Fix trailing whitespace in debug import script
  • Remove trailing space at end of print statement
  • Add missing newline at end of file
deployment/cloud-run/debug_api_import.py
Update changelog with quality improvements
  • Document addition of lint-fix script
  • List minor formatting and logging cleanups in scripts
  • Mark these changes under a new 'Code Quality Improvements' section
CHANGELOG.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Aug 16, 2025

Copy link
Copy Markdown

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Warning

Rate limit exceeded

@cursor[bot] has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 13 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 814ce9b and 84d9a8a.

⛔ Files ignored due to path filters (23)
  • src/models/__pycache__/__init__.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/emotion_detection/__pycache__/__init__.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/emotion_detection/__pycache__/api_demo.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/emotion_detection/__pycache__/bert_classifier.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/emotion_detection/__pycache__/dataset_loader.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/emotion_detection/__pycache__/hf_loader.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/emotion_detection/__pycache__/labels.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/emotion_detection/__pycache__/training_pipeline.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/secure_loader/__pycache__/__init__.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/secure_loader/__pycache__/integrity_checker.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/secure_loader/__pycache__/model_validator.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/secure_loader/__pycache__/sandbox_executor.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/secure_loader/__pycache__/secure_model_loader.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/summarization/__pycache__/__init__.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/summarization/__pycache__/api_demo.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/summarization/__pycache__/dataset_loader.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/summarization/__pycache__/t5_summarizer.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/summarization/__pycache__/training_pipeline.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/voice_processing/__pycache__/__init__.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/voice_processing/__pycache__/api_demo.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/voice_processing/__pycache__/audio_preprocessor.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/voice_processing/__pycache__/transcription_api.cpython-313.pyc is excluded by !**/*.pyc
  • src/models/voice_processing/__pycache__/whisper_transcriber.cpython-313.pyc is excluded by !**/*.pyc
📒 Files selected for processing (4)
  • scripts/database/check_pgvector.py (1 hunks)
  • scripts/fix_linting_issues.py (1 hunks)
  • scripts/testing/config.py (2 hunks)
  • src/api_rate_limiter.py (11 hunks)

Walkthrough

Adds a new CLI lint-fixer script, updates pgvector check with clearer messaging, error logging, and explicit cursor use, adjusts formatting and a lint suppression in a debug script and testing config, and reforms whitespace/typing and default IP blacklist behavior in the API rate limiter. CHANGELOG updated.

Changes

Cohort / File(s) Summary of Changes
Changelog Update
CHANGELOG.md
Added an "Unreleased" Code Quality Improvements subsection listing the new lint-fixer script, pgvector check improvements, and readability/formatting tweaks.
Lint Fixer Tool (New)
scripts/fix_linting_issues.py
New CLI that finds Python files (excludes build/VCS/cache), and applies three fixers: trailing whitespace removal (with optional .bak backup), indentation/syntax detection (reports only), and blank-line whitespace trimming; prints per-file progress and a final summary; UTF-8 I/O.
PgVector Check Enhancements
scripts/database/check_pgvector.py
Renamed internal flag (is_installedextension_installed) and return value to match; added explicit cur = conn.cursor() initialization; improved error logging with logging.error(..., exc_info=True) and return False on exceptions; when not installed, prints step-by-step install/enable guidance. No public API signature change.
Debug Script Formatting
deployment/cloud-run/debug_api_import.py
Added trailing newline and a lint suppression comment (# noqa: T201) on the final print; no runtime behavior changes.
Testing Config Formatting
scripts/testing/config.py
Reflowed line breaks/indentation in _get_base_url and an exception return block; no behavioral changes.
API Rate Limiter Adjustments
src/api_rate_limiter.py
Mostly whitespace/indentation reflow; changed allow_request return typing from Tuple[bool, str, Dict] to tuple[bool, str, dict]; add_rate_limiting now explicitly sets enable_ip_blacklist=True and enable_ip_whitelist=False (changing default IP-listing behavior).

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer
  participant CLI as fix_linting_issues.py
  participant Finder as File Finder
  participant Fixers as Fixers (Trailing / Indent / Blank)
  participant FS as File System

  Dev->>CLI: Run script (python scripts/fix_linting_issues.py [--backup])
  CLI->>Finder: find_python_files(project_root)
  Finder-->>CLI: List[Path]
  loop For each file
    CLI->>Fixers: fix_trailing_whitespace(file)
    Fixers->>FS: Read/Write if needed
    Fixers-->>CLI: (modified, issues)
    CLI->>Fixers: fix_indentation_issues(file)
    Fixers->>FS: Read (parse) and report issues
    Fixers-->>CLI: (modified, issues)
    CLI->>Fixers: fix_blank_lines_with_whitespace(file)
    Fixers->>FS: Read/Write if needed
    Fixers-->>CLI: (modified, issues)
    CLI-->>Dev: Per-file results
  end
  CLI-->>Dev: Final summary (totals, details)
Loading
sequenceDiagram
  participant Script as check_pgvector.py
  participant DB as Postgres
  participant Logger as Logger

  Script->>DB: Connect and query pg_extension for 'pgvector' (cur = conn.cursor())
  alt Extension installed
    Script->>Logger: Log installed status
    Script-->>Caller: True
  else Not installed
    Script->>Logger: Log NOT installed + installation steps instructions
    Script-->>Caller: False
  end
  opt Exception
    Script->>Logger: logging.error(..., exc_info=True)
    Script-->>Caller: False
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A nibble of whitespace, a hop through the tree,
I tidy the lines so the code runs free.
PgVector whispers yes or gives steps to install,
Rate limits now guarded by rules one and all.
I thump, I grin — lint bunnies banished, hurrah! 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/code-quality-only

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @uelkerd, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on enhancing overall code quality and maintainability. It introduces a new utility script to automatically fix common linting issues, alongside minor formatting and readability improvements in existing database check, API rate limiter, and testing configuration files. The changes aim to ensure consistent code style and improve the developer experience.

Highlights

  • New Linting Fix Script: A new Python script, scripts/fix_linting_issues.py, has been added. This script automates the correction of common PEP8-style issues such as trailing whitespace, inconsistent indentation, and whitespace on blank lines across Python files in the project. It helps maintain code consistency and readability.
  • Database Check Script Refinement: The scripts/database/check_pgvector.py script received minor code quality enhancements. Specifically, a variable name was clarified from is_installed to extension_installed for better readability, and some indentation was adjusted.
  • API Rate Limiter and Testing Config Readability: Formatting improvements were applied to src/api_rate_limiter.py and scripts/testing/config.py. These changes primarily involve adjusting line breaks and removing extraneous whitespace to enhance code readability and adherence to style guidelines.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@deepsource-io

deepsource-io Bot commented Aug 16, 2025

Copy link
Copy Markdown
Contributor

Here's the code health analysis summary for commits 560f195..84d9a8a. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource Test coverage LogoTest coverage⚠️ Artifact not reportedTimed out: Artifact was never reportedView Check ↗
DeepSource Python LogoPython✅ Success
🎯 31 occurences resolved
View Check ↗
DeepSource Terraform LogoTerraform✅ SuccessView Check ↗
DeepSource Secrets LogoSecrets✅ SuccessView Check ↗
DeepSource Shell LogoShell✅ SuccessView Check ↗
DeepSource Docker LogoDocker✅ SuccessView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR focuses on code quality improvements by adding automated linting tools and making minor refactoring improvements across the codebase. The main change is the introduction of a comprehensive Python script to fix common PEP8 style issues automatically.

  • Added a linting fix automation script to address trailing whitespace, indentation issues, and blank line formatting
  • Improved variable naming in database check utility for better readability
  • Updated changelog to document these code quality improvements

Reviewed Changes

Copilot reviewed 3 out of 6 changed files in this pull request and generated 4 comments.

File Description
scripts/fix_linting_issues.py New automated script to fix common PEP8 linting issues across Python files
scripts/database/check_pgvector.py Improved variable naming from is_installed to extension_installed
CHANGELOG.md Added documentation of code quality improvements

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread scripts/fix_linting_issues.py Outdated
Comment thread scripts/fix_linting_issues.py Outdated
Comment thread scripts/fix_linting_issues.py Outdated
Comment thread scripts/fix_linting_issues.py Outdated

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments

### Comment 1
<location> `scripts/fix_linting_issues.py:60` </location>
<code_context>
+    except Exception as e:
+        return False, [f"Error processing file: {e}"]
+
+def fix_indentation_issues(file_path: Path) -> Tuple[bool, List[str]]:
+    """Fix indentation issues in a file."""
+    try:
</code_context>

<issue_to_address>
Indentation fix logic is overly simplistic and may not handle real-world cases robustly.

The logic only checks for lines ending with 'and' or 'or', which misses many indentation scenarios and may miscorrect valid code. Consider using a parsing tool like ast or a linter for more reliable results.

Suggested implementation:

```python
        # Use ast to check for indentation errors
        import ast

        try:
            ast.parse(original_content)
            # If parsing succeeds, assume no indentation issues
            return False, []
        except IndentationError as ie:
            issues_fixed.append(f"Indentation error: {ie}")
            return False, issues_fixed
        except SyntaxError as se:
            issues_fixed.append(f"Syntax error (may be indentation related): {se}")
            return False, issues_fixed

```

- Remove any other logic in the function that attempts to "fix" indentation by line heuristics, as it is unreliable.
- If you want to automatically fix indentation, consider integrating a linter/formatter like `autopep8` or `black` and running it on the file, but do not attempt to fix indentation with custom line-based logic.
</issue_to_address>

### Comment 2
<location> `scripts/fix_linting_issues.py:157` </location>
<code_context>
+    for file_path in python_files:
+        print(f"\nProcessing: {file_path.relative_to(project_root)}")
+
+        _ = False  # Track if file was modified (renamed from file_fixed)
+        file_issues = []
+
</code_context>

<issue_to_address>
Using '_' as a flag for file modification is confusing and non-idiomatic.

Rename '_' to a more descriptive variable name like 'file_fixed' to clarify its purpose and follow Python conventions.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
    for file_path in python_files:
        print(f"\nProcessing: {file_path.relative_to(project_root)}")

        _ = False  # Track if file was modified (renamed from file_fixed)
        file_issues = []
=======
    for file_path in python_files:
        print(f"\nProcessing: {file_path.relative_to(project_root)}")

        file_fixed = False  # Track if file was modified
        file_issues = []
>>>>>>> REPLACE

</suggested_fix>

### Comment 3
<location> `scripts/fix_linting_issues.py:17` </location>
<code_context>
+    python_files = []
+    for root, dirs, files in os.walk(project_root):
+        # Skip certain directories
+        dirs[:] = [d for d in dirs if d not in {'.git', '__pycache__', '.venv', 'venv', 'node_modules', 'build', 'dist'}]
+
+        for file in files:
</code_context>

<issue_to_address>
Hardcoded directory exclusions may miss other common build or cache folders.

Make the excluded directories list configurable or add other common cache/build folders to ensure all unwanted files are skipped.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
def find_python_files(project_root: Path) -> List[Path]:
    """Find all Python files in the project."""
    python_files = []
    for root, dirs, files in os.walk(project_root):
        # Skip certain directories
        dirs[:] = [d for d in dirs if d not in {'.git', '__pycache__', '.venv', 'venv', 'node_modules', 'build', 'dist'}]

        for file in files:
            if file.endswith('.py'):
                python_files.append(Path(root) / file)

    return python_files
=======
EXCLUDED_DIRS = {
    '.git', '__pycache__', '.venv', 'venv', 'node_modules', 'build', 'dist',
    '.mypy_cache', '.pytest_cache', '.cache', '.coverage', '.eggs', '.tox',
    '.idea', '.vscode', '.DS_Store'
}

def find_python_files(project_root: Path, excluded_dirs: set = EXCLUDED_DIRS) -> List[Path]:
    """Find all Python files in the project, skipping excluded directories."""
    python_files = []
    for root, dirs, files in os.walk(project_root):
        # Skip certain directories
        dirs[:] = [d for d in dirs if d not in excluded_dirs]

        for file in files:
            if file.endswith('.py'):
                python_files.append(Path(root) / file)

    return python_files
>>>>>>> REPLACE

</suggested_fix>

### Comment 4
<location> `scripts/fix_linting_issues.py:28` </location>
<code_context>
+def fix_trailing_whitespace(file_path: Path) -> Tuple[bool, List[str]]:
+    """Fix trailing whitespace in a file."""
+    try:
+        with open(file_path, 'r', encoding='utf-8') as f:
+            content = f.read()
+
</code_context>

<issue_to_address>
Reading entire file content into memory may be inefficient for large files.

Refactor to process files line by line to reduce memory usage when handling large files.

Suggested implementation:

```python
def fix_trailing_whitespace(file_path: Path) -> Tuple[bool, List[str]]:
    """Fix trailing whitespace in a file, processing line by line for efficiency."""
    import tempfile
    changed = False
    changed_lines = []
    try:

```

```python
    try:
        with open(file_path, 'r', encoding='utf-8') as src, tempfile.NamedTemporaryFile('w', delete=False, encoding='utf-8') as tmp:
            for line in src:
                stripped_line = line.rstrip()
                if stripped_line != line.rstrip('\n'):
                    changed = True
                    changed_lines.append(stripped_line)
                tmp.write(stripped_line + '\n')
        if changed:
            os.replace(tmp.name, file_path)
        else:
            os.remove(tmp.name)
        return changed, changed_lines

```
</issue_to_address>

### Comment 5
<location> `scripts/fix_linting_issues.py:200` </location>
<code_context>
+            print(f"  - {issue}")
+
+    print("\n✅ Linting issues fix completed!")
+    print("\n💡 Next steps:")
+    print("  1. Review the changes")
+    print("  2. Test that functionality is preserved")
+    print("  3. Commit the fixes")
+    print("  4. Run linting tools to verify")
+
+if __name__ == "__main__":
</code_context>

<issue_to_address>
Script does not create backups before modifying files, risking accidental data loss.

Add a backup option or user warning to prevent unintended data loss when files are modified.

Suggested implementation:

```python
import argparse
import shutil
import os

def main():
    parser = argparse.ArgumentParser(description="Fix linting issues in files.")
    parser.add_argument("--backup", action="store_true", help="Create backups of files before modifying them.")
    args = parser.parse_args()

    # Warn user if not backing up
    if not args.backup:
        print("⚠️ WARNING: No backups will be created before modifying files. This may result in accidental data loss.")
        print("   Use the --backup option to create .bak files before changes are made.\n")

    # ... rest of your main logic ...
    # When modifying files, add backup logic:
    # Example:
    # if args.backup:
    #     shutil.copyfile(file_path, file_path + ".bak")
    # <then modify file as usual>

    # At the end, update instructions:
    print("\n✅ Linting issues fix completed!")
    print("\n💡 Next steps:")
    print("  1. Review the changes")
    print("  2. Test that functionality is preserved")
    print("  3. Commit the fixes")
    print("  4. Run linting tools to verify")
    print("  5. If you used --backup, verify .bak files were created for safety.")

if __name__ == "__main__":
    main()

```

You will need to:
- Integrate the backup logic into the part of your script that modifies files. Before writing changes to a file, check if `args.backup` is True and, if so, copy the file to a `.bak` version.
- Ensure that all file modification operations respect the backup option.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread scripts/fix_linting_issues.py Outdated
Comment thread scripts/fix_linting_issues.py Outdated
Comment thread scripts/fix_linting_issues.py Outdated
Comment thread scripts/fix_linting_issues.py Outdated
Comment on lines +200 to +204
print("\n💡 Next steps:")
print(" 1. Review the changes")
print(" 2. Test that functionality is preserved")
print(" 3. Commit the fixes")
print(" 4. Run linting tools to verify")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Script does not create backups before modifying files, risking accidental data loss.

Add a backup option or user warning to prevent unintended data loss when files are modified.

Suggested implementation:

import argparse
import shutil
import os

def main():
    parser = argparse.ArgumentParser(description="Fix linting issues in files.")
    parser.add_argument("--backup", action="store_true", help="Create backups of files before modifying them.")
    args = parser.parse_args()

    # Warn user if not backing up
    if not args.backup:
        print("⚠️ WARNING: No backups will be created before modifying files. This may result in accidental data loss.")
        print("   Use the --backup option to create .bak files before changes are made.\n")

    # ... rest of your main logic ...
    # When modifying files, add backup logic:
    # Example:
    # if args.backup:
    #     shutil.copyfile(file_path, file_path + ".bak")
    # <then modify file as usual>

    # At the end, update instructions:
    print("\n✅ Linting issues fix completed!")
    print("\n💡 Next steps:")
    print("  1. Review the changes")
    print("  2. Test that functionality is preserved")
    print("  3. Commit the fixes")
    print("  4. Run linting tools to verify")
    print("  5. If you used --backup, verify .bak files were created for safety.")

if __name__ == "__main__":
    main()

You will need to:

  • Integrate the backup logic into the part of your script that modifies files. Before writing changes to a file, check if args.backup is True and, if so, copy the file to a .bak version.
  • Ensure that all file modification operations respect the backup option.

Comment thread scripts/fix_linting_issues.py Outdated
Comment thread scripts/testing/config.py Outdated
Comment thread src/api_rate_limiter.py

def _analyze_user_agent(self, user_agent: str) -> int:
"""Analyze user agent for suspicious patterns. Returns score (0-10)."""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (code-quality): We've found these issues:

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new script for fixing linting issues and applies several code quality improvements across the codebase. The changes are generally positive, cleaning up formatting, improving variable names, and updating the changelog. The new lint-fixing script is a valuable addition for maintaining code quality. I've identified a few minor issues in the new script and one of the modified files, mainly related to code clarity and redundant code, which can be easily addressed.

Comment thread scripts/database/check_pgvector.py
Comment thread scripts/fix_linting_issues.py Outdated
Comment thread scripts/fix_linting_issues.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🔭 Outside diff range comments (2)
scripts/testing/config.py (1)

138-149: Potential NameError in type annotations referencing requests.Response

These annotations reference requests.Response but requests isn’t imported at module scope; absent from __future__ import annotations, this can raise at import time when annotations are evaluated. Easiest fix: quote the annotations.

-    def get(self, endpoint: str, **kwargs) -> requests.Response:
+    def get(self, endpoint: str, **kwargs) -> "requests.Response":
         """Make GET request with common configuration."""
         url = f"{self.base_url}{endpoint}"
         headers = {**self.headers, **kwargs.get('headers', {})}
         return self.session.get(url, headers=headers, timeout=self.timeout, **kwargs)

-    def post(self, endpoint: str, json_data: dict, **kwargs) -> requests.Response:
+    def post(self, endpoint: str, json_data: dict, **kwargs) -> "requests.Response":
         """Make POST request with common configuration."""
         url = f"{self.base_url}{endpoint}"
         headers = {**self.headers, **kwargs.get('headers', {})}
         return self.session.post(url, json=json_data, headers=headers, timeout=self.timeout, **kwargs)

Alternative fixes (pick one):

  • Import requests at module top and remove the function-local import in create_api_client.
  • Add from __future__ import annotations at the very top of the file to defer annotation evaluation.
src/api_rate_limiter.py (1)

141-146: Concurrency slot leak: release_request never called in middleware

allow_request increments concurrent_requests, but the middleware doesn’t call release_request after call_next. This will “leak” slots and eventually block clients. Ensure release in a finally block.

-        # Add rate limit headers
-        response = await call_next(request)
-        response.headers["X-RateLimit-Limit"] = str(self._cfg.requests_per_minute)
-        response.headers["X-RateLimit-Remaining"] = str(meta.get("tokens_remaining", 0))
-        response.headers["X-RateLimit-Reset"] = str(meta.get("reset_time", 0))
-        return response
+        # Add rate limit headers
+        try:
+            response = await call_next(request)
+            response.headers["X-RateLimit-Limit"] = str(self._cfg.requests_per_minute)
+            response.headers["X-RateLimit-Remaining"] = str(meta.get("tokens_remaining", 0))
+            response.headers["X-RateLimit-Reset"] = str(meta.get("reset_time", 0))
+            return response
+        finally:
+            # Ensure concurrent slot is released even if downstream raises
+            self._limiter.release_request(client_ip, user_agent)
🧹 Nitpick comments (17)
CHANGELOG.md (1)

7-10: Align with Keep a Changelog headings and split Added vs Changed

To better follow Keep a Changelog, consider splitting into “Added” and “Changed” (the lint-fix script is an addition; the other two are changes). Also, Unreleased sections typically omit a date (optional nit).

Apply this diff to restructure the subsection:

-### Code Quality Improvements
-- Add `scripts/fix_linting_issues.py` to automate PEP8-style fixes.
-- Improve logging and formatting in `scripts/database/check_pgvector.py`.
-- Tidy API rate limiter and testing config for readability.
+### Added
+- `scripts/fix_linting_issues.py` to automate PEP8-style fixes.
+
+### Changed
+- Improve logging and formatting in `scripts/database/check_pgvector.py`.
+- Tidy API rate limiter and testing config for readability.
scripts/testing/config.py (1)

30-33: Fix multi-line continuation indentation for env URL resolution

The continuation indentation here is non-standard and may trigger linters (E127/E128). Recommend aligning after the opening parenthesis or using a vertical “or” layout.

-        env_url = (os.environ.get("API_BASE_URL") or
-                      os.environ.get("CLOUD_RUN_API_URL") or
-                  os.environ.get("MODEL_API_BASE_URL"))
+        env_url = (
+            os.environ.get("API_BASE_URL")
+            or os.environ.get("CLOUD_RUN_API_URL")
+            or os.environ.get("MODEL_API_BASE_URL")
+        )
deployment/cloud-run/debug_api_import.py (2)

3-3: Remove over-indentation in the docstring line

Ruff D208: one-line docstrings shouldn’t be indented. Dedent the content.

-    Debug script to isolate the 'int' object is not callable error
+Debug script to isolate the 'int' object is not callable error

97-97: Printing in scripts: either switch to logging or ignore T201 for this file

Ruff T201 flags print. Since this is an ad-hoc debug script, printing is acceptable. If you prefer to keep prints, annotate to avoid lint noise; otherwise, switch to logging.

Option A (suppress for this line):

-print("\n🔍 Debug complete. Check above for any import issues.")
+print("\n🔍 Debug complete. Check above for any import issues.")  # noqa: T201

Option B (per-file ignore in pyproject.toml):

[tool.ruff]
per-file-ignores = { "deployment/cloud-run/debug_api_import.py" = ["T201"] }
src/api_rate_limiter.py (1)

347-353: Modernize return type to built-in generics (ruff UP006)

Adopt PEP 585 built-in generics for annotations.

-    def allow_request(self, client_ip: str, user_agent: str = "") -> Tuple[bool, str, Dict]:
+    def allow_request(self, client_ip: str, user_agent: str = "") -> tuple[bool, str, dict]:

Follow-up: you can also drop Dict/Tuple imports from typing once migrated across the file.

scripts/database/check_pgvector.py (2)

86-89: Log database errors at error level and include traceback

Currently uses info level; elevate to error with exc_info for diagnosability.

-    except psycopg2.Error as e:
-        logging.info(f"Error connecting to PostgreSQL: {e}")
-        return False
+    except psycopg2.Error as e:
+        logging.error("Error connecting to PostgreSQL", exc_info=True)
+        return False

59-61: Minor: align the comment indentation with surrounding code

Pure nit: the comment is over-indented relative to the code block.

-            # Create a cursor
+        # Create a cursor
scripts/fix_linting_issues.py (10)

2-6: Docstring: generalize tool reference and scope

DeepSource is mentioned explicitly, but the script is generic. Small tweak to reflect that it targets common linters.

-"""
-🔧 SAMO Linting Issues Fix Script
-==================================
-Fixes trailing whitespace and indentation issues identified by DeepSource.
-"""
+"""
+🔧 SAMO Linting Issues Fix Script
+==================================
+Fixes trailing whitespace, stray blank-line whitespace, and simple continuation-indentation issues flagged by common linters (e.g., Ruff/Flake8). Use with care.
+"""

10-10: Adopt built-in generics (PEP 585) and drop typing.List/Tuple

Ruff UP006 suggests using list/tuple generics. Apply if your minimum Python is 3.9+. Otherwise keep current typing.

Please confirm the project’s minimum Python version is >= 3.9 before applying.

-from typing import List, Tuple
+# (typing import no longer needed for generics on Python 3.9+)

-def find_python_files(project_root: Path) -> List[Path]:
+def find_python_files(project_root: Path) -> list[Path]:

-def fix_trailing_whitespace(file_path: Path) -> Tuple[bool, List[str]]:
+def fix_trailing_whitespace(file_path: Path) -> tuple[bool, list[str]]:

-def fix_indentation_issues(file_path: Path) -> Tuple[bool, List[str]]:
+def fix_indentation_issues(file_path: Path) -> tuple[bool, list[str]]:

-def fix_blank_lines_with_whitespace(file_path: Path) -> Tuple[bool, List[str]]:
+def fix_blank_lines_with_whitespace(file_path: Path) -> tuple[bool, list[str]]:

Also applies to: 12-12, 25-25, 60-60, 102-102


28-28: Remove unnecessary open mode 'r'

Ruff UP015: 'r' is the default for open(). This is minor but keeps things idiomatic.

-        with open(file_path, 'r', encoding='utf-8') as f:
+        with open(file_path, encoding='utf-8') as f:

Apply similarly at other read sites:

-        with open(file_path, 'r', encoding='utf-8') as f:
+        with open(file_path, encoding='utf-8') as f:

Also applies to: 63-63, 105-105


36-44: Avoid reassigning loop variable; drop unused _

This fixes PLW2901 and removes the unused _. Behavior unchanged.

-        for i, line in enumerate(lines, 1):
-            # Remove trailing whitespace
-            if line.rstrip() != line:
-                _ = line  # Store original line for reference (renamed from original_line)
-                line = line.rstrip()
-                issues_fixed.append(f"Line {i}: Removed trailing whitespace")
-
-            fixed_lines.append(line)
+        for i, line in enumerate(lines, 1):
+            stripped = line.rstrip()
+            if stripped != line:
+                issues_fixed.append(f"Line {i}: Removed trailing whitespace")
+            fixed_lines.append(stripped)

Note: This function normalizes newlines to LF and enforces a trailing newline. If your repo intentionally preserves CRLF or mixed endings, we can preserve original newline style instead. Happy to provide a variant that uses splitlines(keepends=True) to avoid converting line endings.


57-59: Include file path in error messages

Makes failures actionable when they occur across many files.

-    except Exception as e:
-        return False, [f"Error processing file: {e}"]
+    except Exception as e:
+        return False, [f"Error processing {file_path}: {e}"]

Also applies to: 99-101, 133-135


113-120: Avoid reassigning loop variable in blank-line fixer

Addresses PLW2901 and keeps the logic straightforward.

-        for i, line in enumerate(lines, 1):
-            # Check if line is blank but contains whitespace
-            if not line.strip() and line != '':
-                issues_fixed.append(f"Line {i}: Removed whitespace from blank line")
-                line = ''
-
-            fixed_lines.append(line)
+        for i, line in enumerate(lines, 1):
+            # Check if line is blank but contains whitespace
+            if not line.strip() and line != '':
+                issues_fixed.append(f"Line {i}: Removed whitespace from blank line")
+                fixed_lines.append('')
+            else:
+                fixed_lines.append(line)

157-157: Remove unused _ toggle in main

_ is used as a discard variable by convention; using it as a real flag reduces readability and it’s not used elsewhere. Safe to remove these assignments.

-        _ = False  # Track if file was modified (renamed from file_fixed)
+        # (removed unused file-modified toggle)

-        if fixed:
-            _ = True
+        if fixed:
             file_issues.extend(issues)

-        if fixed:
-            _ = True
+        if fixed:
             file_issues.extend(issues)

-        if fixed:
-            _ = True
+        if fixed:
             file_issues.extend(issues)

Also applies to: 163-163, 169-169, 175-175


12-23: Optional: respect .gitignore or track-only files

Walking the entire tree may touch vendored or generated Python files outside the coarse skip list. Consider using git ls-files to operate only on tracked .py files, or parse .gitignore to skip ignored paths. This limits churn and speeds up runs.

If you want, I can provide a variant that uses git ls-files with a fallback to os.walk when Git isn’t available.


25-55: Optional: preserve original newline style

The current implementation normalizes newlines to LF and enforces a trailing newline, which may be intended. If you need to preserve per-file newline style (CRLF vs LF), we can switch to splitlines(keepends=True) and only strip trailing spaces before the newline.

Want me to provide a patch that preserves native newline endings while still removing trailing whitespace?


8-11: Avoid duplication with existing maintenance fixer

There’s already scripts/maintenance/fix_code_quality.py with a trailing whitespace fixer. Consolidating logic (shared utility module or reuse) avoids drift and double-maintenance of similar functionality.

Would you like a follow-up patch to extract a shared helper (e.g., scripts/lib/quality.py) and have both scripts import it?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 560f195 and 4af601a.

📒 Files selected for processing (6)
  • CHANGELOG.md (1 hunks)
  • deployment/cloud-run/debug_api_import.py (2 hunks)
  • scripts/database/check_pgvector.py (2 hunks)
  • scripts/fix_linting_issues.py (1 hunks)
  • scripts/testing/config.py (2 hunks)
  • src/api_rate_limiter.py (11 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
scripts/fix_linting_issues.py (1)
scripts/maintenance/fix_code_quality.py (2)
  • fix_trailing_whitespace (100-104)
  • main (158-162)
src/api_rate_limiter.py (1)
deployment/secure_api_server.py (2)
  • add_to_blacklist (604-617)
  • add_to_whitelist (621-634)
🪛 Ruff (0.12.2)
deployment/cloud-run/debug_api_import.py

3-3: Docstring is over-indented

Remove over-indentation

(D208)


97-97: print found

Remove print

(T201)

scripts/fix_linting_issues.py

12-12: Use list instead of List for type annotation

Replace with list

(UP006)


25-25: Use tuple instead of Tuple for type annotation

Replace with tuple

(UP006)


25-25: Use list instead of List for type annotation

Replace with list

(UP006)


28-28: Unnecessary mode argument

Remove mode argument

(UP015)


40-40: for loop variable line overwritten by assignment target

(PLW2901)


60-60: Use tuple instead of Tuple for type annotation

Replace with tuple

(UP006)


60-60: Use list instead of List for type annotation

Replace with list

(UP006)


63-63: Unnecessary mode argument

Remove mode argument

(UP015)


82-82: for loop variable line overwritten by assignment target

(PLW2901)


102-102: Use tuple instead of Tuple for type annotation

Replace with tuple

(UP006)


102-102: Use list instead of List for type annotation

Replace with list

(UP006)


105-105: Unnecessary mode argument

Remove mode argument

(UP015)


117-117: for loop variable line overwritten by assignment target

(PLW2901)

src/api_rate_limiter.py

347-347: Use tuple instead of Tuple for type annotation

Replace with tuple

(UP006)


347-347: Use dict instead of Dict for type annotation

Replace with dict

(UP006)

⏰ 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). (1)
  • GitHub Check: Analyze (python)
🔇 Additional comments (3)
scripts/testing/config.py (1)

202-202: LGTM on formatting touch-up

The closing brace reindent in the exception path is harmless and improves consistency.

src/api_rate_limiter.py (1)

247-251: Readability improvement in compound any(...) condition looks good

The adjusted indentation of the combined UA heuristics improves clarity without changing behavior.

scripts/database/check_pgvector.py (1)

66-79: Great UX: actionable guidance when pgvector isn’t installed

Clear installation and enablement steps, plus the explicit boolean return, improve the script’s utility and calling-code ergonomics.

Comment thread scripts/fix_linting_issues.py Outdated
Comment thread scripts/fix_linting_issues.py Outdated
cursoragent and others added 3 commits August 16, 2025 14:04
Co-authored-by: denizcan.uelker <denizcan.uelker@mercedes-benz.com>
…used vars and redundant flag; AST-only indentation detection; add backup option and efficient line-by-line whitespace fix; configurable excluded dirs; consistent 4-space indentation\n- check_pgvector: fix mis-indented comment to match PEP8
…to Added/Changed sections\n- testing/config: fix multiline continuation indent for env URL\n- debug script: dedent one-line docstring; suppress T201 on final print\n- rate_limiter: adopt built-in generics for allow_request return type\n- check_pgvector: log DB errors at error level with traceback

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
scripts/fix_linting_issues.py (1)

150-176: Backup can be overwritten when multiple fixers modify the same file; also remove unused flag

Currently each fixer creates a .bak, so later fixers overwrite the original backup. Create the backup once per file before the first modification. Also, file_fixed is assigned but never used (ruff F841).

-        file_fixed = False  # Track if file was modified
+        backup_created = False  # Ensure we only create one backup per file
         fixed_issues: List[str] = []
         detected_issues: List[str] = []
 
         # Fix trailing whitespace
-        fixed, issues = fix_trailing_whitespace(file_path, backup=args.backup)
+        fixed, issues = fix_trailing_whitespace(file_path, backup=(args.backup and not backup_created))
         if issues:
             if fixed:
-                file_fixed = True
+                if args.backup and not backup_created:
+                    backup_created = True
                 fixed_issues.extend(issues)
             else:
                 detected_issues.extend(issues)
 
         # Detect indentation issues (no auto-fix)
         fixed, issues = fix_indentation_issues(file_path)
         if issues:
             # These are detections only; no modifications performed here
             detected_issues.extend(issues)
 
         # Fix blank lines with whitespace
-        fixed, issues = fix_blank_lines_with_whitespace(file_path, backup=args.backup)
+        fixed, issues = fix_blank_lines_with_whitespace(file_path, backup=(args.backup and not backup_created))
         if issues:
             if fixed:
-                file_fixed = True
+                if args.backup and not backup_created:
+                    backup_created = True
                 fixed_issues.extend(issues)
             else:
                 detected_issues.extend(issues)

If you prefer to centralize backups, an alternative is to create the backup once at the start of processing each file when args.backup is set and before invoking any fixers.

🧹 Nitpick comments (7)
scripts/fix_linting_issues.py (7)

3-6: Docstring misrepresents current behavior (indentation is detected, not auto-fixed)

The script no longer attempts to auto-fix indentation; it only detects it via AST. Reflect that in the docstring to avoid surprising users.

 """
 🔧 SAMO Linting Issues Fix Script
 ==================================
-Fixes trailing whitespace and indentation issues identified by DeepSource.
+Fixes trailing whitespace and blank-line whitespace; detects indentation/syntax issues (no auto-fix).
 """

69-70: Drop redundant open mode 'r' in read-only contexts

The default mode for open is 'r'. Removing it satisfies ruff UP015 and keeps code idiomatic.

-        with open(file_path, 'r', encoding='utf-8') as f:
+        with open(file_path, encoding='utf-8') as f:
@@
-        with open(file_path, 'r', encoding='utf-8') as f:
+        with open(file_path, encoding='utf-8') as f:

Also applies to: 87-88


95-103: Avoid reassigning the loop variable (ruff PLW2901); compute an output line instead

This prevents potential confusion and linter warnings.

-        for i, line in enumerate(lines, 1):
-            # Check if line is blank but contains whitespace
-            if not line.strip() and line != '':
-                issues_fixed.append(f"Line {i}: Removed whitespace from blank line")
-                line = ''
-
-            fixed_lines.append(line)
+        for i, line in enumerate(lines, 1):
+            # Check if line is blank but contains whitespace
+            blank_with_ws = not line.strip() and line != ''
+            out_line = '' if blank_with_ws else line
+            if blank_with_ws:
+                issues_fixed.append(f"Line {i}: Removed whitespace from blank line")
+            fixed_lines.append(out_line)

24-33: Use list-extend with a generator for clarity and a minor speed-up

This simplifies the nested loop in find_python_files.

-    python_files = []
+    python_files: List[Path] = []
@@
-        for file in files:
-            if file.endswith('.py'):
-                python_files.append(Path(root) / file)
+        python_files.extend(
+            Path(root) / file for file in files if file.endswith('.py')
+        )

13-14: Adopt PEP 585 built-in generics (ruff UP006) and trim typing imports

This keeps annotations modern and consistent with the rest of the codebase.

-from typing import List, Tuple, Optional, Set
+from typing import Optional
@@
-def find_python_files(project_root: Path, excluded_dirs: Optional[Set[str]] = None) -> List[Path]:
+def find_python_files(project_root: Path, excluded_dirs: Optional[set[str]] = None) -> list[Path]:
@@
-def fix_trailing_whitespace(file_path: Path, backup: bool = False) -> Tuple[bool, List[str]]:
+def fix_trailing_whitespace(file_path: Path, backup: bool = False) -> tuple[bool, list[str]]:
-    issues_fixed: List[str] = []
+    issues_fixed: list[str] = []
@@
-def fix_indentation_issues(file_path: Path) -> Tuple[bool, List[str]]:
+def fix_indentation_issues(file_path: Path) -> tuple[bool, list[str]]:
@@
-def fix_blank_lines_with_whitespace(file_path: Path, backup: bool = False) -> Tuple[bool, List[str]]:
+def fix_blank_lines_with_whitespace(file_path: Path, backup: bool = False) -> tuple[bool, list[str]]:
@@
-        fixed_lines: List[str] = []
-        issues_fixed: List[str] = []
+        fixed_lines: list[str] = []
+        issues_fixed: list[str] = []
@@
-    all_issues: List[str] = []
+    all_issues: list[str] = []
@@
-        fixed_issues: List[str] = []
-        detected_issues: List[str] = []
+        fixed_issues: list[str] = []
+        detected_issues: list[str] = []

Also applies to: 15-16, 35-39, 66-67, 84-85, 92-93, 144-145, 151-153


15-23: Optional: promote excluded dirs to a module-level constant for reuse/configurability

Slightly improves readability and makes it easier to share with other scripts.

Add near the imports:

EXCLUDED_DIRS: set[str] = {
    '.git', '__pycache__', '.venv', 'venv', 'node_modules', 'build', 'dist',
    '.mypy_cache', '.pytest_cache', '.cache', '.coverage', '.eggs', '.tox',
    '.idea', '.vscode'
}

Then inside find_python_files:

if excluded_dirs is None:
    excluded_dirs = EXCLUDED_DIRS

I intentionally omitted '.DS_Store' because it's a file, not a directory.


120-120: Optional: reduce branching in main by extracting a process_file() helper

Ruff flags PLR0912 (15 > 12 branches). Not urgent, but factoring the per-file logic into a helper would improve readability and testability.

Happy to draft a small process_file(file_path, backup) helper and associated tests if you'd like.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4af601a and e40b223.

📒 Files selected for processing (1)
  • scripts/fix_linting_issues.py (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
scripts/fix_linting_issues.py (1)
scripts/maintenance/fix_code_quality.py (2)
  • fix_trailing_whitespace (100-104)
  • main (158-162)
🪛 Ruff (0.12.2)
scripts/fix_linting_issues.py

15-15: Use set instead of Set for type annotation

Replace with set

(UP006)


15-15: Use list instead of List for type annotation

Replace with list

(UP006)


35-35: Use tuple instead of Tuple for type annotation

Replace with tuple

(UP006)


35-35: Use list instead of List for type annotation

Replace with list

(UP006)


38-38: Use list instead of List for type annotation

Replace with list

(UP006)


40-40: Unnecessary mode argument

Remove mode argument

(UP015)


53-53: os.replace() should be replaced by Path.replace()

(PTH105)


55-55: os.remove() should be replaced by Path.unlink()

(PTH107)


61-61: os.remove() should be replaced by Path.unlink()

(PTH107)


62-63: try-except-pass detected, consider logging the exception

(S110)


66-66: Use tuple instead of Tuple for type annotation

Replace with tuple

(UP006)


66-66: Use list instead of List for type annotation

Replace with list

(UP006)


69-69: Unnecessary mode argument

Remove mode argument

(UP015)


84-84: Use tuple instead of Tuple for type annotation

Replace with tuple

(UP006)


84-84: Use list instead of List for type annotation

Replace with list

(UP006)


87-87: Unnecessary mode argument

Remove mode argument

(UP015)


92-92: Use list instead of List for type annotation

Replace with list

(UP006)


93-93: Use list instead of List for type annotation

Replace with list

(UP006)


99-99: for loop variable line overwritten by assignment target

(PLW2901)


120-120: Too many branches (15 > 12)

(PLR0912)


144-144: Use list instead of List for type annotation

Replace with list

(UP006)


151-151: Use list instead of List for type annotation

Replace with list

(UP006)


152-152: Use list instead of List for type annotation

Replace with list

(UP006)


173-173: Local variable file_fixed is assigned to but never used

Remove assignment to unused variable file_fixed

(F841)

⏰ 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). (1)
  • GitHub Check: Analyze (python)
🔇 Additional comments (1)
scripts/fix_linting_issues.py (1)

66-81: LGTM on switching indentation handling to detection-only

Replacing heuristic indentation edits with AST-based detection eliminates risky auto-modifications in Python code.

Comment thread scripts/fix_linting_issues.py Outdated
…lize docstring; adopt built-in generics; remove redundant 'r' mode for open; include file path in error messages; use extend() for file discovery\n- testing config: simplify env var resolution with walrus op

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
scripts/database/check_pgvector.py (1)

59-61: Comment indentation fix looks good

The “Create a cursor” comment is now properly aligned with the code it describes.

🧹 Nitpick comments (9)
scripts/database/check_pgvector.py (4)

86-88: Use logging.exception and drop unused exception variable

Replace the unused e binding and prefer logging.exception(...) over logging.error(..., exc_info=True) to satisfy linters and improve readability.

-    except psycopg2.Error as e:
-        logging.error("Error connecting to PostgreSQL", exc_info=True)
+    except psycopg2.Error:
+        logging.exception("Error connecting to PostgreSQL")
         return False

49-57: Close connection/cursor reliably with context managers

Use context managers to ensure the cursor and connection are properly closed even if an exception occurs after use. This also removes the need for manual close calls.

-        conn = psycopg2.connect(
-            dbname=DB_NAME,
-            user=DB_USER,
-            password=DB_PASSWORD,
-            host=DB_HOST,
-            port=DB_PORT,
-        )
-        conn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
-
-        # Create a cursor
-        cur = conn.cursor()
-
-        # Check if vector extension is available
-        cur.execute("SELECT extname FROM pg_extension WHERE extname = 'vector';")
-        extension_installed = cur.fetchone() is not None
+        with psycopg2.connect(
+            dbname=DB_NAME,
+            user=DB_USER,
+            password=DB_PASSWORD,
+            host=DB_HOST,
+            port=DB_PORT,
+        ) as conn:
+            conn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
+
+            # Create a cursor
+            with conn.cursor() as cur:
+                # Check if vector extension is available
+                cur.execute("SELECT extname FROM pg_extension WHERE extname = 'vector';")
+                extension_installed = cur.fetchone() is not None
@@
-        # Close cursor and connection
-        cur.close()
-        conn.close()
+        # Cursor and connection are closed by context managers

Also applies to: 59-61, 80-83


91-93: Ensure info-level log messages are visible when running as a script

By default, logging is WARNING+. Configure the root logger in the CLI path so users see the guidance messages.

 if __name__ == "__main__":
+    logging.basicConfig(level=logging.INFO, format="%(message)s")
     is_installed = check_pgvector()
     sys.exit(0 if is_installed else 1)

72-74: Make apt package hint version-agnostic

The package name is PostgreSQL-major-version-specific on Debian/Ubuntu. Suggest a version-agnostic hint to avoid confusion.

-            logging.info("   - On Ubuntu/Debian: sudo apt install postgresql-15-pgvector")
+            logging.info("   - On Ubuntu/Debian: sudo apt install 'postgresql-<version>-pgvector'  # e.g., 14/15/16")
             logging.info("   - On macOS with Homebrew: brew install pgvector")
             logging.info("   - From source: https://github.com/pgvector/pgvector#installation")
scripts/fix_linting_issues.py (5)

13-13: Modernize type hints to built-in generics (PEP 585)

Adopt list, set, and tuple built-ins for annotations and drop unused imports. Aligns with Ruff UP006.

-from typing import List, Tuple, Optional, Set
+from typing import Optional
@@
-def find_python_files(project_root: Path, excluded_dirs: Optional[Set[str]] = None) -> List[Path]:
+def find_python_files(project_root: Path, excluded_dirs: Optional[set[str]] = None) -> list[Path]:
@@
-def fix_trailing_whitespace(file_path: Path, backup: bool = False) -> Tuple[bool, List[str]]:
+def fix_trailing_whitespace(file_path: Path, backup: bool = False) -> tuple[bool, list[str]]:
@@
-    issues_fixed: List[str] = []
+    issues_fixed: list[str] = []
@@
-def fix_indentation_issues(file_path: Path) -> Tuple[bool, List[str]]:
+def fix_indentation_issues(file_path: Path) -> tuple[bool, list[str]]:
@@
-def fix_blank_lines_with_whitespace(file_path: Path, backup: bool = False) -> Tuple[bool, List[str]]:
+def fix_blank_lines_with_whitespace(file_path: Path, backup: bool = False) -> tuple[bool, list[str]]:
@@
-        fixed_lines: List[str] = []
-        issues_fixed: List[str] = []
+        fixed_lines: list[str] = []
+        issues_fixed: list[str] = []
@@
-    all_issues: List[str] = []
+    all_issues: list[str] = []
@@
-        fixed_issues: List[str] = []
-        detected_issues: List[str] = []
+        fixed_issues: list[str] = []
+        detected_issues: list[str] = []

Also applies to: 15-15, 35-35, 38-38, 66-66, 84-84, 92-93, 144-144, 150-151


8-13: Pathlib-based replace/unlink, safer temp cleanup, and minor I/O tweaks

Use Path.replace/unlink (ruff PTH105/PTH107), drop redundant 'r' mode (UP015), and prefer contextlib.suppress for quiet cleanup. Also simplify backup path.

 import os
+import contextlib
 import argparse
 import shutil
 import tempfile
 from pathlib import Path
 from typing import List, Tuple, Optional, Set
@@
-    try:
-        with open(file_path, 'r', encoding='utf-8') as src, tempfile.NamedTemporaryFile('w', delete=False, encoding='utf-8') as tmp:
+    try:
+        with open(file_path, encoding='utf-8') as src, tempfile.NamedTemporaryFile('w', delete=False, encoding='utf-8') as tmp:
             for i, line in enumerate(src, 1):
                 # Remove trailing whitespace (including tabs/spaces) and normalize newline
                 stripped_line_no_nl = line.rstrip('\r\n')
                 stripped_line = stripped_line_no_nl.rstrip()
                 if stripped_line != stripped_line_no_nl:
                     changed = True
                     issues_fixed.append(f"Line {i}: Removed trailing whitespace")
                 tmp.write(stripped_line + '\n')
         # If content changed, optionally back up and replace
         if changed:
             if backup:
-                shutil.copyfile(file_path, str(file_path) + '.bak')
-            os.replace(tmp.name, file_path)
+                shutil.copyfile(file_path, f"{file_path}.bak")
+            Path(tmp.name).replace(file_path)
         else:
-            os.remove(tmp.name)
+            Path(tmp.name).unlink(missing_ok=True)
         return changed, issues_fixed
     except Exception as e:
         # Best-effort cleanup of temp file if it still exists
-        try:
-            if 'tmp' in locals():
-                os.remove(tmp.name)
-        except Exception:
-            pass
+        if 'tmp' in locals():
+            with contextlib.suppress(FileNotFoundError):
+                Path(tmp.name).unlink()
         return False, [f"Error processing file: {e}"]

Also applies to: 35-65


69-69: Drop redundant read mode in open()

open(..., 'r', ...) is redundant; default mode is read. This satisfies Ruff UP015.

-        with open(file_path, 'r', encoding='utf-8') as f:
+        with open(file_path, encoding='utf-8') as f:
@@
-        with open(file_path, 'r', encoding='utf-8') as f:
+        with open(file_path, encoding='utf-8') as f:

Also applies to: 87-87


95-103: Avoid overwriting loop variable in-place (PLW2901); simplify blank-line handling

Write the chosen value directly to fixed_lines without rebinding line.

-        for i, line in enumerate(lines, 1):
+        for i, line in enumerate(lines, 1):
             # Check if line is blank but contains whitespace
             if not line.strip() and line != '':
                 issues_fixed.append(f"Line {i}: Removed whitespace from blank line")
-                line = ''
-
-            fixed_lines.append(line)
+                fixed_lines.append('')
+                continue
+
+            fixed_lines.append(line)

29-33: Minor loop simplification

Use a comprehension with extend to tighten the file collection loop.

-        for file in files:
-            if file.endswith('.py'):
-                python_files.append(Path(root) / file)
+        python_files.extend(
+            Path(root) / file for file in files if file.endswith('.py')
+        )
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e40b223 and 814ce9b.

📒 Files selected for processing (6)
  • CHANGELOG.md (1 hunks)
  • deployment/cloud-run/debug_api_import.py (1 hunks)
  • scripts/database/check_pgvector.py (2 hunks)
  • scripts/fix_linting_issues.py (1 hunks)
  • scripts/testing/config.py (2 hunks)
  • src/api_rate_limiter.py (11 hunks)
✅ Files skipped from review due to trivial changes (1)
  • scripts/testing/config.py
🚧 Files skipped from review as they are similar to previous changes (3)
  • CHANGELOG.md
  • deployment/cloud-run/debug_api_import.py
  • src/api_rate_limiter.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
scripts/fix_linting_issues.py (1)
scripts/maintenance/fix_code_quality.py (2)
  • fix_trailing_whitespace (100-104)
  • main (158-162)
🪛 Ruff (0.12.2)
scripts/database/check_pgvector.py

86-86: Local variable e is assigned to but never used

Remove assignment to unused variable e

(F841)


87-87: Logging .exception(...) should be used instead of .error(..., exc_info=True)

(G201)

scripts/fix_linting_issues.py

15-15: Use set instead of Set for type annotation

Replace with set

(UP006)


15-15: Use list instead of List for type annotation

Replace with list

(UP006)


35-35: Use tuple instead of Tuple for type annotation

Replace with tuple

(UP006)


35-35: Use list instead of List for type annotation

Replace with list

(UP006)


38-38: Use list instead of List for type annotation

Replace with list

(UP006)


40-40: Unnecessary mode argument

Remove mode argument

(UP015)


53-53: os.replace() should be replaced by Path.replace()

(PTH105)


55-55: os.remove() should be replaced by Path.unlink()

(PTH107)


61-61: os.remove() should be replaced by Path.unlink()

(PTH107)


62-63: try-except-pass detected, consider logging the exception

(S110)


66-66: Use tuple instead of Tuple for type annotation

Replace with tuple

(UP006)


66-66: Use list instead of List for type annotation

Replace with list

(UP006)


69-69: Unnecessary mode argument

Remove mode argument

(UP015)


84-84: Use tuple instead of Tuple for type annotation

Replace with tuple

(UP006)


84-84: Use list instead of List for type annotation

Replace with list

(UP006)


87-87: Unnecessary mode argument

Remove mode argument

(UP015)


92-92: Use list instead of List for type annotation

Replace with list

(UP006)


93-93: Use list instead of List for type annotation

Replace with list

(UP006)


99-99: for loop variable line overwritten by assignment target

(PLW2901)


120-120: Too many branches (15 > 12)

(PLR0912)


144-144: Use list instead of List for type annotation

Replace with list

(UP006)


150-150: Use list instead of List for type annotation

Replace with list

(UP006)


151-151: Use list instead of List for type annotation

Replace with list

(UP006)

🔇 Additional comments (1)
scripts/fix_linting_issues.py (1)

120-208: Overall: solid, safe defaults and clear UX

Nice job making indentation checks non-destructive and providing a backup option, per-file reporting, and a summary. The script aligns well with the PR objectives.

cursoragent and others added 13 commits August 16, 2025 14:18
…erics everywhere; include file path in errors; extend() for discovery\n- check_pgvector: keep lines under limit if practical (no functional change)
…ops; minor IO tweaks\n\n- check_pgvector: use with-statement for conn/cursor; logging.exception; version-agnostic apt hint; configure logging in __main__\n- fix_linting_issues: pathlib/contextlib for temp cleanup; built-in generics already updated; cleaner backup and open usage
Co-authored-by: denizcan.uelker <denizcan.uelker@mercedes-benz.com>
@d-ulker
d-ulker merged commit cc57b4e into main Aug 16, 2025
12 of 13 checks passed
@d-ulker
d-ulker deleted the fix/code-quality-only branch August 16, 2025 14:45
d-ulker added a commit that referenced this pull request Sep 7, 2025
chore: add lint-fix script; minor code quality improvements in db check, rate limiter, testing config; update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants