Repository navigation
do not throw when there is no contingency - #242
Merged
Merged
Conversation
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
This comment was marked as low quality.
This comment was marked as low quality.
![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.
Mathieu-Deharbe
self-requested a review
May 20, 2026 09:34
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
--------- Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.java (1)
227-229:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
NO_CALCULATIONstatus is still persisted asDIVERGED.The early-return logic at lines 111-114 sets the result status to
NO_CALCULATION, butsaveResultstill maps any non-CONVERGEDload-flow status toSecurityAnalysisStatus.DIVERGED. This makes no-contingency runs indistinguishable from actual divergence in persisted results.💡 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 227 - 229, The persisted status mapping in SecurityAnalysisWorkerService (inside saveResult) always maps any non-CONVERGED load-flow status to SecurityAnalysisStatus.DIVERGED, which overwrites NO_CALCULATION set earlier; update the mapping logic in saveResult to first check for LoadFlowResult.ComponentResult.Status.NO_CALCULATION and map it to SecurityAnalysisStatus.NO_CALCULATION, otherwise map CONVERGED to SecurityAnalysisStatus.CONVERGED and all other statuses to SecurityAnalysisStatus.DIVERGED so NO_CALCULATION is preserved.
🤖 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.
Duplicate comments:
In
`@src/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.java`:
- Around line 227-229: The persisted status mapping in
SecurityAnalysisWorkerService (inside saveResult) always maps any non-CONVERGED
load-flow status to SecurityAnalysisStatus.DIVERGED, which overwrites
NO_CALCULATION set earlier; update the mapping logic in saveResult to first
check for LoadFlowResult.ComponentResult.Status.NO_CALCULATION and map it to
SecurityAnalysisStatus.NO_CALCULATION, otherwise map CONVERGED to
SecurityAnalysisStatus.CONVERGED and all other statuses to
SecurityAnalysisStatus.DIVERGED so NO_CALCULATION is preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5e9e5c17-1d51-47a4-9edd-7f300263a6b7
📒 Files selected for processing (4)
src/main/java/org/gridsuite/securityanalysis/server/error/AllContingencyListMissingException.javasrc/main/java/org/gridsuite/securityanalysis/server/service/ActionsService.javasrc/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.javasrc/test/java/org/gridsuite/securityanalysis/server/service/ActionsServiceTest.java
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
Mathieu-Deharbe
requested changes
May 21, 2026
Contributor
There was a problem hiding this comment.
You have already seen it : SecurityAnalysisControllerTest tests don't work anymore.
Looks like it is because they expect a CONVERGED and get a NO_CALCULATION. But the order of 'expected' and 'actual' seems to be reversed in simpleRunRequest in SecurityAnalysisControllerTest.
The stopTest runs forever too.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.java (1)
110-111:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a null guard before streaming contingencies.
At Line 111,
runContext.getContingencies().stream()can throw if contingencies are null (e.g., upstream fetch returns null). Please mirror the guard already added inpreRunbefore callingstream().Proposed fix
- if (runContext.getContingencies().stream().allMatch(contingencyInfos -> contingencyInfos.getContingency() == null)) { + List<ContingencyInfos> contingenciesInfos = runContext.getContingencies(); + if (contingenciesInfos == null || contingenciesInfos.stream().allMatch(contingencyInfos -> contingencyInfos.getContingency() == null)) { return CompletableFuture.completedFuture( new SecurityAnalysisResult(new LimitViolationsResult(Collections.emptyList()), NO_CALCULATION, Collections.emptyList())); }🤖 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 - 111, getCompletableFuture currently calls runContext.getContingencies().stream() without guarding for a null contingencies list; add the same null check used in preRun so you only call stream() when runContext.getContingencies() != null (or fallback to Collections.emptyList()) to avoid NPEs. Locate the getCompletableFuture method and wrap or replace the direct stream call on runContext.getContingencies() with a null-safe access (e.g., check runContext.getContingencies() != null before streaming or use a null-to-empty list utility) so the method behaves safely when upstream returns null.
♻️ Duplicate comments (1)
src/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.java (1)
223-229:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist
NO_CALCULATIONas its own security-analysis status.
saveResultstill maps every non-CONVERGEDload-flow status toDIVERGED, soNO_CALCULATIONis not distinguishable after persistence. At Line 227 this should explicitly map toSecurityAnalysisStatus.NO_CALCULATION.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 223 - 229, The saveResult method currently maps any non-CONVERGED pre-contingency load-flow status to SecurityAnalysisStatus.DIVERGED; update the mapping in saveResult (look at resultService.insert call using resultContext.getResultUuid() and result.getPreContingencyResult()) to explicitly check for LoadFlowResult.ComponentResult.Status.NO_CALCULATION and pass SecurityAnalysisStatus.NO_CALCULATION in that case, otherwise keep CONVERGED -> SecurityAnalysisStatus.CONVERGED and all other statuses -> SecurityAnalysisStatus.DIVERGED.
🤖 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.
Outside diff comments:
In
`@src/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.java`:
- Around line 110-111: getCompletableFuture currently calls
runContext.getContingencies().stream() without guarding for a null contingencies
list; add the same null check used in preRun so you only call stream() when
runContext.getContingencies() != null (or fallback to Collections.emptyList())
to avoid NPEs. Locate the getCompletableFuture method and wrap or replace the
direct stream call on runContext.getContingencies() with a null-safe access
(e.g., check runContext.getContingencies() != null before streaming or use a
null-to-empty list utility) so the method behaves safely when upstream returns
null.
---
Duplicate comments:
In
`@src/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.java`:
- Around line 223-229: The saveResult method currently maps any non-CONVERGED
pre-contingency load-flow status to SecurityAnalysisStatus.DIVERGED; update the
mapping in saveResult (look at resultService.insert call using
resultContext.getResultUuid() and result.getPreContingencyResult()) to
explicitly check for LoadFlowResult.ComponentResult.Status.NO_CALCULATION and
pass SecurityAnalysisStatus.NO_CALCULATION in that case, otherwise keep
CONVERGED -> SecurityAnalysisStatus.CONVERGED and all other statuses ->
SecurityAnalysisStatus.DIVERGED.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f85b3eba-ec6c-496e-ae5e-ca7f233fe61f
📒 Files selected for processing (3)
src/main/java/org/gridsuite/securityanalysis/server/service/SecurityAnalysisWorkerService.javasrc/main/resources/org/gridsuite/securityanalysis/server/reports.propertiessrc/test/java/org/gridsuite/securityanalysis/server/SecurityAnalysisControllerTest.java
|
Mathieu-Deharbe
approved these changes
May 21, 2026
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
2 participants
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