Update app.py

app.py

@@ -1,5 +1,8 @@
 import streamlit as st
 from pathlib import Path # Ensure Path is imported if used for settings.LOGO_PATH
+# Make sure sqlalchemy.orm.exc is available if you want to catch DetachedInstanceError specifically
+from sqlalchemy.orm.exc import DetachedInstanceError as SQLAlchemyDetachedInstanceError
+
 
 from config.settings import settings
 from models import create_db_and_tables, get_session_context, User, ChatMessage, ChatSession
@@ -28,10 +31,10 @@ init_db()
 
 # --- Session State Initialization ---
 if 'authenticated_user' not in st.session_state:
-    st.session_state.authenticated_user = None # Stores User object (ideally one that is "live" or has loaded attributes)
+    st.session_state.authenticated_user = None
 if 'current_chat_session_id' not in st.session_state:
     st.session_state.current_chat_session_id = None
-if 'chat_messages' not in st.session_state: # For the current active chat
+if 'chat_messages' not in st.session_state:
     st.session_state.chat_messages = []
 
 
@@ -39,50 +42,41 @@ if 'chat_messages' not in st.session_state: # For the current active chat
 def display_login_form():
     with st.form("login_form"):
         st.subheader("Login")
-        # Use unique keys for inputs if there's any chance of collision or for better state tracking
         username_input = st.text_input("Username", key="login_username_input")
         password_input = st.text_input("Password", type="password", key="login_password_input")
         submit_button = st.form_submit_button("Login")
 
         if submit_button:
-            # IMPORTANT: 'authenticate_user' should return a User object where essential attributes
-            # like 'id' and 'username' are already loaded (not expired).
-            # If 'authenticate_user' involves a commit (e.g., updating last_login) and
-            # session.expire_on_commit=True (default), it should call db.refresh(user_object)
-            # before closing its session and returning the user object.
             user_from_auth = authenticate_user(username_input, password_input)
 
             if user_from_auth:
-                # FIX 1: For the success message, use the 'username_input' from the form directly.
-                # This avoids accessing 'user_from_auth.username' which might be on a detached/expired instance.
+                # Success message uses the username from form input, which is safe.
                 st.success(f"Welcome back, {username_input}!")
                 app_logger.info(f"User {username_input} logged in successfully.")
 
                 try:
                     with get_session_context() as db_session:
-                        # FIX 2: Re-fetch or merge the user into the current db_session to ensure it's "live"
-                        # This assumes 'user_from_auth.id' is accessible (i.e., was loaded by authenticate_user).
-                        # If 'user_from_auth.id' itself is expired, 'authenticate_user' MUST be fixed.
-                        try:
-                            user_id = user_from_auth.id
-                        except AttributeError:
-                            st.error("Authentication error: User data is incomplete. Please try again.")
-                            app_logger.error(f"User object from authenticate_user for '{username_input}' lacks 'id' attribute or it's inaccessible.")
-                            st.session_state.authenticated_user = None # Clear potentially bad state
-                            st.rerun()
-                            return
+                        # WORKAROUND for DetachedInstanceError from user_from_auth.id:
+                        # Re-fetch the user by username within this new session.
+                        # This ensures 'live_user' is attached and its attributes are loaded.
+                        # The PREFERRED FIX is in 'services.auth.authenticate_user' to return
+                        # a User object with its 'id' (and other essential attributes) already loaded/refreshed.
+                        app_logger.info(f"Attempting to fetch live user details for '{username_input}' in new session.")
+                        live_user = db_session.query(User).filter(User.username == username_input).first()
 
-                        live_user = db_session.get(User, user_id)
                         if not live_user:
-                            st.error("Critical user session error. Please log out and log in again.")
-                            app_logger.error(f"Failed to re-fetch user with id {user_id} (username: {username_input}) in new session.")
-                            st.session_state.authenticated_user = None # Clear broken state
-                            st.rerun()
+                            # This case means authentication (somehow) passed,
+                            # but the user is not findable by username immediately after.
+                            # This indicates a more serious data consistency issue or flaw in authenticate_user.
+                            st.error("Authentication inconsistency. User details not found after successful login. Please contact support.")
+                            app_logger.error(f"CRITICAL: User '{username_input}' authenticated but then not found by username in new session context.")
+                            st.session_state.authenticated_user = None # Clear any partial auth state
+                            st.rerun() # Rerun to show login page again
                             return
 
-                        # Store the "live" user object (attached to db_session or with loaded attributes)
-                        # in session_state. This 'live_user' will be used by the sidebar.
+                        # Store the "live" user object (attached to db_session and attributes loaded)
                         st.session_state.authenticated_user = live_user
+                        app_logger.info(f"Live user '{live_user.username}' (ID: {live_user.id}) fetched and stored in session_state.")
 
                         # Now use 'live_user' for creating the chat session
                         new_chat_session = ChatSession(user_id=live_user.id, title=f"Session for {live_user.username}")
@@ -91,11 +85,27 @@ def display_login_form():
                         db_session.refresh(new_chat_session) # Refresh to get DB-generated values for new_chat_session
                         st.session_state.current_chat_session_id = new_chat_session.id
                         st.session_state.chat_messages = [] # Clear previous messages
+                        app_logger.info(f"New chat session (ID: {new_chat_session.id}) created for user {live_user.username}.")
                     st.rerun() # Rerun to reflect login state and navigate
+
+                except SQLAlchemyDetachedInstanceError as detached_error:
+                    # This specific catch might be less likely if the above workaround (re-fetching live_user) is effective.
+                    # However, it's good for diagnostics if the issue persists due to other detached instances.
+                    app_logger.error(
+                        f"DetachedInstanceError occurred during session setup for user '{username_input}'. "
+                        f"This typically means an object from 'authenticate_user' or 'create_user_in_db' "
+                        f"had expired attributes. Error: {detached_error}",
+                        exc_info=True
+                    )
+                    st.error(
+                        "Could not start a new session due to an object state issue. "
+                        "Please ensure user services return objects with loaded attributes."
+                    )
+                    st.session_state.authenticated_user = None
                 except Exception as e:
                     app_logger.error(f"Error creating chat session for user {username_input}: {e}", exc_info=True)
                     st.error(f"Could not start a new session: {e}")
-                    # Optionally, clear authenticated_user if session creation is critical
+                    # Optionally, clear authenticated_user if session creation is critical and failed
                     # st.session_state.authenticated_user = None
                     # st.rerun()
             else:
@@ -123,19 +133,13 @@ def display_signup_form():
                     email=new_email if new_email else None
                 )
                 # 'create_user_in_db' should return a User object with attributes loaded
-                # (e.g., by calling session.refresh(user) before its session closes).
+                # (e.g., by calling session.refresh(user) before its session closes if a commit occurred).
                 user = create_user_in_db(user_data)
                 if user:
-                    # Use 'new_username' (from form input) for the message to avoid DetachedInstanceError
                     st.success(f"Account created for {new_username}. Please log in.")
                     app_logger.info(f"Account created for {new_username}.")
-                    # Optionally log them in directly:
-                    # To do this safely, 'user' returned by create_user_in_db must be "live"
-                    # or its attributes fully loaded. You might need to re-fetch it in a new session here
-                    # if you intend to store it in st.session_state.authenticated_user.
-                    # For now, redirecting to login is simpler.
                 else:
-                    st.error("Username might already be taken or an error occurred during signup.")
+                    st.error("Username might already be taken or another error occurred during signup.")
                     app_logger.warning(f"Signup failed for username: {new_username}")
 
 # --- Main App Logic ---
@@ -149,19 +153,15 @@ if not st.session_state.authenticated_user:
     with signup_tab:
         display_signup_form()
 else:
-    # If authenticated
     with st.sidebar:
-        # Accessing st.session_state.authenticated_user.username.
-        # This relies on the object stored in authenticated_user having its username loaded.
-        # The `display_login_form` now stores the 'live_user' which should have this.
         try:
+            # This relies on st.session_state.authenticated_user being a "live" object
+            # with 'username' attribute accessible. The login logic now aims to ensure this.
             st.markdown(f"### Welcome, {st.session_state.authenticated_user.username}!")
-        except AttributeError as e: # Fallback if username somehow isn't accessible
+        except AttributeError as e:
              st.markdown(f"### Welcome!")
-             app_logger.error(f"Could not access username for authenticated_user in sidebar: {e}. User object: {st.session_state.authenticated_user}")
+             app_logger.error(f"Could not access username for authenticated_user in sidebar: {e}. User object: {st.session_state.authenticated_user}", exc_info=True)
 
-
-        # Example for conditional logo display
         logo_path_str = getattr(settings, "LOGO_PATH", None)
         if logo_path_str:
             logo_path = Path(logo_path_str)
@@ -172,24 +172,26 @@ else:
                     app_logger.warning(f"Could not load logo from {logo_path_str}: {e}")
             else:
                 app_logger.warning(f"Logo path specified but does not exist: {logo_path_str}")
-        elif settings.APP_TITLE: # Fallback to title if no logo
+        elif settings.APP_TITLE:
              st.markdown(f"#### {settings.APP_TITLE}")
 
         st.markdown("---")
 
         if st.button("Logout"):
-            if hasattr(st.session_state.authenticated_user, 'username'):
-                app_logger.info(f"User {st.session_state.authenticated_user.username} logging out.")
-            else:
-                app_logger.info("User logging out (username not accessible from session state object).")
+            username_on_logout = "UnknownUser"
+            try:
+                username_on_logout = st.session_state.authenticated_user.username
+            except Exception:
+                app_logger.warning("Could not get username during logout from session state object.")
+            app_logger.info(f"User {username_on_logout} logging out.")
+
             st.session_state.authenticated_user = None
             st.session_state.current_chat_session_id = None
             st.session_state.chat_messages = []
             st.success("You have been logged out.")
             st.rerun()
 
-    # Main area content (could be your "Home" page if no `pages/1_Home.py`)
-    st.sidebar.success("Select a page from the navigation.") # More generic message
+    st.sidebar.success("Select a page from the navigation.")
     st.header(f"Dashboard - {settings.APP_TITLE}")
     st.markdown("Navigate using the sidebar to consult with the AI or view your reports.")
     st.markdown("---")