fix(llm): stabilize consensus finding severity - #152
Conversation
|
Warning Review limit reached
Next review available in: 3 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesLLM consensus behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ConfiguredLLMRuns
participant ConsensusAggregation
participant FindingsOutput
ConfiguredLLMRuns->>ConsensusAggregation: provide findings and run results
ConsensusAggregation->>ConsensusAggregation: deduplicate findings and rank severities
ConsensusAggregation->>FindingsOutput: emit majority findings and consensus metadata
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
skill_scanner/core/analyzers/llm_analyzer.py (1)
542-607: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsensus aggregation logic is correct.
The dedup-per-run, vote-tallying, and majority-filter logic correctly implements order-independent, highest-observed severity selection. This matches the tests in
tests/test_llm_analyzer.py:559-627, including run-order permutations, failed-run denominator tracking, and below-threshold rejection.Consider extracting the vote-tallying loop (lines 547-572) into a helper method. The line-range-change-details flags this block as high complexity, and splitting it improves readability without changing behavior.
♻️ Proposed refactor to extract vote tallying
+ `@staticmethod` + def _tally_consensus_votes( + all_run_findings: list[list[Finding]], + ) -> tuple[ + dict[tuple[str, str, str], int], + dict[tuple[str, str, str], Finding], + dict[tuple[str, str, str], dict[Severity, int]], + ]: + """Count one vote per run per finding key, keeping the highest severity seen.""" + finding_counts: dict[tuple[str, str, str], int] = {} + finding_map: dict[tuple[str, str, str], Finding] = {} + severity_votes: dict[tuple[str, str, str], dict[Severity, int]] = {} + + for run_findings in all_run_findings: + findings_by_key: dict[tuple[str, str, str], Finding] = {} + for f in run_findings: + key = (f.rule_id, f.category.value, f.file_path or "") + previous = findings_by_key.get(key) + if ( + previous is None + or _CONSENSUS_SEVERITY_RANK[f.severity] > _CONSENSUS_SEVERITY_RANK[previous.severity] + ): + findings_by_key[key] = f + + for key, finding in findings_by_key.items(): + finding_counts[key] = finding_counts.get(key, 0) + 1 + votes_for_key = severity_votes.setdefault(key, {}) + votes_for_key[finding.severity] = votes_for_key.get(finding.severity, 0) + 1 + + current = finding_map.get(key) + if ( + current is None + or _CONSENSUS_SEVERITY_RANK[finding.severity] > _CONSENSUS_SEVERITY_RANK[current.severity] + ): + finding_map[key] = finding + + return finding_counts, finding_map, severity_votes + async def _consensus_analyze(self, messages: list[dict], skill: Skill) -> list[Finding]: ... - # Count one vote per run for each unique finding. If a single response - # duplicates a key at different severities, that run casts its highest - # severity only. - finding_counts: dict[tuple[str, str, str], int] = {} - finding_map: dict[tuple[str, str, str], Finding] = {} - severity_votes: dict[tuple[str, str, str], dict[Severity, int]] = {} - - for run_findings in all_run_findings: - findings_by_key: dict[tuple[str, str, str], Finding] = {} - for f in run_findings: - key = (f.rule_id, f.category.value, f.file_path or "") - previous = findings_by_key.get(key) - if ( - previous is None - or _CONSENSUS_SEVERITY_RANK[f.severity] > _CONSENSUS_SEVERITY_RANK[previous.severity] - ): - findings_by_key[key] = f - - for key, finding in findings_by_key.items(): - finding_counts[key] = finding_counts.get(key, 0) + 1 - votes_for_key = severity_votes.setdefault(key, {}) - votes_for_key[finding.severity] = votes_for_key.get(finding.severity, 0) + 1 - - current = finding_map.get(key) - if ( - current is None - or _CONSENSUS_SEVERITY_RANK[finding.severity] > _CONSENSUS_SEVERITY_RANK[current.severity] - ): - finding_map[key] = finding + # Count one vote per run for each unique finding. If a single response + # duplicates a key at different severities, that run casts its highest + # severity only. + finding_counts, finding_map, severity_votes = self._tally_consensus_votes(all_run_findings)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skill_scanner/core/analyzers/llm_analyzer.py` around lines 542 - 607, Extract the per-run deduplication and vote-tallying block in the consensus aggregation flow into a focused helper method, preserving the existing highest-severity-per-key selection and updates to finding_counts, finding_map, and severity_votes. Update the surrounding method to invoke the helper without changing majority filtering, ordering, metadata, or failure-denominator behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/analyzers/llm-analyzer.md`:
- Around line 218-221: Restrict the documentation claims to deterministic
severity selection rather than overall aggregation stability. Update
docs/architecture/analyzers/llm-analyzer.md lines 218-221 and README.md lines
196-203 to use matching severity-scoped wording, while preserving the
clarification that complete equal-severity findings may still vary by response
order.
---
Nitpick comments:
In `@skill_scanner/core/analyzers/llm_analyzer.py`:
- Around line 542-607: Extract the per-run deduplication and vote-tallying block
in the consensus aggregation flow into a focused helper method, preserving the
existing highest-severity-per-key selection and updates to finding_counts,
finding_map, and severity_votes. Update the surrounding method to invoke the
helper without changing majority filtering, ordering, metadata, or
failure-denominator behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 197fac20-dd5c-44dc-95fa-f7d7d8866de2
📒 Files selected for processing (5)
README.mddocs/architecture/analyzers/llm-analyzer.mddocs/features/index.mdskill_scanner/core/analyzers/llm_analyzer.pytests/test_llm_analyzer.py
1 participant
Summary
Validation
uv run ruff check .uv run pytest -q tests/test_llm_analyzer.py(94 passed)uv run pytest -q(1,597 passed, 5 skipped, 1 xfailed)Closes #125
Summary by CodeRabbit
New Features
Documentation