diff --git a/docs/agent/decisions/012-portfolio-no-orm.md b/docs/agent/decisions/012-portfolio-no-orm.md new file mode 100644 index 00000000..58835368 --- /dev/null +++ b/docs/agent/decisions/012-portfolio-no-orm.md @@ -0,0 +1,92 @@ +--- +type: decision +status: active +date: 2026-03-20 +agent_author: "claude" +tags: [portfolio, database, supabase, orm, prisma] +related_files: + - tradingagents/portfolio/supabase_client.py + - tradingagents/portfolio/repository.py + - tradingagents/portfolio/migrations/001_initial_schema.sql +--- + +## Context + +When designing the Portfolio Manager data layer (Phase 1), the question arose: +should we use an ORM (specifically **Prisma**) or keep the raw `supabase-py` +client that the scaffolding already plans to use? + +The options considered were: + +| Option | Description | +|--------|-------------| +| **Raw `supabase-py`** (chosen) | Direct Supabase PostgREST client, builder-pattern API | +| **Prisma Python** (`prisma-client-py`) | Code-generated type-safe ORM backed by Node.js | +| **SQLAlchemy** | Full ORM with Core + ORM layers, Alembic migrations | + +## The Decision + +**Use raw `supabase-py` without an ORM for the portfolio data layer.** + +The data access layer (`supabase_client.py`) wraps the Supabase client directly. +Our own `Portfolio`, `Holding`, `Trade`, and `PortfolioSnapshot` dataclasses +provide the type-safety layer; serialisation is handled by `to_dict()` / +`from_dict()` on each model. + +## Why Not Prisma + +1. **Node.js runtime dependency** — `prisma-client-py` uses Prisma's Node.js + engine at code-generation time. This adds a non-Python runtime requirement + to a Python-only project. + +2. **Conflicts with Supabase's migration tooling** — the project already uses + Supabase's SQL migration files (`migrations/001_initial_schema.sql`) and the + Supabase dashboard for schema changes. Prisma's `prisma migrate` maintains + its own shadow database and migration state, creating two competing systems. + +3. **Code generation build step** — every schema change requires running + `prisma generate` before the Python code works. This complicates CI, local + setup, and agent-driven development. + +4. **Overkill for 4 tables** — the portfolio schema has exactly 4 tables with + straightforward CRUD. Prisma's relationship traversal and complex query + features offer no benefit here. + +## Why Not SQLAlchemy + +1. **Not using a local database** — the database is managed by Supabase (hosted + PostgreSQL). SQLAlchemy's connection-pooling and engine management are + designed for direct database connections, which bypass Supabase's PostgREST + API and Row Level Security. + +2. **Extra dependency** — SQLAlchemy + Alembic would be significant new + dependencies for a non-DB-heavy app. + +## Why Raw `supabase-py` Is Sufficient + +- `supabase-py` provides a clean builder-pattern API: + `client.table("holdings").select("*").eq("portfolio_id", id).execute()` +- Our dataclasses already provide compile-time type safety and lossless + serialisation; the client only handles transport. +- Migrations are plain SQL files — readable, versionable, Supabase-native. +- `SupabaseClient` is a thin singleton wrapper that translates HTTP errors into + domain exceptions — this gives us the ORM-like error-handling benefit without + the complexity. + +## Constraints + +- **Do not** add an ORM dependency (`prisma-client-py`, `sqlalchemy`, `tortoise-orm`) + to `pyproject.toml` without revisiting this decision. +- **Do not** bypass `SupabaseClient` by importing `supabase` directly in other + modules — always go through `PortfolioRepository`. +- If the schema grows beyond ~10 tables or requires complex multi-table joins, + revisit this decision and consider SQLAlchemy Core (not the ORM layer) with + direct `asyncpg` connections. + +## Actionable Rules + +- All DB access goes through `PortfolioRepository` → `SupabaseClient`. +- Migrations are `.sql` files in `tradingagents/portfolio/migrations/`, run via + the Supabase SQL Editor or `supabase db push`. +- Type safety comes from dataclass `to_dict()` / `from_dict()` — not from a + code-generated ORM schema. diff --git a/docs/portfolio/00_overview.md b/docs/portfolio/00_overview.md index a6f7ab46..741a2605 100644 --- a/docs/portfolio/00_overview.md +++ b/docs/portfolio/00_overview.md @@ -72,6 +72,32 @@ investment portfolio end-to-end. It performs the following actions in sequence: The `report_path` column in the `portfolios` table points to the daily portfolio subdirectory on disk: `reports/daily/{date}/portfolio/`. +### Data Access Layer: raw `supabase-py` (no ORM) + +The Python code talks to Supabase through the raw `supabase-py` client — **no +ORM** (Prisma, SQLAlchemy, etc.) is used. + +**Why not Prisma?** +- `prisma-client-py` requires a Node.js runtime for code generation — an + extra non-Python dependency in a Python-only project. +- Prisma's `prisma migrate` conflicts with Supabase's own SQL migration tooling + (we use `.sql` files in `tradingagents/portfolio/migrations/`). +- 4 tables with straightforward CRUD don't benefit from a code-generated ORM. + +**Why not SQLAlchemy?** +- Supabase is accessed via PostgREST (HTTP API), not a direct TCP database + connection. SQLAlchemy is designed for direct connections and would bypass + Supabase's Row Level Security. +- Extra dependency overhead for a non-DB-heavy feature. + +**`supabase-py` is sufficient because:** +- Its builder-pattern API (`client.table("holdings").select("*").eq(...)`) + covers all needed queries cleanly. +- Our own dataclasses handle type-safety via `to_dict()` / `from_dict()`. +- Plain SQL migration files are readable, versionable, and Supabase-native. + +> Full rationale: `docs/agent/decisions/012-portfolio-no-orm.md` + --- ## 5-Phase Workflow diff --git a/tradingagents/portfolio/supabase_client.py b/tradingagents/portfolio/supabase_client.py index f2d7b0e6..efd87b34 100644 --- a/tradingagents/portfolio/supabase_client.py +++ b/tradingagents/portfolio/supabase_client.py @@ -1,9 +1,15 @@ """Supabase database client for the Portfolio Manager. -Thin wrapper around ``supabase-py`` that: +Thin wrapper around ``supabase-py`` (no ORM) that: - Provides a singleton connection (one client per process) - Translates Supabase / HTTP errors into domain exceptions -- Converts raw DB rows into typed model instances +- Converts raw DB rows into typed model instances via ``Model.from_dict()`` + +**No ORM is used here by design** — see +``docs/agent/decisions/012-portfolio-no-orm.md`` for the full rationale. +In short: ``supabase-py``'s builder-pattern API is sufficient for 4 tables; +Prisma and SQLAlchemy add build-step / runtime complexity that isn't warranted +for this non-DB-heavy feature. Usage::