# NoteFlow Quality Standards Reference

> All sprint implementations MUST comply with these standards. Run quality gates before PR.

---

## Quick Reference: Quality Commands

```bash
# Python Backend
pytest tests/quality/              # All quality checks (23+ rules)
ruff check src/noteflow            # Linting
mypy src/noteflow                  # Type checking (strict)
basedpyright                       # Additional type checks

# TypeScript/React Frontend
cd client
npm run test:quality               # Frontend quality checks
npm run lint                       # ESLint

# Rust/Tauri
cd client
npm run quality:rs                 # Rust quality script
cargo clippy                       # Rust linting

# Full Suite
npm run quality:all                # TS + Rust quality
```

---

## Python Standards (`src/noteflow/`)

### Type Safety (STRICT)

| Rule | Description | Enforcement |
|------|-------------|-------------|
| No `# type: ignore` | Forbidden without justification | mypy strict |
| No `Any` type | Use specific types always | basedpyright |
| Union syntax | Use `str \| None` over `Optional[str]` | ruff UP |
| Return annotations | All public functions must have returns | mypy |

### Code Limits

| Metric | Threshold | Max Violations | Location |
|--------|-----------|----------------|----------|
| Module lines (soft) | 500 | 5 | `test_code_smells.py` |
| Module lines (hard) | 750 | 0 | `test_code_smells.py` |
| Function lines (src) | 75 | 7 | `test_code_smells.py` |
| Function lines (tests) | 50 | 3 | `test_test_smells.py` |
| Function complexity | 15 | 2 | `test_code_smells.py` |
| Parameters | 7 | 35 | `test_code_smells.py` |
| Class methods | 20 | 1 | `test_code_smells.py` |
| Class lines (god class) | 500 | 1 | `test_code_smells.py` |
| Nesting depth | 5 | 2 | `test_code_smells.py` |
| Feature envy | 5+ accesses | 5 | `test_code_smells.py` |

### Magic Values & Literals (`test_magic_values.py`)

| Rule | Max Allowed | Target | Description |
|------|-------------|--------|-------------|
| Magic numbers (>100) | 10 | 0 | Use named constants |
| Repeated string literals | 30 | 0 | Extract to constants |
| Hardcoded paths | 0 | 0 | Use Path objects/config |
| Hardcoded credentials | 0 | 0 | Use env vars/secrets |

### Stale Code (`test_stale_code.py`)

| Rule | Max Allowed | Target | Description |
|------|-------------|--------|-------------|
| Stale TODO/FIXME comments | 10 | 0 | Address or remove |
| Commented-out code blocks | 0 | 0 | Remove dead code |
| Unused imports | 5 | 0 | Remove or use |
| Unreachable code | 0 | 0 | Remove dead paths |
| Deprecated patterns | 5 | 0 | Modernize code |

### Duplicate Code (`test_duplicate_code.py`)

| Rule | Max Allowed | Target | Description |
|------|-------------|--------|-------------|
| Duplicate function bodies | 1 | 0 | Extract shared functions |
| Repeated code patterns | 177 | 50 | Refactor to reduce duplication |

### Unnecessary Wrappers (`test_unnecessary_wrappers.py`)

| Rule | Max Allowed | Target | Description |
|------|-------------|--------|-------------|
| Trivial wrapper functions | varies | 0 | Remove or add value |
| Alias imports | varies | 0 | Import directly |
| Redundant type aliases | 2 | 0 | Use original types |
| Passthrough classes | 1 | 0 | Flatten hierarchy |

### Decentralized Helpers (`test_decentralized_helpers.py`)

| Rule | Max Allowed | Target | Description |
|------|-------------|--------|-------------|
| Scattered helper functions | 15 | 5 | Consolidate to utils |
| Utility modules not centralized | 0 | 0 | Move to shared location |
| Duplicate helper implementations | 25 | 0 | Deduplicate |

### Test Requirements

**Current thresholds** (to be reduced each sprint):

| Rule | Max Allowed | Target | File |
|------|-------------|--------|------|
| Assertion roulette (>3 assertions without msg) | 50 | 0 | `test_test_smells.py` |
| Conditional test logic | 40 | 0 | `test_test_smells.py` |
| Empty tests | 0 | 0 | `test_test_smells.py` |
| Sleepy tests (time.sleep) | 3 | 0 | `test_test_smells.py` |
| Tests without assertions | 5 | 0 | `test_test_smells.py` |
| Redundant assertions | 0 | 0 | `test_test_smells.py` |
| Print statements in tests | 5 | 0 | `test_test_smells.py` |
| Skipped tests without reason | 0 | 0 | `test_test_smells.py` |
| Exception handling (broad try/except) | 3 | 0 | `test_test_smells.py` |
| Magic numbers in assertions | 50 | 10 | `test_test_smells.py` |
| Sensitive equality (str/repr compare) | 10 | 0 | `test_test_smells.py` |
| Eager tests (>10 method calls) | 10 | 0 | `test_test_smells.py` |
| Duplicate test names | 15 | 0 | `test_test_smells.py` |
| Hardcoded test data paths | 0 | 0 | `test_test_smells.py` |
| Long test methods (>50 lines) | 3 | 0 | `test_test_smells.py` |
| unittest-style assertions | 0 | 0 | `test_test_smells.py` |
| Session fixtures with mutation | 0 | 0 | `test_test_smells.py` |
| Fixtures without type hints | 10 | 0 | `test_test_smells.py` |
| Unused fixture parameters | 5 | 0 | `test_test_smells.py` |
| Fixtures with wrong scope | 5 | 0 | `test_test_smells.py` |
| Conftest fixture duplication | 0 | 0 | `test_test_smells.py` |
| pytest.raises without match= | 50 | 0 | `test_test_smells.py` |
| Cross-file fixture duplicates | 0 | 0 | `test_test_smells.py` |

**Reduction schedule**:
- After each sprint, reduce non-zero thresholds by 20% (rounded down)
- Goal: All thresholds at target values by Sprint 8

### Docstring Requirements

- Write imperatively with proper punctuation
- All public functions, classes, modules documented
- Document complex business rules and edge cases

---

## TypeScript/React Standards (`client/src/`)

### Type Safety

| Rule | Max Allowed | File |
|------|-------------|------|
| `any` type usage | 10 | `code-quality.test.ts` |
| Unsafe type assertions (`as any/unknown/never`) | 5 | `code-quality.test.ts` |
| TypeScript suppressions (@ts-ignore) | 3 | `code-quality.test.ts` |

### Code Quality

| Rule | Max Allowed | Description |
|------|-------------|-------------|
| Repeated string literals | 5 | Same string in multiple files |
| Complex JSX patterns | 10 | Repeated component structures |
| Scattered helper functions | 2 | format/parse/convert scattered |
| TODO/FIXME comments | 15 | Unaddressed tech debt |
| Commented-out code | 10 | Stale code blocks |
| Trivial wrapper components | 3 | Components that just spread props |
| Magic numbers (>3 digits) | 5 | Use named constants |
| Hardcoded colors in JSX | 3 | Use theme/CSS variables |
| Hardcoded API endpoints | 0 | Use config |
| Long files (>500 lines) | 3 | Split into modules |
| Complex inline styles | 5 | Use CSS/Tailwind |
| Deeply nested ternaries | 0 | Use if/switch |
| Excessive prop spreading | 2 | Consider context |

### Naming Conventions

- Components: PascalCase (`RecordingPanel`, not `recordingPanel`)
- Hooks: `use` prefix (`useAudioLevel`)
- Utils: camelCase (`formatDuration`)
- Constants: SCREAMING_SNAKE_CASE (`MAX_RETRIES`)

---

## Rust/Tauri Standards (`client/src-tauri/src/`)

### Code Quality Checks

| Check | Threshold | Description |
|-------|-----------|-------------|
| Magic numbers | Warning | Numbers >100 not in const |
| Repeated strings | >3 occurrences | Extract to constants |
| TODO/FIXME comments | >10 | Address or remove |
| Long functions | >100 lines | Split into helpers |
| Deep nesting | >7 levels (28 spaces) | Flatten control flow |
| unwrap() calls | >20 | Use ? or expect() |
| clone() per file | >10 suspicious | Review ownership (excludes Arc::clone, handles) |
| Parameters | >5 | Use struct/builder |
| Duplicate error messages | >2 | Use error enum |
| File size | >500 lines | Split module |
| Scattered helpers | >10 files | Consolidate format_/parse_/convert_ functions |

### Clippy Enforcement

```bash
cargo clippy -- -W unused_imports -W dead_code
```

Must pass with zero warnings for:
- Unused imports
- Dead code
- Missing docs on public items

---

## Pre-Commit Checklist

Before any PR:

```markdown
## Python
- [ ] `pytest tests/quality/` passes
- [ ] `ruff check --fix .` run
- [ ] `mypy src/noteflow` clean
- [ ] No `# type: ignore` without comment
- [ ] Docstrings on all new public functions

## TypeScript/React
- [ ] `npm run test:quality` passes
- [ ] `npm run lint` clean
- [ ] No `any` types added
- [ ] Components use PascalCase

## Rust
- [ ] `npm run quality:rs` passes
- [ ] `cargo clippy` clean
- [ ] No unwrap() in error paths
- [ ] Error types documented
```

---

## Architecture Patterns

### Hexagonal Architecture (Python)

```
domain/          → Entities, value objects, ports (interfaces)
application/     → Use cases, services, orchestration
infrastructure/  → Implementations, adapters, external services
grpc/            → Transport layer, proto definitions
```

### File Organization

| Layer | Pattern | Example |
|-------|---------|---------|
| Domain | `entities/meeting.py`, `ports/repository.py` | Pure business logic |
| Application | `services/meeting_service.py` | Orchestrates domain |
| Infrastructure | `persistence/repositories/meeting_repo.py` | Implements ports |
| gRPC | `_mixins/meeting.py` | Transport handlers |

### Naming Conventions

| Type | Convention | Example |
|------|------------|---------|
| Domain entity | Singular noun | `Meeting`, `Segment` |
| Service | NounService | `MeetingService`, `SummarizationService` |
| Repository | NounRepository | `MeetingRepository` |
| Port | NounPort (Protocol) | `SummarizationPort` |
| Mixin | NounMixin | `StreamingMixin` |
| Factory | create_noun() | `create_summarization_service()` |

---

## Testable Code Patterns

### Protocol-Based Dependency Injection

All services MUST use **constructor injection** with **Protocol-based abstractions** for testability.

**References**:
- [ArjanCodes: Python DI Best Practices](https://arjancodes.com/blog/python-dependency-injection-best-practices/)
- [Real Python: SOLID Principles](https://realpython.com/solid-principles-python/)

### Key Principles

| Principle | Description | Example |
|-----------|-------------|---------|
| Constructor injection | All dependencies passed via `__init__` | `Service(repo: RepositoryPort)` |
| Protocol abstractions | Use `typing.Protocol` for interfaces | `class RepositoryPort(Protocol)` |
| Factory functions | Create configured instances | `create_service() -> Service` |
| No global state | Avoid singletons and module-level state | Use DI instead of `get_instance()` |

### Pattern: Service with Protocol Dependencies

```python
from typing import Protocol

# 1. Define port (interface) in domain layer
class NerPort(Protocol):
    """Port for NER operations."""

    def extract(self, text: str) -> list[NamedEntity]:
        """Extract named entities from text."""
        ...


# 2. Application service depends on protocol (not concrete impl)
class NerService:
    """Application service for NER operations."""

    def __init__(
        self,
        ner_engine: NerPort,  # Protocol, not SpacyNerEngine
        uow_factory: Callable[[], UnitOfWork],
    ) -> None:
        self._ner_engine = ner_engine
        self._uow_factory = uow_factory

    async def extract_entities(self, meeting_id: MeetingId) -> list[NamedEntity]:
        """Extract entities from meeting transcript."""
        async with self._uow_factory() as uow:
            meeting = await uow.meetings.get(meeting_id)
            return self._ner_engine.extract(meeting.transcript)


# 3. Infrastructure implements the protocol
class SpacyNerEngine:
    """spaCy implementation of NerPort."""

    def __init__(self, model_name: str = "en_core_web_sm") -> None:
        self._nlp = spacy.load(model_name)

    def extract(self, text: str) -> list[NamedEntity]:
        """Extract entities using spaCy."""
        doc = self._nlp(text)
        return [NamedEntity.from_spacy(ent) for ent in doc.ents]


# 4. Factory function wires dependencies
def create_ner_service(
    model_name: str = "en_core_web_sm",
    uow_factory: Callable[[], UnitOfWork] | None = None,
) -> NerService:
    """Create NER service with dependencies."""
    engine = SpacyNerEngine(model_name)
    factory = uow_factory or SQLAlchemyUnitOfWork
    return NerService(engine, factory)
```

### Testing with Mock Protocols

```python
@pytest.fixture
def mock_ner_engine() -> MagicMock:
    """Create mock NER engine implementing NerPort."""
    engine = MagicMock(spec=NerPort)
    engine.extract.return_value = [
        NamedEntity.create("Test Person", EntityCategory.PERSON, [1], 0.9),
    ]
    return engine


@pytest.fixture
def ner_service(mock_ner_engine: MagicMock, mock_uow_factory: Callable) -> NerService:
    """Create NER service with mock dependencies."""
    return NerService(mock_ner_engine, mock_uow_factory)


def test_extract_entities_calls_engine(
    ner_service: NerService,
    mock_ner_engine: MagicMock,
) -> None:
    """Extraction delegates to NER engine."""
    await ner_service.extract_entities(MeetingId(uuid4()))

    mock_ner_engine.extract.assert_called_once()
```

### Anti-Patterns (AVOID)

```python
# ❌ WRONG: Direct instantiation in service
class BadService:
    def __init__(self) -> None:
        self._engine = SpacyNerEngine()  # Untestable!

# ❌ WRONG: Module-level singleton
_engine = SpacyNerEngine()  # Global state!

def get_engine() -> SpacyNerEngine:
    return _engine

# ❌ WRONG: Concrete type dependency
class BadService:
    def __init__(self, engine: SpacyNerEngine) -> None:  # Concrete, not Protocol!
        self._engine = engine
```

### Modern Library Recommendations

| Category | Library | Rationale |
|----------|---------|-----------|
| OAuth 2.0 | **Authlib** | Built-in PKCE, async support, handles edge cases |
| HTTP Client | **httpx** | Modern async, compatible with Authlib |
| NER | **spaCy** or **GLiNER** | spaCy for production, GLiNER for zero-shot |
| Validation | **Pydantic** | Already used in project |
| Testing | **pytest** | With `pytest.mark.parametrize` |

---

## Test Patterns

### CRITICAL: No Conditionals in Tests

**FORBIDDEN in test code:**
- `if`/`else` statements with assertions
- `for` loops with assertions
- `while` loops
- Conditional logic that determines test behavior

**USE INSTEAD: `pytest.mark.parametrize`**

```python
# ❌ WRONG: Conditional test logic
def test_entity_extraction(engine: NerEngine) -> None:
    for text, expected in test_cases:  # FORBIDDEN
        entities = engine.extract(text)
        if expected:  # FORBIDDEN
            assert entities

# ✅ CORRECT: Parametrized tests
@pytest.mark.parametrize(
    ("text", "expected_category"),
    [
        pytest.param("John Smith", EntityCategory.PERSON, id="person"),
        pytest.param("Google", EntityCategory.COMPANY, id="company"),
        pytest.param("New York", EntityCategory.LOCATION, id="location"),
    ],
)
def test_entity_extraction(
    engine: NerEngine,
    text: str,
    expected_category: EntityCategory,
) -> None:
    """Extract entity of expected category."""
    entities = engine.extract(text)
    matching = [e for e in entities if e.category == expected_category]
    assert matching, f"Expected {expected_category.value} in: {text}"
```

### Parametrization Best Practices

```python
# Use pytest.param with descriptive IDs
@pytest.mark.parametrize(
    ("input_value", "expected_output", "description"),
    [
        pytest.param("", [], id="empty-input"),
        pytest.param("hello", ["hello"], id="single-word"),
        pytest.param("a b c", ["a", "b", "c"], id="multiple-words"),
    ],
)
def test_tokenize(input_value: str, expected_output: list[str], description: str) -> None:
    """Tokenize input produces expected tokens."""
    assert tokenize(input_value) == expected_output

# Class-based organization for related tests
class TestAuthorizationUrl:
    """Test authorization URL generation."""

    @pytest.mark.parametrize(
        ("provider", "expected_host"),
        [
            pytest.param(OAuthProvider.GOOGLE, "accounts.google.com", id="google"),
            pytest.param(OAuthProvider.MICROSOFT, "login.microsoftonline.com", id="microsoft"),
        ],
    )
    def test_generates_valid_url(self, provider: OAuthProvider, expected_host: str) -> None:
        """Generate URL for each provider."""
        url = generate_auth_url(provider)
        assert expected_host in url
```

### Fixture Scoping for Performance

```python
# Module-scoped for expensive operations (model loading, DB setup)
@pytest.fixture(scope="module")
def ner_engine() -> SpacyNerEngine:
    """Load spaCy model once per test module."""
    return SpacyNerEngine("en_core_web_sm")

# Function-scoped for mutable state
@pytest.fixture
def mock_uow() -> AsyncMock:
    """Fresh mock for each test."""
    return AsyncMock(spec=UnitOfWork)
```

### Required Test Elements

1. **Type hints** on fixtures and test functions
2. **Docstring** explaining what's being tested
3. **AAA pattern** (Arrange/Act/Assert) with comments
4. **Specific assertions** with messages for complex checks
5. **pytest.raises with match=** for exception tests
6. **`pytest.param` with IDs** for parametrized tests
7. **No conditionals or loops** around assertions

---

## Code Reuse Checklist

Before creating new code, check:

| Location | Contains |
|----------|----------|
| `domain/entities/` | Existing entity types |
| `domain/ports/` | Existing port interfaces |
| `infrastructure/converters/` | Entity ↔ ORM converters |
| `grpc/_mixins/converters.py` | Proto ↔ Domain converters |
| `infrastructure/*/protocols.py` | Infrastructure interfaces |
| `application/services/` | Existing service patterns |

### Shared Utilities

| File | Functions |
|------|-----------|
| `infrastructure/export/_formatting.py` | `format_timestamp()`, `format_datetime()` |
| `infrastructure/security/keystore.py` | `_generate_key()`, `_decode_and_validate_key()` |
| `infrastructure/summarization/_parsing.py` | `build_transcript_prompt()`, `parse_llm_response()` |
| `infrastructure/diarization/assigner.py` | `assign_speaker()`, `assign_speakers_batch()` |

---

## Documentation Requirements

Every new feature must include:

1. **Unit tests** covering core logic
2. **Integration tests** for end-to-end flow (where applicable)
3. **Docstrings** on all public APIs
4. **CLAUDE.md updates** if architectural patterns change
5. **Proto changes** documented in commit message

---

## Shared Test Fixtures

### Available Fixtures (`tests/conftest.py`)

**DO NOT redefine these fixtures in test files**. Use them from conftest.py.

| Fixture | Type | Scope | Description |
|---------|------|-------|-------------|
| `crypto` | `CryptoService` | function | Encryption service for test data |
| `meetings_dir` | `Path` | function | Temporary directory for meeting assets |
| `mock_uow` | `AsyncMock` | function | Mock Unit of Work with all repositories |
| `mock_uow_factory` | `type` | function | Factory that returns mock_uow |
| `temp_db` | `Engine` | session | Temporary SQLite database |
| `async_session` | `AsyncSession` | function | Async SQLAlchemy session |
| `grpc_server` | `NoteFlowServicer` | function | Test gRPC server instance |
| `grpc_client` | `NoteFlowClient` | function | Test gRPC client |
| `sample_meeting` | `Meeting` | function | Pre-populated meeting entity |
| `sample_segments` | `list[Segment]` | function | Sample transcript segments |
| `mock_ner_engine` | `MagicMock` | function | Mock NER engine |
| `mock_oauth_manager` | `MagicMock` | function | Mock OAuth manager |
| `mock_calendar_settings` | `CalendarSettings` | function | Calendar settings with test OAuth creds |

### Usage Pattern

```python
# CORRECT: Use shared fixtures
def test_meeting_creation(mock_uow: AsyncMock, sample_meeting: Meeting) -> None:
    """Create meeting uses repository correctly."""
    mock_uow.meetings.save.return_value = None
    # ... test logic

# INCORRECT: Do not redefine fixtures
@pytest.fixture
def mock_uow():  # DON'T DO THIS - use conftest.py fixture
    return AsyncMock()
```

### Cross-File Fixture Detection

The `test_test_smells.py` quality check detects when fixtures are redefined:

```python
# Fails quality check - fixture "mock_uow" already in conftest.py
@pytest.fixture
def mock_uow():
    ...
```

Move new shared fixtures to `tests/conftest.py` to avoid duplication.

### Adding New Shared Fixtures

When adding a fixture that could be reused:

1. Check if a similar fixture exists in `tests/conftest.py`
2. If not, add it to `tests/conftest.py` with:
   - Type annotation on the return
   - Docstring explaining the fixture
   - Appropriate scope (`function`, `class`, `module`, `session`)

```python
@pytest.fixture
def new_shared_fixture() -> SomeType:
    """Provide X for Y tests.

    Returns:
        Configured SomeType instance.
    """
    return SomeType(...)
```

---

## Sprint-Specific Quality Requirements

Each sprint must:

1. **Not increase** any quality threshold violations
2. **Reduce** at least one threshold toward target
3. **Add fixtures** to conftest.py (not test files)
4. **Run quality suite** before PR:
   ```bash
   pytest tests/quality/ -v
   ```
5. **Document** any threshold exceptions in PR description
