perf(contact-lookup-details): fetch contact log types once, not once per log

[chriskehayias]

Closes .claude/TODO/n-plus-1-contact-log-types-lookup.md.

The problem

getContactLogTypes() was called inside the logs.map() callback in getContactLogsByContactId, so the same small Contact_Log_Types lookup table was refetched once per log that had a type set. A contact with 50 typed logs meant 50 identical fetches on every page load.

Performance only — the mapped output was always correct.

The fix

Fetch once, index into a Map. The map becomes synchronous, so Promise.all and the async callback go away.

The naive hoist would not have been behavior-neutral, which is the one non-obvious part of this change. The old code fetched the lookup table only when at least one log had a type — so a contact with no logs, or only untyped ones, made no request and could not fail on one. Hoisting unconditionally would newly throw there if the lookup fetch failed. The logs.some(...) guard preserves the old behavior exactly.

Why the suite missed it

The file was at 100% statements / 100% branches the whole time. The test mocked getContactLogTypes and never asserted a call count, so the loop was invisible. This is the §1 thesis of TestCoverage.md landing again: high coverage is not evidence of correctness.

New guards:

• toHaveBeenCalledTimes(1) against a five-log fixture spanning two type IDs plus a null, asserting both the count and the full mapped output
• not.toHaveBeenCalled() for the all-untyped and empty-logs paths — this is what pins the some guard
• an empty type name still maps to null, not ''

Verified by mutation: restoring the call inside the map fails the count assertion and nothing else. The other three tests pin behavior rather than efficiency, which is the correct split.

Also scoped one pre-existing mockResolvedValue to mockResolvedValueOnce — vi.clearAllMocks() clears calls but not implementations, so it was leaking a resolved value into later tests in the file.

Considered and skipped: service-level memoization

The TODO floated caching getContactLogTypes() in ContactLogService. Deliberately not done. After this change the remaining callers are one per page load and one per contact-logs.tsx mount; caching on a process-wide singleton would hide a newly added contact log type until restart, for a single-digit request saving. Rationale recorded in §5.6 so it isn't relitigated.

Verification

• 579 tests / 32 files pass; no coverage threshold failures (contact-lookup-details/actions.ts stays at 100%/100%)
• eslint clean, tsc --noEmit clean, npm run build succeeds
• No Ministry Platform data was read or written — every test mocks at a boundary above the network

TestCoverage.md §5.6 flipped to ✅ and the §1 summary table now lists only §5.7 as open. The TODO's stale §7.6 cross-reference went with the deleted file (§7 is the per-file appendix and has no subsections).

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!