diff --git a/server-rs/src/data/actual_listings.rs b/server-rs/src/data/actual_listings.rs index 761b77d..efb9289 100644 --- a/server-rs/src/data/actual_listings.rs +++ b/server-rs/src/data/actual_listings.rs @@ -1,6 +1,6 @@ use std::path::Path; -use anyhow::{Context, Result}; +use anyhow::{bail, Context, Result}; use polars::lazy::frame::LazyFrame; use polars::prelude::*; use serde::Serialize; @@ -12,6 +12,19 @@ use crate::utils::{normalize_postcode, GridIndex, InternedColumn}; const GRID_CELL_SIZE: f32 = 0.01; +/// Features `build_filter_feature_data` stamps on from the listing's own fields +/// after reading the parquet, so their absence as a column is expected. +const OVERLAID_LISTING_FEATURES: &[&str] = &[ + "Total floor area (sqm)", + "Number of bedrooms & living rooms", + "Estimated current price", + "Last known price", + "Est. price per sqm", + "Price per sqm", + "Property type", + "Leasehold/Freehold", +]; + /// One observation on a listing's accruing asking-price timeline, recovered from /// the scraper's forward-only store (finder/price_history.py). `reason` is one of /// "listed" | "reduced" | "increased"; `date` is `YYYY-MM-DD`. @@ -215,19 +228,44 @@ fn build_filter_feature_data( let mut feature_data = vec![NAN_U16; row_count * num_features]; let quant = property_data.quant_ref(); let mut encoded_columns = 0usize; + let mut missing_columns: Vec<&str> = Vec::new(); for (feat_idx, name) in property_data.feature_names.iter().enumerate() { if feat_idx < property_data.num_numeric { if let Some(values) = extract_optional_feature_f32(df, name)? { encode_numeric_feature(&mut feature_data, property_data, &quant, feat_idx, values); encoded_columns += 1; + } else { + missing_columns.push(name); } } else if let Some(values) = extract_optional_feature_str(df, name)? { encode_enum_feature(&mut feature_data, property_data, feat_idx, values); encoded_columns += 1; + } else { + missing_columns.push(name); } } + // A feature the enriched parquet doesn't carry stays NAN_U16 for every row, + // and filtering on it then rejects every listing (see + // `row_passes_listing_filters`), so refuse to boot rather than serve a + // filter that silently matches nothing. This mirrors the combined-schema + // check in `data/property/loading.rs`, whose absence here is what let the + // council columns ship missing. The overlay features are exempt: they are + // stamped on from the listing's own fields straight after this loop. + let missing_columns: Vec<&str> = missing_columns + .into_iter() + .filter(|name| !OVERLAID_LISTING_FEATURES.contains(name)) + .collect(); + if !missing_columns.is_empty() { + bail!( + "Configured features {:?} not found in the enriched listings parquet. \ + Filtering on them would match no listings. Rebuild it with \ + `make -f Makefile.data enrich-actual-listings`.", + missing_columns + ); + } + overlay_numeric_feature( &mut feature_data, property_data, @@ -745,6 +783,43 @@ mod tests { assert!(!any_listing.listing_url.is_empty()); } + fn property_data_declaring(feature: &str) -> PropertyData { + let mut property_data = PropertyData::empty_for_tests(); + property_data.feature_names = vec![feature.to_string()]; + property_data.num_features = 1; + property_data.num_numeric = 1; + property_data + } + + fn build_matrix_for(property_data: &PropertyData, df: &DataFrame) -> Result> { + build_filter_feature_data(df, Some(property_data), &[], &[], &[], &[], &[], &[]) + } + + #[test] + fn rejects_listings_parquet_missing_a_declared_feature() { + // A declared column the parquet lacks would stay NAN_U16 for every row, + // and `row_passes_listing_filters` would then reject every listing. Fail + // the boot instead of serving a filter that matches nothing. + let df = df!("Some other column" => [1.0f64]).expect("frame"); + let property_data = property_data_declaring("% Council housing"); + + let error = build_matrix_for(&property_data, &df).expect_err("must reject"); + + let message = error.to_string(); + assert!(message.contains("% Council housing"), "{message}"); + assert!(message.contains("enrich-actual-listings"), "{message}"); + } + + #[test] + fn accepts_overlaid_features_absent_from_the_parquet() { + // The overlay features are stamped on from the listing's own fields + // after the read, so their absence as a column is expected, not fatal. + let df = df!("Some other column" => [1.0f64]).expect("frame"); + let property_data = property_data_declaring(OVERLAID_LISTING_FEATURES[0]); + + build_matrix_for(&property_data, &df).expect("overlaid feature must not be fatal"); + } + #[test] fn extracts_price_history_from_parquet() { let path = PathBuf::from("src/data/testdata/listings_price_history.parquet");