test(api): cover functions the process-global mock constraint left untested - #23
Merged
Merged
Conversation
…tested With the suite on --isolate, route tests stubbing a module no longer replace it for other files, so these get real unit tests and the NOTE comments explaining the gaps go away: - engine: getCandidates and runFullMatchingPass — eligibility, active- contact exclusion, ranking, topN, rerank fallback and re-sort, and the per-applicant degradation (a missing embedding or a throwing score gives that applicant [] without aborting the pass, including across batches). A mutation removing the per-applicant catch fails two of these. - match-state: promoteAppliedToMatched (portal floor, dedup, applied-only). - admin.service: listApplicants (filter, paging, secrets stripped) and deactivateApplicant (soft-delete write, malformed id, no match). - ai.service: generateChatCompletion with fetch stubbed — request shape, and that every failure mode returns "" instead of throwing. - proposals: drop the stale import-path note.
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate coverage gaps and one tautological threshold assertion remain.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds isolated API unit-test coverage for matching, applicant administration, match promotion, and AI completion fallbacks.
Changes:
- Covers matching eligibility, ranking, degradation, and batching.
- Tests applicant listing/deactivation, promotion rules, and AI failure handling.
- Removes obsolete test-isolation notes.
File summaries
| File | Reviewed changes |
|---|---|
api/src/__tests__/unit/matching/engine.test.ts |
Adds candidate and full-pass matching tests. Moderate finding (3 votes): test doubles should enforce status predicates and cover inactive applicants. |
api/src/__tests__/unit/services/ai.service.test.ts |
Tests chat completion requests and fallbacks. Moderate finding (2 votes): add malformed-body or rejected-JSON coverage. |
api/src/__tests__/unit/services/admin.service.test.ts |
Tests applicant listing, secret omission, pagination, and deactivation. |
api/src/__tests__/unit/matching/proposals.test.ts |
Removes an obsolete mock-isolation note. |
api/src/__tests__/unit/services/match-state.service.test.ts |
Tests applicant promotion rules. Nit (1 vote): pin the documented 0.6 threshold rather than importing it from the implementation. |
Review details
Suppressed comments (1)
api/src/tests/unit/services/match-state.service.test.ts:715
- The expected threshold is imported from the same implementation under test, so this assertion is tautological: changing
PORTAL_MIN_SCOREand the query together would still pass. Pin the documented 0.6 floor (or assert the exported constant separately) to make this regression check meaningful.
expect(filter).toEqual({ status: "proposed", score: { $gte: PORTAL_MIN_SCORE } });
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…lformed 2xx chat bodies Addresses review: the applicants double ignored status.$in, so eligibility tests would pass even if dating/inactive applicants leaked into matching — now honored, with inactive/dating fixtures (dropping the engine's status filter fails 3 tests). Adds the res.json()-rejects path of generateChatCompletion's never-throws contract.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Unit tests for functions that were previously untestable because Bun's
mock.module()was process-global (route tests stubbed their modules for every file). Now that the API suite runs with--isolate(#10), each of those gaps is closed and its NOTE comment removed:engine.test.tsgetCandidates,runFullMatchingPass— eligibility, active-contact exclusion, ranking +topN, rerank fallback and re-sort, 10-candidate cap, and per-applicant degradation (missing embedding / throwing score → that applicant gets[], the pass continues, including across concurrency batches)match-state.service.test.tspromoteAppliedToMatched— portal-score floor, participant dedup, only promotes fromappliedadmin.service.test.tslistApplicants(filter, paging,magicToken/passwordHashnever returned),deactivateApplicant(soft-delete write, malformed id, no match)ai.service.test.tsgenerateChatCompletionwithfetchstubbed — request shape, and every failure mode returns""instead of throwingproposals.test.tsWhy
runFullMatchingPass's per-applicant degradation shipped in #10 without a unit test, specifically because of this constraint. The engine tests now pin it: a mutation that removes the per-applicantcatchfails two of them.getOrComputeEmbeddingsisn't covered here on purpose — #22 adds its tests alongside a fix to its fallback semantics.Testing
bun run typecheckcleanbun run test:api: 609 pass / 0 fail (was 572; +37)bun test --isolate --randomizeacross 14 seeds🤖 Generated with Claude Code