Skip to content

Conversation

@octoaide
Copy link
Contributor

@octoaide octoaide bot commented Dec 3, 2025

Closes #77

This pull request addresses issue #77 by centralizing the ThreatLevel enum definition and integrating it into the EventMessage structure. The goal is to enable Semi-supervised and Unsupervised engines to share a common, extended definition of threat levels, moving it from the review-web repository into review-protocol.

Summary of Changes:

  1. Added ThreatLevel enum: Defined ThreatLevel in src/types.rs with Low, Medium, High, and VeryHigh variants, aligning with the issue's requirements for extension and shared definition.
  2. Added triage_score to EventMessage: The EventMessage struct in src/types.rs now includes an Option<ThreatLevel> field named triage_score to capture the threat level of a detection event while maintaining backward compatibility.
  3. Added serialization test: A new test ensures that all ThreatLevel variants can be correctly serialized and deserialized using bincode.
  4. Updated CHANGELOG.md: Documented the addition of the ThreatLevel enum and the triage_score field.
  5. Updated test code: Modified existing test cases in src/client/api.rs and src/server.rs that create EventMessage instances to include the new triage_score field.

@octoaide octoaide bot requested a review from syncpark December 3, 2025 04:13
@codecov
Copy link

codecov bot commented Dec 3, 2025

Codecov Report

❌ Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.76%. Comparing base (a5c90af) to head (311eb4e).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/types.rs 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
+ Coverage   86.69%   86.76%   +0.07%     
==========================================
  Files          11       11              
  Lines        2254     2274      +20     
==========================================
+ Hits         1954     1973      +19     
- Misses        300      301       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

src/types.rs Outdated
#[serde(with = "jiff::fmt::serde::timestamp::nanosecond::required")]
pub time: jiff::Timestamp,
pub kind: EventKind,
pub triage_score: Option<ThreatLevel>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete this triage_score field as it is not needed.


### Added

- `ThreatLevel` enum to represent the threat level of detection events. The enum
Copy link
Contributor

Choose a reason for hiding this comment

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

It's only needed to add the ThreatLevel enum definition. There's no need to add the triage_score field to the EventMessage struct.

@octoaide octoaide bot force-pushed the octoaide/add-threat-level-and-triage-score branch from c4d5b63 to 9e63def Compare December 4, 2025 16:54
@octoaide
Copy link
Contributor Author

octoaide bot commented Dec 4, 2025

Hello syncpark! Thank you for your insightful feedback regarding the triage_score field and the ThreatLevel enum. I've taken a deep dive and made the necessary adjustments to this pull request.

Based on your clear guidance that the triage_score field was not needed, I've diligently removed it from the EventMessage struct, specifically at src/types.rs:287.

Following your suggestion to only include the ThreatLevel enum definition, I've also updated CHANGELOG.md. The changelog now accurately reflects only the ThreatLevel enum addition, with all previous mentions of triage_score removed.

To maintain code integrity and ensure all aspects of the change are covered, I've thoroughly updated all test files that referenced triage_score. This involved:

  • Adjusting the event_message_with_blocklist_radius_serialization test in src/types.rs to remove triage_score and its associated assertion.
  • Removing triage_score: None from six EventMessage instances within src/server.rs.
  • Cleaning up the test EventMessage instance in src/client/api.rs by removing triage_score: None.
    These updates ensure our tests are perfectly aligned with the revised EventMessage structure.

Before finalizing, I ran markdownlint on CHANGELOG.md, which reported no issues. Most importantly, I've verified that the build is stable and all 60 tests pass successfully with all features enabled!

All these changes have been committed and pushed to the branch. I'm confident this fully addresses your valuable feedback. Please take another look and let me know if you have any further suggestions!

@octoaide octoaide bot requested a review from syncpark December 4, 2025 16:55
@octoaide
Copy link
Contributor Author

octoaide bot commented Dec 8, 2025

I reviewed the feedback and implemented the requested change. I added VeryLow as the first variant of the ThreatLevel enum to restore the intended balance of the scale.

What I changed:

  • Added ThreatLevel::VeryLow as the first enum element (src/types.rs).
  • Updated the serialization round-trip test to include VeryLow so all variants serialize/deserialise correctly (src/types.rs).
  • Updated CHANGELOG.md to reflect five variants and include VeryLow in the list.

I ran the test suite and everything passes (60 tests passed). I also checked markdownlint for the changelog — no issues. All changes have been committed and pushed.

Thanks for the pointer — this should address the balance concern.

@octoaide octoaide bot requested a review from syncpark December 8, 2025 05:53
@msk msk self-assigned this Dec 8, 2025
@msk msk merged commit 9221e5a into main Dec 8, 2025
10 checks passed
@msk msk deleted the octoaide/add-threat-level-and-triage-score branch December 8, 2025 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ThreatLevel enum definition

3 participants