[app-server] generate v2 types into a v2/ namespace #6061
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
owenlin0
force-pushed
the
owen/separate_v2_namespace
branch
from
ba14c1f to
7026f63
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "ServerRequest", | ||
| ]; | ||
|
|
||
| for (name, schema) in bundle { | ||
| let mut schema_value = serde_json::to_value(schema)?; | ||
| annotate_schema(&mut schema_value, Some(name.as_str())); | ||
| // Helper to process a bundle map into a definitions object, optionally rewriting | ||
| // refs to v2 but only for known v2 type names. When `drop_v2_defs` is true, any | ||
| // hoisted definitions whose name is a v2 type will be skipped to avoid | ||
| // duplicating them at the root while still preserving references by rewriting. | ||
| fn append_bundle( | ||
| src: BTreeMap<String, RootSchema>, | ||
| out_defs: &mut Map<String, Value>, | ||
| v2_names: &HashSet<String>, | ||
| rewrite_v2_refs: bool, | ||
| drop_v2_defs: bool, | ||
| ) -> Result<()> { | ||
| for (name, schema) in src { | ||
| let mut schema_value = serde_json::to_value(schema)?; | ||
| annotate_schema(&mut schema_value, Some(name.as_str())); | ||
| if rewrite_v2_refs { | ||
| rewrite_json_refs_to_v2_in_set(&mut schema_value, v2_names); | ||
| } | ||
|
|
||
| if let Value::Object(ref mut obj) = schema_value | ||
| && let Some(defs) = obj.remove("definitions") | ||
| && let Value::Object(defs_obj) = defs | ||
| { | ||
| for (def_name, mut def_schema) in defs_obj { | ||
| if !SPECIAL_DEFINITIONS.contains(&def_name.as_str()) { | ||
| annotate_schema(&mut def_schema, Some(def_name.as_str())); | ||
| definitions.insert(def_name, def_schema); | ||
| if let Value::Object(ref mut obj) = schema_value | ||
| && let Some(defs) = obj.remove("definitions") | ||
| && let Value::Object(defs_obj) = defs | ||
| { | ||
| for (def_name, mut def_schema) in defs_obj { | ||
| if !SPECIAL_DEFINITIONS.contains(&def_name.as_str()) { | ||
| annotate_schema(&mut def_schema, Some(def_name.as_str())); | ||
| if rewrite_v2_refs { | ||
| rewrite_json_refs_to_v2_in_set(&mut def_schema, v2_names); | ||
| } |
There was a problem hiding this comment.
The new JSON schema generation only collects v2 type names from the top‑level bundle (let v2_names = bundle_v2.keys().cloned().collect()), and rewrite_json_refs_to_v2_in_set rewrites $refs only for names in that set. Schemars emits additional nested definitions for many of these v2 types (e.g. AccountApiKey, variant structs, etc.), and those definitions are now moved under definitions.v2 but are not present in v2_names. Any references to those nested definitions remain as #/definitions/<Name> while the actual definition lives under #/definitions/v2/<Name>, leaving invalid references in the aggregated codex_app_server_protocol.schemas.json. The rewrite set should include all definition names (including hoisted ones) to keep refs aligned with where the definitions end up.
Useful? React with 👍 / 👎.
While working on Thread/Turn/Items type definitions, I realize we will run into name conflicts between v1 and v2 APIs (e.g.
UserInputwhich won't be reusable since v1 usesUserInputfromprotocol/which uses snake_case, but we want to expose camelCase everywhere).To set us up for a clean and isolated v2 API, generate types into a
v2/namespace for both typescript and JSON schema.In the VSCode extension, v2 APIs will be updated to: