Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions radis/chats/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def register_app():
MainMenuItem(
url_name="chat_list",
label="Chats",
order=8,
)
)

Expand Down
13 changes: 13 additions & 0 deletions radis/reports/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,23 @@ class ReportsConfig(AppConfig):
name = "radis.reports"

def ready(self):
register_app()
# Put calls to db stuff in this signal handler
post_migrate.connect(init_db, sender=self)


def register_app():
from adit_radis_shared.common.site import MainMenuItem, register_main_menu_item
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The imports for MainMenuItem and register_main_menu_item are placed inside the register_app function. While this works, it's generally considered best practice in Python to place all imports at the top of the module for clarity and to avoid potential circular import issues or repeated imports if the function were called multiple times in different contexts. Moving these imports to the top of the apps.py file would improve code readability and consistency.

from adit_radis_shared.common.site import MainMenuItem, register_main_menu_item


register_main_menu_item(
MainMenuItem(
url_name="report_overview",
label="Overview",
order=9,
)
)


def init_db(**kwargs):
from .models import ReportsAppSettings

Expand Down
1 change: 1 addition & 0 deletions radis/reports/management/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

1 change: 1 addition & 0 deletions radis/reports/management/commands/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

73 changes: 73 additions & 0 deletions radis/reports/management/commands/rebuild_report_overview_stats.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
from django.contrib.auth.models import Group
from django.core.management.base import BaseCommand
from django.db import transaction
from django.db.models import Count
from django.db.models.functions import TruncYear

from radis.reports.models import (
Report,
ReportLanguageStat,
ReportModalityStat,
ReportOverviewTotal,
ReportYearStat,
)


class Command(BaseCommand):
help = "Rebuild per-group report overview statistics."

def handle(self, *args, **options):
groups = Group.objects.all().only("id")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using Group.objects.all().only("id") will still load all Group objects into memory, even if only the id is accessed. For very large numbers of groups, this could lead to high memory consumption. Consider using .iterator() to process groups one by one, which is more memory-efficient for large querysets.

Suggested change
groups = Group.objects.all().only("id")
groups = Group.objects.all().only("id").iterator()

for group in groups:
report_qs = Report.objects.filter(groups=group)
total_count = report_qs.count()

year_counts = (
report_qs.annotate(year=TruncYear("study_datetime"))
.values("year")
.annotate(count=Count("id"))
.order_by()
)
modality_counts = (
report_qs.values("modalities__code")
.annotate(count=Count("id", distinct=True))
.order_by()
)
language_counts = (
report_qs.values("language__code").annotate(count=Count("id")).order_by()
)

year_stats = [
ReportYearStat(group=group, year=item["year"].year, count=item["count"])
for item in year_counts
if item["year"]
]
modality_stats = [
ReportModalityStat(
group=group,
modality_code=item["modalities__code"] or "Unknown",
count=item["count"],
)
for item in modality_counts
]
language_stats = [
ReportLanguageStat(
group=group,
language_code=item["language__code"] or "Unknown",
count=item["count"],
)
for item in language_counts
]

with transaction.atomic():
ReportOverviewTotal.objects.update_or_create(
group=group, defaults={"total_count": total_count}
)
ReportYearStat.objects.filter(group=group).delete()
ReportModalityStat.objects.filter(group=group).delete()
ReportLanguageStat.objects.filter(group=group).delete()
ReportYearStat.objects.bulk_create(year_stats)
ReportModalityStat.objects.bulk_create(modality_stats)
ReportLanguageStat.objects.bulk_create(language_stats)

self.stdout.write(self.style.SUCCESS("Overview stats rebuilt."))
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Generated by Django 6.0.1 on 2026-01-26 20:16

import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('auth', '0012_alter_user_first_name_max_length'),
('reports', '0013_alter_report_options'),
]

operations = [
migrations.CreateModel(
name='ReportOverviewTotal',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('total_count', models.PositiveIntegerField()),
('updated_at', models.DateTimeField(auto_now=True)),
('group', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='report_overview_total', to='auth.group')),
],
),
migrations.CreateModel(
name='ReportLanguageStat',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('language_code', models.CharField(max_length=10)),
('count', models.PositiveIntegerField()),
('group', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='report_language_stats', to='auth.group')),
],
options={
'constraints': [models.UniqueConstraint(fields=('group', 'language_code'), name='unique_report_language_stat')],
},
),
migrations.CreateModel(
name='ReportModalityStat',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('modality_code', models.CharField(max_length=16)),
('count', models.PositiveIntegerField()),
('group', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='report_modality_stats', to='auth.group')),
],
options={
'constraints': [models.UniqueConstraint(fields=('group', 'modality_code'), name='unique_report_modality_stat')],
},
),
migrations.CreateModel(
name='ReportYearStat',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('year', models.PositiveIntegerField()),
('count', models.PositiveIntegerField()),
('group', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='report_year_stats', to='auth.group')),
],
options={
'constraints': [models.UniqueConstraint(fields=('group', 'year'), name='unique_report_year_stat')],
},
),
]
59 changes: 59 additions & 0 deletions radis/reports/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,62 @@ class Meta:

def __str__(self) -> str:
return f"{self.key}: {self.value}"


class ReportOverviewTotal(models.Model):
group = models.OneToOneField(
Group,
on_delete=models.CASCADE,
related_name="report_overview_total",
)
total_count = models.PositiveIntegerField()
updated_at = models.DateTimeField(auto_now=True)

def __str__(self) -> str:
return f"{self.group.pk} total={self.total_count}"


class ReportYearStat(models.Model):
group = models.ForeignKey(Group, on_delete=models.CASCADE, related_name="report_year_stats")
year = models.PositiveIntegerField()
count = models.PositiveIntegerField()

class Meta:
constraints = [
models.UniqueConstraint(fields=["group", "year"], name="unique_report_year_stat")
]

def __str__(self) -> str:
return f"{self.group.pk} {self.year}={self.count}"


class ReportModalityStat(models.Model):
group = models.ForeignKey(Group, on_delete=models.CASCADE, related_name="report_modality_stats")
modality_code = models.CharField(max_length=16)
count = models.PositiveIntegerField()

class Meta:
constraints = [
models.UniqueConstraint(
fields=["group", "modality_code"], name="unique_report_modality_stat"
)
]

def __str__(self) -> str:
return f"{self.group.pk} {self.modality_code}={self.count}"


class ReportLanguageStat(models.Model):
group = models.ForeignKey(Group, on_delete=models.CASCADE, related_name="report_language_stats")
language_code = models.CharField(max_length=10)
count = models.PositiveIntegerField()

class Meta:
constraints = [
models.UniqueConstraint(
fields=["group", "language_code"], name="unique_report_language_stat"
)
]

def __str__(self) -> str:
return f"{self.group.pk} {self.language_code}={self.count}"
13 changes: 13 additions & 0 deletions radis/reports/tasks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import logging

from django.core.management import call_command
from procrastinate.contrib.django import app

logger = logging.getLogger(__name__)


@app.periodic(cron="0 * * * *") # hourly
@app.task
def rebuild_report_overview_stats_task(*args, **kwargs) -> None:
logger.info("Rebuilding report overview stats")
call_command("rebuild_report_overview_stats")
Comment on lines +9 to +13
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find and examine the file
find . -name "tasks.py" -path "*/reports/*" | head -5

Repository: openradx/radis

Length of output: 82


🏁 Script executed:

# Check the actual content around lines 9-13
cat -n radis/reports/tasks.py | sed -n '1,20p'

Repository: openradx/radis

Length of output: 516


🌐 Web query:

Procrastinate Python library task decorator signature requirements *args **kwargs

💡 Result:

In Procrastinate, a job stores and replays only keyword arguments (task_kwargs). Tasks are therefore expected to be called as task(**task_kwargs) (or task(context, **task_kwargs) when pass_context=True)—there’s no “positional args” payload in the job model. [1], [2], [3]

Practical implications for task signatures and decorators:

  • When deferring you should pass kwargs, e.g. my_task.defer(a=1, b=2) (and the CLI also takes a JSON object of kwargs). [3]
  • A task can be defined with explicit parameters (def my_task(a: int, b: int): ...) or as a catch-all (def my_task(**kwargs): ...), but Procrastinate fundamentally works in terms of keyword args. [1], [3], [4]
  • If you set pass_context=True, Procrastinate passes a context positional argument in addition to job kwargs; so a robust signature is typically def my_task(context, *, a: int, b: int): ... (or accept *args, **kwargs if you want to be generic). [1]
  • If you write your own decorator around @app.task, make the wrapper accept *job_args, **job_kwargs and forward them, so it won’t break when Procrastinate injects context and/or keyword args. [5]
  • The kwargs must be JSON-serializable, since connectors use a JSON dump/load function to store/load job arguments. [1]

Sources: Procrastinate docs/reference and how-tos. [1], [3], [5]


🏁 Script executed:

# Check Ruff configuration
find . -name "ruff.toml" -o -name "pyproject.toml" -o -name ".ruff.toml" | head -5

Repository: openradx/radis

Length of output: 116


🏁 Script executed:

# Look for ruff config in pyproject.toml or similar
if [ -f "pyproject.toml" ]; then
  grep -A 20 "\[tool.ruff" pyproject.toml | head -30
fi

Repository: openradx/radis

Length of output: 402


🏁 Script executed:

# Search for other task definitions in the codebase to check for consistency
rg "@app\.task|@app\.periodic" --type py -A 2 | head -50

Repository: openradx/radis

Length of output: 1677


**Remove unused *args and kwargs from periodic task signature.

Line 11 defines unused parameters that Ruff flags as ARG001. Procrastinate periodic tasks receive no positional or keyword arguments from the scheduler, so these can be safely removed.

🛠️ Suggested fix
 `@app.periodic`(cron="0 * * * *")  # hourly
 `@app.task`
-def rebuild_report_overview_stats_task(*args, **kwargs) -> None:
+def rebuild_report_overview_stats_task() -> None:
     logger.info("Rebuilding report overview stats")
     call_command("rebuild_report_overview_stats")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@app.periodic(cron="0 * * * *") # hourly
@app.task
def rebuild_report_overview_stats_task(*args, **kwargs) -> None:
logger.info("Rebuilding report overview stats")
call_command("rebuild_report_overview_stats")
`@app.periodic`(cron="0 * * * *") # hourly
`@app.task`
def rebuild_report_overview_stats_task() -> None:
logger.info("Rebuilding report overview stats")
call_command("rebuild_report_overview_stats")
🧰 Tools
🪛 Ruff (0.14.14)

11-11: Unused function argument: args

(ARG001)


11-11: Unused function argument: kwargs

(ARG001)

🤖 Prompt for AI Agents
In `@radis/reports/tasks.py` around lines 9 - 13, The periodic task function
rebuild_report_overview_stats_task currently declares unused parameters (*args,
**kwargs) which triggers ARG001; remove these parameters from the function
signature so it becomes def rebuild_report_overview_stats_task() -> None:,
leaving the `@app.periodic` and `@app.task` decorators and the function body
(logger.info and call_command("rebuild_report_overview_stats")) unchanged.

133 changes: 133 additions & 0 deletions radis/reports/templates/reports/report_overview.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
{% extends "reports/reports_layout.html" %}
{% block title %}
Reports Overview
{% endblock title %}
{% block heading %}
<c-page-heading title="Reports Overview" />
{% endblock heading %}
{% block content %}
{% if not stats_ready %}
<div class="alert alert-warning">
Overview stats have not been generated yet. Ask an admin to run
<code>uv run python manage.py rebuild_report_overview_stats</code>.
</div>
{% endif %}
<div class="row g-3 mb-4">
<div class="col-md-4">
<div class="card h-100">
<div class="card-body">
<h6 class="card-title text-muted">Total Reports</h6>
<div class="display-6">{{ total_count }}</div>
</div>
</div>
</div>
<div class="col-md-4">
<div class="card h-100">
<div class="card-body">
<h6 class="card-title text-muted">{{ last_year }} Reports</h6>
<div class="display-6">{{ last_year_count }}</div>
</div>
</div>
</div>
<div class="col-md-4">
<div class="card h-100">
<div class="card-body">
<h6 class="card-title text-muted">YoY Change ({{ last_year }} vs {{ prev_year }})</h6>
<div class="display-6">
{% if yoy_change is not None %}
{% if yoy_change >= 0 %}+{% endif %}{{ yoy_change|floatformat:1 }}%
{% else %}
{% endif %}
</div>
</div>
</div>
</div>
</div>

<div class="row g-4">
<div class="col-lg-4">
<div class="card h-100">
<div class="card-body">
<h5 class="card-title">Top Modalities</h5>
<table class="table table-sm">
<thead>
<tr>
<th>Modality</th>
<th class="text-end">Reports</th>
</tr>
</thead>
<tbody>
{% for item in top_modalities %}
<tr>
<td>{{ item.modality_code|default:"Unknown" }}</td>
<td class="text-end">{{ item.count }}</td>
</tr>
{% endfor %}
{% if other_modality_count %}
<tr>
<td>Other</td>
<td class="text-end">{{ other_modality_count }}</td>
</tr>
{% endif %}
</tbody>
</table>
</div>
</div>
</div>

<div class="col-lg-4">
<div class="card h-100">
<div class="card-body">
<h5 class="card-title">Top Languages</h5>
<table class="table table-sm">
<thead>
<tr>
<th>Language</th>
<th class="text-end">Reports</th>
</tr>
</thead>
<tbody>
{% for item in top_languages %}
<tr>
<td>{{ item.language_code }}</td>
<td class="text-end">{{ item.count }}</td>
</tr>
{% endfor %}
{% if other_language_count %}
<tr>
<td>Other</td>
<td class="text-end">{{ other_language_count }}</td>
</tr>
{% endif %}
</tbody>
</table>
</div>
</div>
</div>

<div class="col-lg-4">
<div class="card h-100">
<div class="card-body">
<h5 class="card-title">Reports by Year (Clinical Time)</h5>
<table class="table table-sm">
<thead>
<tr>
<th>Year</th>
<th class="text-end">Reports</th>
</tr>
</thead>
<tbody>
{% for item in year_counts %}
<tr>
<td>{{ item.year }}</td>
<td class="text-end">{{ item.count }}</td>
</tr>
{% endfor %}
</tbody>
</table>
</div>
</div>
</div>
</div>
{% endblock content %}
Loading