perf(compliance-assessments): replace per-row progress walk with aggregate

[tarkadia]

Why

The goal is to reduce the cost of calculating progress, especially during batch operations such as builtin metrics backfilling.

Changes

In backend/core/models.py, the ComplianceAssessment.progress property was refactored to use a faster helper: _get_progress_counts().

Before this change, progress was using get_requirement_assessments(include_non_assessable=False), which loads a large amount of related data we don't need. It was prefetching Questions, Answers, Choices, Evidences, Applied Controls, and other related objects, even though progress only needs to know 3 things :

• How many assessable requirements exist
• Are there any Implementation Groups and have any of them been selected
• How many requirements are already considered assessed

The new implementation keeps the same logic, but computes way faster than before (~1 second instead of almost 1 minute for 81 audits on my end).

There are now 2 different ways to compute progress:

• If no Implementation Groups are selected, it uses a direct database aggregation to compute total and assessed
• If Implementation Groups are selected, it loads only the minimal fields needed for filtering and counting, then applies the group intersection in Python

Summary by CodeRabbit

• Refactor
  • Improved compliance assessment progress calculation: progress percentages now come from optimized counts for more accurate and consistent reporting.
  • Reduced memory use when filtering by implementation groups by streaming and limiting fields, avoiding loading full records.
  • Better scalability and faster responses for large datasets and filtered views.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 946fd1d2-211c-4ef3-8273-163e6aed56d6

📥 Commits

Reviewing files that changed from the base of the PR and between cf14a96 and cb8cb16.

📒 Files selected for processing (1)

• backend/core/models.py

✅ Files skipped from review due to trivial changes (1)

• backend/core/models.py

---

📝 Walkthrough

Walkthrough

Adds ComplianceAssessment._get_progress_counts() to compute (total, assessed) for assessable RequirementAssessments and updates ComplianceAssessment.progress to derive its percentage from that helper. Uses a DB aggregate() when no implementation-group filter is set, otherwise streams a field-limited queryset and applies in-Python implementation-group checks.

Changes

Compliance Assessment Progress Computation

Layer / File(s)	Summary
Helper (data/behavior) backend/core/models.py	Adds ComplianceAssessment._get_progress_counts(self) -> tuple[int,int] to compute (total, assessed) for assessable RequirementAssessments using criteria result != NOT_ASSESSED OR score IS NOT NULL.
Core Implementation backend/core/models.py	When selected_implementation_groups is empty, uses a single DB aggregate() to compute counts. When filtered, streams a field-limited queryset (only(...).iterator()) and checks requirement.implementation_groups.isdisjoint(selected_igs) in Python to include/exclude each assessment.
Wiring / Public API backend/core/models.py	Updates ComplianceAssessment.progress to call _get_progress_counts() and compute int((assessed/total)*100) (or 0 if total == 0).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hop through rows both near and wide,
Counting bits of progress with gentle pride.
A SQL nibble, then Python sifts the rest,
Tallying each assessed and doing my best.
🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% 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 accurately describes the primary change: replacing a per-row progress calculation walk with database aggregate operations to improve performance.
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/perf_ComplianceAssessment_object_processing

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

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

[monsieurswag]

There is a small logic bug in the "slow path" assessed increment condition which MUST be fixed.

Also remove the comments, the code is already really clean, the comments just make it less readable.

Once these are fixed, this is ready to be merged.

[monsieurswag]

The ) or requirement_assessment.score is not None: should be replaced by ) and requirement_assessment.score is not None: At backend/core/models.py:7895:

  if (
      requirement_assessment.result
      != RequirementAssessment.Result.NOT_ASSESSED
  ) or requirement_assessment.score is not None:
      assessed += 1

With the current code a requirement with a None score OR with a NOT_ASSESSED result would still be considered as assessed (would still increment assessed).

[tarkadia]

I think it should be left as is, because sometimes, audits can be assessed by just giving a score to the requirement nodes without giving a specific result on them. Putting and would change the original logic. The previous version of the code was like this, with an OR :

def progress(self) -> int:
    requirement_assessments = list(
        self.get_requirement_assessments(include_non_assessable=False)
    )
    total_cnt = len(requirement_assessments)
    assessed_cnt = len(
        [
            r
            for r in requirement_assessments
            if (r.result != RequirementAssessment.Result.NOT_ASSESSED)
            or r.score != None
        ]
    )
    return int((assessed_cnt / total_cnt) * 100) if total_cnt > 0 else 0

We could've put and if the code was written like this:

if (
    requirement_assessment.result
    == RequirementAssessment.Result.NOT_ASSESSED
) and requirement_assessment.score is None:
    continue

assessed += 1

[tarkadia]

@ab-smith can you confirm this?

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/core/models.py`:
- Around line 7845-7849: There is a whitespace-only blank line breaking ruff
format in the block around the docstring and the RequirementAssessment query;
remove the stray blank line between the docstring and the requirements =
RequirementAssessment.objects.filter(...) statement (the function that returns
"(total, assessed) counts for assessable requirements") and re-run the formatter
(ruff format) on backend/core/models.py so the file passes `ruff format
--check`.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0e230cf1-a0f1-46fb-b4e3-861a077eb6c3

📥 Commits

Reviewing files that changed from the base of the PR and between 8887753 and b1454aa.

📒 Files selected for processing (1)

• backend/core/models.py

[nas-tabchiche]

LGTM

addressed