feat(workstream-c): add cheatsheet categorizer and grouping#934
Open
shreeshtripurwarcomp23-coder wants to merge 2 commits into
Open
feat(workstream-c): add cheatsheet categorizer and grouping#934shreeshtripurwarcomp23-coder wants to merge 2 commits into
shreeshtripurwarcomp23-coder wants to merge 2 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughAdds a cheatsheet categorizer module with controlled taxonomy labels, deterministic and LLM-assisted categorization, stable grouping by label set, and a new unittest suite covering taxonomy integrity, categorization behavior, validation, and grouping. ChangesCheatsheet Categorization System
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@application/tests/test_cheatsheet_categorizer.py`:
- Around line 42-48: The _make_record helper function creates CheatsheetRecord
instances with an empty summary string, but CheatsheetRecord.__post_init__
enforces that summary must be non-empty, causing construction to fail before
test assertions run. Replace the empty string assignment `summary=""` in the
_make_record function with a minimal non-empty placeholder value (such as a
single space, period, or descriptive placeholder text like "Test summary") to
satisfy the validation requirement.
In
`@application/utils/external_project_parsers/parsers/cheatsheet_categorizer.py`:
- Around line 194-210: The __post_init__ method in CheatsheetRecord currently
validates only string-typed fields but does not validate the element types of
list-typed fields (headings and category_hints). This causes runtime crashes
when non-string items reach code expecting to call " ".join() on these fields.
Add validation in __post_init__ to ensure that headings and category_hints are
lists containing only string elements, raising a ValueError with a clear message
if any element is not a string, so that parser input validation fails fast at
construction time rather than later during string joining operations.
- Around line 366-381: The `_validate_labels` function currently allows
`uncategorized` to be returned alongside other valid labels, which violates the
sentinel semantics where `uncategorized` should only be returned when it is the
sole valid label. Modify the function to add logic after building the deduped
list: if `uncategorized` (or the appropriate constant reference from TAXONOMY)
is present in the result AND there are other valid labels alongside it, remove
the `uncategorized` entry from the returned list. This ensures that
`uncategorized` is only returned when no other categories match, preserving its
role as a fallback indicator and preventing inconsistent downstream grouping and
UX.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: ca9bd7d2-9738-4eb1-9bf6-5c1b32faf0db
📒 Files selected for processing (2)
application/tests/test_cheatsheet_categorizer.pyapplication/utils/external_project_parsers/parsers/cheatsheet_categorizer.py
Implements Workstream C from the RFC: Autonomous LLM Pipeline for OWASP Cheat Sheet to CRE Mapping. - categorize_cheatsheet(record) with 29-label controlled taxonomy - group_cheatsheets(records) with stable sha256-based group IDs - Deterministic keyword/rule baseline, no LLM dependency required - LLM-optional path with safe fallback on any failure - UNCATEGORIZED sentinel stripped when real labels are present - Uses CheatsheetRecord from application.defs.cheatsheet_defs (Workstream B, PR OWASP#921) - 50 tests covering all acceptance criteria from RFC Issue C
shreeshtripurwarcomp23-coder
force-pushed
the
workstream-c-clean
branch
from
f8472d4 to
12e041f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
application/utils/external_project_parsers/parsers/cheatsheet_categorizer.py (1)
328-345: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn validated LLM labels in canonical order.
categorize_cheatsheet()documents a sorted result, but_validate_labels()preserves the LLM’s order. That makes the LLM path non-deterministic for the same label set and breaks the public contract.Suggested fix
real = [lbl for lbl in deduped if lbl != UNCATEGORIZED] - return real if real else deduped + return sorted(real if real else deduped)🤖 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 `@application/utils/external_project_parsers/parsers/cheatsheet_categorizer.py` around lines 328 - 345, The `_validate_labels` helper in `cheatsheet_categorizer.py` preserves the LLM’s input order, which conflicts with the documented sorted output from `categorize_cheatsheet()`. Update `_validate_labels()` to return approved labels in the canonical `TAXONOMY` order after filtering and deduping, while still stripping `UNCATEGORIZED` when real labels exist. Keep the fix localized to `_validate_labels` so the LLM path matches the deterministic contract everywhere it is used.
🤖 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
`@application/utils/external_project_parsers/parsers/cheatsheet_categorizer.py`:
- Around line 280-291: `group_cheatsheets()` is currently keying the `bucket` by
the truncated `CheatsheetGroup.make_group_id(labels)` value, which can merge
distinct label sets on collision; change the grouping map to use the canonical
sorted unique labels as the key while still deriving `group_id` from
`CheatsheetGroup.make_group_id` for the created `CheatsheetGroup`. Update the
`bucket` population logic in `group_cheatsheets()` so groups are formed strictly
by label set identity, and keep `CheatsheetGroup(group_id=...,
labels=sorted(labels))` as the stored object.
---
Duplicate comments:
In
`@application/utils/external_project_parsers/parsers/cheatsheet_categorizer.py`:
- Around line 328-345: The `_validate_labels` helper in
`cheatsheet_categorizer.py` preserves the LLM’s input order, which conflicts
with the documented sorted output from `categorize_cheatsheet()`. Update
`_validate_labels()` to return approved labels in the canonical `TAXONOMY` order
after filtering and deduping, while still stripping `UNCATEGORIZED` when real
labels exist. Keep the fix localized to `_validate_labels` so the LLM path
matches the deterministic contract everywhere it is used.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: a607e96a-00c8-4fb8-9d70-f8a4dd5baeaa
📒 Files selected for processing (2)
application/tests/test_cheatsheet_categorizer.pyapplication/utils/external_project_parsers/parsers/cheatsheet_categorizer.py
🚧 Files skipped from review as they are similar to previous changes (1)
- application/tests/test_cheatsheet_categorizer.py
…categorizer.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Shreesh Tripurwar <shreesh.tripurwar_comp23@pccoer.in>
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
1 participant
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.
Uh oh!
There was an error while loading. Please reload this page.