From e0ec07e6675a85760bee7dc93c196ce96babe490 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 20 Feb 2026 11:45:29 +0000 Subject: [PATCH 1/2] test(inference): complete Phase 1 foundation tests Add comprehensive tests for all three Phase 1 items: - Option application: DefaultGenerateConfig idempotency, field isolation (WithMaxTokens leaves others at defaults), bad-input acceptance (negative temperature, negative TopK), empty variadic StopTokens, WithLogits default-is-false, partial-options preserve defaults, last-wins overrides for all GenerateOption and LoadOption types. - Backend registry: concurrent read/write safety (70 goroutines with -race), overwrite-keeps-count, capturingBackend verifies LoadModel forwards all options to both explicit and default backends, Get after overwrite returns latest, List returns independent slices. - Default() platform preference: registration order is irrelevant (metal wins regardless), all-preferred-unavailable falls back to custom, multiple custom backends finds the available one, empty path forwarding. 85 tests, 100% statement coverage, -race clean. Co-Authored-By: Charon --- inference_test.go | 240 ++++++++++++++++++++++++++++++++++++++++++++++ options_test.go | 122 ++++++++++++++++++++++- 2 files changed, 361 insertions(+), 1 deletion(-) diff --git a/inference_test.go b/inference_test.go index ae680fb..cefd2aa 100644 --- a/inference_test.go +++ b/inference_test.go @@ -5,6 +5,7 @@ import ( "fmt" "iter" "sort" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -37,6 +38,20 @@ func (s *stubBackend) LoadModel(path string, opts ...LoadOption) (TextModel, err return &stubTextModel{backend: s.name, path: path}, nil } +// capturingBackend records the LoadOption values it received. +type capturingBackend struct { + name string + available bool + capturedOpts []LoadOption +} + +func (c *capturingBackend) Name() string { return c.name } +func (c *capturingBackend) Available() bool { return c.available } +func (c *capturingBackend) LoadModel(path string, opts ...LoadOption) (TextModel, error) { + c.capturedOpts = opts + return &stubTextModel{backend: c.name, path: path}, nil +} + // stubTextModel is a minimal TextModel for testing LoadModel routing. type stubTextModel struct { backend string @@ -454,3 +469,228 @@ func TestGenerateMetrics_Good(t *testing.T) { assert.Equal(t, uint64(1<<30), m.PeakMemoryBytes) assert.Equal(t, uint64(512<<20), m.ActiveMemoryBytes) } + +// --- Concurrent registry access --- + +func TestRegistry_Good_ConcurrentAccess(t *testing.T) { + // Verify the registry is safe for concurrent reads and writes. + // The -race flag will catch data races if the mutex is broken. + resetBackends(t) + + var wg sync.WaitGroup + + // Concurrent writers. + for i := range 20 { + wg.Add(1) + go func(id int) { + defer wg.Done() + Register(&stubBackend{ + name: fmt.Sprintf("backend_%d", id), + available: true, + }) + }(i) + } + + // Concurrent readers interleaved with writers. + for range 20 { + wg.Add(1) + go func() { + defer wg.Done() + _ = List() + }() + } + + for range 20 { + wg.Add(1) + go func() { + defer wg.Done() + _, _ = Get("backend_0") + }() + } + + for range 10 { + wg.Add(1) + go func() { + defer wg.Done() + _, _ = Default() + }() + } + + wg.Wait() + + // After all goroutines finish, verify all 20 backends are registered. + names := List() + assert.Len(t, names, 20, "all 20 backends should be registered after concurrent writes") +} + +// --- Register overwrite count --- + +func TestRegister_Ugly_OverwriteKeepsCount(t *testing.T) { + resetBackends(t) + + Register(&stubBackend{name: "alpha", available: true}) + Register(&stubBackend{name: "beta", available: true}) + Register(&stubBackend{name: "alpha", available: false}) // overwrite + + names := List() + assert.Len(t, names, 2, "overwriting a backend should not increase the count") +} + +// --- Default with all preferred unavailable and custom available --- + +func TestDefault_Ugly_AllPreferredUnavailableCustomAvailable(t *testing.T) { + resetBackends(t) + + Register(&stubBackend{name: "metal", available: false}) + Register(&stubBackend{name: "rocm", available: false}) + Register(&stubBackend{name: "llama_cpp", available: false}) + Register(&stubBackend{name: "custom_vulkan", available: true}) + + b, err := Default() + require.NoError(t, err) + assert.Equal(t, "custom_vulkan", b.Name(), + "should fall back to custom backend when all preferred backends are unavailable") +} + +// --- Default with multiple custom backends --- + +func TestDefault_Ugly_MultipleCustomBackends(t *testing.T) { + resetBackends(t) + + // Only non-preferred backends registered — one available, one not. + Register(&stubBackend{name: "custom_a", available: false}) + Register(&stubBackend{name: "custom_b", available: true}) + + b, err := Default() + require.NoError(t, err) + assert.Equal(t, "custom_b", b.Name(), + "should find the available custom backend in the fallback loop") +} + +// --- LoadModel option forwarding --- + +func TestLoadModel_Good_ExplicitBackendForwardsOptions(t *testing.T) { + resetBackends(t) + + cb := &capturingBackend{name: "cap", available: true} + Register(cb) + + opts := []LoadOption{ + WithBackend("cap"), + WithContextLen(4096), + WithGPULayers(16), + } + m, err := LoadModel("/path/to/model", opts...) + require.NoError(t, err) + require.NotNil(t, m) + + // The capturing backend should have received all options. + assert.Len(t, cb.capturedOpts, len(opts), + "all LoadOptions should be forwarded to the backend") + + // Verify the forwarded options produce the correct config. + cfg := ApplyLoadOpts(cb.capturedOpts) + assert.Equal(t, "cap", cfg.Backend) + assert.Equal(t, 4096, cfg.ContextLen) + assert.Equal(t, 16, cfg.GPULayers) + require.NoError(t, m.Close()) +} + +func TestLoadModel_Good_DefaultBackendForwardsOptions(t *testing.T) { + resetBackends(t) + + cb := &capturingBackend{name: "metal", available: true} + Register(cb) + + opts := []LoadOption{ + WithContextLen(8192), + WithGPULayers(-1), + WithParallelSlots(2), + } + m, err := LoadModel("/path/to/model", opts...) + require.NoError(t, err) + require.NotNil(t, m) + + // The default backend should have received all options. + assert.Len(t, cb.capturedOpts, len(opts), + "all LoadOptions should be forwarded to the default backend") + + cfg := ApplyLoadOpts(cb.capturedOpts) + assert.Equal(t, 8192, cfg.ContextLen) + assert.Equal(t, -1, cfg.GPULayers) + assert.Equal(t, 2, cfg.ParallelSlots) + require.NoError(t, m.Close()) +} + +// --- Default preference order does not depend on registration order --- + +func TestDefault_Good_RegistrationOrderIrrelevant(t *testing.T) { + // Register in reverse priority order — metal should still be chosen. + resetBackends(t) + + Register(&stubBackend{name: "llama_cpp", available: true}) + Register(&stubBackend{name: "rocm", available: true}) + Register(&stubBackend{name: "metal", available: true}) + + b, err := Default() + require.NoError(t, err) + assert.Equal(t, "metal", b.Name(), + "metal should win regardless of registration order") + + // Register in yet another order. + resetBackends(t) + + Register(&stubBackend{name: "rocm", available: true}) + Register(&stubBackend{name: "metal", available: true}) + Register(&stubBackend{name: "llama_cpp", available: true}) + + b, err = Default() + require.NoError(t, err) + assert.Equal(t, "metal", b.Name(), + "metal should win regardless of registration order") +} + +// --- LoadModel with empty path --- + +func TestLoadModel_Ugly_EmptyPath(t *testing.T) { + resetBackends(t) + + Register(&stubBackend{name: "metal", available: true}) + + // Empty path is accepted at this layer — backend decides what to do. + m, err := LoadModel("") + require.NoError(t, err) + sm := m.(*stubTextModel) + assert.Equal(t, "", sm.path, "empty path should be forwarded to the backend as-is") + require.NoError(t, m.Close()) +} + +// --- Get after register and overwrite --- + +func TestGet_Good_AfterOverwrite(t *testing.T) { + resetBackends(t) + + Register(&stubBackend{name: "gpu", available: false}) + Register(&stubBackend{name: "gpu", available: true}) // overwrite + + b, ok := Get("gpu") + require.True(t, ok) + assert.True(t, b.Available(), "Get should return the most recently registered backend") +} + +// --- List returns new slice each call --- + +func TestList_Good_IndependentSlices(t *testing.T) { + resetBackends(t) + + Register(&stubBackend{name: "alpha", available: true}) + + a := List() + b := List() + assert.Equal(t, a, b, "both calls should return the same names") + + // Mutating one slice should not affect the other. + a[0] = "mutated" + c := List() + assert.NotEqual(t, a[0], c[0], "List should return independent slices") +} diff --git a/options_test.go b/options_test.go index 0e4462f..63522c4 100644 --- a/options_test.go +++ b/options_test.go @@ -7,6 +7,15 @@ import ( "github.com/stretchr/testify/require" ) +// --- DefaultGenerateConfig stability --- + +func TestDefaultGenerateConfig_Good_Idempotent(t *testing.T) { + // Calling DefaultGenerateConfig twice should yield identical results. + a := DefaultGenerateConfig() + b := DefaultGenerateConfig() + assert.Equal(t, a, b, "DefaultGenerateConfig should be idempotent") +} + // --- GenerateConfig defaults --- func TestDefaultGenerateConfig_Good(t *testing.T) { @@ -41,7 +50,7 @@ func TestWithMaxTokens_Good(t *testing.T) { } func TestWithMaxTokens_Bad(t *testing.T) { - // Zero and negative values are accepted (no validation in options layer) + // Zero and negative values are accepted (no validation in options layer). cfg := ApplyGenerateOpts([]GenerateOption{WithMaxTokens(0)}) assert.Equal(t, 0, cfg.MaxTokens) @@ -49,6 +58,19 @@ func TestWithMaxTokens_Bad(t *testing.T) { assert.Equal(t, -1, cfg.MaxTokens) } +func TestWithMaxTokens_Good_OtherFieldsUnchanged(t *testing.T) { + // Setting MaxTokens should not affect other fields. + cfg := ApplyGenerateOpts([]GenerateOption{WithMaxTokens(512)}) + def := DefaultGenerateConfig() + assert.Equal(t, 512, cfg.MaxTokens) + assert.Equal(t, def.Temperature, cfg.Temperature, "Temperature should remain at default") + assert.Equal(t, def.TopK, cfg.TopK, "TopK should remain at default") + assert.Equal(t, def.TopP, cfg.TopP, "TopP should remain at default") + assert.Nil(t, cfg.StopTokens, "StopTokens should remain nil") + assert.Equal(t, def.RepeatPenalty, cfg.RepeatPenalty, "RepeatPenalty should remain at default") + assert.Equal(t, def.ReturnLogits, cfg.ReturnLogits, "ReturnLogits should remain at default") +} + // --- WithTemperature --- func TestWithTemperature_Good(t *testing.T) { @@ -70,6 +92,12 @@ func TestWithTemperature_Good(t *testing.T) { } } +func TestWithTemperature_Bad(t *testing.T) { + // Negative temperature is accepted at the options layer (no validation). + cfg := ApplyGenerateOpts([]GenerateOption{WithTemperature(-0.5)}) + assert.InDelta(t, -0.5, cfg.Temperature, 0.0001, "negative temperature should be stored as-is") +} + // --- WithTopK --- func TestWithTopK_Good(t *testing.T) { @@ -91,6 +119,12 @@ func TestWithTopK_Good(t *testing.T) { } } +func TestWithTopK_Bad(t *testing.T) { + // Negative TopK is accepted at the options layer (no validation). + cfg := ApplyGenerateOpts([]GenerateOption{WithTopK(-1)}) + assert.Equal(t, -1, cfg.TopK, "negative TopK should be stored as-is") +} + // --- WithTopP --- func TestWithTopP_Good(t *testing.T) { @@ -126,6 +160,13 @@ func TestWithStopTokens_Good(t *testing.T) { }) } +func TestWithStopTokens_Bad(t *testing.T) { + // Empty variadic — Go passes nil to the variadic parameter, so StopTokens + // is set to nil (same as default). This is a known Go behaviour. + cfg := ApplyGenerateOpts([]GenerateOption{WithStopTokens()}) + assert.Nil(t, cfg.StopTokens, "empty variadic should set StopTokens to nil") +} + func TestWithStopTokens_Ugly(t *testing.T) { // Last call wins — stop tokens are replaced, not merged. cfg := ApplyGenerateOpts([]GenerateOption{ @@ -163,6 +204,12 @@ func TestWithLogits_Good(t *testing.T) { assert.True(t, cfg.ReturnLogits) } +func TestWithLogits_Good_DefaultIsFalse(t *testing.T) { + // Without WithLogits, ReturnLogits stays false. + cfg := ApplyGenerateOpts([]GenerateOption{WithMaxTokens(64)}) + assert.False(t, cfg.ReturnLogits, "ReturnLogits should be false when WithLogits is not applied") +} + // --- ApplyGenerateOpts --- func TestApplyGenerateOpts_Good(t *testing.T) { @@ -198,6 +245,22 @@ func TestApplyGenerateOpts_Good(t *testing.T) { }) } +func TestApplyGenerateOpts_Good_PartialOptions(t *testing.T) { + // Applying only some options should preserve defaults for the rest. + cfg := ApplyGenerateOpts([]GenerateOption{ + WithTemperature(0.8), + WithTopK(50), + }) + def := DefaultGenerateConfig() + assert.Equal(t, def.MaxTokens, cfg.MaxTokens, "MaxTokens should remain at default") + assert.InDelta(t, 0.8, cfg.Temperature, 0.0001) + assert.Equal(t, 50, cfg.TopK) + assert.Equal(t, def.TopP, cfg.TopP, "TopP should remain at default") + assert.Nil(t, cfg.StopTokens, "StopTokens should remain nil") + assert.Equal(t, def.RepeatPenalty, cfg.RepeatPenalty, "RepeatPenalty should remain at default") + assert.False(t, cfg.ReturnLogits, "ReturnLogits should remain false") +} + func TestApplyGenerateOpts_Ugly(t *testing.T) { t.Run("last_option_wins", func(t *testing.T) { cfg := ApplyGenerateOpts([]GenerateOption{ @@ -215,6 +278,30 @@ func TestApplyGenerateOpts_Ugly(t *testing.T) { }) assert.InDelta(t, 1.0, cfg.Temperature, 0.0001, "last WithTemperature should win") }) + + t.Run("topk_override", func(t *testing.T) { + cfg := ApplyGenerateOpts([]GenerateOption{ + WithTopK(10), + WithTopK(50), + }) + assert.Equal(t, 50, cfg.TopK, "last WithTopK should win") + }) + + t.Run("topp_override", func(t *testing.T) { + cfg := ApplyGenerateOpts([]GenerateOption{ + WithTopP(0.5), + WithTopP(0.95), + }) + assert.InDelta(t, 0.95, cfg.TopP, 0.0001, "last WithTopP should win") + }) + + t.Run("repeat_penalty_override", func(t *testing.T) { + cfg := ApplyGenerateOpts([]GenerateOption{ + WithRepeatPenalty(1.1), + WithRepeatPenalty(1.5), + }) + assert.InDelta(t, 1.5, cfg.RepeatPenalty, 0.0001, "last WithRepeatPenalty should win") + }) } // --- LoadConfig defaults --- @@ -333,6 +420,15 @@ func TestApplyLoadOpts_Good_Combined(t *testing.T) { assert.Equal(t, 2, cfg.ParallelSlots) } +func TestApplyLoadOpts_Good_PartialOptions(t *testing.T) { + // Applying only some options should preserve defaults for the rest. + cfg := ApplyLoadOpts([]LoadOption{WithContextLen(4096)}) + assert.Equal(t, "", cfg.Backend, "Backend should remain at default (auto-detect)") + assert.Equal(t, 4096, cfg.ContextLen) + assert.Equal(t, -1, cfg.GPULayers, "GPULayers should remain at default (-1)") + assert.Equal(t, 0, cfg.ParallelSlots, "ParallelSlots should remain at default (0)") +} + func TestApplyLoadOpts_Ugly(t *testing.T) { t.Run("last_option_wins", func(t *testing.T) { cfg := ApplyLoadOpts([]LoadOption{ @@ -347,4 +443,28 @@ func TestApplyLoadOpts_Ugly(t *testing.T) { require.Equal(t, -1, cfg.GPULayers, "empty opts should keep default GPULayers=-1") assert.Equal(t, "", cfg.Backend) }) + + t.Run("context_len_override", func(t *testing.T) { + cfg := ApplyLoadOpts([]LoadOption{ + WithContextLen(2048), + WithContextLen(8192), + }) + assert.Equal(t, 8192, cfg.ContextLen, "last WithContextLen should win") + }) + + t.Run("gpu_layers_override", func(t *testing.T) { + cfg := ApplyLoadOpts([]LoadOption{ + WithGPULayers(24), + WithGPULayers(0), + }) + assert.Equal(t, 0, cfg.GPULayers, "last WithGPULayers should win") + }) + + t.Run("parallel_slots_override", func(t *testing.T) { + cfg := ApplyLoadOpts([]LoadOption{ + WithParallelSlots(4), + WithParallelSlots(1), + }) + assert.Equal(t, 1, cfg.ParallelSlots, "last WithParallelSlots should win") + }) } From 35da94a138b4c0f676cd196c92d5d1f5b4b504f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 20 Feb 2026 11:45:50 +0000 Subject: [PATCH 2/2] docs: mark Phase 1 foundation tests as complete Co-Authored-By: Charon --- TODO.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/TODO.md b/TODO.md index 30d1592..342be94 100644 --- a/TODO.md +++ b/TODO.md @@ -6,9 +6,9 @@ Dispatched from core/go orchestration. This package is minimal by design. ## Phase 1: Foundation — `d76448d` (Charon) -- [x] **Add tests for option application** — Verify GenerateConfig defaults, all With* options, ApplyGenerateOpts/ApplyLoadOpts behaviour. Comprehensive API tests (1,074 LOC). -- [x] **Add tests for backend registry** — Register, Get, List, Default priority order, LoadModel routing. -- [x] **Add tests for Default() platform preference** — Verify metal > rocm > llama_cpp ordering. +- [x] **Add tests for option application** — Verify GenerateConfig defaults, all With* options, ApplyGenerateOpts/ApplyLoadOpts behaviour. Comprehensive API tests (1,074 LOC). (`d76448d`, `c633be1`) +- [x] **Add tests for backend registry** — Register, Get, List, Default priority order, LoadModel routing. (`d76448d`, `c633be1`) +- [x] **Add tests for Default() platform preference** — Verify metal > rocm > llama_cpp ordering. (`d76448d`, `c633be1`) ## Phase 2: Integration — COMPLETE