Skip to content

Conversation

@marcorudolphflex
Copy link
Contributor

@marcorudolphflex marcorudolphflex commented Oct 29, 2025

Add mypy checks to our CI pipeline.
Is done immediately after linting and uses the config from pyproject.toml.

Greptile Overview

Updated On: 2025-10-29 15:37:34 UTC

Greptile Summary

Adds mypy static type checking to the CI pipeline to enforce type safety in the tidy3d/web directory. The implementation is clean and well-integrated.

Key Changes:

  • New mypy job in CI workflow that runs after the determine-test-scope job (only when code_quality_tests == true)
  • Installs mypy version 1.13.0 and executes type checks using configuration from pyproject.toml
  • Pre-commit hook updated from file-based filtering (files: ^tidy3d/web) to config-based filtering (pass_filenames: false)
  • Added files = ["tidy3d/web"] configuration in pyproject.toml to specify the directory for type checking
  • Mypy job properly integrated into pr-requirements-pass with a dedicated validation step that fails the pipeline if type checks fail

Implementation Notes:

  • The mypy job uses Python 3.10, consistent with the project's mypy configuration
  • The job correctly depends on determine-test-scope and only runs when code quality tests are enabled
  • Both CI and pre-commit hook use the same configuration source (pyproject.toml), ensuring consistency

Confidence Score: 5/5

  • This PR is safe to merge - all changes are correctly implemented with proper CI integration
  • Score of 5 reflects clean implementation: (1) mypy job is properly configured with hardcoded Python 3.10 version, (2) correctly integrated into pr-requirements-pass job dependencies with validation step, (3) pre-commit hook properly delegates file selection to config, (4) all three files work together cohesively
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
.github/workflows/tidy3d-python-client-tests.yml 5/5 Adds mypy job to CI that installs mypy 1.13.0, runs type checks with config from pyproject.toml, properly integrated into pr-requirements-pass job with validation step
.pre-commit-config.yaml 5/5 Changes mypy hook from file-based filtering (files: ^tidy3d/web) to config-based filtering (pass_filenames: false), correctly delegating file selection to pyproject.toml
pyproject.toml 5/5 Adds files = ["tidy3d/web"] to mypy configuration to specify which directory to type check, properly configured to work with CI and pre-commit hook

Sequence Diagram

sequenceDiagram
    participant PR as Pull Request
    participant Scope as determine-test-scope
    participant Lint as lint job
    participant Mypy as mypy job
    participant Final as pr-requirements-pass

    PR->>Scope: Trigger workflow
    Scope->>Scope: Determine if code_quality_tests=true
    
    alt code_quality_tests == true
        Scope->>Lint: Run ruff linting
        Scope->>Mypy: Run mypy type checks
        
        Lint->>Lint: Check formatting and style
        Lint-->>Final: Report result
        
        Mypy->>Mypy: Checkout code
        Mypy->>Mypy: Setup Python 3.10
        Mypy->>Mypy: Install mypy==1.13.0
        Mypy->>Mypy: Run mypy --config-file=pyproject.toml
        Note over Mypy: Reads files from pyproject.toml<br/>(tidy3d/web directory)
        Mypy-->>Final: Report result
        
        Final->>Final: Check lint result
        Final->>Final: Check mypy result (if code_quality_tests)
        Final->>Final: Validate all required jobs
        Final-->>PR: Pass/Fail result
    end
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (2)

  1. .github/workflows/tidy3d-python-client-tests.yml, line 661-669 (link)

    logic: Add mypy job to the needs list since it's a code quality check that should be required for PR approval

  2. .github/workflows/tidy3d-python-client-tests.yml, line 670-679 (link)

    logic: Add mypy result check similar to other code quality checks

          - name: check-mypy-result
            if: ${{ needs.determine-test-scope.outputs.code_quality_tests == 'true' && needs.mypy.result != 'success' }}
            run: |
              echo "❌ Mypy type checking failed."
              exit 1
    

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@marcorudolphflex
Copy link
Contributor Author

@greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Collaborator

@yaugenst-flex yaugenst-flex left a comment

Choose a reason for hiding this comment

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

LGTM!

@github-actions
Copy link
Contributor

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

@marcorudolphflex marcorudolphflex added this pull request to the merge queue Oct 29, 2025
Merged via the queue into develop with commit 2884777 Oct 29, 2025
56 of 58 checks passed
@marcorudolphflex marcorudolphflex deleted the FXC-3718-add-mypy-to-CI branch October 29, 2025 16:45
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