BanGUI/Docs/Refactoring.md

BanGUI — Refactoring Instructions for AI Agents

This document is the single source of truth for any AI agent performing a refactoring task on the BanGUI codebase.
Read it in full before writing a single line of code.
The authoritative description of every module, its responsibilities, and the allowed dependency direction is in Architekture.md. Always cross-reference it.

---

0. Golden Rules

1. Architecture first. Every change must comply with the layered architecture defined in Architekture.md §2. Dependencies flow inward: routers → services → repositories. Never add an import that reverses this direction.
2. One concern per file. Each module has an explicitly stated purpose in Architekture.md. Do not add responsibilities to a module that do not belong there.
3. No behaviour change. Refactoring must preserve all existing behaviour. If a function's public signature, return value, or side-effects must change, that is a feature — create a separate task for it.
4. Tests stay green. Run the full test suite (pytest backend/) before and after every change. Do not submit work that introduces new failures.
5. Smallest diff wins. Prefer targeted edits. Do not rewrite a file when a few lines suffice.

---

1. Before You Start

1.1 Understand the project

Read the following documents in order:

1. Architekture.md — full system overview, component map, module purposes, dependency rules.
2. Docs/Backend-Development.md — coding conventions, testing strategy, environment setup.
3. Docs/Tasks.md — open issues and planned work; avoid touching areas that have pending conflicting changes.

1.2 Map the code to the architecture

Before editing, locate every file that is in scope:

backend/app/
  routers/          HTTP layer — zero business logic
  services/         Business logic — orchestrates repositories + clients
  repositories/     Data access — raw SQL only
  models/           Pydantic schemas
  tasks/            APScheduler jobs
  utils/            Pure helpers, no framework deps
  main.py           App factory, lifespan, middleware
  config.py         Pydantic settings
  dependencies.py   FastAPI Depends() wiring

frontend/src/
  api/              Typed fetch wrappers + endpoint constants
  components/       Presentational UI, no API calls
  hooks/            All state, side-effects, API calls
  pages/            Route components — orchestration only
  providers/        React context
  types/            TypeScript interfaces
  utils/            Pure helpers

Confirm which layer every file you intend to touch belongs to. If unsure, consult Architekture.md §2.2 (backend) or Architekture.md §3.2 (frontend).

1.3 Run the baseline

# Backend
pytest backend/ -x --tb=short

# Frontend
cd frontend && npm run test

Record the number of passing tests. After refactoring, that number must be equal or higher.

---

2. Backend Refactoring

2.1 Routers (app/routers/)

Allowed content: request parsing, response serialisation, dependency injection via Depends(), delegation to a service, HTTP error mapping.
Forbidden content: SQL queries, business logic, direct use of fail2ban_client, any logic that would also make sense in a unit test without an HTTP request.

Checklist:

• Every handler calls exactly one service method per logical operation.
• No if/elif chains that implement business rules — move these to the service.
• No raw SQL or repository imports.
• All response models are Pydantic schemas from app/models/.
• HTTP status codes are consistent with API conventions (200 OK, 201 Created, 204 No Content, 400/422 for client errors, 404 for missing resources, 500 only for unexpected failures).

2.2 Services (app/services/)

Allowed content: business rules, coordination between repositories and external clients, validation that goes beyond Pydantic, fail2ban command orchestration.
Forbidden content: raw SQL, direct aiosqlite calls, FastAPI HTTPException (raise domain exceptions instead and let the router or exception handler convert them).

Checklist:

• Service classes / functions accept plain Python types or domain models — not Request or Response objects.
• No direct aiosqlite usage — go through a repository.
• No HTTPException — raise a custom domain exception or a plain ValueError/RuntimeError with a clear message.
• No circular imports between services — if two services need each other's logic, extract the shared logic to a utility or a third service.

2.3 Repositories (app/repositories/)

Allowed content: SQL queries, result mapping to domain models, transaction management.
Forbidden content: business logic, fail2ban calls, HTTP concerns, logging beyond debug-level traces.

Checklist:

• Every public method accepts a db: aiosqlite.Connection parameter — sessions are not managed internally.
• Methods return typed domain models or plain Python primitives, never raw aiosqlite.Row objects exposed to callers.
• No business rules (e.g., no "if this setting is missing, create a default" logic — that belongs in the service).

2.4 Models (app/models/)

• Keep Request, Response, and Domain model types clearly separated (see Architekture.md §2.2).
• Do not use response models as function arguments inside service or repository code.
• Validators (@field_validator, @model_validator) belong in models only when they concern data shape, not business rules.

2.5 Tasks (app/tasks/)

• Tasks must be thin: fetch inputs → call one service method → log result.
• Error handling must be inside the task (APScheduler swallows unhandled exceptions — log them explicitly).
• No direct repository or fail2ban_client use; go through a service.

2.6 Utils (app/utils/)

• Must have zero framework dependencies (no FastAPI, no aiosqlite imports).
• Must be pure or near-pure functions.
• fail2ban_client.py is the single exception — it wraps the socket protocol but still has no service-layer logic.

2.7 Dependencies (app/dependencies.py)

• This file is the only place where service constructors are called and injected.
• Do not construct services inside router handlers; always receive them via Depends().

---

3. Frontend Refactoring

3.1 Pages (src/pages/)

Allowed content: composing components and hooks, layout decisions, routing.
Forbidden content: direct fetch/axios calls, inline business logic, state management beyond what is needed to coordinate child components.

Checklist:

• All data fetching goes through a hook from src/hooks/.
• No API function from src/api/ is called directly inside a page component.

3.2 Components (src/components/)

Allowed content: rendering, styling, event handlers that call prop callbacks.
Forbidden content: API calls, hook-level state (prefer lifting state to the page or a dedicated hook), direct use of src/api/.

Checklist:

• Components receive all data via props.
• Components emit changes via callback props (onXxx).
• No useEffect that calls an API function — that belongs in a hook.

3.3 Hooks (src/hooks/)

Allowed content: useState, useEffect, useCallback, useRef; calls to src/api/; local state derivation.
Forbidden content: JSX rendering, Fluent UI components.

Checklist:

• Each hook has a single, focused concern matching its name (e.g., useBans only manages ban data).
• Hooks return a stable interface: { data, loading, error, refetch } or equivalent.
• Shared logic between hooks is extracted to src/utils/ (pure) or a parent hook (stateful).

3.4 API layer (src/api/)

• client.ts is the only place that calls fetch. All other api files call client.ts.
• endpoints.ts is the single source of truth for URL strings.
• API functions must be typed: explicit request and response TypeScript interfaces from src/types/.

3.5 Types (src/types/)

• Interfaces must match the backend Pydantic response schemas exactly (field names, optionality).
• Do not use any. Use unknown and narrow with type guards when the shape is genuinely unknown.

---

4. General Code Quality Rules

Naming

• Python: snake_case for variables/functions, PascalCase for classes.
• TypeScript: camelCase for variables/functions, PascalCase for components and types.
• File names must match the primary export they contain.

Error handling

• Backend: raise typed exceptions; map them to HTTP status codes in main.py exception handlers or in the router — nowhere else.
• Frontend: all API call error states are represented in hook return values; never swallow errors silently.

Logging (backend)

• Use structlog with bound context loggers — never bare print().
• Log at debug in repositories, info in services for meaningful events, warning/error in tasks and exception handlers.
• Never log sensitive data (passwords, session tokens, raw IP lists larger than a handful of entries).

Async correctness (backend)

• Every function that touches I/O (database, fail2ban socket, HTTP) must be async def.
• Never call asyncio.run() inside a running event loop.
• Do not use time.sleep() — use await asyncio.sleep().

---

5. Refactoring Workflow

Follow this sequence for every refactoring task:

1. Read the relevant section of Architekture.md for the files you will touch.
2. Run the full test suite to confirm the baseline.
3. Identify the violation or smell: which rule from this document does it break?
4. Plan the minimal change: what is the smallest edit that fixes the violation?
5. Edit the code. One logical change per commit.
6. Verify imports: nothing new violates the dependency direction.
7. Run the full test suite. All previously passing tests must still pass.
8. Update any affected docstrings or inline comments to reflect the new structure.
9. Do not update Architekture.md unless the refactor changes the documented structure — that requires a separate review.

---

6. Common Violations to Look For

Violation	Where it typically appears	Fix
Business logic in a router handler	app/routers/*.py	Extract logic to the corresponding service
Direct aiosqlite calls in a service	app/services/*.py	Move the query into the matching repository
HTTPException raised inside a service	app/services/*.py	Raise a domain exception; catch and convert it in the router or exception handler
API call inside a React component	src/components/*.tsx	Move to a hook; pass data via props
Hardcoded URL string in a hook or component	src/hooks/*.ts, src/components/*.tsx	Use the constant from src/api/endpoints.ts
any type in TypeScript	anywhere in src/	Replace with a concrete interface from src/types/
print() statements in production code	backend/app/**/*.py	Replace with structlog logger
Synchronous I/O in an async function	backend/app/**/*.py	Use the async equivalent
A repository method that contains an if with a business rule	app/repositories/*.py	Move the rule to the service layer

---

7. Out of Scope

Do not make the following changes unless explicitly instructed in a separate task:

• Adding new API endpoints or pages.
• Changing database schema or migration files.
• Upgrading dependencies.
• Altering Docker or CI configuration.
• Modifying Architekture.md or Tasks.md.