v3.5.0 - Fix CI bugs, dev refactoring, and issue fixes #229
Conversation
…228 Signed-off-by: Shahm Najeeb <[email protected]>
…ly the model and vectorizer Find the port [here](https://github.com/DefinetlyNotAI/Logicytics_VulnScan) Signed-off-by: Shahm Najeeb <[email protected]>
…ructions Signed-off-by: Shahm Najeeb <[email protected]>
…entence-transformers Signed-off-by: Shahm Najeeb <[email protected]>
Signed-off-by: Shahm Najeeb <[email protected]>
Also tested logicytics in default mode Also updated security.md Minor fix with a bug in dump_memory.py where a `general memory dump error: binary mode doesn't take an encoding argument` occured Refactored the dev questions Signed-off-by: Shahm Najeeb <[email protected]>
Signed-off-by: Shahm Najeeb <[email protected]>
DefinetlyNotAI
added
type/Documentation
DefinetlyNotAI
added
request/Normal
WalkthroughThis update removes the VulnScan training, data generation, and visualization tools, along with related documentation and dependencies. Several workflow files are switched to Windows runners, and the CodeQL workflow is deleted. Minor code tweaks adjust prompts, fix file handling, and update config paths and labels. Dependency and documentation files are streamlined to reflect these changes. Changes
Sequence Diagram(s)No sequence diagram generated as changes are mostly removals, config tweaks, and minor code adjustments without new or altered control flow. Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This pull request upgrades Logicytics to version 3.5.0, focusing on CI infrastructure improvements, code refactoring, and bug fixes. The update transitions CI workflows from Ubuntu to Windows and includes significant repository reorganization by moving VulnScan tools to a separate repository.
- CI workflow migration from Ubuntu to Windows for better compatibility
- Code refactoring in dev tools, flag handling, and file management
- Bug fixes including binary mode encoding error and removed unimplemented features
Reviewed Changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| SECURITY.md | Updated supported versions table with v3.5.x and deprecation of older versions |
| README.md | Updated installation instructions and changed CodeClimate badge to Qlty badge |
| PLANS.md | Removed completed tasks from project roadmap |
| CODE/vulnscan.py | Fixed import order and updated VulnScan model path reference |
| CODE/packet_sniffer.py | Renamed exception variable for consistency |
| CODE/logicytics/Flag.py | Removed unimplemented flags, refactored type hints, and cleaned up imports |
| CODE/logicytics/FileManagement.py | Added missing return statements to methods |
| CODE/dump_memory.py | Fixed binary file opening by removing conflicting encoding parameter |
| CODE/config.ini | Updated version to 3.5.0 and file paths for VulnScan components |
| CODE/_dev.py | Simplified dev question prompts by removing prefixes |
| .github/workflows/* | Migrated all CI workflows from ubuntu-latest to windows-latest |
| .github/config.yml | Updated issue label format |
| Multiple VulnScan files | Removed VulnScan training and tool files as part of repository separation |
Files not reviewed (1)
- .idea/csv-editor.xml: Language not supported
3 new issues
|
Signed-off-by: Shahm Najeeb <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
SECURITY.md (1)
7-16: Release-date duplicates look suspiciousBoth 3.4.x and 3.3.x share the exact “January 3 2025” major-release date.
If that isn’t intentional, tweak the date so the support table doesn’t confuse readers (or bots that parse it).README.md (1)
40-46: Tiny wording + spelling tweak for the PyTorch note
- “Nvidea” → “Nvidia”
- The bullet about installer settings reads a bit like a CLI command; rephrase for clarity.
- If you have a supported GPU, it is recommended to install the Nvidea GPU version of PyTorch for better performance. + If you’ve got an Nvidia GPU, grab the CUDA-enabled build of PyTorch for a nice speed boost. - Settings should be: `Stable -> Windows -> Pip -> Python` and if you have a supported CUDA version, select that too - else CPU. + On the PyTorch site pick: **Stable ▸ Windows ▸ Pip ▸ Python**. + Choose your matching CUDA toolkit (or “CPU-only” if you’re GPU-less)..github/config.yml (1)
18-18: Update any docs that still referenceprogress:InvalidThe bot now adds
issue/Invalid; hunt for leftover mentions (workflow filters, docs, labels) to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
.github/config.yml(1 hunks).github/workflows/codeql.yml(0 hunks).github/workflows/dependency-review.yml(1 hunks).github/workflows/greetings.yml(1 hunks).github/workflows/scorecard.yml(1 hunks).github/workflows/stale.yml(1 hunks).idea/csv-editor.xml(1 hunks)CODE/Logicytics.py(0 hunks)CODE/VulnScan/README.md(0 hunks)CODE/VulnScan/tools/_study_network.py(0 hunks)CODE/VulnScan/tools/_test_gpu_acceleration.py(0 hunks)CODE/VulnScan/tools/_vectorizer.py(0 hunks)CODE/VulnScan/v3/_generate_data.py(0 hunks)CODE/VulnScan/v3/_train.py(0 hunks)CODE/_dev.py(2 hunks)CODE/config.ini(1 hunks)CODE/dump_memory.py(1 hunks)CODE/logicytics/FileManagement.py(2 hunks)CODE/logicytics/Flag.py(2 hunks)CODE/packet_sniffer.py(1 hunks)CODE/vulnscan.py(2 hunks)PLANS.md(0 hunks)README.md(2 hunks)SECURITY.md(1 hunks)requirements.txt(1 hunks)
💤 Files with no reviewable changes (9)
- CODE/Logicytics.py
- PLANS.md
- .github/workflows/codeql.yml
- CODE/VulnScan/tools/_test_gpu_acceleration.py
- CODE/VulnScan/v3/_generate_data.py
- CODE/VulnScan/README.md
- CODE/VulnScan/tools/_vectorizer.py
- CODE/VulnScan/v3/_train.py
- CODE/VulnScan/tools/_study_network.py
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: DefinetlyNotAI
PR: DefinetlyNotAI/Logicytics#158
File: CODE/VulnScan/v3/_generate_data.py:177-197
Timestamp: 2024-12-11T10:05:34.954Z
Learning: The `_generate_data.py` function in `CODE/VulnScan/v3/_generate_data.py` will be deprecated due to memory issues and won't be fixed in the current PR.
Learnt from: DefinetlyNotAI
PR: DefinetlyNotAI/Logicytics#158
File: CODE/VulnScan/v3/_generate_data.py:0-0
Timestamp: 2024-12-11T10:30:48.699Z
Learning: In `CODE/VulnScan/v3/_generate_data.py`, the deprecation due to memory issues will occur only when the type is `Model SenseMacro`, and it won't be fixed in the current PR.
Learnt from: DefinetlyNotAI
PR: DefinetlyNotAI/Logicytics#158
File: CODE/VulnScan/v2-deprecated/_generate_data.py:106-109
Timestamp: 2024-12-11T09:55:36.823Z
Learning: The script `CODE/VulnScan/v2-deprecated/_generate_data.py` is designed to generate files and should not delete the generated files after generation.
Learnt from: DefinetlyNotAI
PR: DefinetlyNotAI/Logicytics#158
File: CODE/VulnScan/v3/_train.py:393-394
Timestamp: 2024-12-11T10:44:02.233Z
Learning: In `CODE/VulnScan/v3/_train.py`, path existence checks are performed in the `validate_data()` function.
Learnt from: DefinetlyNotAI
PR: DefinetlyNotAI/Logicytics#225
File: CODE/logicytics/Flag.py:583-592
Timestamp: 2025-05-31T12:42:30.615Z
Learning: In the Logicytics codebase, DefinetlyNotAI prefers using exit() over sys.exit() and doesn't want suggestions to change this pattern.
Learnt from: DefinetlyNotAI
PR: DefinetlyNotAI/Logicytics#225
File: CODE/logicytics/Flag.py:583-592
Timestamp: 2025-05-31T12:42:30.615Z
Learning: In the Logicytics codebase, DefinetlyNotAI prefers to allow lines longer than 100 characters when they make sense contextually, rather than enforcing strict line length limits.
CODE/config.ini (2)
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/VulnScan/v3/_generate_data.py:172-175
Timestamp: 2024-12-11T10:52:39.417Z
Learning: In CODE/VulnScan/v3/_generate_data.py, the config section 'VulnScan.generate Settings' always exists, and any installation issues are handled elsewhere. No need to add error handling for missing config sections in this file.
Learnt from: ski-sketch
PR: #160
File: CODE/bluetooth_details.py:5-6
Timestamp: 2024-12-12T08:36:07.659Z
Learning: In the Logicytics project, the logger must be initialized under if __name__ == "__main__": in bluetooth_details.py, and should not be moved into functions or accept parameters.
CODE/dump_memory.py (5)
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/dump_memory.py:183-203
Timestamp: 2024-12-11T10:53:07.774Z
Learning: In dump_memory.py, when writing to Ram_Dump.txt, the data is saved in HEX format. Adding an encoding parameter can corrupt the file. Leave out the encoding when opening Ram_Dump.txt.
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/dump_memory.py:161-177
Timestamp: 2024-12-11T10:53:59.261Z
Learning: In dump_memory.py, the SystemRam_Info.txt file is saved with the proper encoding by default, so specifying encoding="utf-8" is not necessary.
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/dump_memory.py:93-152
Timestamp: 2024-12-11T10:46:20.128Z
Learning: Avoid refactoring the process handle in read_memory() function in CODE/dump_memory.py as it may break the code.
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/dump_memory.py:87-91
Timestamp: 2024-12-11T10:45:11.595Z
Learning: In CODE/dump_memory.py, for functions like get_system_info(), prefer to handle exceptions and logging manually, rather than adding additional error handling as suggested.
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/dump_memory.py:93-152
Timestamp: 2024-12-11T09:54:12.940Z
Learning: In the 'Logicytics' project, scripts like dump_memory.py are not supposed to consider legality concerns during code reviews.
README.md (2)
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/vulnscan.py:48-48
Timestamp: 2024-12-11T10:49:03.037Z
Learning: In CODE/vulnscan.py, when using PyTorch 2.5.1, it's appropriate to include the weights_only=False parameter in torch.load() to prevent deprecation warnings during the False to True migration.
Learnt from: DefinetlyNotAI
PR: #225
File: CODE/logicytics/Flag.py:583-592
Timestamp: 2025-05-31T12:42:30.615Z
Learning: In the Logicytics codebase, DefinetlyNotAI prefers to allow lines longer than 100 characters when they make sense contextually, rather than enforcing strict line length limits.
CODE/vulnscan.py (8)
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/VulnScan/v3/_train.py:393-394
Timestamp: 2024-12-11T10:44:02.233Z
Learning: In CODE/VulnScan/v3/_train.py, path existence checks are performed in the validate_data() function.
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/VulnScan/v3/_generate_data.py:45-69
Timestamp: 2024-12-11T10:01:14.449Z
Learning: In CODE/VulnScan/v3/_generate_data.py, the script is not supposed to handle security concerns.
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/VulnScan/v3/_generate_data.py:177-197
Timestamp: 2024-12-11T10:05:34.954Z
Learning: The _generate_data.py function in CODE/VulnScan/v3/_generate_data.py will be deprecated due to memory issues and won't be fixed in the current PR.
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/VulnScan/v3/_generate_data.py:0-0
Timestamp: 2024-12-11T10:30:48.699Z
Learning: In CODE/VulnScan/v3/_generate_data.py, the deprecation due to memory issues will occur only when the type is Model SenseMacro, and it won't be fixed in the current PR.
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/VulnScan/v2-deprecated/_generate_data.py:12-20
Timestamp: 2024-12-11T09:55:02.216Z
Learning: In CODE/VulnScan/v2-deprecated/_generate_data.py, the create_sensitive_file function doesn't need to include warnings about legality or sensitive data in the docstring.
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/VulnScan/v3/_generate_data.py:172-175
Timestamp: 2024-12-11T10:52:39.417Z
Learning: In CODE/VulnScan/v3/_generate_data.py, the config section 'VulnScan.generate Settings' always exists, and any installation issues are handled elsewhere. No need to add error handling for missing config sections in this file.
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/VulnScan/v3/_generate_data.py:45-69
Timestamp: 2024-12-11T10:01:31.708Z
Learning: In CODE/VulnScan/v3/_generate_data.py, it's intentional to generate realistic sensitive data, and security considerations for data generation aren't a concern.
Learnt from: DefinetlyNotAI
PR: #158
File: CODE/VulnScan/v2-deprecated/_generate_data.py:106-109
Timestamp: 2024-12-11T09:55:36.823Z
Learning: The script CODE/VulnScan/v2-deprecated/_generate_data.py is designed to generate files and should not delete the generated files after generation.
🧬 Code Graph Analysis (1)
CODE/packet_sniffer.py (1)
CODE/logicytics/Logger.py (1)
warning(253-261)
🪛 GitHub Actions: Automatic Dependency Submission (Python)
requirements.txt
[error] 1-1: Python dependency resolution failed: No matching distribution found for pywin32 required by wmi. This caused pip-compile --dry-run to fail with exit code 1.
🔇 Additional comments (15)
CODE/dump_memory.py (1)
101-101: Perfect fix for the binary mode issue!You nailed it by removing the encoding parameter when opening the file in binary mode. In Python, you can't mix binary mode ("wb") with text encoding - it's like trying to put ketchup on ice cream, it just doesn't work! The file now properly handles the raw binary data without any encoding conflicts.
CODE/logicytics/FileManagement.py (2)
33-33: Good practice - explicit return statement!Adding the explicit
return Nonemakes your code way clearer! Even though Python would return None anyway, being explicit about it helps anyone reading the code understand exactly what's happening. This matches perfectly with your type hint that says the method can return either a string or None.
149-149: Consistency win - explicit return statement!Just like the other method, adding
return Nonehere makes your code super consistent and clear. When someone reads this, they immediately know that if no errors happen, the method returns None. It's like being upfront about what your code does instead of leaving people guessing!CODE/packet_sniffer.py (2)
70-71: Good variable naming improvement!Changing
etoerrmakes the code way more readable. It's immediately clear what the variable represents when someone's debugging.
115-116: Consistent naming across the file - nice work!You've applied the same
errvariable naming in both exception handlers. This kind of consistency makes the codebase much easier to follow..idea/csv-editor.xml (1)
1-16: Standard IDE config file looks good!This is just setting up how your IDE handles CSV files - pretty standard stuff. Having it in version control means everyone on the team gets the same CSV editing experience, which is actually pretty helpful.
CODE/_dev.py (2)
102-106: Much friendlier prompts now!Dropping the "[-] " prefix makes these questions feel way less intimidating. Instead of feeling like you're getting scolded, it's more like a helpful checklist. Good call on the UX improvement!
142-142: Consistent prompt styling throughout!You've kept the same friendly tone here too. When someone's reviewing their file changes, a neutral question feels way better than something that looks like a warning.
CODE/logicytics/Flag.py (3)
213-221: Way better type hints!Making the return type explicit as
dictinstead of leaving it unclear is super helpful. Anyone reading this code (or using an IDE) immediately knows what they're getting back.
235-235: Nice type consistency!Having
save_historytake adictparameter that matches whatload_historyreturns makes total sense. Good job keeping the types aligned!
547-550: Good cleanup of unimplemented stuff!Removing those placeholder flags that weren't actually implemented yet keeps the code cleaner. No point having dead code hanging around confusing people.
CODE/config.ini (1)
29-30: Double-check the file list for typos before shippingThe
filesentry now points to
vulnscan\Model SenseMini .3n3.pth/Vectorizer .3n3.pkl(note the extra space right before the extension).
A stray space changes the actual filename on disk, so lookups likePath("vulnscan/Model SenseMini .3n3.pth")will 404 in real life.Consider trimming those spaces, or at least quoting the exact filenames in the repo so Windows doesn’t silently “fix” the path for you.
.github/workflows/stale.yml (1)
18-18: Looks fine – action is Node-only
actions/staleis JavaScript-based and works on any runner, so the Windows switch shouldn’t hurt..github/workflows/greetings.yml (1)
10-10: Windows runner OK here
actions/first-interactionis also Node-only; the change is safe..github/workflows/dependency-review.yml (1)
17-17: Double-check execution timeThe dependency-review action runs fine on Windows, but Windows runners are noticeably slower and scarcer in the free tier. Make sure the longer queue time is acceptable.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Shahm Najeeb <[email protected]>
github-project-automation
bot
moved this from Waiting on Review
to In Progress
in Issue Board
Pull Request Template
Prerequisites
--devflag, if required.PR Type
update
Description
This update changes the CI configuration to run jobs on Windows instead of Ubuntu. It includes running the
--devflag to updateconfig.iniwith new update info and testing Logicytics in default mode. The security documentation (security.md) has been updated, and a bug fix was applied todump_memory.pyto resolve a binary mode encoding error. Dev questions were refactored, and unimplemented flags were removed. File management and flag handling were refactored, along with an update to requirements for thesentence-transformerspackage. The README and installation instructions for the VulnScan feature have been updated. VulnScan tools and the v3 module were moved to a separate repository, retaining only the model and vectorizer in this repo. Lastly, issue labels and the CodeClimate badge were updated for better clarity.Motivation and Context
Switching CI jobs to Windows ensures better compatibility and aligns with target deployment environments. Refactoring and bug fixes improve code stability and maintainability. Moving VulnScan tools to a dedicated repo simplifies this project and clarifies separation of concerns. Updated documentation and requirements make setup easier for users and contributors, enhancing overall project quality and usability.
Credit
N/A
Issues Fixed
#228
Summary by CodeRabbit
Bug Fixes
New Features
Refactor
Chores
Documentation