Skip to content

Conversation

@dguido
Copy link
Member

@dguido dguido commented Jul 29, 2025

Summary

This PR enhances the cookiecutter-python template with stricter code quality standards based on Trail of Bits' engineering practices. The changes enforce consistent, high-quality Python code across all generated projects.

Key Changes

1. Modern Python Tooling

  • Python 3.11+ requirement (removed 3.9/3.10 support)
  • pyright replaces mypy for strict type checking
  • ruff consolidated configuration for linting and formatting
  • uv package manager with pre-commit hooks

2. Strict Code Quality Standards

Enforces quantitative code complexity limits:

  • Cyclomatic complexity ≤ 8
  • Max 50 lines per function
  • Max 5 positional parameters
  • Max 12 branches per function
  • Max 6 return statements
  • 100-character line length
  • Google-style docstrings required

3. Enhanced Developer Experience

  • Pre-commit hooks run automatically:
    • ruff format and ruff check --fix
    • pyright strict type checking
    • pytest (fast tests only, excluding @pytest.mark.slow)
    • interrogate docstring coverage (if enabled)
  • Simplified Makefile with intuitive aliases:
    • make fix - primary formatter command
    • make lint - all quality checks
    • make typecheck - just type checking
  • Tests beside code - supports test_*.py and *_test.py in src/

4. Project Documentation

  • CLAUDE.md template - AI assistant instructions with:
    • Project-specific code standards
    • Quick command reference
    • Testing guidelines with pytest markers
    • General Python preferences (FastAPI, Polars, etc.)
  • Clear guidance on maintaining code quality

5. Configuration Improvements

  • Consolidated ruff config in pyproject.toml (removed separate ruff.toml)
  • Fixed license field to modern SPDX format
  • Proper pytest configuration for co-located tests
  • Pre-commit stages fixed (changed deprecated stages: [commit] to stages: [pre-commit])

What Changed From Initial PR

After code review and testing, we:

  • Replaced experimental ty with stable pyright
  • Removed data library option (rarely needed for Trail of Bits projects)
  • Removed FastAPI web framework option (too complex for template)
  • Fixed all CI failures and deprecation warnings
  • Simplified formatter aliases in Makefile
  • Added pytest marker documentation for slow test filtering

Test Plan

  • CI passes all checks
  • Generated projects pass all linting/type checks
  • Pre-commit hooks work correctly
  • Makefile targets function as expected
  • Documentation is clear and accurate

🤖 Generated with Claude Code

dguido and others added 15 commits July 29, 2025 18:58
- Update Python requirement to 3.11+ to match CLAUDE.md standards
- Replace mypy with Astral's experimental ty type checker
- Add comprehensive ruff.toml with strict code quality constraints:
  - Cyclomatic complexity ≤ 8
  - Max 5 positional args, 12 branches, 6 returns
  - Google-style docstrings enforced
- Add data library option (polars/pandas/none) in cookiecutter.json
- Create pre-commit hooks configuration for automated checks
- Add CLAUDE.md template with project-specific instructions
- Configure pytest to support tests beside code
- Update Makefile with common commands (ty, fix, check)
- Remove Python 3.9/3.10 from CI matrix

These changes align the cookiecutter template with modern Python
development practices and enforce stricter code quality standards.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add web_framework choice in cookiecutter.json (fastapi/none)
- Include FastAPI and uvicorn dependencies when selected
- Create basic FastAPI app template with health check endpoint
- Add 'make serve' command to run development server
- Update CLAUDE.md to express strong preference for FastAPI over Flask
- Configure post-generation hook to remove _app.py if not using FastAPI

This enforces the preference for modern async frameworks and
explicitly discourages Flask usage in new projects.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix ty version to 0.0.1a16 (current alpha release)
- Remove --strict flag which doesn't exist in ty
- Update ty commands to explicitly check src directory
- Remove strict=true from pyproject.toml ty config

ty is still in early alpha, so we're using the latest available
version with basic configuration.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Apply ruff formatting to fix CI failure.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Remove the data_library choice (polars/pandas/none) as it's rarely
needed for Trail of Bits projects. This simplifies the template and
reduces unnecessary options during project creation.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove FastAPI web framework option to reduce complexity
- Consolidate ruff configuration from ruff.toml into pyproject.toml
- Fix Makefile redundancies: keep 'fix' and 'format' as aliases
- Remove duplicated ty command from lint target
- Clean up CLAUDE.md to remove FastAPI references

This simplifies the PR to focus on core improvements: stricter code
quality standards, ty type checker, and pre-commit hooks.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
The self-test CI workflow expects 'make reformat' to exist.
Add it back as an alias alongside 'fix' and 'format'.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Replace deprecated 'license-files' with proper license field syntax.
Use 'license = {text = "SPDX-ID"}' for standard licenses and
'license = {file = "LICENSE"}' for proprietary licenses.

This fixes the RUF200 parsing error in generated projects.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Change 'search_path' to 'src' in [tool.ty] configuration.
The ty type checker expects 'src' as the field name, not 'search_path'.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Remove the [tool.ty] section entirely as ty is in early alpha and
the configuration format is not well documented. Let ty use its
default configuration which should work for standard project layouts.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Replace experimental ty (0.0.1a16) with stable pyright (1.1+)
- Configure pyright with standard type checking mode
- Make pre-commit pytest less aggressive: only run fast tests (-k "not slow")
- Remove -q flag from pytest for better debugging visibility
- Clarify CLAUDE.md framework preferences as general guidelines
- Add missing newline to pre-commit config
- Update all references from ty to pyright in docs and commands

This makes the type checking more reliable while maintaining the
same strict code quality standards.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove accidentally included test-pyright directory
- Fix pre-commit deprecation: use stages: [pre-commit] instead of [commit]
- Change pyright to strict mode for better type safety
- Add pyright include paths: ["src", "test"]
- Add pytest marker documentation for @pytest.mark.slow
- Simplify Makefile: keep 'fix' as primary command, 'reformat' as alias

All changes tested locally - lint, fix, reformat, and pre-commit hooks
work correctly with strict pyright configuration.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link

@oldsj oldsj left a comment

Choose a reason for hiding this comment

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

I was able to test this with uvx cookiecutter gh:trailofbits/cookiecutter-python --checkout enhance-cookiecutter-standards and tests passed with make check , looks good!

# and let Dependabot periodically perform this update.
"ruff ~= 0.6.2",
"mypy >= 1.0",
"pyright >= 1.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

@ESultanik You have experience with Python type checkers. Do you have a preference of mypy over pyright or any other type checker?

Copy link
Contributor

@Ninja3047 Ninja3047 left a comment

Choose a reason for hiding this comment

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

i think for pre-commit I would strongly prefer we stick to just auto fixing formatting
running through the entire test suite would make development really annoying especially with work in progress stuff

Comment on lines +24 to +41
{%- if cookiecutter.docstring_coverage %}
- id: interrogate
name: interrogate docstring coverage
entry: uv run interrogate -c pyproject.toml
language: system
types: [python]
pass_filenames: false
{%- endif %}

- id: pytest
name: pytest
entry: uv run pytest
language: system
types: [python]
pass_filenames: false
require_serial: true
# Only run on pre-commit, not on push
stages: [pre-commit]
Copy link
Contributor

Choose a reason for hiding this comment

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

unclear to me if this is really best practice since python projects could potentially have a ton of tests and run slowly so running on every pre-commit will be extremely annoying

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should remove the pre-commit logic altogether, or gate it behind a non-default question in the template creation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the pre-commit logic was useful for ensuring automated LLM workflows would be checked for linting errors and failing tests.

Since this is a template, I think the ideal setup should be the default. If tests take too long, that's a bit of a code smell or you need to separate your tests into unit tests and integration tests and only run the unit tests on pre-commit.

Lots of subjectivity here, but it's easier to remove stuff than know what to add.

Comment on lines +3 to +12
repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.14.8
hooks:
# Run the linter.
- id: ruff-check
args: [--fix]
# Run the formatter.
- id: ruff-format
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of having separate steps in the pre-commit, it would be better to re-use the steps in the makefile by calling it directly to avoid having potentially divergent check logic

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.

7 participants