mirror of
https://github.com/openai/codex.git
synced 2026-11-10 16:44:38 +00:00
Evaluate recent and character-routed skill selection (#38993)
## What changed - Add shadow selectors that fuse recently invoked skills with character routing-card matches using reciprocal rank fusion. - Add a second variant that also incorporates weighted lexical matches. - Record both variants in the existing shadow-selection experiment, capped at 50 candidates. ## Testing - Cover cold starts, duplicate removal, short exact matches, deterministic ranking, result limits, and truncation metadata. - Extend the skills integration test to verify both selectors recover a skill invoked on an earlier turn. GitOrigin-RevId: 4ff9e91da8cb49ecbce54ab8e760820616e1a1d3
This commit is contained in:
1 parent
8bf50439f2
commit
ea10ae7d0f
5 files changed
+348
-2
No files matched your search
@@ -2,6 +2,7 @@ mod character_ngram;
|
||||
mod character_routing_card;
|
||||
mod fielded_bm25;
|
||||
mod lru;
|
||||
mod lru_plus_character_routing;
|
||||
mod lru_plus_lexical;
|
||||
mod multi_query_lexical;
|
||||
mod routing_card_lexical;
|
||||
@@ -12,6 +13,8 @@ pub(crate) use character_routing_card::CharacterRoutingCardSkillSelector;
|
||||
use codex_skills::SkillDependencies;
|
||||
pub(crate) use fielded_bm25::FieldedBm25SkillSelector;
|
||||
pub(crate) use lru::LruSkillSelector;
|
||||
pub(crate) use lru_plus_character_routing::LruPlusCharacterRoutingSkillSelector;
|
||||
pub(crate) use lru_plus_character_routing::LruPlusLexicalCharacterRoutingSkillSelector;
|
||||
pub(crate) use lru_plus_lexical::LruPlusLexicalSkillSelector;
|
||||
pub(crate) use multi_query_lexical::MultiQueryLexicalSkillSelector;
|
||||
pub(crate) use routing_card_lexical::RoutingCardLexicalSkillSelector;
|
||||
|
||||
@@ -0,0 +1,136 @@
|
||||
use super::CharacterRoutingCardSkillSelector;
|
||||
use super::CheapSkillSelection;
|
||||
use super::CheapSkillSelector;
|
||||
use super::LruSkillSelector;
|
||||
use super::SkillSelectionDocument;
|
||||
use super::WeightedLexicalSkillSelector;
|
||||
use super::rrf_lexical_char::fuse_rankings_with_constant;
|
||||
|
||||
const MAX_RESULTS: usize = 50;
|
||||
const RRF_K: usize = 10;
|
||||
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub(crate) struct LruPlusCharacterRoutingSkillSelector {
|
||||
lru_selector: LruSkillSelector,
|
||||
character_selector: CharacterRoutingCardSkillSelector,
|
||||
}
|
||||
|
||||
impl LruPlusCharacterRoutingSkillSelector {
|
||||
pub(crate) fn new(
|
||||
lru_selector: LruSkillSelector,
|
||||
character_selector: CharacterRoutingCardSkillSelector,
|
||||
) -> Self {
|
||||
Self {
|
||||
lru_selector,
|
||||
character_selector,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl CheapSkillSelector for LruPlusCharacterRoutingSkillSelector {
|
||||
fn method(&self) -> &'static str {
|
||||
"lru_plus_character_routing_v1"
|
||||
}
|
||||
|
||||
fn select(
|
||||
&self,
|
||||
query: &str,
|
||||
documents: &[SkillSelectionDocument<'_>],
|
||||
limit: usize,
|
||||
) -> CheapSkillSelection {
|
||||
if limit == 0 {
|
||||
return CheapSkillSelection::default();
|
||||
}
|
||||
|
||||
let recent = self
|
||||
.lru_selector
|
||||
.select(query, documents, /*limit*/ MAX_RESULTS);
|
||||
let character =
|
||||
self.character_selector
|
||||
.select(query, documents, /*limit*/ MAX_RESULTS);
|
||||
let candidate_ids = fuse_rankings_with_constant(
|
||||
[&recent.candidate_ids, &character.candidate_ids],
|
||||
limit.min(MAX_RESULTS),
|
||||
/*rank_constant*/ RRF_K,
|
||||
);
|
||||
|
||||
CheapSkillSelection {
|
||||
candidate_ids,
|
||||
query_term_count: recent.query_term_count.max(character.query_term_count),
|
||||
query_truncated: recent.query_truncated || character.query_truncated,
|
||||
candidate_set_truncated: recent.candidate_set_truncated
|
||||
|| character.candidate_set_truncated,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub(crate) struct LruPlusLexicalCharacterRoutingSkillSelector {
|
||||
lru_selector: LruSkillSelector,
|
||||
character_selector: CharacterRoutingCardSkillSelector,
|
||||
}
|
||||
|
||||
impl LruPlusLexicalCharacterRoutingSkillSelector {
|
||||
pub(crate) fn new(
|
||||
lru_selector: LruSkillSelector,
|
||||
character_selector: CharacterRoutingCardSkillSelector,
|
||||
) -> Self {
|
||||
Self {
|
||||
lru_selector,
|
||||
character_selector,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl CheapSkillSelector for LruPlusLexicalCharacterRoutingSkillSelector {
|
||||
fn method(&self) -> &'static str {
|
||||
"lru_plus_lexical_character_routing_v1"
|
||||
}
|
||||
|
||||
fn select(
|
||||
&self,
|
||||
query: &str,
|
||||
documents: &[SkillSelectionDocument<'_>],
|
||||
limit: usize,
|
||||
) -> CheapSkillSelection {
|
||||
if limit == 0 {
|
||||
return CheapSkillSelection::default();
|
||||
}
|
||||
|
||||
let recent = self
|
||||
.lru_selector
|
||||
.select(query, documents, /*limit*/ MAX_RESULTS);
|
||||
let lexical =
|
||||
WeightedLexicalSkillSelector.select(query, documents, /*limit*/ MAX_RESULTS);
|
||||
let character =
|
||||
self.character_selector
|
||||
.select(query, documents, /*limit*/ MAX_RESULTS);
|
||||
let candidate_ids = fuse_rankings_with_constant(
|
||||
[
|
||||
&recent.candidate_ids,
|
||||
&lexical.candidate_ids,
|
||||
&character.candidate_ids,
|
||||
],
|
||||
limit.min(MAX_RESULTS),
|
||||
/*rank_constant*/ RRF_K,
|
||||
);
|
||||
|
||||
CheapSkillSelection {
|
||||
candidate_ids,
|
||||
query_term_count: recent
|
||||
.query_term_count
|
||||
.max(lexical.query_term_count)
|
||||
.max(character.query_term_count),
|
||||
query_truncated: recent.query_truncated
|
||||
|| lexical.query_truncated
|
||||
|| character.query_truncated,
|
||||
candidate_set_truncated: recent.candidate_set_truncated
|
||||
|| lexical.candidate_set_truncated
|
||||
|| character.candidate_set_truncated,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "lru_plus_character_routing_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,179 @@
|
||||
use codex_skills::SkillDependencies;
|
||||
use codex_skills::SkillToolDependency;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn cold_start_uses_character_routing_metadata() {
|
||||
let dependencies = SkillDependencies {
|
||||
tools: vec![SkillToolDependency {
|
||||
r#type: "app".to_string(),
|
||||
value: "slack".to_string(),
|
||||
description: None,
|
||||
transport: None,
|
||||
command: None,
|
||||
url: None,
|
||||
oauth_callback_port: None,
|
||||
}],
|
||||
};
|
||||
let documents = [
|
||||
document(/*id*/ 10, "compilation", "Compile Rust crates."),
|
||||
SkillSelectionDocument {
|
||||
dependencies: Some(&dependencies),
|
||||
..document(/*id*/ 20, "team-communication", "Share team updates.")
|
||||
},
|
||||
];
|
||||
let expected = CheapSkillSelection {
|
||||
candidate_ids: vec![20],
|
||||
query_term_count: 1,
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
for selector in selectors(Vec::new(), CharacterRoutingCardSkillSelector::default()) {
|
||||
let method = selector.method();
|
||||
assert_eq!(
|
||||
expected,
|
||||
selector.select("slack", &documents, /*limit*/ 50),
|
||||
"{method}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recent_and_text_matches_are_merged_without_duplicates() {
|
||||
let documents = [
|
||||
document(/*id*/ 10, "rust-build", "Inspect compiler errors."),
|
||||
document(
|
||||
/*id*/ 20,
|
||||
"python-tools",
|
||||
"Manage Python environments.",
|
||||
),
|
||||
document(/*id*/ 30, "monorepo", "Repository conventions."),
|
||||
];
|
||||
let expected = CheapSkillSelection {
|
||||
candidate_ids: vec![20, 30, 10],
|
||||
query_term_count: 1,
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
for selector in selectors(
|
||||
vec![30, 10, 20, 20, 999],
|
||||
CharacterRoutingCardSkillSelector::default(),
|
||||
) {
|
||||
let method = selector.method();
|
||||
assert_eq!(
|
||||
expected,
|
||||
selector.select("python", &documents, /*limit*/ 50),
|
||||
"{method}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn three_way_fusion_retains_short_exact_matches() {
|
||||
let documents = [
|
||||
document(/*id*/ 10, "ci", ""),
|
||||
document(/*id*/ 20, "python", ""),
|
||||
document(/*id*/ 30, "rust", ""),
|
||||
];
|
||||
let expected_rankings = [vec![20, 30], vec![20, 10, 30]];
|
||||
|
||||
for (selector, candidate_ids) in
|
||||
selectors(vec![30], CharacterRoutingCardSkillSelector::default())
|
||||
.into_iter()
|
||||
.zip(expected_rankings)
|
||||
{
|
||||
let method = selector.method();
|
||||
assert_eq!(
|
||||
CheapSkillSelection {
|
||||
candidate_ids,
|
||||
query_term_count: 2,
|
||||
..Default::default()
|
||||
},
|
||||
selector.select("ci python", &documents, /*limit*/ 50),
|
||||
"{method}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fusion_is_deterministic_capped_and_reports_input_bounds() {
|
||||
let documents = (0..1_001)
|
||||
.map(|id| document(id, "python", ""))
|
||||
.collect::<Vec<_>>();
|
||||
let query = "python ".repeat(4 * 1024);
|
||||
let expected_rankings = [
|
||||
[0, 999].into_iter().chain(1..49).collect::<Vec<_>>(),
|
||||
(0..11)
|
||||
.chain(std::iter::once(999))
|
||||
.chain(11..49)
|
||||
.collect::<Vec<_>>(),
|
||||
];
|
||||
|
||||
for (selector, candidate_ids) in selectors(
|
||||
vec![1_000, 999, 999, 1_002],
|
||||
CharacterRoutingCardSkillSelector::default(),
|
||||
)
|
||||
.into_iter()
|
||||
.zip(expected_rankings)
|
||||
{
|
||||
let method = selector.method();
|
||||
let expected = CheapSkillSelection {
|
||||
candidate_ids,
|
||||
query_term_count: 64,
|
||||
query_truncated: true,
|
||||
candidate_set_truncated: true,
|
||||
};
|
||||
assert_eq!(
|
||||
expected,
|
||||
selector.select(&query, &documents, /*limit*/ 50),
|
||||
"{method}"
|
||||
);
|
||||
assert_eq!(
|
||||
expected,
|
||||
selector.select(&query, &documents, usize::MAX),
|
||||
"{method}"
|
||||
);
|
||||
assert_eq!(
|
||||
CheapSkillSelection {
|
||||
candidate_ids: expected.candidate_ids[..7].to_vec(),
|
||||
..expected
|
||||
},
|
||||
selector.select(&query, &documents, /*limit*/ 7),
|
||||
"{method}"
|
||||
);
|
||||
assert_eq!(
|
||||
CheapSkillSelection::default(),
|
||||
selector.select(&query, &documents, /*limit*/ 0),
|
||||
"{method}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn selectors(
|
||||
recent_skill_ids: Vec<usize>,
|
||||
character_selector: CharacterRoutingCardSkillSelector,
|
||||
) -> [Box<dyn CheapSkillSelector>; 2] {
|
||||
let lru_selector = LruSkillSelector::new(recent_skill_ids);
|
||||
[
|
||||
Box::new(LruPlusCharacterRoutingSkillSelector::new(
|
||||
lru_selector.clone(),
|
||||
character_selector.clone(),
|
||||
)),
|
||||
Box::new(LruPlusLexicalCharacterRoutingSkillSelector::new(
|
||||
lru_selector,
|
||||
character_selector,
|
||||
)),
|
||||
]
|
||||
}
|
||||
|
||||
fn document<'a>(id: usize, name: &'a str, description: &'a str) -> SkillSelectionDocument<'a> {
|
||||
SkillSelectionDocument {
|
||||
id,
|
||||
name,
|
||||
short_description: None,
|
||||
description,
|
||||
dependencies: None,
|
||||
}
|
||||
}
|
||||
@@ -21,6 +21,8 @@ use crate::dynamic_skill_selector::CharacterRoutingCardSkillSelector;
|
||||
use crate::dynamic_skill_selector::CheapSkillSelection;
|
||||
use crate::dynamic_skill_selector::CheapSkillSelector;
|
||||
use crate::dynamic_skill_selector::FieldedBm25SkillSelector;
|
||||
use crate::dynamic_skill_selector::LruPlusCharacterRoutingSkillSelector;
|
||||
use crate::dynamic_skill_selector::LruPlusLexicalCharacterRoutingSkillSelector;
|
||||
use crate::dynamic_skill_selector::LruPlusLexicalSkillSelector;
|
||||
use crate::dynamic_skill_selector::LruSkillSelector;
|
||||
use crate::dynamic_skill_selector::MultiQueryLexicalSkillSelector;
|
||||
@@ -122,7 +124,15 @@ impl ShadowSelectionExperiment {
|
||||
let routing_selector = CharacterRoutingCardSkillSelector::new(catalog, host_snapshot);
|
||||
let lru_selector = LruSkillSelector::new(recent_skill_ids);
|
||||
let lru_plus_lexical_selector = LruPlusLexicalSkillSelector::new(lru_selector.clone());
|
||||
let mut ranked_selections = Vec::with_capacity(self.selectors.len() + 3);
|
||||
let lru_plus_character_selector = LruPlusCharacterRoutingSkillSelector::new(
|
||||
lru_selector.clone(),
|
||||
routing_selector.clone(),
|
||||
);
|
||||
let lru_plus_lexical_character_selector = LruPlusLexicalCharacterRoutingSkillSelector::new(
|
||||
lru_selector.clone(),
|
||||
routing_selector.clone(),
|
||||
);
|
||||
let mut ranked_selections = Vec::with_capacity(self.selectors.len() + 5);
|
||||
|
||||
for selector in self
|
||||
.selectors
|
||||
@@ -132,6 +142,8 @@ impl ShadowSelectionExperiment {
|
||||
&routing_selector as &dyn CheapSkillSelector,
|
||||
&lru_selector as &dyn CheapSkillSelector,
|
||||
&lru_plus_lexical_selector as &dyn CheapSkillSelector,
|
||||
&lru_plus_character_selector as &dyn CheapSkillSelector,
|
||||
&lru_plus_lexical_character_selector as &dyn CheapSkillSelector,
|
||||
])
|
||||
{
|
||||
let start = Instant::now();
|
||||
|
||||
@@ -948,7 +948,13 @@ async fn shadow_lru_selector_recovers_a_skill_invoked_on_an_earlier_turn() -> Te
|
||||
.find(|attribute| attribute.key.as_str() == "method")?
|
||||
.value
|
||||
.as_str();
|
||||
if method != "lru_v1" && method != "lru_plus_lexical_v1" {
|
||||
if !matches!(
|
||||
method.as_ref(),
|
||||
"lru_v1"
|
||||
| "lru_plus_lexical_v1"
|
||||
| "lru_plus_character_routing_v1"
|
||||
| "lru_plus_lexical_character_routing_v1"
|
||||
) {
|
||||
return None;
|
||||
}
|
||||
let hit = point
|
||||
@@ -966,6 +972,16 @@ async fn shadow_lru_selector_recovers_a_skill_invoked_on_an_earlier_turn() -> Te
|
||||
|
||||
assert_eq!(
|
||||
vec![
|
||||
(
|
||||
"lru_plus_character_routing_v1".to_string(),
|
||||
"true".to_string(),
|
||||
2,
|
||||
),
|
||||
(
|
||||
"lru_plus_lexical_character_routing_v1".to_string(),
|
||||
"true".to_string(),
|
||||
2,
|
||||
),
|
||||
("lru_plus_lexical_v1".to_string(), "true".to_string(), 2),
|
||||
("lru_v1".to_string(), "false".to_string(), 1),
|
||||
("lru_v1".to_string(), "true".to_string(), 1),
|
||||
|
||||
Reference in new issue
Block a user