fix(config): correct off-by-one in round-robin model selection

rrCounter.Add(1) returns the post-increment value, so the first call
returns 1 and computes idx = 1 % N, skipping index 0 entirely on the
initial request. Subtract 1 so the sequence starts at 0.

Closes #1153
This commit is contained in:
Nikolas de Hor 2026-03-10 13:48:27 -03:00
parent 9cd2d21800
commit c31318b374
2 changed files with 54 additions and 2 deletions

View file

@ -914,8 +914,11 @@ func (c *Config) GetModelConfig(modelName string) (*ModelConfig, error) {
return &matches[0], nil
}
// Multiple configs - use round-robin for load balancing
idx := rrCounter.Add(1) % uint64(len(matches))
// Multiple configs - use round-robin for load balancing.
// Subtract 1 because Add returns the new (post-increment) value,
// so without the subtraction the first entry (index 0) is skipped
// on the initial call.
idx := (rrCounter.Add(1) - 1) % uint64(len(matches))
return &matches[idx], nil
}

View file

@ -80,6 +80,55 @@ func TestGetModelConfig_RoundRobin(t *testing.T) {
}
}
func TestGetModelConfig_RoundRobinSequence(t *testing.T) {
cfg := &Config{
ModelList: []ModelConfig{
{ModelName: "seq-model", Model: "openai/model-a", APIKey: "key1"},
{ModelName: "seq-model", Model: "openai/model-b", APIKey: "key2"},
{ModelName: "seq-model", Model: "openai/model-c", APIKey: "key3"},
},
}
// Three consecutive calls must produce three distinct models,
// proving that every entry is reachable within one full cycle.
seen := make(map[string]bool)
for range 3 {
result, err := cfg.GetModelConfig("seq-model")
if err != nil {
t.Fatalf("GetModelConfig() error = %v", err)
}
seen[result.Model] = true
}
if len(seen) != 3 {
t.Errorf("Expected all 3 models within one cycle, got %d distinct: %v", len(seen), seen)
}
}
func TestGetModelConfig_TwoEntries_BothReachable(t *testing.T) {
cfg := &Config{
ModelList: []ModelConfig{
{ModelName: "pair", Model: "openai/first", APIKey: "key1"},
{ModelName: "pair", Model: "openai/second", APIKey: "key2"},
},
}
// Two calls must hit both entries — the old off-by-one bug caused the
// first entry to be skipped on every other pair of calls.
seen := make(map[string]bool)
for range 2 {
result, err := cfg.GetModelConfig("pair")
if err != nil {
t.Fatalf("GetModelConfig() error = %v", err)
}
seen[result.Model] = true
}
if !seen["openai/first"] || !seen["openai/second"] {
t.Errorf("Both entries should be reachable within 2 calls, got: %v", seen)
}
}
func TestGetModelConfig_Concurrent(t *testing.T) {
cfg := &Config{
ModelList: []ModelConfig{