feat(query): group findings by field with aggregation - #1304
Conversation
Add a --group option to `secator q` and `secator report show` that collapses
findings by one or more fields (processing-side, post-query) with
auto-aggregation of a per-type field.
- New _group_by / _group_aggregate class attrs on OutputType, set for
Vulnerability (name / matched_at), Tag (category,name / match) and
Technology (product / match).
- --group is an optional-value option: bare --group uses the per-type
defaults; --group "<field>[,<field>]" overrides the group fields.
- group_findings() helper collapses each group to the newest finding, keeps a
_group_count and a truncated list of aggregated values (".. and X more").
- Console exporter shows "(count: N)" next to grouped lines; an Info message
per grouped type is printed at the end.
- Types without _group_by (or non-finding results) pass through unchanged;
--group with an explicit field but no groupable types warns; --group is
ignored (with a warning) when --format is used.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VWZJdNgJKrn1XHkENKqVo7
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughChangesFinding grouping
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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)
secator/query/utils.py (1)
558-612: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog load failures instead of silently swallowing them.
The
except Exception: passat lines 600-603 silently degrades grouping: ifcls.load(rep)fails, the representative stays a dict, so the aggregate field is never set (line 604 guards onnot isinstance(rep, dict)) and_group_countis never set (line 609). The finding appears ungrouped with no count or aggregated values, and no diagnostic is emitted.Ruff flags this as S110 (try-except-pass) and BLE001 (blind exception). Consider logging the failure at debug level so schema mismatches or data issues are traceable.
♻️ Proposed fix
try: rep = cls.load(rep) - except Exception: - pass + except Exception as exc: + logger.debug('group_findings: failed to load %s representative: %s', rep.get('_type'), exc)🤖 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 `@secator/query/utils.py` around lines 558 - 612, In group_findings, replace the silent broad exception around cls.load(rep) with a debug-level log that includes the exception and relevant finding/type context; use the module’s existing logger or add an appropriate logger without introducing a blind except-pass. Preserve the fallback behavior while making load failures traceable and satisfying Ruff S110/BLE001.Source: Linters/SAST tools
🤖 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 `@secator/cli.py`:
- Around line 1459-1481: Update the warning condition in the group-processing
block after grouped_types is populated so it triggers when grouping was
requested, including bare --group where group == '', and no groupable types were
found; preserve the existing warning message and avoid warning when --group was
not provided.
---
Nitpick comments:
In `@secator/query/utils.py`:
- Around line 558-612: In group_findings, replace the silent broad exception
around cls.load(rep) with a debug-level log that includes the exception and
relevant finding/type context; use the module’s existing logger or add an
appropriate logger without introducing a blind except-pass. Preserve the
fallback behavior while making load failures traceable and satisfying Ruff
S110/BLE001.
🪄 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.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0fbd523e-a333-42b1-9454-629ffde1cbe1
📒 Files selected for processing (8)
secator/cli.pysecator/exporters/console.pysecator/output_types/_base.pysecator/output_types/tag.pysecator/output_types/technology.pysecator/output_types/vulnerability.pysecator/query/utils.pytests/unit/test_query_utils.py
|
@claude address coderabbit review comment |
|
Claude finished @ocervell's task in 3m 7s —— View job Addressing CodeRabbit review comments
Changes made (commit
|
- secator/cli.py: fire the "no groupable finding types" warning for bare --group too (group == '' was falsy, so the warning never fired when only per-type defaults were requested). - secator/query/utils.py: log load failures in group_findings via debug() instead of silently swallowing them (Ruff S110/BLE001). Co-authored-by: Olivier Cervello <9629314+ocervell@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
2 participants
Closes #1219.
Adds a
--groupoption tosecator qandsecator report showthat groups findings by one or more fields with auto-aggregation, done on the processing side after the query (no backend query changes), as the issue suggested.Usage
secator q vulnerability --group— group vulnerabilities by their default field(s).secator q vulnerability --group severity— group by an explicit field.--groupuses per-type defaults;--group "<field>[,<field>]"overrides.What it does
_group_by/_group_aggregateclass attrs onOutputType, set for the three types the issue lists:Vulnerability→ group byname, aggregatematched_atTag→ group bycategory,name, aggregatematchTechnology→ group byproduct, aggregatematchgroup_findings()collapses each group to the newest finding, keeping a_group_countand a truncated list of the aggregated field's values (.. and X morepast 5).(count: N)next to each grouped line.Infomessage per grouped type is printed at the end (<type> grouped by <field>. To show complete results, remove the --group option.).Degradation
_group_by(and non-finding results) pass through unchanged.--group <field>with no groupable finding types present → warning, no silent mis-answer.--groupcombined with--formatis ignored with a warning (format already reshapes output).Notes / assumptions
$grouppipeline needed, matching the issue's "processing side before displaying" guidance.Technologyis a typo —Technologyhas nomatched_atfield; its location field ismatch, so that is used (same asTag).--grouppath forsecator q/report show. The issue's hedged "secator r listshould probably default to grouped vuln counts" is a separate command/code path and was left out to keep scope tight — easy follow-up if wanted.Test
Added
TestGroupFindingsintests/unit/test_query_utils.py(group-by-name + aggregate + truncation asserts).secator test unitquery/report/output-type suites pass (240 tests).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--groupflag to query and report display commands.Tests