fix(meta): batch findings and handle truncated responses - #153
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMeta-analysis now processes findings in token-budgeted batches. It preserves global indices, handles truncated and malformed responses, merges results deterministically, and reports degraded findings through metadata and ChangesMeta-analysis batching and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MetaAnalyzer
participant LLMProvider
participant ResponseParser
MetaAnalyzer->>LLMProvider: Send indexed finding batch
LLMProvider-->>MetaAnalyzer: Return response, finish reason, and token usage
MetaAnalyzer->>ResponseParser: Validate JSON and classification indices
ResponseParser-->>MetaAnalyzer: Return classifications or parse error
MetaAnalyzer->>MetaAnalyzer: Split truncated batches or create degraded results
MetaAnalyzer->>MetaAnalyzer: Normalize and merge all batch results
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ce7a2323c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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 (3)
tests/test_meta_analyzer_batching.py (1)
190-213: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the two remaining degradation codes.
The suite covers
META_BATCH_PARSE_FAILEDandMETA_BATCH_INCOMPLETE. It does not coverMETA_BATCH_REQUEST_FAILED, which occurs when_make_llm_requestraises, orMETA_BATCH_TRUNCATED, which occurs when a single-finding batch is truncated. Both paths must retain each finding exactly once and mark it degraded.I can write both tests if you want.
🤖 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 `@tests/test_meta_analyzer_batching.py` around lines 190 - 213, Add tests alongside test_incomplete_batch_is_filled_once_without_duplicate_indices covering META_BATCH_REQUEST_FAILED when _make_llm_request raises and META_BATCH_TRUNCATED for a truncated single-finding batch. Assert each path retains every finding exactly once, marks retained findings with meta_analysis_degraded=True, and emits the corresponding degradation code.skill_scanner/core/analyzers/meta_analyzer.py (2)
607-613: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winStop calling the provider after a request failure, and narrow the caught exception.
Line 607 converts every exception into a degraded batch. If the failure is permanent, for example an invalid API key or an unknown model, each remaining batch still runs
self.max_retriesrequests. A 257-finding scan then issues many failing calls and reports the configuration error only asMETA_BATCH_REQUEST_FAILEDwarnings. Ruff also flags the blindexcept Exception(BLE001).Short-circuit the remaining batches after the first request failure, and degrade them locally.
♻️ Proposed short-circuit in `analyze_with_findings`
batch_size = self._max_findings_per_batch() result = MetaAnalysisResult() + request_failed = False for batch_number, start in enumerate(range(0, len(findings), batch_size), start=1): indices = list(range(start, min(start + batch_size, len(findings)))) + if request_failed: + self._merge_batch_result( + result, + self._degraded_batch_result( + findings, + indices, + code="META_BATCH_REQUEST_FAILED", + message=( + "Skipped after an earlier meta-analysis request failure; " + "this batch was retained unchanged." + ), + ), + ) + continue logger.info(batch_result = await self._analyze_batch( skill=skill, findings=findings, indices=indices, skill_context=skill_context, analyzers_used=analyzers_used, start_tag=start_tag, end_tag=end_tag, ) + request_failed = any( + warning["code"] == "META_BATCH_REQUEST_FAILED" for warning in batch_result.analysis_warnings + ) self._merge_batch_result(result, batch_result)🤖 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/meta_analyzer.py` around lines 607 - 613, Update analyze_with_findings to stop invoking the provider after the first batch request failure, locally degrading all remaining batches with the existing failure result behavior. Narrow the exception handling around the provider request to the specific request/provider exception types rather than catching Exception, while preserving the initial batch’s META_BATCH_REQUEST_FAILED reporting.Source: Linters/SAST tools
1096-1103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove or call the fallback path from another caller.
The only
_parse_response(...)call passesfallback_on_error=False, so the defaultfallback_on_error=Truebranch is unreachable and duplicates_degraded_batch_result.🤖 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/meta_analyzer.py` around lines 1096 - 1103, Update _parse_response and its callers so the unreachable fallback_on_error=True path is removed, or ensure another caller explicitly uses it if that behavior is required. Since the existing call passes fallback_on_error=False and duplicates _degraded_batch_result, eliminate the unused default parameter and fallback branch while preserving current response parsing and degraded-result 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 `@skill_scanner/core/analyzers/meta_analyzer.py`:
- Around line 1050-1063: Update the finish-reason handling in the
response-processing method around choice and finish_reason to recognize every
provider-specific truncation value used by the configured routes, including
normalized length/max_tokens and retained raw max-token indicators. Keep raising
MetaAnalysisTruncatedError for all such values so the batch orchestrator takes
its bisect path, and document the mapping near the check.
---
Nitpick comments:
In `@skill_scanner/core/analyzers/meta_analyzer.py`:
- Around line 607-613: Update analyze_with_findings to stop invoking the
provider after the first batch request failure, locally degrading all remaining
batches with the existing failure result behavior. Narrow the exception handling
around the provider request to the specific request/provider exception types
rather than catching Exception, while preserving the initial batch’s
META_BATCH_REQUEST_FAILED reporting.
- Around line 1096-1103: Update _parse_response and its callers so the
unreachable fallback_on_error=True path is removed, or ensure another caller
explicitly uses it if that behavior is required. Since the existing call passes
fallback_on_error=False and duplicates _degraded_batch_result, eliminate the
unused default parameter and fallback branch while preserving current response
parsing and degraded-result behavior.
In `@tests/test_meta_analyzer_batching.py`:
- Around line 190-213: Add tests alongside
test_incomplete_batch_is_filled_once_without_duplicate_indices covering
META_BATCH_REQUEST_FAILED when _make_llm_request raises and META_BATCH_TRUNCATED
for a truncated single-finding batch. Assert each path retains every finding
exactly once, marks retained findings with meta_analysis_degraded=True, and
emits the corresponding degradation code.
🪄 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: b25d73db-1ee4-4c4a-88c4-7c2e350692d5
📒 Files selected for processing (2)
skill_scanner/core/analyzers/meta_analyzer.pytests/test_meta_analyzer_batching.py
2ef3430 to
e55989f
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
analysis_warnings,meta_analysis_status, and per-finding metadataaiohttpto 3.14.3 afterCVE-2026-59881made the security workflow failFor inputs larger than one batch, correlations and priority rankings are necessarily batch-local and are merged deterministically in batch order. Inputs that fit one batch keep the existing single-call behavior.
Validation
uv run pip-audit(no known vulnerabilities)Closes #137
Summary by CodeRabbit
New Features
Bug Fixes