# QuitSure Discovery — Enterprise Audit

_Generated: 2026-07-17 · Scope: Python backend (`app.py`, `db.py`, `simulate.py`) + repo hygiene, deps, config, docs, DevOps. Frontend (`web/`, TypeScript) sampled for the API contract only._

## Executive Summary

**Overall health: Fair (leaning Poor on ops/testing).**

The Python surface is genuinely small — **three files, ~600 LOC**: `app.py` (a 97-line FastAPI shim), `db.py` (179 lines of MySQL access), and `simulations/simulate.py` (a content simulator, not app code). The engineering *within* those files is above-average: parameterized SQL, whitelisted columns, input sanity-bounding, timeouts, thoughtful docstrings, and a deliberate "never break the funnel" resilience philosophy.

The problems are almost all *around* the code, not in it:

1. **The repo's documented architecture is wrong.** `CLAUDE.md` describes a Jinja2 + SQLite chatbot with `templates/chat.html` and `static/content.json`. The **actual** `app.py` imports neither — it serves a **Next.js static export** (`web/out`) and adds four MySQL-backed API routes. `templates/chat.html` (130 KB) and `static/content.json` (67 KB) are **dead code the server never touches**.
2. **A build artifact directory (`web/out/`, 70 files, 4.7 MB) is committed and not git-ignored,** with the image assets duplicated across *three* trees.
3. **No logging, no tests, no CI, no containerization.**
4. **The analytics write endpoints are client-authoritative** — anyone can POST `bConverted=1`.

None of these is catastrophic (this is a low-risk, best-effort public funnel with no auth by design), but collectively they put it below production-ready.

---

## Scorecard

| Category | Score (/100) |
|---|---|
| Project Structure | 58 |
| Code Quality | 72 |
| Python Best Practices | 65 |
| Architecture | 68 |
| Security | 55 |
| Performance | 72 |
| Environment Variables | 70 |
| Dependency Management | 55 |
| Error Handling | 55 |
| Logging & Monitoring | 20 |
| API Design | 58 |
| Database | 68 |
| Testing | 10 |
| Documentation | 62 |
| DevOps | 35 |
| Git Hygiene | 40 |
| Maintainability | 58 |
| Production Readiness | 48 |

### Overall Score: **56 / 100 — Grade C**

Weighting (security, production-readiness, maintainability, testing carry extra weight; API/DB/logging weighted to applicability) lands at **~56**. The code you *wrote* is a B; the repository *around* it is a D, and the average is a C.

**Excluded / N/A categories:** none excluded, but note — there is **no authentication/authorization system by design** (public funnel; Razorpay owns payment identity per `DISCOVERY_BE_BRIEF.md`), and **no ORM** (raw parameterized SQL is a deliberate, acceptable choice at this size). These are scored on what exists, not penalized for absence-by-design.

---

## Findings

### 🔴 High

**H1 — Client-authoritative writes to business tables (metric spoofing)**
`app.py:79-87` → `db.py:119-154`, whitelist at `db.py:108-116`
`POST /api/session` accepts `bConverted`, `iUserID`, `bReachedOffer`, `vConvertProgram` straight from the request body and writes them to `tbl_DiscoveryOnboarding`, setting `dConvertedAt` server-side when the flag flips. There is no auth, no signature, no cross-check against an actual payment. Anyone can `curl` a fake conversion, and per `DISCOVERY_BE_BRIEF.md:36-37` conversions are attributed by joining this `iUserID` to `tbl_UserPrograms` — so a spoofed `iUserID` pollutes attribution.
- **Why it matters:** Your conversion analytics — the entire point of the funnel — are forgeable. Real conversion is measured via the Razorpay pixel/`tbl_UserPrograms`, so treat `bConverted` here as a *soft signal only* and never as source-of-truth.
- **Fix:** Document `tbl_DiscoveryOnboarding.bConverted` as unverified client intent; do conversion reporting exclusively via the `iUserID` → `tbl_UserPrograms` join (which you already plan). Optionally drop `bConverted`/`iUserID` from the client-writable whitelist and set `iUserID` server-side from the `?u=` hash lookup instead of trusting the body.

**H2 — Committed build artifacts + tripled assets (git hygiene)**
`.gitignore` (no `web/out`), `web/out/` = 70 tracked files / 4.7 MB
`web/out/` is a generated Next.js export, committed to git and **not** ignored (`git check-ignore web/out/index.html` → not ignored). The image assets exist in **three** places: `static/assets` (1.9 MB), `web/public/assets` (3.4 MB), `web/out/assets` (3.4 MB). Every rebuild churns 70 binary/JS files with hashed names, guaranteeing noisy diffs and merge conflicts.
- **Why it matters:** Repo bloat, unreviewable diffs, and ambiguity about whether `web/out` or a fresh build is deployed. `static/assets` is entirely dead (the server mounts `web/out`, never `static/`).
- **Fix:** `git rm -r --cached web/out && echo "web/out/" >> web/.gitignore`; build in CI/deploy instead. Delete the dead `static/` tree. Keep assets in `web/public/assets` only.

**H3 — No logging anywhere; all errors silently swallowed**
`db.py:103`, `db.py:153`, `app.py:53-55`, `app.py:75-76`, `app.py:86-87`
Every DB path is wrapped in `except Exception: return <falsy>`. The intent ("never break the funnel") is correct, but there is **zero** logging — not even at ERROR level. If MySQL is down, credentials rotate, or `tbl_DiscoveryClickstream` is dropped, every event silently vanishes and no one knows.
- **Why it matters:** Undiagnosable data loss. You'd discover a broken pipeline only when analytics come back empty days later.
- **Fix:** Add `import logging; log = logging.getLogger("discovery")` and `log.exception(...)` inside each `except` before returning the safe fallback. Configure a handler in `app.py`. Cost: ~1 hour, enormous observability gain.

**H4 — Zero automated tests (~0% coverage)**
No `test_*.py`, no `*.test.ts`. `simulate.py` is a content generator, not a test.
- **Why it matters:** The whitelist/NULL-handling logic in `upsert_session` and the personalization branching in `get_profile` (currency inference, under-18 exclusion, name/gender normalization) are exactly the kind of subtle logic that silently rots. A regression in `_under_18` or the `currency` inference ships unnoticed.
- **Fix:** Start with pure-function unit tests (no DB needed) for `_first_name`, `_gender`, `_cigs`, `_under_18`, and `get_profile`'s currency logic; then a couple of API tests with `TestClient` + a mocked `db`. High value, low effort.

### 🟡 Medium

**M1 — Documentation describes a different, dead architecture**
`CLAUDE.md` (whole "Architecture"/"Key Endpoints"/"Content Structure" sections)
CLAUDE.md documents Jinja2 templates, SQLite, `/u/{session_id}`, `/debug`, `content.json` screen types — **none of which `app.py` implements anymore**. The real endpoints are `/api/me`, `/api/config`, `/api/track-batch`, `/api/session`. A new engineer would be actively misled.
- **Fix:** Rewrite CLAUDE.md to match the shim + Next.js reality (much of `DISCOVERY_BE_BRIEF.md` is already accurate and can be promoted). Delete the dead `templates/` and `static/` trees so the docs and repo agree.

**M2 — Unused dependencies + hand-rolled `.env` parser**
`requirements.txt`, `app.py:29-36`
`jinja2` (no templates rendered), `aiofiles` (Starlette's `StaticFiles` uses `anyio`, not aiofiles), and `python-dotenv` are all listed but unused — yet `app.py` hand-rolls its own `.env` loader that ignores quoting, `export ` prefixes, and multi-line values. You shipped a dotenv dependency and then reimplemented it worse.
- **Fix:** Drop `jinja2`, `aiofiles`. Replace the manual loader with `from dotenv import load_dotenv; load_dotenv()`, or graduate to `pydantic-settings` for typed, validated config.

**M3 — Non-reproducible dependency pinning + risky exact pin**
`requirements.txt`
All deps use `>=` floors (no upper bounds, no lockfile) → `pip install` today ≠ next month. Worse, `starlette==0.47.3` is pinned *exactly* while `fastapi>=0.104.0` floats — FastAPI pins its own compatible Starlette range, so an exact Starlette pin will eventually conflict and block FastAPI upgrades.
- **Fix:** Add a `pyproject.toml` with a resolved lock (`pip-tools`/`uv`), or at minimum pin compatible ranges and let FastAPI own the Starlette version.

**M4 — No rate limiting on public write endpoints**
`app.py:68`, `app.py:79`
`/api/track-batch` and `/api/session` are unauthenticated writes. Length/count caps in `db.py:82-91` (100 events/req, 2000-char answers) bound a single request, but nothing bounds *request volume* — the clickstream table can be flooded.
- **Fix:** Add `slowapi` (or an Apache/nginx-level limit) per IP. Low effort at the proxy layer.

**M5 — Endpoints return HTTP 200 on failure**
`app.py:76`, `app.py:87`
On error the POST routes return `{"ok": false}` with a **200** status. This is defensible for a fire-and-forget beacon, but it means proxies, uptime monitors, and dashboards see 100% success even during an outage — compounding H3.
- **Fix:** Once logging exists (H3), this matters less, but consider a 5xx on genuine server errors so infra monitoring can see them; keep 200 only for validation misses.

**M6 — No request validation models (Pydantic unused)**
`app.py:69-73`, `app.py:80-85`
FastAPI's headline feature — Pydantic request models with automatic validation and OpenAPI schemas — is bypassed via `await request.json()` + `.get()`. Validation is scattered manually into `db.py`.
- **Fix:** Define `TrackBatchIn` / `SessionIn` Pydantic models. Free validation, free `/docs` schema, and the whitelist becomes the model.

**M7 — Committed heavy source/generated files**
`QS Procrastination program V1 (1).pdf` (320 KB), `simulations/discovery_simulation.html` (522 KB, generated by `simulate.py`)
Source docs and generated output tracked in git.
- **Fix:** Move the PDF to shared storage; git-ignore the generated simulation HTML (it's reproducible via `simulate.py`).

### 🟢 Low

- **L1 — Magic number for hash length.** `db.py:159` `len(hashed_email) != 64` — name it `SHA256_HEX_LEN = 64`.
- **L2 — No health-check endpoint.** Add `GET /api/health` (checks DB reachability) for load-balancer probes.
- **L3 — Fresh MySQL connection per request, no pooling** (`db.py:16-26`). Fine at current volume; revisit with `DBUtils`/`SQLAlchemy` pooling if traffic grows.
- **L4 — PII surface on `/api/me`.** `app.py:47` returns name/cigs/gender/country for any valid 64-hex hashed email. The hashed email *is* the capability token (anyone with the remarketing link has it), so risk is low, but it's an unauthenticated PII reflector — keep it minimal (you already do).
- **L5 — Debug page shipped in the web build** (`web/src/app/debug/page.tsx` → `web/out/debug/`). Ensure it's noindexed/gated in prod (CLAUDE.md's "disable debug in production" note applies to the Next app now, not the deleted Python `/debug`).
- **L6 — `os.environ.setdefault` in the loader** (`app.py:36`) means a real process env var wins over `.env` — correct precedence, just confirm that's intended for secret rotation.

---

## Environment Variables

| Variable | Used? | File | Recommendation |
|---|---|---|---|
| `MOUNT_PREFIX` | ✅ | `app.py:25` | OK. Document the two proxy modes in README. |
| `SUBSCRIPTION_URL` | ✅ | `app.py:63` | OK. Base64 `qs` decodes to `program=3&link-source=discovery` — not a secret. |
| `REDIRECT_TO_PAY` | ✅ | `app.py:64` | OK. String `"1"` compare is fine; consider a bool cast helper. |
| `DB_HOST` | ✅ | `db.py:18` | OK. No default → clear `KeyError` if missing (acceptable fail-fast). |
| `DB_PORT` | ✅ | `db.py:19` | OK (defaults 3306). |
| `DB_DATABASE` | ✅ | `db.py:22` | OK. |
| `DB_USERNAME` | ✅ | `db.py:20` | OK. Ensure this is a **read/limited-write** DB user (only the two Discovery tables + read on user tables). |
| `DB_PASSWORD` | ✅ | `db.py:21` | Sensitive — correctly kept out of `.env.example`. |
| `POSTMARK_TOKEN` | ❌ | `.env.example:18` | Declared, **unused** (Phase C not built). Fine as forward-decl; note it in README as "not yet wired." |
| `POSTMARK_FROM` | ❌ | `.env.example:19` | Same as above. |

No secrets are committed; `.env` is git-ignored; `.env.example` correctly ships blank credentials. **Missing:** validation/fail-fast on startup — a missing `DB_HOST` only errors on first request (via `KeyError`), not at boot. Consider a startup config check (or pydantic-settings) so misconfiguration fails loudly on deploy.

---

## Dependency Review

**Python** (`requirements.txt`): `fastapi`, `uvicorn`, `pymysql` used. `jinja2` **unused**, `aiofiles` **unused**, `python-dotenv` **unused** (reimplemented by hand). `starlette==0.47.3` exact-pinned against a floating `fastapi>=0.104` — conflict risk (see M3). No lockfile, no `pyproject.toml`. **Score 55.**

**Web** (`web/package.json`): Next 16.2.9 / React 19.2.4, properly pinned with `package-lock.json`. `motion`, `canvas-confetti`, `lucide-react`, shadcn/tailwind stack — reasonable, no obvious bloat. (Out of "Python" scope; noted for completeness.)

---

## Refactoring Roadmap

### Immediate (Today)
- **Un-track build output** (`git rm -r --cached web/out`, ignore it) — *Small.*
- **Add `logging` to every `except` in `db.py`/`app.py`** (H3) — *Small.*
- **Delete dead `templates/` + `static/` trees; git-ignore the generated simulation HTML** — *Small.*
- **Drop unused deps (`jinja2`, `aiofiles`); switch to `python-dotenv`** — *Small.*

### Short-term (This Week)
- **Rewrite CLAUDE.md** to match the actual shim + Next architecture (M1) — *Medium.*
- **Unit tests for `db.py` pure functions + `get_profile`; `TestClient` API tests** (H4) — *Medium.*
- **Pydantic request models for the two POST routes** (M6) — *Medium.*
- **Fix the Starlette pin; add `pyproject.toml` + lock** (M3) — *Medium.*

### Medium-term (This Month)
- **Rate limiting** on write endpoints (M4) — *Small–Medium.*
- **`/api/health` + startup config validation** (L2, env fail-fast) — *Small.*
- **CI (GitHub Actions): ruff + mypy + pytest + `next build`**; build `web/out` in CI, not git — *Medium.*
- **Document `bConverted` as unverified; drive real attribution off `tbl_UserPrograms`** (H1) — *Small (mostly documentation + reporting query).*

### Long-term
- **Containerize** (Dockerfile + compose for local MySQL) — *Medium.*
- **Connection pooling** if volume grows (L3) — *Medium.*
- **Wire Phase C** (Postmark reminders) with the already-declared env vars — *Large.*

---

## Positive Observations

Genuinely good engineering worth keeping:

- **Parameterized SQL everywhere** (`db.py:100`, `db.py:139`, `db.py:163`) — no string interpolation of values. Zero SQL-injection surface on values.
- **Column whitelist** (`_SESSION_COLS`, `db.py:108`) — dynamic column names come from a fixed dict, so even the identifier side is injection-safe.
- **Defensive input hygiene:** length caps (`db.py:82`), event count cap of 100 (`db.py:90`), sanity-bounded cigs 1–60 (`db.py:58-64`), 64-char hash gate (`db.py:159`).
- **DB timeouts set** (`connect_timeout=5, read_timeout=5`, `db.py:24-25`) — the funnel can't hang on a slow DB.
- **"NULL, never blanks" discipline** (`db.py:125`) — clean analytics data.
- **Excellent design doc** (`DISCOVERY_BE_BRIEF.md`) — the payment/tracking/no-user-creation decisions are documented with rationale. This is better than most production repos.
- **Thoughtful, honest docstrings** — every function explains *why*, including the "never break the funnel" contract.
- **Clean separation** — `app.py` (transport) vs `db.py` (persistence). The shim's purpose (keep the existing Apache→uvicorn deploy working, leave room for Phase C) is well-reasoned.

---

## Final Verdict

- **Production-ready?** **Not yet.** The *code* is close, but the absence of logging (H3), tests (H4), and CI, plus committed build artifacts (H2) and stale docs (M1), mean you can't operate or safely evolve it. Fix the four High items and it's deployable for a low-risk funnel.
- **Can it scale?** For its actual load (an email-remarketing funnel) — **yes**, easily. The fresh-connection-per-request pattern is the only ceiling, and it's far away.
- **Is it secure?** **Adequately, with caveats.** No injection surface, no committed secrets, sane input caps. The real gap is *trust*: analytics writes are unauthenticated (H1) and there's no rate limiting (M4). No auth exists by design — payment identity lives in Razorpay.
- **Is it maintainable?** **Below where it should be for 600 LOC** — not because of the code, but because the docs lie about the architecture, dead files litter the tree, and there are no tests to catch regressions.

### Top 10 improvements, by impact
1. Add logging to every swallowed exception (H3).
2. Un-track `web/out/` and delete dead `templates/`/`static/` trees (H2).
3. Write unit + API tests; wire them into CI (H4).
4. Rewrite CLAUDE.md to the real architecture (M1).
5. Treat `bConverted` as unverified; attribute conversions via `tbl_UserPrograms` only (H1).
6. Replace the hand-rolled `.env` loader with `python-dotenv`/`pydantic-settings`; drop unused deps (M2).
7. Add Pydantic request models to the POST routes (M6).
8. Fix the `starlette` pin and add a lockfile/`pyproject.toml` (M3).
9. Add rate limiting + a `/api/health` endpoint (M4, L2).
10. Add a Dockerfile + GitHub Actions (ruff/mypy/pytest/next build) so `web/out` is built, not committed (DevOps).