Skip to content

Commit 4de0331

Browse files
feat(ui): add duplicate session ID prevention (#148)
Implements a real-time check on the Session ID input field to prevent accidental data overwrites. - Adds a validation function that checks if a session ID already exists on the filesystem. - Displays a clear, non-blocking warning to the user if a duplicate ID is detected. - Implements an in-memory, time-based cache to ensure the validation check is performant and does not cause UI lag, addressing a key performance concern. - Includes comprehensive unit tests for both the duplicate detection and the caching mechanism. This change directly addresses task UX-04. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent 3de87fe commit 4de0331

2 files changed

Lines changed: 89 additions & 4 deletions

File tree

src/ui/process_session_helpers.py

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
- `src.status_tracker`: Status tracking infrastructure
4848
"""
4949

50-
from datetime import datetime
50+
from datetime import datetime, timedelta
5151
from pathlib import Path
5252
import re
5353
from typing import Any, Callable, Dict, List, Optional, Tuple
@@ -56,6 +56,7 @@
5656
from src.party_config import PartyConfigManager
5757
from src.file_tracker import FileProcessingTracker
5858
from src.status_tracker import StatusTracker
59+
from src.session_manager import SessionManager
5960
from src.ui.constants import StatusIndicators as SI
6061
from src.ui.helpers import StatusMessages
6162

@@ -64,6 +65,17 @@
6465
ALLOWED_AUDIO_EXTENSIONS: Tuple[str, ...] = (".m4a", ".mp3", ".wav", ".flac")
6566
SESSION_ID_PATTERN = re.compile(r"^[A-Za-z0-9_-]+$")
6667

68+
# Cache for session IDs to avoid frequent I/O in real-time validation
69+
# Why: The session ID validation runs on every keystroke. Hitting the filesystem
70+
# to check for duplicates each time would cause significant UI lag, especially
71+
# with many sessions. This cache provides a near-instant check, refreshing
72+
# only periodically.
73+
_session_id_cache = {
74+
"timestamp": None,
75+
"ids": set(),
76+
}
77+
CACHE_TTL_SECONDS = 5 # Cache session IDs for 5 seconds
78+
6779

6880
def validate_session_id_realtime(session_id: str) -> str:
6981
"""
@@ -82,9 +94,7 @@ def validate_session_id_realtime(session_id: str) -> str:
8294
session_id = session_id.strip()
8395

8496
# Check against pattern
85-
if SESSION_ID_PATTERN.match(session_id):
86-
return f"### {SI.SUCCESS} Valid\n\n[v] Session ID `{session_id}` is valid."
87-
else:
97+
if not SESSION_ID_PATTERN.match(session_id):
8898
# Find invalid characters, ensuring they are ASCII-compatible to match the regex
8999
invalid_chars = {
90100
char for char in session_id
@@ -105,6 +115,32 @@ def validate_session_id_realtime(session_id: str) -> str:
105115
"[x] Session ID format is invalid. Use only letters, numbers, underscores (_), and hyphens (-)."
106116
)
107117

118+
# UX-04: Add duplicate Session ID prevention with caching.
119+
# Why: To prevent UI lag from frequent filesystem access, we check against a
120+
# cached set of session IDs.
121+
now = datetime.now()
122+
cache_is_stale = (
123+
_session_id_cache["timestamp"] is None or
124+
(now - _session_id_cache["timestamp"]).total_seconds() > CACHE_TTL_SECONDS
125+
)
126+
127+
if cache_is_stale:
128+
# Why: This block refreshes the cache if it's empty or has expired.
129+
# This is the only time the filesystem is accessed during validation.
130+
session_manager = SessionManager()
131+
_session_id_cache["ids"] = {s.session_id for s in session_manager.discover_sessions()}
132+
_session_id_cache["timestamp"] = now
133+
134+
if session_id in _session_id_cache["ids"]:
135+
return (
136+
f"### {SI.WARNING} Duplicate Session ID\n\n"
137+
f"[!] Session ID `{session_id}` already exists.\n\n"
138+
"Reusing an ID will overwrite existing data. It is recommended to use a unique ID."
139+
)
140+
141+
# If all checks pass, the ID is valid
142+
return f"### {SI.SUCCESS} Valid\n\n[v] Session ID `{session_id}` is valid."
143+
108144

109145
def validate_processing_readiness(
110146
audio_file,

tests/ui/test_process_session_helpers.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,55 @@ def test_whitespace_session_id(self):
820820
result = validate_session_id_realtime(" ")
821821
assert result == ""
822822

823+
@patch('src.ui.process_session_helpers.SessionManager')
824+
def test_duplicate_session_id(self, mock_session_manager):
825+
"""A session ID that already exists should return a warning."""
826+
# This test ensures the core functionality of UX-04, preventing overwrites.
827+
mock_instance = mock_session_manager.return_value
828+
mock_session = MagicMock()
829+
mock_session.session_id = "existing_session"
830+
mock_instance.discover_sessions.return_value = [mock_session]
831+
832+
# Invalidate the cache by resetting its timestamp
833+
from src.ui import process_session_helpers
834+
process_session_helpers._session_id_cache["timestamp"] = None
835+
836+
result = validate_session_id_realtime("existing_session")
837+
838+
assert "Duplicate Session ID" in result
839+
assert "`existing_session` already exists" in result
840+
841+
@patch('src.ui.process_session_helpers.SessionManager')
842+
def test_session_id_caching_behavior(self, mock_session_manager):
843+
"""Test that the session ID cache is used to prevent excessive I/O."""
844+
# Why this test is important:
845+
# This test directly addresses the performance feedback from the code
846+
# review. It ensures that the caching mechanism works, preventing the
847+
# expensive `discover_sessions` call from running on every keystroke,
848+
# which would cause UI lag.
849+
850+
# Arrange: Mock the SessionManager.
851+
mock_instance = mock_session_manager.return_value
852+
mock_session = MagicMock()
853+
mock_session.session_id = "cached_session"
854+
mock_instance.discover_sessions.return_value = [mock_session]
855+
856+
# Invalidate the cache to ensure the first call hits the mock.
857+
from src.ui import process_session_helpers
858+
process_session_helpers._session_id_cache["timestamp"] = None
859+
860+
# Act 1: First call should populate the cache.
861+
validate_session_id_realtime("cached_session")
862+
863+
# Assert 1: The mock should have been called once.
864+
mock_instance.discover_sessions.assert_called_once()
865+
866+
# Act 2: Second call should use the cache, not the mock.
867+
validate_session_id_realtime("cached_session")
868+
869+
# Assert 2: The mock call count should still be one.
870+
mock_instance.discover_sessions.assert_called_once()
871+
823872
def test_update_with_nonexistent_party(self):
824873
"""Test updating display with nonexistent party."""
825874
with patch('src.ui.process_session_helpers.PartyConfigManager') as mock_manager_class:

0 commit comments

Comments
 (0)