feat(clp-s::filter): Add interfaces and definitions for the top-level… - #2381
Open
ShangDanLuXian wants to merge 3 commits into
Open
feat(clp-s::filter): Add interfaces and definitions for the top-level…#2381ShangDanLuXian wants to merge 3 commits into
ShangDanLuXian wants to merge 3 commits into
Conversation
Contributor
WalkthroughChangesPacked Filter index foundations
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
… index framework.
ShangDanLuXian
force-pushed
the
top_level_indexing
branch
from
July 15, 2026 20:00
90aa625 to
89adc4e
Compare
ShangDanLuXian
marked this pull request as ready for review
July 20, 2026 12:51
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@components/core/src/clp_s/filter/PackedFilterSpecification.hpp`:
- Line 6: Change intra-target includes to quoted relative headers: in
components/core/src/clp_s/filter/PackedFilterSpecification.hpp lines 6-6 use
"IndexDefs.hpp"; in
components/core/src/clp_s/filter/IndexBuilderSpecification.hpp lines 10-12 use
"IndexBuilder.hpp", "IndexDefs.hpp", and "PackedFilterSpecification.hpp"; and in
components/core/src/clp_s/filter/IndexRunner.hpp lines 9-9 use "BitmapView.hpp".
Keep angle brackets only for external-target dependencies.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 6b3be727-c21c-4f2f-b732-dffb015a37de
📒 Files selected for processing (9)
components/core/cmake/Options/options.cmakecomponents/core/src/clp_s/CMakeLists.txtcomponents/core/src/clp_s/filter/CMakeLists.txtcomponents/core/src/clp_s/filter/IndexBuilder.hppcomponents/core/src/clp_s/filter/IndexBuilderSpecification.hppcomponents/core/src/clp_s/filter/IndexDefs.hppcomponents/core/src/clp_s/filter/IndexRunner.hppcomponents/core/src/clp_s/filter/PackedFilterSpecification.hppcomponents/core/src/clp_s/filter/tests/test-clp_s-index_defs.cpp
|
|
||
| #include <cstddef> | ||
|
|
||
| #include <clp_s/filter/IndexDefs.hpp> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Use quoted relative includes for headers within the same CMake target.
Based on learnings, headers within components/core/src/clp_s/** should use quoted relative includes (e.g., "IndexDefs.hpp") when including other headers from the same CMake target, and reserve angle-bracket includes for external targets. Since these files and their dependencies belong to the clp_s_filter target, they should use relative quotes for these intra-target dependencies.
components/core/src/clp_s/filter/PackedFilterSpecification.hpp#L6-L6: replace<clp_s/filter/IndexDefs.hpp>with"IndexDefs.hpp".components/core/src/clp_s/filter/IndexBuilderSpecification.hpp#L10-L12: replace the angle-bracket includes with"IndexBuilder.hpp","IndexDefs.hpp", and"PackedFilterSpecification.hpp".components/core/src/clp_s/filter/IndexRunner.hpp#L9-L9: replace<clp_s/filter/BitmapView.hpp>with"BitmapView.hpp".
📍 Affects 3 files
components/core/src/clp_s/filter/PackedFilterSpecification.hpp#L6-L6(this comment)components/core/src/clp_s/filter/IndexBuilderSpecification.hpp#L10-L12components/core/src/clp_s/filter/IndexRunner.hpp#L9-L9
🤖 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 `@components/core/src/clp_s/filter/PackedFilterSpecification.hpp` at line 6,
Change intra-target includes to quoted relative headers: in
components/core/src/clp_s/filter/PackedFilterSpecification.hpp lines 6-6 use
"IndexDefs.hpp"; in
components/core/src/clp_s/filter/IndexBuilderSpecification.hpp lines 10-12 use
"IndexBuilder.hpp", "IndexDefs.hpp", and "PackedFilterSpecification.hpp"; and in
components/core/src/clp_s/filter/IndexRunner.hpp lines 9-9 use "BitmapView.hpp".
Keep angle brackets only for external-target dependencies.
Source: Learnings
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.
Description
This PR adds the core definitions and interfaces for a top-level indexing framework for clp-s.
The framework will allow indexes to be built over groups of archives and packed into a single
"Packed Filter" object, which can then be used at search time to prune archives that cannot
contain any results matching a query. This is the first PR in a series; it contains only
headers (interfaces and definitions) and a unit test, with no behavior changes.
Specifically, this PR adds:
IndexDefs.hpp: Core types for the framework:index_id_t, a 16-bit index-type identifier whose ID space is partitioned into reservedranges (official open-source, official closed-source, and custom indices).
index_version_tandarchive_version_t, 32-bit semantic versions encoded the same way asclp-s archive versions, with
constexprhelpers to encode and decompose them.ArchiveSectionandarchive_section_bitmap_t, a flag set describing which sections of anarchive an index needs to read while building.
IndexBuilder.hpp: The interface for building an index. The framework feeds an implementationone archive at a time, and the implementation produces one self-delimiting serialized blob per
archive.
IndexRunner.hpp: The interface for using a deserialized index at filtering time. A runnernarrows the set of candidate archives for a query by clearing bits in a
CandidateArchiveBitmapView(built on theBitmapViewclass introduced in feat(clp-s::filter): AddBitmapViewclass for bitmaps passed across FFI boundaries. #2337).IndexBuilderSpecification.hpp: Describes a registeredIndexBuilderimplementation: thearchive sections it reads, the half-open range of archive versions it supports, its index
version, and a factory for creating instances of it.
PackedFilterSpecification.hpp: A framework-provided description of the Packed Filter beingbuilt (archive count and the shared archive version).
Follow-up PRs will add the index registry, the Packed Filter on-disk format (writer/reader), and
a first concrete index implementation (a Bloom filter over each archive's variable dictionary).
Checklist
breaking change.
Validation performed
test-clp_s-index_defs.cpp); all 3 test cases (15 assertions) pass. Thetest covers index-version encode/decode round-tripping, Index ID range classification
(including range boundaries), and
ArchiveSectionbitmap composition and membership checks.Summary by CodeRabbit
New Features
Tests