Closed
Conversation
Review Summary by QodoAdd unified grid and summary ensemble import dialog with file search
WalkthroughsDescription• Add unified dialog for importing grid and summary ensemble files • Implement file search and filtering with ensemble grouping modes • Integrate new import feature into toolbar and main menu • Make importSingleEnsembleFileSet public for external callers • Fix debug build error in RigActiveCellInfo and lambda capture issue Diagramflowchart LR
A["User triggers<br/>Import Feature"] --> B["RicImportGridAndSummaryEnsembleDialog"]
B --> C["Search and filter<br/>grid/summary files"]
C --> D["Group by ensemble<br/>mode"]
D --> E["Select realizations"]
E --> F["RicImportGridAndSummaryEnsembleFeature"]
F --> G["Create grid and<br/>summary ensembles"]
File Changes1. ApplicationLibCode/Commands/RicImportGridAndSummaryEnsembleDialog.h
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
ApplicationLibCode/Commands/RicImportGridAndSummaryEnsembleFeature.cpp
Outdated
Show resolved
Hide resolved
Comment on lines
91
to
96
| void RigActiveCellInfo::setCellResultIndex( ReservoirCellIndex reservoirCellIndex, ActiveCellIndex reservoirCellResultIndex ) | ||
| { | ||
| CVF_TIGHT_ASSERT( reservoirCellResultIndex < m_reservoirCellToActiveCell.size() ); | ||
| CVF_TIGHT_ASSERT( reservoirCellResultIndex.value() < m_reservoirCellToActiveCell.size() ); | ||
|
|
||
| m_reservoirCellToActiveCell[reservoirCellIndex.value()] = reservoirCellResultIndex; | ||
| } |
There was a problem hiding this comment.
2. Wrong bounds assert 🐞 Bug ✓ Correctness
RigActiveCellInfo::setCellResultIndex asserts on reservoirCellResultIndex but indexes the vector using reservoirCellIndex, allowing out-of-bounds writes. This is inconsistent with other methods which correctly assert reservoirCellIndex before indexing.
Agent Prompt
### Issue description
`RigActiveCellInfo::setCellResultIndex` currently asserts the bounds of `reservoirCellResultIndex` but then indexes the vector using `reservoirCellIndex`, so an invalid `reservoirCellIndex` can still write out of bounds.
### Issue Context
Other methods in the same file (`isActive`, `cellResultIndex`) correctly assert `reservoirCellIndex.value() < m_reservoirCellToActiveCell.size()` before indexing.
### Fix Focus Areas
- ApplicationLibCode/ReservoirDataModel/RigActiveCellInfo.cpp[91-96]
### Suggested change
Replace:
- `CVF_TIGHT_ASSERT( reservoirCellResultIndex.value() < m_reservoirCellToActiveCell.size() );`
with:
- `CVF_TIGHT_ASSERT( reservoirCellIndex.value() < m_reservoirCellToActiveCell.size() );`
Optionally add a second assert if there is a well-defined upper bound for `reservoirCellResultIndex`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…ansion, unused member
…ined import dialog Make importSingleEnsembleFileSet public in RicImportEnsembleFeature to allow external callers.
… post-import updates, fix icon order
…Eclipse Cases menu
Replace RiaDefines.h include with Summary/RiaSummaryDefines.h since EnsembleGroupingMode is defined there. Add RimSummaryCaseMainCollection.h include to the feature cpp to resolve undefined type error.
…d of legacy import helpers
… files for RimEnsembleFileSet
…cating base paths before creating file set
…ility Update the feature to use a new dedicated SVG icon, add the action to the main toolbar, and move it to the top level of the Import menu for better accessibility.
…ation and UX improvements - Create a default 3D view after importing grid ensemble - Create a default summary multi-plot after importing summary ensemble - Show plot main window when summary ensemble is imported - Restore previously active main window after import (500ms deferred raise/activate) - Add RimReservoirGridEnsemble::viewCollection() public accessor - Fix RimEnsembleFileSetTools::createGridEnsemblesFromFileSets to call createGridCasesFromEnsembleFileSet() before loadDataAndUpdate() so cases are populated immediately (not only on project reload) - Restore most recently used path pattern in dialog when preference is enabled - Add GridAndSummaryEnsemble.svg icon and register in resource file - Move menu item from Eclipse Cases submenu to top-level Import menu
…nd fix import guards - Move firstLevelItems, setCheckedStateChildItems, findItemsMatching and populateComboBoxHistoryFromRegistry to public static methods on RicRecursiveFileSearchDialog, eliminating identical copies in RicImportGridAndSummaryEnsembleDialog - Skip grid ensemble creation when no grid files are found - Skip summary ensemble creation when no summary files are found - Keep focus in plot window when no grid ensembles are created
magnesj
force-pushed
the
unified-emsemble-import
branch
from
8e5a8a8 to
c546d49
Compare
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.
No description provided.