fix: remove unused group-list annotation helper (#110)

[agent-p1p]

Closes #110

Summary

• Removed the unused annotated_group_list() landing-page annotation helper from forensics/analysis.py.
• Dropped the now-unused Exists / OuterRef imports.
• Updated stale test/doc comments so annotated_group_list no longer appears as a plausible live group-list path.

Verification

• DJANGO_DEBUG=1 uv run python manage.py test — 155 tests OK
• DJANGO_DEBUG=1 uv run python manage.py check — no issues
• uv run ruff check . — clean
• uv run ruff format --check . — clean
• DJANGO_DEBUG=1 uv run python manage.py makemigrations --check --dry-run — no changes

Note: just is not installed in this worker, so I ran the equivalent just check commands directly.

Sensitive paths

None. Changed files are forensics/analysis.py and forensics/tests.py; no migrations, models, settings, auth, or token paths touched.

---

Summary by CodeRabbit

• Bug Fixes

  • Improved group listing calculations so event and file counts are computed more consistently, which should help keep landing pages and group detail views accurate.
  • Updated handling for group-related statistics to rely on the existing listing flow, reducing mismatches in displayed counts.

• Documentation

  • Clarified a few inline descriptions to better match the current group listing behavior.

[stage-review]

Ready to review this PR? Stage has broken it down into 2 individual chapters for you:

	Title
1	Remove unused annotated_group_list helper
2	Update documentation and test comments

_{Chapters generated by Stage for commit c3214a5 on Jun 24, 2026 6:08am UTC.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 88668425-d24f-4a58-aa5f-875f47891f38

📥 Commits

Reviewing files that changed from the base of the PR and between 4ea2bf4 and c3214a5.

📒 Files selected for processing (2)

• forensics/analysis.py
• forensics/tests.py

---

Walkthrough

Removes the dead annotated_group_list() function from forensics/analysis.py along with the now-unused Exists and OuterRef ORM imports. Updates the audit_file_counts_for_groups() docstring wording and revises three test comment/docstring references from annotated_group_list to group_list_rows.

Changes

Remove annotated_group_list() dead code

Layer / File(s)	Summary
Remove annotated_group_list() and clean up imports/docstring forensics/analysis.py	Drops Exists and OuterRef from the django.db.models import, deletes the annotated_group_list() function that annotated AuditGroup with event/file/fork aggregates via subqueries, and rewords the audit_file_counts_for_groups() docstring to describe the separate M2M counting rationale.
Update test comments to reference group_list_rows forensics/tests.py	Rephrases three comment/docstring spans: clarifies that audit_file_count includes duplicate-only files, replaces annotated_group_list with group_list_rows as the path requiring validation_status=VALID, and renames a nearby comment from "Landing page per-group annotations" to "Landing page per-group rows".

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.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 clearly names the main change: removing the unused group-list annotation helper.
Linked Issues check	✅ Passed	The PR deletes annotated_group_list() and updates comments to reflect the live group-list path, matching #110.
Out of Scope Changes check	✅ Passed	Changes stay within scope: dead-code removal plus comment/docstring cleanup and unused import deletion.

_{✏️ 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 pip/goggles-110

---

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

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

[agent-p1p]

Adversarial review — Pip (cursor-reviewer)

Verdict: APPROVE — 0 blocking, 0 suggestions, 2 nitpicks (no change required)

Clean, correctly-scoped dead-code removal. Independently verified annotated_group_list has zero references repo-wide (no call sites, imports, templates, getattr/string indirection, or management-command usage), and that Exists/OuterRef appear nowhere else in the codebase after the import trim — the retained Count/Max/Min/Q are all still used by the live aggregates. The function was genuinely dead: the production landing page (views.group_list → group_list_rows → event_stats_for_groups + audit_file_counts_for_groups) is the only path.

Correctness

The three comment/docstring rewrites are factually accurate against live behavior:

• audit_file_count is derived from the AuditFile.groups M2M through-table (no event filter), so it does count the duplicate-only / zero-stored-event file — confirmed by the == 2 assertion and the M2M counting code.
• The live group-list path filters on structural_quarantine_exclusion() + parse_status=VALID and does not require validation_status=VALID, so attributing the old valid-files-only behavior to group_list_rows (and asserting the partially-invalid file now counts toward engine/event totals) is correct.

Test coverage

No coverage hole results from the deletion. The live group_list_rows path is exercised by multiple dedicated tests — per-group stats, fork activity, display/search refs, the out-of-range wall_time_ms 500-guard, a bounded query-count guard, and the partial-invalid count-parity regression. These are far stronger drift guards than the deleted helper ever had, directly satisfying the maintainability goal of #110.

Verification (re-run independently)

• manage.py test — 155 tests OK
• manage.py check — clean
• makemigrations --check --dry-run — no changes
• ruff check . — clean
• diff touches only forensics/analysis.py and forensics/tests.py (8 insertions / 38 deletions); all executable test assertions unchanged.

No security/privacy regression — no sensitive-data handling, logging, or storage behavior touched.

Nitpicks (no change required)

• forensics/analysis.py — the rewritten audit_file_counts_for_groups docstring describes the single-annotation design in the negative ("the thing we deliberately don't do") rather than only the live design. The explanation is accurate and the planner-blowup rationale is worth keeping; purely stylistic.
• Docstring-coverage advisory (CodeRabbit, 50%): group_list_rows lacks a docstring, but it didn't have one before either — this PR doesn't introduce the gap and fixing it is out of scope.

---

Reviewed by Pip 🐾 via Cursor CLI (claude-opus-4-8-xhigh). CI green: Django tests, pip-audit, CodeRabbit (no actionable comments).