Skip to content

Commit 55a16b0

Browse files
author
Mateusz
committed
Address PR review feedback
1 parent c502ca0 commit 55a16b0

10 files changed

Lines changed: 64 additions & 29 deletions

File tree

‎internal/infra/modelcatalog/modelsdev/normalize_test.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,37 @@ func TestParseModelIDs(t *testing.T) {
5757
}
5858
}
5959

60+
func TestParseModelIDs_EdgeCases(t *testing.T) {
61+
t.Parallel()
62+
63+
tests := []struct {
64+
name string
65+
raw []byte
66+
want []string
67+
wantErr bool
68+
}{
69+
{name: "empty", raw: nil, wantErr: true},
70+
{name: "null root", raw: []byte("null"), wantErr: true},
71+
{name: "empty object", raw: []byte("{}"), want: []string{}},
72+
{name: "provider without models", raw: []byte(`{"openai":{"id":"openai"}}`), want: []string{}},
73+
{name: "empty model ids skipped", raw: []byte(`{"openai":{"id":"openai","models":[{"id":""}]}}`), want: []string{}},
74+
}
75+
76+
for _, tt := range tests {
77+
t.Run(tt.name, func(t *testing.T) {
78+
t.Parallel()
79+
80+
got, err := modelsdev.ParseModelIDs(tt.raw)
81+
if (err != nil) != tt.wantErr {
82+
t.Fatalf("ParseModelIDs() error = %v, wantErr %v", err, tt.wantErr)
83+
}
84+
if !tt.wantErr && !reflect.DeepEqual(got, tt.want) {
85+
t.Fatalf("ParseModelIDs() = %+v, want %+v", got, tt.want)
86+
}
87+
})
88+
}
89+
}
90+
6091
func TestParseSnapshot_richMapping(t *testing.T) {
6192
t.Parallel()
6293
raw := []byte(`{

‎internal/pluginreg/backend_prefix_inventory_test.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@ func TestStandardBackends_exposeInventoryPrefixes(t *testing.T) {
1717
}
1818

1919
for _, id := range standardBackendFactoryIDs(t) {
20-
if id == "bedrock" {
21-
continue
22-
}
2320
t.Run(id, func(t *testing.T) {
2421
t.Parallel()
2522

‎internal/plugins/backends/bedrock/prefix_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
func TestNewWithContext_exposesInventoryPrefix(t *testing.T) {
99
t.Parallel()
1010

11+
// Use a canceled context to exercise the construction error path and ensure
12+
// BackendPrefixes is set even when the Bedrock runtime client is unavailable.
1113
ctx, cancel := context.WithCancel(context.Background())
1214
cancel()
1315

‎internal/plugins/backends/ollama/inventory.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,9 @@ func (p *inventoryProvider) probeCapabilities(ctx context.Context, models []mode
309309
}
310310
}
311311
p.capsMu.Lock()
312-
p.capsByID = capsByID
312+
for id, caps := range capsByID {
313+
p.capsByID[id] = caps
314+
}
313315
p.capsMu.Unlock()
314316
}
315317

‎internal/plugins/backends/ollama/inventory_test.go‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"net/http"
66
"net/http/httptest"
77
"strings"
8+
"sync/atomic"
89
"testing"
910
"time"
1011

@@ -27,9 +28,9 @@ func TestInventory_localModeEnumeratesLocalOnly(t *testing.T) {
2728
}))
2829
t.Cleanup(local.Close)
2930

30-
var cloudHits int
31+
var cloudHits atomic.Int32
3132
cloud := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
32-
cloudHits++
33+
cloudHits.Add(1)
3334
_, _ = w.Write([]byte(`{"models":[{"name":"deepseek-v3.2"}]}`))
3435
}))
3536
t.Cleanup(cloud.Close)
@@ -51,8 +52,8 @@ func TestInventory_localModeEnumeratesLocalOnly(t *testing.T) {
5152
if err != nil {
5253
t.Fatal(err)
5354
}
54-
if cloudHits != 0 {
55-
t.Fatalf("cloud discovery invoked %d times", cloudHits)
55+
if cloudHits.Load() != 0 {
56+
t.Fatalf("cloud discovery invoked %d times", cloudHits.Load())
5657
}
5758
if len(snap.Models) != 2 {
5859
t.Fatalf("models = %+v", snap.Models)
@@ -81,9 +82,9 @@ func TestInventory_localIgnoresCloudToggle(t *testing.T) {
8182
}))
8283
t.Cleanup(local.Close)
8384

84-
var cloudHits int
85+
var cloudHits atomic.Int32
8586
cloud := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
86-
cloudHits++
87+
cloudHits.Add(1)
8788
_, _ = w.Write([]byte(`{"models":[{"name":"deepseek-v3.2"}]}`))
8889
}))
8990
t.Cleanup(cloud.Close)
@@ -107,8 +108,8 @@ func TestInventory_localIgnoresCloudToggle(t *testing.T) {
107108
if err != nil {
108109
t.Fatal(err)
109110
}
110-
if cloudHits != 0 {
111-
t.Fatalf("cloud discovery invoked %d times", cloudHits)
111+
if cloudHits.Load() != 0 {
112+
t.Fatalf("cloud discovery invoked %d times", cloudHits.Load())
112113
}
113114
if len(snap.Models) != 1 || snap.Models[0].NativeID != "llama3:latest" {
114115
t.Fatalf("models = %+v", snap.Models)
@@ -121,10 +122,10 @@ func TestInventory_localIgnoresCloudToggle(t *testing.T) {
121122
func TestInventory_cloudModeEnumeratesCloudOnly(t *testing.T) {
122123
t.Parallel()
123124

124-
var localHits int
125+
var localHits atomic.Int32
125126
local := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
126127
if r.URL.Path == "/v1/models" {
127-
localHits++
128+
localHits.Add(1)
128129
}
129130
http.NotFound(w, r)
130131
}))
@@ -152,8 +153,8 @@ func TestInventory_cloudModeEnumeratesCloudOnly(t *testing.T) {
152153
if err != nil {
153154
t.Fatal(err)
154155
}
155-
if localHits != 0 {
156-
t.Fatalf("local discovery invoked %d times", localHits)
156+
if localHits.Load() != 0 {
157+
t.Fatalf("local discovery invoked %d times", localHits.Load())
157158
}
158159
if len(snap.Models) != 2 {
159160
t.Fatalf("models = %+v", snap.Models)
@@ -176,10 +177,10 @@ func TestInventory_cloudModeEnumeratesCloudOnly(t *testing.T) {
176177
func TestInventory_cloudIgnoresLocalToggle(t *testing.T) {
177178
t.Parallel()
178179

179-
var localHits int
180+
var localHits atomic.Int32
180181
local := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
181182
if r.URL.Path == "/v1/models" {
182-
localHits++
183+
localHits.Add(1)
183184
}
184185
http.NotFound(w, r)
185186
}))
@@ -209,8 +210,8 @@ func TestInventory_cloudIgnoresLocalToggle(t *testing.T) {
209210
if err != nil {
210211
t.Fatal(err)
211212
}
212-
if localHits != 0 {
213-
t.Fatalf("local discovery invoked %d times", localHits)
213+
if localHits.Load() != 0 {
214+
t.Fatalf("local discovery invoked %d times", localHits.Load())
214215
}
215216
if len(snap.Models) != 1 || snap.Models[0].NativeID != "deepseek-v3.2" {
216217
t.Fatalf("models = %+v", snap.Models)

‎internal/plugins/backends/ollama/version.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const minResponsesVersion = "0.13.3"
1717
func NativeRootFromBaseURL(baseURL string) string {
1818
u := strings.TrimRight(strings.TrimSpace(baseURL), "/")
1919
if strings.HasSuffix(strings.ToLower(u), "/v1") {
20-
return strings.TrimSuffix(u, "/v1")
20+
return u[:len(u)-3]
2121
}
2222
return u
2323
}
@@ -31,9 +31,6 @@ func ParseSemver(raw string) (Semver, error) {
3131
raw = raw[:idx]
3232
}
3333
parts := strings.Split(raw, ".")
34-
if len(parts) == 0 {
35-
return Semver{}, fmt.Errorf("ollama: invalid version %q", raw)
36-
}
3734
for len(parts) < 3 {
3835
parts = append(parts, "0")
3936
}

‎internal/plugins/backends/ollama/version_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ func TestNativeRootFromBaseURL(t *testing.T) {
1313
}{
1414
{"http://localhost:11434/v1", "http://localhost:11434"},
1515
{"http://localhost:11434/v1/", "http://localhost:11434"},
16+
{"http://localhost:11434/V1", "http://localhost:11434"},
17+
{"http://localhost:11434/V1/", "http://localhost:11434"},
1618
{"http://127.0.0.1:11434/v1", "http://127.0.0.1:11434"},
1719
{"http://localhost:11434", "http://localhost:11434"},
1820
}

‎internal/plugins/frontends/routeselect/routeselect.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// Package routeselect derives explicit route selectors from model identifiers.
2+
// Route selectors use "backend-prefix:model" to route requests to a backend family.
13
package routeselect
24

35
import (
@@ -17,10 +19,10 @@ var inlineRoutePrefixes = map[string]struct{}{
1719
"openai-legacy": {},
1820
"openai-responses": {},
1921
"openrouter": {},
20-
"stub": {},
2122
}
2223

23-
// InlineOrDefault returns model when it has a known route prefix, otherwise defaultRoute.
24+
// InlineOrDefault returns model when it has a known backend prefix before the colon delimiter.
25+
// Otherwise it returns defaultRoute with surrounding whitespace removed.
2426
func InlineOrDefault(model, defaultRoute string) string {
2527
model = strings.TrimSpace(model)
2628
prefix, _, ok := strings.Cut(model, ":")
@@ -32,7 +34,8 @@ func InlineOrDefault(model, defaultRoute string) string {
3234
return strings.TrimSpace(defaultRoute)
3335
}
3436

35-
// FromModelOrDefault parses model from JSON body and returns it if it has an inline route prefix, otherwise defaultRoute.
37+
// FromModelOrDefault parses body for a model field and returns it when it carries a known inline route prefix.
38+
// If decoding fails or the model has no known prefix, it returns defaultRoute with surrounding whitespace removed.
3639
func FromModelOrDefault(body []byte, defaultRoute string) string {
3740
var req struct {
3841
Model string `json:"model"`

‎internal/plugins/frontends/routeselect/routeselect_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func TestInlineOrDefault(t *testing.T) {
2727
},
2828
{
2929
name: "unknown prefix",
30-
model: "xyz:claude-3-5-sonnet",
30+
model: "stub:claude-3-5-sonnet",
3131
defaultRoute: "openai-legacy:gpt-4o",
3232
want: "openai-legacy:gpt-4o",
3333
},

‎scripts/ollama-text-smoke.ps1‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ try {
280280
Stop-Process -Id $script:proc.Id -Force
281281
$script:proc.WaitForExit(5000) | Out-Null
282282
}
283-
if (-not $ok -and $script:proc -and $script:proc.ExitCode -ne 0 -and $script:proc.ExitCode -ne $null) {
283+
if (-not $ok -and $script:proc -and $script:proc.ExitCode -ne 0 -and $null -ne $script:proc.ExitCode) {
284284
Write-Host "lipstd stdout: $stdoutPath"
285285
Write-Host "lipstd stderr: $stderrPath"
286286
}

0 commit comments

Comments
 (0)