-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: use user's configured name in conversation transcript summaries #5320
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -81,6 +81,7 @@ def _get_structured( | |||||
| conversation: Union[Conversation, CreateConversation, ExternalIntegrationCreateConversation], | ||||||
| force_process: bool = False, | ||||||
| people: List[Person] = None, | ||||||
| user_name: str = None, | ||||||
| ) -> Tuple[Structured, bool]: | ||||||
| try: | ||||||
| tz = notification_db.get_user_time_zone(uid) | ||||||
|
|
@@ -139,7 +140,7 @@ def _get_structured( | |||||
| # not supported conversation source | ||||||
| raise HTTPException(status_code=400, detail=f'Invalid conversation source: {conversation.text_source}') | ||||||
|
|
||||||
| transcript_text = conversation.get_transcript(False, people=people) | ||||||
| transcript_text = conversation.get_transcript(False, people=people, user_name=user_name) | ||||||
|
|
||||||
| # For re-processing, we don't discard, just re-structure. | ||||||
| if force_process: | ||||||
|
|
@@ -345,7 +346,7 @@ def _trigger_apps( | |||||
| def execute_app(app): | ||||||
| with track_usage(uid, Features.CONVERSATION_APPS): | ||||||
| result = get_app_result( | ||||||
| conversation.get_transcript(False, people=people), conversation.photos, app, language_code=language_code | ||||||
| conversation.get_transcript(False, people=people, user_name=user_name), conversation.photos, app, language_code=language_code | ||||||
| ).strip() | ||||||
| conversation.apps_results.append(AppResult(app_id=app.id, content=result)) | ||||||
| if not is_reprocess: | ||||||
|
|
@@ -631,7 +632,10 @@ def process_conversation( | |||||
| people_data = users_db.get_people_by_ids(uid, list(set(person_ids))) | ||||||
| people = [Person(**p) for p in people_data] | ||||||
|
|
||||||
| structured, discarded = _get_structured(uid, language_code, conversation, force_process, people=people) | ||||||
| from database.auth import get_user_name | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move import to module level per coding standards The import is inside the function, violating rule 40d2fa4f (Backend Python import rules - no in-function imports). While this file has other in-function imports, best practice is to import at the module level. Move to top of file:
Suggested change
Context Used: Rule from |
||||||
| user_name = get_user_name(uid) | ||||||
|
|
||||||
| structured, discarded = _get_structured(uid, language_code, conversation, force_process, people=people, user_name=user_name) | ||||||
| conversation = _get_conversation_obj(uid, structured, conversation) | ||||||
|
|
||||||
| # AI-based folder assignment | ||||||
|
|
||||||
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.
user_nameundefined here - will cause NameErroruser_nameis not a parameter of_trigger_apps()nor is it defined in the function scope. Whenexecute_appruns, it will raiseNameError: name 'user_name' is not defined.Fix needed:
user_name: str = Noneparameter to_trigger_apps()signature (line 286)user_name=user_namewhen calling_trigger_apps()(line 696)And at line 696: