release: v1.1.7 — live model discovery for all providers#125
release: v1.1.7 — live model discovery for all providers#125Christiantyemele wants to merge 6 commits into
Conversation
- Fireworks: paginate /inference/v1/models (limit=200, cursor-based), 30s timeout, structured error logging, accept all accounts/fireworks/models/ prefixed models, minimal 2-model fallback - Anthropic: paginate /v1/models (limit=1000, after_id cursor), 30s timeout, structured error logging, minimal 2-model fallback - OpenAI: request /v1/models with limit=500, structured error handling, minimal 2-model fallback (previously returned bare error) - All providers: graceful degradation chain (API → CLI → hardcoded fallback with tracing::warn), no more silent failures - Update default Fireworks model from llama-v3p1-8b-instruct to deepseek-v3p1 across fireworks.rs, fallback.rs, mod.rs, process.rs - Add 3 new unit tests for fallback lists (Anthropic, OpenAI, Fireworks)
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
… error logging
The OpenAI-compatible /inference/v1/models endpoint does not support
the ?limit= query parameter — that's only for the native
/v1/accounts/{account_id}/models API. Adding it may cause unexpected
responses or empty results, which explains why live discovery was
failing and only the hardcoded fallback was shown.
Also adds detailed tracing::warn! for every failure path in the
Fireworks discovery so the exact error is visible in logs.
… discovery The OpenAI-compatible /inference/v1/models endpoint returns HTTP 500 'Error listing deployed models' — it's broken on Fireworks' side. Switched to the native Fireworks API which: - Uses /v1/accounts/fireworks/models?pageSize=200 with pageToken pagination - Returns rich model metadata (supportsServerless, contextLength, supportsTools, supportsImageInput, displayName) - Filters for serverless models with contextLength > 0 (excludes image gen, embeddings, rerankers) - Falls back to the OpenAI-compat endpoint if the native API returns an error - Shows context length, tool support, and vision capability in descriptions
Snif ReviewThis change replaces hardcoded model lists with live API-based model discovery for Anthropic, OpenAI, and Fireworks providers, adding pagination, 30s HTTP timeouts, structured error logging, and minimal hardcoded fallbacks. It also updates the default Fireworks model from llama-v3p1-8b-instruct to deepseek-v3p1. No issues found. Change looks clean. Review detailsChanged files:
Context analyzed: 99 related files (26 via semantic similarity, 99 via keyword matching). Stats: 1000 lines | |
The TUI appeared to freeze after entering the API key because discover_models() was called before any UI was drawn. Now shows a 'Discovering available models...' message with the provider name while the HTTP request is in flight.
Snif ReviewThis change replaces hardcoded model lists with live API-based model discovery for Anthropic, OpenAI, and Fireworks providers, adding pagination, timeouts, structured error logging, and minimal fallback lists. It also updates the default Fireworks model from llama-v3p1-8b-instruct to deepseek-v3p1 and adds a loading screen during model discovery in the TUI setup. No issues found. Change looks clean. Review detailsChanged files:
Context analyzed: 99 related files (25 via semantic similarity, 99 via keyword matching). Stats: 1060 lines | |
- Add max page limit (5 pages) and 60s overall timeout for Fireworks native API - Add connect_timeout (10s) to all HTTP clients - Filter empty nextPageToken strings that would cause infinite loops - Return partial results on timeout/API error instead of failing entirely - Add max pages to Anthropic and OpenAI-compat pagination too - Fix indentation issue in discover_openai_models (removed stray loop)
Snif ReviewThis change replaces hardcoded model lists with live API-based model discovery for Anthropic, OpenAI, and Fireworks providers, adding pagination, timeouts, structured error logging, and minimal fallback lists. It also updates the default Fireworks model from llama-v3p1-8b-instruct to deepseek-v3p1 and adds a loading screen in the TUI setup flow. No issues found. Change looks clean. Review detailsChanged files:
Context analyzed: 99 related files (25 via semantic similarity, 99 via keyword matching). Stats: 1117 lines | |
| fn ensure_default_branch_via_orphan_checkout(&self) -> Result<()> { | ||
| // Save current HEAD so we can restore it after creating the branch | ||
| let saved_head = Command::new("git") | ||
| .args(["rev-parse", "HEAD"]) | ||
| .current_dir(&self.project_root) | ||
| .output() | ||
| .ok() | ||
| .and_then(|o| { | ||
| if o.status.success() { | ||
| Some(String::from_utf8_lossy(&o.stdout).trim().to_string()) | ||
| } else { | ||
| None | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
⚠️ Bug: Orphan-checkout fallback mutates main repo & leaves detached HEAD
ensure_default_branch_via_orphan_checkout runs git checkout --orphan, git rm -rf --cached ., and git commit directly in self.project_root — the primary repository working tree/index, not an isolated worktree. Two concrete problems:
-
Detached HEAD after restore:
saved_headis captured viagit rev-parse HEAD, which is a commit SHA.restore_headthen runsgit checkout <sha>, which leaves the project root in a detached HEAD state rather than back on the original branch (e.g.main). Subsequent git operations on the project root then operate on a detached HEAD. -
Checkout-back can fail: after
git rm -rf --cached .the working-tree files become untracked.git checkout <sha>can abort with "would be overwritten by checkout" when those untracked files conflict, leaving the repo stuck on the orphan branch with an empty index.
Because this runs in the shared project root (and create_worktree may be invoked for multiple pairs against the same root — see pair.rs:1119), a failure here corrupts state for all agents. The safer commit-tree path is tried first, but the fallback is still reachable. Consider performing branch creation entirely via plumbing (commit-tree + push) and never touching HEAD/index, or capturing the original ref with git symbolic-ref --short HEAD (falling back to the SHA) and restoring that, plus using git checkout -f / stashing to avoid the untracked-file abort.
Restore the original branch name instead of a detached SHA.:
// Capture the symbolic ref name when on a branch, else the SHA.
let saved_head = Command::new("git")
.args(["symbolic-ref", "--short", "-q", "HEAD"])
.current_dir(&self.project_root)
.output().ok()
.filter(|o| o.status.success())
.map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string())
.filter(|s| !s.is_empty())
.or_else(|| /* fall back to rev-parse HEAD as before */ saved_head_sha());
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| let _ = Command::new("git") | ||
| .args(["branch", "-d", &self.default_branch]) | ||
| .current_dir(&self.project_root) | ||
| .output(); |
There was a problem hiding this comment.
💡 Bug: Orphan branch cleanup uses git branch -d and will fail
After creating and pushing the orphan default branch, cleanup runs git branch -d <default_branch>. The orphan branch's commit is not merged into any other branch, so -d (safe delete) refuses to delete it ("not fully merged"), leaving a stray local branch ref behind. If the intent is to always remove the temporary local ref, use -D (force delete). Note this is only reached after HEAD has been restored, so deleting the checked-out branch is not a concern.
Force-delete the temporary local orphan branch ref.:
let _ = Command::new("git")
.args(["branch", "-D", &self.default_branch])
.current_dir(&self.project_root)
.output();
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| // Push the commit as the default branch to origin | ||
| let push_output = Command::new("git") | ||
| .args([ | ||
| "push", | ||
| "origin", | ||
| &format!("{}:refs/heads/{}", commit_hash, self.default_branch), | ||
| ]) | ||
| .current_dir(&self.project_root) | ||
| .output(); | ||
|
|
||
| match push_output { | ||
| Ok(o) if o.status.success() => { | ||
| info!(branch = %self.default_branch, "Default branch created on remote via commit-tree"); | ||
| } | ||
| Ok(o) => { |
There was a problem hiding this comment.
💡 Bug: Default-branch push from project_root may lack token auth
Both ensure_default_branch_via_commit_tree and ensure_default_branch_via_orphan_checkout run git push origin ... in self.project_root. Token-based remote authentication is only applied to worktrees via configure_remote_with_token (and the github_token is not even passed into ensure_default_branch_on_remote). If the project root's origin URL is a plain https://github.com/... without an embedded x-access-token, these pushes will fail authentication, so the default branch is never created and the feature silently degrades (only a SetupWarning is emitted). Consider configuring the project-root remote with the token (as is done for worktrees) before pushing, or passing the github_token through and using an authenticated push URL.
Was this helpful? React with 👍 / 👎
CI failed: The CI build failed because the code does not conform to the project's rustfmt standards, specifically due to unformatted changes in the agent-client and TUI modules.OverviewThe CI pipeline failed in the 'Format' job because the submitted code does not match the repository's rustfmt style configuration. The logs confirm that multiple files were detected with formatting differences during the FailuresRustfmt Violation (confidence: high)
Summary
Code Review
|
| Auto-apply | Compact |
|
|
Important
Your trial ends in 2 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.
Was this helpful? React with 👍 / 👎 | Gitar
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ); | ||
| } | ||
| // If even slimmed JSON is too large, truncate to just issue numbers and titles | ||
| let minimal: Vec<serde_json::Value> = slimmed |
There was a problem hiding this comment.
[logic] (confidence: 90%)
The minimal truncation path extracts "status" but not "state". For GitHub issues, the open/closed indicator is in the "state" field, not "status". The keep_keys list includes both "state" (index 2) and "status" (index 8), but the minimal path only preserves "status", which is typically null for GitHub issue data. This means heavily truncated tool results lose the issue's open/closed status — critical information for the LLM's decision-making.
Impact: When tool results are large enough to trigger heavy truncation, the LLM receives GitHub issues without knowing whether they are open or closed, which can lead to incorrect decisions (e.g., trying to work on already-closed issues or ignoring open ones).
Evidence:
let minimal: Vec<serde_json::Value> = slimmed
.into_iter()
.map(|item| {
let number = item.get("number").cloned().unwrap_or(serde_json::Value::Null);
let title = item.get("title").cloned().unwrap_or(serde_json::Value::Null);
let status = item.get("status").cloned().unwrap_or(serde_json::Value::Null);
serde_json::json!({
"number": number,
"title": title,
"status": status,
})
})
.collect();
Suggestion: Add "state" to the minimal extraction alongside or instead of "status": let state = item.get("state").cloned().unwrap_or(serde_json::Value::Null); and include it in the output JSON.
| } | ||
| }; | ||
|
|
||
| // Push the commit as the default branch to origin |
There was a problem hiding this comment.
[logic] (confidence: 75%)
The ensure_default_branch_via_orphan_checkout fallback saves the current HEAD as a commit hash via git rev-parse HEAD, then restores it with git checkout . This puts the repository into a detached HEAD state instead of restoring the original branch name. Subsequent git operations that expect to be on a named branch (e.g., git pull, git status showing branch info) will behave unexpectedly.
Impact: After the orphan checkout fallback runs, the project root repository is left in detached HEAD state. This can cause subsequent git operations to fail or produce unexpected results, particularly git pull or any workflow that checks the current branch name.
Evidence:
let saved_head = Command::new("git")
.args(["rev-parse", "HEAD"])
...
fn restore_head(project_root: &Path, saved_head: Option<String>) {
if let Some(ref head) = saved_head {
let _ = Command::new("git")
.args(["checkout", head])
...
Suggestion: Save the branch name instead of the commit hash using git rev-parse --abbrev-ref HEAD, and restore with git checkout to stay on the original branch.
Snif ReviewThis release replaces hardcoded model lists with live API discovery for Anthropic, OpenAI, and Fireworks providers, adds pagination and timeouts to model fetching, introduces tool result truncation for large GitHub API responses, updates the default Fireworks model to deepseek-v3p1, adds custom deserializers for pr_number to handle LLM-generated empty strings, and improves default branch detection and creation for new repositories. Found 2 issues. See inline comments for details. Review detailsChanged files:
Context analyzed: 99 related files (29 via semantic similarity, 99 via keyword matching). Stats: 1955 lines | |
1 participant
Changes
Live model discovery for all providers (no more hardcoded lists)
Fireworks
/inference/v1/modelswithlimit=200and cursor-based pagination (afterparam +has_more) to discover all models, not just the first 50tracing::warn!/tracing::debug!) for network, HTTP, and JSON parse errorsContent-Typeheader addedaccounts/fireworks/models/prefixed models (the endpoint already returns only inference-ready models), with exclusion for clearly non-chat families (embedding, whisper, tts, dall-e, flux, etc.)Anthropic
/v1/modelswithlimit=1000andafter_idcursordiscover_claude_models()wrapper with 4 hardcoded 2024-era modelsdiscover_known_anthropic_models()fallbackOpenAI
/v1/models?limit=500for full model listErr("No OpenAI API key configured")with no fallback — now has Codex CLI → OpenAI API → minimal 2-modeldiscover_known_openai_models()Default model update
llama-v3p1-8b-instructtodeepseek-v3p1across all filesTests
test_discover_known_anthropic_modelstest_discover_known_openai_modelstest_is_fireworks_chat_modelfor new prefix-based filtertest_discover_known_fireworks_modelsfor reduced fallbacktest_default_model_for_providerfor new default