refactor: remove redundant browser_state update methods (#9)

- Remove unused update_browser_state_audio_status method
- Remove redundant update_browser_state_extracted_text method
- Improve ensure_browser_state_completeness to handle empty app_session_id
- Consolidate browser_state management to use UserSession as single source of truth
- Update tests to reflect new behavior where UserSession defaults are used to complete browser_state
- All unit tests and E2E tests passing

This change eliminates duplicate default value definitions and improves code maintainability
by centralizing browser_state management in the UserSession class.

tests/unit/test_browser_state_management.py

@@ -46,8 +46,17 @@ class TestBrowserStateManagement:
         assert user_session.session_id == existing_session_id
         assert updated_browser_state["app_session_id"] == existing_session_id
 
-        # Browser state should remain unchanged
-        assert updated_browser_state == browser_state_with_session
+        # Browser state should be completed with UserSession defaults while preserving existing values
+        # Check that original values are preserved
+        assert updated_browser_state["ui_state"]["podcast_text"] == "Previous session content"
+        assert updated_browser_state["ui_state"]["terms_agreed"] is True
+        assert updated_browser_state["user_settings"]["current_api_type"] == "openai"
+        assert updated_browser_state["user_settings"]["character1"] == "Kyushu Sora"
+
+        # Check that missing fields are filled with defaults
+        assert "current_script" in updated_browser_state["audio_generation_state"]
+        assert "character2" in updated_browser_state["user_settings"]
+        assert "document_type" in updated_browser_state["user_settings"]
 
     def test_session_independence_from_gradio_hash(self):
         """Test that app session ID remains stable even when Gradio session hash changes."""

tests/unit/test_ui_initialization.py

@@ -212,62 +212,24 @@ class TestAudioStateUpdate:
         """Set up test fixtures."""
         self.app = PaperPodcastApp()
 
-    def test_update_browser_state_audio_status(self):
-        """Test updating audio status in BrowserState."""
-        from yomitalk.user_session import UserSession
-
-        user_session = UserSession()
+    def test_update_browser_state_ui_content(self):
+        """Test updating UI content in BrowserState."""
         browser_state = {
-            "app_session_id": user_session.session_id,
-            "audio_generation_state": {
-                "is_generating": True,
-                "progress": 0.5,
-                "status": "generating",
-                "current_script": "Test script",
-                "final_audio_path": None,
-                "streaming_parts": ["part1.wav"],
-                "generation_id": "gen123",
-            },
+            "app_session_id": "test-session",
+            "audio_generation_state": {},
             "user_settings": {},
-            "ui_state": {},
+            "ui_state": {"podcast_text": "old text", "terms_agreed": False},
         }
 
-        # Update the browser state audio status
-        updated_state = self.app.update_browser_state_audio_status(user_session, browser_state)
-
-        # Verify the audio generation state is preserved/updated
-        audio_state = updated_state["audio_generation_state"]
-        assert "is_generating" in audio_state
-        assert "progress" in audio_state
-        assert "status" in audio_state
-        assert "current_script" in audio_state
-
-    def test_update_browser_state_audio_status_with_none_session(self):
-        """Test audio status update handles None user session gracefully."""
-        browser_state = {"app_session_id": "test-uuid", "audio_generation_state": {}, "user_settings": {}, "ui_state": {}}
-
-        # Should handle None session gracefully - create dummy user session for testing
-        user_session = Mock()
-        user_session.session_id = "test-uuid"
-        user_session.is_audio_generating.return_value = False
-        user_session.get_audio_generation_progress.return_value = 0.0
-        user_session.get_audio_generation_status.return_value = {
-            "is_generating": False,
-            "progress": 0.0,
-            "status": "idle",
-            "current_script": "",
-            "final_audio_path": None,
-            "streaming_parts": [],
-            "generation_id": None,
-            "start_time": None,
-            "estimated_total_parts": 1,
-        }
-        user_session.get_current_script.return_value = ""
-        user_session.get_final_audio_path.return_value = None
-        user_session.get_streaming_audio_parts.return_value = []
+        # Update UI content
+        updated_state = self.app.update_browser_state_ui_content(browser_state, "new podcast text", True)
 
-        updated_state = self.app.update_browser_state_audio_status(user_session, browser_state)
+        # Verify UI state was updated
+        ui_state = updated_state["ui_state"]
+        assert ui_state["podcast_text"] == "new podcast text"
+        assert ui_state["terms_agreed"] is True
 
-        # Should return updated state with audio information
-        assert updated_state is not browser_state  # Different object
+        # Verify other sections are preserved
+        assert updated_state["app_session_id"] == "test-session"
         assert "audio_generation_state" in updated_state
+        assert "user_settings" in updated_state

yomitalk/app.py

@@ -64,37 +64,26 @@ class PaperPodcastApp:
             # Restore settings from browser state
             user_session.update_settings_from_browser_state(browser_state)
 
-            # Return browser state as-is since it contains all needed state
-            return user_session, browser_state
+            # Ensure browser_state completeness using user_session defaults
+            complete_browser_state = user_session.ensure_browser_state_completeness(browser_state)
+
+            return user_session, complete_browser_state
         else:
             # Create new session with UUID-based ID
             user_session = UserSession()  # Will generate new UUID
             logger.info(f"User session initialized: {user_session.session_id}")
 
-            # Initialize browser state with new session ID
-            browser_state["app_session_id"] = user_session.session_id
-
-            # Sync current settings to browser state
-            browser_state = user_session.sync_settings_to_browser_state(browser_state)
-
-            return user_session, browser_state
-
-    def update_browser_state_audio_status(self, user_session: UserSession, browser_state: Dict[str, Any]) -> Dict[str, Any]:
-        """Update BrowserState with current audio generation status."""
-        if user_session is None:
-            return browser_state.copy()
-
-        audio_status = user_session.get_audio_generation_status(browser_state)
+            # Use user_session's default state structure to complete browser_state
+            complete_browser_state = user_session.ensure_browser_state_completeness(browser_state)
 
-        # Create a copy of browser state and update audio generation section
-        updated_state = browser_state.copy()
-        updated_state["audio_generation_state"] = browser_state["audio_generation_state"].copy()
-        updated_state["audio_generation_state"].update(audio_status)
-
-        return updated_state
+            return user_session, complete_browser_state
 
     def update_browser_state_ui_content(self, browser_state: Dict[str, Any], podcast_text: str, terms_agreed: bool, extracted_text: str = "") -> Dict[str, Any]:
-        """Update BrowserState with UI content for recovery."""
+        """Update BrowserState with UI content for recovery.
+
+        Note: This method is being phased out in favor of UserSession.ensure_browser_state_completeness.
+        Consider using user_session.ensure_browser_state_completeness instead for new code.
+        """
         updated_state = browser_state.copy()
 
         # Update ui_state section in the new BrowserState structure
@@ -1421,34 +1410,8 @@ class PaperPodcastApp:
                 )
 
             # Initialize BrowserState for persistent session management - stores all session data in localStorage
-            browser_state = gr.BrowserState(
-                {
-                    "app_session_id": "",  # App-generated persistent session ID
-                    "audio_generation_state": {
-                        "is_generating": False,
-                        "progress": 0.0,
-                        "status": "idle",
-                        "current_script": "",
-                        "final_audio_path": None,
-                        "streaming_parts": [],
-                        "generation_id": None,
-                        "start_time": None,
-                        "estimated_total_parts": 1,
-                    },
-                    "user_settings": {
-                        "current_api_type": "gemini",
-                        "document_type": "research_paper",
-                        "podcast_mode": "academic",
-                        "character1": "Zundamon",
-                        "character2": "Shikoku Metan",
-                        "openai_max_tokens": 4000,
-                        "gemini_max_tokens": 8000,
-                        "openai_model": "gpt-4o-mini",
-                        "gemini_model": "gemini-1.5-flash",
-                    },
-                    "ui_state": {"podcast_text": "", "terms_agreed": False},
-                }
-            )
+            # Start with empty dict and let initialize_session_and_ui populate with proper defaults
+            browser_state = gr.BrowserState({})
             # Initialize regular State for UserSession object (not serializable to localStorage)
             user_session = gr.State()
 
@@ -1736,13 +1699,8 @@ class PaperPodcastApp:
                 outputs=[generate_btn, browser_state],
             )
 
-            # extracted_textの変更時にもbrowser_stateを更新
-            extracted_text.change(
-                fn=self.update_browser_state_extracted_text,
-                inputs=[extracted_text, browser_state],
-                outputs=[browser_state],
-                queue=False,
-            )
+            # Note: extracted_text changes don't need to update browser_state
+            # as extracted_text is temporary and not persisted
 
         return app
 
@@ -1962,10 +1920,6 @@ class PaperPodcastApp:
 
         return button_update, updated_browser_state
 
-    def update_browser_state_extracted_text(self, extracted_text: str, browser_state: Dict[str, Any]) -> Dict[str, Any]:
-        """Update browser state with extracted text changes."""
-        return self.update_browser_state_ui_content(browser_state, browser_state.get("podcast_text", ""), browser_state.get("terms_agreed", False))
-
     def set_document_type(self, doc_type: str, user_session: UserSession, browser_state: Dict[str, Any]) -> Tuple[UserSession, Dict[str, Any]]:
         """
         ドキュメントタイプを設定します。
@@ -2120,10 +2074,6 @@ class PaperPodcastApp:
                     updated_browser_state["audio_generation_state"]["final_audio_path"] = final_audio
                     updated_browser_state["audio_generation_state"]["status"] = "completed"
 
-        # Update BrowserState with current session status and UI content
-        updated_browser_state = self.update_browser_state_audio_status(user_session, updated_browser_state)
-        updated_browser_state = self.update_browser_state_ui_content(updated_browser_state, restored_podcast_text, restored_terms_agreed)
-
         # Step 4: Create UI component updates (enable all components)
         logger.info(f"Enabling UI components for session {user_session.session_id}")
         logger.debug(f"UI sync values: document_type={document_type}, podcast_mode={podcast_mode}, character1={character1}, character2={character2}")

yomitalk/user_session.py

@@ -282,6 +282,84 @@ class UserSession:
 
         return browser_state
 
+    def get_default_browser_state_structure(self) -> Dict[str, Any]:
+        """Get default browser state structure based on current user session settings.
+
+        This method provides the single source of truth for default values,
+        preventing duplication between user_session and browser_state defaults.
+
+        Returns:
+            Dict[str, Any]: Default browser state structure with current session values
+        """
+        return {
+            "app_session_id": self.session_id,
+            "audio_generation_state": {
+                "is_generating": False,
+                "progress": 0.0,
+                "status": "idle",
+                "current_script": "",
+                "final_audio_path": None,
+                "streaming_parts": [],
+                "generation_id": None,
+                "start_time": None,
+                "estimated_total_parts": 1,
+            },
+            "user_settings": {
+                "current_api_type": self.text_processor.current_api_type.name.lower() if self.text_processor.current_api_type else "gemini",
+                "document_type": self.text_processor.prompt_manager.current_document_type.value,
+                "podcast_mode": self.text_processor.prompt_manager.current_mode.value,
+                "character1": self.text_processor.prompt_manager.char_mapping.get("Character1", "Zundamon"),
+                "character2": self.text_processor.prompt_manager.char_mapping.get("Character2", "Shikoku Metan"),
+                "openai_max_tokens": self.text_processor.openai_model.get_max_tokens(),
+                "gemini_max_tokens": self.text_processor.gemini_model.get_max_tokens(),
+                "openai_model": self.text_processor.openai_model.model_name,
+                "gemini_model": self.text_processor.gemini_model.model_name,
+            },
+            "ui_state": {"podcast_text": "", "terms_agreed": False},
+        }
+
+    def ensure_browser_state_completeness(self, browser_state: Dict[str, Any]) -> Dict[str, Any]:
+        """Ensure browser_state has complete structure using user_session defaults.
+
+        This method merges incomplete browser_state with user_session-based defaults,
+        giving priority to user_session values when browser_state is missing fields.
+
+        Args:
+            browser_state (Dict[str, Any]): Current browser state (might be incomplete)
+
+        Returns:
+            Dict[str, Any]: Complete browser state with user_session defaults applied
+        """
+        # Get default structure from current user session state
+        default_structure = self.get_default_browser_state_structure()
+
+        # If browser_state is completely empty, use defaults
+        if not browser_state:
+            return default_structure
+
+        # Merge browser_state into defaults - this gives priority to user_session defaults
+        # for missing fields while preserving existing browser_state values
+        result = default_structure.copy()
+
+        # Recursively merge dictionaries, keeping browser_state values where they exist
+        def deep_merge(default_dict: dict, browser_dict: dict) -> dict:
+            merged = default_dict.copy()
+            for key, value in browser_dict.items():
+                if key in merged and isinstance(merged[key], dict) and isinstance(value, dict):
+                    merged[key] = deep_merge(merged[key], value)
+                else:
+                    merged[key] = value
+            return merged
+
+        merged_result = deep_merge(result, browser_state)
+
+        # Special case: always ensure app_session_id matches current session
+        # This handles the case where browser_state has empty session_id
+        if not merged_result.get("app_session_id"):
+            merged_result["app_session_id"] = self.session_id
+
+        return merged_result
+
     # Temporary compatibility methods for tests - will be removed
     @property
     def audio_generation_state(self) -> Dict[str, Any]: