docs: 更新 HTTP 错误码、用户积分、占卜运行及用户资料协议文档
This commit is contained in:
@@ -0,0 +1,221 @@
|
||||
# Database Guidelines
|
||||
|
||||
> Database patterns and conventions for this project.
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This project uses:
|
||||
- **SQLAlchemy 2.0** with async support (`asyncpg` driver)
|
||||
- **Alembic** for schema migrations
|
||||
- **Supabase Auth** as identity source
|
||||
- **PostgreSQL** as primary database
|
||||
- **Soft delete** pattern with `deleted_at` column
|
||||
|
||||
---
|
||||
|
||||
## Query Patterns
|
||||
|
||||
### Base Repository Pattern
|
||||
|
||||
All repositories inherit from `BaseRepository` (`core.db.base_repository.BaseRepository`):
|
||||
|
||||
```python
|
||||
from core.db.base_repository import BaseRepository
|
||||
from models.agent_chat_session import AgentChatSession
|
||||
|
||||
class AgentRepository(BaseRepository[AgentChatSession]):
|
||||
def __init__(self, session: AsyncSession) -> None:
|
||||
super().__init__(session, AgentChatSession)
|
||||
```
|
||||
|
||||
**Provided methods:**
|
||||
- `get_by_id(entity_id)` - Get single entity by ID
|
||||
- `get_one(*filters)` - Get single entity with filters
|
||||
- `update_by_id(entity_id, update_data)` - Update entity
|
||||
- `soft_delete_by_id(entity_id)` - Soft delete (sets `deleted_at`)
|
||||
|
||||
**Soft delete behavior:**
|
||||
- Queries **automatically exclude** deleted records
|
||||
- `deleted_at` is set to `datetime.now(timezone.utc)`
|
||||
- Repository checks if model has `deleted_at` column before applying filter
|
||||
|
||||
### Query Examples
|
||||
|
||||
```python
|
||||
# Get by ID (auto-filters deleted)
|
||||
session = await repository.get_by_id(session_id)
|
||||
|
||||
# Get with custom filters
|
||||
session = await repository.get_one(
|
||||
AgentChatSession.owner_id == user_id,
|
||||
AgentChatSession.id == session_id,
|
||||
)
|
||||
|
||||
# Update entity
|
||||
updated = await repository.update_by_id(
|
||||
session_id,
|
||||
{"title": "New Title", "updated_at": datetime.now(timezone.utc)},
|
||||
)
|
||||
```
|
||||
|
||||
### Custom Queries
|
||||
|
||||
For complex queries, add custom methods to repository:
|
||||
|
||||
```python
|
||||
class AgentRepository(BaseRepository[AgentChatSession]):
|
||||
async def get_active_sessions_by_user(
|
||||
self, user_id: UUID
|
||||
) -> list[AgentChatSession]:
|
||||
stmt = (
|
||||
select(self._model)
|
||||
.where(self._model.owner_id == user_id)
|
||||
.order_by(self._model.created_at.desc())
|
||||
)
|
||||
stmt = self._apply_soft_delete_filter(stmt)
|
||||
result = await self._session.execute(stmt)
|
||||
return list(result.scalars().all())
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Migrations
|
||||
|
||||
### Creating Migrations
|
||||
|
||||
**Use `dev-migrate.sh` script:**
|
||||
|
||||
```bash
|
||||
# Create new migration
|
||||
./infra/scripts/dev-migrate.sh revision --autogenerate -m "add user preferences table"
|
||||
```
|
||||
|
||||
**Migration file naming:**
|
||||
- Format: `YYYYMMDD_####_<description>.py`
|
||||
- Example: `20260411_0001_initial_llm_schema.py`
|
||||
|
||||
**Migration structure:**
|
||||
|
||||
```python
|
||||
def upgrade() -> None:
|
||||
op.create_table(
|
||||
"llm_factory",
|
||||
sa.Column("id", sa.UUID(), nullable=False),
|
||||
sa.Column("name", sa.String(length=50), nullable=False),
|
||||
sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.text("now()")),
|
||||
sa.Column("updated_at", sa.DateTime(timezone=True), server_default=sa.text("now()")),
|
||||
sa.Column("deleted_at", sa.DateTime(timezone=True), nullable=True),
|
||||
sa.PrimaryKeyConstraint("id"),
|
||||
sa.UniqueConstraint("name"),
|
||||
)
|
||||
op.create_index("ix_llm_factory_name", "llm_factory", ["name"], unique=True)
|
||||
_enable_rls("llm_factory") # RLS for Supabase Auth
|
||||
|
||||
def downgrade() -> None:
|
||||
op.drop_index("ix_llm_factory_name", table_name="llm_factory")
|
||||
op.drop_table("llm_factory")
|
||||
```
|
||||
|
||||
### Running Migrations
|
||||
|
||||
**Three modes (via `dev-migrate.sh`):**
|
||||
|
||||
```bash
|
||||
# Migrations only
|
||||
./infra/scripts/dev-migrate.sh migrate
|
||||
|
||||
# Seed data only
|
||||
./infra/scripts/dev-migrate.sh init-data
|
||||
|
||||
# Both (migrate + seed)
|
||||
./infra/scripts/dev-migrate.sh bootstrap
|
||||
```
|
||||
|
||||
**Alembic is the ONLY source of truth for schema changes.**
|
||||
|
||||
---
|
||||
|
||||
## Naming Conventions
|
||||
|
||||
### Tables
|
||||
- **snake_case**: `agent_chat_session`, `llm_factory`, `points_ledger`
|
||||
- **Plural or singular**: Based on domain semantics
|
||||
- **Timestamps**: `created_at`, `updated_at`, `deleted_at` (always timezone-aware)
|
||||
|
||||
### Columns
|
||||
- **snake_case**: `owner_id`, `model_code`, `request_url`
|
||||
- **Foreign keys**: `<entity>_id` (e.g., `factory_id`, `owner_id`)
|
||||
- **Indexes**: `ix_<table>_<column>` (e.g., `ix_llms_model_code`)
|
||||
|
||||
### Models
|
||||
- **PascalCase**: `AgentChatSession`, `LlmFactory`
|
||||
- **File location**: `models/<entity>.py`
|
||||
|
||||
---
|
||||
|
||||
## Transactions
|
||||
|
||||
### Service Layer Responsibility
|
||||
|
||||
**Services own transaction boundaries:**
|
||||
|
||||
```python
|
||||
class AgentService:
|
||||
async def enqueue_run(self, ...) -> TaskAccepted:
|
||||
async with self._repository._session.begin(): # Transaction start
|
||||
# Multiple repository calls
|
||||
session = await self._repository.get_by_id(session_id)
|
||||
await self._repository.update_by_id(session_id, {...})
|
||||
# Transaction commit on successful exit
|
||||
```
|
||||
|
||||
### Repository Layer
|
||||
|
||||
**Repositories do NOT manage transactions:**
|
||||
|
||||
```python
|
||||
# Repository uses session, but doesn't commit
|
||||
result = await self._session.execute(stmt)
|
||||
await self._session.flush() # Flush to DB, but don't commit
|
||||
return result.scalar_one_or_none()
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Common Mistakes
|
||||
|
||||
### ❌ Using raw SQL without Alembic
|
||||
- **Wrong**: Creating tables directly in code
|
||||
- **Right**: All DDL changes via Alembic migrations
|
||||
|
||||
### ❌ Hardcoding database credentials
|
||||
- **Wrong**: `password = "postgres123"`
|
||||
- **Right**: Use `core.config.settings.Settings.database.password`
|
||||
|
||||
### ❌ Bypassing soft delete filter
|
||||
- **Wrong**: `select(Model).where(Model.id == x)` (includes deleted)
|
||||
- **Right**: Use `BaseRepository` methods or apply `self._apply_soft_delete_filter()`
|
||||
|
||||
### ❌ Using `session.commit()` in repository
|
||||
- **Wrong**: Repository calling `await session.commit()`
|
||||
- **Right**: Let service layer manage transactions with `session.begin()`
|
||||
|
||||
### ❌ Not using timezone-aware timestamps
|
||||
- **Wrong**: `datetime.now()`
|
||||
- **Right**: `datetime.now(timezone.utc)`
|
||||
|
||||
### ❌ Getting `owner_id` from client payload
|
||||
- **Wrong**: `owner_id = payload.owner_id`
|
||||
- **Right**: `owner_id = current_user.id` (from verified JWT sub claim)
|
||||
|
||||
---
|
||||
|
||||
## Database Access Rules
|
||||
|
||||
**From AGENTS.md:**
|
||||
1. Use `supabase mcp` tools (`supabase_execute_sql`, `supabase_list_tables`) to **view** data
|
||||
2. Use **Alembic migrations** to **modify** schema
|
||||
3. Use **service-role DB access only** in backend
|
||||
4. Soft delete uses `deleted_at`; reads exclude deleted by default
|
||||
@@ -0,0 +1,145 @@
|
||||
# Directory Structure
|
||||
|
||||
> How backend code is organized in this project.
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This project follows a **layered architecture** with clear separation of concerns:
|
||||
- **Schema → Repository → Service** layering pattern
|
||||
- Router layer for HTTP endpoints
|
||||
- Core infrastructure modules
|
||||
- Domain schemas for business models
|
||||
|
||||
---
|
||||
|
||||
## Directory Layout
|
||||
|
||||
```
|
||||
backend/
|
||||
├── src/
|
||||
│ ├── core/ # Infrastructure and cross-cutting concerns
|
||||
│ │ ├── agentscope/ # Agent runtime framework
|
||||
│ │ ├── auth/ # Authentication models and dependencies
|
||||
│ │ ├── config/ # Settings and configuration
|
||||
│ │ ├── db/ # Database base classes and session
|
||||
│ │ ├── http/ # HTTP utilities (errors, middleware)
|
||||
│ │ └── logging/ # Structured logging setup
|
||||
│ ├── models/ # SQLAlchemy ORM models
|
||||
│ ├── schemas/ # Business data models (Pydantic/dataclass)
|
||||
│ │ ├── agent/ # Agent-related schemas
|
||||
│ │ ├── domain/ # Domain objects
|
||||
│ │ └── ...
|
||||
│ ├── services/ # Shared infrastructure services
|
||||
│ │ ├── base/ # Base service interfaces
|
||||
│ │ ├── caches/ # Cache implementations
|
||||
│ │ └── llm_pricing/ # LLM pricing service
|
||||
│ └── v1/ # API v1 versioned modules
|
||||
│ ├── agent/ # Agent module
|
||||
│ ├── auth/ # Authentication module
|
||||
│ ├── memories/ # Memories module
|
||||
│ ├── points/ # Points module
|
||||
│ └── ... # Other modules
|
||||
├── tests/ # Test suites
|
||||
├── alembic/ # Database migrations
|
||||
│ └── versions/ # Migration files
|
||||
└── pyproject.toml # Project configuration
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Module Organization
|
||||
|
||||
Each module under `v1/` follows consistent structure:
|
||||
|
||||
```
|
||||
v1/<module>/
|
||||
├── __init__.py # Module exports
|
||||
├── router.py # FastAPI router (HTTP endpoints)
|
||||
├── schemas.py # Request/response schemas (Pydantic)
|
||||
├── repository.py # Data access layer (CRUD + queries)
|
||||
├── service.py # Business logic layer (authz + transactions)
|
||||
├── dependencies.py # FastAPI dependencies (DI)
|
||||
└── utils.py # Module utilities (optional)
|
||||
```
|
||||
|
||||
**Layering rules:**
|
||||
- **Router** → handles HTTP, calls service, returns response
|
||||
- **Service** → business logic, authz, transaction boundary, raises domain errors
|
||||
- **Repository** → CRUD + query composition only, no auth decisions
|
||||
|
||||
**Example:** `v1/agent/` module:
|
||||
- `router.py` defines `/agent` endpoints
|
||||
- `service.py` enforces authz (`ensure_session_owner`) and coordinates repositories
|
||||
- `repository.py` handles database queries for agent sessions
|
||||
|
||||
---
|
||||
|
||||
## Naming Conventions
|
||||
|
||||
### Files
|
||||
- **snake_case**: Python files (e.g., `agent_service.py`)
|
||||
- **Module directories**: Singular or plural based on domain (e.g., `agent/`, `memories/`)
|
||||
|
||||
### Classes
|
||||
- **PascalCase**: Classes (`AgentService`, `AgentRepository`)
|
||||
- **Suffixes**:
|
||||
- `*Service` - Business logic layer
|
||||
- `*Repository` - Data access layer
|
||||
- `*Model` / `*Schema` - Data models (Pydantic)
|
||||
- `*Settings` - Configuration classes
|
||||
|
||||
### Functions/Variables
|
||||
- **snake_case**: Functions and variables (`get_by_id`, `soft_delete_by_id`)
|
||||
- **Private prefix**: `_` for internal methods (`_apply_soft_delete_filter`)
|
||||
|
||||
### Database Tables
|
||||
- **snake_case**: Table names (`agent_chat_session`, `llm_factory`)
|
||||
- **Timestamps**: `created_at`, `updated_at`, `deleted_at` (soft delete)
|
||||
- **Foreign keys**: `<entity>_id` (e.g., `factory_id`, `owner_id`)
|
||||
|
||||
---
|
||||
|
||||
## Examples
|
||||
|
||||
### Well-organized module: `v1/agent/`
|
||||
```
|
||||
v1/agent/
|
||||
├── router.py # HTTP endpoints, dependencies injection
|
||||
├── service.py # Business logic, authz checks (ensure_session_owner)
|
||||
├── repository.py # Database queries
|
||||
├── schemas.py # Request/response DTOs
|
||||
├── dependencies.py # Service instantiation (DI)
|
||||
├── utils.py # Helper functions
|
||||
└── system_agents_config.py # Module config
|
||||
```
|
||||
|
||||
### Core infrastructure: `core/logging/`
|
||||
```
|
||||
core/logging/
|
||||
├── logger.py # get_logger() interface
|
||||
├── config.py # Logging configuration
|
||||
├── formatters.py # Structured log formatters
|
||||
├── handlers.py # Log handlers
|
||||
├── filters.py # Sensitive field filters
|
||||
└── middleware.py # Request logging middleware
|
||||
```
|
||||
|
||||
### ORM models: `models/`
|
||||
```python
|
||||
# models/agent_chat_session.py
|
||||
class AgentChatSession(Base):
|
||||
__tablename__ = "agent_chat_session"
|
||||
id: Mapped[uuid.UUID] = ...
|
||||
owner_id: Mapped[uuid.UUID] = ...
|
||||
```
|
||||
|
||||
### Domain schemas: `schemas/domain/`
|
||||
```python
|
||||
# schemas/domain/chat_message.py
|
||||
class AgentChatMessageMetadata(BaseModel):
|
||||
role: str
|
||||
content: str
|
||||
# ... business fields
|
||||
```
|
||||
@@ -0,0 +1,293 @@
|
||||
# Error Handling
|
||||
|
||||
> How errors are handled in this project.
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This project follows **RFC 7807 Problem Details** for HTTP API errors:
|
||||
|
||||
- **Standard format**: `application/problem+json`
|
||||
- **Error codes**: Machine-readable `UPPER_SNAKE_CASE`
|
||||
- **Source of truth**: `docs/protocols/common/http-error-codes.md`
|
||||
- **Exception class**: `ApiProblemError` for domain errors
|
||||
- **Layered approach**: Routers handle HTTP, Services raise domain errors
|
||||
|
||||
---
|
||||
|
||||
## Error Types
|
||||
|
||||
### `ApiProblemError`
|
||||
|
||||
**Custom exception for business logic errors:**
|
||||
|
||||
```python
|
||||
from core.http.errors import ApiProblemError, problem_payload
|
||||
|
||||
# Raise in service/repository/dependencies
|
||||
raise ApiProblemError(
|
||||
status_code=403,
|
||||
detail=problem_payload(code="AGENT_FORBIDDEN", detail="Forbidden"),
|
||||
)
|
||||
|
||||
# With params
|
||||
raise ApiProblemError(
|
||||
status_code=422,
|
||||
detail=problem_payload(
|
||||
code="INVALID_RUNTIME_MODE",
|
||||
detail="Invalid runtime mode",
|
||||
params={"mode": mode, "valid_modes": ["standard", "divination"]}
|
||||
),
|
||||
)
|
||||
```
|
||||
|
||||
**Properties:**
|
||||
- `status_code`: HTTP status (int)
|
||||
- `code`: Machine-readable error code (str, `UPPER_SNAKE_CASE`)
|
||||
- `detail`: Human-readable message (str)
|
||||
- `params`: Optional additional context (dict)
|
||||
|
||||
### `HTTPException` (FastAPI)
|
||||
|
||||
**Use ONLY in router layer for HTTP transport errors:**
|
||||
|
||||
```python
|
||||
from fastapi import HTTPException, status
|
||||
|
||||
# Only in routers, for simple HTTP errors
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail="Resource not found"
|
||||
)
|
||||
```
|
||||
|
||||
**For business logic → use `ApiProblemError` instead.**
|
||||
|
||||
---
|
||||
|
||||
## Error Handling Patterns
|
||||
|
||||
### Service Layer (Business Logic)
|
||||
|
||||
**Services should raise `ApiProblemError`:**
|
||||
|
||||
```python
|
||||
class AgentService:
|
||||
async def enqueue_run(self, *, run_input: RunAgentInput, current_user: CurrentUser) -> TaskAccepted:
|
||||
# Validate business rules
|
||||
if owner_id != str(current_user.id):
|
||||
raise ApiProblemError(
|
||||
status_code=403,
|
||||
detail=problem_payload(
|
||||
code="AGENT_FORBIDDEN",
|
||||
detail="Forbidden"
|
||||
),
|
||||
)
|
||||
|
||||
# Return success result
|
||||
return TaskAccepted(run_id=run_id, thread_id=thread_id)
|
||||
```
|
||||
|
||||
### Repository Layer (Data Access)
|
||||
|
||||
**Re-raise exceptions as `ApiProblemError` when needed:**
|
||||
|
||||
```python
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
from core.http.errors import ApiProblemError, problem_payload
|
||||
|
||||
class AgentRepository(BaseRepository[AgentChatSession]):
|
||||
async def create_session(self, ...) -> AgentChatSession:
|
||||
try:
|
||||
self._session.add(session)
|
||||
await self._session.flush()
|
||||
return session
|
||||
except IntegrityError as exc:
|
||||
raise ApiProblemError(
|
||||
status_code=409,
|
||||
detail=problem_payload(
|
||||
code="DUPLICATE_SESSION",
|
||||
detail="Session already exists"
|
||||
),
|
||||
) from exc
|
||||
```
|
||||
|
||||
### Router Layer (HTTP Endpoints)
|
||||
|
||||
**Routers handle `ApiProblemError` and convert to HTTP response:**
|
||||
|
||||
```python
|
||||
from fastapi import APIRouter, HTTPException
|
||||
from core.http.errors import ApiProblemError
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
@router.post("/agent/run")
|
||||
async def create_run(
|
||||
response: Response,
|
||||
current_user: CurrentUser = Depends(get_current_user),
|
||||
service: AgentService = Depends(get_agent_service),
|
||||
):
|
||||
try:
|
||||
result = await service.enqueue_run(run_input=input, current_user=current_user)
|
||||
return result
|
||||
except ApiProblemError as exc:
|
||||
response.status_code = exc.status_code
|
||||
return {
|
||||
"code": exc.code,
|
||||
"detail": exc.detail,
|
||||
**({"params": exc.params} if exc.params else {}),
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## API Error Responses
|
||||
|
||||
### RFC 7807 Format
|
||||
|
||||
**All API errors return `application/problem+json`:**
|
||||
|
||||
```json
|
||||
{
|
||||
"code": "AGENT_FORBIDDEN",
|
||||
"detail": "Forbidden",
|
||||
"params": {
|
||||
"resource": "agent_session",
|
||||
"action": "update"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Error code registry:**
|
||||
- **Single source of truth**: `docs/protocols/common/http-error-codes.md`
|
||||
- **Update requirement**: Any create/modify/deprecate of codes requires doc update
|
||||
|
||||
### Error Response Construction
|
||||
|
||||
**Use `problem_payload()` helper:**
|
||||
|
||||
```python
|
||||
from core.http.errors import problem_payload
|
||||
|
||||
# Simple error
|
||||
detail = problem_payload(
|
||||
code="INVALID_INPUT",
|
||||
detail="Invalid input data"
|
||||
)
|
||||
|
||||
# With params
|
||||
detail = problem_payload(
|
||||
code="VALIDATION_ERROR",
|
||||
detail="Validation failed",
|
||||
params={"field": "email", "reason": "invalid format"}
|
||||
)
|
||||
|
||||
# Pass to ApiProblemError
|
||||
raise ApiProblemError(status_code=422, detail=detail)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Common Mistakes
|
||||
|
||||
### ❌ Catching and ignoring exceptions
|
||||
|
||||
```python
|
||||
# WRONG: Silent failure destroys debuggability
|
||||
try:
|
||||
await service.do_something()
|
||||
except Exception:
|
||||
pass # Never do this
|
||||
```
|
||||
|
||||
**Right way:** Re-raise or convert to typed error:
|
||||
|
||||
```python
|
||||
try:
|
||||
await service.do_something()
|
||||
except SomeSpecificError as exc:
|
||||
raise ApiProblemError(
|
||||
status_code=400,
|
||||
detail=problem_payload(code="OPERATION_FAILED", detail=str(exc)),
|
||||
) from exc
|
||||
```
|
||||
|
||||
### ❌ Using `print()` for errors
|
||||
|
||||
```python
|
||||
# WRONG: Use logging
|
||||
except Exception as e:
|
||||
print(f"Error: {e}") # Never use print in runtime code
|
||||
```
|
||||
|
||||
**Right way:** Use `core.logging`:
|
||||
|
||||
```python
|
||||
from core.logging import get_logger
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
||||
try:
|
||||
await service.do_something()
|
||||
except Exception as e:
|
||||
logger.error("operation_failed", error=str(e), exc_info=True)
|
||||
raise
|
||||
```
|
||||
|
||||
### ❌ Using free-text `detail` as only error contract
|
||||
|
||||
```python
|
||||
# WRONG: No machine-readable code
|
||||
raise HTTPException(status_code=400, detail="Something went wrong")
|
||||
```
|
||||
|
||||
**Right way:** Use `code` field:
|
||||
|
||||
```python
|
||||
raise ApiProblemError(
|
||||
status_code=400,
|
||||
detail=problem_payload(code="OPERATION_FAILED", detail="Something went wrong"),
|
||||
)
|
||||
```
|
||||
|
||||
### ❌ Using `HTTPException` in service/repository layer
|
||||
|
||||
```python
|
||||
# WRONG: HTTP-specific error in business logic
|
||||
class AgentService:
|
||||
async def enqueue_run(self, ...):
|
||||
raise HTTPException(status_code=403, detail="Forbidden") # Coupled to HTTP
|
||||
```
|
||||
|
||||
**Right way:** Use domain error:
|
||||
|
||||
```python
|
||||
class AgentService:
|
||||
async def enqueue_run(self, ...):
|
||||
raise ApiProblemError(
|
||||
status_code=403,
|
||||
detail=problem_payload(code="AGENT_FORBIDDEN", detail="Forbidden"),
|
||||
)
|
||||
```
|
||||
|
||||
### ❌ Not logging exceptions
|
||||
|
||||
```python
|
||||
# WRONG: Exception is caught but not logged
|
||||
try:
|
||||
await service.do_something()
|
||||
except Exception:
|
||||
raise ApiProblemError(...) # Where did it come from?
|
||||
```
|
||||
|
||||
**Right way:** Log before re-raising:
|
||||
|
||||
```python
|
||||
try:
|
||||
await service.do_something()
|
||||
except Exception as exc:
|
||||
logger.error("operation_failed", error=str(exc), exc_info=True)
|
||||
raise ApiProblemError(...) from exc
|
||||
```
|
||||
@@ -0,0 +1,38 @@
|
||||
# Backend Development Guidelines
|
||||
|
||||
> Best practices for backend development in this project.
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This directory contains guidelines for backend development. Fill in each file with your project's specific conventions.
|
||||
|
||||
---
|
||||
|
||||
## Guidelines Index
|
||||
|
||||
| Guide | Description | Status |
|
||||
|-------|-------------|--------|
|
||||
| [Directory Structure](./directory-structure.md) | Module organization and file layout | Filled |
|
||||
| [Database Guidelines](./database-guidelines.md) | ORM patterns, queries, migrations | Filled |
|
||||
| [Error Handling](./error-handling.md) | Error types, handling strategies | Filled |
|
||||
| [Quality Guidelines](./quality-guidelines.md) | Code standards, forbidden patterns | Filled |
|
||||
| [Logging Guidelines](./logging-guidelines.md) | Structured logging, log levels | Filled |
|
||||
|
||||
---
|
||||
|
||||
## How to Fill These Guidelines
|
||||
|
||||
For each guideline file:
|
||||
|
||||
1. Document your project's **actual conventions** (not ideals)
|
||||
2. Include **code examples** from your codebase
|
||||
3. List **forbidden patterns** and why
|
||||
4. Add **common mistakes** your team has made
|
||||
|
||||
The goal is to help AI assistants and new team members understand how YOUR project works.
|
||||
|
||||
---
|
||||
|
||||
**Language**: All documentation should be written in **English**.
|
||||
@@ -0,0 +1,340 @@
|
||||
# Logging Guidelines
|
||||
|
||||
> How logging is done in this project.
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This project uses **structlog** for structured, JSON-formatted logging:
|
||||
|
||||
- **Library**: `structlog` with standard library integration
|
||||
- **Interface**: `get_logger(name)` from `core.logging`
|
||||
- **Format**: JSON logs in production, readable logs in development
|
||||
- **Sensitive fields**: Automatically redacted (passwords, tokens, etc.)
|
||||
|
||||
---
|
||||
|
||||
## Log Levels
|
||||
|
||||
Use appropriate log levels based on event importance:
|
||||
|
||||
| Level | When to Use | Examples | Noise Level |
|
||||
|-------|-------------|----------|-------------|
|
||||
| **ERROR** | All exceptions and failures | Database connection failed, unhandled exception | Required, never skip |
|
||||
| **WARNING** | Degraded behavior, retry, fallback | Cache miss, using fallback data, retry attempt | Minimal, only when action taken |
|
||||
| **INFO** | Key business events | User login, message sent, run started | Minimal, only milestones |
|
||||
| **DEBUG** | Detailed flow tracing (dev only) | Function entry/exit, variable values | High, avoid in release |
|
||||
|
||||
---
|
||||
|
||||
## Structured Logging
|
||||
|
||||
### Logger Setup
|
||||
|
||||
```python
|
||||
from core.logging import get_logger
|
||||
|
||||
logger = get_logger("v1.agent.service")
|
||||
```
|
||||
|
||||
**Module naming convention:**
|
||||
- Feature modules: `v1.<feature>.<component>` (e.g., `v1.agent.service`)
|
||||
- Core modules: `core.<module>.<submodule>` (e.g., `core.logging.middleware`)
|
||||
|
||||
### Log Format
|
||||
|
||||
**JSON structure:**
|
||||
|
||||
```json
|
||||
{
|
||||
"event": "user_logged_in",
|
||||
"user_id": "123e4567-e89b-12d3-a456-426614174000",
|
||||
"timestamp": "2026-04-10T12:34:56.789Z",
|
||||
"level": "info",
|
||||
"logger": "v1.agent.service",
|
||||
"process": 12345,
|
||||
"thread": 140123456789024
|
||||
}
|
||||
```
|
||||
|
||||
### Logging Methods
|
||||
|
||||
```python
|
||||
# Simple event
|
||||
logger.info("user_logged_in", user_id=user.id)
|
||||
|
||||
# With extra context
|
||||
logger.info(
|
||||
"run_enqueued",
|
||||
run_id=run_id,
|
||||
thread_id=thread_id,
|
||||
user_id=current_user.id,
|
||||
)
|
||||
|
||||
# Error with exception info
|
||||
try:
|
||||
await repository.create_session(...)
|
||||
except IntegrityError as exc:
|
||||
logger.error(
|
||||
"session_creation_failed",
|
||||
error=str(exc),
|
||||
exc_info=True, # Include stack trace
|
||||
user_id=user_id,
|
||||
)
|
||||
raise
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## What to Log
|
||||
|
||||
### Always Log
|
||||
|
||||
1. **Exceptions**: Every catch block that handles an exception must log it
|
||||
2. **Business milestones**: Key domain events (login, run started, message sent)
|
||||
3. **Failed operations**: Any operation that fails or requires retry
|
||||
4. **Auth events**: Login/logout, token refresh, auth failures
|
||||
5. **External calls**: LLM API calls, database queries (at DEBUG level)
|
||||
|
||||
### Log with Context
|
||||
|
||||
```python
|
||||
# Include relevant identifiers
|
||||
logger.info(
|
||||
"run_completed",
|
||||
run_id=run_id,
|
||||
thread_id=thread_id,
|
||||
duration_seconds=duration,
|
||||
)
|
||||
|
||||
# Include error details
|
||||
logger.error(
|
||||
"database_error",
|
||||
operation="create_session",
|
||||
error=str(exc),
|
||||
exc_info=True,
|
||||
)
|
||||
```
|
||||
|
||||
### Log Levels by Event Type
|
||||
|
||||
**INFO - Business Milestones:**
|
||||
```python
|
||||
logger.info("user_logged_in", user_id=user.id)
|
||||
logger.info("run_started", run_id=run_id, thread_id=thread_id)
|
||||
logger.info("run_completed", run_id=run_id, duration_seconds=duration)
|
||||
logger.info("message_sent", message_id=msg_id, conversation_id=conv_id)
|
||||
```
|
||||
|
||||
**WARNING - Degraded Behavior:**
|
||||
```python
|
||||
logger.warning("cache_miss", key=cache_key, fallback="reload_from_db")
|
||||
logger.warning("retry_attempt", operation="llm_call", attempt=2, max_attempts=3)
|
||||
logger.warning("fallback_data_used", reason="external_api_timeout")
|
||||
```
|
||||
|
||||
**ERROR - Failures:**
|
||||
```python
|
||||
logger.error("database_connection_failed", error=str(exc), exc_info=True)
|
||||
logger.error("validation_failed", field="email", reason="invalid_format")
|
||||
logger.error("auth_failed", user_id=user_id, reason="invalid_token")
|
||||
try:
|
||||
await service.do_operation()
|
||||
except Exception as exc:
|
||||
logger.error("operation_failed", error=str(exc), exc_info=True)
|
||||
raise
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## What NOT to Log
|
||||
|
||||
### Forbidden: Sensitive Data
|
||||
|
||||
**Never log:**
|
||||
- Passwords (even hashed)
|
||||
- API keys (`ERYAO__*` environment variables)
|
||||
- JWT tokens
|
||||
- Access tokens / refresh tokens
|
||||
- Session IDs (except for tracing)
|
||||
- PII (personally identifiable information) - names, emails, phone numbers
|
||||
- Credit card numbers
|
||||
- Social security numbers
|
||||
- Secret keys
|
||||
- Authorization headers
|
||||
- Cookies
|
||||
|
||||
### Forbidden: Debug in Production
|
||||
|
||||
```python
|
||||
# WRONG: Debug logs in release builds
|
||||
import os
|
||||
if os.getenv("DEBUG"):
|
||||
logger.debug("variable_value", var=some_data) # Never do this
|
||||
```
|
||||
|
||||
**Right way:** Use runtime log level configuration:
|
||||
|
||||
```python
|
||||
# Config handles log level based on environment
|
||||
# runtime:
|
||||
# log_level: "INFO" # production
|
||||
# log_level: "DEBUG" # development
|
||||
logger.debug("variable_value", var=some_data) # Will be filtered in production
|
||||
```
|
||||
|
||||
### Forbidden: Iteration Logs
|
||||
|
||||
```python
|
||||
# WRONG: Log every iteration
|
||||
for item in items:
|
||||
logger.debug("processing_item", item_id=item.id) # Too noisy
|
||||
process(item)
|
||||
```
|
||||
|
||||
**Right way:** Log only failures:
|
||||
|
||||
```python
|
||||
for item in items:
|
||||
try:
|
||||
process(item)
|
||||
except Exception as exc:
|
||||
logger.error("item_processing_failed", item_id=item.id, error=str(exc))
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Configuration
|
||||
|
||||
### Settings
|
||||
|
||||
```python
|
||||
# core.config.settings.RuntimeSettings
|
||||
class RuntimeSettings(BaseModel):
|
||||
log_level: str = "INFO" # DEBUG, INFO, WARNING, ERROR
|
||||
log_json: bool = True # JSON format (production) vs readable (dev)
|
||||
log_rotation: Literal["time", "size", "none"] = "time"
|
||||
log_rotation_when: str = "midnight" # Daily rotation
|
||||
log_rotation_interval: int = 1
|
||||
log_rotation_backup_count: int = 14 # Keep 14 days
|
||||
log_file_name: str = "" # Auto-generated: "{service_name}.log"
|
||||
log_sensitive_fields: list[str] = [
|
||||
"password", "secret", "token", "api_key",
|
||||
"authorization", "cookie", "client_ip", "user_id",
|
||||
]
|
||||
```
|
||||
|
||||
### Log Files
|
||||
|
||||
**Location:** `logs/{service_name}.log` and `logs/errors/{service_name}.error.log`
|
||||
|
||||
**Example:**
|
||||
- `logs/app.log` - All logs
|
||||
- `logs/errors/app.error.log` - Errors only
|
||||
|
||||
---
|
||||
|
||||
## Common Mistakes
|
||||
|
||||
### ❌ Using `print()` instead of logging
|
||||
|
||||
```python
|
||||
# WRONG: Never use print in runtime code
|
||||
print(f"User {user_id} logged in")
|
||||
```
|
||||
|
||||
**Right:** Use `logger.info`:
|
||||
|
||||
```python
|
||||
logger.info("user_logged_in", user_id=user_id)
|
||||
```
|
||||
|
||||
### ❌ Logging sensitive data
|
||||
|
||||
```python
|
||||
# WRONG: Logging password
|
||||
logger.info("user_login", username=username, password=password)
|
||||
```
|
||||
|
||||
**Right:** Exclude sensitive fields:
|
||||
|
||||
```python
|
||||
logger.info("user_login", username=username) # Password excluded
|
||||
```
|
||||
|
||||
### ❌ Skipping error logging
|
||||
|
||||
```python
|
||||
# WRONG: Exception is suppressed
|
||||
try:
|
||||
await service.do_something()
|
||||
except Exception:
|
||||
pass # No logging, no re-raise
|
||||
```
|
||||
|
||||
**Right:** Log and re-raise:
|
||||
|
||||
```python
|
||||
try:
|
||||
await service.do_something()
|
||||
except Exception as exc:
|
||||
logger.error("operation_failed", error=str(exc), exc_info=True)
|
||||
raise
|
||||
```
|
||||
|
||||
### ❌ Info flooding
|
||||
|
||||
```python
|
||||
# WRONG: Log every database query
|
||||
results = await repository.get_all()
|
||||
logger.info("query_completed", count=len(results)) # Too noisy
|
||||
```
|
||||
|
||||
**Right:** Log only significant operations:
|
||||
|
||||
```python
|
||||
results = await repository.get_all()
|
||||
# Only log if something notable happens
|
||||
if len(results) > MAX_THRESHOLD:
|
||||
logger.warning("high_result_count", count=len(results))
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Examples from Codebase
|
||||
|
||||
### Service Layer Logging
|
||||
|
||||
```python
|
||||
# v1/agent/service.py
|
||||
logger = get_logger(__name__)
|
||||
|
||||
async def enqueue_run(self, ...) -> TaskAccepted:
|
||||
logger.info(
|
||||
"run_enqueued",
|
||||
run_id=run_id,
|
||||
thread_id=thread_id,
|
||||
user_id=current_user.id,
|
||||
)
|
||||
# ...
|
||||
```
|
||||
|
||||
### Router Layer Logging
|
||||
|
||||
```python
|
||||
# v1/agent/router.py
|
||||
logger = get_logger("v1.agent.router")
|
||||
|
||||
async def _acquire_sse_slot(*, user_id: str) -> bool:
|
||||
try:
|
||||
redis = await get_or_init_redis_client()
|
||||
# ...
|
||||
except Exception as exc: # noqa: BLE001
|
||||
logger.warning(
|
||||
"sse_slot_acquire_failed",
|
||||
user_id=user_id,
|
||||
reason=str(exc),
|
||||
)
|
||||
return True # Graceful degradation
|
||||
```
|
||||
@@ -0,0 +1,396 @@
|
||||
# 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
|
||||
|
||||
```python
|
||||
# WRONG: Suppress linter warnings
|
||||
result = some_function() # noqa: BLE001, reportImplicitRelativeImport
|
||||
```
|
||||
|
||||
**Right way:** Fix the underlying issue:
|
||||
|
||||
```python
|
||||
# Fix exception handling
|
||||
except Exception as exc:
|
||||
logger.error("operation_failed", error=str(exc))
|
||||
raise
|
||||
```
|
||||
|
||||
### ❌ Using `print()` in Runtime Code
|
||||
|
||||
```python
|
||||
# WRONG: Never use print in production code
|
||||
print(f"Processing user {user_id}")
|
||||
```
|
||||
|
||||
**Right way:** Use `core.logging`:
|
||||
|
||||
```python
|
||||
from core.logging import get_logger
|
||||
|
||||
logger = get_logger(__name__)
|
||||
logger.info("processing_user", user_id=user_id)
|
||||
```
|
||||
|
||||
### ❌ Hardcoding Secrets
|
||||
|
||||
```python
|
||||
# WRONG: Hardcode credentials
|
||||
api_key = "sk-abcd1234..."
|
||||
password = "postgres123"
|
||||
```
|
||||
|
||||
**Right way:** Use `core.config.settings`:
|
||||
|
||||
```python
|
||||
from core.config.settings import config
|
||||
|
||||
api_key = config.llm.provider_keys["openai"]
|
||||
password = config.database.password
|
||||
```
|
||||
|
||||
### ❌ Manual Environment Parsing
|
||||
|
||||
```python
|
||||
# WRONG: Direct os.getenv
|
||||
import os
|
||||
db_host = os.getenv("DATABASE_HOST", "localhost")
|
||||
```
|
||||
|
||||
**Right way:** Use `Settings`:
|
||||
|
||||
```python
|
||||
from core.config.settings import config
|
||||
|
||||
db_host = config.database.host
|
||||
```
|
||||
|
||||
### ❌ Silent Exception Handling
|
||||
|
||||
```python
|
||||
# WRONG: Swallow exception
|
||||
try:
|
||||
await service.do_something()
|
||||
except Exception:
|
||||
pass # Destroys debuggability
|
||||
```
|
||||
|
||||
**Right way:** Log and propagate:
|
||||
|
||||
```python
|
||||
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
|
||||
|
||||
```python
|
||||
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
|
||||
|
||||
```python
|
||||
# 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
|
||||
|
||||
```python
|
||||
# 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
|
||||
|
||||
```python
|
||||
# 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
|
||||
|
||||
```python
|
||||
# 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
|
||||
|
||||
```python
|
||||
# pyproject.toml
|
||||
[tool.pytest.ini_options]
|
||||
testpaths = ["backend/tests"]
|
||||
addopts = "-q --import-mode=importlib"
|
||||
asyncio_mode = "auto"
|
||||
pythonpath = ["backend/src"]
|
||||
```
|
||||
|
||||
### Example Test
|
||||
|
||||
```python
|
||||
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:**
|
||||
```bash
|
||||
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:**
|
||||
```bash
|
||||
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:**
|
||||
```bash
|
||||
uv run pre-commit install
|
||||
```
|
||||
|
||||
**Runs automatically on:**
|
||||
- `ruff check`
|
||||
- `basedpyright`
|
||||
- Formatting checks
|
||||
|
||||
---
|
||||
|
||||
## Common Mistakes
|
||||
|
||||
### ❌ Missing Type Annotations
|
||||
|
||||
```python
|
||||
# WRONG: No type hints
|
||||
def get_user(user_id):
|
||||
return repository.get_by_id(user_id)
|
||||
```
|
||||
|
||||
**Right:**
|
||||
```python
|
||||
async def get_user(user_id: UUID) -> User | None:
|
||||
return await repository.get_by_id(user_id)
|
||||
```
|
||||
|
||||
### ❌ Catch-All Exception Handling
|
||||
|
||||
```python
|
||||
# WRONG: Catching all exceptions without logging
|
||||
except Exception:
|
||||
return None
|
||||
```
|
||||
|
||||
**Right:**
|
||||
```python
|
||||
except SomeSpecificError as exc:
|
||||
logger.error("operation_failed", error=str(exc), exc_info=True)
|
||||
raise ApiProblemError(...) from exc
|
||||
```
|
||||
|
||||
### ❌ Timezone-Naive Datetimes
|
||||
|
||||
```python
|
||||
# WRONG: No timezone
|
||||
created_at = datetime.now()
|
||||
```
|
||||
|
||||
**Right:**
|
||||
```python
|
||||
created_at = datetime.now(timezone.utc)
|
||||
```
|
||||
|
||||
### ❌ Testing with Hardcoded Credentials
|
||||
|
||||
```python
|
||||
# WRONG: Hardcoded test DB
|
||||
DATABASE_URL = "postgres://user:pass@localhost:5432/test"
|
||||
```
|
||||
|
||||
**Right:**
|
||||
```python
|
||||
from core.config.settings import config
|
||||
|
||||
# Use settings.test.*
|
||||
assert config.database.host is not None
|
||||
```
|
||||
@@ -0,0 +1,297 @@
|
||||
# Directory Structure
|
||||
|
||||
> How Flutter app code is organized in this project.
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This Flutter app follows a **feature-first architecture** with clear separation of concerns:
|
||||
|
||||
- **Feature modules** in `features/` for bounded product capabilities
|
||||
- **Core infrastructure** in `core/` for cross-feature protocols
|
||||
- **Shared UI components** in `shared/` for reusable widgets
|
||||
- **Data layer** in `data/` for infrastructure abstractions
|
||||
|
||||
---
|
||||
|
||||
## Directory Layout
|
||||
|
||||
```
|
||||
apps/lib/
|
||||
├── main.dart # Only root entry file
|
||||
├── app/ # App bootstrap & DI
|
||||
│ ├── di/ # Dependency injection setup
|
||||
│ ├── router.dart # Route definitions
|
||||
│ └── app.dart # App configuration
|
||||
├── core/ # Cross-feature infrastructure
|
||||
│ ├── auth/ # Session store, auth state
|
||||
│ ├── config/ # Env configuration
|
||||
│ ├── logging/ # Structured logging
|
||||
│ └── network/ # HTTP client, error mapping
|
||||
├── data/ # Shared infrastructure ONLY
|
||||
│ ├── cache/ # Cache implementations
|
||||
│ ├── network/ # Network adapters
|
||||
│ └── storage/ # Local storage
|
||||
├── features/ # Feature modules
|
||||
│ ├── auth/ # Authentication feature
|
||||
│ ├── home/ # Home feature
|
||||
│ ├── divination/ # Divination feature
|
||||
│ ├── settings/ # Settings feature
|
||||
│ └── ... # Other features
|
||||
├── shared/ # Reusable UI components
|
||||
│ ├── widgets/ # Shared widgets
|
||||
│ └── theme/ # App theme, design tokens
|
||||
└── l10n/ # Localization
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Module Organization
|
||||
|
||||
### Feature Module Structure
|
||||
|
||||
Each feature follows consistent structure:
|
||||
|
||||
```
|
||||
features/<feature>/
|
||||
├── data/ # Data layer
|
||||
│ ├── apis/ # API clients
|
||||
│ ├── repositories/ # Repository implementations
|
||||
│ ├── services/ # Feature-specific services
|
||||
│ └── models/ # Data models/DTOs
|
||||
└── presentation/ # Presentation layer
|
||||
├── bloc/ # State management (BLoC/Cubit)
|
||||
├── screens/ # Screen widgets
|
||||
└── widgets/ # Feature-specific widgets
|
||||
```
|
||||
|
||||
**Example: `features/auth/`**
|
||||
|
||||
```
|
||||
features/auth/
|
||||
├── data/
|
||||
│ ├── apis/
|
||||
│ │ └── auth_api.dart # HTTP API calls
|
||||
│ ├── repositories/
|
||||
│ │ └── auth_repository.dart # Repository implementation
|
||||
│ └── models/
|
||||
│ ├── auth_user.dart # User model
|
||||
│ └── session_response.dart # Session DTO
|
||||
└── presentation/
|
||||
├── bloc/
|
||||
│ ├── auth_bloc.dart # AuthBloc (ChangeNotifier)
|
||||
│ └── auth_state.dart # AuthState
|
||||
└── screens/
|
||||
└── login_screen.dart # Login screen widget
|
||||
```
|
||||
|
||||
### Core Module Structure
|
||||
|
||||
**Core contains cross-feature infrastructure:**
|
||||
|
||||
```
|
||||
core/
|
||||
├── auth/
|
||||
│ └── session_store.dart # Global session management
|
||||
├── config/
|
||||
│ └── env.dart # Environment configuration
|
||||
├── logging/
|
||||
│ ├── logger.dart # Logger interface
|
||||
│ ├── log_service.dart # LogService implementation
|
||||
│ ├── log_entry.dart # Log entry model
|
||||
│ └── error_handler.dart # Global error handler
|
||||
└── network/
|
||||
├── api_problem.dart # RFC7807 error model
|
||||
└── api_problem_mapper.dart # Error mapping
|
||||
```
|
||||
|
||||
### Shared Widget Structure
|
||||
|
||||
**Shared contains reusable UI components:**
|
||||
|
||||
```
|
||||
shared/
|
||||
├── widgets/
|
||||
│ ├── app_banner.dart # App-wide banner
|
||||
│ ├── app_loading_indicator.dart # Loading indicator
|
||||
│ ├── bottom_nav_bar.dart # Navigation bar
|
||||
│ └── divination/ # Divination domain widgets
|
||||
│ ├── gua_icon.dart # Gua icon widget
|
||||
│ ├── yao_glyph.dart # Yao glyph widget
|
||||
│ └── ...
|
||||
└── theme/
|
||||
├── app_theme.dart # Theme definition
|
||||
└── design_tokens.dart # Spacing, radius, colors
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Placement Rules
|
||||
|
||||
### Where to Put Code
|
||||
|
||||
| Code Type | Location | Reason |
|
||||
|-----------|----------|--------|
|
||||
| Feature business logic | `features/<feature>/` | Bounded context |
|
||||
| Cross-feature protocol | `core/` | Shared by multiple features |
|
||||
| Reusable UI widget | `shared/widgets/` | Reusable by multiple screens |
|
||||
| Infrastructure abstraction | `data/` | Cache/network/storage |
|
||||
| Feature repository/model | `features/<feature>/data/` | Feature-scoped data |
|
||||
|
||||
### Decision Tree
|
||||
|
||||
```
|
||||
Is it feature-specific business logic?
|
||||
→ Yes: features/<feature>/
|
||||
→ No: Is it reusable UI?
|
||||
→ Yes: shared/widgets/
|
||||
→ No: Is it infrastructure?
|
||||
→ Yes: data/
|
||||
→ No: Is it cross-feature protocol?
|
||||
→ Yes: core/
|
||||
→ No: Re-evaluate
|
||||
```
|
||||
|
||||
### Forbidden Patterns
|
||||
|
||||
**❌ Do NOT:**
|
||||
|
||||
1. Place feature business repositories in `data/`
|
||||
- Wrong: `data/repositories/auth_repository.dart`
|
||||
- Right: `features/auth/data/repositories/auth_repository.dart`
|
||||
|
||||
2. Create directories under `lib/` other than allowed second-level
|
||||
- Wrong: `lib/utils/`, `lib/helpers/`, `lib/constants/`
|
||||
- Right: Use `core/`, `shared/`, or feature-specific locations
|
||||
|
||||
3. Put feature-specific UI in `shared/widgets/`
|
||||
- Wrong: `shared/widgets/auth_login_form.dart`
|
||||
- Right: `features/auth/presentation/widgets/login_form.dart`
|
||||
|
||||
4. Import feature data layer from other features
|
||||
- Wrong: `import 'package:app/features/auth/data/repositories/auth_repository.dart'` in `features/home/`
|
||||
- Right: Access via app-level facade or DI
|
||||
|
||||
---
|
||||
|
||||
## Naming Conventions
|
||||
|
||||
### Files
|
||||
|
||||
- **snake_case**: `auth_bloc.dart`, `login_screen.dart`
|
||||
- **Feature prefix for shared**: `app_loading_indicator.dart`, `app_banner.dart`
|
||||
|
||||
### Classes
|
||||
|
||||
- **PascalCase**: `AuthBloc`, `AuthState`, `LoginScreen`
|
||||
- **Suffixes**:
|
||||
- `*Bloc` / `*Cubit` - State management
|
||||
- `*Repository` - Data access
|
||||
- `*Api` - HTTP clients
|
||||
- `*Service` - Business services
|
||||
- `*Screen` - Screen widgets
|
||||
- `*Widget` - Reusable widgets
|
||||
|
||||
### Directories
|
||||
|
||||
- **Plural for collections**: `screens/`, `widgets/`, `models/`
|
||||
- **Singular for features**: `auth/`, `home/`, `divination/`
|
||||
|
||||
---
|
||||
|
||||
## Examples
|
||||
|
||||
### Well-organized Feature: `features/divination/`
|
||||
|
||||
```
|
||||
features/divination/
|
||||
├── data/
|
||||
│ ├── apis/
|
||||
│ │ └── divination_api.dart # HTTP API
|
||||
│ ├── repositories/
|
||||
│ │ └── divination_repository.dart # Repository
|
||||
│ ├── services/
|
||||
│ │ └── voice_recorder.dart # Feature service
|
||||
│ └── models/
|
||||
│ ├── divination_result.dart # Domain model
|
||||
│ ├── divination_params.dart # Request params
|
||||
│ └── follow_up_message.dart # Message model
|
||||
└── presentation/
|
||||
└── screens/
|
||||
├── divination_screen.dart # Main screen
|
||||
├── auto_divination_screen.dart # Auto mode
|
||||
├── manual_divination_screen.dart # Manual mode
|
||||
└── follow_up_chat_screen.dart # Follow-up chat
|
||||
```
|
||||
|
||||
### Shared Widget: `shared/widgets/app_loading_indicator.dart`
|
||||
|
||||
```dart
|
||||
import 'package:flutter/material.dart';
|
||||
|
||||
class AppLoadingIndicator extends StatelessWidget {
|
||||
const AppLoadingIndicator({super.key});
|
||||
|
||||
@override
|
||||
Widget build(BuildContext context) {
|
||||
return const CircularProgressIndicator();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Core Infrastructure: `core/logging/logger.dart`
|
||||
|
||||
```dart
|
||||
import 'log_service.dart';
|
||||
|
||||
class Logger {
|
||||
final String module;
|
||||
|
||||
Logger(this.module, this._service);
|
||||
|
||||
static void setLogService(LogService service) {
|
||||
_globalLogService = service;
|
||||
}
|
||||
|
||||
void error({
|
||||
required String message,
|
||||
required Object error,
|
||||
required StackTrace stackTrace,
|
||||
}) {
|
||||
_service!.error(
|
||||
message: message,
|
||||
error: error,
|
||||
stackTrace: stackTrace,
|
||||
module: module,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Logger getLogger(String module) => Logger.get(module);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Key Principles
|
||||
|
||||
### Directory Contract (Must)
|
||||
|
||||
1. **Only allowed second-level directories**: `app/`, `core/`, `data/`, `features/`, `shared/`, `l10n/`
|
||||
2. **Only one root entry**: `lib/main.dart`
|
||||
3. **No ad-hoc directories**: No `utils/`, `helpers/`, `constants/` under `lib/`
|
||||
4. **Feature isolation**: Features should not import each other's data layer
|
||||
|
||||
### Layer Boundaries
|
||||
|
||||
1. **Presentation** → **Data** (via Repository interface)
|
||||
2. **Data** → **Core/Infrastructure** (via DI)
|
||||
3. **Core** → **Nothing** (foundation layer)
|
||||
4. **Shared** → **Core** (for utilities)
|
||||
5. **Feature** → **Core** + **Shared** (for cross-cutting concerns)
|
||||
|
||||
### Data Layer Boundary (Must)
|
||||
|
||||
- `data/` = infrastructure abstractions (cache/network/storage)
|
||||
- `features/<feature>/data/` = feature business repositories/models
|
||||
- **NEVER** mix these boundaries
|
||||
@@ -0,0 +1,505 @@
|
||||
# Error Handling
|
||||
|
||||
> How errors are handled in Flutter app.
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This app follows **RFC 7807 Problem Details** for error handling:
|
||||
|
||||
- **Error model**: `ApiProblem` class
|
||||
- **Error parsing**: `api_problem_mapper.dart` maps HTTP errors to `ApiProblem`
|
||||
- **Error codes**: Machine-readable codes from `docs/protocols/common/http-error-codes.md`
|
||||
- **User messages**: Safe, localized messages via `l10n`
|
||||
|
||||
---
|
||||
|
||||
## Error Types
|
||||
|
||||
### `ApiProblem`
|
||||
|
||||
**Custom exception for HTTP API errors:**
|
||||
|
||||
```dart
|
||||
// core/network/api_problem.dart
|
||||
class ApiProblem implements Exception {
|
||||
ApiProblem({
|
||||
required this.status,
|
||||
required this.title,
|
||||
required this.detail,
|
||||
this.code,
|
||||
});
|
||||
|
||||
final int status;
|
||||
final String title;
|
||||
final String detail;
|
||||
final String? code;
|
||||
|
||||
String toUserMessage() {
|
||||
return 'Request failed';
|
||||
}
|
||||
|
||||
@override
|
||||
String toString() => toUserMessage();
|
||||
}
|
||||
```
|
||||
|
||||
**Properties:**
|
||||
- `status`: HTTP status code (int)
|
||||
- `title`: Error title (str)
|
||||
- `detail`: Human-readable detail (str)
|
||||
- `code`: Machine-readable error code (str?, `UPPER_SNAKE_CASE`)
|
||||
|
||||
---
|
||||
|
||||
## Error Parsing
|
||||
|
||||
### RFC 7807 Response Format
|
||||
|
||||
**Backend returns `application/problem+json`:**
|
||||
|
||||
```json
|
||||
{
|
||||
"code": "AGENT_FORBIDDEN",
|
||||
"detail": "Forbidden",
|
||||
"params": {
|
||||
"resource": "agent_session",
|
||||
"action": "update"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Frontend parsing:**
|
||||
|
||||
```dart
|
||||
// core/network/api_problem_mapper.dart
|
||||
class ApiProblemMapper {
|
||||
static ApiProblem? tryParse(Response response) {
|
||||
if (response.statusCode < 400) return null;
|
||||
|
||||
try {
|
||||
final json = jsonDecode(response.body);
|
||||
return ApiProblem(
|
||||
status: response.statusCode,
|
||||
title: json['title'] ?? 'Error',
|
||||
detail: json['detail'] ?? 'Request failed',
|
||||
code: json['code'],
|
||||
);
|
||||
} catch (e) {
|
||||
return ApiProblem(
|
||||
status: response.statusCode,
|
||||
title: 'Error',
|
||||
detail: 'Request failed',
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Error Code Mapping
|
||||
|
||||
**Map error codes to l10n keys:**
|
||||
|
||||
```dart
|
||||
String getLocalizedErrorMessage(BuildContext context, ApiProblem problem) {
|
||||
final code = problem.code;
|
||||
|
||||
if (code == null) {
|
||||
return _getStatusGenericMessage(context, problem.status);
|
||||
}
|
||||
|
||||
// Map code to l10n key
|
||||
final l10nKey = _codeToL10nKey[code];
|
||||
if (l10nKey != null) {
|
||||
return AppLocalizations.of(context)!.getString(l10nKey);
|
||||
}
|
||||
|
||||
// Fallback
|
||||
return AppLocalizations.of(context)!.genericErrorMessage;
|
||||
}
|
||||
|
||||
const _codeToL10nKey = {
|
||||
'AGENT_FORBIDDEN': 'error_agent_forbidden',
|
||||
'INVALID_INPUT': 'error_invalid_input',
|
||||
'VALIDATION_ERROR': 'error_validation_failed',
|
||||
// ...
|
||||
};
|
||||
```
|
||||
|
||||
### Fallback Order
|
||||
|
||||
**Unknown error code handling:**
|
||||
|
||||
```
|
||||
1. code -> l10n key (if code exists)
|
||||
2. status -> status-generic localized message
|
||||
3. safe generic localized message
|
||||
```
|
||||
|
||||
```dart
|
||||
String getStatusGenericMessage(BuildContext context, int status) {
|
||||
switch (status) {
|
||||
case 401:
|
||||
return AppLocalizations.of(context)!.errorUnauthorized;
|
||||
case 403:
|
||||
return AppLocalizations.of(context)!.errorForbidden;
|
||||
case 404:
|
||||
return AppLocalizations.of(context)!.errorNotFound;
|
||||
case 500:
|
||||
return AppLocalizations.of(context)!.errorServerError;
|
||||
default:
|
||||
return AppLocalizations.of(context)!.genericErrorMessage;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Error Handling Patterns
|
||||
|
||||
### API Layer
|
||||
|
||||
**Catch network errors and map to ApiProblem:**
|
||||
|
||||
```dart
|
||||
// features/auth/data/apis/auth_api.dart
|
||||
class AuthApi {
|
||||
Future<AuthUser> loginWithEmailOtp({
|
||||
required String email,
|
||||
required String otp,
|
||||
}) async {
|
||||
try {
|
||||
final response = await _httpClient.post(
|
||||
'/auth/login',
|
||||
body: {'email': email, 'otp': otp},
|
||||
);
|
||||
|
||||
final problem = ApiProblemMapper.tryParse(response);
|
||||
if (problem != null) {
|
||||
throw problem;
|
||||
}
|
||||
|
||||
return AuthUser.fromJson(jsonDecode(response.body));
|
||||
} on SocketException {
|
||||
throw ApiProblem(
|
||||
status: 0,
|
||||
title: 'Network Error',
|
||||
detail: 'No internet connection',
|
||||
);
|
||||
} on TimeoutException {
|
||||
throw ApiProblem(
|
||||
status: 0,
|
||||
title: 'Timeout',
|
||||
detail: 'Request timed out',
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Repository Layer
|
||||
|
||||
**Propagate errors with context:**
|
||||
|
||||
```dart
|
||||
// features/auth/data/repositories/auth_repository.dart
|
||||
class AuthRepositoryImpl implements AuthRepository {
|
||||
final AuthApi _api;
|
||||
final SessionStore _sessionStore;
|
||||
|
||||
@override
|
||||
Future<AuthUser> loginWithEmailOtp({
|
||||
required String email,
|
||||
required String otp,
|
||||
}) async {
|
||||
try {
|
||||
final user = await _api.loginWithEmailOtp(email: email, otp: otp);
|
||||
await _sessionStore.save(user.session);
|
||||
return user;
|
||||
} on ApiProblem {
|
||||
rethrow;
|
||||
} catch (e, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Login failed',
|
||||
error: e,
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
throw ApiProblem(
|
||||
status: 500,
|
||||
title: 'Error',
|
||||
detail: 'Request failed, please try again',
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### BLoC/State Layer
|
||||
|
||||
**Handle and log errors:**
|
||||
|
||||
```dart
|
||||
// features/auth/presentation/bloc/auth_bloc.dart
|
||||
Future<void> start() async {
|
||||
_state = _state.copyWith(status: AuthStatus.loading);
|
||||
notifyListeners();
|
||||
|
||||
try {
|
||||
final user = await _repository.recoverSession();
|
||||
if (user == null) {
|
||||
_state = const AuthState(status: AuthStatus.unauthenticated);
|
||||
} else {
|
||||
_state = AuthState(status: AuthStatus.authenticated, user: user);
|
||||
}
|
||||
notifyListeners();
|
||||
} on ApiProblem catch (problem) {
|
||||
_logger.error(
|
||||
message: 'Session recovery failed',
|
||||
error: problem,
|
||||
stackTrace: StackTrace.current,
|
||||
);
|
||||
await _repository.clearLocalSession();
|
||||
_state = AuthState(
|
||||
status: AuthStatus.unauthenticated,
|
||||
errorMessage: problem.toUserMessage(),
|
||||
);
|
||||
notifyListeners();
|
||||
} catch (e, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Session recovery failed: ${e.runtimeType}',
|
||||
error: e,
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
await _repository.clearLocalSession();
|
||||
_state = AuthState(
|
||||
status: AuthStatus.unauthenticated,
|
||||
errorMessage: 'Request failed, please try again',
|
||||
);
|
||||
notifyListeners();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### UI Layer
|
||||
|
||||
**Display errors with Toast/Banner:**
|
||||
|
||||
```dart
|
||||
// features/auth/presentation/screens/login_screen.dart
|
||||
class LoginScreen extends StatelessWidget {
|
||||
Future<void> _handleLogin() async {
|
||||
try {
|
||||
await authBloc.loginWithOtp(email: email, otp: otp);
|
||||
// Success - navigate or show success
|
||||
} on ApiProblem catch (problem) {
|
||||
Toast.show(
|
||||
context: context,
|
||||
message: problem.toUserMessage(),
|
||||
type: ToastType.error,
|
||||
);
|
||||
} catch (e) {
|
||||
Toast.show(
|
||||
context: context,
|
||||
message: 'Request failed, please try again',
|
||||
type: ToastType.error,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Global Error Handling
|
||||
|
||||
### 401 Session Invalidation
|
||||
|
||||
**AuthBloc handles global 401 callback:**
|
||||
|
||||
```dart
|
||||
// features/auth/presentation/bloc/auth_bloc.dart
|
||||
class AuthBloc extends ChangeNotifier {
|
||||
bool _handlingUnauthorized = false;
|
||||
|
||||
Future<void> handleUnauthorized401() async {
|
||||
if (_handlingUnauthorized) return;
|
||||
_handlingUnauthorized = true;
|
||||
|
||||
try {
|
||||
await _repository.clearLocalSession();
|
||||
_logger.warning(message: 'Session invalidated by 401 callback');
|
||||
_state = const AuthState(status: AuthStatus.unauthenticated);
|
||||
notifyListeners();
|
||||
} finally {
|
||||
_handlingUnauthorized = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**HttpClient intercepts 401:**
|
||||
|
||||
```dart
|
||||
// core/network/http_client.dart
|
||||
class HttpClient {
|
||||
Future<Response> get(String path) async {
|
||||
final response = await _innerClient.get(path);
|
||||
|
||||
if (response.statusCode == 401) {
|
||||
// Trigger global 401 callback
|
||||
await ServiceLocator.authBloc.handleUnauthorized401();
|
||||
}
|
||||
|
||||
return response;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Common Mistakes
|
||||
|
||||
### ❌ Ignoring Error Codes
|
||||
|
||||
```dart
|
||||
// WRONG: Only using detail text
|
||||
if (response.statusCode == 400) {
|
||||
showError(json['detail']); // Unstable contract
|
||||
}
|
||||
```
|
||||
|
||||
**Right: Use code field:**
|
||||
|
||||
```dart
|
||||
final code = json['code'];
|
||||
final message = getLocalizedErrorMessage(context, code);
|
||||
showError(message);
|
||||
```
|
||||
|
||||
### ❌ Nested Try-Catch Without Logging
|
||||
|
||||
```dart
|
||||
// WRONG: Silent failure
|
||||
try {
|
||||
await operation();
|
||||
} catch (e) {
|
||||
// No logging, no re-raise
|
||||
return null;
|
||||
}
|
||||
```
|
||||
|
||||
**Right: Log and propagate:**
|
||||
|
||||
```dart
|
||||
try {
|
||||
await operation();
|
||||
} catch (e, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Operation failed',
|
||||
error: e,
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
rethrow;
|
||||
}
|
||||
```
|
||||
|
||||
### ❌ Feature-Level Token Clearing
|
||||
|
||||
```dart
|
||||
// WRONG: Feature clearing tokens directly
|
||||
class SomeBloc {
|
||||
Future<void> handleError() async {
|
||||
await tokenStore.clear(); // Wrong!
|
||||
state = ErrorState();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Right: Global callback via AuthBloc:**
|
||||
|
||||
```dart
|
||||
// Only AuthBloc should clear session
|
||||
// HttpClient triggers AuthBloc.handleUnauthorized401()
|
||||
```
|
||||
|
||||
### ❌ Localized Error from Detail
|
||||
|
||||
```dart
|
||||
// WRONG: Translating free-text detail
|
||||
final message = localize(json['detail']);
|
||||
```
|
||||
|
||||
**Right: Map code to l10n:**
|
||||
|
||||
```dart
|
||||
final code = json['code'];
|
||||
final l10nKey = _codeToL10nKey[code];
|
||||
final message = l10nKey != null
|
||||
? AppLocalizations.of(context)!.getString(l10nKey)
|
||||
: genericMessage;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Error Response Contract
|
||||
|
||||
**Single source of truth: `docs/protocols/common/http-error-codes.md`**
|
||||
|
||||
**Workflow:**
|
||||
|
||||
1. Backend defines new error code → update protocol doc
|
||||
2. Frontend updates code-to-l10n mapping
|
||||
3. Both sides use same `code` field
|
||||
|
||||
**Example protocol update:**
|
||||
|
||||
```markdown
|
||||
## AGENT_FORBIDDEN
|
||||
|
||||
- **Code**: `AGENT_FORBIDDEN`
|
||||
- **Status**: 403
|
||||
- **Detail**: "Forbidden"
|
||||
- **L10n Key**: `error_agent_forbidden`
|
||||
- **Description**: User does not have permission to access agent resource
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Testing Error Handling
|
||||
|
||||
### Unit Testing
|
||||
|
||||
```dart
|
||||
test('ApiProblemMapper parses error response', () {
|
||||
final response = MockResponse(
|
||||
statusCode: 403,
|
||||
body: jsonEncode({
|
||||
'code': 'AGENT_FORBIDDEN',
|
||||
'detail': 'Forbidden',
|
||||
}),
|
||||
);
|
||||
|
||||
final problem = ApiProblemMapper.tryParse(response);
|
||||
|
||||
expect(problem, isNotNull);
|
||||
expect(problem!.status, 403);
|
||||
expect(problem.code, 'AGENT_FORBIDDEN');
|
||||
});
|
||||
```
|
||||
|
||||
### Widget Testing
|
||||
|
||||
```dart
|
||||
testWidgets('Login screen shows error on failed login', (tester) async {
|
||||
await tester.pumpWidget(MyApp());
|
||||
|
||||
// Trigger error
|
||||
await tester.enterText(find.byKey(Key('email_field')), 'test@example.com');
|
||||
await tester.tap(find.byKey(Key('login_button')));
|
||||
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
expect(find.text('Request failed, please try again'), findsOneWidget);
|
||||
});
|
||||
```
|
||||
@@ -0,0 +1,122 @@
|
||||
# Flutter App Development Guidelines
|
||||
|
||||
> Best practices for Flutter app development in this project.
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This directory contains guidelines for Flutter app development. Fill in each file with your project's specific conventions.
|
||||
|
||||
---
|
||||
|
||||
## Guidelines Index
|
||||
|
||||
| Guide | Description | Status |
|
||||
|-------|-------------|--------|
|
||||
| [Directory Structure](./directory-structure.md) | Module organization and file layout | Filled |
|
||||
| [State Management](./state-management.md) | BLoC/Cubit, repository pattern, DI | Filled |
|
||||
| [Error Handling](./error-handling.md) | ApiProblem, error parsing, l10n mapping | Filled |
|
||||
| [Quality Guidelines](./quality-guidelines.md) | Code standards, forbidden patterns | Filled |
|
||||
| [Logging Guidelines](./logging-guidelines.md) | Structured logging, log levels | Filled |
|
||||
|
||||
---
|
||||
|
||||
## How These Guidelines Were Filled
|
||||
|
||||
Each guideline file was populated based on:
|
||||
|
||||
1. **Existing codebase patterns** from `apps/lib/`
|
||||
2. **AGENTS.md rules** from `apps/AGENTS.md`
|
||||
3. **Real code examples** from `features/auth/`, `core/logging/`, etc.
|
||||
|
||||
---
|
||||
|
||||
## Quick Start
|
||||
|
||||
### Directory Structure
|
||||
|
||||
- **Feature modules**: `features/<feature>/data/` + `features/<feature>/presentation/`
|
||||
- **Core infrastructure**: `core/` (cross-feature)
|
||||
- **Shared widgets**: `shared/widgets/`
|
||||
- **Data layer**: `data/` (infrastructure only, not feature repositories)
|
||||
|
||||
### State Management
|
||||
|
||||
- **Pattern**: `ChangeNotifier` + immutable state
|
||||
- **State**: Use `copyWith` pattern
|
||||
- **DI**: Singleton blocs/repositories via ServiceLocator
|
||||
|
||||
### Error Handling
|
||||
|
||||
- **Format**: RFC 7807 (`ApiProblem`)
|
||||
- **Mapping**: Error code → l10n key
|
||||
- **Global**: 401 handled via `AuthBloc.handleUnauthorized401()`
|
||||
|
||||
### Logging
|
||||
|
||||
- **Library**: Custom `Logger` class
|
||||
- **Module path**: `features.<feature>.<component>`
|
||||
- **Error**: Always log with `error`, `stackTrace`, `message`
|
||||
- **Forbidden**: PII, tokens, passwords
|
||||
|
||||
### Quality
|
||||
|
||||
- **Colors**: Semantic colors from `Theme.of(context).colorScheme`
|
||||
- **Spacing**: Use `AppSpacing` / `AppRadius` tokens
|
||||
- **Tests**: Unit, widget, integration tests for critical flows
|
||||
|
||||
---
|
||||
|
||||
## Key Principles
|
||||
|
||||
### Layer Boundaries
|
||||
|
||||
1. **Presentation** → **Repository** (via interface)
|
||||
2. **Repository** → **Core/Infrastructure** (via DI)
|
||||
3. **Core** → **Nothing** (foundation layer)
|
||||
4. **Shared** → **Core** (for utilities)
|
||||
5. **Feature** → **Core** + **Shared** (for cross-cutting concerns)
|
||||
|
||||
### Data Layer Boundary (Must)
|
||||
|
||||
- `data/` = infrastructure abstractions (cache/network/storage)
|
||||
- `features/<feature>/data/` = feature business repositories/models
|
||||
- **NEVER** mix these boundaries
|
||||
|
||||
### Error Code Contract (Must)
|
||||
|
||||
- **Single source of truth**: `docs/protocols/common/http-error-codes.md`
|
||||
- **Frontend mapping**: Error code → l10n key
|
||||
- **Backend updates → Frontend mapping updates** (must stay in sync)
|
||||
|
||||
---
|
||||
|
||||
## Common Anti-Patterns
|
||||
|
||||
### ❌ Do NOT:
|
||||
|
||||
1. Hardcode colors (`Color(0xFF...)`, `Colors.blue`)
|
||||
2. Hardcode spacing (`Padding(padding: EdgeInsets.all(16.0))`)
|
||||
3. Place feature repositories in shared `data/`
|
||||
4. Import feature data layer from other features
|
||||
5. Create new second-level directories under `lib/`
|
||||
6. Use `print()` instead of Logger
|
||||
7. Log PII/tokens/passwords
|
||||
8. Catch exceptions without logging
|
||||
|
||||
---
|
||||
|
||||
## Architecture Decision Records
|
||||
|
||||
When making architectural decisions, document:
|
||||
|
||||
1. **Context**: What problem are we solving?
|
||||
2. **Decision**: What approach did we choose?
|
||||
3. **Consequences**: What are the trade-offs?
|
||||
|
||||
Add new ADRs to this directory if needed.
|
||||
|
||||
---
|
||||
|
||||
**Language**: All documentation should be written in **English**.
|
||||
@@ -0,0 +1,542 @@
|
||||
# Logging Guidelines
|
||||
|
||||
> How logging is done in Flutter app.
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This app uses **structured logging** with custom `Logger` class:
|
||||
|
||||
- **Library**: Custom `Logger` class in `core/logging/`
|
||||
- **Interface**: `getLogger(module)` from `core/logging/logger.dart`
|
||||
- **Log levels**: `debug`, `info`, `warning`, `error`
|
||||
- **Sensitive fields**: Never log passwords, tokens, PII
|
||||
|
||||
---
|
||||
|
||||
## Logger Setup
|
||||
|
||||
### Import and Initialize
|
||||
|
||||
```dart
|
||||
import 'core/logging/logger.dart';
|
||||
|
||||
class SomeBloc extends ChangeNotifier {
|
||||
final Logger _logger = getLogger('features.auth.bloc');
|
||||
}
|
||||
```
|
||||
|
||||
### Module Naming Convention
|
||||
|
||||
| Feature | Module Path |
|
||||
|---------|------------|
|
||||
| auth | `features.auth` |
|
||||
| home | `features.home` |
|
||||
| divination | `features.divination` |
|
||||
| settings | `features.settings` |
|
||||
|
||||
**Examples:**
|
||||
|
||||
```dart
|
||||
// features/auth/presentation/bloc/auth_bloc.dart
|
||||
final Logger _logger = getLogger('features.auth.bloc');
|
||||
|
||||
// features/home/data/repositories/home_repository.dart
|
||||
final Logger _logger = getLogger('features.home.repository');
|
||||
|
||||
// core/network/http_client.dart
|
||||
final Logger _logger = getLogger('core.network.http_client');
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Log Levels
|
||||
|
||||
| Level | When to Use | Noise Level | Required |
|
||||
|-------|-------------|-------------|----------|
|
||||
| **error** | All exceptions and failures | Required | Never skip |
|
||||
| **warning** | Degraded behavior, retry, fallback | Minimal | Only when action taken |
|
||||
| **info** | Key business events | Minimal | Only milestones |
|
||||
| **debug** | Detailed flow tracing (dev only) | High | Avoid in release |
|
||||
|
||||
---
|
||||
|
||||
## Error Logging Requirements
|
||||
|
||||
### Every try-catch MUST log the exception:
|
||||
|
||||
```dart
|
||||
try {
|
||||
await _repository.someOperation();
|
||||
} catch (e, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Operation failed: $operationName',
|
||||
error: e,
|
||||
stackTrace: stackTrace,
|
||||
extra: {'context': 'relevant_data'},
|
||||
);
|
||||
// handle error
|
||||
}
|
||||
```
|
||||
|
||||
### Error Logging Pattern
|
||||
|
||||
```dart
|
||||
// features/auth/presentation/bloc/auth_bloc.dart
|
||||
Future<void> start() async {
|
||||
try {
|
||||
final user = await _repository.recoverSession();
|
||||
// ...
|
||||
} catch (error, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Session recovery failed: ${error.runtimeType}',
|
||||
error: error.runtimeType.toString(),
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
await _repository.clearLocalSession();
|
||||
_state = AuthState(
|
||||
status: AuthStatus.unauthenticated,
|
||||
errorMessage: _toSafeMessage(error),
|
||||
);
|
||||
notifyListeners();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Info Logging Requirements
|
||||
|
||||
### Only log these milestone events:
|
||||
|
||||
- User login/logout
|
||||
- Message sent/received
|
||||
- Data sync completed
|
||||
- Important state transitions
|
||||
|
||||
### Info Logging Pattern
|
||||
|
||||
```dart
|
||||
// Login success
|
||||
_logger.info(
|
||||
message: 'User logged in',
|
||||
extra: {'user_id': user.id},
|
||||
);
|
||||
|
||||
// Run success
|
||||
_logger.info(
|
||||
message: 'Run completed',
|
||||
extra: {
|
||||
'run_id': runId,
|
||||
'thread_id': threadId,
|
||||
'duration_ms': duration.inMilliseconds,
|
||||
},
|
||||
);
|
||||
```
|
||||
|
||||
### ❌ DO NOT log for every operation:
|
||||
|
||||
```dart
|
||||
// WRONG: Logging every keystroke
|
||||
onChanged: (value) {
|
||||
_logger.info('Input changed: $value'); // Too noisy
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Warning Logging Requirements
|
||||
|
||||
### Only log when taking corrective action:
|
||||
|
||||
- Retrying after failure
|
||||
- Using fallback data
|
||||
- Skipping malformed data
|
||||
- Deprecation warnings
|
||||
|
||||
### Warning Logging Pattern
|
||||
|
||||
```dart
|
||||
// Cache miss with fallback
|
||||
_logger.warning(
|
||||
message: 'Cache miss, loading from remote',
|
||||
extra: {'key': cacheKey},
|
||||
);
|
||||
|
||||
// Retry attempt
|
||||
_logger.warning(
|
||||
message: 'Retry attempt',
|
||||
extra: {'attempt': 2, 'max_attempts': 3},
|
||||
);
|
||||
|
||||
// Fallback data
|
||||
_logger.warning(
|
||||
message: 'Using fallback data due to network timeout',
|
||||
extra: {'timeout_ms': 5000},
|
||||
);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Debug Logging
|
||||
|
||||
### Use sparingly, only in debug builds:
|
||||
|
||||
```dart
|
||||
if (kDebugMode) {
|
||||
_logger.debug(
|
||||
message: 'Variable value',
|
||||
extra: {'variable': expensiveObject.toString()},
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
**Note:** Debug logs are automatically filtered in release builds.
|
||||
|
||||
---
|
||||
|
||||
## Prohibited Practices
|
||||
|
||||
### ❌ Never log sensitive data
|
||||
|
||||
```dart
|
||||
// WRONG: Logging password
|
||||
_logger.info(message: 'User login', extra: {'password': password});
|
||||
|
||||
// WRONG: Logging token
|
||||
_logger.debug(message: 'API call', extra: {'token': accessToken});
|
||||
|
||||
// WRONG: Logging PII
|
||||
_logger.info(message: 'User profile', extra: {'email': userEmail});
|
||||
```
|
||||
|
||||
### ❌ Never log at debug level in production
|
||||
|
||||
```dart
|
||||
// WRONG: Will log in release build
|
||||
_logger.debug(message: 'Debug info: $sensitiveData');
|
||||
```
|
||||
|
||||
**Right way:** Use `kDebugMode` guard:
|
||||
|
||||
```dart
|
||||
if (kDebugMode) {
|
||||
_logger.debug(message: 'Variable value', extra: {'var': value});
|
||||
}
|
||||
```
|
||||
|
||||
### ❌ Never skip error logging
|
||||
|
||||
```dart
|
||||
// WRONG: Exception is caught but not logged
|
||||
try {
|
||||
await operation();
|
||||
} catch (e) {
|
||||
state = ErrorState();
|
||||
// Missing error log!
|
||||
}
|
||||
```
|
||||
|
||||
### ❌ Never log in every iteration
|
||||
|
||||
```dart
|
||||
// WRONG: Log every iteration
|
||||
for (item in items) {
|
||||
_logger.debug('Processing item: ${item.id}'); // Too noisy
|
||||
process(item);
|
||||
}
|
||||
```
|
||||
|
||||
**Right: Log only failures:**
|
||||
|
||||
```dart
|
||||
for (item in items) {
|
||||
try {
|
||||
process(item);
|
||||
} catch (e, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Item processing failed',
|
||||
error: e,
|
||||
stackTrace: stackTrace,
|
||||
extra: {'item_id': item.id},
|
||||
);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Logger Implementation
|
||||
|
||||
### Core Logger Class
|
||||
|
||||
```dart
|
||||
// core/logging/logger.dart
|
||||
import 'log_entry.dart';
|
||||
import 'log_service.dart';
|
||||
|
||||
class Logger {
|
||||
final String module;
|
||||
final LogService? _service;
|
||||
final bool _isNoOp;
|
||||
|
||||
Logger(this.module, this._service) : _isNoOp = _service == null;
|
||||
|
||||
factory Logger.get(String module) {
|
||||
return Logger(module, _globalLogService);
|
||||
}
|
||||
|
||||
void error({
|
||||
required String message,
|
||||
required Object error,
|
||||
required StackTrace stackTrace,
|
||||
Map<String, dynamic>? extra,
|
||||
}) {
|
||||
if (_isNoOp) {
|
||||
debugPrint(LogEntry(
|
||||
message: message,
|
||||
module: module,
|
||||
errorType: error.runtimeType.toString(),
|
||||
).toConsoleString());
|
||||
return;
|
||||
}
|
||||
_service!.error(
|
||||
message: message,
|
||||
error: error,
|
||||
stackTrace: stackTrace,
|
||||
module: module,
|
||||
extra: extra ?? {},
|
||||
);
|
||||
}
|
||||
|
||||
void info({
|
||||
required String message,
|
||||
Map<String, dynamic>? extra,
|
||||
}) {
|
||||
if (_isNoOp) return;
|
||||
_service!.info(
|
||||
message: message,
|
||||
module: module,
|
||||
extra: extra ?? {},
|
||||
);
|
||||
}
|
||||
|
||||
void warning({
|
||||
required String message,
|
||||
Map<String, dynamic>? extra,
|
||||
}) {
|
||||
if (_isNoOp) return;
|
||||
_service!.warning(
|
||||
message: message,
|
||||
module: module,
|
||||
extra: extra ?? {},
|
||||
);
|
||||
}
|
||||
|
||||
void debug({
|
||||
required String message,
|
||||
Map<String, dynamic>? extra,
|
||||
}) {
|
||||
if (_isNoOp || !kDebugMode) return;
|
||||
_service!.debug(
|
||||
message: message,
|
||||
module: module,
|
||||
extra: extra ?? {},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Logger getLogger(String module) => Logger.get(module);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Log Entry Structure
|
||||
|
||||
### LogEntry Fields
|
||||
|
||||
```dart
|
||||
// core/logging/log_entry.dart
|
||||
class LogEntry {
|
||||
final DateTime timestamp;
|
||||
final LogLevel level;
|
||||
final String message;
|
||||
final String module;
|
||||
final String? errorType;
|
||||
final String? errorMessage;
|
||||
final String? stackTrace;
|
||||
final Map<String, dynamic>? extra;
|
||||
}
|
||||
|
||||
enum LogLevel { debug, info, warning, error }
|
||||
```
|
||||
|
||||
### Log Format
|
||||
|
||||
```json
|
||||
{
|
||||
"timestamp": "2026-04-10T12:34:56.789Z",
|
||||
"level": "error",
|
||||
"module": "features.auth.bloc",
|
||||
"message": "Session recovery failed: SocketException",
|
||||
"error_type": "SocketException",
|
||||
"error_message": "Connection refused",
|
||||
"stack_trace": "...",
|
||||
"extra": {
|
||||
"user_id": "123e4567-e89b-12d3-a456-426614174000"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Common Mistakes
|
||||
|
||||
### ❌ Using `print()` instead of Logger
|
||||
|
||||
```dart
|
||||
// WRONG: Never use print in production code
|
||||
print('User logged in: $userId');
|
||||
```
|
||||
|
||||
**Right:** Use `Logger`:
|
||||
|
||||
```dart
|
||||
_logger.info(message: 'User logged in', extra: {'user_id': userId});
|
||||
```
|
||||
|
||||
### ❌ Logging sensitive data
|
||||
|
||||
```dart
|
||||
// WRONG: Logging PII
|
||||
_logger.info(message: 'User profile', extra: {
|
||||
'email': userEmail,
|
||||
'phone': userPhone,
|
||||
});
|
||||
```
|
||||
|
||||
**Right:** Exclude sensitive fields:
|
||||
|
||||
```dart
|
||||
_logger.info(message: 'User profile loaded', extra: {'user_id': userId});
|
||||
```
|
||||
|
||||
### ❌ Catching without logging
|
||||
|
||||
```dart
|
||||
// WRONG: Silent failure
|
||||
try {
|
||||
await service.doSomething();
|
||||
} catch (e) {
|
||||
// No logging, no re-raise
|
||||
return null;
|
||||
}
|
||||
```
|
||||
|
||||
**Right:** Log and re-raise:
|
||||
|
||||
```dart
|
||||
try {
|
||||
await service.doSomething();
|
||||
} catch (e, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Operation failed',
|
||||
error: e,
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
rethrow;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Examples from Codebase
|
||||
|
||||
### AuthBloc Error Handling
|
||||
|
||||
```dart
|
||||
// features/auth/presentation/bloc/auth_bloc.dart
|
||||
class AuthBloc extends ChangeNotifier {
|
||||
final Logger _logger = getLogger('features.auth.bloc');
|
||||
|
||||
Future<void> start() async {
|
||||
try {
|
||||
final user = await _repository.recoverSession();
|
||||
if (user == null) {
|
||||
_state = const AuthState(status: AuthStatus.unauthenticated);
|
||||
} else {
|
||||
_state = AuthState(status: AuthStatus.authenticated, user: user);
|
||||
}
|
||||
notifyListeners();
|
||||
} catch (error, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Session recovery failed: ${error.runtimeType}',
|
||||
error: error.runtimeType.toString(),
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
await _repository.clearLocalSession();
|
||||
_state = AuthState(
|
||||
status: AuthStatus.unauthenticated,
|
||||
errorMessage: _toSafeMessage(error),
|
||||
);
|
||||
notifyListeners();
|
||||
}
|
||||
}
|
||||
|
||||
Future<void> logout() async {
|
||||
_logger.info(message: 'User logged out');
|
||||
_state = const AuthState(status: AuthStatus.unauthenticated);
|
||||
notifyListeners();
|
||||
|
||||
unawaited(
|
||||
_repository.logout().catchError((Object error, StackTrace stackTrace) {
|
||||
_logger.error(
|
||||
message: 'User logout failed: ${error.runtimeType}',
|
||||
error: error.runtimeType.toString(),
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
}),
|
||||
);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Network Error Handling
|
||||
|
||||
```dart
|
||||
// core/network/http_client.dart
|
||||
final Logger _logger = getLogger('core.network.http_client');
|
||||
|
||||
Future<Response> get(String path) async {
|
||||
try {
|
||||
final response = await _innerClient.get(path);
|
||||
final problem = ApiProblemMapper.tryParse(response);
|
||||
|
||||
if (problem != null && response.statusCode != 401) {
|
||||
_logger.warning(
|
||||
message: 'HTTP error response',
|
||||
extra: {
|
||||
'status': response.statusCode,
|
||||
'path': path,
|
||||
'code': problem.code,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
return response;
|
||||
} on SocketException catch (e, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Network error',
|
||||
error: e,
|
||||
stackTrace: stackTrace,
|
||||
extra: {'path': path},
|
||||
);
|
||||
throw ApiProblem(
|
||||
status: 0,
|
||||
title: 'Network Error',
|
||||
detail: 'No internet connection',
|
||||
);
|
||||
}
|
||||
}
|
||||
```
|
||||
@@ -0,0 +1,533 @@
|
||||
# Quality Guidelines
|
||||
|
||||
> Code quality standards for Flutter app development.
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This app enforces quality through:
|
||||
|
||||
- **Linting**: Flutter/Dart analysis
|
||||
- **Architecture**: Feature-first with clear boundaries
|
||||
- **Testing**: Widget tests, unit tests, integration tests
|
||||
- **Code review**: Checklist-based reviews
|
||||
|
||||
---
|
||||
|
||||
## Forbidden Patterns
|
||||
|
||||
### ❌ Hardcoded Colors
|
||||
|
||||
```dart
|
||||
// WRONG: Hardcoded hex color
|
||||
Container(
|
||||
color: Color(0xFF2196F3),
|
||||
)
|
||||
|
||||
// WRONG: Using Colors.*
|
||||
Container(
|
||||
color: Colors.blue,
|
||||
)
|
||||
```
|
||||
|
||||
**Right: Use semantic colors:**
|
||||
|
||||
```dart
|
||||
// Semantic colors from theme
|
||||
Container(
|
||||
color: Theme.of(context).colorScheme.primary,
|
||||
)
|
||||
|
||||
// Brand palette colors
|
||||
Container(
|
||||
color: Theme.of(context).extension<AppColorPalette>()!.brandPrimary,
|
||||
)
|
||||
```
|
||||
|
||||
### ❌ Hardcoded Spacing
|
||||
|
||||
```dart
|
||||
// WRONG: Magic numbers
|
||||
Padding(
|
||||
padding: EdgeInsets.all(16.0),
|
||||
)
|
||||
|
||||
// WRONG: Hardcoded values
|
||||
SizedBox(height: 24.0)
|
||||
```
|
||||
|
||||
**Right: Use `AppSpacing` / `AppRadius`:**
|
||||
|
||||
```dart
|
||||
import 'shared/theme/design_tokens.dart';
|
||||
|
||||
Padding(
|
||||
padding: AppSpacing.allMedium,
|
||||
)
|
||||
|
||||
SizedBox(height: AppSpacing.large)
|
||||
```
|
||||
|
||||
### ❌ Feature Data Import from Other Features
|
||||
|
||||
```dart
|
||||
// WRONG: Direct import from another feature
|
||||
import 'package:app/features/auth/data/repositories/auth_repository.dart';
|
||||
```
|
||||
|
||||
**Right: Access via app-level facade or DI:**
|
||||
|
||||
```dart
|
||||
final authRepository = ServiceLocator.authRepository;
|
||||
```
|
||||
|
||||
### ❌ Creating Per-Widget State Instances
|
||||
|
||||
```dart
|
||||
// WRONG: New instance per build
|
||||
class MyWidget extends StatelessWidget {
|
||||
final authBloc = AuthBloc(); // New instance every time
|
||||
}
|
||||
```
|
||||
|
||||
**Right: Use singleton from DI:**
|
||||
|
||||
```dart
|
||||
class MyWidget extends StatelessWidget {
|
||||
@override
|
||||
Widget build(BuildContext context) {
|
||||
final authBloc = ServiceLocator.authBloc; // Singleton
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### ❌ Place Feature Repositories in Shared Data Layer
|
||||
|
||||
```dart
|
||||
// WRONG: Feature repository in shared data/
|
||||
// data/repositories/auth_repository.dart
|
||||
class AuthRepositoryImpl implements AuthRepository {}
|
||||
```
|
||||
|
||||
**Right: Keep in feature's data layer:**
|
||||
|
||||
```dart
|
||||
// features/auth/data/repositories/auth_repository.dart
|
||||
class AuthRepositoryImpl implements AuthRepository {}
|
||||
```
|
||||
|
||||
### ❌ New Second-Level Directories Under `lib/`
|
||||
|
||||
```dart
|
||||
// WRONG: Ad-hoc directories
|
||||
lib/
|
||||
utils/
|
||||
helpers/
|
||||
constants/
|
||||
```
|
||||
|
||||
**Right: Use allowed directories only:**
|
||||
|
||||
```dart
|
||||
lib/
|
||||
app/ # Bootstrap, DI, router
|
||||
core/ # Cross-feature infrastructure
|
||||
data/ # Shared infrastructure
|
||||
features/ # Feature modules
|
||||
shared/ # Reusable widgets
|
||||
l10n/ # Localization
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Required Patterns
|
||||
|
||||
### ✅ Semantic Color System
|
||||
|
||||
```dart
|
||||
import 'package:flutter/material.dart';
|
||||
|
||||
// Use semantic colors
|
||||
ThemeData lightTheme = ThemeData(
|
||||
colorScheme: ColorScheme.light(
|
||||
primary: Color(0xFF2196F3),
|
||||
secondary: Color(0xFF03DAC6),
|
||||
surface: Color(0xFFFFFFFF),
|
||||
error: Color(0xFFB00020),
|
||||
),
|
||||
);
|
||||
|
||||
// Access via theme
|
||||
Container(
|
||||
color: Theme.of(context).colorScheme.primary,
|
||||
)
|
||||
```
|
||||
|
||||
### ✅ Design Tokens for Spacing
|
||||
|
||||
```dart
|
||||
// shared/theme/design_tokens.dart
|
||||
class AppSpacing {
|
||||
static const double xsmall = 4.0;
|
||||
static const double small = 8.0;
|
||||
static const double medium = 16.0;
|
||||
static const double large = 24.0;
|
||||
static const double xlarge = 32.0;
|
||||
|
||||
static EdgeInsets get allMedium => EdgeInsets.all(medium);
|
||||
static EdgeInsets get horizontalMedium => EdgeInsets.symmetric(horizontal: medium);
|
||||
}
|
||||
|
||||
class AppRadius {
|
||||
static const double small = 4.0;
|
||||
static const double medium = 8.0;
|
||||
static const double large = 12.0;
|
||||
}
|
||||
```
|
||||
|
||||
### ✅ Repository Pattern with DI
|
||||
|
||||
```dart
|
||||
// app/di/di.dart
|
||||
class ServiceLocator {
|
||||
static late AuthRepository authRepository;
|
||||
static late AuthBloc authBloc;
|
||||
|
||||
static void setup() {
|
||||
authRepository = AuthRepositoryImpl(
|
||||
api: AuthApiImpl(),
|
||||
sessionStore: SessionStore(),
|
||||
);
|
||||
|
||||
authBloc = AuthBloc(repository: authRepository);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### ✅ Immutable State with copyWith
|
||||
|
||||
```dart
|
||||
class AuthState {
|
||||
const AuthState({
|
||||
required this.status,
|
||||
this.user,
|
||||
this.errorMessage,
|
||||
});
|
||||
|
||||
final AuthStatus status;
|
||||
final AuthUser? user;
|
||||
final String? errorMessage;
|
||||
|
||||
AuthState copyWith({
|
||||
AuthStatus? status,
|
||||
AuthUser? user,
|
||||
String? errorMessage,
|
||||
}) {
|
||||
return AuthState(
|
||||
status: status ?? this.status,
|
||||
user: user ?? this.user,
|
||||
errorMessage: errorMessage ?? this.errorMessage,
|
||||
);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### ✅ Error Logging in Try-Catch
|
||||
|
||||
```dart
|
||||
try {
|
||||
await _repository.operation();
|
||||
} catch (e, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Operation failed',
|
||||
error: e,
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
rethrow;
|
||||
}
|
||||
```
|
||||
|
||||
### ✅ Logger Module Naming
|
||||
|
||||
```dart
|
||||
class AuthBloc extends ChangeNotifier {
|
||||
final Logger _logger = getLogger('features.auth.bloc');
|
||||
}
|
||||
|
||||
class HomeRepository {
|
||||
final Logger _logger = getLogger('features.home.repository');
|
||||
}
|
||||
|
||||
class HttpClient {
|
||||
final Logger _logger = getLogger('core.network.http_client');
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Testing Requirements
|
||||
|
||||
### Test Organization
|
||||
|
||||
```
|
||||
apps/test/
|
||||
├── unit/ # Unit tests
|
||||
├── widgets/ # Widget tests
|
||||
├── integration/ # Integration tests
|
||||
└── test_utils/ # Test utilities
|
||||
```
|
||||
|
||||
### Test Requirements from AGENTS.md
|
||||
|
||||
1. **Prioritize tests for**: model parsing, service logic, high-regression flows
|
||||
2. **Auth/Home/Cache changes**: must include targeted regression tests
|
||||
3. **Simple static UI changes**: may skip tests
|
||||
4. **Test credentials**: use environment config, never hardcode
|
||||
|
||||
### Unit Testing
|
||||
|
||||
```dart
|
||||
test('AuthBloc login success', () async {
|
||||
final mockRepo = MockAuthRepository();
|
||||
final bloc = AuthBloc(repository: mockRepo);
|
||||
|
||||
when(mockRepo.loginWithEmailOtp(
|
||||
email: 'test@example.com',
|
||||
otp: '123456',
|
||||
)).thenAnswer((_) async => AuthUser(id: '123', email: 'test@example.com'));
|
||||
|
||||
await bloc.loginWithOtp(email: 'test@example.com', otp: '123456');
|
||||
|
||||
expect(bloc.state.status, AuthStatus.authenticated);
|
||||
expect(bloc.state.user?.id, '123');
|
||||
});
|
||||
```
|
||||
|
||||
### Widget Testing
|
||||
|
||||
```dart
|
||||
testWidgets('Login screen shows error on failed login', (tester) async {
|
||||
await tester.pumpWidget(MyApp());
|
||||
|
||||
await tester.enterText(find.byKey(Key('email_field')), 'test@example.com');
|
||||
await tester.enterText(find.byKey(Key('otp_field')), 'wrong');
|
||||
await tester.tap(find.byKey(Key('login_button')));
|
||||
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
expect(find.text('Request failed, please try again'), findsOneWidget);
|
||||
});
|
||||
```
|
||||
|
||||
### Integration Testing
|
||||
|
||||
```dart
|
||||
testWidgets('User can login with OTP', (tester) async {
|
||||
await tester.pumpWidget(MyApp());
|
||||
|
||||
// Navigate to login
|
||||
await tester.tap(find.text('Login'));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
// Enter credentials
|
||||
await tester.enterText(find.byKey(Key('email_field')), 'user@example.com');
|
||||
await tester.tap(find.byKey(Key('send_otp_button')));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
// Enter OTP
|
||||
await tester.enterText(find.byKey(Key('otp_field')), '123456');
|
||||
await tester.tap(find.byKey(Key('login_button')));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
// Verify success
|
||||
expect(find.text('Welcome'), findsOneWidget);
|
||||
});
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Code Review Checklist
|
||||
|
||||
### Architecture
|
||||
|
||||
- [ ] Follows `features/<feature>/data/` and `features/<feature>/presentation/` structure
|
||||
- [ ] Shared infrastructure in `data/` (not feature repositories/models)
|
||||
- [ ] Reusable widgets in `shared/widgets/` (not feature-specific)
|
||||
- [ ] Cross-feature code in `core/`
|
||||
- [ ] No new second-level directories under `lib/`
|
||||
|
||||
### State Management
|
||||
|
||||
- [ ] Uses `ChangeNotifier` + immutable state
|
||||
- [ ] State classes have `copyWith` method
|
||||
- [ ] Singleton blocs via DI (not per-widget instances)
|
||||
- [ ] Errors are logged before state changes
|
||||
|
||||
### UI
|
||||
|
||||
- [ ] Uses semantic colors from `Theme.of(context).colorScheme`
|
||||
- [ ] Uses `AppSpacing` / `AppRadius` (no hardcoded values)
|
||||
- [ ] Follows design system tokens
|
||||
- [ ] Toast/Banner for user feedback (no `print()`)
|
||||
|
||||
### Data
|
||||
|
||||
- [ ] Repository pattern for data access
|
||||
- [ ] `ApiProblem` for error handling
|
||||
- [ ] Error codes mapped to l10n keys
|
||||
- [ ] 401 handled via global callback
|
||||
|
||||
### Logging
|
||||
|
||||
- [ ] Logger initialized with module path (`features.<feature>.<component>`)
|
||||
- [ ] All exceptions logged with `error`, `stackTrace`, and `message`
|
||||
- [ ] No PII/tokens/passwords in logs
|
||||
- [ ] Info logs only for milestones (login, logout, run completed)
|
||||
|
||||
### Testing
|
||||
|
||||
- [ ] Unit tests for bloc logic
|
||||
- [ ] Widget tests for UI
|
||||
- [ ] Integration tests for critical flows
|
||||
- [ ] Mocks use `when/thenAnswer/thenReturn`
|
||||
|
||||
---
|
||||
|
||||
## High-Risk Modules Checklist
|
||||
|
||||
From AGENTS.md, these modules require extra attention:
|
||||
|
||||
### Auth
|
||||
|
||||
- [ ] `AuthBloc` is single source of truth
|
||||
- [ ] 401 invalidation goes through global callback
|
||||
- [ ] No feature-level token clearing or direct login navigation
|
||||
|
||||
### Home Message Viewport
|
||||
|
||||
- [ ] Auto-scroll/anchor restore is event-driven
|
||||
- [ ] Viewport preserved during history prepend
|
||||
- [ ] User reading position maintained
|
||||
|
||||
### Cache / Repository
|
||||
|
||||
- [ ] Reads/writes go through repository layer
|
||||
- [ ] Cache keys/invalidation in repository (not UI/Bloc)
|
||||
- [ ] Feature-scoped TTL policy defined in repository
|
||||
- [ ] No per-screen/per-widget cache store instances
|
||||
- [ ] Cross-feature access via app-level facade
|
||||
|
||||
---
|
||||
|
||||
## Common Mistakes
|
||||
|
||||
### ❌ Mutable State
|
||||
|
||||
```dart
|
||||
// WRONG: Direct mutation
|
||||
class AuthBloc extends ChangeNotifier {
|
||||
AuthUser? user; // Mutable
|
||||
|
||||
void login() {
|
||||
user = fetchUser(); // Direct mutation
|
||||
notifyListeners();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Right: Immutable state:**
|
||||
|
||||
```dart
|
||||
class AuthBloc extends ChangeNotifier {
|
||||
AuthState _state = AuthState.initial;
|
||||
|
||||
AuthState get state => _state;
|
||||
|
||||
void login() async {
|
||||
final user = await fetchUser();
|
||||
_state = _state.copyWith(user: user, status: AuthStatus.authenticated);
|
||||
notifyListeners();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### ❌ Missing Error Logging
|
||||
|
||||
```dart
|
||||
// WRONG: No logging
|
||||
try {
|
||||
await operation();
|
||||
} catch (e) {
|
||||
state = ErrorState();
|
||||
}
|
||||
```
|
||||
|
||||
**Right: Log before state change:**
|
||||
|
||||
```dart
|
||||
try {
|
||||
await operation();
|
||||
} catch (e, stackTrace) {
|
||||
_logger.error(message: 'Operation failed', error: e, stackTrace: stackTrace);
|
||||
state = ErrorState();
|
||||
}
|
||||
```
|
||||
|
||||
### ❌ Hardcoded Strings
|
||||
|
||||
```dart
|
||||
// WRONG: User-facing string
|
||||
Text('Login Failed')
|
||||
```
|
||||
|
||||
**Right: Use l10n:**
|
||||
|
||||
```dart
|
||||
Text(AppLocalizations.of(context)!.loginFailed)
|
||||
```
|
||||
|
||||
### ❌ Bypassing Design System
|
||||
|
||||
```dart
|
||||
// WRONG: Magic numbers
|
||||
Padding(
|
||||
padding: EdgeInsets.fromLTRB(16, 8, 16, 8),
|
||||
)
|
||||
```
|
||||
|
||||
**Right: Use tokens:**
|
||||
|
||||
```dart
|
||||
Padding(
|
||||
padding: EdgeInsets.symmetric(
|
||||
horizontal: AppSpacing.medium,
|
||||
vertical: AppSpacing.small,
|
||||
),
|
||||
)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Flutter-Specific Quality
|
||||
|
||||
### Widget Best Practices
|
||||
|
||||
- Use `const` constructors for immutable widgets
|
||||
- Prefer `StatelessWidget` over `StatefulWidget` when possible
|
||||
- Use `const` when creating widgets in build methods
|
||||
- Avoid `print()` - use Logger instead
|
||||
|
||||
### Performance
|
||||
|
||||
- Use `ListView.builder` for long lists
|
||||
- Avoid rebuilding expensive widgets unnecessarily
|
||||
- Use `const` widgets to prevent rebuilds
|
||||
- Use `RepaintBoundary` for complex animations
|
||||
|
||||
### Code Style
|
||||
|
||||
- Follow Dart style guide
|
||||
- Use `flutter analyze` to catch issues
|
||||
- Run `dart format .` before commits
|
||||
- Follow naming conventions: `lowerCamelCase` for variables, `UpperCamelCase` for types
|
||||
@@ -0,0 +1,398 @@
|
||||
# State Management
|
||||
|
||||
> State management patterns in Flutter app.
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This app uses **ChangeNotifier + Provider** for state management:
|
||||
|
||||
- **AuthBloc** uses `ChangeNotifier` for auth state
|
||||
- **State classes** are immutable value objects
|
||||
- **Repository pattern** separates data access from business logic
|
||||
- **DI** via provider/factory pattern
|
||||
|
||||
---
|
||||
|
||||
## State Management Pattern
|
||||
|
||||
### Bloc/Cubit Pattern
|
||||
|
||||
**Use `ChangeNotifier` for complex state:**
|
||||
|
||||
```dart
|
||||
// features/auth/presentation/bloc/auth_bloc.dart
|
||||
class AuthBloc extends ChangeNotifier {
|
||||
AuthBloc({required AuthRepository repository}) : _repository = repository;
|
||||
|
||||
final AuthRepository _repository;
|
||||
final Logger _logger = getLogger('features.auth.bloc');
|
||||
AuthState _state = AuthState.initial;
|
||||
|
||||
AuthState get state => _state;
|
||||
|
||||
Future<void> loginWithOtp({
|
||||
required String email,
|
||||
required String otp,
|
||||
}) async {
|
||||
final user = await _repository.loginWithEmailOtp(email: email, otp: otp);
|
||||
_logger.info(message: 'User logged in', extra: {'user_id': user.id});
|
||||
_state = AuthState(status: AuthStatus.authenticated, user: user);
|
||||
notifyListeners();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Immutable State
|
||||
|
||||
**State classes should be immutable:**
|
||||
|
||||
```dart
|
||||
// features/auth/presentation/bloc/auth_state.dart
|
||||
class AuthState {
|
||||
const AuthState({
|
||||
required this.status,
|
||||
this.user,
|
||||
this.errorMessage,
|
||||
});
|
||||
|
||||
final AuthStatus status;
|
||||
final AuthUser? user;
|
||||
final String? errorMessage;
|
||||
|
||||
factory AuthState.initial() => const AuthState(status: AuthStatus.initial);
|
||||
|
||||
AuthState copyWith({
|
||||
AuthStatus? status,
|
||||
AuthUser? user,
|
||||
String? errorMessage,
|
||||
}) {
|
||||
return AuthState(
|
||||
status: status ?? this.status,
|
||||
user: user ?? this.user,
|
||||
errorMessage: errorMessage ?? this.errorMessage,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
enum AuthStatus {
|
||||
initial,
|
||||
loading,
|
||||
authenticated,
|
||||
unauthenticated,
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Repository Pattern
|
||||
|
||||
### Repository Interface
|
||||
|
||||
**Define interface in presentation layer:**
|
||||
|
||||
```dart
|
||||
// features/auth/data/repositories/auth_repository.dart
|
||||
abstract class AuthRepository {
|
||||
Future<AuthUser?> recoverSession();
|
||||
Future<void> sendOtp(String email);
|
||||
Future<AuthUser> loginWithEmailOtp({
|
||||
required String email,
|
||||
required String otp,
|
||||
});
|
||||
Future<void> logout();
|
||||
Future<void> clearLocalSession();
|
||||
}
|
||||
```
|
||||
|
||||
### Repository Implementation
|
||||
|
||||
**Implement with data sources:**
|
||||
|
||||
```dart
|
||||
class AuthRepositoryImpl implements AuthRepository {
|
||||
AuthRepositoryImpl({
|
||||
required AuthApi api,
|
||||
required SessionStore sessionStore,
|
||||
}) : _api = api,
|
||||
_sessionStore = sessionStore;
|
||||
|
||||
final AuthApi _api;
|
||||
final SessionStore _sessionStore;
|
||||
|
||||
@override
|
||||
Future<AuthUser?> recoverSession() async {
|
||||
final session = await _sessionStore.load();
|
||||
if (session == null) return null;
|
||||
|
||||
try {
|
||||
return await _api.getCurrentUser(session.accessToken);
|
||||
} catch (e) {
|
||||
await clearLocalSession();
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Error Handling in State
|
||||
|
||||
### Try-Catch with Logging
|
||||
|
||||
**Every async operation should handle errors:**
|
||||
|
||||
```dart
|
||||
Future<void> start() async {
|
||||
_state = _state.copyWith(status: AuthStatus.loading);
|
||||
notifyListeners();
|
||||
|
||||
try {
|
||||
final user = await _repository.recoverSession();
|
||||
if (user == null) {
|
||||
_state = const AuthState(status: AuthStatus.unauthenticated);
|
||||
} else {
|
||||
_state = AuthState(status: AuthStatus.authenticated, user: user);
|
||||
}
|
||||
notifyListeners();
|
||||
} catch (error, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Session recovery failed: ${error.runtimeType}',
|
||||
error: error,
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
await _repository.clearLocalSession();
|
||||
_state = AuthState(
|
||||
status: AuthStatus.unauthenticated,
|
||||
errorMessage: _toSafeMessage(error),
|
||||
);
|
||||
notifyListeners();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Global Error Handling
|
||||
|
||||
**401 Session Invalidation:**
|
||||
|
||||
```dart
|
||||
// Global callback for 401 errors
|
||||
Future<void> handleUnauthorized401() async {
|
||||
if (_handlingUnauthorized) return;
|
||||
_handlingUnauthorized = true;
|
||||
|
||||
try {
|
||||
await _repository.clearLocalSession();
|
||||
_logger.warning(message: 'Session invalidated by 401 callback');
|
||||
_state = const AuthState(status: AuthStatus.unauthenticated);
|
||||
notifyListeners();
|
||||
} finally {
|
||||
_handlingUnauthorized = false;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## DI Pattern
|
||||
|
||||
### Factory Registration
|
||||
|
||||
**Register repositories and blocs:**
|
||||
|
||||
```dart
|
||||
// app/di/di.dart
|
||||
class ServiceLocator {
|
||||
static late AuthRepository authRepository;
|
||||
static late AuthBloc authBloc;
|
||||
|
||||
static void setup() {
|
||||
authRepository = AuthRepositoryImpl(
|
||||
api: AuthApiImpl(),
|
||||
sessionStore: SessionStore(),
|
||||
);
|
||||
|
||||
authBloc = AuthBloc(repository: authRepository);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Widget Access
|
||||
|
||||
**Access via Provider or global:**
|
||||
|
||||
```dart
|
||||
// Using global reference
|
||||
final authBloc = ServiceLocator.authBloc;
|
||||
|
||||
// In widget
|
||||
class LoginScreen extends StatelessWidget {
|
||||
@override
|
||||
Widget build(BuildContext context) {
|
||||
return ListenableBuilder(
|
||||
listenable: authBloc,
|
||||
builder: (context, child) {
|
||||
if (authBloc.state.status == AuthStatus.loading) {
|
||||
return AppLoadingIndicator();
|
||||
}
|
||||
return LoginForm();
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Common Mistakes
|
||||
|
||||
### ❌ Mutable State
|
||||
|
||||
```dart
|
||||
// WRONG: Mutating state directly
|
||||
class AuthBloc extends ChangeNotifier {
|
||||
AuthUser? user;
|
||||
|
||||
void login() {
|
||||
user = fetchUser(); // Direct mutation
|
||||
notifyListeners();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Right: Immutable state with copyWith:**
|
||||
|
||||
```dart
|
||||
class AuthBloc extends ChangeNotifier {
|
||||
AuthState _state = AuthState.initial;
|
||||
|
||||
AuthState get state => _state;
|
||||
|
||||
void login() async {
|
||||
final user = await fetchUser();
|
||||
_state = _state.copyWith(user: user, status: AuthStatus.authenticated);
|
||||
notifyListeners();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### ❌ Importing Feature Data Layer from Other Features
|
||||
|
||||
```dart
|
||||
// WRONG: Direct import from another feature
|
||||
import 'package:app/features/auth/data/repositories/auth_repository.dart';
|
||||
```
|
||||
|
||||
**Right: Access via app-level facade or DI:**
|
||||
|
||||
```dart
|
||||
final authRepository = ServiceLocator.authRepository;
|
||||
```
|
||||
|
||||
### ❌ Skipping Error Logging
|
||||
|
||||
```dart
|
||||
// WRONG: No logging
|
||||
try {
|
||||
await repository.operation();
|
||||
} catch (e) {
|
||||
state = AuthState(status: AuthStatus.error);
|
||||
}
|
||||
```
|
||||
|
||||
**Right: Log before state change:**
|
||||
|
||||
```dart
|
||||
try {
|
||||
await repository.operation();
|
||||
} catch (e, stackTrace) {
|
||||
_logger.error(
|
||||
message: 'Operation failed',
|
||||
error: e,
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
state = AuthState(status: AuthStatus.error);
|
||||
}
|
||||
```
|
||||
|
||||
### ❌ Creating Per-Widget State Instances
|
||||
|
||||
```dart
|
||||
// WRONG: New instance per widget
|
||||
class MyWidget extends StatelessWidget {
|
||||
final authBloc = AuthBloc(); // New instance every build
|
||||
}
|
||||
```
|
||||
|
||||
**Right: Use DI singleton:**
|
||||
|
||||
```dart
|
||||
class MyWidget extends StatelessWidget {
|
||||
@override
|
||||
Widget build(BuildContext context) {
|
||||
final authBloc = ServiceLocator.authBloc; // Singleton
|
||||
// ...
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Module Naming Convention
|
||||
|
||||
**Logger module path:**
|
||||
|
||||
| Feature | Module Path |
|
||||
|---------|------------|
|
||||
| auth | `features.auth` |
|
||||
| home | `features.home` |
|
||||
| divination | `features.divination` |
|
||||
| settings | `features.settings` |
|
||||
|
||||
```dart
|
||||
class AuthBloc extends ChangeNotifier {
|
||||
final Logger _logger = getLogger('features.auth.bloc');
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Testing State
|
||||
|
||||
### Unit Testing Bloc
|
||||
|
||||
```dart
|
||||
test('AuthBloc login success', () async {
|
||||
final mockRepo = MockAuthRepository();
|
||||
final bloc = AuthBloc(repository: mockRepo);
|
||||
|
||||
when(mockRepo.loginWithEmailOtp(
|
||||
email: 'test@example.com',
|
||||
otp: '123456',
|
||||
)).thenAnswer((_) async => AuthUser(id: '123', email: 'test@example.com'));
|
||||
|
||||
await bloc.loginWithOtp(email: 'test@example.com', otp: '123456');
|
||||
|
||||
expect(bloc.state.status, AuthStatus.authenticated);
|
||||
expect(bloc.state.user?.id, '123');
|
||||
});
|
||||
```
|
||||
|
||||
### Integration Testing
|
||||
|
||||
```dart
|
||||
testWidgets('Login screen shows error on failed login', (tester) async {
|
||||
await tester.pumpWidget(MyApp());
|
||||
|
||||
await tester.enterText(find.byKey(Key('email_field')), 'test@example.com');
|
||||
await tester.enterText(find.byKey(Key('otp_field')), 'wrong');
|
||||
await tester.tap(find.byKey(Key('login_button')));
|
||||
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
expect(find.text('Request failed, please try again'), findsOneWidget);
|
||||
});
|
||||
```
|
||||
@@ -0,0 +1,105 @@
|
||||
# Code Reuse Thinking Guide
|
||||
|
||||
> **Purpose**: Stop and think before creating new code - does it already exist?
|
||||
|
||||
---
|
||||
|
||||
## The Problem
|
||||
|
||||
**Duplicated code is the #1 source of inconsistency bugs.**
|
||||
|
||||
When you copy-paste or rewrite existing logic:
|
||||
- Bug fixes don't propagate
|
||||
- Behavior diverges over time
|
||||
- Codebase becomes harder to understand
|
||||
|
||||
---
|
||||
|
||||
## Before Writing New Code
|
||||
|
||||
### Step 1: Search First
|
||||
|
||||
```bash
|
||||
# Search for similar function names
|
||||
grep -r "functionName" .
|
||||
|
||||
# Search for similar logic
|
||||
grep -r "keyword" .
|
||||
```
|
||||
|
||||
### Step 2: Ask These Questions
|
||||
|
||||
| Question | If Yes... |
|
||||
|----------|-----------|
|
||||
| Does a similar function exist? | Use or extend it |
|
||||
| Is this pattern used elsewhere? | Follow the existing pattern |
|
||||
| Could this be a shared utility? | Create it in the right place |
|
||||
| Am I copying code from another file? | **STOP** - extract to shared |
|
||||
|
||||
---
|
||||
|
||||
## Common Duplication Patterns
|
||||
|
||||
### Pattern 1: Copy-Paste Functions
|
||||
|
||||
**Bad**: Copying a validation function to another file
|
||||
|
||||
**Good**: Extract to shared utilities, import where needed
|
||||
|
||||
### Pattern 2: Similar Components
|
||||
|
||||
**Bad**: Creating a new component that's 80% similar to existing
|
||||
|
||||
**Good**: Extend existing component with props/variants
|
||||
|
||||
### Pattern 3: Repeated Constants
|
||||
|
||||
**Bad**: Defining the same constant in multiple files
|
||||
|
||||
**Good**: Single source of truth, import everywhere
|
||||
|
||||
---
|
||||
|
||||
## When to Abstract
|
||||
|
||||
**Abstract when**:
|
||||
- Same code appears 3+ times
|
||||
- Logic is complex enough to have bugs
|
||||
- Multiple people might need this
|
||||
|
||||
**Don't abstract when**:
|
||||
- Only used once
|
||||
- Trivial one-liner
|
||||
- Abstraction would be more complex than duplication
|
||||
|
||||
---
|
||||
|
||||
## After Batch Modifications
|
||||
|
||||
When you've made similar changes to multiple files:
|
||||
|
||||
1. **Review**: Did you catch all instances?
|
||||
2. **Search**: Run grep to find any missed
|
||||
3. **Consider**: Should this be abstracted?
|
||||
|
||||
---
|
||||
|
||||
## Gotcha: Asymmetric Mechanisms Producing Same Output
|
||||
|
||||
**Problem**: When two different mechanisms must produce the same file set (e.g., recursive directory copy for init vs. manual `files.set()` for update), structural changes (renaming, moving, adding subdirectories) only propagate through the automatic mechanism. The manual one silently drifts.
|
||||
|
||||
**Symptom**: Init works perfectly, but update creates files at wrong paths or misses files entirely.
|
||||
|
||||
**Prevention checklist**:
|
||||
- [ ] When migrating directory structures, search for ALL code paths that reference the old structure
|
||||
- [ ] If one path is auto-derived (glob/copy) and another is manually listed, the manual one needs updating
|
||||
- [ ] Add a regression test that compares outputs from both mechanisms
|
||||
|
||||
---
|
||||
|
||||
## Checklist Before Commit
|
||||
|
||||
- [ ] Searched for existing similar code
|
||||
- [ ] No copy-pasted logic that should be shared
|
||||
- [ ] Constants defined in one place
|
||||
- [ ] Similar patterns follow same structure
|
||||
@@ -0,0 +1,94 @@
|
||||
# Cross-Layer Thinking Guide
|
||||
|
||||
> **Purpose**: Think through data flow across layers before implementing.
|
||||
|
||||
---
|
||||
|
||||
## The Problem
|
||||
|
||||
**Most bugs happen at layer boundaries**, not within layers.
|
||||
|
||||
Common cross-layer bugs:
|
||||
- API returns format A, frontend expects format B
|
||||
- Database stores X, service transforms to Y, but loses data
|
||||
- Multiple layers implement the same logic differently
|
||||
|
||||
---
|
||||
|
||||
## Before Implementing Cross-Layer Features
|
||||
|
||||
### Step 1: Map the Data Flow
|
||||
|
||||
Draw out how data moves:
|
||||
|
||||
```
|
||||
Source → Transform → Store → Retrieve → Transform → Display
|
||||
```
|
||||
|
||||
For each arrow, ask:
|
||||
- What format is the data in?
|
||||
- What could go wrong?
|
||||
- Who is responsible for validation?
|
||||
|
||||
### Step 2: Identify Boundaries
|
||||
|
||||
| Boundary | Common Issues |
|
||||
|----------|---------------|
|
||||
| API ↔ Service | Type mismatches, missing fields |
|
||||
| Service ↔ Database | Format conversions, null handling |
|
||||
| Backend ↔ Frontend | Serialization, date formats |
|
||||
| Component ↔ Component | Props shape changes |
|
||||
|
||||
### Step 3: Define Contracts
|
||||
|
||||
For each boundary:
|
||||
- What is the exact input format?
|
||||
- What is the exact output format?
|
||||
- What errors can occur?
|
||||
|
||||
---
|
||||
|
||||
## Common Cross-Layer Mistakes
|
||||
|
||||
### Mistake 1: Implicit Format Assumptions
|
||||
|
||||
**Bad**: Assuming date format without checking
|
||||
|
||||
**Good**: Explicit format conversion at boundaries
|
||||
|
||||
### Mistake 2: Scattered Validation
|
||||
|
||||
**Bad**: Validating the same thing in multiple layers
|
||||
|
||||
**Good**: Validate once at the entry point
|
||||
|
||||
### Mistake 3: Leaky Abstractions
|
||||
|
||||
**Bad**: Component knows about database schema
|
||||
|
||||
**Good**: Each layer only knows its neighbors
|
||||
|
||||
---
|
||||
|
||||
## Checklist for Cross-Layer Features
|
||||
|
||||
Before implementation:
|
||||
- [ ] Mapped the complete data flow
|
||||
- [ ] Identified all layer boundaries
|
||||
- [ ] Defined format at each boundary
|
||||
- [ ] Decided where validation happens
|
||||
|
||||
After implementation:
|
||||
- [ ] Tested with edge cases (null, empty, invalid)
|
||||
- [ ] Verified error handling at each boundary
|
||||
- [ ] Checked data survives round-trip
|
||||
|
||||
---
|
||||
|
||||
## When to Create Flow Documentation
|
||||
|
||||
Create detailed flow docs when:
|
||||
- Feature spans 3+ layers
|
||||
- Multiple teams are involved
|
||||
- Data format is complex
|
||||
- Feature has caused bugs before
|
||||
@@ -0,0 +1,79 @@
|
||||
# Thinking Guides
|
||||
|
||||
> **Purpose**: Expand your thinking to catch things you might not have considered.
|
||||
|
||||
---
|
||||
|
||||
## Why Thinking Guides?
|
||||
|
||||
**Most bugs and tech debt come from "didn't think of that"**, not from lack of skill:
|
||||
|
||||
- Didn't think about what happens at layer boundaries → cross-layer bugs
|
||||
- Didn't think about code patterns repeating → duplicated code everywhere
|
||||
- Didn't think about edge cases → runtime errors
|
||||
- Didn't think about future maintainers → unreadable code
|
||||
|
||||
These guides help you **ask the right questions before coding**.
|
||||
|
||||
---
|
||||
|
||||
## Available Guides
|
||||
|
||||
| Guide | Purpose | When to Use |
|
||||
|-------|---------|-------------|
|
||||
| [Code Reuse Thinking Guide](./code-reuse-thinking-guide.md) | Identify patterns and reduce duplication | When you notice repeated patterns |
|
||||
| [Cross-Layer Thinking Guide](./cross-layer-thinking-guide.md) | Think through data flow across layers | Features spanning multiple layers |
|
||||
|
||||
---
|
||||
|
||||
## Quick Reference: Thinking Triggers
|
||||
|
||||
### When to Think About Cross-Layer Issues
|
||||
|
||||
- [ ] Feature touches 3+ layers (API, Service, Component, Database)
|
||||
- [ ] Data format changes between layers
|
||||
- [ ] Multiple consumers need the same data
|
||||
- [ ] You're not sure where to put some logic
|
||||
|
||||
→ Read [Cross-Layer Thinking Guide](./cross-layer-thinking-guide.md)
|
||||
|
||||
### When to Think About Code Reuse
|
||||
|
||||
- [ ] You're writing similar code to something that exists
|
||||
- [ ] You see the same pattern repeated 3+ times
|
||||
- [ ] You're adding a new field to multiple places
|
||||
- [ ] **You're modifying any constant or config**
|
||||
- [ ] **You're creating a new utility/helper function** ← Search first!
|
||||
|
||||
→ Read [Code Reuse Thinking Guide](./code-reuse-thinking-guide.md)
|
||||
|
||||
---
|
||||
|
||||
## Pre-Modification Rule (CRITICAL)
|
||||
|
||||
> **Before changing ANY value, ALWAYS search first!**
|
||||
|
||||
```bash
|
||||
# Search for the value you're about to change
|
||||
grep -r "value_to_change" .
|
||||
```
|
||||
|
||||
This single habit prevents most "forgot to update X" bugs.
|
||||
|
||||
---
|
||||
|
||||
## How to Use This Directory
|
||||
|
||||
1. **Before coding**: Skim the relevant thinking guide
|
||||
2. **During coding**: If something feels repetitive or complex, check the guides
|
||||
3. **After bugs**: Add new insights to the relevant guide (learn from mistakes)
|
||||
|
||||
---
|
||||
|
||||
## Contributing
|
||||
|
||||
Found a new "didn't think of that" moment? Add it to the relevant guide.
|
||||
|
||||
---
|
||||
|
||||
**Core Principle**: 30 minutes of thinking saves 3 hours of debugging.
|
||||
Reference in New Issue
Block a user