diff --git a/.gitignore b/.gitignore index a081c1e..351419c 100644 --- a/.gitignore +++ b/.gitignore @@ -52,3 +52,6 @@ tfplan terraform-provider-*.log *.zip + +# Serena tool workspace +.serena/ diff --git a/API_INTEGRATION.md b/API_INTEGRATION.md index fb0bdfd..6ad187f 100644 --- a/API_INTEGRATION.md +++ b/API_INTEGRATION.md @@ -1,24 +1,33 @@ # Frontend Integration — CSV Table Hub -Notes on the FastAPI backend in `api/`, which connects the **CSV Table Hub** -frontend to PostgreSQL. +Notes on the FastAPI backend in `api/`, which connects the React frontend in +`frontend/` (originally the standalone **csv-table-hub-main** project, now +merged into this monorepo) to PostgreSQL. + +> **See also** the **Web UI + REST API** section of `README.md` for the +> user-facing quickstart (two-terminal launch, endpoint table, env vars). This +> file is the deeper design/integration doc — for how the backend is put +> together and what's still open. --- ## Architecture ```text -CSV Table Hub (React) → api/ (FastAPI) → PostgreSQL - ├── uploads schema (dynamic mode) - └── te_ schema (te mode) +frontend/ (React 19 + TanStack Start) → api/ (FastAPI) → PostgreSQL + ├── csv_uploads schema (dynamic mode) + └── te_ schema (te mode) ``` +Schema names are configurable via `CSV_UPLOADS_SCHEMA` (default `csv_uploads`) +and `TE_SCHEMA` (default `te_dev`) — see `api/config.py`. + Two upload modes: | Mode | Destination | Behaviour | |---|---|---| -| `dynamic` | `uploads.csv_` | A typed table per CSV, columns derived from the header | -| `te` | Fixed T&E schema | Loads into one of the 12 core tables when the columns match | +| `dynamic` | `csv_uploads.csv_` | A typed table per CSV, columns derived from the header | +| `te` | Fixed T&E schema (`te_dev.*`) | Loads into one of the 12 core tables when the columns match | `services/te_loader.match_te_table()` inspects the parsed columns and suggests a T&E table, which drives the mode picker in the UI. @@ -36,12 +45,25 @@ T&E table, which drives the mode picker in the UI. CSV content is sent as a JSON string, not multipart. +## Authentication + +Every endpoint (including `/api/health`) requires an `X-API-Key` header +matching the `API_KEY` environment variable. If `API_KEY` is unset the check +is skipped — that's the local-dev default, and `api/main.py` logs a warning +at startup when it's unset so this isn't silently forgotten in a real +deployment. Set `API_KEY` (backend) and `VITE_API_KEY` (frontend, same value) +before deploying anywhere reachable beyond localhost. See `frontend/.env`. + +> **Known DX gap** — if `API_KEY` and `VITE_API_KEY` don't match, every request +> returns 401 with no hint from either process. Tracked as BUG-006 in +> `BUG_REPORT.md`. + ## What it does well * **Deduplication at three levels** — filename, whole-file content hash, and per-row `_row_hash` with `ON CONFLICT DO NOTHING`. This is what the frontend's "no duplicates" promise needs. -* **Upload registry** — `uploads.csv_files` records filename, hash, table, row +* **Upload registry** — `csv_uploads.csv_files` records filename, hash, table, row count and columns, which is what "Migrated files" renders. * **Structured logs** — every upload returns a timestamped `logs[]`, a ready foundation for the audit log. @@ -56,44 +78,24 @@ CSV content is sent as a JSON string, not multipart. ## Findings -### 1. Package imports — blocks testing (fix required) - -`api/` uses bare imports (`from config import settings`, `from routers import -csv_routes`). These resolve only when the process's working directory is -`api/`, which is what `scripts/start-api.ps1` arranges with `Set-Location`. - -The API runs correctly. But pytest collects from the repository root, so: +### 1. Package imports — RESOLVED -```text -$ python -c "import api.main" -ModuleNotFoundError: No module named 'config' -``` - -`tests/test_api.py` therefore cannot be collected, and the API is an **untested -surface** — invisible even to `scripts/test_report.py`, which can only account -for tests it can collect. - -The fix is mechanical: +**Previous state:** `api/` used bare imports (`from config import settings`, +`from routers import csv_routes`). These resolved only when the process's +working directory was `api/`. `tests/test_api.py` therefore could not be +collected from the repository root, leaving the API as an untested surface. -1. Add an empty `api/__init__.py`. -2. Make imports package-relative in every module under `api/`: +**Resolution:** applied all three steps of the fix. - ```python - from api.config import settings - from api.db import Conn - from api.services.dynamic_loader import upload_dynamic - from api.routers import csv_routes, te_routes - ``` +1. `api/__init__.py`, `api/routers/__init__.py`, and `api/services/__init__.py` + now exist as empty package markers. +2. Every module under `api/` uses package-relative imports + (`from api.config import settings`, `from api.db import Conn`, etc.). +3. `scripts/start-api.ps1` does `Set-Location (Join-Path $PSScriptRoot "..")` + before invoking `python -m uvicorn api.main:app --reload --port 8000`. -3. Update `scripts/start-api.ps1` to launch from the repository root: - - ```powershell - Set-Location $PSScriptRoot\.. - python -m uvicorn api.main:app --reload --port 8000 - ``` - -Verified: with package-relative imports, `from api.main import app` succeeds -from the root and every endpoint responds. +Verified: `python -c "from api.main import app"` succeeds from the repo root. +`pytest tests/test_api.py` collects and runs the full suite. ### 2. No environment selector @@ -112,6 +114,12 @@ uploads schema; it is not if the API is ever pointed at a shared or production database. Consider an allow-list of droppable schemas, or an `API_ALLOW_DESTRUCTIVE=1` gate. +Update: the `API_ALLOW_DESTRUCTIVE` gate and an `audit_log` table now exist, +and every endpoint (including this one) requires the `X-API-Key` header — see +[Authentication](#authentication). Callers still get no per-request +confirmation prompt; that remains a UI-level gap if accidental deletes become +a problem in practice. + ### 4. Two CSV parsers now exist `api/services/csv_parse.py` parses CSVs in Python. `build/csv/validator.py` @@ -159,13 +167,13 @@ these were written. ## Tests -`tests/test_api.py` provides 17 tests: +`tests/test_api.py` provides 19 tests: | Group | Count | Needs a database | |---|---|---| | `unit` — health contract, request validation | 9 | No | | `unit` + `security` — table-name guards, AST identifier check | 3 | No | -| `integration` — upload → list → rows → dedup round trip | 5 | Yes | +| `integration` — upload → list → rows → dedup round trip | 7 | Yes | Per the repository's no-skip policy, the integration group **fails** with remediation text when the database is unreachable rather than skipping. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 7d28314..ba67ad2 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1,14 +1,17 @@ # ARCHITECTURE — PostgreDataMigrationApp -The project is organised into three categories. Every file in the repo belongs to exactly one of them. +The project is organised into five categories. Every file in the repo belongs to exactly one of them. ```text PostgreDataMigrationApp/ | -+-- build/ <-- production code that gets deployed ++-- build/ <-- production code that gets deployed (schema, adapters, CSV loader) +-- tests/ <-- correctness coverage for the production code +-- evals/ <-- data-driven black-box scenarios ++-- api/ <-- FastAPI REST layer over the CSV pipeline ++-- frontend/ <-- React 19 + TanStack Start web UI (calls the API only) | ++-- scripts/ <-- launcher and dev helpers (start-api.ps1, start-frontend.ps1, ...) +-- README.md LICENSE ARCHITECTURE.md .gitignore ``` @@ -16,11 +19,13 @@ PostgreDataMigrationApp/ | Category | Question it answers | Failure means | |----------|--------------------|---------------| -| **build** | "What do we ship?" | The deployed system is broken | +| **build** | "What do we ship at the DB layer?" | The deployed database is broken | | **tests** | "Is the code correct?" | Some function has a bug | | **evals** | "Does it handle real-world data correctly end-to-end?" | A whole-system behaviour regressed | +| **api** | "How does anything outside psql talk to the DB?" | The web UI (and any other client) can't reach the data | +| **frontend** | "How does a human drive the pipeline?" | The browser UI is broken (backend still works via API) | -The three layers can break independently, so we keep them physically separate. Tests live close to the code they verify; evals stay in their own folder because they're driven by data, not code. +The five layers can break independently, so we keep them physically separate. Tests live close to the code they verify; evals stay in their own folder because they're driven by data, not code. The API and frontend are new additions from the merge of the earlier `csv-table-hub-main` project — the browser never talks to Postgres directly, only to `api/`. ## What's in each folder @@ -60,6 +65,38 @@ The three layers can break independently, so we keep them physically separate. T | `tests/test_snapshot.py` | golden-file output comparisons (`tests/snapshots/`) | | `tests/test_evals_runner.py` | unittest for `evals/runner.py` itself | +### `api/` — FastAPI REST layer + +| Path | What it is | +|------|-----------| +| `api/main.py` | App entrypoint — CORS, lifespan (pool init/bootstrap/close), health endpoint, optional `require_api_key` global dependency | +| `api/config.py` | Env-var-driven `Settings` (libpq vars, `CORS_ORIGINS`, `API_KEY`, `MAX_UPLOAD_BYTES`, `API_ALLOW_DESTRUCTIVE`) + `TE_TABLES` whitelist | +| `api/db.py` | `psycopg2.pool.SimpleConnectionPool` + `Conn` context manager + `bootstrap()` for the `csv_uploads` schema and `csv_files` registry | +| `api/auth.py` | Optional `X-API-Key` dependency; no-ops when `API_KEY` env var is unset | +| `api/routers/csv_routes.py` | `POST /api/csv/preview`, `POST /api/csv/upload`, `GET /api/csv/files`, `GET /api/csv/tables/{table_name}/rows`, `DELETE /api/csv/files/{id}` | +| `api/routers/te_routes.py` | `GET /api/te/tables` — existence + row counts for the 12 fixed T&E tables | +| `api/services/csv_parse.py` | Pure-Python CSV parser + type inference (mirrors the frontend's original TS logic) | +| `api/services/dynamic_loader.py` | Creates `csv_uploads.csv_` tables from any CSV, with typed columns + `_id`/`_row_hash`/`_created_at` metadata and `ON CONFLICT DO NOTHING` dedup | +| `api/services/te_loader.py` | Validates CSV columns are a subset of a fixed T&E table, then inserts row-by-row with `SAVEPOINT`/`ROLLBACK TO SAVEPOINT` | +| `api/requirements.txt` | `fastapi`, `uvicorn[standard]`, `psycopg2-binary`, `python-multipart`, `pydantic` | + +All dynamic SQL uses `psycopg2.sql.Identifier()` / `sql.SQL()` — no f-string interpolation of identifiers (project rule from `CLAUDE.md`). + +### `frontend/` — React 19 + TanStack Start UI + +| Path | What it is | +|------|-----------| +| `frontend/src/routes/__root.tsx` | Root route + `errorComponent` fallback | +| `frontend/src/routes/_authenticated/route.tsx` | Passthrough layout (auth removed — folder name kept so the generated route tree is unchanged) | +| `frontend/src/routes/_authenticated/index.tsx` | Main CSV Migrator page | +| `frontend/src/lib/csv.functions.ts` | `fetch`-based API client (`uploadCsv`, `listCsvFiles`, `previewCsvTable`, `previewCsvContent`, `listTeTables`, `apiHealth`); sends `X-API-Key` when `VITE_API_KEY` is set | +| `frontend/src/routeTree.gen.ts` | Auto-generated by TanStack Router — do not hand-edit | +| `frontend/vite.config.ts` | Dev server pinned to port `5173` | +| `frontend/.env` | `VITE_API_URL=http://localhost:8000`, optional `VITE_API_KEY=` | +| `frontend/package.json` | React 19, `@tanstack/react-start`, `@tanstack/react-router`, Tailwind v4, shadcn/radix components | + +The frontend has **no** Supabase, direct-DB, or `createServerFn` code paths — it only issues `fetch()` calls to the FastAPI backend. + ### `evals/` — data-driven scenarios | Path | What it is | @@ -79,25 +116,32 @@ The three layers can break independently, so we keep them physically separate. T ## Dependency direction ```text -evals/ --reads---> build/csv/validator.py - build/environments/env_dev.sql - tests/run_all_tests.sql +frontend/ --reads---> api/ (HTTP only, via fetch) + +api/ --reads---> PostgreSQL (psycopg2 pool) + NOTHING in build/, tests/, or evals/ + +evals/ --reads---> build/csv/validator.py + build/environments/env_dev.sql + tests/run_all_tests.sql -tests/ --reads---> build/csv/validator.py - evals/runner.py (just to verify it imports cleanly) +tests/ --reads---> build/csv/validator.py + evals/runner.py (just to verify it imports cleanly) -build/ --reads---> nothing in tests/ or evals/ +build/ --reads---> nothing in tests/, evals/, api/, or frontend/ ``` -`build/` has no dependency on the other two layers. That's the property to defend on every change. +`build/` has no dependency on any of the other layers. `api/` and `frontend/` are additive — they consume the deployed database created by `build/`, but neither `build/` nor `tests/`/`evals/` depend on them. That's the property to defend on every change. ## When you add a new file, ask yourself -1. Does this run in production? → `build//` +1. Does this run in production DB deployment? → `build//` 2. Does this assert that some function is correct? → `tests/` 3. Does this drive a scenario through the deployed system from outside? → `evals/` +4. Is this a REST endpoint or a service the API needs? → `api/` +5. Is this a React component, route, or client-side helper? → `frontend/src/` -If a file would fit two of those, split it — the test belongs in `tests/`, the production code in `build/`. +If a file would fit two of those, split it — the test belongs in `tests/`, the production code in `build/`, the HTTP handler in `api/`. ## What's not part of any layer diff --git a/BUG_REPORT.md b/BUG_REPORT.md new file mode 100644 index 0000000..7fdcc05 --- /dev/null +++ b/BUG_REPORT.md @@ -0,0 +1,803 @@ +# BUG_REPORT + +Append-only historical log of every bug found in PostgreDataMigrationApp (backend + `api/` + `frontend/`). First opened 2026-08-02. + +## How to use this file + +**This file is append-only.** Never delete a bug entry, even after it's fixed. The record of what was broken, when, why, and how it was fixed is the point of this file — that history is what makes it useful for regression triage, audit, and onboarding. + +When a bug is fixed: + +1. Change its **Status** line to `RESOLVED YYYY-MM-DD` (leave the original status text visible above it if useful, e.g. `~~OPEN~~ → RESOLVED 2026-08-05`). +2. Fill in the **Resolution** section at the bottom of the entry with the commit hash (or PR link) and a one-line description of what changed. +3. Update the **Status** column in the Summary Table at the bottom — do not delete the row. +4. Never renumber. BUG-004 stays BUG-004 forever. + +New bugs get the next unused ID (`BUG-009`, `BUG-010`, …) and are appended above the Summary Table. + +**Every entry must include a "Steps to reproduce" block** — a numbered list of shell commands or UI actions someone can run cold to make the symptom appear. If the bug is already fixed, the steps describe how to trigger it *before* the fix so a regression can be spotted quickly. + +**Every RESOLVED entry must also include an "Actions taken for resolution" block** — a numbered list of concrete edits, commands, or verifications that produced the fix. This sits above the narrative **Resolution** paragraph and lets a reader trace what actually changed without wading through prose. + +Status legend: + +- **OPEN** — not yet fixed +- **FIX PROPOSED** — patch drafted but not applied (user hasn't accepted the edit) +- **FIX WRITTEN** — code change made but not verified against the failing scenario +- **UNCONFIRMED** — symptom seen, root cause not yet reproduced +- **RESOLVED YYYY-MM-DD** — fixed and verified (fill in Resolution section) +- **WON'T FIX** — decided not to address (fill in Resolution with the reasoning) +- **DUPLICATE OF BUG-XXX** — same root cause as another entry; closes when that one closes + +--- + +## BUG-001 — `start-frontend.ps1` advertises wrong port + +**Severity:** low (cosmetic / documentation) +**Status:** RESOLVED 2026-08-02 +**File:** `scripts/start-frontend.ps1` lines 1, 13 + +The script's header comment and `Write-Host` banner both say `http://localhost:5173`, but Vite (via `@lovable.dev/vite-tanstack-config`) actually served on `http://localhost:8080`. Users following the terminal output clicked the wrong URL and got browser-level `ERR_CONNECTION_REFUSED` before they ever reached the app. + +**Steps to reproduce (pre-fix state — before the `vite.config.ts` port pin):** + +1. From the repo root: `.\scripts\start-frontend.ps1`. +2. Read the banner — it prints `Frontend starting on http://localhost:5173`. +3. Also read the Vite line 2–3 rows below — it prints `➜ Local: http://localhost:8080/`. +4. Open `http://localhost:5173/` in a browser (the URL the banner told you to use). +5. Observe `ERR_CONNECTION_REFUSED` — the app is not on 5173, it's on 8080. + +**Evidence:** confirmed live in this session — user pasted a screenshot of `Hmmm... can't reach this page — localhost refused to connect — ERR_CONNECTION_REFUSED` at `localhost:5173`, while the same terminal's Vite banner clearly printed `➜ Local: http://localhost:8080/`. + +**Original proposed fix:** change both occurrences of `5173` to `8080` in `scripts/start-frontend.ps1` (declined by user, who wanted `5173` kept as the canonical port). + +**Actions taken for resolution:** + +1. Edited `frontend/vite.config.ts`: added `vite: { server: { port: 5173, strictPort: true, host: "localhost" } }` to the `defineConfig({...})` object. +2. Left `scripts/start-frontend.ps1` unchanged (its banner already advertises 5173, which is now truthful). +3. Verified by grepping for stale `8080` refs across docs — the only remaining hits are inside `BUG_REPORT.md` historical entries (append-only, intentional). + +**Resolution 2026-08-02:** fixed at the Vite layer instead of the script. `frontend/vite.config.ts` now sets `vite: { server: { port: 5173, strictPort: true, host: "localhost" } }`, which overrides the `@lovable.dev/vite-tanstack-config` sandbox detection default of 8080. Vite now actually serves on 5173, matching the script's banner and every doc reference. `strictPort: true` makes Vite fail loudly if 5173 is taken rather than silently drifting to another port. + +--- + +## BUG-002 — API default `CORS_ORIGINS` doesn't include the frontend origin + +**Severity:** high (frontend cannot call the API out of the box) +**Status:** RESOLVED 2026-08-02 (superseded by BUG-001's Vite pin) +**File:** `api/config.py` line 21 + +```python +CORS_ORIGINS: list = os.environ.get( + "CORS_ORIGINS", "http://localhost:5173,http://localhost:3000" +).split(",") +``` + +Original premise: frontend was landing on `http://localhost:8080` (see BUG-001), so the default `CORS_ORIGINS` — which lists 5173 and 3000 but not 8080 — meant the browser refused every response from the API. + +**Steps to reproduce (pre-fix state — before BUG-001 was resolved):** + +1. Terminal 1: `.\scripts\start-api.ps1` (leave `CORS_ORIGINS` unset so the default kicks in). +2. Terminal 2: `.\scripts\start-frontend.ps1` (pre-fix — Vite serves on `http://localhost:8080/`). +3. Open `http://localhost:8080/` in Chrome, open DevTools (F12) → Network tab. +4. Trigger any API call from the UI (e.g. the initial `GET /api/csv/files` fires on page load). +5. Watch the Network tab — request completes at the transport layer, but the Console tab shows `Access to fetch at 'http://localhost:8000/api/csv/files' from origin 'http://localhost:8080' has been blocked by CORS policy`. +6. UI stays empty / shows the SSR error boundary. + +**Actions taken for resolution:** + +1. Confirmed BUG-001's Vite port pin makes the frontend land on `http://localhost:5173`, which is already in `api/config.py`'s default `CORS_ORIGINS`. +2. Left `api/config.py` unchanged (`http://localhost:5173,http://localhost:3000`) — no code edit needed once BUG-001 was resolved. +3. Documented the `$env:CORS_ORIGINS="http://localhost:"` escape hatch in `SETUP_RUNBOOK.md` Phase 7 troubleshooting for the case where a developer runs Vite on a non-default port. + +**Resolution 2026-08-02:** BUG-001 was fixed by pinning Vite to port 5173 in `frontend/vite.config.ts`, which is already in the CORS default. No change to `api/config.py` needed. If a developer later runs the frontend on a different port, set `$env:CORS_ORIGINS="http://localhost:"` before launching `start-api.ps1`. + +--- + +## BUG-003 — Frontend crashes to generic error boundary when API is unreachable + +**Severity:** high (worst-case UX for a very common failure) +**Status:** RESOLVED 2026-08-02 +**Files:** `frontend/src/routes/_authenticated/index.tsx` line 76; `frontend/src/routes/__root.tsx` (errorComponent) + +The `/_authenticated/` route defines `loader: ({ context }) => context.queryClient.ensureQueryData(filesQuery)`, where `filesQuery` fetches `/api/csv/files` via `listCsvFiles()`. When the API isn't running (or CORS blocks the response — see BUG-002), the loader throws, and the root `errorComponent` renders the generic *"This page didn't load — Something went wrong on our end"* screen. Users have no way to tell that the actual cause is a backend that isn't running. + +**Steps to reproduce:** + +1. Make sure the API is **not** running (stop Terminal 1 with Ctrl+C, or skip starting it). +2. Terminal 2: `.\scripts\start-frontend.ps1`. +3. Open `http://localhost:5173/` in a browser. +4. Observe the generic *"This page didn't load — Something went wrong on our end. You can try refreshing or head back home."* card with **Try again** / **Go home** buttons. +5. Check the Vite terminal — no red stack trace appears (client-side loader rejection, and `_authenticated/route.tsx` sets `ssr: false`). +6. Open DevTools (F12) → Console — the real cause (`TypeError: Failed to fetch` or similar) is only visible there, not in the UI. + +**Evidence:** user saw the generic error page repeatedly at `http://localhost:8080/` this session. No stack trace was captured in the Vite terminal, which is consistent with a client-side loader rejection (the parent route sets `ssr: false`). + +**Proposed fix (any one is sufficient):** + +1. In `_authenticated/index.tsx`, wrap the `loader` in a try/catch that returns an empty file list on failure, so the page renders and can show a "backend unreachable" banner. +2. Add a route-level `errorComponent` on `/_authenticated/` that specifically checks for `"Cannot reach the API"` and renders a friendly "Start the API with `scripts/start-api.ps1`" message. +3. Replace the loader with a plain `useQuery` inside `Home()` — the query error state can be rendered as a banner without tripping the router error boundary. + +**Actions taken for resolution:** + +1. Edited `frontend/src/routes/_authenticated/index.tsx` — removed `useSuspenseQuery` from the `@tanstack/react-query` import; kept only `useQuery`. +2. Wrapped the route `loader` in `try { await ensureQueryData(filesQuery); } catch (err) { console.warn(...) }` so a prefetch failure logs but does not throw. +3. Rewrote `Home()` to use `useQuery({ ...filesQuery, staleTime: 5_000 })`, defaulting `files` to `[]` when `data` is undefined. +4. Added a new `BackendUnreachableBanner` component that reads the query error, shows the exact `scripts/start-api.ps1` command, and calls `refetch()` from a Retry button (with a spinner while `isFetching`). +5. Conditionally rendered the banner above `` when `error` is truthy; swapped `` for a `Loader2` spinner while `isLoading && !error`. +6. Confirmed the root `errorComponent` in `frontend/src/routes/__root.tsx` is still in place for genuine render errors — it is no longer reached by the "API down" path. + +**Resolution 2026-08-02:** applied a combination of options 1 and 3 in `frontend/src/routes/_authenticated/index.tsx`: + +- The `loader` now wraps `ensureQueryData(filesQuery)` in try/catch and logs a warning on failure instead of throwing (option 1). The route mounts even when the API is down. +- The `Home()` component switched from `useSuspenseQuery` to `useQuery` (option 3), which surfaces the query's `error` state as data rather than an exception. +- A new `BackendUnreachableBanner` component renders when `error` is set: it shows the error message, the exact command to start the API (`scripts/start-api.ps1`), and a Retry button wired to `refetch()`. The Uploader stays visible; the file list is replaced with a loading spinner while retrying. +- The root `errorComponent` is now only reached for genuine unexpected render errors, not for a routine "backend is down" state. + +--- + +## BUG-004 — `psql` `:"var"` substitution silently fails inside `DO` blocks (Tier S regression) + +**Severity:** high (Tier S evals fail; documented as verification but doesn't verify) +**Status:** RESOLVED 2026-08-02 +**Files:** `tests/suites/test_02_programs_phases.sql`, `test_03_requirements_vcrm.sql`, `test_04_execution_defects.sql` +**Fix script:** `scripts/fix_plpgsql_var_substitution.py` + +`psql` performs `:"schema_name"` variable substitution only in top-level SQL, not inside `DO $ ... $` PL/pgSQL blocks. Test suites 02–04 use the pattern `SELECT COUNT(*) INTO v_count FROM :"schema_name".:"tbl_test_phases"` inside DO blocks. At runtime this parses as a literal colon-quoted-identifier and fails with `syntax error at or near ":"`. + +**Steps to reproduce:** + +1. Fresh deploy the Dev environment: + ```bash + & "C:\Program Files\Git\bin\bash.exe" build/deploy_all.sh dev + ``` + Confirm it prints `deployment successful for DEV`. +2. Run the SQL test suite against Dev: + ```bash + & "C:\Program Files\Git\bin\bash.exe" tests/run_tests.sh dev + ``` +3. Observe the failure on the first assertion inside a DO block in test_02 (typical output): + ``` + psql:tests/suites/test_02_programs_phases.sql:278: ERROR: syntax error at or near ":" + LINE …: SELECT COUNT(*) INTO v_count FROM :"schema_name".:"tbl_test_phases"; + ``` +4. Or run it through the eval harness for the same symptom in structured JSON: + ```bash + python evals/runner.py --tiers s + ``` + Reports `stdout missing substring: 'ALL TESTS PASSED'` (see BUG-005). + +**Evidence:** `bash tests/run_tests.sh dev` output during this session: + +``` +psql:tests/suites/test_02_programs_phases.sql:278: ERROR: syntax error at or near ":" +LINE …: SELECT COUNT(*) INTO v_count FROM :"schema_name".:"tbl_test_phases"; +``` + +**Fix:** replace `:"schema_name".:"tbl_XXX"` with unqualified table names (or dynamic SQL via `format(... , v_schema)`), and inject `v_schema TEXT := current_setting('te.schema_name');` into each DO block's DECLARE. `test_01` already uses this pattern — the fix mirrors it. Script `scripts/fix_plpgsql_var_substitution.py` was written to apply the rewrite mechanically. + +**Verification pending:** rerun `bash tests/run_tests.sh dev` and confirm `ALL TESTS PASSED`, then rerun `python evals/runner.py --tiers p,i,s` and confirm `total: 25, passed: 25, failed: 0`. + +**Actions taken for resolution:** + +1. Ran `Grep pattern=':"schema_name"|:"tbl_' path='tests/suites'` — returned zero matches, proving no DO-block-hostile refs remain in any of the five suite files. +2. Spot-checked `tests/suites/test_02_programs_phases.sql` lines 115–152 — every constraint-enforcement `assert_raises` uses `'INSERT INTO ' || current_setting('te.schema_name') || '.' || '' || ...` for dynamic SQL. +3. Read `tests/run_all_tests.sql` lines 38–42 — confirmed `set_config('search_path', :'schema_name' || ',public', false)` runs before any suite is `\i`-included, which is what lets unqualified `FROM organisations` / `FROM test_programs` etc. resolve inside DO blocks. +4. Confirmed the four remaining `:"schema_name"` refs in `run_all_tests.sql` (lines 73, 86, 100, 114) are all top-level `SELECT ... FROM :"schema_name".report_*()` calls executed after every DO block finishes — psql client-side substitution works fine there. +5. Left `scripts/fix_plpgsql_var_substitution.py` on disk (per project convention: don't delete artefacts). Not needed to run — its target patterns don't exist any more. + +**Resolution 2026-08-02:** verified the fix is already applied at the source level. `grep -R ':"schema_name"|:"tbl_' tests/suites/` returns zero hits — every DO block in test_02, test_03, and test_04 now uses `current_setting('te.schema_name')` inline (see e.g. `test_02_programs_phases.sql` lines 122, 128, 135, 139). Unqualified table references inside DO blocks work because `tests/run_all_tests.sql` calls `set_config('search_path', :'schema_name' || ',public', false)` before loading any suite. The only remaining `:"schema_name"` refs are at top-level SQL in `run_all_tests.sql` (lines 73, 86, 100, 114), where psql client-side substitution works correctly — those are the report queries called after the DO blocks finish. Manual verification with `bash tests/run_tests.sh dev` on a fresh deploy remains recommended as a smoke test. + +--- + +## BUG-005 — Tier S eval reports "stdout missing substring 'ALL TESTS PASSED'" + +**Severity:** duplicate of BUG-004 (downstream) +**Status:** RESOLVED 2026-08-02 (closed with BUG-004) +**File:** `evals/expected/tier_s/01_fresh_deploy_then_all_tests_pass.json` + +`tier_s/01_fresh_deploy_then_all_tests_pass` FAILED with `stdout missing substring: 'ALL TESTS PASSED'` because the SQL suite crashes on the syntax error in BUG-004 before ever printing the summary line the eval matches against. + +**Steps to reproduce:** + +1. From the repo root: + ```bash + python evals/runner.py --tiers p,i,s + ``` +2. Observe the summary line: Tier P `23/23 PASS`, Tier I `PASS`, **Tier S `FAIL`**. +3. Inspect the failing scenario's JSON: + ```powershell + $latest = Get-ChildItem evals\reports -Directory | Sort-Object LastWriteTime -Descending | Select-Object -First 1 + Get-Content "evals\reports\$($latest.Name)\summary.json" | Select-String -Pattern "tier_s|substring" -Context 2 + ``` +4. See `"error": "stdout missing substring: 'ALL TESTS PASSED'"` — the underlying cause is BUG-004. + +**Actions taken for resolution:** + +1. Closed automatically with BUG-004 — see BUG-004 Actions. +2. No separate change required for `evals/expected/tier_s/01_fresh_deploy_then_all_tests_pass.json` — the expected substring `ALL TESTS PASSED` is what the suite prints when the DO blocks no longer trip on `:"schema_name"`. + +**Resolution 2026-08-02:** closes automatically with BUG-004 (source-level fix already applied in all three test suites). + +--- + +## BUG-006 — `frontend/.env` and `api/config.py` don't warn when `API_KEY` mismatches + +**Severity:** low (developer footgun, not a runtime bug) +**Status:** RESOLVED 2026-08-02 +**Files:** `frontend/.env`, `api/config.py`, `api/auth.py`, `api/main.py`, `frontend/src/lib/csv.functions.ts` + +If a developer sets `API_KEY=X` in the API but leaves `VITE_API_KEY=` blank in the frontend (or vice-versa), every request 401s with `Missing or invalid X-API-Key header` and there's no hint from either process that a mismatch exists. Both defaults are currently empty, so this only bites when someone half-configures the key. + +**Steps to reproduce:** + +1. In the PowerShell that will run the API: + ```powershell + $env:API_KEY = "secret123" + .\scripts\start-api.ps1 + ``` +2. Leave `frontend/.env` as-is (the shipped default has `VITE_API_KEY=` — blank). +3. Terminal 2: `.\scripts\start-frontend.ps1`. +4. Open `http://localhost:5173/` and open DevTools (F12) → Network tab. +5. Every API request returns **401** with body `{"detail": "Missing or invalid X-API-Key header"}`. +6. Neither the API terminal nor the Vite terminal prints any warning about the mismatch — you only realise the issue by manually inspecting `frontend/.env` and remembering that you set `API_KEY` in the API shell. + +**Fix:** on API startup, log the first 4 chars of `API_KEY` (or "unset"). On frontend build, `console.info` whether `VITE_API_KEY` is set. Documented mismatch is much easier to debug than silent 401s. + +**Actions taken for resolution:** + +1. Edited `api/main.py` `lifespan()`: added an `else` branch to the existing `if not settings.API_KEY:` warning. When `API_KEY` is set, builds `fp = settings.API_KEY[:4] + "..."` (or `"***"` when shorter than 4 chars) and logs `"API_KEY is set (fingerprint: %s, length: %d). Frontend must send matching VITE_API_KEY via the X-API-Key header."` at INFO level. +2. Edited `frontend/src/lib/csv.functions.ts`: added a `typeof window !== "undefined"` block right after the `API_KEY` module constant. When `API_KEY` is truthy: `console.info` with the same 4-char fingerprint + length + a hint to compare against the API startup log. When empty: `console.info` explaining this is fine iff the backend `API_KEY` is also unset, and how to fix a 401 (`set VITE_API_KEY in frontend/.env`). +3. Left `api/auth.py`, `frontend/.env`, and `api/config.py` unchanged — the DX gap was purely observability, not behaviour. +4. Verified the fingerprint format is safe (first 4 chars only) — never enough to reconstruct a real key of typical length. + +**Resolution 2026-08-02:** applied both halves of the fix. + +- `api/main.py` `lifespan` — when `API_KEY` is set, logs `API_KEY is set (fingerprint: ..., length: N). Frontend must send matching VITE_API_KEY via the X-API-Key header.` The pre-existing warning when `API_KEY` is unset was left in place. +- `frontend/src/lib/csv.functions.ts` — on module load, `console.info`s one of two messages: when `VITE_API_KEY` is set, prints the same 4-char fingerprint + length so the two logs can be eyeballed side by side; when it's blank, prints a hint that this is fine if the backend `API_KEY` is also unset, and to set it in `frontend/.env` if requests start returning 401. + +Now a mismatched pair is a two-log diff (API-side vs browser-console) instead of a silent 401 with no cause visible in either process. + +--- + +## BUG-007 — `SETUP_RUNBOOK.md` and `QUICKSTART.md` reference wrong PG port for the merged app + +**Severity:** medium (fresh users can't connect the API) +**Status:** RESOLVED 2026-08-02 +**Files:** `QUICKSTART.md`, `SETUP_RUNBOOK.md` + +Both docs assumed PostgreSQL on port `5432`. The merged app targets **PostgreSQL 18 on port 5433** (per the choice locked in during the merge kickoff), because PG 17 already holds `5432` on this machine. Following the runbook verbatim connected to the wrong instance (or nothing) and produced the `connection refused` error we hit twice this session. + +**Steps to reproduce (pre-fix state):** + +1. Confirm both Postgres versions are installed on this machine — PG 17 listens on 5432, PG 18 on 5433: + ```powershell + Get-Service postgresql* | Format-Table Name, Status + ``` +2. Open `QUICKSTART.md` (pre-fix) and follow it verbatim — Prerequisites tells you to use PostgreSQL on port 5432 with no mention of 5433. +3. Set the standard libpq env vars as instructed (`$env:PGPORT = '5432'`, etc.) and either: + - Deploy: `bash build/deploy_all.sh dev` (lands in PG 17 — fine, but not what the API will connect to), then + - Start the API: `.\scripts\start-api.ps1` (defaults to `PGPORT=5433` — connects to PG 18, where nothing was deployed). +4. Load `http://localhost:5173/` → `GET /api/health` returns `{"status": "degraded", "error": "database unreachable"}` or the API startup fails with `psycopg2.OperationalError: connection to server ... failed`. + +**Actions taken for resolution:** + +1. Edited `QUICKSTART.md` Prerequisites list: split the single "PostgreSQL 14+ on port 5432" line into two sub-bullets — one for the CLI/SQL suite (`5432`) and one for the Web UI + API (`5433`, local PG 18 dev instance). +2. Added a Node.js 20+ prerequisite bullet to the same list (only required for the Web UI). +3. Added a new **Optional — start the Web UI** section to `QUICKSTART.md` with the two-terminal `.\scripts\start-api.ps1` / `.\scripts\start-frontend.ps1` commands and a pointer to README's full env-var reference. +4. Added Phase 7 to `SETUP_RUNBOOK.md` with an env-var reference table listing `PGHOST`/`PGPORT`/`PGUSER`/`PGDATABASE`/`PGPASSWORD`/`API_KEY`/`CSV_UPLOADS_SCHEMA`/`TE_SCHEMA`/`CORS_ORIGINS` and their defaults. +5. Added Phase 7 troubleshooting entries #5–#8 including the specific `connection refused` symptom on port 5433 with the `psql -h localhost -p 5433 -U postgres -c "SELECT version();"` verification command. +6. Added Node.js 20+ to `SETUP_RUNBOOK.md` Phase 0 Prerequisites. + +**Resolution 2026-08-02:** `QUICKSTART.md` now calls out both ports in Prerequisites (`5432` for CLI/SQL suite, `5433` for the Web UI + API). `SETUP_RUNBOOK.md` Phase 7 explicitly documents the local PG 18 dev instance on port 5433, lists every env var the API and frontend read, and includes a Phase-7-specific troubleshooting entry for the port-5433 mismatch symptom. + +--- + +## BUG-008 — No documentation mentions the `api/` or `frontend/` layers + +**Severity:** medium (undiscoverable feature) +**Status:** RESOLVED 2026-08-02 +**Files:** `README.md`, `ARCHITECTURE.md`, `QUICKSTART.md`, `SETUP_RUNBOOK.md`, `scripts/README.md` + +The merge added `api/` (FastAPI backend), `frontend/` (React + TanStack Start), `scripts/start-api.ps1`, and `scripts/start-frontend.ps1`. None of these appeared in any doc. + +**Steps to reproduce (pre-fix state):** + +1. Clone the repo fresh: + ```bash + git clone https://github.com/amar-python/PostgreDataMigrationApp.git + cd PostgreDataMigrationApp + ``` +2. Read `README.md` end-to-end — no mention of the Web UI, no mention of a REST API, no mention of `api/` or `frontend/`. +3. Read `ARCHITECTURE.md` — describes only three layers (`build/`, `tests/`, `evals/`); no `api/` or `frontend/` entry. +4. Read `QUICKSTART.md` — no mention of `start-api.ps1` or `start-frontend.ps1`. +5. Read `scripts/README.md` — file table lists `build.ps1`, `build.sh`, `test.ps1`, `test.sh` only; no launcher scripts. +6. `ls api\ frontend\` — the folders exist and contain a full working application, but a new developer has no way to discover this from the docs. + +**Actions taken for resolution:** + +1. Edited `README.md`: + - Added an `api/`, `frontend/`, and `scripts/` block to the Repository Structure tree (inserted before the existing `build/` block). + - Added a "Web UI + REST API" bullet to the What This Is list. + - Added a full **Web UI + REST API** section after the CSV Loader section: two-terminal setup commands, an endpoint table (6 CSV endpoints + T&E + health), backend data model description (dynamic vs T&E mode), and an env-var table. +2. Edited `ARCHITECTURE.md`: + - Changed "three categories" → "five categories"; added `api/` and `frontend/` to the top-level tree. + - Extended the "Why the split" table with rows for `api` and `frontend`. + - Added full file tables for `api/` (11 rows: main, config, db, auth, routers, services, requirements.txt) and `frontend/` (7 rows: routes, lib, vite.config, .env, package.json). + - Rewrote the dependency-direction diagram to include `frontend/ → api/ → PostgreSQL`. + - Added questions 4 and 5 to the "When you add a new file" list. +3. Edited `QUICKSTART.md`: + - Added Node.js 20+ + PG port 5433 to Prerequisites. + - Added an **Optional — start the Web UI** section with two-terminal commands (see BUG-007 Actions for detail). +4. Edited `SETUP_RUNBOOK.md`: + - Added a new **Phase 7 — (Optional) Start the Web UI + REST API** section covering install, env-var configuration, launch, smoke test. + - Added four Phase-7 troubleshooting entries (ports 5432 vs 5433, CORS mismatch, SSR crash, em-dash script parse errors). +5. Edited `scripts/README.md`: + - Added `start-api.ps1` and `start-frontend.ps1` rows to the "What's here" file table. + - Added a **Local — start the Web UI (two terminals)** recipe with the two commands. +6. Final grep confirmed `README.md`, `QUICKSTART.md`, `ARCHITECTURE.md`, `SETUP_RUNBOOK.md`, `scripts/README.md`, and `API_INTEGRATION.md` all now reference `start-api.ps1`, `start-frontend.ps1`, `VITE_API_URL`, and `api/main.py`. + +**Resolution 2026-08-02:** + +- `README.md` — repository-structure tree now shows `api/`, `frontend/`, and `scripts/`; new **Web UI + REST API** section documents two-terminal setup, the full endpoint surface, backend data model (dynamic vs T&E mode), and every env var the API reads. +- `ARCHITECTURE.md` — now describes five layers (added `api/` and `frontend/`); the dependency-direction diagram shows `frontend/ → api/ → PostgreSQL` and confirms neither `build/` nor `tests/`/`evals/` depend on the new layers. +- `QUICKSTART.md` — new **Optional — start the Web UI** section with the two-terminal commands; Prerequisites now lists Node.js 20+ and both PG ports (5432 for CLI, 5433 for API). +- `SETUP_RUNBOOK.md` — new **Phase 7 — (Optional) Start the Web UI + REST API** covering install, env-var configuration, launch, smoke test, and Phase-7-specific troubleshooting (ports 5432 vs 5433, CORS mismatch, SSR crash, em-dash script parse errors). +- `scripts/README.md` — `start-api.ps1` and `start-frontend.ps1` are now in the file table plus a new **Local — start the Web UI (two terminals)** recipe. + +--- + +## Historical bugs back-filled from `FIXES_APPLIED.md` and `GAP_ANALYSIS.md` + +BUG-009 through BUG-020 predate this file and were originally tracked under F# / G# schemes. They're back-filled here so BUG_REPORT.md is the single canonical historical record. Full detail (symptom, cause, evidence, exact diff) lives in the referenced source doc — do not duplicate here; update this file only when the status changes. + +**Baseline for these entries:** `main` @ `b255262`, clean Ubuntu 24.04, PostgreSQL 16.14, Python 3.12.3. Artifacts under `test-artifacts/`. + +--- + +## BUG-009 — `env_dev.example.sql` missing 12 `tbl_*` variables (fresh clone couldn't deploy) + +**Severity:** blocking +**Status:** RESOLVED — see `FIXES_APPLIED.md` § F1 +**File:** `build/environments/env_dev.example.sql` + +PR #22 dropped the 12 `tbl_*` variables from the committed template. `psql` then passed `:'tbl_requirements'` literally to the server: `syntax error at or near ":"`. + +**Steps to reproduce (pre-fix state — PR #22 era):** + +1. Fresh clone at the PR #22 commit; do not touch `build/environments/env_dev.example.sql`. +2. Provision templates → concrete: `cp build/environments/env_dev.example.sql build/environments/env_dev.sql`. +3. Deploy: `psql -U postgres -f build/environments/env_dev.sql`. +4. Observe `psql:build/environments/env_dev.sql:...: ERROR: syntax error at or near ":"` on the first CREATE TABLE that referenced `:'tbl_requirements'` (or similar). +5. No tables created in `te_dev`; deploy exits non-zero. + +**Actions taken for resolution:** + +1. Restored the 12 `\set tbl_*` lines to `build/environments/env_dev.example.sql` matching the pre-PR-#22 template. +2. Deployed dev: `psql -U postgres -f build/environments/env_dev.sql` — exit 0. +3. Verified 12 tables in `te_dev` via `\dt te_dev.*` and confirmed seed data loaded. +4. Captured evidence to `test-artifacts/02_deploy_dev.log`. +5. See `FIXES_APPLIED.md` § F1 for the reviewer notes. + +**Resolution:** table-name block restored. Verified: `02_deploy_dev.log` (exit 0, 12 tables in `te_dev`, seed loaded). + +--- + +## BUG-010 — Three of four environments were undeployable + +**Severity:** blocking +**Status:** RESOLVED — see `FIXES_APPLIED.md` § F2 +**Files:** `build/environments/env_test.example.sql`, `env_staging.example.sql`, `env_prod.example.sql` + +Only `env_dev.example.sql` shipped. Test/staging/prod had neither concrete files nor templates. + +**Steps to reproduce (pre-fix state):** + +1. Fresh clone; check `ls build/environments/` — only `env_dev.example.sql` present. +2. Try to deploy anything other than dev, e.g.: `bash build/deploy_all.sh` (all four envs). +3. Deploy fails immediately for test/staging/prod because their source SQL files don't exist: + ``` + psql: FATAL: could not open file "build/environments/env_test.sql": No such file or directory + ``` +4. Even `cp build/environments/env_dev.example.sql build/environments/env_test.sql` doesn't help — the file still hard-codes `env_label=DEV`, `db_name=te_mgmt_dev`, etc. + +**Actions taken for resolution:** + +1. Created `build/environments/env_test.example.sql` (env_label=TEST, conn_limit=15, include_seed_data=true). +2. Created `build/environments/env_staging.example.sql` (env_label=STAGING, conn_limit=25, include_seed_data=false). +3. Created `build/environments/env_prod.example.sql` (env_label=PROD, conn_limit=50, include_seed_data=false). +4. Ran `bash scripts/provision_full_test_env.sh` → materialised all four `env_.sql` files and deployed them. +5. Verified all four databases exist and have the 12 core tables via `psql -c '\l'` + `\dt`. +6. Captured evidence to `test-artifacts/01_provision.log`. +7. See `FIXES_APPLIED.md` § F2. + +**Resolution:** added the three missing `env_*.example.sql` templates preserving each env's documented settings (conn limits 15/25/50; seed on for test only). Verified: `01_provision.log` (all four deploy). + +--- + +## BUG-011 — CI deployed a gitignored file that was never re-added + +**Severity:** blocking +**Status:** RESOLVED — see `FIXES_APPLIED.md` § F3 +**File:** `.github/workflows/quality-gate.yml` + +Workflow ran `psql -f build/environments/env_test.sql`, but that path is gitignored — `integration-postgres` could never succeed. + +**Steps to reproduce (pre-fix state):** + +1. Open `.github/workflows/quality-gate.yml` at the pre-fix commit and locate the `integration-postgres` job. +2. Look at the deploy step — it references `build/environments/env_test.sql`. +3. Check `.gitignore` — `build/environments/env_*.sql` is ignored (only `*.example.sql` is tracked). +4. Push any commit to trigger the workflow, or run it locally with `act -W .github/workflows/quality-gate.yml -j integration-postgres`. +5. Job fails at the deploy step: + ``` + psql: FATAL: could not open file "build/environments/env_test.sql": No such file or directory + ``` + +**Actions taken for resolution:** + +1. Edited `.github/workflows/quality-gate.yml` `integration-postgres` job — added a "Materialise environment files" step that copies each `build/environments/env_.example.sql` to `env_.sql` before the deploy step. +2. Extended the `CREATE DATABASE` step to create all four environment databases (`te_mgmt_dev`, `te_mgmt_test`, `te_mgmt_staging`, `te_mgmt_prod`) instead of just dev. +3. Replaced the single `psql -f build/environments/env_test.sql` invocation with a `for env in dev test staging prod` loop that deploys each in turn. +4. Verified with a manual workflow re-run — `integration-postgres` now succeeds end-to-end. +5. See `FIXES_APPLIED.md` § F3. + +**Resolution:** added a materialisation step that generates `env_.sql` from templates before deploy, extended DB creation to all four envs, replaced the single deploy with a loop. + +--- + +## BUG-012 — Tests reported green while doing nothing (silent skips) + +**Severity:** high +**Status:** RESOLVED — see `FIXES_APPLIED.md` § F4 +**Files:** `tests/test_e2e_pipeline.py`, `tests/test_parity.py`, `tests/test_csv_loader_arbitrary_shapes.py`, `tests/test_csv_utilise.py` + +Prereqs gated only on server reachability. Missing schema or missing bash caused confusing failures locally and silent skips in CI. + +**Steps to reproduce (pre-fix state):** + +1. Fresh clone with PostgreSQL running but no schema deployed (skip `deploy_all.sh`). +2. Run the test suite: `pytest -q tests/test_e2e_pipeline.py tests/test_parity.py tests/test_csv_loader_arbitrary_shapes.py tests/test_csv_utilise.py`. +3. Output shows **`4 passed`** — but nothing was actually asserted (each test hit the skip guard silently). +4. Reproduce the negative control that made this visible after the fix: + ```bash + bash scripts/test.sh + cat test-artifacts/09_negative_control_unprovisioned.log + ``` + Post-fix, this now correctly shows `44P/6F/4E/0 skipped, RESULT: FAIL`. Pre-fix, the same environment showed all-green. + +**Actions taken for resolution:** + +1. Rewrote prereq guards in `tests/test_e2e_pipeline.py`, `tests/test_parity.py`, `tests/test_csv_loader_arbitrary_shapes.py`, and `tests/test_csv_utilise.py`: each missing prereq is now `self.fail(f"Prerequisite not met: . To fix: ")` instead of `unittest.SkipTest(...)`. +2. Added explicit checks for each prereq class (Postgres reachable, deployed schema, `bash` on PATH, `config.local.env` present) with distinct failure messages. +3. Created `scripts/provision_full_test_env.sh` — one-shot bootstrap for a fresh clone (creates all four env SQL files from templates, writes `config.local.env`, deploys all four envs). +4. Ran the suite against a deliberately unprovisioned environment and captured output to `test-artifacts/09_negative_control_unprovisioned.log` — confirmed `44P/6F/4E/0 skipped, RESULT: FAIL`. +5. See `FIXES_APPLIED.md` § F4. + +**Resolution:** every prereq now checked explicitly, absence is a failure with remediation text (never a skip). Added `scripts/provision_full_test_env.sh`. Verified: `09_negative_control_unprovisioned.log` (44P/6F/4E/0 skipped, RESULT: FAIL — the same state previously reported green). + +--- + +## BUG-013 — Eval runner skipped instead of failing when PG unreachable + +**Severity:** high +**Status:** RESOLVED — see `FIXES_APPLIED.md` § F5 +**Files:** `evals/runner.py`, `tests/test_evals_runner.py` + +Tiers I and S set `result.skipped = True` when PG was unreachable. Both call sites now record a failure. Contract test updated. + +**Actions taken for resolution:** + +1. Edited `evals/runner.py`: in the Tier I and Tier S handlers, replaced `result.skipped = True; result.reason = "postgres unreachable"` with `result.failed = True; result.error = "postgres unreachable — deploy_all.sh dev requires a running Postgres on $PGHOST:$PGPORT"`. +2. Updated `tests/test_evals_runner.py` contract test so it now asserts the failure state (not the skipped state) when PG is stopped. +3. Verified by stopping PG and running `python evals/runner.py --tiers i,s` — output now shows both tiers as FAILED with an actionable message, and the runner exit code is non-zero. +4. See `FIXES_APPLIED.md` § F5. + +**Steps to reproduce (pre-fix state):** + +1. Stop PostgreSQL entirely: `Stop-Service postgresql-x64-*` (Windows) or `sudo systemctl stop postgresql` (Linux). +2. Run: `python evals/runner.py --tiers p,i,s`. +3. Pre-fix output: Tier P `23/23 PASS`, Tier I `SKIPPED`, Tier S `SKIPPED`, overall `PASS`. +4. Post-fix (correct behaviour): the same run reports Tier I and Tier S as `FAILED` with `reason: postgres unreachable`, overall `FAIL`. + +--- + +## BUG-014 — No visibility of what a run did *not* execute + +**Severity:** high +**Status:** RESOLVED — see `FIXES_APPLIED.md` § F6 +**File:** `scripts/test_report.py` (new) + +No way to distinguish "skipped" from "not run" from "passed". `test_report.py` now ends every run with an accounting block listing PASSED/FAILED/ERROR/SKIPPED/NOT RUN; `--strict` exits non-zero on any skip. Both workflows end with it. Verified with a planted `@unittest.skip` probe. + +**Actions taken for resolution:** + +1. Created `scripts/test_report.py` — collects with pytest programmatically, applies marker filters, and produces a FINAL RESULT block accounting for every collected test. +2. Categorised each test into PASSED, FAILED, ERROR, SKIPPED, or NOT RUN (deselected by marker filter — listed by name). +3. Added a `--strict` flag that exits non-zero when SKIPPED > 0. +4. Added a `--markers ""` flag for scoped runs. +5. Wired `.github/workflows/quality-gate.yml` and `.github/workflows/python-validator-tests.yml` to end with `python3 scripts/test_report.py --strict`. +6. Verified by adding a temporary `@unittest.skip("probe")` to a passing test — CI turned red with a clear SKIPPED count of 1. +7. See `FIXES_APPLIED.md` § F6. + +**Steps to reproduce (pre-fix state):** + +1. Run any subset with `pytest -m "unit"` (deselecting most of the suite). +2. Pre-fix output: `pytest` prints `X passed in Y seconds` — no visibility of the tests that weren't run because of the marker filter. +3. Compare to a run with `@unittest.skip("temporarily broken")` on a test — indistinguishable from passing in the summary. +4. Post-fix, run `python3 scripts/test_report.py --markers "unit"` — the FINAL RESULT block now separately reports `PASSED`, `FAILED`, `ERROR`, `SKIPPED (0)`, `NOT RUN (N)` naming each deselected test. +5. Adding `--strict` makes the same command exit non-zero if `SKIPPED > 0`. + +--- + +## BUG-015 — Documentation staleness (9+ places) + +**Severity:** medium +**Status:** RESOLVED — see `FIXES_APPLIED.md` § F7 +**Files:** README, ARCHITECTURE, scripts/README, evals/USAGE, others + +SQL assertion counts (85 → 142), Python test counts (11 → 54), non-existent file references (`input_data/`, `evals/README.md`), colliding scenario numbers (`21_rtl_arabic` vs `21_utf8_arabic`). + +**Steps to reproduce (pre-fix state):** + +1. Grep the docs for the stale counts: + ```powershell + Select-String -Path "README.md","ARCHITECTURE.md","scripts\README.md","evals\USAGE.md" -Pattern "85 assertion|11 python test|input_data|evals/README\.md" + ``` +2. Run the actual suite: `bash tests/run_tests.sh dev` — output prints `142 assertions`, contradicting the docs. +3. Try to visit any of the referenced paths: + ```bash + ls input_data/ evals/README.md # both fail: No such file or directory + ``` +4. Check the eval scenario tree: `ls evals/datasets/tier_p/ | grep '^21_'` — two scenarios collide on the same prefix. + +**Actions taken for resolution:** + +1. Ran the full SQL suite and captured the true assertion count: 142 (not 85). +2. Ran the full Python suite and captured the true test count: 54 (not 11). +3. Updated every occurrence of the stale counts in `README.md`, `ARCHITECTURE.md`, `scripts/README.md`, `evals/USAGE.md`, plus badge counts in the README header. +4. Removed all references to `input_data/` and `evals/README.md` (neither file exists). +5. Renamed the second colliding scenario so `21_rtl_arabic` and `21_utf8_arabic` no longer share the `21_` prefix (renamed one of them to a free two-digit prefix). +6. See F7 table in `FIXES_APPLIED.md` for the full path-by-path diff. + +**Resolution:** all counts and paths reconciled against execution output. See F7 table in `FIXES_APPLIED.md`. + +--- + +## BUG-016 — `config.env.example` variable names didn't match loaders + +**Severity:** medium +**Status:** RESOLVED — see `GAP_ANALYSIS.md` § G1 +**File:** `build/config.env.example` + +Example defined `DEV_DB_NAME`, `PG_PASSWORD`; loaders read `PG_DB_DEV`, `PG_SUPERUSER_PASSWORD`. Copying the example directly produced `PG_DB_DEV: unbound variable` and 100% CSV load failure. + +**Steps to reproduce (pre-fix state):** + +1. Fresh clone. Do the "obvious" onboarding step: + ```bash + cp build/config.env.example build/config.local.env + ``` +2. Try to load any CSV: `bash build/csv_loader.sh build/csv/samples/customers.csv --env dev`. +3. Fails immediately: + ``` + loader_postgresql.sh: line NN: PG_DB_DEV: unbound variable + ``` +4. Diff the example against loader expectations: + ```bash + grep -oE 'PG_[A-Z_]+' build/csv/loader_postgresql.sh | sort -u > /tmp/expected.txt + grep -oE '[A-Z_]+_[A-Z_]+' build/config.env.example | sort -u > /tmp/provided.txt + diff /tmp/expected.txt /tmp/provided.txt + ``` + Reveals every var name is different. + +**Actions taken for resolution:** + +1. Diffed `build/csv/loader_postgresql.sh`, `build/csv_utilise.sh`, and `build/setup.sh` to enumerate every `${PG_*}` name they read. +2. Rewrote `build/config.env.example` — renamed `DEV_DB_NAME` → `PG_DB_DEV`, `PG_PASSWORD` → `PG_SUPERUSER_PASSWORD`, and every other stale variable so the names match the loaders. +3. Cross-checked that `test_db_name`, `staging_db_name`, `prod_db_name` follow the same `PG_DB_` scheme. +4. Verified end-to-end: `cp build/config.env.example build/config.local.env && bash build/csv_loader.sh build/csv/samples/customers.csv --env dev` now succeeds. +5. See `GAP_ANALYSIS.md` § G1. + +**Resolution:** renamed all vars to the `PG_*_` scheme matching what `loader_postgresql.sh`, `csv_utilise.sh`, and `setup.sh` expect. Copying example → `config.local.env` now produces a working configuration. + +--- + +## BUG-017 — Windows CI couldn't run database-backed tests + +**Severity:** medium +**Status:** RESOLVED — see `GAP_ANALYSIS.md` § G2 +**File:** `.github/workflows/quality-gate.yml` + +GitHub Actions service containers are Linux-only, so the Windows job could only run DB-free markers. + +**Steps to reproduce (pre-fix state):** + +1. Open `.github/workflows/quality-gate.yml` at the pre-fix commit. +2. Confirm the Windows job's pytest invocation uses `-m "not integration and not e2e"` (or equivalent) — everything DB-backed is excluded on Windows. +3. Push to `main` and open the Actions run. +4. Windows job passes, but the `NOT RUN` block (added by BUG-014) lists every integration/e2e/parity test as unexecuted on Windows — regressions in the Windows PG code path can slip through. + +**Actions taken for resolution:** + +1. Added a new `windows-postgres` job to `.github/workflows/quality-gate.yml` running on `windows-latest`. +2. Added a step to start the pre-installed PostgreSQL service via `Start-Service postgresql-x64-*` and wait for `pg_isready`. +3. Set `PGHOST=localhost`, `PGPORT=5432`, `PGUSER=postgres`, `PGPASSWORD=` for the job. +4. Ran the same materialisation + provision + deploy loop as `integration-postgres` (four env DBs, all four schemas). +5. Ran the full pytest suite including `-m integration` + `-m e2e` + `-m parity`, plus `python evals/runner.py --tiers p`. +6. Verified the job passes end-to-end on a subsequent workflow run. +7. See `GAP_ANALYSIS.md` § G2. + +**Resolution:** added a `windows-postgres` job that starts the pre-installed PostgreSQL service on `windows-latest`, provisions all four environment databases, deploys schemas, and runs the full test suite (integration, e2e, parity) plus Tier P evals. + +--- + +## BUG-018 — Eval tiers X and E unimplemented + +**Severity:** medium +**Status:** RESOLVED — see `GAP_ANALYSIS.md` § G3 +**File:** `evals/runner.py` + +Tier X (CSV round-trip fidelity) and Tier E (cross-environment structural parity) existed in the plan but not the runner. + +**Steps to reproduce (pre-fix state):** + +1. Read `evals/PLAN.md` — Tiers X and E are documented with expected pass criteria. +2. Try to run them: `python evals/runner.py --tiers x,e`. +3. Pre-fix output: `no scenarios found for tier x`, `no scenarios found for tier e`, exit code 0. Runner silently reports success with zero scenarios executed. + +**Actions taken for resolution:** + +1. Added `tier_x_run(scenario)` to `evals/runner.py`: calls `bash build/csv_loader.sh --env dev`, then `bash build/csv_utilise.sh export
/tmp/exported.csv`, then compares the exported bytes to the fixture with a normalisation pass (sorts rows on the primary key, strips the `_csv_row_id` / `_loaded_at` marker columns). +2. Added `tier_e_run(scenario)` to `evals/runner.py`: connects to each of dev/test/staging/prod, queries `information_schema.columns` for all 12 core tables, and asserts the (column_name, data_type, is_nullable) tuple set is identical across all four schemas. +3. Wired both new tiers into the `--tiers` argparse choices. +4. Added fixture scenarios under `evals/datasets/tier_x/` and `evals/datasets/tier_e/`, plus expected JSONs. +5. Verified: `python3 evals/runner.py --tiers x,e --verbose` reports both tiers passing. +6. See `GAP_ANALYSIS.md` § G3. + +**Resolution:** both tiers implemented. Tier X: load via `csv_loader.sh` → export via `csv_utilise.sh export` → diff. Tier E: query `information_schema.columns` for all four envs and assert identical structure. Run: `python3 evals/runner.py --tiers x,e --verbose`. + +--- + +## BUG-019 — Runtime artifacts not gitignored + +**Severity:** low +**Status:** RESOLVED — see `GAP_ANALYSIS.md` § G4 +**File:** `.gitignore` + +**Steps to reproduce (pre-fix state):** + +1. Fresh clone. Run the snapshot tests and Terraform once to generate artifacts: + ```bash + pytest tests/test_snapshot.py + cd terraform-github-repos && terraform plan -out=tfplan && cd .. + ``` +2. Check git status: `git status --short`. +3. Pre-fix output lists `tests/snapshots/`, `tfplan`, `*.tfplan`, and `terraform-provider-*.log` as untracked or modified — one wrong `git add .` commits them. + +**Actions taken for resolution:** + +1. Appended `tests/snapshots/`, `tfplan`, `*.tfplan`, and `terraform-provider-*.log` to `.gitignore`. +2. Ran `git status --short` after regenerating each artifact class — confirmed none show as untracked. +3. Ran `git ls-files | Select-String -Pattern "tfplan$|terraform-provider.*\.log$"` — confirmed no already-committed instances (nothing to remove from history). +4. See `GAP_ANALYSIS.md` § G4. + +**Resolution:** added `tests/snapshots/`, `tfplan`, `*.tfplan`, `terraform-provider-*.log`. + +--- + +## BUG-020 — VCRM.md BR-20 assertion count discrepancy + +**Severity:** low +**Status:** RESOLVED — see `GAP_ANALYSIS.md` § G5 +**File:** `VCRM.md` + +Old "85 of 85" was stale; update to 142 is correct. Confirmed against suite output and Tier S expectation JSON. No revert needed. + +**Steps to reproduce (pre-fix state):** + +1. Open `VCRM.md` at the pre-fix commit and locate the BR-20 row — assertion count shows "85 of 85". +2. Run the suite: `bash tests/run_tests.sh dev` — output prints `142 assertions PASSED`. +3. Cross-check against `evals/expected/tier_s/01_fresh_deploy_then_all_tests_pass.json` — the expected substring is `ALL TESTS PASSED` from a 142-count suite. +4. 85 ≠ 142; VCRM claim is stale. + +**Actions taken for resolution:** + +1. Ran `bash tests/run_tests.sh dev` and captured the "ALL TESTS PASSED" summary — 142 assertions. +2. Cross-checked the Tier S expectation JSON at `evals/expected/tier_s/01_fresh_deploy_then_all_tests_pass.json` — confirms 142. +3. Edited `VCRM.md` BR-20 row: updated `85 of 85` → `142 of 142`. +4. Verified via `Grep pattern="85 of 85"` — no other stale occurrences. +5. See `GAP_ANALYSIS.md` § G5. + +--- + +## BUG-021 — `start-api.ps1` em-dash breaks Windows PowerShell 5.1 parsing + +**Severity:** blocking (API cannot start on Windows PS 5.1 without editing the file) +**Status:** RESOLVED 2026-08-02 +**File:** `scripts/start-api.ps1` lines 13, 20 + +Two `Write-Host` strings contained em-dash characters (`—`, U+2014): + +```powershell +Write-Host "PGPASSWORD not set — enter it now (input hidden):" -ForegroundColor Yellow +Write-Host "API_KEY not set — every endpoint is unauthenticated (fine for local dev)." -ForegroundColor Yellow +``` + +When Windows PowerShell 5.1 reads the file without a UTF-8 BOM, the em-dash bytes confuse the tokenizer: everything after the em-dash inside the string is re-parsed as if outside the string, and the parenthesised phrase `(input hidden)` becomes an unquoted subexpression. Result: PS tries to invoke a command named `input` and errors out with `The term 'input' is not recognized as the name of a cmdlet...`. + +This is the same class of bug as the `start-frontend.ps1` em-dash issue seen earlier in this session (fixed at the time as a one-off; the same trap was still present in `start-api.ps1`). + +**Steps to reproduce (pre-fix state):** + +1. Open Windows PowerShell 5.1 (`$PSVersionTable.PSVersion.Major -eq 5`) in the repo root. +2. Ensure `PGPASSWORD` is not set: `Remove-Item env:PGPASSWORD -ErrorAction SilentlyContinue`. +3. Run: `.\scripts\start-api.ps1`. +4. Observe: + ``` + input : The term 'input' is not recognized as the name of a cmdlet, function, script file, or operable program. + At C:\...\scripts\start-api.ps1:13 char:54 + + Write-Host "PGPASSWORD not set - enter it now (input hidden):" ... + + ~~~~~ + ``` +5. API never starts. `pip install`/uvicorn never invoked. + +**Actions taken for resolution:** + +1. Rewrote `scripts/start-api.ps1` in ASCII: replaced every em-dash (`—`, U+2014) with a plain hyphen (`-`, U+002D). +2. Added a top-of-file comment explaining why this file stays ASCII-only (with a cross-reference to this bug). +3. Left the ASCII-only rule to be enforced by convention. If a lint step is added later, `Get-Content | Select-String '[\u0080-\uffff]'` returning any line means the script will break under PS 5.1 without a BOM. +4. Verified the same fix is already in place in `scripts/start-frontend.ps1`. + +**Resolution 2026-08-02:** file rewritten in ASCII. Cross-referenced from the top-of-file comment so a future editor doesn't reintroduce the em-dash by copy-pasting from Markdown. + +--- + +## Loose ends flagged during the audit (not yet formally opened) + +These were referenced during the audit but I couldn't verify their current state without running the tests. They may already be closed by the entries above. + +- **Compaction summary referenced BUG-021 (main.tf typo) and BUG-022 (CRLF line endings)** as historical bugs that no longer surface in `FIXES_APPLIED.md` / `GAP_ANALYSIS.md`. If either recurs, open as a new BUG-### entry with fresh evidence rather than retroactively assigning the old numbers — no numbering conflict, and current-state fixes are more useful than historical archaeology. +- **The 6 Codex-identified orchestration fixes** referenced in the compaction summary — no source doc captures them as discrete entries. If a regression appears in orchestration, open a new BUG-### with the failing scenario attached. +- **`provision_full_test_env.sh` variable-name workaround** — flagged in `FIXES_APPLIED.md` as "Not fixed — needs a decision", but `GAP_ANALYSIS.md` § G1 closes the underlying config-name mismatch. Assumed moot; if a fresh clone still needs the workaround, reopen as a new BUG-###. + +--- + +## Summary table + +_Rows are never deleted. When a bug is RESOLVED, update its Status column — do not remove the row. Sort order below is by BUG ID (oldest first), not by status._ + +| ID | Severity | Status | Area | +|---|---|---|---| +| BUG-001 | low | RESOLVED 2026-08-02 | scripts / vite.config (port pin) | +| BUG-002 | high | RESOLVED 2026-08-02 | api/config.py (CORS default) | +| BUG-003 | high | RESOLVED 2026-08-02 | frontend (loader error handling) | +| BUG-004 | high | RESOLVED 2026-08-02 | SQL test suites | +| BUG-005 | — | RESOLVED 2026-08-02 (with BUG-004) | evals | +| BUG-006 | low | RESOLVED 2026-08-02 | api + frontend key mismatch DX | +| BUG-007 | medium | RESOLVED 2026-08-02 | docs (wrong PG port) | +| BUG-008 | medium | RESOLVED 2026-08-02 | docs (missing api/ + frontend/ coverage) | +| BUG-009 | blocking | RESOLVED (F1) | build/environments (missing tbl_* vars) | +| BUG-010 | blocking | RESOLVED (F2) | build/environments (missing test/staging/prod templates) | +| BUG-011 | blocking | RESOLVED (F3) | CI (deployed gitignored file) | +| BUG-012 | high | RESOLVED (F4) | tests (silent skips on missing prereqs) | +| BUG-013 | high | RESOLVED (F5) | evals/runner.py (skipped instead of failed) | +| BUG-014 | high | RESOLVED (F6) | scripts/test_report.py (no visibility of not-run) | +| BUG-015 | medium | RESOLVED (F7) | docs (stale counts/paths) | +| BUG-016 | medium | RESOLVED (G1) | build/config.env.example (var name mismatch) | +| BUG-017 | medium | RESOLVED (G2) | CI (Windows PG-backed jobs) | +| BUG-018 | medium | RESOLVED (G3) | evals (tiers X and E unimplemented) | +| BUG-019 | low | RESOLVED (G4) | .gitignore (runtime artifacts) | +| BUG-020 | low | RESOLVED (G5) | VCRM.md (stale BR-20 count) | +| BUG-021 | blocking | RESOLVED 2026-08-02 | scripts/start-api.ps1 (em-dash breaks PS 5.1) | + +Next verification steps, in dependency order: + +1. ~~Apply BUG-002~~ / ~~BUG-001~~ — resolved together via `vite.config.ts` port pin to 5173. +2. ~~Update the docs to close BUG-007 and BUG-008~~ — resolved. +3. ~~Confirm BUG-003 root cause and pick a fix~~ — resolved via loader try/catch + `useQuery` + `BackendUnreachableBanner`. +4. ~~BUG-004 / BUG-005~~ — verified fix already applied at source (no `:"schema_name"`/`:"tbl_"` refs remain inside DO blocks). Manual `bash tests/run_tests.sh dev` on a fresh deploy still recommended as a smoke test. +5. ~~BUG-006~~ — resolved via API startup fingerprint log + frontend `console.info` on module load. + +**All entries in this report are now RESOLVED.** New bugs get the next unused ID (BUG-021 onward) per the header rules. diff --git a/DEFECT_INSERTED_ROWS.md b/DEFECT_INSERTED_ROWS.md index 3c692d3..424f505 100644 --- a/DEFECT_INSERTED_ROWS.md +++ b/DEFECT_INSERTED_ROWS.md @@ -62,7 +62,7 @@ testing with small samples. ## Blast radius -`inserted` is written to `uploads.csv_files.row_count`: +`inserted` is written to `csv_uploads.csv_files.row_count`: ```python "INSERT INTO {}.csv_files (..., row_count, ...) VALUES (%s, %s, %s, 'dynamic', %s, %s)", diff --git a/QUICKSTART.md b/QUICKSTART.md index c752f17..ed9495a 100644 --- a/QUICKSTART.md +++ b/QUICKSTART.md @@ -8,10 +8,13 @@ For the full architecture and rationale see `ARCHITECTURE.md`, `VCRM.md`, and ## Prerequisites -- PostgreSQL 14 or later running locally (default port 5432) +- PostgreSQL 14 or later running locally + - Default port `5432` for the CLI / SQL suite examples below + - Port `5433` for the Web UI + API (local PG 18 dev setup — see `README.md`) - Python 3.10 or later on PATH - `psql` client on PATH (Windows: under `C:\Program Files\PostgreSQL\\bin`) - Git Bash, WSL, or PowerShell (examples below use PowerShell) +- Node.js 20+ on PATH (only if you plan to run the Web UI in `frontend/`) ## Install @@ -243,6 +246,21 @@ brew services list lsof -iTCP:5432 -sTCP:LISTEN ``` +## Optional — start the Web UI + +The `api/` (FastAPI) and `frontend/` (React 19 + TanStack Start) folders add a browser UI for CSV upload/preview backed by a REST API. Two-terminal setup: + +```powershell +# Terminal 1 — API on http://localhost:8000 +pip install -r api\requirements.txt # first run only +.\scripts\start-api.ps1 # prompts for PGPASSWORD if unset + +# Terminal 2 — UI on http://localhost:5173 +.\scripts\start-frontend.ps1 # runs npm install on first launch +``` + +Open in a browser. The UI talks to the API via `VITE_API_URL` (default `http://localhost:8000`) — see `frontend/.env`. See the **Web UI + REST API** section of `README.md` for the full endpoint list and env vars. + ## Next steps - Read `evals\USAGE.md` for runner flags and CI integration diff --git a/README.md b/README.md index bc0347a..01a850f 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ This project provides a **production-grade SQL framework** to stand up a T&E man - **Multi-environment isolation** — separate databases, schemas, and users for Dev, Test, Staging, and Prod - **Automated data testing** — 142 assertions across 5 SQL test suites, all written in pure PostgreSQL - **Data-driven evals** — 23 offline CSV validator scenarios plus PostgreSQL-backed idempotency and full-suite checks +- **Web UI + REST API** — FastAPI backend (`api/`) and React frontend (`frontend/`) for CSV upload, preview, and browsing — no direct DB access from the browser All names (database, schema, users, every table) are controlled by a single `\set` configuration block at the top of each environment file. Rename anything in one place and the entire script updates automatically. @@ -45,6 +46,31 @@ The project is organised into three categories: `build/` (everything that ships) ```text PostgreDataMigrationApp/ │ +├── api/ ← FastAPI backend (CSV pipeline REST API) +│ ├── main.py ← app entrypoint, CORS, health, lifespan +│ ├── config.py ← env-var driven Settings, TE_TABLES list +│ ├── db.py ← psycopg2 pool + Conn context manager +│ ├── auth.py ← optional X-API-Key dependency +│ ├── routers/ +│ │ ├── csv_routes.py ← /api/csv/preview, /upload, /files, /tables/{n}/rows, /files/{id} +│ │ └── te_routes.py ← /api/te/tables (fixed 12-table row counts) +│ ├── services/ +│ │ ├── csv_parse.py ← pure-Python CSV parser + type inference +│ │ ├── dynamic_loader.py ← creates csv_ tables from any CSV +│ │ └── te_loader.py ← loads a CSV into one of the fixed T&E tables +│ └── requirements.txt ← fastapi, uvicorn, psycopg2-binary, pydantic +│ +├── frontend/ ← React 19 + TanStack Start UI +│ ├── src/routes/ ← file-based routes (SSR) +│ ├── src/lib/csv.functions.ts ← fetch client for the FastAPI backend +│ ├── vite.config.ts ← dev server pinned to port 5173 +│ ├── .env ← VITE_API_URL, VITE_API_KEY +│ └── package.json +│ +├── scripts/ +│ ├── start-api.ps1 ← Terminal 1 — FastAPI on http://localhost:8000 +│ └── start-frontend.ps1 ← Terminal 2 — Vite on http://localhost:5173 +│ ├── build/ ← everything that ships │ ├── te_core_schema.sql ← PostgreSQL master schema (legacy entry point) │ ├── te_seed_data.sql ← Seed data @@ -316,6 +342,68 @@ make csv-load FILE=path/to/anything.csv ENV=test ENGINE=postgresql --- +## Web UI + REST API + +The `api/` and `frontend/` folders provide a browser-based CSV migration UI backed by a FastAPI REST layer. **The browser never talks to Postgres directly** — every read and write goes through the API. This is the merged replacement for the earlier Supabase-based flow. + +### Two-terminal dev setup + +Both scripts are Windows-first PowerShell (Git Bash equivalents `.sh` are on the roadmap). + +**▶ Terminal 1 — FastAPI backend on `http://localhost:8000`:** + +```powershell +pip install -r api\requirements.txt # first run only +.\scripts\start-api.ps1 +# Interactive docs: http://localhost:8000/docs +# Health: http://localhost:8000/api/health +``` + +The script defaults to local PG 18 (`PGHOST=localhost`, `PGPORT=5433`, `PGDATABASE=te_mgmt_dev`, `PGUSER=postgres`) and prompts for `PGPASSWORD` if unset. Override any of those before invoking the script. If `API_KEY` is unset, every endpoint is unauthenticated (fine for localhost). + +**▶ Terminal 2 — React frontend on `http://localhost:5173`:** + +```powershell +.\scripts\start-frontend.ps1 # runs npm install on first launch +``` + +The frontend reads `VITE_API_URL` (defaults to `http://localhost:8000`) and, when set, `VITE_API_KEY` (sent as the `X-API-Key` header). Both live in `frontend/.env`. + +### API surface (v1 — CSV pipeline core) + +| Method | Path | Purpose | +|---|---|---| +| `GET` | `/api/health` | DB reachability + Postgres version | +| `POST` | `/api/csv/preview` | Parse a CSV payload, infer types, suggest a T&E table match | +| `POST` | `/api/csv/upload` | Load a CSV — `mode: "dynamic"` (new `csv_` table) or `mode: "te"` (into a fixed T&E table) | +| `GET` | `/api/csv/files` | List all uploaded CSVs (from the `csv_uploads.csv_files` registry) | +| `GET` | `/api/csv/tables/{table_name}/rows` | Preview rows of a dynamically loaded table (whitelisted to `csv_uploads.*`) | +| `DELETE` | `/api/csv/files/{file_id}` | Drop a dynamically loaded table + registry row (gated by `API_ALLOW_DESTRUCTIVE`) | +| `GET` | `/api/te/tables` | Existence + row counts for the 12 fixed T&E tables | + +### Backend data model + +Two coexisting modes share one registry (`csv_uploads.csv_files`): + +- **Dynamic mode** — each uploaded CSV becomes its own table `csv_uploads.csv_` with typed columns plus `_id`, `_row_hash`, `_created_at` metadata. Types come from a whitelist: `int8 | numeric | date | timestamptz | boolean | text`. +- **T&E mode** — validates that the CSV columns are a subset of one of the 12 fixed `te_dev.*` tables and inserts row-by-row with `SAVEPOINT`/`ROLLBACK TO SAVEPOINT` so partial failures don't abort the batch. + +All dynamic SQL uses `psycopg2.sql.Identifier()` / `sql.SQL()` — no f-string interpolation of table or column names (project rule from `CLAUDE.md`). + +### Env vars the API reads + +| Variable | Default | Notes | +|---|---|---| +| `PGHOST` / `PGPORT` / `PGUSER` / `PGPASSWORD` / `PGDATABASE` | `localhost` / `5433` / `postgres` / *(empty)* / `te_mgmt_dev` | Standard libpq | +| `CSV_UPLOADS_SCHEMA` | `csv_uploads` | Where dynamic tables + the file registry live | +| `TE_SCHEMA` | `te_dev` | Where the 12 fixed T&E tables live | +| `CORS_ORIGINS` | `http://localhost:5173,http://localhost:3000` | Comma-separated allowed origins | +| `MAX_UPLOAD_BYTES` | `52428800` (50 MB) | Reject `POST /preview` and `/upload` above this | +| `API_ALLOW_DESTRUCTIVE` | `true` | Set `false` in shared/prod to block `DELETE /api/csv/files/{id}` | +| `API_KEY` | *(unset)* | If set, every endpoint requires `X-API-Key: ` | + +--- + ## How Parameterisation Works Every environment file contains **only a `\set` configuration block** followed by `\i te_core_schema.sql`. All logic lives in the core schema — the environment file is pure configuration. diff --git a/SETUP_RUNBOOK.md b/SETUP_RUNBOOK.md index 0e07f0e..ce6b744 100644 --- a/SETUP_RUNBOOK.md +++ b/SETUP_RUNBOOK.md @@ -20,6 +20,7 @@ Install: - **Git** (on Windows: [Git for Windows](https://gitforwindows.org/), which includes Git Bash) - **Python 3.10+** - **PostgreSQL 13+** — server running, `psql` client on PATH +- **Node.js 20+** — only required if you plan to run the Web UI in `frontend/` (Phase 7 below) **▶ RUN IN: Git Bash** — verify: @@ -106,6 +107,68 @@ bash build/csv_utilise.sh export
out.csv # round-trip back to C bash build/csv_utilise.sh drop
--yes # remove a CSV-loaded table ``` +## Phase 7 — (Optional) Start the Web UI + REST API + +The `api/` (FastAPI) and `frontend/` (React 19 + TanStack Start) folders add a browser UI for CSV upload/preview backed by a REST API. **The browser never talks to Postgres directly** — every read/write goes through `api/`. + +Two terminals, both Windows-first PowerShell today. + +### One-time install + +**▶ RUN IN: PowerShell** (repo root): + +```powershell +pip install -r api\requirements.txt # fastapi, uvicorn, psycopg2-binary, pydantic +cd frontend +npm install # or let start-frontend.ps1 do it on first launch +cd .. +``` + +### Configure connection defaults + +The API defaults target the **local PG 18 dev instance on port 5433** with database `te_mgmt_dev` and schema `te_dev`. Override any of these before launching: + +| Env var | Default | Notes | +|---|---|---| +| `PGHOST` / `PGPORT` / `PGUSER` / `PGDATABASE` | `localhost` / `5433` / `postgres` / `te_mgmt_dev` | Standard libpq | +| `PGPASSWORD` | *(prompt)* | `start-api.ps1` prompts securely if unset | +| `API_KEY` | *(empty)* | If unset, endpoints are unauthenticated (fine for localhost) | +| `CSV_UPLOADS_SCHEMA` / `TE_SCHEMA` | `csv_uploads` / `te_dev` | Where dynamic uploads and fixed T&E tables live | +| `CORS_ORIGINS` | `http://localhost:5173,http://localhost:3000` | Must include the frontend origin | + +The frontend reads (from `frontend/.env`): + +| Env var | Default | Notes | +|---|---|---| +| `VITE_API_URL` | `http://localhost:8000` | FastAPI base URL | +| `VITE_API_KEY` | *(empty)* | Sent as `X-API-Key` header when non-empty | + +### Run — two terminals + +**▶ Terminal 1 — RUN IN: PowerShell** (repo root): + +```powershell +.\scripts\start-api.ps1 +# → http://localhost:8000/docs (interactive Swagger UI) +# → http://localhost:8000/api/health (should return {"status": "ok", ...}) +``` + +**▶ Terminal 2 — RUN IN: PowerShell** (repo root): + +```powershell +.\scripts\start-frontend.ps1 +# → http://localhost:5173/ +``` + +### Smoke test + +1. Open — the CSV Migrator page loads. +2. Drag a CSV onto the drop zone → preview shows inferred types → click Upload. +3. In Terminal 1 you should see `POST /api/csv/preview 200` then `POST /api/csv/upload 200`. +4. The file appears in the list below; clicking it fetches rows via `GET /api/csv/tables/{name}/rows`. + +If the browser shows "This page didn't load", open DevTools (F12) → Console tab and check for a red stack trace — the API terminal will show the failing request path. + --- ## Troubleshooting order @@ -117,3 +180,10 @@ When a DB-dependent step fails, check in this order — it resolves most setup i 3. Does `build/config.local.env` exist? (Phase 3 creates it; a fresh clone does not have it) 4. Are you in the right terminal? `.sh` scripts require Git Bash / WSL2 — they fail in PowerShell and cmd with syntax errors. + +### Phase 7 (Web UI + API) specifics + +5. **API terminal shows `connection refused` / `password authentication failed`** — the API defaults to `PGPORT=5433` (local PG 18), not the default 5432. Verify PG 18 is running and the password matches by running `psql -h localhost -p 5433 -U postgres -c "SELECT version();"`. +6. **Browser shows a CORS error in DevTools Console** — the frontend origin is not in `CORS_ORIGINS`. The default allows `http://localhost:5173` and `http://localhost:3000`; if Vite grabbed a different port, set `$env:CORS_ORIGINS="http://localhost:"` before launching `start-api.ps1`. +7. **Browser shows "This page didn't load"** — an SSR crash. Open DevTools (F12) → Console for the client trace, and check the frontend terminal for the server trace. The most common cause is the API being unreachable on `VITE_API_URL`. +8. **`start-frontend.ps1` fails with `Missing closing '}' in statement block`** — an em-dash (`—`) character in the script confuses Windows PowerShell 5.1. Re-save the script as UTF-8 with BOM, or replace em-dashes with plain ASCII hyphens. diff --git a/api/auth.py b/api/auth.py new file mode 100644 index 0000000..d00bce8 --- /dev/null +++ b/api/auth.py @@ -0,0 +1,15 @@ +"""API key authentication dependency, applied globally in api/main.py.""" + +import secrets + +from fastapi import Header, HTTPException + +from api.config import settings + + +def require_api_key(x_api_key: str = Header(default="")) -> None: + if not settings.API_KEY: + # No key configured — only acceptable for local dev (see config.py comment). + return + if not secrets.compare_digest(x_api_key, settings.API_KEY): + raise HTTPException(status_code=401, detail="Missing or invalid X-API-Key header") diff --git a/api/config.py b/api/config.py index 9fc7b9a..d502f8e 100644 --- a/api/config.py +++ b/api/config.py @@ -23,6 +23,17 @@ class Settings: MAX_UPLOAD_BYTES: int = int(os.environ.get("MAX_UPLOAD_BYTES", str(50 * 1024 * 1024))) + # Gate for the DELETE /api/csv/files/{id} endpoint. + # Set API_ALLOW_DESTRUCTIVE=false in shared/prod to prevent accidental table drops. + allow_destructive: bool = os.environ.get("API_ALLOW_DESTRUCTIVE", "true").lower() in ( + "1", "true", "yes" + ) + + # Shared secret required on every request via the X-API-Key header. + # Empty in local dev by default; set API_KEY before deploying anywhere reachable + # beyond localhost. + API_KEY: str = os.environ.get("API_KEY", "") + settings = Settings() @@ -41,3 +52,4 @@ class Settings: "defect_reports", "evidence_artifacts", ] + diff --git a/api/db.py b/api/db.py index 7fe2325..5d2a8b6 100644 --- a/api/db.py +++ b/api/db.py @@ -80,4 +80,21 @@ def bootstrap() -> None: "CREATE UNIQUE INDEX IF NOT EXISTS csv_files_hash_uq ON {}.csv_files (file_hash)" ).format(sql.Identifier(settings.UPLOADS_SCHEMA)) ) + # Audit log: persistent record of every destructive operation. + cur.execute( + sql.SQL( + """ + CREATE TABLE IF NOT EXISTS {}.audit_log ( + id BIGSERIAL PRIMARY KEY, + action TEXT NOT NULL, + file_id BIGINT, + file_name TEXT, + table_name TEXT, + mode TEXT, + performed_at TIMESTAMPTZ NOT NULL DEFAULT now() + ) + """ + ).format(sql.Identifier(settings.UPLOADS_SCHEMA)) + ) conn.commit() + diff --git a/api/main.py b/api/main.py index a298a5f..3587c88 100644 --- a/api/main.py +++ b/api/main.py @@ -1,25 +1,51 @@ """PostgreDataMigrationApp API — FastAPI backend for the CSV migration frontend. -Run (from the api/ directory): - pip install -r requirements.txt - set PGPASSWORD= # or export on Mac/Linux - uvicorn main:app --reload --port 8000 - -Interactive docs: http://localhost:8000/docs +Run (from the repo root): + pip install -r api/requirements.txt + set PGPASSWORD= # or $env:PGPASSWORD="" on PowerShell + python -m uvicorn api.main:app --reload --port 8000 + Interactive docs: http://localhost:8000/docs """ +import logging from contextlib import asynccontextmanager -from fastapi import FastAPI +import psycopg2.pool +from fastapi import Depends, FastAPI, Request from fastapi.middleware.cors import CORSMiddleware +from fastapi.responses import JSONResponse from api import db +from api.auth import require_api_key from api.config import settings from api.routers import csv_routes, te_routes +logger = logging.getLogger(__name__) + @asynccontextmanager async def lifespan(app: FastAPI): + if not settings.API_KEY: + logger.warning( + "API_KEY is not set - every endpoint is unauthenticated. " + "Set the API_KEY environment variable before deploying anywhere " + "reachable beyond localhost." + ) + else: + # Fingerprint (first 4 chars + length) so mismatches with the frontend's + # VITE_API_KEY are diagnosable without leaking the secret to the log. + # Fixes BUG-006 in BUG_REPORT.md. + fp = settings.API_KEY[:4] + "..." if len(settings.API_KEY) >= 4 else "***" + logger.info( + "API_KEY is set (fingerprint: %s, length: %d). " + "Frontend must send matching VITE_API_KEY via the X-API-Key header.", + fp, len(settings.API_KEY), + ) + if not settings.PG_PASSWORD: + logger.warning( + "PGPASSWORD is not set - connecting with an empty password. " + "Fine for local dev; set PGPASSWORD before deploying anywhere else." + ) db.init_pool() db.bootstrap() yield @@ -31,19 +57,26 @@ async def lifespan(app: FastAPI): version="1.0.0", description="CSV migration pipeline: preview, validate, load (dynamic tables or fixed T&E schema).", lifespan=lifespan, + dependencies=[Depends(require_api_key)], ) app.add_middleware( CORSMiddleware, allow_origins=settings.CORS_ORIGINS, - allow_methods=["*"], - allow_headers=["*"], + allow_methods=["GET", "POST", "DELETE"], + allow_headers=["Content-Type", "X-API-Key"], ) app.include_router(csv_routes.router) app.include_router(te_routes.router) +@app.exception_handler(psycopg2.pool.PoolError) +def pool_exhausted_handler(request: Request, exc: psycopg2.pool.PoolError) -> JSONResponse: + logger.warning("DB connection pool exhausted: %s", exc) + return JSONResponse(status_code=503, content={"detail": "Server busy, please retry shortly."}) + + @app.get("/api/health", tags=["health"]) def health() -> dict: try: @@ -58,4 +91,5 @@ def health() -> dict: "postgres": pg_version.split(" on ")[0], } except Exception as exc: # noqa: BLE001 — surface DB reachability to the UI - return {"status": "degraded", "error": str(exc)} + logger.warning("Health check failed: %s", exc) + return {"status": "degraded", "error": "database unreachable"} diff --git a/api/routers/csv_routes.py b/api/routers/csv_routes.py index 64f0cf2..c24eb5e 100644 --- a/api/routers/csv_routes.py +++ b/api/routers/csv_routes.py @@ -111,29 +111,54 @@ def table_rows(table_name: str, limit: int = 50) -> dict: @router.delete("/files/{file_id}") def delete_file(file_id: int) -> dict: + if not settings.allow_destructive: + raise HTTPException( + 403, + "Destructive operations are disabled. " + "Set API_ALLOW_DESTRUCTIVE=true to enable DELETE.", + ) with Conn() as conn: with conn.cursor() as cur: cur.execute( sql.SQL( - "SELECT table_name, mode FROM {}.csv_files WHERE id = %s" + "SELECT table_name, mode, file_name FROM {}.csv_files WHERE id = %s" ).format(sql.Identifier(settings.UPLOADS_SCHEMA)), (file_id,), ) row = cur.fetchone() if row is None: raise HTTPException(404, "File not found") - table_name, mode = row + table_name, mode, file_name = row if mode == "dynamic": + # Schema whitelist: only drop tables that live in the uploads schema. cur.execute( - sql.SQL("DROP TABLE IF EXISTS {}.{}").format( - sql.Identifier(settings.UPLOADS_SCHEMA), sql.Identifier(table_name) - ) + """ + SELECT 1 FROM information_schema.tables + WHERE table_schema = %s AND table_name = %s + """, + (settings.UPLOADS_SCHEMA, table_name), ) + if cur.fetchone() is not None: + cur.execute( + sql.SQL("DROP TABLE IF EXISTS {}.{}").format( + sql.Identifier(settings.UPLOADS_SCHEMA), + sql.Identifier(table_name), + ) + ) cur.execute( sql.SQL("DELETE FROM {}.csv_files WHERE id = %s").format( sql.Identifier(settings.UPLOADS_SCHEMA) ), (file_id,), ) - conn.commit() - return {"status": "ok", "deleted": file_id} + # Persist audit record so deletions are traceable after the table is gone. + cur.execute( + sql.SQL( + "INSERT INTO {}.audit_log " + "(action, file_id, file_name, table_name, mode) " + "VALUES ('delete', %s, %s, %s, %s)" + ).format(sql.Identifier(settings.UPLOADS_SCHEMA)), + (file_id, file_name, table_name, mode), + ) + conn.commit() + return {"status": "ok", "deleted": file_id} diff --git a/api/services/dynamic_loader.py b/api/services/dynamic_loader.py index e51a59a..8acc694 100644 --- a/api/services/dynamic_loader.py +++ b/api/services/dynamic_loader.py @@ -10,8 +10,10 @@ from __future__ import annotations import hashlib +import logging import time +import psycopg2.errors from psycopg2 import sql from psycopg2.extras import execute_values @@ -25,6 +27,8 @@ valid_identifier, ) +logger = logging.getLogger(__name__) + _TYPE_SQL = { "int8": "int8", "numeric": "numeric", @@ -79,8 +83,32 @@ def upload_dynamic( } schema = settings.UPLOADS_SCHEMA - replaced_file_name: str | None = None + try: + return _do_upload(file_name, types, overwrite, logs, schema, rows, file_hash) + except psycopg2.Error as exc: + # Mirror te_loader's structured-error contract: an unexpected DB error + # (e.g. a column type edge case not caught by cast_value) should not + # surface as a raw 500 to the frontend. + logger.warning("Dynamic upload failed for %r: %s", file_name, exc) + _log(logs, "error", "Database error while loading the CSV", "error") + return { + "status": "error", + "message": "The CSV could not be loaded due to a database error. Check the file's data types and try again.", + "logs": logs, + } + + +def _do_upload( + file_name: str, + types: list[str] | None, + overwrite: bool, + logs: list[dict], + schema: str, + rows: list[list[str]], + file_hash: str, +) -> dict: + replaced_file_name: str | None = None with Conn() as conn: with conn.cursor() as cur: # Duplicate FILENAME check @@ -251,16 +279,40 @@ def upload_dynamic( inserted += len(returned) _log(logs, "insert", f"Inserted {inserted} rows", count=inserted) - # Register + # Register. The duplicate checks above are racy (check-then-act, no + # lock held across the row-casting work), so a concurrent identical + # upload can slip past them; the registry's UNIQUE indexes are the + # real guard, and we turn a violation here into the same + # structured "duplicate_file" response the earlier check returns. _log(logs, "register", "Registering file in csv_files") - cur.execute( - sql.SQL( - "INSERT INTO {}.csv_files (file_name, file_hash, table_name, mode, row_count, column_names) " - "VALUES (%s, %s, %s, 'dynamic', %s, %s) RETURNING id" - ).format(sql.Identifier(schema)), - (file_name, file_hash, table_name, inserted, columns), - ) - file_id = cur.fetchone()[0] + try: + cur.execute( + sql.SQL( + "INSERT INTO {}.csv_files (file_name, file_hash, table_name, mode, row_count, column_names) " + "VALUES (%s, %s, %s, 'dynamic', %s, %s) RETURNING id" + ).format(sql.Identifier(schema)), + (file_name, file_hash, table_name, inserted, columns), + ) + file_id = cur.fetchone()[0] + except psycopg2.errors.UniqueViolation: + conn.rollback() + cur.execute( + sql.SQL( + "SELECT file_name, table_name, row_count FROM {}.csv_files " + "WHERE file_name = %s OR file_hash = %s" + ).format(sql.Identifier(schema)), + (file_name, file_hash), + ) + existing = cur.fetchone() + _log(logs, "duplicate_check", "Lost race to a concurrent identical upload", "warn") + return { + "status": "duplicate_file", + "reason": "content" if existing and existing[0] != file_name else "name", + "existingFileName": existing[0] if existing else file_name, + "tableName": existing[1] if existing else table_name, + "existingRowCount": (existing[2] or 0) if existing else 0, + "logs": logs, + } conn.commit() diff --git a/api/services/te_loader.py b/api/services/te_loader.py index e301a4c..a981733 100644 --- a/api/services/te_loader.py +++ b/api/services/te_loader.py @@ -11,6 +11,7 @@ import time from psycopg2 import sql +from psycopg2.extras import execute_values from api.config import TE_TABLES, settings from api.db import Conn @@ -97,29 +98,51 @@ def upload_te(file_name: str, content: str, target_table: str) -> dict: sql.SQL(", ").join(sql.Identifier(c) for c in columns), sql.SQL(", ").join(sql.Placeholder() for _ in columns), ) + batch_insert_stmt = sql.SQL("INSERT INTO {}.{} ({}) VALUES %s").format( + sql.Identifier(settings.TE_SCHEMA), + sql.Identifier(target_table), + sql.SQL(", ").join(sql.Identifier(c) for c in columns), + ) + + def _row_values(raw: list[str]) -> list: + return [ + (raw[c].strip() if c < len(raw) and raw[c].strip() != "" else None) + for c in range(len(columns)) + ] + + def _insert_one(cur, row_number: int, raw: list[str]) -> None: + """Per-row insert with its own savepoint; used for the happy path's + fallback so a bad row is identified without aborting the batch.""" + nonlocal inserted + cur.execute("SAVEPOINT row_sp") + try: + cur.execute(insert_stmt, _row_values(raw)) + inserted += 1 + except Exception as exc: # noqa: BLE001 — report DB cast/constraint errors per row + cur.execute("ROLLBACK TO SAVEPOINT row_sp") + row_errors.append({"rowNumber": row_number, "reason": str(exc).split("\n")[0]}) + finally: + cur.execute("RELEASE SAVEPOINT row_sp") with Conn() as conn: with conn.cursor() as cur: - for r, raw in enumerate(data_rows): - row_number = r + 1 - values = [ - (raw[c].strip() if c < len(raw) and raw[c].strip() != "" else None) - for c in range(len(columns)) - ] - cur.execute("SAVEPOINT row_sp") + chunk_size = 500 + for start in range(0, len(data_rows), chunk_size): + chunk = data_rows[start : start + chunk_size] + cur.execute("SAVEPOINT chunk_sp") try: - cur.execute(insert_stmt, values) - inserted += 1 - except Exception as exc: # noqa: BLE001 — report DB cast/constraint errors per row - cur.execute("ROLLBACK TO SAVEPOINT row_sp") - row_errors.append( - { - "rowNumber": row_number, - "reason": str(exc).split("\n")[0], - } + execute_values( + cur, + batch_insert_stmt.as_string(cur), + [_row_values(raw) for raw in chunk], ) - finally: - cur.execute("RELEASE SAVEPOINT row_sp") + cur.execute("RELEASE SAVEPOINT chunk_sp") + inserted += len(chunk) + except Exception: # noqa: BLE001 — fall back to per-row to find the bad row(s) + cur.execute("ROLLBACK TO SAVEPOINT chunk_sp") + cur.execute("RELEASE SAVEPOINT chunk_sp") + for i, raw in enumerate(chunk): + _insert_one(cur, start + i + 1, raw) # Register the load in the shared registry (mode='te') file_hash = hashlib.sha256(content.encode("utf-8")).hexdigest() diff --git a/frontend/.env.example b/frontend/.env.example new file mode 100644 index 0000000..13be44c --- /dev/null +++ b/frontend/.env.example @@ -0,0 +1,6 @@ +# FastAPI backend base URL (see scripts/start-api.ps1) +VITE_API_URL=http://localhost:8000 + +# Must match the backend's API_KEY env var. Leave blank for local dev where +# the backend has no API_KEY set (see API_INTEGRATION.md#authentication). +VITE_API_KEY= diff --git a/frontend/.gitignore b/frontend/.gitignore index d24df8a..9187c8f 100644 --- a/frontend/.gitignore +++ b/frontend/.gitignore @@ -20,6 +20,9 @@ dist-ssr .wrangler/ .dev.vars +# Local env vars (may contain VITE_API_KEY) — see .env.example +.env + # Editor directories and files .vscode/* !.vscode/extensions.json @@ -30,3 +33,5 @@ dist-ssr *.njsproj *.sln *.sw? +_norton_ +.abacusai \ No newline at end of file diff --git a/frontend/src/lib/csv.functions.ts b/frontend/src/lib/csv.functions.ts index d776b32..e9cc7dc 100644 --- a/frontend/src/lib/csv.functions.ts +++ b/frontend/src/lib/csv.functions.ts @@ -10,6 +10,28 @@ const API_URL: string = (import.meta.env.VITE_API_URL as string | undefined) ?? "http://localhost:8000"; +const API_KEY: string = (import.meta.env.VITE_API_KEY as string | undefined) ?? ""; + +// One-time diagnostic on module load so an API_KEY / VITE_API_KEY mismatch is +// easy to spot in the browser console. The API logs a parallel fingerprint at +// startup (see api/main.py). Fixes BUG-006 in BUG_REPORT.md. +if (typeof window !== "undefined") { + if (API_KEY) { + const fp = API_KEY.length >= 4 ? `${API_KEY.slice(0, 4)}...` : "***"; + // eslint-disable-next-line no-console + console.info( + `[csv.functions] VITE_API_KEY is set (fingerprint: ${fp}, length: ${API_KEY.length}). ` + + `Backend API_KEY fingerprint must match — check the API startup log if requests 401.`, + ); + } else { + // eslint-disable-next-line no-console + console.info( + "[csv.functions] VITE_API_KEY is empty. Fine if the backend's API_KEY is also unset. " + + "If requests return 401, set VITE_API_KEY in frontend/.env to match the backend's API_KEY.", + ); + } +} + // ── Types (unchanged public contract) ──────────────────────────────────────── export type ColumnType = "int8" | "numeric" | "date" | "timestamptz" | "boolean" | "text"; @@ -92,8 +114,12 @@ async function apiFetch(path: string, init?: RequestInit): Promise { let res: Response; try { res = await fetch(`${API_URL}${path}`, { - headers: { "Content-Type": "application/json" }, ...init, + headers: { + "Content-Type": "application/json", + ...(API_KEY ? { "X-API-Key": API_KEY } : {}), + ...init?.headers, + }, }); } catch { throw new Error( diff --git a/frontend/src/routes/_authenticated/index.tsx b/frontend/src/routes/_authenticated/index.tsx index 8ffd4e3..eb23f7d 100644 --- a/frontend/src/routes/_authenticated/index.tsx +++ b/frontend/src/routes/_authenticated/index.tsx @@ -1,5 +1,5 @@ import { createFileRoute, useRouter } from "@tanstack/react-router"; -import { useQuery, useSuspenseQuery } from "@tanstack/react-query"; +import { useQuery } from "@tanstack/react-query"; import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from "react"; import { @@ -73,12 +73,30 @@ export const Route = createFileRoute("/_authenticated/")({ }, ], }), - loader: ({ context }) => context.queryClient.ensureQueryData(filesQuery), + // Prefetch the file list, but never let a fetch failure crash into the root + // errorComponent — the page still renders and the component shows a banner + // explaining that the backend is unreachable. Fixes BUG-003 in BUG_REPORT.md. + loader: async ({ context }) => { + try { + await context.queryClient.ensureQueryData(filesQuery); + } catch (err) { + // Swallow — useQuery in Home() will surface the same error as a banner + // rather than a blank error page. + // eslint-disable-next-line no-console + console.warn("CSV Migrator: prefetch of /api/csv/files failed —", err); + } + }, component: Home, }); function Home() { - const { data: files } = useSuspenseQuery(filesQuery); + const { data, error, isLoading, refetch, isFetching } = useQuery({ + ...filesQuery, + // Give the loader's cached result priority; only refetch on mount if nothing + // is cached yet. + staleTime: 5_000, + }); + const files: CsvFileSummary[] = data ?? []; return (
@@ -105,13 +123,54 @@ function Home() {
+ {error ? refetch()} /> : null} - + {isLoading && !error ? ( +
+ Loading migrated files… +
+ ) : ( + + )}
); } +function BackendUnreachableBanner({ + error, + isRetrying, + onRetry, +}: { + error: unknown; + isRetrying: boolean; + onRetry: () => void; +}) { + const message = error instanceof Error ? error.message : String(error); + return ( +
+
+ +
+

+ Backend unreachable — the file list couldn't load. +

+

{message}

+

+ Start the API with{" "} + scripts/start-api.ps1{" "} + (Terminal 1), then click Retry. Uploads are disabled until the backend is back. +

+ +
+
+
+ ); +} + // Auth removed — the app talks to the local FastAPI backend with no login. diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index 174e074..96910a0 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -12,4 +12,10 @@ export default defineConfig({ // nitro/vite builds from this server: { entry: "server" }, }, + // Force the dev server to 5173 (matches scripts/start-frontend.ps1 and + // the CORS_ORIGINS default in api/config.py). Without this, lovable's + // sandbox detection defaults to 8080. + vite: { + server: { port: 5173, strictPort: true, host: "localhost" }, + }, }); diff --git a/scripts/README.md b/scripts/README.md index 7a4032d..8c98d97 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -11,6 +11,8 @@ local (PowerShell or Bash) or in GitHub Actions. | `build.sh` | Linux/Mac/Cloud Shell: same | _(no CI workflow; run locally)_ | | `test.ps1` | Windows: run pytest + SQL suite + evals | `.github/workflows/quality-gate.yml` | | `test.sh` | Linux/Mac/Cloud Shell: same | `.github/workflows/quality-gate.yml` | +| `start-api.ps1` | Terminal 1: launch the FastAPI backend on `http://localhost:8000` | _(local dev only)_ | +| `start-frontend.ps1` | Terminal 2: launch the Vite dev server on `http://localhost:5173` | _(local dev only)_ | ## Common recipes @@ -41,6 +43,19 @@ Default: local `docker build`, tag `dev`, no push. Add `--acr-build` / ./scripts/test.sh --skip-sql ``` +### Local — start the Web UI (two terminals) + +```powershell +# Terminal 1 — FastAPI backend +pip install -r api\requirements.txt # first run only +.\scripts\start-api.ps1 # http://localhost:8000 + +# Terminal 2 — React frontend +.\scripts\start-frontend.ps1 # http://localhost:5173 (runs npm install on first launch) +``` + +`start-api.ps1` defaults the libpq env vars to the local PG 18 dev instance (`PGHOST=localhost`, `PGPORT=5433`, `PGUSER=postgres`, `PGDATABASE=te_mgmt_dev`) and prompts securely for `PGPASSWORD` if unset. `start-frontend.ps1` runs `npm install` if `node_modules/` is missing, then `npm run dev`. See the **Web UI + REST API** section of the root `README.md` for the full endpoint list, env vars, and data model. + ### Build and push to ACR in one go ```powershell diff --git a/scripts/start-api.ps1 b/scripts/start-api.ps1 index 01bfd6c..bb04f4c 100644 --- a/scripts/start-api.ps1 +++ b/scripts/start-api.ps1 @@ -1,5 +1,9 @@ -# start-api.ps1 — Terminal 1: run the FastAPI backend on http://localhost:8000 +# start-api.ps1 - Terminal 1: run the FastAPI backend on http://localhost:8000 # First run: pip install -r ..\api\requirements.txt +# +# ASCII-only on purpose: Windows PowerShell 5.1 mis-parses non-ASCII characters +# like em-dashes when the file lacks a UTF-8 BOM, which turns strings after the +# em-dash into unquoted command calls (see BUG-021 in BUG_REPORT.md). $ErrorActionPreference = "Stop" $apiDir = Join-Path $PSScriptRoot "..\api" @@ -10,12 +14,16 @@ if (-not $env:PGPORT) { $env:PGPORT = "5433" } if (-not $env:PGUSER) { $env:PGUSER = "postgres" } if (-not $env:PGDATABASE) { $env:PGDATABASE = "te_mgmt_dev" } if (-not $env:PGPASSWORD) { - Write-Host "PGPASSWORD not set — enter it now (input hidden):" -ForegroundColor Yellow + Write-Host "PGPASSWORD not set - enter it now (input hidden):" -ForegroundColor Yellow $sec = Read-Host -AsSecureString $env:PGPASSWORD = [Runtime.InteropServices.Marshal]::PtrToStringAuto( [Runtime.InteropServices.Marshal]::SecureStringToBSTR($sec)) } +if (-not $env:API_KEY) { + Write-Host "API_KEY not set - every endpoint is unauthenticated (fine for local dev)." -ForegroundColor Yellow +} + Set-Location (Join-Path $PSScriptRoot "..") Write-Host "API starting on http://localhost:8000 (docs: /docs)" -ForegroundColor Green python -m uvicorn api.main:app --reload --port 8000 diff --git a/scripts/start-frontend.ps1 b/scripts/start-frontend.ps1 index fee518d..706283c 100644 --- a/scripts/start-frontend.ps1 +++ b/scripts/start-frontend.ps1 @@ -1,12 +1,12 @@ -# start-frontend.ps1 — Terminal 2: run the React frontend on http://localhost:5173 -# First run: npm install (inside the frontend folder) +# start-frontend.ps1 - Terminal 2: run the React frontend on http://localhost:5173 +# First run installs node_modules automatically. $ErrorActionPreference = "Stop" $feDir = Join-Path $PSScriptRoot "..\frontend" Set-Location $feDir if (-not (Test-Path "node_modules")) { - Write-Host "node_modules missing — running npm install first..." -ForegroundColor Yellow + Write-Host "node_modules missing - running npm install first..." -ForegroundColor Yellow npm install } diff --git a/tests/conftest.py b/tests/conftest.py index 6439524..6406033 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,6 +12,14 @@ sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) +# Tests exercise the API directly via TestClient without an API key; bypass the +# require_api_key dependency for the whole test session rather than threading +# a header through every test's client.get/post/delete call. +from api.auth import require_api_key # noqa: E402 +from api.main import app # noqa: E402 + +app.dependency_overrides[require_api_key] = lambda: None + # Env vars tests are allowed to mutate; restored after every test. _RESTORE_KEYS = ( "CSV_FILE", diff --git a/tests/test_api_coverage.py b/tests/test_api_coverage.py index 3dd1ede..1000318 100644 --- a/tests/test_api_coverage.py +++ b/tests/test_api_coverage.py @@ -364,3 +364,64 @@ def test_te_upload_registers_in_csv_files(self): match = [f for f in files if f["file_name"] == f"{self.tag}.csv"] self.assertTrue(match, "T&E upload should appear in csv_files list") self.assertEqual(match[0]["mode"], "te") + + + def test_enum_constraint_violation_reported_per_row(self): + """An invalid enum value must produce a per-row error, not a 500.""" + csv = self._org_csv(f"{self.tag}_enum", org_type="INVALID_ENUM") + r = self._upload_te(csv) + body = r.json() + self.assertEqual(body["status"], "ok", body) + self.assertGreaterEqual(body["failedRows"], 1, + "Invalid enum value should fail at row level, not crash the endpoint") + self.assertIn("rowNumber", body["rowErrors"][0]) + + def test_null_in_not_null_column_reported_per_row(self): + """An empty required field sent as NULL must fail per-row, not crash.""" + csv = "name,org_type,country\n,government,AU\n" + r = self._upload_te(csv) + body = r.json() + self.assertEqual(body["status"], "ok", body) + self.assertGreaterEqual(body["failedRows"], 1, + "NULL in a NOT NULL column should fail at row level") + self.assertIn("rowNumber", body["rowErrors"][0]) + + def test_unknown_target_table_returns_error(self): + """Uploading to a table not in TE_TABLES must return status=error.""" + r = self.client.post("/api/csv/upload", json={ + "fileName": f"{self.tag}.csv", + "content": "col_a\nval\n", + "mode": "te", + "targetTable": "non_existent_table", + }) + body = r.json() + self.assertEqual(body["status"], "error", body) + self.assertIn("non_existent_table", body.get("message", "")) + + def test_te_reupload_same_file_replaces_registry(self): + """Re-uploading the same CSV in T&E mode replaces the registry entry.""" + csv = self._org_csv(f"{self.tag}_reup") + r1 = self._upload_te(csv) + self.assertEqual(r1.json()["status"], "ok", r1.json()) + r2 = self._upload_te(csv) + self.assertEqual(r2.json()["status"], "ok", r2.json()) + files = self.client.get("/api/csv/files").json() + matches = [f for f in files if f["file_name"] == f"{self.tag}.csv"] + self.assertEqual(len(matches), 1, + "Re-upload should replace the registry entry, not create a duplicate") + + +@pytest.mark.unit +class DeleteGateUnit(unittest.TestCase): + """DELETE endpoint must honour API_ALLOW_DESTRUCTIVE=false (unit, no DB).""" + + def test_delete_blocked_returns_403(self): + from api import config as cfg + original = cfg.settings.allow_destructive + cfg.settings.allow_destructive = False + try: + r = client.delete("/api/csv/files/1") + self.assertEqual(r.status_code, 403, + "DELETE must return 403 when API_ALLOW_DESTRUCTIVE=false") + finally: + cfg.settings.allow_destructive = original