refactor(ai): provider interface + registry, remove unreachable session-failed state - #381
Open
dobrinyonkov wants to merge 3 commits into
Open
refactor(ai): provider interface + registry, remove unreachable session-failed state#381dobrinyonkov wants to merge 3 commits into
dobrinyonkov wants to merge 3 commits into
Conversation
… interface
Introduces the Provider abstraction (three required methods: checkAvailability,
sendMessage, destroy; two optional: downloadModel, getUsageInfo) and swaps
PromptClient out for GeminiNanoProvider — a single file that speaks the
background port protocol internally.
- PromptBuilder gains buildMessages({appInfo, history, userMessage,
inspectionContext, consoleErrors}) returning the full [system, ...history,
wrappedUser] array on every send. buildSeedMessages is removed.
- GeminiNanoProvider caches the messages the underlying Prompt API session was
seeded with. sendMessage compares messages.slice(0, -1) to the cache; if they
differ (URL change, clearConversation, port disconnect from an idle-killed
service worker), it recreates the session before streaming.
- AssistantController takes a provider instead of a promptClient. Loses the
session-lifecycle vocabulary: no _pendingReseed, no _trackReseed, no explicit
createSession calls, no hasActiveSession checks. sendUserMessage builds the
messages array once and hands it to provider.sendMessage(messages, {onChunk}).
- PromptClient.js and PromptClient.spec.js are deleted. Coverage moves to
GeminiNanoProvider.spec.js (same fake-port pattern, plus prefix-cache and
cancellation scenarios).
- AssistantController.spec.js is rewritten around a fake Provider.
First slice of the multi-provider-support work. Registry and OpenAIProvider
land in follow-up slices.
Refs: .scratch/multi-provider-support/issues/01
AssistantController now constructs its provider through a static registry
instead of taking one directly. The registry lives at modules/ai/providers/
index.js and holds a single entry so far — 'gemini-nano' — with the shape
{displayName, ProviderClass, configSchema} the settings modal will render
against in a later slice.
- providers/index.js exports the PROVIDERS map and a createProvider(name,
config) factory. Unknown names throw.
- AssistantController takes {providerName, providerConfig, createProvider}
instead of {provider}. createProvider defaults to the registry factory and
doubles as a test seam so specs can swap in a fake without registering it.
- AIChat passes 'gemini-nano' and an empty config through to the controller.
- AssistantController.spec.js uses the createProvider seam to inject its
fake Provider. No new registry-standalone test — it's a trivial static
object, covered transitively by the controller specs.
No user-visible change. Validated end-to-end against the mock harness
(port init → send → clear) with all existing behaviours intact.
Refs: .scratch/multi-provider-support/issues/02
Moving session recovery into the provider (previous commit) left the controller with no code path that emits `session-failed`. The provider rebuilds its session when its cached prefix diverges, so a killed background worker recovers transparently on the next send. Any failure that does surface now reports as `streaming-failed`. This deletes the now-dead handling the state left behind: - AIChat capability-config entry and its clear-button comment - the .status-session-failed LESS rule (and recompiled theme CSS) - the two AIChat.spec tests and the stale canonical-states entry No reachable behavior changes — the removed state could not fire.
dobrinyonkov
changed the title
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
1 participant
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.
What this does
Refactors the AI assistant to sit behind a small Provider interface. This is the foundation for multi-provider support (the OpenAI provider follows in a stacked PR).
Three commits:
PromptClient→GeminiNanoProviderbehind a Provider interface. The class implements an explicit contract:checkAvailability,sendMessage,destroy, plus optionaldownloadModelandgetUsageInfo.PROVIDERSmap and acreateProvider(name, config)factory. Gemini Nano is the only provider registered here.session-failedcapability state (see below).One deliberate behavior change: session recovery moves into the provider
This is not a pure no-op, and I want to be clear about the one thing that changed.
On master, the controller owned session recovery. It tracked
_pendingReseedand checkedhasActiveSession()before every send, so a Send after Chrome killed the idle background worker would reseed first. A failed reseed surfaced as a distinctsession-failedstate.After this refactor, the provider owns recovery.
GeminiNanoProvidercaches its seed prefix and rebuilds the session whenever the prefix diverges. When the worker dies, the port disconnect nulls the cached prefix, so the nextsendMessagerecreates the session and answers transparently. Same observable recovery, one mechanism, one place.Because nothing calls the old reseed path anymore, no code emits
session-failed. The state became unreachable. Commit 3 deletes the dead handling it left behind:AIChatcapability-config entry and its comment.status-session-failedLESS rule and the recompiled theme CSSAIChat.spectests for the state and a stale entry in the canonical-states listAny failure that does surface now reports as
streaming-failed.Known limitations (pre-existing, not introduced here)
_pendingReseedprovided (a Send racingclearConversationorsetUrl) are gone. In practice a real user cannot hit the microtask window, since a confirm dialog and page navigation separate the actions. The controller-level tests for these races were removed with_pendingReseed. Worth a follow-up test at the provider level.destroy()clearing handlers beforeport.disconnect()can leave an in-flight_createSessionpromise unsettled. This exists identically on master. Not a regression.Testing
npx grunt karma:CI— 581 tests pass.eslintandjshintclean.Review path
Read
GeminiNanoProvider.jsandproviders/index.js. Everything else is the rename propagating through call sites and tests, plus thesession-faileddeletion.