Repository navigation
Ranking system - #125
Ranking system#125
Conversation
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughRanking System ImplementationThis PR adds a configurable ranking subsystem to improve semantic classification accuracy and prioritization of extracted strings. Architectural Impact
Binary Format Handling
Semantic Classification Accuracy
API Changes
Testing & Backwards Compatibility
Summary of Risk / Review Focus
WalkthroughAdds a new ranking subsystem (RankingConfig, RankingEngine) that scores and sorts FoundString entries using section weights, tag boosts, and a confidence-based noise penalty; also derives Hash for Changes
Sequence Diagram(s)sequenceDiagram
participant Engine as RankingEngine
participant Config as RankingConfig
participant SI as SectionInfo
participant FS as FoundString
Engine->>Config: read section_weights, tag_boosts, noise_penalty_multiplier
Engine->>FS: request tags, confidence
alt section_info present
Engine->>SI: read section type and weight
SI-->>Engine: section_type, weight
else no section_info
Config-->>Engine: default section weight (20)
end
Engine->>Engine: compute section_weight_score
Engine->>Engine: sum semantic_boost_score from tag_boosts
Engine->>Engine: compute noise_penalty_score from confidence
Engine->>FS: set final score (section + semantic - noise)
alt debug_mode
Engine->>FS: attach debug fields (section_weight, semantic_boost, noise_penalty)
end
Engine->>Engine: rank_strings sorts FoundString slice (stable, descending)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This pull request introduces a comprehensive ranking system for prioritizing and scoring strings extracted from binaries. The system combines section-based weighting, semantic tag boosting, and noise penalty calculations to produce meaningful relevance scores.
Changes:
- Added new
rankingmodule with configurable scoring engine that combines section weights, semantic tag boosts, and confidence-based noise penalties - Derived
Hashtrait forTagandSectionTypeenums to enable their use as HashMap keys in the ranking configuration - Exported
RankingConfigandRankingEnginefrom the classification module for external use
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/types.rs | Added Hash trait derivation to Tag and SectionType enums to support their use as HashMap keys in ranking configuration |
| src/classification/ranking.rs | New module implementing the complete ranking system with configurable weights, comprehensive scoring algorithm, and extensive unit tests |
| src/classification/mod.rs | Added public module declaration and re-exports for RankingConfig and RankingEngine |
| pub fn calculate_score( | ||
| &self, | ||
| found_string: &mut FoundString, | ||
| section_info: Option<&SectionInfo>, | ||
| ) { |
There was a problem hiding this comment.
The calculate_score method lacks documentation for its parameters and behavior. Add documentation explaining:
- What
found_stringparameter represents and that it will be mutated with score and optional debug fields - What
section_infoparameter represents and the behavior when it's None (uses default weight of 20) - The scoring formula: section_weight_score + semantic_boost_score - noise_penalty_score
- When debug fields are populated (only when debug_mode is true)
| use std::collections::HashMap; | ||
|
|
||
| use crate::types::{FoundString, SectionInfo, SectionType, Tag}; |
There was a problem hiding this comment.
The ranking module is missing module-level documentation. According to the project's coding guidelines, modules should have documentation explaining their purpose, usage, and key concepts. Consider adding a doc comment at the top of the file that describes:
- The purpose of the ranking system
- How the scoring algorithm works (section weights + semantic boosts - noise penalties)
- Basic usage example with RankingEngine
- The relationship between RankingConfig and RankingEngine
| #[non_exhaustive] | ||
| pub struct RankingConfig { | ||
| pub section_weights: HashMap<SectionType, i32>, | ||
| pub tag_boosts: HashMap<Tag, i32>, |
There was a problem hiding this comment.
The RankingConfig struct is missing type-level documentation. Public API types should include documentation explaining:
- What the struct configures
- The meaning and expected ranges of each field (e.g., what do section_weights values represent?)
- How the configuration affects scoring
- Usage examples or references to RankingEngine
Additionally, the public fields lack individual documentation comments explaining their purpose and valid value ranges.
| #[non_exhaustive] | |
| pub struct RankingConfig { | |
| pub section_weights: HashMap<SectionType, i32>, | |
| pub tag_boosts: HashMap<Tag, i32>, | |
| #[non_exhaustive] | |
| /// Configuration for the `RankingEngine` scoring algorithm. | |
| /// | |
| /// This struct controls how different container sections and semantic tags | |
| /// contribute to the final score of a `FoundString`, and how aggressively | |
| /// noisy or low-value strings are penalized. | |
| /// | |
| /// All integer values in this configuration are treated as relative weights | |
| /// or penalties in the scoring formula: higher values increase the influence | |
| /// of a given factor, while lower or negative values reduce it. Typical | |
| /// values are in the range `0..=200`, but any `i32` is accepted so callers | |
| /// can tune ranking behavior for their use case. | |
| /// | |
| /// The default configuration used by `RankingEngine` can be obtained via | |
| /// [`RankingConfig::new`] or [`RankingConfig::default`]. | |
| pub struct RankingConfig { | |
| /// Per-section weighting applied when scoring a `FoundString` based on | |
| /// the `SectionType` it was extracted from. | |
| /// | |
| /// Higher values cause strings from the corresponding section to rank | |
| /// higher relative to other sections. Typical values are in the range | |
| /// `0..=200`. Negative values are allowed to strongly de-prioritize | |
| /// specific section types if desired. | |
| pub section_weights: HashMap<SectionType, i32>, | |
| /// Per-tag scoring boosts applied when a `FoundString` has a particular | |
| /// semantic `Tag`. | |
| /// | |
| /// The value is added as a relative bonus to the base score of the | |
| /// string. Higher values make strings with that tag more prominent in | |
| /// ranked output. Typical values are in the range `0..=200`. Negative | |
| /// values can be used to penalize certain tags. | |
| pub tag_boosts: HashMap<Tag, i32>, | |
| /// Global multiplier applied to noise-related penalties in the scoring | |
| /// algorithm. | |
| /// | |
| /// Larger values increase the impact of noise detection (noisy strings | |
| /// lose more score), while smaller values reduce it. A value of `0` | |
| /// effectively disables noise penalties. Typical values are in the range | |
| /// `0..=200`. |
| #[non_exhaustive] | ||
| pub struct RankingEngine { | ||
| config: RankingConfig, | ||
| debug_mode: bool, | ||
| } | ||
|
|
||
| impl RankingEngine { | ||
| #[must_use] | ||
| pub fn new(debug_mode: bool) -> Self { | ||
| Self::with_config(RankingConfig::default(), debug_mode) | ||
| } | ||
|
|
||
| #[must_use] | ||
| pub fn with_config(config: RankingConfig, debug_mode: bool) -> Self { | ||
| Self { config, debug_mode } | ||
| } | ||
|
|
There was a problem hiding this comment.
The RankingEngine struct is missing type-level documentation. Public API types should include documentation explaining:
- What the engine does (ranks and scores strings)
- The difference between debug_mode and non-debug_mode
- Basic usage workflow (construct engine, call calculate_score for each string, then rank_strings)
- The relationship with RankingConfig
Additionally, the public methods lack documentation for their parameters and behavior.
| #[non_exhaustive] | |
| pub struct RankingEngine { | |
| config: RankingConfig, | |
| debug_mode: bool, | |
| } | |
| impl RankingEngine { | |
| #[must_use] | |
| pub fn new(debug_mode: bool) -> Self { | |
| Self::with_config(RankingConfig::default(), debug_mode) | |
| } | |
| #[must_use] | |
| pub fn with_config(config: RankingConfig, debug_mode: bool) -> Self { | |
| Self { config, debug_mode } | |
| } | |
| #[non_exhaustive] | |
| /// Engine for scoring and ranking `FoundString` values. | |
| /// | |
| /// `RankingEngine` applies a configurable scoring model to each `FoundString` | |
| /// and then orders strings by their final score. The scoring model combines: | |
| /// | |
| /// - Section weights from `RankingConfig`, based on the `SectionType` and | |
| /// per-section `SectionInfo::weight`. | |
| /// - Semantic tag boosts from `RankingConfig::tag_boosts`. | |
| /// - A noise penalty derived from `FoundString::confidence` and | |
| /// `RankingConfig::noise_penalty_multiplier`. | |
| /// | |
| /// The `debug_mode` flag controls whether per-component scoring details are | |
| /// written back to `FoundString` after `calculate_score` is called. When | |
| /// `debug_mode` is `true`, the engine populates fields such as | |
| /// `section_weight`, `semantic_boost`, and `noise_penalty` to make the | |
| /// scoring breakdown visible to callers. When `false`, only the aggregate | |
| /// `score` field is updated. | |
| /// | |
| /// Typical usage: | |
| /// 1. Optionally construct a `RankingConfig` with custom weights and boosts. | |
| /// 2. Construct a `RankingEngine` with `new` or `with_config`, choosing | |
| /// whether `debug_mode` should be enabled. | |
| /// 3. For each `FoundString`, call `calculate_score`, passing the string and | |
| /// an optional `SectionInfo` describing its origin. | |
| /// 4. Once all scores are computed, call `rank_strings` on a collection of | |
| /// `FoundString` values to sort them by score in descending order. | |
| pub struct RankingEngine { | |
| config: RankingConfig, | |
| debug_mode: bool, | |
| } | |
| impl RankingEngine { | |
| /// Creates a new `RankingEngine` using the default `RankingConfig`. | |
| /// | |
| /// The `debug_mode` flag controls whether `calculate_score` populates | |
| /// detailed scoring components (`section_weight`, `semantic_boost`, | |
| /// `noise_penalty`) on each `FoundString`. When `debug_mode` is `false`, | |
| /// only the aggregate `score` field is updated. | |
| #[must_use] | |
| pub fn new(debug_mode: bool) -> Self { | |
| Self::with_config(RankingConfig::default(), debug_mode) | |
| } | |
| /// Creates a new `RankingEngine` with an explicit `RankingConfig`. | |
| /// | |
| /// - `config` provides section weights, tag boosts, and the noise penalty | |
| /// multiplier that control how scores are computed. | |
| /// - `debug_mode` controls whether per-component scoring details are written | |
| /// back to each `FoundString` when `calculate_score` is called. | |
| #[must_use] | |
| pub fn with_config(config: RankingConfig, debug_mode: bool) -> Self { | |
| Self { config, debug_mode } | |
| } | |
| /// Computes the score for a single `FoundString`. | |
| /// | |
| /// This method uses the current `RankingConfig` and optional `SectionInfo` | |
| /// to derive a final integer score: | |
| /// | |
| /// - Starts from a base section weight derived from `section_info` and the | |
| /// configured `section_weights`, or a default when no match is found. | |
| /// - Applies the dynamic `SectionInfo::weight` factor if provided. | |
| /// - Adds semantic tag boosts for each tag present on the string. | |
| /// - Subtracts a noise penalty based on `FoundString::confidence`. | |
| /// | |
| /// The resulting value is written to `found_string.score`. If the engine | |
| /// was constructed with `debug_mode == true`, the method also populates | |
| /// `found_string.section_weight`, `found_string.semantic_boost`, and | |
| /// `found_string.noise_penalty` with the intermediate values used to | |
| /// produce the final score. | |
| /// | |
| /// - `found_string` - The string record to score; it is modified in place. | |
| /// - `section_info` - Optional metadata about the section from which the | |
| /// string was extracted. When `None`, a default base section weight is | |
| /// used. |
|
@copilot open a new pull request to apply changes based on the comments in this thread Run |
|
@unclesp1d3r I've opened a new pull request, #126, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/classification/ranking.rs`:
- Around line 113-120: The doc comment for rank_strings incorrectly states it
performs an "unstable sort" while the implementation uses strings.sort_by(...)
which is a stable sort; either change the call to strings.sort_unstable_by(|a,
b| b.score.cmp(&a.score)) to match the "unstable" wording, or update the doc
comment to say "stable sort" (or remove the unstable mention) if stability is
desired—edit the rank_strings function and its doc comment accordingly to make
implementation and documentation consistent.
* Initial plan * docs: add comprehensive documentation to ranking module Co-authored-by: unclesp1d3r <251112+unclesp1d3r@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: unclesp1d3r <251112+unclesp1d3r@users.noreply.github.com>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/classification/ranking.rs`:
- Around line 258-260: The noise penalty calculation currently truncates to i32
(noise_penalty_score = penalty_f32 as i32) causing inconsistent rounding versus
the section weight conversion which uses .round(); change the conversion of
penalty_f32 to use rounding instead of truncation so it matches the section
weight behavior (i.e., compute noise_penalty_score by rounding penalty_f32 to
the nearest integer); locate the code that computes penalty_f32 from
found_string.confidence and self.config.noise_penalty_multiplier and replace the
direct cast with a rounded-to-i32 conversion to keep consistency.
♻️ Duplicate comments (1)
src/classification/ranking.rs (1)
272-279: Doc/implementation mismatch:sort_byis stable, not unstable.The documentation claims "unstable sort" but
sort_bypreserves relative order of equal elements. Either switch tosort_unstable_byfor the slight performance benefit, or correct the documentation.Option 1: Use unstable sort (matches docs)
pub fn rank_strings(&self, strings: &mut [FoundString]) { - strings.sort_by(|a, b| b.score.cmp(&a.score)); + strings.sort_unstable_by(|a, b| b.score.cmp(&a.score)); }Option 2: Fix documentation (if stability matters)
/// Call `calculate_score` for each `FoundString` before invoking this method, - /// otherwise ordering will reflect uninitialized or stale scores. This is an - /// unstable sort, so the relative order of equal scores is not preserved. + /// otherwise ordering will reflect uninitialized or stale scores. This is a + /// stable sort, so the relative order of equal scores is preserved. pub fn rank_strings(&self, strings: &mut [FoundString]) {
| let confidence = found_string.confidence.clamp(0.0, 1.0); | ||
| let penalty_f32 = (1.0 - confidence) * self.config.noise_penalty_multiplier as f32; | ||
| let noise_penalty_score = penalty_f32 as i32; |
There was a problem hiding this comment.
Inconsistent rounding behavior between section weight and noise penalty calculations.
Line 249 uses .round() for the section weight conversion, but line 260 uses direct truncation (as i32). This creates subtle inconsistencies - for example, 0.9999 would round to 1 but truncate to 0.
Proposed fix for consistent rounding
let confidence = found_string.confidence.clamp(0.0, 1.0);
let penalty_f32 = (1.0 - confidence) * self.config.noise_penalty_multiplier as f32;
- let noise_penalty_score = penalty_f32 as i32;
+ let noise_penalty_score = penalty_f32.round() as i32;Note: This will change the expected values in test_score_calculation_file_path_code_section (line 387) from 39 to 40, which is arguably the more intuitive result for a 0.4 penalty factor.
📝 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.
| let confidence = found_string.confidence.clamp(0.0, 1.0); | |
| let penalty_f32 = (1.0 - confidence) * self.config.noise_penalty_multiplier as f32; | |
| let noise_penalty_score = penalty_f32 as i32; | |
| let confidence = found_string.confidence.clamp(0.0, 1.0); | |
| let penalty_f32 = (1.0 - confidence) * self.config.noise_penalty_multiplier as f32; | |
| let noise_penalty_score = penalty_f32.round() as i32; |
🤖 Prompt for AI Agents
In `@src/classification/ranking.rs` around lines 258 - 260, The noise penalty
calculation currently truncates to i32 (noise_penalty_score = penalty_f32 as
i32) causing inconsistent rounding versus the section weight conversion which
uses .round(); change the conversion of penalty_f32 to use rounding instead of
truncation so it matches the section weight behavior (i.e., compute
noise_penalty_score by rounding penalty_f32 to the nearest integer); locate the
code that computes penalty_f32 from found_string.confidence and
self.config.noise_penalty_multiplier and replace the direct cast with a
rounded-to-i32 conversion to keep consistency.
* feat(ranking): add ranking configuration and engine Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io> * feat(ranking): mark RankingConfig and RankingEngine as non_exhaustive Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io> * Add comprehensive documentation to ranking module (#126) * Initial plan * docs: add comprehensive documentation to ranking module Co-authored-by: unclesp1d3r <251112+unclesp1d3r@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: unclesp1d3r <251112+unclesp1d3r@users.noreply.github.com> * fix(ranking): change rank_strings to use stable sort Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io> --------- Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
This pull request introduces a new ranking system for classifying and prioritizing found strings in binaries. The main addition is the
rankingmodule, which provides configurable scoring and ranking logic based on section types, semantic tags, and string confidence. It also makes necessary changes to support the new ranking logic, including trait derivations and public exports.New ranking system and scoring logic:
rankingmodule withRankingConfigandRankingEnginetypes, allowing configurable scoring of found strings based on section weights, tag boosts, and noise penalties. This includes methods for score calculation and sorting, as well as comprehensive unit tests.RankingConfigandRankingEnginefrom the crate root for external use.Support for new ranking system:
Hashtrait for theTagandSectionTypeenums intypes.rsto allow their use as keys in hash maps for scoring configuration. [1] [2]