Skip to content

SIS Backend — Open-Items Audit — 2026-07-03

What this file is. The single consolidated audit of everything still open in the SIS backend, merging the prior architecture-flaw and quality/convention audits into one open-items list. It is not a closure ledger: remediated findings are dropped entirely, there are no "done" marks, and there is no history section. Every claim is grounded in working-tree source (file:line); line numbers were accurate at audit time — verify against the current tree before quoting, they shift.

Scope. This pass deliberately excludes two axes tracked in their own files and left untouched here: test coverage (2026-07-02_test-cov.txt, 2026-07-02_tests-todo.md) and the architecture judgment narrative (2026-07-02_architecture-judgment.md). Missed-tests / e2e-infrastructure findings are therefore not restated below.

Severity (blast-radius × likelihood, weighted for this system — multi-tenant, children's PII, GDPR-relevant): serious = structural debt with real correctness, security-regression, or scaling consequences · latent = real but currently fenced by other factors · medium = maintainability/contract cost with a coordination dependency · low = polish / convention-drift with real-but-small cost. Architecture anchor #5 keeps its original non-contiguous number so the wave specs and memory keep pointing at it; the quality backlog is an unnumbered per-area list. (R4 + R6 were remediated 2026-07-06 and dropped per the open-items policy — spec docs/superpowers/specs/2026-07-02-school-relationship-flags-ay-create-contract-design.md.)

Part 1 — Architecture flaws (open)

One architecture finding remains open. The security/integrity boundaries themselves are now enforced structurally (RLS on all tenant-bearing tables as app_user, boot/CI drift guards, DB-backed uniqueness), so what is left is not a boundary-enforcement gap: one serious module-coupling/scaling cliff. (The two latent data-contract footguns previously tracked here — R4 school-relationship flag divergence, R6 AY create-pin asymmetry — were remediated 2026-07-06 and dropped; spec docs/superpowers/specs/2026-07-02-school-relationship-flags-ay-create-contract-design.md.)

# Flaw Severity Anchor evidence
5 Cross-module mutations run as one synchronous transaction serious (5/10) curriculum.service.ts, students/class-selection-sync.ts, curriculum/selection-consistency.ts, prisma.service.ts

5. Cross-module mutations run as one synchronous transaction — serious (5/10)

The flaw. A curriculum edit, a setup bulkSync, a homeroom/SG create, and a selection write each fan out synchronously across 4 modules inside one interactive Prisma transaction, all "required" (any failure rolls the parent back — none offloaded to the afterCommitQueue, which is used only for invitation mail). The worst case — setup bulkSync — is O(curricula × selections): it loops every curriculum, takes FOR UPDATE, captures pre-images of every selection, syncs structure, replays findSelectionInconsistencies over every selection, and seeds SGs — touching students, subject_groups, and homerooms tables, all in one transaction.

Blast radius. (1) Scaling cliff: transaction size is unbounded by data; a large secondary school finalizing setup or editing a heavily-enrolled curriculum hits the transaction-timeout ceiling and the entire edit aborts with no partial-progress path. (2) Architectural blocker: curriculum ↔ students ↔ subject-groups ↔ homerooms is one consistency boundary today — it cannot be split into independent services (the stated "modules → microservices" goal) without a distributed-transaction/saga layer.

Concrete failure scenario. A tenant with ~2,500 enrolled students under one curriculum; an admin renames a subject and re-levels HL→SL via PATCH /curricula/:id. The handler locks the curriculum, captures pre-images for all 2,500 selections, syncs structure, then replays consistency over all 2,500 plus a committed-substance diff, then seeds SGs — exceeding the interactive-transaction timeout. Prisma aborts (P2028), the edit rolls back, and concurrent selection writes were blocked behind FOR UPDATE the whole time. Retry hits the same wall.

Evidence. - Forward sync writes a selection from inside the homeroom/SG transaction; backward prune deletes class memberships from inside the selection transaction — src/students/class-selection-sync.ts. - Post-mutate sweep replays over every selection — src/curriculum/selection-consistency.ts. - Grid PATCH cascade and the bulkSync loop (O(curricula × selections)) — src/curriculum/curriculum.service.ts. - Transaction budget is raised to 30s timeout / 10s maxWait precisely because bulkSync was aborting under the 5s default — src/prisma/prisma.service.ts. - afterCommitQueue exists but is used only for mail — src/common/utils/after-commit-queue.ts. - The team's own maintainability chapter acknowledges the coupling and proposes a domain-event hook — docs/16-maintainability.md.

Honest mitigations. Correctness under concurrency is sound (FOR UPDATE on edits, FOR SHARE on selection writes serialize the race the sweep depends on) — this is a latency/abort + evolvability risk, not data corruption. The READY→READY seed is diff-restricted to newly-offered cells.

Fix (descoped to the canary). Near-term: ship a selection-count scaling e2e so the cliff shows up in CI. Deferred until the scale trigger fires: page the selection replay; move the idempotent sweep/seed into an afterCommitQueue task that re-opens short per-batch transactions; introduce the SelectionChanged / HomeroomStudentMoved domain-event seam (even synchronous in-process) — the seam a future service split needs. Governing spec: docs/superpowers/specs/2026-06-25-wave4-domain-event-seam-design.md (Draft; Q1 replay-vs-write concurrency and Q2 eventual-consistency-window acceptability unresolved).

Still-valid disconfirmations (checked, not flaws)

  • Raw SQL is tenant-safe. All $queryRaw/$executeRaw sites carry tenant_id in the WHERE; the RLS backstop (enforced at the DB for app_user) is the load-bearing control the app-layer guard cannot provide.
  • Attendance / audit-log FK-less snapshots are correct. The plain-column-no-FK design on AttendanceRecord context columns and the soft entityId on AuditLog are intentional immutability guarantees.
  • update/remove do not retarget the year. They act on the record's own academicYearId (fetched by { id, tenantId }); only create pins to ACTIVE (now a documented contract — former R6).
  • The RBAC runtime path is fail-closed. FieldFilterInterceptor's one fail-open branch is unreachable on correctly-decorated routes (the guard populates permissions first).

Part 2 — Quality / convention backlog (open)

All LOW unless marked. These are drift-away-from-convention polish items — no cross-tenant leak, missing record filter, or broken write gate is among them. The one medium is the FE-coordinated route rename.

Cross-module / architecture

  • [medium · FE-coordinated] Three modules mount controllers under /students; correctness rests on a commented import order. src/homerooms/eligible-students.controller.ts and src/subject-groups/eligible-students.controller.ts both @Controller('students') with literal @Get('eligible-for-*'), while students.controller.ts has @Get(':id'); src/app.module.ts:103-111 carries a comment requiring both picker modules be imported before StudentsModule (route-registration order) or Express 5 captures the literals with :id. Only the existing e2e keeps this from being higher severity. Fix: give the pickers a non-colliding prefix (e.g. @Controller('homerooms') @Get('eligible-students')) — FE-coordinated if the path changes; otherwise treat "no two modules share a :param route prefix" as an architectural rule.
  • [bad-practice] Cross-module deep imports bypass the barrel even though the symbol is on it. auth/auth.service.ts:18 (../files/files.service), permissions/permissions.service.ts:4 (../custom-fields/custom-fields.service), and curriculum/curriculum.queries.ts:5-6 (evaluationScaleRefSelect/subjectLevelRefSelect from foreign .queries files). (The referents → ../students/school-relationships.recompute deep import was removed 2026-07-06 when that module was deleted — audit R4.) Fix: repoint to the barrels. A no-restricted-imports rule banning ../*/*.{service,queries,policy,mapper} from outside the owning folder would make this self-enforcing.
  • [missed-pattern] tenantId read inconsistently — @TenantId() in most controllers, req.user.tenantId in a few. permissions.controller.ts, roles.controller.ts, role-assignments.controller.ts, and setup.controller.ts read req.user.tenantId directly via @Req(); @TenantId() is the documented single extraction site used everywhere else. Both are type-safe — pure drift. Fix: migrate the four to @TenantId() when next editing them (auth.controller.ts legitimately needs @Req()).
  • [missed-pattern] Setup bulkSync transaction convention diverges across the wizard. departments.service.ts uses the shared runBulkSyncTx; rooms.service.ts:476 and curriculum.service.ts:393 hand-roll $transaction + their own P2002 catch; school.service.ts uses none. Fix: converge rooms/curriculum onto runBulkSyncTx, or at least route their duplicate-key mapping through mapP2002.
  • [potential-bug] school.bulkSync writes the school row then the logo file with no enclosing transaction. src/school/school.service.ts:172-185 — school.upsert(...) then a separate await applyLogo(...) (storage + FK update), yet applyLogo JSDoc says "replace it atomically." A logo-apply failure leaves the scalar fields committed — the only setup write path with zero transaction. Blast radius small (per-tenant singleton, wizard-driven). Fix: wrap the pair in one $transaction, or drop the "atomically" wording.
  • [missed-pattern] mapP2002 typed-409 strategy adopted in only some unique-constraint modules; the rest race read-then-check. evaluation-scales, subject-levels, room-types, timetable-templates still do read-then-check and fall back to the filter's generic CONFLICT { entity } on the race. Fix: prefer mapP2002 when next touching those write paths so the race closes and the typed code survives (see also SUBJECT_LEVEL below).
  • [missed-pattern] Dynamic-delegate "in-use reference counter" hand-rolls its own as unknown as {…} cast in the two preset modules. evaluation-scales.service.ts (existsInUse/referenceCounts/computeInUseMap) and subject-levels.service.ts (computeInUseMap) each punch through the delegate typing per call. getPersonDelegate already exists to own exactly this single cast. Fix: add one thin accessor that owns the client[model] as unknown as … cast; the three call sites keep their own methods (findFirst/count/groupBy).
  • [bad-architecture] @Global() on FilesModule + PermissionsModule hides real dependencies. files/files.module.ts:23 and permissions/permissions.module.ts:16 are global; auth.module.ts imports neither yet auth.service.ts injects both. For a "module = future microservice" north star, globally-ambient domain providers are exactly the coupling a split must untangle. Fix: defer unless/until a service extraction is planned; then drop @Global() and import explicitly.
  • [overengineering] permissions↔custom-fields module-level forwardRef guards a cycle that does not exist. permissions.module.ts:18 imports: [forwardRef(() => CustomFieldsModule)], but custom-fields.module.ts does not import PermissionsModule (it pulls the guard via the @Global() PermissionsModule). The provider-level @Inject(forwardRef(...)) is genuinely needed; the module-level one is not. Fix: replace with plain imports: [CustomFieldsModule] (keep the provider-level forwardRef); verify by booting.

Domain / academic-year integrity (rollover-blockers, latent today)

  • [domain] Curriculum-selection write + bulk-assign never call assertYearWritable, and admin write bypasses the window gate. students/curriculum-selection.service.ts write gates referents on window-OPEN + referentCanWrite but lets isAdmin through with no archive guard, resolving the student's stored academicYearId (:153); bulk-curriculum-assignment.service.ts likewise omits assertYearWritable. Harmless today (no API path produces an ARCHIVED year), but the stored-year writers are the paths that genuinely escape the guard once archiving ships. assertYearWritable's own docstring frames it as universal; homerooms/SGs/timetables also skip it but are moot (active-year-only). Fix (rollover prerequisite): add assertYearWritable to the curriculum-selection write + bulk-assign paths; document the active-year-only surfaces as intentionally exempt.
  • [potential-bug] Timetable publish gate is a TOCTOU. timetables.service.ts:565-621 — the PUBLISH branch runs diagnose() → single-PUBLISHED findFirst pre-check → updateTimetable(PUBLISHED) sequentially with no enclosing $transaction (contrast mutateLessonAndDiagnose, which wraps mutate+diagnose in one tx). A concurrent DRAFT edit can introduce an ERROR between diagnose and publish, shipping an invalid timetable. Admin-only + narrow window. Fix: wrap the publish branch in $transaction and re-run buildDiagnosticsSnapshot + conflict-check inside.

students

  • [type-system] loadDepartmentGradeCatalog forces a tx as unknown as PrismaService double-cast. students.service.ts:1491 calls it with the cast; the helper (students.queries.ts:385) types its first param prisma: PrismaService instead of PrismaService | Prisma.TransactionClient — the lone queries.ts helper opting out of the client-union convention every sibling follows. Fix: widen the param to the union; delete the cast.
  • [missed-pattern] getRolloverReviewPage hand-rolls pagination + filter assembly inline. students.service.ts:1297-1332 — manual page/limit/skip clamping and an inline Prisma.StudentWhereInput instead of a buildListArgsFor… helper in students.queries.ts. Fix: extract a pure buildListArgsForRolloverReview helper.
  • [verbosity] In-tx consistency-reject block duplicates the out-of-tx one. students/curriculum-selection.service.ts:320-326 (out-of-tx) and :374-388 (in-tx locked replay) copy-paste the same findSelectionInconsistencies + splitSelectionViolations + SELECTION_VALIDATION_FAILED throw. Fix: extract a local assertConsistent(structure, isAdmin) closure called from both passes.

curriculum

  • [duplications] P2003 SubjectGroup-in-use disambiguation triplicated with divergent count semantics. curriculum.service.ts:551-582 (bulkSync catch) re-implements the disambiguation that throwIfSubjectGroupInUse already does; the service paths count ALL SGs { curriculumSubjectId } while the canonical guard guardAndClearStandaloneSGs (curriculum-structure-sync.ts:438) counts only SGs with students (assignments: { some: {} }) — so the displayed subjectGroupCount can contradict the "taught to students" message on a rare TOCTOU. Fix: have bulkSync's catch call throwIfSubjectGroupInUse; align its count filter to assignments: { some: {} }.
  • [bad-practice] CURRICULUM_IN_USE_BY_SELECTIONS in bulkSync emits empty curriculumId / fixed selectionCount: 1. curriculum.service.ts:581 — the P2003 selections fallback passes { curriculumId: '', selectionCount: 1 }, defeating the typed params the FE renders. Fix: thread the real id + a real count, or drop the placeholders.
  • [type-system] expandPreset returns an un-annotated shape that reaches the wire with no DTO tie. curriculum.service.ts:599 — no declared return type; the { name, subjects, optionBlocks } value is forwarded to the setup endpoint with no response DTO, so the contract is inference-only end-to-end. Fix: give expandPreset an explicit return interface and reference it from the endpoint's Swagger.
  • [verbosity] ctx ? Policy.where(ctx) : { tenantId } visibility boilerplate copy-pasted ~10×. Across curriculum.service.ts, selection-window.service.ts, curriculum.queries.ts:109 (and mirrored in homerooms.service.ts:1117/1142, subject-groups.service.ts:979/1002, referents :156). No shared helper. Fix: add a one-line visibleWhere(policy, ctx, tenantId) helper.
  • [stale-doc] assertAllCriteriaShapes JSDoc still claims a removed "contiguous 1..n" rule. curriculum-structure-sync.ts:251 — the JSDoc says unique names + contiguous 1..n are enforced, but assertCriteriaShape (175-197) only checks unique names (ordinal was removed). Fix: drop the "+ contiguous 1..n" clause.

timetables

  • [potential-bug] STUDENT_CONFLICT renders the student UUID as its displayName. timetables.diagnostics.ts:227 — student: { id: sid, displayName: sid }; the snapshot carries only studentIds (no names), so the FE gets a raw UUID as the human label while every other violation category denormalizes a real name. Fix: batch student names into the snapshot (one student.findMany mirroring the homeroom-roster batch) and use them; or leave a code comment marking it a v1 limitation.
  • [duplications] checkSameDayContiguity and checkSameRoomDouble rebuild an identical byGroupDay index. timetables.diagnostics.ts:586,621 — byte-for-byte Map<${subjectGroupId}|${weekday}, SnapshotLesson[]> loops. Fix: extract a groupByGroupDay(lessons) helper.
  • [bad-practice] Secondary snapshot/view queries omit a defensive tenantId. timetables.queries.ts:357,453,538,548 and viewWhere in timetables.service.ts:718 filter by timetableId/target id but never tenantId. No leak today (transitively tenant-constrained), but the standing rule is every tenanted query carries an explicit tenantId. Fix: add tenantId to these wheres and the viewWhere target predicates.
  • [duplications] GenerationService.toResponse duplicates TimetablesService.toResponse. generation/generation.service.ts:267 and timetables.service.ts:49 — identical TimetableSummaryRow → TimetableResponseDto projection. Fix: export a free toTimetableResponse(row) from timetables.queries.ts; call from both.
  • [code-smell] TIMETABLE_SOLVER_TIME_LIMIT_SECONDS is read + parsed twice, unvalidated. generation/generation.service.ts:144 (solver config) and generation/solver.port.ts:190 (fetch abort budget) both Number(process.env… ?? '300') with no Number.isFinite guard — a malformed value yields NaN → AbortSignal.timeout(NaN). Fix: one resolveSolverTimeLimitSeconds() that coerces + falls back on NaN/≤0, consumed by both.
  • [type-system] Two response-DTO sub-objects use inline/object types instead of DTO classes. dto/timetable-response.dto.ts:29 (subjectGroup inline object) and :47 (effectiveWindow via Swagger type:'object'); the file already defines NamedRef/SlotGrid. Fix: promote to SubjectGroupRef + DateWindowDto classes.
  • [code-smell] setStatus publish-conflict backstop emits blockingTimetableId: ''. timetables.service.ts:617 — the mapP2002 backstop throws TIMETABLE_PUBLISH_CONFLICT with an empty-string id the contract types as a required id. Fix: re-query the blocking row's id in the catch, or make the param optional and omit it on the index-race branch.

timetable-templates

  • [potential-bug] "Too many slots" overflow reuses reason: 'EMPTY'. timetable-templates.service.ts:987-993 — the slots.length > MAX_SLOTS_PER_DAY_TEMPLATE branch throws DAY_TEMPLATE_INVALID_SLOTS with reason: 'EMPTY' (the semantic opposite), and the reason union has no over-max member. Fix: add a 'TOO_MANY_SLOTS' member to the union in error-codes.ts and emit it.

permissions

  • [potential-bug] Parametric-overlap admission is unreachable on @AppliesPolicy-only routes; JSDoc + docs imply otherwise. roles.guard.ts:21 JSDoc claims parameterDimensions is "populated by the JWT payload at login," but it is populated only inside ensurePermissionsLoaded (which ScopeGuard/ActionGuard trigger) — so on command-center dashboard routes (no scope/action guard) the overlap branch sees undefined → [] and never fires. Bites nothing today only by coincidence (no @AppliesPolicy-only route uses a policy with parametricBranches); docs/04-rbac.md:1023 invariant #4 implies it holds everywhere. Fix: correct the JSDoc (it is not a JWT claim) and narrow invariant #4 to "routes with a scope/action gate."
  • [code-smell] system-managed-fields hardcodes raw entity-key strings. permissions/constants/system-managed-fields.ts:20 — outer record typed Record<string, …> keyed by 'students'/'teachers'/etc. rather than Partial<Record<EntityKeyValue, …>> + EntityKey.*; a typo or rename silently makes the sub-field rule a no-op. Fix: type + key it with the EntityKey constants (compiler-checked).
  • [duplications] buildEntityDto and buildRoleEntityPermissions duplicate the scope/action assembly. permissions.service.ts:331 and :461 — identical NONE-filter + catalogue-actions→booleans construction. Fix: extract a buildScopeActionMaps(...) both call, then layer their differences.

teachers

  • [code-smell] TEACHER_EXTRA_REGISTRY key invalid_dept is dead. teachers/constants/import-schema.ts:227 — the validator emits departments:invalid and the aggregator keys on the post-: suffix (invalid), so invalid_dept never matches. Fix: drop it (or rename keys to the resolved suffixes) and trim the comment.
  • [potential-bug] assertTeachersInTenant maps a bad teacher reference to 409 CONFLICT. teachers/assert-teachers-in-tenant.helper.ts:32 — a malformed/forbidden reference should be 400/404, not 409. FE-facing status nit (TEACHER_NOT_IN_TENANT already gives a stable code). Fix: switch to NOT_FOUND/BAD_REQUEST after checking the cross-module call sites (homerooms/subject-groups) don't rely on 409.

setup

  • [potential-bug] ParseGroupIdPipe in operator admits inherited prototype keys. setup/pipes/parse-group-id.pipe.ts:8 — !(value in SETUP_GROUPS) lets constructor/toString/etc. pass the "known group" gate. Tiny blast radius (routing hint only, response stays tenant-scoped). Fix: Object.prototype.hasOwnProperty.call(SETUP_GROUPS, value) or test the enum.
  • [type-system] SetupCompleteStateDto.data uses Swagger type: Object. setup/dto/setup-state-response.dto.ts:213 — emits an untyped {} schema on a field whose TS type is literally null. Fix: drop type: Object.

subject-groups

  • [code-smell] Dead query function findOptionBlockSelectorIds. subject-groups/subject-groups.queries.ts:295 — exported, no non-test caller (in-block eligibility is decided inline by classifySubjectGroupEligibility). Fix: delete it.
  • [missed-pattern] Create-path gate re-queries OptionBlock.maxSelections the caller already holds. subject-groups.service.ts:1190 — assertStudentsEligible issues its own optionBlock.findUnique for maxSelections even on create, where the resolved anchor already carries it. Fix: give assertStudentsEligible an optional maxSelections? and pass it from create; keep the findUnique fallback for add/move.
  • [domain] moveStudents checks same-subject but not same-grade — API-direct cross-grade move possible. subject-groups.service.ts:915 — two standalone SGs sharing curriculumSubjectId can live in different grades, and move re-points with no eligibility re-check, producing a roster where the SG teaches grade H but holds a grade-G student. FE never does this; API doesn't enforce it. Fix: also require source.gradeId === target.gradeId (confirm same-subject/different-grade standalone SGs are a real config first).

command-center

  • [duplications] Duplicated referent-caller predicate across service + queries. completeness.queries.ts:177 and completeness.service.ts:344 both carry the self-narrow predicate (a comment cross-links them); one decides which rows return, the other which row gets isMe. Fix: extract one pure isSelfNarrowedReferent(ctx) in shared/.
  • [missed-pattern] csvToArray is a weaker reimplementation of @TransformToArray(). command-center/shared/query-helpers.ts:17 — no trim/empty-filter/undefined-on-empty, used via @Transform in three query DTOs. Fix: replace with @TransformToArray() and delete csvToArray.

homerooms

  • [potential-bug] Move/add-path P2002 on (studentId, subjectGroupId) maps to the misleading STUDENT_ALREADY_IN_HOMEROOM. homerooms.service.ts:1326 — mapWizardP2002Target tests curriculum_subject_id then /student_?id/i; the (studentId, subjectGroupId) unique lacks curriculum_subject_id, so its race falls through to the homeroom branch. Rare (broader constraint usually trips first). Fix: add a JSDoc note on the fall-through, or tighten the branch with a homeroom-context token only if it surfaces.

departments

  • [missed-pattern] removeForDepartment hand-rolls delete + P2025 catch the base owns. grades.service.ts:241-253 — reproduces the base remove's P2025→NOT_FOUND mapping. Fix: route through super.remove(id, tenantId) after the ownership/existence guard; keep validateDepartmentOwnership as the 404-on-invisible-parent guard.
  • [code-smell] Policies carry unreachable student/referent/staff pass-through branches. departments.policy.ts:33,41,42 + grades.policy.ts:27,35,36 — those roles are never granted the read scope, so the branches are dead and (being tenant-flat) would silently grant tenant-wide enumeration if ever granted. Fix: delete them or replace 'pass-through' with NEVER_MATCH_WHERE.
  • [type-system] HookContext is an untyped string-index bag. common/interfaces/crud.interfaces.ts:1-4 — { [key: string]: unknown }; departments.service.ts:172,189 reads/writes ctx._studentPlatformAccessWasEnabled through it, so a typo silently disables the ON→OFF invitation cascade. Fix: declare a typed DepartmentsHookContext slot and type the hook ctx params.
  • [bad-practice] Detail grades list documented with an inline Swagger object schema. dto/scopes/department-configuration.dto.ts:215-227 — inline items:{ type:'object', properties:{…} } hand-duplicating GradeListItemDto (same file, line 238). Fix: @ApiProperty({ type: [GradeListItemDto] }).
  • [duplications] bulkSync param + getSetupSummary return inline the department-with-periods shape. departments.service.ts:656-673 and :910-927 — the full nested object type spelled out twice instead of referencing the wizard's DepartmentItemDto. Fix: reference a shared DTO/type.
  • [verbosity] toDetailResponse re-implements base scope assembly by hand. departments.service.ts:574-642 — manually rebuilds configuration (native fields + pickCustomFields + computeMissingFields + mergeDynamicScopes), so the field list is maintained in two places. Fix: build the base scoped response and augment configuration with grades + calendar.
  • [stale-doc] Governing calendar plan describes a nullable inherit-from-AY model that was reworked before ship. docs/superpowers/plans/2026-06-04-department-scoped-academic-calendar.md — describes nullable calendarStartDate/EndDate (null = inherit AY), resolveEffective*, Period.departmentId NULL default set, and the removed PeriodType.EXTRA; shipped code uses mandatory per-department dates. Fix: add a "Superseded / as-shipped" banner (dates mandatory, no inherit, EXTRA removed) pointing at project_department_scoped_calendar.

rooms

  • [duplications] timeToMinutes duplicated between validation and the DTO validator. rooms/rooms.validation.ts exports timeToMinutes; rooms/dto/scopes/room-configuration.dto.ts:40-43 defines an identical inline toMinutes closure in EndTimeAfterStartTime. Fix: import the exported helper.
  • [verbosity] bulkSync/syncRooms input shapes are large inline literals, duplicated. rooms.service.ts:458-471 and :624-634 — the same { roomTypes:[…]; rooms:[…] } / room-item shape spelled out twice. Fix: one named RoomsBulkSyncInput type referenced by both.
  • [stale-doc] Stale test comment references a removed "lazy seed." rooms/rooms-import.spec.ts:11 — "Standard types the lazy seed guarantees…"; standard types are now platform-global rows (tenantId IS NULL), no per-tenant lazy seed. Fix: update the comment.

auth

  • [missed-pattern] completeProfileSelection re-implements the centralized assertProfileAvailable. auth.service.ts:472-485 — inlines resolveAvailableProfiles + the availability throw the helper (:1092) already centralizes. Fix: call assertProfileAvailable(..., 'idempotent') (or add a 'select-profile' member mapping to 400).
  • [bad-practice] JWT_EXPIRATION default handling diverges. auth.module.ts:19-20 uses configService.get('JWT_EXPIRATION') ?? '1h' while auth.service.ts/cookie-helper.service.ts getOrThrow — so the '1h' fallback is dead code that falsely reads as "missing var tolerated." Fix: use getOrThrow in the module too.
  • [potential-bug] accessTokenExpiresAt recomputed from Date.now() rather than the signed JWT. auth.service.ts:580-581 — an independent re-derivation of the token's real exp; agrees today only because both read the same env var. Fix: return the decoded token's exp.
  • [bad-practice] LoginDto.email is unbounded while password is length-capped. auth/dto/login.dto.ts:11 — @IsEmail() with no @MaxLength on an unauthenticated route (feeds user.findMany + argon2 dummy-verify). Fix: add @MaxLength(254).
  • [stale-doc] Selection-token TTL is 10m but DTO JSDoc still advertises 60s. auth/dto/auth-response.dto.ts:121 + auth/dto/profile-selection-response.dto.ts:17 render "Short-lived JWT (60s)" verbatim into the public OpenAPI doc; the real TTL is SELECTION_TOKEN_TTL = '10m' (docs/03-auth.md already corrected). Fix: change both DTO comments to 10m (or drop the parenthetical).

referents

  • [duplications] deleteDocumentById issues the same row lookup twice. referents.service.ts:590 — its own findFirst({ id, ...ReferentsPolicy.where(ctx) }) + assertCallerIsSelfOrAdmin, then personDocs.deleteById runs requireRow with the identical WHERE again. Fix: run config.assertCanWrite inside PersonDocumentService.deleteById (coordinate with files/) and drop the pre-check; or accept the duplication (admin/self-only, low traffic).
  • [verbosity] School-relationship recompute over-fires on link-metadata-only PATCH. referents.service.ts:373 — every successful PATCH triggers a full per-linked-student recompute, even when only canWrite/isAuthorizedPickup/contact fields changed (flags derive only from taxCode/email/relationshipType). Fail-soft + post-commit, so bounded. Fix: gate the recompute on a flag-relevant change.
  • [missed-pattern] create hand-rolls field assembly instead of reusing flattenDto. referents.service.ts:211 — pickDefined({ ...identity, ...optionalContacts, ...documents }) duplicating flattenDto, diverging only to split email out. Fix: call flattenDto(dto) then omit(..., 'email'), or a tiny shared helper.

invitations

  • [stale-doc] StudentInvitationDto omitted from the controller extraModels. invitations/invitations.swagger.ts:32-38 — lists the other three recipient DTOs but not STUDENT (still emitted transitively via ProjectedInvitationDtoDocs). Cosmetic drift + a latent footgun if that decorator is ever dropped. Fix: add StudentInvitationDto for parity.
  • [verbosity] tokenTtlDays re-parses env on every access via a getter. invitations.service.ts:125-127 — configService.get + Number.parseInt per recipient (up to 500×/batch) on static deployment config. Fix: resolve once in the constructor into a readonly tokenTtlMs.

files

  • [duplications] 10 MB upload cap hardcoded across controllers instead of a shared constant. files/files.service.ts:162 has a non-exported MAX_BYTES; Multer limits.fileSize is hand-written across the person controllers + rooms as 10_485_760 or 10 * 1024 * 1024 (two spellings). Fix: export MAX_BYTES (alongside the already-exported MAX_FILES_PER_UPLOAD) and reference it everywhere.
  • [type-system] Storage-error log hardcodes bucket: 'storage'. files/files.service.ts:289 — a dead literal (FilesService holds no bucket name), so an operator inspecting a failed put gets nothing useful; storageKey already carries the useful prefix. Fix: drop the bucket: 'storage' field.

evaluation-scales

  • [verbosity] existsInUse/referenceCounts/computeInUseMap each re-walk SCALE_FK_REFERENCES with a separate hand-written delegate cast. evaluation-scales.service.ts:562,597,646 — three near-identical Promise.all(map(...)) loops, each with its own as unknown as {…} punch-through. Fix: one private probeReferences(ids, op) that owns the cast once (see the cross-module delegate-cast item).
  • [code-smell] totalReferences hand-lists the six FK keys instead of summing the typed counts. evaluation-scales.service.ts:617 — a seventh FK column would silently under-count. Fix: Object.values(counts).reduce((a,b)=>a+b, 0).
  • [stale-doc] Design spec references a stale pre-grid-remodel reference shape. docs/superpowers/specs/2026-05-18-evaluation-scales-design.md:403 — references: { curricula; studyPlans; planSubjects; blockSubjects } and cascade against removed StudyPlan/OptionBlockSubject columns; live contract is six grid-era columns. Fix: add a one-line "Superseded" banner near §5/§7.

custom-fields

  • [type-system] Controller handlers omit explicit return-type annotations. custom-fields.controller.ts:54,71,89,111,138 — create/findAll/findOne/update/remove infer from the service, with no compile-time link to the advertised CustomFieldResponseDto. Fix: annotate the handlers.
  • [duplications] DefinitionWithScope type duplicates the inline toResponse param type. custom-fields.service.ts:759 re-declares CustomFieldDefinition & { scope: { key } } inline instead of the existing named type (already imported). Fix: annotate toResponse(def: DefinitionWithScope).
  • [stale-doc] custom-fields.service JSDoc lists referents and room_types as custom-field entity tables. custom-fields.service.ts:53-54 — neither is in the real whitelist CUSTOM_FIELD_ENTITY_KEYS (which omits both and includes homerooms/subject_groups). Benign (assertValidEntityTable rejects any non-whitelisted key) but misleading. Fix: replace the example list with the actual members.

academic-years

  • [verbosity] create re-fetches via getByIdForTenant — a round-trip with a known-empty result. academic-years.service.ts:160 — a new DRAFT year always has empty related lists, so the re-fetch is guaranteed { departments: [], grades: [], curricula: [] }. Fix (create path only): build the detail DTO directly from the create result; leave the update-path re-fetch.

school

  • [missed-pattern] @AppliesPolicy(SchoolPolicy) is wired but SchoolPolicy.where is never ANDed in the service. school.controller.ts:41 + school.service.ts (update filters by { tenantId } only) — the policy's only live effect is @RequireRoles('admin'). Fix: for a tenant-singleton with an admin-only write, drop SchoolPolicy in favour of a plain @RequireRoles('admin').
  • [domain] Logo "≤ 5 MiB raw" cap is advertised but never enforced on decoded bytes. school.service.ts:22 — decodePngLogo checks the PNG signature + a min length but never buffer.length against a max; only the ~7M-char base64 string is bounded. Fix: soften the copy, or add one if (buffer.length > MAX_LOGO_BYTES) throw ….

subject-levels

  • [type-system] computeInUseMap casts the Prisma delegate through as unknown as { groupBy }. subject-levels.service.ts:292 — only two FK refs (both keyed levelId), so two direct typed groupBy calls would type-check natively. Fix: replace the cast loop with direct calls (keep LEVEL_FK_REFERENCES only for the delete-time counts if wanted).
  • [potential-bug] Name-uniqueness check is case-insensitive but the DB uniques are case-sensitive, and inserts aren't wrapped in mapP2002. subject-levels.service.ts:148,200 — a concurrent same-case insert surfaces as a raw P2002 → 500 instead of the typed SUBJECT_LEVEL_NAME_TAKEN 409. Narrow window, low-write catalog. Fix: wrap create (+ update) in mapP2002.

mailer

  • [code-smell] "Single-subscriber by design" comment contradicts the multi-handler fan-out. mailer/mailer.service.ts:39 — the field is handlers[], the code loops all handlers, and the port doc says "fan out to all." Fix: drop the comment (or collapse to one handler field if single-subscriber really is the invariant).
  • [type-system] Redundant optional chain on the non-optional ResendEvent.data. mailer/resend/resend-webhook.controller.ts:51 — event.data?.email_id where data is declared non-optional (types.ts) and the mapper accesses it with no chain. Fix: drop the ?. (body is verified-signed JSON parsed against the type).

infra (logger / health / config)

  • [bad-practice] Logger gates pretty-vs-JSON on NODE_ENV === 'production' only. logger/logger.module.ts:15 — diverges from the canonical isProductionLikeEnv (common/constants/environments.ts), so stage/development run pino-pretty (a log-aggregation foot-gun). Fix: gate on !isProductionLikeEnv(...) so only local/test get pretty output.
  • [type-system] Health handlers return untyped object literals; HealthResponseDto is unused as a type. health/health.controller.ts:49,56,66 — inline { status, timestamp } with no return type, while the DTO exists but is referenced only by Swagger. Fix: annotate the three handlers : HealthResponseDto.
  • [stale-doc] 429 documented only on the /health alias, not /health/live or /health/ready. health/health.swagger.ts:29 — the global ThrottlerGuard rate-limits all three, but only the alias declares ApiTooManyRequestsResponse (and the platform probes hit /health/ready). Fix: add the same response to ApiLivenessCheck + ApiReadinessCheck.

common (services / utils / framework)

  • [domain] getImportPreview count/items are tenant-wide, not academic-year-scoped. common/services/base-tenanted-crud.service.ts:867 — where = { tenantId } with no academicYearId, while findAll composes the year; once a tenant has a second year the import-summary headline count over-reports. Harmless during single-year setup. Fix (rollover): scope the preview to the active year; add a // TODO(rollover) marker now.
  • [potential-bug] import-field-rules maxLength measures untrimmed raw.length. common/utils/import-field-rules.ts:77 — inconsistent with the trimmed empty-check and trimmed persistence, so a value within max after trimming but over with whitespace is wrongly rejected. Fix: measure trimmed.length.
  • [stale-doc] Stale comment in omit.ts references a cast that does not exist. common/utils/omit.ts:11 — the comment describes "cast restores correct Omit shape" but line 12 is a bare return result;. Fix: delete the comment.
  • [type-system] ROLLOVER_INVALID_STUDENT_IDS typed in ErrorParamsMap as params, but thrown + documented under data. error-codes.ts:317 declares it as params; both throw sites (students.service.ts:1446,1484) pass it under data (typed unknown, so no compile check), and the example/docs put it under data. The type protects the field nobody fills. Fix: drop the ErrorParamsMap entry + type the throws as AppException<RolloverInvalidData>, or move the payload to params. Add a one-line error-chapter rule: a code is params-shaped or data-carrying, never params-declared-but-data-thrown.
  • [type-system] Empty-object {} param types defeat the ParamsFor guard for two timetable-template codes. error-codes.ts:493,527 — DAY_TEMPLATE_REQUIRES_PERIOD: {} and WEEK_TEMPLATE_REQUIRES_OPERATIVE_DAY: {}; in TS {} accepts any non-nullish value (the opposite of "no params"), whereas omission resolves to never. Both throw sites pass no params. Fix: delete both entries (behavior-preserving).
  • [potential-bug] ApiCreateEndpoint/ApiUpdateEndpoint hardcode "Duplicate email within tenant" conflict copy for all entities. common/decorators/api-crud-helpers.ts:123,190 — rendered verbatim into the public OpenAPI doc; departments/grades opt into conflict: true for name/ordinal conflicts. Fix: add an optional conflictDescription? defaulting to a generic message; per-entity overrides where it matters.

schema / migrations (doc-hygiene)

  • [stale-doc] schema.prisma cites "Prisma 5", a truncated migration name, and two different breadcrumbs for the active-year partial unique. schema.prisma:1295 (and :1258) — the project is on Prisma 7, the real folder is 20260525150000_active_year_unique_and_invitation_token_ttl, and a second JSDoc block cites 20260429000000_… for the same invariant with a different index name. Fix: correct the reference, drop "Prisma 5", and reconcile the two breadcrumbs to the migration whose index name the code relies on.
  • [stale-doc] diet_type constant-USING column-wipe pattern is undocumented in docs/12. The 20260622120000_align_teacher_staff_diet_type_enum migration uses ALTER COLUMN … TYPE … USING 'STANDARD' (hazard #4) — a sanctioned decision for disposable free-text values, but the precedent isn't captured, so the next such migration relitigates it. Fix: add a short "constant-USING column wipe" note to docs/12's worked-examples (the duplicate-slug / double-timestamp note is already present).

solver (solver/)

  • [duplications] Registered _UnauthorizedError handler is dead on the generate path. solver/app/main.py:107-111 — generate() wraps _verify_bearer in its own try/except rebuilding the identical 401 envelope inline, so the registered handler never fires. Fix: make _verify_bearer a Depends and delete the inline try/except (or delete the unused handler) — one source.
  • [potential-bug] Unguarded int(content-length) can yield a raw 500 that violates the error-shape contract. solver/app/main.py:115 — a non-numeric Content-Length raises ValueError before the body-parse try, surfacing as Starlette's bare 500 with no {"error": …} envelope. Fix: drop the header pre-check and rely on the existing post-read len(body) > _MAX_BODY_BYTES guard.
  • [type-system] minimizationIterations/minimizationBudgetExhausted are computed and typed but never forwarded. The solver populates them and generation.types.ts declares them, but generation.service.ts:165-167 builds solveMetadata with only wallClockMs + objectiveValue and SolveMetadataDto (dto/generation-result.dto.ts:21-31) exposes only those two — a contract field that implies data flows through when it doesn't. Fix: either drop the two fields or add them to the DTO + INFEASIBLE-branch data.
  • [bad-practice] Solver INTERNAL_ERROR response echoes the raw exception string. solver/app/main.py:145 — "message": str(exc) on the 500 path; the BE discards the body anyway and logger.exception already captures it server-side. Fix: return a generic message (the INVALID_INPUT str(exc) is user-actionable and can stay).

docs

  • [stale-doc] Signed-off / shipped specs still carry status: Draft frontmatter. ~15 specs match ^status: Draft, several with a filled §12 Sign-off ("Approved by: Fabio Barbieri") on a landed, REFERENCE-documented feature; the design-gate procedure defines the field but no step advances it, so a landed spec is indistinguishable from a pending one. Fix: either drop the frontmatter status: field (the filled §12 is the real signal) or add a "flip status: to Approved at sign-off" step to the design-gate procedure. (Genuinely-pending specs — e.g. the combined-classes and Wave-4 domain-event drafts — stay Draft.)

Method note. Architecture findings distilled from the 2026-06-25 → 06-29 → 07-02 architecture-audit lineage (10 parallel investigators: 8 hypotheses + 2 open scouts, each instructed to disconfirm as well as confirm). Quality findings distilled from the 2026-06-22 quality audit (per-module + cross-module deep-reads, adversarially verified) and re-verified against the working tree on 2026-07-03 — items fixed since (e.g. the ParametricContext cast, staff DELETE role allowlist, files let row implicit-any, docs/03 TTL text, the evaluation-scales preset-name unique index, the migration double-timestamp note) were dropped. Test-coverage and architecture-judgment axes are out of scope for this pass and tracked in their own files. file:line anchors were accurate at re-verification time — verify before quoting.