Skip to content

Conversation

@jonathan-eq
Copy link
Contributor

@jonathan-eq jonathan-eq commented Jan 6, 2026

Issue
Resolves #12547

Approach
Short description of the approach

(Screenshot of new behavior in GUI if applicable)

  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

@jonathan-eq jonathan-eq added the release-notes:maintenance Automatically categorise as maintenance change in release notes label Jan 6, 2026
@jonathan-eq jonathan-eq force-pushed the remove-responses-metadata branch 4 times, most recently from bc7d24f to 9f433b6 Compare January 9, 2026 13:59
@jonathan-eq
Copy link
Contributor Author

jonathan-eq commented Jan 9, 2026

This needs a storage migration
EDIT: It doesnt :)

@jonathan-eq jonathan-eq force-pushed the remove-responses-metadata branch 2 times, most recently from aac29f6 to 209b748 Compare January 13, 2026 10:52
@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.65%. Comparing base (94945a9) to head (0b92e6e).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12575      +/-   ##
==========================================
- Coverage   90.67%   90.65%   -0.03%     
==========================================
  Files         429      429              
  Lines       29801    29797       -4     
==========================================
- Hits        27023    27011      -12     
- Misses       2778     2786       +8     
Flag Coverage Δ
cli-tests 37.52% <24.24%> (-0.04%) ⬇️
gui-tests 69.35% <96.96%> (-0.05%) ⬇️
performance-and-unit-tests 73.91% <90.90%> (-0.02%) ⬇️
test 38.03% <27.27%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jonathan-eq jonathan-eq force-pushed the remove-responses-metadata branch 3 times, most recently from 09d6536 to 8b402e7 Compare January 14, 2026 14:18
@jonathan-eq jonathan-eq marked this pull request as ready for review January 14, 2026 14:19
@jonathan-eq jonathan-eq force-pushed the remove-responses-metadata branch 2 times, most recently from 7fc7810 to 006629c Compare January 15, 2026 11:16
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 15, 2026

Merging this PR will not alter performance

✅ 22 untouched benchmarks


Comparing jonathan-eq:remove-responses-metadata (0b92e6e) with main (94945a9)

Open in CodSpeed

@jonathan-eq jonathan-eq force-pushed the remove-responses-metadata branch 3 times, most recently from 906955f to a4df782 Compare January 15, 2026 14:30
@jonathan-eq jonathan-eq self-assigned this Jan 15, 2026
@frode-aarstad frode-aarstad requested a review from Copilot January 19, 2026 08:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request removes the ResponseMetadata class and its associated metadata property from the response configuration system. The PR refactors the codebase to use ResponseConfig objects directly instead of extracting metadata from them.

Changes:

  • Removed ResponseMetadata class from response_config.py and replaced the abstract metadata property with a new filter_on property that provides server-side filtering information
  • Updated all response config subclasses (GenDataConfig, SummaryConfig, RFTConfig, EverestResponse) to remove their metadata property implementations
  • Refactored the dark storage endpoints and plot API to work directly with ResponseConfig objects and their attributes
  • Added comprehensive test coverage for the new filter_on property behavior

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/ert/config/response_config.py Removed ResponseMetadata class and metadata property; added filter_on property with detailed documentation
src/ert/config/gen_data_config.py Removed metadata property; implemented filter_on property to expose report_step filtering
src/ert/config/summary_config.py Removed metadata property implementation
src/ert/config/rft_config.py Removed metadata property implementation
src/ert/config/everest_response.py Removed metadata property implementation
src/ert/config/init.py Removed ResponseMetadata from exports
src/ert/dark_storage/endpoints/experiments.py Updated to serialize response configs directly instead of metadata list
src/ert/gui/tools/plot/plot_api.py Refactored to use response config directly; updated key definition logic to extract filter_on from config
src/ert/gui/tools/plot/plot_window.py Updated to check response field instead of response_metadata
src/ert/gui/tools/plot/data_type_proxy_model.py Updated to check response.has_finalized_keys instead of response_metadata.finalized
tests/ert/unit_tests/config/test_gen_data_config.py Added new test for filter_on property functionality
tests/ert/unit_tests/gui/tools/plot/test_plot_api.py Updated test expectations to use ResponseConfig instead of ResponseMetadata
tests/ert/unit_tests/gui/tools/plot/conftest.py Updated mock data structure to match new ResponseConfig serialization
tests/ert/unit_tests/dark_storage/test_http_endpoints.py Updated test expectations for new response structure
tests/ert/unit_tests/dark_storage/test_dark_storage_state.py Updated to extract keys from nested config structure
tests/ert/performance_tests/test_dark_storage_performance.py Updated to use key_info.key instead of key_info.response_metadata.response_key

assert gdc.report_steps_list[0] == sorted(report_steps)


def test_report_step_list_is_added_to_filter_on():
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test name doesn't follow the specification style required by the coding guidelines. It should start with "test_that_" and describe the expected behavior or invariant being tested.

Consider renaming to something like: "test_that_filter_on_property_maps_report_steps_to_each_key"

Copilot generated this review using guidance from repository custom instructions.
"""Describe optional server-side filtering for this response.
Some responses contain an additional *discrete* dimension which is stored as a
column in the response dataset (often part of :pyattr:`primary_key`). When
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Sphinx cross-reference format is incorrect. ":pyattr:" should be ":py:attr:" for proper rendering in documentation.

Suggested change
column in the response dataset (often part of :pyattr:`primary_key`). When
column in the response dataset (often part of :py:attr:`primary_key`). When

Copilot uses AI. Check for mistakes.
"""Expose ``report_step`` as a discrete filter for ``GEN_DATA`` keys.
``GEN_DATA`` responses may be produced for multiple report steps. The stored
response data includes a ``report_step`` column (see :pyattr:`primary_key`).
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Sphinx cross-reference format is incorrect. ":pyattr:" should be ":py:attr:" for proper rendering in documentation.

Suggested change
response data includes a ``report_step`` column (see :pyattr:`primary_key`).
response data includes a ``report_step`` column (see :py:attr:`primary_key`).

Copilot uses AI. Check for mistakes.
@jonathan-eq jonathan-eq force-pushed the remove-responses-metadata branch from a4df782 to e311dc7 Compare January 19, 2026 08:42
},
responses={
response_type: [m.model_dump() for m in config.metadata]
response_type: [config.model_dump()]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to have in the list?

},
responses={
response_type: [m.model_dump() for m in config.metadata]
response_type: [config.model_dump()]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to have in the list?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@jonathan-eq jonathan-eq force-pushed the remove-responses-metadata branch from 0090df1 to ab42637 Compare January 19, 2026 12:35
@jonathan-eq jonathan-eq force-pushed the remove-responses-metadata branch from ab42637 to bed7d5b Compare January 19, 2026 12:43
@jonathan-eq jonathan-eq force-pushed the remove-responses-metadata branch from bed7d5b to 0b92e6e Compare January 19, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes:maintenance Automatically categorise as maintenance change in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove ResponseMetadata

3 participants