mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Gate terminal visualization instructions in TUI (#26013)
## Summary - add `Feature::TerminalVisualizationInstructions` as `UnderDevelopment`, disabled by default - keep terminal visualization instructions inside the TUI package - append them to existing developer instructions for TUI start, resume, and fork flows only when enabled - intentionally do not apply them to `codex exec` ## Rollout Control behavior is unchanged. TUI dogfooders can enable `terminal_visualization_instructions`; no default user receives the new terminal-specific instructions. The shared visualization-selection rule is supplied separately through the `codex_proxy_model_3` Statsig layer for every target Codex model slug in the gated cohort. This TUI feature determines how to render an appropriate visualization on the terminal surface; the model-layer treatment determines when to use one. ## Validation - `cargo test -p codex-tui terminal_visualization_instructions_are_gated_for_all_tui_thread_flows --lib` - `cargo test -p codex-features --lib` - `cargo fmt --all -- --check` - `git diff --check` - GPT-5.4 and GPT-5.5 real prompt-pipeline smoke tests: both visualized the positive mapping case, abstained on the negative route case, and passed exact prompt-stack verification on CLI and App - refreshed onto current `main` with a clean merge and reran the focused validation The full 53-probe all-model treatment comparison and requested production coding evals remain rollout gates before broadening beyond the initial employee cohort. This PR remains open for normal human review.
This commit is contained in:
committed by
GitHub
Unverified
parent
ffe90cb5c3
commit
61a913d9c8
@@ -614,6 +614,9 @@
|
||||
"terminal_resize_reflow": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"terminal_visualization_instructions": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"tool_call_mcp_elicitation": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -4737,6 +4740,9 @@
|
||||
"terminal_resize_reflow": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"terminal_visualization_instructions": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"tool_call_mcp_elicitation": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -99,6 +99,8 @@ pub enum Feature {
|
||||
UnifiedExecZshFork,
|
||||
/// Reflow transcript scrollback when the terminal is resized.
|
||||
TerminalResizeReflow,
|
||||
/// Add terminal-specific visualization guidance to TUI developer instructions.
|
||||
TerminalVisualizationInstructions,
|
||||
/// Stream structured progress while apply_patch input is being generated.
|
||||
ApplyPatchStreamingEvents,
|
||||
/// Allow exec tools to request additional permissions while staying sandboxed.
|
||||
@@ -1118,6 +1120,12 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::TerminalVisualizationInstructions,
|
||||
key: "terminal_visualization_instructions",
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::GuardianApproval,
|
||||
key: "guardian_approval",
|
||||
|
||||
@@ -11,6 +11,7 @@ use crate::session_state::MessageHistoryMetadata;
|
||||
use crate::session_state::ThreadSessionState;
|
||||
use crate::status::StatusAccountDisplay;
|
||||
use crate::status::plan_type_display_name;
|
||||
use crate::terminal_visualization_instructions::with_terminal_visualization_instructions;
|
||||
use codex_app_server_client::AppServerClient;
|
||||
use codex_app_server_client::AppServerEvent;
|
||||
use codex_app_server_client::AppServerRequestHandle;
|
||||
@@ -1412,6 +1413,9 @@ fn thread_start_params_from_config(
|
||||
ephemeral: Some(config.ephemeral),
|
||||
session_start_source,
|
||||
thread_source: Some(ThreadSource::User),
|
||||
developer_instructions: with_terminal_visualization_instructions(
|
||||
config, /*control_instructions*/ None,
|
||||
),
|
||||
..ThreadStartParams::default()
|
||||
}
|
||||
}
|
||||
@@ -1444,6 +1448,9 @@ fn thread_resume_params_from_config(
|
||||
sandbox,
|
||||
permissions,
|
||||
config: config_request_overrides_from_config(&config),
|
||||
developer_instructions: with_terminal_visualization_instructions(
|
||||
&config, /*control_instructions*/ None,
|
||||
),
|
||||
..ThreadResumeParams::default()
|
||||
}
|
||||
}
|
||||
@@ -1477,7 +1484,10 @@ fn thread_fork_params_from_config(
|
||||
permissions,
|
||||
config: config_request_overrides_from_config(&config),
|
||||
base_instructions: config.base_instructions.clone(),
|
||||
developer_instructions: config.developer_instructions.clone(),
|
||||
developer_instructions: with_terminal_visualization_instructions(
|
||||
&config,
|
||||
config.developer_instructions.clone(),
|
||||
),
|
||||
ephemeral: config.ephemeral,
|
||||
thread_source: Some(ThreadSource::User),
|
||||
..ThreadForkParams::default()
|
||||
@@ -1748,6 +1758,7 @@ mod tests {
|
||||
use codex_app_server_protocol::ThreadStatus;
|
||||
use codex_app_server_protocol::Turn;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
@@ -2235,6 +2246,79 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn terminal_visualization_instructions_are_gated_for_all_tui_thread_flows() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
let mut config = build_config(&temp_dir).await;
|
||||
config.developer_instructions = Some("Developer override.".to_string());
|
||||
let thread_id = ThreadId::new();
|
||||
|
||||
let control_start = thread_start_params_from_config(
|
||||
&config,
|
||||
ThreadParamsMode::Embedded,
|
||||
/*remote_cwd_override*/ None,
|
||||
/*session_start_source*/ None,
|
||||
);
|
||||
let control_resume = thread_resume_params_from_config(
|
||||
config.clone(),
|
||||
thread_id,
|
||||
ThreadParamsMode::Embedded,
|
||||
/*remote_cwd_override*/ None,
|
||||
);
|
||||
let control_fork = thread_fork_params_from_config(
|
||||
config.clone(),
|
||||
thread_id,
|
||||
ThreadParamsMode::Embedded,
|
||||
/*remote_cwd_override*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(control_start.developer_instructions, None);
|
||||
assert_eq!(control_resume.developer_instructions, None);
|
||||
assert_eq!(
|
||||
control_fork.developer_instructions.as_deref(),
|
||||
Some("Developer override.")
|
||||
);
|
||||
|
||||
let _ = config
|
||||
.features
|
||||
.enable(Feature::TerminalVisualizationInstructions);
|
||||
let treatment_start = thread_start_params_from_config(
|
||||
&config,
|
||||
ThreadParamsMode::Embedded,
|
||||
/*remote_cwd_override*/ None,
|
||||
/*session_start_source*/ None,
|
||||
);
|
||||
let treatment_resume = thread_resume_params_from_config(
|
||||
config.clone(),
|
||||
thread_id,
|
||||
ThreadParamsMode::Embedded,
|
||||
/*remote_cwd_override*/ None,
|
||||
);
|
||||
let treatment_fork = thread_fork_params_from_config(
|
||||
config,
|
||||
thread_id,
|
||||
ThreadParamsMode::Embedded,
|
||||
/*remote_cwd_override*/ None,
|
||||
);
|
||||
let expected = format!(
|
||||
"Developer override.\n\n{}",
|
||||
crate::terminal_visualization_instructions::TERMINAL_VISUALIZATION_INSTRUCTIONS
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
treatment_start.developer_instructions.as_deref(),
|
||||
Some(expected.as_str())
|
||||
);
|
||||
assert_eq!(
|
||||
treatment_resume.developer_instructions.as_deref(),
|
||||
Some(expected.as_str())
|
||||
);
|
||||
assert_eq!(
|
||||
treatment_fork.developer_instructions.as_deref(),
|
||||
Some(expected.as_str())
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resume_response_restores_turns_from_thread_items() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
@@ -188,6 +188,7 @@ mod terminal_hyperlinks;
|
||||
mod terminal_palette;
|
||||
mod terminal_probe;
|
||||
mod terminal_title;
|
||||
mod terminal_visualization_instructions;
|
||||
mod text_formatting;
|
||||
mod theme_picker;
|
||||
mod token_usage;
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
use crate::legacy_core::config::Config;
|
||||
use codex_features::Feature;
|
||||
|
||||
pub(crate) const TERMINAL_VISUALIZATION_INSTRUCTIONS: &str = "\
|
||||
- This surface is a terminal. When the formatting rules require a visual, include one in the final answer using compact ASCII diagrams, trees, timelines, or tables.
|
||||
- Use tables for exact mappings or comparisons rather than collapsing known mappings into prose.
|
||||
- Use trees for hierarchy or one-to-many relationships, and diagrams or timelines for sequence, change, or state transferred between records across event order.
|
||||
- Use only ASCII characters in visuals.";
|
||||
|
||||
pub(crate) fn with_terminal_visualization_instructions(
|
||||
config: &Config,
|
||||
control_instructions: Option<String>,
|
||||
) -> Option<String> {
|
||||
if !config
|
||||
.features
|
||||
.enabled(Feature::TerminalVisualizationInstructions)
|
||||
{
|
||||
return control_instructions;
|
||||
}
|
||||
|
||||
let existing_instructions =
|
||||
control_instructions.or_else(|| config.developer_instructions.clone());
|
||||
Some(match existing_instructions.as_deref() {
|
||||
Some(existing) if !existing.trim().is_empty() => {
|
||||
format!("{existing}\n\n{TERMINAL_VISUALIZATION_INSTRUCTIONS}")
|
||||
}
|
||||
_ => TERMINAL_VISUALIZATION_INSTRUCTIONS.to_string(),
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user