Skip to content
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

Fix delta on combining memories #825

Merged
merged 1 commit into from
Sep 12, 2024
Merged

Conversation

beastoin
Copy link
Collaborator

@beastoin beastoin commented Sep 12, 2024

Summary by Entelligence.AI

  • New Feature: Introduced a timer_starts attribute in the ProcessingMemory class to enhance transcription accuracy. This feature adjusts segment start and end times based on a delta value, improving the overall user experience with more precise transcriptions.
  • Refactor: Updated the condition for syncing processing transcript to ensure smoother operation and better performance.
  • Chore: Implemented tracking of timer start during the creation of processing memory, providing more detailed insights for debugging and optimization purposes.

@beastoin beastoin merged commit 858ad35 into main Sep 12, 2024
1 check passed
@beastoin beastoin deleted the drhfehza_processing_memory_combining branch September 12, 2024 13:28
Copy link

github-actions bot commented Sep 12, 2024

Image description Entelligence.AI

Walkthrough

This update introduces a new attribute timer_starts to the ProcessingMemory class, enhancing the precision of segment timing. It also refines the condition for syncing processing transcript and improves the tracking of timer start during the creation of processing memory.

Changes

Files Summary
backend/models/processing_memory.py, backend/routers/transcribe.py Introduced a new attribute timer_starts in ProcessingMemory class. Adjusted segment start and end times based on a delta value. Modified the condition for syncing processing transcript. Improved tracking of timer start in the creation of processing memory.

🎉🤖

With each tick and tock of the clock,

Our code now tracks time with more stock.

In memory's lane, timers start,

Bringing precision, playing its part.

Syncing transcripts, a task so grand,

Now handled better, by our code's hand.

Celebrate these changes, oh so fine,

For our code's rhythm is now in line. 🎉🤖


Uplevel your code reviews with Entelligence.AI Pro

Entelligence.AI Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, Entelligence.AI Pro is free for open source projects.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Image description Entelligence.AI

Commits Files that changed from the base of the PR and between 5fa5053 and 3b78f2b commits.
Files selected (2)
  • backend/models/processing_memory.py (1)
  • backend/routers/transcribe.py (5)
Review comments generated (4)
  • Review: 3
  • LGTM: 1

Tips

Chat with Image description Entelligence.AI Bot (@Entelligence.AI)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @Entelligence.AI in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @Entelligence.AI: ignore anywhere in the PR description to pause further reviews from the bot.

@@ -14,6 +14,7 @@ class ProcessingMemory(BaseModel):
audio_url: Optional[str] = None
created_at: datetime
timer_start: float
timer_starts: List[float] = []

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The addition of timer_starts attribute seems fine. However, it's important to ensure that this list is managed properly throughout the codebase. If the list grows indefinitely, it could lead to memory issues. Consider implementing a mechanism to limit its size or clean up old entries if not needed.

-    timer_starts: List[float] = []
+    timer_starts: List[float] = []  # Ensure proper management to avoid potential memory issues

Comment on lines +79 to +81
if delta_seconds > 0:
new_segment.start += delta_seconds
new_segment.end += delta_seconds

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The code is modifying the start and end attributes of new_segment directly. This could lead to unexpected behavior if these objects are used elsewhere in the code, as they are being mutated here. Consider creating a new object with the updated values instead of mutating the existing one.

-            new_segment.start += delta_seconds
-            new_segment.end += delta_seconds
+            adjusted_segment = Segment(start=new_segment.start + delta_seconds, end=new_segment.end + delta_seconds)

Comment on lines +322 to +324
# Track timer start
processing_memory.timer_starts.append(timer_start)

Choose a reason for hiding this comment

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

Image description Entelligence.AI

You're appending timer_start to processing_memory.timer_starts, but there's no check to ensure that timer_start is not already in the list. If it's possible for the same timer_start value to be added multiple times, you should add a check to prevent duplicates.

-        processing_memory.timer_starts.append(timer_start)
+        if timer_start not in processing_memory.timer_starts:
+            processing_memory.timer_starts.append(timer_start)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant