Skip to content

feat(llma): wire up auto-create hook, generated types, and MCP tools #54367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andrewm4894 wants to merge 1 commit into
base: andy/llma-eval-reports-4-frontend
Choose a base branch
from
+10 −0
Open

feat(llma): wire up auto-create hook, generated types, and MCP tools #54367

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions products/llm_analytics/backend/api/evaluations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,16 @@ def _get_config_length(instance) -> int:
def perform_create(self, serializer):
instance = serializer.save()

# Auto-create a default report config so reports are generated from the start.
# Users can later add email/Slack delivery targets if they want notifications.
from products.llm_analytics.backend.models.evaluation_reports import EvaluationReport
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Apr 13, 2026

Choose a reason for hiding this comment

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

P2 Deferred import should be at module level

There's no circular-import risk here — evaluation_reports.py only imports from posthog.models.utils and standard library modules. Move this to the top of the file alongside the other ..models.* imports, using the same relative-import style:

Suggested change
from products.llm_analytics.backend.models.evaluation_reports import EvaluationReport
from ..models.evaluation_reports import EvaluationReport
Prompt To Fix With AI 
This is a comment left during a code review.
Path: products/llm_analytics/backend/api/evaluations.py
Line: 240

Comment:
**Deferred import should be at module level**

There's no circular-import risk here — `evaluation_reports.py` only imports from `posthog.models.utils` and standard library modules. Move this to the top of the file alongside the other `..models.*` imports, using the same relative-import style:

```suggestion
        from ..models.evaluation_reports import EvaluationReport
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Member Author

@andrewm4894 andrewm4894 Apr 13, 2026

Choose a reason for hiding this comment

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

Agreed, no circular import risk here. Will move to module-level alongside the other ..models.* imports.


EvaluationReport.objects.create(
team=self.team,
evaluation=instance,
start_date=instance.created_at,
)
Comment on lines 235 to +246
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Apr 13, 2026

Choose a reason for hiding this comment

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

P1 Auto-create and evaluation save are not in the same transaction

serializer.save() commits the evaluation in its own implicit transaction. If EvaluationReport.objects.create() then raises (e.g. a DB error or constraint violation), the user receives a 500 but the evaluation row is already persisted. They will likely retry, creating a duplicate evaluation, while the original silently exists without any report.

Wrap both operations in transaction.atomic() so a failed report creation rolls back the evaluation too:

from django.db import transaction

def perform_create(self, serializer):
    with transaction.atomic():
        instance = serializer.save()
        from ..models.evaluation_reports import EvaluationReport
        EvaluationReport.objects.create(
            team=self.team,
            evaluation=instance,
            start_date=instance.created_at,
        )
    # tracking/analytics below can remain outside the transaction
Prompt To Fix With AI 
This is a comment left during a code review.
Path: products/llm_analytics/backend/api/evaluations.py
Line: 235-246

Comment:
**Auto-create and evaluation save are not in the same transaction**

`serializer.save()` commits the evaluation in its own implicit transaction. If `EvaluationReport.objects.create()` then raises (e.g. a DB error or constraint violation), the user receives a 500 but the evaluation row is already persisted. They will likely retry, creating a duplicate evaluation, while the original silently exists without any report.

Wrap both operations in `transaction.atomic()` so a failed report creation rolls back the evaluation too:

```python
from django.db import transaction

def perform_create(self, serializer):
    with transaction.atomic():
        instance = serializer.save()
        from ..models.evaluation_reports import EvaluationReport
        EvaluationReport.objects.create(
            team=self.team,
            evaluation=instance,
            start_date=instance.created_at,
        )
    # tracking/analytics below can remain outside the transaction
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Member Author

@andrewm4894 andrewm4894 Apr 13, 2026

Choose a reason for hiding this comment

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

Valid point. Wrapping both in transaction.atomic() is the right call — if the report creation fails we shouldn't leave an orphaned evaluation. Will fix.


# Calculate properties for tracking
conditions = instance.conditions or []
condition_count = len(conditions)
Expand Down
Loading