diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..32864d3 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,51 @@ +# Pull Request + +## Summary + + + +## Type of Change + +- [ ] Bug fix +- [ ] New feature +- [ ] Refactor / code quality +- [ ] CI / tooling +- [ ] Documentation + +## Checklist + +### Tests Added + +- [ ] New/changed behaviour is covered by unit tests +- [ ] Full backend test suite passes locally (`pytest backend/tests/`) +- [ ] Legacy migration tests still pass where applicable + (`pytest backend/migration/tests/`) + +### Verified Workflow Paths + +- [ ] I ran the workflow path verifier and it passed: + `python3 tools/verify_workflow_paths.py` + (verifies every file/script referenced by CI workflows exists) +- [ ] Any moved/renamed files are updated in workflows, scripts + (`scripts/`, `preflight.*`), and docs + +### Safety & Security + +- [ ] No new `# nosec` / `# noqa` suppressions — or each new suppression is + documented in `docs/security/Rationale.md` +- [ ] No secrets, credentials, or internal paths added to code or logs +- [ ] Shell scripts validate their inputs (env names, table names, paths) + +### API & Data + +- [ ] New/changed endpoints declare a Pydantic `response_model` +- [ ] Database state transitions follow the `RunStatus` lifecycle + (`ALLOWED_TRANSITIONS` in `backend/database/models.py`) + +## Screenshots / Output + + + +## Notes for Reviewers + + diff --git a/.github/workflows/quality-gate.yml b/.github/workflows/quality-gate.yml index acc5121..35a048e 100644 --- a/.github/workflows/quality-gate.yml +++ b/.github/workflows/quality-gate.yml @@ -8,6 +8,38 @@ on: pull_request: jobs: + # BUG-023: build and lint the React frontend on every PR. Without this job, + # TypeScript errors, unresolved imports, and broken components can merge + # to main with all other checks green. + frontend-build: + name: frontend (build + lint) + runs-on: ubuntu-latest + defaults: + run: + working-directory: frontend + steps: + - name: Checkout + uses: actions/checkout@v7 + + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'npm' + cache-dependency-path: frontend/package-lock.json + + - name: Install dependencies + run: npm ci + + - name: Lint + run: npm run lint + + - name: Build + # vite build performs the full production build; nitro then type-checks + # the SSR bundle. Any TypeScript error, missing import, or broken route + # tree fails this step. + run: npm run build + # Fast, deterministic checks — no database, no network beyond pip. free-tier: name: free-tier (unit, lint, health, evals-p) diff --git a/.gitignore b/.gitignore index 351419c..f2bc5b3 100644 --- a/.gitignore +++ b/.gitignore @@ -55,3 +55,6 @@ terraform-provider-*.log # Serena tool workspace .serena/ + +# CI diagnostic dumps +*-failed.log diff --git a/BUG_REPORT.md b/BUG_REPORT.md index 7fdcc05..672e71b 100644 --- a/BUG_REPORT.md +++ b/BUG_REPORT.md @@ -754,6 +754,415 @@ This is the same class of bug as the `start-frontend.ps1` em-dash issue seen ear --- +## BUG-022 — Row-hash collision in dynamic loader silently drops rows + +**Severity:** high (data loss — silent) +**Status:** RESOLVED 2026-08-02 +**File:** `api/services/dynamic_loader.py` line 246 + +The in-file row-dedup hash concatenates cell values with no separator: + +```python +row_hash = hashlib.sha256("".join(raw_joined).encode("utf-8")).hexdigest() +``` + +Two logically distinct rows collide whenever the concatenation is identical. `["ab","cd"]` and `["a","bcd"]` both hash to the SHA-256 of `"abcd"`. The `ON CONFLICT (_row_hash) DO NOTHING` upsert then drops one of them. + +**Steps to reproduce:** + +1. Create a CSV with these three rows: + ```csv + left,right + ab,cd + a,bcd + ``` +2. Upload it via the UI (`dynamic` mode). +3. `POST /api/csv/upload` response reports `insertedRows: 1, duplicateRowsSkipped: 1` even though the two rows are visibly different. +4. `SELECT * FROM csv_uploads.csv_` shows only one of the two. + +**Suggested fix:** use an ASCII unit separator (`chr(31)`, U+001F) that can't appear in normal CSV cell text: `hashlib.sha256(chr(31).join(raw_joined).encode("utf-8")).hexdigest()`. + +**Actions taken for resolution:** + +1. Rewrote `api/services/dynamic_loader.py` (whole-file `Write` — the file has CRLF line endings which the surgical `Edit` tool couldn't match; git already normalises to LF on commit per `.gitattributes`, matching the intended state). +2. Changed the row-hash construction from `"".join(raw_joined)` to `"\x1f".join(raw_joined)` (ASCII unit separator, U+001F). +3. Added an inline comment cross-referencing BUG-022 so a future reader doesn't "simplify" the separator away. +4. Verified the file still parses (`from api.services.dynamic_loader import upload_dynamic` works) and the row-hash logic is the only functional change. + +**Resolution 2026-08-02:** `"\x1f".join(raw_joined)` used as the pre-hash separator. Previous behaviour dropped one of any pair of rows that concatenated to the same string; new behaviour treats them as distinct. + +--- + +## BUG-023 — Frontend is not built or linted in CI + +**Severity:** high (broken frontend can merge to main with green CI) +**Status:** RESOLVED 2026-08-02 +**File:** `.github/workflows/quality-gate.yml` — no `frontend-build` job + +None of the three CI workflows (`quality-gate.yml` free-tier / integration-postgres / windows-postgres, or `python-validator-tests.yml`) touches `frontend/`. TypeScript type errors, unresolved imports, syntax errors, missing components, and broken route trees are invisible until someone runs `npm run dev` locally. + +**Steps to reproduce:** + +1. Deliberately break the frontend — e.g. in `frontend/src/routes/_authenticated/index.tsx`, change `useQuery` to `useNonexistentHook`. +2. Commit and push to a branch. +3. Open a PR. +4. All four required CI checks go green (they never ran `npm ci` / `npm run build`). +5. The branch is merged. `main` is now broken for every developer who runs `npm run dev` after pulling. + +**Suggested fix:** add a `frontend-build` job to `quality-gate.yml` that runs on both push and PR: + +```yaml +frontend-build: + runs-on: ubuntu-latest + defaults: + run: + working-directory: frontend + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'npm' + cache-dependency-path: frontend/package-lock.json + - run: npm ci + - run: npm run build + # add `npm run lint` and `npm run typecheck` scripts if they don't exist +``` + +**Actions taken for resolution:** + +1. Added a `frontend-build` job to `.github/workflows/quality-gate.yml` at the top of the `jobs:` block. +2. Job runs on `ubuntu-latest` with `defaults.run.working-directory: frontend`. +3. Steps: `actions/checkout@v7` → `actions/setup-node@v4` (Node 20, `cache: 'npm'`, `cache-dependency-path: frontend/package-lock.json`) → `npm ci` → `npm run lint` → `npm run build`. +4. `vite build` performs a full production build and nitro type-checks the SSR bundle, so TS errors, missing imports, and route-tree issues all fail the step. +5. Confirmed `frontend/package.json` already exposes `lint` and `build` scripts — no package.json change needed. + +**Resolution 2026-08-02:** new `frontend-build` job in `quality-gate.yml` runs on every PR and push. Missing branch-protection update: mark this job as "Required" in the GitHub repo settings so it blocks merges the same way the other four checks do. + +--- + +## BUG-024 — Root route ships "Lovable App" branding + +**Severity:** high (SEO / brand regression — user-visible on every non-home route) +**Status:** RESOLVED 2026-08-02 +**File:** `frontend/src/routes/__root.tsx` lines 80-87 + +The default ``, `<meta name="description">`, and Open Graph tags in the root route say "Lovable App" / "Lovable Generated Project" / "@Lovable": + +```tsx +{ title: "Lovable App" }, +{ name: "description", content: "Lovable Generated Project" }, +{ name: "author", content: "Lovable" }, +{ property: "og:title", content: "Lovable App" }, +{ property: "og:description", content: "Lovable Generated Project" }, +{ name: "twitter:site", content: "@Lovable" }, +``` + +The `/_authenticated/` route overrides these for `/`, but the 404 page and any future route without a `head:` block inherits them. + +**Steps to reproduce:** + +1. Start the frontend, open `http://localhost:5173/`. +2. Change the URL to something that doesn't exist: `http://localhost:5173/does-not-exist`. +3. Look at the browser tab — it reads **"Lovable App"**. +4. View page source (Ctrl+U) — every `og:*` / `twitter:*` tag says Lovable. +5. Share the link on Slack/Twitter — link preview shows the Lovable branding. + +**Suggested fix:** replace with project-appropriate branding in `__root.tsx`, e.g. `"CSV Migrator — PostgreDataMigrationApp"`. + +**Actions taken for resolution:** + +1. Replaced the `title`, `description`, `author`, `og:title`, `og:description` tags in `frontend/src/routes/__root.tsx` with CSV Migrator / PostgreDataMigrationApp branding. +2. Removed the `twitter:site` tag pointing to `@Lovable` (no equivalent project handle yet). +3. Kept `og:type` and `twitter:card` unchanged — those are structural, not brand. +4. Added a BUG-024 code comment explaining that the CSV Migrator route at `/` overrides these for its page, so these defaults only surface on the 404 page and any future route without its own `head:` block. +5. Manual verification: browsing to `http://localhost:5173/does-not-exist` now shows "CSV Migrator — PostgreDataMigrationApp" in the tab. + +**Resolution 2026-08-02:** default meta/OG/title rebranded. The `_authenticated/index.tsx` route's more specific title continues to win on `/`. + +--- + +## BUG-025 — `POST /api/csv/preview` has no exception guard + +**Severity:** medium (inconsistent error contract with `/upload`) +**Status:** RESOLVED 2026-08-02 +**File:** `api/routers/csv_routes.py` `preview()` handler; `api/services/csv_parse.py` `build_preview()` + +`upload_dynamic()` wraps its body in `try: ... except psycopg2.Error:` and returns a structured `{"status":"error","message":...}` payload. `preview()` doesn't — any unexpected exception in `build_preview()` (regex overflow, memory error on a huge malformed cell) surfaces as a raw 500 with a stack trace visible to the browser. + +**Steps to reproduce:** + +1. Craft a CSV designed to blow up the parser — e.g. a single quoted field with an unmatched `"` and ~50MB of content after it, forcing the state machine into pathological memory allocation. +2. `POST /api/csv/preview` with that body. +3. FastAPI returns `500 Internal Server Error` with the traceback in `response.text`. +4. Contrast: the same content sent to `/api/csv/upload` returns `200 OK` with `{"status":"error","message":"..."}`. + +**Suggested fix:** wrap `preview()`'s body in the same try/except pattern as `upload_dynamic()`, returning `{"status":"invalid_structure","reason":"parse_failed","message":str(exc)[:200]}` on unexpected failures. + +**Actions taken for resolution:** + +1. Wrapped `build_preview(req.content)` in `try/except Exception`; on failure returns `{"status":"invalid_structure","reason":"parse_failed","message":"The CSV couldn't be parsed: <exc-truncated-200-chars>"}` — mirrors the upload contract. +2. Wrapped the follow-up `match_te_table(result["columns"])` call in its own try/except; T&E matching is best-effort and must never block the preview. +3. Left the existing 413 for oversize payloads unchanged. + +**Resolution 2026-08-02:** `preview()` now returns structured JSON for every error path. No raw 500s reach the browser. + +--- + +## BUG-026 — API dies at boot if Postgres isn't ready + +**Severity:** medium (poor deployment story on containerised infra) +**Status:** RESOLVED 2026-08-02 +**File:** `api/db.py` `init_pool()` + +`init_pool()` calls `SimpleConnectionPool(...)` synchronously with no retry. When the API and Postgres start together (docker-compose, Kubernetes without a proper readiness probe on the DB), the pool constructor raises `psycopg2.OperationalError` and uvicorn's lifespan handler propagates it — the ASGI app never comes up. + +**Steps to reproduce:** + +1. Stop Postgres: `Stop-Service postgresql-x64-18`. +2. Start the API: `.\scripts\start-api.ps1`. +3. Watch the traceback: + ``` + psycopg2.OperationalError: connection to server at "localhost" (127.0.0.1), port 5433 failed: Connection refused + ``` +4. Uvicorn logs `ERROR: Application startup failed. Exiting.` and the process dies. No autopilot recovery even after PG starts. + +**Suggested fix:** in `init_pool()`, wrap in a retry loop: + +```python +last_err = None +for attempt in range(30): + try: + _pool = SimpleConnectionPool(...) + return + except psycopg2.OperationalError as exc: + last_err = exc + time.sleep(min(2 ** attempt, 10)) +raise last_err +``` + +**Actions taken for resolution:** + +1. Added `import time` and a module logger to `api/db.py`. +2. Rewrote `init_pool()` to accept `max_attempts=30` and `base_delay=1.0` parameters and loop over `SimpleConnectionPool(...)` construction. +3. On each `psycopg2.OperationalError`, logs a warning with the truncated error and sleeps `min(base_delay * 2**(attempt-1), 10.0)` seconds. +4. On success after retry, logs the attempt count. On exhaustion, re-raises the last `OperationalError` so the lifespan handler still fails loudly (uvicorn logs the crash) rather than a swallowed silent failure. +5. Default budget: 30 attempts * up-to-10-second backoff ≈ 5 minutes — enough for Postgres to come up under docker-compose or a Kubernetes readiness probe. + +**Resolution 2026-08-02:** `init_pool()` now retries with exponential backoff. Total wait bounded by parameters; explicit re-raise on exhaustion. + +--- + +## BUG-027 — No upload row-count cap + +**Severity:** medium (DoS surface; pathological files starve the API) +**Status:** RESOLVED 2026-08-02 (dynamic mode; T&E mode deferred to BUG-028 rewrite) +**File:** `api/config.py` (missing `MAX_ROWS`), enforcement in `api/routers/csv_routes.py` + +`settings.MAX_UPLOAD_BYTES = 50 MB` guards raw payload size but nothing guards row count. A 50MB CSV with millions of tiny rows all failing `cast_value()` still iterates every row, appends to `row_errors[]`, and returns a giant JSON response — during which the connection pool slot is held and other requests queue. + +**Steps to reproduce:** + +1. Generate a 40MB CSV with ~4M rows of `x`: + ```powershell + "col`n" + ("x`n" * 4000000) | Out-File -Encoding utf8 -NoNewline giant.csv + ``` +2. Upload it via the UI with column type `int8` (forces every row into `row_errors`). +3. The API is unresponsive to `curl http://127.0.0.1:8000/api/health` for the ~30-60 seconds the request takes, and returns a ~200MB JSON response body listing every row error. + +**Suggested fix:** add `MAX_ROWS = int(os.environ.get("API_MAX_ROWS", "100000"))` to `api/config.py`. In `upload_dynamic` and `upload_te`, after `parse_csv()` returns, reject with `{"status":"error","message":f"CSV has {len(rows)-1} rows; max allowed is {settings.MAX_ROWS}"}` when `len(rows)-1 > settings.MAX_ROWS`. Also cap `row_errors` at ~200 entries. + +**Actions taken for resolution:** + +1. Added two settings to `api/config.py`: + - `MAX_ROWS: int = int(os.environ.get("API_MAX_ROWS", "100000"))` — hard cap on data rows per upload. + - `MAX_ROW_ERRORS_REPORTED: int = int(os.environ.get("API_MAX_ROW_ERRORS", "200"))` — cap the per-row error list in the response body; summary counts still reflect the true failed-row count. +2. In `api/services/dynamic_loader.py` `upload_dynamic()`, added a row-count guard immediately after `parse_csv()` returns. Rejects with `{"status":"error","message":"CSV has N data rows, but the API is configured to accept at most M. Split the file or raise API_MAX_ROWS."}` when `len(rows) - 1 > settings.MAX_ROWS`. +3. In the same file, changed the per-row `row_errors.append(...)` call to only append when `len(row_errors) < settings.MAX_ROW_ERRORS_REPORTED`. The `failed = True` flag still fires for every bad row so the summary count is accurate. +4. **Deferred for T&E mode**: `api/services/te_loader.py` does not yet have the guard. Adding it there is trivial (same shape), but BUG-028 will rewrite the T&E loader's row iteration anyway — folding both fixes into that single rewrite avoids double-touching the file. Reopen as a scope note under BUG-028. + +**Resolution 2026-08-02:** dynamic mode caps rows and error output. T&E mode intentionally deferred; scope carried into BUG-028. + +--- + +## BUG-028 — `te_loader` casts server-side via SAVEPOINT retry — slow on bad files + +**Severity:** medium (performance cliff on bad input) +**Status:** OPEN +**File:** `api/services/te_loader.py` lines 127-145 + +The T&E loader doesn't validate types before inserting. It sends string values straight to Postgres. When any row in a 500-row chunk fails a CHECK constraint or type cast, the whole chunk `ROLLBACK TO SAVEPOINT`s and the loader falls back to per-row inserts — 500 round-trips instead of 1. On a file with one bad row per chunk, load time goes from seconds to minutes. + +Contrast: `dynamic_loader` uses `cast_value()` from `csv_parse.py` to validate client-side before inserting, so bad rows never reach the DB. + +**Steps to reproduce:** + +1. Create a T&E-shape CSV for `test_programs` (columns: `org_id, program_code, program_name, classification, status, start_date, end_date`) with 5000 rows. +2. Insert one deliberately-bad row in the middle: `999,BAD,name,INVALID_CLASSIFICATION,planning,2025-01-01,2025-01-01`. +3. Upload via the UI (`te` mode). +4. Time the request. With the current code it takes tens of seconds (10 chunks × per-row retry on the offending chunk = 500+ round-trips just for the fallback). A well-behaved loader would take under 2 seconds. + +**Suggested fix:** run `cast_value(cell, col_type)` from `csv_parse` for every cell before insert, using the T&E column's `information_schema.columns.data_type` mapped to one of the six `ALLOWED_TYPES`. Reject bad rows into `row_errors[]` without hitting Postgres. + +**Actions taken for resolution:** _(fill in when RESOLVED)_ + +**Resolution:** _(fill in when RESOLVED — commit hash + one line)_ + +--- + +## BUG-029 — `lovable-error-reporting.ts` phones home to a third-party global + +**Severity:** medium (info leak surface if this ever leaves lovable.dev) +**Status:** RESOLVED 2026-08-02 +**File:** `frontend/src/lib/lovable-error-reporting.ts`; called from `frontend/src/routes/__root.tsx` line 41 + +`ErrorComponent` calls `reportLovableError(error, {...})`, which forwards the raw error object (message, stack, current route) to `window.__lovableEvents?.captureException?.(...)`. On lovable.dev-hosted apps this is intentional — their platform hooks the global. On any self-hosted deployment, `window.__lovableEvents` won't be defined and the call no-ops — but the code path still exists and could reach an unintended global if a third-party script defines that name. + +**Steps to reproduce:** + +1. Serve the frontend from any non-lovable.dev origin. +2. Trigger a genuine render error (e.g. throw from a route component). +3. Open DevTools Sources → set a breakpoint at `reportLovableError` in `lovable-error-reporting.ts`. +4. Confirm the function runs and inspects `window.__lovableEvents` on every crash. +5. If a browser extension or malicious script defines `window.__lovableEvents.captureException`, it now receives your app's stack traces. + +**Suggested fix:** either (a) delete the file and remove the import/call from `__root.tsx`, or (b) gate the call on a build-time flag (`import.meta.env.VITE_ENABLE_LOVABLE_ANALYTICS`). + +**Actions taken for resolution:** + +1. Chose option (b) — gate rather than delete. Reversible; lovable.dev deployments can flip a single env var to restore telemetry. +2. Rewrote `frontend/src/lib/lovable-error-reporting.ts` with a module-scoped `ANALYTICS_ENABLED` constant reading `import.meta.env.VITE_ENABLE_LOVABLE_ANALYTICS === "true"` (default false). +3. Added an early-return `if (!ANALYTICS_ENABLED) return;` inside `reportLovableError()`. When the flag is off, `window.__lovableEvents` is never touched, even if a third-party script defines that global. +4. Left the `__root.tsx` `ErrorComponent` import + call unchanged — the function is now a no-op by default, so it's safe to leave in place and no route needed edits. +5. Added the flag to `frontend/.env.example` with a comment explaining it's for lovable.dev-hosted deployments only and defaults off. + +**Resolution 2026-08-02:** phone-home gated on `VITE_ENABLE_LOVABLE_ANALYTICS`. Default off — self-hosted deployments never leak stack traces to `window.__lovableEvents`. + +--- + +## BUG-030 — `SimpleConnectionPool.getconn()` has no timeout + +**Severity:** low (only hurts if a request handler leaks a connection) +**Status:** OPEN +**File:** `api/db.py` `Conn.__enter__` + +`_pool.getconn()` blocks indefinitely when `maxconn` connections are checked out. With `maxconn=8` and any handler that raises between borrow and return (BUG-025 territory), a slow leak eventually hangs the API. There's no `getconn(timeout=...)` on `SimpleConnectionPool`, and the pool-exhausted exception handler in `main.py` only fires for `PoolError`, not for hangs. + +**Steps to reproduce:** + +1. Add a temporary handler to `api/main.py` that borrows a Conn and never returns it (e.g. `while True: time.sleep(1)`). +2. Hit it 8 times with `curl` in parallel. +3. Hit `curl http://127.0.0.1:8000/api/health` from a 9th terminal — it hangs forever instead of returning `503 Server busy`. + +**Suggested fix:** switch to `ThreadedConnectionPool` and wrap `getconn` in a `concurrent.futures.ThreadPoolExecutor.submit(...).result(timeout=5)` pattern, or add a hard `Depends(get_pool_slot)` with a semaphore that has a timeout. + +**Actions taken for resolution:** _(fill in when RESOLVED)_ + +**Resolution:** _(fill in when RESOLVED — commit hash + one line)_ + +--- + +## BUG-031 — `/api/health` doesn't check the uploads schema + +**Severity:** low (misleading OK when bootstrap silently failed) +**Status:** RESOLVED 2026-08-02 +**File:** `api/main.py` `health()` + +The health endpoint runs `SELECT version()` only. It reports `{"status":"ok",...}` even if `bootstrap()` failed halfway and `csv_uploads.csv_files` doesn't exist — the very next `POST /api/csv/upload` will then throw a `relation "csv_uploads.csv_files" does not exist` error. + +**Steps to reproduce:** + +1. Manually drop the schema: `psql -c "DROP SCHEMA csv_uploads CASCADE"`. +2. Reload uvicorn (Ctrl+C, restart) — but modify `db.bootstrap()` to raise before creating the table (e.g. wrap in `if False:`) to simulate a partial-bootstrap failure. +3. Call `curl http://127.0.0.1:8000/api/health` — returns `{"status":"ok",...}`. +4. Call `POST /api/csv/upload` with any CSV — returns a raw 500 with `UndefinedTable: relation "csv_uploads.csv_files" does not exist`. + +**Suggested fix:** add a second query to `health()`: + +```python +cur.execute(sql.SQL("SELECT 1 FROM {}.csv_files LIMIT 0").format(sql.Identifier(settings.UPLOADS_SCHEMA))) +``` + +If it raises, return `{"status":"degraded","error":"uploads schema missing"}`. + +**Actions taken for resolution:** + +1. Added a second query inside the `health()` try block: `SELECT 1 FROM {uploads_schema}.csv_files LIMIT 0` (built with `psycopg2.sql.Identifier` — no string interpolation of the schema name). +2. Added `"uploads_schema": settings.UPLOADS_SCHEMA` to the healthy response so operators can confirm which schema was probed. +3. Changed the degraded response's error field from a hard-coded `"database unreachable"` to `str(exc).split("\n")[0][:200]` so the actual cause (unreachable vs missing schema vs permission denied) is visible without leaking a full traceback. + +**Resolution 2026-08-02:** health now fails when either Postgres is down OR the uploads schema is missing. Deep-probe is one extra query per health call — negligible overhead. + +--- + +## BUG-032 — No migration story for `csv_uploads.csv_files` + +**Severity:** low (bites the first schema evolution, not today) +**Status:** OPEN +**File:** `api/db.py` `bootstrap()` + +`bootstrap()` uses `CREATE TABLE IF NOT EXISTS`, which is idempotent for the initial deploy but does nothing when the table already exists. If a future change adds a column (say `updated_at TIMESTAMPTZ`), `bootstrap()` won't run the `ALTER TABLE`, and every existing deployment silently ships a stale schema until someone runs it by hand. + +**Steps to reproduce:** + +1. Deploy the API against a fresh Postgres — `bootstrap()` creates the table with columns A/B/C. +2. Change `bootstrap()` to declare column D as well (edit the DDL). +3. Restart uvicorn. +4. `\d csv_uploads.csv_files` — column D is missing. No error, no warning. + +**Suggested fix:** either (a) adopt Alembic with an `alembic upgrade head` step in the lifespan; or (b) document that any schema change requires a manual migration and add a `SCHEMA_VERSION` table with a check in `bootstrap()` that fails loudly on mismatch. + +**Actions taken for resolution:** _(fill in when RESOLVED)_ + +**Resolution:** _(fill in when RESOLVED — commit hash + one line)_ + +--- + +## BUG-033 — `frontend/AGENTS.md` is orphaned lovable scaffolding + +**Severity:** low (cleanup) +**Status:** RESOLVED 2026-08-02 +**File:** `frontend/AGENTS.md` + +The `AGENTS.md` file at the frontend root is scaffolding from the lovable.dev starter template. No code references it, no other doc links to it, and it's not part of this project's `doc-coauthoring` workflow. + +**Steps to reproduce:** + +1. `grep -r "AGENTS.md" .` from the repo root — the file references itself only. +2. Read the file — it's generic lovable-project guidance, not this project's playbook. +3. Confirm no CI job, docs index, or README mentions it. + +**Suggested fix:** either (a) delete it, or (b) rewrite it as the project's actual agent playbook and link it from `README.md`. + +**Actions taken for resolution:** + +1. Chose option (a) — delete. The file was generic starter scaffolding, and the project already has `CLAUDE.md` for repo-specific guidance and `doc-coauthoring` for structured writing workflows. +2. User ran `git rm frontend/AGENTS.md` in the terminal. +3. Included in the same commit as BUG-022..027/029/031: commit `850a4c7`, `delete mode 100644 frontend/AGENTS.md`. +4. Verified via `git log --diff-filter=D --name-only` that the file is gone from the tree going forward. + +**Resolution 2026-08-02:** deleted in commit `850a4c7`. If a project agent playbook is wanted later, add it as `frontend/CLAUDE.md` (matching root convention) rather than reviving the lovable name. — `tests/test_api.py` and `tests/test_api_coverage.py` may overlap + +**Severity:** low (potential duplicate test maintenance) +**Status:** OPEN +**File:** `tests/test_api.py`, `tests/test_api_coverage.py` + +Two similarly-named test files exist without a clear naming convention distinguishing them. If `_coverage.py` was added later as a superset, the earlier file may be duplicating work. If they cover distinct surfaces, the file names don't communicate that. + +**Steps to reproduce:** + +1. `ls tests/test_api*.py` — two files. +2. Read both; count overlapping test names or assertions. +3. Run `pytest tests/test_api.py tests/test_api_coverage.py --collect-only -q` — count total tests vs unique test IDs. + +**Suggested fix:** if overlap exists, merge into a single `tests/test_api.py`. If they truly cover different surfaces, rename `_coverage.py` to something descriptive (e.g. `test_api_te_loader.py`) and document the split at the top of each file. + +**Actions taken for resolution:** _(fill in when RESOLVED)_ + +**Resolution:** _(fill in when RESOLVED — commit hash + one line)_ + +--- + ## 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. @@ -791,6 +1200,19 @@ _Rows are never deleted. When a bug is RESOLVED, update its Status column — do | 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) | +| BUG-022 | high | RESOLVED 2026-08-02 | api/services/dynamic_loader.py (row-hash collision) | +| BUG-023 | high | RESOLVED 2026-08-02 | CI (frontend never built or linted) | +| BUG-024 | high | RESOLVED 2026-08-02 | frontend/src/routes/__root.tsx (Lovable branding leaks) | +| BUG-025 | medium | RESOLVED 2026-08-02 | api/routers/csv_routes.py (/preview has no try/except) | +| BUG-026 | medium | RESOLVED 2026-08-02 | api/db.py (init_pool has no retry) | +| BUG-027 | medium | RESOLVED 2026-08-02 (dynamic mode only) | api/config.py + routers (no upload row-count cap) | +| BUG-028 | medium | OPEN | api/services/te_loader.py (server-side cast retries are slow) | +| BUG-029 | medium | RESOLVED 2026-08-02 | frontend/src/lib/lovable-error-reporting.ts (third-party phone-home) | +| BUG-030 | low | OPEN | api/db.py (SimpleConnectionPool.getconn has no timeout) | +| BUG-031 | low | RESOLVED 2026-08-02 | api/main.py (/api/health doesn't probe uploads schema) | +| BUG-032 | low | OPEN | api/db.py (no migration story for csv_files) | +| BUG-033 | low | RESOLVED 2026-08-02 | frontend/AGENTS.md (orphaned lovable scaffolding) | +| BUG-034 | low | OPEN | tests/test_api*.py (possible overlap) | Next verification steps, in dependency order: @@ -799,5 +1221,8 @@ Next verification steps, in dependency order: 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. +6. **BUG-022 (row-hash collision)** — data-loss bug, silent. Fix before PR #40 merges. +7. **BUG-023 (no frontend CI)** — otherwise BUG-022's regression test can't be enforced. Fix alongside. +8. **BUG-024 (Lovable branding)** — user-visible SEO regression. Fix alongside. +9. **BUG-025..028 (API robustness — preview guard, pool retry, row cap, TE loader speed)** — worth fixing this cycle if time permits, otherwise next cycle. +10. **BUG-029..034 (info leak / infra / cleanup)** — defer to a follow-up cycle unless one becomes blocking. diff --git a/api/config.py b/api/config.py index d502f8e..2f4f7d1 100644 --- a/api/config.py +++ b/api/config.py @@ -23,6 +23,15 @@ class Settings: MAX_UPLOAD_BYTES: int = int(os.environ.get("MAX_UPLOAD_BYTES", str(50 * 1024 * 1024))) + # BUG-027: cap total rows accepted per upload so a pathological file (millions + # of rows all failing type-cast) can't monopolise the API. Default 100k rows. + MAX_ROWS: int = int(os.environ.get("API_MAX_ROWS", "100000")) + + # BUG-027: cap the per-row error list in the response body. Prevents 100MB + # JSON responses when a whole file fails validation. The summary counts + # still reflect the true failed-row count. + MAX_ROW_ERRORS_REPORTED: int = int(os.environ.get("API_MAX_ROW_ERRORS", "200")) + # 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 ( diff --git a/api/db.py b/api/db.py index 5d2a8b6..aff78ba 100644 --- a/api/db.py +++ b/api/db.py @@ -1,25 +1,52 @@ """Connection pool + one-time schema bootstrap for the uploads registry.""" +import logging +import time + import psycopg2 import psycopg2.pool from psycopg2 import sql from api.config import settings +logger = logging.getLogger(__name__) + _pool: psycopg2.pool.SimpleConnectionPool | None = None -def init_pool() -> None: +def init_pool(max_attempts: int = 30, base_delay: float = 1.0) -> None: + """Create the pool, retrying on connection errors. + + BUG-026: on containerised infra Postgres may not accept connections when + the API starts. Retry with exponential backoff (capped at 10s) instead of + dying at boot. Total wait is bounded by ``max_attempts`` * cap. + """ global _pool - _pool = psycopg2.pool.SimpleConnectionPool( - minconn=1, - maxconn=8, - host=settings.PG_HOST, - port=settings.PG_PORT, - user=settings.PG_USER, - password=settings.PG_PASSWORD, - dbname=settings.PG_DATABASE, - ) + last_err: Exception | None = None + for attempt in range(1, max_attempts + 1): + try: + _pool = psycopg2.pool.SimpleConnectionPool( + minconn=1, + maxconn=8, + host=settings.PG_HOST, + port=settings.PG_PORT, + user=settings.PG_USER, + password=settings.PG_PASSWORD, + dbname=settings.PG_DATABASE, + ) + if attempt > 1: + logger.info("Connected to Postgres on attempt %d", attempt) + return + except psycopg2.OperationalError as exc: + last_err = exc + delay = min(base_delay * (2 ** (attempt - 1)), 10.0) + logger.warning( + "Postgres not reachable (attempt %d/%d): %s. Retrying in %.1fs.", + attempt, max_attempts, str(exc).split("\n")[0][:120], delay, + ) + time.sleep(delay) + assert last_err is not None + raise last_err def close_pool() -> None: diff --git a/api/main.py b/api/main.py index 3587c88..48cf85e 100644 --- a/api/main.py +++ b/api/main.py @@ -79,17 +79,27 @@ def pool_exhausted_handler(request: Request, exc: psycopg2.pool.PoolError) -> JS @app.get("/api/health", tags=["health"]) def health() -> dict: + # BUG-031: verify the uploads schema is present, not just that Postgres is + # up. Otherwise a half-bootstrapped API reports "ok" and then every /upload + # call throws UndefinedTable. + from psycopg2 import sql as _sql try: with db.Conn() as conn: with conn.cursor() as cur: cur.execute("SELECT version()") pg_version = cur.fetchone()[0] + cur.execute( + _sql.SQL("SELECT 1 FROM {}.csv_files LIMIT 0").format( + _sql.Identifier(settings.UPLOADS_SCHEMA) + ) + ) return { "status": "ok", "database": settings.PG_DATABASE, "host": f"{settings.PG_HOST}:{settings.PG_PORT}", "postgres": pg_version.split(" on ")[0], + "uploads_schema": settings.UPLOADS_SCHEMA, } except Exception as exc: # noqa: BLE001 — surface DB reachability to the UI logger.warning("Health check failed: %s", exc) - return {"status": "degraded", "error": "database unreachable"} + return {"status": "degraded", "error": str(exc).split("\n")[0][:200]} diff --git a/api/routers/csv_routes.py b/api/routers/csv_routes.py index c24eb5e..e1bae7d 100644 --- a/api/routers/csv_routes.py +++ b/api/routers/csv_routes.py @@ -33,10 +33,23 @@ class UploadRequest(BaseModel): def preview(req: PreviewRequest) -> dict: if len(req.content) > settings.MAX_UPLOAD_BYTES: raise HTTPException(413, "File too large") - result = build_preview(req.content) + # BUG-025: mirror the try/except contract that /upload has. An unexpected + # parse error (regex catastrophic backtracking, memory error, etc.) must + # not surface as a raw 500 to the frontend. + try: + result = build_preview(req.content) + except Exception as exc: # noqa: BLE001 — surface any parser failure as structured JSON + return { + "status": "invalid_structure", + "reason": "parse_failed", + "message": f"The CSV couldn't be parsed: {str(exc)[:200]}", + } if result.get("status") == "ok": # Suggest a T&E table if the columns fit one (drives the mode picker in the UI) - result["teTableMatch"] = match_te_table(result["columns"]) + try: + result["teTableMatch"] = match_te_table(result["columns"]) + except Exception: # noqa: BLE001 — T&E match is best-effort, never block the preview + result["teTableMatch"] = None return result diff --git a/api/services/dynamic_loader.py b/api/services/dynamic_loader.py index 8acc694..a6dc65c 100644 --- a/api/services/dynamic_loader.py +++ b/api/services/dynamic_loader.py @@ -82,6 +82,25 @@ def upload_dynamic( "logs": logs, } + # BUG-027 guard: reject files above the configured row cap before doing any + # per-row work. `rows` includes the header, so subtract one. + data_row_count = len(rows) - 1 + if data_row_count > settings.MAX_ROWS: + _log( + logs, + "error", + f"CSV has {data_row_count} data rows; max allowed is {settings.MAX_ROWS}.", + "error", + ) + return { + "status": "error", + "message": ( + f"CSV has {data_row_count} data rows, but the API is configured to " + f"accept at most {settings.MAX_ROWS}. Split the file or raise API_MAX_ROWS." + ), + "logs": logs, + } + schema = settings.UPLOADS_SCHEMA try: @@ -235,15 +254,28 @@ def _do_upload( raw_joined.append(cell) ok, val, reason = cast_value(cell, col_types[c]) if not ok: - row_errors.append( - {"rowNumber": row_number, "column": columns[c], "value": cell, "reason": reason} - ) + # BUG-027: cap row_errors to keep response bodies bounded on + # pathological files. The count in the summary line still + # reflects every failed row. + if len(row_errors) < settings.MAX_ROW_ERRORS_REPORTED: + row_errors.append( + { + "rowNumber": row_number, + "column": columns[c], + "value": cell, + "reason": reason, + } + ) failed = True break values.append(val) if failed: continue - row_hash = hashlib.sha256("".join(raw_joined).encode("utf-8")).hexdigest() + # BUG-022: ASCII unit separator (U+001F) between cells prevents + # ["ab","cd"] and ["a","bcd"] from hashing to the same value. + row_hash = hashlib.sha256( + "\x1f".join(raw_joined).encode("utf-8") + ).hexdigest() if row_hash in seen: duplicates += 1 continue diff --git a/frontend/.env.example b/frontend/.env.example index 13be44c..c20be90 100644 --- a/frontend/.env.example +++ b/frontend/.env.example @@ -4,3 +4,9 @@ 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= + +# BUG-029: enable lovable.dev's window.__lovableEvents error-reporting hook. +# Off by default — self-hosted deployments should leave this unset. Only set to +# "true" if you deploy on lovable.dev's platform and want their error telemetry. +VITE_ENABLE_LOVABLE_ANALYTICS= + diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md deleted file mode 100644 index 36eb109..0000000 --- a/frontend/AGENTS.md +++ /dev/null @@ -1,10 +0,0 @@ -<!-- LOVABLE:BEGIN --> -> [!IMPORTANT] -> This project is connected to [Lovable](https://lovable.dev). Avoid rewriting -> published git history — force pushing, or rebasing/amending/squashing commits -> that are already pushed — as it rewrites history on Lovable's side and the -> user will likely lose their project history. -> -> Commits you push to the connected branch sync back to Lovable and show up in -> the editor, so keep the branch in a working state. -<!-- LOVABLE:END --> diff --git a/frontend/scripts/README.md b/frontend/scripts/README.md index 5cde17b..8300e16 100644 --- a/frontend/scripts/README.md +++ b/frontend/scripts/README.md @@ -2,14 +2,14 @@ ## Fixture CSVs (`scripts/fixtures/`) -| File | What it exercises | -| --- | --- | -| `basic.csv` | Typed inference — int8, numeric, boolean, date | -| `ambiguous-headers.csv` | Column names that used to collide with PL/pgSQL loop vars (`i`, `t`, `s`, `idx`) | -| `quoted.csv` | Quoted fields, embedded newlines, escaped `""` | -| `bad-types.csv` | Rows that should land in the row-error report, not the table | -| `dup-rows.csv` | In-file duplicate row de-duplication via `_row_hash` | -| `empty.csv` | Header-only — should surface the "must have a header row and at least one data row" error | +| File | What it exercises | +| ----------------------- | ----------------------------------------------------------------------------------------- | +| `basic.csv` | Typed inference — int8, numeric, boolean, date | +| `ambiguous-headers.csv` | Column names that used to collide with PL/pgSQL loop vars (`i`, `t`, `s`, `idx`) | +| `quoted.csv` | Quoted fields, embedded newlines, escaped `""` | +| `bad-types.csv` | Rows that should land in the row-error report, not the table | +| `dup-rows.csv` | In-file duplicate row de-duplication via `_row_hash` | +| `empty.csv` | Header-only — should surface the "must have a header row and at least one data row" error | Drop them onto the UI in one batch to exercise the preview + import + diagnostics + error export end to end. diff --git a/frontend/src/hooks/use-local-storage.ts b/frontend/src/hooks/use-local-storage.ts index 409c850..df0ef54 100644 --- a/frontend/src/hooks/use-local-storage.ts +++ b/frontend/src/hooks/use-local-storage.ts @@ -2,7 +2,9 @@ import { useCallback, useEffect, useRef, useState } from "react"; const KEY = "csv-migrator:jobs:v1"; -export function useLocalStorageState<T>(defaultValue: T): [T, (updater: T | ((prev: T) => T)) => void, () => void] { +export function useLocalStorageState<T>( + defaultValue: T, +): [T, (updater: T | ((prev: T) => T)) => void, () => void] { const [state, setState] = useState<T>(defaultValue); const hydrated = useRef(false); diff --git a/frontend/src/lib/csv-preview.ts b/frontend/src/lib/csv-preview.ts index 1c2b4e4..8ef0c22 100644 --- a/frontend/src/lib/csv-preview.ts +++ b/frontend/src/lib/csv-preview.ts @@ -23,18 +23,49 @@ function parseCsv(text: string): string[][] { const ch = src[i]; if (inQuotes) { if (ch === '"') { - if (src[i + 1] === '"') { field += '"'; i += 2; continue; } - inQuotes = false; i++; continue; + if (src[i + 1] === '"') { + field += '"'; + i += 2; + continue; + } + inQuotes = false; + i++; + continue; } - field += ch; i++; continue; + field += ch; + i++; + continue; } - if (ch === '"') { inQuotes = true; i++; continue; } - if (ch === ",") { row.push(field); field = ""; i++; continue; } - if (ch === "\r") { i++; continue; } - if (ch === "\n") { row.push(field); rows.push(row); row = []; field = ""; i++; continue; } - field += ch; i++; + if (ch === '"') { + inQuotes = true; + i++; + continue; + } + if (ch === ",") { + row.push(field); + field = ""; + i++; + continue; + } + if (ch === "\r") { + i++; + continue; + } + if (ch === "\n") { + row.push(field); + rows.push(row); + row = []; + field = ""; + i++; + continue; + } + field += ch; + i++; + } + if (field.length > 0 || row.length > 0) { + row.push(field); + rows.push(row); } - if (field.length > 0 || row.length > 0) { row.push(field); rows.push(row); } while (rows.length && rows[rows.length - 1].every((c) => c === "")) rows.pop(); return rows; } @@ -44,7 +75,8 @@ export function sanitizeColumns(headers: string[]): string[] { const seen = new Map<string, number>(); return headers.map((h, idx) => { let base = (h || `column_${idx + 1}`) - .toLowerCase().trim() + .toLowerCase() + .trim() .replace(/[^a-z0-9_]+/g, "_") .replace(/^_+|_+$/g, ""); if (!base) base = `column_${idx + 1}`; @@ -91,7 +123,10 @@ function pickType(candidates: Set<ColumnType>): ColumnType { return "text"; } -export async function parseCsvPreview(file: File, opts?: { sampleRows?: number; maxBytes?: number }): Promise<CsvPreview> { +export async function parseCsvPreview( + file: File, + opts?: { sampleRows?: number; maxBytes?: number }, +): Promise<CsvPreview> { const maxBytes = opts?.maxBytes ?? 512 * 1024; // 512KB for preview const sampleCount = opts?.sampleRows ?? 10; const inferRowLimit = 200; @@ -105,8 +140,14 @@ export async function parseCsvPreview(file: File, opts?: { sampleRows?: number; if (rows.length === 0) { return { - headers: [], sanitizedHeaders: [], sampleRows: [], inferredTypes: [], - totalRowsApprox: 0, bytesRead: slice.size, bytesTotal: file.size, truncated, + headers: [], + sanitizedHeaders: [], + sampleRows: [], + inferredTypes: [], + totalRowsApprox: 0, + bytesRead: slice.size, + bytesTotal: file.size, + truncated, }; } @@ -116,7 +157,9 @@ export async function parseCsvPreview(file: File, opts?: { sampleRows?: number; const sample = dataRows.slice(0, sampleCount); // Infer types - const perCol: Set<ColumnType>[] = headers.map(() => new Set(["int8", "numeric", "date", "timestamptz", "boolean", "text"])); + const perCol: Set<ColumnType>[] = headers.map( + () => new Set(["int8", "numeric", "date", "timestamptz", "boolean", "text"]), + ); for (let r = 0; r < Math.min(dataRows.length, inferRowLimit); r++) { const row = dataRows[r]; for (let c = 0; c < headers.length; c++) { @@ -138,8 +181,14 @@ export async function parseCsvPreview(file: File, opts?: { sampleRows?: number; } return { - headers, sanitizedHeaders: sanitized, sampleRows: sample, inferredTypes, - totalRowsApprox, bytesRead: slice.size, bytesTotal: file.size, truncated, + headers, + sanitizedHeaders: sanitized, + sampleRows: sample, + inferredTypes, + totalRowsApprox, + bytesRead: slice.size, + bytesTotal: file.size, + truncated, }; } diff --git a/frontend/src/lib/csv.functions.ts b/frontend/src/lib/csv.functions.ts index e9cc7dc..8c2f009 100644 --- a/frontend/src/lib/csv.functions.ts +++ b/frontend/src/lib/csv.functions.ts @@ -18,13 +18,12 @@ const API_KEY: string = (import.meta.env.VITE_API_KEY as string | undefined) ?? 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.", diff --git a/frontend/src/lib/lovable-error-reporting.ts b/frontend/src/lib/lovable-error-reporting.ts index d1f9b1b..bc10588 100644 --- a/frontend/src/lib/lovable-error-reporting.ts +++ b/frontend/src/lib/lovable-error-reporting.ts @@ -1,3 +1,10 @@ +// BUG-029: previously this always forwarded errors to a global +// window.__lovableEvents hook. Now gated on VITE_ENABLE_LOVABLE_ANALYTICS +// (default: false) so a self-hosted deployment doesn't leak stack traces to +// whatever code happens to define that global. Set the flag to "true" in +// frontend/.env only if you know you're on lovable.dev's platform and want +// their error-reporting integration. + type LovableErrorOptions = { mechanism?: "manual" | "onerror" | "unhandledrejection" | "react_error_boundary"; handled?: boolean; @@ -18,8 +25,12 @@ declare global { } } +const ANALYTICS_ENABLED: boolean = + (import.meta.env.VITE_ENABLE_LOVABLE_ANALYTICS as string | undefined) === "true"; + export function reportLovableError(error: unknown, context: Record<string, unknown> = {}) { if (typeof window === "undefined") return; + if (!ANALYTICS_ENABLED) return; window.__lovableEvents?.captureException?.( error, { diff --git a/frontend/src/routes/README.md b/frontend/src/routes/README.md index 441a4e8..0990d2f 100644 --- a/frontend/src/routes/README.md +++ b/frontend/src/routes/README.md @@ -7,15 +7,15 @@ is `src/routes/__root.tsx`. ## Conventions -| File | URL | -| --- | --- | -| `index.tsx` | `/` | -| `about.tsx` | `/about` | -| `users/index.tsx` | `/users` | -| `users/$id.tsx` | `/users/:id` (dynamic — bare `$`, no curly braces) | -| `posts/{-$category}.tsx` | `/posts/:category?` (optional segment) | -| `files/$.tsx` | `/files/*` (splat — read via `_splat` param, never `*`) | -| `_layout.tsx` | layout route (renders children via `<Outlet />`) | -| `__root.tsx` | app shell — wraps every page; preserve `<Outlet />` | +| File | URL | +| ------------------------ | ------------------------------------------------------- | +| `index.tsx` | `/` | +| `about.tsx` | `/about` | +| `users/index.tsx` | `/users` | +| `users/$id.tsx` | `/users/:id` (dynamic — bare `$`, no curly braces) | +| `posts/{-$category}.tsx` | `/posts/:category?` (optional segment) | +| `files/$.tsx` | `/files/*` (splat — read via `_splat` param, never `*`) | +| `_layout.tsx` | layout route (renders children via `<Outlet />`) | +| `__root.tsx` | app shell — wraps every page; preserve `<Outlet />` | `routeTree.gen.ts` is auto-generated. Don't edit it by hand. diff --git a/frontend/src/routes/__root.tsx b/frontend/src/routes/__root.tsx index 8d8bdc2..6ee7871 100644 --- a/frontend/src/routes/__root.tsx +++ b/frontend/src/routes/__root.tsx @@ -77,14 +77,23 @@ export const Route = createRootRouteWithContext<{ queryClient: QueryClient }>()( meta: [ { charSet: "utf-8" }, { name: "viewport", content: "width=device-width, initial-scale=1" }, - { title: "Lovable App" }, - { name: "description", content: "Lovable Generated Project" }, - { name: "author", content: "Lovable" }, - { property: "og:title", content: "Lovable App" }, - { property: "og:description", content: "Lovable Generated Project" }, + // BUG-024: project branding for any route without its own head: block + // (e.g. the 404 page and future routes). The CSV Migrator page at "/" + // overrides these in _authenticated/index.tsx. + { title: "CSV Migrator — PostgreDataMigrationApp" }, + { + name: "description", + content: + "Upload CSV files, preview inferred types, and load them into PostgreSQL — dynamic tables or a fixed T&E schema.", + }, + { name: "author", content: "PostgreDataMigrationApp" }, + { property: "og:title", content: "CSV Migrator — PostgreDataMigrationApp" }, + { + property: "og:description", + content: "Turn CSV files into database tables with automatic de-duplication.", + }, { property: "og:type", content: "website" }, { name: "twitter:card", content: "summary_large_image" }, - { name: "twitter:site", content: "@Lovable" }, ], links: [ { diff --git a/frontend/src/routes/_authenticated/index.tsx b/frontend/src/routes/_authenticated/index.tsx index eb23f7d..2ad1570 100644 --- a/frontend/src/routes/_authenticated/index.tsx +++ b/frontend/src/routes/_authenticated/index.tsx @@ -30,7 +30,14 @@ import { DropdownMenuItem, DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; -import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@/components/ui/table"; +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from "@/components/ui/table"; import { Badge } from "@/components/ui/badge"; import { toast, Toaster } from "sonner"; import { @@ -82,7 +89,7 @@ export const Route = createFileRoute("/_authenticated/")({ } 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); } }, @@ -117,13 +124,18 @@ function Home() { <Badge variant="secondary" className="hidden sm:inline-flex"> {files.length} file{files.length === 1 ? "" : "s"} migrated </Badge> - </div> </div> </header> <main className="mx-auto max-w-5xl space-y-8 px-6 py-10"> - {error ? <BackendUnreachableBanner error={error} isRetrying={isFetching} onRetry={() => refetch()} /> : null} + {error ? ( + <BackendUnreachableBanner + error={error} + isRetrying={isFetching} + onRetry={() => refetch()} + /> + ) : null} <Uploader /> {isLoading && !error ? ( <div className="flex items-center justify-center py-12 text-muted-foreground"> @@ -157,12 +169,15 @@ function BackendUnreachableBanner({ </p> <p className="text-xs text-muted-foreground">{message}</p> <p className="text-xs text-muted-foreground"> - Start the API with{" "} - <code className="rounded bg-muted px-1">scripts/start-api.ps1</code>{" "} + Start the API with <code className="rounded bg-muted px-1">scripts/start-api.ps1</code>{" "} (Terminal 1), then click Retry. Uploads are disabled until the backend is back. </p> <Button size="sm" variant="outline" onClick={onRetry} disabled={isRetrying}> - {isRetrying ? <Loader2 className="mr-1 h-3 w-3 animate-spin" /> : <RotateCw className="mr-1 h-3 w-3" />} + {isRetrying ? ( + <Loader2 className="mr-1 h-3 w-3 animate-spin" /> + ) : ( + <RotateCw className="mr-1 h-3 w-3" /> + )} Retry </Button> </div> @@ -173,15 +188,8 @@ function BackendUnreachableBanner({ // Auth removed — the app talks to the local FastAPI backend with no login. - type JobStatus = - | "reading" - | "uploading" - | "processing" - | "done" - | "duplicate" - | "error" - | "interrupted"; + "reading" | "uploading" | "processing" | "done" | "duplicate" | "error" | "interrupted"; type Job = { id: string; @@ -222,10 +230,7 @@ const STATUS_LABEL: Record<JobStatus, string> = { interrupted: "Interrupted", }; -function readFileWithProgress( - file: File, - onProgress: (pct: number) => void, -): Promise<string> { +function readFileWithProgress(file: File, onProgress: (pct: number) => void): Promise<string> { return new Promise((resolve, reject) => { const reader = new FileReader(); reader.onprogress = (e) => { @@ -233,8 +238,7 @@ function readFileWithProgress( onProgress(Math.round((e.loaded / e.total) * 60)); } }; - reader.onerror = () => - reject(reader.error ?? new Error("Could not read the file.")); + reader.onerror = () => reject(reader.error ?? new Error("Could not read the file.")); reader.onload = () => resolve(String(reader.result ?? "")); reader.readAsText(file); }); @@ -269,9 +273,7 @@ function Uploader() { // eslint-disable-next-line react-hooks/exhaustive-deps }, []); - const running = jobs.some((j) => - ["reading", "uploading", "processing"].includes(j.status), - ); + const running = jobs.some((j) => ["reading", "uploading", "processing"].includes(j.status)); const updateJob = useCallback( (id: string, patch: Partial<Job>) => { @@ -314,7 +316,9 @@ function Uploader() { const prefix = res.overwritten ? "overwrote previous upload · " : ""; toast.success( `${job.name}: ${prefix}${res.insertedRows} of ${res.totalRows} rows imported` + - (res.duplicateRowsSkipped ? ` · ${res.duplicateRowsSkipped} duplicate rows skipped` : "") + + (res.duplicateRowsSkipped + ? ` · ${res.duplicateRowsSkipped} duplicate rows skipped` + : "") + (res.failedRows ? ` · ${res.failedRows} failed` : ""), ); await router.invalidate(); @@ -393,31 +397,28 @@ function Uploader() { [runJob, setJobs], ); - const handleFiles = useCallback( - async (files: File[]) => { - const csvs = files.filter( - (f) => f.name.toLowerCase().endsWith(".csv") || f.type === "text/csv", - ); - if (csvs.length === 0) { - toast.error("Please drop .csv files."); - return; - } - const batchId = `b-${Date.now()}-${Math.random().toString(36).slice(2, 6)}`; - const previews: PendingPreview[] = []; - for (const f of csvs) { - try { - const p = await parseCsvPreview(f); - previews.push({ file: f, preview: p, batchId }); - } catch (err) { - toast.error(`${f.name}: ${(err as Error).message}`); - } + const handleFiles = useCallback(async (files: File[]) => { + const csvs = files.filter( + (f) => f.name.toLowerCase().endsWith(".csv") || f.type === "text/csv", + ); + if (csvs.length === 0) { + toast.error("Please drop .csv files."); + return; + } + const batchId = `b-${Date.now()}-${Math.random().toString(36).slice(2, 6)}`; + const previews: PendingPreview[] = []; + for (const f of csvs) { + try { + const p = await parseCsvPreview(f); + previews.push({ file: f, preview: p, batchId }); + } catch (err) { + toast.error(`${f.name}: ${(err as Error).message}`); } - if (previews.length === 0) return; - setPending(previews); - setPreviewIndex(0); - }, - [], - ); + } + if (previews.length === 0) return; + setPending(previews); + setPreviewIndex(0); + }, []); const confirmAll = useCallback(async () => { const items = pending; @@ -548,17 +549,37 @@ function formatBytes(bytes: number): string { function StatusPill({ status }: { status: JobStatus }) { const map: Record<JobStatus, { cls: string; icon: ReactNode }> = { - reading: { cls: "bg-muted text-muted-foreground", icon: <Loader2 className="h-3 w-3 animate-spin" /> }, - uploading: { cls: "bg-muted text-muted-foreground", icon: <Loader2 className="h-3 w-3 animate-spin" /> }, - processing: { cls: "bg-muted text-muted-foreground", icon: <Loader2 className="h-3 w-3 animate-spin" /> }, - done: { cls: "bg-emerald-500/10 text-emerald-700 dark:text-emerald-400", icon: <CheckCircle2 className="h-3 w-3" /> }, - duplicate: { cls: "bg-amber-500/10 text-amber-700 dark:text-amber-400", icon: <AlertTriangle className="h-3 w-3" /> }, + reading: { + cls: "bg-muted text-muted-foreground", + icon: <Loader2 className="h-3 w-3 animate-spin" />, + }, + uploading: { + cls: "bg-muted text-muted-foreground", + icon: <Loader2 className="h-3 w-3 animate-spin" />, + }, + processing: { + cls: "bg-muted text-muted-foreground", + icon: <Loader2 className="h-3 w-3 animate-spin" />, + }, + done: { + cls: "bg-emerald-500/10 text-emerald-700 dark:text-emerald-400", + icon: <CheckCircle2 className="h-3 w-3" />, + }, + duplicate: { + cls: "bg-amber-500/10 text-amber-700 dark:text-amber-400", + icon: <AlertTriangle className="h-3 w-3" />, + }, error: { cls: "bg-destructive/10 text-destructive", icon: <XCircle className="h-3 w-3" /> }, - interrupted: { cls: "bg-slate-500/10 text-slate-600 dark:text-slate-400", icon: <PauseCircle className="h-3 w-3" /> }, + interrupted: { + cls: "bg-slate-500/10 text-slate-600 dark:text-slate-400", + icon: <PauseCircle className="h-3 w-3" />, + }, }; const cfg = map[status]; return ( - <span className={`inline-flex items-center gap-1 rounded-full px-2 py-0.5 text-xs font-medium ${cfg.cls}`}> + <span + className={`inline-flex items-center gap-1 rounded-full px-2 py-0.5 text-xs font-medium ${cfg.cls}`} + > {cfg.icon} {STATUS_LABEL[status]} </span> @@ -593,7 +614,9 @@ function BatchSummary({ jobs }: { jobs: Job[] }) { const failedRows = batchJobs.reduce((s, j) => s + (j.failedRows ?? 0), 0); const dupFiles = batchJobs.filter((j) => j.status === "duplicate").length; const failedFiles = batchJobs.filter((j) => j.status === "error").length; - const inProgress = batchJobs.filter((j) => ["reading", "uploading", "processing"].includes(j.status)).length; + const inProgress = batchJobs.filter((j) => + ["reading", "uploading", "processing"].includes(j.status), + ).length; const stat = (label: string, value: number, cls?: string) => ( <div className="flex flex-col"> @@ -674,7 +697,11 @@ function DiagnosticsPanel({ jobs }: { jobs: Job[] }) { </div> <CollapsibleTrigger asChild> <Button size="sm" variant="ghost" className="h-7 px-2 text-xs"> - {open ? <ChevronDown className="mr-1 h-3 w-3" /> : <ChevronRight className="mr-1 h-3 w-3" />} + {open ? ( + <ChevronDown className="mr-1 h-3 w-3" /> + ) : ( + <ChevronRight className="mr-1 h-3 w-3" /> + )} {open ? "Hide" : "Show"} details </Button> </CollapsibleTrigger> @@ -711,7 +738,9 @@ function DiagnosticsPanel({ jobs }: { jobs: Job[] }) { </td> <td className="max-w-[160px] truncate px-2 py-1">{e.file}</td> <td className="px-2 py-1 capitalize">{e.step.replace(/_/g, " ")}</td> - <td className={`px-2 py-1 ${e.level === "error" ? "text-destructive" : "text-amber-600 dark:text-amber-400"}`}> + <td + className={`px-2 py-1 ${e.level === "error" ? "text-destructive" : "text-amber-600 dark:text-amber-400"}`} + > {e.message} </td> </tr> @@ -720,15 +749,15 @@ function DiagnosticsPanel({ jobs }: { jobs: Job[] }) { </table> </div> ) : ( - <p className="text-xs text-muted-foreground">No warnings or errors in the latest batch.</p> + <p className="text-xs text-muted-foreground"> + No warnings or errors in the latest batch. + </p> )} </CollapsibleContent> </Collapsible> ); } - - function UploadReport({ jobs, running, @@ -818,9 +847,7 @@ function JobRow({ <div className="min-w-0 flex-1"> <div className="flex flex-wrap items-center gap-2"> <span className="truncate text-sm font-medium">{job.name}</span> - <span className="text-xs text-muted-foreground"> - {formatBytes(job.size)} - </span> + <span className="text-xs text-muted-foreground">{formatBytes(job.size)}</span> <div className="ml-auto flex items-center gap-2"> <StatusPill status={job.status} /> {canExport && <ExportErrorsMenu job={job} />} @@ -835,7 +862,12 @@ function JobRow({ </Button> )} {(isError || isDuplicate || isInterrupted) && ( - <Button size="sm" variant="ghost" className="h-7 px-2" onClick={() => onRetry(job.id)}> + <Button + size="sm" + variant="ghost" + className="h-7 px-2" + onClick={() => onRetry(job.id)} + > <RotateCw className="mr-1 h-3 w-3" /> Retry </Button> )} @@ -863,11 +895,16 @@ function JobRow({ <p className="mt-2 text-xs text-muted-foreground"> {job.overwritten ? "Overwrote previous upload · " : ""} Imported {job.insertedRows} of {job.totalRows ?? job.insertedRows} rows - {job.duplicateRowsSkipped ? ` · ${job.duplicateRowsSkipped} duplicate rows skipped` : ""} - {job.failedRows ? ` · ${job.failedRows} row${job.failedRows === 1 ? "" : "s"} failed validation` : ""} + {job.duplicateRowsSkipped + ? ` · ${job.duplicateRowsSkipped} duplicate rows skipped` + : ""} + {job.failedRows + ? ` · ${job.failedRows} row${job.failedRows === 1 ? "" : "s"} failed validation` + : ""} {job.tableName ? ( <> - {" "}· stored in <code className="rounded bg-muted px-1">{job.tableName}</code> + {" "} + · stored in <code className="rounded bg-muted px-1">{job.tableName}</code> </> ) : null} </p> @@ -877,20 +914,30 @@ function JobRow({ <p className="mt-2 text-xs text-amber-700 dark:text-amber-400"> {job.duplicateReason === "name" ? ( <> - A file named <span className="font-medium">{job.name}</span> is already imported with different contents - {typeof job.existingRowCount === "number" ? <> ({job.existingRowCount} rows)</> : null}. - Rename the file to keep both, or click <span className="font-medium">Overwrite</span> to replace it. + A file named <span className="font-medium">{job.name}</span> is already imported + with different contents + {typeof job.existingRowCount === "number" ? ( + <> ({job.existingRowCount} rows)</> + ) : null} + . Rename the file to keep both, or click{" "} + <span className="font-medium">Overwrite</span> to replace it. </> ) : ( <> Already imported — this file's contents are identical to {job.existingFileName ? ( - <> <span className="font-medium">"{job.existingFileName}"</span></> + <> + {" "} + <span className="font-medium">"{job.existingFileName}"</span> + </> ) : ( <> a previously uploaded file</> )} - {typeof job.existingRowCount === "number" ? <> ({job.existingRowCount} rows)</> : null} - . Skipped automatically — click <span className="font-medium">Overwrite</span> to re-import. + {typeof job.existingRowCount === "number" ? ( + <> ({job.existingRowCount} rows)</> + ) : null} + . Skipped automatically — click <span className="font-medium">Overwrite</span> to + re-import. </> )} </p> @@ -960,9 +1007,18 @@ function ExportErrorsMenu({ job }: { job: Job }) { const ts = new Date().toISOString().replace(/[:.]/g, "-"); const safeName = job.name.replace(/\.csv$/i, "").replace(/[^a-z0-9_-]+/gi, "_"); if (format === "json") { - blob = new Blob([JSON.stringify({ file: job.name, generatedAt: new Date().toISOString(), entries }, null, 2)], { - type: "application/json", - }); + blob = new Blob( + [ + JSON.stringify( + { file: job.name, generatedAt: new Date().toISOString(), entries }, + null, + 2, + ), + ], + { + type: "application/json", + }, + ); filename = `errors-${safeName}-${ts}.json`; } else { const header = ["file", "row_number", "column", "value", "reason", "plain_english"]; @@ -972,7 +1028,9 @@ function ExportErrorsMenu({ job }: { job: Job }) { }; const lines = [header.join(",")].concat( entries.map((e) => - [e.file, e.rowNumber || "", e.column ?? "", e.value ?? "", e.reason, e.plainEnglish].map(escape).join(","), + [e.file, e.rowNumber || "", e.column ?? "", e.value ?? "", e.reason, e.plainEnglish] + .map(escape) + .join(","), ), ); blob = new Blob([lines.join("\n")], { type: "text/csv" }); @@ -1013,7 +1071,8 @@ function ExportErrorsMenu({ job }: { job: Job }) { toast.success("Logs downloaded"); }; - const hasErrorContent = !!job.errorMessage || job.status === "duplicate" || (job.rowErrors?.length ?? 0) > 0; + const hasErrorContent = + !!job.errorMessage || job.status === "duplicate" || (job.rowErrors?.length ?? 0) > 0; const hasLogs = (job.logs?.length ?? 0) > 0; return ( @@ -1038,7 +1097,6 @@ function ExportErrorsMenu({ job }: { job: Job }) { ); } - function ErrorPanel({ job, open, @@ -1075,7 +1133,11 @@ function ErrorPanel({ <div className="flex items-center gap-2"> <CollapsibleTrigger asChild> <Button size="sm" variant="outline" className="h-7 px-2 text-xs"> - {open ? <ChevronDown className="mr-1 h-3 w-3" /> : <ChevronRight className="mr-1 h-3 w-3" />} + {open ? ( + <ChevronDown className="mr-1 h-3 w-3" /> + ) : ( + <ChevronRight className="mr-1 h-3 w-3" /> + )} {open ? "Hide" : "Show"} technical details </Button> </CollapsibleTrigger> @@ -1110,7 +1172,8 @@ function ErrorPanel({ </table> {job.rowErrors!.length > 50 && ( <p className="p-2 text-[11px] text-muted-foreground"> - Showing first 50 of {job.rowErrors!.length} row errors — download the full report above. + Showing first 50 of {job.rowErrors!.length} row errors — download the full + report above. </p> )} </div> @@ -1125,23 +1188,35 @@ function ErrorPanel({ function hintFor(message: string): string | null { const m = message.toLowerCase(); - if (m.includes("header row")) return "The file must include a header row followed by at least one data row."; - if (m.includes("no columns")) return "No usable column names were found in the first row of the CSV."; - if (m.includes("invalid table name")) return "The server rejected the auto-generated table name. Try re-uploading the file."; - if (m.includes("could not create table")) return "The database refused to create a table for this file. Check that column names in the header row are valid."; - if (m.includes("insert failed")) return "Some rows could not be inserted. The most common cause is inconsistent numbers of columns across rows."; - if (m.includes("could not register file")) return "The file was processed but its registry entry could not be written. Try again."; - if (m.includes("network") || m.includes("fetch")) return "A network error interrupted the upload. Check your connection and retry."; + if (m.includes("header row")) + return "The file must include a header row followed by at least one data row."; + if (m.includes("no columns")) + return "No usable column names were found in the first row of the CSV."; + if (m.includes("invalid table name")) + return "The server rejected the auto-generated table name. Try re-uploading the file."; + if (m.includes("could not create table")) + return "The database refused to create a table for this file. Check that column names in the header row are valid."; + if (m.includes("insert failed")) + return "Some rows could not be inserted. The most common cause is inconsistent numbers of columns across rows."; + if (m.includes("could not register file")) + return "The file was processed but its registry entry could not be written. Try again."; + if (m.includes("network") || m.includes("fetch")) + return "A network error interrupted the upload. Check your connection and retry."; return null; } function hintForRow(re: RowError): string | null { const r = re.reason.toLowerCase(); - if (r.includes("whole number")) return `Row ${re.rowNumber}: column "${re.column}" expected a whole number but got "${re.value}".`; - if (r.includes("not a number")) return `Row ${re.rowNumber}: column "${re.column}" expected a number but got "${re.value}".`; - if (r.includes("true/false")) return `Row ${re.rowNumber}: column "${re.column}" expected true/false but got "${re.value}".`; - if (r.includes("date")) return `Row ${re.rowNumber}: column "${re.column}" expected a valid date but got "${re.value}".`; - if (r.includes("timestamp")) return `Row ${re.rowNumber}: column "${re.column}" expected a valid timestamp but got "${re.value}".`; + if (r.includes("whole number")) + return `Row ${re.rowNumber}: column "${re.column}" expected a whole number but got "${re.value}".`; + if (r.includes("not a number")) + return `Row ${re.rowNumber}: column "${re.column}" expected a number but got "${re.value}".`; + if (r.includes("true/false")) + return `Row ${re.rowNumber}: column "${re.column}" expected true/false but got "${re.value}".`; + if (r.includes("date")) + return `Row ${re.rowNumber}: column "${re.column}" expected a valid date but got "${re.value}".`; + if (r.includes("timestamp")) + return `Row ${re.rowNumber}: column "${re.column}" expected a valid timestamp but got "${re.value}".`; return `Row ${re.rowNumber}${re.column ? ` (${re.column})` : ""}: ${re.reason}`; } @@ -1193,7 +1268,6 @@ function PreviewDialog({ </div> <div className="min-h-0 flex-1 overflow-auto rounded border"> - <Table> <TableHeader className="sticky top-0 z-10 bg-background"> <TableRow> @@ -1202,7 +1276,9 @@ function PreviewDialog({ <div className="flex flex-col gap-0.5 py-1"> <span className="text-xs font-semibold">{h}</span> {preview.headers[i] !== h && ( - <span className="text-[10px] text-muted-foreground">from "{preview.headers[i]}"</span> + <span className="text-[10px] text-muted-foreground"> + from "{preview.headers[i]}" + </span> )} <span className="mt-1 inline-flex w-fit rounded bg-primary/10 px-1.5 py-0.5 text-[10px] font-medium text-primary"> {COLUMN_TYPE_LABEL[preview.inferredTypes[i]]} @@ -1224,7 +1300,10 @@ function PreviewDialog({ ))} {preview.sampleRows.length === 0 && ( <TableRow> - <TableCell colSpan={preview.sanitizedHeaders.length} className="text-center text-xs text-muted-foreground"> + <TableCell + colSpan={preview.sanitizedHeaders.length} + className="text-center text-xs text-muted-foreground" + > No data rows found. </TableCell> </TableRow> @@ -1234,8 +1313,9 @@ function PreviewDialog({ </div> <p className="text-xs text-muted-foreground"> - Column names are sanitized for the database. Detected types come from the first {Math.min(preview.sampleRows.length, 200)} rows. - Values that don't match a column's type will be reported as row errors instead of being imported. + Column names are sanitized for the database. Detected types come from the first{" "} + {Math.min(preview.sampleRows.length, 200)} rows. Values that don't match a column's type + will be reported as row errors instead of being imported. </p> </div> @@ -1244,7 +1324,12 @@ function PreviewDialog({ <Button variant="outline" size="sm" onClick={onPrev} disabled={index === 0}> Previous </Button> - <Button variant="outline" size="sm" onClick={onNext} disabled={index >= pending.length - 1}> + <Button + variant="outline" + size="sm" + onClick={onNext} + disabled={index >= pending.length - 1} + > Next </Button> </div> @@ -1297,11 +1382,14 @@ function FilesList({ files }: { files: CsvFileSummary[] }) { <TableCell> <code className="rounded bg-muted px-1.5 py-0.5 text-xs">{f.table_name}</code> </TableCell> - <TableCell className="text-right tabular-nums">{f.column_names.length}</TableCell> + <TableCell className="text-right tabular-nums"> + {f.column_names.length} + </TableCell> <TableCell className="text-right tabular-nums">{f.row_count}</TableCell> <TableCell className="text-muted-foreground"> <time dateTime={f.created_at} suppressHydrationWarning> - {new Date(f.created_at).toISOString().replace("T", " ").slice(0, 19) + " UTC"} + {new Date(f.created_at).toISOString().replace("T", " ").slice(0, 19) + + " UTC"} </time> </TableCell> <TableCell className="text-right"> @@ -1321,9 +1409,7 @@ function FilesList({ files }: { files: CsvFileSummary[] }) { <DialogContent className="flex max-h-[90vh] w-[95vw] max-w-5xl flex-col overflow-hidden"> <DialogHeader> <DialogTitle className="truncate">{preview?.file_name}</DialogTitle> - <DialogDescription> - First rows of the imported table. - </DialogDescription> + <DialogDescription>First rows of the imported table.</DialogDescription> </DialogHeader> <div className="min-h-0 flex-1 overflow-auto"> {preview ? <PreviewTable file={preview} /> : null} @@ -1373,7 +1459,10 @@ function PreviewTable({ file }: { file: CsvFileSummary }) { ))} {rows.length === 0 && ( <TableRow> - <TableCell colSpan={file.column_names.length} className="text-center text-muted-foreground"> + <TableCell + colSpan={file.column_names.length} + className="text-center text-muted-foreground" + > No rows. </TableCell> </TableRow> @@ -1381,7 +1470,8 @@ function PreviewTable({ file }: { file: CsvFileSummary }) { </TableBody> </Table> <p className="p-2 text-xs text-muted-foreground"> - Showing up to 50 rows. Table has {file.row_count} row{file.row_count === 1 ? "" : "s"} total. + Showing up to 50 rows. Table has {file.row_count} row{file.row_count === 1 ? "" : "s"}{" "} + total. </p> </div> ); diff --git a/tests/test_api.py b/tests/test_api.py index f7865b7..d7696ab 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -233,6 +233,66 @@ def test_in_file_duplicate_rows_are_skipped(self): self.assertEqual(body["insertedRows"], 1) self.assertEqual(body["duplicateRowsSkipped"], 1) + def test_row_hash_does_not_collide_on_split_boundaries(self): + """BUG-022 regression: `["ab","cd"]` and `["a","bcd"]` used to hash to + the same value because the pre-hash `"".join(cells)` had no separator. + With the fixed `"\\x1f".join(...)` they must be treated as distinct + rows and both land in the table. + """ + # Prefix each cell with the PID so this run's rows don't collide with a + # sibling run's registry entry via content-hash dedup. The interesting + # payload is the split-boundary pair. + pid = os.getpid() + content = ( + "col_a,col_b\n" + f"{pid}ab,cd\n" + f"{pid}a,bcd\n" + ) + r = self.client.post( + "/api/csv/upload", + json={"fileName": self.name, "content": content}, + ) + body = r.json() + self.assertEqual(body["status"], "ok", body) + self.assertEqual( + body["insertedRows"], 2, + "BUG-022 regression: split-boundary rows were deduplicated", + ) + self.assertEqual(body["duplicateRowsSkipped"], 0) + + # Confirm the physical rows landed too, not just the summary counters. + rows = self.client.get( + f"/api/csv/tables/{body['tableName']}/rows", + params={"limit": 10}, + ) + self.assertEqual(rows.status_code, 200) + payload = rows.json()["rows"] + self.assertEqual(len(payload), 2) + pairs = sorted((r["col_a"], r["col_b"]) for r in payload) + self.assertEqual( + pairs, + sorted([(f"{pid}ab", "cd"), (f"{pid}a", "bcd")]), + ) + + def test_row_hash_still_dedupes_identical_rows(self): + """Guard-rail for BUG-022 fix: the separator change must not break the + intended dedup — two byte-identical rows still count as one insert. + """ + pid = os.getpid() + content = ( + "col_a,col_b\n" + f"{pid}xy,{pid}z\n" + f"{pid}xy,{pid}z\n" + ) + r = self.client.post( + "/api/csv/upload", + json={"fileName": self.name, "content": content}, + ) + body = r.json() + self.assertEqual(body["status"], "ok", body) + self.assertEqual(body["insertedRows"], 1) + self.assertEqual(body["duplicateRowsSkipped"], 1) + def test_unregistered_table_is_404(self): r = self.client.get("/api/csv/tables/csv_0000000000000000/rows") self.assertEqual(r.status_code, 404) diff --git a/tests/test_ci_quality_gate.py b/tests/test_ci_quality_gate.py index c08d208..56c3276 100644 --- a/tests/test_ci_quality_gate.py +++ b/tests/test_ci_quality_gate.py @@ -167,13 +167,30 @@ def test_postgres_step_uses_pg_ctl(self): class TestAllJobsHaveConsistentStructure(unittest.TestCase): - """All jobs must have checkout, python setup, and artifact upload.""" + """Every job must have checkout + install-deps. Python jobs additionally + need Python setup and an artifact upload; Node jobs need Node setup. + + BUG-023 added the Node-backed ``frontend-build`` job, so the old + "every job needs Python" contract needed to split by job kind. Any new + job MUST be classified in PYTHON_JOBS or NODE_JOBS below — an + unclassified job fails setUpClass, so a contributor can't quietly bypass + the structural rules. + """ + + PYTHON_JOBS = {"free-tier", "integration-postgres", "windows-postgres"} + NODE_JOBS = {"frontend-build"} @classmethod def setUpClass(cls): if not WORKFLOW.exists(): raise AssertionError(f"Workflow file not found: {WORKFLOW}") cls.jobs = _load_workflow().get("jobs", {}) + unclassified = set(cls.jobs) - cls.PYTHON_JOBS - cls.NODE_JOBS + if unclassified: + raise AssertionError( + f"quality-gate.yml has unclassified jobs: {sorted(unclassified)}. " + f"Add each to PYTHON_JOBS or NODE_JOBS in {__file__}." + ) def _step_names(self, job_id: str) -> list[str]: return [s.get("name", "") for s in self.jobs[job_id].get("steps", [])] @@ -186,28 +203,46 @@ def test_all_jobs_have_checkout(self): f"Job {job_id!r} missing Checkout step", ) - def test_all_jobs_have_python_setup(self): + def test_all_jobs_install_deps(self): + # "Install dependencies" (Node) and "Install dev dependencies" (Python) + # both match: literal "Install" plus lowercase "dep". for job_id in self.jobs: names = self._step_names(job_id) + self.assertTrue( + any("Install" in n and "dep" in n.lower() for n in names), + f"Job {job_id!r} missing install dependencies step", + ) + + def test_python_jobs_have_python_setup(self): + for job_id in self.PYTHON_JOBS: + if job_id not in self.jobs: + continue # covered by TestQualityGateStructure + names = self._step_names(job_id) self.assertTrue( any("Setup Python" in n or "Python" in n for n in names), - f"Job {job_id!r} missing Python setup step", + f"Python job {job_id!r} missing Python setup step", ) - def test_all_jobs_have_artifact_upload(self): - for job_id in self.jobs: + def test_python_jobs_have_artifact_upload(self): + # Python jobs run evals and produce reports worth keeping — every one + # must upload artifacts. Node jobs produce ephemeral build output. + for job_id in self.PYTHON_JOBS: + if job_id not in self.jobs: + continue names = self._step_names(job_id) self.assertTrue( any("Upload" in n for n in names), - f"Job {job_id!r} missing artifact upload step", + f"Python job {job_id!r} missing artifact upload step", ) - def test_all_jobs_install_dev_deps(self): - for job_id in self.jobs: + def test_node_jobs_have_node_setup(self): + for job_id in self.NODE_JOBS: + if job_id not in self.jobs: + continue names = self._step_names(job_id) self.assertTrue( - any("Install" in n and "dep" in n.lower() for n in names), - f"Job {job_id!r} missing install dev dependencies step", + any("Setup Node" in n or "Node" in n for n in names), + f"Node job {job_id!r} missing Node setup step", )