mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Persist additional tool definitions in rollout history (#50435)
## Why `additional_tools` response items were excluded from rollout persistence, dropping their tool definitions from saved history. ## What changed - Persist `additional_tools` items in both legacy and paginated history modes. - Expose the variant in generated JSON schemas, TypeScript types, and Python models, with an optional `id`, a `role`, and arbitrary JSON tool definitions. ## Testing Add a recorder round-trip test that verifies tool definitions survive recording and loading in both history modes without parse errors. GitOrigin-RevId: 0e7c7b1c10401ec6513fb744de642a1cf9e461d7
This commit is contained in:
@@ -2911,6 +2911,37 @@
|
||||
},
|
||||
"ResponseItem": {
|
||||
"oneOf": [
|
||||
{
|
||||
"properties": {
|
||||
"id": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"role": {
|
||||
"type": "string"
|
||||
},
|
||||
"tools": {
|
||||
"items": true,
|
||||
"type": "array"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
"additional_tools"
|
||||
],
|
||||
"title": "AdditionalToolsResponseItemType",
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"role",
|
||||
"tools",
|
||||
"type"
|
||||
],
|
||||
"title": "AdditionalToolsResponseItem",
|
||||
"type": "object"
|
||||
},
|
||||
{
|
||||
"properties": {
|
||||
"content": {
|
||||
|
||||
+31
@@ -18691,6 +18691,37 @@
|
||||
},
|
||||
"ResponseItem": {
|
||||
"oneOf": [
|
||||
{
|
||||
"properties": {
|
||||
"id": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"role": {
|
||||
"type": "string"
|
||||
},
|
||||
"tools": {
|
||||
"items": true,
|
||||
"type": "array"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
"additional_tools"
|
||||
],
|
||||
"title": "AdditionalToolsResponseItemType",
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"role",
|
||||
"tools",
|
||||
"type"
|
||||
],
|
||||
"title": "AdditionalToolsResponseItem",
|
||||
"type": "object"
|
||||
},
|
||||
{
|
||||
"properties": {
|
||||
"content": {
|
||||
|
||||
+31
@@ -14703,6 +14703,37 @@
|
||||
},
|
||||
"ResponseItem": {
|
||||
"oneOf": [
|
||||
{
|
||||
"properties": {
|
||||
"id": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"role": {
|
||||
"type": "string"
|
||||
},
|
||||
"tools": {
|
||||
"items": true,
|
||||
"type": "array"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
"additional_tools"
|
||||
],
|
||||
"title": "AdditionalToolsResponseItemType",
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"role",
|
||||
"tools",
|
||||
"type"
|
||||
],
|
||||
"title": "AdditionalToolsResponseItem",
|
||||
"type": "object"
|
||||
},
|
||||
{
|
||||
"properties": {
|
||||
"content": {
|
||||
|
||||
+31
@@ -482,6 +482,37 @@
|
||||
},
|
||||
"ResponseItem": {
|
||||
"oneOf": [
|
||||
{
|
||||
"properties": {
|
||||
"id": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"role": {
|
||||
"type": "string"
|
||||
},
|
||||
"tools": {
|
||||
"items": true,
|
||||
"type": "array"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
"additional_tools"
|
||||
],
|
||||
"title": "AdditionalToolsResponseItemType",
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"role",
|
||||
"tools",
|
||||
"type"
|
||||
],
|
||||
"title": "AdditionalToolsResponseItem",
|
||||
"type": "object"
|
||||
},
|
||||
{
|
||||
"properties": {
|
||||
"content": {
|
||||
|
||||
@@ -553,6 +553,37 @@
|
||||
},
|
||||
"ResponseItem": {
|
||||
"oneOf": [
|
||||
{
|
||||
"properties": {
|
||||
"id": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"role": {
|
||||
"type": "string"
|
||||
},
|
||||
"tools": {
|
||||
"items": true,
|
||||
"type": "array"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
"additional_tools"
|
||||
],
|
||||
"title": "AdditionalToolsResponseItemType",
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"role",
|
||||
"tools",
|
||||
"type"
|
||||
],
|
||||
"title": "AdditionalToolsResponseItem",
|
||||
"type": "object"
|
||||
},
|
||||
{
|
||||
"properties": {
|
||||
"content": {
|
||||
|
||||
BIN
Binary file not shown.
BIN
Binary file not shown.
@@ -14,7 +14,7 @@ import type { ReasoningItemReasoningSummary } from "./ReasoningItemReasoningSumm
|
||||
import type { ResponseItemId } from "./ResponseItemId";
|
||||
import type { WebSearchAction } from "./WebSearchAction";
|
||||
|
||||
export type ResponseItem = { "type": "message", id?: ResponseItemId, role: string, content: Array<ContentItem>, phase?: MessagePhase, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "agent_message", id?: ResponseItemId, author: string, recipient: string, content: Array<AgentMessageInputContent>, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "reasoning", id?: ResponseItemId, summary: Array<ReasoningItemReasoningSummary>, content?: Array<ReasoningItemContent>, encrypted_content: string | null, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "local_shell_call",
|
||||
export type ResponseItem = { "type": "additional_tools", id?: ResponseItemId, role: string, tools: unknown[], } | { "type": "message", id?: ResponseItemId, role: string, content: Array<ContentItem>, phase?: MessagePhase, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "agent_message", id?: ResponseItemId, author: string, recipient: string, content: Array<AgentMessageInputContent>, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "reasoning", id?: ResponseItemId, summary: Array<ReasoningItemReasoningSummary>, content?: Array<ReasoningItemContent>, encrypted_content: string | null, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "local_shell_call",
|
||||
/**
|
||||
* Legacy id field retained for compatibility with older payloads.
|
||||
*/
|
||||
|
||||
@@ -1010,12 +1010,12 @@ impl InternalChatMessageMetadataPassthrough {
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)]
|
||||
#[serde(tag = "type", rename_all = "snake_case")]
|
||||
pub enum ResponseItem {
|
||||
#[schemars(skip)]
|
||||
#[ts(skip)]
|
||||
AdditionalTools {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
id: Option<ResponseItemId>,
|
||||
role: String,
|
||||
#[ts(type = "unknown[]")]
|
||||
tools: Vec<serde_json::Value>,
|
||||
},
|
||||
Message {
|
||||
|
||||
@@ -76,7 +76,8 @@ pub fn into_persisted_rollout_items(
|
||||
#[inline]
|
||||
pub fn should_persist_response_item(item: &ResponseItem) -> bool {
|
||||
match item {
|
||||
ResponseItem::Message { .. }
|
||||
ResponseItem::AdditionalTools { .. }
|
||||
| ResponseItem::Message { .. }
|
||||
| ResponseItem::AgentMessage { .. }
|
||||
| ResponseItem::Reasoning { .. }
|
||||
| ResponseItem::LocalShellCall { .. }
|
||||
@@ -91,9 +92,7 @@ pub fn should_persist_response_item(item: &ResponseItem) -> bool {
|
||||
| ResponseItem::ConfigurationUpdate { .. }
|
||||
| ResponseItem::Compaction { .. }
|
||||
| ResponseItem::ContextCompaction { .. } => true,
|
||||
ResponseItem::AdditionalTools { .. }
|
||||
| ResponseItem::CompactionTrigger { .. }
|
||||
| ResponseItem::Other => false,
|
||||
ResponseItem::CompactionTrigger { .. } | ResponseItem::Other => false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -613,6 +613,58 @@ fn strip_legacy_ghost_snapshot_keeps_checkpoint_metadata_aligned() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn recorder_preserves_additional_tools_in_both_history_modes() -> std::io::Result<()> {
|
||||
for history_mode in [ThreadHistoryMode::Legacy, ThreadHistoryMode::Paginated] {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let config = test_config(home.path());
|
||||
let thread_id = ThreadId::new();
|
||||
let recorder = RolloutRecorder::new(
|
||||
&config,
|
||||
RolloutRecorderParams::new(
|
||||
thread_id,
|
||||
/*forked_from_id*/ None,
|
||||
/*parent_thread_id*/ None,
|
||||
SessionSource::Exec,
|
||||
/*thread_source*/ None,
|
||||
"test_originator".to_string(),
|
||||
BaseInstructions::default(),
|
||||
Vec::new(),
|
||||
)
|
||||
.with_history_mode(history_mode),
|
||||
)
|
||||
.await?;
|
||||
let item = RolloutItem::ResponseItem(
|
||||
ResponseItem::AdditionalTools {
|
||||
id: None,
|
||||
role: "developer".to_string(),
|
||||
tools: vec![serde_json::json!({
|
||||
"type": "namespace",
|
||||
"name": "functions",
|
||||
"description": "Available tools.",
|
||||
"tools": [{"type": "function", "name": "lookup",
|
||||
"description": "Look up a value.",
|
||||
"parameters": {"type": "object", "properties": {}}}]
|
||||
})],
|
||||
}
|
||||
.into(),
|
||||
);
|
||||
let persisted = crate::persisted_rollout_items(std::slice::from_ref(&item), history_mode);
|
||||
recorder.record_canonical_items(&persisted).await?;
|
||||
recorder.flush().await?;
|
||||
let (items, loaded_thread_id, parse_errors) =
|
||||
RolloutRecorder::load_rollout_items(recorder.rollout_path()).await?;
|
||||
assert_eq!(loaded_thread_id, Some(thread_id));
|
||||
assert_eq!(parse_errors, 0);
|
||||
assert_eq!(
|
||||
serde_json::to_value(&items[1..])?,
|
||||
serde_json::to_value([item])?
|
||||
);
|
||||
recorder.shutdown().await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn recorder_materializes_on_flush_with_pending_items() -> std::io::Result<()> {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
|
||||
@@ -3934,6 +3934,16 @@ class ResourceTemplate(BaseModel):
|
||||
uri_template: Annotated[str, Field(alias="uriTemplate")]
|
||||
|
||||
|
||||
class AdditionalToolsResponseItem(BaseModel):
|
||||
model_config = ConfigDict(
|
||||
populate_by_name=True,
|
||||
)
|
||||
id: str | None = None
|
||||
role: str
|
||||
tools: list
|
||||
type: Annotated[Literal["additional_tools"], Field(title="AdditionalToolsResponseItemType")]
|
||||
|
||||
|
||||
class AgentMessageResponseItem(BaseModel):
|
||||
model_config = ConfigDict(
|
||||
populate_by_name=True,
|
||||
@@ -11147,7 +11157,8 @@ class CustomToolCallOutputResponseItem(BaseModel):
|
||||
|
||||
class ResponseItem(
|
||||
RootModel[
|
||||
MessageResponseItem
|
||||
AdditionalToolsResponseItem
|
||||
| MessageResponseItem
|
||||
| AgentMessageResponseItem
|
||||
| ReasoningResponseItem
|
||||
| LocalShellCallResponseItem
|
||||
@@ -11170,7 +11181,8 @@ class ResponseItem(
|
||||
populate_by_name=True,
|
||||
)
|
||||
root: (
|
||||
MessageResponseItem
|
||||
AdditionalToolsResponseItem
|
||||
| MessageResponseItem
|
||||
| AgentMessageResponseItem
|
||||
| ReasoningResponseItem
|
||||
| LocalShellCallResponseItem
|
||||
|
||||
Reference in New Issue
Block a user