mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Preserve uncapped Guardian classifier instructions (#39822)
## Why Guardian v2 applied an implicit token limit to classifier instructions even when no limit was configured, which could truncate the rendered policy. ## What changed - Leave classifier instructions unbounded by default. - Continue honoring `max_classifier_instruction_tokens` from local or model configuration, with local configuration taking precedence. ## Testing - Cover full policy rendering without a configured cap and truncation when an explicit cap is present. GitOrigin-RevId: 70eb42d43858e0656129dd438b10b60940fd97c6
This commit is contained in:
@@ -31,7 +31,8 @@ pub(crate) struct GuardianV2Config {
|
||||
pub(crate) max_tool_call_lag: usize,
|
||||
pub(crate) reasoning_effort: ReasoningEffort,
|
||||
pub(crate) max_action_tokens: usize,
|
||||
pub(crate) max_classifier_instruction_tokens: usize,
|
||||
/// No truncation limit is applied unless local or model configuration supplies one.
|
||||
pub(crate) max_classifier_instruction_tokens: Option<usize>,
|
||||
pub(crate) reuse_parent_compaction: bool,
|
||||
pub(crate) max_parent_compaction_tokens: usize,
|
||||
pub(crate) sandboxed_exec_commands: bool,
|
||||
@@ -160,11 +161,10 @@ impl GuardianV2Config {
|
||||
DEFAULT_MODEL_CONTEXT_ITEM_TOKENS,
|
||||
"max_action_tokens",
|
||||
)?;
|
||||
let max_classifier_instruction_tokens = bounded_tokens(
|
||||
configured.max_classifier_instruction_tokens,
|
||||
DEFAULT_MODEL_CONTEXT_ITEM_TOKENS,
|
||||
"max_classifier_instruction_tokens",
|
||||
)?;
|
||||
let max_classifier_instruction_tokens = configured
|
||||
.max_classifier_instruction_tokens
|
||||
.map(|tokens| bounded_tokens(Some(tokens), tokens, "max_classifier_instruction_tokens"))
|
||||
.transpose()?;
|
||||
let max_parent_compaction_tokens = bounded_tokens(
|
||||
configured.max_parent_compaction_tokens,
|
||||
DEFAULT_PARENT_COMPACTION_TOKENS,
|
||||
@@ -227,13 +227,15 @@ impl GuardianV2Config {
|
||||
.classifier_instructions
|
||||
.as_deref()
|
||||
.unwrap_or(DEFAULT_CLASSIFIER_INSTRUCTIONS);
|
||||
if template.contains("{{ tenant_policy_config }}") {
|
||||
// Preserve the placeholder until the actual policy is available.
|
||||
// The final rendered instruction is bounded before sampling.
|
||||
template.to_owned()
|
||||
} else {
|
||||
if let Some(max_tokens) = max_classifier_instruction_tokens
|
||||
&& !template.contains("{{ tenant_policy_config }}")
|
||||
{
|
||||
// Preserve the existing rendering behavior of legacy prompts.
|
||||
truncate_entry(template, max_classifier_instruction_tokens)
|
||||
truncate_entry(template, max_tokens)
|
||||
} else {
|
||||
// Preserve placeholders until the actual policy is available, and
|
||||
// preserve the full prompt when no instruction limit is configured.
|
||||
template.to_owned()
|
||||
}
|
||||
},
|
||||
review_threshold,
|
||||
@@ -281,7 +283,10 @@ impl GuardianV2Config {
|
||||
self.classifier_instructions
|
||||
)
|
||||
};
|
||||
truncate_entry(&instructions, self.max_classifier_instruction_tokens)
|
||||
match self.max_classifier_instruction_tokens {
|
||||
Some(max_tokens) => truncate_entry(&instructions, max_tokens),
|
||||
None => instructions,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -66,7 +66,7 @@ fn evaluated_configuration_preserves_rendered_prompt_and_gate() {
|
||||
.unwrap();
|
||||
assert_eq!(config.review_threshold, 0.5);
|
||||
assert_eq!(config.reasoning_effort, ReasoningEffort::Low);
|
||||
assert_eq!(config.max_classifier_instruction_tokens, 30_000);
|
||||
assert_eq!(config.max_classifier_instruction_tokens, Some(30_000));
|
||||
assert_eq!(
|
||||
config.classifier_instructions,
|
||||
DEFAULT_CLASSIFIER_INSTRUCTIONS
|
||||
@@ -150,7 +150,10 @@ fn model_prompt_and_explicit_threshold_precedence_are_preserved() {
|
||||
|
||||
#[test]
|
||||
fn model_runtime_settings_preserve_local_overrides() {
|
||||
let prompt = "legacy instructions ".repeat(3_000);
|
||||
let defaults = GuardianV2ModelConfig {
|
||||
classifier_instructions: Some(prompt.clone()),
|
||||
max_classifier_instruction_tokens: Some(256),
|
||||
max_tool_call_lag: Some(1),
|
||||
reuse_parent_compaction: Some(false),
|
||||
transcript: Some(GuardianV2TranscriptModelConfig {
|
||||
@@ -165,14 +168,16 @@ fn model_runtime_settings_preserve_local_overrides() {
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
(
|
||||
inherited.max_classifier_instruction_tokens,
|
||||
inherited.max_tool_call_lag,
|
||||
inherited.reuse_parent_compaction,
|
||||
inherited.transcript.include_images,
|
||||
),
|
||||
(1, false, true)
|
||||
(Some(256), 1, false, true)
|
||||
);
|
||||
|
||||
let overridden = GuardianV2Config::from_overrides(GuardianV2ConfigToml {
|
||||
max_classifier_instruction_tokens: Some(512),
|
||||
max_tool_call_lag: Some(4),
|
||||
reuse_parent_compaction: Some(true),
|
||||
transcript: Some(GuardianV2TranscriptConfigToml {
|
||||
@@ -186,10 +191,23 @@ fn model_runtime_settings_preserve_local_overrides() {
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
(
|
||||
overridden.max_classifier_instruction_tokens,
|
||||
overridden.max_tool_call_lag,
|
||||
overridden.reuse_parent_compaction,
|
||||
overridden.transcript.include_images,
|
||||
),
|
||||
(4, true, false)
|
||||
(Some(512), 4, true, false)
|
||||
);
|
||||
|
||||
let uncapped_defaults = GuardianV2ModelConfig {
|
||||
max_classifier_instruction_tokens: None,
|
||||
..defaults
|
||||
};
|
||||
let uncapped = inherited
|
||||
.with_model_defaults(Some(&uncapped_defaults))
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
uncapped.render_classifier_instructions("Tenant policy."),
|
||||
format!("{prompt}\n\n# Security Policy\nTenant policy.")
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1604,7 +1604,7 @@ async fn contributor_uses_catalog_policy_without_a_configured_override() -> Resu
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn contributor_bounds_configured_policy_in_luna_developer_instructions() -> Result<()> {
|
||||
async fn contributor_preserves_uncapped_classifier_instructions() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let guardian_policy = format!(
|
||||
@@ -1617,6 +1617,39 @@ async fn contributor_bounds_configured_policy_in_luna_developer_instructions() -
|
||||
Some(&guardian_policy),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
request["input"][1],
|
||||
json!({
|
||||
"type": "message",
|
||||
"role": "developer",
|
||||
"content": [{
|
||||
"type": "input_text",
|
||||
"text": crate::async_scorer::config::DEFAULT_CLASSIFIER_INSTRUCTIONS
|
||||
.replace("{{ tenant_policy_config }}", &guardian_policy),
|
||||
}],
|
||||
})
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn contributor_bounds_configured_policy_in_luna_developer_instructions() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let guardian_policy = format!(
|
||||
"Reject unsafe uploads.\n{}\nRequire explicit approval.",
|
||||
"é".repeat(20_000)
|
||||
);
|
||||
let (request, _test, _registry) = sample_configured_conversation_history(
|
||||
Vec::new(),
|
||||
r#"{"path":"README.md"}"#,
|
||||
Some(&guardian_policy),
|
||||
"[features.guardianv2]\nenabled = true\nmax_classifier_instruction_tokens = 10000\n",
|
||||
/*model_defaults*/ None,
|
||||
)
|
||||
.await?;
|
||||
let instructions = request["input"][1]["content"][0]["text"]
|
||||
.as_str()
|
||||
.expect("Luna request should contain developer instructions");
|
||||
|
||||
Reference in New Issue
Block a user