fix(training): defer completion email and share canonical training IDs

[tofikwest]

Follow-up to #3501, addressing both cubic review findings on apps/portal/src/app/api/portal/complete-training/route.ts.

1. The "best-effort" email was blocking the response

The route awaited sendCompletionEmailIfComplete before returning. That call hits POST /v1/training/send-{hipaa-,}completion-email, which resolves the member, renders a PDF certificate and calls the email provider — so every final-video completion made the employee wait seconds for work whose outcome they never see. Node's fetch also has no default request timeout, so a slow or unreachable API could stall the response for minutes even though the completion row was already committed.

• Deferred with after() from next/server, so it runs once the response has been sent.
• Bounded the request with AbortSignal.timeout(30s) so a hung API can't pin the invocation open.

2. Video IDs and HIPAA eligibility were duplicated

The route restated the valid-video-ID list and the HIPAA framework gate that TrainingService.markVideoComplete owns. That was the fifth copy — the same constants were already hardcoded in training.service.ts, frameworks-people-score.helper.ts, and the two apps' training-videos.ts / hipaa-training-content.ts.

Moved them to @trycompai/company, the existing shared-definitions package that api, app and portal all already consume (same pattern as evidence-forms):

• GENERAL_TRAINING_VIDEO_IDS, HIPAA_TRAINING_ID, isTrainingVideoId
• HIPAA_FRAMEWORK_NAME, HIPAA_TRAINING_UNAVAILABLE_MESSAGE, hipaaFrameworkInstanceWhere — so both completion paths reject ineligible orgs identically

The rendered video lists now satisfies the canonical ID union, so adding a video to the UI without adding it to the shared list is a compile error rather than a silent desync (verified: swapping sat-5 for sat-6 fails typecheck).

Also

Fixed training-hipaa.spec.ts, which was failing on main — its @db mock was never given frameworkInstance when the HIPAA gate was added. Added coverage for the gate's rejection path, mirroring the portal route's test.

Verification

• apps/portal — 14/14 vitest pass; tsc --noEmit clean (0 errors)
• apps/api — training + frameworks-people-score specs 20/20 pass; no new type errors (22 pre-existing, all in unrelated modules)
• apps/app — no new type errors (15 pre-existing, all in unrelated test files)
• Prettier clean on all changed files

---

Summary by cubic

Defer the completion email until after the response (with a 30s timeout) and centralize training IDs and HIPAA eligibility in @trycompai/company so the API and portal share the same rules. Key the app and portal video lists by the canonical IDs to make them exhaustive both ways and type-safe.

• Bug Fixes

  • Deferred the completion email with after() and bounded the request with AbortSignal.timeout(30_000). Tests confirm the response doesn’t wait and the request uses an AbortSignal.
  • Unified HIPAA eligibility and error copy via shared helpers (hipaaFrameworkInstanceWhere, HIPAA_TRAINING_UNAVAILABLE_MESSAGE).
  • Fixed training-hipaa.spec.ts by mocking frameworkInstance and adding the rejection case for ineligible orgs.

• Refactors

  • Centralized IDs and helpers in @trycompai/company (GENERAL_TRAINING_VIDEO_IDS, HIPAA_TRAINING_ID, isTrainingVideoId, HIPAA_FRAMEWORK_NAME, HIPAA_TRAINING_UNAVAILABLE_MESSAGE, hipaaFrameworkInstanceWhere), and updated the portal route, Nest TrainingService, and frameworks-people-score.helper.ts to use them.
  • Made the rendered video lists exhaustive both ways by keying them as Record<GeneralTrainingVideoId, ...>; the HIPAA ID stays a local literal tied by type-only link to the canonical union to avoid runtime deps while preventing drift.

^{Written for commit 5cb8566. Summary will update on new commits.}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
app	Ready	Preview, Comment	Jul 28, 2026 9:53pm
comp-framework-editor	Ready	Preview, Comment	Jul 28, 2026 9:53pm
portal	Ready	Preview, Comment	Jul 28, 2026 9:53pm

[cubic-dev-ai]

1 issue found across 11 files

Confidence score: 5/5

• apps/api/src/training/training.service.ts exceeding the 300-line limit raises maintainability risk by concentrating too many responsibilities, which can make future changes harder to review and increase the chance of regressions in this service—extract a focused unit (such as certificate/email resolution) to keep the import-driven addition isolated.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/api/src/training/training.service.ts">

<violation number="1" location="apps/api/src/training/training.service.ts:14">
P3: `TrainingService` now exceeds the API's 300-line file limit (310 lines). Extract a focused responsibility (for example certificate/email resolution) before adding this import-driven functionality.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P3: TrainingService now exceeds the API's 300-line file limit (310 lines). Extract a focused responsibility (for example certificate/email resolution) before adding this import-driven functionality.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/training/training.service.ts, line 14:

<comment>`TrainingService` now exceeds the API's 300-line file limit (310 lines). Extract a focused responsibility (for example certificate/email resolution) before adding this import-driven functionality.</comment>

<file context>
@@ -5,13 +5,16 @@ import {
+  HIPAA_TRAINING_UNAVAILABLE_MESSAGE,
+  hipaaFrameworkInstanceWhere,
+  isTrainingVideoId,
+} from '@trycompai/company';
 import { TrainingEmailService } from './training-email.service';
 import { TrainingCertificatePdfService } from './training-certificate-pdf.service';
</file context>

[tofikwest]

Addressed both findings.

P2 (exhaustiveness) — valid, fixed. You're right, and it was the dangerous direction: hasCompletedAllTraining requires a row for every canonical ID, so a canonical ID with no rendered video leaves employees permanently unable to finish training.

Note the suggested Exclude<GeneralTrainingVideoId, (typeof trainingVideos)[number]['id']> would not have caught it here — the readonly TrainingVideo[] annotation widens id to string, so the Exclude resolves to never and the guard passes vacuously.

Keyed the data by the canonical union instead — Record<GeneralTrainingVideoId, Omit<TrainingVideo, 'id'>> is exhaustive by construction. Verified by introducing each drift and compiling:

• canonical gains sat-6, UI doesn't render it → TS2741: Property '"sat-6"' is missing ... but required in type 'Record<...>'
• UI renders sat-6, canonical doesn't have it → TS2353: ''sat-6'' does not exist in type 'Record<"sat-1" | ... | "sat-5", ...>'

P3 (300-line limit) — not introduced by this PR. training.service.ts is 309 lines on main and 310 here; the violation predates the change by one line (git show origin/main:apps/api/src/training/training.service.ts | wc -l). Extracting certificate/email resolution means new DI wiring and test surface, which doesn't belong in a PR about deferring an email — better as its own change.

Verification: bun run --filter '@trycompai/portal' build and --filter '@trycompai/app' build both exit 0; portal vitest 14/14; api training + people-score specs 20/20; portal tsc 0 errors; app tsc at its pre-existing baseline of 15 (all in unrelated test files).

[claudfuen]

🎉 This PR is included in version 3.111.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀