إنتقل إلى المحتوى الرئيسي

REVIEW.md — EduSuite

[Shared] Check against the PR's story and acceptance scenarios​

Read the PR description's "Story" and "Acceptance Scenarios (Given-When-Then)" sections (see .github/PULL_REQUEST_TEMPLATE.md) before reviewing the diff.

  • Flag any listed acceptance scenario with no code path in the diff that satisfies it — Missing scenario coverage (Important), name the scenario specifically.
  • Flag diffs that contradict a listed scenario, quoting the scenario.
  • If the section is empty, report once as a nit; don't guess intent from the diff alone.
  • Don't flag a scenario explicitly deferred per "Notes for the reviewer."

[Shared] Escalate to Important​

  • Rejected DB-access patterns (Policy §6, A.10). Any fastify.decorate('db', ...), or a raw DB client/transaction passed as an explicit parameter, instead of getDb() / withTenantScope() from @rarit-kernel/platform-core.
  • Direct vertical-to-vertical imports (Policy §4). Any import from another vertical (e.g. verticals/amaar, verticals/clinivio) into EduSuite, or vice versa — even with no cycle. Correct fix: promote to a @rarit-kernel/* package (A.8).
  • New engine-to-engine import creating a reverse dependency (A.7). If a change adds an import from one kernel engine into another, check whether the target already imports back — that's the IAM↔Notifications shape A.7 exists to prevent.
  • RLS enabled without FORCE (A.11.3). Any new ENABLE ROW LEVEL SECURITY on a tenant-scoped table without a paired FORCE ROW LEVEL SECURITY.
  • Cross-tenant reads outside withPlatformAdminScope() (A.11.2 / ACT-001). Any query that appears to read across tenants without going through the approved function with a logged reason.
  • Route handlers doing DB access directly. A DB query inside routes/, or a services/ function accepting request/reply. Routes → services → repositories is mandatory.
  • Mutations to IAM/Payment state with no paired Audit write in the same transaction.

[EduSuite-specific] MVP scope guard — School K-12 only​

EduSuite's committed MVP scope is School K-12 only, Must/High priority items only (~43 stories / ~160 points per the current Requirements Backlog) — everything else was deliberately cut in the August 2026 simultaneous-launch pivot, not deferred by accident.

  • Flag as Important any PR that adds a route, module, manifest entry, or DB table for a facility type or scope other than School K-12 (e.g. university/higher-ed, vocational/training-center patterns) unless the PR description explicitly references an approved backlog item reintroducing that scope.
  • Flag as Important any PR implementing a Should/Could/Won't-priority backlog item unless the PR description says it was explicitly reprioritized (link the backlog change).
  • This is a scope check, not a quality check — if uncertain whether something is in scope, say so as a question rather than asserting a violation; don't guess at backlog status from the diff alone.

[EduSuite-specific] Entitlements manifest patterns​

EduSuite is the kernel's reference app for entitlement types beyond simple booleans (see examples/edusuite-api in rarit-saas-kernel, built specifically to prove the Entitlements Engine generalizes across limit, boolean, and metered types — e.g. the Enrollment module).

  • Flag any new EduSuite module that checks a tenant's plan or seat count directly instead of calling checkEntitlement(key) — this logic belongs in the Entitlements Engine, never reimplemented locally (see Entitlements_Engine_Developer_Guide.md, "Mental model").
  • Flag any entitlement key used in a route or manifest that isn't first declared in that module's own manifest.json — the manifest is the only place entitlement keys are defined.
  • Flag a limit-type entitlement (e.g. max enrolled students, max active classes) enforced with a hardcoded number instead of reading the resolved entitlement value.

Report as-is (default severity)​

  • New API routes without an integration test.
  • Manifest or event-definition logic placed under v1/ instead of core/ (version-axis mixup, not a REST contract change).

Nit only​

  • General CLAUDE.md footguns already covered there — don't duplicate.
  • Naming/formatting inconsistencies with no functional effect.

Skip​

  • *.md, docs/**, signed-records/**.
  • Generated files: openapi.json, dist/**, build/**.

Nit volume​

At most five nits per review; mention any remainder as a count in the summary.

Notes for reviewers​

Findings are informational — Code Review does not approve or block PRs (see ACT-005). Review Behavior: Once after PR creation.