Skip to content

Commit f861fce

Browse files
authored
Fix documentation inaccuracies, add unit tests, and clean up docs (#580)
* Fix documentation inaccuracies and clean up dev dependencies - chatbot-system.md: correct state namespaces (system/user/context/temp), replace JSON Logic with CEL syntax, fix sync execution model description, fix processor hierarchy, add /v1 prefix to endpoints, update schema - cms.md: remove fictional webhook registration endpoints, add missing schema fields (school_id, visibility, search_document), condense stale Landbot migration references - architecture-service-layer.md: refactor from 2294 to 287 lines, separate current implementation from aspirational design, verify all service and repository inventories against code - pyproject.toml: add psutil to dev deps, remove unused playwright/ pytest-playwright * Deduplicate docs, add unit tests, and create design notes Documentation: - CLAUDE.md: deduplicate with README (216 to 108 lines), keep AI-specific instructions, reference README for shared content - README.md: absorb unique content from CLAUDE.md, add LOCAL_BUILD_ONLY flag, document @pytest.mark.isolated, reference setup-test-env.sh - docs/cms.md: annotate analytics endpoints as real SQL vs placeholder, trim verbose response examples (-313 lines) - docs/design-session-replay.md: refactor from 886 to 104 lines, verify all components against code, remove pseudo-code for implemented features - docs/testing-credentials.md: consolidate on seed script as primary approach, fix OpenAPI URLs (/docs to /v1/docs), fix absolute paths - docs/architecture-roadmap.md: new file preserving design thinking cut from architecture refactor (UoW, CQRS, migration strategy, events) - docs/analytics-design-note.md: new design note categorizing 21 analytics endpoints (8 real SQL, 5 hardcoded fake, 4 deferred, 5 no backend) Tests: - app/tests/unit/test_cms_workflow.py: 66 new unit tests covering publish, bulk ops, validation, variants, content lifecycle - app/tests/unit/test_chatbot_integrations.py: 49 new unit tests covering helper functions and Pydantic models * Remove legacy setup_test_environment.py and its documentation The seed script (scripts/seed_admin_ui_data.py) is the canonical test data setup. The legacy Playwright-focused script created a different school with fewer roles and is no longer needed.
1 parent 2316ff8 commit f861fce

13 files changed

+2382
-1633
lines changed

CLAUDE.md

Lines changed: 57 additions & 164 deletions
Original file line numberDiff line numberDiff line change
@@ -6,210 +6,103 @@
66
- **REST API Consistency**: DELETE operations should return appropriate HTTP status codes per REST conventions (typically 204 No Content for successful deletions with no response body, or 200 OK if returning meaningful response data).
77
- **Declarative Database Infrastructure**: Use `alembic_utils` with Python-defined functions and triggers in `app/db/functions.py` and `app/db/triggers.py` for version-controlled, type-safe database logic.
88

9-
Only Professional Comments Should Be used.
9+
## Code Style
10+
11+
Only professional comments should be used:
1012
- Remove task-focused comments like "OLD import removed"
1113
- Remove "NEW:" markers etc.
1214
- Focus on why not what
1315
- Remove comments that just restate the code
1416

17+
Ruff for linting (configured in `pyproject.toml`).
18+
1519
## Development Commands
1620

17-
### Dependencies
18-
- **Install dependencies**: `poetry install`
19-
- **Add new dependency**: `poetry add <package_name>`
20-
- **Update dependencies**: `poetry update`
21+
See [README.md](README.md) for full setup, testing, migration, and deployment instructions.
2122

22-
### Database Operations
23-
- **Apply migrations**: `poetry run alembic upgrade head`
24-
- **Create new migration**: `poetry run alembic revision --autogenerate -m "Description"`
25-
- **Set database connection**: `export SQLALCHEMY_DATABASE_URI=postgresql://postgres:password@localhost/postgres`
23+
Quick reference:
2624

27-
### Testing
28-
- **Run all tests**: `bash scripts/start-tests.sh` or `poetry run pytest -v app/tests`
29-
- **Run integration tests in Docker**: `bash scripts/integration-tests.sh` (recommended - provides proper environment)
30-
- **Run integration tests locally**: Direct pytest may have async fixture issues, use Docker instead
31-
- **Single test**: `poetry run pytest -v app/tests/integration/test_specific.py::test_function`
32-
33-
**Important Note**: Integration tests should be run using `bash scripts/integration-tests.sh` which provides the proper Docker environment with database migrations and all dependencies. Running integration tests directly with pytest may encounter async fixture configuration issues. Ensure no conflicting postgres containers are running on port 5432.
34-
35-
### Code Quality
36-
- **Lint code**: `poetry run ruff check`
37-
- **Fix linting issues**: `poetry run ruff check --fix`
38-
- **Pre-commit hooks**: `poetry run pre-commit run --all-files`
39-
40-
### Local Development
41-
- **Start with Docker Compose**: `docker compose up -d`
42-
- **Run API directly**: `uvicorn app.main:app --reload`
43-
- **Run internal API**: `gunicorn --workers=1 --worker-class=uvicorn.workers.UvicornWorker app.internal_api:internal_app`
44-
45-
### Seed Test Data (Admin UI / Chatflows)
46-
Use the declarative fixture + seeder to create a consistent school, users, books, CMS content, and flows.
47-
48-
**Config**: `scripts/fixtures/admin-ui-seed.json`
49-
**Seeder**: `scripts/seed_admin_ui_data.py`
50-
51-
```bash
52-
# Seed data and print JWTs for each user role
53-
docker compose run --rm --entrypoint python \
54-
-v "$PWD/scripts:/app/scripts" \
55-
api /app/scripts/seed_admin_ui_data.py --emit-tokens --tokens-format json
56-
```
25+
| Task | Command |
26+
|------|---------|
27+
| Install dependencies | `poetry install` |
28+
| Start stack | `docker compose up -d --build` |
29+
| Apply migrations | `docker compose run --rm migration` |
30+
| Seed data | `docker compose run --rm --entrypoint python -v "$PWD/scripts:/app/scripts" api /app/scripts/seed_admin_ui_data.py --emit-tokens --tokens-format json` |
31+
| Unit tests | `poetry run pytest app/tests/unit/ -v` |
32+
| Integration tests | `bash scripts/integration-tests.sh` |
33+
| Single test | `poetry run pytest -v app/tests/integration/test_specific.py::test_function` |
34+
| Lint | `poetry run ruff check` |
35+
| Lint fix | `poetry run ruff check --fix` |
36+
| Run API directly | `uvicorn app.main:app --reload` |
37+
| Run internal API | `gunicorn --workers=1 --worker-class=uvicorn.workers.UvicornWorker app.internal_api:internal_app` |
38+
39+
**Important**: Integration tests should be run using `bash scripts/integration-tests.sh` which provides the proper Docker environment. Running integration tests directly with pytest may encounter async fixture issues. Ensure no conflicting postgres containers are running on port 5432.
5740

5841
### Configuring Local User Permissions
59-
To grant admin access for testing features like the CMS/chatflow builder in the admin UI:
42+
43+
To grant admin access for testing the CMS/chatflow builder in the admin UI:
6044

6145
```sql
62-
-- Update user type to WRIVETED admin
6346
UPDATE users SET type = 'WRIVETED' WHERE email = 'your-email@example.com';
6447

65-
-- Create corresponding wriveted_admins record (required for joined-table inheritance)
6648
INSERT INTO wriveted_admins (id)
6749
SELECT id FROM users WHERE email = 'your-email@example.com';
6850
```
6951

70-
Or via Python:
71-
```python
72-
export SQLALCHEMY_DATABASE_URI=postgresql://postgres:password@localhost/postgres
73-
poetry run python -c "
74-
from sqlalchemy import create_engine, text
75-
engine = create_engine('postgresql://postgres:password@localhost/postgres')
76-
with engine.begin() as conn:
77-
result = conn.execute(text(\"SELECT id FROM users WHERE email = 'your-email@example.com'\"))
78-
user_id = result.fetchone()[0]
79-
conn.execute(text(f\"UPDATE users SET type = 'WRIVETED' WHERE id = '{user_id}'\"))
80-
conn.execute(text(f\"INSERT INTO wriveted_admins (id) VALUES ('{user_id}')\"))
81-
"
82-
```
83-
84-
After updating, log out and back in to the admin UI to get a new JWT with updated permissions.
85-
86-
## Declarative Database Pattern
87-
88-
### Overview
89-
All PostgreSQL functions, triggers, and complex database objects are defined declaratively in Python using `alembic_utils`. This ensures version control, type safety, and maintainability.
90-
91-
### Key Files
92-
- **`app/db/functions.py`**: PostgreSQL function definitions
93-
- **`app/db/triggers.py`**: Trigger definitions that reference functions
94-
- **Migrations**: Use `op.create_entity()` and `op.drop_entity()` for declarative objects
95-
96-
### Example Pattern
97-
```python
98-
# In app/db/functions.py
99-
from alembic_utils.pg_function import PGFunction
100-
101-
my_function = PGFunction(
102-
schema="public",
103-
signature="my_function_name()",
104-
definition="returns trigger LANGUAGE plpgsql AS $$ ... $$"
105-
)
106-
107-
# In app/db/triggers.py
108-
from alembic_utils.pg_trigger import PGTrigger
109-
from app.db.functions import my_function
110-
111-
my_trigger = PGTrigger(
112-
schema="public",
113-
signature="trg_my_trigger",
114-
on_entity="public.my_table",
115-
definition=f"... EXECUTE FUNCTION {my_function.signature}"
116-
)
117-
118-
# In migration
119-
def upgrade():
120-
op.create_entity(my_function)
121-
op.create_entity(my_trigger)
122-
```
123-
124-
### Benefits
125-
- **Single Source of Truth**: Database logic defined once in Python
126-
- **Version Control**: All changes tracked in git
127-
- **Type Safety**: Python validates syntax before deployment
128-
- **Migration Safety**: Automatic up/down migration generation
129-
- **IDE Support**: Full Python tooling available
52+
After updating, log out and back in to get a new JWT with updated permissions.
13053

13154
## Architecture Overview
13255

13356
### Dual API Structure
134-
The application consists of two separate FastAPI applications:
13557
- **Public API** (`app.main:app`): External-facing REST API with authentication/authorization
13658
- **Internal API** (`app.internal_api:internal_app`): Background task processing, webhook handling
13759

138-
### Database Architecture
60+
### Database
13961
- **ORM**: SQLAlchemy 2.0 with async support (asyncpg driver)
14062
- **Migrations**: Alembic for schema management
14163
- **Base Class**: Custom `Base` class with auto-generated table names
14264
- **User Model**: Uses joined-table inheritance for different user types (Student, Educator, Parent, etc.)
65+
- **Connection**: Always use `SQLALCHEMY_DATABASE_URI` environment variable
14366

144-
### Key Domain Models
145-
- **Users**: Hierarchical user system (User → Student/Educator/Parent/etc.)
146-
- **Books**: Work → Edition → CollectionItem relationship
147-
- **Schools**: Schools contain ClassGroups and Users
148-
- **Collections**: Library collections with items and activity tracking
149-
- **Labels**: AI-powered book categorization system via LabelSets
150-
151-
### Authentication & Authorization
152-
- **Firebase Authentication**: Users authenticate via Firebase, exchange token for Wriveted JWT
153-
- **RBAC**: Role-based access control with principals (user-xyz, school-1)
154-
- **Service Accounts**: Long-lived tokens for LMS integrations
155-
156-
### Configuration
157-
- **Settings**: Pydantic-based configuration in `app.config.py`
158-
- **Environment Variables**: Database connection, API keys, feature flags
159-
- **GCP Integration**: Cloud SQL, Cloud Storage, Cloud Tasks
160-
161-
### API Structure
162-
- **External API**: Routes in `app/api/` with dependencies in `app/api/dependencies/`
163-
- **Schemas**: Pydantic models for request/response in `app/schemas/`
164-
- **CRUD Operations**: Database operations in `app/crud/` (legacy pattern)
165-
- **Repositories**: Domain-focused data access interfaces in `app/repositories/`
67+
### Code Organization
68+
- **Routes**: `app/api/` with dependencies in `app/api/dependencies/`
69+
- **Schemas**: Pydantic request/response models in `app/schemas/`
70+
- **Repositories**: Domain-focused data access in `app/repositories/` (modern pattern)
71+
- **CRUD**: Legacy data access in `app/crud/` (being phased out)
16672
- **Services**: Business logic in `app/services/`
73+
- **Configuration**: Pydantic-based settings in `app/config.py`
16774

168-
169-
## Development Notes
170-
171-
### Database Connection Patterns
172-
- Always use environment variable `SQLALCHEMY_DATABASE_URI` for connections
173-
- Local development: `postgresql://postgres:password@localhost/postgres`
174-
175-
### Testing Environment
176-
- Integration tests use Docker Compose with real PostgreSQL
177-
- Tests are in `app/tests/integration/` and `app/tests/unit/`
178-
- Test configuration in `conftest.py` files
179-
180-
### Code Style
181-
- Ruff for linting (configured in `pyproject.toml`)
75+
See [docs/architecture-service-layer.md](docs/architecture-service-layer.md) for the full service layer architecture.
18276

18377
### Migration Workflow
18478
1. Modify SQLAlchemy models in `app/models/`
18579
2. Add imports to `app/models/__init__.py`
18680
3. Generate migration: `poetry run alembic revision --autogenerate -m "Description"`
187-
4. Review generated migration file manually. Try ensure models are source of truth.
81+
4. Review generated migration file manually. Models are source of truth.
18882
5. Apply: `poetry run alembic upgrade head`
18983

190-
## Integration Test Insights & Patterns
84+
## Common Patterns and Pitfalls
19185

192-
### Data Access Patterns
193-
- **Legacy CRUD Pattern**: Some classes still use generic `CRUDBase` (being phased out)
194-
- **Modern Repository Pattern**: New domain-focused repositories in `app/repositories/`
195-
- **Service Layer**: Business logic extracted from data access layer to `app/services/`
196-
- **Async Operations**: All new services use proper async/await patterns consistently
197-
- **Field Validation**: Pydantic schemas use consistent field names between database and API
86+
### Data Access
87+
- **Repository pattern** is the modern approach; `app/crud/` is legacy (being phased out)
88+
- All new services should use proper async/await patterns
89+
- Pydantic schemas use consistent field names between database and API
19890

199-
### API Endpoint Patterns
200-
- **Authentication**: Many endpoints require service account or admin authentication
201-
- **Validation Endpoints**: Custom endpoints like `/flows/{id}/validate` for business logic validation
202-
- **Query Parameters**: Support filtering, searching, pagination consistently across list endpoints
91+
### API Endpoints
92+
- Many endpoints require service account or admin authentication
93+
- Custom validation endpoints like `/flows/{id}/validate` for business logic
94+
- List endpoints support filtering, searching, and pagination consistently
20395

204-
### Testing Best Practices
205-
- **Docker Environment**: Always use `bash scripts/integration-tests.sh` for proper database setup
206-
- **Test Isolation**: Clean up test data in fixtures to prevent test interference
207-
- **Async Context**: Be careful with SQLAlchemy async operations
208-
209-
### Performance Considerations
210-
- **Bulk Operations**: Implement batch create/update operations for efficiency
211-
- **Query Optimization**: Full-text search uses PostgreSQL tsvector and GIN indexes
212-
213-
### Common Pitfalls
214-
- **Status Code Expectations**: Verify actual API behavior vs REST conventions
215-
- **Async/Await Consistency**: Ensure all database operations use proper async patterns
96+
### Testing
97+
- Always use `bash scripts/integration-tests.sh` for integration tests (proper Docker environment)
98+
- Clean up test data in fixtures to prevent test interference
99+
- Be careful with SQLAlchemy async session management in tests
100+
- Test configuration lives in `conftest.py` files
101+
102+
### Performance
103+
- Bulk operations: use batch create/update for efficiency
104+
- Full-text search uses PostgreSQL tsvector and GIN indexes
105+
106+
### REST Conventions
107+
- Verify actual API behavior vs REST conventions for status codes
108+
- Ensure async/await consistency across all database operations

README.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,29 @@ The public API is available at `http://localhost:8000`. The seed script prints J
6666

6767
> **Note:** The `api` service volume-mounts `./app` so code changes are live without rebuild. The `scripts/` directory is _not_ mounted by default -- the seed command above uses `-v` to mount it explicitly.
6868
69+
### Running without Docker
70+
71+
```bash
72+
# Public API
73+
uvicorn app.main:app --reload
74+
75+
# Internal API
76+
gunicorn --workers=1 --worker-class=uvicorn.workers.UvicornWorker app.internal_api:internal_app
77+
```
78+
79+
### Configuring local admin access
80+
81+
To grant admin access for testing the CMS/chatflow builder in the admin UI:
82+
83+
```sql
84+
UPDATE users SET type = 'WRIVETED' WHERE email = 'your-email@example.com';
85+
86+
INSERT INTO wriveted_admins (id)
87+
SELECT id FROM users WHERE email = 'your-email@example.com';
88+
```
89+
90+
After updating, log out and back in to get a new JWT with updated permissions.
91+
6992
## Chatflow runtime
7093

7194
The chat runtime (`app/services/chat_runtime.py`) drives Huey's interactive reading-preference conversations. Flows are directed graphs of nodes (messages, questions, actions, conditions) defined in the admin UI and stored as JSON.
@@ -90,7 +113,10 @@ CMS content (questions, messages, jokes, facts) is managed via the API and surfa
90113

91114
### Unit tests (no database required)
92115

116+
Unit tests require several environment variables to be set. Use the helper script:
117+
93118
```bash
119+
source scripts/setup-test-env.sh
94120
poetry run pytest app/tests/unit/ -v
95121
```
96122

@@ -102,8 +128,22 @@ The recommended way to run integration tests -- provides a proper environment wi
102128
bash scripts/integration-tests.sh
103129
```
104130

131+
If you don't have GCR credentials (e.g. first-time setup), skip remote Docker cache pulls:
132+
133+
```bash
134+
LOCAL_BUILD_ONLY=1 bash scripts/integration-tests.sh
135+
```
136+
105137
Ensure no conflicting PostgreSQL containers are running on port 5432.
106138

139+
### Isolated tests
140+
141+
Some tests require isolation from other tests (e.g. connection pool stress tests). These are marked with `@pytest.mark.isolated` and skipped during normal test runs. CI runs them separately:
142+
143+
```bash
144+
bash scripts/integration-tests.sh --run-isolated-tests
145+
```
146+
107147
### Single test
108148

109149
```bash
@@ -118,6 +158,8 @@ Requires a running Docker stack with seeded data:
118158
python scripts/test_huey_flow_e2e.py
119159
```
120160

161+
See [docs/testing-credentials.md](docs/testing-credentials.md) for test data setup and authentication tokens.
162+
121163
## Database migrations
122164

123165
Uses [Alembic](https://alembic.sqlalchemy.org/) with SQLAlchemy 2.0 models. PostgreSQL functions and triggers are defined declaratively in Python using `alembic_utils` (`app/db/functions.py`, `app/db/triggers.py`).

0 commit comments

Comments
 (0)