⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.
Context
buildReviewRiskExplanation in src/signals/review-risk.ts (lines ~29-56) is a pure function
implementing a 5-way recommendation decision: likely_duplicate → maintainer_lane →
needs_author → review → watch. It is advisory only — it does not gate, dispatch, or affect
merge/close decisions, it only produces an explanation string surfaced to callers.
Despite the real branching logic, it has no direct unit test exercising each branch:
test/unit/routes-review-risk.test.ts calls buildReviewRiskExplanation only to compute its own
"expected" value, then asserts the REST/MCP routes match that self-same computed value — a
parity check between two callers, not a correctness check of the function's branch logic itself.
test/unit/support/mcp-cli-harness.ts only exercises a fixture-mocked HTTP server, never the real
function. No other test file in test/unit/ imports or calls buildReviewRiskExplanation
directly.
A clean precedent for the kind of direct, small unit-test file this needs already exists in the
same directory family: test/unit/contributor-readiness-band.test.ts directly unit-tests a
similarly-sized pure function from src/signals/.
Requirements
- Add direct unit test coverage for
buildReviewRiskExplanation that exercises each of the 5
possible recommendation outcomes (likely_duplicate, maintainer_lane, needs_author,
review, watch) by constructing the minimal PreflightResult/RoleContext-shaped input
needed to deterministically produce each branch — not by mocking or stubbing the function
itself.
- Do not modify
buildReviewRiskExplanation's implementation — this issue is coverage-only. If
writing the tests reveals an actual bug in the branch logic, note it in the PR description but
still land the coverage; do not silently "fix" behavior as a side effect without it being an
explicit, separately-called-out change in the PR body.
- Tests must live in a new or existing
test/unit/ file that imports the real function from
src/signals/review-risk.ts directly (mirror test/unit/contributor-readiness-band.test.ts's
structure) — not through the REST/MCP route layer.
Deliverables
Both deliverables are required in this single PR.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted, on every changed line/branch in
src/**. Since review-risk.ts itself is not being modified, this issue's own patch-coverage
surface is the new test file — it must actually execute (not stub) all 5 branches of
buildReviewRiskExplanation.
Expected Outcome
buildReviewRiskExplanation's 5-way branch logic has direct, real unit-test coverage independent
of the existing route-parity checks, so a future regression in the branch logic itself (not just a
route/function mismatch) would be caught.
Links & Resources
src/signals/review-risk.ts (function under test, ~lines 29-56)
test/unit/contributor-readiness-band.test.ts (structural precedent to mirror)
test/unit/routes-review-risk.test.ts (existing parity-only test — do not duplicate its
approach, this issue needs direct correctness coverage)
Context
buildReviewRiskExplanationinsrc/signals/review-risk.ts(lines ~29-56) is a pure functionimplementing a 5-way
recommendationdecision:likely_duplicate→maintainer_lane→needs_author→review→watch. It is advisory only — it does not gate, dispatch, or affectmerge/close decisions, it only produces an explanation string surfaced to callers.
Despite the real branching logic, it has no direct unit test exercising each branch:
test/unit/routes-review-risk.test.tscallsbuildReviewRiskExplanationonly to compute its own"expected" value, then asserts the REST/MCP routes match that self-same computed value — a
parity check between two callers, not a correctness check of the function's branch logic itself.
test/unit/support/mcp-cli-harness.tsonly exercises a fixture-mocked HTTP server, never the realfunction. No other test file in
test/unit/imports or callsbuildReviewRiskExplanationdirectly.
A clean precedent for the kind of direct, small unit-test file this needs already exists in the
same directory family:
test/unit/contributor-readiness-band.test.tsdirectly unit-tests asimilarly-sized pure function from
src/signals/.Requirements
buildReviewRiskExplanationthat exercises each of the 5possible
recommendationoutcomes (likely_duplicate,maintainer_lane,needs_author,review,watch) by constructing the minimalPreflightResult/RoleContext-shaped inputneeded to deterministically produce each branch — not by mocking or stubbing the function
itself.
buildReviewRiskExplanation's implementation — this issue is coverage-only. Ifwriting the tests reveals an actual bug in the branch logic, note it in the PR description but
still land the coverage; do not silently "fix" behavior as a side effect without it being an
explicit, separately-called-out change in the PR body.
test/unit/file that imports the real function fromsrc/signals/review-risk.tsdirectly (mirrortest/unit/contributor-readiness-band.test.ts'sstructure) — not through the REST/MCP route layer.
Deliverables
test/unit/review-risk.test.ts, or an addition to an existingsrc/signals-scoped test file if one already directly tests sibling functions inreview-risk.ts) with one test case perrecommendationbranch (5 total), each assertingboth the
recommendationvalue and the explanation text/shape returned for that branch.routes-review-risk.test.ts's fixtures wholesale, so the test is a real correctness checkindependent of that file's parity assertions.
Both deliverables are required in this single PR.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted, on every changed line/branch in
src/**. Sincereview-risk.tsitself is not being modified, this issue's own patch-coveragesurface is the new test file — it must actually execute (not stub) all 5 branches of
buildReviewRiskExplanation.Expected Outcome
buildReviewRiskExplanation's 5-way branch logic has direct, real unit-test coverage independentof the existing route-parity checks, so a future regression in the branch logic itself (not just a
route/function mismatch) would be caught.
Links & Resources
src/signals/review-risk.ts(function under test, ~lines 29-56)test/unit/contributor-readiness-band.test.ts(structural precedent to mirror)test/unit/routes-review-risk.test.ts(existing parity-only test — do not duplicate itsapproach, this issue needs direct correctness coverage)