fix: review findings

- screen transitions snapshot the previous frame (ratatui swaps buffers
  after draw, so the old snapshot was always blank); frame loop extracted
  into a testable Runner
- .city loading rejects structurally inconsistent files: Grid deserialize
  checks size vs data, load_city checks header vs body and runs
  City::validate (now also checks Net link endpoints)
- saving from the map hands the live map screen back instead of a fresh
  explore screen
- signage glyphs covered by the unicode-width test

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

crates/city/src/app.rs

@@ -6,6 +6,9 @@ use anyhow::Result;
 use citygen::{City, GenParams};
 use crossterm::event::{self, Event, KeyEvent, KeyEventKind};
 use ratatui::Frame;
+use ratatui::Terminal;
+use ratatui::backend::Backend;
+use ratatui::buffer::Buffer;
 use ratatui::layout::{Alignment, Rect};
 use ratatui::style::{Color, Style};
 use ratatui::widgets::Paragraph;
@@ -90,11 +93,78 @@ pub fn draw_too_small(f: &mut Frame) {
     );
 }
 
-/// The fixed-rate loop: poll input, tick, update, draw.
-pub fn run(mut screen: Box<dyn Screen>, mut st: AppState) -> Result<()> {
+/// One screen plus the state the loop threads through frames.
+pub struct Runner {
+    screen: Box<dyn Screen>,
+    pub st: AppState,
+    transition: Option<Transition>,
+    /// Our own copy of the last drawn frame; ratatui swaps buffers after every draw.
+    last_frame: Option<Buffer>,
+}
+
+impl Runner {
+    pub fn new(screen: Box<dyn Screen>, st: AppState) -> Self {
+        Runner {
+            screen,
+            st,
+            transition: None,
+            last_frame: None,
+        }
+    }
+
+    #[cfg(test)]
+    pub fn transition(&self) -> Option<&Transition> {
+        self.transition.as_ref()
+    }
+
+    /// Runs one frame: tick, update with `keys`, draw. Returns false when the app quits.
+    pub fn frame<B: Backend>(&mut self, term: &mut Terminal<B>, keys: &[KeyEvent]) -> Result<bool> {
+        self.st.tick += 1;
+        if self
+            .transition
+            .as_ref()
+            .is_some_and(|t| t.active(self.st.tick))
+            && !keys.is_empty()
+        {
+            self.transition = None;
+        }
+        match self.screen.update(&mut self.st, keys) {
+            ScreenCmd::Stay => {}
+            ScreenCmd::Switch { to, fx } => {
+                let snapshot = self
+                    .last_frame
+                    .clone()
+                    .unwrap_or_else(|| Buffer::empty(term.get_frame().area()));
+                self.screen = to;
+                self.transition = Some(Transition::new(fx, snapshot, self.st.tick));
+            }
+            ScreenCmd::Quit => return Ok(false),
+        }
+        let st = &self.st;
+        let screen = &self.screen;
+        let transition = &self.transition;
+        let mut captured: Option<Buffer> = None;
+        term.draw(|f| {
+            if f.area().width < MIN_W || f.area().height < MIN_H {
+                draw_too_small(f);
+            } else {
+                screen.draw(st, f);
+                if let Some(t) = transition {
+                    t.apply(f, st.tick);
+                }
+            }
+            captured = Some(f.buffer_mut().clone());
+        })?;
+        self.last_frame = captured;
+        Ok(true)
+    }
+}
+
+/// The fixed-rate loop: poll input, run a frame, sleep the remainder.
+pub fn run(screen: Box<dyn Screen>, st: AppState) -> Result<()> {
     install_panic_hook();
     let (_guard, mut term) = TerminalGuard::enter()?;
-    let mut transition: Option<Transition> = None;
+    let mut runner = Runner::new(screen, st);
     loop {
         let start = Instant::now();
         let mut keys = Vec::new();
@@ -105,29 +175,9 @@ pub fn run(mut screen: Box<dyn Screen>, mut st: AppState) -> Result<()> {
                 keys.push(k);
             }
         }
-        st.tick += 1;
-        if transition.as_ref().is_some_and(|t| t.active(st.tick)) && !keys.is_empty() {
-            transition = None;
+        if !runner.frame(&mut term, &keys)? {
+            break;
         }
-        match screen.update(&mut st, &keys) {
-            ScreenCmd::Stay => {}
-            ScreenCmd::Switch { to, fx } => {
-                let snapshot = term.current_buffer_mut().clone();
-                screen = to;
-                transition = Some(Transition::new(fx, snapshot, st.tick));
-            }
-            ScreenCmd::Quit => break,
-        }
-        term.draw(|f| {
-            if f.area().width < MIN_W || f.area().height < MIN_H {
-                draw_too_small(f);
-                return;
-            }
-            screen.draw(&st, f);
-            if let Some(t) = &transition {
-                t.apply(f, st.tick);
-            }
-        })?;
         if let Some(rest) = FRAME.checked_sub(start.elapsed()) {
             std::thread::sleep(rest);
         }
@@ -164,6 +214,42 @@ mod tests {
         assert!(text(&t).contains("100"));
     }
 
+    struct Says(&'static str, bool);
+
+    impl Screen for Says {
+        fn update(&mut self, _st: &mut AppState, keys: &[KeyEvent]) -> ScreenCmd {
+            if self.1 && !keys.is_empty() {
+                return ScreenCmd::Switch {
+                    to: Box::new(Says("WORLD", false)),
+                    fx: TransitionKind::SlideUp,
+                };
+            }
+            ScreenCmd::Stay
+        }
+        fn draw(&self, _st: &AppState, f: &mut Frame) {
+            f.render_widget(ratatui::widgets::Paragraph::new(self.0), f.area());
+        }
+    }
+
+    #[test]
+    fn switch_snapshots_the_previous_frame() {
+        let mut t = Terminal::new(TestBackend::new(100, 30)).unwrap();
+        let mut runner = Runner::new(Box::new(Says("HELLO", true)), AppState::new());
+        assert!(runner.frame(&mut t, &[]).unwrap());
+        assert!(text(&t).contains("HELLO"));
+        let key = KeyEvent::new(
+            crossterm::event::KeyCode::Enter,
+            crossterm::event::KeyModifiers::NONE,
+        );
+        assert!(runner.frame(&mut t, &[key]).unwrap());
+        let tr = runner.transition().expect("a transition is running");
+        let snapshot: String = (0..5)
+            .map(|x| tr.from[(x, 0)].symbol().to_string())
+            .collect();
+        assert_eq!(snapshot, "HELLO");
+        assert!(text(&t).contains("WORLD") || text(&t).contains("HELLO"));
+    }
+
     #[test]
     fn app_state_status_expires() {
         let mut st = AppState::new();

crates/city/src/save.rs

@@ -81,9 +81,36 @@ fn split<'a>(bytes: &'a [u8], path: &Path) -> Result<(Header, &'a [u8])> {
 
 pub fn load_city(path: &Path) -> Result<City> {
     let bytes = std::fs::read(path).with_context(|| format!("reading {}", path.display()))?;
-    let (_, body) = split(&bytes, path)?;
+    let (header, body) = split(&bytes, path)?;
     let city: City = postcard::from_bytes(body)
         .with_context(|| format!("{}: corrupt or truncated city data", path.display()))?;
+    if (header.w, header.h) != (city.size.w, city.size.h) {
+        bail!(
+            "{}: header says {}x{} but the city is {}x{}",
+            path.display(),
+            header.w,
+            header.h,
+            city.size.w,
+            city.size.h
+        );
+    }
+    let strata_ok = city.strata.len() == citygen::StratumKind::PHYSICAL.len()
+        && city
+            .strata
+            .iter()
+            .all(|s| s.tiles.size() == city.size && s.light.size() == city.size);
+    if !strata_ok {
+        bail!("{}: strata do not match the city size", path.display());
+    }
+    if let Err(violations) = city.validate() {
+        let first: Vec<String> = violations.iter().take(3).map(|v| v.to_string()).collect();
+        bail!(
+            "{}: city violates its invariants ({} problems, e.g. {})",
+            path.display(),
+            violations.len(),
+            first.join("; ")
+        );
+    }
     Ok(city)
 }
 
@@ -166,6 +193,42 @@ mod tests {
         assert!(list_cities(&dir.join("missing")).unwrap().is_empty());
     }
 
+    #[test]
+    fn rejects_structurally_inconsistent_cities() {
+        let dir = tmp("inconsistent");
+        let p = dir.join("x.city");
+        // Header disagrees with the body.
+        let c = tiny();
+        save_city(&p, &c).unwrap();
+        let mut bytes = std::fs::read(&p).unwrap();
+        let header_len = u32::from_le_bytes([bytes[6], bytes[7], bytes[8], bytes[9]]) as usize;
+        let mut header: Header = postcard::from_bytes(&bytes[10..10 + header_len]).unwrap();
+        header.w = 96;
+        let hb = postcard::to_allocvec(&header).unwrap();
+        let body = bytes.split_off(10 + header_len);
+        bytes.truncate(6);
+        bytes.extend_from_slice(&(hb.len() as u32).to_le_bytes());
+        bytes.extend_from_slice(&hb);
+        bytes.extend_from_slice(&body);
+        std::fs::write(&p, &bytes).unwrap();
+        let err = load_city(&p).unwrap_err().to_string();
+        assert!(err.contains("header"), "{err}");
+        // Body violates the city invariants (tiles reference districts that do not exist).
+        let mut broken = tiny();
+        broken.districts.clear();
+        save_city(&p, &broken).unwrap();
+        let err = load_city(&p).unwrap_err().to_string();
+        assert!(
+            err.contains("invariant") || err.contains("missing"),
+            "{err}"
+        );
+        // A grid whose data length disagrees with its size never gets past deserialization.
+        let mut odd = tiny();
+        odd.strata.pop();
+        save_city(&p, &odd).unwrap();
+        assert!(load_city(&p).is_err());
+    }
+
     #[test]
     fn rejects_bad_magic_version_and_truncation() {
         let dir = tmp("bad");

crates/city/src/screens/explore.rs

@@ -164,8 +164,10 @@ impl MapScreen {
             KeyCode::Char('?') => self.help = !self.help,
             KeyCode::Char('s') => {
                 let name = st.city.as_ref().map(|c| c.name.clone())?;
+                // The save prompt hands this very screen back, state intact.
+                let live = std::mem::replace(self, MapScreen::explore(st.tick));
                 return Some(ScreenCmd::Switch {
-                    to: Box::new(Files::save(st.tick, &name)),
+                    to: Box::new(Files::save(st.tick, &name, Some(Box::new(live)))),
                     fx: TransitionKind::None,
                 });
             }
@@ -510,12 +512,24 @@ mod tests {
         assert!(buffer_text(t.backend().buffer()).contains("move"));
         s.update(&mut st, &[key(KeyCode::Esc)]);
         assert!(!s.help);
+        // Saving while walking must bring the walk back, not a fresh explore screen.
+        s.stratum = StratumKind::Undercity;
+        let ScreenCmd::Switch { to: mut files, .. } = s.update(&mut st, &[key(KeyCode::Char('s'))])
+        else {
+            panic!("s should open the save screen");
+        };
+        let ScreenCmd::Switch { to: back, .. } = files.update(&mut st, &[key(KeyCode::Esc)]) else {
+            panic!("esc should return to the map");
+        };
+        st.tick += 500; // let transient statuses expire so the mode hints show
+        t.draw(|f| back.draw(&st, f)).unwrap();
+        let text = buffer_text(t.backend().buffer());
+        assert!(
+            text.contains("tab camera") && text.contains("Undercity"),
+            "{text}"
+        );
         s.update(&mut st, &[key(KeyCode::Tab)]);
         assert_eq!(s.mode, Mode::Explore);
-        assert!(matches!(
-            s.update(&mut st, &[key(KeyCode::Char('s'))]),
-            ScreenCmd::Switch { .. }
-        ));
         assert!(matches!(
             s.update(&mut st, &[key(KeyCode::Char('q'))]),
             ScreenCmd::Switch { .. }

crates/city/src/screens/files.rs

@@ -26,6 +26,8 @@ pub struct Files {
     entries: Vec<CityFileInfo>,
     selected: usize,
     error: Option<String>,
+    /// The screen to return to after saving; a fresh explore screen when absent.
+    back: Option<Box<dyn Screen>>,
 }
 
 /// `YYYY-MM-DD HH:MM` in UTC, without a date crate (civil-from-days algorithm).
@@ -58,8 +60,8 @@ impl Files {
         Self::load_in(default_dir(), _tick)
     }
 
-    pub fn save(tick: u64, name: &str) -> Self {
-        Self::save_in(default_dir(), tick, name)
+    pub fn save(tick: u64, name: &str, back: Option<Box<dyn Screen>>) -> Self {
+        Self::save_in(default_dir(), tick, name, back)
     }
 
     pub fn load_in(dir: PathBuf, _tick: u64) -> Self {
@@ -70,10 +72,11 @@ impl Files {
             entries,
             selected: 0,
             error: None,
+            back: None,
         }
     }
 
-    pub fn save_in(dir: PathBuf, _tick: u64, name: &str) -> Self {
+    pub fn save_in(dir: PathBuf, _tick: u64, name: &str, back: Option<Box<dyn Screen>>) -> Self {
         let form = Form {
             fields: vec![Field::Text {
                 label: "Name".into(),
@@ -89,12 +92,17 @@ impl Files {
             entries: Vec::new(),
             selected: 0,
             error: None,
+            back,
         }
     }
 
-    fn back_to_map(st: &AppState) -> ScreenCmd {
+    fn back_to_map(&mut self, st: &AppState) -> ScreenCmd {
+        let to = self
+            .back
+            .take()
+            .unwrap_or_else(|| crate::screens::explore_screen(st.tick));
         ScreenCmd::Switch {
-            to: crate::screens::explore_screen(st.tick),
+            to,
             fx: TransitionKind::None,
         }
     }
@@ -141,7 +149,7 @@ impl Screen for Files {
                 },
                 Mode::Save(form) => {
                     if k.code == KeyCode::Esc {
-                        return Self::back_to_map(st);
+                        return self.back_to_map(st);
                     }
                     if k.code == KeyCode::Enter {
                         let name = form.text("Name").unwrap_or("").trim().to_string();
@@ -151,13 +159,13 @@ impl Screen for Files {
                             name
                         };
                         let Some(city) = st.city.as_ref() else {
-                            return Self::back_to_map(st);
+                            return self.back_to_map(st);
                         };
                         let path = self.dir.join(format!("{name}.city"));
                         match save_city(&path, city) {
                             Ok(()) => {
                                 st.set_status(format!("saved {}", path.display()), 120);
-                                return Self::back_to_map(st);
+                                return self.back_to_map(st);
                             }
                             Err(e) => self.error = Some(e.to_string()),
                         }
@@ -299,7 +307,7 @@ mod tests {
             .unwrap(),
         );
         let name = st.city.as_ref().unwrap().name.clone();
-        let mut save = Files::save_in(dir.clone(), 0, &name);
+        let mut save = Files::save_in(dir.clone(), 0, &name, None);
         let mut t = Terminal::new(TestBackend::new(100, 30)).unwrap();
         t.draw(|f| save.draw(&st, f)).unwrap();
         assert!(buffer_text(t.backend().buffer()).contains("SAVE"));
@@ -319,6 +327,25 @@ mod tests {
             ScreenCmd::Switch { .. }
         ));
         assert_eq!(st.city.as_ref().map(|c| c.seed), Some(9));
+        // Saving from a live map screen hands that same screen back, state intact.
+        let mut map = crate::screens::explore::MapScreen::explore(0);
+        map.mode = crate::screens::explore::Mode::Walk;
+        map.stratum = citygen::StratumKind::Undercity;
+        map.digital = true;
+        st.player = Some(crate::walker::Player::spawn(st.city.as_ref().unwrap(), 1));
+        let mut save = Files::save_in(dir.clone(), 0, "again", Some(Box::new(map)));
+        let ScreenCmd::Switch { to, .. } = save.update(&mut st, &[key(KeyCode::Enter)]) else {
+            panic!("save should switch back");
+        };
+        st.tick += 500; // let the "saved" status expire so the mode hints show
+        let mut t3 = Terminal::new(TestBackend::new(120, 40)).unwrap();
+        t3.draw(|f| to.draw(&st, f)).unwrap();
+        let text = buffer_text(t3.backend().buffer());
+        assert!(
+            text.contains("tab camera") && text.contains("Undercity"),
+            "{text}"
+        );
+
         let mut empty = Files::load_in(tmp("empty"), 0);
         t.draw(|f| empty.draw(&st, f)).unwrap();
         assert!(buffer_text(t.backend().buffer()).contains("no saved cities"));

crates/citygen/src/geom.rs

@@ -189,13 +189,42 @@ impl Dir {
     }
 }
 
-/// Dense row-major grid of `T`.
+/// Dense row-major grid of `T`. Deserialization rejects data that does not match `size`.
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(try_from = "RawGrid<T>")]
+#[serde(bound(deserialize = "T: Deserialize<'de>"))]
 pub struct Grid<T> {
     size: Size,
     data: Vec<T>,
 }
 
+/// The on-disk shape of a grid, before the size/data consistency check.
+#[derive(Deserialize)]
+struct RawGrid<T> {
+    size: Size,
+    data: Vec<T>,
+}
+
+impl<T> TryFrom<RawGrid<T>> for Grid<T> {
+    type Error = String;
+
+    fn try_from(raw: RawGrid<T>) -> Result<Self, Self::Error> {
+        if raw.data.len() != raw.size.area() {
+            return Err(format!(
+                "grid of {}x{} needs {} cells, found {}",
+                raw.size.w,
+                raw.size.h,
+                raw.size.area(),
+                raw.data.len()
+            ));
+        }
+        Ok(Grid {
+            size: raw.size,
+            data: raw.data,
+        })
+    }
+}
+
 impl<T: Clone> Grid<T> {
     pub fn filled(size: Size, v: T) -> Self {
         Self {
@@ -308,6 +337,24 @@ mod tests {
         assert_eq!(g.count(|&v| v == 9), 4);
     }
 
+    #[test]
+    fn grid_rejects_inconsistent_serialized_data() {
+        #[derive(serde::Serialize)]
+        struct Fake {
+            size: Size,
+            data: Vec<u8>,
+        }
+        let bad = ron::to_string(&Fake {
+            size: Size { w: 4, h: 3 },
+            data: vec![0; 5],
+        })
+        .unwrap();
+        assert!(ron::from_str::<Grid<u8>>(&bad).is_err());
+        let good = ron::to_string(&Grid::filled(Size { w: 4, h: 3 }, 7u8)).unwrap();
+        let g: Grid<u8> = ron::from_str(&good).unwrap();
+        assert_eq!(g.count(|&v| v == 7), 12);
+    }
+
     #[test]
     fn dir_delta_opposite() {
         assert_eq!(Dir::N.delta(), (0, -1));

crates/citygen/src/validate.rs

@@ -152,6 +152,14 @@ impl City {
             }
         }
 
+        for link in &self.net.links {
+            for id in [link.0, link.1] {
+                if id.index() >= self.net.nodes.len() {
+                    out.push(Violation::NetAnchor(id));
+                }
+            }
+        }
+
         for d in &self.districts {
             let has_building = self.buildings.iter().any(|b| b.district == d.id);
             let has_public = self.connectors.iter().any(|c| {