-
Notifications
You must be signed in to change notification settings - Fork 6
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.
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
Operational Learning Summary Generation #2227
base: develop
Are you sure you want to change the base?
Conversation
b3019de
to
5c0c71f
Compare
77a1791
to
33d0ef5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Round 1
per/models.py
Outdated
|
||
class OpsLearningPromptResponseCache(models.Model): | ||
class PromptType(models.IntegerChoices): | ||
PRIMARY = 0, _("primary") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's try to avoid using 0, as it can have issue if we use if/else with it.
Let's do the same for others as well
per/models.py
Outdated
insights1_confidence_level = models.CharField(verbose_name=_("insights 1 confidence level"), null=True, blank=True) | ||
insights2_confidence_level = models.CharField(verbose_name=_("insights 2 confidence level"), null=True, blank=True) | ||
insights3_confidence_level = models.CharField(verbose_name=_("insights 3 confidence level"), null=True, blank=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add max_length here as well?
deployments/factories/project.py
Outdated
@@ -22,7 +22,7 @@ class SectorTagFactory(factory.django.DjangoModelFactory): | |||
class Meta: | |||
model = models.SectorTag | |||
|
|||
title = fuzzy.FuzzyText(length=50, prefix="sect-tag-") | |||
title = factory.Faker("sentence", nb_words=5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why we are changing this?
NOTE: We used "sect-tag-" to make it readable among error diffs and snapshot. Also changing this will require changes on lots of existing snapshot
per/drf_views.py
Outdated
@@ -701,6 +717,21 @@ class Meta: | |||
"appeal_code__region": ("exact", "in"), | |||
} | |||
|
|||
def get_cache_response(self, queryset, name, value): | |||
if ops_learning_cache_response := OpsLearningCacheResponse.objects.filter(id=value).first(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also add additional check
if ops_learning_cache_response := OpsLearningCacheResponse.objects.filter(id=value).first(): | |
if value and (ops_learning_cache_response := OpsLearningCacheResponse.objects.filter(id=value).first()): |
per/serializers.py
Outdated
@@ -1046,6 +1049,29 @@ class Meta: | |||
exclude = ("learning", "type", "organization", "sector", "per_component") | |||
|
|||
|
|||
class AppealDetailSerializer(serializers.ModelSerializer): | |||
atype = serializers.SerializerMethodField() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use this instead? SerializerMethodField doesn't provide proper schema
atype = serializers.SerializerMethodField() | |
atype_display = serializers.CharField(source='get_atype_display', read_only=True) |
per/serializers.py
Outdated
] | ||
|
||
def get_extract_count(self, obj) -> int: | ||
return obj.used_ops_learning.count() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
N+1?
per/ops_learning_summary.py
Outdated
client = AzureOpenAI( | ||
azure_endpoint=settings.AZURE_OPENAI_ENDPOINT, api_key=settings.AZURE_OPENAI_KEY, api_version="2023-05-15" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use cached_property for this
Add a TODO: to create a separate object for integration with Open AI for common use cases
per/task.py
Outdated
logger.error(e) | ||
raise e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logger.error(e) | |
raise e | |
# TODO: Add extra context for which ops learning cache response failed | |
logger.error("Ops learning process failed", exc_info=True) | |
return False | |
return True |
per/task.py
Outdated
@shared_task | ||
def generate_summary(ops_learning_summary_id: int, filter_data: dict): | ||
try: | ||
ops_learning_summary_instance = OpsLearningCacheResponse.objects.filter(id=ops_learning_summary_id).first() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this outside try
ops_learning_summary_instance = OpsLearningCacheResponse.objects.filter(id=ops_learning_summary_id).first() | |
ops_learning_summary_instance = OpsLearningCacheResponse.objects.filter(id=ops_learning_summary_id).first() | |
if ops_learning_summary_instance is None: | |
# TODO: Add extra context for which ops learning cache response failed | |
logger.error("Missing Ops learning cache response triggered") | |
return False |
33e896f
to
b12217f
Compare
fda24ea
to
adb4a95
Compare
adb4a95
to
5193556
Compare
Code refactor of Prioritizing opslearning
Update primary instruction and Add New prompt instructions
Changes on mocking and response Add splitting confidence level value
5193556
to
e41116b
Compare
Change latest prompt with used excerpts Add OpenAiChat class for AzureOpenAI Handle Empty df for Ops and recursive function for summary
3a3bc3c
to
f5f14b2
Compare
c512fea
to
cc11ba1
Compare
Addresses
Changes
python manage.py create_dummy_opslearningsummary
to generate mockChecklist
Things that should succeed before merging.
Release
If there is a version update, make sure to tag the repository with the latest version.