feat(ai): openai gpt-5.5 chat support#4012
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds two new OpenAI models — GPT-5.5 and GPT-5 Mini — to the agent stack. On the Rust side, a new 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4193afb to
1531e33
Compare
ae95a9e to
d20f9a9
Compare
- agent: route Gpt5_5/Gpt5Mini through rig's OpenAI Responses API with reasoning params, non-strict tools, and provider-aware model selection - depend on macro-inc/rig fork (upstream main + with_non_strict_tools()) until the patch lands upstream - persist/replay tool calls with call_id so OpenAI conversations survive the DB round-trip (fc_-prefixed item ids, verified against live API) - re-enable ListEntities now that non-strict mode sends its schema verbatim - frontend: model picker entries + generated schemas for the new models
1531e33 to
19a406e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rust/cloud-storage/agent/src/agent_loop.rs (1)
178-178: ⚡ Quick winAdd
errtotracing::instrumentforResult-returning function.Per coding guidelines,
tracing::instrumentshould includeerrwhen the function returnsResult.Suggested fix
- #[tracing::instrument(name = "invoke_agent", skip_all)] + #[tracing::instrument(name = "invoke_agent", skip_all, err)]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/cloud-storage/agent/src/agent_loop.rs` at line 178, The tracing::instrument macro decorator on the invoke_agent function is missing the err parameter. According to coding guidelines, when a function returns Result, the tracing::instrument attribute must include err to enable automatic error logging. Add err as an additional parameter to the existing tracing::instrument macro on the invoke_agent function, keeping the existing name and skip_all parameters.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/cloud-storage/ai_tools/src/bin/gen_ai_request_schemas.rs`:
- Around line 42-44: The condition checking for `Some(parent)` from
`std::path::Path::new(&out_path).parent()` does not guard against empty parent
paths. When the output path is just a filename (e.g., `out.json`), the parent
exists but is empty, and calling `create_dir_all` on an empty path will error.
Add an additional check inside the existing `if let Some(parent)` block to
verify the parent path is not empty before calling `create_dir_all(parent)`.
This can be done by checking if the parent path has any components or is not
equal to an empty path.
---
Nitpick comments:
In `@rust/cloud-storage/agent/src/agent_loop.rs`:
- Line 178: The tracing::instrument macro decorator on the invoke_agent function
is missing the err parameter. According to coding guidelines, when a function
returns Result, the tracing::instrument attribute must include err to enable
automatic error logging. Add err as an additional parameter to the existing
tracing::instrument macro on the invoke_agent function, keeping the existing
name and skip_all parameters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f49b2f33-783f-45a2-ab6e-040bab4b50a0
⛔ Files ignored due to path filters (5)
js/app/packages/core/component/AI/assets/openai.svgis excluded by!**/*.svgjs/app/packages/service-clients/service-cognition/generated/schemas/agentModel.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/model.tsis excluded by!**/generated/**js/app/packages/service-clients/service-scheduled-action/generated/schemas/agentModel.tsis excluded by!**/generated/**rust/cloud-storage/Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (18)
js/app/packages/core/component/AI/constant/model.tsjs/app/packages/service-clients/service-cognition/openapi.jsonjs/app/packages/service-clients/service-scheduled-action/openapi.jsonrust/cloud-storage/agent/Cargo.tomlrust/cloud-storage/agent/src/agent_loop.rsrust/cloud-storage/agent/src/anthropic_model.rsrust/cloud-storage/agent/src/anthropic_model/test.rsrust/cloud-storage/agent/src/completion.rsrust/cloud-storage/agent/src/convert.rsrust/cloud-storage/agent/src/convert/test.rsrust/cloud-storage/agent/src/lib.rsrust/cloud-storage/agent/src/model.rsrust/cloud-storage/agent/src/model/test.rsrust/cloud-storage/agent/src/tool_adapter.rsrust/cloud-storage/agent/src/tool_adapter/test.rsrust/cloud-storage/ai_tools/src/bin/gen_ai_request_schemas.rsrust/cloud-storage/document_cognition_service/src/core/model.rsrust/cloud-storage/document_cognition_service/src/models_bin.rs
💤 Files with no reviewable changes (2)
- rust/cloud-storage/agent/src/anthropic_model/test.rs
- rust/cloud-storage/agent/src/anthropic_model.rs
| if let Some(parent) = std::path::Path::new(&out_path).parent() { | ||
| std::fs::create_dir_all(parent).expect("create output dir"); | ||
| } |
There was a problem hiding this comment.
Guard empty parent paths before create_dir_all.
Line 42-44 can fail for filename-only outputs (e.g., gen_ai_request_schemas out.json) because parent() may be empty, and create_dir_all("") errors. Skip directory creation when the parent path is empty.
Proposed fix
- if let Some(parent) = std::path::Path::new(&out_path).parent() {
- std::fs::create_dir_all(parent).expect("create output dir");
+ if let Some(parent) = std::path::Path::new(&out_path).parent() {
+ if !parent.as_os_str().is_empty() {
+ std::fs::create_dir_all(parent).expect("create output dir");
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if let Some(parent) = std::path::Path::new(&out_path).parent() { | |
| std::fs::create_dir_all(parent).expect("create output dir"); | |
| } | |
| if let Some(parent) = std::path::Path::new(&out_path).parent() { | |
| if !parent.as_os_str().is_empty() { | |
| std::fs::create_dir_all(parent).expect("create output dir"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/cloud-storage/ai_tools/src/bin/gen_ai_request_schemas.rs` around lines
42 - 44, The condition checking for `Some(parent)` from
`std::path::Path::new(&out_path).parent()` does not guard against empty parent
paths. When the output path is just a filename (e.g., `out.json`), the parent
exists but is empty, and calling `create_dir_all` on an empty path will error.
Add an additional check inside the existing `if let Some(parent)` block to
verify the parent path is not empty before calling `create_dir_all(parent)`.
This can be done by checking if the parent path has any components or is not
equal to an empty path.
No description provided.