# Py Code Style

> Review Python code style with emphasis on configured tooling, import organization, naming clarity, public API documentation, formatting consistency, and string transformation patterns. Use when reviewing style/lint changes, pyproject lint configuration, docstrings, imports, naming, readability issues, or pluralization logic after correctness findings.

- Skill: `codesigils/py-code-style` (Agent Skill)
- Install (CLI): `npx skillmds@latest add codesigils/py-code-style`
- Raw SKILL.md: https://api.skillmd.com/api/skills/codesigils/py-code-style/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Docs & Writing
- Author: CodeSigils (https://skillmd.com/u/codesigils)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/codesigils/py-code-style

---


# Python Code-Style Review

Use style findings after correctness findings. Defer to the project's configured
formatter and linter when they exist.

**Freshness:** stable (no external references) — review rules based on core Python conventions, not volatile APIs.

## Review Rules

### Rule: style-defer-to-tooling
**Impact:** LOW-MEDIUM
**Applies when:** The project has `ruff`, `black`, `isort`, `mypy`, or `pyright` configuration.
**Skip when:** No tooling exists and the issue is purely subjective.
**Python:** any
**Tools:** ruff | mypy | pyright | project-configured
**Review signal:** Review feedback contradicts or duplicates configured automated tooling.

**Incorrect:**
```python
# Reviewer asks for single quotes while pyproject config uses double quotes.
name = 'Ada'
```

**Correct:**
```python
# Follow configured formatter output.
name = "Ada"
```

**Reason:** Style review should reinforce automated tools, not create a parallel subjective standard.

### Rule: style-import-organization
**Impact:** LOW-MEDIUM
**Applies when:** Imports are added or moved.
**Skip when:** The project formatter/linter will auto-fix the issue and CI already enforces it.
**Python:** any
**Tools:** ruff | project-configured
**Review signal:** Standard library, third-party, and local imports are mixed, or relative imports are added without need.

**Incorrect:**
```python
from myapp.models import User
import os
import httpx
```

**Correct:**
```python
import os

import httpx

from myapp.models import User
```

**Reason:** Predictable import grouping reduces merge churn and makes dependencies easier to scan.

### Rule: style-naming-clarity
**Impact:** LOW-MEDIUM
**Applies when:** New public names, modules, classes, functions, or constants are introduced.
**Skip when:** The name follows a domain convention or matches an external API.
**Python:** any
**Tools:** ruff | project-configured
**Review signal:** Abbreviated or misleading names obscure the domain concept.

**Incorrect:**
```python
def proc_usr(u: User) -> Result:
    ...
```

**Correct:**
```python
def process_user(user: User) -> Result:
    ...
```

**Reason:** Clear names reduce the need for comments and make reviewable intent visible.

### Rule: style-public-docstring
**Impact:** LOW-MEDIUM
**Applies when:** A public class, function, or method has non-obvious behavior, side effects, or failure modes.
**Skip when:** The public API is self-evident and fully described by its name and type signature.
**Python:** any
**Tools:** ruff | project-configured
**Review signal:** Public APIs with important arguments, return values, raised exceptions, or examples lack docstrings.

**Incorrect:**
```python
def process_batch(items: list[Item], max_workers: int = 4) -> BatchResult:
    ...
```

**Correct:**
```python
def process_batch(items: list[Item], max_workers: int = 4) -> BatchResult:
    """Process items concurrently and return successes plus per-item failures."""
    ...
```

**Reason:** Docstrings are most valuable where types alone do not communicate side effects or failure semantics.

### Rule: style-line-length-readability
**Impact:** LOW
**Applies when:** A changed line is hard to review because it combines multiple calls, conditions, or string fragments.
**Skip when:** The configured formatter keeps the line and readability is acceptable.
**Python:** any
**Tools:** ruff | project-configured
**Review signal:** Long expressions are technically valid but obscure intermediate meaning.

**Incorrect:**
```python
return client.fetch(user.id, include_orders=True, include_invoices=True, timeout=timeout, retry_policy=retry_policy)
```

**Correct:**
```python
return client.fetch(
    user.id,
    include_orders=True,
    include_invoices=True,
    timeout=timeout,
    retry_policy=retry_policy,
)
```

**Reason:** Review style should improve scanability where automated formatting alone is not enough.

### Rule: style-string-regex-hygiene
**Impact:** MEDIUM
**Applies when:** Code contains regex patterns, string interpolation, or string construction that could be simplified.
**Skip when:** The regex is inherently complex (e.g., parsing nested structures) and the escaping is unavoidable.
**Python:** any
**Tools:** ruff | project-configured
**Review signal:** Manual escape sequences in character classes, verbose string building, or regex patterns that could use Python convenience features.

**Incorrect:**
```python
# Unnecessary backslash before double-quote in raw string character class
re.search(r'python-version:\s*[\\"']{ver}[\\"']', text)

# Verbose string construction
expected = "[" + ", ".join(f'"{v}"' for v in VERSIONS) + "]"

# Hardcoded string used in multiple places
if filepath == ".agents/skills/skill-discovery":  # also defined in another file
```

**Correct:**
```python
# Switch quote delimiter to avoid escaping inside character class
re.search(rf"python-version:\s*[\"']{ver}[\"']", text)

# Use a magic string constant — readable at a glance
EXPECTED_YAML = '["3.10", "3.14"]'

# Extract to a named constant — single source of truth
SYMLINK_ENTRY = ".agents/skills/skill-discovery"
if filepath == SYMLINK_ENTRY:
```

**Reason:** Python provides quote-delimiter switching, f-strings, `re.escape()`, and named constants to avoid manual escaping. Complex escape sequences are error-prone for both humans and AI agents, and often indicate a simpler approach exists.

### Rule: style-regex-escape-strategy
**Impact:** MEDIUM
**Applies when:** Code builds regex patterns by concatenating user input, configuration values, or dynamic strings.
**Skip when:** The pattern is a static literal with no interpolated values.
**Python:** any
**Tools:** ruff | project-configured
**Review signal:** Manual escaping of regex metacharacters in interpolated values, or missing `re.escape()` on user-controlled input.

**Incorrect:**
```python
# Manual escaping — error-prone, misses edge cases
domain = "example.com"
pattern = r"https?://" + domain.replace(".", "\\.") + r"/.*"

# User input not escaped — ReDoS or wrong match
def find_users(query: str) -> list[str]:
    return re.findall(rf"User: {query}", log_text)
```

**Correct:**
```python
# re.escape() handles all metacharacters correctly
domain = "example.com"
pattern = rf"https?://{re.escape(domain)}/.*"

# Escape user input before embedding in regex
def find_users(query: str) -> list[str]:
    return re.findall(rf"User: {re.escape(query)}", log_text)
```

**Reason:** `re.escape()` is the canonical way to sanitize strings for regex interpolation. Manual escaping misses characters (e.g., `{`, `}`, `(`) and creates maintenance burden when regex syntax evolves.

### Rule: style-quote-delimiter-strategy
**Impact:** LOW
**Applies when:** Raw strings contain quotes that require escaping inside the string delimiter.
**Skip when:** The regex or string is simple enough that escaping is minimal and clear.
**Python:** any
**Tools:** ruff | project-configured
**Review signal:** Unnecessary backslash-escaping of quotes inside raw strings, or mixed delimiter styles within the same module.

**Incorrect:**
```python
# Double-quoted raw string — " needs escaping, ' doesn't
re.search(r"version:\s*[\"']?[\"']", text)  # confusing

# Single-quoted raw string — ' needs escaping, " doesn't  
re.search(r'version:\s*[\"']?[\"']', text)  # worse
```

**Correct:**
```python
# Choose delimiter so the character class needs no escaping
re.search(r"version:\s*[\"']?[\"']", text)   # " is in class, use " delimiter → ' doesn't escape
re.search(r'version:\s*[\"'']?[\"'']', text)  # ' is in class, use ' delimiter → " doesn't escape

# Or use a character class with the delimiter's quote first
re.search(r"[\"']+", text)   # Either quote — no escaping needed
```

**Reason:** Consistent delimiter choice eliminates escape noise. If the character class contains `"`, use `'` as the string delimiter (or vice versa). Pick one convention per project and follow it.

### Rule: style-constant-placement
**Impact:** LOW-MEDIUM
**Applies when:** Named values are defined as module-level constants or hardcoded in multiple locations.
**Skip when:** The value is truly local to a single function and used nowhere else.
**Python:** any
**Tools:** ruff | project-configured
**Review signal:** Magic strings/numbers repeated across files, or module-level constants that are only used in one function.

**Incorrect:**
```python
# Hardcoded in multiple places — no single source of truth
if status == "completed":  # also in validator.py and test_validators.py
    ...

# Module-level constant used only in one function
MAX_RETRIES = 3  # defined at top, used only in fetch_with_retry()

def fetch_with_retry(url: str) -> Response:
    for _ in range(MAX_RETRIES):  # could be local
        ...
```

**Correct:**
```python
# Extracted to a shared constant — single source of truth
# In constants.py or at module top
COMPLETED_STATUS = "completed"

if status == COMPLETED_STATUS:
    ...

# Function-local when only used there
def fetch_with_retry(url: str) -> Response:
    max_retries = 3  # local — doesn't need module scope
    for _ in range(max_retries):
        ...
```

**Reason:** Constants belong at the narrowest scope that covers all their uses. Module-level is for values shared across functions or files; function-local is for values used in one place. Repeated magic strings should be extracted to a single definition.

### Rule: style-deduplicate-reusable-patterns
**Impact:** MEDIUM
**Applies when:** Code review reveals duplicated logic, repeated boilerplate, or patterns that appear in 2+ places with minor variations.
**Skip when:** The duplication is intentional (e.g., independent implementations that may diverge) or the abstraction would be harder to understand than the repetition.
**Python:** any
**Tools:** ruff | project-configured
**Review signal:** Nearly identical code blocks, repeated setup/teardown sequences, or copy-pasted logic that could be extracted into a shared function, class, or utility.

**Incorrect:**
```python
# Same validation logic in two places
def validate_user(user: User) -> bool:
    if not user.name:
        return False
    if not user.email or "@" not in user.email:
        return False
    if user.age < 0 or user.age > 150:
        return False
    return True

def validate_admin(admin: Admin) -> bool:
    if not admin.name:
        return False
    if not admin.email or "@" not in admin.email:
        return False
    if admin.age < 0 or admin.age > 150:
        return False
    return True

# Repeated test setup
def test_create():
    db = create_test_db()
    db.connect()
    user = User(name="test", db=db)
    ...
    db.close()

def test_update():
    db = create_test_db()
    db.connect()
    user = User(name="test", db=db)
    ...
    db.close()
```

**Correct:**
```python
# Shared validation logic
def validate_person(name: str, email: str, age: int) -> bool:
    if not name:
        return False
    if not email or "@" not in email:
        return False
    if age < 0 or age > 150:
        return False
    return True

def validate_user(user: User) -> bool:
    return validate_person(user.name, user.email, user.age)

def validate_admin(admin: Admin) -> bool:
    return validate_person(admin.name, admin.email, admin.age)

# Shared fixture or helper
@pytest.fixture
def user_with_db():
    user = User(name="test", email="test@example.com")
    db.connect()
    yield user
    db.disconnect()
```

**Reason:** Duplicated code drifts — when one copy is updated and the other isn't, bugs follow. Extract shared logic into named functions, fixtures, or utilities. The threshold is 2+ occurrences with minor variations: if the abstraction is clearer than the repetition, extract it.

### Rule: style-use-inflection-for-pluralization
**Impact:** MEDIUM
**Applies when:** Code manually pluralizes or singularizes strings using hand-written rules (e.g., `if word.endswith("y"): word[:-1] + "ies"`). Domain prefixes in metadata, table-of-contents generators, schema-to-folder mappings, and filename convention checkers are common triggers.
**Skip when:** The project already uses `inflection` or another pluralization library, or the string transformation is domain-specific and not standard English pluralization.
**Python:** any
**Tools:** inflection
**Review signal:** Manual pluralization patterns: `word.endswith("y")` + slice arithmetic, `word.endswith(("s", "sh", "ch"))` + `"es"` append, or any dictionary of irregular plurals. Fragile edge cases: words like "person"→"people" or "analysis"→"analyses" are almost always missed by hand-written rules.
**Incorrect:**
```python
def pluralize(word: str) -> str:
    """Manually pluralize — misses irregulars and edge cases."""
    if word.endswith("y") and word[-2] not in "aeiou":
        return word[:-1] + "ies"
    if word.endswith(("s", "sh", "ch")):
        return word + "es"
    return word + "s"

# Used in a verifier that checks filename/frontmatter consistency
domain = "skill"
tag = pluralize(domain)  # "skills" — works
domain = "person"
tag = pluralize(domain)  # "persons" — wrong, should be "people"
```

**Correct:**
```python
import inflection

domain = "skill"
tag = inflection.pluralize(domain)  # "skills"
domain = "person"
tag = inflection.pluralize(domain)  # "people"
domain = "analysis"
tag = inflection.pluralize(domain)  # "analyses"

# Singularize works the other direction
filename_tag = "personalities"
domain = inflection.singularize(filename_tag)  # "personality"
```

**Reason:** English pluralization has dozens of irregular cases (`person`→`people`, `mouse`→`mice`, `datum`→`data`). Hand-written `endswith("y")` rules silently fail on these. `inflection` (a pure-Python library, ported from Ruby's ActiveSupport) handles all standard English pluralization in a single function call. It is the standard solution — do not reinvent it.

## Sensitive Evidence Safety

If changed code or tool output reveals a suspected credential, token, private
key, secret-bearing URL, or other sensitive value, do not quote or reproduce the
value. Report only its existence and location. Treat filename and pattern checks
as heuristic evidence, not proof that a repository is secret-free.

If the exposure appears credible, make it the first finding, stop lower-priority
review, and recommend revocation or rotation. Never place sensitive values in
reports, generated examples, or commit subjects or bodies.