Fix quicktype and regenerate types#345
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93ee64b21c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f533ac0fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 679d91e793
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
|
Codex (@codex) review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 679d91e793
ℹ️ 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 (@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 (@codex) address that feedback".
| let mut merged = provider_reasoning.clone(); | ||
| merged.effort = canonical_reasoning.effort; | ||
| merged.summary = canonical_reasoning.summary; | ||
| merged.generate_summary = canonical_reasoning.generate_summary; |
There was a problem hiding this comment.
Preserve raw Responses reasoning fields
When a request originates from Responses and has raw reasoning preserved in extras, this merge overwrites the raw effort/summary/generate_summary with the canonical values. The canonical conversion is lossy for existing valid inputs (for example summary: "concise" maps to universal Auto and serializes back as "auto"; a summary-only request can also synthesize a default effort), so a Responses round-trip changes the user's request even though the raw provider value is available.
Useful? React with 👍 / 👎.
| pub caller_type: ToolCallerType, | ||
| pub caller_id: String, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub caller_id: Option<String>, |
There was a problem hiding this comment.
Mark caller_id optional in bindings
For direct callers this field is now None and serde omits it because of skip_serializing_if, producing JSON like caller: { "type": "direct" }. Without #[ts(optional)], the generated TypeScript ToolCaller type requires caller_id: string | null, so TypeScript consumers reading Rust-produced universal JSON for direct tool calls will see a shape that does not match the binding.
Useful? React with 👍 / 👎.
|
Looks good besides the bot comments I have a pr for this one that fixes the merge stuff/some of the issues called out above: #354 We automatically make new PRs in this repo every monday where we try to regenerate types for everything so we probably would have caught it today if we weren't out of anthropic credits 😬 |
The OpenAI schema gen pipeline is currently broken because the OpenAPI schema is dramatically different and breaks QuickType. I have a PR open here glideapps/quicktype#2914 but in the meantime have that patched in this repo. I ran it, updated the types, and fixed some things.
In general I think we need to be careful about not letting this pipeline break.