fix: resolve AI feedback provider per-request instead of once at boot - #52
Open
Darth-Ginger wants to merge 2 commits into
Open
Conversation
Live in-session scoring feedback (services/ai/index.ts) picked its provider once, at process import, from process.env.AI_PROVIDER, and never revisited that choice. Meanwhile scenario/inject/debrief generation already read the provider from getAISettings() (DB-backed, admin-panel configurable) on every call. The result: switching providers in the admin panel had no effect on live feedback until the backend process was restarted — and even a correct env var at boot wouldn't select Claude, since the check compared against the literal string 'claude' while the DB enum (and admin panel) use 'anthropic'. generateFeedback() now resolves the active provider via getAISettings() on every call, matching the pattern already used in adaptive-inject.ts. This also fixes the 'claude'/'anthropic' string mismatch. initAIProvider() is kept as a deprecated no-op so it's not a breaking change for callers. Verified against a live deployment: with the old code, a decision submitted after the DB provider was changed to 'ollama' (with the process still up from a boot where AI_PROVIDER=scripted) returned scripted feedback with no attempt to reach Ollama. With this fix, the same request logs a genuine "Primary AI provider unavailable, falling back to scripted" warning when Ollama is down, and returns real Ollama-generated feedback (verified with a local mock Ollama endpoint) when it's reachable — both without restarting the process.
resolveProvider() handled anthropic and ollama but never openai, even though ai-config.ts already stores and connection-tests OpenAI-compatible settings (baseUrl, apiKey, model). Activating openai in the admin panel silently fell back to scripted feedback with no error surfaced. Adds OpenAIProvider (same shape as ClaudeProvider/OllamaProvider) using the OpenAI-compatible /v1/chat/completions endpoint, so it also works against compatible proxies (e.g. Gemini's OpenAI-compat layer), not just api.openai.com.
Author
|
Added a follow-up commit (8aa6aa4): wires up While validating this fix live, found that activating Verified live against a self-hosted deployment with the provider pointed at Gemini's OpenAI-compatible endpoint — request succeeds end-to-end with a valid model.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #51
Problem
services/ai/index.tspicked the live-feedback AI provider once, at processimport, from
process.env.AI_PROVIDER, and never revisited that choice.Meanwhile scenario/inject/debrief generation already read the provider from
getAISettings()(DB-backed, admin-panel configurable) on every call. Soswitching providers in the admin panel had no effect on live in-session
scoring feedback until the process was restarted — and even a correct env
var at boot couldn't select Claude, since the boot-time check compared
against the literal string
'claude', while the DB enum (and admin panel)use
'anthropic'.See #51 for full root-cause details and reproduction steps.
Fix
generateFeedback()now resolves the active provider viagetAISettings()on every call, matching the pattern already used in
services/ai/adaptive-inject.ts. This also fixes the'claude'/'anthropic'string mismatch.initAIProvider()is kept exported as adeprecated no-op so this isn't a breaking change for any external callers.
Diff is confined to
backend/src/services/ai/index.ts— no changes toai-config.tsorsocket/index.ts, both of which were already correct.Testing
npx tsc --noEmit— clean.npx vitest run src/services/ai/claude.test.ts— passing (unaffected,included as a sanity check since it exercises a class this file
instantiates).
read-through):
ollamaandOllama unreachable, a real decision submission over
socket.ionow logs"Primary AI provider unavailable, falling back to scripted"andcorrectly falls back — proving the provider is now resolved from the DB
per-request (previously this path was never reached at all, since
primaryProviderwas frozennullfrom ascriptedboot).ollama.baseUrlat a local mockOllama endpoint (
/api/tags+/api/generate), submitted another livedecision, and got the mock's AI-generated-style text back as the
decision's feedback (
Decision.aiProviderrecorded asollama:llama3),confirming the full round trip through
OllamaProviderworks via thisresolution path — all without restarting the backend process.