do not throw when there is no contingency#242
Open
EtienneLt wants to merge 1 commit into
Open
Conversation
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
📝 WalkthroughWalkthroughThe PR updates ChangesNo-contingency handling in security analysis
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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: 2
🤖 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
`@src/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.java`:
- Around line 110-113: The early-return for all-null contingencies sets the
result status to NO_CALCULATION but downstream saveResult still maps any
non-CONVERGED loadflow status to SecurityAnalysisStatus.DIVERGED, making
NO_CALCULATION indistinguishable; update the code path in the saveResult logic
(or the method that maps LoadFlowResult/status to SecurityAnalysisStatus) to
explicitly check for the NO_CALCULATION sentinel (from
SecurityAnalysisWorkerService/runContext result creation) and map it to a
distinct SecurityAnalysisStatus (e.g., NO_CALCULATION) instead of DIVERGED,
ensuring functions like saveResult, mapLoadflowStatusToSecurityStatus, or
wherever SecurityAnalysisStatus is derived are updated to handle NO_CALCULATION
specially.
- Around line 185-188: The catch for IllegalArgumentException in
SecurityAnalysisWorkerService should not assume every exception means "no
contingency"; add a helper isNoContingencyException(IllegalArgumentException)
that checks the exception message for a contagion of "contingenc"
(case-insensitive), and change the catch to: if isNoContingencyException(e) then
call logNoContingencies(runContext), initialize runContext.contingencies to an
empty list (e.g., Collections.emptyList()) and return; otherwise rethrow or
propagate the exception so real validation/runtime errors aren't swallowed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 376ef0bc-b82a-4990-8083-9d7ead8ca369
📒 Files selected for processing (2)
src/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.javasrc/main/resources/org/gridsuite/securityanalysis/server/reports.properties
Comment on lines
+110
to
+113
| if (runContext.getContingencies().stream().allMatch(contingencyInfos -> contingencyInfos.getContingency() == null)) { | ||
| return CompletableFuture.completedFuture( | ||
| new SecurityAnalysisResult(new LimitViolationsResult(Collections.emptyList()), NO_CALCULATION, Collections.emptyList())); | ||
| } |
There was a problem hiding this comment.
NO_CALCULATION is introduced but may be persisted as DIVERGED.
With the new early return at Line 110, saveResult still maps any non-CONVERGED load-flow status to SecurityAnalysisStatus.DIVERGED (Line 228 onward). This makes no-contingency runs indistinguishable from real divergence.
💡 Proposed fix
@@
protected void saveResult(Network network, AbstractResultContext<SecurityAnalysisRunContext> resultContext, SecurityAnalysisResult result) {
+ LoadFlowResult.ComponentResult.Status lfStatus = result.getPreContingencyResult().getStatus();
+ SecurityAnalysisStatus saStatus =
+ lfStatus == LoadFlowResult.ComponentResult.Status.CONVERGED
+ ? SecurityAnalysisStatus.CONVERGED
+ : (lfStatus == NO_CALCULATION
+ ? SecurityAnalysisStatus.NO_CALCULATION
+ : SecurityAnalysisStatus.DIVERGED);
resultService.insert(network,
resultContext.getResultUuid(),
result,
- result.getPreContingencyResult().getStatus() == LoadFlowResult.ComponentResult.Status.CONVERGED
- ? SecurityAnalysisStatus.CONVERGED
- : SecurityAnalysisStatus.DIVERGED);
+ saStatus);
}🤖 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
`@src/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.java`
around lines 110 - 113, The early-return for all-null contingencies sets the
result status to NO_CALCULATION but downstream saveResult still maps any
non-CONVERGED loadflow status to SecurityAnalysisStatus.DIVERGED, making
NO_CALCULATION indistinguishable; update the code path in the saveResult logic
(or the method that maps LoadFlowResult/status to SecurityAnalysisStatus) to
explicitly check for the NO_CALCULATION sentinel (from
SecurityAnalysisWorkerService/runContext result creation) and map it to a
distinct SecurityAnalysisStatus (e.g., NO_CALCULATION) instead of DIVERGED,
ensuring functions like saveResult, mapLoadflowStatusToSecurityStatus, or
wherever SecurityAnalysisStatus is derived are updated to handle NO_CALCULATION
specially.
Comment on lines
185
to
+188
| } catch (IllegalArgumentException e) { | ||
| throw new SecurityAnalysisException(SecurityAnalysisBusinessErrorCode.CONTINGENCY_LIST_CONFIG_EMPTY, "The configuration does not contain any contingency."); | ||
| // Illegal argument exception seems to be linked to no contingency | ||
| logNoContingencies(runContext); | ||
| return; |
There was a problem hiding this comment.
Avoid swallowing all IllegalArgumentException as “no contingency”.
At Line 185, treating every IllegalArgumentException as a no-contingency case can mask real validation/runtime defects. Also, this branch returns without initializing runContext contingencies, which can break downstream logic that streams over them (Line 110).
💡 Proposed fix
} catch (IllegalArgumentException e) {
- // Illegal argument exception seems to be linked to no contingency
- logNoContingencies(runContext);
- return;
+ if (isNoContingencyException(e)) {
+ runContext.setContingencies(Collections.emptyList());
+ logNoContingencies(runContext);
+ return;
+ }
+ throw e;
}private static boolean isNoContingencyException(IllegalArgumentException e) {
return e.getMessage() != null && e.getMessage().toLowerCase(Locale.ROOT).contains("contingenc");
}🤖 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
`@src/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.java`
around lines 185 - 188, The catch for IllegalArgumentException in
SecurityAnalysisWorkerService should not assume every exception means "no
contingency"; add a helper isNoContingencyException(IllegalArgumentException)
that checks the exception message for a contagion of "contingenc"
(case-insensitive), and change the catch to: if isNoContingencyException(e) then
call logNoContingencies(runContext), initialize runContext.contingencies to an
empty list (e.g., Collections.emptyList()) and return; otherwise rethrow or
propagate the exception so real validation/runtime errors aren't swallowed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary