From 351b18797328ef8acd76173e28f9d48520b96a30 Mon Sep 17 00:00:00 2001 From: Mathias Date: Thu, 17 Sep 2026 00:21:15 +0200 Subject: [PATCH] =?UTF-8?q?fix(config):=20delete=20the=20model-routing=20c?= =?UTF-8?q?hain=20=E2=80=94=20nothing=20has=20called=20it=20since=20April?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #116. `config/models.yaml` declared a default chain and six per-skill chains. The issue filed against it said the deployed service does not read the file. That was true and understated: **nothing reads it at all**, and the loader written to read it is dead too. The whole chain goes: - `config/models.yaml` — data with no consumer - `internal/config/models.go` — `LoadModels` and `ModelFor`, whose only callers are in `models_test.go` - `internal/config/config.go` — the `ModelsFile` field, assigned from `SUPERVISOR_MODELS_FILE` and never read - `.env.example` — the variable that configures the path to the file nobody opens Deleting the YAML alone would have left a loader pointing at a missing file: one decoy replaced by another. This subsystem had THREE PASSING TESTS. That is what kept it invisible. Green assertions proving a function nobody calls works correctly are worse than no tests, because coverage is exactly the signal someone would use to decide the code is alive. The repo's own `docs/superpowers/specs/2026-04-20-model-orchestration-design.md` said so in April — "the model routing config exists but is dead weight" — and it survived five more months anyway. Two of the six models it named no longer exist (`iguana/qwen3-coder-next`, retired in infra#416, and `koala/qwen3-coder-30b`, gone earlier). Nobody noticed because the file being unread is the only reason it never failed. Live routing is env, on the deployed `ingestion` service: `INGEST_LLM_MODEL`, `BRAIN_LLM_PRIMARY_MODEL`, and four more. Not one name from the deleted file appears there. Env is the real configuration surface; this file read as though it were, which is why editing it to change routing would have changed nothing, silently. `go build ./...` clean, `task check` EXIT=0. Co-Authored-By: Claude Opus 5 --- .env.example | 1 - config/models.yaml | 26 ----------------- internal/config/config.go | 2 -- internal/config/models.go | 49 ------------------------------- internal/config/models_test.go | 53 ---------------------------------- 5 files changed, 131 deletions(-) delete mode 100644 config/models.yaml delete mode 100644 internal/config/models.go delete mode 100644 internal/config/models_test.go diff --git a/.env.example b/.env.example index a46d5ef..49076f5 100644 --- a/.env.example +++ b/.env.example @@ -1,7 +1,6 @@ # Supervisor MCP server SUPERVISOR_PORT=3200 SUPERVISOR_CONFIG_DIR=./config/supervisor -SUPERVISOR_MODELS_FILE=./config/models.yaml # LiteLLM gateway (iguana) LITELLM_BASE_URL=http://iguana:4000 diff --git a/config/models.yaml b/config/models.yaml deleted file mode 100644 index 949f94a..0000000 --- a/config/models.yaml +++ /dev/null @@ -1,26 +0,0 @@ -# Model selection — first entry per skill is used. -# Override per-call by passing model in the MCP tool args. -# Model names come from LiteLLM /v1/models (host/name format). - -default_chain: - - iguana/qwen3-coder-next - -skills: - tdd: - chain: - - koala/qwen3-coder-30b - review: - chain: - - iguana/devstral - debug: - chain: - - iguana/deepseek-r1-14b - spec: - chain: - - koala/phi4-14b - retrospective: - chain: - - iguana/qwen3-coder-next - trainer: - chain: - - iguana/qwen3-coder-next diff --git a/internal/config/config.go b/internal/config/config.go index 1da69de..d120c6f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -7,7 +7,6 @@ type Config struct { LiteLLMBaseURL string // LITELLM_BASE_URL, default http://iguana:4000 LiteLLMAPIKey string // LITELLM_API_KEY ConfigDir string // SUPERVISOR_CONFIG_DIR, default ./config/supervisor - ModelsFile string // SUPERVISOR_MODELS_FILE, default /../models.yaml IngestBaseURL string // INGEST_BASE_URL, default http://localhost:3300 IngestSvcURL string // INGEST_SVC_URL — base URL for brain_ingest (/ingest, /ingest-path) KBRetrievalURL string // KB_RETRIEVAL_URL — base URL for brain_search @@ -23,7 +22,6 @@ func Load() (Config, error) { LiteLLMAPIKey: os.Getenv("LITELLM_API_KEY"), ConfigDir: envOr("SUPERVISOR_CONFIG_DIR", "./config/supervisor"), } - cfg.ModelsFile = envOr("SUPERVISOR_MODELS_FILE", cfg.ConfigDir+"/../models.yaml") cfg.IngestBaseURL = envOr("INGEST_BASE_URL", "http://localhost:3300") cfg.IngestSvcURL = envOr("INGEST_SVC_URL", "") cfg.KBRetrievalURL = envOr("KB_RETRIEVAL_URL", "") diff --git a/internal/config/models.go b/internal/config/models.go deleted file mode 100644 index fefed6c..0000000 --- a/internal/config/models.go +++ /dev/null @@ -1,49 +0,0 @@ -package config - -import ( - "fmt" - "os" - - "gopkg.in/yaml.v3" -) - -type skillChain struct { - Chain []string `yaml:"chain"` -} - -type modelsFile struct { - DefaultChain []string `yaml:"default_chain"` - Skills map[string]skillChain `yaml:"skills"` -} - -type Models struct { - data modelsFile -} - -func LoadModels(path string) (Models, error) { - raw, err := os.ReadFile(path) - if err != nil { - return Models{}, fmt.Errorf("load models: %w", err) - } - var f modelsFile - if err := yaml.Unmarshal(raw, &f); err != nil { - return Models{}, fmt.Errorf("parse models: %w", err) - } - return Models{data: f}, nil -} - -// ModelFor returns the primary model to use for a skill. -// If override is non-empty, it is returned directly. -// Falls back to default_chain[0] when the skill has no explicit entry. -func (m Models) ModelFor(skill, override string) string { - if override != "" { - return override - } - if sc, ok := m.data.Skills[skill]; ok && len(sc.Chain) > 0 { - return sc.Chain[0] - } - if len(m.data.DefaultChain) > 0 { - return m.data.DefaultChain[0] - } - return "" -} diff --git a/internal/config/models_test.go b/internal/config/models_test.go deleted file mode 100644 index 7772daf..0000000 --- a/internal/config/models_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package config_test - -import ( - "os" - "path/filepath" - "testing" - - "git.d-ma.be/mathias/hyperguild/internal/config" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -const testYAML = ` -default_chain: - - ollama/qwen3-coder-30b-tuned - - claude-sonnet-4-6 - -skills: - review: - chain: - - ollama/devstral-tuned - - ollama/gemma4 - - claude-sonnet-4-6 - spec: - chain: - - ollama/phi4 - - claude-opus-4-6 -` - -func writeModels(t *testing.T, content string) string { - t.Helper() - f := filepath.Join(t.TempDir(), "models.yaml") - require.NoError(t, os.WriteFile(f, []byte(content), 0o644)) - return f -} - -func TestModelsModelForSkillWithEntry(t *testing.T) { - m, err := config.LoadModels(writeModels(t, testYAML)) - require.NoError(t, err) - assert.Equal(t, "ollama/devstral-tuned", m.ModelFor("review", "")) -} - -func TestModelsModelForDefaultFallback(t *testing.T) { - m, err := config.LoadModels(writeModels(t, testYAML)) - require.NoError(t, err) - assert.Equal(t, "ollama/qwen3-coder-30b-tuned", m.ModelFor("trainer", "")) -} - -func TestModelsModelForCallerOverride(t *testing.T) { - m, err := config.LoadModels(writeModels(t, testYAML)) - require.NoError(t, err) - assert.Equal(t, "claude-opus-4-6", m.ModelFor("review", "claude-opus-4-6")) -} -- 2.54.0