[yzxcj797]

Summary

detect_changes exempts lifecycle and construction methods from test-gap reporting, but the review surfaces still computed their own untested lists. This could report setUp, tearDown, __construct, and similar methods as coverage gaps even when change analysis correctly reported zero gaps.

This change centralizes the lifecycle predicate and applies it consistently to:

• minimal review-context test-gap counts;
• generated review guidance;
• risk scoring, so exempt lifecycle methods no longer retain the 0.30 untested penalty.

Fixes #865.

Testing

• pytest tests/test_changes.py tests/test_pr854_edges.py tests/test_pr865_review_guidance.py -q — 47 passed
• Added regressions for:
  • review guidance omitting exempt lifecycle methods while retaining real gaps;
  • lifecycle risk having no untested penalty.
• ruff check on all changed files — passes
• git diff --check — passes

A full Windows test run was also executed: 2,625 tests passed and 75 failed. The failures are in pre-existing Windows/path/encoding-sensitive areas unrelated to these files; the targeted change-related suites pass.

[github-actions]

code-review-graph review

Overall risk: 0.65 (MEDIUM) — 11 changed function(s)/class(es), 0 affected flow(s), 4 test gap(s)

Risk-scored changes

Risk	Level	Symbol	Location	Tested
0.65	medium	code_review_graph/tools/review.py::_generate_review_guidance	code_review_graph/tools/review.py:236	yes
0.50	medium	code_review_graph/tools/review.py::get_review_context	code_review_graph/tools/review.py:31	no
0.40	medium	code_review_graph/changes.py::is_test_gap_exempt	code_review_graph/changes.py:35	no
0.35	low	tests/test_pr865_review_guidance.py::_function	tests/test_pr865_review_guidance.py:8	yes
0.30	low	code_review_graph/changes.py::compute_risk_score	code_review_graph/changes.py:329	yes
0.30	low	code_review_graph/changes.py::analyze_changes	code_review_graph/changes.py:399	yes
0.25	low	tests/test_pr865_review_guidance.py::test_review_guidance_exempts_lifecycle_methods	tests/test_pr865_review_guidance.py:17	(test)
0.05	low	tests/test_changes.py::TestChanges	tests/test_changes.py:24	no
0.05	low	tests/test_changes.py::TestChanges.test_risk_score_lifecycle_method_has_no_untested_penalty	tests/test_changes.py:248	(test)
0.05	low	tests/test_pr854_edges.py::TestExemptListEdges	tests/test_pr854_edges.py:18	no

Test gaps

• code_review_graph/changes.py::is_test_gap_exempt (code_review_graph/changes.py:35)
• code_review_graph/tools/review.py::get_review_context (code_review_graph/tools/review.py:31)
• tests/test_changes.py::TestChanges (tests/test_changes.py:24)
• tests/test_pr854_edges.py::TestExemptListEdges (tests/test_pr854_edges.py:18)

Token savings: this graph-backed report used ~17,432 fewer tokens (~88%) than reading every changed file in full (estimated, chars/4 approximation).

---

Powered by code-review-graph — local-first analysis; no code leaves the CI runner.

[tirth8205]

Changes required: a name-only lifecycle exemption suppresses coverage risk for production code. Run python -m pytest tests/test_changes.py -k lifecycle_method_has_no_untested_penalty; the new test explicitly creates an untested setUp function in a.py and requires compute_risk_score to return zero, and the same policy also exempts production constructors. Restrict noise suppression to verified test lifecycle nodes and retain coverage risk for substantive production logic.

[tirth8205]

Checks pass and the code looks right. Missing: tests for the edge cases it introduces.

• A test for get_review_context(..., detail_level="minimal") asserting result["test_gaps"] excludes lifecycle methods (both mixed exempt+real -> 1 and all-exempt -> 0), using a seeded .code-review-graph/graph.db like tests/test_tools.py::_seed_repo_relative_graph.
• A test for _generate_review_guidance when all changed functions are exempt: no 'lack test coverage' line is emitted (falls through to 'Changes appear well-contained').
• An end-to-end get_review_context (standard detail_level) test asserting context["review_guidance"] omits lifecycle names and still names real gaps, with real GraphNode objects rather than SimpleNamespace.
• (Optional) A test pinning that a covered lifecycle method now scores 0.0 for the coverage term (previously 0.05), so the semantic change is deliberate and documented.

Every new branch in the code should have a test, including the failure branch. Disagree with any of these? Say so here first.

PRs now target staging, not main. Yours was retargeted already, so nothing to do there.

[tirth8205]

This no longer merges into staging. 2 files conflict and the branch is 257 commits behind:

• code_review_graph/changes.py
• code_review_graph/tools/review.py

Worth knowing before you resolve:

• code_review_graph/changes.py: staging moved _GIT_TIMEOUT into constants.py as GIT_TIMEOUT and added a separate discovery budget.

git fetch origin
git merge origin/staging
# resolve, then
uv run pytest tests/ -q
uv run ruff check code_review_graph/
uv run mypy code_review_graph/ --ignore-missing-imports --no-strict-optional
git push

I have not reviewed the change itself yet. That comes once it merges and the checks run against the merged state, since staging has moved a long way and the result is what matters.

[tirth8205]

Thank you for this PR. I checked the risk scores. I also ran the review tests with staging merged in.

Before I can merge it, please fix these:

1. compute_risk_score skips coverage risk by name only (code_review_graph/changes.py:371). So real code loses its coverage risk. An untested ProductionService.__init__ scores 0.0, not 0.3. A plain setUp in installer.py also scores 0.0. Please skip this risk only for real test lifecycle methods. For example, use nodes with is_test, or nodes in test files. Then update test_risk_score_lifecycle_method_has_no_untested_penalty.
2. test_review_guidance_exempts_lifecycle_methods in tests/test_pr865_review_guidance.py fails after the merge. Staging now needs file_path on each node. The message now says "have no direct test". Please add file_path and update the expected text.
3. The tests from my Sep 15 comment are still missing. Please add a get_review_context test with lifecycle methods. Please also add a test where every changed function is exempt.

Please update your branch with the latest staging. There are conflicts in code_review_graph/changes.py and code_review_graph/tools/review.py.

After you push the fix, I will test it again.

[Copilot]

🟡 Changes recommended

The regression does not cover the separate minimal-context path, leaving the test-gap counting inconsistency able to recur undetected.

1 open issue

• Add minimal-context test-gap regression coverage · New

What changed in this PR

This PR aligns review-context coverage reporting and risk scoring with the lifecycle-method exemptions already used by change analysis, addressing #865.

Changes:

• Centralizes the lifecycle test-gap predicate for reuse across analysis and review guidance.
• Excludes exempt methods from minimal-context gap counts, guidance, and the coverage-related risk penalty.
• Adds guidance and risk-score regressions and updates existing exemption expectations.

File	Description
tests/​test_pr865_review_guidance.py	Adds regression coverage for lifecycle exemptions in guidance.
tests/​test_pr854_edges.py	Updates lifecycle-only risk expectations.
tests/​test_changes.py	Tests removal of the lifecycle coverage-risk penalty.
code_review_graph/​tools/​review.py	Applies exemptions to minimal gap counts and guidance.
code_review_graph/​changes.py	Introduces the shared predicate and applies it to risk scoring and gap analysis.

_{Review effort: Balanced}