Add total (non-partial) TextElement placeholder accessors (#9545)

## Summary
- Make `TextElement` placeholders private and add a text-backed accessor
to avoid assuming `Some`.
- Since they are optional in the protocol, we want to make sure any
accessors properly handle the None case (getting the placeholder using
the byte range in the text)
- Preserve placeholders during protocol/app-server conversions using the
accessor fallback.
- Update TUI composer/remap logic and tests to use the new
constructor/accessor.

codex-rs/tui/src/bottom_pane/chat_composer.rs

@@ -557,12 +557,7 @@ impl ChatComposer {
 
         let image_placeholders: HashSet<String> = text_elements
             .iter()
-            .filter_map(|elem| {
-                elem.placeholder.as_ref().cloned().or_else(|| {
-                    text.get(elem.byte_range.start..elem.byte_range.end)
-                        .map(str::to_string)
-                })
-            })
+            .filter_map(|elem| elem.placeholder(&text).map(str::to_string))
             .collect();
         for (idx, path) in local_image_paths.into_iter().enumerate() {
             let placeholder = local_image_label_text(idx + 1);
@@ -1240,13 +1235,13 @@ impl ChatComposer {
                     return None;
                 }
                 let placeholder = trimmed.get(new_start..new_end).map(str::to_string);
-                Some(TextElement {
-                    byte_range: ByteRange {
+                Some(TextElement::new(
+                    ByteRange {
                         start: new_start,
                         end: new_end,
                     },
                     placeholder,
-                })
+                ))
             })
             .collect()
     }
@@ -1287,7 +1282,7 @@ impl ChatComposer {
                 rebuilt.push_str(&text[cursor..start]);
             }
             let elem_text = &text[start..end];
-            let placeholder = elem.placeholder;
+            let placeholder = elem.placeholder(text).map(str::to_string);
             let replacement = placeholder
                 .as_deref()
                 .and_then(|ph| pending_by_placeholder.get_mut(ph))
@@ -1301,13 +1296,13 @@ impl ChatComposer {
                 rebuilt.push_str(elem_text);
                 let new_end = rebuilt.len();
                 let placeholder = placeholder.or_else(|| Some(elem_text.to_string()));
-                rebuilt_elements.push(TextElement {
-                    byte_range: ByteRange {
+                rebuilt_elements.push(TextElement::new(
+                    ByteRange {
                         start: new_start,
                         end: new_end,
                     },
                     placeholder,
-                });
+                ));
             }
             cursor = end;
         }
@@ -4232,7 +4227,7 @@ mod tests {
             } => {
                 assert_eq!(text, "[Image #1] hi");
                 assert_eq!(text_elements.len(), 1);
-                assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
+                assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
                 assert_eq!(
                     text_elements[0].byte_range,
                     ByteRange {
@@ -4261,10 +4256,7 @@ mod tests {
 
         let placeholder = local_image_label_text(1);
         let text = format!("{placeholder} restored");
-        let text_elements = vec![TextElement {
-            byte_range: (0..placeholder.len()).into(),
-            placeholder: None,
-        }];
+        let text_elements = vec![TextElement::new((0..placeholder.len()).into(), None)];
         let path = PathBuf::from("/tmp/image1.png");
 
         composer.set_text_content(text, text_elements, vec![path.clone()]);
@@ -4301,7 +4293,7 @@ mod tests {
                 let expected = format!("{large_content} [Image #1]");
                 assert_eq!(text, expected);
                 assert_eq!(text_elements.len(), 1);
-                assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
+                assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
                 assert_eq!(
                     text_elements[0].byte_range,
                     ByteRange {
@@ -4345,7 +4337,7 @@ mod tests {
                 let trimmed = large_content.trim().to_string();
                 assert_eq!(text, format!("{trimmed} [Image #1]"));
                 assert_eq!(text_elements.len(), 1);
-                assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
+                assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
                 assert_eq!(
                     text_elements[0].byte_range,
                     ByteRange {
@@ -4389,7 +4381,7 @@ mod tests {
                 assert_eq!(text, "line1\nline2\n [Image #1]");
                 assert!(!text.contains('\r'));
                 assert_eq!(text_elements.len(), 1);
-                assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
+                assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
                 assert_eq!(
                     text_elements[0].byte_range,
                     ByteRange {
@@ -4474,7 +4466,7 @@ mod tests {
             } => {
                 assert_eq!(text, "[Image #1]");
                 assert_eq!(text_elements.len(), 1);
-                assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
+                assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
                 assert_eq!(
                     text_elements[0].byte_range,
                     ByteRange {
@@ -4664,20 +4656,20 @@ mod tests {
         let start2 = text.find(&placeholder2).expect("placeholder2 present");
         let start1 = text.find(&placeholder1).expect("placeholder1 present");
         let text_elements = vec![
-            TextElement {
-                byte_range: ByteRange {
+            TextElement::new(
+                ByteRange {
                     start: start2,
                     end: start2 + placeholder2.len(),
                 },
-                placeholder: Some(placeholder2),
-            },
-            TextElement {
-                byte_range: ByteRange {
+                Some(placeholder2),
+            ),
+            TextElement::new(
+                ByteRange {
                     start: start1,
                     end: start1 + placeholder1.len(),
                 },
-                placeholder: Some(placeholder1.clone()),
-            },
+                Some(placeholder1.clone()),
+            ),
         ];
         composer.set_text_content(text, text_elements, vec![path1, path2.clone()]);
 

codex-rs/tui/src/bottom_pane/textarea.rs

@@ -755,13 +755,13 @@ impl TextArea {
             .iter()
             .map(|e| {
                 let placeholder = self.text.get(e.range.clone()).map(str::to_string);
-                UserTextElement {
-                    byte_range: ByteRange {
+                UserTextElement::new(
+                    ByteRange {
                         start: e.range.start,
                         end: e.range.end,
                     },
                     placeholder,
-                }
+                )
             })
             .collect()
     }

codex-rs/tui/src/chatwidget.rs

@@ -614,9 +614,9 @@ fn remap_placeholders_for_message(message: UserMessage, next_label: &mut usize)
         }
 
         let original = text.get(start..end).unwrap_or("");
-        let placeholder_key = elem.placeholder.as_deref().unwrap_or(original);
-        let replacement = mapping
-            .get(placeholder_key)
+        let placeholder = elem.placeholder(&text);
+        let replacement = placeholder
+            .and_then(|ph| mapping.get(ph))
             .map(String::as_str)
             .unwrap_or(original);
 
@@ -624,8 +624,8 @@ fn remap_placeholders_for_message(message: UserMessage, next_label: &mut usize)
         rebuilt.push_str(replacement);
         let elem_end = rebuilt.len();
 
-        if let Some(remapped) = mapping.get(placeholder_key) {
-            elem.placeholder = Some(remapped.clone());
+        if let Some(remapped) = placeholder.and_then(|ph| mapping.get(ph)) {
+            elem.set_placeholder(Some(remapped.clone()));
         }
         elem.byte_range = (elem_start..elem_end).into();
         rebuilt_elements.push(elem);

codex-rs/tui/src/chatwidget/tests.rs

@@ -192,10 +192,10 @@ async fn replayed_user_message_preserves_text_elements_and_local_images() {
 
     let placeholder = "[Image #1]";
     let message = format!("{placeholder} replayed");
-    let text_elements = vec![TextElement {
-        byte_range: (0..placeholder.len()).into(),
-        placeholder: Some(placeholder.to_string()),
-    }];
+    let text_elements = vec![TextElement::new(
+        (0..placeholder.len()).into(),
+        Some(placeholder.to_string()),
+    )];
     let local_images = vec![PathBuf::from("/tmp/replay.png")];
 
     let conversation_id = ThreadId::new();
@@ -274,10 +274,10 @@ async fn submission_preserves_text_elements_and_local_images() {
 
     let placeholder = "[Image #1]";
     let text = format!("{placeholder} submit");
-    let text_elements = vec![TextElement {
-        byte_range: (0..placeholder.len()).into(),
-        placeholder: Some(placeholder.to_string()),
-    }];
+    let text_elements = vec![TextElement::new(
+        (0..placeholder.len()).into(),
+        Some(placeholder.to_string()),
+    )];
     let local_images = vec![PathBuf::from("/tmp/submitted.png")];
 
     chat.bottom_pane
@@ -330,26 +330,26 @@ async fn interrupted_turn_restores_queued_messages_with_images_and_elements() {
 
     let first_placeholder = "[Image #1]";
     let first_text = format!("{first_placeholder} first");
-    let first_elements = vec![TextElement {
-        byte_range: (0..first_placeholder.len()).into(),
-        placeholder: Some(first_placeholder.to_string()),
-    }];
+    let first_elements = vec![TextElement::new(
+        (0..first_placeholder.len()).into(),
+        Some(first_placeholder.to_string()),
+    )];
     let first_images = [PathBuf::from("/tmp/first.png")];
 
     let second_placeholder = "[Image #1]";
     let second_text = format!("{second_placeholder} second");
-    let second_elements = vec![TextElement {
-        byte_range: (0..second_placeholder.len()).into(),
-        placeholder: Some(second_placeholder.to_string()),
-    }];
+    let second_elements = vec![TextElement::new(
+        (0..second_placeholder.len()).into(),
+        Some(second_placeholder.to_string()),
+    )];
     let second_images = [PathBuf::from("/tmp/second.png")];
 
     let existing_placeholder = "[Image #1]";
     let existing_text = format!("{existing_placeholder} existing");
-    let existing_elements = vec![TextElement {
-        byte_range: (0..existing_placeholder.len()).into(),
-        placeholder: Some(existing_placeholder.to_string()),
-    }];
+    let existing_elements = vec![TextElement::new(
+        (0..existing_placeholder.len()).into(),
+        Some(existing_placeholder.to_string()),
+    )];
     let existing_images = vec![PathBuf::from("/tmp/existing.png")];
 
     chat.queued_user_messages.push_back(UserMessage {
@@ -392,18 +392,18 @@ async fn interrupted_turn_restores_queued_messages_with_images_and_elements() {
     let second_start = first.len() + 1;
     let third_start = second_start + second.len() + 1;
     let expected_elements = vec![
-        TextElement {
-            byte_range: (first_start..first_start + "[Image #1]".len()).into(),
-            placeholder: Some("[Image #1]".to_string()),
-        },
-        TextElement {
-            byte_range: (second_start..second_start + "[Image #2]".len()).into(),
-            placeholder: Some("[Image #2]".to_string()),
-        },
-        TextElement {
-            byte_range: (third_start..third_start + "[Image #3]".len()).into(),
-            placeholder: Some("[Image #3]".to_string()),
-        },
+        TextElement::new(
+            (first_start..first_start + "[Image #1]".len()).into(),
+            Some("[Image #1]".to_string()),
+        ),
+        TextElement::new(
+            (second_start..second_start + "[Image #2]".len()).into(),
+            Some("[Image #2]".to_string()),
+        ),
+        TextElement::new(
+            (third_start..third_start + "[Image #3]".len()).into(),
+            Some("[Image #3]".to_string()),
+        ),
     ];
     assert_eq!(chat.bottom_pane.composer_text_elements(), expected_elements);
     assert_eq!(
@@ -422,14 +422,14 @@ async fn remap_placeholders_uses_attachment_labels() {
     let placeholder_two = "[Image #2]";
     let text = format!("{placeholder_two} before {placeholder_one}");
     let elements = vec![
-        TextElement {
-            byte_range: (0..placeholder_two.len()).into(),
-            placeholder: Some(placeholder_two.to_string()),
-        },
-        TextElement {
-            byte_range: ("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
-            placeholder: Some(placeholder_one.to_string()),
-        },
+        TextElement::new(
+            (0..placeholder_two.len()).into(),
+            Some(placeholder_two.to_string()),
+        ),
+        TextElement::new(
+            ("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
+            Some(placeholder_one.to_string()),
+        ),
     ];
 
     let attachments = vec![
@@ -454,15 +454,14 @@ async fn remap_placeholders_uses_attachment_labels() {
     assert_eq!(
         remapped.text_elements,
         vec![
-            TextElement {
-                byte_range: (0.."[Image #4]".len()).into(),
-                placeholder: Some("[Image #4]".to_string()),
-            },
-            TextElement {
-                byte_range: ("[Image #4] before ".len().."[Image #4] before [Image #3]".len())
-                    .into(),
-                placeholder: Some("[Image #3]".to_string()),
-            },
+            TextElement::new(
+                (0.."[Image #4]".len()).into(),
+                Some("[Image #4]".to_string()),
+            ),
+            TextElement::new(
+                ("[Image #4] before ".len().."[Image #4] before [Image #3]".len()).into(),
+                Some("[Image #3]".to_string()),
+            ),
         ]
     );
     assert_eq!(
@@ -486,14 +485,11 @@ async fn remap_placeholders_uses_byte_ranges_when_placeholder_missing() {
     let placeholder_two = "[Image #2]";
     let text = format!("{placeholder_two} before {placeholder_one}");
     let elements = vec![
-        TextElement {
-            byte_range: (0..placeholder_two.len()).into(),
-            placeholder: None,
-        },
-        TextElement {
-            byte_range: ("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
-            placeholder: None,
-        },
+        TextElement::new((0..placeholder_two.len()).into(), None),
+        TextElement::new(
+            ("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
+            None,
+        ),
     ];
 
     let attachments = vec![
@@ -518,15 +514,14 @@ async fn remap_placeholders_uses_byte_ranges_when_placeholder_missing() {
     assert_eq!(
         remapped.text_elements,
         vec![
-            TextElement {
-                byte_range: (0.."[Image #4]".len()).into(),
-                placeholder: Some("[Image #4]".to_string()),
-            },
-            TextElement {
-                byte_range: ("[Image #4] before ".len().."[Image #4] before [Image #3]".len())
-                    .into(),
-                placeholder: Some("[Image #3]".to_string()),
-            },
+            TextElement::new(
+                (0.."[Image #4]".len()).into(),
+                Some("[Image #4]".to_string()),
+            ),
+            TextElement::new(
+                ("[Image #4] before ".len().."[Image #4] before [Image #3]".len()).into(),
+                Some("[Image #3]".to_string()),
+            ),
         ]
     );
     assert_eq!(