[efegokdemir]

Fixes #771.

Batch scans now recalculate risk from gap-fill findings using the shared scoring policy, so the score, recommendation, Markdown details and exit status agree. The failure threshold remains the existing score > 50 rule. In the pre-fix code, the reported high-risk case scored 0/100 and returned SAFE; the regression now covers the corrected verdict and the incomplete-scan CAUTION case.

• Focused regression: test_gap_fill_preserves_caution_for_incomplete_scan (3 passed in focused selection).
• Ruff check, Ruff format check and git diff --check passed.

[rng1995]

[SkillSpector Review]

@efegokdemir thank you for closing the gap where multilingual gap-fill findings appeared in the batch report but never counted toward its verdict.

Value and readiness

The fix is correct and valuable. On main, run_one builds entry["risk_assessment"] from the graph result and only then appends gap-fill issues. A non-English skill with three HIGH gap-fill findings therefore still reports 0/100 LOW SAFE, is left out of the Markdown HIGH/CRITICAL section, and exits 0. The PR recomputes the score with the report node's own _compute_risk_score, over active_findings + gap_findings with the graph's has_executable_scripts and component_metadata. Score, band, recommendation and the batch's score > 50 exit rule now agree with the issues list. One test gap on the fail-closed path must be closed before merge.

Material findings

1. Blocker: contrib/batch_scan/runner.py:801. The one piece of new decision logic, keeping CAUTION when report() already failed closed (an incomplete or degraded scan upgraded from SAFE to CAUTION), has no test. Both new tests start from SAFE or use an empty gap-fill result, so deleting this branch would still pass CI. An incomplete scan plus one LOW gap-fill finding would then be relabeled SAFE. Please add a case where the graph result has risk_recommendation="CAUTION" with a SAFE-band score (as report() produces for an incomplete scan), gap-fill returns one LOW finding, and the test asserts the recommendation stays CAUTION while the score includes the finding.

Optional:

• The PR description says --fail-on-findings exit status now reflects gap-fill findings. contrib.batch_scan has no such flag; its exit 1 comes from score > 50. The code is right, but please correct the description so release notes do not overstate it.
• _compute_risk_score is a private report-node helper. Contrib already imports private internals, but a one-line comment naming report() as the policy source would help keep the two in sync.

PIC tradeoffs

The gap-fill rule IDs (P5–P8, MP1–MP3, RA1–RA2) are core static rule IDs, so the shared per-rule diminishing-returns policy applies across static and gap-fill findings. LLM gap-fill findings with confidence ≥ 0.7 can now move a skill to DO NOT INSTALL and exit 1. I believe that is the intended outcome of #771, but it is a verdict-policy change for batch users.

Verification and gaps

• Read entry_from_result, run_one (merged with main, including #796), run_gap_fill, _compute_risk_score and the report node's fail-closed block. The graph state carries active_findings, component_metadata and has_executable_scripts, and report() only ever upgrades SAFE to CAUTION, which this PR mirrors.
• The first new test asserts 75 / HIGH / DO NOT INSTALL, the Markdown HIGH/CRITICAL details and exit 1. The second asserts that an empty gap-fill result leaves the core assessment unchanged. Both would catch a revert of the recompute.
• git merge-tree against main b07f181d is clean. #796 changed runner.py imports and patches, but not this block. CI is green. I did not run the tests (static review).

---

Decision: Changes Requested (reviewed head f259d3aeb8e8d6b0cf4f5b2129d4e0a7ab4f81f7)

[rng1995]

[SkillSpector Review]

Value and readiness

The previously missing fail-closed regression is now present. Batch risk recomputation includes nonempty gap-fill findings, preserves an existing CAUTION when recomputation would otherwise say SAFE, and leaves the original assessment alone for empty gap-fill results. Ready for final review of this behavior.

Material findings

No new or still-open code findings. The new test_gap_fill_preserves_caution_for_incomplete_scan asserts a positive LOW finding score while recommendation remains CAUTION. The PR description now accurately ties batch failure to score rather than a nonexistent flag.

PIC tradeoffs

Gap-fill findings can increase the batch installation recommendation and exit status; this matches the stated #771 outcome.

Verification and gaps

This was a static review of the complete diff, pinned-head source, added regression assertions, discussion/review history and hosted check results. No contributor code or tests were executed. Read runner.py recomputation and all three regressions, including Markdown score/details/exit status. The previous requested test is resolved. Hosted checks succeeded where run, Docker smoke skipped. The branch currently conflicts with main; resolve the conflict and obtain fresh CI before merging.

Reviewed head: dd705e619aae8326c24f4bc35203db96110dee7b.

[rng1995]

[SkillSpector Review]

Value and readiness

Ready on the updated head. The previous required incomplete-scan regression remains present, and GitHub now reports CLEAN/MERGEABLE after the earlier integration conflict.

Material findings

No new or still-open required changes found. Verified the original blocker: a nonempty LOW gap-fill result increases the score while an existing fail-closed CAUTION is preserved. The empty-result case leaves the original assessment unchanged.

PIC tradeoffs

Gap-fill findings now participate in the same diminishing-return and confidence policy as core findings. The batch failure threshold remains score > 50, accurately described in the PR; no separate scoring policy is introduced.

Verification and gaps

Read the complete current diff, #771, all prior marked reviews, inline finding and author reply, and pinned runner context. Reviewed all three score/completeness tests, Markdown issue-detail assertions and exit-status checks. Current hosted checks resolve the prior conflict-validation hold; Docker smoke is explicitly skipped. Hosted checks: changes SUCCESS, lint SUCCESS, test-unit SUCCESS, OpenCode TypeScript Tests SUCCESS, DCO Check SUCCESS, docker-smoke SKIPPED. This is a read-only static review; no contributor code, tests, imports, builds or scanner invocations were executed.

Reviewed head: 54d857c7c7113d322bfb224ac3ecd641e1251ff4.