fix(compliance): show ancestors whose implementation groups dont't match selected ones but whose descendants do by nas-tabchiche · Pull Request #4001 · intuitem/ciso-assistant-community

nas-tabchiche · 2026-04-22T15:25:04Z

Summary

Tree view under Associated Requirements showed 0 requirements when a user filtered an audit by a custom implementation group that only appeared on leaves, if an ancestor node carried a different (pre-existing) IG. Table mode and Flash mode were unaffected because they filter at the RA level and skip the hierarchical filter.
filter_graph_by_implementation_groups short-circuited on a node's own IGs: if the node had IGs of its own and none matched, the entire subtree was pruned — even when descendants did match.
Fix keeps an ancestor whenever its own IGs match or any of its children survived filtering. Purely additive: never keeps fewer nodes than before.

Reproduction (the reported case)

Copy ISO 27001:2022. Both top-level nodes (core, annex-a) carry pre-tagged IGs (['Clauses'], ['SoA']).
In the framework builder, add a custom IG P1 and tag some A.5.x leaves.
Publish, create an audit filtered by P1.
Before the fix: Associated Requirements shows 0; Table mode / Flash mode show the tagged leaves.
After the fix: all three views agree.

Test plan

New unit tests in backend/core/tests/test_helpers.py cover the regression and neighbouring edge cases (5 tests, all passing).
Full test_helpers.py suite still green (38/38).
Manual: publish a framework with a custom IG tagged on leaves that live under an ancestor with a different IG; audit filtered by the custom IG shows the expected leaves.

Summary by CodeRabbit

Bug Fixes
- Fixes implementation-group filtering so ancestor nodes are preserved when descendants match selected groups; nodes without specified implementation groups are treated as empty and handled correctly.
Tests
- Added tests covering filtering behavior: empty selection, ancestor retention, sibling removal, branch pruning, and handling of missing/empty implementation groups.

…h but whose descendants did

nas-tabchiche · 2026-04-22T15:27:07Z

+        if any(group in node_groups for group in implementation_groups):
+            return True


actual application diff is these 2 lines

coderabbitai · 2026-04-22T15:30:15Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 46daea5d-7929-41e0-a55c-758e763253fe

📥 Commits

Reviewing files that changed from the base of the PR and between eb42953 and 7218610.

📒 Files selected for processing (1)

backend/core/helpers.py

📝 Walkthrough

Walkthrough

filter_graph_by_implementation_groups() was changed to treat missing implementation_groups as empty lists and to retain ancestor nodes when any descendant matches the selected implementation groups; nodes are now kept if they match or have surviving (matching) children after recursive filtering.

Changes

Graph Filtering Behavior

Layer / File(s)	Summary
Predicate / Data normalization `backend/core/helpers.py`	`should_include_node()` now normalizes `node["implementation_groups"]` to `[]` when missing and treats membership check accordingly.
Core filtering logic `backend/core/helpers.py`	Node inclusion now returns true if the node’s groups match OR if the node has any children remaining after recursive filtering (ancestors of matching descendants are preserved).
Tests `backend/core/tests/test_helpers.py`	Added helper constructors (`_leaf`, `_branch`) and 6 tests covering empty-selection passthrough, ancestor retention for matching descendants, sibling removal, branch removal when no matches, direct-match retention, and handling of missing/empty `implementation_groups`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit hops through graphs of green, 🐰
Ancestors once hidden now stand in view,
Missing groups made empty, tidy and keen,
Descendants found — their parents stay true,
Glad hops and tiny code-review cheers!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and specifically describes the main fix: ancestors are now retained when their descendants match the selected implementation groups, even if the ancestor's own groups don't match.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/ig-filter-ancestor-igs

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.

❤️ Share

_{Review rate limit: 6/8 reviews remaining, refill in 11 minutes and 31 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

ab-smith · 2026-04-22T15:45:18Z

isn't this the expected behaviour? during authoring, we put the IG on the assessable nodes not their parents

tarkadia

Works great! Howerver, if a parent node is assessable and you select an IG that belongs to the child only and not to the parent, CISO throws a 404 Error when clicking on the parent.

You can reproduce the issue with the following Excel file, by selecting the Group 2 IG and clicking on the A - Level 1 node (a 404 Error also occurs if the parent node doesn't have any defined IGs) :
problem.xlsx

Maybe we could fix this in another PR ?

UPDATE

Fixed in PR #4063

tarkadia · 2026-04-24T16:20:39Z

isn't this the expected behaviour? during authoring, we put the IG on the assessable nodes not their parents

I think this becomes an issue if we encounter a specific framework where parents are assessable and have different IGs from their children. We do have frameworks with assessable parents, but we didn't encouter this specific issue with IGs until it was reported.

EDIT : We DID encounter this issue in the past, but we were using workaround consisting of adding the children's IGs to the parents' IGs

tarkadia

Good 2 Go

fix(compliance): tree filter kept ancestors whose own IGs didn't matc…

eb42953

…h but whose descendants did

nas-tabchiche requested review from ab-smith, martinzerty, monsieurswag and tarkadia April 22, 2026 15:26

nas-tabchiche linked an issue Apr 22, 2026 that may be closed by this pull request

Framework Builder: Implementation Groups are empty in 27001 audit #3985

Open

nas-tabchiche commented Apr 22, 2026

View reviewed changes

ab-smith marked this pull request as draft April 22, 2026 16:28

nas-tabchiche changed the title ~~fix(compliance): tree filter kept ancestors whose own IGs didn't match but whose descendants did~~ fix(compliance): show ancestors whose implementation groups dont't match selected ones but whose descendants do Apr 22, 2026

tarkadia reviewed Apr 24, 2026

View reviewed changes

melinoix marked this pull request as ready for review May 4, 2026 10:02

Merge branch 'main' into fix/ig-filter-ancestor-igs

7218610

tarkadia self-requested a review May 4, 2026 14:29

tarkadia approved these changes May 4, 2026

View reviewed changes

tarkadia added the bugfix A bug has been fixed label May 4, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(compliance): show ancestors whose implementation groups dont't match selected ones but whose descendants do#4001

fix(compliance): show ancestors whose implementation groups dont't match selected ones but whose descendants do#4001
nas-tabchiche wants to merge 2 commits intomainfrom
fix/ig-filter-ancestor-igs

nas-tabchiche commented Apr 22, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

nas-tabchiche Apr 22, 2026

Uh oh!

coderabbitai Bot commented Apr 22, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

ab-smith commented Apr 22, 2026

Uh oh!

tarkadia left a comment •

edited

Loading

Uh oh!

tarkadia commented Apr 24, 2026 •

edited

Loading

Uh oh!

tarkadia left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		if any(group in node_groups for group in implementation_groups):
		return True

Conversation

nas-tabchiche commented Apr 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Reproduction (the reported case)

Test plan

Summary by CodeRabbit

Uh oh!

nas-tabchiche Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

ab-smith commented Apr 22, 2026

Uh oh!

tarkadia left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

UPDATE

Uh oh!

tarkadia commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tarkadia left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nas-tabchiche commented Apr 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 22, 2026 •

edited

Loading

tarkadia left a comment •

edited

Loading

tarkadia commented Apr 24, 2026 •

edited

Loading