docs(iconography): complete Fluent + Material icon sets (20+20) #560
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -224,8 +224,6 @@ client_request_definitions! { | |
| params: v2::ThreadUnsubscribeParams, | ||
| response: v2::ThreadUnsubscribeResponse, | ||
| }, | ||
| #[experimental("thread/increment_elicitation")] | ||
| /// Increment the thread-local out-of-band elicitation counter. | ||
| /// | ||
|
|
@@ -243,7 +241,6 @@ client_request_definitions! { | |
| params: v2::ThreadDecrementElicitationParams, | ||
| response: v2::ThreadDecrementElicitationResponse, | ||
| }, | ||
| ThreadSetName => "thread/name/set" { | ||
| params: v2::ThreadSetNameParams, | ||
| response: v2::ThreadSetNameResponse, | ||
|
|
@@ -756,8 +753,6 @@ server_request_definitions! { | |
| response: v2::ToolRequestUserInputResponse, | ||
| }, | ||
|
|
||
| /// Request input for an MCP server elicitation. | ||
| McpServerElicitationRequest => "mcpServer/elicitation/request" { | ||
| params: v2::McpServerElicitationRequestParams, | ||
|
|
@@ -770,7 +765,6 @@ server_request_definitions! { | |
| response: v2::PermissionsRequestApprovalResponse, | ||
| }, | ||
|
|
||
| /// Execute a dynamic tool call on the client. | ||
| DynamicToolCall => "item/tool/call" { | ||
| params: v2::DynamicToolCallParams, | ||
|
|
@@ -887,10 +881,7 @@ server_notification_definitions! { | |
| ThreadArchived => "thread/archived" (v2::ThreadArchivedNotification), | ||
| ThreadUnarchived => "thread/unarchived" (v2::ThreadUnarchivedNotification), | ||
| ThreadClosed => "thread/closed" (v2::ThreadClosedNotification), | ||
| SkillsChanged => "skills/changed" (v2::SkillsChangedNotification), | ||
| ThreadNameUpdated => "thread/name/updated" (v2::ThreadNameUpdatedNotification), | ||
| ThreadTokenUsageUpdated => "thread/tokenUsage/updated" (v2::ThreadTokenUsageUpdatedNotification), | ||
| TurnStarted => "turn/started" (v2::TurnStartedNotification), | ||
|
|
@@ -935,11 +926,8 @@ server_notification_definitions! { | |
| ThreadRealtimeStarted => "thread/realtime/started" (v2::ThreadRealtimeStartedNotification), | ||
| #[experimental("thread/realtime/itemAdded")] | ||
| ThreadRealtimeItemAdded => "thread/realtime/itemAdded" (v2::ThreadRealtimeItemAddedNotification), | ||
| #[experimental("thread/realtime/transcriptUpdated")] | ||
| ThreadRealtimeTranscriptUpdated => "thread/realtime/transcriptUpdated" (v2::ThreadRealtimeTranscriptUpdatedNotification), | ||
| #[experimental("thread/realtime/outputAudio/delta")] | ||
| ThreadRealtimeOutputAudioDelta => "thread/realtime/outputAudio/delta" (v2::ThreadRealtimeOutputAudioDeltaNotification), | ||
| #[experimental("thread/realtime/error")] | ||
|
|
@@ -969,20 +957,16 @@ mod tests { | |
| use codex_protocol::ThreadId; | ||
| use codex_protocol::account::PlanType; | ||
| use codex_protocol::parse_command::ParsedCommand; | ||
| use codex_protocol::protocol::AskForApproval; | ||
| use codex_protocol::protocol::RealtimeConversationVersion; | ||
| use codex_utils_absolute_path::AbsolutePathBuf; | ||
| use pretty_assertions::assert_eq; | ||
| use serde_json::json; | ||
| use std::path::PathBuf; | ||
|
|
||
| fn absolute_path(path: &str) -> AbsolutePathBuf { | ||
| AbsolutePathBuf::from_absolute_path(path).expect("absolute path") | ||
| } | ||
| fn absolute_path_string(path: &str) -> String { | ||
| let trimmed = path.trim_start_matches('/'); | ||
| if cfg!(windows) { | ||
|
|
@@ -994,7 +978,6 @@ mod tests { | |
|
|
||
| fn absolute_path(path: &str) -> AbsolutePathBuf { | ||
| AbsolutePathBuf::from_absolute_path(absolute_path_string(path)).expect("absolute path") | ||
| } | ||
|
|
||
| #[test] | ||
|
|
@@ -1631,10 +1614,7 @@ mod tests { | |
| sample_rate: 24_000, | ||
| num_channels: 1, | ||
| samples_per_channel: Some(512), | ||
| item_id: None, | ||
| }, | ||
| }, | ||
| ); | ||
|
|
@@ -1647,12 +1627,9 @@ mod tests { | |
| "data": "AQID", | ||
| "sampleRate": 24000, | ||
| "numChannels": 1, | ||
| "samplesPerChannel": 512 | ||
| "samplesPerChannel": 512, | ||
|
Comment on lines
1630
to
1631
There was a problem hiding this comment. Suggestion: The JSON assertion contains Severity Level: Major
|
||
| "itemId": null | ||
| } | ||
| } | ||
| }), | ||
|
|
@@ -1689,10 +1666,7 @@ mod tests { | |
| ServerNotification::ThreadRealtimeStarted(v2::ThreadRealtimeStartedNotification { | ||
| thread_id: "thr_123".to_string(), | ||
| session_id: Some("sess_456".to_string()), | ||
| version: RealtimeConversationVersion::V1, | ||
| }); | ||
| let reason = crate::experimental_api::ExperimentalApi::experimental_reason(¬ification); | ||
| assert_eq!(reason, Some("thread/realtime/started")); | ||
|
|
@@ -1708,10 +1682,7 @@ mod tests { | |
| sample_rate: 24_000, | ||
| num_channels: 1, | ||
| samples_per_channel: Some(512), | ||
| item_id: None, | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,11 @@ use std::collections::HashMap; | |
| use std::path::PathBuf; | ||
|
|
||
| use codex_git_utils::GitSha; | ||
| use uuid::Uuid; | ||
| use codex_protocol::ThreadId; | ||
| use codex_protocol::user_input::ByteRange; | ||
| use codex_protocol::user_input::TextElement; | ||
|
There was a problem hiding this comment. Suggestion: Severity Level: Critical 🚨- ❌ App-server protocol crate fails to compile.
- ❌ v1 user-input text element conversions unavailable.
- ❌ Workspace build for app-server and CLI blocked.Steps of Reproduction ✅1. Open `codex-rs/app-server-protocol/src/protocol/v1.rs` and observe at line 8 (from the
PR hunk) the import `use codex_protocol::user_input::TextElement;` without any alias, and
note that there is no other definition or import of `CoreTextElement` anywhere in this
file (lines 1–378 as read from the repository).
2. Scroll down in the same file to lines 326–337 where the conversion implementations are
defined: `impl From<CoreTextElement> for V1TextElement` and `impl From<V1TextElement> for
CoreTextElement`, both referring to a `CoreTextElement` type that is not brought into
scope in `v1.rs`.
3. Compare this with `codex-rs/app-server-protocol/src/protocol/v2.rs`, where at line 90
the core text element is explicitly imported as an alias via `use
codex_protocol::user_input::TextElement as CoreTextElement;`, and later conversion impls
`impl From<CoreTextElement> for TextElement` and `impl From<TextElement> for
CoreTextElement` compile successfully because the alias matches the type name used in the
impls.
4. From the workspace root `/workspace/helios-cli`, run `cargo build` (or `cargo build -p
codex-rs-app-server-protocol` depending on the workspace configuration) and observe the
Rust compiler error at `codex-rs/app-server-protocol/src/protocol/v1.rs:326` and `:335`
stating that the type `CoreTextElement` cannot be found in this scope, demonstrating that
the current import `use codex_protocol::user_input::TextElement;` leaves the conversion
target type unresolved and breaks the v1↔core text-element conversion path until it is
updated to `use codex_protocol::user_input::TextElement as CoreTextElement;` in line with
`v2.rs`.Fix in Cursor | Fix in VSCode Claude (Use Cmd/Ctrl + Click for best experience) Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** codex-rs/app-server-protocol/src/protocol/v1.rs
**Line:** 8:8
**Comment:**
*Type Error: `CoreTextElement` is used by the conversion impls in this file, but this import brings in `TextElement` without the alias those impls rely on. That leaves the conversion type unresolved and breaks the v1↔core text-element conversion path. Import it with the correct alias so the conversion impls target the intended core type.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix |
||
| use codex_protocol::protocol::EventMsg; | ||
| use codex_protocol::config_types::ForcedLoginMethod; | ||
| use codex_protocol::config_types::ReasoningSummary; | ||
| use codex_protocol::config_types::SandboxMode; | ||
|
|
@@ -243,7 +247,6 @@ pub struct SandboxSettings { | |
| pub struct InterruptConversationResponse { | ||
| pub abort_reason: TurnAbortReason, | ||
| } | ||
|
|
||
| #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] | ||
| #[serde(rename_all = "camelCase")] | ||
|
|
@@ -292,16 +295,16 @@ pub struct V1ByteRange { | |
| pub end: usize, | ||
| } | ||
|
|
||
| impl From<ByteRange> for V1ByteRange { | ||
| fn from(value: ByteRange) -> Self { | ||
| Self { | ||
| start: value.start, | ||
| end: value.end, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl From<V1ByteRange> for ByteRange { | ||
| fn from(value: V1ByteRange) -> Self { | ||
| Self { | ||
| start: value.start, | ||
|
|
@@ -373,5 +376,3 @@ pub struct SessionConfiguredNotification { | |
| pub struct AuthStatusChangeNotification { | ||
| pub auth_method: Option<AuthMode>, | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: This creates a second
absolute_pathfunction in the same test module, which conflicts with the later platform-normalizing version and causes duplicate-definition breakage. Remove the duplicate helper and keep only the cross-platform implementation. [possible bug]Severity Level: Major⚠️
Steps of Reproduction ✅
Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖