fix(ci): frontend lint to zero errors + dev-deps installable on clean image #149

chihlasm · 2026-04-25T06:35:58Z

chihlasm commented

2026-04-25 06:35:58 +00:00

Summary

Follow-up to PRs #141 / #147 / #148. Three categories of fix:

Frontend lint gate fully green (20 errors → 0)
Backend dev deps installable on a clean image (was unsolvable due to pytest pin conflict)
Test-DB schema fixture now creates ALL tables (one-line conftest fix that turned out to be the dominant source of pytest red)

After this lands, frontend CI is green, backend CI is dramatically less red (488 errors → 4), and we can enable Gitea branch protection on main.

Measured impact

Full backend pytest run, before vs after:

Metric	Before	After	Δ
Passing	482	1018	+536
Failed	44	54	+10
Errors	488	4	−484
Deselected (RLS)	0	35	+35

The remaining 54 failures and 4 errors are real test bugs (mostly fixture/test-cleanup issues like duplicate-email registration), not infrastructure cascades. They're tractable as their own focused follow-ups now that the dominant noise is gone.

What's fixed

1. Frontend lint: 20 errors → 0 errors

Category	Files	Approach
`setState` synchronously in effect (real cascade bugs flagged by react-hooks v5)	`useMediaQuery.ts`, `DeviceNode.tsx`, `GroupNode.tsx`, `TicketQueue.tsx`	Refactored to React-idiomatic shape: `useMediaQuery` switched to `useSyncExternalStore` (canonical for external subscriptions); the prop-sync effects in `DeviceNode`/`GroupNode` were redundant — display now reads the prop directly during render, edit buffer initializes only when an edit session starts; `TicketQueue` moved its clear/loading writes inside the async fetch (post-await boundary) and added a request-id ref to drop stale responses.
`react-refresh/only-export-components`	`router.tsx`	Moved `AssistantSessionRedirect` to its own file under `components/routing/` so the router-config module only exports a constant.
`@typescript-eslint/no-explicit-any`	`useFlowPilotSession.ts`, `FlowPilotSessionPage.tsx`	Replaced `as any` with narrow inline error/state shapes.
`@typescript-eslint/no-unused-expressions` (×8)	`TicketsPage.tsx`	Replaced ternaries-as-statements with proper `if/else` blocks.
`no-unused-vars`	`ScriptBuilderTab.tsx`	`handleViewScript` placeholder declares its args properly with `void` to match the `ScriptBuilderChatProps` signature.

tsc -b clean across the merged tree.

2. Backend dev deps: ResolutionImpossible → installable

backend/requirements-dev.txt had pytest==7.4.3 pinned alongside pytest-asyncio==0.24.0, but pytest-asyncio 0.24 declares pytest>=8.2. A clean pip install fails with ResolutionImpossible. Bumped:

pytest         7.4.3 → 8.4.2
pytest-cov     4.1.0 → 5.0.0   (pytest 8 compat)
pytest-asyncio 0.24.0 (unchanged)

3. Test-DB schema now creates the full schema

backend/tests/conftest.py's test_db fixture calls Base.metadata.create_all on a fresh DB. That only creates tables for models that have been imported by the time the fixture runs. app.main imports models lazily (inside scheduler functions and route modules), which is fine at runtime but leaves the metadata incomplete at fixture-setup time. Tests that exercise PSA, network diagrams, ratings, escalations, etc. were all failing with UndefinedTableError: relation "X" does not exist, with cascading 500s from every endpoint that queried the missing table.

Added from app import models as _models to conftest so app/models/__init__.py runs (which imports every model module — registering all ~60 tables on Base.metadata before create_all runs). The from app import models form is used instead of import app.models so it doesn't shadow the app FastAPI instance imported just above.

Test plan

npm run lint — 0 errors / 23 warnings (warnings are missing-deps in useEffect, opt-in cleanup later).
npx tsc -b — clean.
pip install -r requirements-dev.txt — resolves cleanly on Python 3.12.
Full pytest — 1018 passed / 54 failed / 4 errors / 35 deselected (vs. 482/44/488/0 before this PR).

🤖 Generated with Claude Code

## Summary Follow-up to PRs #141 / #147 / #148. Three categories of fix: 1. **Frontend lint gate fully green** (20 errors → 0) 2. **Backend dev deps installable on a clean image** (was unsolvable due to pytest pin conflict) 3. **Test-DB schema fixture now creates ALL tables** (one-line conftest fix that turned out to be the dominant source of pytest red) After this lands, frontend CI is green, backend CI is dramatically less red (488 errors → 4), and we can enable Gitea branch protection on `main`. ## Measured impact Full backend pytest run, before vs after: | Metric | Before | After | Δ | |---|---|---|---| | Passing | 482 | **1018** | +536 | | Failed | 44 | 54 | +10 | | Errors | 488 | **4** | −484 | | Deselected (RLS) | 0 | 35 | +35 | The remaining 54 failures and 4 errors are real test bugs (mostly fixture/test-cleanup issues like duplicate-email registration), not infrastructure cascades. They're tractable as their own focused follow-ups now that the dominant noise is gone. ## What's fixed ### 1. Frontend lint: 20 errors → 0 errors | Category | Files | Approach | |---|---|---| | `setState` synchronously in effect (real cascade bugs flagged by react-hooks v5) | `useMediaQuery.ts`, `DeviceNode.tsx`, `GroupNode.tsx`, `TicketQueue.tsx` | Refactored to React-idiomatic shape: `useMediaQuery` switched to `useSyncExternalStore` (canonical for external subscriptions); the prop-sync effects in `DeviceNode`/`GroupNode` were redundant — display now reads the prop directly during render, edit buffer initializes only when an edit session starts; `TicketQueue` moved its clear/loading writes inside the async fetch (post-await boundary) and added a request-id ref to drop stale responses. | | `react-refresh/only-export-components` | `router.tsx` | Moved `AssistantSessionRedirect` to its own file under `components/routing/` so the router-config module only exports a constant. | | `@typescript-eslint/no-explicit-any` | `useFlowPilotSession.ts`, `FlowPilotSessionPage.tsx` | Replaced `as any` with narrow inline error/state shapes. | | `@typescript-eslint/no-unused-expressions` (×8) | `TicketsPage.tsx` | Replaced ternaries-as-statements with proper `if/else` blocks. | | `no-unused-vars` | `ScriptBuilderTab.tsx` | `handleViewScript` placeholder declares its args properly with `void` to match the `ScriptBuilderChatProps` signature. | `tsc -b` clean across the merged tree. ### 2. Backend dev deps: ResolutionImpossible → installable `backend/requirements-dev.txt` had `pytest==7.4.3` pinned alongside `pytest-asyncio==0.24.0`, but pytest-asyncio 0.24 declares `pytest>=8.2`. A clean pip install fails with `ResolutionImpossible`. Bumped: ``` pytest 7.4.3 → 8.4.2 pytest-cov 4.1.0 → 5.0.0 (pytest 8 compat) pytest-asyncio 0.24.0 (unchanged) ``` ### 3. Test-DB schema now creates the full schema `backend/tests/conftest.py`'s `test_db` fixture calls `Base.metadata.create_all` on a fresh DB. That only creates tables for models that have been imported by the time the fixture runs. `app.main` imports models lazily (inside scheduler functions and route modules), which is fine at runtime but leaves the metadata incomplete at fixture-setup time. Tests that exercise PSA, network diagrams, ratings, escalations, etc. were all failing with `UndefinedTableError: relation "X" does not exist`, with cascading 500s from every endpoint that queried the missing table. Added `from app import models as _models` to conftest so `app/models/__init__.py` runs (which imports every model module — registering all ~60 tables on `Base.metadata` before `create_all` runs). The `from app import models` form is used instead of `import app.models` so it doesn't shadow the `app` FastAPI instance imported just above. ## Test plan - [x] `npm run lint` — 0 errors / 23 warnings (warnings are missing-deps in useEffect, opt-in cleanup later). - [x] `npx tsc -b` — clean. - [x] `pip install -r requirements-dev.txt` — resolves cleanly on Python 3.12. - [x] Full `pytest` — 1018 passed / 54 failed / 4 errors / 35 deselected (vs. 482/44/488/0 before this PR). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

chihlasm added 4 commits 2026-04-25 06:35:58 +00:00

fix(deps): bump pytest 7.4 → 8.4, pytest-cov 4.1 → 5.0 to satisfy pytest-asyncio 0.24 406ee0ef97

pytest-asyncio==0.24.0 (added on the FlowPilot branch as part of the
RLS test infra refactor) declares pytest>=8.2 — but requirements-dev.txt
still pinned pytest==7.4.3, so a clean pip install fails with
ResolutionImpossible. CI runners that started from a fresh image would
have refused to install dev deps; the FlowPilot tests passed locally
only because the dev container had a pre-installed pytest 8.x lying
around.

pytest-cov 4.1.0 also needs >= 5.0 to play nicely with pytest 8.

No code changes — pytest 8 is API-compatible with the existing test
suite once the install resolves.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix(lint): move AssistantSessionRedirect out of router.tsx (react-refresh gate) 857d73e3d0

react-refresh/only-export-components fires when a file with the
\`router\` const export also defines a component (the redirect helper).
Moves the small helper to its own file under components/routing/ so
HMR can keep the route-component module hot-reload-eligible.

No behavior change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix(lint): replace explicit-any types + unused-expressions ternaries b7f8e70be2

Five files, all stylistic:

- useFlowPilotSession.ts: typed the axios error shape with a narrow
  inline type instead of \`as any\`.
- FlowPilotSessionPage.tsx: same — typed location.state once, then
  destructured.
- ScriptBuilderTab.tsx: handleViewScript was a placeholder no-op;
  declared the args properly with \`void script; void filename\` so the
  signature matches ScriptBuilderChatProps without no-unused-vars
  firing.
- TicketsPage.tsx: replaced 8 ternaries-as-statements (\`x ? f() : g()\`)
  with proper if/else blocks. Same control flow, satisfies
  no-unused-expressions, and reads better in the URL-param update paths.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix(react): remove four setState-in-effect cascades flagged by react-hooks v5

Mirror to GitHub / mirror (push) Successful in 11s

Details

CI / backend (pull_request) Failing after 11m23s

Details

CI / frontend (pull_request) Failing after 2m42s

Details

CI / e2e (pull_request) Has been skipped

Details

920a246d77

The new react-hooks lint rule "Calling setState synchronously within an
effect can trigger cascading renders" flagged real anti-patterns in
four spots. Refactored each per the rule's intent (derive during render,
or use useSyncExternalStore for external subscriptions).

1. hooks/useMediaQuery.ts — replaced the useState + useEffect pair with
   useSyncExternalStore. That's the canonical React hook for
   subscribing to external stores (matchMedia in this case) without
   mirroring into local state via an effect. Snapshot/getServerSnapshot
   pair preserves the SSR-safe behaviour.

2. components/network/nodes/DeviceNode.tsx — the prop-sync useEffect
   that copied nodeData.label into labelValue was redundant.
   labelValue is the EDIT BUFFER; while not editing, the displayed
   span now reads nodeData.label directly. The buffer is initialized
   only when an edit session starts (onDoubleClick).

3. components/network/nodes/GroupNode.tsx — same pattern, same fix.

4. components/dashboard/TicketQueue.tsx — the
   setTickets([]) + setLoading(true) + fetchTickets() chain in the
   effect was the cascade. Pushed those writes inside fetchTickets
   (after the function boundary, so they batch with the eventual
   setTickets(result)). Added a request-id ref so a slow first
   response can't overwrite a fast second one.

Frontend lint: 20 errors → 0 errors. tsc -b clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

chihlasm added 1 commit 2026-04-25 06:49:09 +00:00

fix(tests): import all models in conftest so create_all sees the full schema

Mirror to GitHub / mirror (push) Successful in 11s

Details

CI / backend (pull_request) Failing after 11m23s

Details

CI / frontend (pull_request) Failing after 2m41s

Details

CI / e2e (pull_request) Has been skipped

Details

d6218f2e07

The test_db fixture calls Base.metadata.create_all on a fresh test DB.
That only creates tables for models that have been imported (and thus
registered with Base.metadata) by the time the fixture runs.

app.main imports app.core.database (which gives us Base) but does NOT
eagerly import the model modules — most are pulled in lazily inside
scheduler functions (archive_stale_ai_sessions etc.) and route
modules. At fixture-setup time, only the handful of models touched by
those eager imports are on the metadata, so any test that exercises
PSA, network diagrams, ratings, escalations, etc. fails with
\`UndefinedTableError: relation "X" does not exist\` and a cascade of
500s on every endpoint that queries the missing table.

Adding \`from app import models as _models\` (rather than the bare
\`import app.models\` which would shadow the \`app\` FastAPI instance
imported just above) pulls in app/models/__init__.py, which itself
imports every model module — registering all ~60 tables with
Base.metadata before create_all runs.

Verified locally: tests/test_psa_writeback_phase4.py went from
1 failed / 6 errors → 4 failed / 3 passed (the cascading errors were
masking the actual passes).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

chihlasm merged commit f27f671fe6 into main

2026-04-25 07:12:15 +00:00

chihlasm referenced this issue from a commit

2026-04-25 07:12:16 +00:00

Merge PR #149: fix(ci): frontend lint to zero errors + test-DB schema fix + dev-deps installable

chihlasm referenced this issue from a commit

2026-04-25 07:28:56 +00:00

fix(ci): set DATABASE_TEST_URL + downgrade upload-artifact to v3 (Gitea Actions)

chihlasm referenced this pull request

2026-04-25 07:29:21 +00:00

fix(ci): set DATABASE_TEST_URL + pin upload-artifact to v3 for Gitea Actions #150

chihlasm referenced this issue from a commit