Add hybrid scan multifile reader basics - #22616
Merged
rapids-bot[bot] merged 19 commits intoJun 2, 2026
Merged
Conversation
github-actions
Bot
added
libcudf
mhaseeb123
marked this pull request as draft
May 21, 2026 01:16
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
mhaseeb123
commented
May 21, 2026
| * | ||
| * @note Detailed usage documentation will be added once all APIs are in place. | ||
| */ | ||
| class hybrid_scan_multifile { |
Contributor
Author
There was a problem hiding this comment.
I am going to eventually remove this file, move this class to hybrid_scan.hpp and make the existing single-file reader (hybrid_scan_reader) a subclass of this one. Only keeping this separate for now to avoid noise.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors hybrid-scan internals to operate on multiple Parquet sources, adds an exported ChangesMulti-source Parquet Hybrid Scan Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Suggested reviewers
🚥 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (2)
cpp/src/io/parquet/experimental/hybrid_scan.cpp (1)
32-40: ⚡ Quick winInstrument touched delegating public methods with
CUDF_FUNC_RANGE().The updated wrapper methods delegate directly to
_implbut currently skip NVTX range tagging.🛠️ Suggested patch
[[nodiscard]] text::byte_range_info hybrid_scan_reader::page_index_byte_range() const { + CUDF_FUNC_RANGE(); return _impl->page_index_byte_range().front(); } [[nodiscard]] FileMetaData hybrid_scan_reader::parquet_metadata() const { + CUDF_FUNC_RANGE(); return _impl->parquet_metadata().front(); } @@ std::vector<cudf::size_type> hybrid_scan_reader::all_row_groups( parquet_reader_options const& options) const { + CUDF_FUNC_RANGE(); CUDF_EXPECTS(options.get_row_groups().size() <= 1, "Encountered invalid size of row group indices in parquet reader options");As per coding guidelines
cpp/src/**/*.{cu,cpp}: "Add CUDF_FUNC_RANGE() in public functions before delegating to detail:: functions".Also applies to: 50-60
🤖 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 `@cpp/src/io/parquet/experimental/hybrid_scan.cpp` around lines 32 - 40, The public wrapper methods hybrid_scan_reader::page_index_byte_range() and hybrid_scan_reader::parquet_metadata() (and other public wrappers around lines 50-60) need NVTX/CUDF range instrumentation: add CUDF_FUNC_RANGE() as the first statement in each of these public functions before delegating to _impl (e.g., before calling _impl->page_index_byte_range().front() and _impl->parquet_metadata().front()) so the calls are properly range-tagged per the cpp/src/**/*.{cu,cpp} guideline.cpp/src/io/parquet/experimental/hybrid_scan_multifile.cpp (1)
28-58: ⚡ Quick winAdd
CUDF_FUNC_RANGE()to the delegating public methods.Several public APIs here delegate directly to
_implbut are missing NVTX range instrumentation (e.g., Lines 28-36, 45-56, 58).🛠️ Suggested patch
std::vector<FileMetaData> hybrid_scan_multifile::parquet_metadata() const { + CUDF_FUNC_RANGE(); return _impl->parquet_metadata(); } std::vector<text::byte_range_info> hybrid_scan_multifile::page_index_byte_range() const { + CUDF_FUNC_RANGE(); return _impl->page_index_byte_range(); } @@ std::vector<std::vector<size_type>> hybrid_scan_multifile::all_row_groups( parquet_reader_options const& options) const { + CUDF_FUNC_RANGE(); return _impl->all_row_groups(options); } size_type hybrid_scan_multifile::total_rows_in_row_groups( cudf::host_span<std::vector<size_type> const> row_group_indices) const { + CUDF_FUNC_RANGE(); if (row_group_indices.empty()) { return 0; } return _impl->total_rows_in_row_groups(row_group_indices); } -void hybrid_scan_multifile::reset_column_selection() const { _impl->reset_column_selection(); } +void hybrid_scan_multifile::reset_column_selection() const +{ + CUDF_FUNC_RANGE(); + _impl->reset_column_selection(); +}As per coding guidelines
cpp/src/**/*.{cu,cpp}: "Add CUDF_FUNC_RANGE() in public functions before delegating to detail:: functions".🤖 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 `@cpp/src/io/parquet/experimental/hybrid_scan_multifile.cpp` around lines 28 - 58, Add NVTX range instrumentation by inserting CUDF_FUNC_RANGE() at the start of each public delegating method that currently lacks it: parquet_metadata(), page_index_byte_range(), all_row_groups(parquet_reader_options const&), total_rows_in_row_groups(cudf::host_span<std::vector<size_type> const>) and reset_column_selection(). Place CUDF_FUNC_RANGE() before delegating to _impl (for total_rows_in_row_groups, ensure the range is present when the function proceeds to call _impl — keep the early empty check but add CUDF_FUNC_RANGE() before the _impl call). This ensures all public APIs wrap their impl delegation with NVTX ranges.
🤖 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 `@cpp/src/io/parquet/experimental/hybrid_scan_helpers.cpp`:
- Around line 166-199: In aggregate_reader_metadata::setup_page_index, when
iterating page_index_bytes and hitting an empty pgidx_bytes currently skipped
silently, emit the documented one-time warning instead of just returning; add a
local bool (e.g., warned_empty_page_index) before the loop, and on first empty
pgidx_bytes log a warning (including the source identifier or index if
available) and set the flag so subsequent empties are not logged, then continue;
ensure you still call file_metadata.setup_page_index(...) for non-empty spans
and keep references to page_index_bytes and per_file_metadata as before.
---
Nitpick comments:
In `@cpp/src/io/parquet/experimental/hybrid_scan_multifile.cpp`:
- Around line 28-58: Add NVTX range instrumentation by inserting
CUDF_FUNC_RANGE() at the start of each public delegating method that currently
lacks it: parquet_metadata(), page_index_byte_range(),
all_row_groups(parquet_reader_options const&),
total_rows_in_row_groups(cudf::host_span<std::vector<size_type> const>) and
reset_column_selection(). Place CUDF_FUNC_RANGE() before delegating to _impl
(for total_rows_in_row_groups, ensure the range is present when the function
proceeds to call _impl — keep the early empty check but add CUDF_FUNC_RANGE()
before the _impl call). This ensures all public APIs wrap their impl delegation
with NVTX ranges.
In `@cpp/src/io/parquet/experimental/hybrid_scan.cpp`:
- Around line 32-40: The public wrapper methods
hybrid_scan_reader::page_index_byte_range() and
hybrid_scan_reader::parquet_metadata() (and other public wrappers around lines
50-60) need NVTX/CUDF range instrumentation: add CUDF_FUNC_RANGE() as the first
statement in each of these public functions before delegating to _impl (e.g.,
before calling _impl->page_index_byte_range().front() and
_impl->parquet_metadata().front()) so the calls are properly range-tagged per
the cpp/src/**/*.{cu,cpp} guideline.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fb3c5e4e-9d78-4b6b-b108-6823a3a1a8fe
📒 Files selected for processing (8)
cpp/CMakeLists.txtcpp/include/cudf/io/experimental/hybrid_scan_multifile.hppcpp/src/io/parquet/experimental/hybrid_scan.cppcpp/src/io/parquet/experimental/hybrid_scan_helpers.cppcpp/src/io/parquet/experimental/hybrid_scan_helpers.hppcpp/src/io/parquet/experimental/hybrid_scan_impl.cppcpp/src/io/parquet/experimental/hybrid_scan_impl.hppcpp/src/io/parquet/experimental/hybrid_scan_multifile.cpp
mhaseeb123
added
2 - In Progress
mhaseeb123
marked this pull request as ready for review
May 21, 2026 19:50
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 `@cpp/tests/io/experimental/hybrid_scan_multifile_filters_test.cpp`:
- Around line 8-9: Add the project gtest wrapper header to the test translation
unit by inserting `#include` <cudf_test/cudf_gtest.hpp> alongside the existing
`#include` <cudf_test/base_fixture.hpp> in hybrid_scan_multifile_filters_test.cpp
so the file uses the project wrapper instead of relying on raw gtest headers;
ensure the new include appears before any test definitions or fixtures (e.g.,
references to BaseFixture/Test macros) to follow the project's test include
convention.
- Around line 23-76: The helpers are defined inside an anonymous namespace which
violates the test-file namespace rule; move the struct multifile_inputs and the
functions build_multifile_inputs and create_empty_parquet_with_stats out of the
anonymous namespace into the global namespace (remove "namespace { ... }"
wrapper) so they live at global scope and keep their signatures and behavior
unchanged; ensure any forward-uses in this file still compile after the move and
remove the closing comment that references the anonymous namespace.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0179ac61-2242-47c5-8f70-421c96a28264
📒 Files selected for processing (3)
cpp/include/cudf/io/experimental/hybrid_scan_multifile.hppcpp/tests/CMakeLists.txtcpp/tests/io/experimental/hybrid_scan_multifile_filters_test.cpp
Comment on lines
+8
to
+9
| #include <cudf_test/base_fixture.hpp> | ||
|
|
There was a problem hiding this comment.
Include cudf_gtest.hpp explicitly in this test file.
Line 8 currently includes only base_fixture.hpp. Please add the direct gtest wrapper include in this TU to match project test conventions.
Suggested patch
`#include` <cudf_test/base_fixture.hpp>
+#include <cudf_test/cudf_gtest.hpp>As per coding guidelines Use #include <cudf_test/cudf_gtest.hpp> in test files, never raw gtest/gtest.h.
📝 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
| #include <cudf_test/base_fixture.hpp> | |
| `#include` <cudf_test/base_fixture.hpp> | |
| `#include` <cudf_test/cudf_gtest.hpp> | |
🤖 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 `@cpp/tests/io/experimental/hybrid_scan_multifile_filters_test.cpp` around
lines 8 - 9, Add the project gtest wrapper header to the test translation unit
by inserting `#include` <cudf_test/cudf_gtest.hpp> alongside the existing `#include`
<cudf_test/base_fixture.hpp> in hybrid_scan_multifile_filters_test.cpp so the
file uses the project wrapper instead of relying on raw gtest headers; ensure
the new include appears before any test definitions or fixtures (e.g.,
references to BaseFixture/Test macros) to follow the project's test include
convention.
Comment on lines
+23
to
+76
| namespace { | ||
|
|
||
| /** | ||
| * @brief Struct to hold multifile datasources, and footer buffers along with their byte spans | ||
| */ | ||
| struct multifile_inputs { | ||
| std::vector<std::unique_ptr<cudf::io::datasource>> datasources; | ||
| std::vector<std::unique_ptr<cudf::io::datasource::buffer>> footer_buffers; | ||
| std::vector<cudf::host_span<uint8_t const>> footer_byte_spans; | ||
| }; | ||
|
|
||
| template <typename Buffers> | ||
| multifile_inputs build_multifile_inputs(Buffers const& file_buffers) | ||
| { | ||
| multifile_inputs out; | ||
| out.datasources.reserve(file_buffers.size()); | ||
| out.footer_buffers.reserve(file_buffers.size()); | ||
| out.footer_byte_spans.reserve(file_buffers.size()); | ||
| for (auto const& buf : file_buffers) { | ||
| out.datasources.emplace_back(cudf::io::datasource::create(cudf::host_span<std::byte const>( | ||
| reinterpret_cast<std::byte const*>(buf.data()), buf.size()))); | ||
| out.footer_buffers.emplace_back( | ||
| cudf::io::parquet::fetch_footer_to_host(*out.datasources.back())); | ||
| out.footer_byte_spans.emplace_back(*out.footer_buffers.back()); | ||
| } | ||
| return out; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Creates a parquet buffer with zero-rows and same schema as table from | ||
| * `create_parquet_with_stats` | ||
| */ | ||
| template <typename T> | ||
| std::vector<char> create_empty_parquet_with_stats() | ||
| { | ||
| auto const non_empty = std::get<0>(create_parquet_with_stats<T, 1>()); | ||
| auto const empty = cudf::empty_like(non_empty->view()); | ||
|
|
||
| cudf::io::table_input_metadata output_metadata(empty->view()); | ||
| output_metadata.column_metadata[0].set_name("col0"); | ||
| output_metadata.column_metadata[1].set_name("col1"); | ||
| output_metadata.column_metadata[2].set_name("col2"); | ||
|
|
||
| std::vector<char> buffer; | ||
| auto out_opts = | ||
| cudf::io::parquet_writer_options::builder(cudf::io::sink_info{&buffer}, empty->view()) | ||
| .metadata(std::move(output_metadata)) | ||
| .stats_level(cudf::io::statistics_freq::STATISTICS_COLUMN) | ||
| .build(); | ||
| cudf::io::write_parquet(out_opts); | ||
| return buffer; | ||
| } | ||
|
|
||
| } // namespace |
There was a problem hiding this comment.
Avoid anonymous namespace in this test source.
Line 23 introduces an anonymous namespace for helpers. Move these declarations to the global namespace to align with the test-file namespace rule.
Suggested patch
-namespace {
-
/**
* `@brief` Struct to hold multifile datasources, and footer buffers along with their byte spans
*/
struct multifile_inputs {
@@
template <typename T>
std::vector<char> create_empty_parquet_with_stats()
@@
return buffer;
}
-
-} // namespaceAs per coding guidelines Test code must be in the global namespace, not in custom namespaces.
📝 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
| namespace { | |
| /** | |
| * @brief Struct to hold multifile datasources, and footer buffers along with their byte spans | |
| */ | |
| struct multifile_inputs { | |
| std::vector<std::unique_ptr<cudf::io::datasource>> datasources; | |
| std::vector<std::unique_ptr<cudf::io::datasource::buffer>> footer_buffers; | |
| std::vector<cudf::host_span<uint8_t const>> footer_byte_spans; | |
| }; | |
| template <typename Buffers> | |
| multifile_inputs build_multifile_inputs(Buffers const& file_buffers) | |
| { | |
| multifile_inputs out; | |
| out.datasources.reserve(file_buffers.size()); | |
| out.footer_buffers.reserve(file_buffers.size()); | |
| out.footer_byte_spans.reserve(file_buffers.size()); | |
| for (auto const& buf : file_buffers) { | |
| out.datasources.emplace_back(cudf::io::datasource::create(cudf::host_span<std::byte const>( | |
| reinterpret_cast<std::byte const*>(buf.data()), buf.size()))); | |
| out.footer_buffers.emplace_back( | |
| cudf::io::parquet::fetch_footer_to_host(*out.datasources.back())); | |
| out.footer_byte_spans.emplace_back(*out.footer_buffers.back()); | |
| } | |
| return out; | |
| } | |
| /** | |
| * @brief Creates a parquet buffer with zero-rows and same schema as table from | |
| * `create_parquet_with_stats` | |
| */ | |
| template <typename T> | |
| std::vector<char> create_empty_parquet_with_stats() | |
| { | |
| auto const non_empty = std::get<0>(create_parquet_with_stats<T, 1>()); | |
| auto const empty = cudf::empty_like(non_empty->view()); | |
| cudf::io::table_input_metadata output_metadata(empty->view()); | |
| output_metadata.column_metadata[0].set_name("col0"); | |
| output_metadata.column_metadata[1].set_name("col1"); | |
| output_metadata.column_metadata[2].set_name("col2"); | |
| std::vector<char> buffer; | |
| auto out_opts = | |
| cudf::io::parquet_writer_options::builder(cudf::io::sink_info{&buffer}, empty->view()) | |
| .metadata(std::move(output_metadata)) | |
| .stats_level(cudf::io::statistics_freq::STATISTICS_COLUMN) | |
| .build(); | |
| cudf::io::write_parquet(out_opts); | |
| return buffer; | |
| } | |
| } // namespace | |
| /** | |
| * `@brief` Struct to hold multifile datasources, and footer buffers along with their byte spans | |
| */ | |
| struct multifile_inputs { | |
| std::vector<std::unique_ptr<cudf::io::datasource>> datasources; | |
| std::vector<std::unique_ptr<cudf::io::datasource::buffer>> footer_buffers; | |
| std::vector<cudf::host_span<uint8_t const>> footer_byte_spans; | |
| }; | |
| template <typename Buffers> | |
| multifile_inputs build_multifile_inputs(Buffers const& file_buffers) | |
| { | |
| multifile_inputs out; | |
| out.datasources.reserve(file_buffers.size()); | |
| out.footer_buffers.reserve(file_buffers.size()); | |
| out.footer_byte_spans.reserve(file_buffers.size()); | |
| for (auto const& buf : file_buffers) { | |
| out.datasources.emplace_back(cudf::io::datasource::create(cudf::host_span<std::byte const>( | |
| reinterpret_cast<std::byte const*>(buf.data()), buf.size()))); | |
| out.footer_buffers.emplace_back( | |
| cudf::io::parquet::fetch_footer_to_host(*out.datasources.back())); | |
| out.footer_byte_spans.emplace_back(*out.footer_buffers.back()); | |
| } | |
| return out; | |
| } | |
| /** | |
| * `@brief` Creates a parquet buffer with zero-rows and same schema as table from | |
| * `create_parquet_with_stats` | |
| */ | |
| template <typename T> | |
| std::vector<char> create_empty_parquet_with_stats() | |
| { | |
| auto const non_empty = std::get<0>(create_parquet_with_stats<T, 1>()); | |
| auto const empty = cudf::empty_like(non_empty->view()); | |
| cudf::io::table_input_metadata output_metadata(empty->view()); | |
| output_metadata.column_metadata[0].set_name("col0"); | |
| output_metadata.column_metadata[1].set_name("col1"); | |
| output_metadata.column_metadata[2].set_name("col2"); | |
| std::vector<char> buffer; | |
| auto out_opts = | |
| cudf::io::parquet_writer_options::builder(cudf::io::sink_info{&buffer}, empty->view()) | |
| .metadata(std::move(output_metadata)) | |
| .stats_level(cudf::io::statistics_freq::STATISTICS_COLUMN) | |
| .build(); | |
| cudf::io::write_parquet(out_opts); | |
| return buffer; | |
| } |
🤖 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 `@cpp/tests/io/experimental/hybrid_scan_multifile_filters_test.cpp` around
lines 23 - 76, The helpers are defined inside an anonymous namespace which
violates the test-file namespace rule; move the struct multifile_inputs and the
functions build_multifile_inputs and create_empty_parquet_with_stats out of the
anonymous namespace into the global namespace (remove "namespace { ... }"
wrapper) so they live at global scope and keep their signatures and behavior
unchanged; ensure any forward-uses in this file still compile after the move and
remove the closing comment that references the anonymous namespace.
PointKernel
reviewed
May 21, 2026
PointKernel
left a comment
PointKernel
left a comment
Member
There was a problem hiding this comment.
Mainly style nits. One thing I noticed is that the API behavior has changed quite a bit from handling a single file/source to supporting multiple ones, while the API names remain the same. We probably want to either update the naming or introduce separate APIs for clarity instead of changing the existing behavior implicitly.
| * @param footer_bytes Host span of Parquet file footer byte spans, one per source | ||
| * @param options Parquet reader options | ||
| */ | ||
| explicit hybrid_scan_multifile(cudf::host_span<cudf::host_span<uint8_t const> const> footer_bytes, |
Member
There was a problem hiding this comment.
@mhaseeb123 can you please check #22560 to see if cudf::host_span is the best fit?
Contributor
Author
There was a problem hiding this comment.
Happy to use std::span here if it can be implicitly converted to host_span to pass on to the existing impl class. If not, I would lean towards keep using host_spans until hybrid_scan_multifile is complete (#22583) and then replace all occurrences across hybrid_scan** at once.
Contributor
Author
There was a problem hiding this comment.
Update: We also gotta make sure that cudf::io::datasource::buffer is also implicitly convertible to std::span
Contributor
Author
These APIs are in a separate new class (single source reader will become its subclass eventually) so same (or pluralized) API names kinda work here as context can be inferred from the reader's type. |
Co-authored-by: Yunsong Wang <12716979+PointKernel@users.noreply.github.com>
Co-authored-by: Yunsong Wang <12716979+PointKernel@users.noreply.github.com>
mhaseeb123
added
3 - Ready for Review
4 tasks
mhaseeb123
commented
May 29, 2026
| CUDF_EXPECTS(row_group_indices.size() == per_file_metadata.size(), | ||
| "Encountered unexpected number of input row group indices", | ||
| std::invalid_argument); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Literally no logical difference here, just using std::accumulate instead of for_each's and changed return type to size_t to allow more than 2B rows
lamarrr
requested changes
May 29, 2026
Contributor
Author
|
/merge |
mhaseeb123
added
5 - Ready to Merge
This was referenced Jun 2, 2026
This was referenced Jun 10, 2026
This was referenced Jun 22, 2026
4 tasks
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
4 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.
Description
Contributes to #22583
This PR adds barebones basic APIs for the
hybrid_scan_multifilereader.Note to reviewers: Please use the "Hide whitespace" feature when reviewing this PR as it will remove a lot of noise from simple indentation
Checklist