fix: switch Gemini models to anthropic provider and strip doubled /v1 prefix#47
Open
fix: switch Gemini models to anthropic provider and strip doubled /v1 prefix#47
Conversation
… prefix Switch the Gemini 3.1 Pro and Gemini 3 Flash custom model entries in SETUP.md and SettingsView.swift from provider="google" to provider="anthropic", and update the surrounding SETUP.md explanation to match. factory-cli 0.105.1 emits POST /v1/v1/messages for custom Gemini models under provider=anthropic, which 404s and triggers tool-call retry loops on the first turn. Add a geminiV1PathFix preference (default on) and a corresponding toggle in the Gemini settings pane. When enabled, ThinkingProxy rewrites /v1/v1/... (and /api/v1/v1/...) back to /v1/... before forwarding upstream.
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThese changes update Gemini model configuration to use the anthropic provider instead of google, and add a new conditional path rewriting feature to fix a factory-cli bug where Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a fix for a path issue in factory-cli where custom Gemini models using the Anthropic provider would incorrectly double the /v1 prefix in request paths. The changes include updating documentation in SETUP.md, adding a new preference toggle in AppPreferences.swift and SettingsView.swift, and implementing the path rewriting logic in ThinkingProxy.swift. A suggestion was made to refactor the path rewriting logic to reduce code duplication in logging and assignment.
Comment on lines
+270
to
+280
| if rewrittenPath.hasPrefix("/v1/v1/") { | ||
| let fixed = String(rewrittenPath.dropFirst(3)) // /v1/v1/messages -> /v1/messages | ||
| NSLog("[ThinkingProxy] Stripping doubled /v1 prefix: \(rewrittenPath) -> \(fixed)") | ||
| ThinkingProxy.fileLog("REWRITE PATH: \(rewrittenPath) -> \(fixed) (factory-cli /v1/v1 fix)") | ||
| rewrittenPath = fixed | ||
| } else if rewrittenPath.hasPrefix("/api/v1/v1/") { | ||
| let fixed = "/api" + String(rewrittenPath.dropFirst(7)) // /api/v1/v1/... -> /api/v1/... | ||
| NSLog("[ThinkingProxy] Stripping doubled /v1 prefix: \(rewrittenPath) -> \(fixed)") | ||
| ThinkingProxy.fileLog("REWRITE PATH: \(rewrittenPath) -> \(fixed) (factory-cli /v1/v1 fix)") | ||
| rewrittenPath = fixed | ||
| } |
Contributor
There was a problem hiding this comment.
The path rewrite logic for stripping the doubled /v1 prefix is correct but contains some code duplication in the logging and assignment. Consider refactoring this to be more concise and to avoid repeating the log messages, which makes the logic easier to maintain.
var fixed: String? = nil
if rewrittenPath.hasPrefix("/v1/v1/") {
fixed = String(rewrittenPath.dropFirst(3)) // /v1/v1/messages -> /v1/messages
} else if rewrittenPath.hasPrefix("/api/v1/v1/") {
fixed = "/api" + String(rewrittenPath.dropFirst(7)) // /api/v1/v1/... -> /api/v1/...
}
if let fixedPath = fixed {
NSLog("[ThinkingProxy] Stripping doubled /v1 prefix: \(rewrittenPath) -> \(fixedPath)")
ThinkingProxy.fileLog("REWRITE PATH: \(rewrittenPath) -> \(fixedPath) (factory-cli /v1/v1 fix)")
rewrittenPath = fixedPath
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@SETUP.md`:
- Line 84: Add an explicit migration note to SETUP.md telling users that if they
have older customModels entries using provider: "google" they must re-apply
models or manually edit ~/.factory/settings.json to update the provider/aliases
(e.g., switch to provider: "anthropic" / "openai" or use the new
Claude/Gemini/Codex aliases); reference the customModels key, the provider:
"google" value, and the settings file path so users know to update those fields
to avoid stale Gemini routing.
In `@src/Sources/SettingsView.swift`:
- Line 1417: Installed detection in SettingsView currently only checks IDs and
can mark old google->gemini-migrated entries as "Applied"; update the logic that
computes the 'Applied' status (the function that sets/returns the Applied state
in SettingsView — e.g., the computeAppliedState/updateAppliedStatus logic) to
validate critical fields beyond the ID: ensure the source's "provider" value
matches the expected provider (e.g., check that "provider" == "anthropic" for
entries using that provider) and verify any other critical config fields used by
routing; if mismatched, treat the entry as not applied so users can re-apply.
Ensure you update the same check used for UI display and any re-apply gating so
both lines (the entries using "provider": "anthropic" and the related duplicate
check) behave consistently.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6cf4cae1-111e-4984-841f-fa338f462c09
📒 Files selected for processing (4)
SETUP.mdsrc/Sources/AppPreferences.swiftsrc/Sources/SettingsView.swiftsrc/Sources/ThinkingProxy.swift
| ``` | ||
|
|
||
| Use the standard Claude and Codex model aliases in the `model` field. Claude entries use `provider: "anthropic"` with `http://localhost:8317`; GPT/Codex and Gemini entries use `provider: "openai"` with `http://localhost:8317/v1`. DroidProxy applies Claude adaptive thinking, Codex reasoning effort, and Gemini thinking levels based on the selected model and the effort/level setting in DroidProxy itself. | ||
| Use the standard Claude and Codex model aliases in the `model` field. Claude and Gemini entries use `provider: "anthropic"` with `http://localhost:8317`; GPT/Codex entries use `provider: "openai"` with `http://localhost:8317/v1`. DroidProxy applies Claude adaptive thinking, Codex reasoning effort, and Gemini thinking levels based on the selected model and the effort/level setting in DroidProxy itself. |
Contributor
There was a problem hiding this comment.
Add an explicit migration note for existing installs.
Please call out that users with older customModels entries (provider: "google") must re-apply models or edit ~/.factory/settings.json; otherwise Gemini routing may remain stale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SETUP.md` at line 84, Add an explicit migration note to SETUP.md telling
users that if they have older customModels entries using provider: "google" they
must re-apply models or manually edit ~/.factory/settings.json to update the
provider/aliases (e.g., switch to provider: "anthropic" / "openai" or use the
new Claude/Gemini/Codex aliases); reference the customModels key, the provider:
"google" value, and the settings file path so users know to update those fields
to avoid stale Gemini routing.
| "maxOutputTokens": 65536, | ||
| "noImageSupport": false, | ||
| "provider": "google" | ||
| "provider": "anthropic" |
Contributor
There was a problem hiding this comment.
Applied status can be incorrect after the Gemini provider migration.
Because install detection only checks IDs, existing entries with old provider: "google" can still show as installed, so users may never re-apply and keep failing routes.
Suggested fix (validate critical fields, not only IDs)
private func checkFactoryModelsInstalled() -> Bool {
let url = factorySettingsURL()
guard let data = try? Data(contentsOf: url),
let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any],
let models = json["customModels"] as? [[String: Any]] else {
return false
}
- let existingIds = Set(models.compactMap { $0["id"] as? String })
+ let existingById: [String: [String: Any]] = Dictionary(
+ uniqueKeysWithValues: models.compactMap { model in
+ guard let id = model["id"] as? String else { return nil }
+ return (id, model)
+ }
+ )
let enabledModels = Self.droidProxyModels.filter { model in
guard let key = providerKey(for: model) else { return true }
return serverManager.isProviderEnabled(key)
}
- let droidIds = Set(enabledModels.compactMap { $0["id"] as? String })
- return !droidIds.isEmpty && droidIds.isSubset(of: existingIds)
+ guard !enabledModels.isEmpty else { return false }
+ return enabledModels.allSatisfy { expected in
+ guard let id = expected["id"] as? String,
+ let existing = existingById[id] else { return false }
+ return (existing["provider"] as? String) == (expected["provider"] as? String)
+ && (existing["baseUrl"] as? String) == (expected["baseUrl"] as? String)
+ && (existing["model"] as? String) == (expected["model"] as? String)
+ }
}Also applies to: 1427-1427
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Sources/SettingsView.swift` at line 1417, Installed detection in
SettingsView currently only checks IDs and can mark old google->gemini-migrated
entries as "Applied"; update the logic that computes the 'Applied' status (the
function that sets/returns the Applied state in SettingsView — e.g., the
computeAppliedState/updateAppliedStatus logic) to validate critical fields
beyond the ID: ensure the source's "provider" value matches the expected
provider (e.g., check that "provider" == "anthropic" for entries using that
provider) and verify any other critical config fields used by routing; if
mismatched, treat the entry as not applied so users can re-apply. Ensure you
update the same check used for UI display and any re-apply gating so both lines
(the entries using "provider": "anthropic" and the related duplicate check)
behave consistently.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
provider: "google"toprovider: "anthropic"inSETUP.mdand thefactoryCustomModelsarray inSettingsView.swift, and update the surrounding SETUP.md prose to match.geminiV1PathFixpreference (default on) plus a checkbox in the Gemini settings pane to toggle it.ThinkingProxy, strip the doubled/v1prefix (/v1/v1/...→/v1/...,/api/v1/v1/...→/api/v1/...) that factory-cli 0.105.1 emits for custom Gemini models underprovider=anthropic, which otherwise 404s and triggers tool-call retry loops on the first turn.Breaking Changes
~/.factory/settings.jsonentries for the DroidProxy Gemini models onprovider: "google"will need to re-apply the custom models (or edit the provider field) to pick up the newanthropicrouting.Testing
~/.factory/settings.jsonnow useprovider: "anthropic"./v1/v1/messages.Fix factory-cli /v1/v1 path bugtoggle and confirm the path rewrite is skipped (paths pass through unchanged).