On this page
Ships visibly brokenCorrectnessRobustnessConfiguration and deploymentSchema and API hygieneFront-endCoverageKnown Gaps & Rough Edges
Everything on this page was verified against the code in /work/repo on this branch, or observed on the branch sandbox. It is grouped by how much it matters, not by where it lives.
None of it is architectural. The design holds together; these are finishing items.
Ships visibly broken
1. Logs → Approvals has no page component
- Sidebar entry:
Sidebar.js:567-569, keydhsApprovals, route/logs/dhsApprovals. - Route:
DashboardRouter.js:810-815, lazy-loading
../components/dashboard/logs/dhsApprovals/DHSApprovals at :399.
- The problem: that directory does not exist. Verified by listing
packages/clinic-web/src/components/dashboard/logs/ — there is no dhsApprovals entry.
On the sandbox the route resolves to a stub reading "DHS Approvals (stub — component not built on this branch)", which is how that build compiles at all.
Impact: cosmetic but highly visible — it is a main-navigation item that any user with View DHS Approvals will find and click. The per-patient Chart → Approvals tab covers the actual need, and the detail route /logs/dhsApproval/:approvalId is implemented (the canary DHSApprovalDetail payload inspector).
Fix: build the page, or drop the sidebar entry and the list route until it exists.
Correctness
2. getInsuranceCompanyDhsInsuranceHierarchy throws on a missing id
packages/server/src/resolvers/queries/DHS/getInsuranceCompanyDhsInsuranceHierarchy.js
The tenancy comparison at :75-77 dereferences insuranceCompany.companyId before the null check at :79-85. A non-existent id therefore produces a TypeError rather than the intended "Insurance company not found", making that branch dead code. It also reads request.session.user.company.id without optional chaining, unlike every other resolver in the module.
Note the intended not-found response would violate the schema anyway — data is declared InsuranceCompany! (non-null) at types.graphql:5567-5571.
Fix: reorder the checks, and make data nullable.
3. dhsManualUpdateOperations picks the first approval only
dhsManualUpdateOperations.js:53:
const dhsApprovalId = operationRecords.find(op => op.dhsApprovalId)?.dhsApprovalId
If the submitted operations belong to different approvals, only the first one's status is updated. Confirm whether that is reachable through the UI; if so it needs either a guard or per-approval handling.
4. Vacuous truth in the manual-update status derivation
dhsManualUpdateOperations.js:69-80 uses .every to derive the new status. An input where every approved is undefined satisfies isAllApproved vacuously and sets the approval to APPROVED. The modal always sends a boolean, so this is not reachable from the UI — but it is reachable through the API.
5. Eligibility "active" is treated inconsistently
- Server (
dhsCheckEligibility.js:116): onlyeligibilityCode.code === 'eligible'stores
ELIGIBLE.
- Client (
DHSCheckEligibility.utils.ts:199-201): either'eligible'or'active',
read from either payerResponse.status or eligibilityCode.code.
A payer returning 'active' would show a green badge while the database records NON_ELIGIBLE. Pick one definition.
6. Re-polling overwrites a terminal status
dhsGetApproval.js:206 updates the approval status with no precondition. Re-polling a CANCELED approval overwrites it with whatever the payer currently reports. Consider guarding terminal states.
7. Duplicate service codes collapse
Both the cron (dhsApprovalsCron.js:251-256) and dhsGetApproval.js:167-169 match service lines on providerServiceCode === operation.code, first match wins. Two operations with the same procedure code both receive the first line's approved amount. Worth confirming whether NPHIES guarantees uniqueness here; if not, this misallocates money.
Robustness
8. No HTTP timeouts on the DHS API calls
Only two calls on the branch carry a timeout: the attachment download (dhsApprovalSubmission.js:206, 10 s) and the cron's poll (dhsApprovalsCron.js:179-191, 10 s).
These have none:
| Call | Site |
|---|---|
POST /api/Login | dhsAuthentication.js:41-47 (and four other resolvers) |
POST /api/v1/Eligibility/CheckInsurance | checkInsurance.js:66-76 |
POST /api/v2/Eligibility/Checkeligibility | dhsCheckEligibility.js:92-102 |
POST /api/v1/Preauth/SubmitApprovalRequest | dhsApprovalSubmission.js:243-253 |
POST /api/v1/Preauth/CancelApproval | dhsCancelApproval.js:95-105 |
A hung upstream holds the request until platform-level limits intervene. One line each.
9. One /api/Login per request-path operation
dhsAuthentication accepts redisClient in its context and never uses it (:14). Five resolvers — checkInsurance, dhsCheckEligibility, dhsApprovalSubmission, dhsGetApproval, dhsCancelApproval — each call it inline, so every user action costs an extra round-trip to DHS.
The cron does cache, at dhs:token:${companyId} with a 3300 s TTL (dhsApprovalsCron.js:16,39). The PR description's claim that "auth tokens [are] cached per company in Redis" is accurate for the cron and not for the request path.
10. Submission rollback leaves the payer and Dentolize out of sync
dhsApprovalSubmission.js:305-312 — if the operation-count assertion fails, the transaction rolls back and the user sees success:false. But DHS has already accepted the submission. The approval exists upstream with no local record, and the user will likely resubmit.
This is the highest-consequence item on the page. It needs an operational runbook at minimum, and ideally a reconciliation query before resubmission is allowed.
11. Eligibility writes are not transactional
dhsCheckEligibility.js:129-158 creates the DHSEligibilityCheck and updates the patient in two separate statements. A failure between them leaves an orphan check row and reports success:false for an operation that actually succeeded upstream.
12. The cron lock is released without a value check
dhsApprovalsCron.js:343-346 releases with an unconditional DEL. A tick that overran the 600 s TTL would delete a successor's lock. Unlikely given the 100-row cap and pLimit(5), but the fix (a value-compare or Lua CAS) is small.
13. The DHS cron is not wrapped in wrapCronJob
Every other job in cronJobs.js is wrapped (:22,33,44,55,66,77,88,99,110); DHS passes a bare arrow function (:121-123). It therefore gets no jobContext observability wrapper.
14. Silent skips in the cron
dhsApprovalsCron.js:165-168 — an approval whose branch or insurer lacks an nphiesCode is skipped with no log and no error; only the failedCount moves. And when there are zero pending approvals, no summary log is emitted at all (:339). Both make operational debugging harder than it needs to be.
15. A cache-reset failure reports a false negative
updateBranchesNphiesCodes.js:59 calls branchCache.resetDetails after the transaction commits. If that throws, the mutation reports failure even though the branch codes were saved.
Configuration and deployment
16. S3_BUCKET_AWS / S3_REGION_AWS are undocumented
validateAttachmentUrl.js:9-11 throws at import time without them, which means dhsApprovalSubmission cannot load and the server does not boot. Neither variable is in packages/server/.env.example, which documents only the three DHS_*_URLs and ENCRYPTION_MASTER_KEY.
17. Encryption key rotation is unsupported
encryption.js:10 uses ENCRYPTION_MASTER_KEY directly — no KDF, no key-version field, no re-encryption job. Rotating it invalidates every DHSIntegration row, and the symptom presents as getDHSClientSecret returning success:false (i.e. looking like "no secret configured") rather than an obvious decryption error.
Documented in the source (:4-5) and in .env.example:47-51, but there is no procedure.
18. The feature-flag seed description contradicts the seed
seedFeatureFlags.js:39-45 describes the flag as "Off by default" while seeding it active: true with positiveRules: "Rule('isBeta', 'EQUALS', true)". The gate is the isBeta rule, not the active bit. An operator who flips active expecting a master switch will be surprised.
Schema and API hygiene
19. input CoverageInput is declared twice
inputs.graphql:950 and :957 — identical definitions. A strict schema validator would reject this.
20. Orphaned permission enum values
MANAGE_DHS_INTEGRATION, MANAGE_DHS_APPROVALS, CHECK_INSURANCE remain in the Postgres Permission type from migration 20260615152553_dhs_permissions, but are absent from schema.prisma and enums.graphql. Postgres cannot drop enum values, so they are permanent. The PR description still lists these as the permissions in use — it should be corrected.
21. updatePatientDetails silently drops three of its own input fields
UpdatePatientDetailsInput declares policyNumber, coverageType and nphiesCode (inputs.graphql:911-924), but none is on the lodash/pick allow-list (updatePatientDetails.js:21-37). They are accepted and discarded.
Conversely, six allow-listed keys (nationality, phoneNumber, email, firstNameE, lastNameE, marital) are not on the input type and are unreachable through the schema.
22. Naming and type inconsistencies
| Item | Note |
|---|---|
updatePatientDetailsResponse | Lower-case leading character, unlike every other type (types.graphql:5573) |
DHSCancelApprovalResponse.isCanceled | Outlives the dropped DB column; now derived from status |
DHSEligibilityCheck.responsePayload | Non-nullable Json in Prisma, nullable JSON in GraphQL |
| Two migration directories | Both named add_insurance_company_to_eligibility_check (20260609102732, 20260614090102) |
updateBranchesNphiesCodes | Stray space after ( in the SDL (schema.graphql:1383) |
23. Stale keys in failure responses
Several guard and catch paths return keys that do not exist on their response type and are silently dropped at serialization:
coverages: null—dhsCheckEligibility.js:25,44,54,180;dhsApprovalSubmission.js:29,57,81,92data: null—dhsCancelApproval.js:25,34,53,62,73,84
Harmless, but it means those failure paths return only success and message.
24. Two error conventions coexist
Most resolvers swallow everything into { success, message }. Four throw real GraphQL errors instead: dhsApprovalDetails, totalDHSApprovals, resolveInsuranceCoverages, getInsuranceCompanyDhsInsuranceHierarchy. Clients must handle both.
Front-end
25. The DHS permission tab is not feature-flagged
Group.js:251 renders the DHS Settings tab unconditionally, unlike the sidebar entry and the integrations tab. Every tenant sees seven DHS permissions whether or not the feature is enabled for them.
26. Wizard validation failures are console-only
DHSApprovalSubmissionModal.tsx:73-80 — Next validates the entire form and logs failures to the console with no toast. An invalid field on an earlier step makes the button appear dead with no visible explanation. Same pattern in ManualUpdateModal.tsx:41-56 and the Check Insurance form handlers.
This is the most likely source of "the button doesn't work" tickets.
27. Approvals-tab refetch ignores the user's view
Approvals.js:53-72 refetches with hardcoded skip: 0, take: 15, orderBy: 'createdAt-desc'. After polling from page 2 or under a filter, the list will not match what the user was looking at.
28. Empty and dead files
| File | State |
|---|---|
DHSApprovalSubmission/DHSApprovalSubmission.queries.tsx | 0 bytes |
DHSApprovalSubmission/DHSApprovalSubmission.constants.tsx | 0 bytes |
DHSCheckEligibility/DHSCheckEligibility.queries.tsx | 0 bytes |
DHS_AUTHENTICATION mutation | Instantiated at useDHSIntegrationSettings.ts:45, never called |
SYSTEM_TYPE_MAP | DHSCheckInsurance.utils.ts:5-17 — exported, superseded by the shared mapping |
29. Duplicated coding tables
The gender / marital / occupation maps are duplicated verbatim between DHSCheckEligibility.utils.ts:94-123 and DHSApprovalSubmission.utils.ts:167-198. They will drift.
30. Inconsistent translation-key prefixing
DHSCheckInsurance frequently double-prefixes — t('dhsCheckInsurance.button_next') from inside the dhsCheckInsurance namespace (CheckInsuranceModal.tsx:708,719,734-738) — while other keys in the same file are unprefixed (:715). Both forms exist in the bundle, so both resolve, but it is fragile.
31. Unreachable conditional fields
Several option lists are deliberately trimmed to one choice with the rest commented out "until their flows are implemented" (DHSApprovalSubmission.utils.ts:4-25,58-67). That makes three conditional branches unreachable through the UI:
relatedClaimPreauthRequestNumber(needstransactionTypeId === 10)- Triage fields (need
encounterClassId === 2) intendedLengthOfStayId(needsclaimPreauthRequestTypeId === 1)
Also, InsurancePolicyStep.tsx renders three Selects — coverageType, relationship, defaultPriceList — with literally empty option lists (:117-145).
32. DHSStatusPopover is used once; the chart duplicates it
The component exists at common/DHSStatusPopover.js and is used only by Approvals.js. The equivalent popover in ChartTable.js:319-450 is hand-inlined rather than reusing it.
33. moment in a dayjs codebase
DHSApprovalSubmission/steps/ReviewStep.tsx:2 imports moment; the rest of the codebase uses dayjs.
Coverage
Unit tests cover the three pure utilities (dhsStatusMapping, dhsErrorUtils, dhsFieldGuards) thoroughly. There is no automated coverage of any resolver, the cron, or any front-end wizard.
Given that the cron writes financial amounts onto operations without human review, that is the most valuable gap to close. The manual matrix in For Quality is the interim mitigation.
One specific hole: 'queued' → PENDING is mapped at dhsStatusMapping.js:17 but is not among the 18 test cases.