test: add snapshot tests for session screens and refactor global dependencies

- Add snapshot tests for SessionDetailScreen and SessionDetailsDialog
- Refactor SessionDetailsDialog to inject best_records via provider
- Refactor BestRecordsView to accept best_records parameter in render method
- Fix screen_manager render timing: render after on_pushed_from completes
- Add set_selected_session_from_index helper to RecordsScreen for testing
- Extend screen_snapshot_test macro to support on_pushed_from pattern
- Remove unused session_detail_screen_mock

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: unhappychoice

src/presentation/game/screen_manager.rs

@@ -271,9 +271,6 @@ impl ScreenManager {
             })?;
         }
 
-        // Force immediate render of the new screen
-        self.render_current_screen()?;
-
         // Flush after initializing new screen
         stdout().flush().map_err(|e| {
             GitTypeError::TerminalError(format!("Failed to flush after screen init: {}", e))
@@ -368,31 +365,33 @@ impl ScreenManager {
             ScreenTransition::None => Ok(()),
             ScreenTransition::Push(screen_type) => {
                 self.prepare_screen_if_needed(&screen_type)?;
-                self.push_screen(screen_type)
+                self.push_screen(screen_type)?;
+                self.render_current_screen()
+            }
+            ScreenTransition::Pop => {
+                self.pop_screen()?;
+                self.render_current_screen()
             }
-            ScreenTransition::Pop => self.pop_screen(),
             ScreenTransition::Replace(screen_type) => {
-                // Use ScreenTransitionManager to handle transition with side effects
                 let validated_screen_type =
                     ScreenTransitionManager::reduce(self.current_screen_type.clone(), screen_type)?;
 
                 self.prepare_screen_if_needed(&validated_screen_type)?;
-                self.set_current_screen(validated_screen_type)
+                self.set_current_screen(validated_screen_type)?;
+                self.render_current_screen()
             }
             ScreenTransition::PopTo(screen_type) => {
-                // Use ScreenTransitionManager to handle transition with side effects
                 let validated_screen_type =
                     ScreenTransitionManager::reduce(self.current_screen_type.clone(), screen_type)?;
 
                 self.prepare_screen_if_needed(&validated_screen_type)?;
-                self.pop_to_screen(validated_screen_type)
+                self.pop_to_screen(validated_screen_type)?;
+                self.render_current_screen()
             }
             ScreenTransition::Exit => {
-                // If we're already on ExitSummary, mark exit requested
                 if self.current_screen_type == ScreenType::TotalSummary {
                     self.exit_requested = true;
                 } else {
-                    // Otherwise, go to ExitSummary screen
                     let _ =
                         self.handle_transition(ScreenTransition::Replace(ScreenType::TotalSummary));
                 }
@@ -448,33 +447,10 @@ impl ScreenManager {
 
                 Ok(())
             }
-            ScreenType::SessionDetail => {
-                // Configure SessionDetail screen with data from History screen
-                self.configure_session_detail_from_history()
-            }
             _ => Ok(()),
         }
     }
 
-    fn configure_session_detail_from_history(&mut self) -> Result<()> {
-        // Get the selected session data from History screen and configure SessionDetail screen
-        self.screens
-            .get(&ScreenType::Records)
-            .and_then(|records_screen| records_screen.as_any().downcast_ref::<RecordsScreen>())
-            .and_then(|records| records.get_selected_session_for_detail().clone())
-            .map(|session_data| {
-                self.screens
-                    .get_mut(&ScreenType::SessionDetail)
-                    .and_then(|screen| screen.as_any_mut().downcast_mut::<SessionDetailScreen>())
-                    .map(|screen| screen.set_session_data(session_data))
-                    .transpose()?;
-                Ok::<_, GitTypeError>(())
-            })
-            .transpose()?;
-
-        Ok(())
-    }
-
     /// Run the ScreenManager main loop
     pub fn run(&mut self) -> Result<()> {
         // Initialize current screen and force initial render

src/presentation/game/screens/records_screen.rs

@@ -183,6 +183,12 @@ impl RecordsScreen {
         &self.selected_session_for_detail
     }
 
+    pub fn set_selected_session_from_index(&mut self, index: usize) {
+        if let Some(session) = self.sessions.get(index) {
+            self.selected_session_for_detail = Some(session.clone());
+        }
+    }
+
     fn render_session_list(&mut self, f: &mut Frame) {
         let chunks = Layout::default()
             .direction(Direction::Vertical)

src/presentation/game/screens/session_detail_screen.rs

@@ -9,12 +9,12 @@ use crate::presentation::game::views::{PerformanceMetricsView, SessionInfoView,
 use crate::presentation::game::{Screen, ScreenDataProvider, ScreenType, UpdateStrategy};
 use crate::presentation::ui::Colors;
 use crate::{GitTypeError, Result};
+use crossterm::event::{KeyCode, KeyModifiers};
 use ratatui::{
     layout::{Alignment, Constraint, Direction, Layout},
     style::{Modifier, Style},
     text::{Line, Span},
     widgets::Paragraph,
-    Frame,
 };
 
 #[derive(Debug, Clone)]
@@ -60,71 +60,6 @@ impl SessionDetailScreen {
             event_bus,
         }
     }
-
-    pub fn set_session_data(&mut self, session_data: SessionDisplayData) -> Result<()> {
-        self.session_data = session_data;
-
-        let session_repo = SessionRepository::new()?;
-        self.stage_results =
-            session_repo.get_session_stage_results(self.session_data.session.id)?;
-        self.stage_scroll_offset = 0;
-
-        Ok(())
-    }
-
-    fn ui(&mut self, f: &mut Frame) {
-        let main_chunks = Layout::default()
-            .direction(Direction::Vertical)
-            .constraints([
-                Constraint::Length(1),
-                Constraint::Min(1),
-                Constraint::Length(1),
-            ])
-            .split(f.area());
-
-        let title = Paragraph::new("Session Details")
-            .style(
-                Style::default()
-                    .fg(Colors::info())
-                    .add_modifier(Modifier::BOLD),
-            )
-            .alignment(Alignment::Left);
-        f.render_widget(title, main_chunks[0]);
-
-        let content_chunks = Layout::default()
-            .direction(Direction::Vertical)
-            .constraints([Constraint::Length(12), Constraint::Min(1)])
-            .split(main_chunks[1]);
-
-        let top_chunks = Layout::default()
-            .direction(Direction::Horizontal)
-            .constraints([Constraint::Percentage(50), Constraint::Percentage(50)])
-            .split(content_chunks[0]);
-
-        SessionInfoView::render(
-            f,
-            top_chunks[0],
-            &self.session_data.session,
-            self.session_data.repository.as_ref(),
-        );
-        PerformanceMetricsView::render(f, top_chunks[1], self.session_data.session_result.as_ref());
-        StageDetailsView::render(
-            f,
-            content_chunks[1],
-            &self.stage_results,
-            self.stage_scroll_offset,
-        );
-
-        let controls_line = Line::from(vec![
-            Span::styled("[↑↓/JK]", Style::default().fg(Colors::key_navigation())),
-            Span::styled(" Scroll Stages  ", Style::default().fg(Colors::text())),
-            Span::styled("[ESC]", Style::default().fg(Colors::error())),
-            Span::styled(" Back", Style::default().fg(Colors::text())),
-        ]);
-
-        let controls = Paragraph::new(controls_line).alignment(Alignment::Center);
-        f.render_widget(controls, main_chunks[2]);
-    }
 }
 
 pub struct SessionDetailScreenDataProvider;
@@ -147,26 +82,48 @@ impl Screen for SessionDetailScreen {
         Box::new(SessionDetailScreenDataProvider)
     }
 
-    fn init_with_data(&mut self, _data: Box<dyn std::any::Any>) -> Result<()> {
+    fn init_with_data(&mut self, data: Box<dyn std::any::Any>) -> Result<()> {
+        let _ = data;
         Ok(())
     }
 
     fn on_pushed_from(&mut self, source_screen: &dyn Screen) -> Result<()> {
-        if let Some(records) = source_screen.as_any().downcast_ref::<RecordsScreen>() {
-            if let Some(session_data) = records.get_selected_session_for_detail() {
-                self.set_session_data(session_data.clone())?;
-                return Ok(());
-            }
+        let records = source_screen
+            .as_any()
+            .downcast_ref::<RecordsScreen>()
+            .ok_or_else(|| {
+                GitTypeError::ScreenInitializationError(
+                    "SessionDetail must be pushed from Records screen".to_string(),
+                )
+            })?;
+
+        let session_data = records
+            .get_selected_session_for_detail()
+            .as_ref()
+            .cloned()
+            .ok_or_else(|| {
+                GitTypeError::ScreenInitializationError(
+                    "SessionDetail requires selected session data from Records screen".to_string(),
+                )
+            })?;
+
+        let session_repo = SessionRepository::new()?;
+        let stage_results = session_repo.get_session_stage_results(session_data.session.id)?;
+
+        self.session_data = session_data;
+        self.stage_results = stage_results;
+        self.stage_scroll_offset = 0;
+
+        if self.session_data.session.id == 0 {
+            return Err(GitTypeError::ScreenInitializationError(
+                "SessionDetail: session id cannot be 0".to_string(),
+            ));
         }
 
-        Err(GitTypeError::ScreenInitializationError(
-            "SessionDetail must be pushed from Records screen with selected session".to_string(),
-        ))
+        Ok(())
     }
 
     fn handle_key_event(&mut self, key_event: crossterm::event::KeyEvent) -> Result<()> {
-        use crossterm::event::{KeyCode, KeyModifiers};
-
         match key_event.code {
             KeyCode::Esc => {
                 self.event_bus.publish(NavigateTo::Pop);
@@ -193,7 +150,62 @@ impl Screen for SessionDetailScreen {
     }
 
     fn render_ratatui(&mut self, frame: &mut ratatui::Frame) -> Result<()> {
-        self.ui(frame);
+        let main_chunks = Layout::default()
+            .direction(Direction::Vertical)
+            .constraints([
+                Constraint::Length(1),
+                Constraint::Min(1),
+                Constraint::Length(1),
+            ])
+            .split(frame.area());
+
+        let title = Paragraph::new("Session Details")
+            .style(
+                Style::default()
+                    .fg(Colors::info())
+                    .add_modifier(Modifier::BOLD),
+            )
+            .alignment(Alignment::Left);
+        frame.render_widget(title, main_chunks[0]);
+
+        let content_chunks = Layout::default()
+            .direction(Direction::Vertical)
+            .constraints([Constraint::Length(12), Constraint::Min(1)])
+            .split(main_chunks[1]);
+
+        let top_chunks = Layout::default()
+            .direction(Direction::Horizontal)
+            .constraints([Constraint::Percentage(50), Constraint::Percentage(50)])
+            .split(content_chunks[0]);
+
+        SessionInfoView::render(
+            frame,
+            top_chunks[0],
+            &self.session_data.session,
+            self.session_data.repository.as_ref(),
+        );
+        PerformanceMetricsView::render(
+            frame,
+            top_chunks[1],
+            self.session_data.session_result.as_ref(),
+        );
+        StageDetailsView::render(
+            frame,
+            content_chunks[1],
+            &self.stage_results,
+            self.stage_scroll_offset,
+        );
+
+        let controls_line = Line::from(vec![
+            Span::styled("[↑↓/JK]", Style::default().fg(Colors::key_navigation())),
+            Span::styled(" Scroll Stages  ", Style::default().fg(Colors::text())),
+            Span::styled("[ESC]", Style::default().fg(Colors::error())),
+            Span::styled(" Back", Style::default().fg(Colors::text())),
+        ]);
+
+        let controls = Paragraph::new(controls_line).alignment(Alignment::Center);
+        frame.render_widget(controls, main_chunks[2]);
+
         Ok(())
     }
 

src/presentation/game/screens/session_details_dialog.rs

@@ -1,6 +1,6 @@
 use crate::domain::events::EventBus;
 use crate::domain::models::SessionResult;
-use crate::domain::repositories::session_repository::{BestStatus, SessionRepository};
+use crate::domain::repositories::session_repository::{BestRecords, BestStatus, SessionRepository};
 use crate::presentation::game::events::NavigateTo;
 use crate::presentation::game::views::{
     BestRecordsView, ControlsView, HeaderView, StageResultsView,
@@ -19,6 +19,7 @@ pub struct SessionDetailsDialogData {
     pub session_result: Option<SessionResult>,
     pub repo_info: Option<GitRepository>,
     pub best_status: Option<BestStatus>,
+    pub best_records: Option<BestRecords>,
 }
 
 pub struct SessionDetailsDialogDataProvider {
@@ -54,10 +55,13 @@ impl ScreenDataProvider for SessionDetailsDialogDataProvider {
             None
         };
 
+        let best_records = SessionRepository::get_best_records_global().ok().flatten();
+
         Ok(Box::new(SessionDetailsDialogData {
             session_result,
             repo_info,
             best_status,
+            best_records,
         }))
     }
 }
@@ -66,6 +70,7 @@ pub struct SessionDetailsDialog {
     session_result: Option<SessionResult>,
     repo_info: Option<GitRepository>,
     best_status: Option<BestStatus>,
+    best_records: Option<BestRecords>,
     event_bus: EventBus,
 }
 
@@ -75,6 +80,7 @@ impl SessionDetailsDialog {
             session_result: None,
             repo_info: None,
             best_status: None,
+            best_records: None,
             event_bus,
         }
     }
@@ -87,7 +93,7 @@ impl SessionDetailsDialog {
 
         // Calculate required content height dynamically
         let stage_count = session_result.stage_results.len();
-        let best_records_lines = if let Ok(Some(_)) = SessionRepository::get_best_records_global() {
+        let best_records_lines = if self.best_records.is_some() {
             5 // Header + 3 records + padding
         } else {
             3 // Just header and no records message
@@ -142,11 +148,12 @@ impl SessionDetailsDialog {
             ])
             .split(main_chunks[2]);
 
-        BestRecordsView::render_with_best_status(
+        BestRecordsView::render(
             f,
             content_chunks[0],
             session_result,
             self.best_status.as_ref(),
+            self.best_records.as_ref(),
         );
         StageResultsView::render(f, content_chunks[1], session_result, &self.repo_info);
         ControlsView::render(f, main_chunks[4]);
@@ -174,6 +181,7 @@ impl Screen for SessionDetailsDialog {
         self.session_result = dialog_data.session_result;
         self.repo_info = dialog_data.repo_info;
         self.best_status = dialog_data.best_status;
+        self.best_records = dialog_data.best_records;
 
         Ok(())
     }