Skip to content

Standardize YAML file extension to .yaml - #13

Merged
AlexAndrewsAI merged 4 commits into
mainfrom
chore/12-p2-standardize-to-yaml
Jul 27, 2026
Merged

Standardize YAML file extension to .yaml#13
AlexAndrewsAI merged 4 commits into
mainfrom
chore/12-p2-standardize-to-yaml

Conversation

@AlexAndrewsAI

Copy link
Copy Markdown
Owner

Description

Renames .github/workflows/ci.yml → .github/workflows/ci.yaml and updates agent instructions to document the convention.

Changes

  • Renamed: .github/workflows/ci.yml → .github/workflows/ci.yaml (contents unchanged)
  • Updated: AGENTS.md — added directive to use .yaml extension for all YAML files
  • Updated: AGENTS_MANUAL_CHECKS.md — same directive added for consistency

Motivation

The repo uses both .yml and .yaml extensions (.pre-commit-config.yaml vs .ci.yml). Standardizing on .yaml — the IANA-recommended extension — removes ambiguity and aligns with the existing pre-commit config naming.

@AlexAndrewsAI AlexAndrewsAI linked an issue Jul 27, 2026 that may be closed by this pull request
@AlexAndrewsAI

Copy link
Copy Markdown
Owner Author

Code Review: python-package-template

Reviewer: opencode (big-pickle)
Date: 2026-07-27
Branch: main

Overall Assessment

A clean, well-organized Python package template with solid fundamentals. The project demonstrates modern tooling choices (uv, ruff, mypy, Pydantic, Typer) and good conventions. However, the existing REVIEW.md (dated 2025-06-24) is stale and contains inaccuracies about the current state of the code. Several concrete bugs exist that break the developer experience out of the box.

Verified Tool Results

Tool Status Notes
uv sync --dev Pass Installs cleanly
ruff check . Pass All checks passed
ruff format --check . Pass 7 files already formatted
uv run pytest FAIL Broken: --cov-fail-under=95 fails because pytest-cov cannot be imported via uv run (mypy's system install shadows the venv)
uv run mypy . FAIL 6 import-not-found errors for pydantic, typer, pytest
uv run pip-audit FAIL Binary not found by uv run
uv run python -m pytest Pass 14/14 tests pass when invoked via python -m

Critical Issues

1. uv run mypy fails with import-not-found errors

Location: pyproject.toml (mypy config)

uv run mypy . produces 6 errors because mypy is resolving to the system-installed mypy (/home/sandbox/.local/bin/mypy) instead of the venv copy. The system mypy cannot see packages installed in the project's .venv. This means uv run mypy . (the documented workflow command) is broken.

Fix options:

  • Add ignore_missing_imports = true to [tool.mypy] in pyproject.toml (quick fix, weakens checking)
  • Ensure mypy is invoked exclusively through the venv: configure the pre-commit hook to use python -m mypy instead of mypy
  • Add [[tool.mypy.overrides]] sections for third-party packages:
[[tool.mypy.overrides]]
module = ["pydantic.*", "typer.*", "pytest.*"]
ignore_missing_imports = true

2. uv run pip-audit fails (binary not found)

Location: .pre-commit-config.yaml, CI workflow, AGENTS.md

pip-audit is listed as a dev dependency and installed successfully, but uv run pip-audit fails with No such file or directory. The entry point script is not being generated in the venv's bin. This breaks the pre-commit hook and CI.

Fix: Use uv run python -m pip_audit instead, or verify the entry point is being installed. This may be an environment-specific issue but should be tested.

3. uv run pytest broken by pytest-cov import path

Location: pyproject.toml:58

The [tool.pytest.ini_options] sets addopts = "--cov=python_package_template --cov-report=term-missing --cov-fail-under=95". When uv run pytest resolves to a system pytest (not the venv pytest-cov), the --cov flags are unrecognized and the run fails. Using uv run python -m pytest works because it forces the venv's Python.

Fix: Either:

  • Remove --cov-fail-under=95 from addopts and make coverage a separate CI step
  • Change the documented command to uv run python -m pytest everywhere
  • Add a conftest.py that conditionally loads pytest-cov

Moderate Issues

4. CLI has redundant Config instantiation logic

Location: python_package_template/cli.py:37-52

config = DEFAULT_CONFIG if name == "World" else Config(name=name)

This compares against a magic string "World" that duplicates the default value in Config. If the default changes in Config, the CLI breaks silently. Should be:

config = Config(name=name) if name != Config.model_fields["name"].default else DEFAULT_CONFIG

Or simply always create a new Config(name=name) — the performance difference is negligible.

5. __init__.py exports DEFAULT_CONFIG but it's a Final singleton

Location: python_package_template/__init__.py:12

Exporting a Final constant in __all__ is fine, but callers might not realize it's frozen and immutable. The __init__.py re-exports it from config.py which is already the canonical location. Consider whether re-exporting is necessary or if users should import from config directly.

6. Existing REVIEW.md is stale

Location: REVIEW.md

The current REVIEW.md (dated 2025-06-24) states:

  • "100% test coverage" — the coverage threshold is 95%, not 100%
  • "Error Handling in CLI... Pydantic validation errors are not caught" — they ARE caught (cli.py:44-46)
  • "Documentation Completeness... README mentions git ls-tree" — the README doesn't mention this
  • "Dependency Pinning... consider pinning to specific minor versions" — uv.lock already pins exact versions

This file should be updated or removed.

Minor Issues

7. Typo in CLI help text

Location: python_package_template/cli.py:27

help="Show the version and exit."

Minor: the callback echoes f"python-package-template version: {__version__}" which includes a colon and space, inconsistent with standard --version output format (typically python-package-template, version 0.1.1).

8. .devin/config.local.json committed to repo

Location: .devin/config.local.json

This is a local tool configuration file that should be gitignored. It leaks tool-specific settings into the repo.

9. AGENTS.md recommends uv run prek install but prek needs uv run prek run --all-files first

Location: AGENTS.md

The workflow says install then run. But prek install creates git hooks that call prek run. The flow should be clearer about when to use each.

10. No conftest.py with shared fixtures

Location: tests/

For a template repo, having a conftest.py with common fixtures (even if empty) sets a better example for scaling.

Security

  • No hardcoded secrets or credentials
  • Pydantic frozen models prevent runtime config mutation
  • pip-audit is in the toolchain (though currently broken)
  • Ruff enables S (flake8-bandit) rules

Recommendation: Fix pip-audit so security scanning actually works in CI.

What's Good

  • Clean module separation: config.py, hello.py, cli.py — each has a single responsibility
  • Pydantic usage: Frozen model with field validation, proper use of Final for singleton
  • Test quality: 14 tests covering happy path, validation, immutability, CLI, __main__, and logging
  • Type hints: disallow_untyped_defs = true in mypy config; all functions are typed
  • Docstrings: Google-style on all public APIs
  • CI matrix: Tests across Python 3.10-3.13
  • Changelog discipline: PR-gated changelog updates in CI
  • py.typed marker: PEP 561 compliance
  • .gitignore aggressiveness: The /.* pattern with explicit allowlist is unusual but well thought out

Recommendations Summary

Priority Issue Fix
Critical uv run mypy broken Add ignore_missing_imports overrides for third-party libs
Critical uv run pip-audit broken Debug entry point or switch to python -m pip_audit
Critical uv run pytest broken Document uv run python -m pytest or fix addopts
Moderate CLI magic string default Use Config.model_fields or always construct fresh Config
Moderate Stale REVIEW.md Update or remove
Minor .devin/config.local.json in repo Add to .gitignore
Minor Version output format Use standard package, version X.Y.Z format

@AlexAndrewsAI AlexAndrewsAI self-assigned this Jul 27, 2026
@AlexAndrewsAI

Copy link
Copy Markdown
Owner Author

Critical Issues

These were determined to be spurious artifacts of sandbox

@AlexAndrewsAI
AlexAndrewsAI merged commit 9d657b8 into main Jul 27, 2026
4 checks passed
@AlexAndrewsAI
AlexAndrewsAI deleted the chore/12-p2-standardize-to-yaml branch July 27, 2026 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P2🥈 standardize to .yaml

1 participant