diff --git a/TODO.md b/TODO.md index fcc349d..61d3062 100644 --- a/TODO.md +++ b/TODO.md @@ -81,10 +81,11 @@ Cheap: the columns exist and the MCP does not offer them. - [x] **`run_mp`**. - [x] **`mul_id` lookup** — go straight to a unit from a MUL record id. 4,126 Meks carry one. -- [ ] **`source`** — which book a unit comes from. Held back because the column - is a comma-joined list (`TR:3039,TR:SW`), so it wants splitting into - something matchable rather than a substring test that would match - `TR:3039` inside `TR:30395` if such a book existed. +- [x] **`source`** — which book a unit comes from. The column is a comma-joined + list (`TR:3039,TR:SW`), so it is split into one and matched as an any-of + list rather than with a substring test that would find `TR:303` inside + `TR:3039`. `list_facets` names the twenty commonest, since a caller + cannot ask for a book without knowing how it is spelled. ## Equipment filtering diff --git a/crates/helm-facet/src/lib.rs b/crates/helm-facet/src/lib.rs index f744519..9101f65 100644 --- a/crates/helm-facet/src/lib.rs +++ b/crates/helm-facet/src/lib.rs @@ -26,7 +26,7 @@ mod query; pub use class::WeaponClass; pub use equipment::Equipment; -pub use query::{FacetQuery, UnitFacets, base_config}; +pub use query::{FacetQuery, UnitFacets, base_config, split_sources}; /// An inclusive numeric range where either end may be open. /// diff --git a/crates/helm-facet/src/query.rs b/crates/helm-facet/src/query.rs index 96281f9..2ccacac 100644 --- a/crates/helm-facet/src/query.rs +++ b/crates/helm-facet/src/query.rs @@ -92,6 +92,26 @@ pub struct UnitFacets { pub run_mp: Option, pub mul_id: Option, pub quirks: Vec, + /// The books this design appears in, split out of the comma-joined column. + /// + /// A design is usually in more than one - `TR:3039,TR:SW` - and a + /// substring test on the joined string matches `TR:303` inside `TR:3039`, + /// which is why this was held back until it could be a list. + pub sources: Vec, +} + +/// Split the `source` column into the books it names. +/// +/// A design is usually in more than one - `TR:3039,TR:SW` - and the column +/// joins them with commas. Matching the joined string with a substring test +/// would find `TR:303` inside `TR:3039`, so it is a list or it is nothing. +pub fn split_sources(source: &str) -> Vec { + source + .split(',') + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string) + .collect() } /// Strip the `OmniMek` suffix a `.mtf` Config line carries. @@ -168,6 +188,11 @@ impl UnitFacets { run_mp: stats.and_then(|s| s.run_mp), mul_id: unit.mul_id, quirks: unit.quirks.clone(), + sources: unit + .source + .as_deref() + .map(split_sources) + .unwrap_or_default(), } } } @@ -204,6 +229,8 @@ pub struct FacetQuery { /// Any-of, like the other name lists. pub rules_levels: Vec, pub configs: Vec, + /// Any-of, by book: a design in any one of them matches. + pub sources: Vec, /// Every one of these must be present, unlike the any-of lists: asking for /// two quirks means wanting a unit that has both. pub quirks: Vec, @@ -240,6 +267,15 @@ impl FacetQuery { if !any_of(&self.tech_bases, u.tech_base.as_deref()) { return false; } + if !self.sources.is_empty() + && !self + .sources + .iter() + .any(|wanted| u.sources.iter().any(|s| s == wanted)) + { + return false; + } + if !any_of(&self.roles, u.role.as_deref()) { return false; } diff --git a/crates/helm-mcp/src/library.rs b/crates/helm-mcp/src/library.rs index f9e6b5f..668dec6 100644 --- a/crates/helm-mcp/src/library.rs +++ b/crates/helm-mcp/src/library.rs @@ -109,6 +109,11 @@ impl Library { mul_id: r.get(27)?, armor_pct: r.get(28)?, quirks: Vec::new(), + sources: r + .get::<_, Option>(7)? + .as_deref() + .map(helm_facet::split_sources) + .unwrap_or_default(), }, )) }) diff --git a/crates/helm-mcp/src/tools.rs b/crates/helm-mcp/src/tools.rs index 2f1ed4b..32b71f4 100644 --- a/crates/helm-mcp/src/tools.rs +++ b/crates/helm-mcp/src/tools.rs @@ -43,6 +43,9 @@ pub struct Filter { /// Any of MegaMek's roles: Juggernaut, Striker, Sniper, Brawler, Scout, /// Missile Boat, Skirmisher, Ambusher, Undetermined. pub role: Option>, + /// Any-of, by the book a design appears in: `TR:3039`, `TR:SW`. A design + /// in any one of them matches, and `list_facets` names the commonest. + pub source: Option>, /// Earliest year of introduction, inclusive. pub year_min: Option, /// Latest year of introduction, inclusive. A unit is available from its @@ -224,6 +227,7 @@ impl Filter { unit_types: Vec::new(), tech_bases: self.tech_base.clone().unwrap_or_default(), roles: self.role.clone().unwrap_or_default(), + sources: self.source.clone().unwrap_or_default(), weight_classes, year: Range::new(self.year_min, self.year_max), tons: Range::new(self.tons_min, self.tons_max), @@ -275,6 +279,24 @@ fn unknown_class(label: &str) -> Value { } /// The quirks worth naming, commonest first. There are too many to list them +/// The commonest books, since there are hundreds and a caller needs to know +/// how one is spelled before it can filter on it. +fn common_sources(lib: &Library, take: usize) -> Vec { + let mut counts: BTreeMap<&str, usize> = BTreeMap::new(); + for facet in &lib.facets { + for source in &facet.sources { + *counts.entry(source.as_str()).or_default() += 1; + } + } + let mut ranked: Vec<(&str, usize)> = counts.into_iter().collect(); + ranked.sort_by(|a, b| b.1.cmp(&a.1).then(a.0.cmp(b.0))); + ranked + .into_iter() + .take(take) + .map(|(name, count)| json!({ "source": name, "units": count })) + .collect() +} + /// all, and a caller wanting a rare one can search for it. fn common_quirks(lib: &Library, n: usize) -> Vec { let mut all: Vec<(&String, usize)> = lib.quirk_counts.iter().map(|(k, v)| (k, *v)).collect(); @@ -355,6 +377,7 @@ pub fn list_facets(lib: &Library) -> Value { "computed_by": lib.stats_producer, "tech_base": facet_values(lib, |f| f.tech_base.clone()), "role": facet_values(lib, |f| f.role.clone()), + "source_most_common": common_sources(lib, 20), "rules_level": facet_values(lib, |f| f.rules_level.clone()), "config": facet_values(lib, |f| f.config.clone()), "quirks_most_common": common_quirks(lib, 20), diff --git a/crates/helm-mcp/tests/tools.rs b/crates/helm-mcp/tests/tools.rs index e36329d..967e3a6 100644 --- a/crates/helm-mcp/tests/tools.rs +++ b/crates/helm-mcp/tests/tools.rs @@ -626,3 +626,49 @@ fn list_facets_offers_values_that_actually_work() { ); } } + +// The books a design appears in, which was held back until it could be a list: +// `TR:3039,TR:SW` is two books, and a substring test on the joined column +// matches `TR:303` inside `TR:3039`. +#[test] +#[ignore = "needs a built database; set HELM_DB"] +fn a_book_selects_the_designs_that_are_in_it() { + let lib = library(); + let succession = tools::find_units( + &lib, + &filter(serde_json::json!({"source": ["TR:SW"]})), + Some(3), + ); + let n = succession["total_matched"].as_i64().unwrap(); + assert!(n > 50, "only {n} designs in TR:SW"); + + // Every hit really is in that book, rather than in one whose name contains + // it. + for unit in succession["units"].as_array().unwrap() { + let detail = tools::get_unit(&lib, unit["name"].as_str().unwrap()); + let source = detail["source"].as_str().unwrap_or_default(); + assert!( + source.split(',').any(|s| s.trim() == "TR:SW"), + "{} is sourced {source}", + unit["name"] + ); + } + + // And the filter narrows: asking for two books is more than one of them. + let both = tools::find_units( + &lib, + &filter(serde_json::json!({"source": ["TR:SW", "TR:3085"]})), + Some(1), + ); + assert!(both["total_matched"].as_i64().unwrap() > n); + + // list_facets has to say how a book is spelled, or nobody can ask for one. + let facets = tools::list_facets(&lib); + let names: Vec<&str> = facets["source_most_common"] + .as_array() + .unwrap() + .iter() + .map(|s| s["source"].as_str().unwrap()) + .collect(); + assert!(names.contains(&"TR:SW"), "{names:?}"); +}