From 8174b51bc722950207d50ac507d2fb12b215c6b9 Mon Sep 17 00:00:00 2001 From: Matt Borgeson Date: Wed, 29 Apr 2026 17:06:14 -0700 Subject: [PATCH 1/4] docs(zod-parity): mark all backlog items closed Updates the survey doc with the closing commits for each of the four files the parity hook flagged. Also rewrites the hook-limitation note to reflect the lesson from doing the cleanup: regex-based field extraction can't tell sibling response classes apart, so a single-file Pydantic module with multiple response classes can produce noise even when each class has a correctly-shaped Zod counterpart. --- docs/zod-parity-followups.md | 62 +++++++++++++----------------------- 1 file changed, 23 insertions(+), 39 deletions(-) diff --git a/docs/zod-parity-followups.md b/docs/zod-parity-followups.md index 73f0298..1d8b3a5 100644 --- a/docs/zod-parity-followups.md +++ b/docs/zod-parity-followups.md @@ -2,49 +2,33 @@ The `check-zod-parity.sh` PostToolUse hook compares fields declared in `backend/app/schemas/*.py` to fields in the matching `src/lib/api/schemas/.ts`. Anything in the Pydantic schema but missing from Zod is silently dropped at parse time — Zod's default `z.object()` strips unknowns. -Survey run on 2026-04-29 against all 22 backend schema files identified four files with real drift. `deal.ts` was partially fixed (12 `DealResponse` fields added); the rest are tracked here. +Survey run on 2026-04-29 against all 22 backend schema files identified four files with real drift. **All four have since been closed** — live parity check returns zero missing fields across `property.ts`, `reporting.ts`, `construction.ts`, and `deal.ts` as of 2026-04-29. -## Priority 1 — `property.ts` +## Status -37 missing fields. `property.py` is the largest schema and most likely to be silently dropping data the UI needs. +| Priority | File | Status | Closing commit | +|----------|------|--------|----------------| +| 1 | `property.ts` | ✅ Closed | `c65eaf9` — fix(schemas): add flat backendPropertySchema for PropertyResponse parity (#4) | +| 2 | `reporting.ts` | ✅ Closed | `bf78179` — fix(schemas): add Zod schemas for all reporting response classes (#5) | +| 3 | `construction.ts` | ✅ Closed | `e1df8f9` — fix(schemas): close construction.ts Zod parity gap (#3) | +| 4 | `deal.ts` sub-class artifacts | ✅ Closed | `a184058` — fix(schemas): close deal.ts sub-class artifact parity gaps (#6) | -Missing (first 10): `acquisition_date, address, avg_rent_per_sf, avg_rent_per_unit, cap_rate, city, county, current_value, data_source, description` — plus 27 more. +To re-verify locally: -To do: -1. Read both `backend/app/schemas/property.py` and `src/lib/api/schemas/property.ts`. -2. Identify which Pydantic class each missing field belongs to (likely `PropertyResponse` vs `PropertyCreate`/`PropertyUpdate`/sub-classes). -3. Add the response-only fields to the Zod input shape with `.nullable().optional()` per CLAUDE.md convention. -4. Run `npm run test:run -- src/features/property` to confirm no regressions. +```bash +for stem in property reporting construction deal; do + py_fields=$(grep -E '^[[:space:]]{4}[a-z][a-z0-9_]*:[[:space:]]' backend/app/schemas/${stem}.py \ + | sed -E 's/^[[:space:]]+([a-z][a-z0-9_]*):.*/\1/' \ + | grep -vE '^(model_config|class_config|_)' | sort -u) + zod_fields=$(grep -E '^[[:space:]]+[a-z][a-z0-9_]*:[[:space:]]*z\.' src/lib/api/schemas/${stem}.ts \ + | sed -E 's/^[[:space:]]+([a-z][a-z0-9_]*):.*/\1/' | sort -u) + echo "=== $stem ===" + comm -23 <(printf '%s\n' "$py_fields") <(printf '%s\n' "$zod_fields") +done +``` -## Priority 2 — `reporting.ts` +## Hook limitation (open) -43+ missing fields. Likely a mix of report config schemas, scheduled report fields, and template metadata. +`check-zod-parity.sh` is regex-based: it grabs every indented `name: type` line in the `.py` file and checks against every Zod field in the `.ts` file, with no awareness of Pydantic class boundaries. That's why a single-file Pydantic module with multiple response classes can generate noise even when each class has its own (correctly-shaped) Zod counterpart — the hook concatenates fields across classes. -Missing (first 10): `category, completed_at, config, configurable, created_at, created_by, day_of_month, day_of_week, default_height, default_width` — plus 33 more. - -Approach: same as property.ts. Reporting code is newer (recently shipped — see commit `6cbc6a3 feat(reports)`) so the gap may reflect intentional separation between backend admin schemas and frontend wire schemas. Verify before adding fields wholesale. - -## Priority 3 — `construction.ts` - -5 missing fields: `count, results, rows_imported, rows_updated, total_value`. - -Small and tractable. These look like batch-import response fields — confirm whether the frontend triggers construction imports and needs the result stats. - -## Priority 4 — `deal.ts` sub-class artifacts - -23 fields the hook flags on `deal.py` are NOT `DealResponse` fields — they belong to sibling response classes that have no Zod schema at all: - -- `ProformaFieldValue`, `ProformaFieldGroup`, `ProformaReturnsResponse` — `category`, `field_name`, `fields`, `groups`, `value_numeric`, `value_text` -- `WatchlistStatusResponse` — `is_watched` -- `StageChangeLogResponse`, `StageHistoryResponse` — `changed_by_user_id`, `history`, `new_stage`, `old_stage`, `reason` -- `KanbanBoardResponse` — `stage_counts`, `stages`, `total_deals` -- `StageMappingResponse` — `folder_to_stage` -- `DealCursorPaginatedResponse` — `has_more`, `next_cursor`, `prev_cursor` -- `DealListResponse` — `items`, `page`, `page_size` (already covered partially via `dealsListResponseSchema`) -- Shared — `deal_id`, `deal_name`, `total` - -If the frontend calls the corresponding endpoints (`/deals/{id}/proforma-returns`, `/deals/{id}/watchlist`, `/deals/{id}/stage-history`, `/deals/kanban`, etc.), each needs its own Zod schema. If those endpoints are unused, the noise can be silenced by extending `check-zod-parity.sh` to scope to a single Pydantic class. - -## Hook limitation - -`check-zod-parity.sh` is regex-based: it grabs every indented `name: type` line in the `.py` file and checks against every Zod field in the `.ts` file, with no awareness of Pydantic class boundaries. That's why a single-file Pydantic module with 8 response classes generates noise like the deal.py case above. A future iteration could parse the AST (via Python `ast` module) and emit one diff per class — out of scope for now, but logged here. +A future iteration could parse the AST (via Python `ast` module) and emit one diff per class. Out of scope for the parity-closure work, but worth logging here so the next person hitting hook noise has context. From 33add5f6fb595190ba47f5a1a814b3b310896af0 Mon Sep 17 00:00:00 2001 From: Matt Borgeson Date: Wed, 29 Apr 2026 17:06:15 -0700 Subject: [PATCH 2/4] ci(e2e): make seed step fatal and pin bcrypt<4 to fix passlib crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related fixes that together unbreak the E2E suite, which has been silently failing on main for weeks. 1. Stop masking seed failures. The seed step was wrapped in `|| echo "::warning::..."` so seed crashes showed up as a warning instead of a failure. The job kept going, ran 60 minutes of auth-gated tests against a database with no demo users, and all those tests failed downstream — wasting CI runtime and obscuring the real cause. Drop the fallback so the job fails fast at minute ~2 if seed breaks. 2. Pin bcrypt<4 to make passlib work. Both requirements.txt and requirements-ci.txt currently pin bcrypt to >=4.1.0, but passlib 1.7.4 (the latest released version) reads bcrypt.__about__ which was removed in bcrypt 4.0: AttributeError: module 'bcrypt' has no attribute '__about__' This crashed `python -m app.database.seed` on every CI run and was the underlying cause of the cascading auth-test failures. Pin bcrypt<4 in both files. There is no passlib 1.7.5 yet, so the pin stays in place until passlib ships a fix or this codebase migrates off passlib (the project has been quiet since 2020 — migrating to bcrypt directly is likely the better long-term play, tracked separately). Together these turn E2E from a broken stamp into something that can fail fast on real problems and pass when the suite is healthy. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/e2e.yml | 4 +++- backend/requirements-ci.txt | 3 ++- backend/requirements.txt | 6 +++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index b31de8f..6c97b3e 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -107,9 +107,11 @@ jobs: # ── Seed demo users ────────────────────────────────────────────── # E2E auth fixtures expect demo users (matt@bandrcapital.com, demo@bandrcapital.com). # Uses the existing seed module at app.database.seed which creates users, properties, and deals. + # Fail the job if seeding fails — without demo users every auth-gated test + # downstream fails and the run wastes 30+ minutes producing meaningless red. - name: Seed database with demo data working-directory: backend - run: python -m app.database.seed || echo "::warning::Database seeding failed — auth E2E tests may fail" + run: python -m app.database.seed # ── Start backend ──────────────────────────────────────────────── - name: Start backend server diff --git a/backend/requirements-ci.txt b/backend/requirements-ci.txt index 41cf242..7566dc0 100644 --- a/backend/requirements-ci.txt +++ b/backend/requirements-ci.txt @@ -43,7 +43,8 @@ apscheduler>=3.10.0 # Authentication & Security PyJWT[crypto]>=2.8.0,<3.0.0 passlib[bcrypt]>=1.7.4 -bcrypt>=4.1.0,<5.0.0 +# bcrypt pinned to <4 — see backend/requirements.txt for context. +bcrypt<4.0 httpx>=0.26.0 # Utilities diff --git a/backend/requirements.txt b/backend/requirements.txt index c140609..4be2e32 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -47,7 +47,11 @@ apscheduler>=3.10.0,<4.0.0 # Authentication & Security PyJWT[crypto]>=2.12.0,<3.0.0 passlib[bcrypt]>=1.7.4,<2.0.0 -bcrypt>=4.1.0,<5.0.0 +# bcrypt pinned to <4 because passlib 1.7.4 reads bcrypt.__about__ which was +# removed in bcrypt 4.0 (AttributeError: module 'bcrypt' has no attribute '__about__'). +# passlib has no 1.7.5 release, so the dependency stays pinned until either passlib +# ships a fix or this codebase migrates off passlib. +bcrypt<4.0 httpx>=0.26.0,<1.0.0 # Utilities From 3103b2df4fa9d83d4a1f97ee03efd4c75bdc606b Mon Sep 17 00:00:00 2001 From: Matt Borgeson Date: Wed, 29 Apr 2026 17:15:49 -0700 Subject: [PATCH 3/4] ci(deps): add reportlab and matplotlib to requirements-ci.txt Test & Coverage was failing at the collection stage with: ModuleNotFoundError: No module named 'reportlab' tests/test_services/test_report_templates.py Both reportlab and matplotlib were added to requirements.txt as part of the report templates feature (commit 6cbc6a3) but the corresponding add to requirements-ci.txt was missed. CI installs from -ci.txt only, so the import in app/services/report_templates.py fails before pytest can even collect the test module. Was masked on the parity PRs because they didn't touch backend files, so backend-ci wasn't triggered. PR #8 changes requirements files, which triggers backend-ci, which surfaced this. --- backend/requirements-ci.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/backend/requirements-ci.txt b/backend/requirements-ci.txt index 7566dc0..633bebd 100644 --- a/backend/requirements-ci.txt +++ b/backend/requirements-ci.txt @@ -40,6 +40,13 @@ structlog>=23.1.0 aiohttp>=3.8.0 apscheduler>=3.10.0 +# Reporting / Charts +# Required by app.services.report_templates and report_charts. Without these, +# tests/test_services/test_report_templates.py fails at import time and pytest +# exits non-zero even though the actual tests would have skipped cleanly. +reportlab>=4.0.0,<5.0.0 +matplotlib>=3.8.0,<4.0.0 + # Authentication & Security PyJWT[crypto]>=2.8.0,<3.0.0 passlib[bcrypt]>=1.7.4 From d00da74d0b3981b778f5edecf3143ea3b37aea9a Mon Sep 17 00:00:00 2001 From: Matt Borgeson Date: Wed, 29 Apr 2026 18:18:35 -0700 Subject: [PATCH 4/4] ci(e2e): bump Playwright workers from 1 to 4 in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bcrypt + seed-fatal + reportlab/matplotlib fixes earlier in this PR unblocked the seed step (proven by the latest CI run — seed now passes cleanly), but the E2E job still hit the 60m wall with 403 tests on a single worker. GitHub's `ubuntu-latest` runners have 4 cores. Running 1 worker leaves 3 idle and serializes a suite that should parallelize. With 4 workers the wall clock drops by roughly 4x — 403 tests at ~5s each / 4 workers ≈ 8 minutes vs. 33+ minutes serial. Comfortably inside 60m. Drop to 2 if memory pressure becomes an issue (Playwright + chromium + backend + frontend + postgres on one runner). --- playwright.config.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/playwright.config.ts b/playwright.config.ts index 6027e19..2d47e6c 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -20,7 +20,9 @@ export default defineConfig({ forbidOnly: isCI, /* Default retries/workers — overridden per-project */ retries: 0, - workers: isCI ? 1 : undefined, + /* CI: 4 workers on a 4-core ubuntu-latest runner. Was 1 — caused 403 tests to + * exhaust the 60m job timeout on every run. Drop to 2 if memory pressure surfaces. */ + workers: isCI ? 4 : undefined, reporter: isCI ? [['json', { outputFile: 'playwright-report/results.json' }], ['html', { open: 'never' }]] : 'html',