mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-29 08:59:31 +00:00
fix: address Copilot PR review feedback
- Fix empty text part for assistant messages with tool calls (curate_contents could drop entire model turn) - Propagate cache_read/creation_input_tokens in complete_with_tools - Log warning on save_credential failure instead of silently ignoring - Fix doc comment to mention underscore in header name pattern - Handle gemini-oauth (hyphen variant) in setup wizard display - Fix docs: thinkingConfig uses thinkingBudget/thinkingLevel, not includeThoughts
This commit is contained in:
+1
-1
@@ -205,7 +205,7 @@ This document tracks feature parity between IronClaw (Rust implementation) and O
|
||||
| Skills (modular capabilities) | ✅ | ✅ | Prompt-based skills with trust gating, attenuation, activation criteria, catalog, selector |
|
||||
| Skill routing blocks | ✅ | 🚧 | ActivationCriteria (keywords, patterns, tags) but no "Use when / Don't use when" blocks |
|
||||
| Skill path compaction | ✅ | ❌ | ~ prefix to reduce prompt tokens |
|
||||
| Thinking modes (off/minimal/low/medium/high/xhigh/adaptive) | ✅ | 🚧 | thinkingConfig for Gemini models (includeThoughts); no per-level control yet |
|
||||
| Thinking modes (off/minimal/low/medium/high/xhigh/adaptive) | ✅ | 🚧 | thinkingConfig for Gemini models (thinkingBudget/thinkingLevel); no per-level control yet |
|
||||
| Per-model thinkingDefault override | ✅ | ❌ | Override thinking level per model; Anthropic Claude 4.6 defaults to adaptive |
|
||||
| Block-level streaming | ✅ | ❌ | |
|
||||
| Tool-level streaming | ✅ | ❌ | |
|
||||
|
||||
@@ -78,7 +78,7 @@ GEMINI_MODEL=gemini-2.5-flash
|
||||
|---|---|---|
|
||||
| Function calling | ✅ | `functionDeclarations` / `functionCall` / `functionResponse` |
|
||||
| `generationConfig` | ✅ | `temperature`, `maxOutputTokens` passed from request |
|
||||
| `thinkingConfig` | ✅ | `includeThoughts: true` for `gemini-3`/`thinking` models |
|
||||
| `thinkingConfig` | ✅ | `thinkingBudget`/`thinkingLevel` for thinking-capable models |
|
||||
| `toolConfig` | ✅ | `functionCallingConfig.mode`: `AUTO`/`ANY`/`NONE` |
|
||||
| SSE streaming | ✅ | Cloud Code API with `streamGenerateContent?alt=sse` |
|
||||
| Token refresh | ✅ | Automatic via refresh token |
|
||||
|
||||
+18
-5
@@ -89,7 +89,7 @@ fn default_safety_settings() -> Vec<serde_json::Value> {
|
||||
|
||||
/// Parse `GEMINI_CLI_CUSTOM_HEADERS` env var in format `key:value,key:value`.
|
||||
/// Commas inside values are preserved — splits only on commas followed by a
|
||||
/// valid HTTP header name pattern (ASCII alphanumeric/hyphen, then `:`).
|
||||
/// valid HTTP header name pattern (ASCII alphanumeric/hyphen/underscore, then `:`).
|
||||
fn parse_custom_headers() -> std::collections::HashMap<String, String> {
|
||||
let mut headers = std::collections::HashMap::new();
|
||||
let env_val = match std::env::var("GEMINI_CLI_CUSTOM_HEADERS") {
|
||||
@@ -397,7 +397,9 @@ impl CredentialManager {
|
||||
if let Some(pid) = self.discover_project_id(&updated.access_token).await {
|
||||
info!(project_id = %pid, "Discovered Cloud Code project");
|
||||
updated.project_id = Some(pid);
|
||||
let _ = self.save_credential(&updated).await;
|
||||
if let Err(e) = self.save_credential(&updated).await {
|
||||
warn!(error = %e, "Failed to persist discovered project_id to credentials file");
|
||||
}
|
||||
}
|
||||
return Ok(updated);
|
||||
}
|
||||
@@ -1617,7 +1619,13 @@ impl GeminiOauthProvider {
|
||||
}));
|
||||
}
|
||||
Role::Assistant => {
|
||||
let mut parts = vec![serde_json::json!({ "text": msg.content })];
|
||||
let mut parts = Vec::new();
|
||||
// Only add text part if content is non-empty (assistant messages
|
||||
// with tool calls often have empty content, and curate_contents
|
||||
// would drop the entire turn if it sees an empty text part).
|
||||
if !msg.content.is_empty() {
|
||||
parts.push(serde_json::json!({ "text": msg.content }));
|
||||
}
|
||||
if let Some(ref calls) = msg.tool_calls {
|
||||
for call in calls {
|
||||
parts.push(serde_json::json!({
|
||||
@@ -1628,6 +1636,11 @@ impl GeminiOauthProvider {
|
||||
}));
|
||||
}
|
||||
}
|
||||
// Fallback: if no parts at all, add empty text to avoid
|
||||
// sending a turn with zero parts (API rejects it).
|
||||
if parts.is_empty() {
|
||||
parts.push(serde_json::json!({ "text": "" }));
|
||||
}
|
||||
contents.push(serde_json::json!({
|
||||
"role": "model",
|
||||
"parts": parts
|
||||
@@ -2080,8 +2093,8 @@ impl LlmProvider for GeminiOauthProvider {
|
||||
input_tokens: response.input_tokens,
|
||||
output_tokens: response.output_tokens,
|
||||
tool_calls,
|
||||
cache_read_input_tokens: 0,
|
||||
cache_creation_input_tokens: 0,
|
||||
cache_read_input_tokens: response.cache_read_input_tokens,
|
||||
cache_creation_input_tokens: response.cache_creation_input_tokens,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+3
-2
@@ -1037,7 +1037,7 @@ impl SetupWizard {
|
||||
} else {
|
||||
match current.as_str() {
|
||||
"nearai" => "NEAR AI".to_string(),
|
||||
"gemini_oauth" => "Gemini API (OAuth)".to_string(),
|
||||
"gemini_oauth" | "gemini-oauth" => "Gemini API (OAuth)".to_string(),
|
||||
_ => {
|
||||
if let Some(def) = registry.find(¤t) {
|
||||
def.setup
|
||||
@@ -1055,6 +1055,7 @@ impl SetupWizard {
|
||||
let is_known = current == "nearai"
|
||||
|| current == "bedrock"
|
||||
|| current == "gemini_oauth"
|
||||
|| current == "gemini-oauth"
|
||||
|| registry.is_known(¤t);
|
||||
|
||||
if is_known && confirm("Keep current provider?", true).map_err(SetupError::Io)? {
|
||||
@@ -1062,7 +1063,7 @@ impl SetupWizard {
|
||||
print_info("Keeping existing AWS Bedrock configuration.");
|
||||
return Ok(());
|
||||
}
|
||||
if current == "gemini_oauth" {
|
||||
if current == "gemini_oauth" || current == "gemini-oauth" {
|
||||
print_info("Keeping existing Gemini CLI OAuth configuration.");
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user