feat: add -no-classify to skip ML page-type model download#2544
feat: add -no-classify to skip ML page-type model download#2544jatinder14 wants to merge 2 commits into
Conversation
JSON/CSV output previously always initialized the dit classifier, which downloads a ~92MB model even when PageType enrichment is unused. Add -nc/-no-classify to opt out, reject combining it with -fpt/-fep, and cover the behavior with unit tests. Closes projectdiscovery#2543
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe CLI adds ChangesPage classification opt-out
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Options
participant Runner
participant Classifier
CLI->>Options: Parse --no-classify
Options->>Options: Evaluate ShouldInitPageClassifier()
Runner->>Options: Request initialization decision
Options-->>Runner: Return true or false
Runner->>Classifier: Initialize when enabled
Poem
🚥 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 |
![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 (1)
runner/runner_test.go (1)
540-550: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the legacy
-fepconflict path.The implementation rejects both
OutputFilterPageTypeandOutputFilterErrorPage, but this test only exercises the former. Add a case withOutputFilterErrorPage: trueso the deprecated alias cannot regress silently.🤖 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 `@runner/runner_test.go` around lines 540 - 550, Add coverage to TestNoClassifyConflictsWithPageTypeFilter for the legacy OutputFilterErrorPage option by validating an Options case with NoClassify and OutputFilterErrorPage set, and assert it returns an error mentioning "-no-classify"; retain the existing OutputFilterPageType coverage.
🤖 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 `@runner/options.go`:
- Line 462: Resolve the duplicate nc shorthand between the no-classify option in
the options flag registration and the existing no-color registration. Keep a
single compatible alias or assign no-classify a unique shorthand, then update
the corresponding README/help text and add a parser-level test verifying the
finalized option mapping.
---
Nitpick comments:
In `@runner/runner_test.go`:
- Around line 540-550: Add coverage to TestNoClassifyConflictsWithPageTypeFilter
for the legacy OutputFilterErrorPage option by validating an Options case with
NoClassify and OutputFilterErrorPage set, and assert it returns an error
mentioning "-no-classify"; retain the existing OutputFilterPageType coverage.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 2297cd0f-e41f-4345-8f12-06eddfb21b4c
📒 Files selected for processing (4)
README.mdrunner/options.gorunner/runner.gorunner/runner_test.go
CodeRabbit correctly flagged that -no-color already owns -nc. Keep the long form -no-classify only and sync the README.
1 participant
Summary
-nc/-no-classifyso-json/-csvdo not initialize the dit page-type classifier (and skip the ~92MB Hugging Face model download) whenKnowledgeBase/PageTypeenrichment is not needed-no-classifywith-fpt/-fep(filtering requires the classifier)Fixes #2543
Test plan
go test ./runner/ -run 'TestShouldInitPageClassifier|TestNoClassifyConflictsWithPageTypeFilter|TestHasMatcherOrFilter'rm -rf ~/.dit && echo https://example.com | ./httpx -json -no-classify— no model download / no classifier init warning about modelhttpx -json -no-classify -fpt error— errors with conflict messageMade with Cursor
Summary by CodeRabbit
-nc/-no-classifyto disable page-type classification and skip ML-based setup.-no-classifyfrom being used together with page-type or error-page filtering.-nc/-no-classifyflag and related behavior.-no-classifyvalidation conflict rules.