eryao/.trellis/spec/backend/quality-guidelines.md

Quality Guidelines

Code quality standards for backend development.

---

Overview

This project enforces quality through:

• Linting: ruff (fast Python linter)
• Type checking: basedpyright (strict type checker)
• Formatting: Consistent code style via linter rules
• Testing: pytest with async support (pytest-asyncio)
• Pre-commit hooks: Automated checks before commits

---

Forbidden Patterns

❌ Bypassing Lint/Type Gates

# WRONG: Suppress linter warnings
result = some_function()  # noqa: BLE001, reportImplicitRelativeImport

Right way: Fix the underlying issue:

# Fix exception handling
except Exception as exc:
    logger.error("operation_failed", error=str(exc))
    raise

❌ Using print() in Runtime Code

# WRONG: Never use print in production code
print(f"Processing user {user_id}")

Right way: Use core.logging:

from core.logging import get_logger

logger = get_logger(__name__)
logger.info("processing_user", user_id=user_id)

❌ Hardcoding Secrets

# WRONG: Hardcode credentials
api_key = "sk-abcd1234..."
password = "postgres123"

Right way: Use core.config.settings:

from core.config.settings import config

api_key = config.llm.provider_keys["openai"]
password = config.database.password

❌ Manual Environment Parsing

# WRONG: Direct os.getenv
import os
db_host = os.getenv("DATABASE_HOST", "localhost")

Right way: Use Settings:

from core.config.settings import config

db_host = config.database.host

❌ Silent Exception Handling

# WRONG: Swallow exception
try:
    await service.do_something()
except Exception:
    pass  # Destroys debuggability

Right way: Log and propagate:

try:
    await service.do_something()
except Exception as exc:
    logger.error("operation_failed", error=str(exc), exc_info=True)
    raise

---

Required Patterns

✅ Strong Typing at Boundaries

from pydantic import BaseModel

# Request/response schemas must use Pydantic
class CreateSessionRequest(BaseModel):
    thread_id: str
    messages: list[Message]

class SessionResponse(BaseModel):
    id: UUID
    owner_id: UUID
    created_at: datetime

✅ Schema-First for Data Contracts

# Define schema first, then implement
# v1/agent/schemas.py
class TaskAccepted(BaseModel):
    run_id: str
    thread_id: str
    accepted_at: datetime = Field(default_factory=lambda: datetime.now(timezone.utc))

# Then use in router
@router.post("/run", response_model=TaskAccepted)
async def create_run(...):
    return TaskAccepted(run_id=run_id, thread_id=thread_id)

✅ Dependency Injection via FastAPI

# v1/agent/dependencies.py
from fastapi import Depends
from core.db.base_repository import BaseRepository

async def get_agent_service(
    session: AsyncSession = Depends(get_session),
) -> AgentService:
    repository = AgentRepository(session)
    return AgentService(repository=repository, ...)

# v1/agent/router.py
@router.post("/run")
async def create_run(
    service: AgentService = Depends(get_agent_service),
):
    return await service.enqueue_run(...)

✅ Configuration via Settings

# All config through Settings
from core.config.settings import config

class SomeService:
    def __init__(self):
        self.redis_url = config.redis.url
        self.db_url = config.database.url
        self.log_level = config.runtime.log_level

✅ Logging with Context

# Always include relevant context
logger.info(
    "run_completed",
    run_id=run_id,
    thread_id=thread_id,
    duration_seconds=duration,
    user_id=current_user.id,
)

---

Testing Requirements

Test Organization

backend/tests/
├── unit/              # Unit tests (no external dependencies)
├── integration/       # Integration tests (DB, Redis, HTTP)
├── conftest.py        # Shared fixtures
└── ...

Test Requirements

From AGENTS.md:

1. TDD when practical: Write tests before/alongside implementation
2. Regression tests: Changed logic/contracts must have regression tests
3. Real DB tests: Use settings.test.* credentials (never hardcode)
4. Integration tests: Start backend via ./infra/scripts/app.sh before running
5. Restart for integration: Use ./infra/scripts/app.sh restart before uv run pytest

Test Configuration

# pyproject.toml
[tool.pytest.ini_options]
testpaths = ["backend/tests"]
addopts = "-q --import-mode=importlib"
asyncio_mode = "auto"
pythonpath = ["backend/src"]

Example Test

import pytest
from httpx import AsyncClient
from core.config.settings import Settings

@pytest.mark.asyncio
async def test_create_session(settings: Settings):
    # Use settings.test.* for real DB tests
    assert settings.test.phone != ""
    
    async with AsyncClient() as client:
        response = await client.post(
            f"{settings.supabase.url}/api/v1/agent/run",
            json={"thread_id": "test-thread"},
        )
        assert response.status_code == 200

---

Code Review Checklist

Architecture

• Follows schema -> repository -> service layering
• Router handles HTTP only, service handles business logic
• Repository does not make auth decisions
• Transaction boundary is in service layer, not repository

Security

• owner_id comes from current_user.id (JWT sub claim), not client payload
• No hardcoded secrets/passwords/tokens
• Sensitive fields are not logged (passwords, tokens, PII)
• Auth checks use ApiProblemError with proper error codes

Data

• Database changes use Alembic migrations
• Soft delete uses deleted_at, queries filter deleted records
• Timezone-aware timestamps: datetime.now(timezone.utc)
• Foreign keys follow <entity>_id naming

Error Handling

• All exceptions are logged before re-raising
• Business logic uses ApiProblemError, not HTTPException
• Error responses use RFC 7807 format with code field
• New error codes documented in docs/protocols/common/http-error-codes.md

Logging

• Uses core.logging.get_logger, not print()
• Log level appropriate for event (ERROR for failures, INFO for milestones)
• Context includes relevant identifiers (run_id, user_id, thread_id)
• No sensitive data in logs

Testing

• Unit tests for service logic
• Integration tests for changed contracts
• Test credentials via settings.test.*
• Regression tests for bug fixes

Code Style

• Passes ruff linting
• Passes basedpyright type checking
• Follows naming conventions (snake_case for functions/variables)
• No broad except Exception: without logging/re-raising

---

Linting & Type Checking

Ruff (Linter)

Run:

uv run ruff check backend/src/

Common fixes:

• Remove unused imports
• Fix line length (max 100 chars)
• Use async def for async functions
• Add type annotations

Basedpyright (Type Checker)

Run:

uv run basedpyright backend/src/

Common fixes:

• Add type hints for function parameters
• Use Optional[T] for optional values
• Fix reportImplicitRelativeImport warnings
• Handle None cases explicitly

Pre-commit Hooks

Setup:

uv run pre-commit install

Runs automatically on:

• ruff check
• basedpyright
• Formatting checks

---

Common Mistakes

❌ Missing Type Annotations

# WRONG: No type hints
def get_user(user_id):
    return repository.get_by_id(user_id)

Right:

async def get_user(user_id: UUID) -> User | None:
    return await repository.get_by_id(user_id)

❌ Catch-All Exception Handling

# WRONG: Catching all exceptions without logging
except Exception:
    return None

Right:

except SomeSpecificError as exc:
    logger.error("operation_failed", error=str(exc), exc_info=True)
    raise ApiProblemError(...) from exc

❌ Timezone-Naive Datetimes

# WRONG: No timezone
created_at = datetime.now()

Right:

created_at = datetime.now(timezone.utc)

❌ Testing with Hardcoded Credentials

# WRONG: Hardcoded test DB
DATABASE_URL = "postgres://user:pass@localhost:5432/test"

Right:

from core.config.settings import config

# Use settings.test.*
assert config.database.host is not None