From 51200321eb7b862a29ffceaba8b19db1934a9b38 Mon Sep 17 00:00:00 2001 From: jif Date: Tue, 21 Jul 2026 17:13:03 +0000 Subject: [PATCH] Add routing-card lexical skill selection (#34581) ## What changed - Add a bounded `routing_card_exact_v1` selector that ranks normalized exact matches across skill names, tool dependencies, short descriptions, and full descriptions, with stronger weights for routing-specific fields. - Run the selector in the shadow selection experiment and include catalog entry dependencies in selection documents. - Report a selection as `selected` when an exact-name match succeeds even if the query contains no searchable terms. ## Testing - Cover dependency ranking, stop-word skill names, exact-term matching, and input bounds. GitOrigin-RevId: dfb0b668d726f64ae5da3b3ce2d8c9b23a664ac0 --- .../ext/skills/src/dynamic_skill_selector.rs | 4 + .../character_ngram_tests.rs | 2 + .../fielded_bm25_tests.rs | 2 + .../multi_query_lexical_tests.rs | 2 + .../routing_card_lexical.rs | 258 ++++++++++++++++++ .../routing_card_lexical_tests.rs | 131 +++++++++ .../rrf_lexical_char_tests.rs | 1 + .../weighted_lexical_tests.rs | 10 + .../skills/src/shadow_selection_experiment.rs | 13 +- 9 files changed, 418 insertions(+), 5 deletions(-) create mode 100644 codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical.rs create mode 100644 codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical_tests.rs diff --git a/codex-rs/ext/skills/src/dynamic_skill_selector.rs b/codex-rs/ext/skills/src/dynamic_skill_selector.rs index 148d99d46b..30027c564f 100644 --- a/codex-rs/ext/skills/src/dynamic_skill_selector.rs +++ b/codex-rs/ext/skills/src/dynamic_skill_selector.rs @@ -1,11 +1,14 @@ mod character_ngram; mod fielded_bm25; mod multi_query_lexical; +mod routing_card_lexical; mod rrf_lexical_char; mod weighted_lexical; pub(crate) use character_ngram::CharacterNgramSkillSelector; +use codex_skills::SkillDependencies; pub(crate) use fielded_bm25::FieldedBm25SkillSelector; pub(crate) use multi_query_lexical::MultiQueryLexicalSkillSelector; +pub(crate) use routing_card_lexical::RoutingCardLexicalSkillSelector; pub(crate) use rrf_lexical_char::RrfLexicalCharSkillSelector; pub(crate) use weighted_lexical::WeightedLexicalSkillSelector; @@ -17,6 +20,7 @@ pub(crate) struct SkillSelectionDocument<'a> { pub name: &'a str, pub short_description: Option<&'a str>, pub description: &'a str, + pub dependencies: Option<&'a SkillDependencies>, } /// Bounded output from one cheap skill-selection method. diff --git a/codex-rs/ext/skills/src/dynamic_skill_selector/character_ngram_tests.rs b/codex-rs/ext/skills/src/dynamic_skill_selector/character_ngram_tests.rs index 4621746f53..fbd78d04b4 100644 --- a/codex-rs/ext/skills/src/dynamic_skill_selector/character_ngram_tests.rs +++ b/codex-rs/ext/skills/src/dynamic_skill_selector/character_ngram_tests.rs @@ -57,6 +57,7 @@ fn ngrams_report_bounded_inputs() { name, short_description: None, description: "match", + dependencies: None, }) .collect::>(); @@ -73,5 +74,6 @@ fn document<'a>(id: usize, name: &'a str, description: &'a str) -> SkillSelectio name, short_description: None, description, + dependencies: None, } } diff --git a/codex-rs/ext/skills/src/dynamic_skill_selector/fielded_bm25_tests.rs b/codex-rs/ext/skills/src/dynamic_skill_selector/fielded_bm25_tests.rs index 14bebaf7c3..2b055b37e1 100644 --- a/codex-rs/ext/skills/src/dynamic_skill_selector/fielded_bm25_tests.rs +++ b/codex-rs/ext/skills/src/dynamic_skill_selector/fielded_bm25_tests.rs @@ -59,6 +59,7 @@ fn bm25_reports_bounded_inputs() { name, short_description: None, description: "match", + dependencies: None, }) .collect::>(); @@ -75,5 +76,6 @@ fn document<'a>(id: usize, name: &'a str, description: &'a str) -> SkillSelectio name, short_description: None, description, + dependencies: None, } } diff --git a/codex-rs/ext/skills/src/dynamic_skill_selector/multi_query_lexical_tests.rs b/codex-rs/ext/skills/src/dynamic_skill_selector/multi_query_lexical_tests.rs index faf60d6406..c425471d98 100644 --- a/codex-rs/ext/skills/src/dynamic_skill_selector/multi_query_lexical_tests.rs +++ b/codex-rs/ext/skills/src/dynamic_skill_selector/multi_query_lexical_tests.rs @@ -69,6 +69,7 @@ fn multi_query_preserves_bounded_input_signals() { name, short_description: None, description: "match logs", + dependencies: None, }) .collect::>(); @@ -86,5 +87,6 @@ fn document<'a>(id: usize, name: &'a str, description: &'a str) -> SkillSelectio name, short_description: None, description, + dependencies: None, } } diff --git a/codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical.rs b/codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical.rs new file mode 100644 index 0000000000..e76afd322b --- /dev/null +++ b/codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical.rs @@ -0,0 +1,258 @@ +use std::collections::HashSet; + +use super::CheapSkillSelection; +use super::CheapSkillSelector; +use super::SkillSelectionDocument; + +const MAX_QUERY_BYTES: usize = 4 * 1024; +const MAX_QUERY_TERMS: usize = 64; +const MAX_DOCUMENT_BYTES: usize = 4 * 1024; +const MAX_FIELD_TERMS: usize = 256; +const MAX_DEPENDENCY_RECORDS: usize = 32; +const MAX_CANDIDATES: usize = 1_000; +const MAX_RESULTS: usize = 50; + +const STOP_WORDS: &[&str] = &[ + "a", "an", "and", "are", "as", "at", "be", "by", "do", "for", "from", "how", "i", "in", "is", + "it", "me", "my", "of", "on", "or", "please", "that", "the", "this", "to", "use", "we", "what", + "when", "where", "which", "with", "you", "your", +]; + +#[derive(Clone, Copy, Debug, Default)] +pub(crate) struct RoutingCardLexicalSkillSelector; + +impl CheapSkillSelector for RoutingCardLexicalSkillSelector { + fn method(&self) -> &'static str { + "routing_card_exact_v1" + } + + fn select( + &self, + query: &str, + documents: &[SkillSelectionDocument<'_>], + limit: usize, + ) -> CheapSkillSelection { + let (query, query_bytes_truncated) = bounded(query, MAX_QUERY_BYTES); + let query_phrase = normalize(query); + let (query_terms, query_terms_truncated) = query_terms(&query_phrase); + let query_truncated = query_bytes_truncated || query_terms_truncated; + let candidate_set_truncated = documents.len() > MAX_CANDIDATES; + if query_phrase.is_empty() || limit == 0 { + return CheapSkillSelection { + query_term_count: query_terms.len(), + query_truncated, + candidate_set_truncated, + ..Default::default() + }; + } + + let mut scored = documents + .iter() + .take(MAX_CANDIDATES) + .filter_map(|document| { + let prepared = PreparedRoutingCard::new(document); + let score = prepared.score(&query_phrase, &query_terms); + (score > 0).then_some((score, document.id, document.name)) + }) + .collect::>(); + scored.sort_by(|left, right| { + right + .0 + .cmp(&left.0) + .then_with(|| left.2.cmp(right.2)) + .then_with(|| left.1.cmp(&right.1)) + }); + + CheapSkillSelection { + candidate_ids: scored + .into_iter() + .take(limit.min(MAX_RESULTS)) + .map(|(_, id, _)| id) + .collect(), + query_term_count: query_terms.len(), + query_truncated, + candidate_set_truncated, + } + } +} + +struct PreparedRoutingCard { + name: String, + name_has_searchable_term: bool, + fields: RoutingFields, +} + +struct RoutingFields { + name: HashSet, + dependencies: HashSet, + short_description: HashSet, + description: HashSet, +} + +impl PreparedRoutingCard { + fn new(document: &SkillSelectionDocument<'_>) -> Self { + let name = normalize_bounded(document.name); + Self { + name_has_searchable_term: name.split_whitespace().any(is_searchable_query_term), + name, + fields: RoutingFields { + name: field_terms(document.name), + dependencies: dependency_terms(document), + short_description: field_terms(document.short_description.unwrap_or_default()), + description: field_terms(document.description), + }, + } + } + + fn score(&self, query_phrase: &str, query_terms: &[&str]) -> u32 { + let mut score = 0u32; + let name_phrase_matches = contains_phrase(query_phrase, &self.name); + if !self.name.is_empty() + && ((self.name_has_searchable_term && name_phrase_matches) || query_phrase == self.name) + { + score = score.saturating_add(256); + } + + let mut matched_query_terms = 0u32; + for query_term in query_terms { + let mut matched = false; + score = score.saturating_add(score_field( + &self.fields.name, + query_term, + /*weight*/ 80, + &mut matched, + )); + score = score.saturating_add(score_field( + &self.fields.dependencies, + query_term, + /*weight*/ 24, + &mut matched, + )); + score = score.saturating_add(score_field( + &self.fields.short_description, + query_term, + /*weight*/ 12, + &mut matched, + )); + score = score.saturating_add(score_field( + &self.fields.description, + query_term, + /*weight*/ 3, + &mut matched, + )); + if matched { + matched_query_terms = matched_query_terms.saturating_add(1); + } + } + score.saturating_add(matched_query_terms.saturating_mul(matched_query_terms)) + } +} + +fn score_field(terms: &HashSet, query_term: &str, weight: u32, matched: &mut bool) -> u32 { + if terms.contains(query_term) { + *matched = true; + weight + } else { + 0 + } +} + +fn dependency_terms(document: &SkillSelectionDocument<'_>) -> HashSet { + let mut terms = HashSet::new(); + let Some(dependencies) = document.dependencies else { + return terms; + }; + let mut remaining_bytes = MAX_DOCUMENT_BYTES; + for tool in dependencies.tools.iter().take(MAX_DEPENDENCY_RECORDS) { + extend_terms(&mut terms, &tool.value, &mut remaining_bytes); + if let Some(description) = tool.description.as_deref() { + extend_terms(&mut terms, description, &mut remaining_bytes); + } + if remaining_bytes == 0 || terms.len() >= MAX_FIELD_TERMS { + break; + } + } + terms +} + +fn field_terms(value: &str) -> HashSet { + normalized_terms(bounded(value, MAX_DOCUMENT_BYTES).0) + .into_iter() + .take(MAX_FIELD_TERMS) + .collect() +} + +fn extend_terms(terms: &mut HashSet, value: &str, remaining_bytes: &mut usize) { + let value = bounded(value, *remaining_bytes).0; + *remaining_bytes = remaining_bytes.saturating_sub(value.len()); + for term in normalized_terms(value) { + if terms.len() == MAX_FIELD_TERMS { + return; + } + terms.insert(term); + } +} + +fn query_terms(query_phrase: &str) -> (Vec<&str>, bool) { + let mut seen = HashSet::new(); + let mut terms = Vec::new(); + for term in query_phrase + .split_whitespace() + .filter(|term| is_searchable_query_term(term)) + { + if !seen.insert(term) { + continue; + } + if terms.len() == MAX_QUERY_TERMS { + return (terms, true); + } + terms.push(term); + } + (terms, false) +} + +fn is_searchable_query_term(term: &str) -> bool { + term.chars().count() >= 2 && !STOP_WORDS.contains(&term) +} + +fn normalize_bounded(value: &str) -> String { + normalize(bounded(value, MAX_DOCUMENT_BYTES).0) +} + +fn normalize(value: &str) -> String { + normalized_terms(value).join(" ") +} + +fn normalized_terms(value: &str) -> Vec { + let mut normalized = String::with_capacity(value.len()); + for character in value.chars() { + if character.is_alphanumeric() { + normalized.extend(character.to_lowercase()); + } else { + normalized.push(' '); + } + } + normalized.split_whitespace().map(str::to_string).collect() +} + +fn contains_phrase(haystack: &str, needle: &str) -> bool { + haystack == needle + || haystack.starts_with(&format!("{needle} ")) + || haystack.ends_with(&format!(" {needle}")) + || haystack.contains(&format!(" {needle} ")) +} + +fn bounded(value: &str, max_bytes: usize) -> (&str, bool) { + if value.len() <= max_bytes { + return (value, false); + } + let mut end = max_bytes; + while !value.is_char_boundary(end) { + end = end.saturating_sub(1); + } + (&value[..end], true) +} + +#[cfg(test)] +#[path = "routing_card_lexical_tests.rs"] +mod tests; diff --git a/codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical_tests.rs b/codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical_tests.rs new file mode 100644 index 0000000000..b8010703aa --- /dev/null +++ b/codex-rs/ext/skills/src/dynamic_skill_selector/routing_card_lexical_tests.rs @@ -0,0 +1,131 @@ +use codex_skills::SkillDependencies; +use codex_skills::SkillToolDependency; +use pretty_assertions::assert_eq; + +use super::*; + +#[test] +fn dependency_names_outrank_description_only_matches() { + let dependencies = SkillDependencies { + tools: vec![SkillToolDependency { + r#type: "app".to_string(), + value: "slack".to_string(), + description: Some("Send messages to Slack channels.".to_string()), + transport: None, + command: None, + url: None, + }], + }; + let documents = [ + SkillSelectionDocument { + id: 1, + name: "team-messaging", + short_description: None, + description: "Work with team communication.", + dependencies: Some(&dependencies), + }, + document( + /*id*/ 2, + "documents", + "Send messages to Slack channels.", + ), + ]; + + let selection = RoutingCardLexicalSkillSelector.select( + "send this update to Slack", + &documents, + /*limit*/ 20, + ); + + assert_eq!(vec![1, 2], selection.candidate_ids); +} + +#[test] +fn stop_word_name_requires_an_exact_query() { + let documents = [document(/*id*/ 1, "do", "Run a task.")]; + + let selection = RoutingCardLexicalSkillSelector.select("do", &documents, /*limit*/ 20); + assert_eq!(vec![1], selection.candidate_ids); + assert_eq!(0, selection.query_term_count); + + let selection = RoutingCardLexicalSkillSelector.select( + "how do I format Rust", + &documents, + /*limit*/ 20, + ); + assert_eq!(Vec::::new(), selection.candidate_ids); +} + +#[test] +fn prefixes_do_not_match_field_terms() { + let documents = [document( + /*id*/ 1, + "presentations", + "Create presentations.", + )]; + + let selection = + RoutingCardLexicalSkillSelector.select("present", &documents, /*limit*/ 20); + + assert_eq!(Vec::::new(), selection.candidate_ids); +} + +#[test] +fn dependency_records_are_bounded() { + let mut tools = (0..MAX_DEPENDENCY_RECORDS) + .map(|_| SkillToolDependency { + r#type: "app".to_string(), + value: "shared".to_string(), + description: None, + transport: None, + command: None, + url: None, + }) + .collect::>(); + tools.push(SkillToolDependency { + r#type: "app".to_string(), + value: "unbounded".to_string(), + description: None, + transport: None, + command: None, + url: None, + }); + let dependencies = SkillDependencies { tools }; + let document = SkillSelectionDocument { + id: 1, + name: "dependency-test", + short_description: None, + description: "Test dependencies.", + dependencies: Some(&dependencies), + }; + + assert_eq!( + HashSet::from(["shared".to_string()]), + dependency_terms(&document) + ); +} + +#[test] +fn selector_reports_bounded_inputs() { + let documents = [document( + /*id*/ 1, + "presentations", + "Create visual decks.", + )]; + let query = "presentations ".repeat(MAX_QUERY_BYTES); + + let selection = RoutingCardLexicalSkillSelector.select(&query, &documents, /*limit*/ 20); + + assert_eq!(vec![1], selection.candidate_ids); + assert!(selection.query_truncated); +} + +fn document<'a>(id: usize, name: &'a str, description: &'a str) -> SkillSelectionDocument<'a> { + SkillSelectionDocument { + id, + name, + short_description: None, + description, + dependencies: None, + } +} diff --git a/codex-rs/ext/skills/src/dynamic_skill_selector/rrf_lexical_char_tests.rs b/codex-rs/ext/skills/src/dynamic_skill_selector/rrf_lexical_char_tests.rs index 2bc55e3002..fb7d3e847c 100644 --- a/codex-rs/ext/skills/src/dynamic_skill_selector/rrf_lexical_char_tests.rs +++ b/codex-rs/ext/skills/src/dynamic_skill_selector/rrf_lexical_char_tests.rs @@ -23,6 +23,7 @@ fn selector_reports_the_combined_input_bounds() { name: "presentations", short_description: None, description: "Create visual decks.", + dependencies: None, }]; let query = "presentation ".repeat(4 * 1024); diff --git a/codex-rs/ext/skills/src/dynamic_skill_selector/weighted_lexical_tests.rs b/codex-rs/ext/skills/src/dynamic_skill_selector/weighted_lexical_tests.rs index 696c160cd9..b90ad37cd6 100644 --- a/codex-rs/ext/skills/src/dynamic_skill_selector/weighted_lexical_tests.rs +++ b/codex-rs/ext/skills/src/dynamic_skill_selector/weighted_lexical_tests.rs @@ -9,18 +9,21 @@ fn lexical_selector_prioritizes_an_exact_skill_name() { name: "slides-helper", short_description: None, description: "Create presentations and visual decks.", + dependencies: None, }, SkillSelectionDocument { id: 20, name: "presentations", short_description: None, description: "Create or edit PowerPoint presentations.", + dependencies: None, }, SkillSelectionDocument { id: 30, name: "spreadsheets", short_description: None, description: "Analyze tabular data.", + dependencies: None, }, ]; @@ -43,12 +46,14 @@ fn lexical_selector_uses_descriptions_and_drops_zero_score_candidates() { name: "ci-helper", short_description: Some("Diagnose continuous integration failures."), description: "Inspect failing GitHub Actions checks and logs.", + dependencies: None, }, SkillSelectionDocument { id: 2, name: "document-editor", short_description: None, description: "Edit Word documents.", + dependencies: None, }, ]; @@ -74,6 +79,7 @@ fn lexical_selector_respects_requested_limit() { name, short_description: None, description: "Fix lint errors.", + dependencies: None, }) .collect::>(); @@ -97,6 +103,7 @@ fn lexical_selector_reports_bounded_inputs() { name, short_description: None, description: "match", + dependencies: None, }) .collect::>(); @@ -124,6 +131,7 @@ fn lexical_selector_caps_query_terms() { name: "term0", short_description: None, description: "term0", + dependencies: None, }]; let selection = WeightedLexicalSkillSelector.select(&query, &documents, /*limit*/ 20); @@ -139,6 +147,7 @@ fn lexical_selector_returns_nothing_for_stop_words_only() { name: "anything", short_description: None, description: "Do anything.", + dependencies: None, }]; let selection = @@ -157,6 +166,7 @@ fn selector_can_be_used_behind_the_shared_trait() { name: "code-review", short_description: None, description: "Review code.", + dependencies: None, }], /*limit*/ 20, ) diff --git a/codex-rs/ext/skills/src/shadow_selection_experiment.rs b/codex-rs/ext/skills/src/shadow_selection_experiment.rs index d79c5f9034..4260cd2a81 100644 --- a/codex-rs/ext/skills/src/shadow_selection_experiment.rs +++ b/codex-rs/ext/skills/src/shadow_selection_experiment.rs @@ -16,6 +16,7 @@ use crate::dynamic_skill_selector::CheapSkillSelection; use crate::dynamic_skill_selector::CheapSkillSelector; use crate::dynamic_skill_selector::FieldedBm25SkillSelector; use crate::dynamic_skill_selector::MultiQueryLexicalSkillSelector; +use crate::dynamic_skill_selector::RoutingCardLexicalSkillSelector; use crate::dynamic_skill_selector::RrfLexicalCharSkillSelector; use crate::dynamic_skill_selector::SkillSelectionDocument; use crate::dynamic_skill_selector::WeightedLexicalSkillSelector; @@ -45,6 +46,7 @@ impl ShadowSelectionExperiment { Box::new(CharacterNgramSkillSelector), Box::new(MultiQueryLexicalSkillSelector), Box::new(RrfLexicalCharSkillSelector), + Box::new(RoutingCardLexicalSkillSelector), ], metrics_client, } @@ -76,6 +78,7 @@ impl ShadowSelectionExperiment { name: entry.name.as_str(), short_description: entry.short_description.as_deref(), description: entry.description.as_str(), + dependencies: entry.dependencies.as_ref(), }) .collect::>(); let eligible_ids = documents @@ -239,12 +242,12 @@ fn sanitize_selected_ids( } fn selection_status(selection: &CheapSkillSelection, selected_entry_count: usize) -> &'static str { - if selection.query_term_count == 0 { - "no_query_terms" - } else if selected_entry_count == 0 { - "no_matches" - } else { + if selected_entry_count > 0 { "selected" + } else if selection.query_term_count == 0 { + "no_query_terms" + } else { + "no_matches" } }