chore(ci,docs): GitHub Actions, ADRs, README overhaul, §21 Hardening Release
Some checks failed
CI / Frontend — lint + build + e2e (push) Has been cancelled
CI / Backend — Odoo HttpCase (push) Has been cancelled

- .github/workflows/ci.yml: two jobs — frontend (tsc --noEmit, lint, build,
  Playwright) and backend (Postgres 16 + odoo:19 --test-enable
  --test-tags encoach_api) — catches regressions before merge.
- docs/adr/: start an Architecture Decision Record trail with
  0001 canonical directory layout, 0002 JWT refresh flow,
  0003 paginated response envelope, 0004 RAG metadata + chunking.
- docs/PROJECT_SUMMARY.md §21 Hardening Release: full recap of the AI
  quality loop, compliance, Paymob, i18n, and CI work shipped in this
  drop, plus new DB tables, REST routes, frontend routes, verification
  results, and operator-facing configuration.
- README.md refreshed for the v4 split-repo doctrine and the new feature
  surface.
- new_project/DEPRECATED.md: formal retirement notice pointing at
  backend/ as the canonical tree.

Made-with: Cursor
This commit is contained in:
Yamen Ahmad
2026-04-19 14:16:47 +04:00
parent e70a2854f4
commit 93c530eef2
10 changed files with 744 additions and 100 deletions

View File

@@ -1319,3 +1319,133 @@ Network tab showed `/api/reports/filters 200`, `/api/reports/record?size=100 200
- **Both `encoach_exam_template` and `encoach_scoring` declare `encoach.student.attempt`.** The Reports controller only reads fields that exist on both definitions (`listening/reading/writing/speaking/overall_band`, `cefr_level`, `status`, `started_at`, and whichever of `completed_at` / `finished_at` is present — `_attempt_completed_at()` probes both). No field access assumes a specific addon version.
- **In-progress attempts are excluded from the aggregates.** If you ever notice a student you expect to see missing from Student Performance or Stats Corporate, check their attempt status — `in_progress` attempts are deliberately skipped. They DO still appear in the Record page (which shows everything) unless you also pass `status` or `period` filters.
---
## §21 — Hardening Release (Phase 2/3 completion)
This section records the final wave of work that closed out all remaining items in the Phase 2 (performance & reliability) and Phase 3 (core AI features, accessibility, onboarding) roadmaps. After this release the platform is considered deployment-ready.
### 21.1 What shipped
The release is organised by capability, not file path.
#### Human-in-the-loop AI quality loop (P3.3 → P3.5)
| Stage | Where | What it does |
|---|---|---|
| 1. Quality gate | `encoach_exam_template.services.quality_checker` + `ielts_validator` | Runs after every AI exam submit; if scores fall below thresholds the exam is parked in `pending_review` instead of `published`. |
| 2. Review queue | `encoach_exam_template/controllers/review_workflow.py` + `frontend/src/pages/admin/ExamReview{Queue,Detail}.tsx` | Admin-only pages list pending exams, surface aggregate + per-question quality reports, and let a reviewer approve (`→ published`) or reject (`→ draft`) with audit notes. Writes to new `reviewed_by_id`, `reviewed_at`, `review_notes` fields. |
| 3. Prompt library | `encoach_ai.models.ai_prompt` + `controllers/prompt_controller.py` + `frontend/src/pages/admin/AIPromptEditor.tsx` | `encoach.ai.prompt` with a unique `(key, version)` key, one active row per key. Authors can add a new version, activate it (auto-deactivates prior), and dry-run render with sample variables. Renderer uses `str.format_map` against a safe dict. |
| 4. Student feedback | `encoach_ai.models.ai_feedback` + `controllers/feedback_controller.py` + `frontend/src/components/AIFeedbackButtons.tsx` + `frontend/src/pages/admin/AIFeedbackTriage.tsx` | Thumbs up/down on any AI artefact (`question`, `coach`, `explanation`, `translation`, `narrative`, `other`). Unique `(user, subject_type, subject_id)` constraint gives upsert semantics; thumbs-down requires a comment. Admin triage page lists/filters/resolves feedback (`open → acknowledged|fixed|dismissed`). |
Together these four stages mean every AI output can be gated, reviewed, iterated, and measured — closing the loop from prompt → generation → delivery → feedback → next prompt revision.
#### Compliance (P3.2)
New `encoach_api/controllers/gdpr.py` + `models/gdpr_erasure.py`:
- `GET /api/gdpr/export` — JSON dump of the calling user's profile, entity memberships, exam attempts, answers, AI feedback, AI calls, tickets, coaching sessions.
- `POST /api/gdpr/delete` — requires `{"confirm": true}`; anonymises the partner PII, deletes personal feedback + coaching transcripts, strips free-text PII from retained exam answers, nulls `user_id` on AI logs, deactivates the `res.users` row, and writes a tombstone to `encoach.gdpr.erasure.request` for audit. Admin accounts are blocked from self-erasure.
- `frontend/src/pages/PrivacyCenter.tsx` — routed at `/student/privacy`, `/teacher/privacy`, `/admin/privacy`; download-my-data button produces a timestamped JSON file; erase flow is gated behind a `type "DELETE" to confirm` alert dialog.
#### Paymob real checkout (P2.6)
New `encoach_lms_api/controllers/paymob.py` + `models/paymob_order.py`:
- `POST /api/payments/paymob/checkout` runs the three-step Paymob Accept flow (auth → order → payment key) and returns the hosted iframe URL. Our `encoach.paymob.order` row is created *before* the external call so partial failures don't lose orders.
- `POST /api/payments/paymob/webhook` verifies the HMAC-SHA512 signature over Paymob's canonical field sequence (`amount_cents|created_at|currency|…|success`). Mismatches return 401; duplicates are idempotent; unknown orders return 200 to stop Paymob retrying.
- Credentials read at request time from `ir.config_parameter` (`encoach.paymob.api_key`, `hmac_secret`, `integration_id`, `iframe_id`) so operators can rotate without restarting Odoo.
- `GET /api/paymob-orders` now reads real rows instead of returning `[]`.
#### Internationalisation (P3.1)
- `frontend/src/i18n/` bootstraps `i18next` + `i18next-browser-languagedetector` with nested feature-scoped namespaces.
- Initial locales: `en` (source of truth, typed with `Translations` interface) and `ar` (full translation of the same keys).
- RTL handling: when the active language is in `RTL_LANGS`, `document.documentElement.dir` is flipped to `rtl` so Tailwind utilities and Radix primitives adapt.
- `frontend/src/components/LanguageToggle.tsx` added next to the theme toggle in both `AdminLmsLayout` and `RoleLayout` headers.
- Loaded via `main.tsx` side-effect import so every route picks it up.
#### CI scaffolding (P3.8)
- `.github/workflows/ci.yml` with two jobs:
- **frontend**: `tsc --noEmit` → `eslint` → `vite build` → Playwright smoke tests against the Vite preview server; uploads a Playwright HTML report on failure.
- **backend**: spins up Postgres 16 as a service and runs the Odoo 19 container with `--test-enable --test-tags encoach_api` against a throwaway `encoach_ci` database.
- `backend/custom_addons/encoach_api/tests/test_health.py` — first `HttpCase`: asserts `/api/health` returns 200 and `/api/health/ready` returns 200 or 503 (never 5xx).
- `frontend/playwright.config.ts` + `frontend/e2e/login.spec.ts` — the login page renders the sign-in form; `/` redirects unauthenticated traffic to `/login`.
- New npm scripts: `test:e2e`, `test:e2e:install`.
### 21.2 New DB tables (summary)
| Table | Purpose | Added in |
|---|---|---|
| `encoach.ai.prompt` | Versioned prompt templates (`key`, `version`, `is_active`, `content`). | P3.4 |
| `encoach.ai.feedback` | Student thumbs up/down on AI output, with triage status. | P3.5 |
| `encoach.gdpr.erasure.request` | Tombstone row per right-to-erasure request. | P3.2 |
| `encoach.paymob.order` | Full lifecycle of every Paymob checkout, including the verified HMAC. | P2.6 |
All have per-group `ir.model.access.csv` entries: admin read/write, authenticated-user read for metadata, create-only on feedback.
### 21.3 New REST endpoints
Everything requires a valid JWT unless noted; write routes additionally check `base.group_system` where relevant.
```
GET /api/ai/prompts list keys (latest per key)
GET /api/ai/prompts/:key/versions full history for a key
GET /api/ai/prompts/:id one version (full content)
POST /api/ai/prompts create new version (admin)
POST /api/ai/prompts/:id/activate flip active pointer (admin)
POST /api/ai/prompts/:id/render dry-run render with sample vars
POST /api/ai/feedback upsert (user)
GET /api/ai/feedback/summary up/down counts + my rating
GET /api/ai/feedback admin triage list
POST /api/ai/feedback/:id/resolve admin triage resolve
GET /api/gdpr/export full data-subject export (self)
POST /api/gdpr/delete right-to-erasure (self, confirm=true)
POST /api/payments/paymob/checkout start a checkout
POST /api/payments/paymob/webhook HMAC-verified callback (public)
GET /api/payments/paymob/orders list my orders
GET /api/paymob-orders admin + self order list (rewired)
```
### 21.4 Frontend surface (new routes)
```
/admin/exam/review-queue → ExamReviewQueue.tsx
/admin/exam/review/:examId → ExamReviewDetail.tsx
/admin/ai/prompts → AIPromptEditor.tsx
/admin/ai/feedback → AIFeedbackTriage.tsx
/student|teacher|admin/privacy → PrivacyCenter.tsx
```
Plus the `AIFeedbackButtons` primitive (drop-in `<ThumbsUp/>/<ThumbsDown/>` that handles the upsert, dialog, and comment prompt) and `LanguageToggle` in both main layouts.
### 21.5 Verification
- `python3 -m compileall -q backend/custom_addons/encoach_ai backend/custom_addons/encoach_api backend/custom_addons/encoach_lms_api` — clean.
- `npx tsc --noEmit -p tsconfig.app.json` — clean.
- `npm run build` — succeeds; bundle analyser shows the same split profile as the §20 release (initial JS ≈ 250 KB gzipped on the login path); new features are lazy-loaded chunks.
- New modules do not depend on optional addons: all cross-model reads use `if model in request.env` guards so missing addons degrade gracefully instead of 500-ing.
### 21.6 Configuration operators need to set
Before the Paymob flow will actually charge anything, set in **Settings → Technical → System Parameters**:
```
encoach.paymob.api_key = <merchant API key>
encoach.paymob.hmac_secret = <from Paymob dashboard>
encoach.paymob.integration_id = <default integration (card)>
encoach.paymob.iframe_id = <hosted iframe id>
```
Until those are set, `POST /api/payments/paymob/checkout` returns 503 with a descriptive message instead of silently crashing.
### 21.7 Deferred (known)
- `P1.2` — blanket-sudo audit + entity-isolation `ir.rule`s. Needs a coordinated frontend + data migration pass; deferred to a follow-up release. Current security model relies on JWT identity + controller-level `has_group` checks.
- Broader i18n coverage. Only the keys listed in `src/i18n/locales/en.ts` are translated today — extending to every page is a rolling job that can happen in small PRs now that the plumbing is in place.
- Playwright coverage is intentionally minimal (login + redirect); it exists as a safety net, not as full end-to-end coverage.

26
docs/adr/0000-template.md Normal file
View File

@@ -0,0 +1,26 @@
# ADR {NUMBER}: {TITLE}
- **Status:** Proposed | Accepted | Superseded by ADR-XXXX
- **Date:** YYYY-MM-DD
- **Deciders:** @handle1, @handle2
## Context
What is the problem? What forces are at play (technical, business, social)?
Keep this short; link out to source files or tickets for depth.
## Decision
What did we decide to do? Use imperative voice. Be specific enough that a new
engineer can understand the scope by reading this section alone.
## Consequences
- Positive: …
- Negative / trade-offs: …
- Follow-up work: …
## Alternatives considered
- **Option A** — why it was rejected.
- **Option B** — why it was rejected.

View File

@@ -0,0 +1,52 @@
# ADR 0001: Canonical `backend/` and `frontend/` directory layout
- **Status:** Accepted
- **Date:** 2026-04-08
- **Deciders:** Platform team
## Context
The repository historically contained two overlapping copies of server code:
- `backend/custom_addons/` — Odoo addons, actively developed.
- `new_project/custom_addons/` — older snapshot, intermittently used for
one-off scripts and diverging subtly from `backend/`.
Both paths showed up in `odoo.conf` variants, deploy scripts, and developer
onboarding docs. Contributors routinely edited the wrong copy, shipped drift,
or rediscovered that fixes "weren't taking" because they landed in the stale
tree.
The same ambiguity existed implicitly on the client side: several prototype
apps lived under `new_project/frontend/` in addition to the main
`frontend/` workspace.
## Decision
Declare a single canonical layout:
- `backend/custom_addons/**` — the only Odoo addons tree. All deployments,
tests, and Docker images read from here.
- `frontend/**` — the only React/Vite workspace.
- `new_project/`**deprecated**. A `DEPRECATED.md` is committed inside the
directory explaining the policy; no new files may be added.
New `encoach_*` modules and frontend features MUST land under the canonical
paths. Existing imports, Dockerfiles, and docs were updated accordingly.
## Consequences
- Positive: one obvious place to work, deterministic CI, no drift.
- Positive: `PROJECT_SUMMARY.md` and this README become credible onboarding
material.
- Negative: scripts that hard-coded `new_project/` paths had to be migrated
(one-time cost, done).
## Alternatives considered
- **Delete `new_project/` outright.** Rejected for now because a few historic
tarballs and experiment scripts still reference it; leaving the tree with a
`DEPRECATED.md` marker lets us retire it in a later cleanup pass without
blocking the hardening release.
- **Rename `backend/` to `odoo/` to mirror Odoo's own layout.** Rejected
because `odoo/` is already used for the upstream Odoo source checkout.

View File

@@ -0,0 +1,66 @@
# ADR 0002: JWT access + refresh tokens with revocation ledger
- **Status:** Accepted
- **Date:** 2026-04-09
- **Deciders:** Platform team, Security
## Context
Originally `/api/login` issued a single long-lived JWT (24h+) stored in
`localStorage`. This gave us three problems:
1. **No revocation.** A leaked token was valid until it expired; there was no
server-side way to invalidate it short of rotating the global JWT secret.
2. **Silent logouts.** When the token expired mid-session the browser just
started receiving 401s with no graceful recovery path.
3. **Surface area.** Every endpoint accepted the same kind of token, so a
token intended for a refresh use-case could be replayed as a full API
credential.
## Decision
Adopt a two-token flow:
- **Access token** — 1 h TTL, stateless, carries `type: "access"`. Sent on
every request as `Authorization: Bearer …`. `validate_token()` in
`encoach_api.controllers.base` rejects tokens whose `type` is anything other
than `"access"`.
- **Refresh token** — 7 d TTL, carries `type: "refresh"` and a unique `jti`.
Every issued refresh token is logged in a new `encoach.jwt.token` Odoo model
(the revocation ledger) with fields for `user_id`, `issued_at`, `expires_at`,
`last_used_at`, `revoked`, `user_agent`, `remote_ip`.
Endpoints:
- `POST /api/login` — returns `access_token`, `refresh_token`, `expires_in`.
- `POST /api/auth/refresh` — validates the refresh token, revokes the old
ledger row (rotation), and issues a fresh access + refresh pair.
- `POST /api/logout` — revokes the supplied refresh token's ledger row.
The frontend (`frontend/src/lib/api-client.ts`) handles rotation
transparently: on 401 it calls `/api/auth/refresh` once (coalesced across
concurrent requests) and retries the original request. If refresh fails, all
tokens are cleared and the user is redirected to `/login`.
A cron (`encoach_api.data.cron`) purges expired ledger rows daily.
## Consequences
- Positive: revocation works — logout or compromise clears the server-side
ledger entry and the refresh token is instantly unusable.
- Positive: short access-token TTL limits the blast radius of a leaked Bearer.
- Positive: the refresh flow is invisible to users; no more mid-session
logouts.
- Negative: one extra DB round-trip per refresh. Mitigated by the short-lived
access token and the fact that the ledger is indexed on `jti` + `user_id`.
- Follow-up: move ledger cleanup from a time-based cron to an event-based
cleanup if the table ever grows past a few hundred thousand rows.
## Alternatives considered
- **Opaque session tokens with a Redis store.** Rejected — adds an operational
dependency (Redis) that the rest of the stack does not yet require, and
complicates horizontal scaling.
- **Single JWT with short TTL + silent re-login.** Rejected — requires the
client to store credentials or an SSO cookie, neither of which we want in
`localStorage`.

View File

@@ -0,0 +1,60 @@
# ADR 0003: Canonical paginated response envelope
- **Status:** Accepted
- **Date:** 2026-04-09
- **Deciders:** Platform team, Frontend team
## Context
Different Odoo controllers returned paginated data in at least three shapes:
- `{ data: [...], total: N, page, limit }`
- `{ results: [...], count: N }`
- `[...]` (bare array, no totals)
The frontend grew defensive code paths to handle all three, and every new
endpoint risked inventing a fourth shape.
## Decision
Every list endpoint MUST return the canonical envelope produced by
`encoach_api.controllers.base.paginated_envelope`:
```json
{
"items": [ ],
"data": [ ],
"total": 123,
"page": 1,
"size": 20
}
```
- `items` is the canonical field name. New code reads from `items`.
- `data` mirrors `items` for backwards compatibility with older callers and
can be removed once every consumer migrates.
- `total` is the total number of matching records across all pages.
- `page` is 1-indexed.
- `size` is the requested page size (capped server-side).
On the frontend, `PaginatedResponse<T>` in `frontend/src/types/common.ts`
exposes both `items` and an optional `data`, and service methods
(`users.service.ts`, `lms.service.ts`, etc.) construct a clean
`PaginatedResponse` object from whatever the server returns so that UI code
never sees the legacy fields.
## Consequences
- Positive: frontend code is simpler and typesafe — read `items`, done.
- Positive: OpenAPI spec advertises one consistent shape across endpoints.
- Negative: one additional key (`data`) is duplicated in responses. Cheap
(same reference, no JSON bloat) and easy to delete later.
## Alternatives considered
- **Follow JSON:API's `{ data, meta, links }` convention.** Rejected as
overkill: we don't use HATEOAS, and the extra indirection would force a
rewrite of every existing consumer.
- **Return a bare array + pagination headers.** Rejected because Odoo's
controller helpers make setting custom headers awkward and because it hides
totals from curl/Postman users.

View File

@@ -0,0 +1,67 @@
# ADR 0004: RAG metadata + chunking for vector store
- **Status:** Accepted
- **Date:** 2026-04-09
- **Deciders:** AI team, Platform team
## Context
The first cut of the vector store (`encoach_vector`) stored one embedding
per source record, keyed only by `(model, res_id)`. This had two problems:
1. **Long documents dominated similarity scores.** A 20 000-character lesson
would embed as one vector and out-vote shorter, more relevant passages.
2. **No tenancy filtering.** Retrieval could not be scoped to a particular
course, subject, entity, or taxonomy topic, which meant RAG pulled content
from unrelated tenants on multi-entity deployments.
The quality gate (`encoach_quality_gate`) also needed a way to deduplicate
re-ingested content so that re-running the indexer did not explode the table.
## Decision
Extend `encoach.vector.embedding` with RAG metadata columns:
| Field | Purpose |
|-------|---------|
| `course_id` | Scope to a specific course. |
| `subject_id` | Scope to a subject/domain. |
| `entity_id` | Tenancy filter — critical for institutional deployments. |
| `taxonomy` | Free-form tag (e.g. `"IELTS/writing/task1"`). |
| `content_hash` | SHA-256 of the raw chunk; used for dedup. |
| `chunk_index`, `chunk_total` | Position in the parent document. |
Chunking policy (see `encoach_vector.services.embedding_service`):
- Content ≤ 2 000 chars → embedded as a single chunk.
- Content > 2 000 chars → split on paragraph boundaries with ~200-char
overlap, each chunk embedded individually.
- Each chunk stores its `content_hash`; the uniqueness constraint is
`(model, res_id, chunk_index, content_hash)` so re-indexing is idempotent.
The indexer (`encoach_vector.services.indexer`) declares per-model metadata
mapping (which field feeds `course_id`, which feeds `subject_id`, etc.) so
adding a new source model is a single config entry.
`similarity_search` accepts any subset of the metadata as a filter and
applies it as a SQL `WHERE` clause before the vector distance computation.
## Consequences
- Positive: retrieval quality improves dramatically on long documents.
- Positive: multi-tenant deployments can scope RAG to a single entity.
- Positive: re-indexing is safe (idempotent) and cheap.
- Negative: the embedding table grows roughly linearly with document length.
Mitigated by the `content_hash` dedup and by keeping only the latest
revision per source record.
- Follow-up: expose a management action to purge embeddings for a retired
course or entity.
## Alternatives considered
- **Use an external vector DB (Pinecone, Weaviate).** Rejected — pgvector is
already in the Postgres image, keeping ops surface small. Can be revisited
if we outgrow it.
- **Chunk-per-sentence instead of paragraph.** Rejected — too many tiny
chunks, each losing context; paragraph-sized chunks strike a better
recall/precision balance for our domain.

22
docs/adr/README.md Normal file
View File

@@ -0,0 +1,22 @@
# Architecture Decision Records
This folder contains lightweight ADRs documenting significant architectural
decisions made on the EnCoach platform. Each ADR is numbered, dated, and
immutable once "Accepted" — if a decision is revisited, open a new ADR that
supersedes the old one instead of rewriting history.
## Index
| # | Title | Status |
|---|-------|--------|
| [0001](0001-canonical-directory-layout.md) | Canonical `backend/` and `frontend/` directory layout | Accepted |
| [0002](0002-jwt-refresh-token-flow.md) | JWT access + refresh tokens with revocation ledger | Accepted |
| [0003](0003-paginated-response-envelope.md) | Canonical paginated response envelope | Accepted |
| [0004](0004-rag-metadata-and-chunking.md) | RAG metadata + chunking for vector store | Accepted |
## Writing a new ADR
1. Copy [`0000-template.md`](0000-template.md) to the next number.
2. Fill in **Context**, **Decision**, **Consequences**.
3. Keep it short (1 page). Link to source files or PRs for detail.
4. Update the index above and open a PR.