refactor(ai): Inspector AI Assistant Architecture V1 (draft)#357
Draft
dobrinyonkov wants to merge 13 commits into
Draft
refactor(ai): Inspector AI Assistant Architecture V1 (draft)#357dobrinyonkov wants to merge 13 commits into
dobrinyonkov wants to merge 13 commits into
Conversation
…tate Chrome Prompt API rejects 'system' role messages that aren't first in the session. Previously every promptStreaming call sent a fresh [system, ...history, user] array, which broke on the second turn since the session already had internal history. Switch to the API's intended model: - createSession accepts initialPrompts; seed with system prompt + any prior user/assistant turns. - promptStreaming sends only the new user message; session retains history internally. - _loadHistory re-seeds the session after loading stored messages so the model has context on chat reopen. Fixes 'system role message must be the first message of a session' errors after Reset history and on multi-turn conversations.
- _loadHistory: clear _messages before repopulating so re-entry doesn't duplicate every turn (every tab switch was bloating the array). - _loadHistory: re-seed the session even when no stored history exists, so switching to a fresh app context refreshes the system prompt (framework version, theme, libraries) instead of keeping the old one. - _loadHistory: guard against re-entry while streaming or already re-seeding to avoid racing concurrent createSession calls. - _initializeSession: skip empty-content turns so the mid-stream assistant placeholder doesn't get replayed as a blank turn. - handleCreateSession (background): create the new LanguageModel session first and only destroy the old one on success — keeps the previous session usable if init throws.
Introduce a PromptBuilder module that owns the system prompt body, application metadata formatting, selected-control (Inspection Context) formatting, truncation rules, and session seed message construction. PromptBuilder is deterministic and free of Chrome APIs. AISessionManager keeps its public surface unchanged and delegates buildSystemPrompt and user-prompt formatting (inside promptStreaming) to a PromptBuilder instance. The Chrome extension port protocol with the background service worker is unchanged. Prompt-formatting tests migrated from AISessionManager.spec.js to a new PromptBuilder.spec.js. AISessionManager.spec.js retains only transport/session assertions.
…undary
Carve the local AI transport (chrome.runtime.connect({ name: 'prompt-api' }),
port message types, streaming async iterator, capability/usage/disconnect
handling) into a new PromptClient module. PromptClient consumes already-built
prompts and seed messages from PromptBuilder and does not perform prompt
construction itself.
Reduce AISessionManager to a thin facade composing PromptBuilder + PromptClient
so AIChat continues to work with no observable behavior change. The Chrome
extension port contract with the background service worker is unchanged.
Migrate AISessionManager.spec.js from internal-field assertions onto
external-behavior delegation tests. Add transport-level PromptClient.spec.js
covering capability state, download progress, session creation, streaming
chunks, usage info, mid-stream error, and mid-stream disconnect, all driven
through a deterministic fake port.
Issue: .scratch/ai-assistant-architecture-v1/issues/02-reshape-prompt-client.md
Captures the domain model and refactor plan for Inspector AI Assistant V1: - CONTEXT.md: canonical glossary (Inspector AI Assistant, Prompt Builder, Prompt Client, Conversation Store, Assistant Controller, etc.) - docs/adr/0001: incremental refactor decision and target boundaries - docs/agents/: issue tracker, triage labels, domain doc conventions - .scratch/ai-assistant-architecture-v1/: PRD and 6 tracer-bullet issues - prompt.md: TDD-flavored runner prompt for autonomous slices No production code changes.
Wrap the existing chat storage logic in a named ConversationStore interface that owns Conversation Memory: load, append, clear, retention limit (50), and storage-key shape per inspected URL. ChatStorageManager becomes a thin delegating shim so AIChat behavior is unchanged. No other module accesses chrome.storage directly for chat history after this slice. Migrates the legacy ChatStorageManager spec onto ConversationStore's public surface; adds tests pinning the retention limit and the role+content-only persistence rule (Inspection Context must not leak into Conversation Memory). Refs: .scratch/ai-assistant-architecture-v1/issues/03-introduce-conversation-store.md
Issue 02 (Reshape session manager into Prompt Client interface) and issue 03 (Introduce Conversation Store boundary) are both shipped: PromptClient owns the prompt-api port transport, ConversationStore owns chrome.storage-backed Conversation Memory, and AISessionManager / ChatStorageManager remain only as thin delegating shims for AIChat. [INTERNAL]
… view Carve out the Assistant Controller boundary so the Inspector AI Assistant workflow lives in a single named place. AssistantController depends only on PromptBuilder, PromptClient, and ConversationStore and owns: - Assistant Capability State resolution (ready, downloadable, downloading, unsupported, unavailable, streaming-failed, unknown — definitive vocabulary finalized in slice 05) - Conversation Memory loading per inspected URL - session creation and reseed with system prompt + replayed turns - per-turn Inspection Context injection (never persisted) - streaming send, save of both turns, streaming-failure recovery - clear-and-reseed and reseed-on-URL-change AIChat is now a thin view over the controller: rendering, markdown, JSON / code viewers, dialogs, scroll behavior, and clipboard helpers stay; session lifecycle, streaming orchestration, history persistence, and context injection are gone from the view and replaced by controller calls plus an event-listener wiring (capability-state-changed, conversation-loaded, stream-chunk, stream-complete, stream-failed, conversation-cleared). A new AssistantController.spec exercises the full V1 Agent Validation Loop with deterministic fakes for PromptClient and ConversationStore and the real PromptBuilder, with no Chrome runtime dependency. Existing DOM-based AIChat tests continue to pass unchanged. [INTERNAL]
… semantics Address standards and spec review findings on the slice 04 commit. Spec fixes (slice 04 / PRD): - sendUserMessage no longer appends the user turn to ConversationStore before the stream completes. On streaming failure, neither the user nor the assistant turn is persisted, so reseed never replays an orphan user message. Matches slice 04 "saving completed user/assistant turns". - session creation failures during initialize, clearConversation, setUrl, and downloadModel reseed now resolve the Assistant Capability State to 'session-failed' instead of throwing. session-failed is one of the canonical first-class states listed in the PRD; it is no longer just an unhandled rejection. - streaming-failed now recovers: a subsequent successful sendUserMessage resurfaces a 'ready' capability state so the developer does not stay stuck on a failure banner (PRD user story #8). - AIChat capability-state routing is explicit per state; 'unknown' and 'streaming-failed' no longer fall through to the unavailable banner, and 'session-failed' surfaces clearly. Refactored the dispatch into a small prototype lookup map to keep cyclomatic complexity within JSHint limits and to make slice 05 additions trivial. Standards / readability: - AssistantController#on JSDoc now justifies the local in-process event bus and explains why this is not a chrome.runtime message dispatch. - AIChat conversation-cleared handler has a comment confirming the innerHTML literal is static, with user/assistant content always routed through _escapeHtml or _parseMarkdown via _addMessage. - Hoisted duplicated test helpers (initializedReady, awaitStreamController) to module scope with JSDoc. 416 -> 421 tests, 5 new tests covering the spec fixes above. grunt lint and grunt test green. [INTERNAL]
…patch Address follow-up review findings on the slice 04 hardening commit. Spec fixes: - Seed Assistant Capability State as the canonical PRD value 'unavailable' (with a 'Checking model status...' message) instead of the ad-hoc string 'unknown'. The PRD enumerates exactly seven first-class capability states and 'unknown' was not one of them; this brings the controller fully in line with the canonical vocabulary before slice 05. - AIChat capability-state dispatch now has an explicit fallback for unmapped Assistant Capability States: it routes to the unavailable banner with the controller's supplied message and logs a console.warn so a missing handler is detectable during development. Previously an unmapped state was silently dropped, which weakened the PRD guarantee that capability states are first-class. Standards / readability: - Extracted each entry of _capabilityStateHandlers into its own documented AIChat.prototype._render*CapabilityState method; the map now holds references rather than anonymous functions, so each renderer has full JSDoc and a stable name in stack traces. - awaitStreamController helper is now bounded by a maxAttempts polling budget (default 500 x 1ms); if production code never calls promptStreaming, the helper rejects with a descriptive error instead of hanging until Mocha's suite-level timeout. 421 -> 422 tests, 1 new test covering the unmapped-state fallback, 1 existing test rewritten to assert canonical PRD vocabulary at construction. grunt lint and grunt test green. [INTERNAL]
Slice 04 (Introduce Assistant Controller and the V1 Agent Validation Loop) is shipped: - AssistantController owns capability resolution, conversation memory lifecycle, session creation and reseed, per-turn Inspection Context injection, streaming, persistence of completed turns, and clear / reseed semantics. It depends only on PromptBuilder, PromptClient, and ConversationStore. - AIChat no longer owns session lifecycle, streaming orchestration, history persistence, or context injection; it interacts only with the controller and listens to its event stream. - The full V1 Agent Validation Loop is exercised by 22 controller-level tests using deterministic fakes for PromptClient and ConversationStore and the real PromptBuilder — no Chrome runtime dependency. - Existing DOM-based AIChat tests still pass; a new view-level test guards against silent-drop of unmapped capability states. Implementing commits: b846529, 5f0530d, 3675be1. [INTERNAL]
Incorporates master's ec1befd (PR #350: fix(ai): seed system+history via initialPrompts; let session manage state). Master's hunks targeted code that has been refactored into PromptBuilder, PromptClient, and AssistantController on this branch; the conflicts were resolved by keeping the post-refactor shape and verifying that both behavior improvements from PR #350 are preserved in their new homes: - background/main.js handleCreateSession still creates the new LanguageModel session before destroying the prior one, so a failure during init leaves the previous session usable. - PromptBuilder.buildSeedMessages still skips empty assistant placeholders when replaying Conversation Memory, so an interrupted mid-stream slot never reseeds as a blank turn. Adds two tests exercising these behaviors at the right boundaries: - AssistantController.spec.js: empty assistant placeholders from Conversation Memory are not replayed into the session seed. - PromptClient.spec.js: a failed second createSession leaves hasActiveSession() === true so the prior background session can remain usable. See .scratch/ai-assistant-architecture-v1/issues/08-merge-master-preserve-behavior.md.
- Remove dead _isReseedingSession field from AIChat that landed via the auto-merge from master. The view no longer owns reseed coordination; the controller does. The field had no reader, no other writer, and no test exercising it. - Shorten the new Assistant Controller test title to match the local imperative-short convention used elsewhere in the file. - Tighten the new Prompt Client create-before-destroy companion test: use async/await sequencing instead of nested .then chains so the test is no longer coupled to the synchronous registration order inside the Promise executor. Known gap (documented as follow-up, out of scope for the merge issue): the actual create-before-destroy ordering invariant lives in background/main.js handleCreateSession, which sits inside the service worker IIFE and is not unit-testable without restructuring (forbidden by issue 08). The added Prompt Client test still covers the client-side companion invariant — that PromptClient does not optimistically clear hasActiveSession on a failed reseed — which is the strongest assertion available from the testable boundary.
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.
Summary
This draft PR introduces the Inspector AI Assistant — Architecture V1 refactor. The goal is to make the AI assistant tab understandable, testable, and safer to change for both developers and AI coding agents.
It contains two clusters of commits:
AI history & session fixes (preceded this work, included on the branch):
1c63b69fix(ai): seed system+history viainitialPrompts; let session manage state8c03d84fix(ai): address PR review comments on session reseed and replayArchitecture V1 — first two structural slices + docs:
d190430refactor(ai): extractPromptBuilderbehindAISessionManagerdelegationa747830refactor(ai): reshapeAISessionManagerintoPromptClienttransport boundary2b59a28docs(ai): add architecture V1 PRD, ADR, glossary and runner promptWhy
The Inspector AI Assistant currently mixes UI rendering, assistant workflow, conversation history, prompt building, Prompt API transport, and streaming behavior. This makes the feature hard to validate without a real Chrome environment with the local model downloaded — both humans and AI coding agents struggle to verify changes.
V1 introduces named boundaries from
CONTEXT.md:prompt-apiportThis PR ships the first two boundaries (
PromptBuilder,PromptClient) plus the planning artifacts. Production behavior of the AI tab is unchanged.What's in this PR
Architecture documentation
CONTEXT.md— canonical glossary (Inspector AI Assistant, Prompt Builder, Prompt Client, Conversation Store, Assistant Controller, Inspection Context, Conversation Memory, Assistant Capability State, Agent Validation Loop, etc.)docs/adr/0001-incremental-ai-assistant-refactor.md— records the incremental refactor decision and target boundariesdocs/agents/— issue tracker, triage labels, domain doc conventions.scratch/ai-assistant-architecture-v1/PRD.md— full PRD with user stories, implementation decisions, testing decisions.scratch/ai-assistant-architecture-v1/issues/— six tracer-bullet slices, ready for autonomous agentsprompt.md— TDD-flavored runner prompt for autonomous slicesCode changes (slices 01 + 02)
PromptBuildermodule owns system prompt content, application metadata formatting, selected-control Inspection Context formatting, truncation rules, and session seed messagesAISessionManagerreshaped into thePromptClientinterface (capability check, model download, session creation, prompt streaming, usage info, session destruction)PromptBuilder; transport stays inPromptClientPromptBuilder.spec.jsAISessionManager.spec.jsprompt-apiport contractAIChatrendering behaviorTesting
grunt test— 396 tests passing (lint + Karma)Why draft
Three more vertical slices remain before the V1 refactor is complete:
Each will land as its own tracer-bullet commit on this branch. The PR will be marked ready for review once the controller seam (slice 04) is in, since that is the slice that delivers the primary agent-validation benefit.
Out of scope (per ADR-0001)
References
.scratch/ai-assistant-architecture-v1/PRD.mddocs/adr/0001-incremental-ai-assistant-refactor.mdCONTEXT.md