From 9d8ff8b341d802e022542c678750d25dd82f5eff Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 3 Jun 2026 17:33:24 +0200 Subject: [PATCH] fix(simulation): fall back on unrecognized settlement_class (#955) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit read_body_settlements propagated a parse error for any non-NULL but unrecognized settlement_class, which the atlas proxy swallowed and then enqueued the body with zero cities — one malformed row silently dropped placement for the entire body. Treat an unknown variant like NULL: fall back to PopulationBudget with a warning. parse_settlement_class stays strict for the D-199 read_set path, which must abort on bad fields. Found in PR #149 review (Hoshe H1). Adds a test covering the fallback. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/src/atlas/city_context_reader.rs | 45 +++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/server/src/atlas/city_context_reader.rs b/server/src/atlas/city_context_reader.rs index bc10b1b2a..eddb02243 100644 --- a/server/src/atlas/city_context_reader.rs +++ b/server/src/atlas/city_context_reader.rs @@ -234,8 +234,15 @@ impl CityContextReader { /// `settlement_class` is NULL at this stage (placement is what *derives* it, /// D-196), so a NULL defaults to `PopulationBudget` — NOT `NameLocked` — /// leaving `match_cities` to tier by population (the largest become the - /// Tier-A capitals). A non-NULL value is parsed as authored. `economic_role` - /// NULL falls back to `residential` (the neutral role). + /// Tier-A capitals). A recognized non-NULL value is parsed as authored; an + /// *unrecognized* value also falls back to `PopulationBudget` (with a warning) + /// rather than erroring — one bad row must not zero out the whole body's + /// placements. `economic_role` NULL falls back to `residential` (the neutral + /// role). + /// + /// Unlike [`read_set`](Self::read_set) (the strict D-199 path that aborts on + /// any malformed field), this method is best-effort: it never fails on row + /// content, only on a DB/connection error. pub fn read_body_settlements( &self, body_id: &str, @@ -270,7 +277,14 @@ impl CityContextReader { r.map_err(|e| CityContextReadError::Db(e.to_string()))?; let city_id = id as u64; let settlement_class = match sclass.as_deref() { - Some(s) => parse_settlement_class(Some(s), city_id)?, + Some(s) => parse_settlement_class(Some(s), city_id).unwrap_or_else(|_| { + tracing::warn!( + city_id, + settlement_class = s, + "unrecognized settlement_class; defaulting to PopulationBudget" + ); + SettlementClass::PopulationBudget + }), None => SettlementClass::PopulationBudget, }; out.push(CityRecord { @@ -914,4 +928,29 @@ mod tests { "unknown body yields no settlements" ); } + + #[test] + fn read_body_settlements_unknown_class_falls_back_without_dropping_body() { + // A single row with an unrecognized settlement_class must NOT fail the + // whole read (which would enqueue the body with zero cities). It falls + // back to PopulationBudget and the other settlements are unaffected. + let db = make_settlements_db(&[ + ("Good", "financial", 500_000, Some("NameLocked")), + ( + "Weird", + "manufacturing", + 200_000, + Some("TotallyBogusVariant"), + ), + ]); + let reader = CityContextReader::open(&db).expect("open"); + let cities = reader.read_body_settlements("PlanetX").expect("read"); + assert_eq!(cities.len(), 2, "one bad row must not drop the whole body"); + assert_eq!(cities[0].settlement_class, SettlementClass::NameLocked); + assert_eq!( + cities[1].settlement_class, + SettlementClass::PopulationBudget, + "unrecognized class defaults to PopulationBudget" + ); + } }