keep only filter ids in back for sc parameters#1185
Draft
EtienneLt wants to merge 11 commits into
Draft
Conversation
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughTypes and utils now serialize short-circuit cluster filters as raw UUID arrays; service-layer enrichment resolves those UUIDs into {id,name} objects via fetchElementNames and returns enriched parameters. Small debug logs added in form reset and parameter update. ChangesFilter ID enrichment and serialization
Suggested reviewers:
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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/features/parameters/short-circuit/short-circuit-parameters-utils.ts`:
- Around line 247-250: The mapper powerElectronicsClustersParam currently
spreads rest and thus persists the form-only active flag; update the mapping in
the block that handles powerElectronicsClustersParam so you destructure and omit
active (e.g., const { filters, active, ...rest } = sParam) and return { ...rest,
filters: lightFilters } so active is not written to
specificParametersPerProvider; then ensure the form-enrichment path (where you
build FormPowerElectronicsCluster for the UI) re-adds active: true when
converting stored PowerElectronicsCluster back to the form model.
In `@src/services/short-circuit-analysis.ts`:
- Around line 35-43: The JSON.parse calls in parseNodeClusters (and the sibling
parsePowerElectronicsClusters used in lines ~46-57) can throw and abort
parameter loading; wrap each JSON.parse(...) in a try/catch and on parse failure
treat the provider-local value as an empty array/set instead of throwing—i.e.,
if parsing succeeds convert the result to the expected Set<string> and set it
into filterIdsFromNodeCluster, otherwise set an empty Set for that provider
(optionally log the parse error), so a single malformed persisted JSON value
doesn't break
enrichShortCircuitParameters()/fetchShortCircuitParameters()/getShortCircuitParameters().
🪄 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: 11924157-f0cc-47fd-bb81-e83b66bb496c
📒 Files selected for processing (3)
src/features/parameters/short-circuit/short-circuit-parameters-utils.tssrc/features/parameters/short-circuit/short-circuit-parameters.type.tssrc/services/short-circuit-analysis.ts
Comment on lines
+35
to
+43
| function parseNodeClusters(parameters: ShortCircuitParametersInfos): Map<string, Set<string>> { | ||
| const filterIdsFromNodeCluster = new Map<string, Set<string>>(); | ||
| Object.entries(parameters.specificParametersPerProvider).forEach(([provider, specificParamValue]) => { | ||
| if (Object.hasOwn(specificParamValue, NODE_CLUSTER_FILTER_IDS)) { | ||
| const filters = JSON.parse(specificParamValue[NODE_CLUSTER_FILTER_IDS]); | ||
| filterIdsFromNodeCluster.set(provider, filters); | ||
| } | ||
| }); | ||
| return filterIdsFromNodeCluster; |
There was a problem hiding this comment.
Malformed persisted JSON now breaks parameter loading.
Both parsing helpers call JSON.parse(...) on stored provider values with no fallback. One bad nodeClusterFilterIds or powerElectronicsClusters string will reject enrichShortCircuitParameters(), which now means both fetchShortCircuitParameters() and getShortCircuitParameters() fail before the form can reset. Treat parse failures as provider-local fallbacks ([]) instead of aborting the whole fetch.
Also applies to: 46-57
🤖 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/services/short-circuit-analysis.ts` around lines 35 - 43, The JSON.parse
calls in parseNodeClusters (and the sibling parsePowerElectronicsClusters used
in lines ~46-57) can throw and abort parameter loading; wrap each
JSON.parse(...) in a try/catch and on parse failure treat the provider-local
value as an empty array/set instead of throwing—i.e., if parsing succeeds
convert the result to the expected Set<string> and set it into
filterIdsFromNodeCluster, otherwise set an empty Set for that provider
(optionally log the parse error), so a single malformed persisted JSON value
doesn't break
enrichShortCircuitParameters()/fetchShortCircuitParameters()/getShortCircuitParameters().
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/features/parameters/short-circuit/short-circuit-parameters-utils.ts (1)
290-307: 💤 Low valueUnused
defaultValuesparameter.The
defaultValuesparameter is never referenced in the function body—onlyspecificParamValueis parsed and mapped. If clusters intentionally have no default-merging logic (unlike materials), remove the unused parameter to avoid confusion.Also, per static analysis: delete the extra blank line at line 307.
♻️ Proposed fix
-const formatElectronicsClustersParamString = ( - defaultValues: PowerElectronicsCluster[], - specificParamValue: string, - snackError: (message: SnackInputs) => void -) => { +const formatElectronicsClustersParamString = ( + specificParamValue: string, + snackError: (message: SnackInputs) => void +) => {And update the call site at line 348-354:
formatted[SHORT_CIRCUIT_POWER_ELECTRONICS_CLUSTERS] = formatElectronicsClustersParamString( - getDefaultShortCircuitSpecificParamsValues([powerElectronicsClustersParam], snackError)?.[ - SHORT_CIRCUIT_POWER_ELECTRONICS_CLUSTERS - ], specificParamsList[SHORT_CIRCUIT_POWER_ELECTRONICS_CLUSTERS] as string, snackError );🤖 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/features/parameters/short-circuit/short-circuit-parameters-utils.ts` around lines 290 - 307, Remove the unused defaultValues parameter from formatElectronicsClustersParamString and update its signature to only accept (specificParamValue: string, snackError: (message: SnackInputs) => void); inside the function keep the call to parsePowerElectronicsClustersParamString and the mapping logic unchanged (referenced symbols: formatElectronicsClustersParamString, parsePowerElectronicsClustersParamString, snackError); then update all call sites that still pass a defaultValues argument to call the new two-argument signature and remove the extra blank line at the end of the file.Source: Linters/SAST tools
🤖 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/features/parameters/short-circuit/short-circuit-parameters-utils.ts`:
- Around line 276-288: The parser parsePowerElectronicsClustersParamString
currently claims to return (PowerElectronicsCluster & { active: boolean })[] but
actually receives enriched filters with objects {id, name}; create a new
EnrichedPowerElectronicsCluster type (e.g., type EnrichedPowerElectronicsCluster
= PowerElectronicsCluster & { filters: {id: UUID; name: string}[]; active:
boolean }) and change parsePowerElectronicsClustersParamString's return type to
EnrichedPowerElectronicsCluster[]; update any callers (notably
formatElectronicsClustersParamString) to accept/handle
EnrichedPowerElectronicsCluster so accessing filter.id and filter.name is
type-safe; ensure JSON.parse is cast to EnrichedPowerElectronicsCluster[] and
keep the same error handling, and remove the extra blank line preceding the
function declaration.
- Around line 364-369: The JSON.parse call for NODE_CLUSTER_FILTER_IDS lacks
error handling and can throw on malformed backend data; wrap the parsing in a
try-catch similar to
parsePowerElectronicsMaterialsParamString/parsePowerElectronicsClustersParamString,
catch the error, call snackWithFallback with an appropriate user-facing message,
and set formatted[NODE_CLUSTER_FILTER_IDS] to an empty array or safe default;
keep the mapping that uses ID and NAME keys from filter.id/filter.name and read
from specificParamsList[NODE_CLUSTER_FILTER_IDS] inside the try block so
failures don't crash the parameter loading flow.
In `@src/features/parameters/short-circuit/use-short-circuit-parameters-form.ts`:
- Line 296: Remove the debug console.log that prints the entire parameters
payload in the reset path: delete or replace the line console.log('reset',
params) in use-short-circuit-parameters-form.ts (the reset handling block that
references params) so production consoles do not receive full study
configuration; if logging is required, log a minimal, non-sensitive message or a
sanitized/summary value instead.
---
Nitpick comments:
In `@src/features/parameters/short-circuit/short-circuit-parameters-utils.ts`:
- Around line 290-307: Remove the unused defaultValues parameter from
formatElectronicsClustersParamString and update its signature to only accept
(specificParamValue: string, snackError: (message: SnackInputs) => void); inside
the function keep the call to parsePowerElectronicsClustersParamString and the
mapping logic unchanged (referenced symbols:
formatElectronicsClustersParamString, parsePowerElectronicsClustersParamString,
snackError); then update all call sites that still pass a defaultValues argument
to call the new two-argument signature and remove the extra blank line at the
end of the file.
🪄 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: ab5e0f71-2d0a-46c6-ad2f-a8a7de16b74b
📒 Files selected for processing (3)
src/features/parameters/short-circuit/short-circuit-parameters-utils.tssrc/features/parameters/short-circuit/use-short-circuit-parameters-form.tssrc/services/short-circuit-analysis.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/short-circuit-analysis.ts
Comment on lines
+364
to
369
| // NODE_CLUSTER_FILTER_IDS is parsed when getting shortcircuit parameters | ||
| const filters = JSON.parse(specificParamsList[NODE_CLUSTER_FILTER_IDS]); | ||
| formatted[NODE_CLUSTER_FILTER_IDS] = filters.map((filter: { filterId: any; filterName: any }) => ({ | ||
| [ID]: filter.filterId, | ||
| [NAME]: filter.filterName, // from back to front -> {id: uuid, name: string} | ||
| formatted[NODE_CLUSTER_FILTER_IDS] = filters.map((filter: { id: any; name: any }) => ({ | ||
| [ID]: filter.id, | ||
| [NAME]: filter.name, // from back to front -> {id: uuid, name: string} | ||
| })); |
There was a problem hiding this comment.
Missing error handling for JSON.parse on node-cluster filter IDs.
Unlike parsePowerElectronicsMaterialsParamString and parsePowerElectronicsClustersParamString, this JSON.parse call has no try-catch. If the backend returns malformed JSON in NODE_CLUSTER_FILTER_IDS, the formatter will throw and crash the parameter loading flow.
Wrap in try-catch with snackWithFallback for consistency:
🐛 Proposed fix
if (Object.hasOwn(specificParamsList, NODE_CLUSTER_FILTER_IDS)) {
- // NODE_CLUSTER_FILTER_IDS is parsed when getting shortcircuit parameters
- const filters = JSON.parse(specificParamsList[NODE_CLUSTER_FILTER_IDS]);
- formatted[NODE_CLUSTER_FILTER_IDS] = filters.map((filter: { id: any; name: any }) => ({
- [ID]: filter.id,
- [NAME]: filter.name, // from back to front -> {id: uuid, name: string}
- }));
+ // NODE_CLUSTER_FILTER_IDS is parsed when getting shortcircuit parameters
+ try {
+ const filters = JSON.parse(specificParamsList[NODE_CLUSTER_FILTER_IDS]);
+ formatted[NODE_CLUSTER_FILTER_IDS] = filters.map((filter: { id: any; name: any }) => ({
+ [ID]: filter.id,
+ [NAME]: filter.name,
+ }));
+ } catch (error) {
+ snackWithFallback(snackError, error, { headerId: 'ShortCircuitNodeClusterFilterIdsParsingError' });
+ formatted[NODE_CLUSTER_FILTER_IDS] = [];
+ }
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // NODE_CLUSTER_FILTER_IDS is parsed when getting shortcircuit parameters | |
| const filters = JSON.parse(specificParamsList[NODE_CLUSTER_FILTER_IDS]); | |
| formatted[NODE_CLUSTER_FILTER_IDS] = filters.map((filter: { filterId: any; filterName: any }) => ({ | |
| [ID]: filter.filterId, | |
| [NAME]: filter.filterName, // from back to front -> {id: uuid, name: string} | |
| formatted[NODE_CLUSTER_FILTER_IDS] = filters.map((filter: { id: any; name: any }) => ({ | |
| [ID]: filter.id, | |
| [NAME]: filter.name, // from back to front -> {id: uuid, name: string} | |
| })); | |
| // NODE_CLUSTER_FILTER_IDS is parsed when getting shortcircuit parameters | |
| try { | |
| const filters = JSON.parse(specificParamsList[NODE_CLUSTER_FILTER_IDS]); | |
| formatted[NODE_CLUSTER_FILTER_IDS] = filters.map((filter: { id: any; name: any }) => ({ | |
| [ID]: filter.id, | |
| [NAME]: filter.name, | |
| })); | |
| } catch (error) { | |
| snackWithFallback(snackError, error, { headerId: 'ShortCircuitNodeClusterFilterIdsParsingError' }); | |
| formatted[NODE_CLUSTER_FILTER_IDS] = []; | |
| } |
🤖 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/features/parameters/short-circuit/short-circuit-parameters-utils.ts`
around lines 364 - 369, The JSON.parse call for NODE_CLUSTER_FILTER_IDS lacks
error handling and can throw on malformed backend data; wrap the parsing in a
try-catch similar to
parsePowerElectronicsMaterialsParamString/parsePowerElectronicsClustersParamString,
catch the error, call snackWithFallback with an appropriate user-facing message,
and set formatted[NODE_CLUSTER_FILTER_IDS] to an empty array or safe default;
keep the mapping that uses ID and NAME keys from filter.id/filter.name and read
from specificParamsList[NODE_CLUSTER_FILTER_IDS] inside the try block so
failures don't crash the parameter loading flow.
| if (!params || !provider || !specificParamsDescription) { | ||
| return; | ||
| } | ||
| console.log('reset', params); |
There was a problem hiding this comment.
Remove debug logging of full parameter payload in reset path.
console.log('reset', params) emits the entire parameters object on a routine UI path and can leak study configuration details in production consoles while adding noise.
Suggested fix
- console.log('reset', params);
reset(toShortCircuitFormValues(params));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| console.log('reset', params); | |
| reset(toShortCircuitFormValues(params)); |
🤖 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/features/parameters/short-circuit/use-short-circuit-parameters-form.ts`
at line 296, Remove the debug console.log that prints the entire parameters
payload in the reset path: delete or replace the line console.log('reset',
params) in use-short-circuit-parameters-form.ts (the reset handling block that
references params) so production consoles do not receive full study
configuration; if logging is required, log a minimal, non-sensitive message or a
sanitized/summary value instead.
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 (2)
src/features/parameters/short-circuit/short-circuit-parameters-utils.ts (2)
246-252:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
activeflag still serialized for clusters (unlike materials).Line 242 correctly strips
activefrom materials:const { active, ...rest } = sParam. However, line 248 only destructuresfilters, leavingactivein...restwhich gets serialized to the backend. This persists UI-only state in the stored parameters.🐛 Proposed fix
finalSpecificParameters[SHORT_CIRCUIT_POWER_ELECTRONICS_CLUSTERS] = JSON.stringify( powerElectronicsClustersParam.map((sParam) => { - const { filters, ...rest } = sParam; + const { filters, active, ...rest } = sParam; const lightFilters = filters?.map((filter) => filter[ID]) ?? []; return { ...rest, filters: lightFilters }; }) );🤖 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/features/parameters/short-circuit/short-circuit-parameters-utils.ts` around lines 246 - 252, The serialized clusters still include the UI-only active flag because the mapping only strips filters; update the mapping for powerElectronicsClustersParam used to set finalSpecificParameters[SHORT_CIRCUIT_POWER_ELECTRONICS_CLUSTERS] to also destructure and remove active (e.g., const { active, filters, ...rest } = sParam) before building the returned object so only the intended fields (with filters reduced to ids) are serialized to the backend.
276-287:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winBuild failure: type mismatch between declared return type and actual runtime data.
parsePowerElectronicsClustersParamStringdeclares its return type as(PowerElectronicsCluster & { active: boolean })[], wherefiltersisUUID[]. However, at line 300,formatElectronicsClustersParamStringmaps each filter as{ id: string; name: string }— the enriched shape returned by the service layer afterenrichShortCircuitParametersresolves IDs to{id, name}objects.Define an enriched type that reflects the actual runtime shape after service-layer enrichment:
🐛 Proposed fix
In
short-circuit-parameters.type.ts, add:export interface EnrichedPowerElectronicsCluster extends BasePowerElectronicsCluster { filters: FilterPOJO[]; }Then in this file:
const parsePowerElectronicsClustersParamString = ( paramString: string, snackError: (message: SnackInputs) => void -): (PowerElectronicsCluster & { active: boolean })[] => { +): (EnrichedPowerElectronicsCluster & { active: boolean })[] => { // Attempt to parse the string into an array of PowerElectronicsCluster objects try { return JSON.parse(paramString);And update
formatElectronicsClustersParamString:const formatElectronicsClustersParamString = ( - defaultValues: PowerElectronicsCluster[], + defaultValues: EnrichedPowerElectronicsCluster[], specificParamValue: string, snackError: (message: SnackInputs) => void ) => { - const electronicsClustersArrayInParams: (PowerElectronicsCluster & { active: boolean })[] = + const electronicsClustersArrayInParams: (EnrichedPowerElectronicsCluster & { active: boolean })[] = parsePowerElectronicsClustersParamString(specificParamValue, snackError); return electronicsClustersArrayInParams.map((cluster) => { const { filters, ...rest } = cluster; return { ...rest, - filters: filters.map((filter: { id: string; name: string }) => ({ + filters: filters.map((filter) => ({ [ID]: filter.id, [NAME]: filter.name, })),Also applies to: 300-303
🤖 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/features/parameters/short-circuit/short-circuit-parameters-utils.ts` around lines 276 - 287, The runtime shape returned by the service is enriched (filters are objects like {id,name}) so create a new type EnrichedPowerElectronicsCluster (e.g. export interface EnrichedPowerElectronicsCluster extends BasePowerElectronicsCluster { filters: FilterPOJO[] }) in short-circuit-parameters.type.ts, then update parsePowerElectronicsClustersParamString to return EnrichedPowerElectronicsCluster[] instead of (PowerElectronicsCluster & { active: boolean })[] and update formatElectronicsClustersParamString to map/expect filters as FilterPOJO ({id:string,name:string}) accordingly; ensure references to BasePowerElectronicsCluster, FilterPOJO, parsePowerElectronicsClustersParamString, and formatElectronicsClustersParamString are updated to use the new EnrichedPowerElectronicsCluster type.Source: Linters/SAST tools
🤖 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/features/parameters/short-circuit/short-circuit-parameters-utils.ts`:
- Around line 246-252: The serialized clusters still include the UI-only active
flag because the mapping only strips filters; update the mapping for
powerElectronicsClustersParam used to set
finalSpecificParameters[SHORT_CIRCUIT_POWER_ELECTRONICS_CLUSTERS] to also
destructure and remove active (e.g., const { active, filters, ...rest } =
sParam) before building the returned object so only the intended fields (with
filters reduced to ids) are serialized to the backend.
- Around line 276-287: The runtime shape returned by the service is enriched
(filters are objects like {id,name}) so create a new type
EnrichedPowerElectronicsCluster (e.g. export interface
EnrichedPowerElectronicsCluster extends BasePowerElectronicsCluster { filters:
FilterPOJO[] }) in short-circuit-parameters.type.ts, then update
parsePowerElectronicsClustersParamString to return
EnrichedPowerElectronicsCluster[] instead of (PowerElectronicsCluster & {
active: boolean })[] and update formatElectronicsClustersParamString to
map/expect filters as FilterPOJO ({id:string,name:string}) accordingly; ensure
references to BasePowerElectronicsCluster, FilterPOJO,
parsePowerElectronicsClustersParamString, and
formatElectronicsClustersParamString are updated to use the new
EnrichedPowerElectronicsCluster type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7ab19e8-0a32-47a9-9f7a-3c2546fffde6
📒 Files selected for processing (3)
src/features/parameters/short-circuit/short-circuit-parameters-utils.tssrc/features/parameters/short-circuit/short-circuit-parameters.type.tssrc/services/short-circuit-analysis.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/short-circuit-analysis.ts
EtienneLt
requested review from
TheMaskedTurtle,
antoinebhs,
ayolab,
flomillot and
klesaulnier
as code owners
June 24, 2026 06:23
EtienneLt
marked this pull request as draft
June 24, 2026 06:23
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